From f034bab144b68cf96c538339e389c4cc7c6d7d63 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 --- 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 602b410a..d9c1c1be 100644 --- a/ovn_octavia_provider/helper.py +++ b/ovn_octavia_provider/helper.py @@ -2189,20 +2189,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 029ea92e..6e89cc1d 100644 --- a/ovn_octavia_provider/tests/unit/test_helper.py +++ b/ovn_octavia_provider/tests/unit/test_helper.py @@ -2177,6 +2177,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] = '{}'