Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 8394: move closeDataPath outside callbacks #8395

Merged
merged 1 commit into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/unreleased/8395-Lyndon-Li
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix issue #8394, don't call closeDataPath in VGDP callbacks, otherwise, the VGDP cleanup will hang
6 changes: 5 additions & 1 deletion pkg/controller/pod_volume_backup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@
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)

Check warning on line 144 in pkg/controller/pod_volume_backup_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/pod_volume_backup_controller.go#L144

Added line #L144 was not covered by tests
return r.errorOut(ctx, &pvb, err, "error updating PodVolumeBackup status", log)
}

Expand All @@ -150,11 +151,13 @@
Name: pvb.Spec.Pod.Name,
}
if err := r.Client.Get(ctx, podNamespacedName, &pod); err != nil {
r.closeDataPath(ctx, pvb.Name)

Check warning on line 154 in pkg/controller/pod_volume_backup_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/pod_volume_backup_controller.go#L154

Added line #L154 was not covered by tests
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)

Check warning on line 160 in pkg/controller/pod_volume_backup_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/pod_volume_backup_controller.go#L160

Added line #L160 was not covered by tests
return r.errorOut(ctx, &pvb, err, "error exposing host path for pod volume", log)
}

Expand All @@ -169,6 +172,7 @@
RepositoryEnsurer: r.repositoryEnsurer,
CredentialGetter: r.credentialGetter,
}); err != nil {
r.closeDataPath(ctx, pvb.Name)

Check warning on line 175 in pkg/controller/pod_volume_backup_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/pod_volume_backup_controller.go#L175

Added line #L175 was not covered by tests
return r.errorOut(ctx, &pvb, err, "error to initialize data path", log)
}

Expand All @@ -193,6 +197,7 @@
ForceFull: false,
Tags: pvb.Spec.Tags,
}); err != nil {
r.closeDataPath(ctx, pvb.Name)

Check warning on line 200 in pkg/controller/pod_volume_backup_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/pod_volume_backup_controller.go#L200

Added line #L200 was not covered by tests
return r.errorOut(ctx, &pvb, err, "error starting data path backup", log)
}

Expand Down Expand Up @@ -361,7 +366,6 @@
}

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
Expand Down
5 changes: 4 additions & 1 deletion pkg/controller/pod_volume_restore_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,13 @@
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)

Check warning on line 134 in pkg/controller/pod_volume_restore_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/pod_volume_restore_controller.go#L134

Added line #L134 was not covered by tests
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)

Check warning on line 140 in pkg/controller/pod_volume_restore_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/pod_volume_restore_controller.go#L140

Added line #L140 was not covered by tests
return c.errorOut(ctx, pvr, err, "error exposing host path for pod volume", log)
}

Expand All @@ -150,10 +152,12 @@
RepositoryEnsurer: c.repositoryEnsurer,
CredentialGetter: c.credentialGetter,
}); err != nil {
c.closeDataPath(ctx, pvr.Name)

Check warning on line 155 in pkg/controller/pod_volume_restore_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/pod_volume_restore_controller.go#L155

Added line #L155 was not covered by tests
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)

Check warning on line 160 in pkg/controller/pod_volume_restore_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/pod_volume_restore_controller.go#L160

Added line #L160 was not covered by tests
return c.errorOut(ctx, pvr, err, "error starting data path restore", log)
}

Expand All @@ -163,7 +167,6 @@
}

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
}
Expand Down
Loading