-
Notifications
You must be signed in to change notification settings - Fork 213
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
minor change: Replace the hardcoded VM SKU #5910
base: master
Are you sure you want to change the base?
Conversation
@@ -619,6 +619,9 @@ func nvidiaDevicePluginDaemonSet() *appsv1.DaemonSet { | |||
}, | |||
}, | |||
Spec: corev1.PodSpec{ | |||
NodeSelector: map[string]string{ |
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.
Is this change necessary for this PR?
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.
I think it is better to have. the "nvidia-device-plugin" can not be pulled on windows node, so if you check the pods on the AKS cluster being tested, you will see a failing pod. it does not fail the e2e test though, but I think we better to fix it.
421a2ff
to
35050f8
Compare
35050f8
to
71f7efa
Compare
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.
The PR looks good. Given it’s the linux E2E tests failing, we should get someone from the Linux team to comment - I’m not sure if these tests are known to fail in master or not. Your change is super unlikely to have caused any to fail, but better safe than sorry.
@cameronmeissner would you please help to take a look at this? |
What type of PR is this?
/kind bug
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Replace the hardcoded VM SKU with the one configured in the config file.
Make the "nvidia-device-plugin" deployed in linux node only. It is not able to deploy to windows node, although it will not fail the test
Requirements:
Special notes for your reviewer:
Release note: