Fix returning non-empty reconcile result and error

aws · Oct 20, 2023 · 581f8f5 · 581f8f5
1 parent daacea9
commit 581f8f5
Show file tree

Hide file tree

Showing 7 changed files with 20 additions and 13 deletions.
diff --git a/go.mod b/go.mod
@@ -7,7 +7,7 @@ require (
 	github.com/PuerkitoBio/goquery v1.8.1
 	github.com/avast/retry-go v3.0.0+incompatible
 	github.com/aws/aws-sdk-go v1.46.0
-	github.com/aws/karpenter-core v0.31.1-0.20231019191151-73c0fd546f75
+	github.com/aws/karpenter-core v0.31.1-0.20231020162441-e7fbaa291c4e
 	github.com/aws/karpenter/tools/kompat v0.0.0-20231010173459-62c25a3ea85c
 	github.com/go-logr/zapr v1.2.4
 	github.com/imdario/mergo v0.3.16

diff --git a/go.sum b/go.sum
@@ -57,8 +57,8 @@ github.com/avast/retry-go v3.0.0+incompatible h1:4SOWQ7Qs+oroOTQOYnAHqelpCO0biHS
 github.com/avast/retry-go v3.0.0+incompatible/go.mod h1:XtSnn+n/sHqQIpZ10K1qAevBhOOCWBLXXy3hyiqqBrY=
 github.com/aws/aws-sdk-go v1.46.0 h1:Igh7W8P+sA6mXJ9yhreOSweefLapcqekhxQlY1llxcM=
 github.com/aws/aws-sdk-go v1.46.0/go.mod h1:aVsgQcEevwlmQ7qHE9I3h+dtQgpqhFB+i8Phjh7fkwI=
-github.com/aws/karpenter-core v0.31.1-0.20231019191151-73c0fd546f75 h1:YGsJA1sH7xW749onvhczydUZksdMm3PB/AJr8qcZllA=
-github.com/aws/karpenter-core v0.31.1-0.20231019191151-73c0fd546f75/go.mod h1:rb3kp/3cj38tACF6udfpmIvKoQMwirSVoHNlrd66LyE=
+github.com/aws/karpenter-core v0.31.1-0.20231020162441-e7fbaa291c4e h1:7heVq5GV1sMgcFlnD6pNElqmbxKMLHX9kaXm9njZC0Y=
+github.com/aws/karpenter-core v0.31.1-0.20231020162441-e7fbaa291c4e/go.mod h1:rb3kp/3cj38tACF6udfpmIvKoQMwirSVoHNlrd66LyE=
 github.com/aws/karpenter/tools/kompat v0.0.0-20231010173459-62c25a3ea85c h1:oXWwIttmjYLbBKhLazG21aQvpJ3NOOr8IXhCJ/p6e/M=
 github.com/aws/karpenter/tools/kompat v0.0.0-20231010173459-62c25a3ea85c/go.mod h1:l/TIBsaCx/IrOr0Xvlj/cHLOf05QzuQKEZ1hx2XWmfU=
 github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=

diff --git a/pkg/controllers/interruption/controller.go b/pkg/controllers/interruption/controller.go
@@ -114,7 +114,10 @@ func (c *Controller) Reconcile(ctx context.Context, _ reconcile.Request) (reconc
 		}
 		errs[i] = c.deleteMessage(ctx, sqsMessages[i])
 	})
-	return reconcile.Result{}, multierr.Combine(errs...)
+	if err = multierr.Combine(errs...); err != nil {
+		return reconcile.Result{}, err
+	}
+	return reconcile.Result{}, nil
 }
 
 func (c *Controller) Name() string {

diff --git a/pkg/controllers/nodeclaim/garbagecollection/controller.go b/pkg/controllers/nodeclaim/garbagecollection/controller.go
@@ -97,8 +97,11 @@ func (c *Controller) Reconcile(ctx context.Context, _ reconcile.Request) (reconc
 			errs[i] = c.garbageCollect(ctx, managedRetrieved[i], nodeList)
 		}
 	})
+	if err = multierr.Combine(errs...); err != nil {
+		return reconcile.Result{}, err
+	}
 	c.successfulCount++
-	return reconcile.Result{RequeueAfter: lo.Ternary(c.successfulCount <= 20, time.Second*10, time.Minute*2)}, multierr.Combine(errs...)
+	return reconcile.Result{RequeueAfter: lo.Ternary(c.successfulCount <= 20, time.Second*10, time.Minute*2)}, nil
 }
 
 func (c *Controller) garbageCollect(ctx context.Context, nodeClaim *v1beta1.NodeClaim, nodeList *v1.NodeList) error {

diff --git a/pkg/controllers/nodeclaim/link/controller.go b/pkg/controllers/nodeclaim/link/controller.go
@@ -96,8 +96,11 @@ func (c *Controller) Reconcile(ctx context.Context, _ reconcile.Request) (reconc
 	workqueue.ParallelizeUntil(ctx, 100, len(retrieved), func(i int) {
 		errs[i] = c.link(ctx, retrieved[i], machineList.Items)
 	})
+	if err = multierr.Combine(errs...); err != nil {
+		return reconcile.Result{}, err
+	}
 	// Effectively, don't requeue this again once it succeeds
-	return reconcile.Result{RequeueAfter: math.MaxInt64}, multierr.Combine(errs...)
+	return reconcile.Result{RequeueAfter: math.MaxInt64}, nil
 }
 
 func (c *Controller) link(ctx context.Context, retrieved *v1beta1.NodeClaim, existingMachines []v1alpha5.Machine) error {

diff --git a/pkg/controllers/nodeclaim/tagging/controller.go b/pkg/controllers/nodeclaim/tagging/controller.go
@@ -61,30 +61,25 @@ func (c *Controller) Name() string {
 
 func (c *Controller) Reconcile(ctx context.Context, nodeClaim *corev1beta1.NodeClaim) (reconcile.Result, error) {
 	stored := nodeClaim.DeepCopy()
-
 	if !isTaggable(nodeClaim) {
 		return reconcile.Result{}, nil
 	}
-
 	ctx = logging.WithLogger(ctx, logging.FromContext(ctx).With("provider-id", nodeClaim.Status.ProviderID))
 	id, err := utils.ParseInstanceID(nodeClaim.Status.ProviderID)
 	if err != nil {
 		// We don't throw an error here since we don't want to retry until the ProviderID has been updated.
 		logging.FromContext(ctx).Errorf("failed to parse instance ID, %w", err)
 		return reconcile.Result{}, nil
 	}
-
-	if err := c.tagInstance(ctx, nodeClaim, id); err != nil {
+	if err = c.tagInstance(ctx, nodeClaim, id); err != nil {
 		return reconcile.Result{}, cloudprovider.IgnoreNodeClaimNotFoundError(err)
 	}
-
 	nodeClaim.Annotations = lo.Assign(nodeClaim.Annotations, map[string]string{v1beta1.AnnotationInstanceTagged: "true"})
 	if !equality.Semantic.DeepEqual(nodeClaim, stored) {
 		if err := c.kubeClient.Patch(ctx, nodeClaim, client.MergeFrom(stored)); err != nil {
 			return reconcile.Result{}, client.IgnoreNotFound(err)
 		}
 	}
-
 	return reconcile.Result{}, nil
 }
 

diff --git a/pkg/controllers/nodeclass/controller.go b/pkg/controllers/nodeclass/controller.go
@@ -92,7 +92,10 @@ func (c *Controller) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeCl
 			err = multierr.Append(err, client.IgnoreNotFound(patchErr))
 		}
 	}
-	return reconcile.Result{RequeueAfter: 5 * time.Minute}, err
+	if err != nil {
+		return reconcile.Result{}, err
+	}
+	return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil
 }
 
 func (c *Controller) Finalize(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (reconcile.Result, error) {