OVS: merge the required OpenFlow version rather than replace

This change modifies the behavior of OVS native and ovs-ofctl bridge
implementations so that instead of configuring the bridge only for the
required OVS protocol version, they add the required version to the
already configured versions.

To achieve this, an add_protocols method is added to the OVSBridge
class, relying on the OVSDB add_db_attribute added in
Ib6ce75846f9b13c1c33f0ced5ccc619ee7860dc1, with the behavior of
making the provided set of versions supported in addition to already
configured ones.

It is aimed to be a cleaner solution to bug 1622644 than the quickfix merge
from I4475865c4f83cb9f3e12c709af752bc490692ca3 .

After this change, the set_protocols method appears useless and is
hence marked for future removal.

Depends-On: I4386aa293f9b18d2e17b4a80d9c7da4b9b46f3c9
Change-Id: Id5ac7e6431c97fc70d8404b16f89533b6f270eee
Related-Bug: 1622644

neutron/agent/common/ovs_lib.py

@@ -19,6 +19,7 @@ import operator
 import time
 import uuid
 
+from debtcollector import removals
 from neutron_lib import exceptions
 from oslo_config import cfg
 from oslo_log import log as logging
@@ -206,15 +207,32 @@ class OVSBridge(BaseOVS):
     def set_standalone_mode(self):
         self._set_bridge_fail_mode(FAILMODE_STANDALONE)
 
+    @removals.remove(
+        message=("Consider using add_protocols instead, or if replacing "
+                 "the whole set of supported protocols is the desired "
+                 "behavior, using set_db_attribute"),
+        version="Ocata",
+        removal_version="Queens")
     def set_protocols(self, protocols):
         self.set_db_attribute('Bridge', self.br_name, 'protocols', protocols,
                               check_error=True)
 
+    def add_protocols(self, *protocols):
+        self.ovsdb.db_add('Bridge', self.br_name,
+                          'protocols', *protocols).execute(check_error=True)
+
     def create(self, secure_mode=False):
         with self.ovsdb.transaction() as txn:
             txn.add(
                 self.ovsdb.add_br(self.br_name,
-                datapath_type=self.datapath_type))
+                                  datapath_type=self.datapath_type))
+            # the ovs-ofctl commands below in run_ofctl use OF10, so we
+            # need to ensure that this version is enabled ; we could reuse
+            # add_protocols, but doing ovsdb.db_add avoids doing two
+            # transactions
+            txn.add(
+                self.ovsdb.db_add('Bridge', self.br_name,
+                                  'protocols', constants.OPENFLOW10))
             if secure_mode:
                 txn.add(self.ovsdb.set_fail_mode(self.br_name,
                                                  FAILMODE_SECURE))

neutron/agent/linux/openvswitch_firewall/firewall.py

@@ -236,7 +236,7 @@ class OVSFirewallDriver(firewall.FirewallDriver):
 
     @staticmethod
     def initialize_bridge(int_br):
-        int_br.set_protocols(OVSFirewallDriver.REQUIRED_PROTOCOLS)
+        int_br.add_protocols(*OVSFirewallDriver.REQUIRED_PROTOCOLS)
         return int_br.deferred(full_ordered=True)
 
     def _drop_all_unmatched_flows(self):

neutron/cmd/sanity/checks.py

@@ -380,7 +380,7 @@ def ovs_conntrack_supported():
 
     with ovs_lib.OVSBridge(br_name) as br:
         try:
-            br.set_protocols(["OpenFlow%d" % i for i in range(10, 15)])
+            br.add_protocols(*["OpenFlow%d" % i for i in range(10, 15)])
         except RuntimeError as e:
             LOG.debug("Exception while checking ovs conntrack support: %s", e)
             return False

neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge.py

@@ -75,7 +75,7 @@ class OVSAgentBridge(ofswitch.OpenFlowSwitchMixin,
                 "port": conf.OVS.of_listen_port,
             }
         ]
-        self.set_protocols([ovs_consts.OPENFLOW10, ovs_consts.OPENFLOW13])
+        self.add_protocols(ovs_consts.OPENFLOW13)
         self.set_controller(controllers)
 
         # NOTE(ivc): Force "out-of-band" controller connection mode (see

neutron/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/ovs_bridge.py

@@ -16,8 +16,6 @@
 
 
 from neutron.agent.common import ovs_lib
-from neutron.plugins.ml2.drivers.openvswitch.agent.common import constants \
-        as ovs_consts
 from neutron.plugins.ml2.drivers.openvswitch.agent.openflow \
     import br_cookie
 from neutron.plugins.ml2.drivers.openvswitch.agent.openflow.ovs_ofctl \
@@ -29,7 +27,6 @@ class OVSAgentBridge(ofswitch.OpenFlowSwitchMixin,
     """Common code for bridges used by OVS agent"""
 
     def setup_controllers(self, conf):
-        self.set_protocols([ovs_consts.OPENFLOW10, ovs_consts.OPENFLOW13])
         self.del_controller()
 
     def drop_port(self, in_port):

neutron/tests/functional/agent/test_ovs_lib.py

@@ -159,6 +159,33 @@ class OVSBridgeTestCase(OVSBridgeTestBase):
             self.br.db_get_val('Bridge', self.br.br_name, 'protocols'),
             "OpenFlow10")
 
+    def test_add_protocols_start_with_one(self):
+        self.br.set_db_attribute('Bridge', self.br.br_name, 'protocols',
+                                 ['OpenFlow10'],
+                                 check_error=True)
+        self.br.add_protocols('OpenFlow13')
+        self.assertEqual(
+            self.br.db_get_val('Bridge', self.br.br_name, 'protocols'),
+            ['OpenFlow10', 'OpenFlow13'])
+
+    def test_add_protocols_start_with_two_add_two(self):
+        self.br.set_db_attribute('Bridge', self.br.br_name, 'protocols',
+                                 ['OpenFlow10', 'OpenFlow12'],
+                                 check_error=True)
+        self.br.add_protocols('OpenFlow13', 'OpenFlow14')
+        self.assertEqual(
+            self.br.db_get_val('Bridge', self.br.br_name, 'protocols'),
+            ['OpenFlow10', 'OpenFlow12', 'OpenFlow13', 'OpenFlow14'])
+
+    def test_add_protocols_add_existing(self):
+        self.br.set_db_attribute('Bridge', self.br.br_name, 'protocols',
+                                 ['OpenFlow10', 'OpenFlow12', 'OpenFlow13'],
+                                 check_error=True)
+        self.br.add_protocols('OpenFlow13')
+        self.assertEqual(
+            self.br.db_get_val('Bridge', self.br.br_name, 'protocols'),
+            ['OpenFlow10', 'OpenFlow12', 'OpenFlow13'])
+
     def test_get_datapath_id(self):
         brdev = ip_lib.IPDevice(self.br.br_name)
         dpid = brdev.link.attributes['link/ether'].replace(':', '')

neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge_test_base.py

@@ -135,12 +135,12 @@ class OVSBridgeTestBase(ovs_test_base.OVSRyuTestBase):
         cfg.OVS.of_listen_address = ""
         cfg.OVS.of_listen_port = ""
 
-        m_set_protocols = mock.patch.object(self.br, 'set_protocols')
+        m_add_protocols = mock.patch.object(self.br, 'add_protocols')
         m_set_controller = mock.patch.object(self.br, 'set_controller')
         m_set_ccm = mock.patch.object(self.br,
                                       'set_controllers_connection_mode')
 
-        with m_set_ccm as set_ccm, m_set_controller, m_set_protocols:
+        with m_set_ccm as set_ccm, m_set_controller, m_add_protocols:
             self.br.setup_controllers(cfg)
             set_ccm.assert_called_once_with("out-of-band")