Delete controller for standalone OVS bridges

The patch adds an OVS extra parameter to delete controller for bridges
configured with standalone fail mode. By default, bridges are created
without having an openflow controllers. If node is restarted, the bridge
is set to standalone mode but if a service managing the bridge sets a
controller, it will remain in the ovsdb.

As ovs-vswitchd sets the bridge behavior to normal MAC learning switch
only if bridge in standalone mode can't communicate with its controller,
leaving controller defined can cause node outage when bridge is used as
management network. In such case controller service, like
neutron-openvswitch-agent, would need to communicate over management
network but given that bridge is in standalone mode but communicates
with controller, management network won't be reachable. This creates a
chicken-egg problem.

By removing controller by default, ovs-vswitchd implements a normal
action rule to the standalone bridge and service can use the bridge as
management network and eventually set the brdige to secure and set the
flows manually.

See opened Bugzilla for more information:
https://bugzilla.redhat.com/show_bug.cgi?id=1473763

Closes-bug: #1712517

Change-Id: Iad48312667834ea8f5c7145595ae89cb5159b36d
(cherry picked from commit f8d76d2cdebfa0d06233a59a8f6539207c5b5a4e)

4 files changed, 28 insertions, 16 deletions

diff --git a/os_net_config/objects.py b/os_net_config/objects.py
index a6d0c03..417f34a 100644
--- a/os_net_config/objects.py
+++ b/os_net_config/objects.py
@@ -30,7 +30,8 @@ logger = logging.getLogger(__name__)
 
 _MAPPED_NICS = None
 
-DEFAULT_OVS_BRIDGE_FAIL_MODE = 'standalone'
+STANDALONE_FAIL_MODE = 'standalone'
+DEFAULT_OVS_BRIDGE_FAIL_MODE = STANDALONE_FAIL_MODE
 
 
 class InvalidConfigException(ValueError):
@@ -184,6 +185,13 @@ def format_ovs_extra(obj, templates):
     return [t.format(name=obj.name) for t in templates or []]
 
 
+def _add_fail_mode(fail_mode):
+    ovs_extra = ['set bridge {name} fail_mode=%s' % fail_mode]
+    if fail_mode == STANDALONE_FAIL_MODE:
+        ovs_extra.append('del-controller {name}')
+    return ovs_extra
+
+
 class Route(object):
     """Base class for network routes."""
 
@@ -469,7 +477,7 @@ class OvsBridge(_BaseOpts):
         self.ovs_options = ovs_options
         ovs_extra = ovs_extra or []
         if fail_mode:
-            ovs_extra.append('set bridge {name} fail_mode=%s' % fail_mode)
+            ovs_extra.extend(_add_fail_mode(fail_mode))
         self.ovs_extra = format_ovs_extra(self, ovs_extra)
         for member in self.members:
             member.bridge_name = name
@@ -526,7 +534,7 @@ class OvsUserBridge(_BaseOpts):
         self.ovs_options = ovs_options
         ovs_extra = ovs_extra or []
         if fail_mode:
-            ovs_extra.append('set bridge {name} fail_mode=%s' % fail_mode)
+            ovs_extra.extend(_add_fail_mode(fail_mode))
         self.ovs_extra = format_ovs_extra(self, ovs_extra)
         for member in self.members:
             member.bridge_name = name
diff --git a/os_net_config/tests/test_impl_eni.py b/os_net_config/tests/test_impl_eni.py
index 51354f8..902cc02 100644
--- a/os_net_config/tests/test_impl_eni.py
+++ b/os_net_config/tests/test_impl_eni.py
@@ -73,8 +73,9 @@ iface br0 inet dhcp
     pre-up ip addr flush dev eth0
 """
 
-_OVS_BRIDGE_DHCP_STANDALONE = _OVS_BRIDGE_DHCP + \
-    "    ovs_extra set bridge br0 fail_mode=standalone\n"
+_OVS_BRIDGE_DHCP_STANDALONE = _OVS_BRIDGE_DHCP + (
+    "    ovs_extra set bridge br0 fail_mode=standalone "
+    "-- del-controller br0\n")
 
 _OVS_BRIDGE_DHCP_SECURE = _OVS_BRIDGE_DHCP + \
     "    ovs_extra set bridge br0 fail_mode=secure\n"
diff --git a/os_net_config/tests/test_impl_ifcfg.py b/os_net_config/tests/test_impl_ifcfg.py
index d41cf60..337eeb3 100644
--- a/os_net_config/tests/test_impl_ifcfg.py
+++ b/os_net_config/tests/test_impl_ifcfg.py
@@ -175,8 +175,9 @@ NM_CONTROLLED=yes
 PEERDNS=no
 """
 
-_OVS_BRIDGE_DHCP_STANDALONE = _OVS_BRIDGE_DHCP + \
-    "OVS_EXTRA=\"set bridge br-ctlplane fail_mode=standalone\"\n"
+_OVS_BRIDGE_DHCP_STANDALONE = _OVS_BRIDGE_DHCP + (
+    "OVS_EXTRA=\"set bridge br-ctlplane fail_mode=standalone "
+    "-- del-controller br-ctlplane\"\n")
 
 _OVS_BRIDGE_DHCP_SECURE = _OVS_BRIDGE_DHCP + \
     "OVS_EXTRA=\"set bridge br-ctlplane fail_mode=secure\"\n"
diff --git a/os_net_config/tests/test_objects.py b/os_net_config/tests/test_objects.py
index eb5adaf..1c07eee 100644
--- a/os_net_config/tests/test_objects.py
+++ b/os_net_config/tests/test_objects.py
@@ -344,10 +344,11 @@ class TestBridge(base.TestCase):
 }
 """
         bridge = objects.object_from_json(json.loads(data))
-        self.assertTrue(2 == len(bridge.ovs_extra))
-        self.assertEqual("bar", bridge.ovs_extra[0])
-        self.assertEqual("set bridge br-foo fail_mode=standalone",
-                         bridge.ovs_extra[1])
+        self.assertTrue(3 == len(bridge.ovs_extra))
+        self.assertItemsEqual(["bar",
+                               "set bridge br-foo fail_mode=standalone",
+                               "del-controller br-foo"],
+                              bridge.ovs_extra)
 
     def test_from_json_ovs_extra_string(self):
         data = """{
@@ -358,10 +359,10 @@ class TestBridge(base.TestCase):
 }
 """
         bridge = objects.object_from_json(json.loads(data))
-        self.assertTrue(2 == len(bridge.ovs_extra))
-        self.assertEqual("bar", bridge.ovs_extra[0])
-        self.assertEqual("set bridge br-foo fail_mode=standalone",
-                         bridge.ovs_extra[1])
+        self.assertItemsEqual(["bar",
+                               "set bridge br-foo fail_mode=standalone",
+                               "del-controller br-foo"],
+                              bridge.ovs_extra)
 
 
 class TestLinuxBridge(base.TestCase):
@@ -770,7 +771,8 @@ class TestOvsTunnel(base.TestCase):
         bridge = objects.object_from_json(json.loads(data))
         self.assertEqual("br-foo", bridge.name)
         self.assertEqual(["set bridge br-foo something",
-                          "set bridge br-foo fail_mode=standalone"],
+                          "set bridge br-foo fail_mode=standalone",
+                          "del-controller br-foo"],
                          bridge.ovs_extra)
         tun0 = bridge.members[0]
         self.assertEqual("tun0", tun0.name)