openstack
/
neutron
Code Issues Proposed changes

Don't setup bridge controller if it is already set 

Setting new controller for bridge every time when neutron-ovs-agent
is restarted or is doing full-sync may cause some short data plane
connectivity loss and is not needed if same controller is already
configured for the bridge.
With this patch neutron-ovs-agent will first check if controller is
configured for the bridge and if it's the same as what should be
configured, it will skip setting it up.

With this patch also protocols added to the bridge will be first checked
if they aren't already there and only missing ones will be added if
necessary.

Setting of the connectivity mode and inactivity probe is
always performed as this don't cause connectivity issues and is cheap
so we can always ensure that those parameters are configured properly.

Closes-Bug: #1948642
Change-Id: Idfa763df8c60d8ae46cd6351d1b6dc7d950b4c67
This commit is contained in:
Slawek Kaplonski 2021-10-25 10:59:24 +02:00
parent 0194856da1
commit 11d166be68
4 changed files with 70 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
neutron/agent/common/ovs_lib.py
View File

@ -284,13 +284,19 @@ 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)
# TODO(ralonsoh): this is a workaround for LP#1948642. When the OF
# protocols are changed, the OF controller is restarted. This sleep
# will provide time to os-ken ``OfctlService`` to receive the update
# events of the restarted controllers and set them as enabled.
time.sleep(1)
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)
# TODO(ralonsoh): this is a workaround for LP#1948642. When the OF
# protocols are changed, the OF controller is restarted. This
# sleep will provide time to os-ken ``OfctlService`` to receive
# the update events of the restarted controllers and set them as
# enabled.
time.sleep(1)
def use_at_least_protocol(self, protocol):
"""Calls to ovs-ofctl will use a protocol version >= 'protocol'"""

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

@ -73,10 +73,14 @@ class OVSAgentBridge(ofswitch.OpenFlowSwitchMixin,
def setup_controllers(self, conf):
url = ipv6_utils.valid_ipv6_url(conf.OVS.of_listen_address,
conf.OVS.of_listen_port)
controllers = ["tcp:" + url]
self.add_protocols(ovs_consts.OPENFLOW10, ovs_consts.OPENFLOW13)
self.set_controller(controllers)
controller = "tcp:" + url
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.OPENFLOW10, ovs_consts.OPENFLOW13)
# NOTE(ivc): Force "out-of-band" controller connection mode (see
# "In-Band Control" [1]).
#

23
neutron/tests/unit/agent/common/test_ovs_lib.py
View File

@ -207,6 +207,29 @@ class OVS_Lib_Test(base.BaseTestCase):
return self.execute.assert_called_once_with(
cmd, run_as_root=True, privsep_exec=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),

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

@ -137,11 +137,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,
@ -149,13 +150,29 @@ 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_add_protocols as add_protocols:
with m_set_controller, m_set_probe:
self.br.setup_controllers(cfg)
set_ccm.assert_called_once_with("out-of-band")
add_protocols.assert_called_once_with(
constants.OPENFLOW10, constants.OPENFLOW13)
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.OPENFLOW10, 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):