diff --git a/releasenotes/notes/fix-action-plan-state-on-failure-69e498d902ada5c5.yaml b/releasenotes/notes/fix-action-plan-state-on-failure-69e498d902ada5c5.yaml new file mode 100644 index 000000000..aa1a7bfdf --- /dev/null +++ b/releasenotes/notes/fix-action-plan-state-on-failure-69e498d902ada5c5.yaml @@ -0,0 +1,13 @@ +--- +fixes: + - | + Previously, if an action failed in an action plan, the state of the + action plan was reported as SUCCEEDED if the execution of the action has + finished regardless of the outcome. + + Watcher will now reflect the actual state of all the actions in the plan + after the execution has finished. If any action has status FAILED, it + will set the state of the action plan as FAILED. This is the expected + behavior according to Watcher documentation. + + For more info see: https://bugs.launchpad.net/watcher/+bug/2106407 diff --git a/watcher/applier/action_plan/default.py b/watcher/applier/action_plan/default.py index 012061a2c..be48ad538 100644 --- a/watcher/applier/action_plan/default.py +++ b/watcher/applier/action_plan/default.py @@ -56,12 +56,30 @@ class DefaultActionPlanHandler(base.BaseActionPlanHandler): applier = default.DefaultApplier(self.ctx, self.service) applier.execute(self.action_plan_uuid) - action_plan.state = objects.action_plan.State.SUCCEEDED + # If any action has failed the action plan should be FAILED + # Define default values for successful execution + ap_state = objects.action_plan.State.SUCCEEDED + notification_kwargs = { + 'phase': fields.NotificationPhase.END + } + + failed_filter = {'action_plan_uuid': self.action_plan_uuid, + 'state': objects.action.State.FAILED} + failed_actions = objects.Action.list( + self.ctx, filters=failed_filter, eager=True) + if failed_actions: + ap_state = objects.action_plan.State.FAILED + notification_kwargs = { + 'phase': fields.NotificationPhase.ERROR, + 'priority': fields.NotificationPriority.ERROR + } + + action_plan.state = ap_state action_plan.save() notifications.action_plan.send_action_notification( self.ctx, action_plan, action=fields.NotificationAction.EXECUTION, - phase=fields.NotificationPhase.END) + **notification_kwargs) except exception.ActionPlanCancelled as e: LOG.exception(e) diff --git a/watcher/tests/applier/action_plan/test_default_action_handler.py b/watcher/tests/applier/action_plan/test_default_action_handler.py index 99c61ecaf..f9e505de0 100644 --- a/watcher/tests/applier/action_plan/test_default_action_handler.py +++ b/watcher/tests/applier/action_plan/test_default_action_handler.py @@ -142,10 +142,10 @@ class TestDefaultActionPlanHandler(base.DbTestCase): phase=objects.fields.NotificationPhase.START), mock.call(self.context, self.action_plan, action=objects.fields.NotificationAction.EXECUTION, - phase=objects.fields.NotificationPhase.END)] - # (amoralej) the actual action_plan.state should beh FAILED. I am - # setting it to SUCCEEDDED and will change it in the fixing change. - self.assertEqual(ap_objects.State.SUCCEEDED, self.action_plan.state) + priority=objects.fields.NotificationPriority.ERROR, + phase=objects.fields.NotificationPhase.ERROR)] + + self.assertEqual(ap_objects.State.FAILED, self.action_plan.state) self.assertEqual( expected_calls, self.m_action_plan_notifications