Skip to content

Commit f97968e

Browse files
committed
Allow detecting whether service https is unset
Make the `https` service configuration property `Optional`. This allows to determine whether the property was omitted or explicitly set by the user. In a future version, we could use that to improve validation or change the default on the server. For now, the behavior is unchanged - an unset `https` is equivalent to `https: true`. Backwards compatibility is preserved for the most part, except two side effects: - Users may see a phantom `https` change in the run plan when redeploying a service after upgrading to CLI 0.20.12+. This, however, will not block the rolling deployment and will not cause any actual changes to the service behavior. - Users with a pre-0.20.12 CLI will not be able to trigger a rolling deployment on a service deployed with a 0.20.12+ CLI and will see the `Failed to apply plan. Resource has been changed` error message.
1 parent 768bdc1 commit f97968e

6 files changed

Lines changed: 39 additions & 12 deletions

File tree

src/dstack/_internal/core/compatibility/runs.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ def get_run_spec_excludes(run_spec: RunSpec) -> IncludeExcludeDictType:
7878
configuration_excludes["router"] = True
7979
elif isinstance(router, SGLangServiceRouterConfig) and router.pd_disaggregation is False:
8080
configuration_excludes["router"] = {"pd_disaggregation": True}
81+
if run_spec.configuration.https is None:
82+
configuration_excludes["https"] = True
8183

8284
if configuration_excludes:
8385
spec_excludes["configuration"] = configuration_excludes

src/dstack/_internal/core/models/configurations.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -857,12 +857,13 @@ class ServiceConfigurationParams(CoreModel):
857857
),
858858
] = None
859859
https: Annotated[
860-
Union[bool, Literal["auto"]],
860+
Optional[Union[bool, Literal["auto"]]],
861861
Field(
862862
description="Enable HTTPS if running with a gateway."
863-
" Set to `auto` to determine automatically based on gateway configuration"
863+
" Set to `auto` to determine automatically based on gateway configuration."
864+
f" Defaults to `{str(SERVICE_HTTPS_DEFAULT).lower()}`"
864865
),
865-
] = SERVICE_HTTPS_DEFAULT
866+
] = None
866867
auth: Annotated[bool, Field(description="Enable the authorization")] = True
867868

868869
scaling: Annotated[

src/dstack/_internal/server/compatibility/runs.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
from packaging.version import Version
44

5-
from dstack._internal.core.models.configurations import ServiceConfiguration
5+
from dstack._internal.core.models.configurations import SERVICE_HTTPS_DEFAULT, ServiceConfiguration
66
from dstack._internal.core.models.runs import Run, RunPlan, RunSpec
77
from dstack._internal.server.compatibility.common import patch_offers_list
88

@@ -34,3 +34,10 @@ def patch_run_spec(run_spec: RunSpec, client_version: Optional[Version]) -> None
3434
):
3535
if run_spec.configuration.probes is None:
3636
run_spec.configuration.probes = []
37+
# Clients prior to 0.20.12 do not support https = None
38+
if (
39+
client_version < Version("0.20.12")
40+
and isinstance(run_spec.configuration, ServiceConfiguration)
41+
and run_spec.configuration.https is None
42+
):
43+
run_spec.configuration.https = SERVICE_HTTPS_DEFAULT

src/dstack/_internal/server/services/runs/spec.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
from dstack._internal.core.errors import ServerClientError
2-
from dstack._internal.core.models.configurations import RUN_PRIORITY_DEFAULT, ServiceConfiguration
2+
from dstack._internal.core.models.configurations import (
3+
RUN_PRIORITY_DEFAULT,
4+
SERVICE_HTTPS_DEFAULT,
5+
ServiceConfiguration,
6+
)
37
from dstack._internal.core.models.repos.virtual import DEFAULT_VIRTUAL_REPO_ID, VirtualRunRepoData
48
from dstack._internal.core.models.runs import LEGACY_REPO_DIR, AnyRunConfiguration, RunSpec
59
from dstack._internal.core.models.volumes import InstanceMountPoint
@@ -203,6 +207,14 @@ def _check_can_update_configuration(
203207
# Currently, the client preserves the original file/dir name it the tarball, but it could
204208
# use some generic names like "file"/"directory" instead.
205209
updatable_fields.append("files")
210+
if (
211+
isinstance(current, ServiceConfiguration)
212+
and isinstance(new, ServiceConfiguration)
213+
and current.https in (None, SERVICE_HTTPS_DEFAULT)
214+
and new.https in (None, SERVICE_HTTPS_DEFAULT)
215+
):
216+
# Allow switching between `https: <explicit-default>` and unset `https`. Has no effect.
217+
updatable_fields.append("https")
206218
diff = diff_models(current, new)
207219
changed_fields = list(diff.keys())
208220
for key in changed_fields:

src/dstack/_internal/server/services/services/__init__.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
)
2323
from dstack._internal.core.models.configurations import (
2424
DEFAULT_REPLICA_GROUP_NAME,
25+
SERVICE_HTTPS_DEFAULT,
2526
ServiceConfiguration,
2627
)
2728
from dstack._internal.core.models.gateways import GatewayConfiguration, GatewayStatus
@@ -240,11 +241,13 @@ def _register_service_in_server(run_model: RunModel, run_spec: RunSpec) -> Servi
240241
"Service with SGLang router configuration requires a gateway. "
241242
"Please configure a gateway with the SGLang router enabled."
242243
)
243-
if run_spec.configuration.https is False:
244-
# Note: if the user sets `https: <default-value>`, it will be ignored silently
245-
# TODO: in 0.19, make `https` Optional to be able to tell if it was set or omitted
244+
if run_spec.configuration.https not in (
245+
None,
246+
"auto",
247+
True, # Default set by pre-0.20.12 clients. TODO(0.21.0?): forbid True too.
248+
):
246249
raise ServerClientError(
247-
"The `https` configuration property is not applicable when running services without a gateway."
250+
f"Setting `https: {run_spec.configuration.https}` is not allowed without a gateway."
248251
" Please configure a gateway or remove the `https` property from the service configuration"
249252
)
250253
# Check if any group has autoscaling (min != max)
@@ -416,6 +419,8 @@ async def unregister_replica(session: AsyncSession, job_model: JobModel):
416419
def _get_service_https(run_spec: RunSpec, configuration: GatewayConfiguration) -> bool:
417420
assert run_spec.configuration.type == "service"
418421
https = run_spec.configuration.https
422+
if https is None:
423+
https = SERVICE_HTTPS_DEFAULT
419424
if https == "auto":
420425
if configuration.certificate is None:
421426
return False

src/tests/_internal/server/services/services/test_services.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@ def _mock_run_model() -> MagicMock:
4545

4646

4747
class TestServiceConfigurationHttps:
48-
def test_default_is_true(self) -> None:
48+
def test_accepts_unset(self) -> None:
4949
conf = ServiceConfiguration(commands=["python serve.py"], port=8000)
50-
assert conf.https is True
50+
assert conf.https is None
5151

5252
def test_accepts_auto(self) -> None:
5353
conf = ServiceConfiguration(commands=["python serve.py"], port=8000, https="auto")
@@ -96,5 +96,5 @@ def test_allows_auto_without_gateway(self) -> None:
9696

9797
def test_rejects_explicit_false_without_gateway(self) -> None:
9898
run_spec = _service_run_spec(https=False)
99-
with pytest.raises(ServerClientError, match="not applicable"):
99+
with pytest.raises(ServerClientError, match="not allowed without a gateway"):
100100
_register_service_in_server(_mock_run_model(), run_spec)

0 commit comments

Comments
 (0)