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/exposer/csi_snapshot.go b/pkg/exposer/csi_snapshot.go index 68dce2437b..7a37b794ca 100644 --- 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 index 88cf15cea1..94188b801e 100644 --- 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 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,