From 31f5c1b979027b43acea780b4c520de925dccbd4 Mon Sep 17 00:00:00 2001 From: Stuart Leeks Date: Mon, 11 Dec 2017 13:47:00 +0000 Subject: [PATCH 1/9] Add --feature-gates handling For kubeletConfig, preserve the existing behaviour of adding Accelerators=true for agent config (for kubernetes 1.6.0 and later) --- .../kubernetes-featuresgates.json | 44 ++++++++++++++++ parts/k8s/kubernetesagentcustomdata.yml | 1 - pkg/acsengine/defaults-kubelet.go | 52 +++++++++++++++++++ 3 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 examples/feature-gates/kubernetes-featuresgates.json diff --git a/examples/feature-gates/kubernetes-featuresgates.json b/examples/feature-gates/kubernetes-featuresgates.json new file mode 100644 index 0000000000..a28621739a --- /dev/null +++ b/examples/feature-gates/kubernetes-featuresgates.json @@ -0,0 +1,44 @@ +{ + "apiVersion": "vlabs", + "properties": { + "orchestratorProfile": { + "orchestratorType": "Kubernetes", + "orchestratorRelease": "1.8", + "kubernetesConfig": { + "kubeletConfig" : { + "--feature-gates": "MountPropagation=true,DebugContainers=true" + }, + "apiServerConfig" : { + "--feature-gates": "MountPropagation=true" + } + } + }, + "masterProfile": { + "count": 1, + "dnsPrefix": "", + "vmSize": "Standard_D2_v2" + }, + "agentPoolProfiles": [ + { + "name": "agentpool1", + "count": 3, + "vmSize": "Standard_D2_v2", + "availabilityProfile": "AvailabilitySet" + } + ], + "linuxProfile": { + "adminUsername": "azureuser", + "ssh": { + "publicKeys": [ + { + "keyData": "" + } + ] + } + }, + "servicePrincipalProfile": { + "clientId": "", + "secret": "" + } + } +} \ No newline at end of file diff --git a/parts/k8s/kubernetesagentcustomdata.yml b/parts/k8s/kubernetesagentcustomdata.yml index 6bf37aab6d..e048233166 100644 --- a/parts/k8s/kubernetesagentcustomdata.yml +++ b/parts/k8s/kubernetesagentcustomdata.yml @@ -110,7 +110,6 @@ write_files: KUBELET_NODE_LABELS={{GetAgentKubernetesLabels . "',variables('labelResourceGroup'),'"}} {{if IsKubernetesVersionGe "1.6.0"}} KUBELET_NON_MASQUERADE_CIDR={{WrapAsVariable "kubernetesNonMasqueradeCidr"}} - KUBELET_FEATURE_GATES=--feature-gates=Accelerators=true {{end}} AGENT_ARTIFACTS_CONFIG_PLACEHOLDER diff --git a/pkg/acsengine/defaults-kubelet.go b/pkg/acsengine/defaults-kubelet.go index 0d5dbe010e..7e50e7dcf2 100644 --- a/pkg/acsengine/defaults-kubelet.go +++ b/pkg/acsengine/defaults-kubelet.go @@ -1,7 +1,11 @@ package acsengine import ( + "bytes" + "fmt" + "sort" "strconv" + "strings" "github.com/Azure/acs-engine/pkg/api" "github.com/Azure/acs-engine/pkg/helpers" @@ -93,6 +97,17 @@ func setKubeletConfig(cs *api.ContainerService) { profile.KubernetesConfig = &api.KubernetesConfig{} } setMissingKubeletValues(profile.KubernetesConfig, o.KubernetesConfig.KubeletConfig) + + // For versions >= 1.6.0 we add the accelerators=true feature gate + // (previously this was handled in parts/k8s/kubernetesagentcustomdata.yml) + if isKubernetesVersionGe(o.OrchestratorVersion, "1.6.0") { + profileFeatureGates := profile.KubernetesConfig.KubeletConfig["--feature-gates"] + if profileFeatureGates != "" { + profile.KubernetesConfig.KubeletConfig = copyMap(profile.KubernetesConfig.KubeletConfig) // make a copy before changing to avoid changing other references (e.g. master) + profile.KubernetesConfig.KubeletConfig["--feature-gates"] = combineValues(profileFeatureGates, "Accelerators=true") + } + } + // TODO - should we perform a merge on the --feature-gates values between o.KubernetesConfig.KubeletConfig and profile.KubernetesConfig? } } @@ -109,3 +124,40 @@ func setMissingKubeletValues(p *api.KubernetesConfig, d map[string]string) { } } } +func copyMap(input map[string]string) map[string]string { + copy := map[string]string{} + for key, value := range input { + copy[key] = value + } + return copy +} +func combineValues(inputs ...string) string { + var valueMap map[string]string + valueMap = make(map[string]string) + for _, input := range inputs { + applyValueStringToMap(valueMap, input) + } + return mapToString(valueMap) +} +func applyValueStringToMap(valueMap map[string]string, input string) { + values := strings.Split(input, ",") + for index := 0; index < len(values); index++ { + // trim spaces (e.g. if the input was "foo=true, bar=true" - we want to drop the space after the comma) + value := strings.Trim(values[index], " ") + valueParts := strings.Split(value, "=") // TODO validate that there are two parts + valueMap[valueParts[0]] = valueParts[1] + } +} +func mapToString(valueMap map[string]string) string { + // Order by key for consistency + keys := []string{} + for key := range valueMap { + keys = append(keys, key) + } + sort.Strings(keys) + var buf bytes.Buffer + for _, key := range keys { + buf.WriteString(fmt.Sprintf("%s=%s,", key, valueMap[key])) + } + return strings.TrimSuffix(buf.String(), ",") +} From 1e786ad9e4037e7163637bdf5e992785840e5731 Mon Sep 17 00:00:00 2001 From: Jack Francis Date: Thu, 11 Jan 2018 14:29:12 -0800 Subject: [PATCH 2/9] simplified default implementation and removed KUBELET_FEATURE_GATES --- .../artifacts/1.5/kuberneteskubelet.service | 2 +- parts/k8s/artifacts/kuberneteskubelet.service | 2 +- pkg/acsengine/defaults-kubelet.go | 23 +++++++++++-------- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/parts/k8s/artifacts/1.5/kuberneteskubelet.service b/parts/k8s/artifacts/1.5/kuberneteskubelet.service index 8a7139bdc5..4d137a2186 100644 --- a/parts/k8s/artifacts/1.5/kuberneteskubelet.service +++ b/parts/k8s/artifacts/1.5/kuberneteskubelet.service @@ -39,7 +39,7 @@ ExecStart=/usr/bin/docker run \ --node-labels="${KUBELET_NODE_LABELS}" \ --hairpin-mode=promiscuous-bridge \ ${KUBELET_CONFIG} \ - --v=2 ${KUBELET_FEATURE_GATES} + --v=2 [Install] WantedBy=multi-user.target diff --git a/parts/k8s/artifacts/kuberneteskubelet.service b/parts/k8s/artifacts/kuberneteskubelet.service index cf84a3136c..23a7e45f7d 100644 --- a/parts/k8s/artifacts/kuberneteskubelet.service +++ b/parts/k8s/artifacts/kuberneteskubelet.service @@ -36,7 +36,7 @@ ExecStart=/usr/bin/docker run \ --require-kubeconfig \ --enable-server \ --node-labels="${KUBELET_NODE_LABELS}" \ - --v=2 ${KUBELET_FEATURE_GATES} \ + --v=2 \ --non-masquerade-cidr=${KUBELET_NON_MASQUERADE_CIDR} \ --volume-plugin-dir=/etc/kubernetes/volumeplugins \ $KUBELET_CONFIG \ diff --git a/pkg/acsengine/defaults-kubelet.go b/pkg/acsengine/defaults-kubelet.go index 7e50e7dcf2..04d8560c37 100644 --- a/pkg/acsengine/defaults-kubelet.go +++ b/pkg/acsengine/defaults-kubelet.go @@ -52,8 +52,13 @@ func setKubeletConfig(cs *api.ContainerService) { "--cloud-config": "/etc/kubernetes/azure.json", } + if isKubernetesVersionGe(o.OrchestratorVersion, "1.6.0") { + defaultKubeletConfig["--feature-gates"] = "Accelerators=true" + } + // If no user-configurable kubelet config values exists, use the defaults setMissingKubeletValues(o.KubernetesConfig, defaultKubeletConfig) + addDefaultFeatureGates(o.KubernetesConfig.KubeletConfig, o.OrchestratorVersion) // Override default cloud-provider? if helpers.IsTrueBoolPointer(o.KubernetesConfig.UseCloudControllerManager) { @@ -90,6 +95,8 @@ func setKubeletConfig(cs *api.ContainerService) { cs.Properties.MasterProfile.KubernetesConfig = &api.KubernetesConfig{} } setMissingKubeletValues(cs.Properties.MasterProfile.KubernetesConfig, o.KubernetesConfig.KubeletConfig) + addDefaultFeatureGates(cs.Properties.MasterProfile.KubernetesConfig.KubeletConfig, o.OrchestratorVersion) + } // Agent-specific kubelet config changes go here for _, profile := range cs.Properties.AgentPoolProfiles { @@ -97,17 +104,13 @@ func setKubeletConfig(cs *api.ContainerService) { profile.KubernetesConfig = &api.KubernetesConfig{} } setMissingKubeletValues(profile.KubernetesConfig, o.KubernetesConfig.KubeletConfig) + addDefaultFeatureGates(profile.KubernetesConfig.KubeletConfig, o.OrchestratorVersion) + } +} - // For versions >= 1.6.0 we add the accelerators=true feature gate - // (previously this was handled in parts/k8s/kubernetesagentcustomdata.yml) - if isKubernetesVersionGe(o.OrchestratorVersion, "1.6.0") { - profileFeatureGates := profile.KubernetesConfig.KubeletConfig["--feature-gates"] - if profileFeatureGates != "" { - profile.KubernetesConfig.KubeletConfig = copyMap(profile.KubernetesConfig.KubeletConfig) // make a copy before changing to avoid changing other references (e.g. master) - profile.KubernetesConfig.KubeletConfig["--feature-gates"] = combineValues(profileFeatureGates, "Accelerators=true") - } - } - // TODO - should we perform a merge on the --feature-gates values between o.KubernetesConfig.KubeletConfig and profile.KubernetesConfig? +func addDefaultFeatureGates(m map[string]string, version string) { + if isKubernetesVersionGe(version, "1.6.0") { + m["--feature-gates"] = combineValues(m["--feature-gates"], "Accelerators=true") } } From ecb8a3580013c26cdb3f4a595c7f055bc605936e Mon Sep 17 00:00:00 2001 From: Jack Francis Date: Thu, 11 Jan 2018 14:41:36 -0800 Subject: [PATCH 3/9] unnecessary default assignment, and simple validation --- pkg/acsengine/defaults-kubelet.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/pkg/acsengine/defaults-kubelet.go b/pkg/acsengine/defaults-kubelet.go index 04d8560c37..0a288a633f 100644 --- a/pkg/acsengine/defaults-kubelet.go +++ b/pkg/acsengine/defaults-kubelet.go @@ -52,10 +52,6 @@ func setKubeletConfig(cs *api.ContainerService) { "--cloud-config": "/etc/kubernetes/azure.json", } - if isKubernetesVersionGe(o.OrchestratorVersion, "1.6.0") { - defaultKubeletConfig["--feature-gates"] = "Accelerators=true" - } - // If no user-configurable kubelet config values exists, use the defaults setMissingKubeletValues(o.KubernetesConfig, defaultKubeletConfig) addDefaultFeatureGates(o.KubernetesConfig.KubeletConfig, o.OrchestratorVersion) @@ -147,8 +143,10 @@ func applyValueStringToMap(valueMap map[string]string, input string) { for index := 0; index < len(values); index++ { // trim spaces (e.g. if the input was "foo=true, bar=true" - we want to drop the space after the comma) value := strings.Trim(values[index], " ") - valueParts := strings.Split(value, "=") // TODO validate that there are two parts - valueMap[valueParts[0]] = valueParts[1] + valueParts := strings.Split(value, "=") + if len(valueParts) == 2 { + valueMap[valueParts[0]] = valueParts[1] + } } } func mapToString(valueMap map[string]string) string { From 3420429496715b3087eb592c27207444dda335ef Mon Sep 17 00:00:00 2001 From: Jack Francis Date: Thu, 11 Jan 2018 14:43:59 -0800 Subject: [PATCH 4/9] removed copyMap func --- pkg/acsengine/defaults-kubelet.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/pkg/acsengine/defaults-kubelet.go b/pkg/acsengine/defaults-kubelet.go index 0a288a633f..33454ba2b3 100644 --- a/pkg/acsengine/defaults-kubelet.go +++ b/pkg/acsengine/defaults-kubelet.go @@ -123,13 +123,7 @@ func setMissingKubeletValues(p *api.KubernetesConfig, d map[string]string) { } } } -func copyMap(input map[string]string) map[string]string { - copy := map[string]string{} - for key, value := range input { - copy[key] = value - } - return copy -} + func combineValues(inputs ...string) string { var valueMap map[string]string valueMap = make(map[string]string) @@ -138,6 +132,7 @@ func combineValues(inputs ...string) string { } return mapToString(valueMap) } + func applyValueStringToMap(valueMap map[string]string, input string) { values := strings.Split(input, ",") for index := 0; index < len(values); index++ { @@ -149,6 +144,7 @@ func applyValueStringToMap(valueMap map[string]string, input string) { } } } + func mapToString(valueMap map[string]string) string { // Order by key for consistency keys := []string{} From 0f590e5c3d096005d51ab27e2f5016ceb4b5dcf1 Mon Sep 17 00:00:00 2001 From: Jack Francis Date: Thu, 11 Jan 2018 15:00:37 -0800 Subject: [PATCH 5/9] enforce min version, "Accelerators=true" is only for agents --- pkg/acsengine/defaults-kubelet.go | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/pkg/acsengine/defaults-kubelet.go b/pkg/acsengine/defaults-kubelet.go index 33454ba2b3..683962ed7c 100644 --- a/pkg/acsengine/defaults-kubelet.go +++ b/pkg/acsengine/defaults-kubelet.go @@ -54,7 +54,7 @@ func setKubeletConfig(cs *api.ContainerService) { // If no user-configurable kubelet config values exists, use the defaults setMissingKubeletValues(o.KubernetesConfig, defaultKubeletConfig) - addDefaultFeatureGates(o.KubernetesConfig.KubeletConfig, o.OrchestratorVersion) + addDefaultFeatureGates(o.KubernetesConfig.KubeletConfig, "", "") // Override default cloud-provider? if helpers.IsTrueBoolPointer(o.KubernetesConfig.UseCloudControllerManager) { @@ -91,7 +91,7 @@ func setKubeletConfig(cs *api.ContainerService) { cs.Properties.MasterProfile.KubernetesConfig = &api.KubernetesConfig{} } setMissingKubeletValues(cs.Properties.MasterProfile.KubernetesConfig, o.KubernetesConfig.KubeletConfig) - addDefaultFeatureGates(cs.Properties.MasterProfile.KubernetesConfig.KubeletConfig, o.OrchestratorVersion) + addDefaultFeatureGates(cs.Properties.MasterProfile.KubernetesConfig.KubeletConfig, "", "") } // Agent-specific kubelet config changes go here @@ -100,13 +100,21 @@ func setKubeletConfig(cs *api.ContainerService) { profile.KubernetesConfig = &api.KubernetesConfig{} } setMissingKubeletValues(profile.KubernetesConfig, o.KubernetesConfig.KubeletConfig) - addDefaultFeatureGates(profile.KubernetesConfig.KubeletConfig, o.OrchestratorVersion) + addDefaultFeatureGates(profile.KubernetesConfig.KubeletConfig, o.OrchestratorVersion, "Accelerators=true") } } -func addDefaultFeatureGates(m map[string]string, version string) { - if isKubernetesVersionGe(version, "1.6.0") { - m["--feature-gates"] = combineValues(m["--feature-gates"], "Accelerators=true") +// combine user-provided --feature-gates vals with defaults +// a minimum k8s version may be declared as required for defaults assignment +func addDefaultFeatureGates(m map[string]string, minVersion string, defaults string) { + if minVersion != "" { + if isKubernetesVersionGe(minVersion, "1.6.0") { + m["--feature-gates"] = combineValues(m["--feature-gates"], defaults) + } else { + m["--feature-gates"] = combineValues(m["--feature-gates"], "") + } + } else { + m["--feature-gates"] = combineValues(m["--feature-gates"], defaults) } } From ded52cdf7147dfcb98338995c1cdcf548d21e523 Mon Sep 17 00:00:00 2001 From: Stuart Leeks Date: Fri, 12 Jan 2018 08:45:13 +0000 Subject: [PATCH 6/9] Pass current version in to addDefaultFeatureGates Generalise addDefaultFeatureGates by passing in current orchestrator version, and deferring the minimum required version to callers --- pkg/acsengine/defaults-kubelet.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/acsengine/defaults-kubelet.go b/pkg/acsengine/defaults-kubelet.go index 683962ed7c..ae4ac505af 100644 --- a/pkg/acsengine/defaults-kubelet.go +++ b/pkg/acsengine/defaults-kubelet.go @@ -54,7 +54,7 @@ func setKubeletConfig(cs *api.ContainerService) { // If no user-configurable kubelet config values exists, use the defaults setMissingKubeletValues(o.KubernetesConfig, defaultKubeletConfig) - addDefaultFeatureGates(o.KubernetesConfig.KubeletConfig, "", "") + addDefaultFeatureGates(o.KubernetesConfig.KubeletConfig, o.OrchestratorVersion, "", "") // Override default cloud-provider? if helpers.IsTrueBoolPointer(o.KubernetesConfig.UseCloudControllerManager) { @@ -91,7 +91,7 @@ func setKubeletConfig(cs *api.ContainerService) { cs.Properties.MasterProfile.KubernetesConfig = &api.KubernetesConfig{} } setMissingKubeletValues(cs.Properties.MasterProfile.KubernetesConfig, o.KubernetesConfig.KubeletConfig) - addDefaultFeatureGates(cs.Properties.MasterProfile.KubernetesConfig.KubeletConfig, "", "") + addDefaultFeatureGates(cs.Properties.MasterProfile.KubernetesConfig.KubeletConfig, o.OrchestratorVersion, "", "") } // Agent-specific kubelet config changes go here @@ -100,15 +100,15 @@ func setKubeletConfig(cs *api.ContainerService) { profile.KubernetesConfig = &api.KubernetesConfig{} } setMissingKubeletValues(profile.KubernetesConfig, o.KubernetesConfig.KubeletConfig) - addDefaultFeatureGates(profile.KubernetesConfig.KubeletConfig, o.OrchestratorVersion, "Accelerators=true") + addDefaultFeatureGates(profile.KubernetesConfig.KubeletConfig, o.OrchestratorVersion, "1.6.0", "Accelerators=true") } } // combine user-provided --feature-gates vals with defaults // a minimum k8s version may be declared as required for defaults assignment -func addDefaultFeatureGates(m map[string]string, minVersion string, defaults string) { +func addDefaultFeatureGates(m map[string]string, version string, minVersion string, defaults string) { if minVersion != "" { - if isKubernetesVersionGe(minVersion, "1.6.0") { + if isKubernetesVersionGe(version, minVersion) { m["--feature-gates"] = combineValues(m["--feature-gates"], defaults) } else { m["--feature-gates"] = combineValues(m["--feature-gates"], "") From afa754129b58c189fb56259fa1e2eda96c1c21ef Mon Sep 17 00:00:00 2001 From: Stuart Leeks Date: Fri, 12 Jan 2018 10:23:11 +0000 Subject: [PATCH 7/9] Add tests for --feature-gates behaviour Test for only adding Accelerators=true for 1.6.0+ Test for correct application of KubeletConfig from top-level vs Master/AgentProfile --- pkg/acsengine/defaults_test.go | 114 +++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/pkg/acsengine/defaults_test.go b/pkg/acsengine/defaults_test.go index 07f09b1ac9..59d56a0524 100644 --- a/pkg/acsengine/defaults_test.go +++ b/pkg/acsengine/defaults_test.go @@ -287,6 +287,100 @@ func TestPointerToBool(t *testing.T) { } } +func TestKubeletFeatureGatesEnsureAcceleratorsNotSetFor1_5_0(t *testing.T) { + mockCS := getMockBaseContainerService("1.5.0") + properties := mockCS.Properties + + // No KubernetesConfig.KubeletConfig set for MasterProfile or AgentProfile + // so they will inherit the top-level config + properties.OrchestratorProfile.KubernetesConfig = getKubernetesConfigWithFeatureGates("TopLevel=true") + + setKubeletConfig(&mockCS) + + // Verify that the Accelerators feature gate has not been applied to master or agents + agentFeatureGates := properties.AgentPoolProfiles[0].KubernetesConfig.KubeletConfig["--feature-gates"] + if agentFeatureGates != "TopLevel=true" { + t.Fatalf("setKubeletConfig modified the agent profile (version 1.5.0): expected 'TopLevel=true' got '%s'", agentFeatureGates) + } + masterFeatureFates := properties.MasterProfile.KubernetesConfig.KubeletConfig["--feature-gates"] + if masterFeatureFates != "TopLevel=true" { + t.Fatalf("setKubeletConfig modified feature gates for master profile: 'TopLevel=true' got '%s'", agentFeatureGates) + } +} +func TestKubeletFeatureGatesEnsureAcceleratorsOnAgentsFor1_6_0(t *testing.T) { + mockCS := getMockBaseContainerService("1.6.0") + properties := mockCS.Properties + + // No KubernetesConfig.KubeletConfig set for MasterProfile or AgentProfile + // so they will inherit the top-level config + properties.OrchestratorProfile.KubernetesConfig = getKubernetesConfigWithFeatureGates("TopLevel=true") + + setKubeletConfig(&mockCS) + + agentFeatureGates := properties.AgentPoolProfiles[0].KubernetesConfig.KubeletConfig["--feature-gates"] + if agentFeatureGates != "Accelerators=true,TopLevel=true" { + t.Fatalf("setKubeletConfig did not add 'Accelerators=true' for agent profile: expected 'Accelerators=true;TopLevel=true' got '%s'", agentFeatureGates) + } + + // Verify that the Accelerators feature gate override has only been applied to the agents + masterFeatureFates := properties.MasterProfile.KubernetesConfig.KubeletConfig["--feature-gates"] + if masterFeatureFates != "TopLevel=true" { + t.Fatalf("setKubeletConfig modified feature gates for master profile: expected 'TopLevel=true' got '%s'", agentFeatureGates) + } +} + +func TestKubeletFeatureGatesEnsureMasterAndAgentConfigUsedFor1_5_0(t *testing.T) { + mockCS := getMockBaseContainerService("1.5.0") + properties := mockCS.Properties + + // Set MasterProfile and AgentProfiles KubernetesConfig.KubeletConfit ig values + // Verify that they are used instead of the top-level config + properties.OrchestratorProfile.KubernetesConfig = getKubernetesConfigWithFeatureGates("TopLevel=true") + properties.MasterProfile = &api.MasterProfile{KubernetesConfig: getKubernetesConfigWithFeatureGates("MasterLevel=true")} + properties.AgentPoolProfiles[0].KubernetesConfig = getKubernetesConfigWithFeatureGates("AgentLevel=true") + + setKubeletConfig(&mockCS) + + agentFeatureGates := properties.AgentPoolProfiles[0].KubernetesConfig.KubeletConfig["--feature-gates"] + if agentFeatureGates != "AgentLevel=true" { + t.Fatalf("setKubeletConfig agent profile: expected 'AgentLevel=true' got '%s'", agentFeatureGates) + } + + // Verify that the Accelerators feature gate override has only been applied to the agents + masterFeatureFates := properties.MasterProfile.KubernetesConfig.KubeletConfig["--feature-gates"] + if masterFeatureFates != "MasterLevel=true" { + t.Fatalf("setKubeletConfig master profile: expected 'MasterLevel=true' got '%s'", agentFeatureGates) + } +} + +func TestKubeletFeatureGatesEnsureMasterAndAgentConfigUsedFor1_6_0(t *testing.T) { + mockCS := getMockBaseContainerService("1.6.0") + properties := mockCS.Properties + + // Set MasterProfile and AgentProfiles KubernetesConfig.KubeletConfig values + // Verify that they are used instead of the top-level config + properties.OrchestratorProfile.KubernetesConfig = getKubernetesConfigWithFeatureGates("TopLevel=true") + properties.MasterProfile = &api.MasterProfile{KubernetesConfig: getKubernetesConfigWithFeatureGates("MasterLevel=true")} + properties.AgentPoolProfiles[0].KubernetesConfig = getKubernetesConfigWithFeatureGates("AgentLevel=true") + + setKubeletConfig(&mockCS) + + agentFeatureGates := properties.AgentPoolProfiles[0].KubernetesConfig.KubeletConfig["--feature-gates"] + if agentFeatureGates != "Accelerators=true,AgentLevel=true" { + t.Fatalf("setKubeletConfig agent profile: expected 'Accelerators=true,AgentLevel=true' got '%s'", agentFeatureGates) + } + + // Verify that the Accelerators feature gate override has only been applied to the agents + masterFeatureFates := properties.MasterProfile.KubernetesConfig.KubeletConfig["--feature-gates"] + if masterFeatureFates != "MasterLevel=true" { + t.Fatalf("setKubeletConfig master profile: expected 'MasterLevel=true' got '%s'", agentFeatureGates) + } +} + +// TODO - tests: +// - check merging +// - check that master/agent overrides are used correctly + func getMockAddon(name string) api.KubernetesAddon { return api.KubernetesAddon{ Name: name, @@ -302,3 +396,23 @@ func getMockAddon(name string) api.KubernetesAddon { }, } } + +func getMockBaseContainerService(orchestratorVersion string) api.ContainerService { + return api.ContainerService{ + Properties: &api.Properties{ + OrchestratorProfile: &api.OrchestratorProfile{ + OrchestratorVersion: orchestratorVersion, + KubernetesConfig: &api.KubernetesConfig{}, + }, + MasterProfile: &api.MasterProfile{}, + AgentPoolProfiles: []*api.AgentPoolProfile{ + {}, + }, + }, + } +} +func getKubernetesConfigWithFeatureGates(featureGates string) *api.KubernetesConfig { + return &api.KubernetesConfig{ + KubeletConfig: map[string]string{"--feature-gates": featureGates}, + } +} From e5f7cd678e306e5b172202c11a0b816ffed28fc3 Mon Sep 17 00:00:00 2001 From: Stuart Leeks Date: Fri, 12 Jan 2018 11:10:50 +0000 Subject: [PATCH 8/9] Avoid ref sharing for kubeletConfig Fix failing test by avoiding reference sharing of kubeletConfig properites on master and agent profiles --- pkg/acsengine/defaults-kubelet.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/acsengine/defaults-kubelet.go b/pkg/acsengine/defaults-kubelet.go index ae4ac505af..1363a3466b 100644 --- a/pkg/acsengine/defaults-kubelet.go +++ b/pkg/acsengine/defaults-kubelet.go @@ -89,6 +89,7 @@ func setKubeletConfig(cs *api.ContainerService) { if cs.Properties.MasterProfile != nil { if cs.Properties.MasterProfile.KubernetesConfig == nil { cs.Properties.MasterProfile.KubernetesConfig = &api.KubernetesConfig{} + cs.Properties.MasterProfile.KubernetesConfig.KubeletConfig = copyMap(cs.Properties.MasterProfile.KubernetesConfig.KubeletConfig) } setMissingKubeletValues(cs.Properties.MasterProfile.KubernetesConfig, o.KubernetesConfig.KubeletConfig) addDefaultFeatureGates(cs.Properties.MasterProfile.KubernetesConfig.KubeletConfig, o.OrchestratorVersion, "", "") @@ -98,6 +99,7 @@ func setKubeletConfig(cs *api.ContainerService) { for _, profile := range cs.Properties.AgentPoolProfiles { if profile.KubernetesConfig == nil { profile.KubernetesConfig = &api.KubernetesConfig{} + profile.KubernetesConfig.KubeletConfig = copyMap(profile.KubernetesConfig.KubeletConfig) } setMissingKubeletValues(profile.KubernetesConfig, o.KubernetesConfig.KubeletConfig) addDefaultFeatureGates(profile.KubernetesConfig.KubeletConfig, o.OrchestratorVersion, "1.6.0", "Accelerators=true") @@ -131,7 +133,13 @@ func setMissingKubeletValues(p *api.KubernetesConfig, d map[string]string) { } } } - +func copyMap(input map[string]string) map[string]string { + copy := map[string]string{} + for key, value := range input { + copy[key] = value + } + return copy +} func combineValues(inputs ...string) string { var valueMap map[string]string valueMap = make(map[string]string) From 0ace7993b8c2e8e324958eb00f71b54380ab3a5e Mon Sep 17 00:00:00 2001 From: Stuart Leeks Date: Tue, 16 Jan 2018 22:29:57 +0000 Subject: [PATCH 9/9] Remove outdated comments --- pkg/acsengine/defaults_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/acsengine/defaults_test.go b/pkg/acsengine/defaults_test.go index 59d56a0524..ab78a51879 100644 --- a/pkg/acsengine/defaults_test.go +++ b/pkg/acsengine/defaults_test.go @@ -377,10 +377,6 @@ func TestKubeletFeatureGatesEnsureMasterAndAgentConfigUsedFor1_6_0(t *testing.T) } } -// TODO - tests: -// - check merging -// - check that master/agent overrides are used correctly - func getMockAddon(name string) api.KubernetesAddon { return api.KubernetesAddon{ Name: name,