fix terminal addons #166
fix terminal addons #166
Conversation
rushmash91
force-pushed
the
feature/addon-retry-logic-for-failed-states
branch
from
4189db8 to
356d989
Compare
|
@rushmash91: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: a-hilaly, rushmash91 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
We should probably do the inFailedState check in delta..if not, sdkUpdate will never be triggered.
To trigger sdkUpdate there needs to be a diff in "spec".
| // If addon is in failed state, we need to force an update regardless of delta | ||
| if inFailedState { | ||
| msg := "Addon is in '" + *latest.ko.Status.Status + "' status, attempting recovery" | ||
| ackcondition.SetSynced(latest, corev1.ConditionFalse, &msg, nil) | ||
| } |
There was a problem hiding this comment.
we can add this one in sdkFind as well
| // If addon is in failed state, always attempt update to recover | ||
| // Otherwise, check if there are differences to update | ||
| if !inFailedState && !delta.DifferentExcept("Spec.Tags") { |
There was a problem hiding this comment.
we probably won't need inFailedState check here if we add a new diff in delta
| @@ -101,6 +99,16 @@ func addonHasTerminalStatus(r *resource) bool { | |||
| return false | |||
| } | |||
|
|
|||
| // addonInFailedState returns true if the supplied addon is in a failed state | |||
| // that requires retry (CREATE_FAILED or UPDATE_FAILED) | |||
| func addonInFailedState(r *resource) bool { | |||
There was a problem hiding this comment.
nit:
| func addonInFailedState(r *resource) bool { | |
| func addonHasFailed(r *resource) bool { |
| if r.ko.Status.Status == nil { | ||
| return false | ||
| } | ||
| cs := *r.ko.Status.Status |
There was a problem hiding this comment.
nit: cs isn't a very descriptive name. Maybe something like addonStatus?
fixes aws-controllers-k8s/community#2569
Description of changes:
Fixes addon error handling by removing
CREATE_FAILEDandUPDATE_FAILEDfrom terminal statuses and adding retry recovery. The controller now detects when addons are in failed states and forcesUpdateAddoncalls regardless of spec changes. This ensures addons can transition from failed states back to ACTIVE through continuous reconciliation attempts.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.