Fixes the revert methods with unhandled exceptions

A number of the revert methods in our flow tasks make database
repository calls but do not handle potential exceptions.  This can
lead to the revert flow aborting at these tasks.

This patch adds exception handling to these repository calls inside
the revert methods.

Change-Id: Ia9b07890fe2c56382041175018679b87298c1b10
Closes-Bug: #1624047

octavia/controller/worker/tasks/amphora_driver_tasks.py

@@ -24,7 +24,7 @@ from taskflow.types import failure
 from octavia.common import constants
 from octavia.db import api as db_apis
 from octavia.db import repositories as repo
-from octavia.i18n import _LW
+from octavia.i18n import _LE, _LW
 
 CONF = cfg.CONF
 CONF.import_group('controller_worker', 'octavia.common.config')
@@ -45,6 +45,57 @@ class BaseAmphoraTask(task.Task):
         self.listener_repo = repo.ListenerRepository()
         self.loadbalancer_repo = repo.LoadBalancerRepository()
 
+    def _mark_amphora_status_error(self, amphora_id):
+        """Sets an amphora status to ERROR.
+
+        NOTE: This should only be called from revert methods.
+
+        :param amphora_id: Amphora ID to set the status to ERROR
+        """
+        try:
+            self.amphora_repo.update(db_apis.get_session(), id=amphora_id,
+                                     status=constants.ERROR)
+        except Exception as e:
+            LOG.error(_LE("Failed to update amphora %(amp)s "
+                          "status to ERROR due to: "
+                          "%(except)s"), {'amp': amphora_id,
+                                          'except': e})
+
+    def _mark_listener_prov_status_error(self, listener_id):
+        """Sets a listener provisioning status to ERROR.
+
+        NOTE: This should only be called from revert methods.
+
+        :param listener_id: Listener ID to set provisioning status to ERROR
+        """
+        try:
+            self.listener_repo.update(db_apis.get_session(),
+                                      id=listener_id,
+                                      provisioning_status=constants.ERROR)
+        except Exception as e:
+            LOG.error(_LE("Failed to update listener %(list)s "
+                          "provisioning status to ERROR due to: "
+                          "%(except)s"), {'list': listener_id,
+                                          'except': e})
+
+    def _mark_loadbalancer_prov_status_error(self, loadbalancer_id):
+        """Sets a load balancer provisioning status to ERROR.
+
+        NOTE: This should only be called from revert methods.
+
+        :param loadbalancer_id: Load balancer ID to set provisioning
+                                status to ERROR
+        """
+        try:
+            self.loadbalancer_repo.update(db_apis.get_session(),
+                                          id=loadbalancer_id,
+                                          provisioning_status=constants.ERROR)
+        except Exception as e:
+            LOG.error(_LE("Failed to update load balancer %(lb)s "
+                          "provisioning status to ERROR due to: "
+                          "%(except)s"), {'lb': loadbalancer_id,
+                                          'except': e})
+
 
 class ListenersUpdate(BaseAmphoraTask):
     """Task to update amphora with all specified listeners' configurations."""
@@ -59,14 +110,10 @@ class ListenersUpdate(BaseAmphoraTask):
         """Handle failed listeners updates."""
 
         LOG.warning(_LW("Reverting listeners updates."))
+
         for listener in loadbalancer.listeners:
-            try:
+            self._mark_listener_prov_status_error(listener.id)
-                self.listener_repo.update(db_apis.get_session(),
+
-                                          id=listener.id,
-                                          provisioning_status=constants.ERROR)
-            except Exception:
-                LOG.warning(_LW("Failed to update listener %s provisioning "
-                                "status..."), listener.id)
         return None
 
 
@@ -82,8 +129,9 @@ class ListenerStop(BaseAmphoraTask):
         """Handle a failed listener stop."""
 
         LOG.warning(_LW("Reverting listener stop."))
-        self.listener_repo.update(db_apis.get_session(), id=listener.id,
+
-                                  provisioning_status=constants.ERROR)
+        self._mark_listener_prov_status_error(listener.id)
+
         return None
 
 
@@ -99,8 +147,9 @@ class ListenerStart(BaseAmphoraTask):
         """Handle a failed listener start."""
 
         LOG.warning(_LW("Reverting listener start."))
-        self.listener_repo.update(db_apis.get_session(), id=listener.id,
+
-                                  provisioning_status=constants.ERROR)
+        self._mark_listener_prov_status_error(listener.id)
+
         return None
 
 
@@ -118,13 +167,8 @@ class ListenersStart(BaseAmphoraTask):
 
         LOG.warning(_LW("Reverting listeners starts."))
         for listener in listeners:
-            try:
+            self._mark_listener_prov_status_error(listener.id)
-                self.listener_repo.update(db_apis.get_session(),
+
-                                          id=listener.id,
-                                          provisioning_status=constants.ERROR)
-            except Exception:
-                LOG.warning(_LW("Failed to update listener %s provisioning "
-                                "status..."), listener.id)
         return None
 
 
@@ -140,8 +184,8 @@ class ListenerDelete(BaseAmphoraTask):
         """Handle a failed listener delete."""
 
         LOG.warning(_LW("Reverting listener delete."))
-        self.listener_repo.update(db_apis.get_session(), id=listener.id,
+
-                                  provisioning_status=constants.ERROR)
+        self._mark_listener_prov_status_error(listener.id)
 
 
 class AmphoraGetInfo(BaseAmphoraTask):
@@ -173,8 +217,7 @@ class AmphoraFinalize(BaseAmphoraTask):
         if isinstance(result, failure.Failure):
             return
         LOG.warning(_LW("Reverting amphora finalize."))
-        self.amphora_repo.update(db_apis.get_session(), id=amphora.id,
+        self._mark_amphora_status_error(amphora.id)
-                                 status=constants.ERROR)
 
 
 class AmphoraPostNetworkPlug(BaseAmphoraTask):
@@ -193,8 +236,7 @@ class AmphoraPostNetworkPlug(BaseAmphoraTask):
         if isinstance(result, failure.Failure):
             return
         LOG.warning(_LW("Reverting post network plug."))
-        self.amphora_repo.update(db_apis.get_session(), id=amphora.id,
+        self._mark_amphora_status_error(amphora.id)
-                                 status=constants.ERROR)
 
 
 class AmphoraePostNetworkPlug(BaseAmphoraTask):
@@ -216,8 +258,7 @@ class AmphoraePostNetworkPlug(BaseAmphoraTask):
             lambda amp: amp.status == constants.AMPHORA_ALLOCATED,
                 loadbalancer.amphorae):
 
-            self.amphora_repo.update(db_apis.get_session(), id=amphora.id,
+            self._mark_amphora_status_error(amphora.id)
-                                     status=constants.ERROR)
 
 
 class AmphoraPostVIPPlug(BaseAmphoraTask):
@@ -234,11 +275,8 @@ class AmphoraPostVIPPlug(BaseAmphoraTask):
         if isinstance(result, failure.Failure):
             return
         LOG.warning(_LW("Reverting post vip plug."))
-        self.amphora_repo.update(db_apis.get_session(), id=amphora.id,
+        self._mark_amphora_status_error(amphora.id)
-                                 status=constants.ERROR)
+        self._mark_loadbalancer_prov_status_error(loadbalancer.id)
-        self.loadbalancer_repo.update(db_apis.get_session(),
-                                      id=loadbalancer.id,
-                                      provisioning_status=constants.ERROR)
 
 
 class AmphoraePostVIPPlug(BaseAmphoraTask):
@@ -257,9 +295,7 @@ class AmphoraePostVIPPlug(BaseAmphoraTask):
         if isinstance(result, failure.Failure):
             return
         LOG.warning(_LW("Reverting amphorae post vip plug."))
-        self.loadbalancer_repo.update(db_apis.get_session(),
+        self._mark_loadbalancer_prov_status_error(loadbalancer.id)
-                                      id=loadbalancer.id,
-                                      provisioning_status=constants.ERROR)
 
 
 class AmphoraCertUpload(BaseAmphoraTask):
@@ -298,8 +334,14 @@ class AmphoraUpdateVRRPInterface(BaseAmphoraTask):
             lambda amp: amp.status == constants.AMPHORA_ALLOCATED,
                 loadbalancer.amphorae):
 
-            self.amphora_repo.update(db_apis.get_session(), amp.id,
+            try:
-                                     vrrp_interface=None)
+                self.amphora_repo.update(db_apis.get_session(), amp.id,
+                                         vrrp_interface=None)
+            except Exception as e:
+                LOG.error(_LE("Failed to update amphora %(amp)s "
+                              "VRRP interface to None due to: "
+                              "%(except)s"), {'amp': amp.id,
+                                              'except': e})
 
 
 class AmphoraVRRPUpdate(BaseAmphoraTask):

octavia/controller/worker/tasks/database_tasks.py

@@ -28,7 +28,7 @@ from octavia.common import data_models
 import octavia.common.tls_utils.cert_parser as cert_parser
 from octavia.db import api as db_apis
 from octavia.db import repositories as repo
-from octavia.i18n import _LI, _LW
+from octavia.i18n import _LE, _LI, _LW
 
 CONF = cfg.CONF
 LOG = logging.getLogger(__name__)
@@ -80,6 +80,57 @@ class BaseDatabaseTask(task.Task):
             LOG.debug('No existing amphora health record to mark busy '
                       'for amphora: %s, skipping.', amphora_id)
 
+    def _mark_amphora_status_error(self, amphora_id):
+        """Sets an amphora status to ERROR.
+
+        NOTE: This should only be called from revert methods.
+
+        :param amphora_id: Amphora ID to set the status to ERROR
+        """
+        try:
+            self.amphora_repo.update(db_apis.get_session(), id=amphora_id,
+                                     status=constants.ERROR)
+        except Exception as e:
+            LOG.error(_LE("Failed to update amphora %(amp)s "
+                          "status to ERROR due to: "
+                          "%(except)s"), {'amp': amphora_id,
+                                          'except': e})
+
+    def _mark_listener_prov_status_error(self, listener_id):
+        """Sets a listener provisioning status to ERROR.
+
+        NOTE: This should only be called from revert methods.
+
+        :param listener_id: Listener ID to set provisioning status to ERROR
+        """
+        try:
+            self.listener_repo.update(db_apis.get_session(),
+                                      id=listener_id,
+                                      provisioning_status=constants.ERROR)
+        except Exception as e:
+            LOG.error(_LE("Failed to update listener %(list)s "
+                          "provisioning status to ERROR due to: "
+                          "%(except)s"), {'list': listener_id,
+                                          'except': e})
+
+    def _mark_loadbalancer_prov_status_error(self, loadbalancer_id):
+        """Sets a load balancer provisioning status to ERROR.
+
+        NOTE: This should only be called from revert methods.
+
+        :param loadbalancer_id: Load balancer ID to set provisioning
+                                status to ERROR
+        """
+        try:
+            self.loadbalancer_repo.update(db_apis.get_session(),
+                                          id=loadbalancer_id,
+                                          provisioning_status=constants.ERROR)
+        except Exception as e:
+            LOG.error(_LE("Failed to update load balancer %(lb)s "
+                          "provisioning status to ERROR due to: "
+                          "%(except)s"), {'lb': loadbalancer_id,
+                                          'except': e})
+
 
 class CreateAmphoraInDB(BaseDatabaseTask):
     """Task to create an initial amphora in the Database."""
@@ -121,7 +172,13 @@ class CreateAmphoraInDB(BaseDatabaseTask):
                     result)
 
         # Delete the amphora for now. May want to just update status later
-        self.amphora_repo.delete(db_apis.get_session(), id=result)
+        try:
+            self.amphora_repo.delete(db_apis.get_session(), id=result)
+        except Exception as e:
+            LOG.error(_LE("Failed to delete amphora %(amp)s "
+                          "in the database due to: "
+                          "%(except)s"), {'amp': result,
+                                          'except': e})
 
 
 class MarkLBAmphoraeDeletedInDB(BaseDatabaseTask):
@@ -449,8 +506,14 @@ class AssociateFailoverAmphoraWithLBID(BaseDatabaseTask):
                with a load balancer.
         :returns: None
         """
-        self.repos.amphora.update(db_apis.get_session(), amphora_id,
+        try:
-                                  loadbalancer_id=None)
+            self.repos.amphora.update(db_apis.get_session(), amphora_id,
+                                      loadbalancer_id=None)
+        except Exception as e:
+            LOG.error(_LE("Failed to update amphora %(amp)s "
+                          "load balancer id to None due to: "
+                          "%(except)s"), {'amp': amphora_id,
+                                          'except': e})
 
 
 class MapLoadbalancerToAmphora(BaseDatabaseTask):
@@ -483,10 +546,7 @@ class MapLoadbalancerToAmphora(BaseDatabaseTask):
     def revert(self, result, loadbalancer_id, *args, **kwargs):
         LOG.warning(_LW("Reverting Amphora allocation for the load "
                         "balancer %s in the database."), loadbalancer_id)
-
+        self._mark_loadbalancer_prov_status_error(loadbalancer_id)
-        self.loadbalancer_repo.update(db_apis.get_session(),
-                                      loadbalancer_id,
-                                      provisioning_status=constants.ERROR)
 
 
 class _MarkAmphoraRoleAndPriorityInDB(BaseDatabaseTask):
@@ -521,9 +581,15 @@ class _MarkAmphoraRoleAndPriorityInDB(BaseDatabaseTask):
         LOG.warning(_LW("Reverting amphora role in DB for amp "
                         "id %(amp)s"),
                     {'amp': amphora.id})
-        self.amphora_repo.update(db_apis.get_session(), amphora.id,
+        try:
-                                 role=None,
+            self.amphora_repo.update(db_apis.get_session(), amphora.id,
-                                 vrrp_priority=None)
+                                     role=None,
+                                     vrrp_priority=None)
+        except Exception as e:
+            LOG.error(_LE("Failed to update amphora %(amp)s "
+                          "role and vrrp_priority to None due to: "
+                          "%(except)s"), {'amp': amphora.id,
+                                          'except': e})
 
 
 class MarkAmphoraMasterInDB(_MarkAmphoraRoleAndPriorityInDB):
@@ -631,8 +697,7 @@ class MarkAmphoraAllocatedInDB(BaseDatabaseTask):
         LOG.warning(_LW("Reverting mark amphora ready in DB for amp "
                         "id %(amp)s and compute id %(comp)s"),
                     {'amp': amphora.id, 'comp': amphora.compute_id})
-        self.amphora_repo.update(db_apis.get_session(), amphora.id,
+        self._mark_amphora_status_error(amphora.id)
-                                 status=constants.ERROR)
 
 
 class MarkAmphoraBootingInDB(BaseDatabaseTask):
@@ -667,9 +732,15 @@ class MarkAmphoraBootingInDB(BaseDatabaseTask):
         LOG.warning(_LW("Reverting mark amphora booting in DB for amp "
                         "id %(amp)s and compute id %(comp)s"),
                     {'amp': amphora_id, 'comp': compute_id})
-        self.amphora_repo.update(db_apis.get_session(), amphora_id,
+        try:
-                                 status=constants.ERROR,
+            self.amphora_repo.update(db_apis.get_session(), amphora_id,
-                                 compute_id=compute_id)
+                                     status=constants.ERROR,
+                                     compute_id=compute_id)
+        except Exception as e:
+            LOG.error(_LE("Failed to update amphora %(amp)s "
+                          "status to ERROR due to: "
+                          "%(except)s"), {'amp': amphora_id,
+                                          'except': e})
 
 
 class MarkAmphoraDeletedInDB(BaseDatabaseTask):
@@ -701,8 +772,7 @@ class MarkAmphoraDeletedInDB(BaseDatabaseTask):
         LOG.warning(_LW("Reverting mark amphora deleted in DB "
                         "for amp id %(amp)s and compute id %(comp)s"),
                     {'amp': amphora.id, 'comp': amphora.compute_id})
-        self.amphora_repo.update(db_apis.get_session(), amphora.id,
+        self._mark_amphora_status_error(amphora.id)
-                                 status=constants.ERROR)
 
 
 class MarkAmphoraPendingDeleteInDB(BaseDatabaseTask):
@@ -734,8 +804,7 @@ class MarkAmphoraPendingDeleteInDB(BaseDatabaseTask):
         LOG.warning(_LW("Reverting mark amphora pending delete in DB "
                         "for amp id %(amp)s and compute id %(comp)s"),
                     {'amp': amphora.id, 'comp': amphora.compute_id})
-        self.amphora_repo.update(db_apis.get_session(), amphora.id,
+        self._mark_amphora_status_error(amphora.id)
-                                 status=constants.ERROR)
 
 
 class MarkAmphoraPendingUpdateInDB(BaseDatabaseTask):
@@ -767,8 +836,7 @@ class MarkAmphoraPendingUpdateInDB(BaseDatabaseTask):
         LOG.warning(_LW("Reverting mark amphora pending update in DB "
                         "for amp id %(amp)s and compute id %(comp)s"),
                     {'amp': amphora.id, 'comp': amphora.compute_id})
-        self.amphora_repo.update(db_apis.get_session(), amphora.id,
+        self._mark_amphora_status_error(amphora.id)
-                                 status=constants.ERROR)
 
 
 class MarkAmphoraReadyInDB(BaseDatabaseTask):
@@ -803,10 +871,16 @@ class MarkAmphoraReadyInDB(BaseDatabaseTask):
         LOG.warning(_LW("Reverting mark amphora ready in DB for amp "
                         "id %(amp)s and compute id %(comp)s"),
                     {'amp': amphora.id, 'comp': amphora.compute_id})
-        self.amphora_repo.update(db_apis.get_session(), amphora.id,
+        try:
-                                 status=constants.ERROR,
+            self.amphora_repo.update(db_apis.get_session(), amphora.id,
-                                 compute_id=amphora.compute_id,
+                                     status=constants.ERROR,
-                                 lb_network_ip=amphora.lb_network_ip)
+                                     compute_id=amphora.compute_id,
+                                     lb_network_ip=amphora.lb_network_ip)
+        except Exception as e:
+            LOG.error(_LE("Failed to update amphora %(amp)s "
+                          "status to ERROR due to: "
+                          "%(except)s"), {'amp': amphora.id,
+                                          'except': e})
 
 
 class UpdateAmphoraComputeId(BaseDatabaseTask):
@@ -931,9 +1005,7 @@ class MarkLBActiveInDB(BaseDatabaseTask):
 
         LOG.warning(_LW("Reverting mark load balancer deleted in DB "
                         "for load balancer id %s"), loadbalancer.id)
-        self.loadbalancer_repo.update(db_apis.get_session(),
+        self._mark_loadbalancer_prov_status_error(loadbalancer.id)
-                                      loadbalancer.id,
-                                      provisioning_status=constants.ERROR)
 
 
 class UpdateLBServerGroupInDB(BaseDatabaseTask):
@@ -965,9 +1037,15 @@ class UpdateLBServerGroupInDB(BaseDatabaseTask):
         LOG.warning(_LW('Reverting Server Group updated with id: %(s1)s for '
                         'load balancer id: %(s2)s '),
                     {'s1': server_group_id, 's2': loadbalancer_id})
-        self.loadbalancer_repo.update(db_apis.get_session(),
+        try:
-                                      id=loadbalancer_id,
+            self.loadbalancer_repo.update(db_apis.get_session(),
-                                      server_group_id=None)
+                                          id=loadbalancer_id,
+                                          server_group_id=None)
+        except Exception as e:
+            LOG.error(_LE("Failed to update load balancer %(lb)s "
+                          "server_group_id to None due to: "
+                          "%(except)s"), {'lb': loadbalancer_id,
+                                          'except': e})
 
 
 class MarkLBDeletedInDB(BaseDatabaseTask):
@@ -998,9 +1076,7 @@ class MarkLBDeletedInDB(BaseDatabaseTask):
 
         LOG.warning(_LW("Reverting mark load balancer deleted in DB "
                         "for load balancer id %s"), loadbalancer.id)
-        self.loadbalancer_repo.update(db_apis.get_session(),
+        self._mark_loadbalancer_prov_status_error(loadbalancer.id)
-                                      loadbalancer.id,
-                                      provisioning_status=constants.ERROR)
 
 
 class MarkLBPendingDeleteInDB(BaseDatabaseTask):
@@ -1032,9 +1108,7 @@ class MarkLBPendingDeleteInDB(BaseDatabaseTask):
 
         LOG.warning(_LW("Reverting mark load balancer pending delete in DB "
                         "for load balancer id %s"), loadbalancer.id)
-        self.loadbalancer_repo.update(db_apis.get_session(),
+        self._mark_loadbalancer_prov_status_error(loadbalancer.id)
-                                      loadbalancer.id,
-                                      provisioning_status=constants.ERROR)
 
 
 class MarkLBAndListenersActiveInDB(BaseDatabaseTask):
@@ -1075,16 +1149,9 @@ class MarkLBAndListenersActiveInDB(BaseDatabaseTask):
                         "listener ids: %(list)s"),
                     {'LB': loadbalancer.id,
                      'list': ', '.join([l.id for l in listeners])})
-        self.loadbalancer_repo.update(db_apis.get_session(),
+        self._mark_loadbalancer_prov_status_error(loadbalancer.id)
-                                      loadbalancer.id,
-                                      provisioning_status=constants.ERROR)
         for listener in listeners:
-            try:
+            self._mark_listener_prov_status_error(listener.id)
-                self.listener_repo.update(db_apis.get_session(), listener.id,
-                                          provisioning_status=constants.ERROR)
-            except Exception:
-                LOG.warning(_LW("Failed to update listener %s provisioning "
-                                "status..."), listener.id)
 
 
 class MarkListenerActiveInDB(BaseDatabaseTask):
@@ -1113,8 +1180,7 @@ class MarkListenerActiveInDB(BaseDatabaseTask):
 
         LOG.warning(_LW("Reverting mark listener active in DB "
                         "for listener id %s"), listener.id)
-        self.listener_repo.update(db_apis.get_session(), listener.id,
+        self._mark_listener_prov_status_error(listener.id)
-                                  provisioning_status=constants.ERROR)
 
 
 class MarkListenerDeletedInDB(BaseDatabaseTask):
@@ -1143,8 +1209,7 @@ class MarkListenerDeletedInDB(BaseDatabaseTask):
 
         LOG.warning(_LW("Reverting mark listener deleted in DB "
                         "for listener id %s"), listener.id)
-        self.listener_repo.update(db_apis.get_session(), listener.id,
+        self._mark_listener_prov_status_error(listener.id)
-                                  provisioning_status=constants.ERROR)
 
 
 class MarkListenerPendingDeleteInDB(BaseDatabaseTask):
@@ -1174,8 +1239,7 @@ class MarkListenerPendingDeleteInDB(BaseDatabaseTask):
 
         LOG.warning(_LW("Reverting mark listener pending delete in DB "
                         "for listener id %s"), listener.id)
-        self.listener_repo.update(db_apis.get_session(), listener.id,
+        self._mark_listener_prov_status_error(listener.id)
-                                  provisioning_status=constants.ERROR)
 
 
 class UpdateLoadbalancerInDB(BaseDatabaseTask):
@@ -1206,9 +1270,7 @@ class UpdateLoadbalancerInDB(BaseDatabaseTask):
         LOG.warning(_LW("Reverting update loadbalancer in DB "
                         "for loadbalancer id %s"), loadbalancer.id)
 
-        self.loadbalancer_repo.update(db_apis.get_session(),
+        self._mark_loadbalancer_prov_status_error(loadbalancer.id)
-                                      loadbalancer.id,
-                                      provisioning_status=constants.ERROR)
 
 
 class UpdateHealthMonInDB(BaseDatabaseTask):
@@ -1239,8 +1301,15 @@ class UpdateHealthMonInDB(BaseDatabaseTask):
         LOG.warning(_LW("Reverting update health monitor in DB "
                         "for health monitor id %s"), health_mon.pool_id)
 # TODO(johnsom) fix this to set the upper ojects to ERROR
-        self.health_mon_repo.update(db_apis.get_session(), health_mon.pool_id,
+        try:
-                                    enabled=0)
+            self.health_mon_repo.update(db_apis.get_session(),
+                                        health_mon.pool_id,
+                                        enabled=0)
+        except Exception as e:
+            LOG.error(_LE("Failed to update health monitor %(hm)s "
+                          "enabled to 0 due to: "
+                          "%(except)s"), {'hm': health_mon.pool_id,
+                                          'except': e})
 
 
 class UpdateListenerInDB(BaseDatabaseTask):
@@ -1270,9 +1339,7 @@ class UpdateListenerInDB(BaseDatabaseTask):
 
         LOG.warning(_LW("Reverting update listener in DB "
                         "for listener id %s"), listener.id)
-# TODO(johnsom) fix this to set the upper objects to ERROR
+        self._mark_listener_prov_status_error(listener.id)
-        self.listener_repo.update(db_apis.get_session(), listener.id,
-                                  enabled=0)
 
 
 class UpdateMemberInDB(BaseDatabaseTask):
@@ -1303,8 +1370,14 @@ class UpdateMemberInDB(BaseDatabaseTask):
         LOG.warning(_LW("Reverting update member in DB "
                         "for member id %s"), member.id)
 # TODO(johnsom) fix this to set the upper objects to ERROR
-        self.member_repo.update(db_apis.get_session(), member.id,
+        try:
-                                enabled=0)
+            self.member_repo.update(db_apis.get_session(), member.id,
+                                    enabled=0)
+        except Exception as e:
+            LOG.error(_LE("Failed to update member %(member)s "
+                          "enabled to 0 due to: "
+                          "%(except)s"), {'member': member.id,
+                                          'except': e})
 
 
 class UpdatePoolInDB(BaseDatabaseTask):
@@ -1335,8 +1408,14 @@ class UpdatePoolInDB(BaseDatabaseTask):
         LOG.warning(_LW("Reverting update pool in DB "
                         "for pool id %s"), pool.id)
 # TODO(johnsom) fix this to set the upper objects to ERROR
-        self.repos.update_pool_and_sp(db_apis.get_session(),
+        try:
-                                      pool.id, {'enabled': 0})
+            self.repos.update_pool_and_sp(db_apis.get_session(),
+                                          pool.id, enabled=0)
+        except Exception as e:
+            LOG.error(_LE("Failed to update pool %(pool)s "
+                          "enabled 0 due to: "
+                          "%(except)s"), {'pool': pool.id,
+                                          'except': e})
 
 
 class UpdateL7PolicyInDB(BaseDatabaseTask):
@@ -1367,8 +1446,14 @@ class UpdateL7PolicyInDB(BaseDatabaseTask):
         LOG.warning(_LW("Reverting update l7policy in DB "
                         "for l7policy id %s"), l7policy.id)
 # TODO(sbalukoff) fix this to set the upper objects to ERROR
-        self.l7policy_repo.update(db_apis.get_session(), l7policy.id,
+        try:
-                                  enabled=0)
+            self.l7policy_repo.update(db_apis.get_session(), l7policy.id,
+                                      enabled=0)
+        except Exception as e:
+            LOG.error(_LE("Failed to update l7policy %(l7p)s "
+                          "enabled to 0 due to: "
+                          "%(except)s"), {'l7p': l7policy.id,
+                                          'except': e})
 
 
 class UpdateL7RuleInDB(BaseDatabaseTask):
@@ -1399,8 +1484,15 @@ class UpdateL7RuleInDB(BaseDatabaseTask):
         LOG.warning(_LW("Reverting update l7rule in DB "
                         "for l7rule id %s"), l7rule.id)
 # TODO(sbalukoff) fix this to set appropriate upper objects to ERROR
-        self.l7policy_repo.update(db_apis.get_session(), l7rule.l7policy.id,
+        try:
-                                  enabled=0)
+            self.l7policy_repo.update(db_apis.get_session(),
+                                      l7rule.l7policy.id,
+                                      enabled=0)
+        except Exception as e:
+            LOG.error(_LE("Failed to update L7rule %(l7r)s "
+                          "enabled to 0 due to: "
+                          "%(except)s"), {'l7r': l7rule.l7policy.id,
+                                          'except': e})
 
 
 class GetAmphoraDetails(BaseDatabaseTask):

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

@@ -85,6 +85,16 @@ class TestAmphoraDriverTasks(base.TestCase):
             provisioning_status=constants.ERROR)
         self.assertIsNone(amp)
 
+        # Test the revert with exception
+        repo.ListenerRepository.update.reset_mock()
+        mock_listener_repo_update.side_effect = Exception('fail')
+        amp = listener_update_obj.revert(_load_balancer_mock)
+        repo.ListenerRepository.update.assert_called_once_with(
+            _session_mock,
+            id=LISTENER_ID,
+            provisioning_status=constants.ERROR)
+        self.assertIsNone(amp)
+
     def test_listeners_update(self,
                               mock_driver,
                               mock_generate_uuid,
@@ -139,6 +149,16 @@ class TestAmphoraDriverTasks(base.TestCase):
             provisioning_status=constants.ERROR)
         self.assertIsNone(amp)
 
+        # Test the revert with exception
+        repo.ListenerRepository.update.reset_mock()
+        mock_listener_repo_update.side_effect = Exception('fail')
+        amp = listener_stop_obj.revert(_listener_mock)
+        repo.ListenerRepository.update.assert_called_once_with(
+            _session_mock,
+            id=LISTENER_ID,
+            provisioning_status=constants.ERROR)
+        self.assertIsNone(amp)
+
     def test_listener_start(self,
                             mock_driver,
                             mock_generate_uuid,
@@ -161,6 +181,16 @@ class TestAmphoraDriverTasks(base.TestCase):
             provisioning_status=constants.ERROR)
         self.assertIsNone(amp)
 
+        # Test the revert with exception
+        repo.ListenerRepository.update.reset_mock()
+        mock_listener_repo_update.side_effect = Exception('fail')
+        amp = listener_start_obj.revert(_listener_mock)
+        repo.ListenerRepository.update.assert_called_once_with(
+            _session_mock,
+            id=LISTENER_ID,
+            provisioning_status=constants.ERROR)
+        self.assertIsNone(amp)
+
     def test_listener_delete(self,
                              mock_driver,
                              mock_generate_uuid,
@@ -183,6 +213,16 @@ class TestAmphoraDriverTasks(base.TestCase):
             provisioning_status=constants.ERROR)
         self.assertIsNone(amp)
 
+        # Test the revert with exception
+        repo.ListenerRepository.update.reset_mock()
+        mock_listener_repo_update.side_effect = Exception('fail')
+        amp = listener_delete_obj.revert(_listener_mock)
+        repo.ListenerRepository.update.assert_called_once_with(
+            _session_mock,
+            id=LISTENER_ID,
+            provisioning_status=constants.ERROR)
+        self.assertIsNone(amp)
+
     def test_amphora_get_info(self,
                               mock_driver,
                               mock_generate_uuid,
@@ -237,6 +277,16 @@ class TestAmphoraDriverTasks(base.TestCase):
             status=constants.ERROR)
         self.assertIsNone(amp)
 
+        # Test revert with exception
+        repo.AmphoraRepository.update.reset_mock()
+        mock_amphora_repo_update.side_effect = Exception('fail')
+        amp = amphora_finalize_obj.revert(None, _amphora_mock)
+        repo.AmphoraRepository.update.assert_called_once_with(
+            _session_mock,
+            id=AMP_ID,
+            status=constants.ERROR)
+        self.assertIsNone(amp)
+
     def test_amphora_post_network_plug(self,
                                        mock_driver,
                                        mock_generate_uuid,
@@ -262,6 +312,17 @@ class TestAmphoraDriverTasks(base.TestCase):
 
         self.assertIsNone(amp)
 
+        # Test revert with exception
+        repo.AmphoraRepository.update.reset_mock()
+        mock_amphora_repo_update.side_effect = Exception('fail')
+        amp = amphora_post_network_plug_obj.revert(None, _amphora_mock)
+        repo.AmphoraRepository.update.assert_called_once_with(
+            _session_mock,
+            id=AMP_ID,
+            status=constants.ERROR)
+
+        self.assertIsNone(amp)
+
     def test_amphorae_post_network_plug(self, mock_driver,
                                         mock_generate_uuid,
                                         mock_log,
@@ -294,6 +355,18 @@ class TestAmphoraDriverTasks(base.TestCase):
 
         self.assertIsNone(amp)
 
+        # Test revert with exception
+        repo.AmphoraRepository.update.reset_mock()
+        mock_amphora_repo_update.side_effect = Exception('fail')
+        amp = amphora_post_network_plug_obj.revert(None, _LB_mock,
+                                                   _deltas_mock)
+        repo.AmphoraRepository.update.assert_called_once_with(
+            _session_mock,
+            id=AMP_ID,
+            status=constants.ERROR)
+
+        self.assertIsNone(amp)
+
     @mock.patch('octavia.db.repositories.LoadBalancerRepository.update')
     def test_amphora_post_vip_plug(self,
                                    mock_loadbalancer_repo_update,
@@ -316,6 +389,27 @@ class TestAmphoraDriverTasks(base.TestCase):
 
         # Test revert
         amp = amphora_post_vip_plug_obj.revert(None, _amphora_mock, _LB_mock)
+        repo.AmphoraRepository.update.assert_called_once_with(
+            _session_mock,
+            id=AMP_ID,
+            status=constants.ERROR)
+        repo.LoadBalancerRepository.update.assert_called_once_with(
+            _session_mock,
+            id=LB_ID,
+            provisioning_status=constants.ERROR)
+
+        self.assertIsNone(amp)
+
+        # Test revert with repo exceptions
+        repo.AmphoraRepository.update.reset_mock()
+        repo.LoadBalancerRepository.update.reset_mock()
+        mock_amphora_repo_update.side_effect = Exception('fail')
+        mock_loadbalancer_repo_update.side_effect = Exception('fail')
+        amp = amphora_post_vip_plug_obj.revert(None, _amphora_mock, _LB_mock)
+        repo.AmphoraRepository.update.assert_called_once_with(
+            _session_mock,
+            id=AMP_ID,
+            status=constants.ERROR)
         repo.LoadBalancerRepository.update.assert_called_once_with(
             _session_mock,
             id=LB_ID,
@@ -351,6 +445,17 @@ class TestAmphoraDriverTasks(base.TestCase):
 
         self.assertIsNone(amp)
 
+        # Test revert with exception
+        repo.LoadBalancerRepository.update.reset_mock()
+        mock_loadbalancer_repo_update.side_effect = Exception('fail')
+        amp = amphora_post_vip_plug_obj.revert(None, _LB_mock)
+        repo.LoadBalancerRepository.update.assert_called_once_with(
+            _session_mock,
+            id=LB_ID,
+            provisioning_status=constants.ERROR)
+
+        self.assertIsNone(amp)
+
     def test_amphora_cert_upload(self,
                                  mock_driver,
                                  mock_generate_uuid,
@@ -380,6 +485,7 @@ class TestAmphoraDriverTasks(base.TestCase):
         amphora_update_vrrp_interface_obj.execute(_LB_mock)
         mock_driver.get_vrrp_interface.assert_called_once_with(_amphora_mock)
 
+        # Test revert
         mock_driver.reset_mock()
 
         _LB_mock.amphorae = _amphorae_mock
@@ -395,6 +501,17 @@ class TestAmphoraDriverTasks(base.TestCase):
         amphora_update_vrrp_interface_obj.revert(failure_obj, _LB_mock)
         self.assertFalse(mock_amphora_repo_update.called)
 
+        # Test revert with exception
+        mock_driver.reset_mock()
+        mock_amphora_repo_update.reset_mock()
+        mock_amphora_repo_update.side_effect = Exception('fail')
+
+        _LB_mock.amphorae = _amphorae_mock
+        amphora_update_vrrp_interface_obj.revert("BADRESULT", _LB_mock)
+        mock_amphora_repo_update.assert_called_with(_session_mock,
+                                                    _amphora_mock.id,
+                                                    vrrp_interface=None)
+
     def test_amphora_vrrp_update(self,
                                  mock_driver,
                                  mock_generate_uuid,

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

@@ -176,21 +176,24 @@ class TestDatabaseTasks(base.TestCase):
         assert(amp_id == _amphora_mock.id)
 
         # Test the revert
+        create_amp_in_db.revert(_tf_failure_mock)
+        self.assertFalse(mock_amphora_repo_delete.called)
 
-# TODO(johnsom) finish when this method is updated
+        mock_amphora_repo_delete.reset_mock()
-        amp = create_amp_in_db.revert(_tf_failure_mock)
+        create_amp_in_db.revert(result='AMP')
+        self.assertTrue(mock_amphora_repo_delete.called)
+        mock_amphora_repo_delete.assert_called_once_with(
+            'TEST',
+            id='AMP')
 
-        self.assertIsNone(amp)
+        # Test revert with exception
-
+        mock_amphora_repo_delete.reset_mock()
-        amp = create_amp_in_db.revert(result='TEST')
+        mock_amphora_repo_delete.side_effect = Exception('fail')
-
+        create_amp_in_db.revert(result='AMP')
-        self.assertIsNone(amp)
+        self.assertTrue(mock_amphora_repo_delete.called)
-
+        mock_amphora_repo_delete.assert_called_once_with(
-#        repo.AmphoraRepository.update.assert_called_once_with(
+            'TEST',
-#            'TEST',
+            id='AMP')
-#            AMP_ID,
-#            status=constants.ERROR,
-#            compute_id=COMPUTE_ID)
 
     @mock.patch('octavia.db.repositories.ListenerRepository.delete')
     def test_delete_listener_in_db(self,
@@ -522,6 +525,16 @@ class TestDatabaseTasks(base.TestCase):
                                                          AMP_ID,
                                                          loadbalancer_id=None)
 
+        # Test revert with exception
+        mock_amphora_repo_update.reset_mock()
+        mock_amphora_repo_update.side_effect = Exception('fail')
+
+        assoc_fo_amp_lb_id.revert(AMP_ID)
+
+        mock_amphora_repo_update.assert_called_once_with('TEST',
+                                                         AMP_ID,
+                                                         loadbalancer_id=None)
+
     @mock.patch('octavia.db.repositories.AmphoraRepository.'
                 'allocate_and_associate',
                 side_effect=[_amphora_mock, None])
@@ -548,6 +561,22 @@ class TestDatabaseTasks(base.TestCase):
 
         self.assertIsNone(amp_id)
 
+        # Test revert
+        map_lb_to_amp.revert(None, self.loadbalancer_mock.id)
+        repo.LoadBalancerRepository.update.assert_called_once_with(
+            'TEST',
+            id=LB_ID,
+            provisioning_status=constants.ERROR)
+
+        # Test revert with exception
+        repo.LoadBalancerRepository.update.reset_mock()
+        mock_loadbalancer_repo_update.side_effect = Exception('fail')
+        map_lb_to_amp.revert(None, self.loadbalancer_mock.id)
+        repo.LoadBalancerRepository.update.assert_called_once_with(
+            'TEST',
+            id=LB_ID,
+            provisioning_status=constants.ERROR)
+
     @mock.patch('octavia.db.repositories.AmphoraRepository.get',
                 return_value=_amphora_mock)
     @mock.patch('octavia.db.repositories.LoadBalancerRepository.get',
@@ -608,7 +637,19 @@ class TestDatabaseTasks(base.TestCase):
 
         repo.AmphoraRepository.update.assert_called_once_with(
             'TEST',
-            AMP_ID,
+            id=AMP_ID,
+            status=constants.ERROR)
+
+        # Test the revert with exception
+
+        mock_amphora_repo_update.reset_mock()
+        mock_amphora_repo_update.side_effect = Exception('fail')
+        mark_amp_allocated_in_db.revert(None, _amphora_mock,
+                                        self.loadbalancer_mock.id)
+
+        repo.AmphoraRepository.update.assert_called_once_with(
+            'TEST',
+            id=AMP_ID,
             status=constants.ERROR)
 
     def test_mark_amphora_booting_in_db(self,
@@ -642,6 +683,19 @@ class TestDatabaseTasks(base.TestCase):
             status=constants.ERROR,
             compute_id=COMPUTE_ID)
 
+        # Test the revert with exception
+
+        mock_amphora_repo_update.reset_mock()
+        mock_amphora_repo_update.side_effect = Exception('fail')
+        mark_amp_booting_in_db.revert(None, _amphora_mock.id,
+                                      _amphora_mock.compute_id)
+
+        repo.AmphoraRepository.update.assert_called_once_with(
+            'TEST',
+            AMP_ID,
+            status=constants.ERROR,
+            compute_id=COMPUTE_ID)
+
     def test_mark_amphora_deleted_in_db(self,
                                         mock_generate_uuid,
                                         mock_LOG,
@@ -660,13 +714,22 @@ class TestDatabaseTasks(base.TestCase):
             status=constants.DELETED)
 
         # Test the revert
-
         mock_amphora_repo_update.reset_mock()
         mark_amp_deleted_in_db.revert(_amphora_mock)
 
         repo.AmphoraRepository.update.assert_called_once_with(
             'TEST',
-            AMP_ID,
+            id=AMP_ID,
+            status=constants.ERROR)
+
+        # Test the revert with exception
+        mock_amphora_repo_update.reset_mock()
+        mock_amphora_repo_update.side_effect = Exception('fail')
+        mark_amp_deleted_in_db.revert(_amphora_mock)
+
+        repo.AmphoraRepository.update.assert_called_once_with(
+            'TEST',
+            id=AMP_ID,
             status=constants.ERROR)
 
     def test_mark_amphora_pending_delete_in_db(self,
@@ -688,13 +751,23 @@ class TestDatabaseTasks(base.TestCase):
             status=constants.PENDING_DELETE)
 
         # Test the revert
-
         mock_amphora_repo_update.reset_mock()
         mark_amp_pending_delete_in_db.revert(_amphora_mock)
 
         repo.AmphoraRepository.update.assert_called_once_with(
             'TEST',
-            AMP_ID,
+            id=AMP_ID,
+            status=constants.ERROR)
+
+        # Test the revert with exception
+        mock_amphora_repo_update.reset_mock()
+        mock_amphora_repo_update.side_effect = Exception('fail')
+
+        mark_amp_pending_delete_in_db.revert(_amphora_mock)
+
+        repo.AmphoraRepository.update.assert_called_once_with(
+            'TEST',
+            id=AMP_ID,
             status=constants.ERROR)
 
     def test_mark_amphora_pending_update_in_db(self,
@@ -716,13 +789,22 @@ class TestDatabaseTasks(base.TestCase):
             status=constants.PENDING_UPDATE)
 
         # Test the revert
-
         mock_amphora_repo_update.reset_mock()
         mark_amp_pending_update_in_db.revert(_amphora_mock)
 
         repo.AmphoraRepository.update.assert_called_once_with(
             'TEST',
-            AMP_ID,
+            id=AMP_ID,
+            status=constants.ERROR)
+
+        # Test the revert with exception
+        mock_amphora_repo_update.reset_mock()
+        mock_amphora_repo_update.side_effect = Exception('fail')
+        mark_amp_pending_update_in_db.revert(_amphora_mock)
+
+        repo.AmphoraRepository.update.assert_called_once_with(
+            'TEST',
+            id=AMP_ID,
             status=constants.ERROR)
 
     def test_mark_amphora_ready_in_db(self,
@@ -758,6 +840,19 @@ class TestDatabaseTasks(base.TestCase):
             compute_id=COMPUTE_ID,
             lb_network_ip=LB_NET_IP)
 
+        # Test the revert with exception
+
+        mock_amphora_repo_update.reset_mock()
+        mock_amphora_repo_update.side_effect = Exception('fail')
+        mark_amp_ready_in_db.revert(_amphora_mock)
+
+        repo.AmphoraRepository.update.assert_called_once_with(
+            'TEST',
+            AMP_ID,
+            status=constants.ERROR,
+            compute_id=COMPUTE_ID,
+            lb_network_ip=LB_NET_IP)
+
     def test_mark_listener_active_in_db(self,
                                         mock_generate_uuid,
                                         mock_LOG,
@@ -776,13 +871,22 @@ class TestDatabaseTasks(base.TestCase):
             provisioning_status=constants.ACTIVE)
 
         # Test the revert
-
         mock_listener_repo_update.reset_mock()
         mark_listener_active.revert(self.listener_mock)
 
         repo.ListenerRepository.update.assert_called_once_with(
             'TEST',
-            LISTENER_ID,
+            id=LISTENER_ID,
+            provisioning_status=constants.ERROR)
+
+        # Test the revert
+        mock_listener_repo_update.reset_mock()
+        mock_listener_repo_update.side_effect = Exception('fail')
+        mark_listener_active.revert(self.listener_mock)
+
+        repo.ListenerRepository.update.assert_called_once_with(
+            'TEST',
+            id=LISTENER_ID,
             provisioning_status=constants.ERROR)
 
     def test_mark_listener_deleted_in_db(self,
@@ -803,13 +907,22 @@ class TestDatabaseTasks(base.TestCase):
             provisioning_status=constants.DELETED)
 
         # Test the revert
-
         mock_listener_repo_update.reset_mock()
         mark_listener_deleted.revert(self.listener_mock)
 
         repo.ListenerRepository.update.assert_called_once_with(
             'TEST',
-            LISTENER_ID,
+            id=LISTENER_ID,
+            provisioning_status=constants.ERROR)
+
+        # Test the revert with exception
+        mock_listener_repo_update.reset_mock()
+        mock_listener_repo_update.side_effect = Exception('fail')
+        mark_listener_deleted.revert(self.listener_mock)
+
+        repo.ListenerRepository.update.assert_called_once_with(
+            'TEST',
+            id=LISTENER_ID,
             provisioning_status=constants.ERROR)
 
     def test_mark_listener_pending_deleted_in_db(self,
@@ -831,13 +944,22 @@ class TestDatabaseTasks(base.TestCase):
             provisioning_status=constants.PENDING_DELETE)
 
         # Test the revert
-
         mock_listener_repo_update.reset_mock()
         mark_listener_pending_delete.revert(self.listener_mock)
 
         repo.ListenerRepository.update.assert_called_once_with(
             'TEST',
-            LISTENER_ID,
+            id=LISTENER_ID,
+            provisioning_status=constants.ERROR)
+
+        # Test the revert with exception
+        mock_listener_repo_update.reset_mock()
+        mock_listener_repo_update.side_effect = Exception('fail')
+        mark_listener_pending_delete.revert(self.listener_mock)
+
+        repo.ListenerRepository.update.assert_called_once_with(
+            'TEST',
+            id=LISTENER_ID,
             provisioning_status=constants.ERROR)
 
     def test_mark_lb_and_listeners_active_in_db(self,
@@ -864,7 +986,6 @@ class TestDatabaseTasks(base.TestCase):
             provisioning_status=constants.ACTIVE)
 
         # Test the revert
-
         mock_loadbalancer_repo_update.reset_mock()
         mock_listener_repo_update.reset_mock()
 
@@ -873,11 +994,29 @@ class TestDatabaseTasks(base.TestCase):
 
         repo.ListenerRepository.update.assert_called_once_with(
             'TEST',
-            LISTENER_ID,
+            id=LISTENER_ID,
             provisioning_status=constants.ERROR)
         repo.LoadBalancerRepository.update.assert_called_once_with(
             'TEST',
-            LB_ID,
+            id=LB_ID,
+            provisioning_status=constants.ERROR)
+
+        # Test the revert with exceptions
+        mock_loadbalancer_repo_update.reset_mock()
+        mock_loadbalancer_repo_update.side_effect = Exception('fail')
+        mock_listener_repo_update.reset_mock()
+        mock_listener_repo_update.side_effect = Exception('fail')
+
+        mark_lb_and_listeners_active.revert(self.loadbalancer_mock,
+                                            [self.listener_mock])
+
+        repo.ListenerRepository.update.assert_called_once_with(
+            'TEST',
+            id=LISTENER_ID,
+            provisioning_status=constants.ERROR)
+        repo.LoadBalancerRepository.update.assert_called_once_with(
+            'TEST',
+            id=LB_ID,
             provisioning_status=constants.ERROR)
 
     @mock.patch('octavia.common.tls_utils.cert_parser.get_cert_expiration',
@@ -934,13 +1073,23 @@ class TestDatabaseTasks(base.TestCase):
         self.assertEqual(0, repo.ListenerRepository.update.call_count)
 
         # Test the revert
-
         mock_loadbalancer_repo_update.reset_mock()
         mark_loadbalancer_active.revert(self.loadbalancer_mock)
 
         repo.LoadBalancerRepository.update.assert_called_once_with(
             'TEST',
-            LB_ID,
+            id=LB_ID,
+            provisioning_status=constants.ERROR)
+        self.assertEqual(0, repo.ListenerRepository.update.call_count)
+
+        # Test the revert with exception
+        mock_loadbalancer_repo_update.reset_mock()
+        mock_loadbalancer_repo_update.side_effect = Exception('fail')
+        mark_loadbalancer_active.revert(self.loadbalancer_mock)
+
+        repo.LoadBalancerRepository.update.assert_called_once_with(
+            'TEST',
+            id=LB_ID,
             provisioning_status=constants.ERROR)
         self.assertEqual(0, repo.ListenerRepository.update.call_count)
 
@@ -975,7 +1124,7 @@ class TestDatabaseTasks(base.TestCase):
 
         repo.LoadBalancerRepository.update.assert_called_once_with(
             'TEST',
-            lb.id,
+            id=lb.id,
             provisioning_status=constants.ERROR)
         self.assertEqual(2, repo.ListenerRepository.update.call_count)
         repo.ListenerRepository.update.has_calls(
@@ -1002,13 +1151,22 @@ class TestDatabaseTasks(base.TestCase):
             provisioning_status=constants.DELETED)
 
         # Test the revert
-
         mock_loadbalancer_repo_update.reset_mock()
         mark_loadbalancer_deleted.revert(self.loadbalancer_mock)
 
         repo.LoadBalancerRepository.update.assert_called_once_with(
             'TEST',
-            LB_ID,
+            id=LB_ID,
+            provisioning_status=constants.ERROR)
+
+        # Test the revert with exception
+        mock_loadbalancer_repo_update.reset_mock()
+        mock_loadbalancer_repo_update.side_effect = Exception('fail')
+        mark_loadbalancer_deleted.revert(self.loadbalancer_mock)
+
+        repo.LoadBalancerRepository.update.assert_called_once_with(
+            'TEST',
+            id=LB_ID,
             provisioning_status=constants.ERROR)
 
     def test_mark_LB_pending_deleted_in_db(self,
@@ -1030,13 +1188,22 @@ class TestDatabaseTasks(base.TestCase):
             provisioning_status=constants.PENDING_DELETE)
 
         # Test the revert
-
         mock_loadbalancer_repo_update.reset_mock()
         mark_loadbalancer_pending_delete.revert(self.loadbalancer_mock)
 
         repo.LoadBalancerRepository.update.assert_called_once_with(
             'TEST',
-            LB_ID,
+            id=LB_ID,
+            provisioning_status=constants.ERROR)
+
+        # Test the revert with exception
+        mock_loadbalancer_repo_update.reset_mock()
+        mock_loadbalancer_repo_update.side_effect = Exception('fail')
+        mark_loadbalancer_pending_delete.revert(self.loadbalancer_mock)
+
+        repo.LoadBalancerRepository.update.assert_called_once_with(
+            'TEST',
+            id=LB_ID,
             provisioning_status=constants.ERROR)
 
     @mock.patch('octavia.db.repositories.HealthMonitorRepository.update')
@@ -1060,10 +1227,20 @@ class TestDatabaseTasks(base.TestCase):
             delay=1, timeout=2)
 
         # Test the revert
-
         mock_health_mon_repo_update.reset_mock()
         update_health_mon.revert(self.health_mon_mock)
 
+# TODO(johnsom) fix this to set the upper ojects to ERROR
+        repo.HealthMonitorRepository.update.assert_called_once_with(
+            'TEST',
+            POOL_ID,
+            enabled=0)
+
+        # Test the revert with exception
+        mock_health_mon_repo_update.reset_mock()
+        mock_health_mon_repo_update.side_effect = Exception('fail')
+        update_health_mon.revert(self.health_mon_mock)
+
 # TODO(johnsom) fix this to set the upper ojects to ERROR
         repo.HealthMonitorRepository.update.assert_called_once_with(
             'TEST',
@@ -1091,9 +1268,23 @@ class TestDatabaseTasks(base.TestCase):
             name='test', description='test2')
 
         # Test the revert
+        mock_loadbalancer_repo_update.reset_mock()
+        update_load_balancer.revert(self.loadbalancer_mock)
 
-        mock_listener_repo_update.reset_mock()
+        repo.LoadBalancerRepository.update.assert_called_once_with(
-        update_load_balancer.revert(self.listener_mock)
+            'TEST',
+            id=LB_ID,
+            provisioning_status=constants.ERROR)
+
+        # Test the revert with exception
+        mock_loadbalancer_repo_update.reset_mock()
+        mock_loadbalancer_repo_update.side_effect = Exception('fail')
+        update_load_balancer.revert(self.loadbalancer_mock)
+
+        repo.LoadBalancerRepository.update.assert_called_once_with(
+            'TEST',
+            id=LB_ID,
+            provisioning_status=constants.ERROR)
 
     @mock.patch('octavia.db.repositories.ListenerRepository.update')
     def test_update_listener_in_db(self,
@@ -1116,15 +1307,21 @@ class TestDatabaseTasks(base.TestCase):
             name='test', description='test2')
 
         # Test the revert
-
         mock_listener_repo_update.reset_mock()
         update_listener.revert(self.listener_mock)
-
-# TODO(johnsom) fix this to set the upper ojects to ERROR
         repo.ListenerRepository.update.assert_called_once_with(
             'TEST',
-            LISTENER_ID,
+            id=LISTENER_ID,
-            enabled=0)
+            provisioning_status=constants.ERROR)
+
+        # Test the revert
+        mock_listener_repo_update.reset_mock()
+        mock_listener_repo_update.side_effect = Exception('fail')
+        update_listener.revert(self.listener_mock)
+        repo.ListenerRepository.update.assert_called_once_with(
+            'TEST',
+            id=LISTENER_ID,
+            provisioning_status=constants.ERROR)
 
     @mock.patch('octavia.db.repositories.MemberRepository.update')
     def test_update_member_in_db(self,
@@ -1147,10 +1344,20 @@ class TestDatabaseTasks(base.TestCase):
             weight=1, ip_address='10.1.0.0')
 
         # Test the revert
-
         mock_member_repo_update.reset_mock()
         update_member.revert(self.member_mock)
 
+# TODO(johnsom) fix this to set the upper ojects to ERROR
+        repo.MemberRepository.update.assert_called_once_with(
+            'TEST',
+            MEMBER_ID,
+            enabled=0)
+
+        # Test the revert
+        mock_member_repo_update.reset_mock()
+        mock_member_repo_update.side_effect = Exception('fail')
+        update_member.revert(self.member_mock)
+
 # TODO(johnsom) fix this to set the upper ojects to ERROR
         repo.MemberRepository.update.assert_called_once_with(
             'TEST',
@@ -1182,7 +1389,6 @@ class TestDatabaseTasks(base.TestCase):
             update_dict)
 
         # Test the revert
-
         mock_repos_pool_update.reset_mock()
         update_pool.revert(self.pool_mock)
 
@@ -1190,7 +1396,18 @@ class TestDatabaseTasks(base.TestCase):
         repo.Repositories.update_pool_and_sp.assert_called_once_with(
             'TEST',
             POOL_ID,
-            {'enabled': 0})
+            enabled=0)
+
+        # Test the revert with exception
+        mock_repos_pool_update.reset_mock()
+        mock_repos_pool_update.side_effect = Exception('fail')
+        update_pool.revert(self.pool_mock)
+
+# TODO(johnsom) fix this to set the upper ojects to ERROR
+        repo.Repositories.update_pool_and_sp.assert_called_once_with(
+            'TEST',
+            POOL_ID,
+            enabled=0)
 
     @mock.patch('octavia.db.repositories.L7PolicyRepository.update')
     def test_update_l7policy_in_db(self,
@@ -1213,10 +1430,20 @@ class TestDatabaseTasks(base.TestCase):
             action=constants.L7POLICY_ACTION_REJECT)
 
         # Test the revert
-
         mock_l7policy_repo_update.reset_mock()
         update_l7policy.revert(self.l7policy_mock)
 
+# TODO(sbalukoff) fix this to set the upper objects to ERROR
+        repo.L7PolicyRepository.update.assert_called_once_with(
+            'TEST',
+            L7POLICY_ID,
+            enabled=0)
+
+        # Test the revert
+        mock_l7policy_repo_update.reset_mock()
+        mock_l7policy_repo_update.side_effect = Exception('fail')
+        update_l7policy.revert(self.l7policy_mock)
+
 # TODO(sbalukoff) fix this to set the upper objects to ERROR
         repo.L7PolicyRepository.update.assert_called_once_with(
             'TEST',
@@ -1251,10 +1478,20 @@ class TestDatabaseTasks(base.TestCase):
             value='/api')
 
         # Test the revert
-
         mock_l7rule_repo_update.reset_mock()
         update_l7rule.revert(self.l7rule_mock)
 
+# TODO(sbalukoff) fix this to set the upper objects to ERROR
+        repo.L7PolicyRepository.update.assert_called_once_with(
+            'TEST',
+            L7POLICY_ID,
+            enabled=0)
+
+        # Test the revert
+        mock_l7rule_repo_update.reset_mock()
+        mock_l7rule_repo_update.side_effect = Exception('fail')
+        update_l7rule.revert(self.l7rule_mock)
+
 # TODO(sbalukoff) fix this to set the upper objects to ERROR
         repo.L7PolicyRepository.update.assert_called_once_with(
             'TEST',
@@ -1336,6 +1573,13 @@ class TestDatabaseTasks(base.TestCase):
         repo.AmphoraRepository.update.assert_called_once_with(
             'TEST', AMP_ID, role=None, vrrp_priority=None)
 
+        # Test revert with exception
+        mock_amphora_repo_update.reset_mock()
+        mock_amphora_repo_update.side_effect = Exception('fail')
+        mark_amp_standalone_indb.revert("BADRESULT", _amphora_mock)
+        repo.AmphoraRepository.update.assert_called_once_with(
+            'TEST', AMP_ID, role=None, vrrp_priority=None)
+
     @mock.patch('octavia.db.repositories.VRRPGroupRepository.create')
     def test_create_vrrp_group_for_lb(self,
                                       mock_vrrp_group_create,
@@ -1444,6 +1688,10 @@ class TestDatabaseTasks(base.TestCase):
             server_group_id=SERVER_GROUP_ID)
 
         # Test the revert
-
         mock_listener_repo_update.reset_mock()
         update_server_group_info.revert(LB_ID, SERVER_GROUP_ID)
+
+        # Test the revert with exception
+        mock_listener_repo_update.reset_mock()
+        mock_loadbalancer_repo_update.side_effect = Exception('fail')
+        update_server_group_info.revert(LB_ID, SERVER_GROUP_ID)