diff --git a/blazar/manager/service.py b/blazar/manager/service.py index 888bf3e2..24f612ec 100644 --- a/blazar/manager/service.py +++ b/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) diff --git a/blazar/status.py b/blazar/status.py index d2df8eea..c87a949b 100644 --- a/blazar/status.py +++ b/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 diff --git a/blazar/tests/manager/test_service.py b/blazar/tests/manager/test_service.py index 9d1b18cd..56971054 100644 --- a/blazar/tests/manager/test_service.py +++ b/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) diff --git a/blazar/tests/test_status.py b/blazar/tests/test_status.py index b2351f19..e86f9863 100644 --- a/blazar/tests/test_status.py +++ b/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 diff --git a/releasenotes/notes/bug-1786031-836c6d6acae08403.yaml b/releasenotes/notes/bug-1786031-836c6d6acae08403.yaml new file mode 100644 index 00000000..10068012 --- /dev/null +++ b/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 `_.