From b90b280c5ad41481f701c13a537a8c7319839e6f Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Fri, 19 Jul 2024 16:38:44 +0300 Subject: [PATCH] kubeadm: fix join bug where kubeletconfig was not patched in memory During kubeadm join in 1.30 kubeadm started respecting the kubeletconfiguration healthz address/port. Previously it hardcoded the health check to localhost:defaultport. A corner case was not handled where the user applies --patches on join to modify the local kubeletconfiguration. This results in kubeletconfiguration patch target patches not being applied to the KubeletConfiguration in memory and the health check running on the address:port which are present in the kubelet-config configmap. Fix that by explicitly calling a new function to patch the KubeletConfiguration in memory. This is scoped to only handle the healthz checks *after* the kubelet config.yaml was already patched and written to disk. --- cmd/kubeadm/app/cmd/phases/join/kubelet.go | 8 +++ cmd/kubeadm/app/phases/kubelet/config.go | 41 ++++++++++++++- cmd/kubeadm/app/phases/kubelet/config_test.go | 51 +++++++++++++++++++ cmd/kubeadm/app/util/apiclient/wait.go | 7 ++- 4 files changed, 105 insertions(+), 2 deletions(-) diff --git a/cmd/kubeadm/app/cmd/phases/join/kubelet.go b/cmd/kubeadm/app/cmd/phases/join/kubelet.go index 7c4d27c519b..25df0c1bc88 100644 --- a/cmd/kubeadm/app/cmd/phases/join/kubelet.go +++ b/cmd/kubeadm/app/cmd/phases/join/kubelet.go @@ -281,6 +281,14 @@ func runKubeletWaitBootstrapPhase(c workflow.RunData) (returnErr error) { _ = os.Remove(bootstrapKubeConfigFile) }() + // Apply patches to the in-memory kubelet configuration so that any configuration changes like kubelet healthz + // address and port options are respected during the wait below. WriteConfigToDisk already applied patches to + // the kubelet.yaml written to disk. This should be done after WriteConfigToDisk because both use the same config + // in memory and we don't want patches to be applied two times to the config that is written to disk. + if err := kubeletphase.ApplyPatchesToConfig(&initCfg.ClusterConfiguration, data.PatchesDir()); err != nil { + return errors.Wrap(err, "could not apply patches to the in-memory kubelet configuration") + } + // Now the kubelet will perform the TLS Bootstrap, transforming /etc/kubernetes/bootstrap-kubelet.conf to /etc/kubernetes/kubelet.conf // Wait for the kubelet to create the /etc/kubernetes/kubelet.conf kubeconfig file. If this process // times out, display a somewhat user-friendly message. diff --git a/cmd/kubeadm/app/phases/kubelet/config.go b/cmd/kubeadm/app/phases/kubelet/config.go index 3236edb58b6..43406ca7c50 100644 --- a/cmd/kubeadm/app/phases/kubelet/config.go +++ b/cmd/kubeadm/app/phases/kubelet/config.go @@ -34,10 +34,13 @@ import ( kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" "k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs" kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" + kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" "k8s.io/kubernetes/cmd/kubeadm/app/util/apiclient" "k8s.io/kubernetes/cmd/kubeadm/app/util/patches" ) +var applyKubeletConfigPatchesFunc = applyKubeletConfigPatches + // WriteConfigToDisk writes the kubelet config object down to a file // Used at "kubeadm init" and "kubeadm upgrade" time func WriteConfigToDisk(cfg *kubeadmapi.ClusterConfiguration, kubeletDir, patchesDir string, output io.Writer) error { @@ -57,7 +60,7 @@ func WriteConfigToDisk(cfg *kubeadmapi.ClusterConfiguration, kubeletDir, patches // Apply patches to the KubeletConfiguration if len(patchesDir) != 0 { - kubeletBytes, err = applyKubeletConfigPatches(kubeletBytes, patchesDir, output) + kubeletBytes, err = applyKubeletConfigPatchesFunc(kubeletBytes, patchesDir, output) if err != nil { return errors.Wrap(err, "could not apply patches to the KubeletConfiguration") } @@ -66,6 +69,42 @@ func WriteConfigToDisk(cfg *kubeadmapi.ClusterConfiguration, kubeletDir, patches return writeConfigBytesToDisk(kubeletBytes, kubeletDir) } +// ApplyPatchesToConfig applies the patches located in patchesDir to the KubeletConfiguration stored +// in the ClusterConfiguration.ComponentConfigs map. +func ApplyPatchesToConfig(cfg *kubeadmapi.ClusterConfiguration, patchesDir string) error { + kubeletCfg, ok := cfg.ComponentConfigs[componentconfigs.KubeletGroup] + if !ok { + return errors.New("no kubelet component config found") + } + + if err := kubeletCfg.Mutate(); err != nil { + return err + } + + kubeletBytes, err := kubeletCfg.Marshal() + if err != nil { + return err + } + + // Apply patches to the KubeletConfiguration. Output is discarded. + if len(patchesDir) != 0 { + kubeletBytes, err = applyKubeletConfigPatchesFunc(kubeletBytes, patchesDir, io.Discard) + if err != nil { + return errors.Wrap(err, "could not apply patches to the KubeletConfiguration") + } + } + + gvkmap, err := kubeadmutil.SplitYAMLDocuments(kubeletBytes) + if err != nil { + return err + } + if err := kubeletCfg.Unmarshal(gvkmap); err != nil { + return err + } + + return nil +} + // CreateConfigMap creates a ConfigMap with the generic kubelet configuration. // Used at "kubeadm init" and "kubeadm upgrade" time func CreateConfigMap(cfg *kubeadmapi.ClusterConfiguration, client clientset.Interface) error { diff --git a/cmd/kubeadm/app/phases/kubelet/config_test.go b/cmd/kubeadm/app/phases/kubelet/config_test.go index f1add0f8312..c95499e090d 100644 --- a/cmd/kubeadm/app/phases/kubelet/config_test.go +++ b/cmd/kubeadm/app/phases/kubelet/config_test.go @@ -18,6 +18,7 @@ package kubelet import ( "bytes" + "fmt" "io" "os" "path/filepath" @@ -28,7 +29,11 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" + kubeletconfig "k8s.io/kubelet/config/v1beta1" + "k8s.io/utils/ptr" + kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" + "k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs" configutil "k8s.io/kubernetes/cmd/kubeadm/app/util/config" ) @@ -103,3 +108,49 @@ func TestApplyKubeletConfigPatches(t *testing.T) { t.Fatalf("expected output:\n%s\ngot\n%s\n", expectedOutput, output) } } + +func TestApplyPatchesToConfig(t *testing.T) { + const ( + expectedAddress = "barfoo" + expectedPort = 4321 + ) + + kc := &kubeletconfig.KubeletConfiguration{ + HealthzBindAddress: "foobar", + HealthzPort: ptr.To[int32](1234), + } + + cfg := &kubeadmapi.ClusterConfiguration{} + cfg.ComponentConfigs = kubeadmapi.ComponentConfigMap{} + + localAPIEndpoint := &kubeadmapi.APIEndpoint{} + nodeRegOps := &kubeadmapi.NodeRegistrationOptions{} + componentconfigs.Default(cfg, localAPIEndpoint, nodeRegOps) + cfg.ComponentConfigs[componentconfigs.KubeletGroup].Set(kc) + + // Change to a fake function that does patching with string replace. + applyKubeletConfigPatchesFunc = func(b []byte, _ string, _ io.Writer) ([]byte, error) { + b = bytes.ReplaceAll(b, []byte("foobar"), []byte(expectedAddress)) + b = bytes.ReplaceAll(b, []byte("1234"), []byte(fmt.Sprintf("%d", expectedPort))) + return b, nil + } + defer func() { + applyKubeletConfigPatchesFunc = applyKubeletConfigPatches + }() + + if err := ApplyPatchesToConfig(cfg, "fakedir"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + new := cfg.ComponentConfigs[componentconfigs.KubeletGroup].Get() + newTyped, ok := new.(*kubeletconfig.KubeletConfiguration) + if !ok { + t.Fatal("could not cast kubelet config") + } + if newTyped.HealthzBindAddress != expectedAddress { + t.Fatalf("expected address: %s, got: %s", expectedAddress, newTyped.HealthzBindAddress) + } + if *newTyped.HealthzPort != expectedPort { + t.Fatalf("expected port: %d, got: %d", expectedPort, *newTyped.HealthzPort) + } +} diff --git a/cmd/kubeadm/app/util/apiclient/wait.go b/cmd/kubeadm/app/util/apiclient/wait.go index 50191503909..d101d534d8e 100644 --- a/cmd/kubeadm/app/util/apiclient/wait.go +++ b/cmd/kubeadm/app/util/apiclient/wait.go @@ -250,7 +250,12 @@ func (w *KubeWaiter) WaitForKubelet(healthzAddress string, healthzPort int32) er healthzEndpoint = fmt.Sprintf("http://%s:%d/healthz", healthzAddress, healthzPort) ) - fmt.Printf("[kubelet-check] Waiting for a healthy kubelet. This can take up to %v\n", w.timeout) + if healthzPort == 0 { + fmt.Println("[kubelet-check] Skipping the kubelet health check because the healthz port is set to 0") + return nil + } + fmt.Printf("[kubelet-check] Waiting for a healthy kubelet at %s. This can take up to %v\n", + healthzEndpoint, w.timeout) formatError := func(cause string) error { return errors.Errorf("The HTTP call equal to 'curl -sSL %s' returned %s\n",