Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(up): Restart services when switching modes #160

Merged
merged 9 commits into from
Nov 21, 2024
Merged

fix(up): Restart services when switching modes #160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ee724a3
fix(up): Restart services when switching modes
IanWoodard Nov 20, 2024
abeb1cf
Remove comment
IanWoodard Nov 20, 2024
8608d3e
Merge branch 'main' into iw/restart-mode
IanWoodard Nov 20, 2024
f378fd6
Adding important test case
IanWoodard Nov 21, 2024
ebb5465
Cleaning up test
IanWoodard Nov 21, 2024
443fcd8
Removing unused test
IanWoodard Nov 21, 2024
43747cb
Merge branch 'main' into iw/restart-mode
IanWoodard Nov 21, 2024
1d9c948
Merge branch 'main' into iw/restart-mode
IanWoodard Nov 21, 2024
8f6f1b1
Addressing review feedback
IanWoodard Nov 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 53 additions & 1 deletion devservices/commands/up.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import concurrent.futures
import os
import subprocess
from argparse import _SubParsersAction
Expand All @@ -21,6 +22,7 @@
from devservices.exceptions import ServiceNotFoundError
from devservices.utils.console import Console
from devservices.utils.console import Status
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
Expand Down Expand Up @@ -63,8 +65,58 @@
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:
IanWoodard marked this conversation as resolved.
Show resolved Hide resolved
capture_exception(mde)
status.failure(str(mde))
exit(1)

Check warning on line 89 in devservices/commands/up.py

View check run for this annotation

Codecov / codecov/patch

devservices/commands/up.py#L82-L89 

Added lines #L82 - L89 were not covered by tests
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=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}"),
lambda: console.warning(f"Starting '{service.name}' in mode: '{mode}'"),
lambda: console.success(f"{service.name} started"),
) as status:
try:
Expand Down
248 changes: 248 additions & 0 deletions tests/commands/test_up.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY
from devservices.constants import DEVSERVICES_DIR_NAME
from devservices.exceptions import DependencyError
from devservices.utils.state import State
from testing.utils import create_config_file
from testing.utils import create_mock_git_repo
from testing.utils import run_git_command


@mock.patch(
Expand Down Expand Up @@ -95,6 +98,7 @@ def test_up_simple(
mock_add_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()
assert "Starting clickhouse" in captured.out.strip()
assert "Starting redis" in captured.out.strip()

Expand Down Expand Up @@ -151,6 +155,9 @@ def test_up_dependency_error(

captured = capsys.readouterr()
assert "Retrieving dependencies" not in captured.out.strip()
assert (
"Starting 'example-service' in mode: 'default'" not in captured.out.strip()
)
assert "Starting clickhouse" not in captured.out.strip()
assert "Starting redis" not in captured.out.strip()

Expand Down Expand Up @@ -207,6 +214,7 @@ def test_up_error(

captured = capsys.readouterr()
assert "Retrieving dependencies" not in captured.out.strip()
assert "Starting 'example-service' in mode: 'default'" not in captured.out.strip()
assert "Starting clickhouse" not in captured.out.strip()
assert "Starting redis" not in captured.out.strip()

Expand Down Expand Up @@ -288,6 +296,7 @@ def test_up_mode_simple(
mock_add_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()
assert "Starting redis" in captured.out.strip()


Expand Down Expand Up @@ -349,5 +358,244 @@ def test_up_mode_does_not_exist(

captured = capsys.readouterr()
assert "Retrieving dependencies" not in captured.out.strip()
assert "Starting 'example-service' in mode: 'test'" not in captured.out.strip()
assert "Starting clickhouse" not in captured.out.strip()
assert "Starting redis" not 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",
),
)
def test_up_switching_modes(
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"),
),
mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")),
):
config = {
"x-sentry-service-config": {
"version": 0.1,
"service_name": "example-service",
"dependencies": {
"redis": {"description": "Redis"},
"clickhouse": {"description": "Clickhouse"},
},
"modes": {"default": ["redis", "clickhouse"], "test": ["redis"]},
},
"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)

state = State()
state.add_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",
"compose",
"-p",
"example-service",
"-f",
f"{service_path}/{DEVSERVICES_DIR_NAME}/{CONFIG_FILE_NAME}",
"up",
"redis",
"-d",
],
check=True,
capture_output=True,
text=True,
env=mock.ANY,
),
],
any_order=True,
)

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(
tmp_path: Path,
capsys: pytest.CaptureFixture[str],
) -> None:
with (
mock.patch(
"devservices.commands.up.DEVSERVICES_DEPENDENCIES_CACHE_DIR",
str(tmp_path / "dependency-dir"),
),
mock.patch(
"devservices.utils.dependencies.DEVSERVICES_DEPENDENCIES_CACHE_DIR",
str(tmp_path / "dependency-dir"),
),
mock.patch(
"devservices.utils.services.get_coderoot",
return_value=str(tmp_path / "code"),
),
mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")),
):
redis_repo_path = tmp_path / "redis"
create_mock_git_repo("blank_repo", redis_repo_path)
mock_git_repo_config = {
"x-sentry-service-config": {
"version": 0.1,
"service_name": "shared-redis",
"dependencies": {},
"modes": {"default": []},
},
"services": {
"redis": {"image": "redis:6.2.14-alpine"},
},
}
create_config_file(redis_repo_path, mock_git_repo_config)
run_git_command(["add", "."], cwd=redis_repo_path)
run_git_command(["commit", "-m", "Add devservices config"], cwd=redis_repo_path)
config = {
"x-sentry-service-config": {
"version": 0.1,
"service_name": "example-service",
"dependencies": {
"redis": {
"description": "Redis",
"remote": {
"repo_name": "redis",
"branch": "main",
"repo_link": f"file://{redis_repo_path}",
},
},
"clickhouse": {"description": "Clickhouse"},
},
"modes": {"default": ["redis", "clickhouse"], "test": ["clickhouse"]},
},
"services": {
"clickhouse": {
"image": "altinity/clickhouse-server:23.8.11.29.altinitystable"
},
},
}
other_config = {
"x-sentry-service-config": {
"version": 0.1,
"service_name": "other-service",
"dependencies": {
"redis": {
"description": "Redis",
"remote": {
"repo_name": "redis",
"branch": "main",
"repo_link": f"file://{redis_repo_path}",
},
},
},
"modes": {"default": ["redis"]},
},
}

service_path = tmp_path / "code" / "example-service"
other_service_path = tmp_path / "code" / "other-service"
create_config_file(service_path, config)
create_config_file(other_service_path, other_config)
os.chdir(service_path)

state = State()
state.add_started_service("example-service", "default")
state.add_started_service("other-service", "default")

args = Namespace(service_name="example-service", debug=False, mode="test")

with mock.patch(
"devservices.commands.up.run_cmd",
return_value=subprocess.CompletedProcess(
args=["docker", "compose", "config", "--services"],
returncode=0,
stdout="clickhouse\n",
),
) as mock_run:
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",
],
mock.ANY,
),
mock.call(
[
"docker",
"compose",
"-p",
"example-service",
"-f",
f"{service_path}/{DEVSERVICES_DIR_NAME}/{CONFIG_FILE_NAME}",
"up",
"clickhouse",
"-d",
],
mock.ANY,
),
],
)

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()