From 37e15f488ddaf2725ccefdf812478563acf7c080 Mon Sep 17 00:00:00 2001 From: Yuvaraj Kakaraparthi Date: Wed, 29 Jun 2022 14:02:27 -0700 Subject: [PATCH] improved error wrapping for hook tracking --- .../topology/cluster/cluster_controller.go | 2 +- .../topology/cluster/desired_state.go | 5 ++-- .../topology/cluster/reconcile_state.go | 3 +- internal/hooks/tracking.go | 30 ++++++++++--------- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/internal/controllers/topology/cluster/cluster_controller.go b/internal/controllers/topology/cluster/cluster_controller.go index 67b2175c8264..f6746e2f5320 100644 --- a/internal/controllers/topology/cluster/cluster_controller.go +++ b/internal/controllers/topology/cluster/cluster_controller.go @@ -353,7 +353,7 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Clu // The BeforeClusterDelete hook returned a non-blocking response. Now the cluster is ready to be deleted. // Lets mark the cluster as `ok-to-delete` if err := hooks.MarkAsOkToDelete(ctx, r.Client, cluster); err != nil { - return ctrl.Result{}, errors.Wrapf(err, "failed to mark %s/%s cluster as ok to delete", cluster.Namespace, cluster.Name) + return ctrl.Result{}, err } } } diff --git a/internal/controllers/topology/cluster/desired_state.go b/internal/controllers/topology/cluster/desired_state.go index a9048c65ce09..1c607d6263fa 100644 --- a/internal/controllers/topology/cluster/desired_state.go +++ b/internal/controllers/topology/cluster/desired_state.go @@ -31,7 +31,6 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/controllers/external" - runtimecatalog "sigs.k8s.io/cluster-api/exp/runtime/catalog" runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/contract" @@ -338,7 +337,7 @@ func (r *Reconciler) computeControlPlaneVersion(ctx context.Context, s *scope.Sc s.UpgradeTracker.MachineDeployments.HoldUpgrades(true) } else { if err := hooks.MarkAsDone(ctx, r.Client, s.Current.Cluster, runtimehooksv1.AfterControlPlaneUpgrade); err != nil { - return "", errors.Wrapf(err, "failed to remove the %s hook from pending hooks tracker", runtimecatalog.HookName(runtimehooksv1.AfterControlPlaneUpgrade)) + return "", err } } } @@ -390,7 +389,7 @@ func (r *Reconciler) computeControlPlaneVersion(ctx context.Context, s *scope.Sc // We are picking up the new version here. // Track the intent of calling the AfterControlPlaneUpgrade and the AfterClusterUpgrade hooks once we are done with the upgrade. if err := hooks.MarkAsPending(ctx, r.Client, s.Current.Cluster, runtimehooksv1.AfterControlPlaneUpgrade, runtimehooksv1.AfterClusterUpgrade); err != nil { - return "", errors.Wrapf(err, "failed to mark the %s hook as pending", []string{runtimecatalog.HookName(runtimehooksv1.AfterControlPlaneUpgrade), runtimecatalog.HookName(runtimehooksv1.AfterClusterUpgrade)}) + return "", err } } diff --git a/internal/controllers/topology/cluster/reconcile_state.go b/internal/controllers/topology/cluster/reconcile_state.go index 0874c2058561..a4a6eaf3f22b 100644 --- a/internal/controllers/topology/cluster/reconcile_state.go +++ b/internal/controllers/topology/cluster/reconcile_state.go @@ -31,7 +31,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - runtimecatalog "sigs.k8s.io/cluster-api/exp/runtime/catalog" runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1" "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/contract" @@ -204,7 +203,7 @@ func (r *Reconciler) callAfterControlPlaneInitialized(ctx context.Context, s *sc // If the cluster topology is being created then track to intent to call the AfterControlPlaneInitialized hook so that we can call it later. if s.Current.Cluster.Spec.InfrastructureRef == nil && s.Current.Cluster.Spec.ControlPlaneRef == nil { if err := hooks.MarkAsPending(ctx, r.Client, s.Current.Cluster, runtimehooksv1.AfterControlPlaneInitialized); err != nil { - return errors.Wrapf(err, "failed to remove the %s hook from pending hooks tracker", runtimecatalog.HookName(runtimehooksv1.AfterControlPlaneInitialized)) + return err } } diff --git a/internal/hooks/tracking.go b/internal/hooks/tracking.go index fc6effbab9f1..d9987f95f960 100644 --- a/internal/hooks/tracking.go +++ b/internal/hooks/tracking.go @@ -34,16 +34,17 @@ import ( // MarkAsPending adds to the object's PendingHooksAnnotation the intent to execute a hook after an operation completes. // Usually this function is called when an operation is starting in order to track the intent to call an After hook later in the process. func MarkAsPending(ctx context.Context, c client.Client, obj client.Object, hooks ...runtimecatalog.Hook) error { + hookNames := []string{} + for _, hook := range hooks { + hookNames = append(hookNames, runtimecatalog.HookName(hook)) + } + patchHelper, err := patch.NewHelper(obj, c) if err != nil { - return errors.Wrapf(err, "failed to create patch helper for %s", tlog.KObj{Obj: obj}) + return errors.Wrapf(err, "failed to mark %q hook(s) as pending: failed to create patch helper for %s", strings.Join(hookNames, ","), tlog.KObj{Obj: obj}) } // Read the annotation of the objects and add the hook to the comma separated list - hookNames := []string{} - for _, hook := range hooks { - hookNames = append(hookNames, runtimecatalog.HookName(hook)) - } annotations := obj.GetAnnotations() if annotations == nil { annotations = map[string]string{} @@ -52,7 +53,7 @@ func MarkAsPending(ctx context.Context, c client.Client, obj client.Object, hook obj.SetAnnotations(annotations) if err := patchHelper.Patch(ctx, obj); err != nil { - return errors.Wrapf(err, "failed to patch %s", tlog.KObj{Obj: obj}) + return errors.Wrapf(err, "failed to mark %q hook(s) as pending: failed to patch %s", strings.Join(hookNames, ","), tlog.KObj{Obj: obj}) } return nil @@ -72,16 +73,17 @@ func IsPending(hook runtimecatalog.Hook, obj client.Object) bool { // Usually this func is called after all the registered extensions for the Hook returned an answer without requests // to hold on to the object's lifecycle (retryAfterSeconds). func MarkAsDone(ctx context.Context, c client.Client, obj client.Object, hooks ...runtimecatalog.Hook) error { + hookNames := []string{} + for _, hook := range hooks { + hookNames = append(hookNames, runtimecatalog.HookName(hook)) + } + patchHelper, err := patch.NewHelper(obj, c) if err != nil { - return errors.Wrapf(err, "failed to create patch helper for %s", tlog.KObj{Obj: obj}) + return errors.Wrapf(err, "failed to mark %q hook(s) as done: failed to create patch helper for %s", strings.Join(hookNames, ","), tlog.KObj{Obj: obj}) } // Read the annotation of the objects and add the hook to the comma separated list - hookNames := []string{} - for _, hook := range hooks { - hookNames = append(hookNames, runtimecatalog.HookName(hook)) - } annotations := obj.GetAnnotations() if annotations == nil { annotations = map[string]string{} @@ -93,7 +95,7 @@ func MarkAsDone(ctx context.Context, c client.Client, obj client.Object, hooks . obj.SetAnnotations(annotations) if err := patchHelper.Patch(ctx, obj); err != nil { - return errors.Wrapf(err, "failed to patch %s", tlog.KObj{Obj: obj}) + return errors.Wrapf(err, "failed to mark %q hook(s) as done: failed to patch %s", strings.Join(hookNames, ","), tlog.KObj{Obj: obj}) } return nil @@ -115,7 +117,7 @@ func IsOkToDelete(obj client.Object) bool { func MarkAsOkToDelete(ctx context.Context, c client.Client, obj client.Object) error { patchHelper, err := patch.NewHelper(obj, c) if err != nil { - return errors.Wrapf(err, "failed to create patch helper for %s", tlog.KObj{Obj: obj}) + return errors.Wrapf(err, "failed to mark %s as ok to delete: failed to create patch helper", tlog.KObj{Obj: obj}) } annotations := obj.GetAnnotations() @@ -126,7 +128,7 @@ func MarkAsOkToDelete(ctx context.Context, c client.Client, obj client.Object) e obj.SetAnnotations(annotations) if err := patchHelper.Patch(ctx, obj); err != nil { - return errors.Wrapf(err, "failed to patch %s", tlog.KObj{Obj: obj}) + return errors.Wrapf(err, "failed to mark %s as ok to delete: failed to patch", tlog.KObj{Obj: obj}) } return nil