From bf03938dd2b8eed949a1aea01f37387732b37251 Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Thu, 11 Apr 2024 19:23:57 +0800 Subject: [PATCH 1/2] issue 7648: don't leak snapshot on failure Signed-off-by: Lyndon-Li --- pkg/exposer/csi_snapshot.go | 48 ++++++++++++--------------------- pkg/util/csi/volume_snapshot.go | 3 +-- 2 files changed, 18 insertions(+), 33 deletions(-) diff --git a/pkg/exposer/csi_snapshot.go b/pkg/exposer/csi_snapshot.go index 637d0805dd..fac00732cd 100644 --- a/pkg/exposer/csi_snapshot.go +++ b/pkg/exposer/csi_snapshot.go @@ -118,60 +118,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 index a9a88fd32b..9cf1951443 100644 --- a/pkg/util/csi/volume_snapshot.go +++ b/pkg/util/csi/volume_snapshot.go @@ -99,7 +99,7 @@ func GetVolumeSnapshotContentForVolumeSnapshot(volSnap *snapshotv1api.VolumeSnap return vsc, nil } -// RetainVSC updates the VSC's deletion policy to Retain and add a finalier 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 { @@ -108,7 +108,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) }) } From dcf760d5f19e44e83839b19b7c7a5f56ff386dc3 Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Fri, 12 Apr 2024 10:21:33 +0800 Subject: [PATCH 2/2] issue 7648:avoid snapshot leak on expose failure Signed-off-by: Lyndon-Li --- changelogs/unreleased/7662-Lyndon-Li | 1 + pkg/util/csi/volume_snapshot_test.go | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/7662-Lyndon-Li diff --git a/changelogs/unreleased/7662-Lyndon-Li b/changelogs/unreleased/7662-Lyndon-Li new file mode 100644 index 0000000000..536dd2dcfb --- /dev/null +++ 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 \ No newline at end of file diff --git a/pkg/util/csi/volume_snapshot_test.go b/pkg/util/csi/volume_snapshot_test.go index 74be22786e..a8200291ac 100644 --- 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,