fix(down): Enable bringing down starting services

devservices/commands/down.py

@@ -76,14 +76,20 @@ def down(args: Namespace) -> None:
     modes = service.config.modes
 
     state = State()
-    started_services = state.get_service_entries(StateTables.STARTED_SERVICES)
-    if service.name not in started_services:
+    starting_services = set(state.get_service_entries(StateTables.STARTING_SERVICES))
+    started_services = set(state.get_service_entries(StateTables.STARTED_SERVICES))
+    active_services = starting_services.union(started_services)
+    if service.name not in active_services:
         console.warning(f"{service.name} is not running")
         exit(0)
 
-    active_modes = state.get_active_modes_for_service(
+    active_starting_modes = state.get_active_modes_for_service(
+        service.name, StateTables.STARTING_SERVICES
+    )
+    active_started_modes = state.get_active_modes_for_service(
         service.name, StateTables.STARTED_SERVICES
     )
+    active_modes = active_starting_modes or active_started_modes
     mode_dependencies = set()
     for active_mode in active_modes:
         active_mode_dependencies = modes.get(active_mode, [])
@@ -111,13 +117,20 @@ def down(args: Namespace) -> None:
 
         # Check if any service depends on the service we are trying to bring down
         # TODO: We should also take into account the active modes of the other services (this is not trivial to do)
-        other_started_services = set(started_services).difference({service.name})
+        other_started_services = active_services.difference({service.name})
         dependent_service_name = None
         for other_started_service in other_started_services:
             other_service = find_matching_service(other_started_service)
-            other_service_active_modes = state.get_active_modes_for_service(
+            other_service_active_starting_modes = state.get_active_modes_for_service(
+                other_service.name, StateTables.STARTING_SERVICES
+            )
+            other_service_active_started_modes = state.get_active_modes_for_service(
                 other_service.name, StateTables.STARTED_SERVICES
             )
+            other_service_active_modes = (
+                other_service_active_starting_modes
+                or other_service_active_started_modes
+            )
             dependency_graph = construct_dependency_graph(
                 other_service, other_service_active_modes
             )
@@ -140,7 +153,7 @@ def down(args: Namespace) -> None:
             )
 
     # TODO: We should factor in healthchecks here before marking service as not running
-    state = State()
+    state.remove_service_entry(service.name, StateTables.STARTING_SERVICES)
     state.remove_service_entry(service.name, StateTables.STARTED_SERVICES)
     if dependent_service_name is None:
         console.success(f"{service.name} stopped")

devservices/commands/up.py

@@ -79,6 +79,8 @@ def up(args: Namespace) -> None:
     modes = service.config.modes
     mode = args.mode
 
+    state = State()
+
     with Status(
         lambda: console.warning(f"Starting '{service.name}' in mode: '{mode}'"),
         lambda: console.success(f"{service.name} started"),
@@ -100,6 +102,8 @@ def up(args: Namespace) -> None:
         except subprocess.CalledProcessError:
             # Network already exists, ignore the error
             pass
+        # Add the service to the starting services table
+        state.update_service_entry(service.name, mode, StateTables.STARTING_SERVICES)
         try:
             mode_dependencies = modes[mode]
             _up(service, [mode], remote_dependencies, mode_dependencies, status)
@@ -108,7 +112,7 @@ def up(args: Namespace) -> None:
             status.failure(f"Failed to start {service.name}: {dce.stderr}")
             exit(1)
     # TODO: We should factor in healthchecks here before marking service as running
-    state = State()
+    state.remove_service_entry(service.name, StateTables.STARTING_SERVICES)
     state.update_service_entry(service.name, mode, StateTables.STARTED_SERVICES)
 
 

devservices/utils/dependencies.py

@@ -263,12 +263,14 @@ def get_non_shared_remote_dependencies(
     service_to_stop: Service, remote_dependencies: set[InstalledRemoteDependency]
 ) -> set[InstalledRemoteDependency]:
     state = State()
-    started_services = state.get_service_entries(StateTables.STARTED_SERVICES)
+    starting_services = set(state.get_service_entries(StateTables.STARTING_SERVICES))
+    started_services = set(state.get_service_entries(StateTables.STARTED_SERVICES))
+    active_services = starting_services.union(started_services)
     # We don't care about the remote dependencies of the service we are stopping
-    started_services.remove(service_to_stop.name)
+    active_services.remove(service_to_stop.name)
     other_running_remote_dependencies: set[InstalledRemoteDependency] = set()
     base_running_service_names: set[str] = set()
-    for started_service_name in started_services:
+    for started_service_name in active_services:
         started_service = find_matching_service(started_service_name)
         for dependency_name in service_to_stop.config.dependencies.keys():
             if dependency_name == started_service.config.service_name:

tests/commands/test_down.py

@@ -37,7 +37,96 @@
     ),
 )
 @mock.patch("devservices.utils.state.State.remove_service_entry")
-def test_down_simple(
+def test_down_starting(
+    mock_remove_service_entry: mock.Mock,
+    mock_run: mock.Mock,
+    tmp_path: Path,
+    capsys: pytest.CaptureFixture[str],
+) -> None:
+    with mock.patch(
+        "devservices.commands.down.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)
+
+        with mock.patch(
+            "devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")
+        ):
+            state = State()
+            state.update_service_entry(
+                "example-service", "default", StateTables.STARTING_SERVICES
+            )
+            down(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_run.assert_called_with(
+            [
+                "docker",
+                "compose",
+                "-p",
+                "example-service",
+                "-f",
+                f"{service_path}/{DEVSERVICES_DIR_NAME}/{CONFIG_FILE_NAME}",
+                "stop",
+                "clickhouse",
+                "redis",
+            ],
+            check=True,
+            capture_output=True,
+            text=True,
+            env=mock.ANY,
+        )
+
+        mock_remove_service_entry.assert_has_calls(
+            [
+                mock.call("example-service", StateTables.STARTING_SERVICES),
+                mock.call("example-service", StateTables.STARTED_SERVICES),
+            ]
+        )
+
+        captured = capsys.readouterr()
+        assert "Stopping clickhouse" in captured.out.strip()
+        assert "Stopping redis" 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.remove_service_entry")
+def test_down_started(
     mock_remove_service_entry: mock.Mock,
     mock_run: mock.Mock,
     tmp_path: Path,
@@ -105,8 +194,11 @@ def test_down_simple(
             env=mock.ANY,
         )
 
-        mock_remove_service_entry.assert_called_with(
-            "example-service", StateTables.STARTED_SERVICES
+        mock_remove_service_entry.assert_has_calls(
+            [
+                mock.call("example-service", StateTables.STARTING_SERVICES),
+                mock.call("example-service", StateTables.STARTED_SERVICES),
+            ]
         )
 
         captured = capsys.readouterr()
@@ -269,8 +361,11 @@ def test_down_mode_simple(
             env=mock.ANY,
         )
 
-        mock_remove_service_entry.assert_called_with(
-            "example-service", StateTables.STARTED_SERVICES
+        mock_remove_service_entry.assert_has_calls(
+            [
+                mock.call("example-service", StateTables.STARTING_SERVICES),
+                mock.call("example-service", StateTables.STARTED_SERVICES),
+            ]
         )
 
         captured = capsys.readouterr()
@@ -440,8 +535,11 @@ def test_down_overlapping_services(
             )
 
         # example-service should be stopped
-        mock_remove_service_entry.assert_called_with(
-            "example-service", StateTables.STARTED_SERVICES
+        mock_remove_service_entry.assert_has_calls(
+            [
+                mock.call("example-service", StateTables.STARTING_SERVICES),
+                mock.call("example-service", StateTables.STARTED_SERVICES),
+            ]
         )
 
 
@@ -603,8 +701,11 @@ def test_down_does_not_stop_service_being_used_by_another_service(
             mock_bring_down_dependency.assert_not_called()
 
         # example-service should be stopped
-        mock_remove_service_entry.assert_called_with(
-            "example-service", StateTables.STARTED_SERVICES
+        mock_remove_service_entry.assert_has_calls(
+            [
+                mock.call("example-service", StateTables.STARTING_SERVICES),
+                mock.call("example-service", StateTables.STARTED_SERVICES),
+            ]
         )
 
 
@@ -773,8 +874,11 @@ def test_down_does_not_stop_nested_service_being_used_by_another_service(
             mock_bring_down_dependency.assert_not_called()
 
         # child-service should be stopped
-        mock_remove_service_entry.assert_called_with(
-            "child-service", StateTables.STARTED_SERVICES
+        mock_remove_service_entry.assert_has_calls(
+            [
+                mock.call("child-service", StateTables.STARTING_SERVICES),
+                mock.call("child-service", StateTables.STARTED_SERVICES),
+            ]
         )
 
 
@@ -889,6 +993,9 @@ def test_down_overlapping_non_remote_services(
             )
 
         # example-service should be stopped
-        mock_remove_service_entry.assert_called_with(
-            "example-service", StateTables.STARTED_SERVICES
+        mock_remove_service_entry.assert_has_calls(
+            [
+                mock.call("example-service", StateTables.STARTING_SERVICES),
+                mock.call("example-service", StateTables.STARTED_SERVICES),
+            ]
         )