From e647a80298def02e4ab34a8e189a2c5ba391455a Mon Sep 17 00:00:00 2001 From: Xing Yang Date: Sat, 24 Aug 2019 12:41:42 -0700 Subject: [PATCH] Add deletion secret as annotation to content --- pkg/controller/framework_test.go | 33 +++++++++-- pkg/controller/snapshot_controller.go | 66 ++++++++++++++------- pkg/controller/snapshot_controller_test.go | 4 +- pkg/controller/snapshot_create_test.go | 24 ++++++-- pkg/controller/snapshot_delete_test.go | 68 +++++++++++++--------- pkg/controller/snapshot_ready_test.go | 48 +++++++-------- pkg/controller/util.go | 12 ++-- pkg/controller/util_test.go | 5 +- 8 files changed, 165 insertions(+), 95 deletions(-) diff --git a/pkg/controller/framework_test.go b/pkg/controller/framework_test.go index a197d82c2..5cc0a8abc 100644 --- a/pkg/controller/framework_test.go +++ b/pkg/controller/framework_test.go @@ -790,11 +790,12 @@ func newTestController(kubeClient kubernetes.Interface, clientset clientset.Inte } // newContent returns a new content with given attributes -func newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, deletionPolicy *crdv1.DeletionPolicy, size *int64, creationTime *int64, withFinalizer bool) *crdv1.VolumeSnapshotContent { +func newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, deletionPolicy *crdv1.DeletionPolicy, size *int64, creationTime *int64, withFinalizer bool, annotations map[string]string) *crdv1.VolumeSnapshotContent { content := crdv1.VolumeSnapshotContent{ ObjectMeta: metav1.ObjectMeta{ Name: name, ResourceVersion: "1", + Annotations: annotations, }, Spec: crdv1.VolumeSnapshotContentSpec{ VolumeSnapshotSource: crdv1.VolumeSnapshotSource{ @@ -833,14 +834,14 @@ func newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToS return &content } -func newContentArray(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, deletionPolicy *crdv1.DeletionPolicy, size *int64, creationTime *int64, withFinalizer bool) []*crdv1.VolumeSnapshotContent { +func newContentArray(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, deletionPolicy *crdv1.DeletionPolicy, size *int64, creationTime *int64, withFinalizer bool, annotations map[string]string) []*crdv1.VolumeSnapshotContent { return []*crdv1.VolumeSnapshotContent{ - newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName, deletionPolicy, size, creationTime, withFinalizer), + newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName, deletionPolicy, size, creationTime, withFinalizer, annotations), } } func newContentWithUnmatchDriverArray(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, deletionPolicy *crdv1.DeletionPolicy, size *int64, creationTime *int64) []*crdv1.VolumeSnapshotContent { - content := newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName, deletionPolicy, size, creationTime, false) + content := newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName, deletionPolicy, size, creationTime, false, nil) content.Spec.VolumeSnapshotSource.CSI.Driver = "fake" return []*crdv1.VolumeSnapshotContent{ content, @@ -1287,6 +1288,28 @@ func secret() *v1.Secret { } } +func secretAnnotations() map[string]string { + return map[string]string{ + AnnDeletionSecretRefName: "secret", + AnnDeletionSecretRefNamespace: "default", + } +} + +func emptyNamespaceSecretAnnotations() map[string]string { + return map[string]string{ + AnnDeletionSecretRefName: "name", + AnnDeletionSecretRefNamespace: "", + } +} + +// this refers to emptySecret(), which is missing data. +func emptyDataSecretAnnotations() map[string]string { + return map[string]string{ + AnnDeletionSecretRefName: "emptysecret", + AnnDeletionSecretRefNamespace: "default", + } +} + type listCall struct { snapshotID string // information to return @@ -1368,7 +1391,7 @@ func (f *fakeSnapshotter) CreateSnapshot(ctx context.Context, snapshotName strin func (f *fakeSnapshotter) DeleteSnapshot(ctx context.Context, snapshotID string, snapshotterCredentials map[string]string) error { if f.deleteCallCounter >= len(f.deleteCalls) { f.t.Errorf("Unexpected CSI Delete Snapshot call: snapshotID=%s, index: %d, calls: %+v", snapshotID, f.createCallCounter, f.createCalls) - return fmt.Errorf("unexpected call") + return fmt.Errorf("unexpected DeleteSnapshot call") } call := f.deleteCalls[f.deleteCallCounter] f.deleteCallCounter++ diff --git a/pkg/controller/snapshot_controller.go b/pkg/controller/snapshot_controller.go index 5333c63f4..b93de0067 100644 --- a/pkg/controller/snapshot_controller.go +++ b/pkg/controller/snapshot_controller.go @@ -523,7 +523,7 @@ func (ctrl *csiSnapshotController) checkandBindSnapshotContent(snapshot *crdv1.V return newContent, nil } -func (ctrl *csiSnapshotController) getCreateSnapshotInput(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotClass, *v1.PersistentVolume, string, map[string]string, error) { +func (ctrl *csiSnapshotController) getCreateSnapshotInput(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotClass, *v1.PersistentVolume, string, *v1.SecretReference, error) { className := snapshot.Spec.VolumeSnapshotClassName klog.V(5).Infof("getCreateSnapshotInput [%s]: VolumeSnapshotClassName [%s]", snapshot.Name, *className) var class *crdv1.VolumeSnapshotClass @@ -553,12 +553,8 @@ func (ctrl *csiSnapshotController) getCreateSnapshotInput(snapshot *crdv1.Volume if err != nil { return nil, nil, "", nil, err } - snapshotterCredentials, err := getCredentials(ctrl.client, snapshotterSecretRef) - if err != nil { - return nil, nil, "", nil, err - } - return class, volume, contentName, snapshotterCredentials, nil + return class, volume, contentName, snapshotterSecretRef, nil } func (ctrl *csiSnapshotController) checkandUpdateBoundSnapshotStatusOperation(snapshot *crdv1.VolumeSnapshot, content *crdv1.VolumeSnapshotContent) (*crdv1.VolumeSnapshot, error) { @@ -580,10 +576,14 @@ func (ctrl *csiSnapshotController) checkandUpdateBoundSnapshotStatusOperation(sn driverName, snapshotID = content.Spec.CSI.Driver, content.Spec.CSI.SnapshotHandle } } else { - class, volume, _, snapshotterCredentials, err := ctrl.getCreateSnapshotInput(snapshot) + class, volume, _, snapshotterSecretRef, err := ctrl.getCreateSnapshotInput(snapshot) if err != nil { return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err) } + snapshotterCredentials, err := getCredentials(ctrl.client, snapshotterSecretRef) + if err != nil { + return nil, err + } driverName, snapshotID, creationTime, size, readyToUse, err = ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials) if err != nil { klog.Errorf("checkandUpdateBoundSnapshotStatusOperation: failed to call create snapshot to check whether the snapshot is ready to use %q", err) @@ -627,11 +627,16 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum return nil, err } - class, volume, contentName, snapshotterCredentials, err := ctrl.getCreateSnapshotInput(snapshot) + class, volume, contentName, snapshotterSecretRef, err := ctrl.getCreateSnapshotInput(snapshot) if err != nil { return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err) } + snapshotterCredentials, err := getCredentials(ctrl.client, snapshotterSecretRef) + if err != nil { + return nil, err + } + driverName, snapshotID, creationTime, size, readyToUse, err := ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials) if err != nil { return nil, fmt.Errorf("failed to take snapshot of the volume, %s: %q", volume.Name, err) @@ -687,6 +692,16 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum DeletionPolicy: class.DeletionPolicy, }, } + + // Set AnnDeletionSecretRefName and AnnDeletionSecretRefNamespace + if snapshotterSecretRef != nil { + klog.V(5).Infof("createSnapshotOperation: set annotation [%s] on content [%s].", AnnDeletionSecretRefName, snapshotContent.Name) + metav1.SetMetaDataAnnotation(&snapshotContent.ObjectMeta, AnnDeletionSecretRefName, snapshotterSecretRef.Name) + + klog.V(5).Infof("syncContent: set annotation [%s] on content [%s].", AnnDeletionSecretRefNamespace, snapshotContent.Name) + metav1.SetMetaDataAnnotation(&snapshotContent.ObjectMeta, AnnDeletionSecretRefNamespace, snapshotterSecretRef.Namespace) + } + klog.V(3).Infof("volume snapshot content %v", snapshotContent) // Try to create the VolumeSnapshotContent object several times for i := 0; i < ctrl.createSnapshotContentRetryCount; i++ { @@ -736,23 +751,30 @@ func (ctrl *csiSnapshotController) deleteSnapshotContentOperation(content *crdv1 // get secrets if VolumeSnapshotClass specifies it var snapshotterCredentials map[string]string - snapshotClassName := content.Spec.VolumeSnapshotClassName - if snapshotClassName != nil { - if snapshotClass, err := ctrl.classLister.Get(*snapshotClassName); err == nil { - // Resolve snapshotting secret credentials. - // No VolumeSnapshot is provided when resolving delete secret names, since the VolumeSnapshot may or may not exist at delete time. - snapshotterSecretRef, err := getSecretReference(snapshotClass.Parameters, content.Name, nil) - if err != nil { - return err - } - snapshotterCredentials, err = getCredentials(ctrl.client, snapshotterSecretRef) - if err != nil { - return err - } + var err error + + // Check if annotation exists + if metav1.HasAnnotation(content.ObjectMeta, AnnDeletionSecretRefName) && metav1.HasAnnotation(content.ObjectMeta, AnnDeletionSecretRefNamespace) { + annDeletionSecretName := content.Annotations[AnnDeletionSecretRefName] + annDeletionSecretNamespace := content.Annotations[AnnDeletionSecretRefNamespace] + + snapshotterSecretRef := &v1.SecretReference{} + + if annDeletionSecretName == "" || annDeletionSecretNamespace == "" { + return fmt.Errorf("cannot delete snapshot %#v, err: secret name or namespace not specified", content.Name) + } + + snapshotterSecretRef.Name = annDeletionSecretName + snapshotterSecretRef.Namespace = annDeletionSecretNamespace + + snapshotterCredentials, err = getCredentials(ctrl.client, snapshotterSecretRef) + if err != nil { + // Continue with deletion, as the secret may have already been deleted. + klog.Errorf("Failed to get credentials for snapshot %s: %s", content.Name, err.Error()) } } - err := ctrl.handler.DeleteSnapshot(content, snapshotterCredentials) + err = ctrl.handler.DeleteSnapshot(content, snapshotterCredentials) if err != nil { ctrl.eventRecorder.Event(content, v1.EventTypeWarning, "SnapshotDeleteError", "Failed to delete snapshot") return fmt.Errorf("failed to delete snapshot %#v, err: %v", content.Name, err) diff --git a/pkg/controller/snapshot_controller_test.go b/pkg/controller/snapshot_controller_test.go index 625f9d700..a0e05d56f 100644 --- a/pkg/controller/snapshot_controller_test.go +++ b/pkg/controller/snapshot_controller_test.go @@ -23,7 +23,7 @@ import ( ) func storeVersion(t *testing.T, prefix string, c cache.Store, version string, expectedReturn bool) { - content := newContent("contentName", classEmpty, "sid1-1", "vuid1-1", "volume1-1", "snapuid1-1", "snap1-1", nil, nil, nil, false) + content := newContent("contentName", classEmpty, "sid1-1", "vuid1-1", "volume1-1", "snapuid1-1", "snap1-1", nil, nil, nil, false, nil) content.ResourceVersion = version ret, err := storeObjectUpdate(c, content, "content") if err != nil { @@ -82,7 +82,7 @@ func TestControllerCacheParsingError(t *testing.T) { // There must be something in the cache to compare with storeVersion(t, "Step1", c, "1", true) - content := newContent("contentName", classEmpty, "sid1-1", "vuid1-1", "volume1-1", "snapuid1-1", "snap1-1", nil, nil, nil, false) + content := newContent("contentName", classEmpty, "sid1-1", "vuid1-1", "volume1-1", "snapuid1-1", "snap1-1", nil, nil, nil, false, nil) content.ResourceVersion = "xxx" _, err := storeObjectUpdate(c, content, "content") if err == nil { diff --git a/pkg/controller/snapshot_create_test.go b/pkg/controller/snapshot_create_test.go index 53358b238..0bf3b53c1 100644 --- a/pkg/controller/snapshot_create_test.go +++ b/pkg/controller/snapshot_create_test.go @@ -69,7 +69,7 @@ func TestCreateSnapshotSync(t *testing.T) { { name: "6-1 - successful create snapshot with snapshot class gold", initialContents: nocontents, - expectedContents: newContentArray("snapcontent-snapuid6-1", classGold, "sid6-1", "pv-uid6-1", "volume6-1", "snapuid6-1", "snap6-1", &deletePolicy, &defaultSize, &timeNowStamp, false), + expectedContents: newContentArray("snapcontent-snapuid6-1", classGold, "sid6-1", "pv-uid6-1", "volume6-1", "snapuid6-1", "snap6-1", &deletePolicy, &defaultSize, &timeNowStamp, false, nil), initialSnapshots: newSnapshotArray("snap6-1", classGold, "", "snapuid6-1", "claim6-1", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap6-1", classGold, "snapcontent-snapuid6-1", "snapuid6-1", "claim6-1", false, nil, metaTimeNowUnix, getSize(defaultSize)), initialClaims: newClaimArray("claim6-1", "pvc-uid6-1", "1Gi", "volume6-1", v1.ClaimBound, &classEmpty), @@ -93,7 +93,7 @@ func TestCreateSnapshotSync(t *testing.T) { { name: "6-2 - successful create snapshot with snapshot class silver", initialContents: nocontents, - expectedContents: newContentArray("snapcontent-snapuid6-2", classSilver, "sid6-2", "pv-uid6-2", "volume6-2", "snapuid6-2", "snap6-2", &deletePolicy, &defaultSize, &timeNowStamp, false), + expectedContents: newContentArray("snapcontent-snapuid6-2", classSilver, "sid6-2", "pv-uid6-2", "volume6-2", "snapuid6-2", "snap6-2", &deletePolicy, &defaultSize, &timeNowStamp, false, nil), initialSnapshots: newSnapshotArray("snap6-2", classSilver, "", "snapuid6-2", "claim6-2", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap6-2", classSilver, "snapcontent-snapuid6-2", "snapuid6-2", "claim6-2", false, nil, metaTimeNowUnix, getSize(defaultSize)), initialClaims: newClaimArray("claim6-2", "pvc-uid6-2", "1Gi", "volume6-2", v1.ClaimBound, &classEmpty), @@ -117,7 +117,7 @@ func TestCreateSnapshotSync(t *testing.T) { { name: "6-3 - successful create snapshot with snapshot class valid-secret-class", initialContents: nocontents, - expectedContents: newContentArray("snapcontent-snapuid6-3", validSecretClass, "sid6-3", "pv-uid6-3", "volume6-3", "snapuid6-3", "snap6-3", &deletePolicy, &defaultSize, &timeNowStamp, false), + expectedContents: newContentArray("snapcontent-snapuid6-3", validSecretClass, "sid6-3", "pv-uid6-3", "volume6-3", "snapuid6-3", "snap6-3", &deletePolicy, &defaultSize, &timeNowStamp, false, secretAnnotations()), initialSnapshots: newSnapshotArray("snap6-3", validSecretClass, "", "snapuid6-3", "claim6-3", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap6-3", validSecretClass, "snapcontent-snapuid6-3", "snapuid6-3", "claim6-3", false, nil, metaTimeNowUnix, getSize(defaultSize)), initialClaims: newClaimArray("claim6-3", "pvc-uid6-3", "1Gi", "volume6-3", v1.ClaimBound, &classEmpty), @@ -143,7 +143,7 @@ func TestCreateSnapshotSync(t *testing.T) { { name: "6-4 - successful create snapshot with snapshot class empty-secret-class", initialContents: nocontents, - expectedContents: newContentArray("snapcontent-snapuid6-4", emptySecretClass, "sid6-4", "pv-uid6-4", "volume6-4", "snapuid6-4", "snap6-4", &deletePolicy, &defaultSize, &timeNowStamp, false), + expectedContents: newContentArray("snapcontent-snapuid6-4", emptySecretClass, "sid6-4", "pv-uid6-4", "volume6-4", "snapuid6-4", "snap6-4", &deletePolicy, &defaultSize, &timeNowStamp, false, emptyDataSecretAnnotations()), initialSnapshots: newSnapshotArray("snap6-4", emptySecretClass, "", "snapuid6-4", "claim6-4", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap6-4", emptySecretClass, "snapcontent-snapuid6-4", "snapuid6-4", "claim6-4", false, nil, metaTimeNowUnix, getSize(defaultSize)), initialClaims: newClaimArray("claim6-4", "pvc-uid6-4", "1Gi", "volume6-4", v1.ClaimBound, &classEmpty), @@ -169,7 +169,7 @@ func TestCreateSnapshotSync(t *testing.T) { { name: "6-5 - successful create snapshot with status uploading", initialContents: nocontents, - expectedContents: newContentArray("snapcontent-snapuid6-5", classGold, "sid6-5", "pv-uid6-5", "volume6-5", "snapuid6-5", "snap6-5", &deletePolicy, &defaultSize, &timeNowStamp, false), + expectedContents: newContentArray("snapcontent-snapuid6-5", classGold, "sid6-5", "pv-uid6-5", "volume6-5", "snapuid6-5", "snap6-5", &deletePolicy, &defaultSize, &timeNowStamp, false, nil), initialSnapshots: newSnapshotArray("snap6-5", classGold, "", "snapuid6-5", "claim6-5", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap6-5", classGold, "snapcontent-snapuid6-5", "snapuid6-5", "claim6-5", false, nil, metaTimeNowUnix, getSize(defaultSize)), initialClaims: newClaimArray("claim6-5", "pvc-uid6-5", "1Gi", "volume6-5", v1.ClaimBound, &classEmpty), @@ -193,7 +193,7 @@ func TestCreateSnapshotSync(t *testing.T) { { name: "6-6 - successful create snapshot with status error uploading", initialContents: nocontents, - expectedContents: newContentArray("snapcontent-snapuid6-6", classGold, "sid6-6", "pv-uid6-6", "volume6-6", "snapuid6-6", "snap6-6", &deletePolicy, &defaultSize, &timeNowStamp, false), + expectedContents: newContentArray("snapcontent-snapuid6-6", classGold, "sid6-6", "pv-uid6-6", "volume6-6", "snapuid6-6", "snap6-6", &deletePolicy, &defaultSize, &timeNowStamp, false, nil), initialSnapshots: newSnapshotArray("snap6-6", classGold, "", "snapuid6-6", "claim6-6", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap6-6", classGold, "snapcontent-snapuid6-6", "snapuid6-6", "claim6-6", false, nil, metaTimeNowUnix, getSize(defaultSize)), initialClaims: newClaimArray("claim6-6", "pvc-uid6-6", "1Gi", "volume6-6", v1.ClaimBound, &classEmpty), @@ -365,6 +365,18 @@ func TestCreateSnapshotSync(t *testing.T) { expectedEvents: []string{"Warning CreateSnapshotContentFailed"}, test: testSyncSnapshot, }, + { + name: "7-10 - fail create snapshot with secret not found", + initialContents: nocontents, + expectedContents: nocontents, + initialSnapshots: newSnapshotArray("snap7-10", validSecretClass, "", "snapuid7-10", "claim7-10", false, nil, nil, nil), + expectedSnapshots: newSnapshotArray("snap7-10", validSecretClass, "", "snapuid7-10", "claim7-10", false, newVolumeError("Failed to create snapshot: error getting secret secret in namespace default: cannot find secret secret"), nil, 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 + errors: noerrors, + test: testSyncSnapshot, + }, } runSyncTests(t, tests, snapshotClasses) } diff --git a/pkg/controller/snapshot_delete_test.go b/pkg/controller/snapshot_delete_test.go index 6be2be119..e94ab56d4 100644 --- a/pkg/controller/snapshot_delete_test.go +++ b/pkg/controller/snapshot_delete_test.go @@ -119,7 +119,7 @@ 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", classEmpty, "sid1-1", "vuid1-1", "volume1-1", "snapuid1-1", "snap1-1", &deletePolicy, nil, nil, true), + initialContents: newContentArray("content1-1", classEmpty, "sid1-1", "vuid1-1", "volume1-1", "snapuid1-1", "snap1-1", &deletePolicy, nil, nil, true, nil), expectedContents: nocontents, initialSnapshots: nosnapshots, expectedSnapshots: nosnapshots, @@ -130,8 +130,8 @@ func TestDeleteSync(t *testing.T) { }, { 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", classEmpty, "sid2-1", "vuid2-1", "volume2-1", "", "snap2-1", &deletePolicy, nil, nil, true), - expectedContents: newContentArray("content2-1", classEmpty, "sid2-1", "vuid2-1", "volume2-1", "", "snap2-1", &deletePolicy, nil, nil, true), + initialContents: newContentArray("content2-1", classEmpty, "sid2-1", "vuid2-1", "volume2-1", "", "snap2-1", &deletePolicy, nil, nil, true, nil), + expectedContents: newContentArray("content2-1", classEmpty, "sid2-1", "vuid2-1", "volume2-1", "", "snap2-1", &deletePolicy, nil, nil, true, nil), initialSnapshots: nosnapshots, expectedSnapshots: nosnapshots, expectedEvents: noevents, @@ -141,7 +141,7 @@ func TestDeleteSync(t *testing.T) { }, { name: "1-2 - successful delete with snapshot class that has empty secret parameter", - initialContents: newContentArray("content1-2", emptySecretClass, "sid1-2", "vuid1-2", "volume1-2", "snapuid1-2", "snap1-2", &deletePolicy, nil, nil, true), + initialContents: newContentArray("content1-2", emptySecretClass, "sid1-2", "vuid1-2", "volume1-2", "snapuid1-2", "snap1-2", &deletePolicy, nil, nil, true, emptyDataSecretAnnotations()), expectedContents: nocontents, initialSnapshots: nosnapshots, expectedSnapshots: nosnapshots, @@ -153,7 +153,7 @@ func TestDeleteSync(t *testing.T) { }, { name: "1-3 - successful delete with snapshot class that has valid secret parameter", - initialContents: newContentArray("content1-3", validSecretClass, "sid1-3", "vuid1-3", "volume1-3", "snapuid1-3", "snap1-3", &deletePolicy, nil, nil, true), + initialContents: newContentArray("content1-3", validSecretClass, "sid1-3", "vuid1-3", "volume1-3", "snapuid1-3", "snap1-3", &deletePolicy, nil, nil, true, secretAnnotations()), expectedContents: nocontents, initialSnapshots: nosnapshots, expectedSnapshots: nosnapshots, @@ -165,8 +165,8 @@ func TestDeleteSync(t *testing.T) { }, { name: "1-4 - fail delete with snapshot class that has invalid secret parameter", - initialContents: newContentArray("content1-4", invalidSecretClass, "sid1-4", "vuid1-4", "volume1-4", "snapuid1-4", "snap1-4", &deletePolicy, nil, nil, true), - expectedContents: newContentArray("content1-4", invalidSecretClass, "sid1-4", "vuid1-4", "volume1-4", "snapuid1-4", "snap1-4", &deletePolicy, nil, nil, true), + initialContents: newContentArray("content1-4", invalidSecretClass, "sid1-4", "vuid1-4", "volume1-4", "snapuid1-4", "snap1-4", &deletePolicy, nil, nil, true, emptyNamespaceSecretAnnotations()), + expectedContents: newContentArray("content1-4", invalidSecretClass, "sid1-4", "vuid1-4", "volume1-4", "snapuid1-4", "snap1-4", &deletePolicy, nil, nil, true, emptyNamespaceSecretAnnotations()), initialSnapshots: nosnapshots, expectedSnapshots: nosnapshots, expectedEvents: noevents, @@ -175,8 +175,8 @@ func TestDeleteSync(t *testing.T) { }, { name: "1-5 - csi driver delete snapshot returns error", - initialContents: newContentArray("content1-5", validSecretClass, "sid1-5", "vuid1-5", "volume1-5", "snapuid1-5", "snap1-5", &deletePolicy, nil, nil, true), - expectedContents: newContentArray("content1-5", validSecretClass, "sid1-5", "vuid1-5", "volume1-5", "snapuid1-5", "snap1-5", &deletePolicy, nil, nil, true), + initialContents: newContentArray("content1-5", validSecretClass, "sid1-5", "vuid1-5", "volume1-5", "snapuid1-5", "snap1-5", &deletePolicy, nil, nil, true, secretAnnotations()), + expectedContents: newContentArray("content1-5", validSecretClass, "sid1-5", "vuid1-5", "volume1-5", "snapuid1-5", "snap1-5", &deletePolicy, nil, nil, true, secretAnnotations()), initialSnapshots: nosnapshots, expectedSnapshots: nosnapshots, initialSecrets: []*v1.Secret{secret()}, @@ -187,8 +187,8 @@ func TestDeleteSync(t *testing.T) { }, { name: "1-6 - api server delete content returns error", - initialContents: newContentArray("content1-6", validSecretClass, "sid1-6", "vuid1-6", "volume1-6", "snapuid1-6", "snap1-6", &deletePolicy, nil, nil, true), - expectedContents: newContentArray("content1-6", validSecretClass, "sid1-6", "vuid1-6", "volume1-6", "snapuid1-6", "snap1-6", &deletePolicy, nil, nil, true), + initialContents: newContentArray("content1-6", validSecretClass, "sid1-6", "vuid1-6", "volume1-6", "snapuid1-6", "snap1-6", &deletePolicy, nil, nil, true, secretAnnotations()), + expectedContents: newContentArray("content1-6", validSecretClass, "sid1-6", "vuid1-6", "volume1-6", "snapuid1-6", "snap1-6", &deletePolicy, nil, nil, true, secretAnnotations()), initialSnapshots: nosnapshots, expectedSnapshots: nosnapshots, initialSecrets: []*v1.Secret{secret()}, @@ -205,7 +205,7 @@ func TestDeleteSync(t *testing.T) { // 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", validSecretClass, "sid1-7", "vuid1-7", "volume1-7", "snapuid1-7", "snap1-7", &deletePolicy, nil, nil, true), + initialContents: newContentArray("content1-7", validSecretClass, "sid1-7", "vuid1-7", "volume1-7", "snapuid1-7", "snap1-7", &deletePolicy, nil, nil, true, secretAnnotations()), expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap1-7", validSecretClass, "content1-7", "snapuid1-7-x", "claim1-7", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap1-7", validSecretClass, "content1-7", "snapuid1-7-x", "claim1-7", false, nil, nil, nil), @@ -218,7 +218,7 @@ func TestDeleteSync(t *testing.T) { { // delete success(?) - content is deleted before doDelete() starts name: "1-8 - content is deleted before deleting", - initialContents: newContentArray("content1-8", validSecretClass, "sid1-8", "vuid1-8", "volume1-8", "snapuid1-8", "snap1-8", &deletePolicy, nil, nil, true), + initialContents: newContentArray("content1-8", validSecretClass, "sid1-8", "vuid1-8", "volume1-8", "snapuid1-8", "snap1-8", &deletePolicy, nil, nil, true, secretAnnotations()), expectedContents: nocontents, initialSnapshots: nosnapshots, expectedSnapshots: nosnapshots, @@ -235,8 +235,8 @@ func TestDeleteSync(t *testing.T) { }, { name: "1-9 - content will not be deleted if it is bound to a snapshot correctly, snapshot uid is specified", - initialContents: newContentArray("content1-9", validSecretClass, "sid1-9", "vuid1-9", "volume1-9", "snapuid1-9", "snap1-9", &deletePolicy, nil, nil, true), - expectedContents: newContentArray("content1-9", validSecretClass, "sid1-9", "vuid1-9", "volume1-9", "snapuid1-9", "snap1-9", &deletePolicy, nil, nil, true), + initialContents: newContentArray("content1-9", validSecretClass, "sid1-9", "vuid1-9", "volume1-9", "snapuid1-9", "snap1-9", &deletePolicy, nil, nil, true, nil), + expectedContents: newContentArray("content1-9", validSecretClass, "sid1-9", "vuid1-9", "volume1-9", "snapuid1-9", "snap1-9", &deletePolicy, nil, nil, true, nil), initialSnapshots: newSnapshotArray("snap1-9", validSecretClass, "content1-9", "snapuid1-9", "claim1-9", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap1-9", validSecretClass, "content1-9", "snapuid1-9", "claim1-9", false, nil, nil, nil), expectedEvents: noevents, @@ -246,7 +246,7 @@ func TestDeleteSync(t *testing.T) { }, { name: "1-10 - should delete content which is bound to a snapshot incorrectly", - initialContents: newContentArray("content1-10", validSecretClass, "sid1-10", "vuid1-10", "volume1-10", "snapuid1-10-x", "snap1-10", &deletePolicy, nil, nil, true), + initialContents: newContentArray("content1-10", validSecretClass, "sid1-10", "vuid1-10", "volume1-10", "snapuid1-10-x", "snap1-10", &deletePolicy, nil, nil, true, secretAnnotations()), expectedContents: nocontents, initialSnapshots: newSnapshotArray("snap1-10", validSecretClass, "content1-10", "snapuid1-10", "claim1-10", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap1-10", validSecretClass, "content1-10", "snapuid1-10", "claim1-10", false, nil, nil, nil), @@ -258,8 +258,8 @@ func TestDeleteSync(t *testing.T) { }, { name: "1-10 - will not delete content with retain policy set which is bound to a snapshot incorrectly", - initialContents: newContentArray("content1-10", validSecretClass, "sid1-10", "vuid1-10", "volume1-10", "snapuid1-10-x", "snap1-10", &retainPolicy, nil, nil, true), - expectedContents: newContentArray("content1-10", validSecretClass, "sid1-10", "vuid1-10", "volume1-10", "snapuid1-10-x", "snap1-10", &retainPolicy, nil, nil, true), + initialContents: newContentArray("content1-10", validSecretClass, "sid1-10", "vuid1-10", "volume1-10", "snapuid1-10-x", "snap1-10", &retainPolicy, nil, nil, true, nil), + expectedContents: newContentArray("content1-10", validSecretClass, "sid1-10", "vuid1-10", "volume1-10", "snapuid1-10-x", "snap1-10", &retainPolicy, nil, nil, true, nil), initialSnapshots: newSnapshotArray("snap1-10", validSecretClass, "content1-10", "snapuid1-10", "claim1-10", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap1-10", validSecretClass, "content1-10", "snapuid1-10", "claim1-10", false, nil, nil, nil), expectedEvents: noevents, @@ -269,8 +269,8 @@ func TestDeleteSync(t *testing.T) { }, { 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", validSecretClass, "sid1-11", "vuid1-11", "volume1-11", "", "snap1-11", &deletePolicy, nil, nil, true), - expectedContents: newContentArray("content1-11", validSecretClass, "sid1-11", "vuid1-11", "volume1-11", "", "snap1-11", &deletePolicy, nil, nil, true), + initialContents: newContentArray("content1-11", validSecretClass, "sid1-11", "vuid1-11", "volume1-11", "", "snap1-11", &deletePolicy, nil, nil, true, nil), + expectedContents: newContentArray("content1-11", validSecretClass, "sid1-11", "vuid1-11", "volume1-11", "", "snap1-11", &deletePolicy, nil, nil, true, nil), initialSnapshots: newSnapshotArray("snap1-11", validSecretClass, "content1-11", "snapuid1-11", "claim1-11", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap1-11", validSecretClass, "content1-11", "snapuid1-11", "claim1-11", false, nil, nil, nil), expectedEvents: noevents, @@ -280,8 +280,8 @@ func TestDeleteSync(t *testing.T) { }, { 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", classEmpty, "sid1-12", "vuid1-12", "volume1-12", "snapuid1-12", "snap1-12", &retainPolicy, nil, nil, true), - expectedContents: newContentArray("content1-12", classEmpty, "sid1-12", "vuid1-12", "volume1-12", "snapuid1-12", "snap1-12", &retainPolicy, nil, nil, true), + initialContents: newContentArray("content1-12", classEmpty, "sid1-12", "vuid1-12", "volume1-12", "snapuid1-12", "snap1-12", &retainPolicy, nil, nil, true, nil), + expectedContents: newContentArray("content1-12", classEmpty, "sid1-12", "vuid1-12", "volume1-12", "snapuid1-12", "snap1-12", &retainPolicy, nil, nil, true, nil), initialSnapshots: nosnapshots, expectedSnapshots: nosnapshots, expectedEvents: noevents, @@ -290,8 +290,8 @@ func TestDeleteSync(t *testing.T) { }, { 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-1", classEmpty, "sid1-1", "vuid1-1", "volume1-1", "snapuid1-1", "snap1-1", nil, nil, nil, true), - expectedContents: newContentArray("content1-1", classEmpty, "sid1-1", "vuid1-1", "volume1-1", "snapuid1-1", "snap1-1", nil, nil, nil, true), + initialContents: newContentArray("content1-1", classEmpty, "sid1-1", "vuid1-1", "volume1-1", "snapuid1-1", "snap1-1", nil, nil, nil, true, nil), + expectedContents: newContentArray("content1-1", classEmpty, "sid1-1", "vuid1-1", "volume1-1", "snapuid1-1", "snap1-1", nil, nil, nil, true, nil), initialSnapshots: nosnapshots, expectedSnapshots: nosnapshots, expectedEvents: noevents, @@ -300,8 +300,8 @@ func TestDeleteSync(t *testing.T) { }, { name: "1-14 - content will not be deleted if it is bound to a snapshot correctly, snapshot uid is specified", - initialContents: newContentArray("content1-14", validSecretClass, "sid1-14", "vuid1-14", "volume1-14", "snapuid1-14", "snap1-14", &retainPolicy, nil, nil, true), - expectedContents: newContentArray("content1-14", validSecretClass, "sid1-14", "vuid1-14", "volume1-14", "snapuid1-14", "snap1-14", &retainPolicy, nil, nil, true), + initialContents: newContentArray("content1-14", validSecretClass, "sid1-14", "vuid1-14", "volume1-14", "snapuid1-14", "snap1-14", &retainPolicy, nil, nil, true, nil), + expectedContents: newContentArray("content1-14", validSecretClass, "sid1-14", "vuid1-14", "volume1-14", "snapuid1-14", "snap1-14", &retainPolicy, nil, nil, true, nil), initialSnapshots: newSnapshotArray("snap1-14", validSecretClass, "content1-14", "snapuid1-14", "claim1-14", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap1-14", validSecretClass, "content1-14", "snapuid1-14", "claim1-14", false, nil, nil, nil), expectedEvents: noevents, @@ -311,8 +311,8 @@ func TestDeleteSync(t *testing.T) { }, { name: "1-15 - content will not be deleted which is bound to a snapshot incorrectly if Deletion policy is not set", - initialContents: newContentArray("content1-10", validSecretClass, "sid1-15", "vuid1-15", "volume1-15", "snapuid1-15-x", "snap1-15", nil, nil, nil, true), - expectedContents: newContentArray("content1-10", validSecretClass, "sid1-15", "vuid1-15", "volume1-15", "snapuid1-15-x", "snap1-15", nil, nil, nil, true), + initialContents: newContentArray("content1-10", validSecretClass, "sid1-15", "vuid1-15", "volume1-15", "snapuid1-15-x", "snap1-15", nil, nil, nil, true, nil), + expectedContents: newContentArray("content1-10", validSecretClass, "sid1-15", "vuid1-15", "volume1-15", "snapuid1-15-x", "snap1-15", nil, nil, nil, true, nil), initialSnapshots: newSnapshotArray("snap1-10", validSecretClass, "content1-15", "snapuid1-15", "claim1-15", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap1-10", validSecretClass, "content1-15", "snapuid1-15", "claim1-15", false, nil, nil, nil), expectedEvents: noevents, @@ -320,6 +320,18 @@ func TestDeleteSync(t *testing.T) { errors: noerrors, test: testSyncContent, }, + { + name: "1-16 - continue delete with snapshot class that has nonexistent secret", + initialContents: newContentArray("content1-16", validSecretClass, "sid1-16", "vuid1-16", "volume1-16", "snapuid1-16", "snap1-16", &deletePolicy, nil, nil, true, secretAnnotations()), + expectedContents: nocontents, + initialSnapshots: nosnapshots, + expectedSnapshots: nosnapshots, + expectedEvents: noevents, + errors: noerrors, + initialSecrets: []*v1.Secret{}, // secret does not exist + expectedDeleteCalls: []deleteCall{{"sid1-16", nil, nil}}, + test: testSyncContent, + }, } runSyncTests(t, tests, snapshotClasses) } diff --git a/pkg/controller/snapshot_ready_test.go b/pkg/controller/snapshot_ready_test.go index 1ccb6bd90..29402d57f 100644 --- a/pkg/controller/snapshot_ready_test.go +++ b/pkg/controller/snapshot_ready_test.go @@ -55,8 +55,8 @@ func TestSync(t *testing.T) { }, { name: "2-2 - could not bind snapshot and content, the VolumeSnapshotRef does not match", - initialContents: newContentArray("content2-2", validSecretClass, "sid2-2", "vuid2-2", "volume2-2", "snapuid2-2-x", "snap2-2", &deletePolicy, nil, nil, false), - expectedContents: newContentArray("content2-2", validSecretClass, "sid2-2", "vuid2-2", "volume2-2", "snapuid2-2-x", "snap2-2", &deletePolicy, nil, nil, false), + initialContents: newContentArray("content2-2", validSecretClass, "sid2-2", "vuid2-2", "volume2-2", "snapuid2-2-x", "snap2-2", &deletePolicy, nil, nil, false, nil), + expectedContents: newContentArray("content2-2", validSecretClass, "sid2-2", "vuid2-2", "volume2-2", "snapuid2-2-x", "snap2-2", &deletePolicy, nil, nil, false, nil), initialSnapshots: newSnapshotArray("snap2-2", validSecretClass, "content2-2", "snapuid2-2", "claim2-2", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap2-2", validSecretClass, "content2-2", "snapuid2-2", "claim2-2", false, newVolumeError("Snapshot failed to bind VolumeSnapshotContent, Could not bind snapshot snap2-2 and content content2-2, the VolumeSnapshotRef does not match"), nil, nil), expectedEvents: []string{"Warning SnapshotBindFailed"}, @@ -65,8 +65,8 @@ func TestSync(t *testing.T) { }, { name: "2-3 - success bind snapshot and content but not ready, no status changed", - initialContents: newContentArray("content2-3", validSecretClass, "sid2-3", "vuid2-3", "volume2-3", "", "snap2-3", &deletePolicy, nil, nil, false), - expectedContents: newContentArray("content2-3", validSecretClass, "sid2-3", "vuid2-3", "volume2-3", "snapuid2-3", "snap2-3", &deletePolicy, nil, nil, false), + initialContents: newContentArray("content2-3", validSecretClass, "sid2-3", "vuid2-3", "volume2-3", "", "snap2-3", &deletePolicy, nil, nil, false, nil), + expectedContents: newContentArray("content2-3", validSecretClass, "sid2-3", "vuid2-3", "volume2-3", "snapuid2-3", "snap2-3", &deletePolicy, nil, nil, false, nil), initialSnapshots: newSnapshotArray("snap2-3", validSecretClass, "content2-3", "snapuid2-3", "claim2-3", false, nil, metaTimeNow, nil), expectedSnapshots: newSnapshotArray("snap2-3", validSecretClass, "content2-3", "snapuid2-3", "claim2-3", false, nil, metaTimeNow, nil), initialClaims: newClaimArray("claim2-3", "pvc-uid2-3", "1Gi", "volume2-3", v1.ClaimBound, &classEmpty), @@ -91,8 +91,8 @@ func TestSync(t *testing.T) { { // nothing changed name: "2-4 - noop", - initialContents: newContentArray("content2-4", validSecretClass, "sid2-4", "vuid2-4", "volume2-4", "snapuid2-4", "snap2-4", &deletePolicy, nil, nil, false), - expectedContents: newContentArray("content2-4", validSecretClass, "sid2-4", "vuid2-4", "volume2-4", "snapuid2-4", "snap2-4", &deletePolicy, nil, nil, false), + initialContents: newContentArray("content2-4", validSecretClass, "sid2-4", "vuid2-4", "volume2-4", "snapuid2-4", "snap2-4", &deletePolicy, nil, nil, false, nil), + expectedContents: newContentArray("content2-4", validSecretClass, "sid2-4", "vuid2-4", "volume2-4", "snapuid2-4", "snap2-4", &deletePolicy, nil, nil, false, nil), initialSnapshots: newSnapshotArray("snap2-4", validSecretClass, "content2-4", "snapuid2-4", "claim2-4", true, nil, metaTimeNow, nil), expectedSnapshots: newSnapshotArray("snap2-4", validSecretClass, "content2-4", "snapuid2-4", "claim2-4", true, nil, metaTimeNow, nil), errors: noerrors, @@ -100,8 +100,8 @@ func TestSync(t *testing.T) { }, { name: "2-5 - snapshot and content bound, status ready false -> true", - initialContents: newContentArray("content2-5", validSecretClass, "sid2-5", "vuid2-5", "volume2-5", "snapuid2-5", "snap2-5", &deletePolicy, nil, nil, false), - expectedContents: newContentArray("content2-5", validSecretClass, "sid2-5", "vuid2-5", "volume2-5", "snapuid2-5", "snap2-5", &deletePolicy, nil, nil, false), + initialContents: newContentArray("content2-5", validSecretClass, "sid2-5", "vuid2-5", "volume2-5", "snapuid2-5", "snap2-5", &deletePolicy, nil, nil, false, nil), + expectedContents: newContentArray("content2-5", validSecretClass, "sid2-5", "vuid2-5", "volume2-5", "snapuid2-5", "snap2-5", &deletePolicy, nil, nil, false, nil), initialSnapshots: newSnapshotArray("snap2-5", validSecretClass, "content2-5", "snapuid2-5", "claim2-5", false, nil, metaTimeNow, nil), expectedSnapshots: newSnapshotArray("snap2-5", validSecretClass, "content2-5", "snapuid2-5", "claim2-5", true, nil, metaTimeNow, nil), initialClaims: newClaimArray("claim2-5", "pvc-uid2-5", "1Gi", "volume2-5", v1.ClaimBound, &classEmpty), @@ -125,8 +125,8 @@ func TestSync(t *testing.T) { }, { name: "2-6 - snapshot bound to prebound content correctly, status ready false -> true, ref.UID '' -> 'snapuid2-6'", - initialContents: newContentArray("content2-6", validSecretClass, "sid2-6", noVolume, noVolume, noBoundUID, "snap2-6", &deletePolicy, nil, &timeNowStamp, false), - expectedContents: newContentArray("content2-6", validSecretClass, "sid2-6", noVolume, noVolume, "snapuid2-6", "snap2-6", &deletePolicy, nil, &timeNowStamp, false), + initialContents: newContentArray("content2-6", validSecretClass, "sid2-6", noVolume, noVolume, noBoundUID, "snap2-6", &deletePolicy, nil, &timeNowStamp, false, nil), + expectedContents: newContentArray("content2-6", validSecretClass, "sid2-6", noVolume, noVolume, "snapuid2-6", "snap2-6", &deletePolicy, nil, &timeNowStamp, false, nil), initialSnapshots: newSnapshotArray("snap2-6", validSecretClass, "content2-6", "snapuid2-6", noClaim, false, nil, metaTimeNow, nil), expectedSnapshots: newSnapshotArray("snap2-6", validSecretClass, "content2-6", "snapuid2-6", noClaim, true, nil, metaTimeNow, nil), expectedListCalls: []listCall{ @@ -140,8 +140,8 @@ func TestSync(t *testing.T) { }, { name: "2-7 - snapshot and content bound, csi driver get status error", - initialContents: newContentArray("content2-7", validSecretClass, "sid2-7", "vuid2-7", "volume2-7", "snapuid2-7", "snap2-7", &deletePolicy, nil, nil, false), - expectedContents: newContentArray("content2-7", validSecretClass, "sid2-7", "vuid2-7", "volume2-7", "snapuid2-7", "snap2-7", &deletePolicy, nil, nil, false), + initialContents: newContentArray("content2-7", validSecretClass, "sid2-7", "vuid2-7", "volume2-7", "snapuid2-7", "snap2-7", &deletePolicy, nil, nil, false, nil), + expectedContents: newContentArray("content2-7", validSecretClass, "sid2-7", "vuid2-7", "volume2-7", "snapuid2-7", "snap2-7", &deletePolicy, nil, nil, false, nil), initialSnapshots: newSnapshotArray("snap2-7", validSecretClass, "content2-7", "snapuid2-7", "claim2-7", false, nil, metaTimeNow, nil), expectedSnapshots: newSnapshotArray("snap2-7", validSecretClass, "content2-7", "snapuid2-7", "claim2-7", false, newVolumeError("Failed to check and update snapshot: mock create snapshot error"), metaTimeNow, nil), expectedEvents: []string{"Warning SnapshotCheckandUpdateFailed"}, @@ -163,8 +163,8 @@ func TestSync(t *testing.T) { }, { name: "2-8 - snapshot and content bound, apiserver update status error", - initialContents: newContentArray("content2-8", validSecretClass, "sid2-8", "vuid2-8", "volume2-8", "snapuid2-8", "snap2-8", &deletePolicy, nil, nil, false), - expectedContents: newContentArray("content2-8", validSecretClass, "sid2-8", "vuid2-8", "volume2-8", "snapuid2-8", "snap2-8", &deletePolicy, nil, nil, false), + initialContents: newContentArray("content2-8", validSecretClass, "sid2-8", "vuid2-8", "volume2-8", "snapuid2-8", "snap2-8", &deletePolicy, nil, nil, false, nil), + expectedContents: newContentArray("content2-8", validSecretClass, "sid2-8", "vuid2-8", "volume2-8", "snapuid2-8", "snap2-8", &deletePolicy, nil, nil, false, nil), initialSnapshots: newSnapshotArray("snap2-8", validSecretClass, "content2-8", "snapuid2-8", "claim2-8", false, nil, metaTimeNow, nil), expectedSnapshots: newSnapshotArray("snap2-8", validSecretClass, "content2-8", "snapuid2-8", "claim2-8", false, newVolumeError("Failed to check and update snapshot: snapshot controller failed to update default/snap2-8 on API server: mock update error"), metaTimeNow, nil), expectedEvents: []string{"Warning SnapshotCheckandUpdateFailed"}, @@ -194,8 +194,8 @@ func TestSync(t *testing.T) { }, { name: "2-9 - bind when snapshot and content matches", - initialContents: newContentArray("content2-9", validSecretClass, "sid2-9", "vuid2-9", "volume2-9", "snapuid2-9", "snap2-9", &deletePolicy, nil, nil, false), - expectedContents: newContentArray("content2-9", validSecretClass, "sid2-9", "vuid2-9", "volume2-9", "snapuid2-9", "snap2-9", &deletePolicy, nil, nil, false), + initialContents: newContentArray("content2-9", validSecretClass, "sid2-9", "vuid2-9", "volume2-9", "snapuid2-9", "snap2-9", &deletePolicy, nil, nil, false, nil), + expectedContents: newContentArray("content2-9", validSecretClass, "sid2-9", "vuid2-9", "volume2-9", "snapuid2-9", "snap2-9", &deletePolicy, nil, nil, false, nil), initialSnapshots: newSnapshotArray("snap2-9", validSecretClass, "", "snapuid2-9", "claim2-9", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap2-9", validSecretClass, "content2-9", "snapuid2-9", "claim2-9", false, nil, nil, nil), errors: noerrors, @@ -203,8 +203,8 @@ func TestSync(t *testing.T) { }, { name: "2-10 - do not bind when snapshot and content not match", - initialContents: newContentArray("content2-10", validSecretClass, "sid2-10", "vuid2-10", "volume2-10", "snapuid2-10-x", "snap2-10", &deletePolicy, nil, nil, false), - expectedContents: newContentArray("content2-10", validSecretClass, "sid2-10", "vuid2-10", "volume2-10", "snapuid2-10-x", "snap2-10", &deletePolicy, nil, nil, false), + initialContents: newContentArray("content2-10", validSecretClass, "sid2-10", "vuid2-10", "volume2-10", "snapuid2-10-x", "snap2-10", &deletePolicy, nil, nil, false, nil), + expectedContents: newContentArray("content2-10", validSecretClass, "sid2-10", "vuid2-10", "volume2-10", "snapuid2-10-x", "snap2-10", &deletePolicy, nil, nil, false, nil), initialSnapshots: newSnapshotArray("snap2-10", validSecretClass, "", "snapuid2-10", "claim2-10", false, newVolumeError("mock driver error"), nil, nil), expectedSnapshots: newSnapshotArray("snap2-10", validSecretClass, "", "snapuid2-10", "claim2-10", false, newVolumeError("mock driver error"), nil, nil), errors: noerrors, @@ -232,8 +232,8 @@ func TestSync(t *testing.T) { }, { name: "3-3 - ready snapshot(everything is well, do nothing)", - initialContents: newContentArray("content3-3", validSecretClass, "sid3-3", "vuid3-3", "volume3-3", "snapuid3-3", "snap3-3", &deletePolicy, nil, nil, false), - expectedContents: newContentArray("content3-3", validSecretClass, "sid3-3", "vuid3-3", "volume3-3", "snapuid3-3", "snap3-3", &deletePolicy, nil, nil, false), + initialContents: newContentArray("content3-3", validSecretClass, "sid3-3", "vuid3-3", "volume3-3", "snapuid3-3", "snap3-3", &deletePolicy, nil, nil, false, nil), + expectedContents: newContentArray("content3-3", validSecretClass, "sid3-3", "vuid3-3", "volume3-3", "snapuid3-3", "snap3-3", &deletePolicy, nil, nil, false, nil), initialSnapshots: newSnapshotArray("snap3-3", validSecretClass, "content3-3", "snapuid3-3", "claim3-3", true, nil, metaTimeNow, nil), expectedSnapshots: newSnapshotArray("snap3-3", validSecretClass, "content3-3", "snapuid3-3", "claim3-3", true, nil, metaTimeNow, nil), errors: noerrors, @@ -241,8 +241,8 @@ func TestSync(t *testing.T) { }, { name: "3-4 - ready snapshot misbound to VolumeSnapshotContent", - initialContents: newContentArray("content3-4", validSecretClass, "sid3-4", "vuid3-4", "volume3-4", "snapuid3-4-x", "snap3-4", &deletePolicy, nil, nil, false), - expectedContents: newContentArray("content3-4", validSecretClass, "sid3-4", "vuid3-4", "volume3-4", "snapuid3-4-x", "snap3-4", &deletePolicy, nil, nil, false), + initialContents: newContentArray("content3-4", validSecretClass, "sid3-4", "vuid3-4", "volume3-4", "snapuid3-4-x", "snap3-4", &deletePolicy, nil, nil, false, nil), + expectedContents: newContentArray("content3-4", validSecretClass, "sid3-4", "vuid3-4", "volume3-4", "snapuid3-4-x", "snap3-4", &deletePolicy, nil, nil, false, nil), initialSnapshots: newSnapshotArray("snap3-4", validSecretClass, "content3-4", "snapuid3-4", "claim3-4", true, nil, metaTimeNow, nil), expectedSnapshots: newSnapshotArray("snap3-4", validSecretClass, "content3-4", "snapuid3-4", "claim3-4", false, newVolumeError("VolumeSnapshotContent is not bound to the VolumeSnapshot correctly"), metaTimeNow, nil), errors: noerrors, @@ -260,8 +260,8 @@ func TestSync(t *testing.T) { }, { name: "3-6 - snapshot bound to content in which the snapshot uid does not match", - initialContents: newContentArray("content3-4", validSecretClass, "sid3-4", "vuid3-4", "volume3-4", "snapuid3-4-x", "snap3-6", &deletePolicy, nil, nil, false), - expectedContents: newContentArray("content3-4", validSecretClass, "sid3-4", "vuid3-4", "volume3-4", "snapuid3-4-x", "snap3-6", &deletePolicy, nil, nil, false), + initialContents: newContentArray("content3-4", validSecretClass, "sid3-4", "vuid3-4", "volume3-4", "snapuid3-4-x", "snap3-6", &deletePolicy, nil, nil, false, nil), + expectedContents: newContentArray("content3-4", validSecretClass, "sid3-4", "vuid3-4", "volume3-4", "snapuid3-4-x", "snap3-6", &deletePolicy, nil, nil, false, nil), initialSnapshots: newSnapshotArray("snap3-5", validSecretClass, "content3-5", "snapuid3-5", "claim3-5", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap3-5", validSecretClass, "content3-5", "snapuid3-5", "claim3-5", false, newVolumeError("VolumeSnapshotContent is missing"), nil, nil), expectedEvents: []string{"Warning SnapshotContentMissing"}, diff --git a/pkg/controller/util.go b/pkg/controller/util.go index b91f32591..bd1945d29 100644 --- a/pkg/controller/util.go +++ b/pkg/controller/util.go @@ -65,6 +65,11 @@ const ( // Name of finalizer on VolumeSnapshotContents that are bound by VolumeSnapshots VolumeSnapshotContentFinalizer = "snapshot.storage.kubernetes.io/volumesnapshotcontent-protection" VolumeSnapshotFinalizer = "snapshot.storage.kubernetes.io/volumesnapshot-protection" + + // Annotation for secret name and namespace will be added to the content + // and used at snapshot content deletion time. + AnnDeletionSecretRefName = "snapshot.storage.kubernetes.io/deletion-secret-name" + AnnDeletionSecretRefNamespace = "snapshot.storage.kubernetes.io/deletion-secret-namespace" ) var snapshotterSecretParams = deprecatedSecretParamsMap{ @@ -217,7 +222,6 @@ func verifyAndGetSecretNameAndNamespaceTemplate(secret deprecatedSecretParamsMap // - ${volumesnapshotcontent.name} // - ${volumesnapshot.namespace} // - ${volumesnapshot.name} -// - ${volumesnapshot.annotations['ANNOTATION_KEY']} (e.g. ${pvc.annotations['example.com/snapshot-create-secret-name']}) // // supported tokens for namespace resolution: // - ${volumesnapshotcontent.name} @@ -262,16 +266,12 @@ func getSecretReference(snapshotClassParams map[string]string, snapContentName s } ref.Namespace = resolvedNamespace - // Secret name template can make use of the VolumeSnapshotContent name, VolumeSnapshot name or namespace, - // or a VolumeSnapshot annotation. + // Secret name template can make use of the VolumeSnapshotContent name, VolumeSnapshot name or namespace. // Note that VolumeSnapshot name and annotations are under the VolumeSnapshot user's control. nameParams := map[string]string{"volumesnapshotcontent.name": snapContentName} if snapshot != nil { nameParams["volumesnapshot.name"] = snapshot.Name nameParams["volumesnapshot.namespace"] = snapshot.Namespace - for k, v := range snapshot.Annotations { - nameParams["volumesnapshot.annotations['"+k+"']"] = v - } } resolvedName, err := resolveTemplate(nameTemplate, nameParams) if err != nil { diff --git a/pkg/controller/util_test.go b/pkg/controller/util_test.go index 4da945761..790a275b4 100644 --- a/pkg/controller/util_test.go +++ b/pkg/controller/util_test.go @@ -70,7 +70,7 @@ func TestGetSecretReference(t *testing.T) { expectRef: nil, expectErr: true, }, - "template - valid": { + "template - invalid": { params: map[string]string{ prefixedSnapshotterSecretNameKey: "static-${volumesnapshotcontent.name}-${volumesnapshot.namespace}-${volumesnapshot.name}-${volumesnapshot.annotations['akey']}", prefixedSnapshotterSecretNamespaceKey: "static-${volumesnapshotcontent.name}-${volumesnapshot.namespace}", @@ -83,7 +83,8 @@ func TestGetSecretReference(t *testing.T) { Annotations: map[string]string{"akey": "avalue"}, }, }, - expectRef: &v1.SecretReference{Name: "static-snapcontentname-snapshotnamespace-snapshotname-avalue", Namespace: "static-snapcontentname-snapshotnamespace"}, + expectRef: nil, + expectErr: true, }, "template - invalid namespace tokens": { params: map[string]string{