From 3f2e845885d5ca386a392e5989e9b623ce254d59 Mon Sep 17 00:00:00 2001
From: Ian Woodard <17186604+IanWoodard@users.noreply.github.com>
Date: Thu, 5 Dec 2024 08:22:31 -0800
Subject: [PATCH 1/8] fix(dependencies): Fixing dependency graph construction
 for simple modes

---
 devservices/commands/up.py        |  5 +++--
 devservices/utils/dependencies.py | 17 ++++++++++++++++-
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/devservices/commands/up.py b/devservices/commands/up.py
index af7237c..242728a 100644
--- a/devservices/commands/up.py
+++ b/devservices/commands/up.py
@@ -142,7 +142,7 @@ def up(args: Namespace) -> None:
             pass
         try:
             mode_dependencies = modes[mode]
-            _up(service, remote_dependencies, mode_dependencies, status)
+            _up(service, [mode], remote_dependencies, mode_dependencies, status)
         except DockerComposeError as dce:
             capture_exception(dce)
             status.failure(f"Failed to start {service.name}: {dce.stderr}")
@@ -163,6 +163,7 @@ def _bring_up_dependency(
 
 def _up(
     service: Service,
+    modes: list[str],
     remote_dependencies: set[InstalledRemoteDependency],
     mode_dependencies: list[str],
     status: Status,
@@ -180,7 +181,7 @@ def _up(
         DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY
     ] = relative_local_dependency_directory
     options = ["-d"]
-    dependency_graph = construct_dependency_graph(service)
+    dependency_graph = construct_dependency_graph(service, modes=modes)
     starting_order = dependency_graph.get_starting_order()
     sorted_remote_dependencies = sorted(
         remote_dependencies, key=lambda dep: starting_order.index(dep.service_name)
diff --git a/devservices/utils/dependencies.py b/devservices/utils/dependencies.py
index 86b8422..dd0249b 100644
--- a/devservices/utils/dependencies.py
+++ b/devservices/utils/dependencies.py
@@ -526,11 +526,26 @@ def get_remote_dependency_config(remote_config: RemoteConfig) -> ServiceConfig:
     return load_service_config_from_file(dependency_repo_dir)
 
 
-def construct_dependency_graph(service: Service) -> DependencyGraph:
+def construct_dependency_graph(
+    service: Service, modes: list[str] | None = None
+) -> DependencyGraph:
     dependency_graph = DependencyGraph()
 
+    if modes is None:
+        modes = ["default"]
+
+    service_mode_dependencies = set()
+    for mode in modes:
+        service_mode_dependencies.update(service.config.modes.get(mode, []))
+
     def _construct_dependency_graph(service_config: ServiceConfig) -> None:
         for dependency_name, dependency in service_config.dependencies.items():
+            # Skip the dependency if it's not in the modes (since it may not be installed and we don't care about it)
+            if (
+                service_config.service_name == service.name
+                and dependency_name not in service_mode_dependencies
+            ):
+                continue
             dependency_graph.add_edge(service_config.service_name, dependency_name)
             if _has_remote_config(dependency.remote):
                 dependency_config = get_remote_dependency_config(dependency.remote)

From 205b96a1668ebe4174c1f3c8de375c07c6085fa8 Mon Sep 17 00:00:00 2001
From: Ian Woodard <17186604+IanWoodard@users.noreply.github.com>
Date: Fri, 6 Dec 2024 08:23:57 -0800
Subject: [PATCH 2/8] ref(modes): Refactoring modes to support multiple
 concurrent modes

---
 devservices/commands/down.py          | 13 +++--
 devservices/commands/list_services.py |  4 +-
 devservices/commands/up.py            | 57 ++------------------
 devservices/utils/dependencies.py     | 17 ++++--
 devservices/utils/state.py            | 31 +++++++----
 tests/commands/test_down.py           |  6 +--
 tests/commands/test_list_services.py  |  8 +--
 tests/commands/test_purge.py          |  6 +--
 tests/commands/test_up.py             | 78 +++++++--------------------
 tests/utils/test_dependencies.py      | 16 +++---
 tests/utils/test_state.py             | 20 +++----
 11 files changed, 95 insertions(+), 161 deletions(-)

diff --git a/devservices/commands/down.py b/devservices/commands/down.py
index 12cbf58..1f78c0a 100644
--- a/devservices/commands/down.py
+++ b/devservices/commands/down.py
@@ -72,15 +72,20 @@ def down(args: Namespace) -> None:
         console.warning(f"{service.name} is not running")
         exit(0)
 
-    mode = state.get_mode_for_service(service.name) or "default"
-    mode_dependencies = modes[mode]
+    active_modes = state.get_active_modes_for_service(service.name)
+    mode_dependencies = set()
+    for active_mode in active_modes:
+        active_mode_dependencies = modes.get(active_mode, [])
+        mode_dependencies.update(active_mode_dependencies)
 
     with Status(
         lambda: console.warning(f"Stopping {service.name}"),
         lambda: console.success(f"{service.name} stopped"),
     ) as status:
         try:
-            remote_dependencies = install_and_verify_dependencies(service, mode=mode)
+            remote_dependencies = install_and_verify_dependencies(
+                service, modes=active_modes
+            )
         except DependencyError as de:
             capture_exception(de)
             status.failure(str(de))
@@ -89,7 +94,7 @@ def down(args: Namespace) -> None:
             service, remote_dependencies
         )
         try:
-            _down(service, remote_dependencies, mode_dependencies, status)
+            _down(service, remote_dependencies, list(mode_dependencies), status)
         except DockerComposeError as dce:
             capture_exception(dce)
             status.failure(f"Failed to stop {service.name}: {dce.stderr}")
diff --git a/devservices/commands/list_services.py b/devservices/commands/list_services.py
index 10f72da..c30f46f 100644
--- a/devservices/commands/list_services.py
+++ b/devservices/commands/list_services.py
@@ -47,9 +47,9 @@ def list_services(args: Namespace) -> None:
 
     for service in services_to_show:
         status = "running" if service.name in running_services else "stopped"
-        mode = state.get_mode_for_service(service.name)
+        active_modes = state.get_active_modes_for_service(service.name)
         console.info(f"- {service.name}")
-        console.info(f"  mode: {mode}")
+        console.info(f"  modes: {active_modes}")
         console.info(f"  status: {status}")
         console.info(f"  location: {service.repo_path}")
 
diff --git a/devservices/commands/up.py b/devservices/commands/up.py
index 242728a..a22e49a 100644
--- a/devservices/commands/up.py
+++ b/devservices/commands/up.py
@@ -1,6 +1,5 @@
 from __future__ import annotations
 
-import concurrent.futures
 import os
 import subprocess
 from argparse import _SubParsersAction
@@ -23,7 +22,6 @@
 from devservices.utils.console import Console
 from devservices.utils.console import Status
 from devservices.utils.dependencies import construct_dependency_graph
-from devservices.utils.dependencies import get_non_shared_remote_dependencies
 from devservices.utils.dependencies import install_and_verify_dependencies
 from devservices.utils.dependencies import InstalledRemoteDependency
 from devservices.utils.docker_compose import get_docker_compose_commands_to_run
@@ -69,64 +67,15 @@ def up(args: Namespace) -> None:
     modes = service.config.modes
     mode = args.mode
 
-    state = State()
-    started_services = state.get_started_services()
-    running_mode = state.get_mode_for_service(service.name) or "default"
-
-    # TODO: Remove this once we properly handle mode switching
-    if service.name in started_services and running_mode != mode:
-        console.warning(
-            f"Service '{service.name}' is already running in mode: '{running_mode}', restarting in mode: '{mode}'"
-        )
-        with Status() as status:
-            try:
-                remote_dependencies = install_and_verify_dependencies(
-                    service, mode=running_mode
-                )
-            except DependencyError as de:
-                capture_exception(de)
-                status.failure(str(de))
-                exit(1)
-            except ModeDoesNotExistError as mde:
-                capture_exception(mde)
-                status.failure(str(mde))
-                exit(1)
-            service_config_file_path = os.path.join(
-                service.repo_path, DEVSERVICES_DIR_NAME, CONFIG_FILE_NAME
-            )
-            current_env = os.environ.copy()
-            running_mode_dependencies = modes[running_mode]
-            remote_dependencies_to_bring_down = get_non_shared_remote_dependencies(
-                service, remote_dependencies
-            )
-            down_docker_compose_commands = get_docker_compose_commands_to_run(
-                service=service,
-                remote_dependencies=list(remote_dependencies_to_bring_down),
-                current_env=current_env,
-                command="down",
-                options=[],
-                service_config_file_path=service_config_file_path,
-                mode_dependencies=running_mode_dependencies,
-            )
-
-            with concurrent.futures.ThreadPoolExecutor() as executor:
-                futures = [
-                    executor.submit(run_cmd, cmd, current_env)
-                    for cmd in down_docker_compose_commands
-                ]
-                for future in concurrent.futures.as_completed(futures):
-                    future.result()
-
-            state.remove_started_service(service.name)
-
     with Status(
         lambda: console.warning(f"Starting '{service.name}' in mode: '{mode}'"),
         lambda: console.success(f"{service.name} started"),
     ) as status:
         try:
             status.info("Retrieving dependencies")
+            # TODO: should we install dependencies of all active modes?
             remote_dependencies = install_and_verify_dependencies(
-                service, force_update_dependencies=True, mode=mode
+                service, force_update_dependencies=True, modes=[mode]
             )
         except DependencyError as de:
             capture_exception(de)
@@ -149,7 +98,7 @@ def up(args: Namespace) -> None:
             exit(1)
     # TODO: We should factor in healthchecks here before marking service as running
     state = State()
-    state.add_started_service(service.name, mode)
+    state.update_started_service(service.name, mode)
 
 
 def _bring_up_dependency(
diff --git a/devservices/utils/dependencies.py b/devservices/utils/dependencies.py
index dd0249b..c956204 100644
--- a/devservices/utils/dependencies.py
+++ b/devservices/utils/dependencies.py
@@ -160,11 +160,20 @@ def _set_config(self, key: str, value: str) -> None:
 
 
 def install_and_verify_dependencies(
-    service: Service, force_update_dependencies: bool = False, mode: str = "default"
+    service: Service,
+    force_update_dependencies: bool = False,
+    modes: list[str] | None = None,
 ) -> set[InstalledRemoteDependency]:
-    if mode not in service.config.modes:
-        raise ModeDoesNotExistError(service_name=service.name, mode=mode)
-    mode_dependencies = set(service.config.modes[mode])
+    """
+    Install and verify dependencies for a service
+    """
+    if modes is None:
+        modes = ["default"]
+    mode_dependencies = set()
+    for mode in modes:
+        if mode not in service.config.modes:
+            raise ModeDoesNotExistError(service_name=service.name, mode=mode)
+        mode_dependencies.update(service.config.modes[mode])
     matching_dependencies = [
         dependency
         for dependency_key, dependency in list(service.config.dependencies.items())
diff --git a/devservices/utils/state.py b/devservices/utils/state.py
index 16670a1..325816f 100644
--- a/devservices/utils/state.py
+++ b/devservices/utils/state.py
@@ -35,17 +35,26 @@ def initialize_database(self) -> None:
         )
         self.conn.commit()
 
-    def add_started_service(self, service_name: str, mode: str) -> None:
+    def update_started_service(self, service_name: str, mode: str) -> None:
         cursor = self.conn.cursor()
         started_services = self.get_started_services()
-        if service_name in started_services:
+        active_modes = self.get_active_modes_for_service(service_name)
+        if service_name in started_services and mode in active_modes:
             return
-        cursor.execute(
-            """
-            INSERT INTO started_services (service_name, mode) VALUES (?, ?)
-        """,
-            (service_name, mode),
-        )
+        if service_name in started_services:
+            cursor.execute(
+                """
+                UPDATE started_services SET mode = ? WHERE service_name = ?
+            """,
+                (",".join(active_modes + [mode]), service_name),
+            )
+        else:
+            cursor.execute(
+                """
+                INSERT INTO started_services (service_name, mode) VALUES (?, ?)
+            """,
+                (service_name, ",".join(active_modes + [mode])),
+            )
         self.conn.commit()
 
     def remove_started_service(self, service_name: str) -> None:
@@ -67,7 +76,7 @@ def get_started_services(self) -> list[str]:
         )
         return [row[0] for row in cursor.fetchall()]
 
-    def get_mode_for_service(self, service_name: str) -> str | None:
+    def get_active_modes_for_service(self, service_name: str) -> list[str]:
         cursor = self.conn.cursor()
         cursor.execute(
             """
@@ -77,8 +86,8 @@ def get_mode_for_service(self, service_name: str) -> str | None:
         )
         result = cursor.fetchone()
         if result is None:
-            return None
-        return str(result[0])
+            return []
+        return str(result[0]).split(",")
 
     def clear_state(self) -> None:
         cursor = self.conn.cursor()
diff --git a/tests/commands/test_down.py b/tests/commands/test_down.py
index 5134482..12b5893 100644
--- a/tests/commands/test_down.py
+++ b/tests/commands/test_down.py
@@ -66,7 +66,7 @@ def test_down_simple(
             "devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")
         ):
             state = State()
-            state.add_started_service("example-service", "default")
+            state.update_started_service("example-service", "default")
             down(args)
 
         # Ensure the DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY is set and is relative
@@ -137,7 +137,7 @@ def test_down_error(
 
     with mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")):
         state = State()
-        state.add_started_service("example-service", "default")
+        state.update_started_service("example-service", "default")
         with pytest.raises(SystemExit):
             down(args)
 
@@ -202,7 +202,7 @@ def test_down_mode_simple(
             "devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")
         ):
             state = State()
-            state.add_started_service("example-service", "test")
+            state.update_started_service("example-service", "test")
             down(args)
 
         # Ensure the DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY is set and is relative
diff --git a/tests/commands/test_list_services.py b/tests/commands/test_list_services.py
index 97ce024..23446c7 100644
--- a/tests/commands/test_list_services.py
+++ b/tests/commands/test_list_services.py
@@ -19,7 +19,7 @@ def test_list_running_services(
         return_value=str(tmp_path / "code"),
     ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")):
         state = State()
-        state.add_started_service("example-service", "default")
+        state.update_started_service("example-service", "default")
         config = {
             "x-sentry-service-config": {
                 "version": 0.1,
@@ -47,7 +47,7 @@ def test_list_running_services(
 
         assert (
             captured.out
-            == f"Running services:\n- example-service\n  mode: default\n  status: running\n  location: {tmp_path / 'code' / 'example-service'}\n"
+            == f"Running services:\n- example-service\n  modes: ['default']\n  status: running\n  location: {tmp_path / 'code' / 'example-service'}\n"
         )
 
 
@@ -57,7 +57,7 @@ def test_list_all_services(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -
         return_value=str(tmp_path / "code"),
     ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")):
         state = State()
-        state.add_started_service("example-service", "default")
+        state.update_started_service("example-service", "default")
         config = {
             "x-sentry-service-config": {
                 "version": 0.1,
@@ -85,5 +85,5 @@ def test_list_all_services(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -
 
         assert (
             captured.out
-            == f"Services installed locally:\n- example-service\n  mode: default\n  status: running\n  location: {tmp_path / 'code' / 'example-service'}\n"
+            == f"Services installed locally:\n- example-service\n  modes: ['default']\n  status: running\n  location: {tmp_path / 'code' / 'example-service'}\n"
         )
diff --git a/tests/commands/test_purge.py b/tests/commands/test_purge.py
index 2a9c576..5613299 100644
--- a/tests/commands/test_purge.py
+++ b/tests/commands/test_purge.py
@@ -54,7 +54,7 @@ def test_purge_with_cache_and_state_and_no_running_containers_confirmed(
         cache_file.write_text("This is a test cache file.")
 
         state = State()
-        state.add_started_service("test-service", "test-mode")
+        state.update_started_service("test-service", "test-mode")
 
         assert cache_file.exists()
         assert state.get_started_services() == ["test-service"]
@@ -96,7 +96,7 @@ def test_purge_with_cache_and_state_and_running_containers_with_networks_confirm
         cache_file.write_text("This is a test cache file.")
 
         state = State()
-        state.add_started_service("test-service", "test-mode")
+        state.update_started_service("test-service", "test-mode")
 
         assert cache_file.exists()
         assert state.get_started_services() == ["test-service"]
@@ -155,7 +155,7 @@ def test_purge_with_cache_and_state_and_running_containers_not_confirmed(
         cache_file.write_text("This is a test cache file.")
 
         state = State()
-        state.add_started_service("test-service", "test-mode")
+        state.update_started_service("test-service", "test-mode")
 
         args = Namespace()
         purge(args)
diff --git a/tests/commands/test_up.py b/tests/commands/test_up.py
index 3e77887..abbf6f0 100644
--- a/tests/commands/test_up.py
+++ b/tests/commands/test_up.py
@@ -30,11 +30,11 @@
         stdout="clickhouse\nredis\n",
     ),
 )
-@mock.patch("devservices.utils.state.State.add_started_service")
+@mock.patch("devservices.utils.state.State.update_started_service")
 @mock.patch("devservices.commands.up._create_devservices_network")
 def test_up_simple(
     mock_create_devservices_network: mock.Mock,
-    mock_add_started_service: mock.Mock,
+    mock_update_started_service: mock.Mock,
     mock_run: mock.Mock,
     tmp_path: Path,
     capsys: pytest.CaptureFixture[str],
@@ -97,7 +97,7 @@ def test_up_simple(
             env=mock.ANY,
         )
 
-        mock_add_started_service.assert_called_with("example-service", "default")
+        mock_update_started_service.assert_called_with("example-service", "default")
         captured = capsys.readouterr()
         assert "Retrieving dependencies" in captured.out.strip()
         assert "Starting 'example-service' in mode: 'default'" in captured.out.strip()
@@ -106,11 +106,11 @@ def test_up_simple(
 
 
 @mock.patch("devservices.utils.docker_compose.subprocess.run")
-@mock.patch("devservices.utils.state.State.add_started_service")
+@mock.patch("devservices.utils.state.State.update_started_service")
 @mock.patch("devservices.commands.up._create_devservices_network")
 def test_up_dependency_error(
     mock_create_devservices_network: mock.Mock,
-    mock_add_started_service: mock.Mock,
+    mock_update_started_service: mock.Mock,
     mock_run: mock.Mock,
     capsys: pytest.CaptureFixture[str],
     tmp_path: Path,
@@ -153,7 +153,7 @@ def test_up_dependency_error(
 
         assert "DependencyError: example-repo (link) on branch" in captured.out.strip()
 
-        mock_add_started_service.assert_not_called()
+        mock_update_started_service.assert_not_called()
 
         captured = capsys.readouterr()
         assert "Retrieving dependencies" not in captured.out.strip()
@@ -165,11 +165,11 @@ def test_up_dependency_error(
 
 
 @mock.patch("devservices.utils.docker_compose.subprocess.run")
-@mock.patch("devservices.utils.state.State.add_started_service")
+@mock.patch("devservices.utils.state.State.update_started_service")
 @mock.patch("devservices.commands.up._create_devservices_network")
 def test_up_error(
     mock_create_devservices_network: mock.Mock,
-    mock_add_started_service: mock.Mock,
+    mock_update_started_service: mock.Mock,
     mock_run: mock.Mock,
     capsys: pytest.CaptureFixture[str],
     tmp_path: Path,
@@ -212,7 +212,7 @@ def test_up_error(
         "Failed to start example-service: Docker Compose error" in captured.out.strip()
     )
 
-    mock_add_started_service.assert_not_called()
+    mock_update_started_service.assert_not_called()
 
     captured = capsys.readouterr()
     assert "Retrieving dependencies" not in captured.out.strip()
@@ -229,11 +229,11 @@ def test_up_error(
         stdout="clickhouse\nredis\n",
     ),
 )
-@mock.patch("devservices.utils.state.State.add_started_service")
+@mock.patch("devservices.utils.state.State.update_started_service")
 @mock.patch("devservices.commands.up._create_devservices_network")
 def test_up_mode_simple(
     mock_create_devservices_network: mock.Mock,
-    mock_add_started_service: mock.Mock,
+    mock_update_started_service: mock.Mock,
     mock_run: mock.Mock,
     tmp_path: Path,
     capsys: pytest.CaptureFixture[str],
@@ -295,7 +295,7 @@ def test_up_mode_simple(
             env=mock.ANY,
         )
 
-        mock_add_started_service.assert_called_with("example-service", "test")
+        mock_update_started_service.assert_called_with("example-service", "test")
         captured = capsys.readouterr()
         assert "Retrieving dependencies" in captured.out.strip()
         assert "Starting 'example-service' in mode: 'test'" in captured.out.strip()
@@ -310,9 +310,9 @@ def test_up_mode_simple(
         stdout="clickhouse\nredis\n",
     ),
 )
-@mock.patch("devservices.utils.state.State.add_started_service")
+@mock.patch("devservices.utils.state.State.update_started_service")
 def test_up_mode_does_not_exist(
-    mock_add_started_service: mock.Mock,
+    mock_update_started_service: mock.Mock,
     mock_run: mock.Mock,
     tmp_path: Path,
     capsys: pytest.CaptureFixture[str],
@@ -356,7 +356,7 @@ def test_up_mode_does_not_exist(
             in captured.out.strip()
         )
 
-        mock_add_started_service.assert_not_called()
+        mock_update_started_service.assert_not_called()
 
         captured = capsys.readouterr()
         assert "Retrieving dependencies" not in captured.out.strip()
@@ -373,7 +373,7 @@ def test_up_mode_does_not_exist(
         stdout="clickhouse\nredis\n",
     ),
 )
-def test_up_switching_modes(
+def test_up_mutliple_modes(
     mock_run: mock.Mock,
     tmp_path: Path,
     capsys: pytest.CaptureFixture[str],
@@ -408,30 +408,13 @@ def test_up_switching_modes(
         os.chdir(service_path)
 
         state = State()
-        state.add_started_service("example-service", "default")
+        state.update_started_service("example-service", "default")
 
         args = Namespace(service_name=None, debug=False, mode="test")
         up(args)
 
         mock_run.assert_has_calls(
             [
-                mock.call(
-                    [
-                        "docker",
-                        "compose",
-                        "-p",
-                        "example-service",
-                        "-f",
-                        f"{service_path}/{DEVSERVICES_DIR_NAME}/{CONFIG_FILE_NAME}",
-                        "down",
-                        "clickhouse",
-                        "redis",
-                    ],
-                    check=True,
-                    capture_output=True,
-                    text=True,
-                    env=mock.ANY,
-                ),
                 mock.call(
                     [
                         "docker",
@@ -454,16 +437,12 @@ def test_up_switching_modes(
         )
 
         captured = capsys.readouterr()
-        assert (
-            "Service 'example-service' is already running in mode: 'default', restarting in mode: 'test'"
-            in captured.out.strip()
-        )
         assert "Starting 'example-service' in mode: 'test'" in captured.out.strip()
         assert "Retrieving dependencies" in captured.out.strip()
         assert "Starting redis" in captured.out.strip()
 
 
-def test_up_switching_modes_overlapping_running_service(
+def test_up_multiple_modes_overlapping_running_service(
     tmp_path: Path,
     capsys: pytest.CaptureFixture[str],
 ) -> None:
@@ -546,8 +525,8 @@ def test_up_switching_modes_overlapping_running_service(
         os.chdir(service_path)
 
         state = State()
-        state.add_started_service("example-service", "default")
-        state.add_started_service("other-service", "default")
+        state.update_started_service("example-service", "default")
+        state.update_started_service("other-service", "default")
 
         args = Namespace(service_name="example-service", debug=False, mode="test")
 
@@ -563,19 +542,6 @@ def test_up_switching_modes_overlapping_running_service(
 
             mock_run.assert_has_calls(
                 [
-                    mock.call(
-                        [
-                            "docker",
-                            "compose",
-                            "-p",
-                            "example-service",
-                            "-f",
-                            f"{service_path}/{DEVSERVICES_DIR_NAME}/{CONFIG_FILE_NAME}",
-                            "down",
-                            "clickhouse",
-                        ],
-                        mock.ANY,
-                    ),
                     mock.call(
                         [
                             "docker",
@@ -594,10 +560,6 @@ def test_up_switching_modes_overlapping_running_service(
             )
 
         captured = capsys.readouterr()
-        assert (
-            "Service 'example-service' is already running in mode: 'default', restarting in mode: 'test'"
-            in captured.out.strip()
-        )
         assert "Starting 'example-service' in mode: 'test'" in captured.out.strip()
         assert "Retrieving dependencies" in captured.out.strip()
         assert "Starting clickhouse" in captured.out.strip()
diff --git a/tests/utils/test_dependencies.py b/tests/utils/test_dependencies.py
index 4cc9e83..3773787 100644
--- a/tests/utils/test_dependencies.py
+++ b/tests/utils/test_dependencies.py
@@ -1270,8 +1270,8 @@ def test_get_non_shared_remote_dependencies_no_shared_dependencies(
 ) -> None:
     with mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")):
         state = State()
-        state.add_started_service("service-1", "default")
-        state.add_started_service("service-2", "default")
+        state.update_started_service("service-1", "default")
+        state.update_started_service("service-2", "default")
     service_to_stop = Service(
         name="service-1",
         repo_path="/path/to/service-1",
@@ -1345,8 +1345,8 @@ def test_get_non_shared_remote_dependencies_shared_dependencies(
 ) -> None:
     with mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")):
         state = State()
-        state.add_started_service("service-1", "default")
-        state.add_started_service("service-2", "default")
+        state.update_started_service("service-1", "default")
+        state.update_started_service("service-2", "default")
     service_to_stop = Service(
         name="service-1",
         repo_path="/path/to/service-1",
@@ -1413,8 +1413,8 @@ def test_get_non_shared_remote_dependencies_complex(
 ) -> None:
     with mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")):
         state = State()
-        state.add_started_service("service-1", "default")
-        state.add_started_service("service-2", "default")
+        state.update_started_service("service-1", "default")
+        state.update_started_service("service-2", "default")
     service_to_stop = Service(
         name="service-1",
         repo_path="/path/to/service-1",
@@ -1544,7 +1544,7 @@ def test_install_and_verify_dependencies_mode_simple(
             },
         ),
     )
-    install_and_verify_dependencies(service, mode="test")
+    install_and_verify_dependencies(service, modes=["test"])
 
     mock_install_dependencies.assert_called_once_with(
         [service.config.dependencies["dependency-1"]]
@@ -1580,7 +1580,7 @@ def test_install_and_verify_dependencies_mode_does_not_exist(tmp_path: Path) ->
         ),
     )
     with pytest.raises(ModeDoesNotExistError):
-        install_and_verify_dependencies(service, mode="unknown-mode")
+        install_and_verify_dependencies(service, modes=["unknown-mode"])
 
 
 def test_construct_dependency_graph_simple(
diff --git a/tests/utils/test_state.py b/tests/utils/test_state.py
index b116f5f..1a17bee 100644
--- a/tests/utils/test_state.py
+++ b/tests/utils/test_state.py
@@ -12,20 +12,20 @@ def test_state_simple(tmp_path: Path) -> None:
         assert state.get_started_services() == []
 
 
-def test_state_add_started_service(tmp_path: Path) -> None:
+def test_state_update_started_service(tmp_path: Path) -> None:
     with mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")):
         state = State()
-        state.add_started_service("example-service", "default")
+        state.update_started_service("example-service", "default")
         assert state.get_started_services() == ["example-service"]
-        assert state.get_mode_for_service("example-service") == "default"
+        assert state.get_active_modes_for_service("example-service") == ["default"]
 
 
 def test_state_remove_started_service(tmp_path: Path) -> None:
     with mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")):
         state = State()
-        state.add_started_service("example-service", "default")
+        state.update_started_service("example-service", "default")
         assert state.get_started_services() == ["example-service"]
-        assert state.get_mode_for_service("example-service") == "default"
+        assert state.get_active_modes_for_service("example-service") == ["default"]
         state.remove_started_service("example-service")
         assert state.get_started_services() == []
 
@@ -40,15 +40,15 @@ def test_state_remove_unknown_service(tmp_path: Path) -> None:
 def test_start_service_twice(tmp_path: Path) -> None:
     with mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")):
         state = State()
-        state.add_started_service("example-service", "default")
+        state.update_started_service("example-service", "default")
         assert state.get_started_services() == ["example-service"]
-        assert state.get_mode_for_service("example-service") == "default"
-        state.add_started_service("example-service", "default")
+        assert state.get_active_modes_for_service("example-service") == ["default"]
+        state.update_started_service("example-service", "default")
         assert state.get_started_services() == ["example-service"]
-        assert state.get_mode_for_service("example-service") == "default"
+        assert state.get_active_modes_for_service("example-service") == ["default"]
 
 
 def test_get_mode_for_nonexistent_service(tmp_path: Path) -> None:
     with mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")):
         state = State()
-        assert state.get_mode_for_service("unknown-service") is None
+        assert state.get_active_modes_for_service("unknown-service") == []

From 214b2b39ed8b489ba901ec8092c0552bd0386d57 Mon Sep 17 00:00:00 2001
From: Ian Woodard <17186604+IanWoodard@users.noreply.github.com>
Date: Fri, 6 Dec 2024 08:36:14 -0800
Subject: [PATCH 3/8] Improving logic to handle nested modes

---
 devservices/utils/dependencies.py | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/devservices/utils/dependencies.py b/devservices/utils/dependencies.py
index dd0249b..26ddaf8 100644
--- a/devservices/utils/dependencies.py
+++ b/devservices/utils/dependencies.py
@@ -531,14 +531,14 @@ def construct_dependency_graph(
 ) -> DependencyGraph:
     dependency_graph = DependencyGraph()
 
-    if modes is None:
-        modes = ["default"]
-
-    service_mode_dependencies = set()
-    for mode in modes:
-        service_mode_dependencies.update(service.config.modes.get(mode, []))
-
-    def _construct_dependency_graph(service_config: ServiceConfig) -> None:
+    def _construct_dependency_graph(
+        service_config: ServiceConfig, modes: list[str]
+    ) -> None:
+        if modes is None:
+            modes = ["default"]
+        service_mode_dependencies = set()
+        for mode in modes:
+            service_mode_dependencies.update(service_config.modes.get(mode, []))
         for dependency_name, dependency in service_config.dependencies.items():
             # Skip the dependency if it's not in the modes (since it may not be installed and we don't care about it)
             if (
@@ -549,7 +549,7 @@ def _construct_dependency_graph(service_config: ServiceConfig) -> None:
             dependency_graph.add_edge(service_config.service_name, dependency_name)
             if _has_remote_config(dependency.remote):
                 dependency_config = get_remote_dependency_config(dependency.remote)
-                _construct_dependency_graph(dependency_config)
+                _construct_dependency_graph(dependency_config, [dependency.remote.mode])
 
-    _construct_dependency_graph(service.config)
+    _construct_dependency_graph(service.config, modes or ["default"])
     return dependency_graph

From 02d2cc5921a473b6fda2fe91cb8dd29e5fe8beb9 Mon Sep 17 00:00:00 2001
From: Ian Woodard <17186604+IanWoodard@users.noreply.github.com>
Date: Fri, 6 Dec 2024 09:10:03 -0800
Subject: [PATCH 4/8] Removing todo

---
 devservices/commands/up.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/devservices/commands/up.py b/devservices/commands/up.py
index a22e49a..a04158c 100644
--- a/devservices/commands/up.py
+++ b/devservices/commands/up.py
@@ -73,7 +73,6 @@ def up(args: Namespace) -> None:
     ) as status:
         try:
             status.info("Retrieving dependencies")
-            # TODO: should we install dependencies of all active modes?
             remote_dependencies = install_and_verify_dependencies(
                 service, force_update_dependencies=True, modes=[mode]
             )

From cacf8a7c3f5ab59d884946cb9da93a814a643011 Mon Sep 17 00:00:00 2001
From: Ian Woodard <17186604+IanWoodard@users.noreply.github.com>
Date: Fri, 6 Dec 2024 09:49:32 -0800
Subject: [PATCH 5/8] Removing left over code

---
 devservices/utils/dependencies.py | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/devservices/utils/dependencies.py b/devservices/utils/dependencies.py
index 26ddaf8..9339e5a 100644
--- a/devservices/utils/dependencies.py
+++ b/devservices/utils/dependencies.py
@@ -541,10 +541,7 @@ def _construct_dependency_graph(
             service_mode_dependencies.update(service_config.modes.get(mode, []))
         for dependency_name, dependency in service_config.dependencies.items():
             # Skip the dependency if it's not in the modes (since it may not be installed and we don't care about it)
-            if (
-                service_config.service_name == service.name
-                and dependency_name not in service_mode_dependencies
-            ):
+            if dependency_name not in service_mode_dependencies:
                 continue
             dependency_graph.add_edge(service_config.service_name, dependency_name)
             if _has_remote_config(dependency.remote):

From c0d1ef71890851ae2f31cd7390a0dbed3e6c22e9 Mon Sep 17 00:00:00 2001
From: Ian Woodard <17186604+IanWoodard@users.noreply.github.com>
Date: Fri, 6 Dec 2024 09:51:01 -0800
Subject: [PATCH 6/8] Removing redundant code

---
 devservices/utils/dependencies.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/devservices/utils/dependencies.py b/devservices/utils/dependencies.py
index 9339e5a..97f89b5 100644
--- a/devservices/utils/dependencies.py
+++ b/devservices/utils/dependencies.py
@@ -534,8 +534,6 @@ def construct_dependency_graph(
     def _construct_dependency_graph(
         service_config: ServiceConfig, modes: list[str]
     ) -> None:
-        if modes is None:
-            modes = ["default"]
         service_mode_dependencies = set()
         for mode in modes:
             service_mode_dependencies.update(service_config.modes.get(mode, []))

From 921ab1857fe2ac9efc1faee02feab45ad32bb6e9 Mon Sep 17 00:00:00 2001
From: Ian Woodard <17186604+IanWoodard@users.noreply.github.com>
Date: Fri, 6 Dec 2024 10:10:58 -0800
Subject: [PATCH 7/8] Improving tests

---
 tests/utils/test_dependencies.py | 60 ++++++++++++++++++++++++++++++--
 1 file changed, 57 insertions(+), 3 deletions(-)

diff --git a/tests/utils/test_dependencies.py b/tests/utils/test_dependencies.py
index 4cc9e83..019dcef 100644
--- a/tests/utils/test_dependencies.py
+++ b/tests/utils/test_dependencies.py
@@ -1863,12 +1863,31 @@ def test_construct_dependency_graph_shared_dependency(
 def test_construct_dependency_graph_complex(
     tmp_path: Path,
 ) -> None:
+    other_service_repo_path = tmp_path / "other-service-repo"
     parent_service_repo_path = tmp_path / "parent-service-repo"
     child_service_repo_path = tmp_path / "child-service-repo"
     grandparent_service_repo_path = tmp_path / "grandparent-service-repo"
+    create_mock_git_repo("blank_repo", other_service_repo_path)
     create_mock_git_repo("blank_repo", parent_service_repo_path)
     create_mock_git_repo("blank_repo", child_service_repo_path)
     create_mock_git_repo("blank_repo", grandparent_service_repo_path)
+    other_service_repo_config = {
+        "x-sentry-service-config": {
+            "version": 0.1,
+            "service_name": "other-service",
+            "dependencies": {
+                "other-service": {
+                    "description": "other-service",
+                },
+            },
+            "modes": {"default": ["other-service"]},
+        },
+        "services": {
+            "other-service": {
+                "image": "other-service",
+            },
+        },
+    }
     parent_service_repo_config = {
         "x-sentry-service-config": {
             "version": 0.1,
@@ -1885,8 +1904,19 @@ def test_construct_dependency_graph_complex(
                 "parent-service": {
                     "description": "parent-service",
                 },
+                "other-service": {
+                    "description": "other-service",
+                    "remote": {
+                        "repo_name": "other-service",
+                        "repo_link": f"file://{other_service_repo_path}",
+                        "branch": "main",
+                    },
+                },
+            },
+            "modes": {
+                "default": ["child-service", "parent-service"],
+                "other": ["other-service"],
             },
-            "modes": {"default": ["child-service", "parent-service"]},
         },
         "services": {
             "parent-service": {
@@ -1902,8 +1932,16 @@ def test_construct_dependency_graph_complex(
                 "child-service": {
                     "description": "child-service",
                 },
+                "other-service": {
+                    "description": "other-service",
+                    "remote": {
+                        "repo_name": "other-service",
+                        "repo_link": f"file://{other_service_repo_path}",
+                        "branch": "main",
+                    },
+                },
             },
-            "modes": {"default": ["child-service"]},
+            "modes": {"default": ["child-service"], "other": ["other-service"]},
         },
         "services": {
             "child-service": {
@@ -1924,11 +1962,22 @@ def test_construct_dependency_graph_complex(
                         "branch": "main",
                     },
                 },
+                "other-service": {
+                    "description": "other-service",
+                    "remote": {
+                        "repo_name": "other-service",
+                        "repo_link": f"file://{other_service_repo_path}",
+                        "branch": "main",
+                    },
+                },
                 "grandparent-service": {
                     "description": "grandparent-service",
                 },
             },
-            "modes": {"default": ["parent-service", "grandparent-service"]},
+            "modes": {
+                "default": ["parent-service", "grandparent-service"],
+                "other": ["other-service"],
+            },
         },
         "services": {
             "grandparent-service": {
@@ -1936,9 +1985,14 @@ def test_construct_dependency_graph_complex(
             },
         },
     }
+    create_config_file(other_service_repo_path, other_service_repo_config)
     create_config_file(parent_service_repo_path, parent_service_repo_config)
     create_config_file(child_service_repo_path, child_service_repo_config)
     create_config_file(grandparent_service_repo_path, grandparent_service_repo_config)
+    run_git_command(["add", "."], cwd=other_service_repo_path)
+    run_git_command(
+        ["commit", "-m", "Add devservices config"], cwd=other_service_repo_path
+    )
     run_git_command(["add", "."], cwd=parent_service_repo_path)
     run_git_command(
         ["commit", "-m", "Add devservices config"], cwd=parent_service_repo_path

From a0b02405a0ec2ae8033b01eb1295f70be8870afa Mon Sep 17 00:00:00 2001
From: Ian Woodard <17186604+IanWoodard@users.noreply.github.com>
Date: Fri, 6 Dec 2024 10:19:05 -0800
Subject: [PATCH 8/8] Removing optional from arg

---
 devservices/utils/dependencies.py | 6 ++----
 tests/utils/test_dependencies.py  | 8 ++++----
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/devservices/utils/dependencies.py b/devservices/utils/dependencies.py
index 97f89b5..5279032 100644
--- a/devservices/utils/dependencies.py
+++ b/devservices/utils/dependencies.py
@@ -526,9 +526,7 @@ def get_remote_dependency_config(remote_config: RemoteConfig) -> ServiceConfig:
     return load_service_config_from_file(dependency_repo_dir)
 
 
-def construct_dependency_graph(
-    service: Service, modes: list[str] | None = None
-) -> DependencyGraph:
+def construct_dependency_graph(service: Service, modes: list[str]) -> DependencyGraph:
     dependency_graph = DependencyGraph()
 
     def _construct_dependency_graph(
@@ -546,5 +544,5 @@ def _construct_dependency_graph(
                 dependency_config = get_remote_dependency_config(dependency.remote)
                 _construct_dependency_graph(dependency_config, [dependency.remote.mode])
 
-    _construct_dependency_graph(service.config, modes or ["default"])
+    _construct_dependency_graph(service.config, modes)
     return dependency_graph
diff --git a/tests/utils/test_dependencies.py b/tests/utils/test_dependencies.py
index 019dcef..ba66b8c 100644
--- a/tests/utils/test_dependencies.py
+++ b/tests/utils/test_dependencies.py
@@ -1637,7 +1637,7 @@ def test_construct_dependency_graph_simple(
         str(tmp_path / "dependency-dir"),
     ):
         install_and_verify_dependencies(service)
-        dependency_graph = construct_dependency_graph(service)
+        dependency_graph = construct_dependency_graph(service, ["default"])
         assert dependency_graph.graph == {
             "dependency-1": set(),
             "test-service": {"dependency-1"},
@@ -1735,7 +1735,7 @@ def test_construct_dependency_graph_one_nested_dependency(
         str(tmp_path / "dependency-dir"),
     ):
         install_and_verify_dependencies(service)
-        dependency_graph = construct_dependency_graph(service)
+        dependency_graph = construct_dependency_graph(service, ["default"])
         assert dependency_graph.graph == {
             "child-service": set(),
             "parent-service": {"child-service"},
@@ -1846,7 +1846,7 @@ def test_construct_dependency_graph_shared_dependency(
         str(tmp_path / "dependency-dir"),
     ):
         install_and_verify_dependencies(service)
-        dependency_graph = construct_dependency_graph(service)
+        dependency_graph = construct_dependency_graph(service, ["default"])
         assert dependency_graph.graph == {
             "child-service": set(),
             "parent-service": {"child-service"},
@@ -2043,7 +2043,7 @@ def test_construct_dependency_graph_complex(
         str(tmp_path / "dependency-dir"),
     ):
         install_and_verify_dependencies(service)
-        dependency_graph = construct_dependency_graph(service)
+        dependency_graph = construct_dependency_graph(service, ["default"])
         assert dependency_graph.graph == {
             "child-service": set(),
             "parent-service": {"child-service"},