From e3ab49c60aa835cb4034568ea25f06472127388c Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Thu, 4 Mar 2021 23:47:15 +0000 Subject: [PATCH] Fix LB failover when IP addresses exhausted When a load balancer failover was performed on a load balancer where the VIP address is on a subnet that has no IP addresses available, the VIP address may be deactivated. This patch corrects the failover flow to not deallocate the VIP address on a failover revert flow due to the subnet being out of IP addresses. Story: 2008625 Task: 41827 Change-Id: I1fe342d2bdf1301dd89ab7dfaa8e6a23e69c252b --- .../worker/v1/flows/load_balancer_flows.py | 4 ++-- .../worker/v1/tasks/network_tasks.py | 14 +++++++++++++ .../worker/v2/flows/load_balancer_flows.py | 4 ++-- .../worker/v2/tasks/network_tasks.py | 14 +++++++++++++ .../worker/v1/tasks/test_network_tasks.py | 16 ++++++++++++++ .../worker/v2/tasks/test_network_tasks.py | 21 +++++++++++++++++++ ...-addresses-exhausted-69110b2fa4683e1a.yaml | 5 +++++ 7 files changed, 74 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/Fix-failover-ip-addresses-exhausted-69110b2fa4683e1a.yaml diff --git a/octavia/controller/worker/v1/flows/load_balancer_flows.py b/octavia/controller/worker/v1/flows/load_balancer_flows.py index 313cdc0238..ead870b894 100644 --- a/octavia/controller/worker/v1/flows/load_balancer_flows.py +++ b/octavia/controller/worker/v1/flows/load_balancer_flows.py @@ -421,8 +421,8 @@ class LoadBalancerFlows(object): # Check that the VIP port exists and is ok failover_LB_flow.add( - network_tasks.AllocateVIP(requires=constants.LOADBALANCER, - provides=constants.VIP)) + network_tasks.AllocateVIPforFailover( + requires=constants.LOADBALANCER, provides=constants.VIP)) # Update the database with the VIP information failover_LB_flow.add(database_tasks.UpdateVIPAfterAllocation( diff --git a/octavia/controller/worker/v1/tasks/network_tasks.py b/octavia/controller/worker/v1/tasks/network_tasks.py index 54903f9c2d..24d8ecfe6a 100644 --- a/octavia/controller/worker/v1/tasks/network_tasks.py +++ b/octavia/controller/worker/v1/tasks/network_tasks.py @@ -469,6 +469,20 @@ class AllocateVIP(BaseNetworkTask): {'vip': vip.ip_address, 'except': str(e)}) +class AllocateVIPforFailover(AllocateVIP): + """Task to allocate/validate the VIP for a failover flow.""" + + def revert(self, result, loadbalancer, *args, **kwargs): + """Handle a failure to allocate vip.""" + + if isinstance(result, failure.Failure): + LOG.exception("Unable to allocate VIP") + return + vip = result + LOG.info("Failover revert is not deallocating vip %s because this is " + "a failover.", vip.ip_address) + + class DeallocateVIP(BaseNetworkTask): """Task to deallocate a VIP.""" diff --git a/octavia/controller/worker/v2/flows/load_balancer_flows.py b/octavia/controller/worker/v2/flows/load_balancer_flows.py index 17f5be9760..446b47fddb 100644 --- a/octavia/controller/worker/v2/flows/load_balancer_flows.py +++ b/octavia/controller/worker/v2/flows/load_balancer_flows.py @@ -406,8 +406,8 @@ class LoadBalancerFlows(object): # Check that the VIP port exists and is ok failover_LB_flow.add( - network_tasks.AllocateVIP(requires=constants.LOADBALANCER, - provides=constants.VIP)) + network_tasks.AllocateVIPforFailover( + requires=constants.LOADBALANCER, provides=constants.VIP)) # Update the database with the VIP information failover_LB_flow.add(database_tasks.UpdateVIPAfterAllocation( diff --git a/octavia/controller/worker/v2/tasks/network_tasks.py b/octavia/controller/worker/v2/tasks/network_tasks.py index 12a926886f..c620c46241 100644 --- a/octavia/controller/worker/v2/tasks/network_tasks.py +++ b/octavia/controller/worker/v2/tasks/network_tasks.py @@ -512,6 +512,20 @@ class AllocateVIP(BaseNetworkTask): {'vip': vip.ip_address, 'except': str(e)}) +class AllocateVIPforFailover(AllocateVIP): + """Task to allocate/validate the VIP for a failover flow.""" + + def revert(self, result, loadbalancer, *args, **kwargs): + """Handle a failure to allocate vip.""" + + if isinstance(result, failure.Failure): + LOG.exception("Unable to allocate VIP") + return + vip = data_models.Vip(**result) + LOG.info("Failover revert is not deallocating vip %s because this is " + "a failover.", vip.ip_address) + + class DeallocateVIP(BaseNetworkTask): """Task to deallocate a VIP.""" 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 adc21508e7..97d4cf1434 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 @@ -716,6 +716,22 @@ class TestNetworkTasks(base.TestCase): net.revert(vip_mock, LB) mock_driver.deallocate_vip.assert_called_once_with(vip_mock) + def test_allocate_vip_for_failover(self, mock_get_net_driver): + mock_driver = mock.MagicMock() + mock_get_net_driver.return_value = mock_driver + net = network_tasks.AllocateVIPforFailover() + + mock_driver.allocate_vip.return_value = LB.vip + + mock_driver.reset_mock() + self.assertEqual(LB.vip, net.execute(LB)) + mock_driver.allocate_vip.assert_called_once_with(LB) + + # revert + vip_mock = mock.MagicMock() + net.revert(vip_mock, LB) + mock_driver.deallocate_vip.assert_not_called() + def test_deallocate_vip(self, mock_get_net_driver): mock_driver = mock.MagicMock() mock_get_net_driver.return_value = mock_driver 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 b9e3fa32cd..f4b94ca7c2 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 @@ -812,6 +812,27 @@ class TestNetworkTasks(base.TestCase): mock_driver.deallocate_vip.assert_called_once_with(o_data_models.Vip( **vip_mock)) + @mock.patch('octavia.db.repositories.LoadBalancerRepository.get') + @mock.patch('octavia.db.api.get_session', return_value=_session_mock) + def test_allocate_vip_for_failover(self, mock_get_session, mock_get_lb, + mock_get_net_driver): + mock_driver = mock.MagicMock() + mock_get_lb.return_value = LB + mock_get_net_driver.return_value = mock_driver + net = network_tasks.AllocateVIPforFailover() + + mock_driver.allocate_vip.return_value = LB.vip + + mock_driver.reset_mock() + self.assertEqual(LB.vip.to_dict(), + net.execute(self.load_balancer_mock)) + mock_driver.allocate_vip.assert_called_once_with(LB) + + # revert + vip_mock = VIP.to_dict() + net.revert(vip_mock, self.load_balancer_mock) + mock_driver.deallocate_vip.assert_not_called() + @mock.patch('octavia.db.repositories.LoadBalancerRepository.get') @mock.patch('octavia.db.api.get_session', return_value=_session_mock) def test_deallocate_vip(self, mock_get_session, mock_get_lb, diff --git a/releasenotes/notes/Fix-failover-ip-addresses-exhausted-69110b2fa4683e1a.yaml b/releasenotes/notes/Fix-failover-ip-addresses-exhausted-69110b2fa4683e1a.yaml new file mode 100644 index 0000000000..692822f833 --- /dev/null +++ b/releasenotes/notes/Fix-failover-ip-addresses-exhausted-69110b2fa4683e1a.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixes an issue with load balancer failover, when the VIP subnet is out of + IP addresses, that could lead to the VIP being deallocated.