Skip to content

Commit

Permalink
rbd: return error if last sync time not present
Browse files Browse the repository at this point in the history
As per the csiaddon spec last sync time is
required parameter in the GetVolumeReplicationInfo
if we are failed to parse the description, return
not found error message instead of nil
which is empty response

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
  • Loading branch information
Madhu-1 committed Nov 1, 2022
1 parent d08e8ee commit 2d64132
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 6 deletions.
3 changes: 3 additions & 0 deletions internal/rbd/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,7 @@ var (
ErrMissingImageNameInVolID = errors.New("rbd image name information can not be empty in volID")
// ErrDecodeClusterIDFromMonsInVolID is returned when mons hash decoding on migration volID.
ErrDecodeClusterIDFromMonsInVolID = errors.New("failed to get clusterID from monitors hash in volID")
// ErrLastSyncTimeNotFound is returned when last sync time is not found for
// the image.
ErrLastSyncTimeNotFound = errors.New("last sync time not found")
)
10 changes: 7 additions & 3 deletions internal/rbd/replicationcontrollerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,9 @@ func (rs *ReplicationServer) GetVolumeReplicationInfo(ctx context.Context,
description := remoteStatus.Description
lastSyncTime, err := getLastSyncTime(description)
if err != nil {
if errors.Is(err, ErrLastSyncTimeNotFound) {
return nil, status.Errorf(codes.NotFound, "failed to get last sync time: %v", err)
}
return nil, status.Errorf(codes.Internal, "failed to get last sync time: %v", err)
}

Expand Down Expand Up @@ -804,13 +807,14 @@ func getLastSyncTime(description string) (*timestamppb.Timestamp, error) {
// description = "replaying,{"bytes_per_second":0.0,
// "bytes_per_snapshot":149504.0,"local_snapshot_timestamp":1662655501
// ,"remote_snapshot_timestamp":1662655501}"
// In case there is no local snapshot timestamp we can pass the default value
// In case there is no local snapshot timestamp return an error as the
// LastSyncTime is required.
if description == "" {
return nil, nil
return nil, fmt.Errorf("%w empty description", ErrLastSyncTimeNotFound)
}
splittedString := strings.SplitN(description, ",", 2)
if len(splittedString) == 1 {
return nil, nil
return nil, fmt.Errorf("%w no local snapshot timestamp", ErrLastSyncTimeNotFound)
}
type localStatus struct {
LocalSnapshotTime int64 `json:"local_snapshot_timestamp"`
Expand Down
6 changes: 3 additions & 3 deletions internal/rbd/replicationcontrollerserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ func TestValidateLastSyncTime(t *testing.T) {
"empty description",
"",
nil,
"",
ErrLastSyncTimeNotFound.Error(),
},
{
"description without local_snapshot_timestamp",
Expand All @@ -467,13 +467,13 @@ func TestValidateLastSyncTime(t *testing.T) {
"description with invalid JSON",
`replaying,{"bytes_per_second":0.0,"bytes_per_snapshot":149504.0","remote_snapshot_timestamp":1662655501`,
nil,
"failed to unmarshal description",
"failed to unmarshal",
},
{
"description with no JSON",
`replaying`,
nil,
"",
ErrLastSyncTimeNotFound.Error(),
},
}
for _, tt := range tests {
Expand Down

0 comments on commit 2d64132

Please sign in to comment.