From 8a985d569a677dc1f7ddc622856f7b5c61085e47 Mon Sep 17 00:00:00 2001 From: Jack Francis Date: Tue, 24 Jul 2018 16:17:30 -0700 Subject: [PATCH 1/5] 30 max pods should be Azure CNI-only --- pkg/acsengine/defaults-kubelet.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/acsengine/defaults-kubelet.go b/pkg/acsengine/defaults-kubelet.go index 173dd49da8..fd3cca54d5 100644 --- a/pkg/acsengine/defaults-kubelet.go +++ b/pkg/acsengine/defaults-kubelet.go @@ -36,7 +36,7 @@ func setKubeletConfig(cs *api.ContainerService) { "--cluster-domain": "cluster.local", "--network-plugin": "cni", "--pod-infra-container-image": cloudSpecConfig.KubernetesSpecConfig.KubernetesImageBase + KubeConfigs[o.OrchestratorVersion]["pause"], - "--max-pods": strconv.Itoa(DefaultKubernetesMaxPodsVNETIntegrated), + "--max-pods": strconv.Itoa(DefaultKubernetesMaxPods), "--eviction-hard": DefaultKubernetesHardEvictionThreshold, "--node-status-update-frequency": KubeConfigs[o.OrchestratorVersion]["nodestatusfreq"], "--image-gc-high-threshold": strconv.Itoa(DefaultKubernetesGCHighThreshold), @@ -65,7 +65,11 @@ func setKubeletConfig(cs *api.ContainerService) { if o.KubernetesConfig.NetworkPolicy != NetworkPolicyCalico { o.KubernetesConfig.KubeletConfig["--network-plugin"] = NetworkPluginKubenet } - o.KubernetesConfig.KubeletConfig["--max-pods"] = strconv.Itoa(DefaultKubernetesMaxPods) + } + + // Apply Azure CNI-specific --max-pods value + if o.KubernetesConfig.NetworkPlugin == NetworkPluginAzure { + o.KubernetesConfig.KubeletConfig["--max-pods"] = strconv.Itoa(DefaultKubernetesMaxPodsVNETIntegrated) } // We don't support user-configurable values for the following, From d82fd02ced31a04ff87b33925fe0ef105673de20 Mon Sep 17 00:00:00 2001 From: Jack Francis Date: Tue, 24 Jul 2018 16:22:20 -0700 Subject: [PATCH 2/5] we want to set the default config --- 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 fd3cca54d5..df5b6cfae6 100644 --- a/pkg/acsengine/defaults-kubelet.go +++ b/pkg/acsengine/defaults-kubelet.go @@ -51,6 +51,11 @@ func setKubeletConfig(cs *api.ContainerService) { "--image-pull-progress-deadline": "30m", } + // Apply Azure CNI-specific --max-pods value + if o.KubernetesConfig.NetworkPlugin == NetworkPluginAzure { + defaultKubeletConfig["--max-pods"] = strconv.Itoa(DefaultKubernetesMaxPodsVNETIntegrated) + } + // If no user-configurable kubelet config values exists, use the defaults setMissingKubeletValues(o.KubernetesConfig, defaultKubeletConfig) addDefaultFeatureGates(o.KubernetesConfig.KubeletConfig, o.OrchestratorVersion, "", "") @@ -67,11 +72,6 @@ func setKubeletConfig(cs *api.ContainerService) { } } - // Apply Azure CNI-specific --max-pods value - if o.KubernetesConfig.NetworkPlugin == NetworkPluginAzure { - o.KubernetesConfig.KubeletConfig["--max-pods"] = strconv.Itoa(DefaultKubernetesMaxPodsVNETIntegrated) - } - // We don't support user-configurable values for the following, // so any of the value assignments below will override user-provided values var overrideKubeletConfig map[string]string From d2cd5a0b60cd6d8f829b07329506f8384bc0424f Mon Sep 17 00:00:00 2001 From: Jack Francis Date: Tue, 24 Jul 2018 16:55:40 -0700 Subject: [PATCH 3/5] add unit tests for user overrides --- pkg/acsengine/defaults-kubelet_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/pkg/acsengine/defaults-kubelet_test.go b/pkg/acsengine/defaults-kubelet_test.go index b2c51d78c3..72945a47b7 100644 --- a/pkg/acsengine/defaults-kubelet_test.go +++ b/pkg/acsengine/defaults-kubelet_test.go @@ -175,6 +175,27 @@ func TestKubeletMaxPods(t *testing.T) { t.Fatalf("got unexpected '--max-pods' kubelet config value for NetworkPolicy=%s: %s", NetworkPluginKubenet, k["--max-pods"]) } + + // Test that user-overrides for --max-pods work as intended + cs = CreateMockContainerService("testcluster", defaultTestClusterVer, 3, 2, false) + cs.Properties.OrchestratorProfile.KubernetesConfig.NetworkPlugin = NetworkPluginKubenet + cs.Properties.OrchestratorProfile.KubernetesConfig.KubeletConfig["--max-pods"] = "99" + setKubeletConfig(cs) + k = cs.Properties.OrchestratorProfile.KubernetesConfig.KubeletConfig + if k["--max-pods"] != "99" { + t.Fatalf("got unexpected '--max-pods' kubelet config value for NetworkPolicy=%s: %s", + NetworkPluginKubenet, k["--max-pods"]) + } + + cs = CreateMockContainerService("testcluster", defaultTestClusterVer, 3, 2, false) + cs.Properties.OrchestratorProfile.KubernetesConfig.NetworkPlugin = NetworkPluginAzure + cs.Properties.OrchestratorProfile.KubernetesConfig.KubeletConfig["--max-pods"] = "99" + setKubeletConfig(cs) + k = cs.Properties.OrchestratorProfile.KubernetesConfig.KubeletConfig + if k["--max-pods"] != "99" { + t.Fatalf("got unexpected '--max-pods' kubelet config value for NetworkPolicy=%s: %s", + NetworkPluginKubenet, k["--max-pods"]) + } } func TestKubeletCalico(t *testing.T) { From 62529f0bd912861c1cd1b3117a5fb29d98cf2e18 Mon Sep 17 00:00:00 2001 From: Jack Francis Date: Wed, 25 Jul 2018 09:14:56 -0700 Subject: [PATCH 4/5] rationalize AKS flow as well --- pkg/api/convertertoagentpoolonlyapi.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/api/convertertoagentpoolonlyapi.go b/pkg/api/convertertoagentpoolonlyapi.go index df6e219204..b6d7642234 100644 --- a/pkg/api/convertertoagentpoolonlyapi.go +++ b/pkg/api/convertertoagentpoolonlyapi.go @@ -487,11 +487,10 @@ func convertV20180331AgentPoolOnlyAgentPoolProfile(agentPoolProfile *v20180331.A var maxPods string // agentPoolProfile.MaxPods is 0 if maxPods field is not provided in API model if agentPoolProfile.MaxPods == nil { - // default is kubenet - if networkProfile == nil || networkProfile.NetworkPlugin == v20180331.Kubenet { - maxPods = DefaultKubernetesMaxPodsKubenet - } else { + if networkProfile.NetworkPlugin == v20180331.Azure { maxPods = DefaultKubernetesMaxPodsAzureCNI + } else { + maxPods = DefaultKubernetesMaxPodsKubenet } } else { maxPods = strconv.Itoa(*agentPoolProfile.MaxPods) From 4a56a7b5d1b6aad960f426c8cc0e3337947c5285 Mon Sep 17 00:00:00 2001 From: Jack Francis Date: Wed, 25 Jul 2018 09:47:34 -0700 Subject: [PATCH 5/5] nil pointer guard --- pkg/api/convertertoagentpoolonlyapi.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/api/convertertoagentpoolonlyapi.go b/pkg/api/convertertoagentpoolonlyapi.go index b6d7642234..2bb67319c1 100644 --- a/pkg/api/convertertoagentpoolonlyapi.go +++ b/pkg/api/convertertoagentpoolonlyapi.go @@ -487,7 +487,7 @@ func convertV20180331AgentPoolOnlyAgentPoolProfile(agentPoolProfile *v20180331.A var maxPods string // agentPoolProfile.MaxPods is 0 if maxPods field is not provided in API model if agentPoolProfile.MaxPods == nil { - if networkProfile.NetworkPlugin == v20180331.Azure { + if networkProfile != nil && networkProfile.NetworkPlugin == v20180331.Azure { maxPods = DefaultKubernetesMaxPodsAzureCNI } else { maxPods = DefaultKubernetesMaxPodsKubenet