diff --git a/doc/source/architecture.rst b/doc/source/architecture.rst index 479599c93..b903a9752 100644 --- a/doc/source/architecture.rst +++ b/doc/source/architecture.rst @@ -407,6 +407,9 @@ be one of the following: - **CANCELLED** : the :ref:`Audit ` was in **PENDING** or **ONGOING** state and was cancelled by the :ref:`Administrator ` +- **SUSPENDED** : the :ref:`Audit ` was in **ONGOING** + state and was suspended by the + :ref:`Administrator ` The following diagram shows the different possible states of an :ref:`Audit ` and what event makes the state change to a new diff --git a/doc/source/image_src/plantuml/audit_state_machine.txt b/doc/source/image_src/plantuml/audit_state_machine.txt index 8f4c9a1d0..860a2026e 100644 --- a/doc/source/image_src/plantuml/audit_state_machine.txt +++ b/doc/source/image_src/plantuml/audit_state_machine.txt @@ -4,11 +4,14 @@ PENDING --> ONGOING: Audit request is received\nby the Watcher Decision Engine ONGOING --> FAILED: Audit fails\n(no solution found, technical error, ...) ONGOING --> SUCCEEDED: The Watcher Decision Engine\ncould find at least one Solution +ONGOING --> SUSPENDED: Administrator wants to\nsuspend the Audit +SUSPENDED --> ONGOING: Administrator wants to\nresume the Audit FAILED --> DELETED : Administrator wants to\narchive/delete the Audit SUCCEEDED --> DELETED : Administrator wants to\narchive/delete the Audit PENDING --> CANCELLED : Administrator cancels\nthe Audit ONGOING --> CANCELLED : Administrator cancels\nthe Audit CANCELLED --> DELETED : Administrator wants to\narchive/delete the Audit +SUSPENDED --> DELETED: Administrator wants to\narchive/delete the Audit DELETED --> [*] @enduml diff --git a/doc/source/images/audit_state_machine.png b/doc/source/images/audit_state_machine.png index 58c64aa48..afe21c937 100644 Binary files a/doc/source/images/audit_state_machine.png and b/doc/source/images/audit_state_machine.png differ diff --git a/releasenotes/notes/suspended-audit-state-07f998c94e9d9a47.yaml b/releasenotes/notes/suspended-audit-state-07f998c94e9d9a47.yaml new file mode 100644 index 000000000..cf66a44ea --- /dev/null +++ b/releasenotes/notes/suspended-audit-state-07f998c94e9d9a47.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + Added SUSPENDED audit state diff --git a/watcher/api/controllers/v1/audit.py b/watcher/api/controllers/v1/audit.py index fed8c43fc..fe29325ab 100644 --- a/watcher/api/controllers/v1/audit.py +++ b/watcher/api/controllers/v1/audit.py @@ -50,21 +50,6 @@ from watcher.decision_engine import rpcapi from watcher import objects -ALLOWED_AUDIT_TRANSITIONS = { - objects.audit.State.PENDING: - [objects.audit.State.ONGOING, objects.audit.State.CANCELLED], - objects.audit.State.ONGOING: - [objects.audit.State.FAILED, objects.audit.State.SUCCEEDED, - objects.audit.State.CANCELLED], - objects.audit.State.FAILED: - [objects.audit.State.DELETED], - objects.audit.State.SUCCEEDED: - [objects.audit.State.DELETED], - objects.audit.State.CANCELLED: - [objects.audit.State.DELETED] - } - - class AuditPostType(wtypes.Base): audit_template_uuid = wtypes.wsattr(types.uuid, mandatory=False) @@ -144,8 +129,15 @@ class AuditPatchType(types.JsonPatchType): @staticmethod def validate(patch): - serialized_patch = {'path': patch.path, 'op': patch.op} - if patch.path in AuditPatchType.mandatory_attrs(): + + def is_new_state_none(p): + return p.path == '/state' and p.op == 'replace' and p.value is None + + serialized_patch = {'path': patch.path, + 'op': patch.op, + 'value': patch.value} + if (patch.path in AuditPatchType.mandatory_attrs() or + is_new_state_none(patch)): msg = _("%(field)s can't be updated.") raise exception.PatchError( patch=serialized_patch, @@ -572,21 +564,22 @@ class AuditsController(rest.RestController): try: audit_dict = audit_to_update.as_dict() + + initial_state = audit_dict['state'] + new_state = api_utils.get_patch_value(patch, 'state') + if not api_utils.check_audit_state_transition( + patch, initial_state): + error_message = _("State transition not allowed: " + "(%(initial_state)s -> %(new_state)s)") + raise exception.PatchError( + patch=patch, + reason=error_message % dict( + initial_state=initial_state, new_state=new_state)) + audit = Audit(**api_utils.apply_jsonpatch(audit_dict, patch)) except api_utils.JSONPATCH_EXCEPTIONS as e: raise exception.PatchError(patch=patch, reason=e) - initial_state = audit_dict['state'] - new_state = api_utils.get_patch_value(patch, 'state') - allowed_states = ALLOWED_AUDIT_TRANSITIONS.get(initial_state, []) - if new_state is not None and new_state not in allowed_states: - error_message = _("State transition not allowed: " - "(%(initial_state)s -> %(new_state)s)") - raise exception.PatchError( - patch=patch, - reason=error_message % dict( - initial_state=initial_state, new_state=new_state)) - # Update only the fields that have changed for field in objects.Audit.fields: try: diff --git a/watcher/api/controllers/v1/utils.py b/watcher/api/controllers/v1/utils.py index 54edcf018..255c9ac04 100644 --- a/watcher/api/controllers/v1/utils.py +++ b/watcher/api/controllers/v1/utils.py @@ -79,6 +79,15 @@ def get_patch_value(patch, key): return p['value'] +def check_audit_state_transition(patch, initial): + is_transition_valid = True + state_value = get_patch_value(patch, "state") + if state_value is not None: + is_transition_valid = objects.audit.AuditStateTransitionManager( + ).check_transition(initial, state_value) + return is_transition_valid + + def as_filters_dict(**filters): filters_dict = {} for filter_name, filter_value in filters.items(): diff --git a/watcher/decision_engine/audit/continuous.py b/watcher/decision_engine/audit/continuous.py index 551bd97c2..91fb530c0 100644 --- a/watcher/decision_engine/audit/continuous.py +++ b/watcher/decision_engine/audit/continuous.py @@ -49,9 +49,7 @@ class ContinuousAuditHandler(base.AuditHandler): def _is_audit_inactive(self, audit): audit = objects.Audit.get_by_uuid( self.context_show_deleted, audit.uuid) - if audit.state in (objects.audit.State.CANCELLED, - objects.audit.State.DELETED, - objects.audit.State.FAILED): + if objects.audit.AuditStateTransitionManager().is_inactive(audit): # if audit isn't in active states, audit's job must be removed to # prevent using of inactive audit in future. job_to_delete = [job for job in self.jobs diff --git a/watcher/objects/audit.py b/watcher/objects/audit.py index 0dc489c5c..fa9bdd77b 100644 --- a/watcher/objects/audit.py +++ b/watcher/objects/audit.py @@ -46,6 +46,9 @@ be one of the following: - **CANCELLED** : the :ref:`Audit ` was in **PENDING** or **ONGOING** state and was cancelled by the :ref:`Administrator ` +- **SUSPENDED** : the :ref:`Audit ` was in **ONGOING** + state and was suspended by the + :ref:`Administrator ` """ import enum @@ -66,6 +69,7 @@ class State(object): CANCELLED = 'CANCELLED' DELETED = 'DELETED' PENDING = 'PENDING' + SUSPENDED = 'SUSPENDED' class AuditType(enum.Enum): @@ -296,3 +300,25 @@ class Audit(base.WatcherPersistentObject, base.WatcherObject, notifications.audit.send_delete(self._context, self) _notify() + + +class AuditStateTransitionManager(object): + + TRANSITIONS = { + State.PENDING: [State.ONGOING, State.CANCELLED], + State.ONGOING: [State.FAILED, State.SUCCEEDED, + State.CANCELLED, State.SUSPENDED], + State.FAILED: [State.DELETED], + State.SUCCEEDED: [State.DELETED], + State.CANCELLED: [State.DELETED], + State.SUSPENDED: [State.ONGOING, State.DELETED], + } + + INACTIVE_STATES = (State.CANCELLED, State.DELETED, + State.FAILED, State.SUSPENDED) + + def check_transition(self, initial, new): + return new in self.TRANSITIONS.get(initial, []) + + def is_inactive(self, audit): + return audit.state in self.INACTIVE_STATES diff --git a/watcher/tests/api/v1/test_audits.py b/watcher/tests/api/v1/test_audits.py index e842eaf8e..47749ee52 100644 --- a/watcher/tests/api/v1/test_audits.py +++ b/watcher/tests/api/v1/test_audits.py @@ -345,23 +345,10 @@ class TestPatch(api_base.FunctionalTest): ALLOWED_TRANSITIONS = [ - {"original_state": objects.audit.State.PENDING, - "new_state": objects.audit.State.ONGOING}, - {"original_state": objects.audit.State.PENDING, - "new_state": objects.audit.State.CANCELLED}, - {"original_state": objects.audit.State.ONGOING, - "new_state": objects.audit.State.FAILED}, - {"original_state": objects.audit.State.ONGOING, - "new_state": objects.audit.State.SUCCEEDED}, - {"original_state": objects.audit.State.ONGOING, - "new_state": objects.audit.State.CANCELLED}, - {"original_state": objects.audit.State.FAILED, - "new_state": objects.audit.State.DELETED}, - {"original_state": objects.audit.State.SUCCEEDED, - "new_state": objects.audit.State.DELETED}, - {"original_state": objects.audit.State.CANCELLED, - "new_state": objects.audit.State.DELETED}, -] + {"original_state": key, "new_state": value} + for key, values in ( + objects.audit.AuditStateTransitionManager.TRANSITIONS.items()) + for value in values] class TestPatchStateTransitionDenied(api_base.FunctionalTest): diff --git a/watcher/tests/db/test_audit.py b/watcher/tests/db/test_audit.py index 46b0a2aba..1b467bad5 100644 --- a/watcher/tests/db/test_audit.py +++ b/watcher/tests/db/test_audit.py @@ -53,6 +53,10 @@ class TestDbAuditFilters(base.DbTestCase): self.audit3 = utils.create_test_audit( audit_template_id=self.audit_template.id, id=3, uuid=None, state=objects.audit.State.CANCELLED) + with freezegun.freeze_time(self.FAKE_OLDER_DATE): + self.audit4 = utils.create_test_audit( + audit_template_id=self.audit_template.id, id=4, uuid=None, + state=objects.audit.State.SUSPENDED) def _soft_delete_audits(self): with freezegun.freeze_time(self.FAKE_TODAY): @@ -92,8 +96,9 @@ class TestDbAuditFilters(base.DbTestCase): res = self.dbapi.get_audit_list( self.context, filters={'deleted': False}) - self.assertEqual([self.audit2['id'], self.audit3['id']], - [r.id for r in res]) + self.assertEqual( + [self.audit2['id'], self.audit3['id'], self.audit4['id']], + [r.id for r in res]) def test_get_audit_list_filter_deleted_at_eq(self): self._soft_delete_audits() @@ -154,7 +159,7 @@ class TestDbAuditFilters(base.DbTestCase): self.context, filters={'created_at__lt': self.FAKE_TODAY}) self.assertEqual( - [self.audit2['id'], self.audit3['id']], + [self.audit2['id'], self.audit3['id'], self.audit4['id']], [r.id for r in res]) def test_get_audit_list_filter_created_at_lte(self): @@ -162,7 +167,7 @@ class TestDbAuditFilters(base.DbTestCase): self.context, filters={'created_at__lte': self.FAKE_OLD_DATE}) self.assertEqual( - [self.audit2['id'], self.audit3['id']], + [self.audit2['id'], self.audit3['id'], self.audit4['id']], [r.id for r in res]) def test_get_audit_list_filter_created_at_gt(self): @@ -230,18 +235,22 @@ class TestDbAuditFilters(base.DbTestCase): def test_get_audit_list_filter_state_in(self): res = self.dbapi.get_audit_list( self.context, - filters={'state__in': (objects.audit.State.FAILED, - objects.audit.State.CANCELLED)}) + filters={ + 'state__in': + objects.audit.AuditStateTransitionManager.INACTIVE_STATES + }) self.assertEqual( - [self.audit2['id'], self.audit3['id']], + [self.audit2['id'], self.audit3['id'], self.audit4['id']], [r.id for r in res]) def test_get_audit_list_filter_state_notin(self): res = self.dbapi.get_audit_list( self.context, - filters={'state__notin': (objects.audit.State.FAILED, - objects.audit.State.CANCELLED)}) + filters={ + 'state__notin': + objects.audit.AuditStateTransitionManager.INACTIVE_STATES + }) self.assertEqual( [self.audit1['id']], diff --git a/watcher/tests/decision_engine/audit/test_audit_handlers.py b/watcher/tests/decision_engine/audit/test_audit_handlers.py index 5d0f1d826..42079c5f3 100644 --- a/watcher/tests/decision_engine/audit/test_audit_handlers.py +++ b/watcher/tests/decision_engine/audit/test_audit_handlers.py @@ -274,16 +274,18 @@ class TestContinuousAuditHandler(base.DbTestCase): audit_handler = continuous.ContinuousAuditHandler(mock.MagicMock()) mock_list.return_value = self.audits mock_jobs.return_value = mock.MagicMock() - self.audits[1].state = objects.audit.State.CANCELLED - calls = [mock.call(audit_handler.execute_audit, 'interval', - args=[mock.ANY, mock.ANY], - seconds=3600, - name='execute_audit', - next_run_time=mock.ANY)] - audit_handler.launch_audits_periodically() - m_add_job.assert_has_calls(calls) - audit_handler.update_audit_state(self.audits[1], - objects.audit.State.CANCELLED) - is_inactive = audit_handler._is_audit_inactive(self.audits[1]) + for state in [objects.audit.State.CANCELLED, + objects.audit.State.SUSPENDED]: + self.audits[1].state = state + calls = [mock.call(audit_handler.execute_audit, 'interval', + args=[mock.ANY, mock.ANY], + seconds=3600, + name='execute_audit', + next_run_time=mock.ANY)] + audit_handler.launch_audits_periodically() + m_add_job.assert_has_calls(calls) + + audit_handler.update_audit_state(self.audits[1], state) + is_inactive = audit_handler._is_audit_inactive(self.audits[1]) self.assertTrue(is_inactive) diff --git a/watcher_tempest_plugin/tests/api/admin/base.py b/watcher_tempest_plugin/tests/api/admin/base.py index bd236a768..d1fba680d 100644 --- a/watcher_tempest_plugin/tests/api/admin/base.py +++ b/watcher_tempest_plugin/tests/api/admin/base.py @@ -27,9 +27,16 @@ class BaseInfraOptimTest(test.BaseTestCase): """Base class for Infrastructure Optimization API tests.""" # States where the object is waiting for some event to perform a transition - IDLE_STATES = ('RECOMMENDED', 'FAILED', 'SUCCEEDED', 'CANCELLED') + IDLE_STATES = ('RECOMMENDED', + 'FAILED', + 'SUCCEEDED', + 'CANCELLED', + 'SUSPENDED') # States where the object can only be DELETED (end of its life-cycle) - FINISHED_STATES = ('FAILED', 'SUCCEEDED', 'CANCELLED', 'SUPERSEDED') + FINISHED_STATES = ('FAILED', + 'SUCCEEDED', + 'CANCELLED', + 'SUPERSEDED') @classmethod def setup_credentials(cls): diff --git a/watcher_tempest_plugin/tests/api/admin/test_audit.py b/watcher_tempest_plugin/tests/api/admin/test_audit.py index d764082a0..44c2f9fc1 100644 --- a/watcher_tempest_plugin/tests/api/admin/test_audit.py +++ b/watcher_tempest_plugin/tests/api/admin/test_audit.py @@ -29,7 +29,7 @@ class TestCreateUpdateDeleteAudit(base.BaseInfraOptimTest): """Tests for audit.""" audit_states = ['ONGOING', 'SUCCEEDED', 'FAILED', - 'CANCELLED', 'DELETED', 'PENDING'] + 'CANCELLED', 'DELETED', 'PENDING', 'SUSPENDED'] def assert_expected(self, expected, actual, keys=('created_at', 'updated_at', @@ -154,7 +154,7 @@ class TestShowListAudit(base.BaseInfraOptimTest): """Tests for audit.""" audit_states = ['ONGOING', 'SUCCEEDED', 'FAILED', - 'CANCELLED', 'DELETED', 'PENDING'] + 'CANCELLED', 'DELETED', 'PENDING', 'SUSPENDED'] @classmethod def resource_setup(cls): diff --git a/watcher_tempest_plugin/tests/scenario/test_execute_basic_optim.py b/watcher_tempest_plugin/tests/scenario/test_execute_basic_optim.py index 50dab9559..b4b5e7611 100644 --- a/watcher_tempest_plugin/tests/scenario/test_execute_basic_optim.py +++ b/watcher_tempest_plugin/tests/scenario/test_execute_basic_optim.py @@ -156,7 +156,7 @@ class TestExecuteBasicStrategy(base.BaseInfraOptimScenarioTest): self.fail("The audit has failed!") _, finished_audit = self.client.show_audit(audit['uuid']) - if finished_audit.get('state') in ('FAILED', 'CANCELLED'): + if finished_audit.get('state') in ('FAILED', 'CANCELLED', 'SUSPENDED'): self.fail("The audit ended in unexpected state: %s!" % finished_audit.get('state'))