fix: Add ClusterCIDRReady condition #7965
Conversation
✅ Deploy Preview for karpenter-docs-prod ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -43,9 +41,10 @@ func (n Readiness) Reconcile(ctx context.Context, nodeClass *v1.EC2NodeClass) (r | |||
| // will not be done at startup but instead in a reconcile loop. | |||
| if nodeClass.AMIFamily() == v1.AMIFamilyAL2023 { | |||
| if err := n.launchTemplateProvider.ResolveClusterCIDR(ctx); err != nil { | |||
| nodeClass.StatusConditions().SetFalse(status.ConditionReady, "NodeClassNotReady", "Failed to detect the cluster CIDR") | |||
There was a problem hiding this comment.
I'm suggesting we create a specific condition for Cluster CIDR readiness rather than directly marking the whole NodeClass as NotReady. This way, when the EKS API becomes available again, the validation controller can see that this specific dependency is healthy and allow the NodeClass to recover properly
There was a problem hiding this comment.
I'm more inclined to just scope this into the validation controller and drop this sub-reconciler entirely @AlexeyPetroff What do you think about just doing an additional validation?
There was a problem hiding this comment.
As far as I understand, the decision to do the Cluster CIDR resolution in the reconcile loop was made to support non-EKS clusters, where the CIDR may not be available at the time of creation
There was a problem hiding this comment.
Right, that's correct -- but validation can still be special cased based on the AMIFamily -- I think we should perform the same check, but I'm just suggesting that we remove this reconciler entirely and capture this in validation -- I'd prefer not to create a separate status condition for this
jonathan-innis
added
lifecycle/stale
triage/needs-information
Fixes #7875
Summary
This PR addresses an issue where EC2NodeClass resources using AL2023 AMI family would become stuck in a
NotReadystate after temporary EKS API unavailability. The problem occurs when attempting to resolve Cluster CIDR during reconciliation - if the API call fails, the resource remains in a NotReady state indefinitely, even after the API becomes available again.Changes
Tested by:
NotReadystateDoes this change impact docs?
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.