Merge pull request #11632 from ilya-zuyev/ilyaz/fix_vm_ctrd_timeout

Restore "containerd: upgrade io.containerd.runtime.v1.linux to io.containerd.runc.v2 (suppot cgroup v2)" #2

Makefile

@@ -23,7 +23,7 @@ KUBERNETES_VERSION ?= $(shell egrep "DefaultKubernetesVersion =" pkg/minikube/co
 KIC_VERSION ?= $(shell egrep "Version =" pkg/drivers/kic/types.go | cut -d \" -f2)
 
 # Default to .0 for higher cache hit rates, as build increments typically don't require new ISO versions
-ISO_VERSION ?= v1.21.0
+ISO_VERSION ?= v1.21.0-1623378770-11632
 # Dashes are valid in semver, but not Linux packaging. Use ~ to delimit alpha/beta
 DEB_VERSION ?= $(subst -,~,$(RAW_VERSION))
 DEB_REVISION ?= 0

deploy/iso/minikube-iso/package/containerd-bin/containerd.service

@@ -16,7 +16,7 @@ ExecStart=/usr/bin/containerd \
       --root ${PERSISTENT_DIR}/var/lib/containerd
 TasksMax=8192
 Delegate=yes
-KillMode=process
+KillMode=mixed
 LimitNOFILE=1048576
 # Having non-zero Limit*s causes performance problems due to accounting overhead
 # in the kernel. We recommend using cgroups to do container-local accounting.

pkg/minikube/bootstrapper/bsutil/kverify/api_server.go

@@ -143,6 +143,22 @@ func APIServerVersionMatch(client *kubernetes.Clientset, expected string) error
 	return nil
 }
 
+// WaitForAPIServerStatus waits for 'to' duration to get apiserver pod running or stopped
+// this functions is intended to use in situations where apiserver process can be recreated
+// by container runtime restart for example and there is a gap before it comes back
+func WaitForAPIServerStatus(cr command.Runner, to time.Duration, hostname string, port int) (state.State, error) {
+	var st state.State
+	err := wait.PollImmediate(200*time.Millisecond, to, func() (bool, error) {
+		var err error
+		st, err = APIServerStatus(cr, hostname, port)
+		if st == state.Stopped {
+			return false, nil
+		}
+		return true, err
+	})
+	return st, err
+}
+
 // APIServerStatus returns apiserver status in libmachine style state.State
 func APIServerStatus(cr command.Runner, hostname string, port int) (state.State, error) {
 	klog.Infof("Checking apiserver status ...")
@@ -207,7 +223,7 @@ func apiServerHealthz(hostname string, port int) (state.State, error) {
 		return nil
 	}
 
-	err = retry.Local(check, 5*time.Second)
+	err = retry.Local(check, 15*time.Second)
 
 	// Don't propagate 'Stopped' upwards as an error message, as clients may interpret the err
 	// as an inability to get status. We need it for retry.Local, however.

pkg/minikube/bootstrapper/kubeadm/kubeadm.go

@@ -568,13 +568,13 @@ func (k *Bootstrapper) needsReconfigure(conf string, hostname string, port int,
 		klog.Infof("needs reconfigure: configs differ:\n%s", rr.Output())
 		return true
 	}
-
-	st, err := kverify.APIServerStatus(k.c, hostname, port)
+	// cruntime.Enable() may restart kube-apiserver but does not wait for it to return back
+	apiStatusTimeout := 3000 * time.Millisecond
+	st, err := kverify.WaitForAPIServerStatus(k.c, apiStatusTimeout, hostname, port)
 	if err != nil {
 		klog.Infof("needs reconfigure: apiserver error: %v", err)
 		return true
 	}
-
 	if st != state.Running {
 		klog.Infof("needs reconfigure: apiserver in state %s", st)
 		return true

pkg/minikube/cluster/pause.go

@@ -47,12 +47,10 @@ func pause(cr cruntime.Manager, r command.Runner, namespaces []string) ([]string
 
 	// Disable the kubelet so it does not attempt to restart paused pods
 	sm := sysinit.New(r)
-	if err := sm.Disable("kubelet"); err != nil {
-		return ids, errors.Wrap(err, "kubelet disable")
-	}
+	klog.Info("kubelet running: ", sm.Active("kubelet"))
 
-	if err := sm.Stop("kubelet"); err != nil {
-		return ids, errors.Wrap(err, "kubelet stop")
+	if err := sm.DisableNow("kubelet"); err != nil {
+		return ids, errors.Wrap(err, "kubelet disable --now")
 	}
 
 	ids, err := cr.ListContainers(cruntime.ListContainersOptions{State: cruntime.Running, Namespaces: namespaces})

pkg/minikube/cruntime/containerd.go

@@ -49,7 +49,6 @@ const (
 	containerdConfigTemplate = `root = "/var/lib/containerd"
 state = "/run/containerd"
 oom_score = 0
-
 [grpc]
   address = "/run/containerd/containerd.sock"
   uid = 0
@@ -79,16 +78,21 @@ oom_score = 0
     enable_selinux = false
     sandbox_image = "{{ .PodInfraContainerImage }}"
     stats_collect_period = 10
-    systemd_cgroup = {{ .SystemdCgroup }}
     enable_tls_streaming = false
     max_container_log_line_size = 16384
+
+	[plugins."io.containerd.grpc.v1.cri"]
+      [plugins."io.containerd.grpc.v1.cri".containerd]
+        [plugins."io.containerd.grpc.v1.cri".containerd.runtimes]
+          [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc]
+            runtime_type = "io.containerd.runc.v2"
+            [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options]
+              SystemdCgroup = {{ .SystemdCgroup }}
+
     [plugins.cri.containerd]
       snapshotter = "overlayfs"
-      no_pivot = true
       [plugins.cri.containerd.default_runtime]
-        runtime_type = "io.containerd.runtime.v1.linux"
-        runtime_engine = ""
-        runtime_root = ""
+        runtime_type = "io.containerd.runc.v2"
       [plugins.cri.containerd.untrusted_workload_runtime]
         runtime_type = ""
         runtime_engine = ""
@@ -107,12 +111,6 @@ oom_score = 0
         {{ end -}}
   [plugins.diff-service]
     default = ["walking"]
-  [plugins.linux]
-    shim = "containerd-shim"
-    runtime = "runc"
-    runtime_root = ""
-    no_shim = false
-    shim_debug = false
   [plugins.scheduler]
     pause_threshold = 0.02
     deletion_threshold = 0

pkg/minikube/cruntime/cruntime.go

@@ -163,6 +163,30 @@ type ListImagesOptions struct {
 // ErrContainerRuntimeNotRunning is thrown when container runtime is not running
 var ErrContainerRuntimeNotRunning = errors.New("container runtime is not running")
 
+// ErrServiceVersion is the error returned when disk image has incompatible version of service
+type ErrServiceVersion struct {
+	// Service is the name of the incompatible service
+	Service string
+	// Installed is the installed version of Service
+	Installed string
+	// Required is the minimum required version of Service
+	Required string
+}
+
+// NewErrServiceVersion creates a new ErrServiceVersion
+func NewErrServiceVersion(svc, required, installed string) *ErrServiceVersion {
+	return &ErrServiceVersion{
+		Service:   svc,
+		Installed: installed,
+		Required:  required,
+	}
+}
+
+func (e ErrServiceVersion) Error() string {
+	return fmt.Sprintf("service %q version is %v. Required: %v",
+		e.Service, e.Installed, e.Required)
+}
+
 // New returns an appropriately configured runtime
 func New(c Config) (Manager, error) {
 	sm := sysinit.New(c.Runner)
@@ -243,3 +267,29 @@ func disableOthers(me Manager, cr CommandRunner) error {
 	}
 	return nil
 }
+
+var requiredContainerdVersion = semver.MustParse("1.4.0")
+
+// compatibleWithVersion checks if current version of "runtime" is compatible with version "v"
+func compatibleWithVersion(runtime, v string) error {
+	if runtime == "containerd" {
+		vv, err := semver.Make(v)
+		if err != nil {
+			return err
+		}
+		if requiredContainerdVersion.GT(vv) {
+			return NewErrServiceVersion(runtime, requiredContainerdVersion.String(), vv.String())
+		}
+	}
+	return nil
+}
+
+// CheckCompatibility checks if the container runtime managed by "cr" is compatible with current minikube code
+// returns: NewErrServiceVersion if not
+func CheckCompatibility(cr Manager) error {
+	v, err := cr.Version()
+	if err != nil {
+		return errors.Wrap(err, "Failed to check container runtime version")
+	}
+	return compatibleWithVersion(cr.Name(), v)
+}

pkg/minikube/download/iso.go

@@ -40,7 +40,7 @@ const fileScheme = "file"
 // DefaultISOURLs returns a list of ISO URL's to consult by default, in priority order
 func DefaultISOURLs() []string {
 	v := version.GetISOVersion()
-	isoBucket := "minikube/iso"
+	isoBucket := "minikube-builds/iso/11632"
 	return []string{
 		fmt.Sprintf("https://storage.googleapis.com/%s/minikube-%s.iso", isoBucket, v),
 		fmt.Sprintf("https://github.com/kubernetes/minikube/releases/download/%s/minikube-%s.iso", v, v),

pkg/minikube/node/advice.go

@@ -17,11 +17,13 @@ limitations under the License.
 package node
 
 import (
+	"fmt"
 	"runtime"
 
 	"github.com/pkg/errors"
 	"k8s.io/minikube/pkg/drivers/kic/oci"
 	"k8s.io/minikube/pkg/minikube/bootstrapper/kubeadm"
+	"k8s.io/minikube/pkg/minikube/cruntime"
 	"k8s.io/minikube/pkg/minikube/exit"
 	"k8s.io/minikube/pkg/minikube/reason"
 	"k8s.io/minikube/pkg/minikube/style"
@@ -62,4 +64,14 @@ func ExitIfFatal(err error) {
 			Advice:   "Ensure that your Docker mountpoints do not have the 'noexec' flag set",
 		}, "The kubeadm binary within the Docker container is not executable")
 	}
+
+	if rtErr, ok := err.(*cruntime.ErrServiceVersion); ok {
+		exit.Message(reason.Kind{
+			ID:       "PROVIDER_INVALID_VERSION",
+			ExitCode: reason.ExGuestConfig,
+			Style:    style.Unsupported,
+			Advice:   "Try to start minikube with '--delete-on-failure=true' option",
+		}, fmt.Sprintf("Your existing minikube instance has version %s of service %v which is too old. "+
+			"Please try to start minikube with --delete-on-failure=true option", rtErr.Installed, rtErr.Service))
+	}
 }

pkg/minikube/node/start.go

@@ -97,6 +97,12 @@ func Start(starter Starter, apiServer bool) (*kubeconfig.Settings, error) {
 
 	// configure the runtime (docker, containerd, crio)
 	cr := configureRuntimes(starter.Runner, *starter.Cfg, sv)
+
+	// check if installed runtime is compatible with current minikube code
+	if err = cruntime.CheckCompatibility(cr); err != nil {
+		return nil, err
+	}
+
 	showVersionInfo(starter.Node.KubernetesVersion, cr)
 
 	// Add "host.minikube.internal" DNS alias (intentionally non-fatal)
@@ -353,7 +359,6 @@ func configureRuntimes(runner cruntime.CommandRunner, cc config.ClusterConfig, k
 	if err != nil {
 		exit.Error(reason.RuntimeEnable, "Failed to start container runtime", err)
 	}
-
 	return cr
 }
 

pkg/minikube/sysinit/openrc.go

@@ -117,9 +117,11 @@ func (s *OpenRC) Disable(svc string) error {
 	return nil
 }
 
-// DisableNow not implemented for openRC
+// DisableNow does Disable + Stop
 func (s *OpenRC) DisableNow(svc string) error {
-	return fmt.Errorf("disable now is not implemented for OpenRC! PRs to fix are welcomed")
+	// supposed to do disable + stop
+	// disable does nothing for OpenRC, so just Stop here
+	return s.Stop(svc)
 }
 
 // Mask does nothing
@@ -132,9 +134,11 @@ func (s *OpenRC) Enable(svc string) error {
 	return nil
 }
 
-// EnableNow  not implemented for openRC
+// EnableNow does Enable + Start
 func (s *OpenRC) EnableNow(svc string) error {
-	return fmt.Errorf("enable now is not implemented for OpenRC! PRs to fix are welcomed")
+	// supposed to do enable + start
+	// enable does nothing for OpenRC, so just Start here
+	return s.Start(svc)
 }
 
 // Unmask does nothing

pkg/minikube/sysinit/systemd.go

@@ -50,7 +50,10 @@ func (s *Systemd) Active(svc string) bool {
 
 // Disable disables a service
 func (s *Systemd) Disable(svc string) error {
-	_, err := s.r.RunCmd(exec.Command("sudo", "systemctl", "disable", svc))
+	cmd := exec.Command("sudo", "systemctl", "disable", svc)
+	// See https://github.com/kubernetes/minikube/issues/11615#issuecomment-861794258
+	cmd.Env = append(cmd.Env, "SYSTEMCTL_SKIP_SYSV=1")
+	_, err := s.r.RunCmd(cmd)
 	return err
 }
 

site/content/en/docs/commands/start.md

@@ -64,7 +64,7 @@ minikube start [flags]
       --insecure-registry strings         Insecure Docker registries to pass to the Docker daemon.  The default service CIDR range will automatically be added.
       --install-addons                    If set, install addons. Defaults to true. (default true)
       --interactive                       Allow user prompts for more information (default true)
-      --iso-url strings                   Locations to fetch the minikube ISO from. (default [https://storage.googleapis.com/minikube/iso/minikube-v1.21.0.iso,https://github.com/kubernetes/minikube/releases/download/v1.21.0/minikube-v1.21.0.iso,https://kubernetes.oss-cn-hangzhou.aliyuncs.com/minikube/iso/minikube-v1.21.0.iso])
+      --iso-url strings                   Locations to fetch the minikube ISO from. (default [https://storage.googleapis.com/minikube-builds/iso/11632/minikube-v1.21.0-1623378770-11632.iso,https://github.com/kubernetes/minikube/releases/download/v1.21.0-1623378770-11632/minikube-v1.21.0-1623378770-11632.iso,https://kubernetes.oss-cn-hangzhou.aliyuncs.com/minikube/iso/minikube-v1.21.0-1623378770-11632.iso])
       --keep-context                      This will keep the existing kubectl context and will create a minikube context.
       --kubernetes-version string         The Kubernetes version that the minikube VM will use (ex: v1.2.3, 'stable' for v1.20.7, 'latest' for v1.22.0-alpha.2). Defaults to 'stable'.
       --kvm-gpu                           Enable experimental NVIDIA GPU support in minikube

test/integration/docker_test.go

@@ -114,7 +114,7 @@ func validateContainerdSystemd(ctx context.Context, t *testing.T, profile string
 	if err != nil {
 		t.Errorf("failed to get docker cgroup driver. args %q: %v", rr.Command(), err)
 	}
-	if !strings.Contains(rr.Output(), "systemd_cgroup = true") {
+	if !strings.Contains(rr.Output(), "SystemdCgroup = true") {
 		t.Fatalf("expected systemd cgroup driver, got: %v", rr.Output())
 	}
 }

test/integration/version_upgrade_test.go

@@ -87,6 +87,11 @@ func TestRunningBinaryUpgrade(t *testing.T) {
 			legacyVersion = "v1.9.0"
 		}
 	}
+	// the version containerd in ISO was upgraded to 1.4.2
+	// we need it to use runc.v2 plugin
+	if ContainerRuntime() == "containerd" {
+		legacyVersion = "v1.16.0"
+	}
 
 	tf, err := installRelease(legacyVersion)
 	if err != nil {
@@ -98,7 +103,7 @@ func TestRunningBinaryUpgrade(t *testing.T) {
 	rr := &RunResult{}
 	r := func() error {
 		c := exec.CommandContext(ctx, tf.Name(), args...)
-		legacyEnv := []string{}
+		var legacyEnv []string
 		// replace the global KUBECONFIG with a fresh kubeconfig
 		// because for minikube<1.17.0 it can not read the new kubeconfigs that have extra "Extenions" block
 		// see: https://github.com/kubernetes/minikube/issues/10210
@@ -155,8 +160,16 @@ func TestStoppedBinaryUpgrade(t *testing.T) {
 		if arm64Platform() {
 			// first release with non-experimental arm64 KIC
 			legacyVersion = "v1.17.0"
+		} else {
+			// v1.8.0 would be selected, but: https://github.com/kubernetes/minikube/issues/8740
+			legacyVersion = "v1.9.0"
 		}
 	}
+	if ContainerRuntime() == "containerd" {
+		// the version containerd in ISO was upgraded to 1.4.2
+		// we need it to use runc.v2 plugin
+		legacyVersion = "v1.16.0"
+	}
 
 	tf, err := installRelease(legacyVersion)
 	if err != nil {
@@ -168,7 +181,7 @@ func TestStoppedBinaryUpgrade(t *testing.T) {
 	rr := &RunResult{}
 	r := func() error {
 		c := exec.CommandContext(ctx, tf.Name(), args...)
-		legacyEnv := []string{}
+		var legacyEnv []string
 		// replace the global KUBECONFIG with a fresh kubeconfig
 		// because for minikube<1.17.0 it can not read the new kubeconfigs that have extra "Extenions" block
 		// see: https://github.com/kubernetes/minikube/issues/10210