From e7f5b8a28f69996002e59be8fa00b86e214c2fef Mon Sep 17 00:00:00 2001 From: Nikhil-Ladha Date: Mon, 16 Dec 2024 10:51:18 +0530 Subject: [PATCH] util: return correct status code for VolumeGroupSnapshot Fix status codes that are returned for Get/Delete RPC calls for VolumeGroup/VolumeGroupSnapshot. Signed-off-by: Nikhil-Ladha (cherry picked from commit 18a62ec9de170b180197b75c0a67b5fc4f6b7e19) --- internal/cephfs/errors/errors.go | 3 +++ internal/cephfs/groupcontrollerserver.go | 12 +++++++---- internal/cephfs/store/volumegroup.go | 6 ++++++ internal/csi-addons/rbd/volumegroup.go | 16 +++++++++++--- internal/rbd/group/group_snapshot.go | 7 ++++++ internal/rbd/group/util.go | 2 +- internal/rbd/group/volume_group.go | 2 +- internal/rbd/group_controllerserver.go | 27 ++++++++++++++++++++---- 8 files changed, 62 insertions(+), 13 deletions(-) diff --git a/internal/cephfs/errors/errors.go b/internal/cephfs/errors/errors.go index a354aa57e..10ed1e3e0 100644 --- a/internal/cephfs/errors/errors.go +++ b/internal/cephfs/errors/errors.go @@ -61,6 +61,9 @@ var ( // ErrQuiesceInProgress is returned when quiesce operation is in progress. ErrQuiesceInProgress = coreError.New("quiesce operation is in progress") + + // ErrGroupNotFound is returned when volume group snapshot is not found in the backend. + ErrGroupNotFound = coreError.New("volume group snapshot not found") ) // IsCloneRetryError returns true if the clone error is pending,in-progress diff --git a/internal/cephfs/groupcontrollerserver.go b/internal/cephfs/groupcontrollerserver.go index 0433aa0fb..ebd75253b 100644 --- a/internal/cephfs/groupcontrollerserver.go +++ b/internal/cephfs/groupcontrollerserver.go @@ -721,12 +721,16 @@ func (cs *ControllerServer) DeleteVolumeGroupSnapshot(ctx context.Context, vgo, vgsi, err := store.NewVolumeGroupOptionsFromID(ctx, req.GetGroupSnapshotId(), cr) if err != nil { - log.ErrorLog(ctx, "failed to get volume group options: %v", err) - err = extractDeleteVolumeGroupError(err) - if err != nil { - return nil, status.Error(codes.Internal, err.Error()) + if !errors.Is(err, cerrors.ErrGroupNotFound) { + log.ErrorLog(ctx, "failed to get volume group options: %v", err) + err = extractDeleteVolumeGroupError(err) + if err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } } + log.ErrorLog(ctx, "VolumeGroupSnapshot %q doesn't exists", req.GetGroupSnapshotId()) + return &csi.DeleteVolumeGroupSnapshotResponse{}, nil } vgo.Destroy() diff --git a/internal/cephfs/store/volumegroup.go b/internal/cephfs/store/volumegroup.go index 1140dcf6c..0b64c3500 100644 --- a/internal/cephfs/store/volumegroup.go +++ b/internal/cephfs/store/volumegroup.go @@ -172,6 +172,12 @@ func NewVolumeGroupOptionsFromID( return nil, nil, err } + if groupAttributes.GroupName == "" { + log.ErrorLog(ctx, "volume group snapshot with id %v not found", volumeGroupSnapshotID) + + return nil, nil, cerrors.ErrGroupNotFound + } + vgs.RequestName = groupAttributes.RequestName vgs.FsVolumeGroupSnapshotName = groupAttributes.GroupName vgs.VolumeGroupSnapshotID = volumeGroupSnapshotID diff --git a/internal/csi-addons/rbd/volumegroup.go b/internal/csi-addons/rbd/volumegroup.go index 10748b237..c069118b0 100644 --- a/internal/csi-addons/rbd/volumegroup.go +++ b/internal/csi-addons/rbd/volumegroup.go @@ -195,7 +195,7 @@ func (vs *VolumeGroupServer) DeleteVolumeGroup( vg, err := mgr.GetVolumeGroupByID(ctx, req.GetVolumeGroupId()) if err != nil { if errors.Is(err, group.ErrRBDGroupNotFound) { - log.DebugLog(ctx, "VolumeGroup %q doesn't exists", req.GetVolumeGroupId()) + log.ErrorLog(ctx, "VolumeGroup %q doesn't exists", req.GetVolumeGroupId()) return &volumegroup.DeleteVolumeGroupResponse{}, nil } @@ -433,9 +433,19 @@ func (vs *VolumeGroupServer) ControllerGetVolumeGroup( // resolve the volume group vg, err := mgr.GetVolumeGroupByID(ctx, req.GetVolumeGroupId()) if err != nil { + if errors.Is(err, group.ErrRBDGroupNotFound) { + log.ErrorLog(ctx, "VolumeGroup %q doesn't exists", req.GetVolumeGroupId()) + + return nil, status.Errorf( + codes.NotFound, + "could not find volume group %q: %s", + req.GetVolumeGroupId(), + err.Error()) + } + return nil, status.Errorf( - codes.NotFound, - "could not find volume group %q: %s", + codes.Internal, + "could not fetch volume group %q: %s", req.GetVolumeGroupId(), err.Error()) } diff --git a/internal/rbd/group/group_snapshot.go b/internal/rbd/group/group_snapshot.go index f565a600a..81daaf010 100644 --- a/internal/rbd/group/group_snapshot.go +++ b/internal/rbd/group/group_snapshot.go @@ -18,6 +18,7 @@ package group import ( "context" + "errors" "fmt" "github.com/container-storage-interface/spec/lib/go/csi" @@ -69,6 +70,12 @@ func GetVolumeGroupSnapshot( attrs, err := vgs.getVolumeGroupAttributes(ctx) if err != nil { + if errors.Is(err, ErrRBDGroupNotFound) { + log.ErrorLog(ctx, "%v, returning empty volume group snapshot %q", vgs, err) + + return vgs, err + } + return nil, fmt.Errorf("failed to get volume attributes for id %q: %w", vgs, err) } diff --git a/internal/rbd/group/util.go b/internal/rbd/group/util.go index 455791a02..6c4e56d5d 100644 --- a/internal/rbd/group/util.go +++ b/internal/rbd/group/util.go @@ -145,7 +145,7 @@ func (cvg *commonVolumeGroup) getVolumeGroupAttributes(ctx context.Context) (*jo attrs = &journal.VolumeGroupAttributes{} } - if attrs.GroupName == "" || attrs.CreationTime == nil { + if attrs.GroupName == "" { log.ErrorLog(ctx, "volume group with id %v not found", cvg.id) return nil, ErrRBDGroupNotFound diff --git a/internal/rbd/group/volume_group.go b/internal/rbd/group/volume_group.go index 50a3b24b5..e8414f3dd 100644 --- a/internal/rbd/group/volume_group.go +++ b/internal/rbd/group/volume_group.go @@ -77,7 +77,7 @@ func GetVolumeGroup( attrs, err := vg.getVolumeGroupAttributes(ctx) if err != nil { - if errors.Is(err, librbd.ErrNotFound) { + if errors.Is(err, ErrRBDGroupNotFound) { log.ErrorLog(ctx, "%v, returning empty volume group %q", vg, err) return vg, err diff --git a/internal/rbd/group_controllerserver.go b/internal/rbd/group_controllerserver.go index 8b447b921..a4b313ea7 100644 --- a/internal/rbd/group_controllerserver.go +++ b/internal/rbd/group_controllerserver.go @@ -18,11 +18,13 @@ package rbd import ( "context" + "errors" "github.com/container-storage-interface/spec/lib/go/csi" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "github.com/ceph/ceph-csi/internal/rbd/group" "github.com/ceph/ceph-csi/internal/rbd/types" "github.com/ceph/ceph-csi/internal/util" "github.com/ceph/ceph-csi/internal/util/log" @@ -213,10 +215,17 @@ func (cs *ControllerServer) DeleteVolumeGroupSnapshot( groupSnapshot, err := mgr.GetVolumeGroupSnapshotByID(ctx, groupSnapshotID) if err != nil { + if errors.Is(err, group.ErrRBDGroupNotFound) { + log.ErrorLog(ctx, "VolumeGroupSnapshot %q doesn't exists", groupSnapshotID) + + return &csi.DeleteVolumeGroupSnapshotResponse{}, nil + } + return nil, status.Errorf( codes.Internal, - "failed to get volume group snapshot with id %q: %v", - groupSnapshotID, err) + "could not fetch volume group snapshot with id %q: %s", + groupSnapshotID, + err.Error()) } defer groupSnapshot.Destroy(ctx) @@ -252,10 +261,20 @@ func (cs *ControllerServer) GetVolumeGroupSnapshot( groupSnapshot, err := mgr.GetVolumeGroupSnapshotByID(ctx, groupSnapshotID) if err != nil { + if errors.Is(err, group.ErrRBDGroupNotFound) { + log.ErrorLog(ctx, "VolumeGroupSnapshot %q doesn't exists", groupSnapshotID) + + return nil, status.Errorf( + codes.NotFound, + "failed to get volume group snapshot with id %q: %v", + groupSnapshotID, err) + } + return nil, status.Errorf( codes.Internal, - "failed to get volume group snapshot with id %q: %v", - groupSnapshotID, err) + "could not fetch volume group snapshot with id %q: %s", + groupSnapshotID, + err.Error()) } defer groupSnapshot.Destroy(ctx)