Skip to content

Commit

Permalink
util: return correct status code for VolumeGroupSnapshot
Browse files Browse the repository at this point in the history
Fix status codes that are returned for Get/Delete RPC calls
for VolumeGroup/VolumeGroupSnapshot.

Signed-off-by: Nikhil-Ladha <nikhilladha1999@gmail.com>
  • Loading branch information
Nikhil-Ladha authored and mergify[bot] committed Dec 19, 2024
1 parent ca1ccdd commit 18a62ec
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 13 deletions.
3 changes: 3 additions & 0 deletions internal/cephfs/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 8 additions & 4 deletions internal/cephfs/groupcontrollerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
6 changes: 6 additions & 0 deletions internal/cephfs/store/volumegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 13 additions & 3 deletions internal/csi-addons/rbd/volumegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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())
}
Expand Down
7 changes: 7 additions & 0 deletions internal/rbd/group/group_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package group

import (
"context"
"errors"
"fmt"

"github.com/container-storage-interface/spec/lib/go/csi"
Expand Down Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion internal/rbd/group/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion internal/rbd/group/volume_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 23 additions & 4 deletions internal/rbd/group_controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down

0 comments on commit 18a62ec

Please sign in to comment.