diff --git a/newsfragments/check_dep_conditions_asyncio_gather.change b/newsfragments/check_dep_conditions_asyncio_gather.change new file mode 100644 index 00000000..3656f4f0 --- /dev/null +++ b/newsfragments/check_dep_conditions_asyncio_gather.change @@ -0,0 +1 @@ +Use per-container ``asyncio.gather`` waits in ``check_dep_conditions`` instead of a single blocking ``podman wait`` with all containers at once. diff --git a/podman_compose.py b/podman_compose.py index 2b6e74e1..8117ae7e 100755 --- a/podman_compose.py +++ b/podman_compose.py @@ -3691,29 +3691,34 @@ async def check_dep_conditions(compose: PodmanCompose, deps: set) -> None: deps_cd.extend(compose.container_names_by_service[d.name]) if deps_cd: - # podman wait will return always with a rc -1. - while True: - try: - await compose.podman.output( - [], "wait", [f"--condition={condition.value}"] + deps_cd - ) - log.debug( - "dependencies for condition %s have been fulfilled on containers %s", - condition.value, - ', '.join(deps_cd), - ) - break - except subprocess.CalledProcessError as _exc: - output = list( - ((_exc.stdout or b"") + (_exc.stderr or b"")).decode().split('\n') - ) - log.debug( - 'Podman wait returned an error (%d) when executing "%s": %s', - _exc.returncode, - _exc.cmd, - output, - ) - await asyncio.sleep(1) + + async def wait_one( + d_cnt: str, condition: ServiceDependencyCondition = condition + ) -> None: + while True: + try: + await compose.podman.output( + [], "wait", [f"--condition={condition.value}", d_cnt] + ) + log.debug( + "dependency for condition %s has been fulfilled on container %s", + condition.value, + d_cnt, + ) + break + except subprocess.CalledProcessError as _exc: + output = list( + ((_exc.stdout or b"") + (_exc.stderr or b"")).decode().split('\n') + ) + log.debug( + 'Podman wait returned an error (%d) when executing "%s": %s', + _exc.returncode, + _exc.cmd, + output, + ) + await asyncio.sleep(1) + + await asyncio.gather(*(wait_one(cnt) for cnt in deps_cd)) async def run_container( diff --git a/tests/unit/test_depends_on.py b/tests/unit/test_depends_on.py index fff6e63a..4f56b0a0 100644 --- a/tests/unit/test_depends_on.py +++ b/tests/unit/test_depends_on.py @@ -1,8 +1,12 @@ +import subprocess import unittest from typing import Any +from unittest import mock from parameterized import parameterized +from podman_compose import ServiceDependency +from podman_compose import check_dep_conditions from podman_compose import flat_deps @@ -51,3 +55,159 @@ def test_flat_deps( dependents, msg="Dependents do not match", ) + + +class TestCheckDepConditions(unittest.IsolatedAsyncioTestCase): + async def test_empty_deps_does_nothing(self) -> None: + """check_dep_conditions with empty deps should return without any waits""" + compose = mock.Mock() + compose.podman.output = mock.AsyncMock() + + await check_dep_conditions(compose, set()) + + compose.podman.output.assert_not_called() + + async def test_per_container_wait_single_condition(self) -> None: + """Each container gets its own podman wait call, not batched together""" + compose = mock.Mock() + compose.podman.output = mock.AsyncMock() + compose.podman_version = None + compose.container_names_by_service = { + "srva": ["cnt_a1", "cnt_a2"], + } + deps = { + ServiceDependency("srva", "service_completed_successfully"), + } + + await check_dep_conditions(compose, deps) + + calls = [c.args for c in compose.podman.output.call_args_list] + self.assertEqual( + calls, + [ + ([], "wait", ["--condition=stopped", "cnt_a1"]), + ([], "wait", ["--condition=stopped", "cnt_a2"]), + ], + ) + + async def test_per_container_wait_multiple_conditions(self) -> None: + """Each condition's containers get individual wait calls""" + compose = mock.Mock() + compose.podman.output = mock.AsyncMock() + compose.podman_version = None + compose.container_names_by_service = { + "srva": ["cnt_a"], + "srvb": ["cnt_b"], + } + deps = { + ServiceDependency("srva", "service_completed_successfully"), + ServiceDependency("srvb", "service_started"), + } + + await check_dep_conditions(compose, deps) + + calls = [c.args for c in compose.podman.output.call_args_list] + self.assertEqual( + calls, + [ + ([], "wait", ["--condition=running", "cnt_b"]), + ([], "wait", ["--condition=stopped", "cnt_a"]), + ], + ) + + async def test_retry_on_error(self) -> None: + """podman wait failure is retried (not fatal)""" + compose = mock.Mock() + compose.podman.output = mock.AsyncMock() + compose.podman.output.side_effect = [ + subprocess.CalledProcessError(1, "podman wait", b"", b"error"), + None, + ] + compose.podman_version = None + compose.container_names_by_service = { + "srva": ["cnt_a"], + } + deps = { + ServiceDependency("srva", "service_completed_successfully"), + } + + await check_dep_conditions(compose, deps) + + self.assertEqual(compose.podman.output.call_count, 2) + + async def test_skips_healthy_below_4_6_0(self) -> None: + """Healthcheck condition is skipped on podman < 4.6.0""" + compose = mock.Mock() + compose.podman.output = mock.AsyncMock() + compose.podman_version = "4.5.0" + compose.container_names_by_service = { + "srva": ["cnt_a"], + } + deps = { + ServiceDependency("srva", "service_healthy"), + } + + await check_dep_conditions(compose, deps) + + compose.podman.output.assert_not_called() + + async def test_skips_unhealthy_below_4_6_0(self) -> None: + """UNHEALTHY condition is also skipped on podman < 4.6.0""" + compose = mock.Mock() + compose.podman.output = mock.AsyncMock() + compose.podman_version = "4.5.0" + compose.container_names_by_service = { + "srva": ["cnt_a"], + } + deps = { + ServiceDependency("srva", "unhealthy"), + } + + await check_dep_conditions(compose, deps) + + compose.podman.output.assert_not_called() + + async def test_healthcheck_on_4_6_0_or_newer(self) -> None: + """HEALTHY waits normally on podman >= 4.6.0 (not skipped)""" + compose = mock.Mock() + compose.podman.output = mock.AsyncMock() + compose.podman_version = "4.6.0" + compose.container_names_by_service = { + "srva": ["cnt_a"], + } + deps = { + ServiceDependency("srva", "service_healthy"), + } + + await check_dep_conditions(compose, deps) + + calls = [c.args for c in compose.podman.output.call_args_list] + self.assertEqual( + calls, + [ + ([], "wait", ["--condition=healthy", "cnt_a"]), + ], + ) + + async def test_healthcheck_when_podman_version_none(self) -> None: + """When podman_version is None, healthcheck is not skipped + (the version gate short-circuits: None is not < 4.6.0)""" + compose = mock.Mock() + compose.podman.output = mock.AsyncMock() + compose.podman_version = None + compose.container_names_by_service = { + "srva": ["cnt_a"], + } + deps = { + ServiceDependency("srva", "service_healthy"), + } + + await check_dep_conditions(compose, deps) + + calls = [c.args for c in compose.podman.output.call_args_list] + self.assertEqual( + calls, + [ + ([], "wait", ["--condition=healthy", "cnt_a"]), + ], + )