diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index 5d564c6927f..e566977db97 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -301,6 +301,33 @@ def get_lsp_security_groups(port, skip_trusted_port=True): ) else port.get('security_groups', []) +def is_lsp_enabled(lsp): + """Return if a Logical Switch Port is enabled + + This method mimics the OVN northd method ``lsp_is_enabled`` + https://shorturl.at/rBSZ7. The "enabled" field can have three values: + * []: from older OVN versions, in this case the LSP is enabled by default. + * [True]: the LSP is enabled. + * [False]: the LSP is disabled. + + :param lsp: ``ovs.db.Row`` with a Logical Switch Port register. + :return: True if the port is enabled, False if not. + """ + return not lsp.enabled or lsp.enabled == [True] + + +def is_lsp_up(lsp): + """Return if a Logical Switch Port is UP + + This method mimics the OVN northd method ``lsp_is_up`` + https://shorturl.at/aoKR6 + + :param lsp: ``ovs.db.Row`` with a Logical Switch Port register. + :return: True if the port is UP, False if not. + """ + return lsp.up == [True] + + def is_snat_enabled(router): return router.get(l3.EXTERNAL_GW_INFO, {}).get('enable_snat', True) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py index cdb28dbc4a3..b35b6bb291f 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py @@ -286,7 +286,7 @@ class PortBindingChassisUpdateEvent(row_event.RowEvent): # ``LogicalSwitchPortUpdateUpEvent`` events. return False - return bool(lsp.up) + return utils.is_lsp_enabled(lsp) and utils.is_lsp_up(lsp) def run(self, event, row, old=None): self.driver.set_port_status_up(row.logical_port) @@ -461,81 +461,84 @@ class PortBindingChassisEvent(row_event.RowEvent): router, host) -class LogicalSwitchPortCreateUpEvent(row_event.RowEvent): - """Row create event - Logical_Switch_Port 'up' = True. +class LogicalSwitchPortCreateEvent(row_event.RowEvent): + """Row create event - Checks Logical_Switch_Port is UP and enabled. On connection, we get a dump of all ports, so if there is a neutron - port that is down that has since been activated, we'll catch it here. - This event will not be generated for new ports getting created. + port that has been activated or deactivated, we'll catch it here. """ def __init__(self, driver): self.driver = driver table = 'Logical_Switch_Port' events = (self.ROW_CREATE,) - super(LogicalSwitchPortCreateUpEvent, self).__init__( - events, table, (('up', '=', True),)) - self.event_name = 'LogicalSwitchPortCreateUpEvent' + super().__init__(events, table, []) + self.event_name = 'LogicalSwitchPortCreateEvent' def run(self, event, row, old): - self.driver.set_port_status_up(row.name) - - -class LogicalSwitchPortCreateDownEvent(row_event.RowEvent): - """Row create event - Logical_Switch_Port 'up' = False - - On connection, we get a dump of all ports, so if there is a neutron - port that is up that has since been deactivated, we'll catch it here. - This event will not be generated for new ports getting created. - """ - def __init__(self, driver): - self.driver = driver - table = 'Logical_Switch_Port' - events = (self.ROW_CREATE,) - super(LogicalSwitchPortCreateDownEvent, self).__init__( - events, table, (('up', '=', False),)) - self.event_name = 'LogicalSwitchPortCreateDownEvent' - - def run(self, event, row, old): - self.driver.set_port_status_down(row.name) + if utils.is_lsp_up(row) and utils.is_lsp_enabled(row): + self.driver.set_port_status_up(row.name) + else: + self.driver.set_port_status_down(row.name) class LogicalSwitchPortUpdateUpEvent(row_event.RowEvent): - """Row update event - Logical_Switch_Port 'up' going from False to True + """Row update event - Logical_Switch_Port UP or enabled going True This happens when the VM goes up. - New value of Logical_Switch_Port 'up' will be True and the old value will - be False. """ def __init__(self, driver): self.driver = driver table = 'Logical_Switch_Port' events = (self.ROW_UPDATE,) super(LogicalSwitchPortUpdateUpEvent, self).__init__( - events, table, (('up', '=', True),), - old_conditions=(('up', '!=', True),)) + events, table, None) self.event_name = 'LogicalSwitchPortUpdateUpEvent' + def match_fn(self, event, row, old): + if not (utils.is_lsp_up(row) and utils.is_lsp_enabled(row)): + return False + + if hasattr(old, 'up') and not utils.is_lsp_up(old): + # The port has transitioned from DOWN to UP, and the admin state + # is UP (lsp.enabled=True) + return True + if hasattr(old, 'enabled') and not utils.is_lsp_enabled(old): + # The user has set the admin state to UP and the port is UP too. + return True + return False + def run(self, event, row, old): self.driver.set_port_status_up(row.name) class LogicalSwitchPortUpdateDownEvent(row_event.RowEvent): - """Row update event - Logical_Switch_Port 'up' going from True to False + """Row update event - Logical_Switch_Port UP or enabled going to False - This happens when the VM goes down. - New value of Logical_Switch_Port 'up' will be False and the old value will - be True. + This happens when the VM goes down or the port is disabled. """ def __init__(self, driver): self.driver = driver table = 'Logical_Switch_Port' events = (self.ROW_UPDATE,) super(LogicalSwitchPortUpdateDownEvent, self).__init__( - events, table, (('up', '=', False),), - old_conditions=(('up', '=', True),)) + events, table, None) self.event_name = 'LogicalSwitchPortUpdateDownEvent' + def match_fn(self, event, row, old): + if (hasattr(old, 'up') and + utils.is_lsp_up(old) and + not utils.is_lsp_up(row)): + # If the port goes DOWN, update the port status to DOWN. + return True + if (hasattr(old, 'enabled') and + utils.is_lsp_enabled(old) and + not utils.is_lsp_enabled(row)): + # If the port is disabled by the user, update the port status to + # DOWN. + return True + return False + def run(self, event, row, old): self.driver.set_port_status_down(row.name) @@ -779,12 +782,10 @@ class OvnNbIdl(OvnIdlDistributedLock): super(OvnNbIdl, self).__init__(driver, remote, schema) self._lsp_update_up_event = LogicalSwitchPortUpdateUpEvent(driver) self._lsp_update_down_event = LogicalSwitchPortUpdateDownEvent(driver) - self._lsp_create_up_event = LogicalSwitchPortCreateUpEvent(driver) - self._lsp_create_down_event = LogicalSwitchPortCreateDownEvent(driver) + self._lsp_create_event = LogicalSwitchPortCreateEvent(driver) self._fip_create_delete_event = FIPAddDeleteEvent(driver) - self.notify_handler.watch_events([self._lsp_create_up_event, - self._lsp_create_down_event, + self.notify_handler.watch_events([self._lsp_create_event, self._lsp_update_up_event, self._lsp_update_down_event, self._fip_create_delete_event]) @@ -804,10 +805,8 @@ class OvnNbIdl(OvnIdlDistributedLock): After the startup, there is no need to watch these events. So unwatch these events. """ - self.notify_handler.unwatch_events([self._lsp_create_up_event, - self._lsp_create_down_event]) - self._lsp_create_up_event = None - self._lsp_create_down_event = None + self.notify_handler.unwatch_events([self._lsp_create_event]) + del self._lsp_create_event def post_connect(self): self.unwatch_logical_switch_port_create_events() diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py index bb78f665ea7..3365a46c885 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py @@ -60,7 +60,9 @@ OVN_NB_SCHEMA = { "port_security": {"type": {"key": "string", "min": 0, "max": "unlimited"}}, - "up": {"type": {"key": "boolean", "min": 0, "max": 1}}}, + "up": {"type": {"key": "boolean", "min": 0, "max": 1}}, + "enabled": {"type": {"key": "boolean", "min": 0, "max": 1}}, + }, "indexes": [["name"]], "isRoot": False, }, @@ -366,7 +368,8 @@ class TestPortBindingChassisUpdateEvent(base.BaseTestCase): pbtable = fakes.FakeOvsdbTable.create_one_ovsdb_table( attrs={'name': 'Port_Binding'}) ovsdb_row = fakes.FakeOvsdbRow.create_one_ovsdb_row - self.driver.nb_ovn.lookup.return_value = ovsdb_row(attrs={'up': True}) + self.driver.nb_ovn.lookup.return_value = ovsdb_row( + attrs={'up': True, 'enabled': True}) # Port binding change. self._test_event( @@ -422,28 +425,46 @@ class TestOvnNbIdlNotifyHandler(test_mech_driver.OVNMechanismDriverTestCase): # Execute the notifications queued self.idl.notify_handler.notify_loop() - def test_lsp_up_create_event(self): - row_data = {"up": True, "name": "foo-name"} + def test_lsp_create_event(self): + row_data = {'name': 'foo'} + + # up and enabled + row_data.update({'up': True, 'enabled': True}) self._test_lsp_helper('create', row_data) - self.mech_driver.set_port_status_up.assert_called_once_with("foo-name") + self.mech_driver.set_port_status_up.assert_called_once_with('foo') self.assertFalse(self.mech_driver.set_port_status_down.called) + self.mech_driver.set_port_status_up.reset_mock() - def test_lsp_down_create_event(self): - row_data = {"up": False, "name": "foo-name"} - self._test_lsp_helper('create', row_data) - self.mech_driver.set_port_status_down.assert_called_once_with( - "foo-name") - self.assertFalse(self.mech_driver.set_port_status_up.called) - - def test_lsp_up_not_set_event(self): - row_data = {"up": ['set', []], "name": "foo-name"} + # up and disabled + row_data.update({'up': True, 'enabled': False}) self._test_lsp_helper('create', row_data) self.assertFalse(self.mech_driver.set_port_status_up.called) - self.assertFalse(self.mech_driver.set_port_status_down.called) + self.mech_driver.set_port_status_down.assert_called_once_with('foo') + self.mech_driver.set_port_status_down.reset_mock() + + # down and enabled + row_data.update({'up': False, 'enabled': True}) + self._test_lsp_helper('create', row_data) + self.assertFalse(self.mech_driver.set_port_status_up.called) + self.mech_driver.set_port_status_down.assert_called_once_with('foo') + self.mech_driver.set_port_status_down.reset_mock() + + # down and disabled + row_data.update({'up': False, 'enabled': False}) + self._test_lsp_helper('create', row_data) + self.assertFalse(self.mech_driver.set_port_status_up.called) + self.mech_driver.set_port_status_down.assert_called_once_with('foo') + self.mech_driver.set_port_status_down.reset_mock() + + # Not set to up + row_data.update({'up': ['set', []], 'enabled': True}) + self._test_lsp_helper('create', row_data) + self.assertFalse(self.mech_driver.set_port_status_up.called) + self.mech_driver.set_port_status_down.assert_called_once_with('foo') def test_unwatch_logical_switch_port_create_events(self): self.idl.unwatch_logical_switch_port_create_events() - row_data = {"up": True, "name": "foo-name"} + row_data = {'up': False, 'name': 'foo-name'} self._test_lsp_helper('create', row_data) self.assertFalse(self.mech_driver.set_port_status_up.called) self.assertFalse(self.mech_driver.set_port_status_down.called) @@ -455,11 +476,10 @@ class TestOvnNbIdlNotifyHandler(test_mech_driver.OVNMechanismDriverTestCase): def test_post_connect(self): self.idl.post_connect() - self.assertIsNone(self.idl._lsp_create_up_event) - self.assertIsNone(self.idl._lsp_create_down_event) + self.assertIsNone(getattr(self.idl, '_lsp_create_event', None)) def test_lsp_up_update_event(self): - new_row_json = {"up": True, "name": "foo-name"} + new_row_json = {'up': True, 'enabled': True, 'name': 'foo-name'} old_row_json = {"up": False} self._test_lsp_helper('update', new_row_json, old_row_json=old_row_json) @@ -467,7 +487,7 @@ class TestOvnNbIdlNotifyHandler(test_mech_driver.OVNMechanismDriverTestCase): self.assertFalse(self.mech_driver.set_port_status_down.called) def test_lsp_down_update_event(self): - new_row_json = {"up": False, "name": "foo-name"} + new_row_json = {'up': False, 'enabled': False, 'name': 'foo-name'} old_row_json = {"up": True} self._test_lsp_helper('update', new_row_json, old_row_json=old_row_json) @@ -476,7 +496,7 @@ class TestOvnNbIdlNotifyHandler(test_mech_driver.OVNMechanismDriverTestCase): self.assertFalse(self.mech_driver.set_port_status_up.called) def test_lsp_up_update_event_no_old_data(self): - new_row_json = {"up": True, "name": "foo-name"} + new_row_json = {'up': True, 'enabled': True, 'name': 'foo-name'} self._test_lsp_helper('update', new_row_json, old_row_json=None) self.assertFalse(self.mech_driver.set_port_status_up.called) diff --git a/releasenotes/notes/port_status_based_on_lsp_up_and_enabled-31c062fc7089f62a.yaml b/releasenotes/notes/port_status_based_on_lsp_up_and_enabled-31c062fc7089f62a.yaml new file mode 100644 index 00000000000..cb9ba3515c5 --- /dev/null +++ b/releasenotes/notes/port_status_based_on_lsp_up_and_enabled-31c062fc7089f62a.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + The Neutron ``port.status`` field ("ACTIVE", "DOWN") is now set based on + the ML2/OVN Logical Switch Port ``up`` and ``enabled`` flags. The user can + now set the ``port.admin_state_up``, that is replicated in the + ``lsp.enabled`` flag, to enable or disable the port. If the port is + disabled, the traffic is stopped and the ``port.status`` is set to "DOWN".