From 412fb302a69ed85f59323bd61f6766d0729b9534 Mon Sep 17 00:00:00 2001 From: johnsom Date: Wed, 8 Feb 2017 21:02:21 -0800 Subject: [PATCH] Fix a bug where ports may not be deleted A loop to delete vip ports had an improperly scoped try block that could, under the right circumstance lead to ports not getting deleted. This patch fixes that scoping. Co-Authored-By: Adam Harwell Change-Id: I162eed5e48b361ed9f4c5b10edcf90bd8a7d4818 --- .../drivers/neutron/allowed_address_pairs.py | 14 +++++++------- octavia/tests/common/data_model_helpers.py | 2 +- .../drivers/neutron/test_allowed_address_pairs.py | 5 ++++- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/octavia/network/drivers/neutron/allowed_address_pairs.py b/octavia/network/drivers/neutron/allowed_address_pairs.py index 3de317ae11..c71634919c 100644 --- a/octavia/network/drivers/neutron/allowed_address_pairs.py +++ b/octavia/network/drivers/neutron/allowed_address_pairs.py @@ -264,14 +264,14 @@ class AllowedAddressPairsDriver(neutron_base.BaseNeutronDriver): This can happen if a failover has occurred. """ - try: - for amphora in six.moves.filter(self._filter_amphora, - vip.load_balancer.amphorae): + for amphora in six.moves.filter(self._filter_amphora, + vip.load_balancer.amphorae): + try: self.neutron_client.delete_port(amphora.vrrp_port_id) - except (neutron_client_exceptions.NotFound, - neutron_client_exceptions.PortNotFoundClient): - LOG.debug('VIP instance port {0} already deleted. ' - 'Skipping.'.format(amphora.vrrp_port_id)) + except (neutron_client_exceptions.NotFound, + neutron_client_exceptions.PortNotFoundClient): + LOG.debug('VIP instance port {0} already deleted. ' + 'Skipping.'.format(amphora.vrrp_port_id)) try: port = self.get_port(vip.port_id) diff --git a/octavia/tests/common/data_model_helpers.py b/octavia/tests/common/data_model_helpers.py index 11f32f2261..550e37d4f0 100644 --- a/octavia/tests/common/data_model_helpers.py +++ b/octavia/tests/common/data_model_helpers.py @@ -41,7 +41,6 @@ def generate_load_balancer(vip=None, amphorae=None): amp.load_balancer = lb amp.load_balancer_id = lb.id amp.status = constants.AMPHORA_ALLOCATED - amp.vrrp_port_id = 'vrrp_port-{0}-id'.format(VIP_SEED) if vip: vip.load_balancer = lb vip.load_balancer_id = lb.id @@ -74,6 +73,7 @@ def generate_amphora(load_balancer=None): status='ACTIVE', lb_network_ip='99.99.99.{0}'.format(AMP_SEED), vrrp_ip='55.55.55.{0}'.format(AMP_SEED), + vrrp_port_id='vrrp_port-{0}-id'.format(AMP_SEED), load_balancer=load_balancer) if load_balancer: amp.load_balancer_id = load_balancer.id 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 a0f83f395c..589d9aec27 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 @@ -120,7 +120,10 @@ class TestAllowedAddressPairsDriver(base.TestCase): } list_security_groups.return_value = security_groups self.driver.deallocate_vip(vip) - delete_port.assert_called_with(vip.port_id) + calls = [mock.call(vip.port_id)] + for amp in lb.amphorae: + calls.append(mock.call(amp.vrrp_port_id)) + delete_port.assert_has_calls(calls, any_order=True) delete_sec_grp.assert_called_once_with(sec_grp_id) def test_deallocate_vip_no_sec_group(self):