From 7e0896800f97cdb305281e4869a29524568f8da3 Mon Sep 17 00:00:00 2001 From: Dmitry Meyer Date: Fri, 5 Dec 2025 07:28:03 +0000 Subject: [PATCH 1/2] Drop legacy working/repo dirs where possible * `working_dir` now defaults to `image`s WORKDIR * `repos[].path` now defaults to `.` (= `working_dir`) * compatibility code removed Part-of: https://github.com/dstackai/dstack/issues/3124 --- runner/cmd/runner/cmd.go | 6 --- runner/consts/consts.go | 2 - runner/internal/executor/executor.go | 5 +- .../cli/services/configurators/run.py | 48 ++----------------- .../_internal/core/backends/base/compute.py | 3 -- .../_internal/core/models/configurations.py | 10 ++-- src/dstack/_internal/core/models/runs.py | 16 +------ .../services/jobs/configurators/base.py | 21 ++++---- .../_internal/server/services/runs/spec.py | 8 ---- src/dstack/_internal/server/testing/common.py | 1 - src/dstack/_internal/settings.py | 8 ---- src/dstack/api/_public/runs.py | 7 +-- .../core/models/test_configurations.py | 8 ++-- .../_internal/server/routers/test_runs.py | 3 -- 14 files changed, 24 insertions(+), 122 deletions(-) diff --git a/runner/cmd/runner/cmd.go b/runner/cmd/runner/cmd.go index ed3f400c42..08f3d5b018 100644 --- a/runner/cmd/runner/cmd.go +++ b/runner/cmd/runner/cmd.go @@ -49,12 +49,6 @@ func App() { Value: consts.RunnerHomeDir, Destination: &homeDir, }, - // TODO: Not used, left for compatibility with old servers. Remove eventually. - &cli.PathFlag{ - Name: "working-dir", - Hidden: true, - Destination: nil, - }, &cli.IntFlag{ Name: "http-port", Usage: "Set a http port", diff --git a/runner/consts/consts.go b/runner/consts/consts.go index aa7262ba17..aa0b8d056f 100644 --- a/runner/consts/consts.go +++ b/runner/consts/consts.go @@ -28,8 +28,6 @@ const ( RunnerHomeDir = "/root" ) -const LegacyRepoDir = "/workflow" - const ( RunnerHTTPPort = 10999 RunnerSSHPort = 10022 diff --git a/runner/internal/executor/executor.go b/runner/internal/executor/executor.go index 7540315a50..6f302e81cb 100644 --- a/runner/internal/executor/executor.go +++ b/runner/internal/executor/executor.go @@ -361,10 +361,7 @@ func (ex *RunExecutor) setJobWorkingDir(ctx context.Context) error { return fmt.Errorf("get working directory: %w", err) } } else { - // We still support relative paths, as 0.19.27 server uses relative paths when possible - // for compatibility with pre-0.19.27 runners. - // Replace consts.LegacyRepoDir with "" eventually. - ex.jobWorkingDir, err = common.ExpandPath(*ex.jobSpec.WorkingDir, consts.LegacyRepoDir, ex.jobHomeDir) + ex.jobWorkingDir, err = common.ExpandPath(*ex.jobSpec.WorkingDir, "", ex.jobHomeDir) if err != nil { return fmt.Errorf("expand working dir path: %w", err) } diff --git a/src/dstack/_internal/cli/services/configurators/run.py b/src/dstack/_internal/cli/services/configurators/run.py index 6006e041de..090977a213 100644 --- a/src/dstack/_internal/cli/services/configurators/run.py +++ b/src/dstack/_internal/cli/services/configurators/run.py @@ -3,7 +3,7 @@ import subprocess import sys import time -from pathlib import Path, PurePosixPath +from pathlib import Path from typing import Dict, List, Optional, Set, TypeVar import gpuhunt @@ -33,7 +33,6 @@ ) from dstack._internal.core.models.common import ApplyAction, RegistryAuth from dstack._internal.core.models.configurations import ( - LEGACY_REPO_DIR, AnyRunConfiguration, ApplyConfigurationType, ConfigurationWithCommandsParams, @@ -57,7 +56,6 @@ get_repo_creds_and_default_branch, load_repo, ) -from dstack._internal.settings import FeatureFlags from dstack._internal.utils.common import local_time from dstack._internal.utils.interpolator import InterpolatorError, VariablesInterpolator from dstack._internal.utils.logging import get_logger @@ -95,48 +93,8 @@ def apply_configuration( self.validate_gpu_vendor_and_image(conf) self.validate_cpu_arch_and_image(conf) - working_dir = conf.working_dir - if working_dir is None: - # Use the default working dir for the image for tasks and services if `commands` - # is not set (emulate pre-0.19.27 JobConfigutor logic), otherwise fall back to - # `/workflow`. - if not FeatureFlags.LEGACY_REPO_DIR_DISABLED and ( - isinstance(conf, DevEnvironmentConfiguration) or conf.commands - ): - # relative path for compatibility with pre-0.19.27 servers - conf.working_dir = "." - warn( - f'The [code]working_dir[/code] is not set — using legacy default [code]"{LEGACY_REPO_DIR}"[/code].' - " Future versions will default to the [code]image[/code]'s working directory." - ) - elif not is_absolute_posix_path(working_dir): - if FeatureFlags.LEGACY_REPO_DIR_DISABLED: - raise ConfigurationError("`working_dir` must be absolute") - legacy_working_dir = PurePosixPath(LEGACY_REPO_DIR) / working_dir - warn( - "[code]working_dir[/code] is relative." - f" Using legacy working directory [code]{legacy_working_dir}[/code]\n\n" - "Future versions will require absolute path\n" - f"To keep using legacy working directory, set" - f" [code]working_dir[/code] to [code]{legacy_working_dir}[/code]\n" - ) - else: - # relative path for compatibility with pre-0.19.27 servers - try: - conf.working_dir = str(PurePosixPath(working_dir).relative_to(LEGACY_REPO_DIR)) - except ValueError: - pass - - if conf.repos and conf.repos[0].path is None: - if FeatureFlags.LEGACY_REPO_DIR_DISABLED: - raise ConfigurationError("`repos[0].path` is not set") - warn( - "[code]repos[0].path[/code] is not set," - f" using legacy repo path [code]{LEGACY_REPO_DIR}[/code]\n\n" - "In a future version the default value will be changed." - f" To keep using [code]{LEGACY_REPO_DIR}[/code], explicitly set" - f" [code]repos[0].path[/code] to [code]{LEGACY_REPO_DIR}[/code]\n" - ) + if conf.working_dir is not None and not is_absolute_posix_path(conf.working_dir): + raise ConfigurationError("working_dir must be absolute") config_manager = ConfigManager() repo = self.get_repo(conf, configuration_path, configurator_args, config_manager) diff --git a/src/dstack/_internal/core/backends/base/compute.py b/src/dstack/_internal/core/backends/base/compute.py index 1178068180..a0ff70c1ba 100644 --- a/src/dstack/_internal/core/backends/base/compute.py +++ b/src/dstack/_internal/core/backends/base/compute.py @@ -26,7 +26,6 @@ ) from dstack._internal.core.models.backends.base import BackendType from dstack._internal.core.models.compute_groups import ComputeGroup, ComputeGroupProvisioningData -from dstack._internal.core.models.configurations import LEGACY_REPO_DIR from dstack._internal.core.models.gateways import ( GatewayComputeConfiguration, GatewayProvisioningData, @@ -971,8 +970,6 @@ def get_docker_commands( f" --ssh-port {DSTACK_RUNNER_SSH_PORT}" " --temp-dir /tmp/runner" " --home-dir /root" - # TODO: Not used, left for compatibility with old runners. Remove eventually. - f" --working-dir {LEGACY_REPO_DIR}" ), ] diff --git a/src/dstack/_internal/core/models/configurations.py b/src/dstack/_internal/core/models/configurations.py index 60edd51c38..158c59b341 100644 --- a/src/dstack/_internal/core/models/configurations.py +++ b/src/dstack/_internal/core/models/configurations.py @@ -131,14 +131,14 @@ class RepoSpec(CoreModel): Field(description="The commit hash"), ] = None path: Annotated[ - Optional[str], + str, Field( description=( "The repo path inside the run container. Relative paths are resolved" - f" relative to the working directory. Defaults to `{LEGACY_REPO_DIR}`" + " relative to the working directory" ) ), - ] = None + ] = "." if_exists: Annotated[ RepoExistsAction, Field( @@ -447,8 +447,8 @@ class BaseRunConfiguration(CoreModel): Field( description=( "The absolute path to the working directory inside the container." - f" Defaults to `{LEGACY_REPO_DIR}`" - ) + " Defaults to the `image`'s default working directory" + ), ), ] = None # deprecated since 0.18.31; has no effect diff --git a/src/dstack/_internal/core/models/runs.py b/src/dstack/_internal/core/models/runs.py index 841d58bd66..13e6a15722 100644 --- a/src/dstack/_internal/core/models/runs.py +++ b/src/dstack/_internal/core/models/runs.py @@ -269,7 +269,6 @@ class JobSpec(CoreModel): retry: Optional[Retry] volumes: Optional[List[MountPoint]] = None ssh_key: Optional[JobSSHKey] = None - # `working_dir` is always absolute (if not None) since 0.19.27 working_dir: Optional[str] # `repo_data` is optional for client compatibility with pre-0.19.17 servers and for compatibility # with jobs submitted before 0.19.17. All new jobs are expected to have non-None `repo_data`. @@ -405,7 +404,7 @@ def schema_extra(schema: Dict[str, Any]): class RunSpec(generate_dual_core_model(RunSpecConfig)): - # TODO: run_name, working_dir are redundant here since they already passed in configuration + # TODO: run_name is redundant here since they already passed in configuration run_name: Annotated[ Optional[str], Field(description="The run name. If not set, the run name is generated automatically."), @@ -436,7 +435,7 @@ class RunSpec(generate_dual_core_model(RunSpecConfig)): Field( description=( "The repo path inside the container. Relative paths are resolved" - f" relative to the working directory. Defaults to `{LEGACY_REPO_DIR}`." + " relative to the working directory." ) ), ] = None @@ -444,17 +443,6 @@ class RunSpec(generate_dual_core_model(RunSpecConfig)): list[FileArchiveMapping], Field(description="The list of file archive ID to container path mappings."), ] = [] - # Server uses configuration.working_dir instead of this field since 0.19.27, but - # the field still exists for compatibility with older servers - working_dir: Annotated[ - Optional[str], - Field( - description=( - "The absolute path to the working directory inside the container." - " Defaults to the default working directory from the `image`." - ) - ), - ] = None configuration_path: Annotated[ Optional[str], Field( diff --git a/src/dstack/_internal/server/services/jobs/configurators/base.py b/src/dstack/_internal/server/services/jobs/configurators/base.py index 4cc2c9079e..0e770b2e90 100644 --- a/src/dstack/_internal/server/services/jobs/configurators/base.py +++ b/src/dstack/_internal/server/services/jobs/configurators/base.py @@ -314,9 +314,12 @@ def _repo_dir(self) -> str: """ Returns absolute or relative path """ + if repos := self.run_spec.configuration.repos: + return repos[0].path + # `repo_dir` may be set while `repos` is empty if the RunSpec was submitted before 0.20.0 repo_dir = self.run_spec.repo_dir # We need this fallback indefinitely, as there may be RunSpecs submitted before - # repos[].path became required, and JobSpec is regenerated from RunSpec on each retry + # `repos[].path` was added, and JobSpec is regenerated from RunSpec on each retry # and in-place update. if repo_dir is None: return LEGACY_REPO_DIR @@ -335,23 +338,15 @@ def _repo_exists_action(self) -> Optional[RepoExistsAction]: def _working_dir(self) -> Optional[str]: """ - Returns path or None + Returns absolute path or None None means the default working directory taken from the image - - Currently, for compatibility with pre-0.19.27 runners, the path may be relative. - Future versions should return only absolute paths """ working_dir = self.run_spec.configuration.working_dir - if working_dir is None: + if working_dir is None or is_absolute_posix_path(working_dir): return working_dir - # Return a relative path if possible - if is_absolute_posix_path(working_dir): - try: - return str(PurePosixPath(working_dir).relative_to(LEGACY_REPO_DIR)) - except ValueError: - pass - return working_dir + # Support for pre-0.20.0 configurations + return str(PurePosixPath(LEGACY_REPO_DIR) / working_dir) def _python(self) -> str: if self.run_spec.configuration.python is not None: diff --git a/src/dstack/_internal/server/services/runs/spec.py b/src/dstack/_internal/server/services/runs/spec.py index dbf62e57d1..73b6d9fc7a 100644 --- a/src/dstack/_internal/server/services/runs/spec.py +++ b/src/dstack/_internal/server/services/runs/spec.py @@ -9,7 +9,6 @@ from dstack._internal.server.models import UserModel from dstack._internal.server.services.docker import is_valid_docker_volume_target from dstack._internal.server.services.resources import set_resources_defaults -from dstack._internal.settings import FeatureFlags from dstack._internal.utils.logging import get_logger logger = get_logger(__name__) @@ -114,13 +113,6 @@ def validate_run_spec_and_set_defaults( raise ServerClientError("ssh_key_pub must be set if the user has no ssh_public_key") if run_spec.configuration.working_dir is None and legacy_repo_dir: run_spec.configuration.working_dir = LEGACY_REPO_DIR - if ( - run_spec.configuration.repos - and run_spec.repo_dir is None - and FeatureFlags.LEGACY_REPO_DIR_DISABLED - and not legacy_repo_dir - ): - raise ServerClientError("Repo path is not set") def check_can_update_run_spec(current_run_spec: RunSpec, new_run_spec: RunSpec): diff --git a/src/dstack/_internal/server/testing/common.py b/src/dstack/_internal/server/testing/common.py index 9c9585982f..8f9459a766 100644 --- a/src/dstack/_internal/server/testing/common.py +++ b/src/dstack/_internal/server/testing/common.py @@ -285,7 +285,6 @@ def get_run_spec( repo_id=repo_id, repo_data=LocalRunRepoData(repo_dir="/"), repo_code_hash=None, - working_dir=None, configuration_path=configuration_path, configuration=configuration or DevEnvironmentConfiguration(ide="vscode"), profile=profile, diff --git a/src/dstack/_internal/settings.py b/src/dstack/_internal/settings.py index 84efb9254d..245681411d 100644 --- a/src/dstack/_internal/settings.py +++ b/src/dstack/_internal/settings.py @@ -38,11 +38,3 @@ class FeatureFlags: # DSTACK_FF_AUTOCREATED_FLEETS_ENABLED enables legacy autocreated fleets: # If there are no fleet suitable for the run, a new fleet is created automatically instead of an error. AUTOCREATED_FLEETS_ENABLED = os.getenv("DSTACK_FF_AUTOCREATED_FLEETS_ENABLED") is not None - - # Enabling LEGACY_REPO_DIR_DISABLED does the following: - # - Changes `working_dir` default value from `/workflow` to the image's working dir, unless - # the client is older than 0.19.27, in which case `/workflow` is still used. - # - Forbids relative `working_dir` (client side only). - # - Makes `repos[].path` required, unless the client is older than 0.19.27, - # in which case `/workflow` is still used. - LEGACY_REPO_DIR_DISABLED = os.getenv("DSTACK_FF_LEGACY_REPO_DIR_DISABLED") is not None diff --git a/src/dstack/api/_public/runs.py b/src/dstack/api/_public/runs.py index 7eea551d6f..5163aa8e60 100644 --- a/src/dstack/api/_public/runs.py +++ b/src/dstack/api/_public/runs.py @@ -467,13 +467,11 @@ def get_run_plan( is not loaded from a file. repo_dir: The path of the cloned repo inside the run container. If not set, defaults first to the `repos[0].path` property of the configuration (for remote - repos only), then to `/workflow`. + repos only). Returns: Run plan. """ - # XXX: not using the LEGACY_REPO_DIR const in the docstring above, as the docs generator, - # apparently, doesn't support f-strings (f"""..."""). if repo is None: repo = VirtualRepo() repo_code_hash = None @@ -520,9 +518,6 @@ def get_run_plan( repo_code_hash=repo_code_hash, repo_dir=repo_dir, file_archives=file_archives, - # Server doesn't use this field since 0.19.27, but we still send it for compatibility - # with older servers - working_dir=configuration.working_dir, configuration_path=configuration_path, configuration=configuration, profile=profile, diff --git a/src/tests/_internal/core/models/test_configurations.py b/src/tests/_internal/core/models/test_configurations.py index 4a43a8c46d..79007fe195 100644 --- a/src/tests/_internal/core/models/test_configurations.py +++ b/src/tests/_internal/core/models/test_configurations.py @@ -76,7 +76,7 @@ def test_shell_invalid(self): class TestRepoSpec: @pytest.mark.parametrize("value", [".", "rel/path", "/abs/path/"]) def test_parse_local_path_no_path(self, value: str): - assert RepoSpec.parse(value) == RepoSpec(local_path=value, path=None) + assert RepoSpec.parse(value) == RepoSpec(local_path=value, path=".") @pytest.mark.parametrize( ["value", "expected_repo_path"], @@ -86,14 +86,14 @@ def test_parse_local_path_with_path(self, value: str, expected_repo_path: str): assert RepoSpec.parse(value) == RepoSpec(local_path=expected_repo_path, path="/repo") def test_parse_windows_abs_local_path_no_path(self): - assert RepoSpec.parse("C:\\repo") == RepoSpec(local_path="C:\\repo", path=None) + assert RepoSpec.parse("C:\\repo") == RepoSpec(local_path="C:\\repo", path=".") def test_parse_windows_abs_local_path_with_path(self): assert RepoSpec.parse("C:\\repo:/repo") == RepoSpec(local_path="C:\\repo", path="/repo") def test_parse_url_no_path(self): assert RepoSpec.parse("https://example.com/repo.git") == RepoSpec( - url="https://example.com/repo.git", path=None + url="https://example.com/repo.git", path="." ) def test_parse_url_with_path(self): @@ -103,7 +103,7 @@ def test_parse_url_with_path(self): def test_parse_scp_no_path(self): assert RepoSpec.parse("git@example.com:repo.git") == RepoSpec( - url="git@example.com:repo.git", path=None + url="git@example.com:repo.git", path="." ) def test_parse_scp_with_path(self): diff --git a/src/tests/_internal/server/routers/test_runs.py b/src/tests/_internal/server/routers/test_runs.py index 71a7ecfdfd..570029a940 100644 --- a/src/tests/_internal/server/routers/test_runs.py +++ b/src/tests/_internal/server/routers/test_runs.py @@ -232,7 +232,6 @@ def get_dev_env_run_plan_dict( "repo_dir": "~/repo", "run_name": run_name, "ssh_key_pub": "ssh_key", - "working_dir": None, } return { "project_name": project_name, @@ -470,7 +469,6 @@ def get_dev_env_run_dict( "repo_dir": "~/repo", "run_name": run_name, "ssh_key_pub": "ssh_key", - "working_dir": None, }, "jobs": [ { @@ -614,7 +612,6 @@ def get_service_run_spec( "repo_dir": "~/repo", "run_name": run_name, "ssh_key_pub": "ssh_key", - "working_dir": None, } From d3790f860a81fe22ebe19062ea40070a61c6f3e4 Mon Sep 17 00:00:00 2001 From: Dmitry Meyer Date: Fri, 5 Dec 2025 09:21:29 +0000 Subject: [PATCH 2/2] Replace /workflow with / in warning --- src/dstack/_internal/utils/ssh.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dstack/_internal/utils/ssh.py b/src/dstack/_internal/utils/ssh.py index 851103af6f..e2b5fef436 100644 --- a/src/dstack/_internal/utils/ssh.py +++ b/src/dstack/_internal/utils/ssh.py @@ -155,8 +155,8 @@ def include_ssh_config(path: PathLike, ssh_config_path: PathLike = default_ssh_c except PermissionError: logger.warning( f"Couldn't update `{ssh_config_path}` due to a permissions problem.\n" - f"The `vscode://vscode-remote/ssh-remote+/workflow` and " - f"`cursor://vscode-remote/ssh-remote+/workflow` links and " + f"The `vscode://vscode-remote/ssh-remote+/` and " + f"`cursor://vscode-remote/ssh-remote+/` links and " f"the `ssh ` command won't work.\n" f"To fix this, make sure `{ssh_config_path}` is writable, or add " f"`Include {path}` to the top of `{ssh_config_path}` manually.",