diff --git a/newsfragments/fix-stop-wrong-dependents-on-compose-down.bugfix b/newsfragments/fix-stop-wrong-dependents-on-compose-down.bugfix new file mode 100644 index 00000000..36fe51d4 --- /dev/null +++ b/newsfragments/fix-stop-wrong-dependents-on-compose-down.bugfix @@ -0,0 +1 @@ +Fix `podman-compose down service` stops wrong dependents diff --git a/podman_compose.py b/podman_compose.py index b4b24d54..0d0957f6 100755 --- a/podman_compose.py +++ b/podman_compose.py @@ -1433,6 +1433,16 @@ def rec_deps( return deps +def calc_dependents(services: dict[str, Any]) -> None: + for name, srv in services.items(): + deps: set[ServiceDependency] = srv.get("_deps", set()) + for dep in deps: + if dep.name in services: + services[dep.name].setdefault(DependField.DEPENDENTS, set()).add( + ServiceDependency(name, dep.condition.value) + ) + + def flat_deps(services: dict[str, Any], with_extends: bool = False) -> None: """ create dependencies "_deps" or update it recursively for all services @@ -1470,6 +1480,8 @@ def flat_deps(services: dict[str, Any], with_extends: bool = False) -> None: for name, srv in services.items(): rec_deps(services, name) + calc_dependents(services) + ################### # Override and reset tags @@ -3024,14 +3036,23 @@ async def create_pods(compose: PodmanCompose) -> None: await compose.podman.run([], "pod", podman_args) -def get_excluded(compose: PodmanCompose, args: argparse.Namespace) -> set[str]: +class DependField(str, Enum): + DEPENDENCIES = "_deps" + DEPENDENTS = "_dependents" + + +def get_excluded( + compose: PodmanCompose, + args: argparse.Namespace, + dep_field: DependField = DependField.DEPENDENCIES, +) -> set[str]: excluded = set() if args.services: excluded = set(compose.services) for service in args.services: # we need 'getattr' as compose_down_parse dose not configure 'no_deps' if service in compose.services and not getattr(args, "no_deps", False): - excluded -= set(x.name for x in compose.services[service]["_deps"]) + excluded -= set(x.name for x in compose.services[service].get(dep_field, set())) excluded.discard(service) log.debug("** excluding: %s", excluded) return excluded @@ -3322,7 +3343,7 @@ def get_volume_names(compose: PodmanCompose, cnt: dict) -> list[str]: @cmd_run(podman_compose, "down", "tear down entire stack") async def compose_down(compose: PodmanCompose, args: argparse.Namespace) -> None: - excluded = get_excluded(compose, args) + excluded = get_excluded(compose, args, DependField.DEPENDENTS) podman_args: list[str] = [] timeout_global = getattr(args, "timeout", None) containers = list(reversed(compose.containers)) diff --git a/tests/integration/compose_down_behavior/__init__.py b/tests/integration/compose_down_behavior/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/integration/compose_down_behavior/docker-compose_default.yaml b/tests/integration/compose_down_behavior/docker-compose_default.yaml new file mode 100644 index 00000000..8eb0ff16 --- /dev/null +++ b/tests/integration/compose_down_behavior/docker-compose_default.yaml @@ -0,0 +1,12 @@ +services: + app: + image: nopush/podman-compose-test + command: ["dumb-init", "/bin/busybox", "httpd", "-f", "-p", "8080"] + depends_on: + - db + db: + image: nopush/podman-compose-test + command: ["dumb-init", "/bin/busybox", "httpd", "-f", "-p", "8080"] + no_deps: + image: nopush/podman-compose-test + command: ["dumb-init", "/bin/busybox", "httpd", "-f", "-p", "8080"] diff --git a/tests/integration/compose_down_behavior/test_compose_down_behavior.py b/tests/integration/compose_down_behavior/test_compose_down_behavior.py new file mode 100644 index 00000000..81f14473 --- /dev/null +++ b/tests/integration/compose_down_behavior/test_compose_down_behavior.py @@ -0,0 +1,81 @@ +# SPDX-License-Identifier: GPL-2.0 + +import os +import unittest + +from parameterized import parameterized + +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(scenario: str) -> str: + return os.path.join( + os.path.join(test_path(), "compose_down_behavior"), f"docker-compose_{scenario}.yaml" + ) + + +class TestComposeDownBehavior(unittest.TestCase, RunSubprocessMixin): + @parameterized.expand([ + ("default", ["down"], set()), + ( + "default", + ["down", "app"], + { + "compose_down_behavior_db_1", + "compose_down_behavior_no_deps_1", + }, + ), + ( + "default", + ["down", "db"], + { + "compose_down_behavior_no_deps_1", + }, + ), + ]) + def test_compose_down( + self, scenario: str, command_args: list[str], expect_containers: set[str] + ) -> None: + try: + self.run_subprocess_assert_returncode( + [podman_compose_path(), "-f", compose_yaml_path(scenario), "up", "-d"], + ) + + self.run_subprocess_assert_returncode( + [ + podman_compose_path(), + "-f", + compose_yaml_path(scenario), + *command_args, + ], + ) + + out, _ = self.run_subprocess_assert_returncode( + [ + podman_compose_path(), + "-f", + compose_yaml_path(scenario), + "ps", + "--format", + '{{ .Names }}', + ], + ) + + actual_containers = set() + for line in out.decode('utf-8').strip().split('\n'): + name = line.strip() + if name: + actual_containers.add(name) + + self.assertEqual(actual_containers, expect_containers) + finally: + self.run_subprocess_assert_returncode([ + podman_compose_path(), + "-f", + compose_yaml_path(scenario), + "down", + "-t", + "0", + ]) diff --git a/tests/unit/test_depends_on.py b/tests/unit/test_depends_on.py new file mode 100644 index 00000000..fff6e63a --- /dev/null +++ b/tests/unit/test_depends_on.py @@ -0,0 +1,53 @@ +import unittest +from typing import Any + +from parameterized import parameterized + +from podman_compose import flat_deps + + +class TestDependsOn(unittest.TestCase): + @parameterized.expand([ + ( + { + "service_a": {}, + "service_b": {"depends_on": {"service_a": {"condition": "healthy"}}}, + "service_c": {"depends_on": {"service_b": {"condition": "healthy"}}}, + }, + # dependencies + { + "service_a": set(), + "service_b": set(["service_a"]), + "service_c": set(["service_a", "service_b"]), + }, + # dependents + { + "service_a": set(["service_b", "service_c"]), + "service_b": set(["service_c"]), + "service_c": set(), + }, + ), + ]) + def test_flat_deps( + self, + services: dict[str, Any], + deps: dict[str, set[str]], + dependents: dict[str, set[str]], + ) -> None: + flat_deps(services) + self.assertEqual( + { + name: set([x.name for x in value.get("_deps", set())]) + for name, value in services.items() + }, + deps, + msg="Dependencies do not match", + ) + self.assertEqual( + { + name: set([x.name for x in value.get("_dependents", set())]) + for name, value in services.items() + }, + dependents, + msg="Dependents do not match", + )