Merge pull request kubernetes#126224 from neolit123/1.31-fix-bug-in-join-patches-healthz

kubeadm: fix join bug where kubeletconfig was not patched in memory

Author: k8s-ci-robot

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.

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 {

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)
+	}
+}

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",