From b3f0c02ac6bdc10a8c5ce461a670342faf9a9fb2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix=20Enrique=20Llorente=20Pastora?=
 <ellorent@redhat.com>
Date: Fri, 20 Sep 2019 10:48:29 +0200
Subject: [PATCH] Set vlan_filtering 1 at linux-bridge without ports (#172)

This PR add vlanfiltering_flag even if there is not port configured at the
linux bridge in the desiredState.

This is necessary in case ports are added outside of nmstate (linux bridge CNI) and vlan
communicatoin and filtering is needed

Signed-off-by: Quique Llorente <ellorent@redhat.com>
---
 pkg/helper/bridges.go              |  5 +----
 pkg/helper/bridges_test.go         |  4 ++--
 test/e2e/simple_bridge_and_bond.go | 28 ++++++++++++++++++++++++++++
 test/e2e/utils.go                  |  6 ++++++
 4 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/pkg/helper/bridges.go b/pkg/helper/bridges.go
index 8bf2a9d7d..8e70d02ee 100644
--- a/pkg/helper/bridges.go
+++ b/pkg/helper/bridges.go
@@ -29,10 +29,7 @@ func getBridgesUp(desiredState nmstatev1alpha1.State) (map[string][]string, erro
 			portList = append(portList, port.String())
 		}
 
-		// Add only the bridge we have some outbound port to configure
-		if len(portList) > 0 {
-			foundBridgesWithPorts[bridgeUp.Get("name").String()] = portList
-		}
+		foundBridgesWithPorts[bridgeUp.Get("name").String()] = portList
 	}
 
 	return foundBridgesWithPorts, nil
diff --git a/pkg/helper/bridges_test.go b/pkg/helper/bridges_test.go
index 0458e36b9..16dd74af0 100644
--- a/pkg/helper/bridges_test.go
+++ b/pkg/helper/bridges_test.go
@@ -116,9 +116,9 @@ var _ = Describe("Network desired state bridge parser", func() {
 		BeforeEach(func() {
 			desiredState = bridgeWithNoPorts
 		})
-		It("should return empty map", func() {
+		It("should return the bridge with empty port list", func() {
 			Expect(err).ToNot(HaveOccurred())
-			Expect(obtainedBridgesAndPorts).To(BeEmpty())
+			Expect(obtainedBridgesAndPorts).To(HaveKeyWithValue("br1", BeEmpty()))
 		})
 	})
 	Context("when there are bridges up", func() {
diff --git a/test/e2e/simple_bridge_and_bond.go b/test/e2e/simple_bridge_and_bond.go
index 9bc0dbc7d..4881c02e1 100644
--- a/test/e2e/simple_bridge_and_bond.go
+++ b/test/e2e/simple_bridge_and_bond.go
@@ -41,6 +41,17 @@ var _ = Describe("NodeNetworkState", func() {
         miimon: '120'
 `)
 
+		br1UpNoPorts = nmstatev1alpha1.State(`interfaces:
+  - name: br1
+    type: linux-bridge
+    state: up
+    bridge:
+      options:
+        stp:
+          enabled: false
+      port: []
+`)
+
 		br1Up = nmstatev1alpha1.State(`interfaces:
   - name: br1
     type: linux-bridge
@@ -94,6 +105,23 @@ var _ = Describe("NodeNetworkState", func() {
 `)
 	)
 	Context("when desiredState is configured", func() {
+		Context("with a linux bridge up with no ports", func() {
+			BeforeEach(func() {
+				updateDesiredState(br1UpNoPorts)
+			})
+			AfterEach(func() {
+				updateDesiredState(br1Absent)
+				for _, node := range nodes {
+					interfacesNameForNode(node).ShouldNot(ContainElement("br1"))
+				}
+			})
+			It("should have the linux bridge at currentState with vlan_filtering 1", func() {
+				for _, node := range nodes {
+					interfacesNameForNode(node).Should(ContainElement("br1"))
+					bridgeDescription(node, "br1").Should(ContainSubstring("vlan_filtering 1"))
+				}
+			})
+		})
 		Context("with a linux bridge up", func() {
 			BeforeEach(func() {
 				updateDesiredState(br1Up)
diff --git a/test/e2e/utils.go b/test/e2e/utils.go
index e9c01276e..cac80aa47 100644
--- a/test/e2e/utils.go
+++ b/test/e2e/utils.go
@@ -368,3 +368,9 @@ func vlansCardinality(node string, connection string) AsyncAssertion {
 	}, ReadTimeout, ReadInterval)
 
 }
+
+func bridgeDescription(node string, bridgeName string) AsyncAssertion {
+	return Eventually(func() (string, error) {
+		return run(node, "sudo", "ip", "-d", "link", "show", "type", "bridge", bridgeName)
+	}, ReadTimeout, ReadTimeout)
+}