From 8f25edc888092848718a96e16d29abeaeadc87c6 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Fri, 28 Oct 2022 09:47:33 +0200 Subject: [PATCH] rbd: return error if last sync time not present 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 --- internal/rbd/errors.go | 3 +++ internal/rbd/replicationcontrollerserver.go | 12 +++++++++--- internal/rbd/replicationcontrollerserver_test.go | 6 +++--- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/internal/rbd/errors.go b/internal/rbd/errors.go index 2443b8797..afcc7a839 100644 --- a/internal/rbd/errors.go +++ b/internal/rbd/errors.go @@ -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") ) diff --git a/internal/rbd/replicationcontrollerserver.go b/internal/rbd/replicationcontrollerserver.go index 1ea1796e9..7c63faa72 100644 --- a/internal/rbd/replicationcontrollerserver.go +++ b/internal/rbd/replicationcontrollerserver.go @@ -767,6 +767,11 @@ 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) + } + log.ErrorLog(ctx, err.Error()) + return nil, status.Errorf(codes.Internal, "failed to get last sync time: %v", err) } @@ -804,13 +809,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("empty description: %w", ErrLastSyncTimeNotFound) } splittedString := strings.SplitN(description, ",", 2) if len(splittedString) == 1 { - return nil, nil + return nil, fmt.Errorf("no local snapshot timestamp: %w", ErrLastSyncTimeNotFound) } type localStatus struct { LocalSnapshotTime int64 `json:"local_snapshot_timestamp"` diff --git a/internal/rbd/replicationcontrollerserver_test.go b/internal/rbd/replicationcontrollerserver_test.go index 7ea849cd1..fa7bae9e5 100644 --- a/internal/rbd/replicationcontrollerserver_test.go +++ b/internal/rbd/replicationcontrollerserver_test.go @@ -455,7 +455,7 @@ func TestValidateLastSyncTime(t *testing.T) { "empty description", "", nil, - "", + ErrLastSyncTimeNotFound.Error(), }, { "description without local_snapshot_timestamp", @@ -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 {