Merge pull request #8402 from Lyndon-Li/release-1.15

[1.15] Issue 8394: move closeDataPath outside callbacks
vmware-tanzu · Nov 13, 2024 · 13d8e7d · 13d8e7d
2 parents 1d4f147 + 56cab06
commit 13d8e7d
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 2 deletions.
diff --git a/changelogs/unreleased/8402-Lyndon-Li b/changelogs/unreleased/8402-Lyndon-Li
@@ -0,0 +1 @@
+Fix issue #8394, don't call closeDataPath in VGDP callbacks, otherwise, the VGDP cleanup will hang
diff --git a/pkg/controller/pod_volume_backup_controller.go b/pkg/controller/pod_volume_backup_controller.go
@@ -141,6 +141,7 @@ func (r *PodVolumeBackupReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 	pvb.Status.Phase = velerov1api.PodVolumeBackupPhaseInProgress
 	pvb.Status.StartTimestamp = &metav1.Time{Time: r.clock.Now()}
 	if err := r.Client.Patch(ctx, &pvb, client.MergeFrom(original)); err != nil {
+		r.closeDataPath(ctx, pvb.Name)
 		return r.errorOut(ctx, &pvb, err, "error updating PodVolumeBackup status", log)
 	}
 
@@ -150,11 +151,13 @@ func (r *PodVolumeBackupReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		Name:      pvb.Spec.Pod.Name,
 	}
 	if err := r.Client.Get(ctx, podNamespacedName, &pod); err != nil {
+		r.closeDataPath(ctx, pvb.Name)
 		return r.errorOut(ctx, &pvb, err, fmt.Sprintf("getting pod %s/%s", pvb.Spec.Pod.Namespace, pvb.Spec.Pod.Name), log)
 	}
 
 	path, err := exposer.GetPodVolumeHostPath(ctx, &pod, pvb.Spec.Volume, r.Client, r.fileSystem, log)
 	if err != nil {
+		r.closeDataPath(ctx, pvb.Name)
 		return r.errorOut(ctx, &pvb, err, "error exposing host path for pod volume", log)
 	}
 
@@ -169,6 +172,7 @@ func (r *PodVolumeBackupReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		RepositoryEnsurer: r.repositoryEnsurer,
 		CredentialGetter:  r.credentialGetter,
 	}); err != nil {
+		r.closeDataPath(ctx, pvb.Name)
 		return r.errorOut(ctx, &pvb, err, "error to initialize data path", log)
 	}
 
@@ -193,6 +197,7 @@ func (r *PodVolumeBackupReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		ForceFull:      false,
 		Tags:           pvb.Spec.Tags,
 	}); err != nil {
+		r.closeDataPath(ctx, pvb.Name)
 		return r.errorOut(ctx, &pvb, err, "error starting data path backup", log)
 	}
 
@@ -361,7 +366,6 @@ func (r *PodVolumeBackupReconciler) closeDataPath(ctx context.Context, pvbName s
 }
 
 func (r *PodVolumeBackupReconciler) errorOut(ctx context.Context, pvb *velerov1api.PodVolumeBackup, err error, msg string, log logrus.FieldLogger) (ctrl.Result, error) {
-	r.closeDataPath(ctx, pvb.Name)
 	_ = UpdatePVBStatusToFailed(ctx, r.Client, pvb, err, msg, r.clock.Now(), log)
 
 	return ctrl.Result{}, err

diff --git a/pkg/controller/pod_volume_restore_controller.go b/pkg/controller/pod_volume_restore_controller.go
@@ -131,11 +131,13 @@ func (c *PodVolumeRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	pvr.Status.Phase = velerov1api.PodVolumeRestorePhaseInProgress
 	pvr.Status.StartTimestamp = &metav1.Time{Time: c.clock.Now()}
 	if err = c.Patch(ctx, pvr, client.MergeFrom(original)); err != nil {
+		c.closeDataPath(ctx, pvr.Name)
 		return c.errorOut(ctx, pvr, err, "error to update status to in progress", log)
 	}
 
 	volumePath, err := exposer.GetPodVolumeHostPath(ctx, pod, pvr.Spec.Volume, c.Client, c.fileSystem, log)
 	if err != nil {
+		c.closeDataPath(ctx, pvr.Name)
 		return c.errorOut(ctx, pvr, err, "error exposing host path for pod volume", log)
 	}
 
@@ -150,10 +152,12 @@ func (c *PodVolumeRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		RepositoryEnsurer: c.repositoryEnsurer,
 		CredentialGetter:  c.credentialGetter,
 	}); err != nil {
+		c.closeDataPath(ctx, pvr.Name)
 		return c.errorOut(ctx, pvr, err, "error to initialize data path", log)
 	}
 
 	if err := fsRestore.StartRestore(pvr.Spec.SnapshotID, volumePath, pvr.Spec.UploaderSettings); err != nil {
+		c.closeDataPath(ctx, pvr.Name)
 		return c.errorOut(ctx, pvr, err, "error starting data path restore", log)
 	}
 
@@ -163,7 +167,6 @@ func (c *PodVolumeRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Req
 }
 
 func (c *PodVolumeRestoreReconciler) errorOut(ctx context.Context, pvr *velerov1api.PodVolumeRestore, err error, msg string, log logrus.FieldLogger) (ctrl.Result, error) {
-	c.closeDataPath(ctx, pvr.Name)
 	_ = UpdatePVRStatusToFailed(ctx, c.Client, pvr, errors.WithMessage(err, msg).Error(), c.clock.Now(), log)
 	return ctrl.Result{}, err
 }