Skip to content

Commit e647a80

Browse files
xing-yangggriffiths
xing-yang
authored and
ggriffiths
committed
Add deletion secret as annotation to content
1 parent ffee5f4 commit e647a80

File tree

8 files changed

+165
-95
lines changed

8 files changed

+165
-95
lines changed

‎pkg/controller/framework_test.go‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -790,11 +790,12 @@ func newTestController(kubeClient kubernetes.Interface, clientset clientset.Inte
790790
}
791791

792792
// newContent returns a new content with given attributes
793-
func newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, deletionPolicy *crdv1.DeletionPolicy, size *int64, creationTime *int64, withFinalizer bool) *crdv1.VolumeSnapshotContent {
793+
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 {
794794
content := crdv1.VolumeSnapshotContent{
795795
ObjectMeta: metav1.ObjectMeta{
796796
Name: name,
797797
ResourceVersion: "1",
798+
Annotations: annotations,
798799
},
799800
Spec: crdv1.VolumeSnapshotContentSpec{
800801
VolumeSnapshotSource: crdv1.VolumeSnapshotSource{
@@ -833,14 +834,14 @@ func newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToS
833834
return &content
834835
}
835836

836-
func newContentArray(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, deletionPolicy *crdv1.DeletionPolicy, size *int64, creationTime *int64, withFinalizer bool) []*crdv1.VolumeSnapshotContent {
837+
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 {
837838
return []*crdv1.VolumeSnapshotContent{
838-
newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName, deletionPolicy, size, creationTime, withFinalizer),
839+
newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName, deletionPolicy, size, creationTime, withFinalizer, annotations),
839840
}
840841
}
841842

842843
func newContentWithUnmatchDriverArray(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, deletionPolicy *crdv1.DeletionPolicy, size *int64, creationTime *int64) []*crdv1.VolumeSnapshotContent {
843-
content := newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName, deletionPolicy, size, creationTime, false)
844+
content := newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName, deletionPolicy, size, creationTime, false, nil)
844845
content.Spec.VolumeSnapshotSource.CSI.Driver = "fake"
845846
return []*crdv1.VolumeSnapshotContent{
846847
content,
@@ -1287,6 +1288,28 @@ func secret() *v1.Secret {
12871288
}
12881289
}
12891290

1291+
func secretAnnotations() map[string]string {
1292+
return map[string]string{
1293+
AnnDeletionSecretRefName: "secret",
1294+
AnnDeletionSecretRefNamespace: "default",
1295+
}
1296+
}
1297+
1298+
func emptyNamespaceSecretAnnotations() map[string]string {
1299+
return map[string]string{
1300+
AnnDeletionSecretRefName: "name",
1301+
AnnDeletionSecretRefNamespace: "",
1302+
}
1303+
}
1304+
1305+
// this refers to emptySecret(), which is missing data.
1306+
func emptyDataSecretAnnotations() map[string]string {
1307+
return map[string]string{
1308+
AnnDeletionSecretRefName: "emptysecret",
1309+
AnnDeletionSecretRefNamespace: "default",
1310+
}
1311+
}
1312+
12901313
type listCall struct {
12911314
snapshotID string
12921315
// information to return
@@ -1368,7 +1391,7 @@ func (f *fakeSnapshotter) CreateSnapshot(ctx context.Context, snapshotName strin
13681391
func (f *fakeSnapshotter) DeleteSnapshot(ctx context.Context, snapshotID string, snapshotterCredentials map[string]string) error {
13691392
if f.deleteCallCounter >= len(f.deleteCalls) {
13701393
f.t.Errorf("Unexpected CSI Delete Snapshot call: snapshotID=%s, index: %d, calls: %+v", snapshotID, f.createCallCounter, f.createCalls)
1371-
return fmt.Errorf("unexpected call")
1394+
return fmt.Errorf("unexpected DeleteSnapshot call")
13721395
}
13731396
call := f.deleteCalls[f.deleteCallCounter]
13741397
f.deleteCallCounter++

‎pkg/controller/snapshot_controller.go‎

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ func (ctrl *csiSnapshotController) checkandBindSnapshotContent(snapshot *crdv1.V
523523
return newContent, nil
524524
}
525525

526-
func (ctrl *csiSnapshotController) getCreateSnapshotInput(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotClass, *v1.PersistentVolume, string, map[string]string, error) {
526+
func (ctrl *csiSnapshotController) getCreateSnapshotInput(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshotClass, *v1.PersistentVolume, string, *v1.SecretReference, error) {
527527
className := snapshot.Spec.VolumeSnapshotClassName
528528
klog.V(5).Infof("getCreateSnapshotInput [%s]: VolumeSnapshotClassName [%s]", snapshot.Name, *className)
529529
var class *crdv1.VolumeSnapshotClass
@@ -553,12 +553,8 @@ func (ctrl *csiSnapshotController) getCreateSnapshotInput(snapshot *crdv1.Volume
553553
if err != nil {
554554
return nil, nil, "", nil, err
555555
}
556-
snapshotterCredentials, err := getCredentials(ctrl.client, snapshotterSecretRef)
557-
if err != nil {
558-
return nil, nil, "", nil, err
559-
}
560556

561-
return class, volume, contentName, snapshotterCredentials, nil
557+
return class, volume, contentName, snapshotterSecretRef, nil
562558
}
563559

564560
func (ctrl *csiSnapshotController) checkandUpdateBoundSnapshotStatusOperation(snapshot *crdv1.VolumeSnapshot, content *crdv1.VolumeSnapshotContent) (*crdv1.VolumeSnapshot, error) {
@@ -580,10 +576,14 @@ func (ctrl *csiSnapshotController) checkandUpdateBoundSnapshotStatusOperation(sn
580576
driverName, snapshotID = content.Spec.CSI.Driver, content.Spec.CSI.SnapshotHandle
581577
}
582578
} else {
583-
class, volume, _, snapshotterCredentials, err := ctrl.getCreateSnapshotInput(snapshot)
579+
class, volume, _, snapshotterSecretRef, err := ctrl.getCreateSnapshotInput(snapshot)
584580
if err != nil {
585581
return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err)
586582
}
583+
snapshotterCredentials, err := getCredentials(ctrl.client, snapshotterSecretRef)
584+
if err != nil {
585+
return nil, err
586+
}
587587
driverName, snapshotID, creationTime, size, readyToUse, err = ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials)
588588
if err != nil {
589589
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
627627
return nil, err
628628
}
629629

630-
class, volume, contentName, snapshotterCredentials, err := ctrl.getCreateSnapshotInput(snapshot)
630+
class, volume, contentName, snapshotterSecretRef, err := ctrl.getCreateSnapshotInput(snapshot)
631631
if err != nil {
632632
return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err)
633633
}
634634

635+
snapshotterCredentials, err := getCredentials(ctrl.client, snapshotterSecretRef)
636+
if err != nil {
637+
return nil, err
638+
}
639+
635640
driverName, snapshotID, creationTime, size, readyToUse, err := ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials)
636641
if err != nil {
637642
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
687692
DeletionPolicy: class.DeletionPolicy,
688693
},
689694
}
695+
696+
// Set AnnDeletionSecretRefName and AnnDeletionSecretRefNamespace
697+
if snapshotterSecretRef != nil {
698+
klog.V(5).Infof("createSnapshotOperation: set annotation [%s] on content [%s].", AnnDeletionSecretRefName, snapshotContent.Name)
699+
metav1.SetMetaDataAnnotation(&snapshotContent.ObjectMeta, AnnDeletionSecretRefName, snapshotterSecretRef.Name)
700+
701+
klog.V(5).Infof("syncContent: set annotation [%s] on content [%s].", AnnDeletionSecretRefNamespace, snapshotContent.Name)
702+
metav1.SetMetaDataAnnotation(&snapshotContent.ObjectMeta, AnnDeletionSecretRefNamespace, snapshotterSecretRef.Namespace)
703+
}
704+
690705
klog.V(3).Infof("volume snapshot content %v", snapshotContent)
691706
// Try to create the VolumeSnapshotContent object several times
692707
for i := 0; i < ctrl.createSnapshotContentRetryCount; i++ {
@@ -736,23 +751,30 @@ func (ctrl *csiSnapshotController) deleteSnapshotContentOperation(content *crdv1
736751

737752
// get secrets if VolumeSnapshotClass specifies it
738753
var snapshotterCredentials map[string]string
739-
snapshotClassName := content.Spec.VolumeSnapshotClassName
740-
if snapshotClassName != nil {
741-
if snapshotClass, err := ctrl.classLister.Get(*snapshotClassName); err == nil {
742-
// Resolve snapshotting secret credentials.
743-
// No VolumeSnapshot is provided when resolving delete secret names, since the VolumeSnapshot may or may not exist at delete time.
744-
snapshotterSecretRef, err := getSecretReference(snapshotClass.Parameters, content.Name, nil)
745-
if err != nil {
746-
return err
747-
}
748-
snapshotterCredentials, err = getCredentials(ctrl.client, snapshotterSecretRef)
749-
if err != nil {
750-
return err
751-
}
754+
var err error
755+
756+
// Check if annotation exists
757+
if metav1.HasAnnotation(content.ObjectMeta, AnnDeletionSecretRefName) && metav1.HasAnnotation(content.ObjectMeta, AnnDeletionSecretRefNamespace) {
758+
annDeletionSecretName := content.Annotations[AnnDeletionSecretRefName]
759+
annDeletionSecretNamespace := content.Annotations[AnnDeletionSecretRefNamespace]
760+
761+
snapshotterSecretRef := &v1.SecretReference{}
762+
763+
if annDeletionSecretName == "" || annDeletionSecretNamespace == "" {
764+
return fmt.Errorf("cannot delete snapshot %#v, err: secret name or namespace not specified", content.Name)
765+
}
766+
767+
snapshotterSecretRef.Name = annDeletionSecretName
768+
snapshotterSecretRef.Namespace = annDeletionSecretNamespace
769+
770+
snapshotterCredentials, err = getCredentials(ctrl.client, snapshotterSecretRef)
771+
if err != nil {
772+
// Continue with deletion, as the secret may have already been deleted.
773+
klog.Errorf("Failed to get credentials for snapshot %s: %s", content.Name, err.Error())
752774
}
753775
}
754776

755-
err := ctrl.handler.DeleteSnapshot(content, snapshotterCredentials)
777+
err = ctrl.handler.DeleteSnapshot(content, snapshotterCredentials)
756778
if err != nil {
757779
ctrl.eventRecorder.Event(content, v1.EventTypeWarning, "SnapshotDeleteError", "Failed to delete snapshot")
758780
return fmt.Errorf("failed to delete snapshot %#v, err: %v", content.Name, err)

‎pkg/controller/snapshot_controller_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
)
2424

2525
func storeVersion(t *testing.T, prefix string, c cache.Store, version string, expectedReturn bool) {
26-
content := newContent("contentName", classEmpty, "sid1-1", "vuid1-1", "volume1-1", "snapuid1-1", "snap1-1", nil, nil, nil, false)
26+
content := newContent("contentName", classEmpty, "sid1-1", "vuid1-1", "volume1-1", "snapuid1-1", "snap1-1", nil, nil, nil, false, nil)
2727
content.ResourceVersion = version
2828
ret, err := storeObjectUpdate(c, content, "content")
2929
if err != nil {
@@ -82,7 +82,7 @@ func TestControllerCacheParsingError(t *testing.T) {
8282
// There must be something in the cache to compare with
8383
storeVersion(t, "Step1", c, "1", true)
8484

85-
content := newContent("contentName", classEmpty, "sid1-1", "vuid1-1", "volume1-1", "snapuid1-1", "snap1-1", nil, nil, nil, false)
85+
content := newContent("contentName", classEmpty, "sid1-1", "vuid1-1", "volume1-1", "snapuid1-1", "snap1-1", nil, nil, nil, false, nil)
8686
content.ResourceVersion = "xxx"
8787
_, err := storeObjectUpdate(c, content, "content")
8888
if err == nil {

‎pkg/controller/snapshot_create_test.go‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func TestCreateSnapshotSync(t *testing.T) {
6969
{
7070
name: "6-1 - successful create snapshot with snapshot class gold",
7171
initialContents: nocontents,
72-
expectedContents: newContentArray("snapcontent-snapuid6-1", classGold, "sid6-1", "pv-uid6-1", "volume6-1", "snapuid6-1", "snap6-1", &deletePolicy, &defaultSize, &timeNowStamp, false),
72+
expectedContents: newContentArray("snapcontent-snapuid6-1", classGold, "sid6-1", "pv-uid6-1", "volume6-1", "snapuid6-1", "snap6-1", &deletePolicy, &defaultSize, &timeNowStamp, false, nil),
7373
initialSnapshots: newSnapshotArray("snap6-1", classGold, "", "snapuid6-1", "claim6-1", false, nil, nil, nil),
7474
expectedSnapshots: newSnapshotArray("snap6-1", classGold, "snapcontent-snapuid6-1", "snapuid6-1", "claim6-1", false, nil, metaTimeNowUnix, getSize(defaultSize)),
7575
initialClaims: newClaimArray("claim6-1", "pvc-uid6-1", "1Gi", "volume6-1", v1.ClaimBound, &classEmpty),
@@ -93,7 +93,7 @@ func TestCreateSnapshotSync(t *testing.T) {
9393
{
9494
name: "6-2 - successful create snapshot with snapshot class silver",
9595
initialContents: nocontents,
96-
expectedContents: newContentArray("snapcontent-snapuid6-2", classSilver, "sid6-2", "pv-uid6-2", "volume6-2", "snapuid6-2", "snap6-2", &deletePolicy, &defaultSize, &timeNowStamp, false),
96+
expectedContents: newContentArray("snapcontent-snapuid6-2", classSilver, "sid6-2", "pv-uid6-2", "volume6-2", "snapuid6-2", "snap6-2", &deletePolicy, &defaultSize, &timeNowStamp, false, nil),
9797
initialSnapshots: newSnapshotArray("snap6-2", classSilver, "", "snapuid6-2", "claim6-2", false, nil, nil, nil),
9898
expectedSnapshots: newSnapshotArray("snap6-2", classSilver, "snapcontent-snapuid6-2", "snapuid6-2", "claim6-2", false, nil, metaTimeNowUnix, getSize(defaultSize)),
9999
initialClaims: newClaimArray("claim6-2", "pvc-uid6-2", "1Gi", "volume6-2", v1.ClaimBound, &classEmpty),
@@ -117,7 +117,7 @@ func TestCreateSnapshotSync(t *testing.T) {
117117
{
118118
name: "6-3 - successful create snapshot with snapshot class valid-secret-class",
119119
initialContents: nocontents,
120-
expectedContents: newContentArray("snapcontent-snapuid6-3", validSecretClass, "sid6-3", "pv-uid6-3", "volume6-3", "snapuid6-3", "snap6-3", &deletePolicy, &defaultSize, &timeNowStamp, false),
120+
expectedContents: newContentArray("snapcontent-snapuid6-3", validSecretClass, "sid6-3", "pv-uid6-3", "volume6-3", "snapuid6-3", "snap6-3", &deletePolicy, &defaultSize, &timeNowStamp, false, secretAnnotations()),
121121
initialSnapshots: newSnapshotArray("snap6-3", validSecretClass, "", "snapuid6-3", "claim6-3", false, nil, nil, nil),
122122
expectedSnapshots: newSnapshotArray("snap6-3", validSecretClass, "snapcontent-snapuid6-3", "snapuid6-3", "claim6-3", false, nil, metaTimeNowUnix, getSize(defaultSize)),
123123
initialClaims: newClaimArray("claim6-3", "pvc-uid6-3", "1Gi", "volume6-3", v1.ClaimBound, &classEmpty),
@@ -143,7 +143,7 @@ func TestCreateSnapshotSync(t *testing.T) {
143143
{
144144
name: "6-4 - successful create snapshot with snapshot class empty-secret-class",
145145
initialContents: nocontents,
146-
expectedContents: newContentArray("snapcontent-snapuid6-4", emptySecretClass, "sid6-4", "pv-uid6-4", "volume6-4", "snapuid6-4", "snap6-4", &deletePolicy, &defaultSize, &timeNowStamp, false),
146+
expectedContents: newContentArray("snapcontent-snapuid6-4", emptySecretClass, "sid6-4", "pv-uid6-4", "volume6-4", "snapuid6-4", "snap6-4", &deletePolicy, &defaultSize, &timeNowStamp, false, emptyDataSecretAnnotations()),
147147
initialSnapshots: newSnapshotArray("snap6-4", emptySecretClass, "", "snapuid6-4", "claim6-4", false, nil, nil, nil),
148148
expectedSnapshots: newSnapshotArray("snap6-4", emptySecretClass, "snapcontent-snapuid6-4", "snapuid6-4", "claim6-4", false, nil, metaTimeNowUnix, getSize(defaultSize)),
149149
initialClaims: newClaimArray("claim6-4", "pvc-uid6-4", "1Gi", "volume6-4", v1.ClaimBound, &classEmpty),
@@ -169,7 +169,7 @@ func TestCreateSnapshotSync(t *testing.T) {
169169
{
170170
name: "6-5 - successful create snapshot with status uploading",
171171
initialContents: nocontents,
172-
expectedContents: newContentArray("snapcontent-snapuid6-5", classGold, "sid6-5", "pv-uid6-5", "volume6-5", "snapuid6-5", "snap6-5", &deletePolicy, &defaultSize, &timeNowStamp, false),
172+
expectedContents: newContentArray("snapcontent-snapuid6-5", classGold, "sid6-5", "pv-uid6-5", "volume6-5", "snapuid6-5", "snap6-5", &deletePolicy, &defaultSize, &timeNowStamp, false, nil),
173173
initialSnapshots: newSnapshotArray("snap6-5", classGold, "", "snapuid6-5", "claim6-5", false, nil, nil, nil),
174174
expectedSnapshots: newSnapshotArray("snap6-5", classGold, "snapcontent-snapuid6-5", "snapuid6-5", "claim6-5", false, nil, metaTimeNowUnix, getSize(defaultSize)),
175175
initialClaims: newClaimArray("claim6-5", "pvc-uid6-5", "1Gi", "volume6-5", v1.ClaimBound, &classEmpty),
@@ -193,7 +193,7 @@ func TestCreateSnapshotSync(t *testing.T) {
193193
{
194194
name: "6-6 - successful create snapshot with status error uploading",
195195
initialContents: nocontents,
196-
expectedContents: newContentArray("snapcontent-snapuid6-6", classGold, "sid6-6", "pv-uid6-6", "volume6-6", "snapuid6-6", "snap6-6", &deletePolicy, &defaultSize, &timeNowStamp, false),
196+
expectedContents: newContentArray("snapcontent-snapuid6-6", classGold, "sid6-6", "pv-uid6-6", "volume6-6", "snapuid6-6", "snap6-6", &deletePolicy, &defaultSize, &timeNowStamp, false, nil),
197197
initialSnapshots: newSnapshotArray("snap6-6", classGold, "", "snapuid6-6", "claim6-6", false, nil, nil, nil),
198198
expectedSnapshots: newSnapshotArray("snap6-6", classGold, "snapcontent-snapuid6-6", "snapuid6-6", "claim6-6", false, nil, metaTimeNowUnix, getSize(defaultSize)),
199199
initialClaims: newClaimArray("claim6-6", "pvc-uid6-6", "1Gi", "volume6-6", v1.ClaimBound, &classEmpty),
@@ -365,6 +365,18 @@ func TestCreateSnapshotSync(t *testing.T) {
365365
expectedEvents: []string{"Warning CreateSnapshotContentFailed"},
366366
test: testSyncSnapshot,
367367
},
368+
{
369+
name: "7-10 - fail create snapshot with secret not found",
370+
initialContents: nocontents,
371+
expectedContents: nocontents,
372+
initialSnapshots: newSnapshotArray("snap7-10", validSecretClass, "", "snapuid7-10", "claim7-10", false, nil, nil, nil),
373+
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),
374+
initialClaims: newClaimArray("claim7-10", "pvc-uid7-10", "1Gi", "volume7-10", v1.ClaimBound, &classEmpty),
375+
initialVolumes: newVolumeArray("volume7-10", "pv-uid7-10", "pv-handle7-10", "1Gi", "pvc-uid7-10", "claim7-10", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
376+
initialSecrets: []*v1.Secret{}, // no initial secret created
377+
errors: noerrors,
378+
test: testSyncSnapshot,
379+
},
368380
}
369381
runSyncTests(t, tests, snapshotClasses)
370382
}

0 commit comments

Comments
 (0)