Merge "OVS: merge the required OpenFlow version rather than replace"

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
@@ -205,15 +206,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")