From fc1a7fea03fbdf053f00ec5c4c018c0d106d4ce9 Mon Sep 17 00:00:00 2001 From: Steven Powell <44844360+spowelljr@users.noreply.github.com> Date: Tue, 18 Jul 2023 13:28:01 -0700 Subject: [PATCH] Revert "Always use cni unless running with dockershim" --- pkg/minikube/cni/cni.go | 27 +++++++------------ .../en/docs/drivers/includes/none_usage.inc | 1 - .../en/docs/drivers/includes/ssh_usage.inc | 1 - test/integration/net_test.go | 20 ++------------ 4 files changed, 12 insertions(+), 37 deletions(-) diff --git a/pkg/minikube/cni/cni.go b/pkg/minikube/cni/cni.go index 00e1c7174f..03c9a7988a 100644 --- a/pkg/minikube/cni/cni.go +++ b/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, err = chooseDefault(*cc) + cnm = chooseDefault(*cc) case "false": cnm = Disabled{cc: *cc} case "kindnet", "true": @@ -117,40 +117,33 @@ func IsDisabled(cc config.ClusterConfig) bool { return true } - def, err := chooseDefault(cc) - if err == nil && def.String() == "Disabled" { + if chooseDefault(cc).String() == "Disabled" { return true } return false } -func chooseDefault(cc config.ClusterConfig) (Manager, error) { +func chooseDefault(cc config.ClusterConfig) Manager { // For backwards compatibility with older profiles using --enable-default-cni if cc.KubernetesConfig.EnableDefaultCNI { klog.Infof("EnableDefaultCNI is true, recommending bridge") - return Bridge{}, nil + return Bridge{} } 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}, nil + return KindNet{cc: cc} } - 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 cc.KubernetesConfig.ContainerRuntime != constants.Docker { if driver.IsKIC(cc.Driver) { klog.Infof("%q driver + %q runtime found, recommending kindnet", cc.Driver, cc.KubernetesConfig.ContainerRuntime) - return KindNet{cc: cc}, nil + return KindNet{cc: cc} } klog.Infof("%q driver + %q runtime found, recommending bridge", cc.Driver, cc.KubernetesConfig.ContainerRuntime) - return Bridge{cc: cc}, nil + return Bridge{cc: cc} } // for docker container runtime and k8s v1.24+ where dockershim and kubenet were removed, we fallback to bridge cni for cri-docker(d) @@ -162,11 +155,11 @@ func chooseDefault(cc config.ClusterConfig) (Manager, error) { 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}, nil + return Bridge{cc: cc} } klog.Infof("CNI unnecessary in this configuration, recommending no CNI") - return Disabled{cc: cc}, nil + return Disabled{cc: cc} } // manifestPath returns the path to the CNI manifest diff --git a/site/content/en/docs/drivers/includes/none_usage.inc b/site/content/en/docs/drivers/includes/none_usage.inc index 05e7e595f5..452612e480 100644 --- a/site/content/en/docs/drivers/includes/none_usage.inc +++ b/site/content/en/docs/drivers/includes/none_usage.inc @@ -14,7 +14,6 @@ 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 --git a/site/content/en/docs/drivers/includes/ssh_usage.inc b/site/content/en/docs/drivers/includes/ssh_usage.inc index c256861515..5b2318aadd 100644 --- a/site/content/en/docs/drivers/includes/ssh_usage.inc +++ b/site/content/en/docs/drivers/includes/ssh_usage.inc @@ -13,7 +13,6 @@ 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 --git a/test/integration/net_test.go b/test/integration/net_test.go index 41b0284423..dae8e1d781 100644 --- a/test/integration/net_test.go +++ b/test/integration/net_test.go @@ -31,7 +31,6 @@ 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" @@ -58,8 +57,7 @@ func TestNetworkPlugins(t *testing.T) { namespace string hairpin bool }{ - // kindnet CNI is used by default and hairpin is enabled - {"auto", []string{}, "", "", "", usingCNI()}, + {"auto", []string{}, "", "", "", true}, {"kubenet", []string{"--network-plugin=kubenet"}, "kubenet", "", "", true}, {"bridge", []string{"--cni=bridge"}, "cni", "", "", true}, {"enable-default-cni", []string{"--enable-default-cni=true"}, "cni", "", "", true}, @@ -90,6 +88,7 @@ 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()) } @@ -213,21 +212,6 @@ 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