From d478a0554fa7377ac77e967326899d0c58aa8208 Mon Sep 17 00:00:00 2001 From: Radim Hrazdil <32546791+rhrazdil@users.noreply.github.com> Date: Tue, 23 Nov 2021 12:07:52 +0100 Subject: [PATCH] Fix issue with missing vlans after upgrade (#891) When upgrading from an older knmstate version that uses vlan-filtering script to a version relying on nmstate API, nmstate fails on verification when reconciling bridge with ports configured by the older knmstate version. This change applies the vlans using nmcli, so that there is no discrepancy between kernel and NM configuration. Signed-off-by: Radim Hrazdil --- pkg/helper/bridges.go | 84 ++++++++++++++++++++++++++++++++++++++ pkg/helper/bridges_test.go | 56 +++++++++++++++++++++++++ pkg/helper/client.go | 5 +++ 3 files changed, 145 insertions(+) diff --git a/pkg/helper/bridges.go b/pkg/helper/bridges.go index c63c872e6..5a7fc9cf0 100644 --- a/pkg/helper/bridges.go +++ b/pkg/helper/bridges.go @@ -1,14 +1,19 @@ package helper import ( + "bytes" "fmt" + "os/exec" + "strings" "github.com/tidwall/gjson" "github.com/tidwall/sjson" 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" ) var defaultVlanFiltering = map[string]interface{}{ @@ -52,6 +57,85 @@ func ApplyDefaultVlanFiltering(desiredState nmstate.State) (nmstate.State, error return nmstate.State{Raw: resultYaml}, nil } +func EnableVlanFiltering() (string, error) { + currentState, err := nmstatectl.Show() + if err != nil { + return "failed to get currentState", err + } + upBridgesWithPorts, err := GetUpLinuxBridgesWithPorts(shared.NewState(currentState)) + if err != nil { + return "failed to list bridges with ports", err + } + out, err := enableVlanFiltering(upBridgesWithPorts) + if err != nil { + return fmt.Sprintf("failed to enable vlan-filtering via nmcli: %s", out), err + } + return "", nil +} + +func GetUpLinuxBridgesWithPorts(desiredState nmstate.State) (map[string][]string, error) { + bridgesWithPorts := map[string][]string{} + + result, err := yaml.YAMLToJSON(desiredState.Raw) + if err != nil { + return bridgesWithPorts, fmt.Errorf("error converting desiredState to JSON: %v", err) + } + + ifaces := gjson.ParseBytes(result).Get("interfaces").Array() + for _, iface := range ifaces { + if !isLinuxBridgeUp(iface) { + continue + } + for _, port := range iface.Get("bridge.port").Array() { + if hasVlanConfiguration(port) { + continue + } + bridgeName := iface.Get("name").String() + bridgesWithPorts[bridgeName] = append(bridgesWithPorts[bridgeName], port.Get("name").String()) + } + } + return bridgesWithPorts, nil +} + +func enableVlanFiltering(upBridgesWithPorts map[string][]string) (string, error) { + for bridge, ports := range upBridgesWithPorts { + out, err := enableBridgeVlanFiltering(bridge) + if err != nil { + return out, err + } + for _, port := range ports { + out, err = enableBridgPortVlans(port) + if err != nil { + return out, err + } + } + } + return "", nil +} + +func enableBridgeVlanFiltering(bridgeName string) (string, error) { + command := "nmcli" + args := []string{"con", "mod", bridgeName, "bridge.vlan-filtering", "yes"} + return runCommand(command, args) +} + +func enableBridgPortVlans(port string) (string, error) { + command := "nmcli" + args := []string{"con", "mod", port, "bridge-port.vlans", "2-4094"} + return runCommand(command, args) +} + +func runCommand(command string, args []string) (string, error) { + cmd := exec.Command(command, args...) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return "", fmt.Errorf("failed to execute %s %s: '%v', '%s', '%s'", command, strings.Join(args, " "), err, stdout.String(), stderr.String()) + } + return stdout.String(), nil +} + 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 052851909..2fd7c0303 100644 --- a/pkg/helper/bridges_test.go +++ b/pkg/helper/bridges_test.go @@ -286,3 +286,59 @@ var _ = Describe("Network desired state bridge parser", 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: 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: absent + bridge: + port: + - name: eth777 +`) + It("should list active bridges with at least one port", func() { + upBridgesWithPorts, err := GetUpLinuxBridgesWithPorts(currentState) + Expect(err).ShouldNot(HaveOccurred()) + Expect(upBridgesWithPorts).To( + SatisfyAll( + HaveKeyWithValue("br3", []string{"eth2", "eth3"}), + HaveKeyWithValue("br4", []string{"eth4"}), + Not(HaveKey("br22")), + Not(HaveKey("br5")), + Not(HaveKey("br6")), + Not(HaveKey("br7")), + )) + }) +}) diff --git a/pkg/helper/client.go b/pkg/helper/client.go index d7882cb84..6cd31780d 100644 --- a/pkg/helper/client.go +++ b/pkg/helper/client.go @@ -129,6 +129,11 @@ func ApplyDesiredState(client client.Client, desiredState shared.State) (string, return "Ignoring empty desired state", nil } + out, err := EnableVlanFiltering() + if err != nil { + return out, fmt.Errorf("failed to enable vlan filtering via nmcli: %s", err.Error()) + } + // Before apply we get the probes that are working fine, they should be // working fine after apply probes := probe.Select(client)