From 48acc66c73d19837aeb1a44dc7949c0390406076 Mon Sep 17 00:00:00 2001 From: Gregory Thiemonge 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 (cherry picked from commit 8b196bb3bb05c9d87dae688750457dbb944d4d1b) (cherry picked from commit 68a3379cda266fac649ef984404090d9cfa246ac) (cherry picked from commit 13070bd173138189c421c92078bf34ccff7697ab) --- .../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 1fbd636cf3..eb4e02a9b9 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 c51c2bae67..5f808cf5ef 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.