Address comments

* use "cni.exclusive=false" by default * only check if the annotation is set, do not expect an actual boolean value * s/AnnotationCniExclusive/AnnotationCNIExclusive
canonical · Nov 26, 2024 · 7449524 · 7449524
1 parent c3c6449
commit 7449524
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 23 deletions.
diff --git a/src/k8s/pkg/k8sd/features/cilium/internal.go b/src/k8s/pkg/k8sd/features/cilium/internal.go
@@ -72,9 +72,7 @@ func internalConfig(annotations types.Annotations) (config, error) {
 		c.vlanBPFBypass = vlanTags
 	}
 
-	if v, ok := annotations.Get(apiv1_annotations.AnnotationCniExclusive); ok {
-		c.cniExclusive = v == "true"
-	} else {
+	if _, ok := annotations.Get(apiv1_annotations.AnnotationCNIExclusive); ok {
 		c.cniExclusive = true
 	}
 

diff --git a/src/k8s/pkg/k8sd/features/cilium/internal_test.go b/src/k8s/pkg/k8sd/features/cilium/internal_test.go
@@ -21,7 +21,7 @@ func TestInternalConfig(t *testing.T) {
 				devices:             "",
 				directRoutingDevice: "",
 				vlanBPFBypass:       nil,
-				cniExclusive:        true,
+				cniExclusive:        false,
 			},
 			expectError: false,
 		},
@@ -31,20 +31,19 @@ func TestInternalConfig(t *testing.T) {
 				apiv1_annotations.AnnotationDevices:             "eth+ lxdbr+",
 				apiv1_annotations.AnnotationDirectRoutingDevice: "eth0",
 				apiv1_annotations.AnnotationVLANBPFBypass:       "1,2,3",
-				apiv1_annotations.AnnotationCniExclusive:        "false",
+				apiv1_annotations.AnnotationCNIExclusive:        "false",
 			},
 			expectedConfig: config{
 				devices:             "eth+ lxdbr+",
 				directRoutingDevice: "eth0",
 				vlanBPFBypass:       []int{1, 2, 3},
-				cniExclusive:        false,
 			},
 			expectError: false,
 		},
 		{
 			name: "Cilum exclusive CNI",
 			annotations: map[string]string{
-				apiv1_annotations.AnnotationCniExclusive: "true",
+				apiv1_annotations.AnnotationCNIExclusive: "true",
 			},
 			expectedConfig: config{
 				devices:             "",
@@ -61,7 +60,6 @@ func TestInternalConfig(t *testing.T) {
 			},
 			expectedConfig: config{
 				vlanBPFBypass: []int{1},
-				cniExclusive:  true,
 			},
 			expectError: false,
 		},
@@ -72,7 +70,6 @@ func TestInternalConfig(t *testing.T) {
 			},
 			expectedConfig: config{
 				vlanBPFBypass: []int{1, 2, 3, 4, 5},
-				cniExclusive:  true,
 			},
 			expectError: false,
 		},
@@ -83,7 +80,6 @@ func TestInternalConfig(t *testing.T) {
 			},
 			expectedConfig: config{
 				vlanBPFBypass: []int{0},
-				cniExclusive:  true,
 			},
 			expectError: false,
 		},
@@ -115,7 +111,6 @@ func TestInternalConfig(t *testing.T) {
 			},
 			expectedConfig: config{
 				vlanBPFBypass: []int{1, 2, 3},
-				cniExclusive:  true,
 			},
 			expectError: false,
 		},
@@ -126,7 +121,6 @@ func TestInternalConfig(t *testing.T) {
 			},
 			expectedConfig: config{
 				vlanBPFBypass: []int{1, 2, 3, 4, 5},
-				cniExclusive:  true,
 			},
 			expectError: false,
 		},
@@ -140,7 +134,7 @@ func TestInternalConfig(t *testing.T) {
 		{
 			name:           "Nil annotations",
 			annotations:    nil,
-			expectedConfig: config{cniExclusive: true},
+			expectedConfig: config{},
 			expectError:    false,
 		},
 		{
@@ -150,7 +144,6 @@ func TestInternalConfig(t *testing.T) {
 			},
 			expectedConfig: config{
 				vlanBPFBypass: []int{1, 2, 3},
-				cniExclusive:  true,
 			},
 			expectError: false,
 		},

diff --git a/src/k8s/pkg/k8sd/features/cilium/network_test.go b/src/k8s/pkg/k8sd/features/cilium/network_test.go
@@ -183,7 +183,7 @@ func TestNetworkEnabled(t *testing.T) {
 		validateNetworkValues(g, callArgs.Values, network, snapM)
 	})
 
-	t.Run("cniExclusiveDisabled", func(t *testing.T) {
+	t.Run("CNIExclusive", func(t *testing.T) {
 		g := NewWithT(t)
 
 		helmM := &helmmock.Mock{}
@@ -203,7 +203,7 @@ func TestNetworkEnabled(t *testing.T) {
 		testAnnotations := types.Annotations{
 			apiv1_annotations.AnnotationDevices:             "eth+ lxdbr+",
 			apiv1_annotations.AnnotationDirectRoutingDevice: "eth0",
-			apiv1_annotations.AnnotationCniExclusive:        "false",
+			apiv1_annotations.AnnotationCNIExclusive:        "true",
 		}
 		status, err := ApplyNetwork(context.Background(), snapM, "127.0.0.1", apiserver, network, testAnnotations)
 
@@ -218,7 +218,7 @@ func TestNetworkEnabled(t *testing.T) {
 		g.Expect(callArgs.State).To(Equal(helm.StatePresent))
 
 		cniValues := callArgs.Values["cni"].(map[string]interface{})
-		g.Expect(cniValues["exclusive"]).To(BeFalse())
+		g.Expect(cniValues["exclusive"]).To(BeTrue())
 	})
 }
 
@@ -435,11 +435,7 @@ func validateNetworkValues(g Gomega, values map[string]any, network types.Networ
 		g.Expect(values["nodePort"].(map[string]any)["directRoutingDevice"]).To(Equal(directRoutingDevice))
 	}
 
-	cniExclusiveStr, exists := annotations.Get(apiv1_annotations.AnnotationCniExclusive)
+	_, exists = annotations.Get(apiv1_annotations.AnnotationCNIExclusive)
 	cniValues := values["cni"].(map[string]interface{})
-	if exists {
-		g.Expect(cniValues["exclusive"]).To(Equal(cniExclusiveStr == "true"))
-	} else {
-		g.Expect(cniValues["exclusive"]).To(BeTrue())
-	}
+	g.Expect(cniValues["exclusive"]).To(Equal(exists))
 }