From e9a55cd2b6eca09e14a18212d8d537eaa7ac17d7 Mon Sep 17 00:00:00 2001 From: Fernando Royo Date: Wed, 19 Jul 2023 12:50:09 +0200 Subject: [PATCH] Add FIP on LogicalSwitchPortUpdate event When a LogicalSwitchPortUpdate event is triggered after removing FIP from LB VIP, the event received include the port affected, but the FIP related is not passing to the handler method. This patch includes the FIP into the info passed to the handler method, simplifying the current handler logic and providing future support for the new multi-vip feature. Also added a match for only manage events including external_id updates. Closes-Bug: #2028161 Change-Id: Ibee3906e8e9575fba7811e989e3e111a026ce45b --- ovn_octavia_provider/event.py | 26 ++++-- ovn_octavia_provider/helper.py | 16 +--- .../tests/unit/test_helper.py | 83 ++++++++++++++++--- 3 files changed, 92 insertions(+), 33 deletions(-) diff --git a/ovn_octavia_provider/event.py b/ovn_octavia_provider/event.py index ef16b9db..95d7c4a3 100644 --- a/ovn_octavia_provider/event.py +++ b/ovn_octavia_provider/event.py @@ -51,19 +51,29 @@ class LogicalSwitchPortUpdateEvent(row_event.RowEvent): self.event_name = 'LogicalSwitchPortUpdateEvent' self.driver = driver + def match_fn(self, event, row, old): + port_name = row.external_ids.get( + ovn_const.OVN_PORT_NAME_EXT_ID_KEY, '') + if hasattr(old, 'external_ids') and port_name.startswith( + ovn_const.LB_VIP_PORT_PREFIX): + return True + return False + def run(self, event, row, old): LOG.debug('LogicalSwitchPortUpdateEvent logged, ' '%(event)s, %(row)s', {'event': event, 'row': row}) - # Get the neutron:port_name from external_ids and check if - # it's a vip port or not. - port_name = row.external_ids.get( - ovn_const.OVN_PORT_NAME_EXT_ID_KEY, '') - if port_name.startswith(ovn_const.LB_VIP_PORT_PREFIX): - # Handle port update only for vip ports created by - # this driver. - self.driver.vip_port_update_handler(row) + fip_old = old.external_ids.get(ovn_const.OVN_PORT_FIP_EXT_ID_KEY) + fip_new = row.external_ids.get(ovn_const.OVN_PORT_FIP_EXT_ID_KEY) + if fip_old != fip_new: + if fip_old and fip_new is None: + action = ovn_const.REQ_INFO_ACTION_DISASSOCIATE + fip = fip_old + else: + action = ovn_const.REQ_INFO_ACTION_ASSOCIATE + fip = fip_new + self.driver.vip_port_update_handler(row, fip, action) class ServiceMonitorUpdateEvent(row_event.RowEvent): diff --git a/ovn_octavia_provider/helper.py b/ovn_octavia_provider/helper.py index 6e13d475..1aa7109e 100644 --- a/ovn_octavia_provider/helper.py +++ b/ovn_octavia_provider/helper.py @@ -328,7 +328,7 @@ class OvnProviderHelper(): lb.uuid, utils.ovn_uuid(info['network'].name)) pass - def vip_port_update_handler(self, vip_lp): + def vip_port_update_handler(self, vip_lp, fip, action): """Handler for VirtualIP port updates. If a floating ip is associated to a vip port, then networking-ovn sets @@ -351,16 +351,9 @@ class OvnProviderHelper(): # Loop over all defined LBs with given ID, because it is possible # than there is more than one (for more than 1 L4 protocol). for lb in ovn_lbs: - fip = vip_lp.external_ids.get(ovn_const.OVN_PORT_FIP_EXT_ID_KEY) - lb_vip_fip = lb.external_ids.get(ovn_const.LB_EXT_IDS_VIP_FIP_KEY) request_info = {'ovn_lb': lb, - 'vip_fip': fip} - if fip and fip != lb_vip_fip: - request_info['action'] = ovn_const.REQ_INFO_ACTION_ASSOCIATE - elif fip is None and fip != lb_vip_fip: - request_info['action'] = ovn_const.REQ_INFO_ACTION_DISASSOCIATE - else: - continue + 'vip_fip': fip, + 'action': action} self.add_request({'type': ovn_const.REQ_TYPE_HANDLE_VIP_FIP, 'info': request_info}) @@ -2323,10 +2316,9 @@ class OvnProviderHelper(): self.ovn_nbdb_api.db_remove( 'Load_Balancer', ovn_lb.uuid, 'external_ids', (ovn_const.LB_EXT_IDS_VIP_FIP_KEY))) - old_fip = ovn_lb.external_ids.get(ovn_const.LB_EXT_IDS_VIP_FIP_KEY) for lbhc in ovn_lb.health_check: # FIPs can only be ipv4, so not dealing with ipv6 [] here - if lbhc.vip.split(":")[0] == old_fip: + if lbhc.vip.split(":")[0] == fip_info['vip_fip']: commands.append( self.ovn_nbdb_api.db_remove('Load_Balancer', ovn_lb.uuid, diff --git a/ovn_octavia_provider/tests/unit/test_helper.py b/ovn_octavia_provider/tests/unit/test_helper.py index d97f7b65..a3f1e1a7 100644 --- a/ovn_octavia_provider/tests/unit/test_helper.py +++ b/ovn_octavia_provider/tests/unit/test_helper.py @@ -2980,21 +2980,27 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase): self.ref_lb1, network_id=self.network.uuid) - def test_logical_switch_port_update_event_vip_port(self): + def test_logical_switch_port_update_event_vip_port_associate(self): self.switch_port_event = ovn_event.LogicalSwitchPortUpdateEvent( self.helper) port_name = '%s%s' % (ovn_const.LB_VIP_PORT_PREFIX, 'foo') + fip = '10.0.0.1' attrs = { 'external_ids': {ovn_const.OVN_PORT_NAME_EXT_ID_KEY: port_name, - ovn_const.OVN_PORT_FIP_EXT_ID_KEY: '10.0.0.1'}} + ovn_const.OVN_PORT_FIP_EXT_ID_KEY: fip}} row = fakes.FakeOvsdbRow.create_one_ovsdb_row( attrs=attrs) - self.switch_port_event.run(mock.ANY, row, mock.ANY) + attrs_old = { + 'external_ids': + {ovn_const.OVN_PORT_NAME_EXT_ID_KEY: port_name}} + old = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs=attrs_old) + self.switch_port_event.run(mock.ANY, row, old) expected_call = { 'info': {'action': 'associate', - 'vip_fip': '10.0.0.1', + 'vip_fip': fip, 'ovn_lb': self.ovn_lb}, 'type': 'handle_vip_fip'} self.mock_add_request.assert_called_once_with(expected_call) @@ -3005,34 +3011,75 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase): attrs = {'external_ids': {}} row = fakes.FakeOvsdbRow.create_one_ovsdb_row( attrs=attrs) - self.switch_port_event.run(mock.ANY, row, mock.ANY) + self.assertFalse(self.switch_port_event.match_fn( + mock.ANY, row, mock.ANY)) self.mock_add_request.assert_not_called() - def test_logical_switch_port_update_event_empty_fip(self): + def test_logical_switch_port_update_event_disassociate(self): self.switch_port_event = ovn_event.LogicalSwitchPortUpdateEvent( self.helper) port_name = '%s%s' % (ovn_const.LB_VIP_PORT_PREFIX, 'foo') + fip = '172.24.4.4' attrs = {'external_ids': {ovn_const.OVN_PORT_NAME_EXT_ID_KEY: port_name}} row = fakes.FakeOvsdbRow.create_one_ovsdb_row( attrs=attrs) - self.switch_port_event.run(mock.ANY, row, mock.ANY) + attrs_old = {'external_ids': + {ovn_const.OVN_PORT_NAME_EXT_ID_KEY: port_name, + ovn_const.OVN_PORT_FIP_EXT_ID_KEY: fip}} + old = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs=attrs_old) + self.switch_port_event.run(mock.ANY, row, old) expected_call = { 'info': {'action': 'disassociate', - 'vip_fip': None, + 'vip_fip': fip, 'ovn_lb': self.ovn_lb}, 'type': 'handle_vip_fip'} self.mock_add_request.assert_called_once_with(expected_call) - def test_logical_switch_port_update_event_not_vip_port(self): + def test_logical_switch_port_update_event_update_unrelated(self): + self.switch_port_event = ovn_event.LogicalSwitchPortUpdateEvent( + self.helper) + port_name = '%s%s' % (ovn_const.LB_VIP_PORT_PREFIX, 'foo') + fip = '172.24.4.4' + attrs = {'external_ids': + {ovn_const.OVN_PORT_NAME_EXT_ID_KEY: port_name, + ovn_const.OVN_PORT_FIP_EXT_ID_KEY: fip}} + row = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs=attrs) + attrs_old = {'external_ids': + {ovn_const.OVN_PORT_NAME_EXT_ID_KEY: port_name, + ovn_const.OVN_PORT_FIP_EXT_ID_KEY: fip}} + old = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs=attrs_old) + self.switch_port_event.run(mock.ANY, row, old) + self.mock_add_request.assert_not_called() + + def test_logical_switch_port_update_event_without_external_ids(self): + self.switch_port_event = ovn_event.LogicalSwitchPortUpdateEvent( + self.helper) + attrs = {} + row = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs=attrs) + old = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs=attrs) + self.switch_port_event.run(mock.ANY, row, old) + self.mock_add_request.assert_not_called() + + def test_logical_switch_port_update_event_wrong_vip_port_name(self): self.switch_port_event = ovn_event.LogicalSwitchPortUpdateEvent( self.helper) port_name = 'foo' row = fakes.FakeOvsdbRow.create_one_ovsdb_row( attrs={'external_ids': {ovn_const.OVN_PORT_NAME_EXT_ID_KEY: port_name}}) - self.switch_port_event.run(mock.ANY, row, mock.ANY) + attrs_old = {'external_ids': + {ovn_const.OVN_PORT_NAME_EXT_ID_KEY: port_name, + ovn_const.OVN_PORT_FIP_EXT_ID_KEY: 'foo'}} + old = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs=attrs_old) + self.assertFalse(self.switch_port_event.match_fn(mock.ANY, row, old)) self.mock_add_request.assert_not_called() @mock.patch('ovn_octavia_provider.common.clients.get_neutron_client') @@ -3052,7 +3099,12 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase): {ovn_const.OVN_PORT_NAME_EXT_ID_KEY: port_name}} row = fakes.FakeOvsdbRow.create_one_ovsdb_row( attrs=attrs) - self.switch_port_event.run(mock.ANY, row, mock.ANY) + attrs_old = {'external_ids': + {ovn_const.OVN_PORT_NAME_EXT_ID_KEY: port_name, + ovn_const.OVN_PORT_FIP_EXT_ID_KEY: '172.24.4.40'}} + old = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs=attrs_old) + self.switch_port_event.run(mock.ANY, row, old) self.mock_add_request.assert_not_called() @mock.patch.object(ovn_helper.OvnProviderHelper, '_execute_commands') @@ -3108,13 +3160,18 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase): {ovn_const.OVN_PORT_NAME_EXT_ID_KEY: port_name}} row = fakes.FakeOvsdbRow.create_one_ovsdb_row( attrs=attrs) - self.switch_port_event.run(mock.ANY, row, mock.ANY) + attrs_old = {'external_ids': + {ovn_const.OVN_PORT_NAME_EXT_ID_KEY: port_name, + ovn_const.OVN_PORT_FIP_EXT_ID_KEY: '172.24.4.40'}} + old = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs=attrs_old) + self.switch_port_event.run(mock.ANY, row, old) def expected_call(lb): return {'type': 'handle_vip_fip', 'info': {'action': mock.ANY, - 'vip_fip': None, + 'vip_fip': '172.24.4.40', 'ovn_lb': lb}} self.mock_add_request.assert_has_calls([