diff --git a/pkg/helper/bridges.go b/pkg/helper/bridges.go index 5a7fc9cf0..3128e8ac5 100644 --- a/pkg/helper/bridges.go +++ b/pkg/helper/bridges.go @@ -11,7 +11,6 @@ import ( yaml "sigs.k8s.io/yaml" - "github.com/nmstate/kubernetes-nmstate/api/shared" nmstate "github.com/nmstate/kubernetes-nmstate/api/shared" "github.com/nmstate/kubernetes-nmstate/pkg/nmstatectl" ) @@ -57,16 +56,21 @@ func ApplyDefaultVlanFiltering(desiredState nmstate.State) (nmstate.State, error return nmstate.State{Raw: resultYaml}, nil } -func EnableVlanFiltering() (string, error) { +func EnableVlanFiltering(desiredState nmstate.State) (string, error) { currentState, err := nmstatectl.Show() if err != nil { return "failed to get currentState", err } - upBridgesWithPorts, err := GetUpLinuxBridgesWithPorts(shared.NewState(currentState)) + upBridgesWithPortsAtCurrentState, err := GetUpLinuxBridgesWithPorts(nmstate.NewState(currentState)) if err != nil { return "failed to list bridges with ports", err } - out, err := enableVlanFiltering(upBridgesWithPorts) + filteredExistingUpBridgesWithPortsAtDesiredState, err := filterExistingLinuxBridgesWithPorts(upBridgesWithPortsAtCurrentState, desiredState) + if err != nil { + return "failed to filter existing bridges with ports from desiredState", err + } + + out, err := enableVlanFiltering(filteredExistingUpBridgesWithPortsAtDesiredState) if err != nil { return fmt.Sprintf("failed to enable vlan-filtering via nmcli: %s", out), err } @@ -97,6 +101,22 @@ func GetUpLinuxBridgesWithPorts(desiredState nmstate.State) (map[string][]string return bridgesWithPorts, nil } +func filterExistingLinuxBridgesWithPorts(bridgesAtCurrentState map[string][]string, desiredState nmstate.State) (map[string][]string, error) { + filteredBridgesWithPorts := map[string][]string{} + bridgesAtDesiredState, err := GetUpLinuxBridgesWithPorts(desiredState) + if err != nil { + return nil, err + } + + for bridge, ports := range bridgesAtDesiredState { + if currentBridgePorts, ok := bridgesAtCurrentState[bridge]; ok && len(currentBridgePorts) > 0 { + filteredBridgesWithPorts[bridge] = intersectSlices(ports, currentBridgePorts) + } + } + + return filteredBridgesWithPorts, nil +} + func enableVlanFiltering(upBridgesWithPorts map[string][]string) (string, error) { for bridge, ports := range upBridgesWithPorts { out, err := enableBridgeVlanFiltering(bridge) @@ -136,6 +156,23 @@ func runCommand(command string, args []string) (string, error) { return stdout.String(), nil } +func intersectSlices(s1, s2 []string) []string { + cache := map[string]bool{} + result := []string{} + + for _, e := range s1 { + cache[e] = false + } + + for _, e := range s2 { + if cached, ok := cache[e]; ok && !cached { + result = append(result, e) + cache[e] = true + } + } + return result +} + func isLinuxBridgeUp(iface gjson.Result) bool { return iface.Get("type").String() == "linux-bridge" && iface.Get("state").String() == "up" } diff --git a/pkg/helper/bridges_test.go b/pkg/helper/bridges_test.go index 2fd7c0303..53fb3351e 100644 --- a/pkg/helper/bridges_test.go +++ b/pkg/helper/bridges_test.go @@ -2,6 +2,7 @@ package helper import ( . "github.com/onsi/ginkgo" + "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" nmstate "github.com/nmstate/kubernetes-nmstate/api/shared" @@ -342,3 +343,178 @@ var _ = Describe("test listing linux bridges with ports", func() { )) }) }) + +var _ = Describe("test listing linux bridges with ports", func() { + currentState := nmstate.NewState(`interfaces: + - name: br22 + type: linux-bridge + state: up + bridge: + port: + - name: test-veth1 + vlan: + enable-native: true + - name: br3 + type: linux-bridge + state: up + bridge: + port: + - name: eth2 + - name: eth3 + - name: eth4 + - name: eth5 + - name: br4 + type: linux-bridge + state: up + bridge: + port: + - name: eth4 + - name: br5 + type: linux-bridge + state: up + bridge: + port: + - name: eth5 + - name: br7 + type: linux-bridge + state: up + bridge: + port: + - name: eth777 + - name: br8 + type: linux-bridge + state: up + bridge: + port: + - name: eth888 +`) + desiredState := nmstate.NewState(`interfaces: +- name: br3 + type: linux-bridge + state: up + bridge: + port: + - name: eth2 + - name: eth3 +- name: br4 + type: linux-bridge + state: up + bridge: + port: + - name: eth4 +- name: br5 + type: linux-bridge + state: up + bridge: + port: [] +- name: br6 + type: linux-bridge + state: down + bridge: + port: + - name: eth666 +- name: br7 + type: linux-bridge + state: up + bridge: + port: + - name: eth777 + - name: eth778 + - name: eth779 +- name: br8 + type: linux-bridge + state: absent + bridge: + port: + - name: eth888 +`) + expectedFilteredExistingBridgesWithPorts := map[string][]string{ + "br3": {"eth2", "eth3"}, + "br4": {"eth4"}, + "br7": {"eth777"}, + } + It("should filter correct bridges and ports", func() { + upBridgesWithPortsAtCurrentState, err := GetUpLinuxBridgesWithPorts(currentState) + Expect(err).ShouldNot(HaveOccurred()) + + filteredExistingUpBridgesWithPorts, err := filterExistingLinuxBridgesWithPorts(upBridgesWithPortsAtCurrentState, desiredState) + Expect(err).ShouldNot(HaveOccurred()) + Expect(filteredExistingUpBridgesWithPorts).To(Equal(expectedFilteredExistingBridgesWithPorts)) + }) +}) + +var _ = Describe("testing slice intersection", func() { + type intersectionCase struct { + s1 []string + s2 []string + expectedIntersection []string + } + + table.DescribeTable("Slice intersection cases", + func(c intersectionCase) { + result := intersectSlices(c.s1, c.s2) + Expect(result).To(Equal(c.expectedIntersection)) + }, + table.Entry( + "Both slices empty", + intersectionCase{ + s1: []string{}, + s2: []string{}, + expectedIntersection: []string{}, + }), + table.Entry("Empty first slice", + intersectionCase{ + s1: []string{}, + s2: []string{"foo"}, + expectedIntersection: []string{}, + }), + table.Entry("Empty second slice", + intersectionCase{ + s1: []string{"foo"}, + s2: []string{}, + expectedIntersection: []string{}, + }), + table.Entry("No common elements", + intersectionCase{ + s1: []string{"foo"}, + s2: []string{"bar"}, + expectedIntersection: []string{}, + }), + table.Entry("One common element with extra in first slice", + intersectionCase{ + s1: []string{"foo", "bar"}, + s2: []string{"bar"}, + expectedIntersection: []string{"bar"}, + }), + table.Entry("One common element with extra in first slice", + intersectionCase{ + s1: []string{"bar"}, + s2: []string{"bar", "foo"}, + expectedIntersection: []string{"bar"}, + }), + table.Entry("One common element with extra in first slice", + intersectionCase{ + s1: []string{"bar"}, + s2: []string{"bar", "foo"}, + expectedIntersection: []string{"bar"}, + }), + table.Entry("Both identical with two elements", + intersectionCase{ + s1: []string{"foo", "bar"}, + s2: []string{"bar", "foo"}, + expectedIntersection: []string{"bar", "foo"}, + }), + table.Entry("Duplicates in first slice", + intersectionCase{ + s1: []string{"foo", "bar", "one", "two", "three", "one", "two", "three"}, + s2: []string{"bar", "foo", "three", "one"}, + expectedIntersection: []string{"bar", "foo", "three", "one"}, + }), + table.Entry("Duplicates in second slice", + intersectionCase{ + s1: []string{"bar", "foo", "three", "one"}, + s2: []string{"foo", "bar", "one", "two", "three", "one", "two", "three"}, + expectedIntersection: []string{"foo", "bar", "one", "three"}, + }), + ) +}) diff --git a/pkg/helper/client.go b/pkg/helper/client.go index 6cd31780d..78a07d476 100644 --- a/pkg/helper/client.go +++ b/pkg/helper/client.go @@ -129,7 +129,7 @@ func ApplyDesiredState(client client.Client, desiredState shared.State) (string, return "Ignoring empty desired state", nil } - out, err := EnableVlanFiltering() + out, err := EnableVlanFiltering(desiredState) if err != nil { return out, fmt.Errorf("failed to enable vlan filtering via nmcli: %s", err.Error()) }