try to fix couple of more things

Author: prezha

pkg/minikube/cni/cni.go

@@ -197,8 +197,9 @@ func applyManifest(cc config.ClusterConfig, r Runner, f assets.CopyableFile) err
 // ConfigureLoopback ensures loopback has expected version ("1.0.0") and valid name ("loopback") in its config file in /etc/cni/net.d
 // cri-o is leaving name out atm (https://github.com/cri-o/cri-o/pull/6273)
 // avoid errors like:
-// Failed to create pod sandbox: rpc error: code = Unknown desc = [failed to set up sandbox container "..." network for pod "...": networkPlugin cni failed to set up pod "..." network: missing network name:,
-// failed to clean up sandbox container "..." network for pod "...": networkPlugin cni failed to teardown pod "..." network: missing network name]
+// - Failed to create pod sandbox: rpc error: code = Unknown desc = [failed to set up sandbox container "..." network for pod "...": networkPlugin cni failed to set up pod "..." network: missing network name:,
+// - failed to clean up sandbox container "..." network for pod "...": networkPlugin cni failed to teardown pod "..." network: missing network name]
+// It is caller's responsibility to restart container runtime for these changes to take effect.
 func ConfigureLoopback(r Runner) error {
 	loopback := "/etc/cni/net.d/*loopback.conf*" // usually: 200-loopback.conf
 	// turn { "cniVersion": "0.3.1", "type": "loopback" }
@@ -226,7 +227,6 @@ func ConfigureDefaultBridgeCNIs(r Runner, networkPlugin string) error {
 	if networkPlugin != "" {
 		return disableAllBridgeCNIs(r)
 	}
-
 	return configureAllBridgeCNIs(r, DefaultPodCIDR)
 }
 
@@ -241,7 +241,7 @@ func disableAllBridgeCNIs(r Runner) error {
 	}
 	configs := strings.Trim(out.Stdout.String(), ", ")
 	if len(configs) == 0 {
-		klog.Infof("no bridge cni configs found in %q - nothing to disable", configs, path)
+		klog.Infof("no bridge cni configs found in %q - nothing to disable", path)
 		return nil
 	}
 	klog.Infof("disabled [%s] bridge cni config(s)", configs)
@@ -289,7 +289,7 @@ func configureAllBridgeCNIs(r Runner, cidr string) error {
 	}
 
 	if len(strings.Trim(configs, ", ")) == 0 {
-		klog.Infof("no bridge cni configs found in %q - nothing to configure", configs, path)
+		klog.Infof("no bridge cni configs found in %q - nothing to configure", path)
 		return nil
 	}
 	klog.Infof("configured [%s] bridge cni config(s)", configs)

pkg/minikube/cruntime/containerd.go

@@ -229,6 +229,7 @@ func (r *Containerd) Enable(disOthers bool, cgroupDriver string, inUserNamespace
 	currentVersion, err := r.Version()
 	if err == nil && semver.MustParse(targetVersion).GT(semver.MustParse(currentVersion)) {
 		klog.Infof("replacing original containerd with v%s-%s-%s", targetVersion, runtime.GOOS, runtime.GOARCH)
+		r.Init.ForceStop("containerd")
 		if err := updateContainerdBinary(r.Runner, targetVersion, runtime.GOOS, runtime.GOARCH); err != nil {
 			klog.Warningf("unable to replace original containerd with v%s-%s-%s: %v", targetVersion, runtime.GOOS, runtime.GOARCH, err)
 		}

pkg/minikube/cruntime/cri.go

@@ -232,7 +232,11 @@ func stopCRIContainers(cr CommandRunner, ids []string) error {
 	klog.Infof("Stopping containers: %s", ids)
 
 	crictl := getCrictlPath(cr)
-	args := append([]string{crictl, "stop"}, ids...)
+	// bring crictl stop timeout on pair with docker:
+	// - docker stop --help => -t, --time int   Seconds to wait for stop before killing it (default 10)
+	// - crictl stop --help => --timeout value, -t value  Seconds to wait to kill the container after a graceful stop is requested (default: 0)
+	// to prevent "stuck" containers blocking ports (eg, "[ERROR Port-2379|2380]: Port 2379|2380 is in use" for etcd during "live" k8s upgrade)
+	args := append([]string{crictl, "stop", "--timeout", "10"}, ids...)
 	c := exec.Command("sudo", args...)
 	if _, err := cr.RunCmd(c); err != nil {
 		return errors.Wrap(err, "crictl")

pkg/minikube/node/start.go

@@ -418,9 +418,21 @@ func configureRuntimes(runner cruntime.CommandRunner, cc config.ClusterConfig, k
 		exit.Error(reason.InternalRuntime, "Failed runtime", err)
 	}
 
-	disableOthers := true
-	if driver.BareMetal(cc.Driver) {
-		disableOthers = false
+	// ensure loopback is properly configured
+	// make sure container runtime is restarted afterwards for these changes to take effect
+	if err := cni.ConfigureLoopback(runner); err != nil {
+		klog.Warningf("unable to name loopback interface in dockerConfigureNetworkPlugin: %v", err)
+	}
+	// ensure all default CNI(s) are properly configured on each and every node (re)start
+	// make sure container runtime is restarted afterwards for these changes to take effect
+	if err := cni.ConfigureDefaultBridgeCNIs(runner, cc.KubernetesConfig.NetworkPlugin); err != nil {
+		klog.Errorf("unable to disable preinstalled bridge CNI(s): %v", err)
+	}
+
+	if kv.GTE(semver.MustParse("1.24.0-alpha.2")) {
+		if err := cruntime.ConfigureNetworkPlugin(cr, runner, cc.KubernetesConfig.NetworkPlugin); err != nil {
+			exit.Error(reason.RuntimeEnable, "Failed to configure network plugin", err)
+		}
 	}
 
 	// Preload is overly invasive for bare metal, and caching is not meaningful.
@@ -440,23 +452,6 @@ func configureRuntimes(runner cruntime.CommandRunner, cc config.ClusterConfig, k
 		}
 	}
 
-	// ensure loopback is properly configured
-	if err := cni.ConfigureLoopback(runner); err != nil {
-		klog.Warningf("unable to name loopback interface in dockerConfigureNetworkPlugin: %v", err)
-	}
-
-	// ensure all default CNI(s) are properly configured on each and every node (re)start
-	// make sure container runtime is restarted afterwards for these changes to take effect
-	if err := cni.ConfigureDefaultBridgeCNIs(runner, cc.KubernetesConfig.NetworkPlugin); err != nil {
-		klog.Errorf("unable to disable preinstalled bridge CNI(s): %v", err)
-	}
-
-	if kv.GTE(semver.MustParse("1.24.0-alpha.2")) {
-		if err := cruntime.ConfigureNetworkPlugin(cr, runner, cc.KubernetesConfig.NetworkPlugin); err != nil {
-			exit.Error(reason.RuntimeEnable, "Failed to configure network plugin", err)
-		}
-	}
-
 	inUserNamespace := strings.Contains(cc.KubernetesConfig.FeatureGates, "KubeletInUserNamespace=true")
 	// for docker container runtime: ensure containerd is properly configured by calling Enable(), as docker could be bound to containerd
 	// it will also "soft" start containerd, but it will not disable others; docker will disable containerd if not used in the next step
@@ -475,6 +470,11 @@ func configureRuntimes(runner cruntime.CommandRunner, cc config.ClusterConfig, k
 			klog.Warningf("cannot ensure containerd is configured properly and reloaded for docker - cluster might be unstable: %v", err)
 		}
 	}
+
+	disableOthers := true
+	if driver.BareMetal(cc.Driver) {
+		disableOthers = false
+	}
 	err = cr.Enable(disableOthers, cgroupDriver(cc), inUserNamespace)
 	if err != nil {
 		exit.Error(reason.RuntimeEnable, "Failed to enable container runtime", err)

test/integration/addons_test.go

@@ -38,6 +38,7 @@ import (
 	"github.com/blang/semver/v4"
 	retryablehttp "github.com/hashicorp/go-retryablehttp"
 	"k8s.io/minikube/pkg/kapi"
+	"k8s.io/minikube/pkg/minikube/constants"
 	"k8s.io/minikube/pkg/minikube/detect"
 	"k8s.io/minikube/pkg/util/retry"
 )
@@ -68,7 +69,14 @@ func TestAddons(t *testing.T) {
 		}
 
 		// MOCK_GOOGLE_TOKEN forces the gcp-auth webhook to use a mock token instead of trying to get a valid one from the credentials.
-		t.Setenv("MOCK_GOOGLE_TOKEN", "true")
+		os.Setenv("MOCK_GOOGLE_TOKEN", "true")
+
+		// for some reason, (Docker_Cloud_Shell) sets 'MINIKUBE_FORCE_SYSTEMD=true' while having cgroupfs set in docker (and probably os itself), which might make it unstable and occasionaly fail:
+		// - I1226 15:05:24.834294   11286 out.go:177]   - MINIKUBE_FORCE_SYSTEMD=true
+		// - I1226 15:05:25.070037   11286 info.go:266] docker info: {... CgroupDriver:cgroupfs ...}
+		// ref: https://storage.googleapis.com/minikube-builds/logs/15463/27154/Docker_Cloud_Shell.html
+		// so we override that here to let minikube auto-detect appropriate cgroup driver
+		os.Setenv(constants.MinikubeForceSystemdEnv, "")
 
 		args := append([]string{"start", "-p", profile, "--wait=true", "--memory=4000", "--alsologtostderr", "--addons=registry", "--addons=metrics-server", "--addons=volumesnapshots", "--addons=csi-hostpath-driver", "--addons=gcp-auth", "--addons=cloud-spanner"}, StartArgs()...)
 		if !NoneDriver() { // none driver does not support ingress

test/integration/json_output_test.go

@@ -120,7 +120,7 @@ func validateDistinctCurrentSteps(ctx context.Context, t *testing.T, ces []*clou
 // validateIncreasingCurrentSteps verifies that for a successful minikube start, 'current step' should be increasing
 func validateIncreasingCurrentSteps(ctx context.Context, t *testing.T, ces []*cloudEvent) {
 	step := -1
-	for _, ce := range ces {
+	for i, ce := range ces {
 		currentStep, exists := ce.data["currentstep"]
 		if !exists || ce.Type() != "io.k8s.sigs.minikube.step" {
 			continue
@@ -130,6 +130,17 @@ func validateIncreasingCurrentSteps(ctx context.Context, t *testing.T, ces []*cl
 			t.Fatalf("current step is not an integer: %v\n%v", currentStep, ce)
 		}
 		if cs <= step {
+			// check if steps are mixed because of goroutines complete in unusual order, but still ok
+			// eg, "Enabling Addons" (goroutine) might complete before or after "Verifying Kubernetes" finishes
+			if i > 0 {
+				prev := ces[i-1].data["name"]
+				cur := ce.data["name"]
+				if cur == "Verifying Kubernetes" && prev == "Enabling Addons" {
+					t.Logf("unusual order of steps, might be ok: %q event came before %q", prev, cur)
+					step = cs
+					continue
+				}
+			}
 			t.Fatalf("current step is not in increasing order: %v", ces)
 		}
 		step = cs

test/integration/preload_test.go

@@ -78,6 +78,6 @@ func TestPreload(t *testing.T) {
 		t.Fatalf("%s failed: %v", rr.Command(), err)
 	}
 	if !strings.Contains(rr.Output(), image) {
-		t.Fatalf("Expected to find %s in output of `docker images`, instead got %s", image, rr.Output())
+		t.Fatalf("Expected to find %s in image list output, instead got %s", image, rr.Output())
 	}
 }