Restore lease status during a non fatal lease update exception

If a lease update raises an exception that is recoverable (such as
specifying an invalid date range) the lease would be set to ERROR
status. Other cases with wrong input have similar effects.

This change allows for non fatal exceptions to be specified in
the lease status code. If one is raised, the lease is restored
to the original status.

Closes-Bug: #1786031
Change-Id: I73df4587d223dc4582cb15e0f4cb8de1a02ffee7

blazar/manager/service.py

@@ -397,7 +397,20 @@ class ManagerService(service_utils.RPCServer):
                     return lease
 
     @status.lease.lease_status(
-        transition=status.lease.UPDATING, result_in=status.lease.STABLE)
+        transition=status.lease.UPDATING,
+        result_in=status.lease.STABLE,
+        non_fatal_exceptions=[
+            common_ex.InvalidInput,
+            exceptions.InvalidRange,
+            exceptions.MissingParameter,
+            exceptions.MalformedRequirements,
+            exceptions.MalformedParameter,
+            exceptions.NotEnoughHostsAvailable,
+            exceptions.InvalidDate,
+            exceptions.CantUpdateParameter,
+            exceptions.InvalidPeriod,
+        ]
+    )
     def update_lease(self, lease_id, values):
         if not values:
             return db_api.lease_get(lease_id)

blazar/status.py

@@ -179,7 +179,7 @@ class LeaseStatus(BaseStatus):
         return (lease['status'] in cls.STABLE)
 
     @classmethod
-    def lease_status(cls, transition, result_in):
+    def lease_status(cls, transition, result_in, non_fatal_exceptions=[]):
         """Decorator for managing a lease status.
 
         This checks and updates a lease status before and after executing a
@@ -189,24 +189,28 @@ class LeaseStatus(BaseStatus):
                            decorated function.
         :param result_in: A tuple of statuses to which a lease transits after
                           executing the decorated function.
+        :param non_fatal_exceptions: A list of exceptions that are non fatal.
+                          If one is raised during execution, the lease status
+                          will be restored.
         """
         def decorator(func):
             def wrapper(*args, **kwargs):
                 # Update a lease status
                 lease_id = kwargs['lease_id']
                 lease = db_api.lease_get(lease_id)
-                if cls.is_valid_transition(lease['status'],
+                original_status = lease['status']
+                if cls.is_valid_transition(original_status,
                                            transition,
                                            lease_id=lease_id):
                     db_api.lease_update(lease_id,
                                         {'status': transition})
                     LOG.debug('Status of lease %s changed from %s to %s.',
-                              lease_id, lease['status'], transition)
+                              lease_id, original_status, transition)
                 else:
                     LOG.warning('Aborting %s. '
                                 'Invalid lease status transition '
                                 'from %s to %s.',
-                                func.__name__, lease['status'],
+                                func.__name__, original_status,
                                 transition)
                     raise exceptions.InvalidStatus
 
@@ -214,10 +218,16 @@ class LeaseStatus(BaseStatus):
                 try:
                     result = func(*args, **kwargs)
                 except Exception as e:
-                    LOG.exception('Lease %s went into ERROR status. %s',
-                                  lease_id, str(e))
-                    db_api.lease_update(lease_id,
-                                        {'status': cls.ERROR})
+                    if type(e) in non_fatal_exceptions:
+                        LOG.exception('Non-fatal exception during transition '
+                                      'of lease %s', lease_id)
+                        db_api.lease_update(lease_id,
+                                            {'status': original_status})
+                    else:
+                        LOG.exception('Lease %s went into ERROR status. %s',
+                                      lease_id, str(e))
+                        db_api.lease_update(lease_id,
+                                            {'status': cls.ERROR})
                     raise e
 
                 # Update a lease status if it exists

blazar/tests/manager/test_service.py

@@ -102,7 +102,7 @@ class FakePluginRaisesException(base.BasePlugin):
 
 class FakeLeaseStatus(object):
     @classmethod
-    def lease_status(cls, transition, result_in):
+    def lease_status(cls, transition, result_in, non_fatal_exceptions=[]):
         def decorator(func):
             def wrapper(*args, **kwargs):
                 return func(*args, **kwargs)

blazar/tests/test_status.py

@@ -242,6 +242,40 @@ class LeaseStatusTestCase(tests.TestCase):
             [call(self.lease_id, {'status': status.LeaseStatus.STARTING}),
              call(self.lease_id, {'status': status.LeaseStatus.ERROR})])
 
+    def test_lease_status_func_allow_non_fatal_exception(self):
+        """Test when non-fatal exception is raised during lease transition.
+
+        When this happens, the exception should still get raised, but the
+        lease should be transitioned to its original status (not ERROR).
+        """
+        lease = {
+            'status': status.LeaseStatus.PENDING
+        }
+        lease_get = self.patch(self.db_api, 'lease_get')
+        lease_get.return_value = lease
+        lease_update = self.patch(self.db_api, 'lease_update')
+        self.patch(self.status.LeaseStatus, 'is_valid_transition'
+                   ).return_value = True
+
+        class NonFatalException(Exception):
+            pass
+
+        @self.status.LeaseStatus.lease_status(
+            transition=status.LeaseStatus.STARTING,
+            result_in=(status.LeaseStatus.ACTIVE,),
+            non_fatal_exceptions=[NonFatalException])
+        def dummy_start_lease(*args, **kwargs):
+            raise NonFatalException
+
+        self.assertRaises(NonFatalException,
+                          dummy_start_lease,
+                          lease_id=self.lease_id)
+
+        lease_get.assert_called_once_with(self.lease_id)
+        lease_update.assert_has_calls(
+            [call(self.lease_id, {'status': status.LeaseStatus.STARTING}),
+             call(self.lease_id, {'status': status.LeaseStatus.PENDING})])
+
     def test_lease_status_mismatch_result_in(self):
         lease = {
             'status': status.LeaseStatus.PENDING

releasenotes/notes/bug-1786031-836c6d6acae08403.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Updating a lease with invalid parameters (for example, specifying an
+    invalid date range) now has no effect on the lease, where previously this
+    would cause the lease to be set to ERROR status. For more details, see `bug
+    1786031 <https://bugs.launchpad.net/blazar/+bug/1786031>`_.