Merge pull request #7262 from qiuming-best/intermediate-pv-delete

Fix intermediate PV delete for data mover
This commit is contained in:
qiuming
2024-01-04 15:45:32 +08:00
committed by GitHub
4 changed files with 234 additions and 20 deletions

View File

@@ -183,10 +183,9 @@ 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.DeletePVCIfAny(ctx, e.kubeClient.CoreV1(), backupPVC.Name, backupPVC.Namespace, curLog)
kube.DeletePVAndPVCIfAny(ctx, e.kubeClient.CoreV1(), backupPVC.Name, backupPVC.Namespace, curLog)
}
}()
@@ -262,7 +261,8 @@ func (e *csiSnapshotExposer) CleanUp(ctx context.Context, ownerObject corev1.Obj
backupVSName := ownerObject.Name
kube.DeletePodIfAny(ctx, e.kubeClient.CoreV1(), backupPodName, ownerObject.Namespace, e.log)
kube.DeletePVCIfAny(ctx, e.kubeClient.CoreV1(), backupPVCName, ownerObject.Namespace, e.log)
kube.DeletePVAndPVCIfAny(ctx, e.kubeClient.CoreV1(), backupPVCName, ownerObject.Namespace, e.log)
csi.DeleteVolumeSnapshotIfAny(ctx, e.csiSnapshotClient, backupVSName, ownerObject.Namespace, e.log)
csi.DeleteVolumeSnapshotIfAny(ctx, e.csiSnapshotClient, vsName, sourceNamespace, e.log)
}
@@ -329,8 +329,8 @@ func (e *csiSnapshotExposer) createBackupVSC(ctx context.Context, ownerObject co
return e.csiSnapshotClient.VolumeSnapshotContents().Create(ctx, vsc, metav1.CreateOptions{})
}
func (e *csiSnapshotExposer) createBackupPVC(ctx context.Context, ownerObject corev1.ObjectReference, backupVS string, storageClass string, accessMode string, resource resource.Quantity) (*corev1.PersistentVolumeClaim, error) {
backupVCName := ownerObject.Name
func (e *csiSnapshotExposer) createBackupPVC(ctx context.Context, ownerObject corev1.ObjectReference, backupVS, storageClass, accessMode string, resource resource.Quantity) (*corev1.PersistentVolumeClaim, error) {
backupPVCName := ownerObject.Name
volumeMode, err := getVolumeModeByAccessMode(accessMode)
if err != nil {
@@ -346,7 +346,7 @@ func (e *csiSnapshotExposer) createBackupPVC(ctx context.Context, ownerObject co
pvc := &corev1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Namespace: ownerObject.Namespace,
Name: backupVCName,
Name: backupPVCName,
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: ownerObject.APIVersion,

View File

@@ -104,7 +104,7 @@ func (e *genericRestoreExposer) Expose(ctx context.Context, ownerObject corev1.O
defer func() {
if err != nil {
kube.DeletePVCIfAny(ctx, e.kubeClient.CoreV1(), restorePVC.Name, restorePVC.Namespace, curLog)
kube.DeletePVAndPVCIfAny(ctx, e.kubeClient.CoreV1(), restorePVC.Name, restorePVC.Namespace, curLog)
}
}()
@@ -165,7 +165,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.DeletePVCIfAny(ctx, e.kubeClient.CoreV1(), restorePVCName, ownerObject.Namespace, e.log)
kube.DeletePVAndPVCIfAny(ctx, e.kubeClient.CoreV1(), restorePVCName, ownerObject.Namespace, e.log)
}
func (e *genericRestoreExposer) RebindVolume(ctx context.Context, ownerObject corev1.ObjectReference, targetPVCName string, sourceNamespace string, timeout time.Duration) error {

View File

@@ -41,16 +41,37 @@ const (
waitInternal = 2 * time.Second
)
// DeletePVCIfAny deletes a PVC by name if it exists, and log an error when the deletion fails
func DeletePVCIfAny(ctx context.Context, pvcGetter corev1client.CoreV1Interface, pvcName string, pvcNamespace string, log logrus.FieldLogger) {
err := pvcGetter.PersistentVolumeClaims(pvcNamespace).Delete(ctx, pvcName, metav1.DeleteOptions{})
// 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) {
pvcObj, err := client.PersistentVolumeClaims(pvcNamespace).Get(ctx, pvcName, metav1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
log.WithError(err).Debugf("Abort deleting PVC, it doesn't exist, %s/%s", pvcNamespace, pvcName)
log.WithError(err).Debugf("Abort deleting PV and PVC, for related PVC doesn't exist, %s/%s", pvcNamespace, pvcName)
return
} else {
log.WithError(err).Errorf("Failed to delete pvc %s/%s", pvcNamespace, pvcName)
log.Warnf("failed to get pvc %s/%s with err %v", pvcNamespace, pvcName, err)
return
}
}
if pvcObj.Spec.VolumeName == "" {
log.Warnf("failed to delete PV, for related PVC %s/%s has no bind volume name", pvcNamespace, pvcName)
} else {
pvObj, err := client.PersistentVolumes().Get(ctx, pvcObj.Spec.VolumeName, metav1.GetOptions{})
if err != nil {
log.Warnf("failed to delete PV %s with err %v", pvcObj.Spec.VolumeName, err)
} else {
_, err = SetPVReclaimPolicy(ctx, client, pvObj, corev1api.PersistentVolumeReclaimDelete)
if err != nil {
log.Warnf("failed to set reclaim policy of PV %s to delete with err %v", pvObj.Name, err)
}
}
}
if err := client.PersistentVolumeClaims(pvcNamespace).Delete(ctx, pvcName, metav1.DeleteOptions{}); err != nil {
log.Warnf("failed to delete pvc %s/%s with err %v", pvcNamespace, pvcName, err)
}
}
// WaitPVCBound wait for binding of a PVC specified by name and returns the bound PV object

View File

@@ -289,10 +289,44 @@ func TestWaitPVCConsumed(t *testing.T) {
}
func TestDeletePVCIfAny(t *testing.T) {
pvObject := &corev1api.PersistentVolume{
ObjectMeta: metav1.ObjectMeta{
Name: "fake-pv",
Annotations: map[string]string{
KubeAnnBoundByController: "true",
},
},
Spec: corev1api.PersistentVolumeSpec{
ClaimRef: &corev1api.ObjectReference{
Kind: "fake-kind",
Namespace: "fake-ns",
Name: "fake-pvc",
},
},
}
pvcObject := &corev1api.PersistentVolumeClaim{
TypeMeta: metav1.TypeMeta{
Kind: "fake-kind-1",
},
ObjectMeta: metav1.ObjectMeta{
Namespace: "fake-namespace",
Name: "fake-pvc",
Annotations: map[string]string{
KubeAnnBindCompleted: "true",
KubeAnnBoundByController: "true",
},
},
}
pvcWithVolume := pvcObject.DeepCopy()
pvcWithVolume.Spec.VolumeName = "fake-pv"
tests := []struct {
name string
pvcName string
pvcNamespace string
pvName string
kubeClientObj []runtime.Object
kubeReactors []reactor
logMessage string
@@ -300,17 +334,69 @@ func TestDeletePVCIfAny(t *testing.T) {
logError string
}{
{
name: "get fail",
name: "pvc not found",
pvcName: "fake-pvc",
pvcNamespace: "fake-namespace",
logMessage: "Abort deleting PVC, it doesn't exist, fake-namespace/fake-pvc",
logMessage: "Abort deleting PV and PVC, for related PVC doesn't exist, fake-namespace/fake-pvc",
logLevel: "level=debug",
},
{
name: "delete fail",
name: "failed to get pvc",
pvcName: "fake-pvc",
pvcNamespace: "fake-namespace",
kubeReactors: []reactor{
{
verb: "get",
resource: "persistentvolumeclaims",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, errors.New("fake-get-error")
},
},
},
logMessage: "failed to get pvc fake-namespace/fake-pvc with err fake-get-error",
logLevel: "level=warning",
},
{
name: "pvc has no volume name",
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,
},
logMessage: "failed to delete PV, for related PVC fake-namespace/fake-pvc has no bind volume name",
logLevel: "level=warning",
},
{
name: "failed to delete pvc",
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",
@@ -319,9 +405,116 @@ func TestDeletePVCIfAny(t *testing.T) {
},
},
},
logMessage: "Failed to delete pvc fake-namespace/fake-pvc",
logLevel: "level=error",
logError: "error=fake-delete-error",
kubeClientObj: []runtime.Object{
pvcObject,
pvObject,
},
logMessage: "failed to delete pvc fake-namespace/fake-pvc with err fake-delete-error",
logLevel: "level=warning",
},
{
name: "failed to get pv",
pvcName: "fake-pvc",
pvcNamespace: "fake-namespace",
pvName: "fake-pv",
kubeReactors: []reactor{
{
verb: "get",
resource: "persistentvolumes",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
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,
pvObject,
},
logMessage: "failed to delete PV fake-pv with err fake-get-error",
logLevel: "level=warning",
},
{
name: "set reclaim policy fail",
pvcName: "fake-pvc",
pvcNamespace: "fake-namespace",
pvName: "fake-pv",
kubeClientObj: []runtime.Object{
pvcWithVolume,
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",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, pvObject, errors.New("fake-patch-error")
},
},
},
logMessage: "failed to set reclaim policy of PV fake-pv to delete with err error patching PV: fake-patch-error",
logLevel: "level=warning",
logError: "fake-patch-error",
},
{
name: "delete pv pvc success",
pvcName: "fake-pvc",
pvcNamespace: "fake-namespace",
pvName: "fake-pv",
kubeClientObj: []runtime.Object{
pvcWithVolume,
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",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, pvObject, nil
},
},
{
verb: "delete",
resource: "persistentvolumeclaims",
reactorFunc: func(action clientTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, pvcWithVolume, nil
},
},
},
},
}
@@ -336,7 +529,7 @@ func TestDeletePVCIfAny(t *testing.T) {
var kubeClient kubernetes.Interface = fakeKubeClient
logMessage := ""
DeletePVCIfAny(context.Background(), kubeClient.CoreV1(), test.pvcName, test.pvcNamespace, velerotest.NewSingleLogger(&logMessage))
DeletePVAndPVCIfAny(context.Background(), kubeClient.CoreV1(), test.pvcName, test.pvcNamespace, velerotest.NewSingleLogger(&logMessage))
if len(test.logMessage) > 0 {
assert.Contains(t, logMessage, test.logMessage)