Merge pull request #16 from stackhpc/upstream/2024.1-2024-11-11

Synchronise 2024.1 with upstream

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):

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):

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.