Merge pull request #1283 from lisongmin/fix-stop-wrong-dependents-on-compose-down

fix: podman-compose down should not stop the upstream dependencies.

Author: p12tic

newsfragments/fix-stop-wrong-dependents-on-compose-down.bugfix

@@ -0,0 +1 @@
+Fix `podman-compose down service` stops wrong dependents

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))

tests/integration/compose_down_behavior/__init__.py

@@ -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"]

tests/integration/compose_down_behavior/docker-compose_default.yaml

@@ -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",
+            ])

tests/integration/compose_down_behavior/test_compose_down_behavior.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",
+        )