Skip to content

Commit a0839d4

Browse files
committed
Render fleet spec diff in CLI
1 parent ea85c59 commit a0839d4

3 files changed

Lines changed: 271 additions & 24 deletions

File tree

src/dstack/_internal/cli/services/configurators/fleet.py

Lines changed: 62 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,10 @@
3434
InstanceGroupPlacement,
3535
)
3636
from dstack._internal.core.models.instances import InstanceStatus, SSHKey
37-
from dstack._internal.core.services.diff import diff_models
37+
from dstack._internal.core.services.diff import copy_model, diff_models
3838
from dstack._internal.utils.common import local_time
3939
from dstack._internal.utils.logging import get_logger
40+
from dstack._internal.utils.nested_list import NestedList, NestedListItem
4041
from dstack._internal.utils.ssh import convert_ssh_key_to_pem, generate_public_key, pkey_from_str
4142
from dstack.api.utils import load_profile
4243

@@ -85,14 +86,10 @@ def _apply_plan(self, plan: FleetPlan, command_args: argparse.Namespace):
8586
)
8687
confirm_message += "Create the fleet?"
8788
else:
89+
effective_spec = plan.get_effective_spec()
90+
diff = _render_fleet_spec_diff(plan.current_resource.spec, effective_spec)
8891
action_message += f"Found fleet [code]{plan.spec.configuration.name}[/]."
89-
if plan.action == ApplyAction.CREATE:
90-
delete_fleet_name = plan.current_resource.name
91-
action_message += (
92-
" Configuration changes detected. Cannot update the fleet in-place"
93-
)
94-
confirm_message += "Re-create the fleet?"
95-
elif plan.current_resource.spec == plan.effective_spec:
92+
if plan.current_resource.spec == effective_spec:
9693
if command_args.yes and not command_args.force:
9794
# --force is required only with --yes,
9895
# otherwise we may ask for force apply interactively.
@@ -103,8 +100,26 @@ def _apply_plan(self, plan: FleetPlan, command_args: argparse.Namespace):
103100
delete_fleet_name = plan.current_resource.name
104101
action_message += " No configuration changes detected."
105102
confirm_message += "Re-create the fleet?"
103+
elif plan.action == ApplyAction.CREATE:
104+
delete_fleet_name = plan.current_resource.name
105+
if diff is not None:
106+
# TODO: Highlight only the fields that block in-place update instead of
107+
# showing the full detected diff here.
108+
action_message += (
109+
f" Detected changes that [error]cannot[/] be updated in-place:\n{diff}"
110+
)
111+
else:
112+
action_message += (
113+
" Configuration changes detected. Cannot update the fleet in-place."
114+
)
115+
confirm_message += "Re-create the fleet?"
106116
else:
107-
action_message += " Configuration changes detected."
117+
if diff is not None:
118+
action_message += (
119+
f" Detected changes that [code]can[/] be updated in-place:\n{diff}"
120+
)
121+
else:
122+
action_message += " Configuration changes detected."
108123
confirm_message += "Update the fleet in-place?"
109124

110125
console.print(action_message)
@@ -357,6 +372,44 @@ def _resolve_ssh_key(ssh_key_path: Optional[str]) -> Optional[SSHKey]:
357372
exit()
358373

359374

375+
def _render_fleet_spec_diff(old_spec: FleetSpec, new_spec: FleetSpec) -> Optional[str]:
376+
old_spec = copy_model(old_spec)
377+
new_spec = copy_model(new_spec)
378+
changed_spec_fields = list(diff_models(old_spec, new_spec))
379+
if not changed_spec_fields:
380+
return None
381+
382+
nested_list = NestedList()
383+
for spec_field in changed_spec_fields:
384+
if spec_field == "merged_profile":
385+
continue
386+
if spec_field == "configuration":
387+
item = NestedListItem(
388+
"Configuration properties:",
389+
children=[
390+
NestedListItem(field)
391+
for field in diff_models(old_spec.configuration, new_spec.configuration)
392+
],
393+
)
394+
elif spec_field == "profile":
395+
item = NestedListItem(
396+
"Profile properties:",
397+
children=[
398+
NestedListItem(field)
399+
for field in diff_models(old_spec.profile, new_spec.profile)
400+
],
401+
)
402+
elif spec_field == "configuration_path":
403+
item = NestedListItem("Configuration path")
404+
else:
405+
item = NestedListItem(spec_field.replace("_", " ").capitalize())
406+
nested_list.children.append(item)
407+
408+
if not nested_list.children:
409+
return None
410+
return nested_list.render()
411+
412+
360413
def _print_plan_header(plan: FleetPlan):
361414
def th(s: str) -> str:
362415
return f"[bold]{s}[/bold]"

src/dstack/_internal/cli/services/configurators/run.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,8 @@ def apply_configuration(
152152
confirm_message = "Stop and override the run?"
153153
elif not run_plan.current_resource.status.is_finished():
154154
stop_run_name = run_plan.current_resource.run_spec.run_name
155+
# TODO: Highlight only the fields that block in-place update instead of
156+
# showing the full detected diff here.
155157
console.print(
156158
f"Active run [code]{conf.name}[/] already exists."
157159
f" Detected changes that [error]cannot[/] be updated in-place:\n{diff}"

src/tests/_internal/cli/services/configurators/test_fleet.py

Lines changed: 207 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,129 @@
11
import argparse
2-
from typing import List, Tuple
2+
from datetime import datetime, timezone
3+
from textwrap import dedent
4+
from typing import List, Optional, Tuple
35
from unittest.mock import Mock
6+
from uuid import uuid4
47

58
import pytest
9+
from rich.console import Console
610

7-
from dstack._internal.cli.services.configurators.fleet import FleetConfigurator
11+
import dstack._internal.cli.services.configurators.fleet as fleet_configurator_module
12+
from dstack._internal.cli.services.configurators.fleet import (
13+
FleetConfigurator,
14+
_render_fleet_spec_diff,
15+
)
816
from dstack._internal.core.errors import ConfigurationError
17+
from dstack._internal.core.models.common import ApplyAction
918
from dstack._internal.core.models.envs import Env
10-
from dstack._internal.core.models.fleets import FleetConfiguration
19+
from dstack._internal.core.models.fleets import (
20+
Fleet,
21+
FleetConfiguration,
22+
FleetNodesSpec,
23+
FleetPlan,
24+
FleetSpec,
25+
FleetStatus,
26+
InstanceGroupPlacement,
27+
)
28+
from dstack._internal.core.models.profiles import Profile
29+
30+
31+
def create_conf() -> FleetConfiguration:
32+
return FleetConfiguration.parse_obj({"ssh_config": {"hosts": ["1.2.3.4"]}})
33+
34+
35+
def apply_args(
36+
conf: FleetConfiguration, args: List[str]
37+
) -> Tuple[FleetConfiguration, argparse.Namespace]:
38+
parser = argparse.ArgumentParser()
39+
configurator = FleetConfigurator(Mock())
40+
configurator.register_args(parser)
41+
conf = conf.copy(deep=True)
42+
configurator_args = parser.parse_args(args)
43+
configurator.apply_args(conf, configurator_args)
44+
return conf, configurator_args
45+
46+
47+
def get_cloud_fleet_spec(
48+
*,
49+
name: str = "test-fleet",
50+
nodes: Optional[FleetNodesSpec] = None,
51+
placement: Optional[InstanceGroupPlacement] = None,
52+
) -> FleetSpec:
53+
if nodes is None:
54+
nodes = FleetNodesSpec(min=0, target=0, max=1)
55+
return FleetSpec(
56+
configuration=FleetConfiguration(
57+
name=name,
58+
nodes=nodes,
59+
placement=placement,
60+
),
61+
configuration_path="fleet.dstack.yml",
62+
profile=Profile(),
63+
)
64+
65+
66+
def get_ssh_fleet_spec(
67+
*,
68+
name: str = "test-fleet",
69+
hosts: Optional[list[str]] = None,
70+
) -> FleetSpec:
71+
if hosts is None:
72+
hosts = ["10.0.0.100"]
73+
return FleetSpec(
74+
configuration=FleetConfiguration.parse_obj(
75+
{
76+
"name": name,
77+
"ssh_config": {"hosts": hosts},
78+
}
79+
),
80+
configuration_path="fleet.dstack.yml",
81+
profile=Profile(),
82+
)
83+
84+
85+
def create_fleet_plan(
86+
*,
87+
current_spec: FleetSpec,
88+
spec: FleetSpec,
89+
action: ApplyAction,
90+
) -> FleetPlan:
91+
return FleetPlan(
92+
project_name="test-project",
93+
user="test-user",
94+
spec=spec,
95+
effective_spec=spec,
96+
current_resource=Fleet(
97+
id=uuid4(),
98+
name=current_spec.configuration.name or "test-fleet",
99+
project_name="test-project",
100+
spec=current_spec,
101+
created_at=datetime.now(timezone.utc),
102+
status=FleetStatus.ACTIVE,
103+
instances=[],
104+
),
105+
offers=[],
106+
total_offers=0,
107+
action=action,
108+
)
109+
110+
111+
def get_command_args() -> argparse.Namespace:
112+
return argparse.Namespace(
113+
yes=False,
114+
force=False,
115+
detach=False,
116+
)
117+
118+
119+
def patch_console_and_confirm(
120+
monkeypatch: pytest.MonkeyPatch,
121+
) -> tuple[Console, Mock]:
122+
console = Console(record=True, force_terminal=False, color_system=None, width=120)
123+
confirm_ask = Mock(return_value=False)
124+
monkeypatch.setattr(fleet_configurator_module, "console", console)
125+
monkeypatch.setattr(fleet_configurator_module, "confirm_ask", confirm_ask)
126+
return console, confirm_ask
11127

12128

13129
class TestFleetConfigurator:
@@ -39,17 +155,93 @@ def test_env_value_from_environ_not_set(self, monkeypatch: pytest.MonkeyPatch):
39155
apply_args(conf, ["--env", "FROM_ENV"])
40156

41157

42-
def create_conf() -> FleetConfiguration:
43-
return FleetConfiguration.parse_obj({"ssh_config": {"hosts": ["1.2.3.4"]}})
158+
class TestApplyPlanMessages:
159+
def test_prints_in_place_update_diff(self, monkeypatch: pytest.MonkeyPatch):
160+
console, confirm_ask = patch_console_and_confirm(monkeypatch)
161+
current_spec = get_cloud_fleet_spec(nodes=FleetNodesSpec(min=0, target=0, max=1))
162+
spec = get_cloud_fleet_spec(nodes=FleetNodesSpec(min=1, target=1, max=1))
163+
plan = create_fleet_plan(
164+
current_spec=current_spec,
165+
spec=spec,
166+
action=ApplyAction.UPDATE,
167+
)
44168

169+
FleetConfigurator(Mock())._apply_plan(plan, get_command_args())
45170

46-
def apply_args(
47-
conf: FleetConfiguration, args: List[str]
48-
) -> Tuple[FleetConfiguration, argparse.Namespace]:
49-
parser = argparse.ArgumentParser()
50-
configurator = FleetConfigurator(Mock())
51-
configurator.register_args(parser)
52-
conf = conf.copy(deep=True)
53-
configurator_args = parser.parse_args(args)
54-
configurator.apply_args(conf, configurator_args)
55-
return conf, configurator_args
171+
output = console.export_text()
172+
assert "Found fleet test-fleet." in output
173+
assert "Detected changes that can be updated in-place:" in output
174+
assert "- Configuration properties:" in output
175+
assert " - nodes" in output
176+
confirm_ask.assert_called_once_with("Update the fleet in-place?")
177+
178+
def test_prints_recreate_diff(self, monkeypatch: pytest.MonkeyPatch):
179+
console, confirm_ask = patch_console_and_confirm(monkeypatch)
180+
current_spec = get_cloud_fleet_spec(placement=InstanceGroupPlacement.ANY)
181+
spec = get_cloud_fleet_spec(placement=InstanceGroupPlacement.CLUSTER)
182+
plan = create_fleet_plan(
183+
current_spec=current_spec,
184+
spec=spec,
185+
action=ApplyAction.CREATE,
186+
)
187+
188+
FleetConfigurator(Mock())._apply_plan(plan, get_command_args())
189+
190+
output = console.export_text()
191+
assert "Found fleet test-fleet." in output
192+
assert "Detected changes that cannot be updated in-place:" in output
193+
assert "- Configuration properties:" in output
194+
assert " - placement" in output
195+
confirm_ask.assert_called_once_with("Re-create the fleet?")
196+
197+
def test_prints_no_diff_message(self, monkeypatch: pytest.MonkeyPatch):
198+
console, confirm_ask = patch_console_and_confirm(monkeypatch)
199+
spec = get_cloud_fleet_spec()
200+
plan = create_fleet_plan(
201+
current_spec=spec,
202+
spec=spec.copy(deep=True),
203+
action=ApplyAction.UPDATE,
204+
)
205+
206+
FleetConfigurator(Mock())._apply_plan(plan, get_command_args())
207+
208+
output = console.export_text()
209+
assert "Found fleet test-fleet." in output
210+
assert "No configuration changes detected." in output
211+
assert "Detected changes that" not in output
212+
confirm_ask.assert_called_once_with("Re-create the fleet?")
213+
214+
215+
class TestRenderFleetSpecDiff:
216+
def test_renders_cloud_nodes_change(self):
217+
old = get_cloud_fleet_spec(nodes=FleetNodesSpec(min=0, target=0, max=1))
218+
new = get_cloud_fleet_spec(nodes=FleetNodesSpec(min=1, target=1, max=1))
219+
220+
assert (
221+
_render_fleet_spec_diff(old, new)
222+
== dedent(
223+
"""
224+
- Configuration properties:
225+
- nodes
226+
"""
227+
).lstrip()
228+
)
229+
230+
def test_renders_ssh_hosts_change(self):
231+
old = get_ssh_fleet_spec(hosts=["10.0.0.100"])
232+
new = get_ssh_fleet_spec(hosts=["10.0.0.100", "10.0.0.101"])
233+
234+
assert (
235+
_render_fleet_spec_diff(old, new)
236+
== dedent(
237+
"""
238+
- Configuration properties:
239+
- ssh_config
240+
"""
241+
).lstrip()
242+
)
243+
244+
def test_no_diff(self):
245+
spec = get_cloud_fleet_spec()
246+
247+
assert _render_fleet_spec_diff(spec, spec.copy(deep=True)) is None

0 commit comments

Comments
 (0)