From dc2f12743b6cf3ebf19dea68ee48d56832fdd9db Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Fri, 3 Feb 2023 12:49:30 -0500 Subject: [PATCH 1/2] Use updated PVB/PVR for patching Failed Phase during startup Use the same pvb/pvr update functions across pkg/controller and pkg/cli/nodeagent for consistency of behavior Signed-off-by: Tiger Kaovilai --- changelogs/unreleased/5830-kaovilai | 1 + pkg/cmd/cli/nodeagent/server.go | 19 +++++++++--------- .../pod_volume_backup_controller.go | 9 ++------- .../pod_volume_restore_controller.go | 7 ++----- pkg/podvolumebackup/util.go | 20 +++++++++++++++++++ pkg/podvolumerestore/util.go | 20 +++++++++++++++++++ 6 files changed, 54 insertions(+), 22 deletions(-) create mode 100644 changelogs/unreleased/5830-kaovilai create mode 100644 pkg/podvolumebackup/util.go create mode 100644 pkg/podvolumerestore/util.go diff --git a/changelogs/unreleased/5830-kaovilai b/changelogs/unreleased/5830-kaovilai new file mode 100644 index 0000000000..5342bb5654 --- /dev/null +++ b/changelogs/unreleased/5830-kaovilai @@ -0,0 +1 @@ +Correct PVB/PVR Failed Phase patching during startup \ No newline at end of file diff --git a/pkg/cmd/cli/nodeagent/server.go b/pkg/cmd/cli/nodeagent/server.go index b154f9bf81..5ff39cca59 100644 --- a/pkg/cmd/cli/nodeagent/server.go +++ b/pkg/cmd/cli/nodeagent/server.go @@ -51,6 +51,8 @@ import ( "github.com/vmware-tanzu/velero/pkg/cmd/util/signals" "github.com/vmware-tanzu/velero/pkg/controller" "github.com/vmware-tanzu/velero/pkg/metrics" + "github.com/vmware-tanzu/velero/pkg/podvolumebackup" + "github.com/vmware-tanzu/velero/pkg/podvolumerestore" "github.com/vmware-tanzu/velero/pkg/util/filesystem" "github.com/vmware-tanzu/velero/pkg/util/logging" ) @@ -303,11 +305,10 @@ func (s *nodeAgentServer) markInProgressPVBsFailed(client ctrlclient.Client) { s.logger.Debugf("the node of podvolumebackup %q is %q, not %q, skip", pvb.GetName(), pvb.Spec.Node, s.nodeName) continue } - original := pvb.DeepCopy() - pvb.Status.Phase = velerov1api.PodVolumeBackupPhaseFailed - pvb.Status.Message = fmt.Sprintf("get a podvolumebackup with status %q during the server starting, mark it as %q", velerov1api.PodVolumeBackupPhaseInProgress, pvb.Status.Phase) - pvb.Status.CompletionTimestamp = &metav1.Time{Time: time.Now()} - if err := client.Patch(s.ctx, &pvbs.Items[i], ctrlclient.MergeFrom(original)); err != nil { + + if err := podvolumebackup.UpdateStatusToFailed(client, s.ctx, &pvbs.Items[i], + fmt.Sprintf("get a podvolumebackup with status %q during the server starting, mark it as %q", velerov1api.PodVolumeBackupPhaseInProgress, velerov1api.PodVolumeBackupPhaseFailed), + time.Now()); err != nil { s.logger.WithError(errors.WithStack(err)).Errorf("failed to patch podvolumebackup %q", pvb.GetName()) continue } @@ -341,11 +342,9 @@ func (s *nodeAgentServer) markInProgressPVRsFailed(client ctrlclient.Client) { continue } - original := pvr.DeepCopy() - pvr.Status.Phase = velerov1api.PodVolumeRestorePhaseFailed - pvr.Status.Message = fmt.Sprintf("get a podvolumerestore with status %q during the server starting, mark it as %q", velerov1api.PodVolumeRestorePhaseInProgress, pvr.Status.Phase) - pvr.Status.CompletionTimestamp = &metav1.Time{Time: time.Now()} - if err := client.Patch(s.ctx, &pvrs.Items[i], ctrlclient.MergeFrom(original)); err != nil { + if err := podvolumerestore.UpdateStatusToFailed(client, s.ctx, &pvrs.Items[i], + fmt.Sprintf("get a podvolumerestore with status %q during the server starting, mark it as %q", velerov1api.PodVolumeRestorePhaseInProgress, velerov1api.PodVolumeRestorePhaseFailed), + time.Now()); err != nil { s.logger.WithError(errors.WithStack(err)).Errorf("failed to patch podvolumerestore %q", pvr.GetName()) continue } diff --git a/pkg/controller/pod_volume_backup_controller.go b/pkg/controller/pod_volume_backup_controller.go index 250435cd02..5aff20571c 100644 --- a/pkg/controller/pod_volume_backup_controller.go +++ b/pkg/controller/pod_volume_backup_controller.go @@ -35,6 +35,7 @@ import ( velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/metrics" "github.com/vmware-tanzu/velero/pkg/podvolume" + "github.com/vmware-tanzu/velero/pkg/podvolumebackup" "github.com/vmware-tanzu/velero/pkg/repository" repokey "github.com/vmware-tanzu/velero/pkg/repository/keys" "github.com/vmware-tanzu/velero/pkg/uploader" @@ -279,16 +280,10 @@ func (r *PodVolumeBackupReconciler) getParentSnapshot(ctx context.Context, log l } func (r *PodVolumeBackupReconciler) updateStatusToFailed(ctx context.Context, pvb *velerov1api.PodVolumeBackup, err error, msg string, log logrus.FieldLogger) (ctrl.Result, error) { - original := pvb.DeepCopy() - pvb.Status.Phase = velerov1api.PodVolumeBackupPhaseFailed - pvb.Status.Message = errors.WithMessage(err, msg).Error() - pvb.Status.CompletionTimestamp = &metav1.Time{Time: r.Clock.Now()} - - if err = r.Client.Patch(ctx, pvb, client.MergeFrom(original)); err != nil { + if err = podvolumebackup.UpdateStatusToFailed(r.Client, ctx, pvb, errors.WithMessage(err, msg).Error(), r.Clock.Now()); err != nil { log.WithError(err).Error("error updating PodVolumeBackup status") return ctrl.Result{}, err } - return ctrl.Result{}, nil } diff --git a/pkg/controller/pod_volume_restore_controller.go b/pkg/controller/pod_volume_restore_controller.go index 5bdaf80390..d5fcabcc1b 100644 --- a/pkg/controller/pod_volume_restore_controller.go +++ b/pkg/controller/pod_volume_restore_controller.go @@ -40,6 +40,7 @@ import ( "github.com/vmware-tanzu/velero/internal/credentials" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/podvolume" + "github.com/vmware-tanzu/velero/pkg/podvolumerestore" "github.com/vmware-tanzu/velero/pkg/repository" repokey "github.com/vmware-tanzu/velero/pkg/repository/keys" "github.com/vmware-tanzu/velero/pkg/restorehelper" @@ -122,11 +123,7 @@ func (c *PodVolumeRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Req } if err = c.processRestore(ctx, pvr, pod, log); err != nil { - original = pvr.DeepCopy() - pvr.Status.Phase = velerov1api.PodVolumeRestorePhaseFailed - pvr.Status.Message = err.Error() - pvr.Status.CompletionTimestamp = &metav1.Time{Time: c.clock.Now()} - if e := c.Patch(ctx, pvr, client.MergeFrom(original)); e != nil { + if e := podvolumerestore.UpdateStatusToFailed(c, ctx, pvr, err.Error(), c.clock.Now()); e != nil { log.WithError(err).Error("Unable to update status to failed") } diff --git a/pkg/podvolumebackup/util.go b/pkg/podvolumebackup/util.go new file mode 100644 index 0000000000..2d4f171de0 --- /dev/null +++ b/pkg/podvolumebackup/util.go @@ -0,0 +1,20 @@ +package podvolumebackup + +import ( + "context" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + pkgclient "sigs.k8s.io/controller-runtime/pkg/client" + + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" +) + +func UpdateStatusToFailed(client pkgclient.Client, ctx context.Context, pvb *velerov1api.PodVolumeBackup, errString string, time time.Time) error { + original := pvb.DeepCopy() + pvb.Status.Phase = velerov1api.PodVolumeBackupPhaseFailed + pvb.Status.Message = errString + pvb.Status.CompletionTimestamp = &metav1.Time{Time: time} + + return client.Patch(ctx, pvb, pkgclient.MergeFrom(original)) +} diff --git a/pkg/podvolumerestore/util.go b/pkg/podvolumerestore/util.go new file mode 100644 index 0000000000..9723997b48 --- /dev/null +++ b/pkg/podvolumerestore/util.go @@ -0,0 +1,20 @@ +package podvolumerestore + +import ( + "context" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + pkgclient "sigs.k8s.io/controller-runtime/pkg/client" + + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" +) + +func UpdateStatusToFailed(client pkgclient.Client, ctx context.Context, pvr *velerov1api.PodVolumeRestore, errString string, time time.Time) error { + original := pvr.DeepCopy() + pvr.Status.Phase = velerov1api.PodVolumeRestorePhaseFailed + pvr.Status.Message = errString + pvr.Status.CompletionTimestamp = &metav1.Time{Time: time} + + return client.Patch(ctx, pvr, pkgclient.MergeFrom(original)) +} From 820fe8a559667028028592eb52ea4f9905f4f570 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Tue, 7 Feb 2023 10:16:54 -0500 Subject: [PATCH 2/2] move UpdatePVXStatusToFailed to controller pkg Signed-off-by: Tiger Kaovilai --- pkg/cmd/cli/nodeagent/server.go | 6 ++---- .../pod_volume_backup_controller.go | 12 +++++++++-- .../pod_volume_restore_controller.go | 13 ++++++++++-- pkg/podvolumebackup/util.go | 20 ------------------- pkg/podvolumerestore/util.go | 20 ------------------- 5 files changed, 23 insertions(+), 48 deletions(-) delete mode 100644 pkg/podvolumebackup/util.go delete mode 100644 pkg/podvolumerestore/util.go diff --git a/pkg/cmd/cli/nodeagent/server.go b/pkg/cmd/cli/nodeagent/server.go index 5ff39cca59..ecbf380a66 100644 --- a/pkg/cmd/cli/nodeagent/server.go +++ b/pkg/cmd/cli/nodeagent/server.go @@ -51,8 +51,6 @@ import ( "github.com/vmware-tanzu/velero/pkg/cmd/util/signals" "github.com/vmware-tanzu/velero/pkg/controller" "github.com/vmware-tanzu/velero/pkg/metrics" - "github.com/vmware-tanzu/velero/pkg/podvolumebackup" - "github.com/vmware-tanzu/velero/pkg/podvolumerestore" "github.com/vmware-tanzu/velero/pkg/util/filesystem" "github.com/vmware-tanzu/velero/pkg/util/logging" ) @@ -306,7 +304,7 @@ func (s *nodeAgentServer) markInProgressPVBsFailed(client ctrlclient.Client) { continue } - if err := podvolumebackup.UpdateStatusToFailed(client, s.ctx, &pvbs.Items[i], + if err := controller.UpdatePVBStatusToFailed(client, s.ctx, &pvbs.Items[i], fmt.Sprintf("get a podvolumebackup with status %q during the server starting, mark it as %q", velerov1api.PodVolumeBackupPhaseInProgress, velerov1api.PodVolumeBackupPhaseFailed), time.Now()); err != nil { s.logger.WithError(errors.WithStack(err)).Errorf("failed to patch podvolumebackup %q", pvb.GetName()) @@ -342,7 +340,7 @@ func (s *nodeAgentServer) markInProgressPVRsFailed(client ctrlclient.Client) { continue } - if err := podvolumerestore.UpdateStatusToFailed(client, s.ctx, &pvrs.Items[i], + if err := controller.UpdatePVRStatusToFailed(client, s.ctx, &pvrs.Items[i], fmt.Sprintf("get a podvolumerestore with status %q during the server starting, mark it as %q", velerov1api.PodVolumeRestorePhaseInProgress, velerov1api.PodVolumeRestorePhaseFailed), time.Now()); err != nil { s.logger.WithError(errors.WithStack(err)).Errorf("failed to patch podvolumerestore %q", pvr.GetName()) diff --git a/pkg/controller/pod_volume_backup_controller.go b/pkg/controller/pod_volume_backup_controller.go index 5aff20571c..3575e6dc54 100644 --- a/pkg/controller/pod_volume_backup_controller.go +++ b/pkg/controller/pod_volume_backup_controller.go @@ -35,7 +35,6 @@ import ( velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/metrics" "github.com/vmware-tanzu/velero/pkg/podvolume" - "github.com/vmware-tanzu/velero/pkg/podvolumebackup" "github.com/vmware-tanzu/velero/pkg/repository" repokey "github.com/vmware-tanzu/velero/pkg/repository/keys" "github.com/vmware-tanzu/velero/pkg/uploader" @@ -280,13 +279,22 @@ func (r *PodVolumeBackupReconciler) getParentSnapshot(ctx context.Context, log l } func (r *PodVolumeBackupReconciler) updateStatusToFailed(ctx context.Context, pvb *velerov1api.PodVolumeBackup, err error, msg string, log logrus.FieldLogger) (ctrl.Result, error) { - if err = podvolumebackup.UpdateStatusToFailed(r.Client, ctx, pvb, errors.WithMessage(err, msg).Error(), r.Clock.Now()); err != nil { + if err = UpdatePVBStatusToFailed(r.Client, ctx, pvb, errors.WithMessage(err, msg).Error(), r.Clock.Now()); err != nil { log.WithError(err).Error("error updating PodVolumeBackup status") return ctrl.Result{}, err } return ctrl.Result{}, nil } +func UpdatePVBStatusToFailed(c client.Client, ctx context.Context, pvb *velerov1api.PodVolumeBackup, errString string, time time.Time) error { + original := pvb.DeepCopy() + pvb.Status.Phase = velerov1api.PodVolumeBackupPhaseFailed + pvb.Status.Message = errString + pvb.Status.CompletionTimestamp = &metav1.Time{Time: time} + + return c.Patch(ctx, pvb, client.MergeFrom(original)) +} + func (r *PodVolumeBackupReconciler) NewBackupProgressUpdater(pvb *velerov1api.PodVolumeBackup, log logrus.FieldLogger, ctx context.Context) *BackupProgressUpdater { return &BackupProgressUpdater{pvb, log, ctx, r.Client} } diff --git a/pkg/controller/pod_volume_restore_controller.go b/pkg/controller/pod_volume_restore_controller.go index d5fcabcc1b..5b717ed158 100644 --- a/pkg/controller/pod_volume_restore_controller.go +++ b/pkg/controller/pod_volume_restore_controller.go @@ -22,6 +22,7 @@ import ( "io/ioutil" "os" "path/filepath" + "time" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -40,7 +41,6 @@ import ( "github.com/vmware-tanzu/velero/internal/credentials" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/podvolume" - "github.com/vmware-tanzu/velero/pkg/podvolumerestore" "github.com/vmware-tanzu/velero/pkg/repository" repokey "github.com/vmware-tanzu/velero/pkg/repository/keys" "github.com/vmware-tanzu/velero/pkg/restorehelper" @@ -123,7 +123,7 @@ func (c *PodVolumeRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Req } if err = c.processRestore(ctx, pvr, pod, log); err != nil { - if e := podvolumerestore.UpdateStatusToFailed(c, ctx, pvr, err.Error(), c.clock.Now()); e != nil { + if e := UpdatePVRStatusToFailed(c, ctx, pvr, err.Error(), c.clock.Now()); e != nil { log.WithError(err).Error("Unable to update status to failed") } @@ -142,6 +142,15 @@ func (c *PodVolumeRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Req return ctrl.Result{}, nil } +func UpdatePVRStatusToFailed(c client.Client, ctx context.Context, pvr *velerov1api.PodVolumeRestore, errString string, time time.Time) error { + original := pvr.DeepCopy() + pvr.Status.Phase = velerov1api.PodVolumeRestorePhaseFailed + pvr.Status.Message = errString + pvr.Status.CompletionTimestamp = &metav1.Time{Time: time} + + return c.Patch(ctx, pvr, client.MergeFrom(original)) +} + func (c *PodVolumeRestoreReconciler) shouldProcess(ctx context.Context, log logrus.FieldLogger, pvr *velerov1api.PodVolumeRestore) (bool, *corev1api.Pod, error) { if !isPVRNew(pvr) { log.Debug("PodVolumeRestore is not new, skip") diff --git a/pkg/podvolumebackup/util.go b/pkg/podvolumebackup/util.go deleted file mode 100644 index 2d4f171de0..0000000000 --- a/pkg/podvolumebackup/util.go +++ /dev/null @@ -1,20 +0,0 @@ -package podvolumebackup - -import ( - "context" - "time" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - pkgclient "sigs.k8s.io/controller-runtime/pkg/client" - - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" -) - -func UpdateStatusToFailed(client pkgclient.Client, ctx context.Context, pvb *velerov1api.PodVolumeBackup, errString string, time time.Time) error { - original := pvb.DeepCopy() - pvb.Status.Phase = velerov1api.PodVolumeBackupPhaseFailed - pvb.Status.Message = errString - pvb.Status.CompletionTimestamp = &metav1.Time{Time: time} - - return client.Patch(ctx, pvb, pkgclient.MergeFrom(original)) -} diff --git a/pkg/podvolumerestore/util.go b/pkg/podvolumerestore/util.go deleted file mode 100644 index 9723997b48..0000000000 --- a/pkg/podvolumerestore/util.go +++ /dev/null @@ -1,20 +0,0 @@ -package podvolumerestore - -import ( - "context" - "time" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - pkgclient "sigs.k8s.io/controller-runtime/pkg/client" - - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" -) - -func UpdateStatusToFailed(client pkgclient.Client, ctx context.Context, pvr *velerov1api.PodVolumeRestore, errString string, time time.Time) error { - original := pvr.DeepCopy() - pvr.Status.Phase = velerov1api.PodVolumeRestorePhaseFailed - pvr.Status.Message = errString - pvr.Status.CompletionTimestamp = &metav1.Time{Time: time} - - return client.Patch(ctx, pvr, pkgclient.MergeFrom(original)) -}