fix: stop running instance if activation restart failed at validation (#1093)

Co-authored-by: Alex <aizquier@redhat.com>

Author: bzwei

src/aap_eda/api/views/activation.py

@@ -413,8 +413,15 @@ def restart(self, request, pk):
                 detail="Activation is disabled and cannot be run."
             )
 
+        # Redis must be available in order to perform the restart.
+        self.redis_is_available()
+
         valid, error = is_activation_valid(activation)
         if not valid:
+            stop_rulebook_process(
+                process_parent_type=ProcessParentType.ACTIVATION,
+                process_parent_id=activation.id,
+            )
             activation.status = ActivationStatus.ERROR
             activation.status_message = error
             activation.save(update_fields=["status", "status_message"])
@@ -424,9 +431,6 @@ def restart(self, request, pk):
                 {"errors": error}, status=status.HTTP_400_BAD_REQUEST
             )
 
-        # Redis must be available in order to perform the restart.
-        self.redis_is_available()
-
         restart_rulebook_process(
             process_parent_type=ProcessParentType.ACTIVATION,
             process_parent_id=activation.id,

src/aap_eda/services/activation/activation_manager.py

@@ -718,7 +718,8 @@ def stop(self):
             return
 
         try:
-            self.set_status(ActivationStatus.STOPPING)
+            if self.db_instance.status != ActivationStatus.ERROR:
+                self.set_status(ActivationStatus.STOPPING)
             self._stop_instance()
 
         except engine_exceptions.ContainerEngineError as exc:
@@ -730,7 +731,10 @@ def stop(self):
             self._error_activation(msg)
             raise exceptions.ActivationStopError(msg) from exc
         user_msg = "Stop requested by user."
-        self.set_status(ActivationStatus.STOPPED, user_msg)
+        if self.db_instance.status != ActivationStatus.ERROR:
+            # do not overwrite the status and message if the activation
+            # is already in error status
+            self.set_status(ActivationStatus.STOPPED, user_msg)
         container_logger = self.container_logger_class(self.latest_instance.id)
         container_logger.write(user_msg, flush=True)
         LOGGER.info(

tests/integration/api/test_activation.py

@@ -961,14 +961,23 @@ def test_restart_activation_with_required_token_deleted(
     assert response.status_code == status.HTTP_201_CREATED
     token = models.AwxToken.objects.get(id=admin_awx_token.id)
     token.delete()
+    activation_id = response.data["id"]
 
-    response = admin_client.post(
-        f"{api_url_v1}/activations/{response.data['id']}/restart/",
-    )
-    assert response.status_code == status.HTTP_400_BAD_REQUEST
-    assert (
-        "The rulebook requires a RH AAP credential." in response.data["errors"]
-    )
+    with mock.patch(
+        "aap_eda.api.views.activation.stop_rulebook_process"
+    ) as mock_stop:
+        response = admin_client.post(
+            f"{api_url_v1}/activations/{activation_id}/restart/",
+        )
+        assert response.status_code == status.HTTP_400_BAD_REQUEST
+        assert (
+            "The rulebook requires a RH AAP credential."
+            in response.data["errors"]
+        )
+        mock_stop.assert_called_once_with(
+            process_parent_type=enums.ProcessParentType.ACTIVATION,
+            process_parent_id=activation_id,
+        )
 
 
 @pytest.mark.django_db

tests/integration/services/activation/test_manager.py

@@ -403,13 +403,37 @@ def test_stop_already_stopped(
     assert not container_engine_mock.cleanup.called
 
 
+@pytest.mark.parametrize(
+    ("before_status", "after_status", "before_msg", "after_msg"),
+    [
+        (
+            enums.ActivationStatus.RUNNING,
+            enums.ActivationStatus.STOPPED,
+            "running",
+            "Stop requested by user.",
+        ),
+        (
+            enums.ActivationStatus.ERROR,
+            enums.ActivationStatus.ERROR,
+            "activation validation failed",
+            "activation validation failed",
+        ),
+    ],
+)
 @pytest.mark.django_db
 def test_stop_running(
     running_activation: models.Activation,
     container_engine_mock: MagicMock,
     eda_caplog: LogCaptureFixture,
+    before_status,
+    after_status,
+    before_msg,
+    after_msg,
 ):
     """Test stop verb when activation is running."""
+    running_activation.status = before_status
+    running_activation.status_message = before_msg
+    running_activation.save(update_fields=["status", "status_message"])
     activation_manager = ActivationManager(
         container_engine=container_engine_mock,
         db_instance=running_activation,
@@ -419,13 +443,13 @@ def test_stop_running(
     assert "Stopping" in eda_caplog.text
     assert "Cleanup operation requested" in eda_caplog.text
     assert "Activation stopped." in eda_caplog.text
-    assert running_activation.status == enums.ActivationStatus.STOPPED
+    assert running_activation.status == after_status
     assert (
         running_activation.latest_instance.status
         == enums.ActivationStatus.STOPPED
     )
     assert running_activation.latest_instance.activation_pod_id is None
-    assert running_activation.status_message == "Stop requested by user."
+    assert running_activation.status_message == after_msg
     assert container_engine_mock.cleanup.called