From d4cb8f428e4f23add5fc643ad23366ec58d7ec9d Mon Sep 17 00:00:00 2001 From: Mark Powers Date: Mon, 20 Sep 2021 20:37:33 +0000 Subject: [PATCH] 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 | 15 +++++++- blazar/status.py | 26 +++++++++----- blazar/tests/manager/test_service.py | 2 +- blazar/tests/test_status.py | 34 +++++++++++++++++++ .../notes/bug-1786031-836c6d6acae08403.yaml | 7 ++++ 5 files changed, 74 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/bug-1786031-836c6d6acae08403.yaml 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 `_.