From d85c3281a1d966efe6a37f2b70a300b8eb749697 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Tue, 26 Nov 2024 08:15:51 +0000 Subject: [PATCH] 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 --- src/k8s/pkg/k8sd/features/cilium/internal.go | 4 +--- src/k8s/pkg/k8sd/features/cilium/internal_test.go | 15 ++++----------- src/k8s/pkg/k8sd/features/cilium/network_test.go | 14 +++++--------- 3 files changed, 10 insertions(+), 23 deletions(-) diff --git a/src/k8s/pkg/k8sd/features/cilium/internal.go b/src/k8s/pkg/k8sd/features/cilium/internal.go index d55fd6195..7391e4288 100644 --- 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 index ad5ce7f06..197ecb106 100644 --- 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 index 86ff7d972..70c015edc 100644 --- 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)) }