Merge pull request #14780 from afbjorklund/docker-cni

Always use cni unless running with dockershim

Author: spowelljr

Diff for: pkg/minikube/cni/cni.go

@@ -87,7 +87,7 @@ func New(cc *config.ClusterConfig) (Manager, error) {
 	var err error
 	switch cc.KubernetesConfig.CNI {
 	case "", "auto":
-		cnm = chooseDefault(*cc)
+		cnm, err = chooseDefault(*cc)
 	case "false":
 		cnm = Disabled{cc: *cc}
 	case "kindnet", "true":
@@ -117,33 +117,40 @@ func IsDisabled(cc config.ClusterConfig) bool {
 		return true
 	}
 
-	if chooseDefault(cc).String() == "Disabled" {
+	def, err := chooseDefault(cc)
+	if err == nil && def.String() == "Disabled" {
 		return true
 	}
 	return false
 }
 
-func chooseDefault(cc config.ClusterConfig) Manager {
+func chooseDefault(cc config.ClusterConfig) (Manager, error) {
 	// For backwards compatibility with older profiles using --enable-default-cni
 	if cc.KubernetesConfig.EnableDefaultCNI {
 		klog.Infof("EnableDefaultCNI is true, recommending bridge")
-		return Bridge{}
+		return Bridge{}, nil
 	}
 
 	if len(cc.Nodes) > 1 || cc.MultiNodeRequested {
 		// Enables KindNet CNI in master in multi node cluster, This solves the network problem
 		// inside pod for multi node clusters. See https://github.com/kubernetes/minikube/issues/9838.
 		klog.Infof("%d nodes found, recommending kindnet", len(cc.Nodes))
-		return KindNet{cc: cc}
+		return KindNet{cc: cc}, nil
 	}
 
-	if cc.KubernetesConfig.ContainerRuntime != constants.Docker {
+	version, err := util.ParseKubernetesVersion(cc.KubernetesConfig.KubernetesVersion)
+	if err != nil {
+		return nil, err
+	}
+
+	if cc.KubernetesConfig.ContainerRuntime != constants.Docker || version.GTE(semver.MustParse("1.24.0-alpha.2")) {
+		// Always use CNI when running with CRI (without dockershim)
 		if driver.IsKIC(cc.Driver) {
 			klog.Infof("%q driver + %q runtime found, recommending kindnet", cc.Driver, cc.KubernetesConfig.ContainerRuntime)
-			return KindNet{cc: cc}
+			return KindNet{cc: cc}, nil
 		}
 		klog.Infof("%q driver + %q runtime found, recommending bridge", cc.Driver, cc.KubernetesConfig.ContainerRuntime)
-		return Bridge{cc: cc}
+		return Bridge{cc: cc}, nil
 	}
 
 	// for docker container runtime and k8s v1.24+ where dockershim and kubenet were removed, we fallback to bridge cni for cri-docker(d)
@@ -155,11 +162,11 @@ func chooseDefault(cc config.ClusterConfig) Manager {
 	kv, err := util.ParseKubernetesVersion(cc.KubernetesConfig.KubernetesVersion)
 	if err == nil && kv.GTE(semver.MustParse("1.24.0-alpha.2")) {
 		klog.Infof("%q driver + %q container runtime found on kubernetes v1.24+, recommending bridge", cc.Driver, cc.KubernetesConfig.ContainerRuntime)
-		return Bridge{cc: cc}
+		return Bridge{cc: cc}, nil
 	}
 
 	klog.Infof("CNI unnecessary in this configuration, recommending no CNI")
-	return Disabled{cc: cc}
+	return Disabled{cc: cc}, nil
 }
 
 // manifestPath returns the path to the CNI manifest

Diff for: site/content/en/docs/drivers/includes/none_usage.inc

@@ -14,6 +14,7 @@ This VM must also meet the [kubeadm requirements](https://kubernetes.io/docs/set
 * iptables (in legacy mode)
 * conntrack
 * crictl
+* cni-plugins
 * SELinux permissive
 * cgroups v1 (v2 is not yet supported by Kubernetes)
 

Diff for: site/content/en/docs/drivers/includes/ssh_usage.inc

@@ -13,6 +13,7 @@ This VM must also meet the [kubeadm requirements](https://kubernetes.io/docs/set
 * iptables (in legacy mode)
 * conntrack
 * crictl
+* cni-plugins
 * SELinux permissive
 * cgroups v1 (v2 is not yet supported by Kubernetes)
 

Diff for: test/integration/net_test.go

@@ -31,6 +31,7 @@ import (
 	"github.com/blang/semver/v4"
 	"k8s.io/minikube/pkg/kapi"
 	"k8s.io/minikube/pkg/minikube/config"
+	"k8s.io/minikube/pkg/minikube/constants"
 	"k8s.io/minikube/pkg/minikube/reason"
 	"k8s.io/minikube/pkg/util"
 	"k8s.io/minikube/pkg/util/retry"
@@ -57,7 +58,8 @@ func TestNetworkPlugins(t *testing.T) {
 			namespace     string
 			hairpin       bool
 		}{
-			{"auto", []string{}, "", "", "", true},
+			// kindnet CNI is used by default and hairpin is enabled
+			{"auto", []string{}, "", "", "", usingCNI()},
 			{"kubenet", []string{"--network-plugin=kubenet"}, "kubenet", "", "", true},
 			{"bridge", []string{"--cni=bridge"}, "cni", "", "", true},
 			{"enable-default-cni", []string{"--enable-default-cni=true"}, "cni", "", "", true},
@@ -88,7 +90,6 @@ func TestNetworkPlugins(t *testing.T) {
 
 				if ContainerRuntime() != "docker" && tc.name == "kubenet" {
 					// CNI is disabled when --network-plugin=kubenet option is passed. See cni.New(..) function
-					// But for containerd/crio CNI has to be configured
 					t.Skipf("Skipping the test as %s container runtimes requires CNI", ContainerRuntime())
 				}
 
@@ -212,6 +213,21 @@ func TestNetworkPlugins(t *testing.T) {
 	})
 }
 
+// usingCNI checks if not using dockershim
+func usingCNI() bool {
+	if ContainerRuntime() != "docker" {
+		return true
+	}
+	version, err := util.ParseKubernetesVersion(constants.DefaultKubernetesVersion)
+	if err != nil {
+		return false
+	}
+	if version.GTE(semver.MustParse("1.24.0-alpha.2")) {
+		return true
+	}
+	return false
+}
+
 // validateFalseCNI checks that minikube returns and error
 // if container runtime is "containerd" or "crio"
 // and --cni=false