Merge pull request #7662 from Lyndon-Li/issue-fix-7648

Issue fix 7648: avoid snapshot leak on expose failure
vmware-tanzu · Apr 12, 2024 · c888f51 · c888f51
2 parents 43aa892 + dcf760d
commit c888f51
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 36 deletions.
diff --git a/changelogs/unreleased/7662-Lyndon-Li b/changelogs/unreleased/7662-Lyndon-Li
@@ -0,0 +1 @@
+Fix issue #7648. Adjust the exposing logic to avoid exposing failure and snapshot leak when expose fails
diff --git a/pkg/exposer/csi_snapshot.go b/pkg/exposer/csi_snapshot.go
@@ -113,60 +113,46 @@ func (e *csiSnapshotExposer) Expose(ctx context.Context, ownerObject corev1.Obje
 
 	curLog.WithField("vsc name", vsc.Name).WithField("vs name", volumeSnapshot.Name).Infof("Got VSC from VS in namespace %s", volumeSnapshot.Namespace)
 
-	retained, err := csi.RetainVSC(ctx, e.csiSnapshotClient, vsc)
+	backupVS, err := e.createBackupVS(ctx, ownerObject, volumeSnapshot)
 	if err != nil {
-		return errors.Wrap(err, "error to retain volume snapshot content")
+		return errors.Wrap(err, "error to create backup volume snapshot")
 	}
 
-	curLog.WithField("vsc name", vsc.Name).WithField("retained", (retained != nil)).Info("Finished to retain VSC")
+	curLog.WithField("vs name", backupVS.Name).Infof("Backup VS is created from %s/%s", volumeSnapshot.Namespace, volumeSnapshot.Name)
 
 	defer func() {
-		if retained != nil {
-			csi.DeleteVolumeSnapshotContentIfAny(ctx, e.csiSnapshotClient, retained.Name, curLog)
+		if err != nil {
+			csi.DeleteVolumeSnapshotIfAny(ctx, e.csiSnapshotClient, backupVS.Name, backupVS.Namespace, curLog)
 		}
 	}()
 
-	err = csi.EnsureDeleteVS(ctx, e.csiSnapshotClient, volumeSnapshot.Name, volumeSnapshot.Namespace, csiExposeParam.OperationTimeout)
-	if err != nil {
-		return errors.Wrap(err, "error to delete volume snapshot")
-	}
-
-	curLog.WithField("vs name", volumeSnapshot.Name).Infof("VS is deleted in namespace %s", volumeSnapshot.Namespace)
-
-	err = csi.RemoveVSCProtect(ctx, e.csiSnapshotClient, vsc.Name, csiExposeParam.ExposeTimeout)
+	backupVSC, err := e.createBackupVSC(ctx, ownerObject, vsc, backupVS)
 	if err != nil {
-		return errors.Wrap(err, "error to remove protect from volume snapshot content")
+		return errors.Wrap(err, "error to create backup volume snapshot content")
 	}
 
-	curLog.WithField("vsc name", vsc.Name).Infof("Removed protect from VSC")
+	curLog.WithField("vsc name", backupVSC.Name).Infof("Backup VSC is created from %s", vsc.Name)
 
-	err = csi.EnsureDeleteVSC(ctx, e.csiSnapshotClient, vsc.Name, csiExposeParam.OperationTimeout)
+	retained, err := csi.RetainVSC(ctx, e.csiSnapshotClient, vsc)
 	if err != nil {
-		return errors.Wrap(err, "error to delete volume snapshot content")
+		return errors.Wrap(err, "error to retain volume snapshot content")
 	}
 
-	curLog.WithField("vsc name", vsc.Name).Infof("VSC is deleted")
-	retained = nil
+	curLog.WithField("vsc name", vsc.Name).WithField("retained", (retained != nil)).Info("Finished to retain VSC")
 
-	backupVS, err := e.createBackupVS(ctx, ownerObject, volumeSnapshot)
+	err = csi.EnsureDeleteVS(ctx, e.csiSnapshotClient, volumeSnapshot.Name, volumeSnapshot.Namespace, csiExposeParam.OperationTimeout)
 	if err != nil {
-		return errors.Wrap(err, "error to create backup volume snapshot")
+		return errors.Wrap(err, "error to delete volume snapshot")
 	}
 
-	curLog.WithField("vs name", backupVS.Name).Infof("Backup VS is created from %s/%s", volumeSnapshot.Namespace, volumeSnapshot.Name)
-
-	defer func() {
-		if err != nil {
-			csi.DeleteVolumeSnapshotIfAny(ctx, e.csiSnapshotClient, backupVS.Name, backupVS.Namespace, curLog)
-		}
-	}()
+	curLog.WithField("vs name", volumeSnapshot.Name).Infof("VS is deleted in namespace %s", volumeSnapshot.Namespace)
 
-	backupVSC, err := e.createBackupVSC(ctx, ownerObject, vsc, backupVS)
+	err = csi.EnsureDeleteVSC(ctx, e.csiSnapshotClient, vsc.Name, csiExposeParam.OperationTimeout)
 	if err != nil {
-		return errors.Wrap(err, "error to create backup volume snapshot content")
+		return errors.Wrap(err, "error to delete volume snapshot content")
 	}
 
-	curLog.WithField("vsc name", backupVSC.Name).Infof("Backup VSC is created from %s", vsc.Name)
+	curLog.WithField("vsc name", vsc.Name).Infof("VSC is deleted")
 
 	var volumeSize resource.Quantity
 	if volumeSnapshot.Status.RestoreSize != nil && !volumeSnapshot.Status.RestoreSize.IsZero() {

diff --git a/pkg/util/csi/volume_snapshot.go b/pkg/util/csi/volume_snapshot.go
@@ -129,8 +129,7 @@ func GetVolumeSnapshotContentForVolumeSnapshot(
 	return vsc, nil
 }
 
-// RetainVSC updates the VSC's deletion policy to Retain and add a
-// finalizer and then return the update VSC
+// RetainVSC updates the VSC's deletion policy to Retain and then return the update VSC
 func RetainVSC(ctx context.Context, snapshotClient snapshotter.SnapshotV1Interface,
 	vsc *snapshotv1api.VolumeSnapshotContent) (*snapshotv1api.VolumeSnapshotContent, error) {
 	if vsc.Spec.DeletionPolicy == snapshotv1api.VolumeSnapshotContentRetain {
@@ -139,7 +138,6 @@ func RetainVSC(ctx context.Context, snapshotClient snapshotter.SnapshotV1Interfa
 
 	return patchVSC(ctx, snapshotClient, vsc, func(updated *snapshotv1api.VolumeSnapshotContent) {
 		updated.Spec.DeletionPolicy = snapshotv1api.VolumeSnapshotContentRetain
-		updated.Finalizers = append(updated.Finalizers, volumeSnapshotContentProtectFinalizer)
 	})
 }
 

diff --git a/pkg/util/csi/volume_snapshot_test.go b/pkg/util/csi/volume_snapshot_test.go
@@ -626,8 +626,7 @@ func TestRetainVSC(t *testing.T) {
 			clientObj: []runtime.Object{vscObj},
 			updated: &snapshotv1api.VolumeSnapshotContent{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:       "fake-vsc",
-					Finalizers: []string{volumeSnapshotContentProtectFinalizer},
+					Name: "fake-vsc",
 				},
 				Spec: snapshotv1api.VolumeSnapshotContentSpec{
 					DeletionPolicy: snapshotv1api.VolumeSnapshotContentRetain,