From 38622442f207bbef0fb8e20dc722d9e6b39aff0b Mon Sep 17 00:00:00 2001
From: Alfredo Moralejo <amoralej@redhat.com>
Date: Thu, 8 May 2025 18:13:41 +0200
Subject: [PATCH] Set actionplan state to FAILED if any action has failed

Currently, an actionplan state is set to SUCCEEDED once the execution
has finished, but that does not imply that all the actions finished
successfully.

This patch is checking 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 and will apply the
appropiate notification parameters. This is the expected behavior according
to Watcher documentation.

The patch is also fixing the unit test for this to set the expected
action plan state to FAILED and notification parameters.

Closes-Bug: #2106407
Change-Id: I7bfc6759b51cd97c26ec13b3918bd8d3b7ac9d4e
(cherry picked from commit 88d81c104edb24f482e588ca6e2914dfa07ee0dd)
Signed-off-by: Alfredo Moralejo <amoralej@redhat.com>
---
 ...lan-state-on-failure-69e498d902ada5c5.yaml | 13 +++++++++++
 watcher/applier/action_plan/default.py        | 22 +++++++++++++++++--
 .../test_default_action_handler.py            |  8 +++----
 3 files changed, 37 insertions(+), 6 deletions(-)
 create mode 100644 releasenotes/notes/fix-action-plan-state-on-failure-69e498d902ada5c5.yaml

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