autoconfigure cni and cri with proper cgroup driver

kubernetes · spowelljr · Jan 19, 2023 · Dec 5, 2022 · Dec 5, 2022 · Dec 5, 2022
commit e59d6217a82ceccec53fc064e831ab4894b4ac26
diff --git a/deploy/iso/minikube-iso/arch/aarch64/package/containerd-bin-aarch64/containerd.service b/deploy/iso/minikube-iso/arch/aarch64/package/containerd-bin-aarch64/containerd.service
@@ -9,7 +9,7 @@ EnvironmentFile=-/etc/sysconfig/containerd
 EnvironmentFile=-/etc/sysconfig/containerd.minikube
 EnvironmentFile=/var/run/minikube/env
 Environment=GOTRACEBACK=crash
-ExecStartPre=-/sbin/modprobe overlay
+ExecStartPre=/sbin/modprobe overlay
 ExecStart=/usr/bin/containerd \
       $CONTAINERD_OPTIONS \
       $CONTAINERD_MINIKUBE_OPTIONS \

diff --git a/deploy/iso/minikube-iso/arch/x86_64/package/containerd-bin/containerd.service b/deploy/iso/minikube-iso/arch/x86_64/package/containerd-bin/containerd.service
@@ -9,7 +9,7 @@ EnvironmentFile=-/etc/sysconfig/containerd
 EnvironmentFile=-/etc/sysconfig/containerd.minikube
 EnvironmentFile=/var/run/minikube/env
 Environment=GOTRACEBACK=crash
-ExecStartPre=-/sbin/modprobe overlay
+ExecStartPre=/sbin/modprobe overlay
 ExecStart=/usr/bin/containerd \
       $CONTAINERD_OPTIONS \
       $CONTAINERD_MINIKUBE_OPTIONS \

diff --git a/hack/preload-images/generate.go b/hack/preload-images/generate.go
@@ -31,6 +31,7 @@ import (
 	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/cruntime"
+	"k8s.io/minikube/pkg/minikube/detect"
 	"k8s.io/minikube/pkg/minikube/localpath"
 	"k8s.io/minikube/pkg/minikube/sysinit"
 	"k8s.io/minikube/pkg/util"
@@ -93,7 +94,8 @@ func generateTarball(kubernetesVersion, containerRuntime, tarballFilename string
 	if err != nil {
 		return errors.Wrap(err, "failed create new runtime")
 	}
-	if err := cr.Enable(true, false, false); err != nil {
+
+	if err := cr.Enable(true, detect.CgroupDriver(), false); err != nil {
 		return errors.Wrap(err, "enable container runtime")
 	}
 

diff --git a/pkg/minikube/cni/cni.go b/pkg/minikube/cni/cni.go
@@ -20,6 +20,7 @@ package cni
 import (
 	"context"
 	"fmt"
+	"net"
 	"os/exec"
 	"path"
 	"path/filepath"
@@ -34,7 +35,6 @@ import (
 	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/constants"
 	"k8s.io/minikube/pkg/minikube/driver"
-	"k8s.io/minikube/pkg/minikube/sysinit"
 	"k8s.io/minikube/pkg/minikube/vmpath"
 )
 
@@ -176,10 +176,6 @@ func manifestAsset(b []byte) assets.CopyableFile {
 
 // applyManifest applies a CNI manifest
 func applyManifest(cc config.ClusterConfig, r Runner, f assets.CopyableFile) error {
-	if err := NameLoopback(r); err != nil {
-		klog.Warningf("unable to name loopback interface in applyManifest: %v", err)
-	}
-
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	defer cancel()
 
@@ -198,12 +194,12 @@ func applyManifest(cc config.ClusterConfig, r Runner, f assets.CopyableFile) err
 	return nil
 }
 
-// NameLoopback ensures loopback has a name in its config file in /etc/cni/net.d
-// cri-o is leaving it out atm (https://github.com/cri-o/cri-o/pull/6273)
+// 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]
-func NameLoopback(r Runner) error {
+func ConfigureLoopback(r Runner) error {
 	loopback := "/etc/cni/net.d/*loopback.conf*" // usually: 200-loopback.conf
 	// turn { "cniVersion": "0.3.1", "type": "loopback" }
 	// into { "cniVersion": "0.3.1", "name": "loopback", "type": "loopback" }
@@ -213,38 +209,90 @@ func NameLoopback(r Runner) error {
 	}
 	if _, err := r.RunCmd(exec.Command(
 		"sudo", "find", filepath.Dir(loopback), "-maxdepth", "1", "-type", "f", "-name", filepath.Base(loopback), "-exec", "sh", "-c",
-		`grep -q loopback {} && ( grep -q name {} || sudo sed -i '/"type": "loopback"/i \ \ \ \ "name": "loopback",' {} )`, ";")); err != nil {
+		`grep -q loopback {} && ( grep -q name {} || sudo sed -i '/"type": "loopback"/i \ \ \ \ "name": "loopback",' {} ) && sudo sed -i 's|"cniVersion": ".*"|"cniVersion": "1.0.0"|g' {}`, ";")); err != nil {
 		return fmt.Errorf("unable to patch loopback config %q: %v", loopback, err)
 	}
 	return nil
 }
 
-// DisableBridgeCNIs disables all default bridge CNIs on a node (designated by runner) by changing extension to "mk_disabled" of *bridge* config file(s) found in /etc/cni/net.d.
-// It's usually called before deploying new CNI or on restarts, to avoid conflicts and flip-flopping of pods' ip addresses.
+// ConfigureDefaultBridgeCNIs configures all default bridge CNIs on a node (designated by runner).
+// If network plugin is set (could be, eg "cni" or "kubenet"), it will disable all default bridges by changing extension to "mk_disabled" of *bridge* config file(s) found in /etc/cni/net.d to avoid conflicts.
+// Otherwise, it will change ip address range to match DefaultPodCIDR in all *bridge* config file(s) found in /etc/cni/net.d.
+// It's usually called before deploying new CNI and on node restarts, to avoid conflicts and flip-flopping of pods' ip addresses.
+// It is caller's responsibility to restart container runtime for these changes to take effect.
 // ref: https://github.com/containernetworking/cni/blob/main/libcni/conf.go
-func DisableAllBridgeCNIs(r Runner, cc config.ClusterConfig) error {
+// ref: https://kubernetes.io/docs/tasks/administer-cluster/migrating-from-dockershim/troubleshooting-cni-plugin-related-errors/
+func ConfigureDefaultBridgeCNIs(r Runner, networkPlugin string) error {
+	if networkPlugin != "" {
+		return disableAllBridgeCNIs(r)
+	}
+
+	return configureAllBridgeCNIs(r, DefaultPodCIDR)
+}
+
+func disableAllBridgeCNIs(r Runner) error {
 	path := "/etc/cni/net.d"
+
 	out, err := r.RunCmd(exec.Command(
-		"sudo", "find", path, "-maxdepth", "1", "-type", "f", "-name", "*bridge*", "-not", "-name", "*.mk_disabled", "-printf", "%p|", "-exec", "sh", "-c",
+		"sudo", "find", path, "-maxdepth", "1", "-type", "f", "-name", "*bridge*", "-not", "-name", "*.mk_disabled", "-printf", "%p, ", "-exec", "sh", "-c",
 		`sudo mv {} {}.mk_disabled`, ";"))
 	if err != nil {
 		return fmt.Errorf("failed to disable all bridge cni configs in %q: %v", path, err)
 	}
-	configs := strings.Trim(out.Stdout.String(), "|")
+	configs := strings.Trim(out.Stdout.String(), ", ")
 	if len(configs) == 0 {
-		klog.Infof("no bridge cni config found in %q - nothing to disable", configs, path)
+		klog.Infof("no bridge cni configs found in %q - nothing to disable", configs, path)
 		return nil
 	}
-	svc := cc.KubernetesConfig.ContainerRuntime
-	klog.Infof("disabled [%s] bridge cni config(s) in %q, now restarting selected %q container runtime", configs, path, svc)
+	klog.Infof("disabled [%s] bridge cni config(s)", configs)
+
+	return nil
+}
 
-	if svc == "cri-o" {
-		svc = "crio"
+func configureAllBridgeCNIs(r Runner, cidr string) error {
+	path := "/etc/cni/net.d"
+	configs := ""
+
+	// non-podman configs:
+	out, err := r.RunCmd(exec.Command(
+		"sudo", "find", path, "-maxdepth", "1", "-type", "f", "-name", "*bridge*", "-not", "-name", "*podman*", "-not", "-name", "*.mk_disabled", "-printf", "%p, ", "-exec", "sh", "-c",
+		// remove ipv6 entries to avoid "failed to set bridge addr: could not add IP address to \"cni0\": permission denied"
+		// ref: https://github.com/cri-o/cri-o/issues/3555
+		// then also remove trailing comma after ipv4 elements, if any
+		// ie, this will transform from, eg:
+		// from: "ranges": [ [{ "subnet": "10.85.0.0/16" }], [{ "subnet": "1100:200::/24" }] ]
+		// to:   "ranges": [ [{ "subnet": "10.244.0.0/16" }] ]
+		// getting something similar to https://github.com/cri-o/cri-o/blob/main/contrib/cni/11-crio-ipv4-bridge.conflist
+		fmt.Sprintf(`sudo sed -i -r -e '/"dst": ".*:.*"/d' -e 's|^(.*)"dst": (.*)[,*]$|\1"dst": \2|g' -e '/"subnet": ".*:.*"/d' -e 's|^(.*)"subnet": ".*"(.*)[,*]$|\1"subnet": "%s"\2|g' {}`, cidr), ";"))
+	if err != nil {
+		klog.Errorf("failed to configure non-podman bridge cni configs in %q: %v", path, err)
+	} else {
+		configs = out.Stdout.String()
 	}
-	if err := sysinit.New(r).Restart(svc); err != nil {
-		klog.Warningf("failed to restart %q container runtime service in %q: %v", svc, cc.Name, err)
-		return err
+
+	// podman config(s):
+	// ref: https://github.com/containers/podman/blob/main/cni/87-podman-bridge.conflist
+	ip, ipnet, err := net.ParseCIDR(cidr)
+	if err != nil || ip.To4() == nil {
+		klog.Errorf("cidr %q is not valid ipv4 address: %v", cidr, err)
+	} else {
+		gateway := ip.Mask(ipnet.Mask)
+		gateway[3]++
+		out, err = r.RunCmd(exec.Command(
+			"sudo", "find", path, "-maxdepth", "1", "-type", "f", "-name", "*bridge*", "-name", "*podman*", "-not", "-name", "*.mk_disabled", "-printf", "%p, ", "-exec", "sh", "-c",
+			fmt.Sprintf(`sudo sed -i -r -e 's|^(.*)"subnet": ".*"(.*)$|\1"subnet": "%s"\2|g' -e 's|^(.*)"gateway": ".*"(.*)$|\1"gateway": "%s"\2|g' {}`, cidr, gateway), ";"))
+		if err != nil {
+			klog.Errorf("failed to configure podman bridge cni configs in %q: %v", path, err)
+		} else {
+			configs += out.Stdout.String()
+		}
+	}
+
+	if len(strings.Trim(configs, ", ")) == 0 {
+		klog.Infof("no bridge cni configs found in %q - nothing to configure", configs, path)
+		return nil
 	}
+	klog.Infof("configured [%s] bridge cni config(s)", configs)
 
 	return nil
 }
diff --git a/pkg/minikube/config/types.go b/pkg/minikube/config/types.go
@@ -115,6 +115,7 @@ type KubernetesConfig struct {
 	APIServerNames      []string
 	APIServerIPs        []net.IP
 	DNSDomain           string
+	CgroupDriver        string
 	ContainerRuntime    string
 	CRISocket           string
 	NetworkPlugin       string
@@ -144,6 +145,7 @@ type Node struct {
 	IP                string
 	Port              int
 	KubernetesVersion string
+	CgroupDriver      string
 	ContainerRuntime  string
 	ControlPlane      bool
 	Worker            bool

diff --git a/pkg/minikube/constants/constants.go b/pkg/minikube/constants/constants.go
@@ -54,6 +54,7 @@ const (
 	SSHPort = 22
 	// RegistryAddonPort os the default registry addon port
 	RegistryAddonPort = 5000
+
 	// Containerd is the default name and spelling for the containerd container runtime
 	Containerd = "containerd"
 	// CRIO is the default name and spelling for the cri-o container runtime
@@ -63,6 +64,12 @@ const (
 	// DefaultContainerRuntime is our default container runtime
 	DefaultContainerRuntime = ""
 
+	// cgroup drivers
+	DefaultCgroupDriver  = "systemd"
+	CgroupfsCgroupDriver = "cgroupfs"
+	SystemdCgroupDriver  = "systemd"
+	UnknownCgroupDriver  = ""
+
 	// APIServerName is the default API server name
 	APIServerName = "minikubeCA"
 	// ClusterDNSDomain is the default DNS domain

diff --git a/pkg/minikube/cruntime/containerd.go b/pkg/minikube/cruntime/containerd.go
@@ -26,6 +26,7 @@ import (
 	"os"
 	"os/exec"
 	"path"
+	"runtime"
 	"strings"
 	"time"
 
@@ -37,6 +38,7 @@ import (
 	"k8s.io/minikube/pkg/minikube/cni"
 	"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/style"
 	"k8s.io/minikube/pkg/minikube/sysinit"
@@ -127,18 +129,36 @@ func (r *Containerd) Available() error {
 }
 
 // generateContainerdConfig sets up /etc/containerd/config.toml & /etc/containerd/containerd.conf.d/02-containerd.conf
-func generateContainerdConfig(cr CommandRunner, imageRepository string, kv semver.Version, forceSystemd bool, insecureRegistry []string, inUserNamespace bool) error {
+func generateContainerdConfig(cr CommandRunner, imageRepository string, kv semver.Version, cgroupDriver string, insecureRegistry []string, inUserNamespace bool) error {
 	pauseImage := images.Pause(kv, imageRepository)
-	if _, err := cr.RunCmd(exec.Command("/bin/bash", "-c", fmt.Sprintf("sudo sed -e 's|^.*sandbox_image = .*$|sandbox_image = \"%s\"|' -i %s", pauseImage, containerdConfigFile))); err != nil {
+	if _, err := cr.RunCmd(exec.Command("sh", "-c", fmt.Sprintf(`sudo sed -i -r 's|^( *)sandbox_image = .*$|\1sandbox_image = %q|' %s`, pauseImage, containerdConfigFile))); err != nil {
 		return errors.Wrap(err, "update sandbox_image")
 	}
-	if _, err := cr.RunCmd(exec.Command("/bin/bash", "-c", fmt.Sprintf("sudo sed -e 's|^.*restrict_oom_score_adj = .*$|restrict_oom_score_adj = %t|' -i %s", inUserNamespace, containerdConfigFile))); err != nil {
+	if _, err := cr.RunCmd(exec.Command("sh", "-c", fmt.Sprintf(`sudo sed -i -r 's|^( *)restrict_oom_score_adj = .*$|\1restrict_oom_score_adj = %t|' %s`, inUserNamespace, containerdConfigFile))); err != nil {
 		return errors.Wrap(err, "update restrict_oom_score_adj")
 	}
-	if _, err := cr.RunCmd(exec.Command("/bin/bash", "-c", fmt.Sprintf("sudo sed -e 's|^.*SystemdCgroup = .*$|SystemdCgroup = %t|' -i %s", forceSystemd, containerdConfigFile))); err != nil {
-		return errors.Wrap(err, "update SystemdCgroup")
+	// configure cgroup driver
+	if cgroupDriver != constants.UnknownCgroupDriver {
+		klog.Infof("configuring containerd to use %q as cgroup driver...", cgroupDriver)
+		useSystemd := cgroupDriver == constants.SystemdCgroupDriver
+		if _, err := cr.RunCmd(exec.Command("sh", "-c", fmt.Sprintf(`sudo sed -i -r 's|^( *)SystemdCgroup = .*$|\1SystemdCgroup = %t|g' %s`, useSystemd, containerdConfigFile))); err != nil {
+			return errors.Wrap(err, "configuring SystemdCgroup")
+		}
+	}
+	// handle deprecated features
+	// ref: https://github.com/containerd/containerd/blob/main/RELEASES.md#deprecated-features
+	if _, err := cr.RunCmd(exec.Command("sh", "-c", fmt.Sprintf(`sudo sed -i -r 's|"io.containerd.runtime.v1.linux"|"io.containerd.runc.v2"|g' %s`, containerdConfigFile))); err != nil {
+		return errors.Wrap(err, "configuring io.containerd.runtime version")
+	}
+	if _, err := cr.RunCmd(exec.Command("sh", "-c", fmt.Sprintf(`sudo sed -i -r 's|"io.containerd.runc.v1"|"io.containerd.runc.v2"|g' %s`, containerdConfigFile))); err != nil {
+		return errors.Wrap(err, "configuring io.containerd.runc version")
+	}
+	// ensure conf_dir is using '/etc/cni/net.d'
+	// TODO (@prezha): this should be removed (ie, not needed) once we remove "hardcoded" '/etc/cni/net.mk' folder in minikube distro
+	if _, err := cr.RunCmd(exec.Command("sh", "-c", `sudo rm -rf /etc/cni/net.mk`)); err != nil {
+		return fmt.Errorf("unable to remove /etc/cni/net.mk directory: %v", err)
 	}
-	if _, err := cr.RunCmd(exec.Command("/bin/bash", "-c", fmt.Sprintf("sudo sed -e 's|^.*conf_dir = .*$|conf_dir = \"%s\"|' -i %s", cni.DefaultConfDir, containerdConfigFile))); err != nil {
+	if _, err := cr.RunCmd(exec.Command("sh", "-c", fmt.Sprintf(`sudo sed -i -r 's|^( *)conf_dir = .*$|\1conf_dir = %q|g' %s`, cni.DefaultConfDir, containerdConfigFile))); err != nil {
 		return errors.Wrap(err, "update conf_dir")
 	}
 
@@ -176,7 +196,7 @@ func generateContainerdConfig(cr CommandRunner, imageRepository string, kv semve
 
 // Enable idempotently enables containerd on a host
 // It is also called by docker.Enable() - if bound to containerd, to enforce proper containerd configuration completed by service restart.
-func (r *Containerd) Enable(disOthers, forceSystemd, inUserNamespace bool) error {
+func (r *Containerd) Enable(disOthers bool, cgroupDriver string, inUserNamespace bool) error {
 	if inUserNamespace {
 		if err := CheckKernelCompatibility(r.Runner, 5, 11); err != nil {
 			// For using overlayfs
@@ -195,13 +215,25 @@ func (r *Containerd) Enable(disOthers, forceSystemd, inUserNamespace bool) error
 	if err := populateCRIConfig(r.Runner, r.SocketPath()); err != nil {
 		return err
 	}
-	if err := generateContainerdConfig(r.Runner, r.ImageRepository, r.KubernetesVersion, forceSystemd, r.InsecureRegistry, inUserNamespace); err != nil {
+
+	if err := generateContainerdConfig(r.Runner, r.ImageRepository, r.KubernetesVersion, cgroupDriver, r.InsecureRegistry, inUserNamespace); err != nil {
 		return err
 	}
 	if err := enableIPForwarding(r.Runner); err != nil {
 		return err
 	}
 
+	// TODO (@prezha): remove this hack after proper version update in minikube release
+	// ref: https://github.com/containerd/containerd/blob/main/RELEASES.md#kubernetes-support
+	targetVersion := "1.6.14"
+	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)
+		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)
+		}
+	}
+
 	// Otherwise, containerd will fail API requests with 'Unimplemented'
 	return r.Init.Restart("containerd")
 }

diff --git a/pkg/minikube/cruntime/crio.go b/pkg/minikube/cruntime/crio.go
@@ -34,6 +34,7 @@ import (
 	"k8s.io/minikube/pkg/minikube/cni"
 	"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/style"
 	"k8s.io/minikube/pkg/minikube/sysinit"
@@ -72,10 +73,15 @@ func generateCRIOConfig(cr CommandRunner, imageRepository string, kv semver.Vers
 	return nil
 }
 
-func (r *CRIO) forceSystemd() error {
-	c := exec.Command("/bin/bash", "-c", fmt.Sprintf("sudo sed -e 's|^.*cgroup_manager = .*$|cgroup_manager = \"systemd\"|' -i %s", crioConfigFile))
+func (r *CRIO) setCGroup(driver string) error {
+	if driver == constants.UnknownCgroupDriver {
+		return fmt.Errorf("unable to configure cri-o to use unknown cgroup driver")
+	}
+
+	klog.Infof("configuring cri-o to use %q as cgroup driver...", driver)
+	c := exec.Command("/bin/bash", "-c", fmt.Sprintf(`sudo sed -i -r 's|^( *)cgroup_manager = .*$|\1cgroup_manager = %q|' %s`, driver, crioConfigFile))
 	if _, err := r.Runner.RunCmd(c); err != nil {
-		return errors.Wrap(err, "force systemd")
+		return errors.Wrap(err, "configuring cgroup_manager")
 	}
 
 	return nil
@@ -185,7 +191,7 @@ Environment="_CRIO_ROOTLESS=1"
 }
 
 // Enable idempotently enables CRIO on a host
-func (r *CRIO) Enable(disOthers, forceSystemd, inUserNamespace bool) error {
+func (r *CRIO) Enable(disOthers bool, cgroupDriver string, inUserNamespace bool) error {
 	if disOthers {
 		if err := disableOthers(r, r.Runner); err != nil {
 			klog.Warningf("disableOthers: %v", err)
@@ -200,10 +206,8 @@ func (r *CRIO) Enable(disOthers, forceSystemd, inUserNamespace bool) error {
 	if err := enableIPForwarding(r.Runner); err != nil {
 		return err
 	}
-	if forceSystemd {
-		if err := r.forceSystemd(); err != nil {
-			return err
-		}
+	if err := r.setCGroup(cgroupDriver); err != nil {
+		return err
 	}
 	if inUserNamespace {
 		if err := CheckKernelCompatibility(r.Runner, 5, 11); err != nil {