From 7c08b63a37e7ef990909d656683d098971228731 Mon Sep 17 00:00:00 2001
From: Sergey Kraynev <sergejyit@gmail.com>
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
---
 .../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 904dee9357..2c8d8ca18e 100644
--- a/octavia/controller/worker/v2/tasks/network_tasks.py
+++ b/octavia/controller/worker/v2/tasks/network_tasks.py
@@ -536,11 +536,10 @@ class PlugVIPAmphora(BaseNetworkTask):
         """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 @@ class PlugVIPAmphora(BaseNetworkTask):
                 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 f40bbdd069..5cf1daad8e 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 @@ class TestNetworkTasks(base.TestCase):
         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 0000000000..cd03963765
--- /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.