Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue fix 7648: avoid snapshot leak on expose failure #7662

Merged
merged 3 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/unreleased/7662-Lyndon-Li
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix issue #7648. Adjust the exposing logic to avoid exposing failure and snapshot leak when expose fails
48 changes: 17 additions & 31 deletions pkg/exposer/csi_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,60 +113,46 @@

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")

Check warning on line 138 in pkg/exposer/csi_snapshot.go

View check run for this annotation

Codecov / codecov/patch

pkg/exposer/csi_snapshot.go#L138

Added line #L138 was not covered by tests
}

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() {
Expand Down
4 changes: 1 addition & 3 deletions pkg/util/csi/volume_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
})
}

Expand Down
3 changes: 1 addition & 2 deletions pkg/util/csi/volume_snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading