From 8b196bb3bb05c9d87dae688750457dbb944d4d1b Mon Sep 17 00:00:00 2001
From: Gregory Thiemonge <gthiemon@redhat.com>
Date: Mon, 17 Jul 2023 11:02:08 +0200
Subject: [PATCH] Fix amphorav1 member deletion bug

When the worker calculates the delta for plugging/unplugging subnets,
it need to check the provisioning_status of the members. But in
amphorav1, this status comes from the LB DB object that is passed to
taskflow. If the provisioning_status is modified by another task
(MarkMemberPendingDeleteInDb), the LB object is not updated.
We need to reload it from the DB to get the current status of the
members.

Note: this patch is for stable branches only as the amphorav1 driver was
      removed during the Bobcat cycle
Closes-Bug: #2027967
Change-Id: Iea0ffee1a7307b0852c5a9faf31739c96e76a7ee
---
 .../worker/v1/tasks/network_tasks.py          |  5 +++++
 .../worker/v1/tasks/test_network_tasks.py     | 19 ++++++++++++++++---
 ...horav1-subnet-member-9921d1ba387ff975.yaml |  6 ++++++
 3 files changed, 27 insertions(+), 3 deletions(-)
 create mode 100644 releasenotes/notes/fix-amphorav1-subnet-member-9921d1ba387ff975.yaml

diff --git a/octavia/controller/worker/v1/tasks/network_tasks.py b/octavia/controller/worker/v1/tasks/network_tasks.py
index 0c49d3cebd..64b7352681 100644
--- a/octavia/controller/worker/v1/tasks/network_tasks.py
+++ b/octavia/controller/worker/v1/tasks/network_tasks.py
@@ -70,6 +70,11 @@ class CalculateAmphoraDelta(BaseNetworkTask):
         else:
             management_nets = CONF.controller_worker.amp_boot_network_list
 
+        # Reload the load balancer, the provisioning status of the members may
+        # have been updated by a previous task
+        loadbalancer = self.lb_repo.get(
+            db_apis.get_session(), id=loadbalancer.id)
+
         desired_subnet_to_net_map = {}
         for mgmt_net_id in management_nets:
             for subnet_id in self.network_driver.get_network(
diff --git a/octavia/tests/unit/controller/worker/v1/tasks/test_network_tasks.py b/octavia/tests/unit/controller/worker/v1/tasks/test_network_tasks.py
index 7fd00f7d35..a8cec9d736 100644
--- a/octavia/tests/unit/controller/worker/v1/tasks/test_network_tasks.py
+++ b/octavia/tests/unit/controller/worker/v1/tasks/test_network_tasks.py
@@ -65,6 +65,7 @@ AMPS_DATA = [o_data_models.Amphora(id=t_constants.MOCK_AMP_ID1,
                                    vrrp_ip=t_constants.MOCK_VRRP_IP3)
              ]
 UPDATE_DICT = {constants.TOPOLOGY: None}
+_session_mock = mock.MagicMock()
 
 
 class TestException(Exception):
@@ -98,7 +99,10 @@ class TestNetworkTasks(base.TestCase):
         conf.config(group="networking", max_retries=1)
         super().setUp()
 
-    def test_calculate_amphora_delta(self, mock_get_net_driver):
+    @mock.patch('octavia.db.repositories.LoadBalancerRepository.get')
+    @mock.patch('octavia.db.api.get_session', return_value=_session_mock)
+    def test_calculate_amphora_delta(self, mock_get_session, mock_lb_repo_get,
+                                     mock_get_net_driver):
         VRRP_PORT_ID = uuidutils.generate_uuid()
         VIP_NETWORK_ID = uuidutils.generate_uuid()
         VIP_SUBNET_ID = uuidutils.generate_uuid()
@@ -155,6 +159,7 @@ class TestNetworkTasks(base.TestCase):
             id=mock.Mock(),
             network_id=DELETE_NETWORK_ID)
 
+        mock_lb_repo_get.return_value = lb_mock
         mock_driver.get_port.return_value = vrrp_port
         mock_driver.get_subnet.return_value = member_subnet
         mock_driver.get_network.return_value = mgmt_net
@@ -177,8 +182,12 @@ class TestNetworkTasks(base.TestCase):
         mock_driver.get_subnet.assert_called_once_with(MEMBER_SUBNET_ID)
         mock_driver.get_plugged_networks.assert_called_once_with(COMPUTE_ID)
 
-    def test_calculate_delta(self, mock_get_net_driver):
+    @mock.patch('octavia.db.repositories.LoadBalancerRepository.get')
+    @mock.patch('octavia.db.api.get_session', return_value=_session_mock)
+    def test_calculate_delta(self, mock_get_session, mock_get_lb,
+                             mock_get_net_driver):
         mock_driver = mock.MagicMock()
+        mock_get_lb.return_value = self.load_balancer_mock
         mock_get_net_driver.return_value = mock_driver
         empty_deltas = {self.amphora_mock.id: data_models.Delta(
             amphora_id=self.amphora_mock.id,
@@ -572,7 +581,11 @@ class TestNetworkTasks(base.TestCase):
         self.assertEqual({self.amphora_mock.id: ndm},
                          calc_delta.execute(self.load_balancer_mock, {}))
 
-    def test_calculate_delta_ipv6_ipv4_subnets(self, mock_get_net_driver):
+    @mock.patch('octavia.db.repositories.LoadBalancerRepository.get')
+    @mock.patch('octavia.db.api.get_session', return_value=_session_mock)
+    def test_calculate_delta_ipv6_ipv4_subnets(self, mock_get_session,
+                                               mock_get_lb,
+                                               mock_get_net_driver):
         mock_driver = mock.MagicMock()
         mock_get_net_driver.return_value = mock_driver
 
diff --git a/releasenotes/notes/fix-amphorav1-subnet-member-9921d1ba387ff975.yaml b/releasenotes/notes/fix-amphorav1-subnet-member-9921d1ba387ff975.yaml
new file mode 100644
index 0000000000..2a55366102
--- /dev/null
+++ b/releasenotes/notes/fix-amphorav1-subnet-member-9921d1ba387ff975.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixed a bug in amphorav1, the subnet of a member that was being deleted was
+    not immediately unplugged from the amphora, but only during the next update
+    of the members.