From 33627fbb4df83028912df5a39a8e84bb10f6fcff Mon Sep 17 00:00:00 2001 From: Gregory Thiemonge Date: Mon, 2 Oct 2023 02:50:44 -0400 Subject: [PATCH] Fix error when deleting LB with broken amp If a load balancer has some broken amps (amphora was unsuccessfully created, entry exists in the DB but with missing fields), the deletion of the load balancer fails because octavia is calling delete_port with None. Closes-Bug: #2037951 Change-Id: I020ded6e1735ea56da46c5170292ba88ad639103 (cherry picked from commit 96870916e0dcd9492f719b1a3ad5d7be15570310) --- .../drivers/neutron/allowed_address_pairs.py | 13 ++++++----- .../neutron/test_allowed_address_pairs.py | 22 +++++++++++++++++++ ...lete-with-broken-amp-10d7f4e85754d7ee.yaml | 6 +++++ 3 files changed, 35 insertions(+), 6 deletions(-) create mode 100644 releasenotes/notes/fix-error-on-delete-with-broken-amp-10d7f4e85754d7ee.yaml diff --git a/octavia/network/drivers/neutron/allowed_address_pairs.py b/octavia/network/drivers/neutron/allowed_address_pairs.py index 0d94734a7d..0e580e9b2a 100644 --- a/octavia/network/drivers/neutron/allowed_address_pairs.py +++ b/octavia/network/drivers/neutron/allowed_address_pairs.py @@ -364,12 +364,13 @@ class AllowedAddressPairsDriver(neutron_base.BaseNeutronDriver): """ try: for amphora in vip.load_balancer.amphorae: - try: - self.network_proxy.delete_port(amphora.vrrp_port_id) - except os_exceptions.ResourceNotFound: - LOG.debug( - 'VIP instance port %s already deleted. Skipping.', - amphora.vrrp_port_id) + if amphora.vrrp_port_id: + try: + self.network_proxy.delete_port(amphora.vrrp_port_id) + except os_exceptions.ResourceNotFound: + LOG.debug( + 'VIP instance port %s already deleted. Skipping.', + amphora.vrrp_port_id) except AttributeError as ex: LOG.warning(f"Cannot delete port from amphorae. Object does not " f"exist ({ex!r})") diff --git a/octavia/tests/unit/network/drivers/neutron/test_allowed_address_pairs.py b/octavia/tests/unit/network/drivers/neutron/test_allowed_address_pairs.py index b5747a2cad..e3aa05d051 100644 --- a/octavia/tests/unit/network/drivers/neutron/test_allowed_address_pairs.py +++ b/octavia/tests/unit/network/drivers/neutron/test_allowed_address_pairs.py @@ -122,6 +122,28 @@ class TestAllowedAddressPairsDriver(base.TestCase): delete_port.assert_has_calls(calls, any_order=True) delete_sec_grp.assert_called_once_with(sec_grp_id) + def test_deallocate_vip_no_vrrp_port(self): + lb = dmh.generate_load_balancer_tree() + lb.vip.load_balancer = lb + # amphora 0 doesn't have a vrrp_port_id + lb.amphorae[0].vrrp_port_id = None + vip = lb.vip + sec_grp_id = 'lb-sec-grp1' + show_port = self.driver.network_proxy.get_port + show_port.return_value = Port( + device_owner=allowed_address_pairs.OCTAVIA_OWNER) + delete_port = self.driver.network_proxy.delete_port + delete_sec_grp = self.driver.network_proxy.delete_security_group + list_security_groups = self.driver.network_proxy.find_security_group + list_security_groups.return_value = SecurityGroup(id=sec_grp_id) + self.driver.deallocate_vip(vip) + # not called for lb.amphorae[0] + calls = [mock.call(vip.port_id), + mock.call(lb.amphorae[1].vrrp_port_id)] + delete_port.assert_has_calls(calls, any_order=True) + self.assertEqual(2, delete_port.call_count) + delete_sec_grp.assert_called_once_with(sec_grp_id) + def test_deallocate_vip_no_port(self): lb = dmh.generate_load_balancer_tree() lb.vip.load_balancer = lb diff --git a/releasenotes/notes/fix-error-on-delete-with-broken-amp-10d7f4e85754d7ee.yaml b/releasenotes/notes/fix-error-on-delete-with-broken-amp-10d7f4e85754d7ee.yaml new file mode 100644 index 0000000000..398aa4db98 --- /dev/null +++ b/releasenotes/notes/fix-error-on-delete-with-broken-amp-10d7f4e85754d7ee.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixed a potential issue when deleting a load balancer with an amphora that + was not fully created, the deletion may have failed when deallocating the + VIP port, leaving the load balancer in ERROR state.