Skip stop actions in pre_condition phase

This patch is implementing detection of certain conditions and moving
the action to SKIPPED status so that, execution is not started. We will
skip it if:

- The instance does not exists
- The instance is already stopped

Implements: blueprint skip-actions-in-pre-condition
Change-Id: I54a0017536a83206f55e5b6cd0637480d5b798fe
Signed-off-by: Alfredo Moralejo <amoralej@redhat.com>

doc/source/actions/stop.rst

@@ -21,3 +21,12 @@ parameter                type   required description
 ======================== ====== ======== ===================================
 ``resource_id``          string yes      UUID of the server instance to stop
 ======================== ====== ======== ===================================
+
+Skipping conditions
+--------------------
+
+Stop actions will be automatically skipped in the pre_condition phase in
+the following cases:
+
+- The server does not exist
+- The server is already stopped

releasenotes/notes/stop-precondition-check-95f2e5fe83355a4e.yaml

@@ -0,0 +1,8 @@
+---
+features:
+  - |
+    Added a pre-condition check to the stop action to prevent executing
+    it when required criteria are not met. In following conditions,
+    the action status will be set to SKIPPED:
+    - Instance is not found
+    - Instance is stopped

watcher/applier/actions/stop.py

@@ -14,7 +14,10 @@
 #
 
 from oslo_log import log
+
+from watcher._i18n import _
 from watcher.applier.actions import base
+from watcher.common import exception
 from watcher.common import nova_helper
 
 LOG = log.getLogger(__name__)
@@ -138,27 +141,27 @@ class Stop(base.BaseAction):
         return False
 
     def pre_condition(self):
-        # Check for instance existence and its state
+        """Check stop preconditions
+
+        Skipping conditions:
+        - Instance does not exist
+        - Instance is stopped
+        """
         nova = nova_helper.NovaHelper(osc=self.osc)
+
+        # Check that the instance exists
         try:
             instance = nova.find_instance(self.instance_uuid)
-            if not instance:
-                LOG.debug(
-                    "Instance %(uuid)s not found during pre-condition check. "
-                    "Considering this acceptable for stop operation.",
-                    {'uuid': self.instance_uuid}
-                )
-                return
+        except nova_helper.nvexceptions.NotFound:
+            raise exception.ActionSkipped(
+                _("Instance %s not found") % self.instance_uuid)
 
-            # Log instance current state
-            current_state = instance.status
-            LOG.debug("Instance %s pre-condition check: state=%s",
-                      self.instance_uuid, current_state)
-
-        except Exception as exc:
-            LOG.exception("Pre-condition check failed for instance %s: %s",
-                          self.instance_uuid, str(exc))
-            raise
+        current_state = instance.status
+        LOG.debug("Instance %s pre-condition check: state=%s",
+                  self.instance_uuid, current_state)
+        if current_state == 'SHUTOFF':
+            raise exception.ActionSkipped(
+                _("Instance %s is already stopped") % self.instance_uuid)
 
     def post_condition(self):
         pass

watcher/tests/applier/actions/test_stop.py

@@ -17,9 +17,12 @@ from unittest import mock
 import fixtures
 import jsonschema
 
+from novaclient.v2 import servers
+
 from watcher.applier.actions import base as baction
 from watcher.applier.actions import stop
 from watcher.common import exception
+from watcher.common import nova_helper
 from watcher.tests import base
 
 
@@ -38,6 +41,12 @@ class TestStop(base.TestCase):
         self.input_parameters = {
             baction.BaseAction.RESOURCE_ID: self.INSTANCE_UUID,
         }
+        instance_info = {
+            'id': self.INSTANCE_UUID,
+            'status': 'ACTIVE',
+        }
+        self.instance = servers.Server(
+            servers.ServerManager, info=instance_info)
         self.action = stop.Stop(mock.Mock())
         self.action.input_parameters = self.input_parameters
 
@@ -68,29 +77,30 @@ class TestStop(base.TestCase):
         self.assertEqual(self.INSTANCE_UUID, self.action.instance_uuid)
 
     def test_pre_condition_instance_not_found(self):
-        self.m_helper.find_instance.return_value = None
+        self.m_helper.find_instance.side_effect = (
+            nova_helper.nvexceptions.NotFound('404'))
 
-        result = self.action.pre_condition()
+        # ActionSkipped is expected because the instance is not found
+        self.assertRaisesRegex(
+            exception.ActionSkipped,
+            f"Instance {self.INSTANCE_UUID} not found",
+            self.action.pre_condition)
 
-        # Instance not found can be considered acceptable (idempotent)
-        self.assertIsNone(result)
         self.m_helper.find_instance.assert_called_once_with(self.INSTANCE_UUID)
 
     def test_pre_condition_instance_already_stopped(self):
-        instance = mock.Mock()
-        instance.status = 'stopped'
-        self.m_helper.find_instance.return_value = instance
+        self.instance.status = 'SHUTOFF'
+        self.m_helper.find_instance.return_value = self.instance
 
-        result = self.action.pre_condition()
-
-        # All valid states should return None (implicit success)
-        self.assertIsNone(result)
+        # ActionSkipped is expected because the instance is already stopped
+        self.assertRaisesRegex(
+            exception.ActionSkipped,
+            f"Instance {self.INSTANCE_UUID} is already stopped",
+            self.action.pre_condition)
         self.m_helper.find_instance.assert_called_once_with(self.INSTANCE_UUID)
 
     def test_pre_condition_instance_active(self):
-        instance = mock.Mock()
-        instance.status = 'active'
-        self.m_helper.find_instance.return_value = instance
+        self.m_helper.find_instance.return_value = self.instance
 
         result = self.action.pre_condition()
 
@@ -115,8 +125,7 @@ class TestStop(base.TestCase):
 
     def test_execute_stop_failure_instance_exists(self):
         # Instance exists but stop operation fails
-        instance = mock.Mock()
-        self.m_helper.find_instance.return_value = instance
+        self.m_helper.find_instance.return_value = self.instance
         self.m_helper.stop_instance.return_value = False
 
         result = self.action.execute()