Merge pull request #442 from Nikhil-Ladha/dfbugs-1261

DFBUGS-1261: [release-4.18] return correct status codes for Get,Delete RPC calls for VolumeGroup/VolumeGroupSnapshot

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

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()

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

internal/csi-addons/rbd/volumegroup.go

@@ -18,10 +18,12 @@ package rbd
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"slices"
 
 	"github.com/ceph/ceph-csi/internal/rbd"
+	"github.com/ceph/ceph-csi/internal/rbd/group"
 	"github.com/ceph/ceph-csi/internal/rbd/types"
 	"github.com/ceph/ceph-csi/internal/util/log"
 
@@ -192,9 +194,15 @@ func (vs *VolumeGroupServer) DeleteVolumeGroup(
 	// 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 &volumegroup.DeleteVolumeGroupResponse{}, nil
+		}
+
 		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())
 	}
@@ -425,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())
 	}

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)
 	}
 

internal/rbd/group/util.go

@@ -145,6 +145,12 @@ func (cvg *commonVolumeGroup) getVolumeGroupAttributes(ctx context.Context) (*jo
 		attrs = &journal.VolumeGroupAttributes{}
 	}
 
+	if attrs.GroupName == "" {
+		log.ErrorLog(ctx, "volume group with id %v not found", cvg.id)
+
+		return nil, ErrRBDGroupNotFound
+	}
+
 	cvg.requestName = attrs.RequestName
 	cvg.name = attrs.GroupName
 	cvg.creationTime = attrs.CreationTime

internal/rbd/group/volume_group.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 
 	"github.com/ceph/go-ceph/rados"
+	librados "github.com/ceph/go-ceph/rados"
 	librbd "github.com/ceph/go-ceph/rbd"
 	"github.com/container-storage-interface/spec/lib/go/csi"
 	"github.com/csi-addons/spec/lib/go/volumegroup"
@@ -31,7 +32,10 @@ import (
 	"github.com/ceph/ceph-csi/internal/util/log"
 )
 
-var ErrRBDGroupNotConnected = errors.New("RBD group is not connected")
+var (
+	ErrRBDGroupNotConnected = fmt.Errorf("%w: RBD group is not connected", librados.ErrNotConnected)
+	ErrRBDGroupNotFound     = fmt.Errorf("%w: RBD group not found", librbd.ErrNotFound)
+)
 
 // volumeGroup handles all requests for 'rbd group' operations.
 type volumeGroup struct {
@@ -73,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

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)