diff --git a/pkg/common-controller/framework_test.go b/pkg/common-controller/framework_test.go index 9acc3eea..0eadcd9a 100644 --- a/pkg/common-controller/framework_test.go +++ b/pkg/common-controller/framework_test.go @@ -173,11 +173,6 @@ func withSnapshotFinalizers(snapshots []*crdv1.VolumeSnapshot, finalizers ...str return snapshots } -func withContentFinalizer(content *crdv1.VolumeSnapshotContent) *crdv1.VolumeSnapshotContent { - content.ObjectMeta.Finalizers = append(content.ObjectMeta.Finalizers, utils.VolumeSnapshotContentFinalizer) - return content -} - func withPVCFinalizer(pvc *v1.PersistentVolumeClaim) *v1.PersistentVolumeClaim { pvc.ObjectMeta.Finalizers = append(pvc.ObjectMeta.Finalizers, utils.PVCFinalizer) return pvc @@ -837,6 +832,18 @@ func withContentAnnotations(contents []*crdv1.VolumeSnapshotContent, annotations return contents } +func withContentSpecSnapshotClassName(contents []*crdv1.VolumeSnapshotContent, volumeSnapshotClassName *string) []*crdv1.VolumeSnapshotContent { + for i := range contents { + contents[i].Spec.VolumeSnapshotClassName = volumeSnapshotClassName + } + return contents +} + +func withContentFinalizer(content *crdv1.VolumeSnapshotContent) *crdv1.VolumeSnapshotContent { + content.ObjectMeta.Finalizers = append(content.ObjectMeta.Finalizers, utils.VolumeSnapshotContentFinalizer) + return content +} + func newContentArray(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHandle, snapshotClassName, desiredSnapshotHandle, volumeHandle string, deletionPolicy crdv1.DeletionPolicy, size, creationTime *int64, withFinalizer bool) []*crdv1.VolumeSnapshotContent { diff --git a/pkg/common-controller/snapshot_controller.go b/pkg/common-controller/snapshot_controller.go index c66e2b09..0a4c7fa5 100644 --- a/pkg/common-controller/snapshot_controller.go +++ b/pkg/common-controller/snapshot_controller.go @@ -173,7 +173,11 @@ func (ctrl *csiSnapshotCommonController) syncSnapshot(snapshot *crdv1.VolumeSnap } } else { klog.V(5).Infof("syncSnapshot[%s]: check if we should add finalizers on snapshot", utils.SnapshotKey(snapshot)) - ctrl.checkandAddSnapshotFinalizers(snapshot) + if err := ctrl.checkandAddSnapshotFinalizers(snapshot); err != nil { + klog.Errorf("error check and add Snapshot finalizers for snapshot [%s]: %v", snapshot.Name, errFinalizer) + ctrl.eventRecorder.Event(snapshot, v1.EventTypeWarning, "SnapshotCheckandUpdateFailed", fmt.Sprintf("Failed to check and update snapshot: %s", err.Error())) + return err + } // Need to build or update snapshot.Status in following cases: // 1) snapshot.Status is nil // 2) snapshot.Status.ReadyToUse is false @@ -301,7 +305,7 @@ func (ctrl *csiSnapshotCommonController) checkandAddSnapshotFinalizers(snapshot if addSourceFinalizer || addBoundFinalizer { // Snapshot is not being deleted -> it should have the finalizer. klog.V(5).Infof("checkandAddSnapshotFinalizers: Add Finalizer for VolumeSnapshot[%s]", utils.SnapshotKey(snapshot)) - ctrl.addSnapshotFinalizer(snapshot, addSourceFinalizer, addBoundFinalizer) + return ctrl.addSnapshotFinalizer(snapshot, addSourceFinalizer, addBoundFinalizer) } return nil } @@ -665,26 +669,6 @@ func (ctrl *csiSnapshotCommonController) updateSnapshotErrorStatusWithEvent(snap return nil } -// isSnapshotConentBeingUsed checks if snapshot content is bound to snapshot. -func (ctrl *csiSnapshotCommonController) isSnapshotContentBeingUsed(content *crdv1.VolumeSnapshotContent) bool { - if content.Spec.VolumeSnapshotRef.Name != "" && content.Spec.VolumeSnapshotRef.Namespace != "" { - snapshotObj, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshots(content.Spec.VolumeSnapshotRef.Namespace).Get(content.Spec.VolumeSnapshotRef.Name, metav1.GetOptions{}) - if err != nil { - klog.Infof("isSnapshotContentBeingUsed: Cannot get snapshot %s from api server: [%v]. VolumeSnapshot object may be deleted already.", content.Spec.VolumeSnapshotRef.Name, err) - return false - } - - // Check if the snapshot content is bound to the snapshot - if utils.IsSnapshotBound(snapshotObj, content) { - klog.Infof("isSnapshotContentBeingUsed: VolumeSnapshot %s is bound to volumeSnapshotContent [%s]", snapshotObj.Name, content.Name) - return true - } - } - - klog.V(5).Infof("isSnapshotContentBeingUsed: Snapshot content %s is not being used", content.Name) - return false -} - // addContentFinalizer adds a Finalizer for VolumeSnapshotContent. func (ctrl *csiSnapshotCommonController) addContentFinalizer(content *crdv1.VolumeSnapshotContent) error { contentClone := content.DeepCopy() @@ -858,7 +842,7 @@ func (ctrl *csiSnapshotCommonController) checkandBindSnapshotContent(snapshot *c } newContent, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Update(contentClone) if err != nil { - klog.V(4).Infof("updating VolumeSnapshotContent[%s] error status failed %v", newContent.Name, err) + klog.V(4).Infof("updating VolumeSnapshotContent[%s] error status failed %v", contentClone.Name, err) return nil, err } diff --git a/pkg/common-controller/snapshot_create_test.go b/pkg/common-controller/snapshot_create_test.go index ab16f644..f0a3b0a8 100644 --- a/pkg/common-controller/snapshot_create_test.go +++ b/pkg/common-controller/snapshot_create_test.go @@ -79,13 +79,18 @@ func TestCreateSnapshotSync(t *testing.T) { test: testSyncSnapshot, }, { - name: "6-2 - successful create snapshot with snapshot class silver", - initialContents: nocontents, - expectedContents: newContentArrayNoStatus("snapcontent-snapuid6-2", "snapuid6-2", "snap6-2", "sid6-2", classSilver, "", "pv-handle6-2", deletionPolicy, nil, nil, false, false), - initialSnapshots: newSnapshotArray("snap6-2", "snapuid6-2", "claim6-2", "", classSilver, "", &False, nil, nil, nil, false, true, nil), - expectedSnapshots: newSnapshotArray("snap6-2", "snapuid6-2", "claim6-2", "", classSilver, "snapcontent-snapuid6-2", &False, nil, nil, nil, false, true, nil), + name: "6-2 - successful create snapshot with validSecretClass and initial secret", + initialContents: nocontents, + expectedContents: withContentAnnotations(newContentArrayNoStatus("snapcontent-snapuid6-2", "snapuid6-2", "snap6-2", "sid6-2", validSecretClass, "", "pv-handle6-2", deletionPolicy, nil, nil, false, false), + map[string]string{ + "snapshot.storage.kubernetes.io/deletion-secret-name": "secret", + "snapshot.storage.kubernetes.io/deletion-secret-namespace": "default", + }), + initialSnapshots: newSnapshotArray("snap6-2", "snapuid6-2", "claim6-2", "", validSecretClass, "", &False, nil, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap6-2", "snapuid6-2", "claim6-2", "", validSecretClass, "snapcontent-snapuid6-2", &False, nil, nil, nil, false, true, nil), initialClaims: newClaimArray("claim6-2", "pvc-uid6-2", "1Gi", "volume6-2", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume6-2", "pv-uid6-2", "pv-handle6-2", "1Gi", "pvc-uid6-2", "claim6-2", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + initialSecrets: []*v1.Secret{secret()}, // no initial secret created errors: noerrors, test: testSyncSnapshot, }, @@ -120,12 +125,12 @@ func TestCreateSnapshotSync(t *testing.T) { {"update", "volumesnapshots", errors.New("mock update error")}, }, test: testSyncSnapshot, }, - /*{ + { name: "7-3 - fail to create snapshot without snapshot class ", initialContents: nocontents, expectedContents: nocontents, - initialSnapshots: newSnapshotArray("snap7-3", "snapuid7-3", "claim7-3", "", "", "", &False, nil, nil, nil, false, true), - expectedSnapshots: newSnapshotArray("snap7-3", "snapuid7-3", "claim7-3", "", "", "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-3: \"failed to retrieve snapshot class from the informer: \\\"volumesnapshotclass.snapshot.storage.k8s.io \\\\\\\"\\\\\\\" not found\\\"\""), false, true), + initialSnapshots: newSnapshotArray("snap7-3", "snapuid7-3", "claim7-3", "", "", "", &False, nil, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap7-3", "snapuid7-3", "claim7-3", "", "", "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-3: \"failed to take snapshot snap7-3 without a snapshot class\""), false, true, nil), initialClaims: newClaimArray("claim7-3", "pvc-uid7-3", "1Gi", "volume7-3", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume7-3", "pv-uid7-3", "pv-handle7-3", "1Gi", "pvc-uid7-3", "claim7-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialStorageClasses: []*storage.StorageClass{diffDriverStorageClass}, @@ -133,7 +138,7 @@ func TestCreateSnapshotSync(t *testing.T) { errors: noerrors, expectSuccess: false, test: testSyncSnapshot, - },*/ + }, { name: "7-4 - fail create snapshot with no-existing claim", initialContents: nocontents, @@ -171,83 +176,87 @@ func TestCreateSnapshotSync(t *testing.T) { expectSuccess: false, test: testSyncSnapshot, }, - /*{ - name: "7-8 - fail create snapshot due to cannot update snapshot status", - initialContents: nocontents, - expectedContents: nocontents, - initialSnapshots: newSnapshotArray("snap7-8", "snapuid7-8", "claim7-8", "", classGold, "", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap7-8", "snapuid7-8", "claim7-8", "", classGold, "", &False, nil, nil, newVolumeError("Failed to create snapshot: snapshot controller failed to update default/snap7-8 on API server: mock update error")), - initialClaims: newClaimArray("claim7-8", "pvc-uid7-8", "1Gi", "volume7-8", v1.ClaimBound, &classEmpty), - initialVolumes: newVolumeArray("volume7-8", "pv-uid7-8", "pv-handle7-8", "1Gi", "pvc-uid7-8", "claim7-8", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), - expectedCreateCalls: []createCall{ - { - snapshotName: "snapshot-snapuid7-8", - volume: newVolume("volume7-8", "pv-uid7-8", "pv-handle7-8", "1Gi", "pvc-uid7-8", "claim7-8", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), - parameters: map[string]string{"param1": "value1"}, - // information to return - driverName: mockDriverName, - size: defaultSize, - snapshotId: "sid7-8", - creationTime: timeNow, - readyToUse: True, - }, - }, - /*errors: []reactorError{ - // Inject error to the forth client.VolumesnapshotV1beta1().VolumeSnapshots().Update call. - // All other calls will succeed. - {"update", "volumesnapshots", errors.New("mock update error")}, - {"update", "volumesnapshots", errors.New("mock update error")}, - {"update", "volumesnapshots", errors.New("mock update error")}, - }, - expectedEvents: []string{"Warning SnapshotContentCreationFailed"}, - expectSuccess: false, - test: testSyncSnapshot, - }, - /*{ - // TODO(xiangqian): this test case needs to be revisited the scenario - // of VolumeSnapshotContent saving failure. Since there will be no content object - // in API server, it could potentially cause leaking issue - name: "7-9 - fail create snapshot due to cannot save snapshot content", - initialContents: nocontents, - expectedContents: nocontents, - initialSnapshots: newSnapshotArray("snap7-9", "snapuid7-9", "claim7-9", "", classGold, "", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap7-9", "snapuid7-9", "claim7-9", "", classGold, "snapcontent-snapuid7-9", &True, metaTimeNowUnix, getSize(defaultSize), nil), - initialClaims: newClaimArray("claim7-9", "pvc-uid7-9", "1Gi", "volume7-9", v1.ClaimBound, &classEmpty), - initialVolumes: newVolumeArray("volume7-9", "pv-uid7-9", "pv-handle7-9", "1Gi", "pvc-uid7-9", "claim7-9", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), - expectedCreateCalls: []createCall{ - { - snapshotName: "snapshot-snapuid7-9", - volume: newVolume("volume7-9", "pv-uid7-9", "pv-handle7-9", "1Gi", "pvc-uid7-9", "claim7-9", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), - parameters: map[string]string{"param1": "value1"}, - // information to return - driverName: mockDriverName, - size: defaultSize, - snapshotId: "sid7-9", - creationTime: timeNow, - readyToUse: True, - }, - }, - errors: []reactorError{ - {"create", "volumesnapshotcontents", errors.New("mock create error")}, - {"create", "volumesnapshotcontents", errors.New("mock create error")}, - {"create", "volumesnapshotcontents", errors.New("mock create error")}, - }, - expectedEvents: []string{"Warning CreateSnapshotContentFailed"}, - test: testSyncSnapshot, - }, - { - name: "7-10 - fail create snapshot with secret not found", - initialContents: nocontents, - expectedContents: nocontents, - initialSnapshots: newSnapshotArray("snap7-10", "snapuid7-10", "claim7-10", "", validSecretClass, "", &False, nil, nil, nil, false, true), - expectedSnapshots: newSnapshotArray("snap7-10", "snapuid7-10", "claim7-10", "", validSecretClass, "", &False, nil, nil, newVolumeError("Failed to create snapshot: error getting secret secret in namespace default: cannot find secret secret"), false, true), - initialClaims: newClaimArray("claim7-10", "pvc-uid7-10", "1Gi", "volume7-10", v1.ClaimBound, &classEmpty), - initialVolumes: newVolumeArray("volume7-10", "pv-uid7-10", "pv-handle7-10", "1Gi", "pvc-uid7-10", "claim7-10", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), - initialSecrets: []*v1.Secret{}, // no initial secret created - errors: noerrors, - test: testSyncSnapshot, - },*/ + { + name: "7-7 - remove pvc finalizer failed", + initialContents: newContentArray("snapcontent-snapuid7-7", "snapuid7-7", "snap7-7", "sid7-7", classGold, "", "pv-handle7-7", deletionPolicy, nil, nil, false), + expectedContents: newContentArray("snapcontent-snapuid7-7", "snapuid7-7", "snap7-7", "sid7-7", classGold, "", "pv-handle7-7", deletionPolicy, nil, nil, false), + initialSnapshots: newSnapshotArray("snap7-7", "snapuid7-7", "claim7-7", "", classGold, "snapcontent-snapuid7-7", &True, nil, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap7-7", "snapuid7-7", "claim7-7", "", classGold, "snapcontent-snapuid7-7", &True, nil, nil, nil, false, true, nil), + initialClaims: newClaimArrayFinalizer("claim7-7", "pvc-uid7-7", "1Gi", "volume7-7", v1.ClaimBound, &classEmpty), + initialVolumes: newVolumeArray("volume7-7", "pv-uid7-7", "pv-handle7-7", "1Gi", "pvc-uid7-7", "claim7-7", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + errors: []reactorError{ + {"update", "persistentvolumeclaims", errors.New("mock update error")}, + {"update", "persistentvolumeclaims", errors.New("mock update error")}, + {"update", "persistentvolumeclaims", errors.New("mock update error")}, + }, + expectSuccess: false, + test: testSyncSnapshot, + }, + { + name: "7-8 - fail create snapshot due to cannot update snapshot status", + initialContents: nocontents, + expectedContents: newContentArrayNoStatus("snapcontent-snapuid7-8", "snapuid7-8", "snap7-8", "sid7-8", classGold, "", "pv-handle7-8", deletionPolicy, nil, nil, false, false), + initialSnapshots: newSnapshotArray("snap7-8", "snapuid7-8", "claim7-8", "", classGold, "", &False, nil, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap7-8", "snapuid7-8", "claim7-8", "", classGold, "", &False, nil, nil, newVolumeError("Snapshot status update failed, snapshot controller failed to update default/snap7-8 on API server: mock update error"), false, true, nil), + initialClaims: newClaimArray("claim7-8", "pvc-uid7-8", "1Gi", "volume7-8", v1.ClaimBound, &classEmpty), + initialVolumes: newVolumeArray("volume7-8", "pv-uid7-8", "pv-handle7-8", "1Gi", "pvc-uid7-8", "claim7-8", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + errors: []reactorError{ + {"update", "volumesnapshots", errors.New("mock update error")}, + {"update", "volumesnapshots", errors.New("mock update error")}, + {"update", "volumesnapshots", errors.New("mock update error")}, + }, + expectedEvents: []string{"Warning SnapshotStatusUpdateFailed"}, + expectSuccess: false, + test: testSyncSnapshot, + }, + { + name: "7-9 - fail create snapshot due to cannot update snapshot status, and failure cannot be recorded either due to additional status update failure.", + initialContents: nocontents, + expectedContents: newContentArrayNoStatus("snapcontent-snapuid7-9", "snapuid7-9", "snap7-9", "sid7-9", classGold, "", "pv-handle7-9", deletionPolicy, nil, nil, false, false), + initialSnapshots: newSnapshotArray("snap7-9", "snapuid7-9", "claim7-9", "", classGold, "", &False, nil, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap7-9", "snapuid7-9", "claim7-9", "", classGold, "", &False, nil, nil, nil, false, true, nil), + initialClaims: newClaimArray("claim7-9", "pvc-uid7-9", "1Gi", "volume7-9", v1.ClaimBound, &classEmpty), + initialVolumes: newVolumeArray("volume7-9", "pv-uid7-9", "pv-handle7-9", "1Gi", "pvc-uid7-9", "claim7-9", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + errors: []reactorError{ + {"update", "volumesnapshots", errors.New("mock update error")}, + {"update", "volumesnapshots", errors.New("mock update error")}, + {"update", "volumesnapshots", errors.New("mock update error")}, + {"update", "volumesnapshots", errors.New("mock update error")}, + }, + expectSuccess: false, + test: testSyncSnapshot, + }, + { + name: "7-10 - fail create snapshot with invalid secret", + initialContents: nocontents, + expectedContents: nocontents, + initialSnapshots: newSnapshotArray("snap7-10", "snapuid7-10", "claim7-10", "", invalidSecretClass, "", &False, nil, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap7-10", "snapuid7-10", "claim7-10", "snap7-10", invalidSecretClass, "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-10: \"failed to get name and namespace template from params: either name and namespace for Snapshotter secrets specified, Both must be specified\""), false, true, nil), + initialClaims: newClaimArray("claim7-10", "pvc-uid7-10", "1Gi", "volume7-10", v1.ClaimBound, &classEmpty), + initialVolumes: newVolumeArray("volume7-10", "pv-uid7-10", "pv-handle7-10", "1Gi", "pvc-uid7-10", "claim7-10", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + initialSecrets: []*v1.Secret{}, // no initial secret created + test: testSyncSnapshot, + }, + { + // TODO(xiangqian): this test case needs to be revisited the scenario + // of VolumeSnapshotContent saving failure. Since there will be no content object + // in API server, it could potentially cause leaking issue + name: "7-11 - fail create snapshot due to cannot save snapshot content", + initialContents: nocontents, + expectedContents: nocontents, + initialSnapshots: newSnapshotArray("snap7-11", "snapuid7-11", "claim7-11", "", classGold, "", &False, nil, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap7-11", "snapuid7-11", "claim7-11", "", classGold, "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error snapshot controller failed to update default/snap7-11 on API server: mock create error"), false, true, nil), + initialClaims: newClaimArray("claim7-11", "pvc-uid7-11", "1Gi", "volume7-11", v1.ClaimBound, &classEmpty), + initialVolumes: newVolumeArray("volume7-11", "pv-uid7-11", "pv-handle7-11", "1Gi", "pvc-uid7-11", "claim7-11", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + errors: []reactorError{ + {"create", "volumesnapshotcontents", errors.New("mock create error")}, + {"create", "volumesnapshotcontents", errors.New("mock create error")}, + {"create", "volumesnapshotcontents", errors.New("mock create error")}, + }, + expectedEvents: []string{"Warning CreateSnapshotContentFailed"}, + test: testSyncSnapshot, + }, } runSyncTests(t, tests, snapshotClasses) } diff --git a/pkg/common-controller/snapshot_delete_test.go b/pkg/common-controller/snapshot_delete_test.go index 782dff15..17ce7ffe 100644 --- a/pkg/common-controller/snapshot_delete_test.go +++ b/pkg/common-controller/snapshot_delete_test.go @@ -35,8 +35,8 @@ var class2Parameters = map[string]string{ } var class3Parameters = map[string]string{ - "param3": "value3", - //utils.SnapshotterSecretNameKey: "name", + "param3": "value3", + utils.PrefixedSnapshotterSecretNameKey: "name", } var class4Parameters = map[string]string{ @@ -45,8 +45,8 @@ var class4Parameters = map[string]string{ } var class5Parameters = map[string]string{ - //utils.SnapshotterSecretNameKey: "secret", - //utils.SnapshotterSecretNamespaceKey: "default", + utils.PrefixedSnapshotterSecretNameKey: "secret", + utils.PrefixedSnapshotterSecretNamespaceKey: "default", } var timeNowMetav1 = metav1.Now() @@ -130,158 +130,43 @@ var snapshotClasses = []*crdv1.VolumeSnapshotClass{ func TestDeleteSync(t *testing.T) { tests := []controllerTest{ { - name: "1-1 - content with empty snapshot class is deleted if it is bound to a non-exist snapshot and also has a snapshot uid specified", - initialContents: newContentArray("content1-1", "snapuid1-1", "snap1-1", "sid1-1", classGold, "", "", deletionPolicy, nil, nil, true), - expectedContents: newContentArray("content1-1", "snapuid1-1", "snap1-1", "sid1-1", classGold, "", "", deletionPolicy, nil, nil, true), - initialSnapshots: nosnapshots, - expectedSnapshots: nosnapshots, + name: "1-1 - noop: content will not be deleted if it is bound to a snapshot correctly, snapshot uid is not specified", + initialContents: newContentArray("content1-1", "", "snap1-1", "sid1-1", validSecretClass, "", "", deletePolicy, nil, nil, true), + expectedContents: newContentArray("content1-1", "", "snap1-1", "sid1-1", validSecretClass, "", "", deletePolicy, nil, nil, true), + initialSnapshots: newSnapshotArray("snap1-1", "snapuid1-1", "claim1-1", "", validSecretClass, "content1-1", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap1-1", "snapuid1-1", "claim1-1", "", validSecretClass, "content1-1", &False, nil, nil, nil, false, true, &timeNowMetav1), expectedEvents: noevents, - errors: noerrors, - //expectedDeleteCalls: []deleteCall{{"sid1-1", nil, nil}}, - test: testSyncContent, - }, - { - name: "2-1 - content with empty snapshot class will not be deleted if it is bound to a non-exist snapshot but it does not have a snapshot uid specified", - initialContents: newContentArray("content2-1", "", "snap2-1", "sid2-1", "", "", "", deletionPolicy, nil, nil, true), - expectedContents: newContentArray("content2-1", "", "snap2-1", "sid2-1", "", "", "", deletionPolicy, nil, nil, true), - initialSnapshots: nosnapshots, - expectedSnapshots: nosnapshots, - expectedEvents: noevents, - errors: noerrors, - //expectedDeleteCalls: []deleteCall{{"sid2-1", nil, nil}}, - test: testSyncContent, - }, - { - name: "1-2 - successful delete with snapshot class that has empty secret parameter", - initialContents: newContentArray("content1-2", "sid1-2", "snap1-2", "sid1-2", emptySecretClass, "", "", deletionPolicy, nil, nil, true), - expectedContents: newContentArray("content1-2", "sid1-2", "snap1-2", "sid1-2", emptySecretClass, "", "", deletionPolicy, nil, nil, true), - initialSnapshots: nosnapshots, - expectedSnapshots: nosnapshots, - initialSecrets: []*v1.Secret{emptySecret()}, - expectedEvents: noevents, - errors: noerrors, - //expectedDeleteCalls: []deleteCall{{"sid1-2", map[string]string{}, nil}}, - test: testSyncContent, - }, - { - name: "1-3 - successful delete with snapshot class that has valid secret parameter", - initialContents: newContentArray("content1-3", "sid1-3", "snap1-3", "sid1-3", validSecretClass, "", "", deletionPolicy, nil, nil, true), - expectedContents: newContentArray("content1-3", "sid1-3", "snap1-3", "sid1-3", validSecretClass, "", "", deletionPolicy, nil, nil, true), - initialSnapshots: nosnapshots, - expectedSnapshots: nosnapshots, - expectedEvents: noevents, - errors: noerrors, initialSecrets: []*v1.Secret{secret()}, - //expectedDeleteCalls: []deleteCall{{"sid1-3", map[string]string{"foo": "bar"}, nil}}, - test: testSyncContent, + errors: noerrors, + test: testSyncContent, }, { - // delete success - snapshot that the content was pointing to was deleted, and another - // with the same name created. - name: "1-7 - prebound content is deleted while the snapshot exists", - initialContents: newContentArray("content1-7", "sid1-7", "snap1-7", "sid1-7", emptySecretClass, "", "", deletionPolicy, nil, nil, true), - expectedContents: newContentArray("content1-7", "sid1-7", "snap1-7", "sid1-7", emptySecretClass, "", "", deletionPolicy, nil, nil, true), - initialSnapshots: newSnapshotArray("snap1-7", "snapuid1-7-x", "claim1-7", "", validSecretClass, "", &False, nil, nil, nil, false, true, nil), - expectedSnapshots: newSnapshotArray("snap1-7", "snapuid1-7-x", "claim1-7", "", validSecretClass, "", &False, nil, nil, nil, false, true, nil), - initialSecrets: []*v1.Secret{secret()}, - //expectedDeleteCalls: []deleteCall{{"sid1-7", map[string]string{"foo": "bar"}, nil}}, - expectedEvents: noevents, - errors: noerrors, - test: testSyncContent, - }, - { - // delete success(?) - content is deleted before doDelete() starts - name: "1-8 - content is deleted before deleting", - initialContents: newContentArray("content1-8", "sid1-8", "snap1-8", "sid1-8", validSecretClass, "", "", deletionPolicy, nil, nil, true), + // delete success - content is deleted before doDelete() starts + name: "1-2 - content is deleted before deleting", + initialContents: newContentArray("content1-2", "sid1-2", "snap1-2", "sid1-2", validSecretClass, "", "", deletionPolicy, nil, nil, true), expectedContents: nocontents, initialSnapshots: nosnapshots, expectedSnapshots: nosnapshots, initialSecrets: []*v1.Secret{secret()}, - //expectedDeleteCalls: []deleteCall{{"sid1-8", map[string]string{"foo": "bar"}, nil}}, - expectedEvents: noevents, - errors: noerrors, + expectedEvents: noevents, + errors: noerrors, test: wrapTestWithInjectedOperation(testSyncContent, func(ctrl *csiSnapshotCommonController, reactor *snapshotReactor) { // Delete the volume before delete operation starts reactor.lock.Lock() - delete(reactor.contents, "content1-8") + delete(reactor.contents, "content1-2") reactor.lock.Unlock() }), }, - /*{ - name: "1-9 - content will not be deleted if it is bound to a snapshot correctly, snapshot uid is specified", - initialContents: newContentArray("content1-9", "snapuid1-9", "snap1-9", "sid1-9", validSecretClass, "", "", deletionPolicy, nil, nil, true), - expectedContents: newContentArray("content1-9", "snapuid1-9", "snap1-9", "sid1-9", validSecretClass, "", "", deletionPolicy, nil, nil, true), - initialSnapshots: newSnapshotArray("snap1-9", "snapuid1-9", "claim1-9", "", validSecretClass, "content1-9", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap1-9", "snapuid1-9", "claim1-9", "", validSecretClass, "content1-9", &True, nil, nil, nil), + { + name: "1-3 - will not delete content with retain policy set which is bound to a snapshot incorrectly", + initialContents: newContentArray("content1-3", "snapuid1-3-x", "snap1-3", "sid1-3", validSecretClass, "", "", retainPolicy, nil, nil, true), + expectedContents: newContentArray("content1-3", "snapuid1-3-x", "snap1-3", "sid1-3", validSecretClass, "", "", retainPolicy, nil, nil, true), + initialSnapshots: newSnapshotArray("snap1-3", "snapuid1-3", "claim1-3", "", validSecretClass, "content1-3", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: newSnapshotArray("snap1-3", "snapuid1-3", "claim1-3", "", validSecretClass, "content1-3", &False, nil, nil, nil, false, true, &timeNowMetav1), expectedEvents: noevents, initialSecrets: []*v1.Secret{secret()}, errors: noerrors, test: testSyncContent, - },*/ - { - name: "1-10 - will not delete content with retain policy set which is bound to a snapshot incorrectly", - initialContents: newContentArray("content1-10", "snapuid1-10-x", "snap1-10", "sid1-10", validSecretClass, "", "", retainPolicy, nil, nil, true), - expectedContents: newContentArray("content1-10", "snapuid1-10-x", "snap1-10", "sid1-10", validSecretClass, "", "", retainPolicy, nil, nil, true), - initialSnapshots: newSnapshotArray("snap1-10", "snapuid1-10", "claim1-10", "", validSecretClass, "content1-10", &False, nil, nil, nil, false, true, nil), - expectedSnapshots: newSnapshotArray("snap1-10", "snapuid1-10", "claim1-10", "", validSecretClass, "content1-10", &False, nil, nil, nil, false, true, nil), - expectedEvents: noevents, - initialSecrets: []*v1.Secret{secret()}, - errors: noerrors, - test: testSyncContent, - }, - { - name: "1-11 - content will not be deleted if it is bound to a snapshot correctly, snapsht uid is not specified", - initialContents: newContentArray("content1-11", "", "snap1-11", "sid1-11", validSecretClass, "", "", deletePolicy, nil, nil, true), - expectedContents: newContentArray("content1-11", "", "snap1-11", "sid1-11", validSecretClass, "", "", deletePolicy, nil, nil, true), - initialSnapshots: newSnapshotArray("snap1-11", "snapuid1-11", "claim1-11", "", validSecretClass, "content1-11", &False, nil, nil, nil, false, true, nil), - expectedSnapshots: newSnapshotArray("snap1-11", "snapuid1-11", "claim1-11", "", validSecretClass, "content1-11", &False, nil, nil, nil, false, true, nil), - expectedEvents: noevents, - initialSecrets: []*v1.Secret{secret()}, - errors: noerrors, - test: testSyncContent, - }, - { - name: "1-12 - content with retain policy will not be deleted if it is bound to a non-exist snapshot and also has a snapshot uid specified", - initialContents: newContentArray("content1-12", "sid1-12", "snap1-11", "sid1-11", validSecretClass, "", "", retainPolicy, nil, nil, true), - expectedContents: newContentArray("content1-12", "sid1-12", "snap1-11", "sid1-11", validSecretClass, "", "", retainPolicy, nil, nil, true), - initialSnapshots: nosnapshots, - expectedSnapshots: nosnapshots, - expectedEvents: noevents, - errors: noerrors, - test: testSyncContent, - }, - { - name: "1-13 - content with empty snapshot class is not deleted when Deletion policy is not set even if it is bound to a non-exist snapshot and also has a snapshot uid specified", - initialContents: newContentArray("content1-13", "sid1-13", "snap1-13", "sid1-13", validSecretClass, "", "", retainPolicy, nil, nil, true), - expectedContents: newContentArray("content1-13", "sid1-13", "snap1-13", "sid1-13", validSecretClass, "", "", retainPolicy, nil, nil, true), - initialSnapshots: nosnapshots, - expectedSnapshots: nosnapshots, - expectedEvents: noevents, - errors: noerrors, - test: testSyncContent, - }, - /*{ - name: "1-14 - content will not be deleted if it is bound to a snapshot correctly, snapshot uid is specified", - initialContents: newContentArray("content1-14", "snapuid1-14", "snap1-14", "sid1-14", validSecretClass, "", "", retainPolicy, nil, nil, true), - expectedContents: newContentArray("content1-14", "snapuid1-14", "snap1-14", "sid1-14", validSecretClass, "", "", retainPolicy, nil, nil, true), - initialSnapshots: newSnapshotArray("snap1-14", "snapuid1-14", "claim1-14", "", validSecretClass, "content1-14", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap1-14", "snapuid1-14", "claim1-14", "", validSecretClass, "content1-14", &True, nil, nil, nil), - expectedEvents: noevents, - initialSecrets: []*v1.Secret{secret()}, - errors: noerrors, - test: testSyncContent, - },*/ - { - name: "1-16 - continue delete with snapshot class that has nonexistent secret", - initialContents: newContentArray("content1-16", "sid1-16", "snap1-16", "sid1-16", emptySecretClass, "", "", deletePolicy, nil, nil, true), - expectedContents: newContentArray("content1-16", "sid1-16", "snap1-16", "sid1-16", emptySecretClass, "", "", deletePolicy, nil, nil, true), - initialSnapshots: nosnapshots, - expectedSnapshots: nosnapshots, - expectedEvents: noevents, - errors: noerrors, - initialSecrets: []*v1.Secret{}, // secret does not exist - //expectedDeleteCalls: []deleteCall{{"sid1-16", nil, nil}}, - test: testSyncContent, }, { name: "3-1 - content will be deleted if snapshot deletion timestamp is set", @@ -314,6 +199,23 @@ func TestDeleteSync(t *testing.T) { expectSuccess: false, test: testSyncSnapshotError, }, + { + name: "3-3 - content will not be deleted if retainPolicy is set", + initialContents: newContentArray("content3-3", "", "snap3-3", "sid3-3", validSecretClass, "", "", retainPolicy, nil, nil, true), + expectedContents: withContentAnnotations(newContentArray("content3-3", "", "snap3-3", "sid3-3", validSecretClass, "", "", retainPolicy, nil, nil, true), + map[string]string{ + "snapshot.storage.kubernetes.io/volumesnapshot-being-deleted": "yes", + }), + initialSnapshots: newSnapshotArray("snap3-3", "snapuid3-3", "claim3-3", "", validSecretClass, "content3-3", &False, nil, nil, nil, false, true, &timeNowMetav1), + expectedSnapshots: withSnapshotFinalizers(newSnapshotArray("snap3-3", "snapuid3-3", "claim3-3", "", validSecretClass, "content3-3", &False, nil, nil, nil, false, false, &timeNowMetav1), + utils.VolumeSnapshotBoundFinalizer, + ), + initialClaims: newClaimArray("claim3-3", "pvc-uid3-3", "1Gi", "volume3-3", v1.ClaimBound, &classEmpty), + expectedEvents: noevents, + initialSecrets: []*v1.Secret{secret()}, + errors: noerrors, + test: testSyncSnapshot, + }, } runSyncTests(t, tests, snapshotClasses) } diff --git a/pkg/common-controller/snapshot_update_test.go b/pkg/common-controller/snapshot_update_test.go index 5ba641bf..06cdb7e1 100644 --- a/pkg/common-controller/snapshot_update_test.go +++ b/pkg/common-controller/snapshot_update_test.go @@ -37,6 +37,8 @@ var volumeErr = &storagev1beta1.VolumeError{ Message: "Failed to upload the snapshot", } +var emptyString = "" + // Test single call to syncSnapshot and syncContent methods. // 1. Fill in the controller with initial data // 2. Call the tested function (syncSnapshot/syncContent) via @@ -56,16 +58,17 @@ func TestSync(t *testing.T) { errors: noerrors, test: testSyncSnapshot, }, - /*{ + { name: "2-2 - snapshot points to a content but content does not point to snapshot(VolumeSnapshotRef does not match)", initialContents: newContentArray("content2-2", "snapuid2-2-x", "snap2-2", "sid2-2", validSecretClass, "", "", deletionPolicy, nil, nil, false), expectedContents: newContentArray("content2-2", "snapuid2-2-x", "snap2-2", "sid2-2", validSecretClass, "", "", deletionPolicy, nil, nil, false), - initialSnapshots: newSnapshotArray("snap2-2", "snapuid2-2", "claim2-2", "", validSecretClass, "content2-2", &False, nil, nil, nil), - expectedSnapshots: newSnapshotArray("snap2-2", "snapuid2-2", "claim2-2", "", validSecretClass, "content2-2", &False, nil, nil, newVolumeError("Snapshot is bound to a VolumeSnapshotContent which is bound to other Snapshot")), - expectedEvents: []string{"Warning InvalidSnapshotBinding"}, + initialSnapshots: newSnapshotArray("snap2-2", "snapuid2-2", "", "content2-2", validSecretClass, "content2-2", &False, nil, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap2-2", "snapuid2-2", "", "content2-2", validSecretClass, "content2-2", &False, nil, nil, newVolumeError("Snapshot failed to bind VolumeSnapshotContent, Could not bind snapshot snap2-2 and content content2-2, the VolumeSnapshotRef does not match"), false, true, nil), + initialClaims: newClaimArray("claim2-2", "pvc-uid2-2", "1Gi", "volume2-2", v1.ClaimBound, &classEmpty), + expectedEvents: []string{"Warning SnapshotBindFailed"}, errors: noerrors, test: testSyncSnapshotError, - },*/ + }, { name: "2-3 - success bind snapshot and content but not ready, no status changed", initialContents: newContentArray("content2-3", "snapuid2-3", "snap2-3", "sid2-3", validSecretClass, "", "", deletionPolicy, nil, nil, false), @@ -75,22 +78,8 @@ func TestSync(t *testing.T) { initialClaims: newClaimArray("claim2-3", "pvc-uid2-3", "1Gi", "volume2-3", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume2-3", "pv-uid2-3", "pv-handle2-3", "1Gi", "pvc-uid2-3", "claim2-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialSecrets: []*v1.Secret{secret()}, - /*expectedCreateCalls: []createCall{ - { - snapshotName: "snapshot-snapuid2-3", - volume: newVolume("volume2-3", "pv-uid2-3", "pv-handle2-3", "1Gi", "pvc-uid2-3", "claim2-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), - parameters: class5Parameters, - secrets: map[string]string{"foo": "bar"}, - // information to return - driverName: mockDriverName, - size: defaultSize, - snapshotId: "sid2-3", - creationTime: timeNow, - readyToUse: False, - }, - },*/ - errors: noerrors, - test: testSyncSnapshot, + errors: noerrors, + test: testSyncSnapshot, }, { // nothing changed @@ -111,22 +100,8 @@ func TestSync(t *testing.T) { initialClaims: newClaimArray("claim2-5", "pvc-uid2-5", "1Gi", "volume2-5", v1.ClaimBound, &classEmpty), initialVolumes: newVolumeArray("volume2-5", "pv-uid2-5", "pv-handle2-5", "1Gi", "pvc-uid2-5", "claim2-5", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialSecrets: []*v1.Secret{secret()}, - /*expectedCreateCalls: []createCall{ - { - snapshotName: "snapshot-snapuid2-5", - volume: newVolume("volume2-5", "pv-uid2-5", "pv-handle2-5", "1Gi", "pvc-uid2-5", "claim2-5", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), - parameters: class5Parameters, - secrets: map[string]string{"foo": "bar"}, - // information to return - driverName: mockDriverName, - size: defaultSize, - snapshotId: "sid2-5", - creationTime: timeNow, - readyToUse: True, - }, - },*/ - errors: noerrors, - test: testSyncSnapshot, + errors: noerrors, + test: testSyncSnapshot, }, { name: "2-6 - snapshot bound to prebound content correctly, status ready false -> true, ref.UID '' -> 'snapuid2-6'", @@ -137,69 +112,47 @@ func TestSync(t *testing.T) { errors: noerrors, test: testSyncSnapshot, }, - /*{ - name: "2-7 - snapshot and content bound, csi driver get status error", - initialContents: newContentArrayWithReadyToUse("content2-7", "snapuid2-7", "snap2-7", "sid2-7", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &False, false), - expectedContents: newContentArrayWithReadyToUse("content2-7", "snapuid2-7", "snap2-7", "sid2-7", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &False, false), - initialSnapshots: newSnapshotArray("snap2-7", "snapuid2-7", "claim2-7", "", validSecretClass, "content2-7", &False, metaTimeNow, nil, nil), - expectedSnapshots: newSnapshotArray("snap2-7", "snapuid2-7", "claim2-7", "", validSecretClass, "content2-7", &False, metaTimeNow, nil, newVolumeError("Failed to check and update snapshot: mock create snapshot error")), + { + name: "2-8 - snapshot and content bound, apiserver update status error", + initialContents: newContentArrayWithReadyToUse("content2-8", "snapuid2-8", "snap2-8", "sid2-8", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &False, false), + expectedContents: newContentArrayWithReadyToUse("content2-8", "snapuid2-8", "snap2-8", "sid2-8", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &False, false), + initialSnapshots: newSnapshotArray("snap2-8", "snapuid2-8", "claim2-8", "", validSecretClass, "content2-8", &False, metaTimeNow, nil, nil, false, false, nil), + expectedSnapshots: newSnapshotArray("snap2-8", "snapuid2-8", "claim2-8", "", validSecretClass, "content2-8", &False, metaTimeNow, nil, nil, false, false, nil), expectedEvents: []string{"Warning SnapshotCheckandUpdateFailed"}, - initialClaims: newClaimArray("claim2-7", "pvc-uid2-7", "1Gi", "volume2-7", v1.ClaimBound, &classEmpty), - initialVolumes: newVolumeArray("volume2-7", "pv-uid2-7", "pv-handle2-7", "1Gi", "pvc-uid2-7", "claim2-7", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + initialClaims: newClaimArray("claim2-8", "pvc-uid2-8", "1Gi", "volume2-8", v1.ClaimBound, &classEmpty), + initialVolumes: newVolumeArray("volume2-8", "pv-uid2-8", "pv-handle2-8", "1Gi", "pvc-uid2-8", "claim2-8", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialSecrets: []*v1.Secret{secret()}, - expectedCreateCalls: []createCall{ - { - snapshotName: "snapshot-snapuid2-7", - volume: newVolume("volume2-7", "pv-uid2-7", "pv-handle2-7", "1Gi", "pvc-uid2-7", "claim2-7", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), - parameters: class5Parameters, - secrets: map[string]string{"foo": "bar"}, - // information to return - err: errors.New("mock create snapshot error"), - }, - }, - errors: noerrors, - test: testSyncSnapshot, - },*/ - /*{ - name: "2-8 - snapshot and content bound, apiserver update status error", - initialContents: newContentArrayWithReadyToUse("content2-8", "snapuid2-8", "snap2-8", "sid2-8", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &False, false), - expectedContents: newContentArrayWithReadyToUse("content2-8", "snapuid2-8", "snap2-8", "sid2-8", validSecretClass, "", "", deletionPolicy, &timeNowStamp, nil, &False, false), - initialSnapshots: newSnapshotArray("snap2-8", "snapuid2-8", "claim2-8", "", validSecretClass, "content2-8", &False, metaTimeNow, nil, nil), - expectedSnapshots: newSnapshotArray("snap2-8", "snapuid2-8", "claim2-8", "", validSecretClass, "content2-8", &False, metaTimeNow, nil, newVolumeError("Failed to check and update snapshot: snapshot controller failed to update default/snap2-8 on API server: mock update error")), - expectedEvents: []string{"Warning SnapshotCheckandUpdateFailed"}, - initialClaims: newClaimArray("claim2-8", "pvc-uid2-8", "1Gi", "volume2-8", v1.ClaimBound, &classEmpty), - initialVolumes: newVolumeArray("volume2-8", "pv-uid2-8", "pv-handle2-8", "1Gi", "pvc-uid2-8", "claim2-8", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), - initialSecrets: []*v1.Secret{secret()}, - /*expectedCreateCalls: []createCall{ - { - snapshotName: "snapshot-snapuid2-8", - volume: newVolume("volume2-8", "pv-uid2-8", "pv-handle2-8", "1Gi", "pvc-uid2-8", "claim2-8", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), - parameters: class5Parameters, - secrets: map[string]string{"foo": "bar"}, - // information to return - driverName: mockDriverName, - size: defaultSize, - snapshotId: "sid2-8", - creationTime: timeNow, - readyToUse: true, - }, - },*/ /* errors: []reactorError{ // Inject error to the first client.VolumesnapshotV1beta1().VolumeSnapshots().Update call. // All other calls will succeed. {"update", "volumesnapshots", errors.New("mock update error")}, }, - test: testSyncSnapshot, - },*/ + test: testSyncSnapshotError, + }, { name: "2-9 - fail on status update as there is not pvc provided", - initialContents: newContentArray("content2-9", "snapuid2-9", "snap2-9", "sid2-9", validSecretClass, "", "", deletionPolicy, nil, nil, false), - expectedContents: newContentArray("content2-9", "snapuid2-9", "snap2-9", "sid2-9", validSecretClass, "", "", deletionPolicy, nil, nil, false), + initialContents: nocontents, + expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap2-9", "snapuid2-9", "claim2-9", "", validSecretClass, "", &False, nil, nil, nil, false, true, nil), - expectedSnapshots: newSnapshotArray("snap2-9", "snapuid2-9", "claim2-9", "", validSecretClass, "content2-9", &True, nil, nil, nil, false, true, nil), - //expectedSnapshots: newSnapshotArray("snap2-9", "snapuid2-9", "claim2-9", "", validSecretClass, "content2-9", &False, nil, nil, newVolumeError("Failed to check and update snapshot: failed to get input parameters to create snapshot snap2-9: \"failed to retrieve PVC claim2-9 from the lister: \\\"persistentvolumeclaim \\\\\\\"claim2-9\\\\\\\" not found\\\"\"")), - errors: noerrors, - test: testSyncSnapshot, + expectedSnapshots: newSnapshotArray("snap2-9", "snapuid2-9", "claim2-9", "", validSecretClass, "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error snapshot controller failed to update snap2-9 on API server: cannot get claim from snapshot"), false, true, nil), + errors: []reactorError{ + {"get", "persistentvolumeclaims", errors.New("mock update error")}, + {"get", "persistentvolumeclaims", errors.New("mock update error")}, + {"get", "persistentvolumeclaims", errors.New("mock update error")}, + }, test: testSyncSnapshot, + }, + { + name: "7-1 - fail to create snapshot with non-existing snapshot class", + initialContents: nocontents, + expectedContents: nocontents, + initialSnapshots: newSnapshotArray("snap7-1", "snapuid7-1", "claim7-1", "", classNonExisting, "", &False, nil, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap7-1", "snapuid7-1", "claim7-1", "", classNonExisting, "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-1: \"failed to retrieve snapshot class non-existing from the informer: \\\"volumesnapshotclass.snapshot.storage.k8s.io \\\\\\\"non-existing\\\\\\\" not found\\\"\""), false, true, nil), + initialClaims: newClaimArray("claim7-1", "pvc-uid7-1", "1Gi", "volume7-1", v1.ClaimBound, &classEmpty), + initialVolumes: newVolumeArray("volume7-1", "pv-uid7-1", "pv-handle7-1", "1Gi", "pvc-uid7-1", "claim7-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), + expectedEvents: []string{"Warning SnapshotContentCreationFailed"}, + errors: noerrors, + expectSuccess: false, + test: testSyncSnapshot, }, { name: "2-10 - do not bind when snapshot and content not match", @@ -210,6 +163,29 @@ func TestSync(t *testing.T) { errors: noerrors, test: testSyncSnapshot, }, + { + name: "2-11 - successful bind snapshot content with volume snapshot classname", + initialContents: withContentSpecSnapshotClassName(newContentArray("content2-11", "snapuid2-11", "snap2-11", "sid2-11", validSecretClass, "", "", deletionPolicy, nil, nil, false), nil), + expectedContents: newContentArray("content2-11", "snapuid2-11", "snap2-11", "sid2-11", validSecretClass, "", "", deletionPolicy, nil, nil, false), + initialSnapshots: newSnapshotArray("snap2-11", "snapuid2-11", "", "content2-11", validSecretClass, "content2-11", &False, nil, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap2-11", "snapuid2-11", "", "content2-11", validSecretClass, "content2-11", &True, nil, nil, nil, false, true, nil), + initialClaims: newClaimArray("claim2-11", "pvc-uid2-11", "1Gi", "volume2-11", v1.ClaimBound, &classEmpty), + errors: noerrors, + test: testSyncSnapshot, + }, + { + name: "2-12 - fail bind snapshot content with volume snapshot classname due to API call failed", + initialContents: withContentSpecSnapshotClassName(newContentArray("content2-12", "snapuid2-12", "snap2-12", "sid2-12", validSecretClass, "", "", deletionPolicy, nil, nil, false), nil), + expectedContents: withContentSpecSnapshotClassName(newContentArray("content2-12", "snapuid2-12", "snap2-12", "sid2-12", validSecretClass, "", "", deletionPolicy, nil, nil, false), nil), + initialSnapshots: newSnapshotArray("snap2-12", "snapuid2-12", "", "content2-12", validSecretClass, "content2-12", &False, nil, nil, nil, false, true, nil), + expectedSnapshots: newSnapshotArray("snap2-12", "snapuid2-12", "", "content2-12", validSecretClass, "content2-12", &False, nil, nil, newVolumeError("Snapshot failed to bind VolumeSnapshotContent, mock update error"), false, true, nil), + initialClaims: newClaimArray("claim2-12", "pvc-uid2-12", "1Gi", "volume2-12", v1.ClaimBound, &classEmpty), + errors: []reactorError{ + // Inject error to the forth client.VolumesnapshotV1beta1().VolumeSnapshots().Update call. + {"update", "volumesnapshotcontents", errors.New("mock update error")}, + }, + test: testSyncSnapshot, + }, { name: "3-1 - ready snapshot lost reference to VolumeSnapshotContent", initialContents: nocontents,