Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix returning non-empty reconcile result and error #4864

Merged
merged 2 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
5 changes: 4 additions & 1 deletion pkg/controllers/interruption/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 4 additions & 1 deletion pkg/controllers/nodeclaim/garbagecollection/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 4 additions & 1 deletion pkg/controllers/nodeclaim/link/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 1 addition & 6 deletions pkg/controllers/nodeclaim/tagging/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
5 changes: 4 additions & 1 deletion pkg/controllers/nodeclass/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion website/static/_redirects
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@
/v0.32/concepts/settings /v0.32/reference/settings
/v0.32/concepts/threat-model /v0.32/reference/threat-model

# TODO @joinnis: Add redirects for the "docs" version and future versions after v0.32 release
# TODO @joinnis: Add redirects for the "docs" version and future versions after v0.32 release