diff --git a/src/dstack/_internal/core/compatibility/runs.py b/src/dstack/_internal/core/compatibility/runs.py index 4ece12392..d8330031b 100644 --- a/src/dstack/_internal/core/compatibility/runs.py +++ b/src/dstack/_internal/core/compatibility/runs.py @@ -78,6 +78,8 @@ def get_run_spec_excludes(run_spec: RunSpec) -> IncludeExcludeDictType: configuration_excludes["router"] = True elif isinstance(router, SGLangServiceRouterConfig) and router.pd_disaggregation is False: configuration_excludes["router"] = {"pd_disaggregation": True} + if run_spec.configuration.https is None: + configuration_excludes["https"] = True if configuration_excludes: spec_excludes["configuration"] = configuration_excludes diff --git a/src/dstack/_internal/core/models/configurations.py b/src/dstack/_internal/core/models/configurations.py index 87e38b496..8c86fd5bd 100644 --- a/src/dstack/_internal/core/models/configurations.py +++ b/src/dstack/_internal/core/models/configurations.py @@ -857,12 +857,13 @@ class ServiceConfigurationParams(CoreModel): ), ] = None https: Annotated[ - Union[bool, Literal["auto"]], + Optional[Union[bool, Literal["auto"]]], Field( description="Enable HTTPS if running with a gateway." - " Set to `auto` to determine automatically based on gateway configuration" + " Set to `auto` to determine automatically based on gateway configuration." + f" Defaults to `{str(SERVICE_HTTPS_DEFAULT).lower()}`" ), - ] = SERVICE_HTTPS_DEFAULT + ] = None auth: Annotated[bool, Field(description="Enable the authorization")] = True scaling: Annotated[ diff --git a/src/dstack/_internal/server/compatibility/runs.py b/src/dstack/_internal/server/compatibility/runs.py index 2e715ce16..77df3f5fa 100644 --- a/src/dstack/_internal/server/compatibility/runs.py +++ b/src/dstack/_internal/server/compatibility/runs.py @@ -2,7 +2,7 @@ from packaging.version import Version -from dstack._internal.core.models.configurations import ServiceConfiguration +from dstack._internal.core.models.configurations import SERVICE_HTTPS_DEFAULT, ServiceConfiguration from dstack._internal.core.models.runs import Run, RunPlan, RunSpec from dstack._internal.server.compatibility.common import patch_offers_list @@ -34,3 +34,10 @@ def patch_run_spec(run_spec: RunSpec, client_version: Optional[Version]) -> None ): if run_spec.configuration.probes is None: run_spec.configuration.probes = [] + # Clients prior to 0.20.12 do not support https = None + if ( + client_version < Version("0.20.12") + and isinstance(run_spec.configuration, ServiceConfiguration) + and run_spec.configuration.https is None + ): + run_spec.configuration.https = SERVICE_HTTPS_DEFAULT diff --git a/src/dstack/_internal/server/services/runs/spec.py b/src/dstack/_internal/server/services/runs/spec.py index a18f151ce..8623fd0a1 100644 --- a/src/dstack/_internal/server/services/runs/spec.py +++ b/src/dstack/_internal/server/services/runs/spec.py @@ -1,5 +1,9 @@ from dstack._internal.core.errors import ServerClientError -from dstack._internal.core.models.configurations import RUN_PRIORITY_DEFAULT, ServiceConfiguration +from dstack._internal.core.models.configurations import ( + RUN_PRIORITY_DEFAULT, + SERVICE_HTTPS_DEFAULT, + ServiceConfiguration, +) from dstack._internal.core.models.repos.virtual import DEFAULT_VIRTUAL_REPO_ID, VirtualRunRepoData from dstack._internal.core.models.runs import LEGACY_REPO_DIR, AnyRunConfiguration, RunSpec from dstack._internal.core.models.volumes import InstanceMountPoint @@ -203,6 +207,14 @@ def _check_can_update_configuration( # Currently, the client preserves the original file/dir name it the tarball, but it could # use some generic names like "file"/"directory" instead. updatable_fields.append("files") + if ( + isinstance(current, ServiceConfiguration) + and isinstance(new, ServiceConfiguration) + and current.https in (None, SERVICE_HTTPS_DEFAULT) + and new.https in (None, SERVICE_HTTPS_DEFAULT) + ): + # Allow switching between `https: ` and unset `https`. Has no effect. + updatable_fields.append("https") diff = diff_models(current, new) changed_fields = list(diff.keys()) for key in changed_fields: diff --git a/src/dstack/_internal/server/services/services/__init__.py b/src/dstack/_internal/server/services/services/__init__.py index 8f0849050..cb5fde255 100644 --- a/src/dstack/_internal/server/services/services/__init__.py +++ b/src/dstack/_internal/server/services/services/__init__.py @@ -22,6 +22,7 @@ ) from dstack._internal.core.models.configurations import ( DEFAULT_REPLICA_GROUP_NAME, + SERVICE_HTTPS_DEFAULT, ServiceConfiguration, ) 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 "Service with SGLang router configuration requires a gateway. " "Please configure a gateway with the SGLang router enabled." ) - if run_spec.configuration.https is False: - # Note: if the user sets `https: `, it will be ignored silently - # TODO: in 0.19, make `https` Optional to be able to tell if it was set or omitted + if run_spec.configuration.https not in ( + None, + "auto", + True, # Default set by pre-0.20.12 clients. TODO(0.21.0?): forbid True too. + ): raise ServerClientError( - "The `https` configuration property is not applicable when running services without a gateway." + f"Setting `https: {run_spec.configuration.https}` is not allowed without a gateway." " Please configure a gateway or remove the `https` property from the service configuration" ) # Check if any group has autoscaling (min != max) @@ -416,6 +419,8 @@ async def unregister_replica(session: AsyncSession, job_model: JobModel): def _get_service_https(run_spec: RunSpec, configuration: GatewayConfiguration) -> bool: assert run_spec.configuration.type == "service" https = run_spec.configuration.https + if https is None: + https = SERVICE_HTTPS_DEFAULT if https == "auto": if configuration.certificate is None: return False diff --git a/src/tests/_internal/server/services/services/test_services.py b/src/tests/_internal/server/services/services/test_services.py index 1ee3bf70e..f028055b8 100644 --- a/src/tests/_internal/server/services/services/test_services.py +++ b/src/tests/_internal/server/services/services/test_services.py @@ -45,9 +45,9 @@ def _mock_run_model() -> MagicMock: class TestServiceConfigurationHttps: - def test_default_is_true(self) -> None: + def test_accepts_unset(self) -> None: conf = ServiceConfiguration(commands=["python serve.py"], port=8000) - assert conf.https is True + assert conf.https is None def test_accepts_auto(self) -> None: conf = ServiceConfiguration(commands=["python serve.py"], port=8000, https="auto") @@ -96,5 +96,5 @@ def test_allows_auto_without_gateway(self) -> None: def test_rejects_explicit_false_without_gateway(self) -> None: run_spec = _service_run_spec(https=False) - with pytest.raises(ServerClientError, match="not applicable"): + with pytest.raises(ServerClientError, match="not allowed without a gateway"): _register_service_in_server(_mock_run_model(), run_spec)