rework logic around determining containers to run healthchecks on

devservices/commands/up.py

@@ -146,16 +146,38 @@ def _up(
         mode_dependencies=mode_dependencies,
     )
 
-    cmd_outputs = []
+    containers_to_check = []
     with concurrent.futures.ThreadPoolExecutor() as dependency_executor:
         futures = [
             dependency_executor.submit(_bring_up_dependency, cmd, current_env, status)
             for cmd in docker_compose_commands
         ]
         for future in concurrent.futures.as_completed(futures):
-            cmd_outputs.append(future.result())
+            _ = future.result()
 
-    check_all_containers_healthy(status)
+    for cmd in docker_compose_commands:
+        try:
+            container_names = subprocess.check_output(
+                [
+                    "docker",
+                    "compose",
+                    "-p",
+                    cmd.project_name,
+                    "-f",
+                    cmd.config_path,
+                    "ps",
+                    "--format",
+                    "{{.Name}}",
+                ],
+                text=True,
+            ).splitlines()
+            containers_to_check.extend(container_names)
+        except subprocess.CalledProcessError:
+            status.failure(
+                f"Failed to get containers to healthcheck for {cmd.project_name}"
+            )
+            exit(1)
+    check_all_containers_healthy(status, containers_to_check)
 
 
 def _create_devservices_network() -> None:

devservices/utils/docker.py

@@ -4,7 +4,6 @@
 import subprocess
 import time
 
-from devservices.constants import DEVSERVICES_ORCHESTRATOR_LABEL
 from devservices.constants import HEALTHCHECK_INTERVAL
 from devservices.constants import HEALTHCHECK_TIMEOUT
 from devservices.exceptions import DockerDaemonNotRunningError
@@ -25,9 +24,8 @@ def check_docker_daemon_running() -> None:
         raise DockerDaemonNotRunningError from e
 
 
-def check_all_containers_healthy(status: Status) -> None:
+def check_all_containers_healthy(status: Status, containers: list[str]) -> None:
     """Ensures all containers are healthy."""
-    containers = get_matching_containers(DEVSERVICES_ORCHESTRATOR_LABEL)
     with concurrent.futures.ThreadPoolExecutor() as healthcheck_executor:
         futures = [
             healthcheck_executor.submit(wait_for_healthy, container, status)
@@ -65,7 +63,10 @@ def wait_for_healthy(container_name: str, status: Status) -> None:
                 stderr=e.stderr,
             ) from e
 
-        if result == "healthy" or result == "unknown":
+        if result == "healthy":
+            return
+        elif result == "unknown":
+            status.warning(f"Container {container_name} does not have a healthcheck.")
             return
 
         # If not healthy, wait and try again
@@ -87,8 +88,7 @@ def get_matching_containers(label: str) -> list[str]:
                 [
                     "docker",
                     "ps",
-                    "--format",
-                    "{{.Names}}",
+                    "-q",
                     "--filter",
                     f"label={label}",
                 ],

tests/commands/test_up.py

@@ -33,7 +33,12 @@
 @mock.patch("devservices.utils.state.State.update_started_service")
 @mock.patch("devservices.commands.up._create_devservices_network")
 @mock.patch("devservices.commands.up.check_all_containers_healthy")
+@mock.patch(
+    "devservices.commands.up.subprocess.check_output",
+    return_value="clickhouse\nredis\n",
+)
 def test_up_simple(
+    mock_subprocess_check_output: mock.Mock,
     mock_check_all_containers_healthy: mock.Mock,
     mock_create_devservices_network: mock.Mock,
     mock_update_started_service: mock.Mock,
@@ -240,7 +245,111 @@ def test_up_error(
 @mock.patch("devservices.utils.state.State.update_started_service")
 @mock.patch("devservices.commands.up._create_devservices_network")
 @mock.patch("devservices.commands.up.check_all_containers_healthy")
+@mock.patch(
+    "devservices.commands.up.subprocess.check_output",
+    side_effect=[
+        "clickhouse\nredis\n",
+        subprocess.CalledProcessError(returncode=1, cmd="docker compose ps"),
+    ],
+)
+def test_up_docker_compose_container_lookup_error(
+    mock_subprocess_check_output: mock.Mock,
+    mock_check_all_containers_healthy: mock.Mock,
+    mock_create_devservices_network: mock.Mock,
+    mock_update_started_service: mock.Mock,
+    mock_run: mock.Mock,
+    tmp_path: Path,
+    capsys: pytest.CaptureFixture[str],
+) -> None:
+    with mock.patch(
+        "devservices.commands.up.DEVSERVICES_DEPENDENCIES_CACHE_DIR",
+        str(tmp_path / "dependency-dir"),
+    ):
+        config = {
+            "x-sentry-service-config": {
+                "version": 0.1,
+                "service_name": "example-service",
+                "dependencies": {
+                    "redis": {"description": "Redis"},
+                    "clickhouse": {"description": "Clickhouse"},
+                },
+                "modes": {"default": ["redis", "clickhouse"]},
+            },
+            "services": {
+                "redis": {"image": "redis:6.2.14-alpine"},
+                "clickhouse": {
+                    "image": "altinity/clickhouse-server:23.8.11.29.altinitystable"
+                },
+            },
+        }
+
+        service_path = tmp_path / "example-service"
+        create_config_file(service_path, config)
+        os.chdir(service_path)
+
+        args = Namespace(service_name=None, debug=False, mode="default")
+
+        with pytest.raises(SystemExit):
+            up(args)
+
+        # Ensure the DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY is set and is relative
+        env_vars = mock_run.call_args[1]["env"]
+        assert (
+            env_vars[DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY]
+            == f"../dependency-dir/{DEPENDENCY_CONFIG_VERSION}"
+        )
+
+        mock_create_devservices_network.assert_called_once()
+
+        mock_run.assert_called_with(
+            [
+                "docker",
+                "compose",
+                "-p",
+                "example-service",
+                "-f",
+                f"{service_path}/{DEVSERVICES_DIR_NAME}/{CONFIG_FILE_NAME}",
+                "up",
+                "clickhouse",
+                "redis",
+                "-d",
+            ],
+            check=True,
+            capture_output=True,
+            text=True,
+            env=mock.ANY,
+        )
+
+        mock_update_started_service.assert_not_called()
+        mock_check_all_containers_healthy.assert_not_called()
+        captured = capsys.readouterr()
+        assert "Retrieving dependencies" in captured.out.strip()
+        assert "Starting 'example-service' in mode: 'default'" in captured.out.strip()
+        assert "Starting clickhouse" in captured.out.strip()
+        assert "Starting redis" in captured.out.strip()
+        assert (
+            "Failed to get containers to healthcheck for example-service"
+            in captured.out.strip()
+        )
+
+
+@mock.patch(
+    "devservices.utils.docker_compose.subprocess.run",
+    return_value=subprocess.CompletedProcess(
+        args=["docker", "compose", "config", "--services"],
+        returncode=0,
+        stdout="clickhouse\nredis\n",
+    ),
+)
+@mock.patch("devservices.utils.state.State.update_started_service")
+@mock.patch("devservices.commands.up._create_devservices_network")
+@mock.patch("devservices.commands.up.check_all_containers_healthy")
+@mock.patch(
+    "devservices.commands.up.subprocess.check_output",
+    return_value="clickhouse\nredis\n",
+)
 def test_up_mode_simple(
+    mock_subprocess_check_output: mock.Mock,
     mock_check_all_containers_healthy: mock.Mock,
     mock_create_devservices_network: mock.Mock,
     mock_update_started_service: mock.Mock,

tests/utils/test_docker.py

@@ -53,8 +53,7 @@ def test_get_matching_containers(
         [
             "docker",
             "ps",
-            "--format",
-            "{{.Names}}",
+            "-q",
             "--filter",
             f"label={DEVSERVICES_ORCHESTRATOR_LABEL}",
         ],
@@ -91,8 +90,7 @@ def test_get_matching_containers_error(
         [
             "docker",
             "ps",
-            "--format",
-            "{{.Names}}",
+            "-q",
             "--filter",
             f"label={DEVSERVICES_ORCHESTRATOR_LABEL}",
         ],
@@ -337,16 +335,12 @@ def test_wait_for_healthy_healthcheck_failed(mock_check_output: mock.Mock) -> No
 
 
 @mock.patch("devservices.utils.docker.subprocess.check_output")
-@mock.patch(
-    "devservices.utils.docker.get_matching_containers",
-    return_value=["container1", "container2"],
-)
 def test_check_all_containers_healthy_success(
-    mock_get_matching_containers: mock.Mock, mock_check_output: mock.Mock
+    mock_check_output: mock.Mock,
 ) -> None:
     mock_status = mock.Mock()
     mock_check_output.side_effect = ["healthy", "healthy"]
-    check_all_containers_healthy(mock_status)
+    check_all_containers_healthy(mock_status, ["container1", "container2"])
     mock_check_output.assert_has_calls(
         [
             mock.call(
@@ -377,19 +371,15 @@ def test_check_all_containers_healthy_success(
 
 
 @mock.patch("devservices.utils.docker.subprocess.check_output")
-@mock.patch(
-    "devservices.utils.docker.get_matching_containers",
-    return_value=["container1", "container2"],
-)
 def test_check_all_containers_healthy_failure(
-    mock_get_matching_containers: mock.Mock, mock_check_output: mock.Mock
+    mock_check_output: mock.Mock,
 ) -> None:
     mock_status = mock.Mock()
     mock_check_output.side_effect = ["healthy", "unhealthy", "unhealthy"]
     with mock.patch("devservices.utils.docker.HEALTHCHECK_TIMEOUT", 2), mock.patch(
         "devservices.utils.docker.HEALTHCHECK_INTERVAL", 1
     ):
-        check_all_containers_healthy(mock_status)
+        check_all_containers_healthy(mock_status, ["container1", "container2"])
     mock_check_output.assert_has_calls(
         [
             mock.call(