diff --git a/go.sum b/go.sum index cd15cfa81c..385a0bf6e5 100644 --- a/go.sum +++ b/go.sum @@ -58,6 +58,7 @@ github.com/google/btree v1.0.0 h1:0udJVsspx3VBr5FwtLhQQtuAsVc79tTq0ocGIPAU6qo= github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/go-cmp v0.2.0 h1:+dTQ8DZQJz0Mb/HjFlkptS1FeQ4cWSnN941F8aEG4SQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= +github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4= github.com/google/gofuzz v1.0.0 h1:A8PeW59pxE9IoFRqBp37U+mSNaQoZ46F1f0f863XSXw= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/uuid v1.1.1 h1:Gkbcsh/GbpXz7lPftLA3P6TYMwjCLYm83jiFQZF/3gY= diff --git a/parts/k8s/cloud-init/artifacts/cse_config.sh b/parts/k8s/cloud-init/artifacts/cse_config.sh index 04e2c542d0..d7ff4f0e57 100755 --- a/parts/k8s/cloud-init/artifacts/cse_config.sh +++ b/parts/k8s/cloud-init/artifacts/cse_config.sh @@ -165,14 +165,20 @@ configureK8s() { chmod 0644 "${APISERVER_PUBLIC_KEY_PATH}" chown root:root "${APISERVER_PUBLIC_KEY_PATH}" - AZURE_JSON_PATH="/etc/kubernetes/azure.json" - touch "${AZURE_JSON_PATH}" - chmod 0600 "${AZURE_JSON_PATH}" - chown root:root "${AZURE_JSON_PATH}" - set +x echo "${KUBELET_PRIVATE_KEY}" | base64 --decode > "${KUBELET_PRIVATE_KEY_PATH}" echo "${APISERVER_PUBLIC_KEY}" | base64 --decode > "${APISERVER_PUBLIC_KEY_PATH}" + configureKubeletServerCert + AZURE_JSON_PATH="/etc/kubernetes/azure.json" + if [[ -n "${MASTER_NODE}" ]]; then + if [[ "${ENABLE_AGGREGATED_APIS}" = True ]]; then + generateAggregatedAPICerts + fi + else + {{- /* If we are a node vm then we only proceed w/ local azure.json configuration if cloud-init has pre-paved that file */}} + wait_for_file 1 1 $AZURE_JSON_PATH || return + fi + {{/* Perform the required JSON escaping */}} SERVICE_PRINCIPAL_CLIENT_SECRET=${SERVICE_PRINCIPAL_CLIENT_SECRET//\\/\\\\} SERVICE_PRINCIPAL_CLIENT_SECRET=${SERVICE_PRINCIPAL_CLIENT_SECRET//\"/\\\"} @@ -218,16 +224,9 @@ configureK8s() { EOF set -x if [[ "${CLOUDPROVIDER_BACKOFF_MODE}" = "v2" ]]; then - sed -i "/cloudProviderBackoffExponent/d" /etc/kubernetes/azure.json - sed -i "/cloudProviderBackoffJitter/d" /etc/kubernetes/azure.json - fi - if [[ -n "${MASTER_NODE}" ]]; then - if [[ "${ENABLE_AGGREGATED_APIS}" = True ]]; then - generateAggregatedAPICerts - fi + sed -i "/cloudProviderBackoffExponent/d" $AZURE_JSON_PATH + sed -i "/cloudProviderBackoffJitter/d" $AZURE_JSON_PATH fi - - configureKubeletServerCert } configureCNI() { diff --git a/parts/k8s/cloud-init/nodecustomdata.yml b/parts/k8s/cloud-init/nodecustomdata.yml index 0ca58d6ad2..e890ac3c12 100644 --- a/parts/k8s/cloud-init/nodecustomdata.yml +++ b/parts/k8s/cloud-init/nodecustomdata.yml @@ -1,6 +1,14 @@ #cloud-config write_files: +{{- if .RequiresCloudproviderConfig}} +- path: /etc/kubernetes/azure.json + permissions: "0600" + owner: root + content: | + #EOF +{{end}} + - path: {{GetCSEHelpersScriptFilepath}} permissions: "0744" encoding: gzip diff --git a/pkg/api/defaults-kubelet.go b/pkg/api/defaults-kubelet.go index f4cad86cd9..eb29781e0f 100644 --- a/pkg/api/defaults-kubelet.go +++ b/pkg/api/defaults-kubelet.go @@ -33,7 +33,7 @@ func (cs *ContainerService) setKubeletConfig(isUpgrade bool) { switch key { case "--anonymous-auth", "--client-ca-file": if !to.Bool(o.KubernetesConfig.EnableSecureKubelet) { // Don't add if EnableSecureKubelet is disabled - staticLinuxKubeletConfig[key] = "" + delete(staticLinuxKubeletConfig, key) } } } @@ -270,13 +270,6 @@ func removeKubeletFlags(k map[string]string, v string) { delete(k, key) } } - - // Get rid of keys with empty string values - for key, val := range k { - if val == "" { - delete(k, key) - } - } } func setMissingKubeletValues(p *KubernetesConfig, d map[string]string) { diff --git a/pkg/api/defaults-kubelet_test.go b/pkg/api/defaults-kubelet_test.go index ef11a2d829..d29a1c189c 100644 --- a/pkg/api/defaults-kubelet_test.go +++ b/pkg/api/defaults-kubelet_test.go @@ -12,6 +12,7 @@ import ( "github.com/Azure/aks-engine/pkg/api/common" "github.com/Azure/aks-engine/pkg/helpers" + "github.com/google/go-cmp/cmp" ) func TestKubeletConfigDefaults(t *testing.T) { @@ -2166,3 +2167,77 @@ func TestReadOnlyPort(t *testing.T) { } } + +func TestRemoveKubeletFlags(t *testing.T) { + cases := []struct { + name string + kubeletConfig map[string]string + version string + expected map[string]string + }{ + { + name: "v1.17.0", + kubeletConfig: map[string]string{ + "--pod-max-pids": "100", + "--cadvisor-port": "1234", + "--allow-privileged": "true", + }, + expected: map[string]string{ + "--pod-max-pids": "100", + }, + version: "1.17.0", + }, + { + name: "v1.9.0", + kubeletConfig: map[string]string{ + "--pod-max-pids": "100", + "--cadvisor-port": "1234", + "--allow-privileged": "true", + }, + expected: map[string]string{ + "--cadvisor-port": "1234", + "--allow-privileged": "true", + }, + version: "1.9.0", + }, + { + name: "v1.14.0", + kubeletConfig: map[string]string{ + "--pod-max-pids": "100", + "--cadvisor-port": "1234", + "--allow-privileged": "true", + }, + expected: map[string]string{ + "--pod-max-pids": "100", + "--allow-privileged": "true", + }, + version: "1.14.0", + }, + { + name: "v1.11.0", + kubeletConfig: map[string]string{ + "--pod-max-pids": "100", + "--cadvisor-port": "1234", + "--allow-privileged": "true", + }, + expected: map[string]string{ + "--pod-max-pids": "100", + "--cadvisor-port": "1234", + "--allow-privileged": "true", + }, + version: "1.11.0", + }, + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + t.Parallel() + removeKubeletFlags(c.kubeletConfig, c.version) + diff := cmp.Diff(c.kubeletConfig, c.expected) + if diff != "" { + t.Errorf("unexpected diff while expecting equal structs: %s", diff) + } + }) + } +} diff --git a/pkg/api/defaults_test.go b/pkg/api/defaults_test.go index ceb133dcca..a8bb7e4cf2 100644 --- a/pkg/api/defaults_test.go +++ b/pkg/api/defaults_test.go @@ -125,16 +125,18 @@ func TestCertsAlreadyPresent(t *testing.T) { func TestSetMissingKubeletValues(t *testing.T) { config := &KubernetesConfig{} defaultKubeletConfig := map[string]string{ - "--network-plugin": "1", - "--pod-infra-container-image": "2", - "--max-pods": "3", - "--eviction-hard": "4", - "--node-status-update-frequency": "5", - "--image-gc-high-threshold": "6", - "--image-gc-low-threshold": "7", - "--non-masquerade-cidr": "8", - "--cloud-provider": "9", - "--pod-max-pids": "10", + "--network-plugin": "1", + "--pod-infra-container-image": "2", + "--max-pods": "3", + "--eviction-hard": "4", + "--node-status-update-frequency": "5", + "--image-gc-high-threshold": "6", + "--image-gc-low-threshold": "7", + "--non-masquerade-cidr": "8", + "--pod-max-pids": "10", + "--cloud-provider": "azure", + "--cloud-config": "/etc/kubernetes/azure.json", + "--azure-container-registry-config": "/etc/kubernetes/azure.json", } setMissingKubeletValues(config, defaultKubeletConfig) for key, val := range defaultKubeletConfig { @@ -147,20 +149,50 @@ func TestSetMissingKubeletValues(t *testing.T) { KubeletConfig: map[string]string{ "--network-plugin": "a", "--pod-infra-container-image": "b", - "--cloud-provider": "c", + "--cloud-provider": "", }, } expectedResult := map[string]string{ - "--network-plugin": "a", - "--pod-infra-container-image": "b", - "--max-pods": "3", - "--eviction-hard": "4", - "--node-status-update-frequency": "5", - "--image-gc-high-threshold": "6", - "--image-gc-low-threshold": "7", - "--non-masquerade-cidr": "8", - "--cloud-provider": "c", - "--pod-max-pids": "10", + "--network-plugin": "a", + "--pod-infra-container-image": "b", + "--max-pods": "3", + "--eviction-hard": "4", + "--node-status-update-frequency": "5", + "--image-gc-high-threshold": "6", + "--image-gc-low-threshold": "7", + "--non-masquerade-cidr": "8", + "--pod-max-pids": "10", + "--cloud-provider": "", + "--cloud-config": "/etc/kubernetes/azure.json", + "--azure-container-registry-config": "/etc/kubernetes/azure.json", + } + setMissingKubeletValues(config, defaultKubeletConfig) + for key, val := range expectedResult { + if config.KubeletConfig[key] != val { + t.Fatalf("setMissingKubeletValue() did not return the expected value %s for key %s, instead returned: %s", val, key, config.KubeletConfig[key]) + } + } + + config = &KubernetesConfig{ + KubeletConfig: map[string]string{ + "--cloud-provider": "", + "--cloud-config": "", + "--azure-container-registry-config": "", + }, + } + expectedResult = map[string]string{ + "--network-plugin": "1", + "--pod-infra-container-image": "2", + "--max-pods": "3", + "--eviction-hard": "4", + "--node-status-update-frequency": "5", + "--image-gc-high-threshold": "6", + "--image-gc-low-threshold": "7", + "--non-masquerade-cidr": "8", + "--pod-max-pids": "10", + "--cloud-provider": "", + "--cloud-config": "", + "--azure-container-registry-config": "", } setMissingKubeletValues(config, defaultKubeletConfig) for key, val := range expectedResult { diff --git a/pkg/api/types.go b/pkg/api/types.go index cfd4f80d72..e16c62deaf 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -1638,6 +1638,36 @@ func (a *AgentPoolProfile) IsUbuntuNonVHD() bool { return a.IsUbuntu() && !a.IsVHDDistro() } +// RequiresCloudproviderConfig returns true if the azure.json cloudprovider config should be delivered to the nodes in this pool +func (a *AgentPoolProfile) RequiresCloudproviderConfig() bool { + if a.KubernetesConfig != nil && a.KubernetesConfig.KubeletConfig != nil { + if v, ok := a.KubernetesConfig.KubeletConfig["--cloud-provider"]; ok { + if v != "" { + return true + } + } else { + return true + } + if v, ok := a.KubernetesConfig.KubeletConfig["--cloud-config"]; ok { + if v != "" { + return true + } + } else { + return true + } + if v, ok := a.KubernetesConfig.KubeletConfig["--azure-container-registry-config"]; ok { + if v != "" { + return true + } + } else { + return true + } + } else { + return true + } + return false +} + // GetKubernetesLabels returns a k8s API-compliant labels string for nodes in this profile func (a *AgentPoolProfile) GetKubernetesLabels(rg string, deprecated bool) string { var buf bytes.Buffer diff --git a/pkg/api/types_test.go b/pkg/api/types_test.go index 90062e0f54..876685b6f2 100644 --- a/pkg/api/types_test.go +++ b/pkg/api/types_test.go @@ -409,6 +409,91 @@ func TestAgentPoolProfileIsUbuntuNonVHD(t *testing.T) { } } +func TestRequiresCloudproviderConfig(t *testing.T) { + cases := []struct { + name string + ap AgentPoolProfile + expected bool + }{ + { + name: "nil", + ap: AgentPoolProfile{}, + expected: true, + }, + { + name: "default", + ap: AgentPoolProfile{ + KubernetesConfig: &KubernetesConfig{ + KubeletConfig: map[string]string{}, + }, + }, + expected: true, + }, + { + name: "--cloud-provider provided", + ap: AgentPoolProfile{ + KubernetesConfig: &KubernetesConfig{ + KubeletConfig: map[string]string{ + "--cloud-provider": "azure", + "--cloud-config": "", + "--azure-container-registry-config": "", + }, + }, + }, + expected: true, + }, + { + name: "--cloud-config provided", + ap: AgentPoolProfile{ + KubernetesConfig: &KubernetesConfig{ + KubeletConfig: map[string]string{ + "--cloud-provider": "", + "--cloud-config": "/etc/kubernetes/azure.json", + "--azure-container-registry-config": "", + }, + }, + }, + expected: true, + }, + { + name: "--azure-container-registry-config provided", + ap: AgentPoolProfile{ + KubernetesConfig: &KubernetesConfig{ + KubeletConfig: map[string]string{ + "--cloud-provider": "", + "--cloud-config": "", + "--azure-container-registry-config": "/etc/kubernetes/azure.json", + }, + }, + }, + expected: true, + }, + { + name: "all 3 flags set explicitly to empty string", + ap: AgentPoolProfile{ + KubernetesConfig: &KubernetesConfig{ + KubeletConfig: map[string]string{ + "--cloud-provider": "", + "--cloud-config": "", + "--azure-container-registry-config": "", + }, + }, + }, + expected: false, + }, + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + t.Parallel() + if c.expected != c.ap.RequiresCloudproviderConfig() { + t.Fatalf("Got unexpected AgentPoolProfile.RequiresCloudproviderConfig() result. Expected: %t. Got: %t.", c.expected, c.ap.RequiresCloudproviderConfig()) + } + }) + } +} + func TestMasterProfileIsVHDDistro(t *testing.T) { cases := []struct { name string diff --git a/pkg/engine/templates_generated.go b/pkg/engine/templates_generated.go index 85d984446c..9ad7d75690 100644 --- a/pkg/engine/templates_generated.go +++ b/pkg/engine/templates_generated.go @@ -34659,14 +34659,20 @@ configureK8s() { chmod 0644 "${APISERVER_PUBLIC_KEY_PATH}" chown root:root "${APISERVER_PUBLIC_KEY_PATH}" - AZURE_JSON_PATH="/etc/kubernetes/azure.json" - touch "${AZURE_JSON_PATH}" - chmod 0600 "${AZURE_JSON_PATH}" - chown root:root "${AZURE_JSON_PATH}" - set +x echo "${KUBELET_PRIVATE_KEY}" | base64 --decode > "${KUBELET_PRIVATE_KEY_PATH}" echo "${APISERVER_PUBLIC_KEY}" | base64 --decode > "${APISERVER_PUBLIC_KEY_PATH}" + configureKubeletServerCert + AZURE_JSON_PATH="/etc/kubernetes/azure.json" + if [[ -n "${MASTER_NODE}" ]]; then + if [[ "${ENABLE_AGGREGATED_APIS}" = True ]]; then + generateAggregatedAPICerts + fi + else + {{- /* If we are a node vm then we only proceed w/ local azure.json configuration if cloud-init has pre-paved that file */}} + wait_for_file 1 1 $AZURE_JSON_PATH || return + fi + {{/* Perform the required JSON escaping */}} SERVICE_PRINCIPAL_CLIENT_SECRET=${SERVICE_PRINCIPAL_CLIENT_SECRET//\\/\\\\} SERVICE_PRINCIPAL_CLIENT_SECRET=${SERVICE_PRINCIPAL_CLIENT_SECRET//\"/\\\"} @@ -34712,16 +34718,9 @@ configureK8s() { EOF set -x if [[ "${CLOUDPROVIDER_BACKOFF_MODE}" = "v2" ]]; then - sed -i "/cloudProviderBackoffExponent/d" /etc/kubernetes/azure.json - sed -i "/cloudProviderBackoffJitter/d" /etc/kubernetes/azure.json - fi - if [[ -n "${MASTER_NODE}" ]]; then - if [[ "${ENABLE_AGGREGATED_APIS}" = True ]]; then - generateAggregatedAPICerts - fi + sed -i "/cloudProviderBackoffExponent/d" $AZURE_JSON_PATH + sed -i "/cloudProviderBackoffJitter/d" $AZURE_JSON_PATH fi - - configureKubeletServerCert } configureCNI() { @@ -38156,6 +38155,14 @@ func k8sCloudInitMasternodecustomdataYml() (*asset, error) { var _k8sCloudInitNodecustomdataYml = []byte(`#cloud-config write_files: +{{- if .RequiresCloudproviderConfig}} +- path: /etc/kubernetes/azure.json + permissions: "0600" + owner: root + content: | + #EOF +{{end}} + - path: {{GetCSEHelpersScriptFilepath}} permissions: "0744" encoding: gzip diff --git a/test/e2e/kubernetes/kubernetes_test.go b/test/e2e/kubernetes/kubernetes_test.go index 50978c1d64..1af1cd9ac1 100644 --- a/test/e2e/kubernetes/kubernetes_test.go +++ b/test/e2e/kubernetes/kubernetes_test.go @@ -225,6 +225,15 @@ var _ = Describe("Azure Container Cluster using the Kubernetes Orchestrator", fu if cfg.BlockSSHPort { Skip("SSH port is blocked") } else if !eng.ExpandedDefinition.Properties.HasNonRegularPriorityScaleset() { + var cloudproviderEnabledPrefixes []string + if eng.ExpandedDefinition.Properties.MasterProfile != nil { + cloudproviderEnabledPrefixes = append(cloudproviderEnabledPrefixes, "k8s-master-") + } + for _, profile := range eng.ExpandedDefinition.Properties.AgentPoolProfiles { + if profile.RequiresCloudproviderConfig() { + cloudproviderEnabledPrefixes = append(cloudproviderEnabledPrefixes, "k8s-"+profile.Name) + } + } nodes, err := node.GetReadyWithRetry(1*time.Second, cfg.Timeout) Expect(err).NotTo(HaveOccurred()) cloudproviderConfigValidateScript := "cloudprovider-config-validate.sh" @@ -236,7 +245,7 @@ var _ = Describe("Azure Container Cluster using the Kubernetes Orchestrator", fu err = sshConn.Execute(cloudproviderConfigValidationCommand, false) Expect(err).NotTo(HaveOccurred()) for _, n := range nodes { - if n.IsUbuntu() && !firstMasterRegexp.MatchString(n.Metadata.Name) { + if n.IsUbuntu() && !firstMasterRegexp.MatchString(n.Metadata.Name) && n.HasSubstring(cloudproviderEnabledPrefixes) { err := sshConn.CopyToRemote(n.Metadata.Name, "/tmp/"+cloudproviderConfigValidateScript) Expect(err).NotTo(HaveOccurred()) err = sshConn.ExecuteRemoteWithRetry(n.Metadata.Name, cloudproviderConfigValidationCommand, false, sleepBetweenRetriesRemoteSSHCommand, cfg.Timeout) diff --git a/test/e2e/test_cluster_configs/base.json b/test/e2e/test_cluster_configs/base.json index 1b486b22dc..0782f97f19 100644 --- a/test/e2e/test_cluster_configs/base.json +++ b/test/e2e/test_cluster_configs/base.json @@ -41,6 +41,13 @@ "name": "pooladded", "count": 1, "vmSize": "Standard_D2_v3", - "availabilityProfile": "VirtualMachineScaleSets" + "availabilityProfile": "VirtualMachineScaleSets", + "kubernetesConfig": { + "kubeletConfig": { + "--cloud-provider": "", + "--cloud-config": "", + "--azure-container-registry-config": "" + } + } } }