From b21dc51baf049caf6d83b753180a63ea2f6d798a Mon Sep 17 00:00:00 2001 From: Fernando Royo Date: Mon, 22 Apr 2024 15:47:46 +0200 Subject: [PATCH] Remove leftover OVN LB HM port upon deletion of a member When a load balancer pool has a Health Monitor associated with it, an OVN LB Health Monitor port is created for each backend member subnet added. When removing backend members, the OVN LB Health Monitor port is cleaned up only if no more members are associated with the Health Monitor pool. However, this assumption is incorrect. This patch corrects this behavior by checking instead if there are more members from the same subnet associated with the pool. It ensures that the OVN LB Health Monitor port is deleted only when the last member from the subnet is deleted. If the port is being used by another different LB Health Monitor, `_clean_up_hm_port` will handle it. Closes-Bug: #2062965 Change-Id: I4c35cc5c6af14bb208f4313bb86e3519df0a30fa (cherry picked from commit f034bab144b68cf96c538339e389c4cc7c6d7d63) --- ovn_octavia_provider/helper.py | 26 ++++++++++++++----- .../tests/unit/test_helper.py | 24 +++++++++++++++++ 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/ovn_octavia_provider/helper.py b/ovn_octavia_provider/helper.py index 9ed8f1fa..6b15d860 100644 --- a/ovn_octavia_provider/helper.py +++ b/ovn_octavia_provider/helper.py @@ -2000,20 +2000,32 @@ class OvnProviderHelper(): user_fault_string=msg, operator_fault_string=msg) + def _members_in_subnet(self, ovn_lb, subnet_id): + for key, value in ovn_lb.external_ids.items(): + if key.startswith(ovn_const.LB_EXT_IDS_POOL_PREFIX): + if value and len(value.split(',')) > 0: + for m_info in value.split(','): + mem_id, mem_ip_port, mem_subnet = m_info.split('_')[1:] + if mem_subnet == subnet_id: + return True + return False + def member_delete(self, member): error_deleting_member = False try: pool_key, ovn_lb = self._find_ovn_lb_by_pool_id( member[constants.POOL_ID]) - pool_status = self._remove_member(member, ovn_lb, pool_key) + self._remove_member(member, ovn_lb, pool_key) - if ovn_lb.health_check and pool_status == constants.OFFLINE: - # NOTE(froyo): if the pool status is OFFLINE there are no more - # members. So we should ensure the hm-port is deleted if no - # more LB are using it. We need to do this call after the - # cleaning of the ip_port_mappings for the ovn LB. - self._clean_up_hm_port(member[constants.SUBNET_ID]) + if ovn_lb.health_check: + mem_subnet = member[constants.SUBNET_ID] + if not self._members_in_subnet(ovn_lb, mem_subnet): + # NOTE(froyo): if member is last member from the subnet + # we should clean up the ovn-lb-hm-port. + # We need to do this call after the cleaning of the + # ip_port_mappings for the ovn LB. + self._clean_up_hm_port(member[constants.SUBNET_ID]) except Exception: LOG.exception(ovn_const.EXCEPTION_MSG, "deletion of member") error_deleting_member = True diff --git a/ovn_octavia_provider/tests/unit/test_helper.py b/ovn_octavia_provider/tests/unit/test_helper.py index ac7e05a7..fe8ed6d4 100644 --- a/ovn_octavia_provider/tests/unit/test_helper.py +++ b/ovn_octavia_provider/tests/unit/test_helper.py @@ -1994,6 +1994,30 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase): self.member_address, delete=True) + @mock.patch.object(ovn_helper.OvnProviderHelper, '_find_ovn_lb_by_pool_id') + @mock.patch.object(ovn_helper.OvnProviderHelper, '_update_hm_member') + @mock.patch.object(ovn_helper.OvnProviderHelper, '_clean_up_hm_port') + def test_member_delete_keep_hm_port(self, del_hm_port, uhm, folbpi): + pool_key = 'pool_%s' % self.pool_id + member2_id = uuidutils.generate_uuid() + member2_address = '192.168.2.150' + member2_line = ( + 'member_%s_%s:%s_%s' % + (member2_id, member2_address, + self.member_port, self.member_subnet_id)) + self.ovn_hm_lb.external_ids[pool_key] = ','.join([self.member_line, + member2_line]) + self.ovn_hm_lb.external_ids[ovn_const.OVN_MEMBER_STATUS_KEY] = \ + '{"%s": "%s","%s": "%s"}' % (self.member_id, constants.ONLINE, + member2_id, constants.ONLINE) + folbpi.return_value = (pool_key, self.ovn_hm_lb) + self.helper.member_delete(self.member) + uhm.assert_called_once_with(self.ovn_hm_lb, + pool_key, + self.member_address, + delete=True) + del_hm_port.assert_not_called() + def test_member_delete_not_found_in_pool(self): self.ovn_lb.external_ids.update({'pool_' + self.pool_id: ''}) self.ovn_lb.external_ids[ovn_const.OVN_MEMBER_STATUS_KEY] = '{}'