-
Notifications
You must be signed in to change notification settings - Fork 957
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: Correct the AMI selector label requirements for NVIDIA and Neuron variants #6976
Conversation
✅ Deploy Preview for karpenter-docs-prod canceled.
|
Pull Request Test Coverage Report for Build 10950107138Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
Looks like this slipped through because our currently supported AMIs don't differentiate between Nvidia and Neuron, so this swap doesn't change the end result. I'd be happy to merge and take a test for that case as a follow-up (separate Nvidia and Neuron AMIs, same AMI family), but also happy to take one here if you wanted to add one.
I'm dreadfully awful with go 😅 - give me 24 hours and let me see if I can add in tests for this |
@jmdeal I tried a naive approach to check that the |
No problem, I'll go ahead and merge a test into this PR later today and we can get this merged. |
8eff7eb
to
6879239
Compare
6879239
to
08e840a
Compare
08e840a
to
b70ea2a
Compare
b70ea2a
to
3f49294
Compare
Sorry for the delay, made a couple additional changes and this should be good to go from my perspective. Since I've touched the PR though I shouldn't be the one to approve, I'm going to get review from someone else on the team. We should be able to merge this shortly |
3f49294
to
00d59a0
Compare
…n variants (aws#6976) Co-authored-by: Jason Deal <[email protected]>
…n variants (#6976) Co-authored-by: Jason Deal <[email protected]>
…n variants (#6976) Co-authored-by: Jason Deal <[email protected]>
Fixes #N/A
Description
karpenter.sh/instance-gpu-count
karpenter.sh/instance-accelerator-count
How was this change tested?
make test
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.