From e7fbaa291c4e9c3d883d4b9423c0f1df1d526374 Mon Sep 17 00:00:00 2001
From: Jonathan Innis <jonathan.innis.ji@gmail.com>
Date: Fri, 20 Oct 2023 09:24:41 -0700
Subject: [PATCH] chore: Fix returning non-empty reconcile result and error
 (#618)

---
 pkg/controllers/node/termination/controller.go            | 6 ++++--
 pkg/controllers/nodeclaim/disruption/controller.go        | 5 ++++-
 pkg/controllers/nodeclaim/garbagecollection/controller.go | 5 ++++-
 pkg/controllers/nodeclaim/lifecycle/controller.go         | 5 ++++-
 pkg/controllers/state/informer/daemonset.go               | 6 ++++--
 5 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/pkg/controllers/node/termination/controller.go b/pkg/controllers/node/termination/controller.go
index bc18b96d356c..60a218c66bcf 100644
--- 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
index 67e6c3743a96..325d72adb1c0 100644
--- 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
index 29a339f09e32..e6cb7559bad0 100644
--- 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
index fbd07be12ca5..ba03128a7710 100644
--- 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
index 8f07c0669e5c..2f0c4f073ca3 100644
--- 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 {