Merge pull request #21642 from medyagh/invoke_kubeadm

Refactor Invoking Kubeadm command to prepare for debian 12

Author: medyagh

pkg/minikube/bootstrapper/bsutil/binaries.go

@@ -28,11 +28,11 @@ import (
 	"golang.org/x/sync/errgroup"
 
 	"k8s.io/klog/v2"
+	"k8s.io/minikube/pkg/minikube/assets"
 	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/constants"
 	"k8s.io/minikube/pkg/minikube/download"
-	"k8s.io/minikube/pkg/minikube/machine"
 	"k8s.io/minikube/pkg/minikube/sysinit"
 	"k8s.io/minikube/pkg/minikube/vmpath"
 )
@@ -68,7 +68,7 @@ func TransferBinaries(cfg config.KubernetesConfig, c command.Runner, sm sysinit.
 			}
 
 			dst := path.Join(dir, name)
-			if err := machine.CopyBinary(c, src, dst); err != nil {
+			if err := copyBinary(c, src, dst); err != nil {
 				return errors.Wrapf(err, "copybinary %s -> %s", src, dst)
 			}
 			return nil
@@ -101,3 +101,21 @@ func binariesExist(cfg config.KubernetesConfig, c command.Runner) (bool, error)
 func binRoot(version string) string {
 	return path.Join(vmpath.GuestPersistentDir, "binaries", version)
 }
+
+// copyBinary copies a locally cached binary to the guest VM
+func copyBinary(cr command.Runner, src, dest string) error {
+	f, err := assets.NewFileAsset(src, path.Dir(dest), path.Base(dest), "0755")
+	if err != nil {
+		return errors.Wrap(err, "new file asset")
+	}
+	defer func() {
+		if err := f.Close(); err != nil {
+			klog.Warningf("error closing the file %s: %v", f.GetSourcePath(), err)
+		}
+	}()
+
+	if err := cr.Copy(f); err != nil {
+		return errors.Wrapf(err, "copy")
+	}
+	return nil
+}

pkg/minikube/bootstrapper/bsutil/binaries_copy_test.go

@@ -0,0 +1,87 @@
+/*
+Copyright 2016 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package bsutil
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"k8s.io/minikube/pkg/minikube/assets"
+	"k8s.io/minikube/pkg/minikube/command"
+)
+
+type copyFailRunner struct {
+	command.Runner
+}
+
+func (copyFailRunner) Copy(_ assets.CopyableFile) error {
+	return fmt.Errorf("test error during copy file")
+}
+
+func newFakeCommandRunnerCopyFail() command.Runner {
+	return copyFailRunner{command.NewFakeCommandRunner()}
+}
+
+func TestCopyBinary(t *testing.T) {
+	tmpDir := t.TempDir()
+	srcFile := filepath.Join(tmpDir, "source")
+	if err := os.WriteFile(srcFile, []byte("data"), 0o644); err != nil {
+		t.Fatalf("failed to create source file: %v", err)
+	}
+
+	tests := []struct {
+		name    string
+		runner  command.Runner
+		src     string
+		dst     string
+		wantErr bool
+	}{
+		{
+			name:    "missing source",
+			runner:  command.NewFakeCommandRunner(),
+			src:     filepath.Join(tmpDir, "missing"),
+			dst:     filepath.Join(tmpDir, "dest"),
+			wantErr: true,
+		},
+		{
+			name:    "success",
+			runner:  command.NewFakeCommandRunner(),
+			src:     srcFile,
+			dst:     filepath.Join(tmpDir, "dest"),
+			wantErr: false,
+		},
+		{
+			name:    "copy failure",
+			runner:  newFakeCommandRunnerCopyFail(),
+			src:     srcFile,
+			dst:     filepath.Join(tmpDir, "dest"),
+			wantErr: true,
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			err := copyBinary(tc.runner, tc.src, tc.dst)
+			if (err != nil) != tc.wantErr {
+				t.Fatalf("copyBinary() error = %v, wantErr %v", err, tc.wantErr)
+			}
+		})
+	}
+}

pkg/minikube/bootstrapper/bsutil/kubeadm.go

@@ -220,9 +220,12 @@ var KubeadmExtraConfigOpts = []string{
 	Kubeproxy,
 }
 
-// InvokeKubeadm returns the invocation command for Kubeadm
-func InvokeKubeadm(version string) string {
-	return fmt.Sprintf("sudo env PATH=\"%s:$PATH\" kubeadm", binRoot(version))
+// KubeadmCmdWithPath returns the invocation command for Kubeadm
+// NOTE: The command must run with the a root shell to expand PATH to the
+// root PATH. On Debian 12 user PATH does not contain /usr/sbin which breaks
+// kubeadm since https://github.com/kubernetes/kubernetes/pull/129450.
+func KubeadmCmdWithPath(version string) string {
+	return fmt.Sprintf("env PATH=\"%s:$PATH\" kubeadm", binRoot(version))
 }
 
 // EtcdDataDir is where etcd data is stored.

pkg/minikube/bootstrapper/certs.go

@@ -43,6 +43,7 @@ import (
 
 	"k8s.io/minikube/pkg/drivers/kic/oci"
 	"k8s.io/minikube/pkg/minikube/assets"
+	"k8s.io/minikube/pkg/minikube/bootstrapper/bsutil"
 	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/constants"
@@ -418,9 +419,8 @@ func renewExpiredKubeadmCerts(cmd command.Runner, cc config.ClusterConfig) error
 		return nil
 	}
 	out.WarningT("kubeadm certificates have expired. Generating new ones...")
-	kubeadmPath := path.Join(vmpath.GuestPersistentDir, "binaries", cc.KubernetesConfig.KubernetesVersion)
-	bashCmd := fmt.Sprintf("sudo env PATH=\"%s:$PATH\" kubeadm certs renew all --config %s", kubeadmPath, constants.KubeadmYamlPath)
-	if _, err := cmd.RunCmd(exec.Command("/bin/bash", "-c", bashCmd)); err != nil {
+	bashCmd := fmt.Sprintf("%s certs renew all --config %s", bsutil.KubeadmCmdWithPath(cc.KubernetesConfig.KubernetesVersion), constants.KubeadmYamlPath)
+	if _, err := cmd.RunCmd(exec.Command("sudo", "/bin/bash", "-c", bashCmd)); err != nil {
 		return errors.Wrap(err, "kubeadm certs renew")
 	}
 	return nil

pkg/minikube/bootstrapper/kubeadm/kubeadm.go

@@ -228,8 +228,16 @@ func (k *Bootstrapper) init(cfg config.ClusterConfig) error {
 	ctx, cancel := context.WithTimeout(context.Background(), initTimeoutMinutes*time.Minute)
 	defer cancel()
 	kr, kw := io.Pipe()
-	c := exec.CommandContext(ctx, "/bin/bash", "-c", fmt.Sprintf("%s init --config %s %s --ignore-preflight-errors=%s",
-		bsutil.InvokeKubeadm(cfg.KubernetesConfig.KubernetesVersion), conf, extraFlags, strings.Join(ignore, ",")))
+
+	cmd := fmt.Sprintf(
+		"%s init --config %s %s --ignore-preflight-errors=%s",
+		bsutil.KubeadmCmdWithPath(cfg.KubernetesConfig.KubernetesVersion),
+		conf,
+		extraFlags,
+		strings.Join(ignore, ","),
+	)
+	c := exec.CommandContext(ctx, "sudo", "/bin/bash", "-c", cmd)
+
 	c.Stdout = kw
 	c.Stderr = kw
 	var wg sync.WaitGroup
@@ -650,7 +658,7 @@ func (k *Bootstrapper) restartPrimaryControlPlane(cfg config.ClusterConfig) erro
 		return errors.Wrap(err, "cp")
 	}
 
-	baseCmd := fmt.Sprintf("%s init", bsutil.InvokeKubeadm(cfg.KubernetesConfig.KubernetesVersion))
+	baseCmd := fmt.Sprintf("%s init", bsutil.KubeadmCmdWithPath(cfg.KubernetesConfig.KubernetesVersion))
 	cmds := []string{
 		fmt.Sprintf("%s phase certs all --config %s", baseCmd, conf),
 		fmt.Sprintf("%s phase kubeconfig all --config %s", baseCmd, conf),
@@ -661,10 +669,10 @@ func (k *Bootstrapper) restartPrimaryControlPlane(cfg config.ClusterConfig) erro
 
 	// Run commands one at a time so that it is easier to root cause failures.
 	for _, c := range cmds {
-		if _, err := k.c.RunCmd(exec.Command("/bin/bash", "-c", c)); err != nil {
+		if _, err := k.c.RunCmd(exec.Command("sudo", "/bin/bash", "-c", c)); err != nil {
 			klog.Errorf("%s failed - will try once more: %v", c, err)
 
-			if _, err := k.c.RunCmd(exec.Command("/bin/bash", "-c", c)); err != nil {
+			if _, err := k.c.RunCmd(exec.Command("sudo", "/bin/bash", "-c", c)); err != nil {
 				return errors.Wrap(err, "run")
 			}
 		}
@@ -703,7 +711,7 @@ func (k *Bootstrapper) restartPrimaryControlPlane(cfg config.ClusterConfig) erro
 		if cfg.KubernetesConfig.ExtraOptions.Exists("kubeadm.skip-phases=addon/kube-proxy") {
 			addons = "coredns"
 		}
-		_, err := k.c.RunCmd(exec.Command("/bin/bash", "-c", fmt.Sprintf("%s phase addon %s --config %s", baseCmd, addons, conf)))
+		_, err := k.c.RunCmd(exec.Command("sudo", "/bin/bash", "-c", fmt.Sprintf("%s phase addon %s --config %s", baseCmd, addons, conf)))
 		return err
 	}
 	if err = retry.Expo(addonPhase, 100*time.Microsecond, 30*time.Second); err != nil {
@@ -759,11 +767,11 @@ func (k *Bootstrapper) JoinCluster(cc config.ClusterConfig, n config.Node, joinC
 			" --apiserver-bind-port=" + strconv.Itoa(n.Port)
 	}
 
-	if _, err := k.c.RunCmd(exec.Command("/bin/bash", "-c", joinCmd)); err != nil {
+	if _, err := k.c.RunCmd(exec.Command("sudo", "/bin/bash", "-c", joinCmd)); err != nil {
 		return errors.Wrapf(err, "kubeadm join")
 	}
 
-	if _, err := k.c.RunCmd(exec.Command("/bin/bash", "-c", "sudo systemctl daemon-reload && sudo systemctl enable kubelet && sudo systemctl start kubelet")); err != nil {
+	if _, err := k.c.RunCmd(exec.Command("sudo", "/bin/bash", "-c", "systemctl daemon-reload && systemctl enable kubelet && systemctl start kubelet")); err != nil {
 		return errors.Wrap(err, "starting kubelet")
 	}
 
@@ -773,14 +781,14 @@ func (k *Bootstrapper) JoinCluster(cc config.ClusterConfig, n config.Node, joinC
 // GenerateToken creates a token and returns the appropriate kubeadm join command to run, or the already existing token
 func (k *Bootstrapper) GenerateToken(cc config.ClusterConfig) (string, error) {
 	// Take that generated token and use it to get a kubeadm join command
-	tokenCmd := exec.Command("/bin/bash", "-c", fmt.Sprintf("%s token create --print-join-command --ttl=0", bsutil.InvokeKubeadm(cc.KubernetesConfig.KubernetesVersion)))
+	tokenCmd := exec.Command("sudo", "/bin/bash", "-c", fmt.Sprintf("%s token create --print-join-command --ttl=0", bsutil.KubeadmCmdWithPath(cc.KubernetesConfig.KubernetesVersion)))
 	r, err := k.c.RunCmd(tokenCmd)
 	if err != nil {
 		return "", errors.Wrap(err, "generating join command")
 	}
 
 	joinCmd := r.Stdout.String()
-	joinCmd = strings.Replace(joinCmd, "kubeadm", bsutil.InvokeKubeadm(cc.KubernetesConfig.KubernetesVersion), 1)
+	joinCmd = strings.Replace(joinCmd, "kubeadm", bsutil.KubeadmCmdWithPath(cc.KubernetesConfig.KubernetesVersion), 1)
 	joinCmd = fmt.Sprintf("%s --ignore-preflight-errors=all", strings.TrimSpace(joinCmd))
 
 	// avoid "Found multiple CRI endpoints on the host. Please define which one do you wish to use by setting the 'criSocket' field in the kubeadm configuration file: unix:///var/run/containerd/containerd.sock, unix:///var/run/cri-dockerd.sock" error
@@ -849,14 +857,14 @@ func (k *Bootstrapper) DeleteCluster(k8s config.KubernetesConfig) error {
 		return errors.Wrap(err, "runtime")
 	}
 
-	ka := bsutil.InvokeKubeadm(k8s.KubernetesVersion)
+	ka := bsutil.KubeadmCmdWithPath(k8s.KubernetesVersion)
 	sp := cr.SocketPath()
 	cmd := fmt.Sprintf("%s reset --cri-socket %s --force", ka, sp)
 	if ver.LT(semver.MustParse("1.11.0")) {
 		cmd = fmt.Sprintf("%s reset --cri-socket %s", ka, sp)
 	}
 
-	rr, derr := k.c.RunCmd(exec.Command("/bin/bash", "-c", cmd))
+	rr, derr := k.c.RunCmd(exec.Command("sudo", "/bin/bash", "-c", cmd))
 	if derr != nil {
 		klog.Warningf("%s: %v", rr.Command(), err)
 	}

pkg/minikube/machine/cache_binaries.go

@@ -17,15 +17,11 @@ limitations under the License.
 package machine
 
 import (
-	"path"
 	"runtime"
 
 	"github.com/pkg/errors"
 	"golang.org/x/sync/errgroup"
-	"k8s.io/klog/v2"
-	"k8s.io/minikube/pkg/minikube/assets"
 	"k8s.io/minikube/pkg/minikube/bootstrapper"
-	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/download"
 )
 
@@ -61,21 +57,3 @@ func CacheBinariesForBootstrapper(version string, excludeBinaries []string, bina
 	}
 	return g.Wait()
 }
-
-// CopyBinary copies a locally cached binary to the guest VM
-func CopyBinary(cr command.Runner, src string, dest string) error {
-	f, err := assets.NewFileAsset(src, path.Dir(dest), path.Base(dest), "0755")
-	if err != nil {
-		return errors.Wrap(err, "new file asset")
-	}
-	defer func() {
-		if err := f.Close(); err != nil {
-			klog.Warningf("error closing the file %s: %v", f.GetSourcePath(), err)
-		}
-	}()
-
-	if err := cr.Copy(f); err != nil {
-		return errors.Wrapf(err, "copy")
-	}
-	return nil
-}

pkg/minikube/machine/cache_binaries_test.go

@@ -17,70 +17,13 @@ limitations under the License.
 package machine
 
 import (
-	"fmt"
 	"strings"
 	"testing"
 
-	"k8s.io/minikube/pkg/minikube/assets"
 	"k8s.io/minikube/pkg/minikube/bootstrapper"
-	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/download"
 )
 
-type copyFailRunner struct {
-	command.Runner
-}
-
-func (copyFailRunner) Copy(_ assets.CopyableFile) error {
-	return fmt.Errorf("test error during copy file")
-}
-
-func newFakeCommandRunnerCopyFail() command.Runner {
-	return copyFailRunner{command.NewFakeCommandRunner()}
-}
-
-func TestCopyBinary(t *testing.T) {
-	var tc = []struct {
-		lastUpdateCheckFilePath string
-		src, dst, desc          string
-		err                     bool
-		runner                  command.Runner
-	}{
-		{
-			desc:   "not existing src",
-			dst:    "/tmp/testCopyBinary1",
-			src:    "/tmp/testCopyBinary2",
-			err:    true,
-			runner: command.NewFakeCommandRunner(),
-		},
-		{
-			desc:   "src /etc/hosts",
-			dst:    "/tmp/testCopyBinary1",
-			src:    "/etc/hosts",
-			err:    false,
-			runner: command.NewFakeCommandRunner(),
-		},
-		{
-			desc:   "existing src, copy fail",
-			dst:    "/etc/passwd",
-			src:    "/etc/hosts",
-			err:    true,
-			runner: newFakeCommandRunnerCopyFail(),
-		},
-	}
-	for _, test := range tc {
-		t.Run(test.desc, func(t *testing.T) {
-			err := CopyBinary(test.runner, test.src, test.dst)
-			if err != nil && !test.err {
-				t.Fatalf("Got unexpected error %v", err)
-			}
-			if err == nil && test.err {
-				t.Fatal("Expected error but got nil")
-			}
-		})
-	}
-}
-
 func TestCacheBinariesForBootstrapper(t *testing.T) {
 	download.DownloadMock = download.CreateDstDownloadMock
 

pkg/minikube/node/node.go

@@ -149,8 +149,8 @@ func teardown(cc config.ClusterConfig, name string) (*config.Node, error) {
 				sp = "unix://" + sp
 			}
 
-			cmd := exec.Command("/bin/bash", "-c", fmt.Sprintf("KUBECONFIG=/var/lib/minikube/kubeconfig %s reset --force --ignore-preflight-errors=all --cri-socket=%s",
-				bsutil.InvokeKubeadm(cc.KubernetesConfig.KubernetesVersion), sp))
+			cmd := exec.Command("sudo", "/bin/bash", "-c", fmt.Sprintf("KUBECONFIG=/var/lib/minikube/kubeconfig %s reset --force --ignore-preflight-errors=all --cri-socket=%s",
+				bsutil.KubeadmCmdWithPath(cc.KubernetesConfig.KubernetesVersion), sp))
 			if _, kerr = r.RunCmd(cmd); kerr == nil {
 				klog.Infof("successfully reset node %q", m)
 			}

pkg/minikube/node/start.go

@@ -346,7 +346,7 @@ func joinCluster(starter Starter, cpBs bootstrapper.Bootstrapper, bs bootstrappe
 
 			// reset node to revert any changes made by previous kubeadm init/join
 			klog.Infof("resetting %s node %q before attempting to rejoin cluster...", role, starter.Node.Name)
-			if _, err := starter.Runner.RunCmd(exec.Command("/bin/bash", "-c", fmt.Sprintf("%s reset --force", bsutil.InvokeKubeadm(starter.Cfg.KubernetesConfig.KubernetesVersion)))); err != nil {
+			if _, err := starter.Runner.RunCmd(exec.Command("sudo", "/bin/bash", "-c", fmt.Sprintf("%s reset --force", bsutil.KubeadmCmdWithPath(starter.Cfg.KubernetesConfig.KubernetesVersion)))); err != nil {
 				klog.Infof("kubeadm reset failed, continuing anyway: %v", err)
 			} else {
 				klog.Infof("successfully reset %s node %q", role, starter.Node.Name)