vmware-tanzu
/
velero
mirror of https://github.com/vmware-tanzu/velero.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #7973 from Lyndon-Li/issue-fix-7972 

Issue 7972: sync the backupPVC deletion in expose clean up
pull/7999/head
Wenkai Yin(尹文开) 2024-07-05 10:35:08 +08:00 committed by GitHub
parent 920396dfd8 7408dbd436
commit 824bebbad7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 84 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
changelogs/unreleased/7973-Lyndon-Li Normal file
View File

@ -0,0 +1 @@
Fix issue #7972, sync the backupPVC deletion in expose clean up

6
pkg/exposer/csi_snapshot.go
View File

@ -170,7 +170,7 @@ func (e *csiSnapshotExposer) Expose(ctx context.Context, ownerObject corev1.Obje
curLog.WithField("pvc name", backupPVC.Name).Info("Backup PVC is created")
defer func() {
if err != nil {
kube.DeletePVAndPVCIfAny(ctx, e.kubeClient.CoreV1(), backupPVC.Name, backupPVC.Namespace, curLog)
kube.DeletePVAndPVCIfAny(ctx, e.kubeClient.CoreV1(), backupPVC.Name, backupPVC.Namespace, 0, curLog)
}
}()
@ -264,13 +264,15 @@ func (e *csiSnapshotExposer) PeekExposed(ctx context.Context, ownerObject corev1
return nil
}
const cleanUpTimeout = time.Minute
func (e *csiSnapshotExposer) CleanUp(ctx context.Context, ownerObject corev1.ObjectReference, vsName string, sourceNamespace string) {
backupPodName := ownerObject.Name
backupPVCName := ownerObject.Name
backupVSName := ownerObject.Name
kube.DeletePodIfAny(ctx, e.kubeClient.CoreV1(), backupPodName, ownerObject.Namespace, e.log)
kube.DeletePVAndPVCIfAny(ctx, e.kubeClient.CoreV1(), backupPVCName, ownerObject.Namespace, e.log)
kube.DeletePVAndPVCIfAny(ctx, e.kubeClient.CoreV1(), backupPVCName, ownerObject.Namespace, cleanUpTimeout, e.log)
csi.DeleteVolumeSnapshotIfAny(ctx, e.csiSnapshotClient, backupVSName, ownerObject.Namespace, e.log)
csi.DeleteVolumeSnapshotIfAny(ctx, e.csiSnapshotClient, vsName, sourceNamespace, e.log)

4
pkg/exposer/generic_restore.go
View File

@ -109,7 +109,7 @@ func (e *genericRestoreExposer) Expose(ctx context.Context, ownerObject corev1.O
defer func() {
if err != nil {
kube.DeletePVAndPVCIfAny(ctx, e.kubeClient.CoreV1(), restorePVC.Name, restorePVC.Namespace, curLog)
kube.DeletePVAndPVCIfAny(ctx, e.kubeClient.CoreV1(), restorePVC.Name, restorePVC.Namespace, 0, curLog)
}
}()
@ -194,7 +194,7 @@ func (e *genericRestoreExposer) CleanUp(ctx context.Context, ownerObject corev1.
restorePVCName := ownerObject.Name
kube.DeletePodIfAny(ctx, e.kubeClient.CoreV1(), restorePodName, ownerObject.Namespace, e.log)
kube.DeletePVAndPVCIfAny(ctx, e.kubeClient.CoreV1(), restorePVCName, ownerObject.Namespace, e.log)
kube.DeletePVAndPVCIfAny(ctx, e.kubeClient.CoreV1(), restorePVCName, ownerObject.Namespace, 0, e.log)
}
func (e *genericRestoreExposer) RebindVolume(ctx context.Context, ownerObject corev1.ObjectReference, targetPVCName string, sourceNamespace string, timeout time.Duration) error {

16
pkg/util/kube/pvc_pv.go
View File

@ -41,9 +41,10 @@ const (
waitInternal = 2 * time.Second
)
// DeletePVAndPVCIfAny deletes PVC and delete the bound PV if it exists and log an error when the deletion fails
// We first set the reclaim policy of the PV to Delete, then PV will be deleted automatically when PVC is deleted.
func DeletePVAndPVCIfAny(ctx context.Context, client corev1client.CoreV1Interface, pvcName, pvcNamespace string, log logrus.FieldLogger) {
// DeletePVAndPVCIfAny deletes PVC and delete the bound PV if it exists and log an error when the deletion fails.
// It first sets the reclaim policy of the PV to Delete, then PV will be deleted automatically when PVC is deleted.
// If ensureTimeout is not 0, it waits until the PVC is deleted or timeout.
func DeletePVAndPVCIfAny(ctx context.Context, client corev1client.CoreV1Interface, pvcName, pvcNamespace string, ensureTimeout time.Duration, log logrus.FieldLogger) {
pvcObj, err := client.PersistentVolumeClaims(pvcNamespace).Get(ctx, pvcName, metav1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
@ -69,7 +70,7 @@ func DeletePVAndPVCIfAny(ctx context.Context, client corev1client.CoreV1Interfac
}
}
if err := client.PersistentVolumeClaims(pvcNamespace).Delete(ctx, pvcName, metav1.DeleteOptions{}); err != nil {
if err := EnsureDeletePVC(ctx, client, pvcName, pvcNamespace, ensureTimeout); err != nil {
log.Warnf("failed to delete pvc %s/%s with err %v", pvcNamespace, pvcName, err)
}
}
@ -118,12 +119,17 @@ func DeletePVIfAny(ctx context.Context, pvGetter corev1client.CoreV1Interface, p
}
// EnsureDeletePVC asserts the existence of a PVC by name, deletes it and waits for its disappearance and returns errors on any failure
// If timeout is 0, it doesn't wait and return nil
func EnsureDeletePVC(ctx context.Context, pvcGetter corev1client.CoreV1Interface, pvc string, namespace string, timeout time.Duration) error {
err := pvcGetter.PersistentVolumeClaims(namespace).Delete(ctx, pvc, metav1.DeleteOptions{})
if err != nil {
return errors.Wrapf(err, "error to delete pvc %s", pvc)
}
if timeout == 0 {
return nil
}
err = wait.PollUntilContextTimeout(ctx, waitInternal, timeout, true, func(ctx context.Context) (bool, error) {
_, err := pvcGetter.PersistentVolumeClaims(namespace).Get(ctx, pvc, metav1.GetOptions{})
if err != nil {
@ -138,7 +144,7 @@ func EnsureDeletePVC(ctx context.Context, pvcGetter corev1client.CoreV1Interface
})
if err != nil {
return errors.Wrapf(err, "error to retrieve pvc info for %s", pvc)
return errors.Wrapf(err, "error to ensure pvc deleted for %s", pvc)
}
return nil

130
pkg/util/kube/pvc_pv_test.go
View File

@ -333,6 +333,7 @@ func TestDeletePVCIfAny(t *testing.T) {
logMessage string
logLevel string
logError string
ensureTimeout time.Duration
}{
{
name: "pvc not found",
@ -362,22 +363,6 @@ func TestDeletePVCIfAny(t *testing.T) {
pvcName: "fake-pvc",
pvcNamespace: "fake-namespace",
pvName: "fake-pv",
kubeReactors: []reactor{
{
verb: "get",
resource: "persistentvolumeclaims",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, pvcObject, nil
},
},
{
verb: "delete",
resource: "persistentvolumeclaims",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, nil
},
},
},
kubeClientObj: []runtime.Object{
pvcObject,
pvObject,
@ -391,13 +376,6 @@ func TestDeletePVCIfAny(t *testing.T) {
pvcNamespace: "fake-namespace",
pvName: "fake-pv",
kubeReactors: []reactor{
{
verb: "get",
resource: "persistentvolumeclaims",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, pvcObject, nil
},
},
{
verb: "delete",
resource: "persistentvolumeclaims",
@ -407,10 +385,10 @@ func TestDeletePVCIfAny(t *testing.T) {
},
},
kubeClientObj: []runtime.Object{
pvcObject,
pvcWithVolume,
pvObject,
},
logMessage: "failed to delete pvc fake-namespace/fake-pvc with err fake-delete-error",
logMessage: "failed to delete pvc fake-namespace/fake-pvc with err error to delete pvc fake-pvc: fake-delete-error",
logLevel: "level=warning",
},
{
@ -426,13 +404,6 @@ func TestDeletePVCIfAny(t *testing.T) {
return true, nil, errors.New("fake-get-error")
},
},
{
verb: "get",
resource: "persistentvolumeclaims",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, pvcWithVolume, nil
},
},
},
kubeClientObj: []runtime.Object{
pvcWithVolume,
@ -451,20 +422,6 @@ func TestDeletePVCIfAny(t *testing.T) {
pvObject,
},
kubeReactors: []reactor{
{
verb: "get",
resource: "persistentvolumeclaims",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, pvcWithVolume, nil
},
},
{
verb: "get",
resource: "persistentvolumes",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, pvObject, nil
},
},
{
verb: "patch",
resource: "persistentvolumes",
@ -486,28 +443,39 @@ func TestDeletePVCIfAny(t *testing.T) {
pvcWithVolume,
pvObject,
},
},
{
name: "delete pv pvc success but wait fail",
pvcName: "fake-pvc",
pvcNamespace: "fake-namespace",
pvName: "fake-pv",
kubeClientObj: []runtime.Object{
pvcWithVolume,
pvObject,
},
kubeReactors: []reactor{
{
verb: "get",
verb: "delete",
resource: "persistentvolumeclaims",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, pvcWithVolume, nil
},
},
{
verb: "get",
resource: "persistentvolumes",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, pvObject, nil
},
},
{
verb: "patch",
resource: "persistentvolumes",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, pvObject, nil
},
},
},
ensureTimeout: time.Second,
logMessage: "failed to delete pvc fake-namespace/fake-pvc with err error to ensure pvc deleted for fake-pvc: context deadline exceeded",
logLevel: "level=warning",
},
{
name: "delete pv pvc success, wait won't succeed but ensureTimeout is 0",
pvcName: "fake-pvc",
pvcNamespace: "fake-namespace",
pvName: "fake-pv",
kubeClientObj: []runtime.Object{
pvcWithVolume,
pvObject,
},
kubeReactors: []reactor{
{
verb: "delete",
resource: "persistentvolumeclaims",
@ -530,7 +498,7 @@ func TestDeletePVCIfAny(t *testing.T) {
var kubeClient kubernetes.Interface = fakeKubeClient
logMessage := ""
DeletePVAndPVCIfAny(context.Background(), kubeClient.CoreV1(), test.pvcName, test.pvcNamespace, velerotest.NewSingleLogger(&logMessage))
DeletePVAndPVCIfAny(context.Background(), kubeClient.CoreV1(), test.pvcName, test.pvcNamespace, test.ensureTimeout, velerotest.NewSingleLogger(&logMessage))
if len(test.logMessage) > 0 {
assert.Contains(t, logMessage, test.logMessage)
@ -623,6 +591,7 @@ func TestEnsureDeletePVC(t *testing.T) {
pvcName string
namespace string
reactors []reactor
timeout time.Duration
err string
}{
{
@ -631,11 +600,27 @@ func TestEnsureDeletePVC(t *testing.T) {
namespace: "fake-ns",
err: "error to delete pvc fake-pvc: persistentvolumeclaims \"fake-pvc\" not found",
},
{
name: "0 timeout",
pvcName: "fake-pvc",
namespace: "fake-ns",
clientObj: []runtime.Object{pvcObject},
reactors: []reactor{
{
verb: "delete",
resource: "persistentvolumeclaims",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, pvcObject, nil
},
},
},
},
{
name: "wait fail",
pvcName: "fake-pvc",
namespace: "fake-ns",
clientObj: []runtime.Object{pvcObject},
timeout: time.Millisecond,
reactors: []reactor{
{
verb: "get",
@ -645,7 +630,24 @@ func TestEnsureDeletePVC(t *testing.T) {
},
},
},
err: "error to retrieve pvc info for fake-pvc: error to get pvc fake-pvc: fake-get-error",
err: "error to ensure pvc deleted for fake-pvc: error to get pvc fake-pvc: fake-get-error",
},
{
name: "wait timeout",
pvcName: "fake-pvc",
namespace: "fake-ns",
clientObj: []runtime.Object{pvcObject},
timeout: time.Millisecond,
reactors: []reactor{
{
verb: "delete",
resource: "persistentvolumeclaims",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, pvcObject, nil
},
},
},
err: "error to ensure pvc deleted for fake-pvc: context deadline exceeded",
},
}
@ -659,7 +661,7 @@ func TestEnsureDeletePVC(t *testing.T) {
var kubeClient kubernetes.Interface = fakeKubeClient
err := EnsureDeletePVC(context.Background(), kubeClient.CoreV1(), test.pvcName, test.namespace, time.Millisecond)
err := EnsureDeletePVC(context.Background(), kubeClient.CoreV1(), test.pvcName, test.namespace, test.timeout)
if err != nil {
assert.EqualError(t, err, test.err)
} else {