Add --feature-gates handling for kubelet and api server (Azure#2032)

* Add --feature-gates handling For kubeletConfig, preserve the existing behaviour of adding Accelerators=true for agent config (for kubernetes 1.6.0 and later) * simplified default implementation and removed KUBELET_FEATURE_GATES * unnecessary default assignment, and simple validation * removed copyMap func * enforce min version, "Accelerators=true" is only for agents * Pass current version in to addDefaultFeatureGates Generalise addDefaultFeatureGates by passing in current orchestrator version, and deferring the minimum required version to callers * 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 * Avoid ref sharing for kubeletConfig Fix failing test by avoiding reference sharing of kubeletConfig properites on master and agent profiles * Remove outdated comments
jessfraz · Jan 17, 2018 · 0f56ecb · 0f56ecb
1 parent 8bd7c2c
commit 0f56ecb
Show file tree

Hide file tree

Showing 6 changed files with 221 additions and 3 deletions.
diff --git a/examples/feature-gates/kubernetes-featuresgates.json 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": ""
+    }
+  }
+}
diff --git 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
@@ -37,7 +37,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 $KUBELET_OPTS \

diff --git a/parts/k8s/kubernetesagentcustomdata.yml b/parts/k8s/kubernetesagentcustomdata.yml
@@ -111,7 +111,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
@@ -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"
@@ -50,6 +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, "", "")
 
 	// Override default cloud-provider?
 	if helpers.IsTrueBoolPointer(o.KubernetesConfig.UseCloudControllerManager) {
@@ -91,15 +96,34 @@ 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, "", "")
+
 	}
 	// Agent-specific kubelet config changes go here
 	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")
+	}
+}
+
+// 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, version string, minVersion string, defaults string) {
+	if minVersion != "" {
+		if isKubernetesVersionGe(version, minVersion) {
+			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)
 	}
 }
 
@@ -116,3 +140,44 @@ 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, "=")
+		if len(valueParts) == 2 {
+			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(), ",")
+}
diff --git a/pkg/acsengine/defaults_test.go b/pkg/acsengine/defaults_test.go
@@ -287,6 +287,96 @@ 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)
+	}
+}
+
 func getMockAddon(name string) api.KubernetesAddon {
 	return api.KubernetesAddon{
 		Name:    name,
@@ -302,3 +392,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},
+	}
+}