Fix #1183 to enforce service_healthy condition

- Change compose-up to create then start container to avoid
  double-execution and enforce dependency condition check
- Skip dependency health check to avoid compose-up hang for podman prior
  to 4.6.0, which doesn't support --condition healthy
- Skip running compose-down when there are no active containers
- Add relevant integration test case

Signed-off-by: Justin Zhang <[email protected]>

Author: schnell18

newsfragments/1184.bugfix

@@ -0,0 +1,3 @@
+- Fixed issue in up command where service_healthy conditions weren't being enforced (#1183)
+- Fixed issue where short-lived containers would execute twice when using the up command in detached mode (#1176)
+- Fixed up command hangs on Podman versions earlier than 4.6.0 (#1178)

podman_compose.py

@@ -2708,6 +2708,18 @@ async def check_dep_conditions(compose: PodmanCompose, deps: set) -> None:
         deps_cd = []
         for d in deps:
             if d.condition == condition:
+                if (
+                    d.condition
+                    in (ServiceDependencyCondition.HEALTHY, ServiceDependencyCondition.UNHEALTHY)
+                ) and strverscmp_lt(compose.podman_version, "4.6.0"):
+                    log.warning(
+                        "Ignored %s condition check due to podman %s doesn't support %s!",
+                        d.name,
+                        compose.podman_version,
+                        condition.value,
+                    )
+                    continue
+
                 deps_cd.extend(compose.container_names_by_service[d.name])
 
         if deps_cd:
@@ -2784,28 +2796,25 @@ async def compose_up(compose: PodmanCompose, args):
         .splitlines()
     )
     diff_hashes = [i for i in hashes if i and i != compose.yaml_hash]
-    if args.force_recreate or len(diff_hashes):
+    if (args.force_recreate and len(hashes) > 0) or len(diff_hashes):
         log.info("recreating: ...")
         down_args = argparse.Namespace(**dict(args.__dict__, volumes=False))
         await compose.commands["down"](compose, down_args)
         log.info("recreating: done\n\n")
     # args.no_recreate disables check for changes (which is not implemented)
 
-    podman_command = "run" if args.detach and not args.no_start else "create"
-
     await create_pods(compose, args)
     for cnt in compose.containers:
         if cnt["_service"] in excluded:
             log.debug("** skipping: %s", cnt["name"])
             continue
-        podman_args = await container_to_args(
-            compose, cnt, detached=args.detach, no_deps=args.no_deps
-        )
-        subproc = await compose.podman.run([], podman_command, podman_args)
-        if podman_command == "run" and subproc is not None:
+        podman_args = await container_to_args(compose, cnt, detached=False, no_deps=args.no_deps)
+        subproc = await compose.podman.run([], "create", podman_args)
+        if not args.no_start and args.detach and subproc is not None:
             await run_container(
                 compose, cnt["name"], deps_from_container(args, cnt), ([], "start", [cnt["name"]])
             )
+
     if args.no_start or args.detach or args.dry_run:
         return
     # TODO: handle already existing

tests/integration/deps/docker-compose-conditional-healthy.yaml

@@ -0,0 +1,23 @@
+version: "3.7"
+services:
+  web:
+    image: nopush/podman-compose-test
+    command: ["dumb-init", "/bin/busybox", "httpd", "-f", "-h", "/etc/", "-p", "8000"]
+    tmpfs:
+      - /run
+      - /tmp
+    healthcheck:
+      test: ["CMD", "wget", "-qO-", "http://localhost:8000/hosts"]
+      start_period: 10s # initialization time for containers that need time to bootstrap
+      interval: 10s # Time between health checks
+      timeout: 5s # Time to wait for a response
+      retries: 3 # Number of consecutive failures before marking as unhealthy
+  sleep:
+    image: nopush/podman-compose-test
+    command: ["dumb-init", "/bin/busybox", "sh", "-c", "sleep 3600"]
+    depends_on:
+      web:
+        condition: service_healthy
+    tmpfs:
+      - /run
+      - /tmp

tests/integration/deps/test_podman_compose_deps.py

@@ -2,7 +2,9 @@
 import os
 import unittest
 
+from tests.integration.test_utils import PodmanAwareRunSubprocessMixin
 from tests.integration.test_utils import RunSubprocessMixin
+from tests.integration.test_utils import is_systemd_available
 from tests.integration.test_utils import podman_compose_path
 from tests.integration.test_utils import test_path
 
@@ -14,7 +16,7 @@ def compose_yaml_path(suffix=""):
 class TestComposeBaseDeps(unittest.TestCase, RunSubprocessMixin):
     def test_deps(self):
         try:
-            output, error = self.run_subprocess_assert_returncode([
+            output, _ = self.run_subprocess_assert_returncode([
                 podman_compose_path(),
                 "-f",
                 compose_yaml_path(),
@@ -37,7 +39,7 @@ def test_deps(self):
 
     def test_run_nodeps(self):
         try:
-            output, error = self.run_subprocess_assert_returncode([
+            output, _ = self.run_subprocess_assert_returncode([
                 podman_compose_path(),
                 "-f",
                 compose_yaml_path(),
@@ -71,7 +73,7 @@ def test_up_nodeps(self):
                 "--detach",
                 "sleep",
             ])
-            output, error = self.run_subprocess_assert_returncode([
+            output, _ = self.run_subprocess_assert_returncode([
                 podman_compose_path(),
                 "-f",
                 compose_yaml_path(),
@@ -144,7 +146,7 @@ class TestComposeConditionalDeps(unittest.TestCase, RunSubprocessMixin):
     def test_deps_succeeds(self):
         suffix = "-conditional-succeeds"
         try:
-            output, error = self.run_subprocess_assert_returncode([
+            output, _ = self.run_subprocess_assert_returncode([
                 podman_compose_path(),
                 "-f",
                 compose_yaml_path(suffix),
@@ -168,7 +170,7 @@ def test_deps_succeeds(self):
     def test_deps_fails(self):
         suffix = "-conditional-fails"
         try:
-            output, error = self.run_subprocess_assert_returncode([
+            output, _ = self.run_subprocess_assert_returncode([
                 podman_compose_path(),
                 "-f",
                 compose_yaml_path(suffix),
@@ -183,3 +185,82 @@ def test_deps_fails(self):
                 compose_yaml_path(suffix),
                 "down",
             ])
+
+
+class TestComposeConditionalDepsHealthy(unittest.TestCase, PodmanAwareRunSubprocessMixin):
+    def setUp(self):
+        self.podman_version = self.retrieve_podman_version()
+
+    def test_up_deps_healthy(self):
+        suffix = "-conditional-healthy"
+        try:
+            self.run_subprocess_assert_returncode([
+                podman_compose_path(),
+                "-f",
+                compose_yaml_path(suffix),
+                "up",
+                "sleep",
+                "--detach",
+            ])
+
+            # Since the command `podman wait --condition=healthy` is invalid prior to 4.6.0,
+            # we only validate healthy status for podman 4.6.0+, which won't be tested in the
+            # CI pipeline of the podman-compose project where podman 4.3.1 is employed.
+            podman_ver_major, podman_ver_minor, podman_ver_patch = self.podman_version
+            if podman_ver_major >= 4 and podman_ver_minor >= 6 and podman_ver_patch >= 0:
+                self.run_subprocess_assert_returncode([
+                    "podman",
+                    "wait",
+                    "--condition=running",
+                    "deps_web_1",
+                    "deps_sleep_1",
+                ])
+
+            # check both web and sleep are running
+            output, _ = self.run_subprocess_assert_returncode([
+                podman_compose_path(),
+                "-f",
+                compose_yaml_path(),
+                "ps",
+                "--format",
+                "{{.ID}}\t{{.Names}}\t{{.Status}}\t{{.StartedAt}}",
+            ])
+
+            # extract container id of web
+            decoded_out = output.decode('utf-8')
+            lines = decoded_out.split("\n")
+
+            web_lines = [line for line in lines if "web" in line]
+            self.assertTrue(web_lines)
+            self.assertEqual(1, len(web_lines))
+            web_cnt_id, web_cnt_name, web_cnt_status, web_cnt_started = web_lines[0].split("\t")
+            self.assertNotEqual("", web_cnt_id)
+            self.assertEqual("deps_web_1", web_cnt_name)
+
+            sleep_lines = [line for line in lines if "sleep" in line]
+            self.assertTrue(sleep_lines)
+            self.assertEqual(1, len(sleep_lines))
+            sleep_cnt_id, sleep_cnt_name, _, sleep_cnt_started = sleep_lines[0].split("\t")
+            self.assertNotEqual("", sleep_cnt_id)
+            self.assertEqual("deps_sleep_1", sleep_cnt_name)
+
+            # When test case is executed inside container like github actions, the absence of
+            # systemd prevents health check from working properly, resulting in failure to
+            # transit to healthy state. As a result, we only assert the `healthy` state where
+            # systemd is functioning.
+            if (
+                is_systemd_available()
+                and podman_ver_major >= 4
+                and podman_ver_minor >= 6
+                and podman_ver_patch >= 0
+            ):
+                self.assertIn("healthy", web_cnt_status)
+            self.assertGreaterEqual(int(sleep_cnt_started), int(web_cnt_started))
+
+        finally:
+            self.run_subprocess_assert_returncode([
+                podman_compose_path(),
+                "-f",
+                compose_yaml_path(),
+                "down",
+            ])

tests/integration/extends/test_podman_compose_extends.py

@@ -80,18 +80,25 @@ def test_extends_service_launch_env1(self):
                 "env1",
             ])
             lines = output.decode('utf-8').split('\n')
-            # HOSTNAME name is random string so is ignored in asserting
-            lines = sorted([line for line in lines if not line.startswith("HOSTNAME")])
+            # Test selected env variables to improve robustness
+            lines = sorted([
+                line
+                for line in lines
+                if line.startswith("BAR")
+                or line.startswith("BAZ")
+                or line.startswith("FOO")
+                or line.startswith("HOME")
+                or line.startswith("PATH")
+                or line.startswith("container")
+            ])
             self.assertEqual(
                 lines,
                 [
-                    '',
                     'BAR=local',
                     'BAZ=local',
                     'FOO=original',
                     'HOME=/root',
                     'PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin',
-                    'TERM=xterm',
                     'container=podman',
                 ],
             )

tests/integration/filesystem/test_podman_compose_filesystem.py

@@ -35,8 +35,7 @@ def test_compose_symlink(self):
                 "container1",
             ])
 
-            # BUG: figure out why cat is called twice
-            self.assertEqual(out, b'data_compose_symlink\ndata_compose_symlink\n')
+            self.assertEqual(out, b'data_compose_symlink\n')
 
         finally:
             out, _ = self.run_subprocess_assert_returncode([

tests/integration/nets_test1/test_podman_compose_nets_test1.py

@@ -59,9 +59,13 @@ def test_nets_test1(self):
             )
 
             # check if Host port is the same as provided by the service port
+            self.assertIsNotNone(container_info['NetworkSettings']["Ports"].get("8001/tcp", None))
+            self.assertGreater(len(container_info['NetworkSettings']["Ports"]["8001/tcp"]), 0)
+            self.assertIsNotNone(
+                container_info['NetworkSettings']["Ports"]["8001/tcp"][0].get("HostPort", None)
+            )
             self.assertEqual(
-                container_info['NetworkSettings']["Ports"],
-                {"8001/tcp": [{"HostIp": "", "HostPort": "8001"}]},
+                container_info['NetworkSettings']["Ports"]["8001/tcp"][0]["HostPort"], "8001"
             )
 
             self.assertEqual(container_info["Config"]["Hostname"], "web1")
@@ -77,9 +81,13 @@ def test_nets_test1(self):
                 list(container_info["NetworkSettings"]["Networks"].keys())[0], "nets_test1_default"
             )
 
+            self.assertIsNotNone(container_info['NetworkSettings']["Ports"].get("8001/tcp", None))
+            self.assertGreater(len(container_info['NetworkSettings']["Ports"]["8001/tcp"]), 0)
+            self.assertIsNotNone(
+                container_info['NetworkSettings']["Ports"]["8001/tcp"][0].get("HostPort", None)
+            )
             self.assertEqual(
-                container_info['NetworkSettings']["Ports"],
-                {"8001/tcp": [{"HostIp": "", "HostPort": "8002"}]},
+                container_info['NetworkSettings']["Ports"]["8001/tcp"][0]["HostPort"], "8002"
             )
 
             self.assertEqual(container_info["Config"]["Hostname"], "web2")

tests/integration/nets_test2/test_podman_compose_nets_test2.py

@@ -59,9 +59,13 @@ def test_nets_test2(self):
             )
 
             # check if Host port is the same as prodvided by the service port
+            self.assertIsNotNone(container_info['NetworkSettings']["Ports"].get("8001/tcp", None))
+            self.assertGreater(len(container_info['NetworkSettings']["Ports"]["8001/tcp"]), 0)
+            self.assertIsNotNone(
+                container_info['NetworkSettings']["Ports"]["8001/tcp"][0].get("HostPort", None)
+            )
             self.assertEqual(
-                container_info['NetworkSettings']["Ports"],
-                {"8001/tcp": [{"HostIp": "", "HostPort": "8001"}]},
+                container_info['NetworkSettings']["Ports"]["8001/tcp"][0]["HostPort"], "8001"
             )
 
             self.assertEqual(container_info["Config"]["Hostname"], "web1")
@@ -78,9 +82,13 @@ def test_nets_test2(self):
                 list(container_info["NetworkSettings"]["Networks"].keys())[0], "nets_test2_mystack"
             )
 
+            self.assertIsNotNone(container_info['NetworkSettings']["Ports"].get("8001/tcp", None))
+            self.assertGreater(len(container_info['NetworkSettings']["Ports"]["8001/tcp"]), 0)
+            self.assertIsNotNone(
+                container_info['NetworkSettings']["Ports"]["8001/tcp"][0].get("HostPort", None)
+            )
             self.assertEqual(
-                container_info['NetworkSettings']["Ports"],
-                {"8001/tcp": [{"HostIp": "", "HostPort": "8002"}]},
+                container_info['NetworkSettings']["Ports"]["8001/tcp"][0]["HostPort"], "8002"
             )
 
             self.assertEqual(container_info["Config"]["Hostname"], "web2")

tests/integration/test_utils.py

@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
 import os
+import re
 import subprocess
 import time
 from pathlib import Path
@@ -21,6 +22,14 @@ def podman_compose_path():
     return os.path.join(base_path(), "podman_compose.py")
 
 
+def is_systemd_available():
+    try:
+        with open("/proc/1/comm", "r", encoding="utf-8") as fh:
+            return fh.read().strip() == "systemd"
+    except FileNotFoundError:
+        return False
+
+
 class RunSubprocessMixin:
     def is_debug_enabled(self):
         return "TESTS_DEBUG" in os.environ
@@ -52,3 +61,15 @@ def run_subprocess_assert_returncode(self, args, expected_returncode=0):
             f"stdout: {decoded_out}\nstderr: {decoded_err}\n",
         )
         return out, err
+
+
+class PodmanAwareRunSubprocessMixin(RunSubprocessMixin):
+    def retrieve_podman_version(self):
+        out, _ = self.run_subprocess_assert_returncode(["podman", "--version"])
+        matcher = re.match(r"\D*(\d+)\.(\d+)\.(\d+)", out.decode('utf-8'))
+        if matcher:
+            major = int(matcher.group(1))
+            minor = int(matcher.group(2))
+            patch = int(matcher.group(3))
+            return (major, minor, patch)
+        raise RuntimeError("Unable to retrieve podman version")

tests/integration/ulimit/test_podman_compose_ulimit.py

@@ -34,14 +34,11 @@ def test_ulimit(self):
                 for el in split_output
                 if not el.startswith(b"soft process") and not el.startswith(b"hard process")
             ]
-            # BUG: figure out why echo is called twice
             self.assertEqual(
                 output_part,
                 [
                     b"soft nofile limit 1001",
                     b"hard nofile limit 1001",
-                    b"soft nofile limit 1001",
-                    b"hard nofile limit 1001",
                 ],
             )
 
@@ -53,8 +50,7 @@ def test_ulimit(self):
             self.assertEqual(
                 out,
                 b"soft process limit 1002\nhard process limit 2002\nsoft nofile limit 1002\n"
-                b"hard nofile limit 1002\nsoft process limit 1002\nhard process limit 2002\n"
-                b"soft nofile limit 1002\nhard nofile limit 1002\n",
+                b"hard nofile limit 1002\n",
             )
 
             out, _ = self.run_subprocess_assert_returncode([
@@ -65,8 +61,7 @@ def test_ulimit(self):
             self.assertEqual(
                 out,
                 b"soft process limit 1003\nhard process limit 2003\nsoft nofile limit 1003\n"
-                b"hard nofile limit 1003\nsoft process limit 1003\nhard process limit 2003\n"
-                b"soft nofile limit 1003\nhard nofile limit 1003\n",
+                b"hard nofile limit 1003\n",
             )
         finally:
             self.run_subprocess_assert_returncode([