chore: force external cloud provider for Kubernetes v1.21+ on Azure Stack Hub #4849
Conversation
| @@ -926,6 +926,18 @@ func (cs *ContainerService) setAddonsConfig(isUpgrade bool) { | |||
| } | |||
| } | |||
|
|
|||
| // Ensure cloud-node-manager is enabled on appropriate upgrades for Azure Stack cloud | |||
| if isUpgrade && | |||
There was a problem hiding this comment.
do we need to check for to.Bool(cs.Properties.OrchestratorProfile.KubernetesConfig.UseCloudControllerManager) && common.IsKubernetesVersionGe(o.OrchestratorVersion, "1.21.0"?
The UseCCM flag is overridden already and supported version list starts on 1.21
There was a problem hiding this comment.
The UseCCM flag is overridden already
I am assuming that upgrade.go is executed first
There was a problem hiding this comment.
Yes, upgrade.go executed first, I will remove the check for k8s version
pkg/api/addons.go
Outdated
| to.Bool(cs.Properties.OrchestratorProfile.KubernetesConfig.UseCloudControllerManager) && | ||
| common.IsKubernetesVersionGe(o.OrchestratorVersion, "1.21.0") { | ||
| // Force enabling cloud-node-manager addon | ||
| log.Infoln("Updating cloud-node-manager addon to 'true' on Azure Stack cloud...") |
There was a problem hiding this comment.
I am not sure this message is adding relevant information given that upgrade.go is already logging something similar.
If you see extra value, then I would use the debug level instead of info
There was a problem hiding this comment.
unnecessary log removed
| @@ -147,6 +149,18 @@ func (cs *ContainerService) setComponentsConfig(isUpgrade bool) { | |||
| } | |||
| } | |||
|
|
|||
| // Ensure cloud-controller-manager is enabled on appropriate upgrades for Azure Stack cloud | |||
pkg/api/vlabs/validate.go
Outdated
| @@ -300,6 +300,10 @@ func (a *Properties) ValidateOrchestratorProfile(isUpdate bool) error { | |||
| } | |||
|
|
|||
| if a.IsAzureStackCloud() { | |||
| if common.IsKubernetesVersionGe(a.OrchestratorProfile.OrchestratorVersion, "1.21.0") && !to.Bool(o.KubernetesConfig.UseCloudControllerManager) { | |||
| return errors.New("useCloudControllerManager should be set to true for Kubernetes 1.21+ cluster on Azure Stack") | |||
There was a problem hiding this comment.
v1.21+ instead of 1.21+
Could you please update the messages so they read Azure Stack Hub instead of Azure Stack?
jadarsie
changed the title
pkg/api/defaults-kubelet.go
Outdated
| @@ -77,6 +77,9 @@ func (cs *ContainerService) setKubeletConfig(isUpgrade bool) { | |||
| staticWindowsKubeletConfig["--image-pull-progress-deadline"] = "20m" | |||
| staticWindowsKubeletConfig["--resolv-conf"] = "\"\"\"\"" | |||
| staticWindowsKubeletConfig["--eviction-hard"] = "\"\"\"\"" | |||
| if to.Bool(o.KubernetesConfig.UseCloudControllerManager) { | |||
There was a problem hiding this comment.
I don't think this is necessary. Windows nodes are never used as control plane nodes, and worker node kubelet runtimes don't have a functional "external cloud-provider" context.
There was a problem hiding this comment.
Thanks, I will remove this
| @@ -191,6 +194,10 @@ func (cs *ContainerService) setKubeletConfig(isUpgrade bool) { | |||
| // if upgrade, force default "--pod-infra-container-image" value | |||
| cs.Properties.MasterProfile.KubernetesConfig.KubeletConfig["--pod-infra-container-image"] = o.KubernetesConfig.KubeletConfig["--pod-infra-container-image"] | |||
| } | |||
| //Ensure cloud-provider setting | |||
There was a problem hiding this comment.
We don't need this, this is already being taken care of in L152-L154
There was a problem hiding this comment.
This is to ensure the KubeletConfig in MasterProfile get updated when we upgrade from a in-tree cloud-provider cluster to a external cloud-provider. Without this line, the previous value for this field will not get overwritten.
| @@ -300,6 +300,10 @@ func (a *Properties) ValidateOrchestratorProfile(isUpdate bool) error { | |||
| } | |||
|
|
|||
| if a.IsAzureStackCloud() { | |||
| if common.IsKubernetesVersionGe(a.OrchestratorProfile.OrchestratorVersion, "1.21.0") && !to.Bool(o.KubernetesConfig.UseCloudControllerManager) { | |||
There was a problem hiding this comment.
This validation happens before defaults enforcement, so this is going to throw an error every time a user tries to create a cluster with an empty UseCloudControllerManager configuration
There was a problem hiding this comment.
Yes, that's what we want. We want users to add this UseCloudControllerManager: true in the apimodel for v1.21+ clusters on Azure Stack Hub, and be aware of the existence of external cloud-provider in these clusters.
Reason for Change:
Start Kubernetes 1.21, only external cloud provider is supported on Azure Stack cloud. Adding the logic to enforce that for deploy and upgrade operations.
Issue Fixed:
Credit Where Due:
Does this change contain code from or inspired by another project?
If "Yes," did you notify that project's maintainers and provide attribution?
Requirements:
Notes: