Skip to content

Commit 9d8665a

Browse files
committed
docs: correct agent label coverage, name prefixes, and validate flag targets
Four documentation claims did not match the code. Label coverage. pkg/k8s/agent/doc.go and docs/user/agent-deployment.md both asserted that every created object — explicitly counting the staging ConfigMap — carries managed-by=aicr, component=snapshot-agent and aicr.run/run-id. The staging ConfigMap carries none of those: it is written by serializer.ConfigMapWriter, which stamps only app.kubernetes.io/name, app.kubernetes.io/component (the header kind) and app.kubernetes.io/version. An earlier review decided not to add the labels at the serializer, because that writer also produces the user's delivered cm:// artifact and stamping the sweep key on a never-delete object is a hazard — so the documentation is corrected and the serializer is left alone. The agent-deployment troubleshooting recipe now addresses the staging ConfigMap by its run-scoped name. Shipped config examples. ADR-020 decision 8 requires examples pinning jobName/serviceAccountName to be updated; four blocks still pinned them (two in cli-config.md, two in cli-reference.md), and one contradicted a neighbouring row this branch already updated — pinning serviceAccountName: aicr for validate while the reference gives the default as aicr-validate. The pins are dropped and both agent.* schema rows now state the fields are optional prefixes with the run ID appended. NameBase godoc. The pkg/client/v1 copy repeated the claim that NameBase "has no effect once either of those is set". The fallback is per name. Validate flag targets. The aicr validate rows for --job-name / --service-account-name attributed them to "the validation Job" and its ServiceAccount. Both feed only the optional live snapshot-capture agent: the validator Jobs are named aicr-<validator>-<hash> and their ServiceAccount aicr-validator-<run-id>, neither influenced by these flags, and both flags are inert when --snapshot is supplied. Refs: ADR-020 Signed-off-by: Alex Yuskauskas <ayuskauskas@nvidia.com>
1 parent fc98c17 commit 9d8665a

5 files changed

Lines changed: 61 additions & 27 deletions

File tree

docs/user/agent-deployment.md

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -333,8 +333,12 @@ Check RBAC permissions. The ServiceAccount name is run-scoped (`aicr-<run-id>`),
333333
Select it by run ID, not by position: with concurrent snapshot runs the namespace
334334
holds one ServiceAccount per run, and `.items[0]` would pick an arbitrary one — so
335335
the checks below could report on a healthy run while you are debugging a failed one.
336-
The CLI logs the run ID when it starts (`snapshot agent run: runID=...`), and every
337-
resource the run created carries it as the `aicr.run/run-id` label:
336+
The CLI logs the run ID when it starts (`snapshot agent run: runID=...`), and the
337+
Job, its pods, and its RBAC resources all carry it as the `aicr.run/run-id`
338+
label. (The staging ConfigMap is the exception — it is written from inside the
339+
pod and carries only `app.kubernetes.io/name`, `app.kubernetes.io/component` and
340+
`app.kubernetes.io/version`; find it by its run-scoped name instead, see
341+
[Job Completes but No Output](#job-completes-but-no-output).)
338342

339343
```shell
340344
# From the failing Job (or use the runID the CLI logged at start).
@@ -381,10 +385,18 @@ Check ConfigMap and container logs:
381385
# "-o cm://<namespace>/<name>", the agent stages its result in a run-scoped
382386
# ConfigMap named "aicr-agent-snapshot-<run-id>" that cleanup deletes when
383387
# the run ends — pass --no-cleanup to keep it around for inspection.
384-
kubectl get configmap -n gpu-operator -l app.kubernetes.io/name=aicr
388+
#
389+
# This object is written by the in-pod agent, not by the CLI, so it does NOT
390+
# carry the aicr.run/run-id label the Job and RBAC resources do. Address it by
391+
# name: with concurrent runs, the label selector below matches every run's
392+
# staging ConfigMap plus any delivered cm:// artifact in the namespace.
393+
kubectl get configmap -n gpu-operator aicr-agent-snapshot-$RUN_ID
385394
386395
# View ConfigMap contents
387-
kubectl get configmap -n gpu-operator -l app.kubernetes.io/name=aicr -o yaml
396+
kubectl get configmap -n gpu-operator aicr-agent-snapshot-$RUN_ID -o yaml
397+
398+
# Or list every aicr-written ConfigMap in the namespace (all runs)
399+
kubectl get configmap -n gpu-operator -l app.kubernetes.io/name=aicr
388400
389401
# View pod logs for errors
390402
kubectl logs -n gpu-operator -l app.kubernetes.io/name=aicr,app.kubernetes.io/component=snapshot-agent

docs/user/cli-config.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@ spec:
8888
namespace: aicr-validation
8989
image: "" # default: ghcr.io/nvidia/aicr:latest
9090
imagePullSecrets: []
91-
jobName: aicr
92-
serviceAccountName: aicr
91+
# jobName / serviceAccountName are optional PREFIXES, not names — the
92+
# run ID is always appended. Omit them to take the defaults.
9393
nodeSelector:
9494
nodeGroup: gpu-worker
9595
tolerations:
@@ -170,8 +170,8 @@ spec:
170170
namespace: aicr-validation
171171
image: ""
172172
imagePullSecrets: []
173-
jobName: aicr
174-
serviceAccountName: aicr
173+
# Optional prefixes; omitted here so the defaults apply
174+
# (jobName: aicr-validate, serviceAccountName: aicr).
175175
nodeSelector:
176176
nodeGroup: gpu-worker
177177
tolerations:
@@ -218,7 +218,7 @@ produced from the live cluster.
218218
| `output.path` | string | Output file path (same as `-o`) |
219219
| `output.format` | string | `yaml` \| `json` \| `table` |
220220
| `output.template` | string | Optional Go template path |
221-
| `agent.*` | object | In-cluster capture Job pod: `namespace`, `image`, `imagePullSecrets`, `jobName`, `serviceAccountName`, `nodeSelector`, `tolerations`, `requireGpu`, `runtimeClassName` (mutually exclusive with `requireGpu`), `os`, `requests`, `limits`. Mirrors `spec.validate.agent` so one file pins matching placement for both |
221+
| `agent.*` | object | In-cluster capture Job pod: `namespace`, `image`, `imagePullSecrets`, `jobName`, `serviceAccountName`, `nodeSelector`, `tolerations`, `requireGpu`, `runtimeClassName` (mutually exclusive with `requireGpu`), `os`, `requests`, `limits`. `jobName` and `serviceAccountName` are optional **prefixes**, not exact names — the run ID is appended (`<prefix>-<run-id>`), so omit them unless you need a custom prefix. Mirrors `spec.validate.agent` so one file pins matching placement for both |
222222
| `execution.timeout` | duration string | e.g. `5m` |
223223
| `execution.noCleanup` | bool | Keep the capture Job after completion |
224224
| `execution.privileged` | bool (tri-state) | Set `false` for PSS-restricted namespaces |
@@ -278,7 +278,7 @@ Inputs to `aicr validate`.
278278
| Field | Type | Notes |
279279
|-------|------|-------|
280280
| `input.recipe` / `.snapshot` | string | Recipe + snapshot to validate |
281-
| `agent.*` | object | In-cluster validation Job pod; same fields and nil-vs-empty semantics as `spec.snapshot.agent` (minus `runtimeClassName`/`os`/`requests`/`limits`) |
281+
| `agent.*` | object | The **live snapshot-capture** Job pod `aicr validate` deploys when `input.snapshot` is empty; same fields and nil-vs-empty semantics as `spec.snapshot.agent` (minus `runtimeClassName`/`os`/`requests`/`limits`). `jobName` and `serviceAccountName` are optional prefixes with the run ID appended (defaults: both `aicr-validate`); they do not name the validator Jobs |
282282
| `execution.phases` | []string | e.g. `[deployment, conformance, performance]` |
283283
| `execution.failOnError` | bool (tri-state) | Absent = CLI default (`true`); explicit `false` opts out |
284284
| `execution.failFast` | bool (tri-state) | Stop after the first failed phase |

docs/user/cli-reference.md

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,8 @@ spec:
201201
namespace: aicr-validation
202202
image: "" # default: ghcr.io/nvidia/aicr:latest
203203
imagePullSecrets: []
204-
jobName: aicr
205-
serviceAccountName: aicr
204+
# jobName / serviceAccountName are optional PREFIXES, not names — the
205+
# run ID is always appended. Omit them to take the defaults.
206206
nodeSelector:
207207
nodeGroup: gpu-worker
208208
tolerations:
@@ -963,8 +963,8 @@ aicr validate [flags]
963963
| `--namespace` | `-n` | string | aicr-validation | Kubernetes namespace for validation Job deployment |
964964
| `--image` | | string | ghcr.io/nvidia/aicr:latest | Container image for validation Job |
965965
| `--image-pull-secret` | | string[] | | Image pull secrets for private registries (repeatable) |
966-
| `--job-name` | | string | aicr-validate | Prefix for the validation Job name; the run ID is always appended (`<prefix>-<run-id>`) |
967-
| `--service-account-name` | | string | aicr-validate | Prefix for the validation Job's ServiceAccount name; the run ID is always appended (`<prefix>-<run-id>`) |
966+
| `--job-name` | | string | aicr-validate | Prefix for the **live snapshot-capture agent's** Job name; the run ID is always appended (`<prefix>-<run-id>`). Inert when `--snapshot` is supplied — no agent is deployed. Does not name the validator Jobs (`aicr-<validator>-<hash>`) |
967+
| `--service-account-name` | | string | aicr-validate | Prefix for the **live snapshot-capture agent's** ServiceAccount, Role, and RoleBinding; the run ID is always appended (`<prefix>-<run-id>`). Inert when `--snapshot` is supplied. Does not name the validator Jobs' ServiceAccount (`aicr-validator-<run-id>`) |
968968
| `--node-selector` | | string[] | | Override GPU node selection for the live snapshot agent (when `--snapshot` is omitted) and inner validation workloads. Replaces platform-specific selectors (e.g., `cloud.google.com/gke-accelerator`, `node.kubernetes.io/instance-type`) on inner workloads like NCCL benchmark pods. Use when GPU nodes have non-standard labels. Does not affect the validator orchestrator Job. (format: key=value, repeatable) |
969969
| `--toleration` | | string[] | | Override tolerations for the live snapshot agent (when `--snapshot` is omitted) and inner validation workloads. When omitted, the snapshot agent tolerates all taints. Does not affect the validator orchestrator Job. (format: key=value:effect, repeatable) |
970970
| `--timeout` | | duration | 5m | Timeout for validation Job completion |
@@ -1170,8 +1170,9 @@ spec:
11701170
namespace: aicr-validation
11711171
image: ghcr.io/nvidia/aicr:v0.19.0
11721172
imagePullSecrets: [registry-secret]
1173-
jobName: aicr-validate
1174-
serviceAccountName: aicr
1173+
# Optional prefixes for the live-capture agent, not names — the run ID
1174+
# is always appended. Omitted here so the defaults apply
1175+
# (both aicr-validate).
11751176
nodeSelector:
11761177
my-org/gpu-pool: "true"
11771178
tolerations: # [] clears the live snapshot agent's tolerate-all default

pkg/client/v1/types.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -186,9 +186,13 @@ type AgentConfig struct {
186186
// snapshot agent and its validator Jobs the same RunID.
187187
RunID string
188188

189-
// NameBase prefixes generated Job/ServiceAccount/RBAC names when
190-
// JobName and ServiceAccountName are left empty; it has no effect
191-
// once either of those is set. Defaults to "aicr" when also empty.
189+
// NameBase prefixes generated Job/ServiceAccount/RBAC names. The
190+
// fallback is per name, not all-or-nothing: the Job uses JobName when
191+
// set and NameBase otherwise, while the ServiceAccount, Role and
192+
// RoleBinding use ServiceAccountName when set and NameBase otherwise.
193+
// Setting only one of the two therefore leaves NameBase governing the
194+
// other. Defaults to "aicr" when also empty.
195+
//
192196
// JobName and ServiceAccountName themselves are optional prefixes,
193197
// not required names — RunID is appended to whichever prefix
194198
// applies, so the deployed object names are always run-scoped.

pkg/k8s/agent/doc.go

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,25 +46,42 @@ Two objects are deliberately NOT run-scoped:
4646
artifact. It is written on purpose and never deleted
4747
(Config.OwnsOutputConfigMap is false for it).
4848
49-
Every created object carries app.kubernetes.io/name=aicr,
50-
app.kubernetes.io/managed-by=aicr,
49+
Every object this package itself creates — the Job, ServiceAccount, Role,
50+
RoleBinding, ClusterRole, and ClusterRoleBinding — carries
51+
app.kubernetes.io/name=aicr, app.kubernetes.io/managed-by=aicr,
5152
app.kubernetes.io/component=snapshot-agent, and aicr.run/run-id=<RunID>, on the
5253
Job's pod template as well as the Job itself. Select agent pods across runs
5354
with the component label; the Job name changes every run.
5455
56+
The staging ConfigMap is the exception: it is written from inside the pod by
57+
pkg/serializer's ConfigMap writer, which stamps app.kubernetes.io/name=aicr,
58+
app.kubernetes.io/component=<snapshot kind> and app.kubernetes.io/version — not
59+
managed-by and not the run-ID label. That writer also produces the user's
60+
delivered cm:// artifact, so it deliberately does not stamp the run-ID sweep
61+
key onto an object this package must never delete. Run scoping for the staging
62+
ConfigMap comes from its name (see StagingConfigMapName), which is what both
63+
Cleanup paths key on.
64+
5565
Job and Pod lifecycle waits use the Kubernetes watch API (not polling) for
5666
efficiency. Pod selection narrows by label and then authorizes the candidate
5767
against the controlling ownerReference carrying the recorded Job UID, since pod
5868
labels are writable by anything that can update pods in the namespace.
5969
6070
# Cleanup
6171
62-
The Deployer records (kind, name, UID) for each object it successfully creates.
63-
Cleanup deletes exactly that set, passing the recorded UID as a
64-
metav1.Preconditions so a same-named object belonging to another run is never
65-
collected; a UID mismatch (Conflict) and a NotFound are both treated as
66-
success. Cleanup also runs on the Deploy failure path, which is why it is
67-
scoped to what was created rather than to configured names.
72+
The Deployer records (kind, name) immediately before each Create and writes the
73+
returned UID onto that entry on success. Cleanup deletes exactly that set,
74+
passing the recorded UID as a metav1.Preconditions so a same-named object
75+
belonging to another run is never collected; a UID mismatch (Conflict) and a
76+
NotFound are both treated as success. Cleanup also runs on the Deploy failure
77+
path, which is why it is scoped to what was created rather than to configured
78+
names.
79+
80+
Recording before the Create is what keeps a lost Create response from orphaning
81+
an object forever: if the apiserver commits the create but the response never
82+
arrives, the entry is already in the set and Cleanup deletes it by its
83+
(run-unique) name with no UID precondition. The one response that proves the
84+
object is not ours — AlreadyExists — discards the entry again.
6885
6986
The staging ConfigMap is written by the in-pod agent, so its UID is observed
7087
when GetSnapshot reads it. When the run owns that ConfigMap and failed before

0 commit comments

Comments
 (0)