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 <[email protected]>
ceph · Dec 16, 2024 · 87acd2b · 87acd2b
1 parent 77d8306
commit 87acd2b
Show file tree

Hide file tree

Showing 8 changed files with 62 additions and 13 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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"
@@ -190,10 +192,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)
 
@@ -229,10 +238,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)