Handle gracefully empty arch in kube env. #4973
Conversation
k8s-ci-robot
added
kind/bug
|
/lgtm |
k8s-ci-robot
added
the
lgtm
| @@ -598,6 +598,8 @@ func ToSystemArchitecture(arch string) SystemArchitecture { | |||
| return Arm64 | |||
| case string(Amd64): | |||
| return Amd64 | |||
| case "": | |||
There was a problem hiding this comment.
Shouldn't we just change default: to return DefaultArch? It seems surprising to differentiate.
There was a problem hiding this comment.
Then we would return DefaultArch for arch == "funny cat" - I'm not sure if this is okay.
We can expect empty string to just be an alias for default, I don't think the same can be said for other values.
There was a problem hiding this comment.
I decided to go with using DefaultArch as default:, given that incorrect value in this place will lead to cluster autoscaler crash in main loop.
|
/approve |
k8s-ci-robot
added
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: MaciekPytel, olagacek The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
k8s-ci-robot
removed
the
lgtm
|
/lgtm |
k8s-ci-robot
removed
the
do-not-merge/hold
k8s-ci-robot
added
the
lgtm
…hitectures PR kubernetes#4973 changed ToSystemArchitecture behavior to return DefaultArch instead of UnknownArch for invalid architectures. This kind of defaulting makes sense while parsing KUBE_ENV, but prevents using the function in contexts where an invalid architecture should result in an error. This commit reverts ToSystemArchitecture to previous behavior, and moves defaulting to the callsite.
Handle gracefully empty arch in kube env.
…hitectures PR kubernetes#4973 changed ToSystemArchitecture behavior to return DefaultArch instead of UnknownArch for invalid architectures. This kind of defaulting makes sense while parsing KUBE_ENV, but prevents using the function in contexts where an invalid architecture should result in an error. This commit reverts ToSystemArchitecture to previous behavior, and moves defaulting to the callsite.
Which component this PR applies to?
cluster-autoscaler
What type of PR is this?
/kind bug
What this PR does / why we need it:
It prevents error in cluster-autoscaler in case empty arch is provided in kube env.