Skip to content

chore: classify controller errors as terminal vs non-terminal - #9542

Open
ketanjani21 wants to merge 1 commit into
aws:mainfrom
ketanjani21:fix/classify-controller-errors-7837
Open

chore: classify controller errors as terminal vs non-terminal#9542
ketanjani21 wants to merge 1 commit into
aws:mainfrom
ketanjani21:fix/classify-controller-errors-7837

Conversation

@ketanjani21

Copy link
Copy Markdown

Fixes #7837

Description

Reconcilers for AMI, CapacityReservation, InstanceProfile, SecurityGroup, and Subnet classes leave their Ready status condition in Unknown on every AWS API error, even when the error is not retryable (IAM misconfiguration, account service-limit hit, etc.). Users have no user-visible signal for the "needs operator action" cases, and the reconciler retries indefinitely.

This PR adds a small ClassifyError helper in pkg/errors that inspects AWS API errors, matches known terminal codes, and returns a (reason, message, retryable) triple. Each of the five nodeclass reconcilers now calls it in its error path. When the error is terminal, the corresponding Ready condition is set to False with the classified reason and message. Transient or unrecognized errors keep the existing behavior (return error, retry).

Terminal codes covered in this pass:

  • UnauthorizedOperation / AccessDenied / AccessDeniedException / AuthFailure — IAM misconfiguration; retrying without operator action will not succeed.
  • LimitExceeded — account service-limit reached; requires a quota increase or resource cleanup.

The list is intentionally conservative; more codes can be added in follow-ups as they surface.

Reference to prior work

Supersedes #7903 (@saku3), which was closed without merging in January 2026. I confirmed with @saku3 on 2026-08-22 that they are not actively working on this. This PR takes a similar approach in a smaller, more targeted form.

How was this change tested?

  • Added table-driven tests in pkg/errors/errors_test.go covering: nil input, non-AWS errors, each of the terminal codes returning retryable=false, and unknown codes returning retryable=true.
  • go build ./... and go vet ./pkg/... clean.
  • Local review of each of the five reconciler call sites to confirm the pattern is applied consistently and the SetFalse call precedes the existing return err (so retries still happen when the code drops the terminal branch on a code we did not classify).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ketanjani21
ketanjani21 force-pushed the fix/classify-controller-errors-7837 branch from c7b4978 to 9881d0f Compare August 25, 2026 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Classify InstanceProfile creation failures/Subnet failures/SecurityGroup failures as terminal or non-terminal

1 participant