-
Notifications
You must be signed in to change notification settings - Fork 583
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
Fixing allPossibleMigStrategiesAreNone helm chart helper. #675
Fixing allPossibleMigStrategiesAreNone helm chart helper. #675
Conversation
0e6b4c5
to
881e398
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.
Sorry, these were pendings.
@@ -130,7 +130,7 @@ Check if migStrategy (from all possible configurations) is "none" | |||
{{- if ne .Values.migStrategy "none" -}} | |||
{{- $result = false -}} | |||
{{- end -}} | |||
{{- else if ne (include "nvidia-device-plugin.configMapName" .) "true" -}} | |||
{{- else if ne (include "nvidia-device-plugin.configMapName" .) "" -}} |
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.
Question: Would it not be better to use nvidia-device-plugin.hasConfigMap
here?
(Looking at #689 I would assume that that may also need to be updated in some way though).
Signed-off-by: Jakub Krzykowski <jakub@krzykowski.it>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
881e398
to
8e36e36
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.
Thanks @jakubkrzykowski. These look good now.
I rebased and incorporated the changes from #749
configMapName
is the empty string""
by default and it breaksallPossibleMigStrategiesAreNone
function with default values.See #670 for details.