Skip to content

Commit

Permalink
Add nil check for groupSnapshotContent in deleteCSIGroupSnapshotOpera…
Browse files Browse the repository at this point in the history
…tion and unit tests

- Add a check to ensure `groupSnapshotContent` is not nil in `deleteCSIGroupSnapshotOperation` to prevent panics.
- Enhance error handling for missing `SnapshotHandle` in `VolumeSnapshotContent` objects during deletion.
- Create initial unit tests in `groupsnapshot_helper_test.go` to cover cases where `groupSnapshotContent` is nil or incomplete.

Signed-off-by: Manish <[email protected]>
  • Loading branch information
manishym committed Dec 10, 2024
1 parent 46e8028 commit a1074f5
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 0 deletions.
4 changes: 4 additions & 0 deletions pkg/sidecar-controller/groupsnapshot_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,9 @@ func (ctrl csiSnapshotSideCarController) removeGroupSnapshotContentFinalizer(gro

// Delete a groupsnapshot: Ask the backend to remove the groupsnapshot device
func (ctrl *csiSnapshotSideCarController) deleteCSIGroupSnapshotOperation(groupSnapshotContent *crdv1beta1.VolumeGroupSnapshotContent) error {
if groupSnapshotContent == nil {
return fmt.Errorf("groupSnapshotContent is nil")
}
klog.V(5).Infof("deleteCSIGroupSnapshotOperation [%s] started", groupSnapshotContent.Name)

snapshotterCredentials, err := ctrl.GetCredentialsFromAnnotationForGroupSnapshot(groupSnapshotContent)
Expand All @@ -258,6 +261,7 @@ func (ctrl *csiSnapshotSideCarController) deleteCSIGroupSnapshotOperation(groupS
} else if groupSnapshotContent.Spec.Source.GroupSnapshotHandles != nil {
ids := groupSnapshotContent.Spec.Source.GroupSnapshotHandles.VolumeSnapshotHandles
snapshotIDs = slices.Clone(ids)

}
}

Expand Down
53 changes: 53 additions & 0 deletions pkg/sidecar-controller/groupsnapshot_helper_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package sidecar_controller

import (
"testing"

crdv1beta1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumegroupsnapshot/v1beta1"

v1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/client-go/tools/record"
)

type fakeContentLister struct {
}

func (f *fakeContentLister) List(selector labels.Selector) (ret []*v1.VolumeSnapshotContent, err error) {
return nil, nil
}
func (f *fakeContentLister) Get(name string) (*v1.VolumeSnapshotContent, error) {
return &v1.VolumeSnapshotContent{}, nil
}

func TestDeleteCSIGroupSnapshotOperation(t *testing.T) {
ctrl := &csiSnapshotSideCarController{
contentLister: &fakeContentLister{},
handler: &csiHandler{},
eventRecorder: &record.FakeRecorder{},
}

defer func() {
if r := recover(); r != nil {
t.Errorf("deleteCSIGroupSnapshotOperation() panicked with: %v", r)
}
}()
err := ctrl.deleteCSIGroupSnapshotOperation(nil)
if err == nil {
t.Errorf("expected deleteCSIGroupSnapshotOperation to return error when groupsnapshotContent is nil: %v", err)
}
gsc := crdv1beta1.VolumeGroupSnapshotContent{
Status: &crdv1beta1.VolumeGroupSnapshotContentStatus{
VolumeSnapshotHandlePairList: []crdv1beta1.VolumeSnapshotHandlePair{
{
VolumeHandle: "test-pv",
SnapshotHandle: "test-vsc",
},
},
},
}
err = ctrl.deleteCSIGroupSnapshotOperation(&gsc)
if err == nil {
t.Errorf("expected deleteCSIGroupSnapshotOperation to return error when groupsnapshotContent is empty: %v", err)
}
}

0 comments on commit a1074f5

Please sign in to comment.