chore: Fix returning non-empty reconcile result and error (#618)

kubernetes-sigs · Oct 20, 2023 · e7fbaa2 · e7fbaa2
1 parent 73c0fd5
commit e7fbaa2
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 7 deletions.
diff --git a/pkg/controllers/node/termination/controller.go b/pkg/controllers/node/termination/controller.go
@@ -99,11 +99,13 @@ func (c *Controller) Finalize(ctx context.Context, node *v1.Node) (reconcile.Res
 		}
 		return reconcile.Result{RequeueAfter: 1 * time.Second}, nil
 	}
-
 	if err := c.cloudProvider.Delete(ctx, nodeclaimutil.NewFromNode(node)); cloudprovider.IgnoreNodeClaimNotFoundError(err) != nil {
 		return reconcile.Result{}, fmt.Errorf("terminating cloudprovider instance, %w", err)
 	}
-	return reconcile.Result{}, c.removeFinalizer(ctx, node)
+	if err := c.removeFinalizer(ctx, node); err != nil {
+		return reconcile.Result{}, err
+	}
+	return reconcile.Result{}, nil
 }
 
 func (c *Controller) deleteAllMachines(ctx context.Context, node *v1.Node) error {

diff --git a/pkg/controllers/nodeclaim/disruption/controller.go b/pkg/controllers/nodeclaim/disruption/controller.go
@@ -96,7 +96,10 @@ func (c *Controller) Reconcile(ctx context.Context, nodeClaim *v1beta1.NodeClaim
 			return reconcile.Result{}, client.IgnoreNotFound(err)
 		}
 	}
-	return result.Min(results...), errs
+	if errs != nil {
+		return reconcile.Result{}, errs
+	}
+	return result.Min(results...), nil
 }
 
 var _ corecontroller.TypedController[*v1beta1.NodeClaim] = (*NodeClaimController)(nil)

diff --git a/pkg/controllers/nodeclaim/garbagecollection/controller.go b/pkg/controllers/nodeclaim/garbagecollection/controller.go
@@ -89,7 +89,10 @@ func (c *Controller) Reconcile(ctx context.Context, _ reconcile.Request) (reconc
 			Debugf("garbage collecting %s with no cloudprovider representation", lo.Ternary(nodeClaims[i].IsMachine, "machine", "nodeclaim"))
 		nodeclaimutil.TerminatedCounter(nodeClaims[i], "garbage_collected").Inc()
 	})
-	return reconcile.Result{RequeueAfter: time.Minute * 2}, multierr.Combine(errs...)
+	if err = multierr.Combine(errs...); err != nil {
+		return reconcile.Result{}, err
+	}
+	return reconcile.Result{RequeueAfter: time.Minute * 2}, nil
 }
 
 func (c *Controller) Builder(_ context.Context, m manager.Manager) corecontroller.Builder {

diff --git a/pkg/controllers/nodeclaim/lifecycle/controller.go b/pkg/controllers/nodeclaim/lifecycle/controller.go
@@ -116,7 +116,10 @@ func (c *Controller) Reconcile(ctx context.Context, nodeClaim *v1beta1.NodeClaim
 		// USE CAUTION when determining whether to increase this timeout or remove this line
 		time.Sleep(time.Second)
 	}
-	return result.Min(results...), errs
+	if errs != nil {
+		return reconcile.Result{}, errs
+	}
+	return result.Min(results...), nil
 }
 
 var _ corecontroller.TypedController[*v1beta1.NodeClaim] = (*NodeClaimController)(nil)

diff --git a/pkg/controllers/state/informer/daemonset.go b/pkg/controllers/state/informer/daemonset.go
@@ -58,8 +58,10 @@ func (c *Controller) Reconcile(ctx context.Context, req reconcile.Request) (reco
 		}
 		return reconcile.Result{}, client.IgnoreNotFound(err)
 	}
-
-	return reconcile.Result{RequeueAfter: time.Minute}, c.cluster.UpdateDaemonSet(ctx, &daemonSet)
+	if err := c.cluster.UpdateDaemonSet(ctx, &daemonSet); err != nil {
+		return reconcile.Result{}, err
+	}
+	return reconcile.Result{RequeueAfter: time.Minute}, nil
 }
 
 func (c *Controller) Builder(_ context.Context, m manager.Manager) corecontroller.Builder {