From 7abd41c83ca563ba88019316efb38ee9ad9533af Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 18 Dec 2024 19:18:05 +0100 Subject: [PATCH] fix: handle 500 podman response when getting status Signed-off-by: Alex --- .../services/activation/activation_manager.py | 13 ++++++- .../services/activation/engine/podman.py | 13 +++++-- .../services/activation/engine/test_podman.py | 32 +++++++++++++++-- .../services/activation/test_manager.py | 36 +++++++++++++++++++ 4 files changed, 88 insertions(+), 6 deletions(-) diff --git a/src/aap_eda/services/activation/activation_manager.py b/src/aap_eda/services/activation/activation_manager.py index e3d3147fc..35da974cf 100644 --- a/src/aap_eda/services/activation/activation_manager.py +++ b/src/aap_eda/services/activation/activation_manager.py @@ -884,10 +884,21 @@ def monitor(self): # get the status of the container container_status = None - with contextlib.suppress(engine_exceptions.ContainerNotFoundError): + try: container_status = self.container_engine.get_status( container_id=self.latest_instance.activation_pod_id, ) + except engine_exceptions.ContainerNotFoundError: + pass + except engine_exceptions.ContainerEngineError as exc: + msg = ( + f"Monitor operation: activation id: {self.db_instance.id} " + f"Failed to get status of the container. Reason: {exc}" + ) + LOGGER.error(msg) + self._error_instance(msg) + self._error_activation(msg) + raise exceptions.ActivationMonitorError(msg) from exc # Activations in running status must have a container # This case prevents cases when the container is externally deleted diff --git a/src/aap_eda/services/activation/engine/podman.py b/src/aap_eda/services/activation/engine/podman.py index 551522a9b..6a67881b4 100644 --- a/src/aap_eda/services/activation/engine/podman.py +++ b/src/aap_eda/services/activation/engine/podman.py @@ -174,12 +174,19 @@ def start(self, request: ContainerRequest, log_handler: LogHandler) -> str: raise exceptions.ContainerStartError(error_message) from e def get_status(self, container_id: str) -> ContainerStatus: - if not self.client.containers.exists(container_id): + try: + container = self.client.containers.get(container_id) + except NotFound: raise exceptions.ContainerNotFoundError( f"Container id {container_id} not found" ) - - container = self.client.containers.get(container_id) + except APIError as exc: + # podman might return a 500 error when the container is not found. + if "no such container" in str(exc): + raise exceptions.ContainerNotFoundError( + f"Container id {container_id} not found" + ) + raise exceptions.ContainerEngineError(str(exc)) from exc error_msg = container.attrs.get("State").get("Error", "") # Check container status diff --git a/tests/integration/services/activation/engine/test_podman.py b/tests/integration/services/activation/engine/test_podman.py index 88813f4c4..f7a8dcfa6 100644 --- a/tests/integration/services/activation/engine/test_podman.py +++ b/tests/integration/services/activation/engine/test_podman.py @@ -32,6 +32,7 @@ ) from aap_eda.services.activation.engine.exceptions import ( ContainerCleanupError, + ContainerEngineError, ContainerEngineInitError, ContainerImagePullError, ContainerLoginError, @@ -532,10 +533,37 @@ def test_engine_get_status(podman_engine): @pytest.mark.django_db -def test_engine_get_status_with_exception(podman_engine): +def test_engine_get_status_with_not_found_exception(podman_engine): engine = podman_engine - engine.client.containers.exists.return_value = None + engine.client.containers.get.side_effect = NotFound("Not found") + + with pytest.raises( + ContainerNotFoundError, match="Container id 100 not found" + ): + engine.get_status("100") + + +@pytest.mark.django_db +def test_engine_get_status_with_api_error_exception(podman_engine): + engine = podman_engine + + engine.client.containers.get.side_effect = APIError("unexpected error") + + with pytest.raises(ContainerEngineError): + engine.get_status("100") + + +@pytest.mark.django_db +def test_engine_get_status_with_500_not_found(podman_engine): + engine = podman_engine + error_msg = ( + "no container with ID " + "fe244749060b9b546af41eb4256f8e527a031748a64ea3ec93bd821daffc8d89 " + "found in database: no such container" + ) + + engine.client.containers.get.side_effect = APIError(error_msg) with pytest.raises( ContainerNotFoundError, match="Container id 100 not found" diff --git a/tests/integration/services/activation/test_manager.py b/tests/integration/services/activation/test_manager.py index 6ee83f104..3d36c790b 100644 --- a/tests/integration/services/activation/test_manager.py +++ b/tests/integration/services/activation/test_manager.py @@ -378,6 +378,42 @@ def test_monitor_to_running_status( assert starting_activation.restart_count == 0 +@pytest.mark.django_db +def test_monitor_to_unexpected_error_status( + running_activation: models.Activation, + container_engine_mock: MagicMock, +): + """Test monitor when get_status returns an unexpected error.""" + activation_manager = ActivationManager( + db_instance=running_activation, + container_engine=container_engine_mock, + ) + container_engine_mock.get_status.side_effect = ( + engine_exceptions.ContainerEngineError("unexpected error") + ) + with pytest.raises(exceptions.ActivationMonitorError): + activation_manager.monitor() + assert running_activation.status == enums.ActivationStatus.ERROR + assert "unexpected error" in running_activation.status_message + + +@pytest.mark.django_db +def test_monitor_container_not_found( + running_activation: models.Activation, + container_engine_mock: MagicMock, +): + """Test monitor when get_status returns a container not found error.""" + activation_manager = ActivationManager( + db_instance=running_activation, + container_engine=container_engine_mock, + ) + container_engine_mock.get_status.side_effect = ( + engine_exceptions.ContainerNotFoundError("Not found") + ) + activation_manager.monitor() + assert running_activation.status == enums.ActivationStatus.FAILED + + @pytest.mark.django_db def test_start_restart( running_activation: models.Activation,