issue 7068: add a finalizer to protect retained VSC

Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
This commit is contained in:
Lyndon-Li
2023-11-13 17:37:18 +08:00
parent e826b70327
commit cb651d0436
4 changed files with 85 additions and 39 deletions

View File

@@ -0,0 +1 @@
Fix issue #7068, due to an behavior of CSI external snapshotter, manipulations of VS and VSC may not be handled in the same order inside external snapshotter as the API is called. So add a protection finalizer to ensure the order

View File

@@ -119,6 +119,8 @@ 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() {
@@ -134,7 +136,7 @@ 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.Name, csiExposeParam.OperationTimeout)
err = csi.EnsureDeleteVSC(ctx, e.csiSnapshotClient, vsc, csiExposeParam.OperationTimeout)
if err != nil {
return errors.Wrap(err, "error to delete volume snapshot content")
}

View File

@@ -31,6 +31,7 @@ import (
"github.com/vmware-tanzu/velero/pkg/util/boolptr"
"github.com/vmware-tanzu/velero/pkg/util/stringptr"
"github.com/vmware-tanzu/velero/pkg/util/stringslice"
snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
snapshotter "github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned/typed/volumesnapshot/v1"
@@ -41,7 +42,8 @@ import (
)
const (
waitInternal = 2 * time.Second
waitInternal = 2 * time.Second
volumeSnapshotContentProtectFinalizer = "velero.io/volume-snapshot-content-protect-finalizer"
)
// WaitVolumeSnapshotReady waits a VS to become ready to use until the timeout reaches
@@ -97,36 +99,17 @@ func GetVolumeSnapshotContentForVolumeSnapshot(volSnap *snapshotv1api.VolumeSnap
return vsc, nil
}
// RetainVSC updates the VSC's deletion policy to Retain and return the update VSC
// RetainVSC updates the VSC's deletion policy to Retain and add a finalier 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 {
return vsc, nil
}
origBytes, err := json.Marshal(vsc)
if err != nil {
return nil, errors.Wrap(err, "error marshaling original VSC")
}
updated := vsc.DeepCopy()
updated.Spec.DeletionPolicy = snapshotv1api.VolumeSnapshotContentRetain
updatedBytes, err := json.Marshal(updated)
if err != nil {
return nil, errors.Wrap(err, "error marshaling updated VSC")
}
patchBytes, err := jsonpatch.CreateMergePatch(origBytes, updatedBytes)
if err != nil {
return nil, errors.Wrap(err, "error creating json merge patch for VSC")
}
retained, err := snapshotClient.VolumeSnapshotContents().Patch(ctx, vsc.Name, types.MergePatchType, patchBytes, metav1.PatchOptions{})
if err != nil {
return nil, errors.Wrap(err, "error patching VSC")
}
return retained, nil
return patchVSC(ctx, snapshotClient, vsc, func(updated *snapshotv1api.VolumeSnapshotContent) {
updated.Spec.DeletionPolicy = snapshotv1api.VolumeSnapshotContentRetain
updated.Finalizers = append(updated.Finalizers, volumeSnapshotContentProtectFinalizer)
})
}
// DeleteVolumeSnapshotContentIfAny deletes a VSC by name if it exists, and log an error when the deletion fails
@@ -171,27 +154,34 @@ func EnsureDeleteVS(ctx context.Context, snapshotClient snapshotter.SnapshotV1In
// 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{})
vsc *snapshotv1api.VolumeSnapshotContent, timeout time.Duration) error {
_, err := patchVSC(ctx, snapshotClient, vsc, func(updated *snapshotv1api.VolumeSnapshotContent) {
updated.Finalizers = stringslice.Except(updated.Finalizers, volumeSnapshotContentProtectFinalizer)
})
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{})
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, vscName, metav1.GetOptions{})
_, err := snapshotClient.VolumeSnapshotContents().Get(ctx, vsc.Name, metav1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
return true, nil
}
return false, errors.Wrapf(err, fmt.Sprintf("error to get VolumeSnapshotContent %s", vscName))
return false, errors.Wrapf(err, fmt.Sprintf("error to get VolumeSnapshotContent %s", vsc.Name))
}
return false, nil
})
if err != nil {
return errors.Wrapf(err, "error to assure VolumeSnapshotContent is deleted, %s", vscName)
return errors.Wrapf(err, "error to assure VolumeSnapshotContent is deleted, %s", vsc.Name)
}
return nil
@@ -208,3 +198,31 @@ func DeleteVolumeSnapshotIfAny(ctx context.Context, snapshotClient snapshotter.S
}
}
}
func patchVSC(ctx context.Context, snapshotClient snapshotter.SnapshotV1Interface,
vsc *snapshotv1api.VolumeSnapshotContent, updateFunc func(*snapshotv1api.VolumeSnapshotContent)) (*snapshotv1api.VolumeSnapshotContent, error) {
origBytes, err := json.Marshal(vsc)
if err != nil {
return nil, errors.Wrap(err, "error marshaling original VSC")
}
updated := vsc.DeepCopy()
updateFunc(updated)
updatedBytes, err := json.Marshal(updated)
if err != nil {
return nil, errors.Wrap(err, "error marshaling updated VSC")
}
patchBytes, err := jsonpatch.CreateMergePatch(origBytes, updatedBytes)
if err != nil {
return nil, errors.Wrap(err, "error creating json merge patch for VSC")
}
patched, err := snapshotClient.VolumeSnapshotContents().Patch(ctx, vsc.Name, types.MergePatchType, patchBytes, metav1.PatchOptions{})
if err != nil {
return nil, errors.Wrap(err, "error patching VSC")
}
return patched, nil
}

View File

@@ -360,17 +360,41 @@ func TestEnsureDeleteVSC(t *testing.T) {
name string
clientObj []runtime.Object
reactors []reactor
vscName string
vscObj *snapshotv1api.VolumeSnapshotContent
err string
}{
{
name: "delete fail",
vscName: "fake-vsc",
err: "error to delete volume snapshot content: volumesnapshotcontents.snapshot.storage.k8s.io \"fake-vsc\" not found",
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: "wait fail",
vscName: "fake-vsc",
vscObj: vscObj,
clientObj: []runtime.Object{vscObj},
reactors: []reactor{
{
@@ -385,7 +409,7 @@ func TestEnsureDeleteVSC(t *testing.T) {
},
{
name: "success",
vscName: "fake-vsc",
vscObj: vscObj,
clientObj: []runtime.Object{vscObj},
},
}
@@ -398,7 +422,7 @@ func TestEnsureDeleteVSC(t *testing.T) {
fakeSnapshotClient.Fake.PrependReactor(reactor.verb, reactor.resource, reactor.reactorFunc)
}
err := EnsureDeleteVSC(context.Background(), fakeSnapshotClient.SnapshotV1(), test.vscName, time.Millisecond)
err := EnsureDeleteVSC(context.Background(), fakeSnapshotClient.SnapshotV1(), test.vscObj, time.Millisecond)
if err != nil {
assert.EqualError(t, err, test.err)
} else {
@@ -601,7 +625,8 @@ func TestRetainVSC(t *testing.T) {
clientObj: []runtime.Object{vscObj},
updated: &snapshotv1api.VolumeSnapshotContent{
ObjectMeta: metav1.ObjectMeta{
Name: "fake-vsc",
Name: "fake-vsc",
Finalizers: []string{volumeSnapshotContentProtectFinalizer},
},
Spec: snapshotv1api.VolumeSnapshotContentSpec{
DeletionPolicy: snapshotv1api.VolumeSnapshotContentRetain,