Merge pull request #16912 from spowelljr/fixCNIRegression

cni: Fix regression in auto selection

Author: spowelljr

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,34 @@ 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")) {
+	if cc.KubernetesConfig.ContainerRuntime != constants.Docker {
 		// 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}, 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 +156,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

pkg/minikube/cni/cni_test.go

@@ -0,0 +1,54 @@
+/*
+Copyright 2023 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package cni
+
+import (
+	"testing"
+
+	"k8s.io/minikube/pkg/minikube/config"
+)
+
+func TestChooseDefault(t *testing.T) {
+	tests := []struct {
+		driver           string
+		containerRuntime string
+		version          string
+		want             string
+	}{
+		{"docker", "docker", "v1.23.0", "Disabled"},
+		{"docker", "docker", "v1.27.0", "bridge CNI"},
+		{"docker", "containerd", "v1.23.0", "CNI"},
+		{"docker", "containerd", "v1.27.0", "CNI"},
+		{"qemu", "docker", "v1.23.0", "Disabled"},
+		{"qemu", "docker", "v1.27.0", "bridge CNI"},
+		{"qemu", "containerd", "v1.23.0", "bridge CNI"},
+		{"qemu", "containerd", "v1.27.0", "bridge CNI"},
+	}
+	for _, tc := range tests {
+		cc := config.ClusterConfig{
+			Driver: tc.driver,
+			KubernetesConfig: config.KubernetesConfig{
+				ContainerRuntime:  tc.containerRuntime,
+				KubernetesVersion: tc.version,
+			},
+		}
+		got := chooseDefault(cc).String()
+		if got != tc.want {
+			t.Errorf("chooseDefault(%+v) = %s; want = %s", cc, got, tc.want)
+		}
+	}
+}