Let an infra path name the namespace it is rendered for (#3046)

This commit is contained in:
fzyzcjy
2026-09-26 20:31:30 +08:00
committed by GitHub
parent 5c323d6338
commit 54f7bae7c2
11 changed files with 262 additions and 50 deletions
+57 -21
View File
@@ -50,10 +50,38 @@ env:
{{- end }}
{{- end }}
{{- define "miles-common.interpolatePath" -}}
{{- $value := .value -}}
{{- range $reference := regexFindAll "\\$\\{[^}]*\\}" $value -1 -}}
{{- if ne $reference "${NAMESPACE}" -}}
{{- fail (printf "%s is %s, which names the unknown variable %s: ${NAMESPACE} is the only variable a path may name, and an unknown one left in place would give every namespace the same literal directory instead of one of its own" $.field $value $reference) -}}
{{- end -}}
{{- end -}}
{{- $value | replace "${NAMESPACE}" .root.Release.Namespace -}}
{{- end }}
{{- define "miles-common.assertInfraPathsResolve" -}}
{{- $_ := include "miles-common.volumes" . -}}
{{- $_ = include "miles-common.volumeMounts" . -}}
{{- $_ = include "miles-common.runsRoot" . -}}
{{- end }}
{{- define "miles-common.runsRoot" -}}
{{- with (.Values.infra.paths | default dict).runsRoot -}}
{{- include "miles-common.interpolatePath" (dict "root" $ "field" "infra.paths.runsRoot" "value" .) -}}
{{- end -}}
{{- end }}
{{- define "miles-common.volumes" -}}
{{- $root := . -}}
{{- range $volume := (.Values.infra.volumes | default list) }}
{{- $source := $volume }}
{{- with $volume.hostPath }}
{{- $path := include "miles-common.interpolatePath" (dict "root" $root "field" (printf "infra.volumes[%s].hostPath.path" $volume.name) "value" .path) }}
{{- $source = mustMergeOverwrite (deepCopy $volume) (dict "hostPath" (dict "path" $path)) }}
{{- end }}
- name: {{ $volume.name | quote }}
{{- include "miles-common.volumeSource" $volume | trim | nindent 2 }}
{{- include "miles-common.volumeSource" $source | trim | nindent 2 }}
{{- end }}
{{- end }}
@@ -76,12 +104,13 @@ emptyDir:
{{- end }}
{{- define "miles-common.volumeMounts" -}}
{{- $root := . -}}
{{- range $volume := (.Values.infra.volumes | default list) }}
{{- range $mount := ($volume.mounts | default list) }}
{{- range $index, $mount := ($volume.mounts | default list) }}
- name: {{ $volume.name | quote }}
mountPath: {{ $mount.mountPath | quote }}
mountPath: {{ include "miles-common.mountPath" (dict "root" $root "volume" $volume "index" $index "mount" $mount) | quote }}
{{- with $mount.subPath }}
subPath: {{ . | quote }}
subPath: {{ include "miles-common.interpolatePath" (dict "root" $root "field" (printf "infra.volumes[%s].mounts[%d].subPath" $volume.name $index) "value" .) | quote }}
{{- end }}
{{- if $mount.readOnly }}
readOnly: true
@@ -90,32 +119,39 @@ emptyDir:
{{- end }}
{{- end }}
{{- define "miles-common.mountPath" -}}
{{- include "miles-common.interpolatePath" (dict "root" .root "field" (printf "infra.volumes[%s].mounts[%d].mountPath" .volume.name (int .index)) "value" .mount.mountPath) -}}
{{- end }}
{{- define "miles-common.assertRunsRootIsMounted" -}}
{{- $runsRoot := (.Values.infra.paths | default dict).runsRoot | default "" -}}
{{- $root := . -}}
{{- $runsRoot := include "miles-common.runsRoot" . -}}
{{- if $runsRoot }}
{{- $writable := list -}}
{{- $readOnly := list -}}
{{- $runsRootPath := clean $runsRoot -}}
{{- $effective := dict -}}
{{- $all := list -}}
{{- range $volume := (.Values.infra.volumes | default list) }}
{{- range $mount := ($volume.mounts | default list) }}
{{- $all = append $all $mount.mountPath }}
{{- if or (eq $runsRoot $mount.mountPath) (hasPrefix (printf "%s/" $mount.mountPath) $runsRoot) }}
{{- if hasKey $volume "emptyDir" }}
{{- fail (printf "infra.paths.runsRoot is %s, which falls under the emptyDir volume %s mounted at %s: an emptyDir belongs to a single pod, so the launcher, the orchestrator and every worker would each write into a directory of their own and no run would ever report a verdict" $runsRoot $volume.name $mountPath) }}
{{- end }}
{{- if $mount.readOnly }}
{{- $readOnly = append $readOnly $mount.mountPath }}
{{- else }}
{{- $writable = append $writable $mount.mountPath }}
{{- range $index, $mount := ($volume.mounts | default list) }}
{{- $mountPath := include "miles-common.mountPath" (dict "root" $root "volume" $volume "index" $index "mount" $mount) }}
{{- $all = append $all $mountPath }}
{{- $cleaned := clean $mountPath }}
{{- if or (eq $runsRootPath $cleaned) (hasPrefix (printf "%s/" (trimSuffix "/" $cleaned)) $runsRootPath) }}
{{- if gt (len $cleaned) (len (get $effective "cleaned")) }}
{{- $effective = dict "cleaned" $cleaned "mountPath" $mountPath "volume" $volume "readOnly" $mount.readOnly }}
{{- end }}
{{- end }}
{{- end }}
{{- end }}
{{- if not $writable }}
{{- if $readOnly }}
{{- fail (printf "infra.paths.runsRoot is %s, which only falls under the read-only mount %s: every run writes its state, values and exit file there" $runsRoot (join ", " $readOnly)) }}
{{- end }}
{{- if not $effective }}
{{- fail (printf "infra.paths.runsRoot is %s, which falls under none of the infra.volumes mounts (%s), so a run would write its state file into the container's own filesystem where the launcher never sees it" $runsRoot (join ", " $all)) }}
{{- end }}
{{- $effectiveVolume := get $effective "volume" }}
{{- $effectiveMountPath := get $effective "mountPath" }}
{{- if hasKey $effectiveVolume "emptyDir" }}
{{- fail (printf "infra.paths.runsRoot is %s, which the emptyDir volume %s mounted at %s provides: an emptyDir belongs to a single pod, so the launcher, the orchestrator and every worker would each write into a directory of their own and no run would ever report a verdict" $runsRoot $effectiveVolume.name $effectiveMountPath) }}
{{- end }}
{{- if get $effective "readOnly" }}
{{- fail (printf "infra.paths.runsRoot is %s, which the read-only mount %s provides: every run writes its state, values and exit file there, and this mount shadows any writable one it nests under" $runsRoot $effectiveMountPath) }}
{{- end }}
{{- end }}
{{- end }}
@@ -0,0 +1 @@
{{- include "miles-common.assertInfraPathsResolve" . }}
+1 -1
View File
@@ -12,7 +12,7 @@ infra:
mounts:
- mountPath: /cluster-storage
paths:
runsRoot: /cluster-storage/miles_data
runsRoot: /cluster-storage/${NAMESPACE}/miles_data
devShm:
mountPath: /dev/shm
hostPath:
@@ -0,0 +1,2 @@
{{- include "miles-common.assertInfraPathsResolve" . }}
{{- include "miles-workbench.assertInfraValuesMountIsReserved" . }}
+1 -1
View File
@@ -14,7 +14,7 @@ infra:
mounts:
- mountPath: /cluster-storage
paths:
runsRoot: /cluster-storage/miles_data
runsRoot: /cluster-storage/${NAMESPACE}/miles_data
devShm:
mountPath: /dev/shm
hostPath:
+21 -19
View File
@@ -6,6 +6,13 @@ Miles runs a job on one of two cluster backends. Ray is the default and needs no
Kubernetes installs the run as a helm release, so the cluster schedules every worker. The training
script is the same either way.
## Ray
Ray is the default backend, so `python scripts/run_*.py train` launches a run as is, with nothing to
configure. Nothing in the Kubernetes chapter below applies to it.
## Kubernetes
<Warning>
**Status.** Under active development: flags, chart values and failure semantics still change. Ray
@@ -14,7 +21,7 @@ Miles to create the cluster's objects itself.
</Warning>
## Launch
### Launch
Run everything from the repository root, with `kubectl` and `helm` on your PATH.
@@ -41,7 +48,7 @@ The recipe defaults to `/root/models`, `/root/datasets` and `/root/shared_data`;
with `python scripts/run_qwen3_4b.py prepare`, or pass `--model-dir`, `--data-dir` and
`--output-dir` to point the recipe elsewhere.
## Observability
### Observability
**Built in**
@@ -57,7 +64,7 @@ with `python scripts/run_qwen3_4b.py prepare`, or pass `--model-dir`, `--data-di
collector, the platform's own dashboards — sees them with no wiring from Miles.
- Prefer it at scale. The built-in following is meant for watching one run, not hundreds of pods.
## Clean up
### Clean up
```bash
python -m miles.utils.external_utils.miles_workbench stop -n "$MILES_NS" 260811-143000-042
@@ -66,30 +73,22 @@ python -m miles.utils.external_utils.miles_workbench uninstall -n "$MILES_NS"
`stop` removes the run and frees its GPUs; `uninstall` removes the workbench.
## Folder convention
### Folder convention
A run is many pods on many machines, and they share nothing but the volumes `infra.yaml` mounts.
A path that is on none of them is the most common way a run fails.
- Every path your script names — `/root/models`, `/root/datasets` — has to be under one of the
mounts, and so does `infra.paths.runsRoot`, where the launcher keeps each run's directory.
- `infra.paths.runsRoot` has to be under a volume every pod shares — a `hostPath` on shared storage
or a read-write-many claim. An `emptyDir` there is rejected: each pod would get its own copy, and
the verdict one pod writes is a file no other pod can read.
- Copying a file into a pod is pointless: pods come and go, the mount survives.
- To run your own branch instead of the image's copy, mount it at the platform-owned source root:
`/root/miles`, `/root/Megatron-LM`, or `/sgl-workspace/sglang`. The chart injects these canonical
roots into `PYTHONPATH`; `infra.env.PYTHONPATH` cannot override them, and a copy mounted anywhere
else is not imported.
- Every path your script names — `/root/models`, `/root/datasets`, `infra.paths.runsRoot` — has to
be under one of those mounts.
- To run your own branch, mount it at `/root/miles`, `/root/Megatron-LM` or `/sgl-workspace/sglang`;
a copy anywhere else is not imported.
## For cluster administrator
### For cluster administrator
Everything above assumes this was done once.
**Install LWS.** Miles deploys its worker pools as
[LeaderWorkerSets](https://github.com/kubernetes-sigs/lws). Install the CRDs and controller, and
grant users rights over them explicitly: LWS ships no aggregation labels, so a namespace `admin`
role does not include them.
grant users rights over them explicitly.
**Give each user a namespace.** The namespace is the real boundary, not the Role: anything that
may create workloads can name another ServiceAccount and read its token. Keep privileged accounts
@@ -112,7 +111,7 @@ infra:
- {mountPath: /root/datasets, subPath: datasets}
- {mountPath: /root/shared_data, subPath: alice/shared_data}
paths:
runsRoot: /cluster-storage/miles_data
runsRoot: /cluster-storage/${NAMESPACE}/miles_data
```
The `hostPath` above stands for a cluster-wide shared filesystem (NFS, Lustre, a CSI mount) already
@@ -120,5 +119,8 @@ mounted at `/cluster-storage` on every node: a per-node directory would lose the
file and the shared checkpoints. Where Pod Security forbids `hostPath`, replace that key with an RWX
`persistentVolumeClaim` and keep the mounts.
Any path in `infra.yaml` — a `hostPath`, a `mountPath`, a `subPath`, `infra.paths.runsRoot` — may
name `${NAMESPACE}`, which the chart replaces with the namespace it is installed into.
`charts/miles-run/values.yaml` shows the full shape, and each chart's `values.schema.json` is the
authoritative field list.
@@ -98,7 +98,7 @@ def execute_train(*, request: ExecuteTrainRequest, config: ExecuteTrainConfig) -
with override_env(env):
specs = compute_specs(args)
chart = chart_dir(repo_base_dir=repo_base_dir)
shared_root = InfraInfo.shared_root(InfraInfo.load(chart, list(config.helm_values)))
shared_root = InfraInfo.shared_root(InfraInfo.load(chart, list(config.helm_values)), namespace=namespace)
run_directory = RunFiles.run_dir(shared_root=shared_root, run_id=run_id)
if config.ci_run:
@@ -1,6 +1,7 @@
from __future__ import annotations
import json
import re
from argparse import Namespace
from pathlib import Path
from typing import Any
@@ -36,6 +37,8 @@ SECTION_OF_CATEGORY = {
_MOONCAKE_COMPONENT = "mooncake-master"
_INFRA_KEY = "infra"
_VALUES_FILE_NAME = "values.yaml"
_NAMESPACE_VARIABLE = "${NAMESPACE}"
_PATH_VARIABLE = re.compile(r"\$\{[^}]*\}")
class MooncakePlan(FrozenStrictBaseModel):
@@ -86,8 +89,8 @@ class MooncakeInfo:
if plan is None:
return train_argv
kwargs = MooncakeInfo.cluster_init_kwargs(plan, host=host)
return ArgvManipulator.set(train_argv, MOONCAKE_INIT_KWARGS_FLAG, json.dumps(kwargs))
rendered = json.dumps(MooncakeInfo.cluster_init_kwargs(plan, host=host))
return ArgvManipulator.set(train_argv, MOONCAKE_INIT_KWARGS_FLAG, rendered)
@staticmethod
def cluster_init_kwargs(plan: MooncakePlan, *, host: str) -> dict[str, Any]:
@@ -112,13 +115,24 @@ class InfraInfo:
return InfraValues.model_validate(_load_helm_values(chart, helm_values_files).get(_INFRA_KEY))
@staticmethod
def shared_root(infra: InfraValues) -> str:
def shared_root(infra: InfraValues, *, namespace: str) -> str:
runs_root = infra.paths.runs_root if infra.paths is not None else None
assert runs_root, (
"infra.paths.runsRoot is unset, so the launcher has no container path to write a run's directory to; "
"set it to an absolute path under one of the infra.volumes mounts"
)
return runs_root.rstrip("/")
return resolve_helm_yaml_entry(runs_root, namespace).rstrip("/")
def resolve_helm_yaml_entry(raw_value: str, namespace: str) -> str:
for reference in _PATH_VARIABLE.findall(raw_value):
if reference != _NAMESPACE_VARIABLE:
raise ValueError(
f"{raw_value} names the unknown variable {reference}: {_NAMESPACE_VARIABLE} is the only variable "
"a path may name, and an unknown one left in place would give every namespace the same literal "
"directory instead of one of its own"
)
return raw_value.replace(_NAMESPACE_VARIABLE, namespace)
def _load_helm_values(chart: str | Path, values_files: list[str] | list[Path]) -> Any:
@@ -2,7 +2,9 @@ import json
from typing import Any
from tests.fast.charts.utils import (
NAMESPACE,
host_path_volume,
pod_spec_of,
render_run,
render_run_error,
requires_helm,
@@ -375,3 +377,111 @@ class TestTheLaunchRecordIsNotAnEnvironmentVariable:
"""run.staticWorkers=[{"name":"router","objectName":"myrun-router","command":["sleep"],"""
""""env":{"MILES_SCRIPT_ENV_REPORT":"hijacked"}}]""",
)
@requires_helm
class TestNamespaceInterpolation:
def test_a_host_path_names_the_namespace_it_is_rendered_for(self):
"""Two agents in two namespaces want two directories on the host, and one values file has to give both."""
objects = render_run(*volumes_args(host_path_volume(path="/data/${NAMESPACE}")))
volumes = pod_spec_of(objects, "StatefulSet", ORCHESTRATOR)["volumes"]
assert {"name": "cluster-storage", "hostPath": {"path": f"/data/{NAMESPACE}", "type": "Directory"}} in volumes
def test_a_mount_path_names_the_namespace(self):
"""A node-local scratch disk is one path on every node, so only the namespace keeps two runs apart."""
container = orchestrator_container(
*volumes_args(
host_path_volume(mounts=[{"mountPath": "/cluster-storage"}, {"mountPath": "/scratch/${NAMESPACE}"}])
)
)
assert {"name": "cluster-storage", "mountPath": f"/scratch/{NAMESPACE}"} in container["volumeMounts"]
def test_a_sub_path_names_the_namespace(self):
"""This is how five agents get five checkouts of the same repo out of one shared volume."""
container = orchestrator_container(
*volumes_args(
host_path_volume(
mounts=[
{"mountPath": "/cluster-storage"},
{"mountPath": "/root/miles", "subPath": "repos/${NAMESPACE}/miles"},
]
)
)
)
assert {
"name": "cluster-storage",
"mountPath": "/root/miles",
"subPath": f"repos/{NAMESPACE}/miles",
} in container["volumeMounts"]
def test_a_path_that_names_no_variable_is_left_exactly_as_written(self):
"""Several releases sharing one directory is a first-class choice, not an escape hatch."""
container = orchestrator_container(*volumes_args(host_path_volume(path="/cluster-storage")))
assert {"name": "cluster-storage", "mountPath": "/cluster-storage"} in container["volumeMounts"]
def test_the_default_runs_root_is_a_directory_this_namespace_has_to_itself(self):
"""Isolation has to be what you get without asking: a default without it makes it one more thing to remember."""
error = render_run_error(*volumes_args(host_path_volume(mounts=[{"mountPath": "/elsewhere"}])))
assert f"/cluster-storage/{NAMESPACE}/miles_data" in error
def test_the_mount_check_compares_resolved_paths(self):
"""Comparing an unresolved runs root against a resolved mount path would refuse one that is in fact mounted."""
container = orchestrator_container(
*volumes_args(host_path_volume(mounts=[{"mountPath": f"/cluster-storage/{NAMESPACE}"}]))
)
assert container["image"]
def test_an_unknown_variable_in_a_host_path_is_refused(self):
"""Left in place it would name one literal directory for every namespace, which is the collision itself."""
error = render_run_error(*volumes_args(host_path_volume(path="/data/${RELEASE}")))
assert "infra.volumes[cluster-storage].hostPath.path" in error
assert "${RELEASE}" in error
def test_an_unknown_variable_in_a_mount_path_is_refused(self):
"""The refusal has to say which mount of which volume, because a values file has many of both."""
error = render_run_error(*volumes_args(host_path_volume(mounts=[{"mountPath": "/cluster-storage/${USER}"}])))
assert "infra.volumes[cluster-storage].mounts[0].mountPath" in error
assert "${USER}" in error
def test_an_unknown_variable_in_a_sub_path_is_refused(self):
"""A misspelt ${NAMESPACE} is the likely typo, and it is the one that silently shares a directory."""
error = render_run_error(
*volumes_args(
host_path_volume(
mounts=[
{"mountPath": "/cluster-storage"},
{"mountPath": "/root/miles", "subPath": "repos/${NAMESPCE}/miles"},
]
)
)
)
assert "infra.volumes[cluster-storage].mounts[1].subPath" in error
assert "${NAMESPCE}" in error
def test_an_unknown_variable_in_the_runs_root_is_refused(self):
"""Every run writes its state and exit file here, so a directory shared by accident is a run lost."""
error = render_run_error("--set", "infra.paths.runsRoot=/cluster-storage/${NAMESPCE}/data")
assert "infra.paths.runsRoot" in error
assert "${NAMESPCE}" in error
def test_an_unknown_variable_is_refused_by_a_render_that_deploys_nothing(self):
"""Every other template of this chart renders for some topology only, so the check needs a home without one."""
error = render_run_error(
"--set-json",
"run.orchestrator.command=[]",
"--set",
"infra.paths.runsRoot=/cluster-storage/${NAMESPCE}/data",
)
assert "infra.paths.runsRoot" in error
assert "${NAMESPCE}" in error
@@ -188,3 +188,34 @@ class TestWorkbenchStatefulSet:
assert "tolerations" not in spec
assert "affinity" not in spec
assert "imagePullSecrets" not in spec
@requires_helm
class TestNamespaceInterpolation:
def test_the_workbench_mounts_what_its_own_namespace_names(self):
"""One workbench per namespace, and the same values file has to give each of them its own checkout."""
objects = render(
*volumes_args(
host_path_volume(
path="/data/${NAMESPACE}",
mounts=[
{"mountPath": "/cluster-storage"},
{"mountPath": "/root/miles", "subPath": "repos/${NAMESPACE}/miles"},
],
)
)
)
assert _volume(objects, "cluster-storage")["hostPath"]["path"] == f"/data/{NAMESPACE}"
assert {
"name": "cluster-storage",
"mountPath": "/root/miles",
"subPath": f"repos/{NAMESPACE}/miles",
} in container(objects)["volumeMounts"]
def test_an_unknown_variable_is_refused_by_a_chart_that_renders_no_run(self):
"""This chart writes the infra.yaml every launch from here reads, so a typo has to stop at this render."""
error = render_error("--set", "infra.paths.runsRoot=/cluster-storage/${NAMESPCE}/data")
assert "infra.paths.runsRoot" in error
assert "${NAMESPCE}" in error
@@ -52,19 +52,35 @@ class TestLaunchPlan:
)
def _resolved(tmp_path, *files: dict) -> str:
def _resolved(tmp_path, *files: dict, namespace: str = "myns") -> str:
paths = []
for index, values in enumerate(files):
path = tmp_path / f"infra-{index}.yaml"
path.write_text(yaml.safe_dump(values))
paths.append(str(path))
return InfraInfo.shared_root(InfraInfo.load(RUN_CHART_DIR, paths))
return InfraInfo.shared_root(InfraInfo.load(RUN_CHART_DIR, paths), namespace=namespace)
class TestSharedRootOf:
def test_falls_back_to_the_chart_defaults_when_no_file_says_otherwise(self, tmp_path):
"""The chart's own values.yaml is the single source of these defaults; Python must not carry a copy."""
assert _resolved(tmp_path, {}) == "/cluster-storage/miles_data"
assert _resolved(tmp_path, {}) == "/cluster-storage/myns/miles_data"
def test_resolves_the_namespace_the_render_resolves(self, tmp_path):
"""The launcher and the render must name one directory: a literal ${NAMESPACE} here would be another."""
values = {"infra": {"paths": {"runsRoot": "/mnt/x/${NAMESPACE}/teamdata"}}}
assert _resolved(tmp_path, values, namespace="tom-other") == "/mnt/x/tom-other/teamdata"
def test_refuses_a_path_that_names_an_unknown_variable(self, tmp_path):
"""A misspelt variable left in place would give every namespace one literal directory to share."""
values = {"infra": {"paths": {"runsRoot": "/mnt/x/${NAMESPCE}/teamdata"}}}
with pytest.raises(ValueError) as error:
_resolved(tmp_path, values)
assert "/mnt/x/${NAMESPCE}/teamdata" in str(error.value)
assert "${NAMESPCE}" in str(error.value)
def test_is_the_container_path_the_values_file_names_and_nothing_derived(self, tmp_path):
"""The runs directory is one path on one mount; deriving it from a volume is what tied it to one disk."""