Merge branch 'main' into feat/runtime-inventory-selection

Author: mchmarny

docs/user/cli-reference.md

@@ -965,8 +965,8 @@ aicr validate [flags]
 | `--image-pull-secret` | | string[] | | Image pull secrets for private registries (repeatable) |
 | `--job-name` | | string | aicr-validate | Name for the validation Job |
 | `--service-account-name` | | string | aicr | ServiceAccount name for validation Job |
-| `--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) |
-| `--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) |
+| `--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) |
+| `--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) |
 | `--timeout` | | duration | 5m | Timeout for validation Job completion |
 | `--no-cleanup` | | bool | false | Skip removal of Job and RBAC resources on completion |
 | `--require-gpu` | | bool | false | Require GPU resources on the validation pod |
@@ -1174,7 +1174,7 @@ spec:
       serviceAccountName: aicr
       nodeSelector:
         my-org/gpu-pool: "true"
-      tolerations:
+      tolerations:                         # [] clears the live snapshot agent's tolerate-all default
         - "gpu-type=h100:NoSchedule"
       requireGpu: true
     execution:
@@ -1211,7 +1211,7 @@ aicr validate --config validate-cluster-a.yaml
 aicr validate --config validate-cluster-b.yaml
 ```
 
-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).
+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).
 
 **Output Structure ([CTRF](https://ctrf.io/) JSON):**
 

docs/user/validation.md

@@ -910,7 +910,9 @@ aicr validate \
   --toleration dedicated=worker-workload:NoExecute
 ```
 
-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.
+These flags affect the inner benchmark pods that run on GPU nodes (NCCL workers, Dynamo workers). The example above supplies `--snapshot`, so it does not launch the live snapshot agent. When `--snapshot` is omitted, the flags also configure that preliminary agent. With no toleration override, the agent tolerates all taints; an explicit `spec.validate.agent.tolerations: []` clears that default. Neither flag affects 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.
 
 ### A check reports `skipped` unexpectedly
 

pkg/cli/validate.go

@@ -135,7 +135,9 @@ func resolveValidateNodeSelector(cmd *cli.Command, resolved *config.ValidateReso
 // inference-perf that want to mirror the target node's taints by default
 // must distinguish "operator opted into tolerate-all" from "operator said
 // nothing". Returning nil here when neither CLI nor config set the field
-// keeps the env var unset, so the inner validator context sees nil.
+// keeps the env var unset, so the inner validator context sees nil. The live
+// snapshot path consumes that same nil as its signal to apply the agent's
+// tolerate-all default at the Job projection boundary.
 func resolveValidateTolerations(cmd *cli.Command, resolved *config.ValidateResolved) ([]corev1.Toleration, error) {
 	if cmd.IsSet("toleration") {
 		tols, err := snapshotter.ParseTolerations(cmd.StringSlice("toleration"))
@@ -465,12 +467,12 @@ func validateCmdFlags() []cli.Flag {
 		},
 		&cli.StringSliceFlag{
 			Name:     "node-selector",
-			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.",
+			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.",
 			Category: catScheduling,
 		},
 		&cli.StringSliceFlag{
 			Name:     "toleration",
-			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.",
+			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.",
 			Category: catScheduling,
 		},
 		&cli.DurationFlag{

pkg/client/v1/types.go

@@ -113,7 +113,8 @@ func (s *Snapshot) Unwrap() *snapshotter.Snapshot {
 // collection Job passed to Client.CollectSnapshot. Facade-owned;
 // field-for-field mirror of pkg/snapshotter.AgentConfig. Tolerations
 // keep k8s.io/api/core/v1.Toleration since kubernetes/api is itself
-// stable.
+// stable. Nil Tolerations use a tolerate-all default; a non-nil empty
+// slice explicitly disables that default.
 //
 // The mirror is enforced, not conventional: TestAgentConfigMirrorsInternal
 // fails when either struct gains, drops, or retypes a field, and every

pkg/snapshotter/agent.go

@@ -68,7 +68,8 @@ type AgentConfig struct {
 	// NodeSelector for targeting specific nodes
 	NodeSelector map[string]string
 
-	// Tolerations for scheduling on tainted nodes
+	// Tolerations for scheduling on tainted nodes. Nil uses
+	// DefaultTolerations; a non-nil empty slice explicitly disables that default.
 	Tolerations []corev1.Toleration
 
 	// Timeout for waiting for Job completion
@@ -147,6 +148,32 @@ type AgentConfig struct {
 	Limits corev1.ResourceList
 }
 
+// buildAgentConfig projects snapshotter configuration onto the deployer's
+// Job configuration. Keep scheduling defaults at this projection boundary so
+// every snapshot-agent caller gets the same nil-versus-empty behavior.
+func buildAgentConfig(config *AgentConfig, agentOutput string) agent.Config {
+	return agent.Config{
+		Namespace:          config.Namespace,
+		ServiceAccountName: config.ServiceAccountName,
+		JobName:            config.JobName,
+		Image:              config.Image,
+		ImagePullSecrets:   config.ImagePullSecrets,
+		NodeSelector:       config.NodeSelector,
+		Tolerations:        effectiveAgentTolerations(config.Tolerations),
+		Output:             agentOutput,
+		Debug:              config.Debug,
+		Privileged:         config.Privileged,
+		RequireGPU:         config.RequireGPU,
+		RuntimeClassName:   config.RuntimeClassName,
+		MaxNodesPerEntry:   config.MaxNodesPerEntry,
+		OS:                 config.OS,
+		ClusterConfigPath:  config.ClusterConfigPath,
+		DiscoverNetwork:    config.DiscoverNetwork,
+		Requests:           config.Requests,
+		Limits:             config.Limits,
+	}
+}
+
 // deployAndWaitForResult handles the common deploy-wait-retrieve lifecycle for an agent Job.
 // It creates the deployer, deploys RBAC and the Job, streams logs, waits for completion,
 // and retrieves the snapshot data from the result ConfigMap.
@@ -169,26 +196,7 @@ func deployAndWaitForResult(ctx context.Context, clientset k8sclient.Interface,
 	// name the injected selector (TOCTOU: node may be cordoned after detection).
 	autoInjectedGPUSelector := maybeInjectGPUNodeSelector(ctx, clientset, config)
 
-	agentConfig := agent.Config{
-		Namespace:          config.Namespace,
-		ServiceAccountName: config.ServiceAccountName,
-		JobName:            config.JobName,
-		Image:              config.Image,
-		ImagePullSecrets:   config.ImagePullSecrets,
-		NodeSelector:       config.NodeSelector,
-		Tolerations:        config.Tolerations,
-		Output:             agentOutput,
-		Debug:              config.Debug,
-		Privileged:         config.Privileged,
-		RequireGPU:         config.RequireGPU,
-		RuntimeClassName:   config.RuntimeClassName,
-		MaxNodesPerEntry:   config.MaxNodesPerEntry,
-		OS:                 config.OS,
-		ClusterConfigPath:  config.ClusterConfigPath,
-		DiscoverNetwork:    config.DiscoverNetwork,
-		Requests:           config.Requests,
-		Limits:             config.Limits,
-	}
+	agentConfig := buildAgentConfig(config, agentOutput)
 
 	deployer := agent.NewDeployer(clientset, agentConfig)
 
@@ -267,18 +275,24 @@ func deployAndWaitForResult(ctx context.Context, clientset k8sclient.Interface,
 			fmt.Fprintln(logWriter(), logs)
 			fmt.Fprintln(logWriter(), "--- end logs ---")
 		}
+		isTransient := errors.IsTransient(waitErr)
 		msg := "job failed"
 		if autoInjectedGPUSelector {
 			msg = "job failed (auto-injected node selector nvidia.com/gpu.present=true — " +
-				"if no GPU nodes are schedulable, target a GPU node explicitly, e.g. " +
-				"--node-selector kubernetes.io/hostname=<gpu-node> " +
-				"(repeat the flag per key=value), or pass --require-gpu to schedule onto " +
-				"a node advertising the nvidia.com/gpu resource)"
+				"verify matching GPU nodes are Ready and schedulable; if tolerations were " +
+				"explicitly cleared or replaced, pass a matching --toleration " +
+				"key=value:effect. To override placement, pass --node-selector " +
+				"kubernetes.io/hostname=<gpu-node>; --require-gpu selects a node " +
+				"advertising the nvidia.com/gpu resource)"
+		} else if isTransient {
+			msg = "job failed (verify target nodes are Ready and schedulable; if " +
+				"tolerations were explicitly cleared or replaced, pass a matching " +
+				"--toleration key=value:effect)"
 		}
 		// A wait that exceeded the deadline (pending pod, image pull, no schedulable
 		// node) is transient and retryable — classify it as ErrCodeTimeout rather
 		// than masking it as a deterministic ErrCodeInternal failure.
-		if errors.IsTransient(waitErr) {
+		if isTransient {
 			return nil, errors.Wrap(errors.ErrCodeTimeout, msg, waitErr)
 		}
 		return nil, errors.Wrap(errors.ErrCodeInternal, msg, waitErr)
@@ -680,6 +694,15 @@ func DefaultTolerations() []corev1.Toleration {
 	}
 }
 
+// effectiveAgentTolerations applies the snapshot agent's scheduling default
+// without collapsing an explicit empty override into that default.
+func effectiveAgentTolerations(tolerations []corev1.Toleration) []corev1.Toleration {
+	if tolerations == nil {
+		return DefaultTolerations()
+	}
+	return tolerations
+}
+
 func validateTaintEffect(effect corev1.TaintEffect) error {
 	switch effect {
 	case corev1.TaintEffectNoSchedule:

pkg/snapshotter/agent_test.go

@@ -18,6 +18,7 @@ import (
 	stderrors "errors"
 	"os"
 	"path/filepath"
+	"reflect"
 	"strings"
 	"testing"
 
@@ -51,6 +52,46 @@ func TestDefaultTolerations(t *testing.T) {
 	}
 }
 
+func TestBuildAgentConfigTolerations(t *testing.T) {
+	explicit := []corev1.Toleration{
+		{
+			Key:      "dedicated",
+			Operator: corev1.TolerationOpEqual,
+			Value:    "gpu-workload",
+			Effect:   corev1.TaintEffectNoSchedule,
+		},
+	}
+	tests := []struct {
+		name  string
+		input []corev1.Toleration
+		want  []corev1.Toleration
+	}{
+		{
+			name: "omitted uses tolerate-all default",
+			want: DefaultTolerations(),
+		},
+		{
+			name:  "explicit empty disables default",
+			input: []corev1.Toleration{},
+			want:  []corev1.Toleration{},
+		},
+		{
+			name:  "explicit tolerations are preserved",
+			input: explicit,
+			want:  explicit,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := buildAgentConfig(&AgentConfig{Tolerations: tt.input}, "snapshot.yaml").Tolerations
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("buildAgentConfig().Tolerations = %#v, want %#v", got, tt.want)
+			}
+		})
+	}
+}
+
 func TestAgentConfig_Defaults(t *testing.T) {
 	// Test that AgentConfig can be instantiated with zero values
 	cfg := AgentConfig{}