Merge "Fix LB failover when IP addresses exhausted" into stable/victoria

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(

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."""
 

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(

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."""
 

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

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,

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.