diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py index b2f3da1844f..84c7fbdd6c3 100644 --- a/neutron/agent/common/ovs_lib.py +++ b/neutron/agent/common/ovs_lib.py @@ -280,8 +280,13 @@ class OVSBridge(BaseOVS): self._set_bridge_fail_mode(FAILMODE_STANDALONE) def add_protocols(self, *protocols): - self.ovsdb.db_add('Bridge', self.br_name, - 'protocols', *protocols).execute(check_error=True) + existing_protocols = self.db_get_val( + 'Bridge', self.br_name, 'protocols') + diff = set(protocols).difference(existing_protocols) + if diff: + self.ovsdb.db_add( + 'Bridge', self.br_name, + 'protocols', *diff).execute(check_error=True) def use_at_least_protocol(self, protocol): """Calls to ovs-ofctl will use a protocol version >= 'protocol'""" diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge.py b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge.py index 52fb05f4079..355c5f8fddc 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge.py @@ -70,15 +70,19 @@ class OVSAgentBridge(ofswitch.OpenFlowSwitchMixin, self._cached_dpid = new_dpid def setup_controllers(self, conf): - controllers = [ + controller = ( "tcp:%(address)s:%(port)s" % { "address": conf.OVS.of_listen_address, "port": conf.OVS.of_listen_port, } - ] - self.add_protocols(ovs_consts.OPENFLOW13) - self.set_controller(controllers) + ) + existing_controllers = self.get_controller() + if controller not in existing_controllers: + LOG.debug("Setting controller %s for bridge %s.", + controller, self.br_name) + self.set_controller([controller]) + self.add_protocols(ovs_consts.OPENFLOW13) # NOTE(ivc): Force "out-of-band" controller connection mode (see # "In-Band Control" [1]). # diff --git a/neutron/tests/unit/agent/common/test_ovs_lib.py b/neutron/tests/unit/agent/common/test_ovs_lib.py index d861312b8b2..bcab17bdd59 100644 --- a/neutron/tests/unit/agent/common/test_ovs_lib.py +++ b/neutron/tests/unit/agent/common/test_ovs_lib.py @@ -207,6 +207,29 @@ class OVS_Lib_Test(base.BaseTestCase): return self.execute.assert_called_once_with(cmd, run_as_root=True, **kwargs) + def test_add_protocols_all_already_set(self): + self.br = ovs_lib.OVSBridge(self.BR_NAME) + with mock.patch.object(self.br, 'db_get_val') as db_get_val, \ + mock.patch.object(self.br.ovsdb, 'db_add') as db_add: + db_get_val.return_value = [p_const.OPENFLOW10, + p_const.OPENFLOW13] + self.br.add_protocols(p_const.OPENFLOW10, p_const.OPENFLOW13) + db_get_val.assert_called_once_with( + 'Bridge', self.BR_NAME, 'protocols') + db_add.assert_not_called() + + def test_add_protocols_some_already_set(self): + self.br = ovs_lib.OVSBridge(self.BR_NAME) + with mock.patch.object(self.br, 'db_get_val') as db_get_val, \ + mock.patch.object(self.br.ovsdb, 'db_add') as db_add: + db_get_val.return_value = [p_const.OPENFLOW10] + self.br.add_protocols(p_const.OPENFLOW10, p_const.OPENFLOW13) + db_get_val.assert_called_once_with( + 'Bridge', self.BR_NAME, 'protocols') + db_add.assert_has_calls([ + mock.call('Bridge', self.BR_NAME, + 'protocols', p_const.OPENFLOW13)]) + def test_add_flow_timeout_set(self): flow_dict = collections.OrderedDict([ ('cookie', 1234), diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge_test_base.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge_test_base.py index 2b170691e71..7773f1fce2a 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge_test_base.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge_test_base.py @@ -136,11 +136,12 @@ class OVSBridgeTestBase(ovs_test_base.OVSOSKenTestBase): self.assertEqual('192.168.0.1', f('192.168.0.1/32')) self.assertEqual(('192.168.0.0', '255.255.255.0'), f('192.168.0.0/24')) - def test__setup_controllers__out_of_band(self): + def _test_setup_controllers(self, existing_controllers): cfg = mock.MagicMock() - cfg.OVS.of_listen_address = "" - cfg.OVS.of_listen_port = "" + cfg.OVS.of_listen_address = "127.0.0.1" + cfg.OVS.of_listen_port = "6633" + m_get_controller = mock.patch.object(self.br, 'get_controller') m_add_protocols = mock.patch.object(self.br, 'add_protocols') m_set_controller = mock.patch.object(self.br, 'set_controller') m_set_probe = mock.patch.object(self.br, @@ -148,10 +149,28 @@ class OVSBridgeTestBase(ovs_test_base.OVSOSKenTestBase): m_set_ccm = mock.patch.object(self.br, 'set_controllers_connection_mode') - with m_set_ccm as set_ccm: - with m_set_controller, m_add_protocols, m_set_probe: - self.br.setup_controllers(cfg) - set_ccm.assert_called_once_with("out-of-band") + with m_set_ccm as set_ccm, \ + m_add_protocols as add_protocols, \ + m_set_controller as set_controller, \ + m_get_controller as get_controller, \ + m_set_probe: + get_controller.return_value = existing_controllers + + self.br.setup_controllers(cfg) + + if existing_controllers: + set_controller.assert_not_called() + else: + set_controller.assert_called_once_with(["tcp:127.0.0.1:6633"]) + set_ccm.assert_called_once_with("out-of-band") + add_protocols.assert_called_once_with(constants.OPENFLOW13) + + def test_setup_controllers(self): + self._test_setup_controllers(existing_controllers=[]) + + def test_setup_controllers_when_already_exists(self): + self._test_setup_controllers( + existing_controllers=["tcp:127.0.0.1:6633"]) class OVSDVRProcessTestMixin(object):