From 01171b04ff80767e42eb96dceb168979a4e3a23a Mon Sep 17 00:00:00 2001 From: Hao Fan Date: Fri, 25 Feb 2022 15:15:53 -0800 Subject: [PATCH 1/6] enforce external cloud provider for Azure Stack cloud 1.21+ --- cmd/upgrade.go | 6 ++++++ pkg/api/addons.go | 12 ++++++++++++ pkg/api/components.go | 14 ++++++++++++++ pkg/api/defaults-kubelet.go | 7 +++++++ pkg/api/vlabs/validate.go | 6 +++++- 5 files changed, 44 insertions(+), 1 deletion(-) diff --git a/cmd/upgrade.go b/cmd/upgrade.go index 50f932fb82..8a1faccffe 100644 --- a/cmd/upgrade.go +++ b/cmd/upgrade.go @@ -222,6 +222,12 @@ func (uc *upgradeCmd) loadCluster() error { } } + // Enforce UseCloudControllerManager for Kubernetes 1.21+ on Azure Stack cloud + if uc.containerService.Properties.IsAzureStackCloud() && common.IsKubernetesVersionGe(uc.upgradeVersion, "1.21.0") { + log.Infoln("Updating UseCloudControllerManager to 'true' on Azure Stack cloud...\n") + uc.containerService.Properties.OrchestratorProfile.KubernetesConfig.UseCloudControllerManager = to.BoolPtr(true) + } + // The cluster-init component is a cluster create-only feature, temporarily disable if enabled if i := api.GetComponentsIndexByName(uc.containerService.Properties.OrchestratorProfile.KubernetesConfig.Components, common.ClusterInitComponentName); i > -1 { if uc.containerService.Properties.OrchestratorProfile.KubernetesConfig.Components[i].IsEnabled() { diff --git a/pkg/api/addons.go b/pkg/api/addons.go index b82f6523e1..6ea3e01d23 100644 --- a/pkg/api/addons.go +++ b/pkg/api/addons.go @@ -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 && + cs.Properties.IsAzureStackCloud() && + 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...\n") + if i := getAddonsIndexByName(o.KubernetesConfig.Addons, common.CloudNodeManagerAddonName); i > -1 { + o.KubernetesConfig.Addons[i] = defaultCloudNodeManagerAddonsConfig + } + } + // Back-compat for older addon specs of cluster-autoscaler if isUpgrade { i := getAddonsIndexByName(o.KubernetesConfig.Addons, common.ClusterAutoscalerAddonName) diff --git a/pkg/api/components.go b/pkg/api/components.go index a4dff36104..8ea34175db 100644 --- a/pkg/api/components.go +++ b/pkg/api/components.go @@ -6,6 +6,8 @@ package api import ( "github.com/Azure/aks-engine/pkg/api/common" "github.com/Azure/go-autorest/autorest/to" + + log "github.com/sirupsen/logrus" ) func (cs *ContainerService) setComponentsConfig(isUpgrade bool) { @@ -147,6 +149,18 @@ func (cs *ContainerService) setComponentsConfig(isUpgrade bool) { } } + // Ensure cloud-controller-manager is enabled on appropriate upgrades for Azure Stack cloud + if isUpgrade && + cs.Properties.IsAzureStackCloud() && + to.Bool(cs.Properties.OrchestratorProfile.KubernetesConfig.UseCloudControllerManager) && + common.IsKubernetesVersionGe(cs.Properties.OrchestratorProfile.OrchestratorVersion, "1.21.0") { + // Force enabling cloud-controller-manager + log.Infoln("Updating cloud-controller-manager component to 'true' on Azure Stack cloud...\n") + if i := GetComponentsIndexByName(kubernetesConfig.Components, common.CloudControllerManagerComponentName); i > -1 { + kubernetesConfig.Components[i] = defaultCloudControllerManagerComponentConfig + } + } + for _, component := range defaultComponents { synthesizeComponentsConfig(kubernetesConfig.Components, component, isUpgrade) } diff --git a/pkg/api/defaults-kubelet.go b/pkg/api/defaults-kubelet.go index 5e676ad228..c91223eafb 100644 --- a/pkg/api/defaults-kubelet.go +++ b/pkg/api/defaults-kubelet.go @@ -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) { + staticWindowsKubeletConfig["--cloud-provider"] = "external" + } nodeStatusUpdateFrequency := GetK8sComponentsByVersionMap(o.KubernetesConfig)[o.OrchestratorVersion]["nodestatusfreq"] if cs.Properties.IsAzureStackCloud() { @@ -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 + if to.Bool(o.KubernetesConfig.UseCloudControllerManager) { + cs.Properties.MasterProfile.KubernetesConfig.KubeletConfig["--cloud-provider"] = "external" + } setMissingKubeletValues(cs.Properties.MasterProfile.KubernetesConfig, o.KubernetesConfig.KubeletConfig) addDefaultFeatureGates(cs.Properties.MasterProfile.KubernetesConfig.KubeletConfig, o.OrchestratorVersion, "", "") diff --git a/pkg/api/vlabs/validate.go b/pkg/api/vlabs/validate.go index e5b3d1c56e..9ff0929bfa 100644 --- a/pkg/api/vlabs/validate.go +++ b/pkg/api/vlabs/validate.go @@ -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") + } + if to.Bool(o.KubernetesConfig.UseInstanceMetadata) { return errors.New("useInstanceMetadata shouldn't be set to true as feature not yet supported on Azure Stack") } @@ -809,7 +813,7 @@ func (a *Properties) validateAddons(isUpdate bool) error { // Validation for addons if they are disabled switch addon.Name { case "cloud-node-manager": - if a.ShouldEnableAzureCloudAddon(addon.Name) { + if a.ShouldEnableAzureCloudAddon(addon.Name) && !a.IsAzureStackCloud() { minVersion := "1.16.0" if a.HasWindows() { minVersion = "1.18.0" From fe5439d9fb5e6a7cdd9f686f883d6da1fa059402 Mon Sep 17 00:00:00 2001 From: Hao Fan Date: Mon, 28 Feb 2022 10:32:06 -0800 Subject: [PATCH 2/6] fix lint --- cmd/upgrade.go | 2 +- pkg/api/addons.go | 2 +- pkg/api/components.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/upgrade.go b/cmd/upgrade.go index 8a1faccffe..754c9de303 100644 --- a/cmd/upgrade.go +++ b/cmd/upgrade.go @@ -224,7 +224,7 @@ func (uc *upgradeCmd) loadCluster() error { // Enforce UseCloudControllerManager for Kubernetes 1.21+ on Azure Stack cloud if uc.containerService.Properties.IsAzureStackCloud() && common.IsKubernetesVersionGe(uc.upgradeVersion, "1.21.0") { - log.Infoln("Updating UseCloudControllerManager to 'true' on Azure Stack cloud...\n") + log.Infoln("Updating UseCloudControllerManager to 'true' on Azure Stack cloud...") uc.containerService.Properties.OrchestratorProfile.KubernetesConfig.UseCloudControllerManager = to.BoolPtr(true) } diff --git a/pkg/api/addons.go b/pkg/api/addons.go index 6ea3e01d23..056ccab542 100644 --- a/pkg/api/addons.go +++ b/pkg/api/addons.go @@ -932,7 +932,7 @@ func (cs *ContainerService) setAddonsConfig(isUpgrade bool) { 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...\n") + log.Infoln("Updating cloud-node-manager addon to 'true' on Azure Stack cloud...") if i := getAddonsIndexByName(o.KubernetesConfig.Addons, common.CloudNodeManagerAddonName); i > -1 { o.KubernetesConfig.Addons[i] = defaultCloudNodeManagerAddonsConfig } diff --git a/pkg/api/components.go b/pkg/api/components.go index 8ea34175db..28cebb860b 100644 --- a/pkg/api/components.go +++ b/pkg/api/components.go @@ -155,7 +155,7 @@ func (cs *ContainerService) setComponentsConfig(isUpgrade bool) { to.Bool(cs.Properties.OrchestratorProfile.KubernetesConfig.UseCloudControllerManager) && common.IsKubernetesVersionGe(cs.Properties.OrchestratorProfile.OrchestratorVersion, "1.21.0") { // Force enabling cloud-controller-manager - log.Infoln("Updating cloud-controller-manager component to 'true' on Azure Stack cloud...\n") + log.Infoln("Updating cloud-controller-manager component to 'true' on Azure Stack cloud...") if i := GetComponentsIndexByName(kubernetesConfig.Components, common.CloudControllerManagerComponentName); i > -1 { kubernetesConfig.Components[i] = defaultCloudControllerManagerComponentConfig } From c1529f37e2ea8c0bec3c61cedad6bc2ef1c2d8cb Mon Sep 17 00:00:00 2001 From: Hao Fan Date: Mon, 28 Feb 2022 13:05:35 -0800 Subject: [PATCH 3/6] address comments --- cmd/upgrade.go | 2 +- pkg/api/addons.go | 4 +--- pkg/api/components.go | 6 +----- pkg/api/vlabs/validate.go | 2 +- pkg/api/vlabs/validate_test.go | 30 +++++++++++++++++++++++++++--- 5 files changed, 31 insertions(+), 13 deletions(-) diff --git a/cmd/upgrade.go b/cmd/upgrade.go index 754c9de303..ca78908159 100644 --- a/cmd/upgrade.go +++ b/cmd/upgrade.go @@ -224,7 +224,7 @@ func (uc *upgradeCmd) loadCluster() error { // Enforce UseCloudControllerManager for Kubernetes 1.21+ on Azure Stack cloud if uc.containerService.Properties.IsAzureStackCloud() && common.IsKubernetesVersionGe(uc.upgradeVersion, "1.21.0") { - log.Infoln("Updating UseCloudControllerManager to 'true' on Azure Stack cloud...") + log.Infoln("The in-tree cloud provider is not longer supported on Azure Stack Hub for v1.21+ clusters, overwriting UseCloudControllerManager to 'true'...") uc.containerService.Properties.OrchestratorProfile.KubernetesConfig.UseCloudControllerManager = to.BoolPtr(true) } diff --git a/pkg/api/addons.go b/pkg/api/addons.go index 056ccab542..fd03be0bf2 100644 --- a/pkg/api/addons.go +++ b/pkg/api/addons.go @@ -929,10 +929,8 @@ func (cs *ContainerService) setAddonsConfig(isUpgrade bool) { // Ensure cloud-node-manager is enabled on appropriate upgrades for Azure Stack cloud if isUpgrade && cs.Properties.IsAzureStackCloud() && - to.Bool(cs.Properties.OrchestratorProfile.KubernetesConfig.UseCloudControllerManager) && - common.IsKubernetesVersionGe(o.OrchestratorVersion, "1.21.0") { + to.Bool(cs.Properties.OrchestratorProfile.KubernetesConfig.UseCloudControllerManager) { // Force enabling cloud-node-manager addon - log.Infoln("Updating cloud-node-manager addon to 'true' on Azure Stack cloud...") if i := getAddonsIndexByName(o.KubernetesConfig.Addons, common.CloudNodeManagerAddonName); i > -1 { o.KubernetesConfig.Addons[i] = defaultCloudNodeManagerAddonsConfig } diff --git a/pkg/api/components.go b/pkg/api/components.go index 28cebb860b..38fbb38cf1 100644 --- a/pkg/api/components.go +++ b/pkg/api/components.go @@ -6,8 +6,6 @@ package api import ( "github.com/Azure/aks-engine/pkg/api/common" "github.com/Azure/go-autorest/autorest/to" - - log "github.com/sirupsen/logrus" ) func (cs *ContainerService) setComponentsConfig(isUpgrade bool) { @@ -152,10 +150,8 @@ func (cs *ContainerService) setComponentsConfig(isUpgrade bool) { // Ensure cloud-controller-manager is enabled on appropriate upgrades for Azure Stack cloud if isUpgrade && cs.Properties.IsAzureStackCloud() && - to.Bool(cs.Properties.OrchestratorProfile.KubernetesConfig.UseCloudControllerManager) && - common.IsKubernetesVersionGe(cs.Properties.OrchestratorProfile.OrchestratorVersion, "1.21.0") { + to.Bool(cs.Properties.OrchestratorProfile.KubernetesConfig.UseCloudControllerManager) { // Force enabling cloud-controller-manager - log.Infoln("Updating cloud-controller-manager component to 'true' on Azure Stack cloud...") if i := GetComponentsIndexByName(kubernetesConfig.Components, common.CloudControllerManagerComponentName); i > -1 { kubernetesConfig.Components[i] = defaultCloudControllerManagerComponentConfig } diff --git a/pkg/api/vlabs/validate.go b/pkg/api/vlabs/validate.go index 9ff0929bfa..47112c58de 100644 --- a/pkg/api/vlabs/validate.go +++ b/pkg/api/vlabs/validate.go @@ -301,7 +301,7 @@ 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") + return errors.New("useCloudControllerManager should be set to true for Kubernetes v1.21+ cluster on Azure Stack Hub") } if to.Bool(o.KubernetesConfig.UseInstanceMetadata) { diff --git a/pkg/api/vlabs/validate_test.go b/pkg/api/vlabs/validate_test.go index f0e90096df..c0e6830cab 100644 --- a/pkg/api/vlabs/validate_test.go +++ b/pkg/api/vlabs/validate_test.go @@ -4719,6 +4719,27 @@ func TestValidateLocation(t *testing.T) { }, expectedErr: errors.New("missing ContainerService Location"), }, + { + name: "AzureStack UseCloudControllerManager is false", + location: "local", + propertiesnil: false, + cs: &ContainerService{ + Location: "local", + Properties: &Properties{ + CustomCloudProfile: &CustomCloudProfile{ + PortalURL: "https://portal.local.cotoso.com", + }, + OrchestratorProfile: &OrchestratorProfile{ + OrchestratorType: Kubernetes, + OrchestratorVersion: common.RationalizeReleaseAndVersion(Kubernetes, "", "", false, false, true), + KubernetesConfig: &KubernetesConfig{ + UseCloudControllerManager: to.BoolPtr(falseVal), + }, + }, + }, + }, + expectedErr: errors.New("useCloudControllerManager should be set to true for Kubernetes v1.21+ cluster on Azure Stack Hub"), + }, { name: "AzureStack UseInstanceMetadata is true", location: "local", @@ -4733,7 +4754,8 @@ func TestValidateLocation(t *testing.T) { OrchestratorType: Kubernetes, OrchestratorVersion: common.RationalizeReleaseAndVersion(Kubernetes, "", "", false, false, true), KubernetesConfig: &KubernetesConfig{ - UseInstanceMetadata: to.BoolPtr(trueVal), + UseCloudControllerManager: to.BoolPtr(trueVal), + UseInstanceMetadata: to.BoolPtr(trueVal), }, }, }, @@ -4754,7 +4776,8 @@ func TestValidateLocation(t *testing.T) { OrchestratorType: Kubernetes, OrchestratorVersion: common.RationalizeReleaseAndVersion(Kubernetes, "", "", false, false, true), KubernetesConfig: &KubernetesConfig{ - EtcdDiskSizeGB: "1024", + UseCloudControllerManager: to.BoolPtr(trueVal), + EtcdDiskSizeGB: "1024", }, }, }, @@ -4775,7 +4798,8 @@ func TestValidateLocation(t *testing.T) { OrchestratorType: Kubernetes, OrchestratorVersion: common.RationalizeReleaseAndVersion(Kubernetes, "", "", false, false, true), KubernetesConfig: &KubernetesConfig{ - EtcdDiskSizeGB: "1024GB", + UseCloudControllerManager: to.BoolPtr(trueVal), + EtcdDiskSizeGB: "1024GB", }, }, }, From 75c83403e10036f6e066cafcfa2fbb14fe646ca6 Mon Sep 17 00:00:00 2001 From: Hao Fan Date: Mon, 28 Feb 2022 13:33:46 -0800 Subject: [PATCH 4/6] update unittest --- pkg/engine/testdata/azurestack/kubernetes-ubuntu16.json | 1 + pkg/engine/testdata/azurestack/kubernetes.json | 1 + 2 files changed, 2 insertions(+) diff --git a/pkg/engine/testdata/azurestack/kubernetes-ubuntu16.json b/pkg/engine/testdata/azurestack/kubernetes-ubuntu16.json index d589b02a7d..303434f5ee 100644 --- a/pkg/engine/testdata/azurestack/kubernetes-ubuntu16.json +++ b/pkg/engine/testdata/azurestack/kubernetes-ubuntu16.json @@ -6,6 +6,7 @@ "kubernetesConfig": { "kubernetesImageBase": "k8s.gcr.io/", "useInstanceMetadata": false, + "useCloudControllerManager": true, "networkPolicy": "none" } }, diff --git a/pkg/engine/testdata/azurestack/kubernetes.json b/pkg/engine/testdata/azurestack/kubernetes.json index e756bfaa77..183f227ad1 100644 --- a/pkg/engine/testdata/azurestack/kubernetes.json +++ b/pkg/engine/testdata/azurestack/kubernetes.json @@ -6,6 +6,7 @@ "kubernetesConfig": { "kubernetesImageBase": "k8s.gcr.io/", "useInstanceMetadata": false, + "useCloudControllerManager": true, "networkPolicy": "none" } }, From 37448ab93ce1d879d09d081f4a5fb22596ed9e21 Mon Sep 17 00:00:00 2001 From: Hao Fan Date: Mon, 28 Feb 2022 14:41:28 -0800 Subject: [PATCH 5/6] update logging --- cmd/upgrade.go | 2 +- pkg/api/vlabs/validate.go | 2 +- pkg/api/vlabs/validate_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/upgrade.go b/cmd/upgrade.go index ca78908159..c6fcca318a 100644 --- a/cmd/upgrade.go +++ b/cmd/upgrade.go @@ -224,7 +224,7 @@ func (uc *upgradeCmd) loadCluster() error { // Enforce UseCloudControllerManager for Kubernetes 1.21+ on Azure Stack cloud if uc.containerService.Properties.IsAzureStackCloud() && common.IsKubernetesVersionGe(uc.upgradeVersion, "1.21.0") { - log.Infoln("The in-tree cloud provider is not longer supported on Azure Stack Hub for v1.21+ clusters, overwriting UseCloudControllerManager to 'true'...") + log.Infoln("The in-tree cloud provider is not longer supported on Azure Stack Hub for v1.21+ clusters, overwriting UseCloudControllerManager to 'true'") uc.containerService.Properties.OrchestratorProfile.KubernetesConfig.UseCloudControllerManager = to.BoolPtr(true) } diff --git a/pkg/api/vlabs/validate.go b/pkg/api/vlabs/validate.go index 47112c58de..04b4764df9 100644 --- a/pkg/api/vlabs/validate.go +++ b/pkg/api/vlabs/validate.go @@ -301,7 +301,7 @@ 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 v1.21+ cluster on Azure Stack Hub") + return errors.New("useCloudControllerManager should be set to true for Kubernetes v1.21+ clusters on Azure Stack Hub") } if to.Bool(o.KubernetesConfig.UseInstanceMetadata) { diff --git a/pkg/api/vlabs/validate_test.go b/pkg/api/vlabs/validate_test.go index c0e6830cab..ffac2fd851 100644 --- a/pkg/api/vlabs/validate_test.go +++ b/pkg/api/vlabs/validate_test.go @@ -4738,7 +4738,7 @@ func TestValidateLocation(t *testing.T) { }, }, }, - expectedErr: errors.New("useCloudControllerManager should be set to true for Kubernetes v1.21+ cluster on Azure Stack Hub"), + expectedErr: errors.New("useCloudControllerManager should be set to true for Kubernetes v1.21+ clusters on Azure Stack Hub"), }, { name: "AzureStack UseInstanceMetadata is true", From c1c18acb828e6d7053e0862c3a29d476e5a8a9ad Mon Sep 17 00:00:00 2001 From: Hao Fan Date: Wed, 2 Mar 2022 09:44:58 -0800 Subject: [PATCH 6/6] remove kubelet flag cloud-provider on windows worker node --- pkg/api/defaults-kubelet.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/api/defaults-kubelet.go b/pkg/api/defaults-kubelet.go index c91223eafb..72f8df1685 100644 --- a/pkg/api/defaults-kubelet.go +++ b/pkg/api/defaults-kubelet.go @@ -77,9 +77,6 @@ func (cs *ContainerService) setKubeletConfig(isUpgrade bool) { staticWindowsKubeletConfig["--image-pull-progress-deadline"] = "20m" staticWindowsKubeletConfig["--resolv-conf"] = "\"\"\"\"" staticWindowsKubeletConfig["--eviction-hard"] = "\"\"\"\"" - if to.Bool(o.KubernetesConfig.UseCloudControllerManager) { - staticWindowsKubeletConfig["--cloud-provider"] = "external" - } nodeStatusUpdateFrequency := GetK8sComponentsByVersionMap(o.KubernetesConfig)[o.OrchestratorVersion]["nodestatusfreq"] if cs.Properties.IsAzureStackCloud() {