Skip to content

Commit

Permalink
Issue 7068: add a finalizer to protect retained VSC
Browse files Browse the repository at this point in the history 
Signed-off-by: Lyndon-Li <[email protected]>
  • Loading branch information
@Lyndon-Li
Lyndon-Li committed Nov 15, 2023
1 parent 9b5678f commit 2c67937
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 46 deletions.
11 changes: 8 additions & 3 deletions pkg/exposer/csi_snapshot.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,6 @@ func (e *csiSnapshotExposer) Expose(ctx context.Context, ownerObject corev1.Obje
return errors.Wrap(err, "error to retain volume snapshot content")
}

vsc = retained

curLog.WithField("vsc name", vsc.Name).WithField("retained", (retained != nil)).Info("Finished to retain VSC")

defer func() {
Expand All @@ -136,7 +134,14 @@ func (e *csiSnapshotExposer) Expose(ctx context.Context, ownerObject corev1.Obje

curLog.WithField("vs name", volumeSnapshot.Name).Infof("VS is deleted in namespace %s", volumeSnapshot.Namespace)

err = csi.EnsureDeleteVSC(ctx, e.csiSnapshotClient, vsc, csiExposeParam.OperationTimeout)
err = csi.RemoveVSCProtect(ctx, e.csiSnapshotClient, vsc.Name, csiExposeParam.ExposeTimeout)
if err != nil {
return errors.Wrap(err, "error to remove protect from volume snapshot content")
}

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

View check run for this annotation

Codecov / codecov/patch

pkg/exposer/csi_snapshot.go#L139-L140 

Added lines #L139 - L140 were not covered by tests

curLog.WithField("vsc name", vsc.Name).Infof("Removed protect from VSC")

err = csi.EnsureDeleteVSC(ctx, e.csiSnapshotClient, vsc.Name, csiExposeParam.OperationTimeout)
if err != nil {
return errors.Wrap(err, "error to delete volume snapshot content")
}
Expand Down
41 changes: 29 additions & 12 deletions pkg/util/csi/volume_snapshot.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,36 +152,53 @@ func EnsureDeleteVS(ctx context.Context, snapshotClient snapshotter.SnapshotV1In
return nil
}

// EnsureDeleteVSC asserts the existence of a VSC by name, deletes it and waits for its disappearance and returns errors on any failure
func EnsureDeleteVSC(ctx context.Context, snapshotClient snapshotter.SnapshotV1Interface,
vsc *snapshotv1api.VolumeSnapshotContent, timeout time.Duration) error {
_, err := patchVSC(ctx, snapshotClient, vsc, func(updated *snapshotv1api.VolumeSnapshotContent) {
updated.Finalizers = stringslice.Except(updated.Finalizers, volumeSnapshotContentProtectFinalizer)
func RemoveVSCProtect(ctx context.Context, snapshotClient snapshotter.SnapshotV1Interface, vscName string, timeout time.Duration) error {
err := wait.PollImmediate(waitInternal, timeout, func() (bool, error) {
vsc, err := snapshotClient.VolumeSnapshotContents().Get(ctx, vscName, metav1.GetOptions{})
if err != nil {
return false, errors.Wrapf(err, "error to get VolumeSnapshotContent %s", vscName)
}

vsc.Finalizers = stringslice.Except(vsc.Finalizers, volumeSnapshotContentProtectFinalizer)

_, err = snapshotClient.VolumeSnapshotContents().Update(ctx, vsc, metav1.UpdateOptions{})
if err == nil {
return true, nil
}

if !apierrors.IsConflict(err) {
return false, errors.Wrapf(err, "error to update VolumeSnapshotContent %s", vscName)
}

return false, nil
})
if err != nil {
return errors.Wrapf(err, "error to remove protect finalizer from vsc %s", vsc.Name)
}

err = snapshotClient.VolumeSnapshotContents().Delete(ctx, vsc.Name, metav1.DeleteOptions{})
return err
}

// EnsureDeleteVSC asserts the existence of a VSC by name, deletes it and waits for its disappearance and returns errors on any failure
func EnsureDeleteVSC(ctx context.Context, snapshotClient snapshotter.SnapshotV1Interface,
vscName string, timeout time.Duration) error {
err := snapshotClient.VolumeSnapshotContents().Delete(ctx, vscName, metav1.DeleteOptions{})
if err != nil && !apierrors.IsNotFound(err) {
return errors.Wrap(err, "error to delete volume snapshot content")
}

err = wait.PollImmediate(waitInternal, timeout, func() (bool, error) {
_, err := snapshotClient.VolumeSnapshotContents().Get(ctx, vsc.Name, metav1.GetOptions{})
_, err := snapshotClient.VolumeSnapshotContents().Get(ctx, vscName, metav1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
return true, nil
}

return false, errors.Wrapf(err, fmt.Sprintf("error to get VolumeSnapshotContent %s", vsc.Name))
return false, errors.Wrapf(err, fmt.Sprintf("error to get VolumeSnapshotContent %s", vscName))
}

return false, nil
})

if err != nil {
return errors.Wrapf(err, "error to assure VolumeSnapshotContent is deleted, %s", vsc.Name)
return errors.Wrapf(err, "error to assure VolumeSnapshotContent is deleted, %s", vscName)
}

return nil
Expand Down
135 changes: 104 additions & 31 deletions pkg/util/csi/volume_snapshot_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import (
"github.com/vmware-tanzu/velero/pkg/util/stringptr"

velerotest "github.com/vmware-tanzu/velero/pkg/test"

apierrors "k8s.io/apimachinery/pkg/api/errors"
)

type reactor struct {
Expand Down Expand Up @@ -360,41 +362,17 @@ func TestEnsureDeleteVSC(t *testing.T) {
name string
clientObj []runtime.Object
reactors []reactor
vscObj *snapshotv1api.VolumeSnapshotContent
vscName string
err string
}{
{
name: "remove finalizer fail",
vscObj: vscObj,
reactors: []reactor{
{
verb: "patch",
resource: "volumesnapshotcontents",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, errors.New("fake-patch-error")
},
},
},
err: "error to remove protect finalizer from vsc fake-vsc: error patching VSC: fake-patch-error",
},
{
name: "delete fail",
vscObj: vscObj,
clientObj: []runtime.Object{vscObj},
reactors: []reactor{
{
verb: "delete",
resource: "volumesnapshotcontents",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, errors.New("fake-delete-error")
},
},
},
err: "error to delete volume snapshot content: fake-delete-error",
name: "delete fail",
vscName: "fake-vsc",
err: "error to delete volume snapshot content: volumesnapshotcontents.snapshot.storage.k8s.io \"fake-vsc\" not found",
},
{
name: "wait fail",
vscObj: vscObj,
vscName: "fake-vsc",
clientObj: []runtime.Object{vscObj},
reactors: []reactor{
{
Expand All @@ -409,7 +387,7 @@ func TestEnsureDeleteVSC(t *testing.T) {
},
{
name: "success",
vscObj: vscObj,
vscName: "fake-vsc",
clientObj: []runtime.Object{vscObj},
},
}
Expand All @@ -422,7 +400,7 @@ func TestEnsureDeleteVSC(t *testing.T) {
fakeSnapshotClient.Fake.PrependReactor(reactor.verb, reactor.resource, reactor.reactorFunc)
}

err := EnsureDeleteVSC(context.Background(), fakeSnapshotClient.SnapshotV1(), test.vscObj, time.Millisecond)
err := EnsureDeleteVSC(context.Background(), fakeSnapshotClient.SnapshotV1(), test.vscName, time.Millisecond)
if err != nil {
assert.EqualError(t, err, test.err)
} else {
Expand Down Expand Up @@ -659,3 +637,98 @@ func TestRetainVSC(t *testing.T) {
})
}
}

func TestRemoveVSCProtect(t *testing.T) {
vscObj := &snapshotv1api.VolumeSnapshotContent{
ObjectMeta: metav1.ObjectMeta{
Name: "fake-vsc",
Finalizers: []string{volumeSnapshotContentProtectFinalizer},
},
}

tests := []struct {
name string
clientObj []runtime.Object
reactors []reactor
vsc string
updated *snapshotv1api.VolumeSnapshotContent
timeout time.Duration
err string
}{
{
name: "get vsc error",
vsc: "fake-vsc",
err: "error to get VolumeSnapshotContent fake-vsc: volumesnapshotcontents.snapshot.storage.k8s.io \"fake-vsc\" not found",
},
{
name: "update vsc fail",
vsc: "fake-vsc",
clientObj: []runtime.Object{vscObj},
reactors: []reactor{
{
verb: "update",
resource: "volumesnapshotcontents",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, errors.New("fake-update-error")
},
},
},
err: "error to update VolumeSnapshotContent fake-vsc: fake-update-error",
},
{
name: "update vsc timeout",
vsc: "fake-vsc",
clientObj: []runtime.Object{vscObj},
reactors: []reactor{
{
verb: "update",
resource: "volumesnapshotcontents",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, &apierrors.StatusError{ErrStatus: metav1.Status{
Reason: metav1.StatusReasonConflict,
}}
},
},
},
timeout: time.Second,
err: "timed out waiting for the condition",
},
{
name: "succeed",
vsc: "fake-vsc",
clientObj: []runtime.Object{vscObj},
timeout: time.Second,
updated: &snapshotv1api.VolumeSnapshotContent{
ObjectMeta: metav1.ObjectMeta{
Name: "fake-vsc",
Finalizers: []string{},
},
},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
fakeSnapshotClient := snapshotFake.NewSimpleClientset(test.clientObj...)

for _, reactor := range test.reactors {
fakeSnapshotClient.Fake.PrependReactor(reactor.verb, reactor.resource, reactor.reactorFunc)
}

err := RemoveVSCProtect(context.Background(), fakeSnapshotClient.SnapshotV1(), test.vsc, test.timeout)

if len(test.err) == 0 {
assert.NoError(t, err)
} else {
assert.EqualError(t, err, test.err)
}

if test.updated != nil {
updated, err := fakeSnapshotClient.SnapshotV1().VolumeSnapshotContents().Get(context.Background(), test.vsc, metav1.GetOptions{})
assert.NoError(t, err)

assert.Equal(t, test.updated.Finalizers, updated.Finalizers)
}
})
}
}

0 comments on commit 2c67937

Please sign in to comment.