Fixes a revert failure in AllocateVIP task

The AllocateVIP task had an unhandled exception in the revert
method that would cause a revert flow to fail.  This left the
load balancer in a PENDING_CREATE state instead of the desired
ERROR end state. Two other revert instances also have this issue.

This patch catches the exceptions and logs the
situation, warning the operator that there may be orphaned objects.

A follow on patch will attempt to address the orphaned objects issue
tracked by: https://bugs.launchpad.net/octavia/+bug/1624016

A follow on patch will address database repository calls with unhandled
exceptions in the revert methods.
Tracked by: https://bugs.launchpad.net/octavia/+bug/1624047

Change-Id: I267b0e884262678c67ccedb16251f12edcbdeb87
Closes-Bug: #1623737

octavia/controller/worker/tasks/compute_tasks.py

@@ -212,7 +212,13 @@ class NovaServerGroupCreate(BaseComputeTask):
         server_group_id = result
         LOG.warning(_LW("Reverting server group create with id:%s"),
                     server_group_id)
-        self.compute.delete_server_group(server_group_id)
+        try:
+            self.compute.delete_server_group(server_group_id)
+        except Exception as e:
+            LOG.error(_LE("Failed to delete server group.  Resources may "
+                          "still be in use for server group: %(sg)s due to "
+                          "error: %(except)s"),
+                      {'sg': server_group_id, 'except': e})
 
 
 class NovaServerGroupDelete(BaseComputeTask):

octavia/controller/worker/tasks/network_tasks.py

@@ -284,7 +284,13 @@ class PlugVIP(BaseNetworkTask):
         LOG.warning(_LW("Unable to plug VIP for loadbalancer id %s"),
                     loadbalancer.id)
 
-        self.network_driver.unplug_vip(loadbalancer, loadbalancer.vip)
+        try:
+            self.network_driver.unplug_vip(loadbalancer, loadbalancer.vip)
+        except Exception as e:
+            LOG.error(_LE("Failed to unplug VIP.  Resources may still "
+                          "be in use from vip: %(vip)s due to "
+                          "error: %(except)s"),
+                      {'vip': loadbalancer.vip.ip_address, 'except': e})
 
 
 class UnplugVIP(BaseNetworkTask):
@@ -322,7 +328,13 @@ class AllocateVIP(BaseNetworkTask):
             return
         vip = result
         LOG.warning(_LW("Deallocating vip %s"), vip.ip_address)
-        self.network_driver.deallocate_vip(vip)
+        try:
+            self.network_driver.deallocate_vip(vip)
+        except Exception as e:
+            LOG.error(_LE("Failed to deallocate VIP.  Resources may still "
+                          "be in use from vip: %(vip)s due to "
+                          "error: %(except)s"),
+                      {'vip': vip.ip_address, 'except': e})
 
 
 class DeallocateVIP(BaseNetworkTask):

octavia/tests/unit/controller/worker/tasks/test_compute_tasks.py

@@ -398,7 +398,12 @@ class TestComputeTasks(base.TestCase):
 
         # Validate that the delete_server_group method was called properly
         mock_driver.delete_server_group.assert_called_once_with(sg_id)
+
+        # Test revert with exception
+        mock_driver.reset_mock()
+        mock_driver.delete_server_group.side_effect = Exception('DelSGExcept')
         nova_sever_group_obj.revert(sg_id)
+        mock_driver.delete_server_group.assert_called_once_with(sg_id)
 
     @mock.patch('stevedore.driver.DriverManager.driver')
     def test_nova_server_group_delete_with_sever_group_id(self, mock_driver):

octavia/tests/unit/controller/worker/tasks/test_network_tasks.py

@@ -373,6 +373,12 @@ class TestNetworkTasks(base.TestCase):
         net.revert(["vip"], LB)
         mock_driver.unplug_vip.assert_called_once_with(LB, LB.vip)
 
+        # revert with exception
+        mock_driver.reset_mock()
+        mock_driver.unplug_vip.side_effect = Exception('UnplugVipException')
+        net.revert(["vip"], LB)
+        mock_driver.unplug_vip.assert_called_once_with(LB, LB.vip)
+
     def test_unplug_vip(self, mock_get_net_driver):
         mock_driver = mock.MagicMock()
         mock_get_net_driver.return_value = mock_driver
@@ -391,11 +397,19 @@ class TestNetworkTasks(base.TestCase):
         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_called_once_with(vip_mock)
 
+        # revert exception
+        mock_driver.reset_mock()
+        mock_driver.deallocate_vip.side_effect = Exception('DeallVipException')
+        vip_mock = mock.MagicMock()
+        net.revert(vip_mock, LB)
+        mock_driver.deallocate_vip.assert_called_once_with(vip_mock)
+
     def test_deallocate_vip(self, mock_get_net_driver):
         mock_driver = mock.MagicMock()
         mock_get_net_driver.return_value = mock_driver