From 2e7d83f7f01526ccb0d2f2d696988ef879c1b547 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Mon, 17 Mar 2025 18:42:32 -0400 Subject: [PATCH] Properly surface errors from build commands the commit 38b13a3 ("Use asyncio for subprocess calls") broke the way exit codes are reported from the podman compose build command. The tasks are awaited as they finish which means that if a later build finishes sucessfully after a failing build, it overwrites status. Previously the `parse_return_code` function would skip updating the status if the new return code was zero, but in removing it, this logic was not carried forward. Fixes: 38b13a3 ("Use asyncio for subprocess calls") Signed-off-by: charliemirabile <46761267+charliemirabile@users.noreply.github.com> --- podman_compose.py | 2 +- .../integration/build_fail_multi/__init__.py | 0 .../build_fail_multi/bad/Dockerfile | 3 ++ .../build_fail_multi/docker-compose.yml | 8 +++++ .../build_fail_multi/good/Dockerfile | 3 ++ .../test_podman_compose_build_fail_multi.py | 31 +++++++++++++++++++ 6 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 tests/integration/build_fail_multi/__init__.py create mode 100644 tests/integration/build_fail_multi/bad/Dockerfile create mode 100644 tests/integration/build_fail_multi/docker-compose.yml create mode 100644 tests/integration/build_fail_multi/good/Dockerfile create mode 100644 tests/integration/build_fail_multi/test_podman_compose_build_fail_multi.py diff --git a/podman_compose.py b/podman_compose.py index df1f9ee..8920065 100755 --- a/podman_compose.py +++ b/podman_compose.py @@ -2613,7 +2613,7 @@ async def compose_build(compose, args): status = 0 for t in asyncio.as_completed(tasks): s = await t - if s is not None: + if s is not None and s != 0: status = s return status diff --git a/tests/integration/build_fail_multi/__init__.py b/tests/integration/build_fail_multi/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/integration/build_fail_multi/bad/Dockerfile b/tests/integration/build_fail_multi/bad/Dockerfile new file mode 100644 index 0000000..8ee7eb8 --- /dev/null +++ b/tests/integration/build_fail_multi/bad/Dockerfile @@ -0,0 +1,3 @@ +FROM busybox + +RUN false diff --git a/tests/integration/build_fail_multi/docker-compose.yml b/tests/integration/build_fail_multi/docker-compose.yml new file mode 100644 index 0000000..d5de8e9 --- /dev/null +++ b/tests/integration/build_fail_multi/docker-compose.yml @@ -0,0 +1,8 @@ +version: "3" +services: + bad: + build: + context: bad + good: + build: + context: good diff --git a/tests/integration/build_fail_multi/good/Dockerfile b/tests/integration/build_fail_multi/good/Dockerfile new file mode 100644 index 0000000..d1c8850 --- /dev/null +++ b/tests/integration/build_fail_multi/good/Dockerfile @@ -0,0 +1,3 @@ +FROM busybox +#ensure that this build finishes second so that it has a chance to overwrite the return code +RUN sleep 0.5 diff --git a/tests/integration/build_fail_multi/test_podman_compose_build_fail_multi.py b/tests/integration/build_fail_multi/test_podman_compose_build_fail_multi.py new file mode 100644 index 0000000..167becb --- /dev/null +++ b/tests/integration/build_fail_multi/test_podman_compose_build_fail_multi.py @@ -0,0 +1,31 @@ +# SPDX-License-Identifier: GPL-2.0 + +import os +import unittest + +from tests.integration.test_utils import RunSubprocessMixin +from tests.integration.test_utils import podman_compose_path +from tests.integration.test_utils import test_path + + +def compose_yaml_path(): + """ "Returns the path to the compose file used for this test module""" + base_path = os.path.join(test_path(), "build_fail_multi") + return os.path.join(base_path, "docker-compose.yml") + + +class TestComposeBuildFailMulti(unittest.TestCase, RunSubprocessMixin): + def test_build_fail_multi(self): + output, error = self.run_subprocess_assert_returncode( + [ + podman_compose_path(), + "-f", + compose_yaml_path(), + "build", + # prevent the successful build from being cached to ensure it runs long enough + "--no-cache", + ], + expected_returncode=1, + ) + self.assertIn("RUN false", str(output)) + self.assertIn("while running runtime: exit status 1", str(error))