From 5a039fcd9fe6927d24cb00485869edf6cb1de629 Mon Sep 17 00:00:00 2001 From: Sergey Kraynev Date: Wed, 23 Oct 2024 15:24:50 +0400 Subject: [PATCH] Do not fail on revert PlugVIPAmphora due undefined db_lb - changed order: get db_lb before getting subnet info, to avoid useless message without db_lb.vip - added default db_lb value for case, when DB calls are failed. in this case error message will be generated without information about VIP Closes-Bug: 2085427 Change-Id: I97acfbd9b26fefbc38b61e36f72f483072390b23 (cherry picked from commit 7c08b63a37e7ef990909d656683d098971228731) --- .../worker/v2/tasks/network_tasks.py | 18 +++---- .../worker/v2/tasks/test_network_tasks.py | 51 +++++++++++++++++++ ...b_on_plug_vip_revert-5c24af124498b246.yaml | 7 +++ 3 files changed, 67 insertions(+), 9 deletions(-) create mode 100644 releasenotes/notes/handle_empty_db_lb_on_plug_vip_revert-5c24af124498b246.yaml diff --git a/octavia/controller/worker/v2/tasks/network_tasks.py b/octavia/controller/worker/v2/tasks/network_tasks.py index 4c83d8722..f4c3abaf9 100644 --- a/octavia/controller/worker/v2/tasks/network_tasks.py +++ b/octavia/controller/worker/v2/tasks/network_tasks.py @@ -536,11 +536,10 @@ def revert(self, result, loadbalancer, amphora, subnet, *args, **kwargs): """Handle a failure to plumb a vip.""" if isinstance(result, failure.Failure): return + lb_id = loadbalancer[constants.LOADBALANCER_ID] LOG.warning("Unable to plug VIP for amphora id %s " "load balancer id %s", - amphora.get(constants.ID), - loadbalancer[constants.LOADBALANCER_ID]) - + amphora.get(constants.ID), lb_id) try: session = db_apis.get_session() with session.begin(): @@ -550,15 +549,16 @@ def revert(self, result, loadbalancer, amphora, subnet, *args, **kwargs): db_amp.ha_port_id = result[constants.HA_PORT_ID] db_subnet = self.network_driver.get_subnet( subnet[constants.ID]) - db_lb = self.loadbalancer_repo.get( - session, - id=loadbalancer[constants.LOADBALANCER_ID]) - + db_lb = self.loadbalancer_repo.get(session, id=lb_id) self.network_driver.unplug_aap_port(db_lb.vip, db_amp, db_subnet) except Exception as e: - LOG.error('Failed to unplug AAP port. Resources may still be in ' - 'use for VIP: %s due to error: %s', db_lb.vip, str(e)) + LOG.error( + 'Failed to unplug AAP port for load balancer: %s. ' + 'Resources may still be in use for VRRP port: %s. ' + 'Due to error: %s', + lb_id, result[constants.VRRP_PORT_ID], str(e) + ) class UnplugVIP(BaseNetworkTask): diff --git a/octavia/tests/unit/controller/worker/v2/tasks/test_network_tasks.py b/octavia/tests/unit/controller/worker/v2/tasks/test_network_tasks.py index 7b8afb282..7c117b86b 100644 --- a/octavia/tests/unit/controller/worker/v2/tasks/test_network_tasks.py +++ b/octavia/tests/unit/controller/worker/v2/tasks/test_network_tasks.py @@ -1526,6 +1526,57 @@ def test_revert_plug_vip_amphora(self, mock_session, mock_lb_get, mock_get, mock_driver.unplug_aap_port.assert_called_once_with( LB.vip, self.db_amphora_mock, mockSubnet) + @mock.patch('octavia.db.repositories.AmphoraRepository.get') + @mock.patch('octavia.db.repositories.LoadBalancerRepository.get') + @mock.patch('octavia.db.api.get_session', return_value=_session_mock) + def test_revert_plug_vip_amphora_subnet_not_found( + self, mock_session, mock_lb_get, mock_get, mock_get_net_driver): + mock_driver = mock.MagicMock() + mock_lb_get.return_value = LB + mock_get.return_value = self.db_amphora_mock + mock_get_net_driver.return_value = mock_driver + net = network_tasks.PlugVIPAmphora() + amphora = {constants.ID: AMPHORA_ID, + constants.LB_NETWORK_IP: IP_ADDRESS} + subnet = {constants.ID: SUBNET_ID} + err_msg = 'Subnet not found' + mock_driver.get_subnet.side_effect = net_base.SubnetNotFound(err_msg) + result = AMPS_DATA[0].to_dict() + net.revert(result, self.load_balancer_mock, amphora, subnet) + mock_driver.unplug_aap_port.assert_not_called() + network_tasks.LOG.error.assert_called_once_with( + 'Failed to unplug AAP port for load balancer: %s. ' + 'Resources may still be in use for VRRP port: %s. ' + 'Due to error: %s', + self.load_balancer_mock[constants.LOADBALANCER_ID], + result[constants.VRRP_PORT_ID], err_msg + ) + + @mock.patch('octavia.db.repositories.AmphoraRepository.get') + @mock.patch('octavia.db.repositories.LoadBalancerRepository.get') + @mock.patch('octavia.db.api.get_session', return_value=_session_mock) + def test_revert_plug_vip_amphora_raise_db_error( + self, mock_session, mock_lb_get, mock_get, mock_get_net_driver): + mock_driver = mock.MagicMock() + mock_lb_get.return_value = LB + err_msg = 'Some Error' + mock_get.side_effect = Exception(err_msg) + net = network_tasks.PlugVIPAmphora() + amphora = {constants.ID: AMPHORA_ID, + constants.LB_NETWORK_IP: IP_ADDRESS} + subnet = {constants.ID: SUBNET_ID} + result = AMPS_DATA[0].to_dict() + net.revert(result, self.load_balancer_mock, amphora, subnet) + mock_driver.unplug_aap_port.assert_not_called() + mock_lb_get.assert_not_called() + network_tasks.LOG.error.assert_called_once_with( + 'Failed to unplug AAP port for load balancer: %s. ' + 'Resources may still be in use for VRRP port: %s. ' + 'Due to error: %s', + self.load_balancer_mock[constants.LOADBALANCER_ID], + result[constants.VRRP_PORT_ID], err_msg + ) + @mock.patch('octavia.controller.worker.v2.tasks.network_tasks.DeletePort.' 'update_progress') def test_delete_port(self, mock_update_progress, mock_get_net_driver): diff --git a/releasenotes/notes/handle_empty_db_lb_on_plug_vip_revert-5c24af124498b246.yaml b/releasenotes/notes/handle_empty_db_lb_on_plug_vip_revert-5c24af124498b246.yaml new file mode 100644 index 000000000..cd0396376 --- /dev/null +++ b/releasenotes/notes/handle_empty_db_lb_on_plug_vip_revert-5c24af124498b246.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fix error on revert PlugVIPAmphora task, when db_lb is not defined + and get_subnet raises NotFound error. It could happen when Amphora + creation failed by timeout and before it VIP network was removed. + As result revert failed with exception.