feat: Added check to filter supported nodes by ami's arch type #7933
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: gnana997 <[email protected]>
✅ Deploy Preview for karpenter-docs-prod ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
gnana997
force-pushed
the
ami-req-node-class
branch
from
325bc21 to
e989255
Compare
gnana997
changed the title
Signed-off-by: gnana997 <[email protected]>
gnana997
force-pushed
the
ami-req-node-class
branch
from
e989255 to
a7cad45
Compare
| for _, ami := range amis { | ||
| if err := instanceType.Requirements.Compatible( | ||
| scheduling.NewNodeSelectorRequirements(ami.Requirements...), | ||
| scheduling.AllowUndefinedWellKnownLabels, |
There was a problem hiding this comment.
Do we allow our Unknown WellKnown labels to be compatible here? Can we check the other places we are doing instance type compatibility with AMIs?
Also, I wonder if we can just re-leverage MapInstanceTypes and then just pull-out the instance types that we support
There was a problem hiding this comment.
@gnana997 Any luck with this update?
| // Filter instance types that don't have compatible AMIs to prevent architecture mismatches | ||
| filteredInstanceTypes := amifamily.FilterInstanceTypesByAMICompatibility(instanceTypesWithOfferings, nodeClass.Status.AMIs) | ||
| if len(filteredInstanceTypes) == 0 { | ||
| log.FromContext(ctx).WithValues( |
There was a problem hiding this comment.
I don't think this is a log -- this is an error IMO
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #7893
Description
Added a check to filter the nodes from the node pool based on the required architecture for the provided AMI in the Node Class
How was this change tested?
Working on adding test cases to check the added filtering check.
Does 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.