Fix #1183 to enforce service_healthy condition

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

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

Author: schnell18

.pre-commit-config.yaml

@@ -23,6 +23,20 @@ repos:
             "-sn", # Don't display the score
             "--rcfile=.pylintrc", # Link to your config file
           ]
+        files: ^[^/]+\.py$
+      # Hook for test code with different config
+      - id: pylint
+        name: pylint-tests
+        entry: pylint
+        language: system
+        types: [python]
+        args:
+          [
+            "-rn", # Only display messages
+            "-sn", # Don't display the score
+            "--rcfile=tests/.pylintrc" # Relax the lint rules for test code
+          ]
+        files: ^tests/
   - repo: https://github.com/codespell-project/codespell
     rev: v2.2.5
     hooks:

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/.pylintrc

@@ -0,0 +1,18 @@
+[MESSAGES CONTROL]
+# C0111 missing-docstring: missing-class-docstring, missing-function-docstring, missing-method-docstring, missing-module-docstrin
+# consider-using-with: we need it for color formatter pipe
+disable=too-many-lines,too-many-branches,too-many-locals,too-many-statements,too-many-arguments,too-many-instance-attributes,fixme,multiple-statements,missing-docstring,line-too-long,consider-using-f-string,consider-using-with,unnecessary-lambda-assignment,broad-exception-caught,duplicate-code,missing-timeout
+# allow _ for ignored variables
+# allow generic names like a,b,c and i,j,k,l,m,n and x,y,z
+# allow k,v for key/value
+# allow e for exceptions, it for iterator, ix for index
+# allow ip for ip address
+# allow w,h for width, height
+# allow op for operation/operator/opcode
+# allow t, t0, t1, t2, and t3 for time
+# allow dt for delta time
+# allow db for database
+# allow ls for list
+# allow p for pipe
+# allow ex for examples, exists ..etc
+good-names=_,a,b,c,dt,db,e,f,fn,fd,i,j,k,v,kv,kw,l,m,n,ls,t,t0,t1,t2,t3,w,h,x,y,z,it,ix,ip,op,p,ex

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

@@ -1,8 +1,10 @@
 # SPDX-License-Identifier: GPL-2.0
 import os
+import time
 import unittest
 
 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,74 @@ def test_deps_fails(self):
                 compose_yaml_path(suffix),
                 "down",
             ])
+
+
+class TestComposeConditionalDepsHealthy(unittest.TestCase, RunSubprocessMixin):
+    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",
+            ])
+
+            # The `podman wait --condition=healthy` is invalid prior to 4.6.0.
+            # Since the podman-compose project uses podman 4.3.1 in github actions, we
+            # use sleep as workaround to wait until the `sleep` container becomes running.
+            time.sleep(3)
+
+            # 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')
+            web_cnt_id = ""
+            web_cnt_name = ""
+            web_cnt_status = ""
+            web_cnt_started = ""
+            sleep_cnt_id = ""
+            sleep_cnt_name = ""
+            sleep_cnt_started = ""
+            for line in decoded_out.split("\n"):
+                if "web" in line:
+                    web_cnt_id, web_cnt_name, web_cnt_status, web_cnt_started = line.split("\t")
+                if "sleep" in line:
+                    sleep_cnt_id, sleep_cnt_name, _, sleep_cnt_started = line.split("\t")
+            self.assertNotEqual("", web_cnt_id)
+            self.assertEqual("deps_web_1", web_cnt_name)
+            self.assertNotEqual("", sleep_cnt_id)
+            self.assertEqual("deps_sleep_1", sleep_cnt_name)
+            if is_systemd_available():
+                self.assertIn("healthy", web_cnt_status)
+            else:
+                # 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 `running` state
+                self.assertIn("running", 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

@@ -21,6 +21,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