improve how cni and cruntimes work together

Author: prezha

cmd/minikube/cmd/start_flags.go

@@ -253,17 +253,14 @@ func generateClusterConfig(cmd *cobra.Command, existing *config.ClusterConfig, k
 		klog.Info("no existing cluster config was found, will generate one from the flags ")
 		cc = generateNewConfigFromFlags(cmd, k8sVersion, drvName)
 
-		cnm, err := cni.New(cc)
+		cnm, err := cni.New(&cc)
 		if err != nil {
 			return cc, config.Node{}, errors.Wrap(err, "cni")
 		}
 
 		if _, ok := cnm.(cni.Disabled); !ok {
 			klog.Infof("Found %q CNI - setting NetworkPlugin=cni", cnm)
 			cc.KubernetesConfig.NetworkPlugin = "cni"
-			if err := setCNIConfDir(&cc, cnm); err != nil {
-				klog.Errorf("unable to set CNI Config Directory: %v", err)
-			}
 		}
 	}
 
@@ -428,24 +425,6 @@ func generateNewConfigFromFlags(cmd *cobra.Command, k8sVersion string, drvName s
 	return cc
 }
 
-// setCNIConfDir sets kubelet's '--cni-conf-dir' flag to custom CNI Config Directory path (same used also by CNI Deployment) to avoid conflicting CNI configs.
-// ref: https://github.com/kubernetes/minikube/issues/10984
-// Note: currently, this change affects only Kindnet CNI (and all multinodes using it), but it can be easily expanded to other/all CNIs if needed.
-func setCNIConfDir(cc *config.ClusterConfig, cnm cni.Manager) error {
-	if _, kindnet := cnm.(cni.KindNet); kindnet {
-		// auto-set custom CNI Config Directory, if not user-specified
-		eo := fmt.Sprintf("kubelet.cni-conf-dir=%s", cni.CustomCNIConfDir)
-		if !cc.KubernetesConfig.ExtraOptions.Exists(eo) {
-			klog.Infof("auto-setting extra-config to %q", eo)
-			if err := cc.KubernetesConfig.ExtraOptions.Set(eo); err != nil {
-				return fmt.Errorf("failed auto-setting extra-config %q: %v", eo, err)
-			}
-			klog.Infof("extra-config set to %q", eo)
-		}
-	}
-	return nil
-}
-
 func checkNumaCount(k8sVersion string) {
 	if viper.GetInt(kvmNUMACount) < 1 || viper.GetInt(kvmNUMACount) > 8 {
 		exit.Message(reason.Usage, "--kvm-numa-count range is 1-8")

pkg/minikube/bootstrapper/bsutil/kubeadm.go

@@ -74,7 +74,7 @@ func GenerateKubeadmYAML(cc config.ClusterConfig, n config.Node, r cruntime.Mana
 		return nil, errors.Wrap(err, "generating extra component config for kubeadm")
 	}
 
-	cnm, err := cni.New(cc)
+	cnm, err := cni.New(&cc)
 	if err != nil {
 		return nil, errors.Wrap(err, "cni")
 	}

pkg/minikube/bootstrapper/kubeadm/kubeadm.go

@@ -259,6 +259,7 @@ func (k *Bootstrapper) init(cfg config.ClusterConfig) error {
 	}
 	kw.Close()
 	wg.Wait()
+
 	if err := k.applyCNI(cfg, true); err != nil {
 		return errors.Wrap(err, "apply cni")
 	}
@@ -330,7 +331,7 @@ func (k *Bootstrapper) applyCNI(cfg config.ClusterConfig, registerStep ...bool)
 		regStep = registerStep[0]
 	}
 
-	cnm, err := cni.New(cfg)
+	cnm, err := cni.New(&cfg)
 	if err != nil {
 		return errors.Wrap(err, "cni config")
 	}
@@ -351,12 +352,6 @@ func (k *Bootstrapper) applyCNI(cfg config.ClusterConfig, registerStep ...bool)
 		return errors.Wrap(err, "cni apply")
 	}
 
-	if cfg.KubernetesConfig.ContainerRuntime == constants.CRIO {
-		if err := cruntime.UpdateCRIONet(k.c, cnm.CIDR()); err != nil {
-			return errors.Wrap(err, "update crio")
-		}
-	}
-
 	return nil
 }
 

pkg/minikube/cni/cni.go

@@ -30,19 +30,31 @@ import (
 	"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/driver"
 	"k8s.io/minikube/pkg/minikube/vmpath"
 )
 
 const (
 	// DefaultPodCIDR is the default CIDR to use in minikube CNI's.
 	DefaultPodCIDR = "10.244.0.0/16"
+
+	// DefaultConfDir is the default CNI Config Directory path
+	DefaultConfDir = "/etc/cni/net.d"
+	// CustomConfDir is the custom CNI Config Directory path used to avoid conflicting CNI configs
+	// ref: https://github.com/kubernetes/minikube/issues/10984 and https://github.com/kubernetes/minikube/pull/11106
+	CustomConfDir = "/etc/cni/net.mk"
 )
 
 var (
-	// CustomCNIConfDir is the custom CNI Config Directory path used to avoid conflicting CNI configs
-	// ref: https://github.com/kubernetes/minikube/issues/10984
-	CustomCNIConfDir = "/etc/cni/net.mk"
+	// ConfDir is the CNI Config Directory path that can be customised, defaulting to DefaultConfDir
+	ConfDir = DefaultConfDir
+
+	// Network is the network name that CNI should use (eg, "kindnet").
+	// Currently, only crio (and podman) can use it, so that setting custom ConfDir is not necessary.
+	// ref: https://github.com/cri-o/cri-o/issues/2121 (and https://github.com/containers/podman/issues/2370)
+	// ref: https://github.com/cri-o/cri-o/blob/master/docs/crio.conf.5.md#crionetwork-table
+	Network = ""
 )
 
 // Runner is the subset of command.Runner this package consumes
@@ -72,38 +84,40 @@ type tmplInput struct {
 }
 
 // New returns a new CNI manager
-func New(cc config.ClusterConfig) (Manager, error) {
+func New(cc *config.ClusterConfig) (Manager, error) {
 	if cc.KubernetesConfig.NetworkPlugin != "" && cc.KubernetesConfig.NetworkPlugin != "cni" {
 		klog.Infof("network plugin configured as %q, returning disabled", cc.KubernetesConfig.NetworkPlugin)
 		return Disabled{}, nil
 	}
 
 	klog.Infof("Creating CNI manager for %q", cc.KubernetesConfig.CNI)
 
-	// respect user-specified custom CNI Config Directory, if any
-	userCNIConfDir := cc.KubernetesConfig.ExtraOptions.Get("cni-conf-dir", "kubelet")
-	if userCNIConfDir != "" {
-		CustomCNIConfDir = userCNIConfDir
-	}
-
+	var cnm Manager
+	var err error
 	switch cc.KubernetesConfig.CNI {
 	case "", "auto":
-		return chooseDefault(cc), nil
+		cnm = chooseDefault(*cc)
 	case "false":
-		return Disabled{cc: cc}, nil
+		cnm = Disabled{cc: *cc}
 	case "kindnet", "true":
-		return KindNet{cc: cc}, nil
+		cnm = KindNet{cc: *cc}
 	case "bridge":
-		return Bridge{cc: cc}, nil
+		cnm = Bridge{cc: *cc}
 	case "calico":
-		return Calico{cc: cc}, nil
+		cnm = Calico{cc: *cc}
 	case "cilium":
-		return Cilium{cc: cc}, nil
+		cnm = Cilium{cc: *cc}
 	case "flannel":
-		return Flannel{cc: cc}, nil
+		cnm = Flannel{cc: *cc}
 	default:
-		return NewCustom(cc, cc.KubernetesConfig.CNI)
+		cnm, err = NewCustom(*cc, cc.KubernetesConfig.CNI)
 	}
+
+	if err := configureCNI(cc, cnm); err != nil {
+		klog.Errorf("unable to set CNI Config Directory: %v", err)
+	}
+
+	return cnm, err
 }
 
 // IsDisabled checks if CNI is disabled
@@ -183,3 +197,33 @@ func applyManifest(cc config.ClusterConfig, r Runner, f assets.CopyableFile) err
 
 	return nil
 }
+
+// configureCNI - to avoid conflicting CNI configs, it sets:
+// - for crio: 'cni_default_network' config param via cni.Network
+// - for containerd and docker: kubelet's '--cni-conf-dir' flag to custom CNI Config Directory path (same used also by CNI Deployment).
+// ref: https://github.com/kubernetes/minikube/issues/10984 and https://github.com/kubernetes/minikube/pull/11106
+// Note: currently, this change affects only Kindnet CNI (and all multinodes using it), but it can be easily expanded to other/all CNIs if needed.
+// Note2: Cilium does not need workaround as they automatically restart pods after CNI is successfully deployed.
+func configureCNI(cc *config.ClusterConfig, cnm Manager) error {
+	if _, kindnet := cnm.(KindNet); kindnet {
+		// crio only needs CNI network name; hopefully others (containerd, docker and kubeadm/kubelet) will follow eventually
+		if cc.KubernetesConfig.ContainerRuntime == constants.CRIO {
+			Network = "kindnet"
+			return nil
+		}
+		// for containerd and docker: auto-set custom CNI via kubelet's 'cni-conf-dir' param, if not user-specified
+		eo := fmt.Sprintf("kubelet.cni-conf-dir=%s", CustomConfDir)
+		if !cc.KubernetesConfig.ExtraOptions.Exists(eo) {
+			klog.Infof("auto-setting extra-config to %q", eo)
+			if err := cc.KubernetesConfig.ExtraOptions.Set(eo); err != nil {
+				return fmt.Errorf("failed auto-setting extra-config %q: %v", eo, err)
+			}
+			ConfDir = CustomConfDir
+			klog.Infof("extra-config set to %q", eo)
+		} else {
+			// respect user-specified custom CNI Config Directory
+			ConfDir = cc.KubernetesConfig.ExtraOptions.Get("cni-conf-dir", "kubelet")
+		}
+	}
+	return nil
+}

pkg/minikube/cni/kindnet.go

@@ -166,7 +166,7 @@ func (c KindNet) manifest() (assets.CopyableFile, error) {
 		DefaultRoute: "0.0.0.0/0", // assumes IPv4
 		PodCIDR:      DefaultPodCIDR,
 		ImageName:    images.KindNet(c.cc.KubernetesConfig.ImageRepository),
-		CNIConfDir:   CustomCNIConfDir,
+		CNIConfDir:   ConfDir,
 	}
 
 	b := bytes.Buffer{}

pkg/minikube/cruntime/containerd.go

@@ -34,6 +34,7 @@ import (
 	"k8s.io/klog/v2"
 	"k8s.io/minikube/pkg/minikube/assets"
 	"k8s.io/minikube/pkg/minikube/bootstrapper/images"
+	"k8s.io/minikube/pkg/minikube/cni"
 	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/download"
@@ -94,7 +95,7 @@ oom_score = 0
         runtime_root = ""
     [plugins.cri.cni]
       bin_dir = "/opt/cni/bin"
-      conf_dir = "/etc/cni/net.d"
+      conf_dir = "{{.CNIConfDir}}"
       conf_template = ""
     [plugins.cri.registry]
       [plugins.cri.registry.mirrors]
@@ -190,10 +191,12 @@ func generateContainerdConfig(cr CommandRunner, imageRepository string, kv semve
 		PodInfraContainerImage string
 		SystemdCgroup          bool
 		InsecureRegistry       []string
+		CNIConfDir             string
 	}{
 		PodInfraContainerImage: pauseImage,
 		SystemdCgroup:          forceSystemd,
 		InsecureRegistry:       insecureRegistry,
+		CNIConfDir:             cni.ConfDir,
 	}
 	var b bytes.Buffer
 	if err := t.Execute(&b, opts); err != nil {

pkg/minikube/cruntime/crio.go

@@ -19,7 +19,6 @@ package cruntime
 import (
 	"encoding/json"
 	"fmt"
-	"net"
 	"os"
 	"os/exec"
 	"path"
@@ -31,6 +30,7 @@ import (
 	"k8s.io/klog/v2"
 	"k8s.io/minikube/pkg/minikube/assets"
 	"k8s.io/minikube/pkg/minikube/bootstrapper/images"
+	"k8s.io/minikube/pkg/minikube/cni"
 	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/download"
@@ -61,6 +61,14 @@ func generateCRIOConfig(cr CommandRunner, imageRepository string, kv semver.Vers
 	if _, err := cr.RunCmd(c); err != nil {
 		return errors.Wrap(err, "generateCRIOConfig.")
 	}
+
+	if cni.Network != "" {
+		klog.Infof("Updating CRIO to use the custom CNI network %q", cni.Network)
+		if _, err := cr.RunCmd(exec.Command("/bin/bash", "-c", fmt.Sprintf("sudo sed -e 's|^.*cni_default_network = .*$|cni_default_network = \"%s\"|' -i %s", cni.Network, crioConfigFile))); err != nil {
+			return errors.Wrap(err, "update network_dir")
+		}
+	}
+
 	return nil
 }
 
@@ -405,30 +413,3 @@ func crioImagesPreloaded(runner command.Runner, images []string) bool {
 func (r *CRIO) ImagesPreloaded(images []string) bool {
 	return crioImagesPreloaded(r.Runner, images)
 }
-
-// UpdateCRIONet updates CRIO CNI network configuration and restarts it
-func UpdateCRIONet(r CommandRunner, cidr string) error {
-	klog.Infof("Updating CRIO to use CIDR: %q", cidr)
-	ip, net, err := net.ParseCIDR(cidr)
-	if err != nil {
-		return errors.Wrap(err, "parse cidr")
-	}
-
-	oldNet := "10.88.0.0/16"
-	oldGw := "10.88.0.1"
-
-	newNet := cidr
-
-	// Assume gateway is first IP in netmask (10.244.0.1, for instance)
-	newGw := ip.Mask(net.Mask)
-	newGw[3]++
-
-	// Update subnets used by 100-crio-bridge.conf & 87-podman-bridge.conflist
-	// avoids: "Error adding network: failed to set bridge addr: could not add IP address to \"cni0\": permission denied"
-	sed := fmt.Sprintf("sed -i -e s#%s#%s# -e s#%s#%s# /etc/cni/net.d/*bridge*", oldNet, newNet, oldGw, newGw)
-	if _, err := r.RunCmd(exec.Command("sudo", "/bin/bash", "-c", sed)); err != nil {
-		klog.Errorf("netconf update failed: %v", err)
-	}
-
-	return sysinit.New(r).Restart("crio")
-}

pkg/minikube/node/start.go

@@ -188,7 +188,7 @@ func Start(starter Starter, apiServer bool) (*kubeconfig.Settings, error) {
 			return nil, errors.Wrap(err, "joining cp")
 		}
 
-		cnm, err := cni.New(*starter.Cfg)
+		cnm, err := cni.New(starter.Cfg)
 		if err != nil {
 			return nil, errors.Wrap(err, "cni")
 		}