diff --git a/newsfragments/1184.bugfix b/newsfragments/1184.bugfix new file mode 100644 index 0000000..9226bd9 --- /dev/null +++ b/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) diff --git a/podman_compose.py b/podman_compose.py index 4808dd9..7cc4ef2 100755 --- a/podman_compose.py +++ b/podman_compose.py @@ -2815,6 +2815,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: @@ -2891,28 +2903,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, rmi=None)) 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 diff --git a/tests/integration/deps/docker-compose-conditional-healthy.yaml b/tests/integration/deps/docker-compose-conditional-healthy.yaml new file mode 100644 index 0000000..59b0d03 --- /dev/null +++ b/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 diff --git a/tests/integration/deps/test_podman_compose_deps.py b/tests/integration/deps/test_podman_compose_deps.py index e15f6ec..9a4bfa5 100644 --- a/tests/integration/deps/test_podman_compose_deps.py +++ b/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 @@ class TestComposeBaseDeps(unittest.TestCase, RunSubprocessMixin): 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 @@ class TestComposeBaseDeps(unittest.TestCase, RunSubprocessMixin): "--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 @@ class TestComposeConditionalDeps(unittest.TestCase, RunSubprocessMixin): 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 @@ class TestComposeConditionalDeps(unittest.TestCase, RunSubprocessMixin): 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", + ]) diff --git a/tests/integration/extends/test_podman_compose_extends.py b/tests/integration/extends/test_podman_compose_extends.py index 30e6a89..f4002d8 100644 --- a/tests/integration/extends/test_podman_compose_extends.py +++ b/tests/integration/extends/test_podman_compose_extends.py @@ -80,18 +80,25 @@ class TestComposeExteds(unittest.TestCase, RunSubprocessMixin): "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', ], ) diff --git a/tests/integration/filesystem/test_podman_compose_filesystem.py b/tests/integration/filesystem/test_podman_compose_filesystem.py index d78e369..9539e6e 100644 --- a/tests/integration/filesystem/test_podman_compose_filesystem.py +++ b/tests/integration/filesystem/test_podman_compose_filesystem.py @@ -35,8 +35,7 @@ class TestFilesystem(unittest.TestCase, RunSubprocessMixin): "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([ diff --git a/tests/integration/nets_test1/test_podman_compose_nets_test1.py b/tests/integration/nets_test1/test_podman_compose_nets_test1.py index 916be05..62db8fe 100644 --- a/tests/integration/nets_test1/test_podman_compose_nets_test1.py +++ b/tests/integration/nets_test1/test_podman_compose_nets_test1.py @@ -59,9 +59,13 @@ class TestComposeNetsTest1(unittest.TestCase, RunSubprocessMixin): ) # 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 @@ class TestComposeNetsTest1(unittest.TestCase, RunSubprocessMixin): 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") diff --git a/tests/integration/nets_test2/test_podman_compose_nets_test2.py b/tests/integration/nets_test2/test_podman_compose_nets_test2.py index 73590ba..93e6d4e 100644 --- a/tests/integration/nets_test2/test_podman_compose_nets_test2.py +++ b/tests/integration/nets_test2/test_podman_compose_nets_test2.py @@ -59,9 +59,13 @@ class TestComposeNetsTest2(unittest.TestCase, RunSubprocessMixin): ) # 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 @@ class TestComposeNetsTest2(unittest.TestCase, RunSubprocessMixin): 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") diff --git a/tests/integration/test_utils.py b/tests/integration/test_utils.py index 475762d..5d170ba 100644 --- a/tests/integration/test_utils.py +++ b/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 @@ class RunSubprocessMixin: 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") diff --git a/tests/integration/ulimit/test_podman_compose_ulimit.py b/tests/integration/ulimit/test_podman_compose_ulimit.py index 283cd27..4d2ef28 100644 --- a/tests/integration/ulimit/test_podman_compose_ulimit.py +++ b/tests/integration/ulimit/test_podman_compose_ulimit.py @@ -34,14 +34,11 @@ class TestUlimit(unittest.TestCase, RunSubprocessMixin): 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 @@ class TestUlimit(unittest.TestCase, RunSubprocessMixin): 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 @@ class TestUlimit(unittest.TestCase, RunSubprocessMixin): 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([