Remove kubelet extra args when kubelet configuration is set

aws · Jun 10, 2024 · f10b3eb · f10b3eb
1 parent d485120
commit f10b3eb
Show file tree

Hide file tree

Showing 16 changed files with 99 additions and 100 deletions.
diff --git a/pkg/providers/cloudstack/config/template-cp.yaml b/pkg/providers/cloudstack/config/template-cp.yaml
@@ -158,7 +158,8 @@ spec:
     files:
 {{- if .kubeletConfiguration }}
     - content: |
-{{ .kubeletConfiguration | indent 8}}
+{{ .kubeletConfiguration | indent 8 -}}
+        providerID: cloudstack:///'{{`{{ ds.meta_data.instance_id }}`}}'
       owner: root:root
       permissions: "0644"
       path: /etc/kubernetes/patches/kubeletconfiguration0+strategic.yaml
@@ -307,12 +308,14 @@ spec:
 {{- end }}
       nodeRegistration:
         criSocket: /var/run/containerd/containerd.sock
+{{- if not .kubeletConfiguration }}
         kubeletExtraArgs:
           provider-id: cloudstack:///'{{`{{ ds.meta_data.instance_id }}`}}'
           read-only-port: "0"
           anonymous-auth: "false"
 {{- if .kubeletExtraArgs }}
 {{ .kubeletExtraArgs.ToYaml | indent 10 }}
+{{- end }}
 {{- end }}
         name: "{{`{{ ds.meta_data.hostname }}`}}"
 {{- if .controlPlaneTaints }}
@@ -333,12 +336,14 @@ spec:
 {{- end }}
       nodeRegistration:
         criSocket: /var/run/containerd/containerd.sock
+{{- if not .kubeletConfiguration }}
         kubeletExtraArgs:
           provider-id: cloudstack:///'{{`{{ ds.meta_data.instance_id }}`}}'
           read-only-port: "0"
           anonymous-auth: "false"
 {{- if .kubeletExtraArgs }}
 {{ .kubeletExtraArgs.ToYaml | indent 10 }}
+{{- end }}
 {{- end }}
         name: "{{`{{ ds.meta_data.hostname }}`}}"
 {{- if .controlPlaneTaints }}

diff --git a/pkg/providers/cloudstack/config/template-md.yaml b/pkg/providers/cloudstack/config/template-md.yaml
@@ -25,20 +25,23 @@ spec:
 {{- else}}
           taints: []
 {{- end }}
+{{- if not .kubeletConfiguration }}
           kubeletExtraArgs:
             provider-id: cloudstack:///'{{`{{ ds.meta_data.instance_id }}`}}'
             read-only-port: "0"
             anonymous-auth: "false"
 {{- if .kubeletExtraArgs }}
 {{ .kubeletExtraArgs.ToYaml | indent 12 }}
+{{- end }}
 {{- end }}
           name: "{{`{{ ds.meta_data.hostname }}`}}"
 {{- if or (or .proxyConfig .registryMirrorMap) .kubeletConfiguration }}
       files:
 {{- end }}
 {{- if .kubeletConfiguration }}
       - content: |
-{{ .kubeletConfiguration | indent 10 }}
+{{ .kubeletConfiguration | indent 10 -}}
+          providerID: cloudstack:///'{{`{{ ds.meta_data.instance_id }}`}}'
         owner: root:root
         permissions: "0644"
         path: /etc/kubernetes/patches/kubeletconfiguration0+strategic.yaml

diff --git a/pkg/providers/cloudstack/template.go b/pkg/providers/cloudstack/template.go
@@ -118,9 +118,6 @@ func buildTemplateMapCP(clusterSpec *cluster.Spec) (map[string]interface{}, erro
 
 	etcdExtraArgs := clusterapi.SecureEtcdTlsCipherSuitesExtraArgs()
 	sharedExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs()
-	kubeletExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs().
-		Append(clusterapi.ResolvConfExtraArgs(clusterSpec.Cluster.Spec.ClusterNetwork.DNS.ResolvConf)).
-		Append(clusterapi.ControlPlaneNodeLabelsExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration))
 	apiServerExtraArgs := clusterapi.OIDCToExtraArgs(clusterSpec.OIDCConfig).
 		Append(clusterapi.AwsIamAuthExtraArgs(clusterSpec.AWSIamConfig)).
 		Append(clusterapi.APIServerExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration.APIServerExtraArgs)).
@@ -190,7 +187,6 @@ func buildTemplateMapCP(clusterSpec *cluster.Spec) (map[string]interface{}, erro
 		"podCidrs":                                   clusterSpec.Cluster.Spec.ClusterNetwork.Pods.CidrBlocks,
 		"serviceCidrs":                               clusterSpec.Cluster.Spec.ClusterNetwork.Services.CidrBlocks,
 		"apiserverExtraArgs":                         apiServerExtraArgs.ToPartialYaml(),
-		"kubeletExtraArgs":                           kubeletExtraArgs.ToPartialYaml(),
 		"etcdExtraArgs":                              etcdExtraArgs.ToPartialYaml(),
 		"etcdCipherSuites":                           crypto.SecureCipherSuitesString(),
 		"controllermanagerExtraArgs":                 controllerManagerExtraArgs.ToPartialYaml(),
@@ -266,8 +262,12 @@ func buildTemplateMapCP(clusterSpec *cluster.Spec) (map[string]interface{}, erro
 		if err != nil {
 			return nil, fmt.Errorf("error marshaling %v", err)
 		}
-
 		values["kubeletConfiguration"] = string(kcString)
+	} else {
+		kubeletExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs().
+			Append(clusterapi.ResolvConfExtraArgs(clusterSpec.Cluster.Spec.ClusterNetwork.DNS.ResolvConf)).
+			Append(clusterapi.ControlPlaneNodeLabelsExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration))
+		values["kubeletExtraArgs"] = kubeletExtraArgs.ToPartialYaml()
 	}
 
 	return values, nil
@@ -349,9 +349,6 @@ func fillProxyConfigurations(values map[string]interface{}, clusterSpec *cluster
 func buildTemplateMapMD(clusterSpec *cluster.Spec, workerNodeGroupConfiguration v1alpha1.WorkerNodeGroupConfiguration) (map[string]interface{}, error) {
 	versionsBundle := clusterSpec.WorkerNodeGroupVersionsBundle(workerNodeGroupConfiguration)
 	format := "cloud-config"
-	kubeletExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs().
-		Append(clusterapi.WorkerNodeLabelsExtraArgs(workerNodeGroupConfiguration)).
-		Append(clusterapi.ResolvConfExtraArgs(clusterSpec.Cluster.Spec.ClusterNetwork.DNS.ResolvConf))
 
 	workerNodeGroupMachineSpec := workerMachineConfig(clusterSpec, workerNodeGroupConfiguration).Spec
 	workerUser := workerNodeGroupMachineSpec.Users[0]
@@ -376,7 +373,6 @@ func buildTemplateMapMD(clusterSpec *cluster.Spec, workerNodeGroupConfiguration
 		"workerSshUsername":                workerNodeGroupMachineSpec.Users[0].Name,
 		"cloudstackWorkerSshAuthorizedKey": workerSSHKey,
 		"format":                           format,
-		"kubeletExtraArgs":                 kubeletExtraArgs.ToPartialYaml(),
 		"eksaSystemNamespace":              constants.EksaSystemNamespace,
 		"workerNodeGroupName":              fmt.Sprintf("%s-%s", clusterSpec.Cluster.Name, workerNodeGroupConfiguration.Name),
 		"workerNodeGroupTaints":            workerNodeGroupConfiguration.Taints,
@@ -410,6 +406,11 @@ func buildTemplateMapMD(clusterSpec *cluster.Spec, workerNodeGroupConfiguration
 		}
 
 		values["kubeletConfiguration"] = string(kcString)
+	} else {
+		kubeletExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs().
+			Append(clusterapi.WorkerNodeLabelsExtraArgs(workerNodeGroupConfiguration)).
+			Append(clusterapi.ResolvConfExtraArgs(clusterSpec.Cluster.Spec.ClusterNetwork.DNS.ResolvConf))
+		values["kubeletExtraArgs"] = kubeletExtraArgs.ToPartialYaml()
 	}
 
 	return values, nil

diff --git a/pkg/providers/cloudstack/template_test.go b/pkg/providers/cloudstack/template_test.go
@@ -179,9 +179,12 @@ func TestVsphereTemplateBuilderGenerateCAPISpecControlPlaneValidKubeletConfigWN(
 			"maxPods": 20,
 		},
 	}
+
 	builder := cloudstack.NewTemplateBuilder(time.Now)
-	_, err := builder.GenerateCAPISpecWorkers(spec, nil, nil)
+	data, err := builder.GenerateCAPISpecWorkers(spec, nil, nil)
 	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(data).To(ContainSubstring("maxPods"))
+	g.Expect(data).To(ContainSubstring("providerID"))
 }
 
 func TestVsphereTemplateBuilderGenerateCAPISpecControlPlaneValidKubeletConfigCP(t *testing.T) {
@@ -194,8 +197,10 @@ func TestVsphereTemplateBuilderGenerateCAPISpecControlPlaneValidKubeletConfigCP(
 	}
 	spec.Cluster.Spec.ExternalEtcdConfiguration = nil
 	builder := cloudstack.NewTemplateBuilder(time.Now)
-	_, err := builder.GenerateCAPISpecControlPlane(spec, func(values map[string]interface{}) {
+	data, err := builder.GenerateCAPISpecControlPlane(spec, func(values map[string]interface{}) {
 		values["controlPlaneTemplateName"] = clusterapi.ControlPlaneMachineTemplateName(spec.Cluster)
 	})
 	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(data).To(ContainSubstring("maxPods"))
+	g.Expect(data).To(ContainSubstring("providerID"))
 }
diff --git a/pkg/providers/docker/config/template-cp.yaml b/pkg/providers/docker/config/template-cp.yaml
@@ -222,11 +222,13 @@ spec:
 {{- end }}
       nodeRegistration:
         criSocket: /var/run/containerd/containerd.sock
+{{- if not .kubeletConfiguration }}
         kubeletExtraArgs:
           eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0%
 {{- if .kubeletExtraArgs }}
 {{ .kubeletExtraArgs.ToYaml | indent 10 }}
 {{- end }}
+{{- end }}
 {{- if not .workerNodeGroupConfigurations }}
         taints: []
 {{- end }}
@@ -247,11 +249,13 @@ spec:
 {{- end }}
       nodeRegistration:
         criSocket: /var/run/containerd/containerd.sock
+{{- if not .kubeletConfiguration }}
         kubeletExtraArgs:
           eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0%
 {{- if .kubeletExtraArgs }}
 {{ .kubeletExtraArgs.ToYaml | indent 10 }}
 {{- end }}
+{{- end }}
 {{- if not .workerNodeGroupConfigurations }}
         taints: []
 {{- end }}

diff --git a/pkg/providers/docker/config/template-md.yaml b/pkg/providers/docker/config/template-md.yaml
@@ -25,11 +25,13 @@ spec:
 {{- else}}
           taints: []
 {{- end }}
+{{- if not .kubeletConfiguration }}
           kubeletExtraArgs:
             eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0%
 {{- if .kubeletExtraArgs }}
 {{ .kubeletExtraArgs.ToYaml | indent 12 }}
 {{- end }}
+{{- end }}
 {{- if or .registryMirrorMap .kubeletConfiguration }}
       files:
 {{- end }}

diff --git a/pkg/providers/docker/controlplane_test.go b/pkg/providers/docker/controlplane_test.go
@@ -98,18 +98,6 @@ func TestControlPlaneSpecNewCluster(t *testing.T) {
 	g.Expect(cp.EtcdMachineTemplate).To(Equal(wantEtcdMachineTemplate))
 }
 
-func TestControlPlaneSpecNoKubeVersion(t *testing.T) {
-	g := NewWithT(t)
-	logger := test.NewNullLogger()
-	ctx := context.Background()
-	client := test.NewFakeKubeClient()
-	spec := testClusterSpec()
-	spec.Cluster.Spec.KubernetesVersion = ""
-
-	_, err := docker.ControlPlaneSpec(ctx, logger, client, spec)
-	g.Expect(err).To(MatchError(ContainSubstring("generating docker control plane yaml spec")))
-}
-
 func TestControlPlaneSpecUpdateMachineTemplates(t *testing.T) {
 	g := NewWithT(t)
 	logger := test.NewNullLogger()

diff --git a/pkg/providers/docker/docker.go b/pkg/providers/docker/docker.go
@@ -281,17 +281,6 @@ func buildTemplateMapCP(clusterSpec *cluster.Spec) (map[string]interface{}, erro
 	versionsBundle := clusterSpec.RootVersionsBundle()
 	etcdExtraArgs := clusterapi.SecureEtcdTlsCipherSuitesExtraArgs()
 	sharedExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs()
-	kubeletExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs().
-		Append(clusterapi.ResolvConfExtraArgs(clusterSpec.Cluster.Spec.ClusterNetwork.DNS.ResolvConf)).
-		Append(clusterapi.ControlPlaneNodeLabelsExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration))
-
-	cgroupDriverArgs, err := kubeletCgroupDriverExtraArgs(clusterSpec.Cluster.Spec.KubernetesVersion)
-	if err != nil {
-		return nil, err
-	}
-	if cgroupDriverArgs != nil {
-		kubeletExtraArgs.Append(cgroupDriverArgs)
-	}
 
 	apiServerExtraArgs := clusterapi.OIDCToExtraArgs(clusterSpec.OIDCConfig).
 		Append(clusterapi.AwsIamAuthExtraArgs(clusterSpec.AWSIamConfig)).
@@ -316,7 +305,6 @@ func buildTemplateMapCP(clusterSpec *cluster.Spec) (map[string]interface{}, erro
 		"apiserverExtraArgs":            apiServerExtraArgs.ToPartialYaml(),
 		"controllermanagerExtraArgs":    controllerManagerExtraArgs.ToPartialYaml(),
 		"schedulerExtraArgs":            sharedExtraArgs.ToPartialYaml(),
-		"kubeletExtraArgs":              kubeletExtraArgs.ToPartialYaml(),
 		"externalEtcdVersion":           versionsBundle.KubeDistro.EtcdVersion,
 		"eksaSystemNamespace":           constants.EksaSystemNamespace,
 		"podCidrs":                      clusterSpec.Cluster.Spec.ClusterNetwork.Pods.CidrBlocks,
@@ -367,34 +355,34 @@ func buildTemplateMapCP(clusterSpec *cluster.Spec) (map[string]interface{}, erro
 		}
 
 		values["kubeletConfiguration"] = string(kcString)
+
+	} else {
+		kubeletExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs().
+			Append(clusterapi.ResolvConfExtraArgs(clusterSpec.Cluster.Spec.ClusterNetwork.DNS.ResolvConf)).
+			Append(clusterapi.ControlPlaneNodeLabelsExtraArgs(clusterSpec.Cluster.Spec.ControlPlaneConfiguration))
+
+		cgroupDriverArgs, err := kubeletCgroupDriverExtraArgs(clusterSpec.Cluster.Spec.KubernetesVersion)
+		if err != nil {
+			return nil, err
+		}
+		if cgroupDriverArgs != nil {
+			kubeletExtraArgs.Append(cgroupDriverArgs)
+		}
+
+		values["kubeletExtraArgs"] = kubeletExtraArgs.ToPartialYaml()
 	}
 
 	return values, nil
 }
 
 func buildTemplateMapMD(clusterSpec *cluster.Spec, workerNodeGroupConfiguration v1alpha1.WorkerNodeGroupConfiguration) (map[string]interface{}, error) {
-	kubeVersion := clusterSpec.Cluster.Spec.KubernetesVersion
-	if workerNodeGroupConfiguration.KubernetesVersion != nil {
-		kubeVersion = *workerNodeGroupConfiguration.KubernetesVersion
-	}
 	versionsBundle := clusterSpec.WorkerNodeGroupVersionsBundle(workerNodeGroupConfiguration)
-	kubeletExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs().
-		Append(clusterapi.WorkerNodeLabelsExtraArgs(workerNodeGroupConfiguration)).
-		Append(clusterapi.ResolvConfExtraArgs(clusterSpec.Cluster.Spec.ClusterNetwork.DNS.ResolvConf))
 
-	cgroupDriverArgs, err := kubeletCgroupDriverExtraArgs(kubeVersion)
-	if err != nil {
-		return nil, err
-	}
-	if cgroupDriverArgs != nil {
-		kubeletExtraArgs.Append(cgroupDriverArgs)
-	}
 	values := map[string]interface{}{
 		"clusterName":           clusterSpec.Cluster.Name,
 		"kubernetesVersion":     versionsBundle.KubeDistro.Kubernetes.Tag,
 		"kindNodeImage":         versionsBundle.EksD.KindNode.VersionedImage(),
 		"eksaSystemNamespace":   constants.EksaSystemNamespace,
-		"kubeletExtraArgs":      kubeletExtraArgs.ToPartialYaml(),
 		"workerReplicas":        *workerNodeGroupConfiguration.Count,
 		"workerNodeGroupName":   fmt.Sprintf("%s-%s", clusterSpec.Cluster.Name, workerNodeGroupConfiguration.Name),
 		"workerNodeGroupTaints": workerNodeGroupConfiguration.Taints,
@@ -416,6 +404,24 @@ func buildTemplateMapMD(clusterSpec *cluster.Spec, workerNodeGroupConfiguration
 		}
 
 		values["kubeletConfiguration"] = string(kcString)
+	} else {
+		kubeVersion := clusterSpec.Cluster.Spec.KubernetesVersion
+		if workerNodeGroupConfiguration.KubernetesVersion != nil {
+			kubeVersion = *workerNodeGroupConfiguration.KubernetesVersion
+		}
+		kubeletExtraArgs := clusterapi.SecureTlsCipherSuitesExtraArgs().
+			Append(clusterapi.WorkerNodeLabelsExtraArgs(workerNodeGroupConfiguration)).
+			Append(clusterapi.ResolvConfExtraArgs(clusterSpec.Cluster.Spec.ClusterNetwork.DNS.ResolvConf))
+
+		cgroupDriverArgs, err := kubeletCgroupDriverExtraArgs(kubeVersion)
+		if err != nil {
+			return nil, err
+		}
+		if cgroupDriverArgs != nil {
+			kubeletExtraArgs.Append(cgroupDriverArgs)
+		}
+
+		values["kubeletExtraArgs"] = kubeletExtraArgs.ToPartialYaml()
 	}
 
 	return values, nil

diff --git a/pkg/providers/docker/docker_test.go b/pkg/providers/docker/docker_test.go
@@ -827,17 +827,6 @@ func TestDockerTemplateBuilderGenerateCAPISpecControlPlane(t *testing.T) {
 			},
 			wantErr: nil,
 		},
-		{
-			name: "kube version not specified",
-			args: args{
-				clusterSpec: test.NewClusterSpec(func(s *cluster.Spec) {
-					s.Cluster.Name = "test-cluster"
-					s.Cluster.Spec.KubernetesVersion = ""
-				}),
-				buildOptions: nil,
-			},
-			wantErr: fmt.Errorf("error building template map for CP "),
-		},
 		{
 			name: "kubelet config specified",
 			args: args{
@@ -922,16 +911,6 @@ func TestDockerTemplateBuilderGenerateCAPISpecWorkers(t *testing.T) {
 		args    args
 		wantErr error
 	}{
-		{
-			name: "kube version not specified",
-			args: args{
-				clusterSpec: test.NewClusterSpec(func(s *cluster.Spec) {
-					s.Cluster.Name = "test-cluster"
-					s.Cluster.Spec.KubernetesVersion = ""
-				}),
-			},
-			wantErr: fmt.Errorf("error building template map for MD "),
-		},
 		{
 			name: "kubelet config specified",
 			args: args{

diff --git a/pkg/providers/docker/reconciler/reconciler_test.go b/pkg/providers/docker/reconciler/reconciler_test.go
@@ -336,15 +336,6 @@ func TestReconcileControlPlaneUnstackedEtcdSuccess(t *testing.T) {
 	)
 }
 
-func TestReconcilerReconcileControlPlaneFailure(t *testing.T) {
-	tt := newReconcilerTest(t)
-	tt.createAllObjs()
-	spec := tt.buildSpec()
-	spec.Cluster.Spec.KubernetesVersion = ""
-	_, err := tt.reconciler().ReconcileControlPlane(tt.ctx, test.NewNullLogger(), spec)
-	tt.Expect(err).To(MatchError(ContainSubstring("generating docker control plane yaml spec")))
-}
-
 type reconcilerTest struct {
 	t testing.TB
 	*WithT

diff --git a/pkg/providers/nutanix/config/cp-template.yaml b/pkg/providers/nutanix/config/cp-template.yaml
@@ -337,10 +337,12 @@ spec:
           # We have to pin the cgroupDriver to cgroupfs as kubeadm >=1.21 defaults to systemd
           # kind will implement systemd support in: https://github.com/kubernetes-sigs/kind/issues/1726
           #cgroup-driver: cgroupfs
+{{- if not .kubeletConfiguration }}
           eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0%
 {{- if .kubeletExtraArgs }}
 {{ .kubeletExtraArgs.ToYaml | indent 10 }}
 {{- end }}
+{{- end }}
 {{- if .controlPlaneTaints }}
         taints:
 {{- range .controlPlaneTaints}}
@@ -361,11 +363,13 @@ spec:
         criSocket: /var/run/containerd/containerd.sock
         kubeletExtraArgs:
           cloud-provider: external
+{{- if not .kubeletConfiguration }}
           read-only-port: "0"
           anonymous-auth: "false"
 {{- if .kubeletExtraArgs }}
 {{ .kubeletExtraArgs.ToYaml | indent 10 }}
 {{- end }}
+{{- end }}
 {{- if .controlPlaneTaints }}
         taints:
 {{- range .controlPlaneTaints}}