diff --git a/pkg/common-controller/framework_test.go b/pkg/common-controller/framework_test.go index 14313f367..8f4974933 100644 --- a/pkg/common-controller/framework_test.go +++ b/pkg/common-controller/framework_test.go @@ -175,7 +175,6 @@ func withSnapshotFinalizers(snapshots []*crdv1.VolumeSnapshot, finalizers ...str func withContentFinalizer(content *crdv1.VolumeSnapshotContent) *crdv1.VolumeSnapshotContent { content.ObjectMeta.Finalizers = append(content.ObjectMeta.Finalizers, utils.VolumeSnapshotContentFinalizer) - metav1.SetMetaDataAnnotation(&content.ObjectMeta, utils.AnnVolumeSnapshotBeingDeleted, "yes") return content } @@ -826,6 +825,18 @@ func newContent(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHa return &content } +func withContentAnnotations(contents []*crdv1.VolumeSnapshotContent, annotations map[string]string) []*crdv1.VolumeSnapshotContent { + for i := range contents { + if contents[i].ObjectMeta.Annotations == nil { + contents[i].ObjectMeta.Annotations = make(map[string]string) + } + for k, v := range annotations { + contents[i].ObjectMeta.Annotations[k] = v + } + } + return contents +} + func newContentArray(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHandle, snapshotClassName, desiredSnapshotHandle, volumeHandle string, deletionPolicy crdv1.DeletionPolicy, size, creationTime *int64, withFinalizer bool) []*crdv1.VolumeSnapshotContent { @@ -1073,6 +1084,14 @@ func testSyncContent(ctrl *csiSnapshotCommonController, reactor *snapshotReactor return ctrl.syncContent(test.initialContents[0]) } +func testSyncContentError(ctrl *csiSnapshotCommonController, reactor *snapshotReactor, test controllerTest) error { + err := ctrl.syncContent(test.initialContents[0]) + if err != nil { + return nil + } + return fmt.Errorf("syncContent succeeded when failure was expected") +} + func testAddPVCFinalizer(ctrl *csiSnapshotCommonController, reactor *snapshotReactor, test controllerTest) error { return ctrl.ensurePVCFinalizer(test.initialSnapshots[0]) } diff --git a/pkg/common-controller/snapshot_delete_test.go b/pkg/common-controller/snapshot_delete_test.go index 250c34a2e..035057ba9 100644 --- a/pkg/common-controller/snapshot_delete_test.go +++ b/pkg/common-controller/snapshot_delete_test.go @@ -18,6 +18,8 @@ package common_controller import ( //"errors" + + "errors" "testing" crdv1 "github.com/kubernetes-csi/external-snapshotter/v2/pkg/apis/volumesnapshot/v1beta1" @@ -40,13 +42,13 @@ var class3Parameters = map[string]string{ } var class4Parameters = map[string]string{ - //utils.SnapshotterSecretNameKey: "emptysecret", - //utils.SnapshotterSecretNamespaceKey: "default", +//utils.SnapshotterSecretNameKey: "emptysecret", +//utils.SnapshotterSecretNamespaceKey: "default", } var class5Parameters = map[string]string{ - //utils.SnapshotterSecretNameKey: "secret", - //utils.SnapshotterSecretNamespaceKey: "default", +//utils.SnapshotterSecretNameKey: "secret", +//utils.SnapshotterSecretNamespaceKey: "default", } var timeNowMetav1 = metav1.Now() @@ -174,22 +176,7 @@ func TestDeleteSync(t *testing.T) { initialSecrets: []*v1.Secret{secret()}, //expectedDeleteCalls: []deleteCall{{"sid1-3", map[string]string{"foo": "bar"}, nil}}, test: testSyncContent, - }, /*{ - name: "1-6 - api server delete content returns error", - initialContents: newContentArray("content1-6", "sid1-6", "snap1-6", "sid1-6", validSecretClass, "", "", deletionPolicy, nil, nil, true), - expectedContents: newContentArray("content1-6", "sid1-6", "snap1-6", "sid1-6", validSecretClass, "", "", deletionPolicy, nil, nil, true), - initialSnapshots: nosnapshots, - expectedSnapshots: nosnapshots, - initialSecrets: []*v1.Secret{secret()}, - //expectedDeleteCalls: []deleteCall{{"sid1-6", map[string]string{"foo": "bar"}, nil}}, - expectedEvents: []string{"Warning SnapshotContentObjectDeleteError"}, - errors: []reactorError{ - // Inject error to the first client.VolumesnapshotV1beta1().VolumeSnapshotContents().Delete call. - // All other calls will succeed. - {"delete", "volumesnapshotcontents", errors.New("mock delete error")}, - }, - test: testSyncContent, - },*/ + }, { // delete success - snapshot that the content was pointing to was deleted, and another // with the same name created. @@ -312,6 +299,23 @@ func TestDeleteSync(t *testing.T) { errors: noerrors, test: testSyncSnapshot, }, + { + name: "3-2 - content will not be deleted if deletion API call fails", + initialContents: newContentArray("content3-2", "", "snap3-2", "sid3-2", validSecretClass, "", "", deletePolicy, nil, nil, true), + expectedContents: newContentArray("content3-2", "", "snap3-2", "sid3-2", validSecretClass, "", "", deletePolicy, nil, nil, true), + initialSnapshots: newSnapshotArray("snap3-2", "snapuid3-2", "claim3-2", "", validSecretClass, "content3-2", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap3-2", "snapuid3-2", "claim3-2", "", validSecretClass, "content3-2", &False, nil, nil, nil, false, true, &timeNowMetav1), + initialClaims: newClaimArray("claim3-2", "pvc-uid3-2", "1Gi", "volume3-2", v1.ClaimBound, &classEmpty), + expectedEvents: []string{"Warning SnapshotContentObjectDeleteError"}, + initialSecrets: []*v1.Secret{secret()}, + errors: []reactorError{ + // Inject error to the first client.VolumesnapshotV1beta1().VolumeSnapshotContents().Delete call. + // All other calls will succeed. + {"delete", "volumesnapshotcontents", errors.New("mock delete error")}, + }, + expectSuccess: false, + test: testSyncSnapshotError, + }, } runSyncTests(t, tests, snapshotClasses) } diff --git a/pkg/common-controller/snapshot_update_test.go b/pkg/common-controller/snapshot_update_test.go index cbb1b5c16..b410799a9 100644 --- a/pkg/common-controller/snapshot_update_test.go +++ b/pkg/common-controller/snapshot_update_test.go @@ -18,9 +18,11 @@ package common_controller import ( //"errors" + "errors" "testing" "time" + "github.com/kubernetes-csi/external-snapshotter/v2/pkg/utils" v1 "k8s.io/api/core/v1" storagev1beta1 "k8s.io/api/storage/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -282,6 +284,59 @@ func TestSync(t *testing.T) { errors: noerrors, test: testSyncSnapshot, }, + { + name: "5-1 - content missing finalizer is updated to have finalizer", + initialContents: newContentArray("content5-1", "snapuid5-1", "snap5-1", "sid5-1", validSecretClass, "", "", deletionPolicy, nil, nil, false), + expectedContents: newContentArray("content5-1", "snapuid5-1", "snap5-1", "sid5-1", validSecretClass, "", "", deletionPolicy, nil, nil, true), + initialClaims: newClaimArray("claim5-1", "pvc-uid5-1", "1Gi", "volume5-1", v1.ClaimBound, &classEmpty), + initialVolumes: newVolumeArray("volume5-1", "pv-uid5-1", "pv-handle5-1", "1Gi", "pvc-uid5-1", "claim5-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + initialSecrets: []*v1.Secret{secret()}, + errors: noerrors, + test: testSyncContent, + }, + { + name: "5-2 - content missing finalizer update attempt fails because of failed API call", + initialContents: newContentArray("content5-2", "snapuid5-2", "snap5-2", "sid5-2", validSecretClass, "", "", deletionPolicy, nil, nil, false), + expectedContents: newContentArray("content5-2", "snapuid5-2", "snap5-2", "sid5-2", validSecretClass, "", "", deletionPolicy, nil, nil, false), + initialClaims: newClaimArray("claim5-2", "pvc-uid5-2", "1Gi", "volume5-2", v1.ClaimBound, &classEmpty), + initialVolumes: newVolumeArray("volume5-2", "pv-uid5-2", "pv-handle5-2", "1Gi", "pvc-uid5-2", "claim5-2", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + initialSecrets: []*v1.Secret{secret()}, + errors: []reactorError{ + // Inject error to the forth client.VolumesnapshotV1beta1().VolumeSnapshots().Update call. + {"update", "volumesnapshotcontents", errors.New("mock update error")}, + }, + expectSuccess: false, + test: testSyncContentError, + }, + { + name: "5-3 - snapshot deletion candidate marked for deletion", + initialSnapshots: newSnapshotArray("snap5-3", "snapuid5-3", "claim5-3", "", validSecretClass, "content5-3", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap5-3", "snapuid5-3", "claim5-3", "", validSecretClass, "content5-3", &False, nil, nil, nil, false, true, &timeNowMetav1), + initialContents: newContentArray("content5-3", "snapuid5-3", "snap5-3", "sid5-3", validSecretClass, "", "", deletionPolicy, nil, nil, true), + expectedContents: withContentAnnotations(newContentArray("content5-3", "snapuid5-3", "snap5-3", "sid5-3", validSecretClass, "", "", deletionPolicy, nil, nil, true), map[string]string{utils.AnnVolumeSnapshotBeingDeleted: "yes"}), + initialClaims: newClaimArray("claim5-3", "pvc-uid5-3", "1Gi", "volume5-3", v1.ClaimBound, &classEmpty), + initialVolumes: newVolumeArray("volume5-3", "pv-uid5-3", "pv-handle5-3", "1Gi", "pvc-uid5-3", "claim5-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + initialSecrets: []*v1.Secret{secret()}, + expectSuccess: true, + test: testSyncContent, + }, + { + name: "5-4 - snapshot deletion candidate fail to mark for deletion due to failed API call", + initialSnapshots: newSnapshotArray("snap5-4", "snapuid5-4", "claim5-4", "", validSecretClass, "content5-4", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap5-4", "snapuid5-4", "claim5-4", "", validSecretClass, "content5-4", &False, nil, nil, nil, false, true, &timeNowMetav1), + initialContents: newContentArray("content5-4", "snapuid5-4", "snap5-4", "sid5-4", validSecretClass, "", "", deletionPolicy, nil, nil, true), + // result of the test framework - annotation is still set in memory, but update call fails. + expectedContents: withContentAnnotations(newContentArray("content5-4", "snapuid5-4", "snap5-4", "sid5-4", validSecretClass, "", "", deletionPolicy, nil, nil, true), map[string]string{utils.AnnVolumeSnapshotBeingDeleted: "yes"}), + initialClaims: newClaimArray("claim5-4", "pvc-uid5-4", "1Gi", "volume5-4", v1.ClaimBound, &classEmpty), + initialVolumes: newVolumeArray("volume5-4", "pv-uid5-4", "pv-handle5-4", "1Gi", "pvc-uid5-4", "claim5-4", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + initialSecrets: []*v1.Secret{secret()}, + errors: []reactorError{ + // Inject error to the forth client.VolumesnapshotV1beta1().VolumeSnapshots().Update call. + {"update", "volumesnapshotcontents", errors.New("mock update error")}, + }, + expectSuccess: false, + test: testSyncContentError, + }, } runSyncTests(t, tests, snapshotClasses)