From 654197d6ba486b84a92976946904c2f50255f3df Mon Sep 17 00:00:00 2001 From: Jimmi Dyson Date: Thu, 13 Mar 2025 21:15:55 +0000 Subject: [PATCH] fix: Retain existing join and init kubeadm config when adding taints Previously a dodgy piece of logic overwrote the whole of the join and init kubeadm config when adding taints due to an incorrect `!=` which should have been `==`. --- .../pkg/testutils/capitest/request/items.go | 18 ++++- .../credentials/inject_test.go | 65 ++++++++++++------- .../mutation/taints/inject_controlplane.go | 4 +- .../generic/mutation/taints/inject_worker.go | 2 +- 4 files changed, 59 insertions(+), 30 deletions(-) diff --git a/common/pkg/testutils/capitest/request/items.go b/common/pkg/testutils/capitest/request/items.go index 73d868f8e..6b4b2a116 100644 --- a/common/pkg/testutils/capitest/request/items.go +++ b/common/pkg/testutils/capitest/request/items.go @@ -69,7 +69,11 @@ func NewKubeadmConfigTemplateRequest( Spec: bootstrapv1.KubeadmConfigSpec{ PostKubeadmCommands: []string{"initial-post-kubeadm"}, JoinConfiguration: &bootstrapv1.JoinConfiguration{ - NodeRegistration: bootstrapv1.NodeRegistrationOptions{}, + NodeRegistration: bootstrapv1.NodeRegistrationOptions{ + KubeletExtraArgs: map[string]string{ + "cloud-provider": "external", + }, + }, }, }, }, @@ -112,10 +116,18 @@ func (b *KubeadmControlPlaneTemplateRequestItemBuilder) NewRequest( Spec: controlplanev1.KubeadmControlPlaneTemplateResourceSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ InitConfiguration: &bootstrapv1.InitConfiguration{ - NodeRegistration: bootstrapv1.NodeRegistrationOptions{}, + NodeRegistration: bootstrapv1.NodeRegistrationOptions{ + KubeletExtraArgs: map[string]string{ + "cloud-provider": "external", + }, + }, }, JoinConfiguration: &bootstrapv1.JoinConfiguration{ - NodeRegistration: bootstrapv1.NodeRegistrationOptions{}, + NodeRegistration: bootstrapv1.NodeRegistrationOptions{ + KubeletExtraArgs: map[string]string{ + "cloud-provider": "external", + }, + }, }, Files: b.files, }, diff --git a/pkg/handlers/generic/mutation/imageregistries/credentials/inject_test.go b/pkg/handlers/generic/mutation/imageregistries/credentials/inject_test.go index 290fb901e..983447304 100644 --- a/pkg/handlers/generic/mutation/imageregistries/credentials/inject_test.go +++ b/pkg/handlers/generic/mutation/imageregistries/credentials/inject_test.go @@ -234,19 +234,27 @@ var _ = Describe("Generate Image registry patches", func() { "/bin/bash /etc/caren/install-kubelet-credential-providers.sh", ), }, + { + Operation: "add", + Path: "/spec/template/spec/kubeadmConfigSpec/initConfiguration/nodeRegistration/kubeletExtraArgs/image-credential-provider-bin-dir", //nolint:lll // Just a long line. + ValueMatcher: gomega.Equal("/etc/kubernetes/image-credential-provider/"), + }, { Operation: "add", - Path: "/spec/template/spec/kubeadmConfigSpec/initConfiguration/nodeRegistration/kubeletExtraArgs", - ValueMatcher: gomega.HaveKeyWithValue( - "image-credential-provider-bin-dir", - "/etc/kubernetes/image-credential-provider/", + Path: "/spec/template/spec/kubeadmConfigSpec/initConfiguration/nodeRegistration/kubeletExtraArgs/image-credential-provider-config", //nolint:lll // Just a long line. + ValueMatcher: gomega.Equal( + "/etc/kubernetes/image-credential-provider-config.yaml", ), }, + { + Operation: "add", + Path: "/spec/template/spec/kubeadmConfigSpec/joinConfiguration/nodeRegistration/kubeletExtraArgs/image-credential-provider-bin-dir", //nolint:lll // Just a long line. + ValueMatcher: gomega.Equal("/etc/kubernetes/image-credential-provider/"), + }, { Operation: "add", - Path: "/spec/template/spec/kubeadmConfigSpec/joinConfiguration/nodeRegistration/kubeletExtraArgs", - ValueMatcher: gomega.HaveKeyWithValue( - "image-credential-provider-config", + Path: "/spec/template/spec/kubeadmConfigSpec/joinConfiguration/nodeRegistration/kubeletExtraArgs/image-credential-provider-config", //nolint:lll // Just a long line. + ValueMatcher: gomega.Equal( "/etc/kubernetes/image-credential-provider-config.yaml", ), }, @@ -297,19 +305,27 @@ var _ = Describe("Generate Image registry patches", func() { "/bin/bash /etc/caren/install-kubelet-credential-providers.sh", ), }, + { + Operation: "add", + Path: "/spec/template/spec/kubeadmConfigSpec/initConfiguration/nodeRegistration/kubeletExtraArgs/image-credential-provider-bin-dir", //nolint:lll // Just a long line. + ValueMatcher: gomega.Equal("/etc/kubernetes/image-credential-provider/"), + }, { Operation: "add", - Path: "/spec/template/spec/kubeadmConfigSpec/initConfiguration/nodeRegistration/kubeletExtraArgs", - ValueMatcher: gomega.HaveKeyWithValue( - "image-credential-provider-bin-dir", - "/etc/kubernetes/image-credential-provider/", + Path: "/spec/template/spec/kubeadmConfigSpec/initConfiguration/nodeRegistration/kubeletExtraArgs/image-credential-provider-config", //nolint:lll // Just a long line. + ValueMatcher: gomega.Equal( + "/etc/kubernetes/image-credential-provider-config.yaml", ), }, + { + Operation: "add", + Path: "/spec/template/spec/kubeadmConfigSpec/joinConfiguration/nodeRegistration/kubeletExtraArgs/image-credential-provider-bin-dir", //nolint:lll // Just a long line. + ValueMatcher: gomega.Equal("/etc/kubernetes/image-credential-provider/"), + }, { Operation: "add", - Path: "/spec/template/spec/kubeadmConfigSpec/joinConfiguration/nodeRegistration/kubeletExtraArgs", - ValueMatcher: gomega.HaveKeyWithValue( - "image-credential-provider-config", + Path: "/spec/template/spec/kubeadmConfigSpec/joinConfiguration/nodeRegistration/kubeletExtraArgs/image-credential-provider-config", //nolint:lll // Just a long line. + ValueMatcher: gomega.Equal( "/etc/kubernetes/image-credential-provider-config.yaml", ), }, @@ -362,12 +378,9 @@ var _ = Describe("Generate Image registry patches", func() { ), }, { - Operation: "add", - Path: "/spec/template/spec/joinConfiguration/nodeRegistration/kubeletExtraArgs", - ValueMatcher: gomega.HaveKeyWithValue( - "image-credential-provider-bin-dir", - "/etc/kubernetes/image-credential-provider/", - ), + Operation: "add", + Path: "/spec/template/spec/joinConfiguration/nodeRegistration/kubeletExtraArgs/image-credential-provider-bin-dir", //nolint:lll // Just a long line. + ValueMatcher: gomega.Equal("/etc/kubernetes/image-credential-provider/"), }, }, }, @@ -424,12 +437,16 @@ var _ = Describe("Generate Image registry patches", func() { "/bin/bash /etc/caren/install-kubelet-credential-providers.sh", ), }, + { + Operation: "add", + Path: "/spec/template/spec/joinConfiguration/nodeRegistration/kubeletExtraArgs/image-credential-provider-bin-dir", //nolint:lll // Just a long line. + ValueMatcher: gomega.Equal("/etc/kubernetes/image-credential-provider/"), + }, { Operation: "add", - Path: "/spec/template/spec/joinConfiguration/nodeRegistration/kubeletExtraArgs", - ValueMatcher: gomega.HaveKeyWithValue( - "image-credential-provider-bin-dir", - "/etc/kubernetes/image-credential-provider/", + Path: "/spec/template/spec/joinConfiguration/nodeRegistration/kubeletExtraArgs/image-credential-provider-config", //nolint:lll // Just a long line. + ValueMatcher: gomega.Equal( + "/etc/kubernetes/image-credential-provider-config.yaml", ), }, }, diff --git a/pkg/handlers/generic/mutation/taints/inject_controlplane.go b/pkg/handlers/generic/mutation/taints/inject_controlplane.go index aa5372dc1..79ce0be8e 100644 --- a/pkg/handlers/generic/mutation/taints/inject_controlplane.go +++ b/pkg/handlers/generic/mutation/taints/inject_controlplane.go @@ -85,10 +85,10 @@ func (h *taintsControlPlanePatchHandler) Mutate( "patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(), "patchedObjectName", ctrlclient.ObjectKeyFromObject(obj), ).Info("adding taints to worker node kubeadm config template") - if obj.Spec.Template.Spec.KubeadmConfigSpec.InitConfiguration != nil { + if obj.Spec.Template.Spec.KubeadmConfigSpec.InitConfiguration == nil { obj.Spec.Template.Spec.KubeadmConfigSpec.InitConfiguration = &bootstrapv1.InitConfiguration{} } - if obj.Spec.Template.Spec.KubeadmConfigSpec.JoinConfiguration != nil { + if obj.Spec.Template.Spec.KubeadmConfigSpec.JoinConfiguration == nil { obj.Spec.Template.Spec.KubeadmConfigSpec.JoinConfiguration = &bootstrapv1.JoinConfiguration{} } obj.Spec.Template.Spec.KubeadmConfigSpec.InitConfiguration.NodeRegistration.Taints = toCoreTaints( diff --git a/pkg/handlers/generic/mutation/taints/inject_worker.go b/pkg/handlers/generic/mutation/taints/inject_worker.go index 90351153a..263f72bf0 100644 --- a/pkg/handlers/generic/mutation/taints/inject_worker.go +++ b/pkg/handlers/generic/mutation/taints/inject_worker.go @@ -87,7 +87,7 @@ func (h *taintsWorkerPatchHandler) Mutate( "patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(), "patchedObjectName", ctrlclient.ObjectKeyFromObject(obj), ).Info("adding taints to worker node kubeadm config template") - if obj.Spec.Template.Spec.JoinConfiguration != nil { + if obj.Spec.Template.Spec.JoinConfiguration == nil { obj.Spec.Template.Spec.JoinConfiguration = &bootstrapv1.JoinConfiguration{} } obj.Spec.Template.Spec.JoinConfiguration.NodeRegistration.Taints = toCoreTaints(