Disable LEGACY_REPO_DIR with feature flag

Enabling DSTACK_FF_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

Part-of: #3124

Author: un-def

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

@@ -57,6 +57,7 @@
     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
@@ -100,14 +101,18 @@ def apply_configuration(
             # 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 isinstance(conf, DevEnvironmentConfiguration) or conf.commands:
+            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."
@@ -124,6 +129,8 @@ def apply_configuration(
                 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"

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

@@ -4,6 +4,7 @@
 from dstack._internal.core.models.configurations import LEGACY_REPO_DIR, ServiceConfiguration
 from dstack._internal.core.models.runs import ApplyRunPlanInput, JobSpec, JobSubmission, RunSpec
 from dstack._internal.server.schemas.runs import GetRunPlanRequest, ListRunsRequest
+from dstack._internal.settings import FeatureFlags
 
 
 def get_list_runs_excludes(list_runs_request: ListRunsRequest) -> IncludeExcludeSetType:
@@ -133,10 +134,15 @@ def get_run_spec_excludes(run_spec: RunSpec) -> IncludeExcludeDictType:
     configuration = run_spec.configuration
     profile = run_spec.profile
 
-    if run_spec.repo_dir in [None, LEGACY_REPO_DIR]:
-        spec_excludes["repo_dir"] = True
-    elif run_spec.repo_dir == "." and configuration.working_dir in [None, LEGACY_REPO_DIR, "."]:
-        spec_excludes["repo_dir"] = True
+    if not FeatureFlags.LEGACY_REPO_DIR_DISABLED:
+        if run_spec.repo_dir in [None, LEGACY_REPO_DIR]:
+            spec_excludes["repo_dir"] = True
+        elif run_spec.repo_dir == "." and configuration.working_dir in [
+            None,
+            LEGACY_REPO_DIR,
+            ".",
+        ]:
+            spec_excludes["repo_dir"] = True
 
     if configuration.fleets is None:
         configuration_excludes["fleets"] = True

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

@@ -35,7 +35,7 @@
 )
 
 
-def use_legacy_default_working_dir(request: Request) -> bool:
+def use_legacy_repo_dir(request: Request) -> bool:
     client_release = cast(Optional[tuple[int, ...]], request.state.client_release)
     return client_release is not None and client_release < (0, 19, 27)
 
@@ -110,7 +110,7 @@ async def get_plan(
     body: GetRunPlanRequest,
     session: Annotated[AsyncSession, Depends(get_session)],
     user_project: Annotated[tuple[UserModel, ProjectModel], Depends(ProjectMember())],
-    legacy_default_working_dir: Annotated[bool, Depends(use_legacy_default_working_dir)],
+    legacy_repo_dir: Annotated[bool, Depends(use_legacy_repo_dir)],
 ):
     """
     Returns a run plan for the given run spec.
@@ -125,7 +125,7 @@ async def get_plan(
         user=user,
         run_spec=body.run_spec,
         max_offers=body.max_offers,
-        legacy_default_working_dir=legacy_default_working_dir,
+        legacy_repo_dir=legacy_repo_dir,
     )
     return CustomORJSONResponse(run_plan)
 
@@ -138,7 +138,7 @@ async def apply_plan(
     body: ApplyRunPlanRequest,
     session: Annotated[AsyncSession, Depends(get_session)],
     user_project: Annotated[tuple[UserModel, ProjectModel], Depends(ProjectMember())],
-    legacy_default_working_dir: Annotated[bool, Depends(use_legacy_default_working_dir)],
+    legacy_repo_dir: Annotated[bool, Depends(use_legacy_repo_dir)],
 ):
     """
     Creates a new run or updates an existing run.
@@ -156,7 +156,7 @@ async def apply_plan(
             project=project,
             plan=body.plan,
             force=body.force,
-            legacy_default_working_dir=legacy_default_working_dir,
+            legacy_repo_dir=legacy_repo_dir,
         )
     )
 

src/dstack/_internal/server/services/jobs/configurators/base.py

@@ -313,6 +313,9 @@ def _repo_dir(self) -> str:
         Returns absolute or relative path
         """
         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
+        # and in-place update.
         if repo_dir is None:
             return LEGACY_REPO_DIR
         return repo_dir

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

@@ -283,7 +283,7 @@ async def get_plan(
     user: UserModel,
     run_spec: RunSpec,
     max_offers: Optional[int],
-    legacy_default_working_dir: bool = False,
+    legacy_repo_dir: bool = False,
 ) -> RunPlan:
     # Spec must be copied by parsing to calculate merged_profile
     effective_run_spec = RunSpec.parse_obj(run_spec.dict())
@@ -296,7 +296,7 @@ async def get_plan(
     validate_run_spec_and_set_defaults(
         user=user,
         run_spec=effective_run_spec,
-        legacy_default_working_dir=legacy_default_working_dir,
+        legacy_repo_dir=legacy_repo_dir,
     )
     profile = effective_run_spec.merged_profile
 
@@ -342,7 +342,7 @@ async def apply_plan(
     project: ProjectModel,
     plan: ApplyRunPlanInput,
     force: bool,
-    legacy_default_working_dir: bool = False,
+    legacy_repo_dir: bool = False,
 ) -> Run:
     run_spec = plan.run_spec
     run_spec = await apply_plugin_policies(
@@ -353,7 +353,7 @@ async def apply_plan(
     # Spec must be copied by parsing to calculate merged_profile
     run_spec = RunSpec.parse_obj(run_spec.dict())
     validate_run_spec_and_set_defaults(
-        user=user, run_spec=run_spec, legacy_default_working_dir=legacy_default_working_dir
+        user=user, run_spec=run_spec, legacy_repo_dir=legacy_repo_dir
     )
     if run_spec.run_name is None:
         return await submit_run(

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

@@ -9,6 +9,7 @@
 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__)
@@ -55,7 +56,7 @@
 
 
 def validate_run_spec_and_set_defaults(
-    user: UserModel, run_spec: RunSpec, legacy_default_working_dir: bool = False
+    user: UserModel, run_spec: RunSpec, legacy_repo_dir: bool = False
 ):
     # This function may set defaults for null run_spec values,
     # although most defaults are resolved when building job_spec
@@ -111,8 +112,10 @@ def validate_run_spec_and_set_defaults(
             run_spec.ssh_key_pub = user.ssh_public_key
         else:
             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_default_working_dir:
+    if run_spec.configuration.working_dir is None and legacy_repo_dir:
         run_spec.configuration.working_dir = LEGACY_REPO_DIR
+    if 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):

src/dstack/_internal/settings.py

@@ -36,3 +36,11 @@ class FeatureFlags:
     """
 
     AUTOCREATED_FLEETS_DISABLED = os.getenv("DSTACK_FF_AUTOCREATED_FLEETS_DISABLED") 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

src/tests/conftest.py

@@ -1,8 +1,10 @@
+import inspect
 import os
 
 import pytest
 
 from dstack._internal.server.testing.conf import postgres_container, session, test_db  # noqa: F401
+from dstack._internal.settings import FeatureFlags
 
 
 def pytest_configure(config):
@@ -40,3 +42,17 @@ def pytest_collection_modifyitems(config, items):
             item.add_marker(skip_windows)
         if not for_windows and is_windows:
             item.add_marker(skip_posix)
+
+
+@pytest.fixture(scope="session", autouse=True)
+def disable_feature_flags():
+    """
+    Disables all feature flags once per test session.
+
+    If you need to test a feature flag, monkeypatch `FeatureFlags` class on a per-test basis.
+    """
+    for name, value in inspect.getmembers(FeatureFlags):
+        if not name.startswith("_") and name.isupper():
+            if not isinstance(value, bool):
+                raise RuntimeError(f"FeatureFlags.{name}: only bool values are supported")
+            setattr(FeatureFlags, name, False)