Skip to content

Commit fa8330d

Browse files
committed
fix(snapshotter): default live agent tolerations
Signed-off-by: Yuan Chen <yuanchen97@gmail.com>
1 parent 90a7f41 commit fa8330d

6 files changed

Lines changed: 68 additions & 13 deletions

File tree

docs/user/cli-reference.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -963,8 +963,8 @@ aicr validate [flags]
963963
| `--image-pull-secret` | | string[] | | Image pull secrets for private registries (repeatable) |
964964
| `--job-name` | | string | aicr-validate | Name for the validation Job |
965965
| `--service-account-name` | | string | aicr | ServiceAccount name for validation Job |
966-
| `--node-selector` | | string[] | | Override GPU node selection for 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) |
967-
| `--toleration` | | string[] | | Override tolerations for validation workloads. Replaces the default tolerate-all policy on inner workloads like NCCL benchmark pods and conformance test pods. Does not affect the validator orchestrator Job. (format: key=value:effect, repeatable) |
966+
| `--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) |
967+
| `--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) |
968968
| `--timeout` | | duration | 5m | Timeout for validation Job completion |
969969
| `--no-cleanup` | | bool | false | Skip removal of Job and RBAC resources on completion |
970970
| `--require-gpu` | | bool | false | Require GPU resources on the validation pod |
@@ -1209,7 +1209,7 @@ aicr validate --config validate-cluster-a.yaml
12091209
aicr validate --config validate-cluster-b.yaml
12101210
```
12111211

1212-
The `--node-selector` and `--toleration` flags control scheduling for the inner validation workloads (NCCL benchmark workers, conformance test pods), not the validator orchestrator Job. For when to use them with non-standard GPU labels or taints, see [Validation](validation.md#non-standard-gpu-labels-or-taints).
1212+
The `--node-selector` and `--toleration` flags control scheduling for the inner validation workloads (NCCL benchmark workers, conformance test pods). When `--snapshot` is omitted, they also configure the preliminary live snapshot agent. They do not configure the validator orchestrator Job. For when to use them with non-standard GPU labels or taints, see [Validation](validation.md#non-standard-gpu-labels-or-taints).
12131213

12141214
**Output Structure ([CTRF](https://ctrf.io/) JSON):**
12151215

docs/user/validation.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -910,7 +910,9 @@ aicr validate \
910910
--toleration dedicated=worker-workload:NoExecute
911911
```
912912

913-
These flags affect the inner benchmark pods that run on GPU nodes (NCCL workers, Dynamo workers), not the validator orchestrator Job itself. For `inference-perf` specifically, `--node-selector` narrows the pool of candidate GPU nodes — the validator then picks the candidate with the most free GPUs (subtracting same-ledger occupancy only — DRA allocations from DRA capacity, device-plugin requests from device-plugin capacity — and skipping DRA candidates that carry scalar `nvidia.com/gpu` workloads) and pins all Dynamo Frontend + worker pods to that node via `kubernetes.io/hostname`. The AIPerf benchmark runner pod is CPU-only, uses a tolerate-all / no-nodeSelector pod spec, and is unaffected by these flags.
913+
These flags affect the inner benchmark pods that run on GPU nodes (NCCL workers, Dynamo workers). When `--snapshot` is omitted, they also configure the preliminary live snapshot agent. With no toleration override, that agent tolerates all taints; an explicit `spec.validate.agent.tolerations: []` clears that default. Neither flag affects the validator orchestrator Job itself.
914+
915+
For `inference-perf` specifically, `--node-selector` narrows the pool of candidate GPU nodes — the validator then picks the candidate with the most free GPUs (subtracting same-ledger occupancy only — DRA allocations from DRA capacity, device-plugin requests from device-plugin capacity — and skipping DRA candidates that carry scalar `nvidia.com/gpu` workloads) and pins all Dynamo Frontend + worker pods to that node via `kubernetes.io/hostname`. The AIPerf benchmark runner pod is CPU-only, uses a tolerate-all / no-nodeSelector pod spec, and is unaffected by these flags.
914916

915917
### A check reports `skipped` unexpectedly
916918

pkg/cli/validate.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -465,12 +465,12 @@ func validateCmdFlags() []cli.Flag {
465465
},
466466
&cli.StringSliceFlag{
467467
Name: "node-selector",
468-
Usage: "Override GPU node selection for validation workloads (format: key=value, can be repeated). Replaces platform-specific selectors on inner workloads (e.g., NCCL benchmark pods). Use when GPU nodes have non-standard labels. Does not affect the validator orchestrator Job.",
468+
Usage: "Override GPU node selection for the live snapshot agent (when --snapshot is omitted) and inner validation workloads (format: key=value, can be repeated). Replaces platform-specific selectors on inner workloads (e.g., NCCL benchmark pods). Does not affect the validator orchestrator Job.",
469469
Category: catScheduling,
470470
},
471471
&cli.StringSliceFlag{
472472
Name: "toleration",
473-
Usage: "Override tolerations for validation workloads (format: key=value:effect, can be repeated). Replaces the default tolerate-all policy on inner workloads. Does not affect the validator orchestrator Job.",
473+
Usage: "Override tolerations for the live snapshot agent (when --snapshot is omitted) and inner validation workloads (format: key=value:effect, can be repeated). When omitted, the snapshot agent tolerates all taints. Does not affect the validator orchestrator Job.",
474474
Category: catScheduling,
475475
},
476476
&cli.DurationFlag{

pkg/client/v1/types.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,8 @@ func (s *Snapshot) Unwrap() *snapshotter.Snapshot {
113113
// collection Job passed to Client.CollectSnapshot. Facade-owned;
114114
// field-for-field mirror of pkg/snapshotter.AgentConfig. Tolerations
115115
// keep k8s.io/api/core/v1.Toleration since kubernetes/api is itself
116-
// stable.
116+
// stable. Nil Tolerations use a tolerate-all default; a non-nil empty
117+
// slice explicitly disables that default.
117118
//
118119
// The mirror is enforced, not conventional: TestAgentConfigMirrorsInternal
119120
// fails when either struct gains, drops, or retypes a field, and every

pkg/snapshotter/agent.go

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ type AgentConfig struct {
6868
// NodeSelector for targeting specific nodes
6969
NodeSelector map[string]string
7070

71-
// Tolerations for scheduling on tainted nodes
71+
// Tolerations for scheduling on tainted nodes. Nil uses
72+
// DefaultTolerations; a non-nil empty slice explicitly disables that default.
7273
Tolerations []corev1.Toleration
7374

7475
// Timeout for waiting for Job completion
@@ -176,7 +177,7 @@ func deployAndWaitForResult(ctx context.Context, clientset k8sclient.Interface,
176177
Image: config.Image,
177178
ImagePullSecrets: config.ImagePullSecrets,
178179
NodeSelector: config.NodeSelector,
179-
Tolerations: config.Tolerations,
180+
Tolerations: effectiveAgentTolerations(config.Tolerations),
180181
Output: agentOutput,
181182
Debug: config.Debug,
182183
Privileged: config.Privileged,
@@ -270,10 +271,11 @@ func deployAndWaitForResult(ctx context.Context, clientset k8sclient.Interface,
270271
msg := "job failed"
271272
if autoInjectedGPUSelector {
272273
msg = "job failed (auto-injected node selector nvidia.com/gpu.present=true — " +
273-
"if no GPU nodes are schedulable, target a GPU node explicitly, e.g. " +
274-
"--node-selector kubernetes.io/hostname=<gpu-node> " +
275-
"(repeat the flag per key=value), or pass --require-gpu to schedule onto " +
276-
"a node advertising the nvidia.com/gpu resource)"
274+
"verify matching GPU nodes are Ready and schedulable; if tolerations were " +
275+
"explicitly cleared or replaced, pass a matching --toleration " +
276+
"key=value:effect. To override placement, pass --node-selector " +
277+
"kubernetes.io/hostname=<gpu-node>; --require-gpu selects a node " +
278+
"advertising the nvidia.com/gpu resource)"
277279
}
278280
// A wait that exceeded the deadline (pending pod, image pull, no schedulable
279281
// node) is transient and retryable — classify it as ErrCodeTimeout rather
@@ -680,6 +682,15 @@ func DefaultTolerations() []corev1.Toleration {
680682
}
681683
}
682684

685+
// effectiveAgentTolerations applies the snapshot agent's scheduling default
686+
// without collapsing an explicit empty override into that default.
687+
func effectiveAgentTolerations(tolerations []corev1.Toleration) []corev1.Toleration {
688+
if tolerations == nil {
689+
return DefaultTolerations()
690+
}
691+
return tolerations
692+
}
693+
683694
func validateTaintEffect(effect corev1.TaintEffect) error {
684695
switch effect {
685696
case corev1.TaintEffectNoSchedule:

pkg/snapshotter/agent_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
stderrors "errors"
1919
"os"
2020
"path/filepath"
21+
"reflect"
2122
"strings"
2223
"testing"
2324

@@ -51,6 +52,46 @@ func TestDefaultTolerations(t *testing.T) {
5152
}
5253
}
5354

55+
func TestEffectiveAgentTolerations(t *testing.T) {
56+
explicit := []corev1.Toleration{
57+
{
58+
Key: "dedicated",
59+
Operator: corev1.TolerationOpEqual,
60+
Value: "gpu-workload",
61+
Effect: corev1.TaintEffectNoSchedule,
62+
},
63+
}
64+
tests := []struct {
65+
name string
66+
input []corev1.Toleration
67+
want []corev1.Toleration
68+
}{
69+
{
70+
name: "omitted uses tolerate-all default",
71+
want: DefaultTolerations(),
72+
},
73+
{
74+
name: "explicit empty disables default",
75+
input: []corev1.Toleration{},
76+
want: []corev1.Toleration{},
77+
},
78+
{
79+
name: "explicit tolerations are preserved",
80+
input: explicit,
81+
want: explicit,
82+
},
83+
}
84+
85+
for _, tt := range tests {
86+
t.Run(tt.name, func(t *testing.T) {
87+
got := effectiveAgentTolerations(tt.input)
88+
if !reflect.DeepEqual(got, tt.want) {
89+
t.Errorf("effectiveAgentTolerations() = %#v, want %#v", got, tt.want)
90+
}
91+
})
92+
}
93+
}
94+
5495
func TestAgentConfig_Defaults(t *testing.T) {
5596
// Test that AgentConfig can be instantiated with zero values
5697
cfg := AgentConfig{}

0 commit comments

Comments
 (0)