fix(ci): make GPU training and inference symmetric#1
Closed
yuanchen8911 wants to merge 1 commit into
Closed
Conversation
|
Welcome to AICR, @yuanchen8911! Thanks for your first pull request. Before review, please ensure:
A maintainer will review this soon. |
yuanchen8911
added a commit
that referenced
this pull request
May 14, 2026
…outing kgateway v2.2 removed InferencePool routing from its Envoy data plane (PR kgateway-dev/kgateway#12689, deprecated in v2.1). The Gateway API Inference Extension support that AICR uses for CNCF AI Conformance Advanced Ingress moved entirely to the separate agentgateway project, which ships its own Helm charts, GatewayClass, controller, and AgentgatewayParameters CRD. AICR's only kgateway consumer was the inference Gateway resource — zero HTTPRoutes, no TrafficPolicy/BackendConfigPolicy/etc. — so this PR replaces kgateway entirely rather than running both side-by-side. Changes: - registry + mixin: kgateway/kgateway-crds (v2.0.0, cr.kgateway.dev) -> agentgateway/agentgateway-crds (v2.2.1, cr.agentgateway.dev) - inference-gateway.yaml: GatewayParameters (gateway.kgateway.dev) -> AgentgatewayParameters (agentgateway.dev/v1alpha1, strategic-merge patch on Deployment spec); gatewayClassName: kgateway -> agentgateway; namespace kgateway-system -> agentgateway-system - health check + component dirs renamed under git mv to preserve history - conformance validator (validators/conformance/inference_gateway_check.go) updated to gate on `agentgateway` component and query the new GatewayClass / namespace / Deployment names - evidence collector script + requirement title updated - GPU H100 inference workflow path filter follows the renamed component directories so PRs touching only agentgateway* still trigger CI - Go test fixtures, undeploy.sh.tmpl + golden files, chainsaw assertions, UAT recipe snapshots regenerated against the new component graph - docs/user (component-catalog, container-images, api-reference, cli-reference) refreshed; container-images.md regenerated via `make bom-docs` - demos/cuj2-* + demos/query.md + demos/images/meta.md updated so kubectl commands and architecture diagrams match the new namespace and component names Conformance-equivalence: AICR's `ai_inference` requirement is data-plane agnostic — it asserts a GatewayClass is Accepted, a Gateway is Programmed, and the InferencePool CRDs exist. Swapping to agentgateway preserves all five evidence checks; only the names in the captured output change. Historical evidence snapshots under docs/conformance/cncf/v1.35/nim-eks/, demos/examples/CUJ2-Test-Report.md, design doc 005, and CHANGELOG entries are intentionally left referencing kgateway as frozen records. Closes follow-up #1 from issue NVIDIA#698. Validation: - make qualify (Go unit tests, golangci-lint, yamllint, 20/20 chainsaw including cli-bundle-agentgateway-templates, vulnerability scan, license headers): all green - Bundle generation verified end-to-end (recipe -> bundle -> deploy.sh layout under /tmp); rendered post chart confirmed to emit AgentgatewayParameters + Gateway with class agentgateway in namespace agentgateway-system - validators/conformance package tests pass (22.6s)
yuanchen8911
added a commit
that referenced
this pull request
Jun 9, 2026
Deployment-phase health-check validation regressed in the recent in-process executor + deepened-check stack (NVIDIA#1235/NVIDIA#1245/NVIDIA#1252): 1. inprocess.go: runAssertWithRetry/runErrorWithRetry retried ANY non-nil error, so a permanent JMESPath evaluation error was retried for the entire assert window (6m) instead of failing fast like the old chainsaw binary did. Classify assertion-engine errors as ErrCodeInvalidRequest (terminal) and short-circuit the retry loops on them. Adds a regression test asserting terminal eval errors fail fast (< AssertRetryInterval) rather than retrying to deadline. 2. recipes/checks/*: the (init)containerStatuses[?...] | length(@) projection throws 'invalid type for: <nil>' on any pod without (init) containers (the common case), feeding defect #1. Coalesce to an empty array across all 22 affected component health checks. 3. recipes/checks/dynamo-platform: validate-deployment-exists asserted a Deployment named 'dynamo-operator', but the chart prefixes it with the release name (dynamo-platform-dynamo-operator-controller-manager), so the assert never matched and retried to the deadline. Match by the stable app.kubernetes.io/name=dynamo-operator label instead. Verified on an EKS H100 cluster: deployment phase goes from an 8m timeout (status=other) to PASSED in ~21s; full deployment 4/4 and conformance 11/11 green.
yuanchen8911
added a commit
that referenced
this pull request
Jun 9, 2026
Deployment-phase health-check validation regressed in the recent in-process executor + deepened-check stack (NVIDIA#1235/NVIDIA#1245/NVIDIA#1252): 1. inprocess.go: runAssertWithRetry/runErrorWithRetry retried ANY non-nil error, so a permanent JMESPath evaluation error was retried for the entire assert window (6m) instead of failing fast like the old chainsaw binary did. Classify assertion-engine errors as ErrCodeInvalidRequest (terminal) and short-circuit the retry loops on them. Adds a regression test asserting terminal eval errors fail fast (< AssertRetryInterval) rather than retrying to deadline. 2. recipes/checks/*: the (init)containerStatuses[?...] | length(@) projection throws 'invalid type for: <nil>' on any pod without (init) containers (the common case), feeding defect #1. Coalesce to an empty array across all 22 affected component health checks. 3. recipes/checks/dynamo-platform: validate-deployment-exists asserted a Deployment named 'dynamo-operator', but the chart prefixes it with the release name (dynamo-platform-dynamo-operator-controller-manager), so the assert never matched and retried to the deadline. Match by the stable app.kubernetes.io/name=dynamo-operator label instead. Verified on an EKS H100 cluster: deployment phase goes from an 8m timeout (status=other) to PASSED in ~21s; full deployment 4/4 and conformance 11/11 green.
yuanchen8911
added a commit
that referenced
this pull request
Jun 9, 2026
Deployment-phase health-check validation regressed in the recent in-process executor + deepened-check stack (NVIDIA#1235/NVIDIA#1245/NVIDIA#1252): 1. inprocess.go: runAssertWithRetry/runErrorWithRetry retried ANY non-nil error, so a permanent JMESPath evaluation error was retried for the entire assert window (6m) instead of failing fast like the old chainsaw binary did. Classify assertion-engine errors as ErrCodeInvalidRequest (terminal) and short-circuit the retry loops on them. Adds a regression test asserting terminal eval errors fail fast (< AssertRetryInterval) rather than retrying to deadline. 2. recipes/checks/*: the (init)containerStatuses[?...] | length(@) projection throws 'invalid type for: <nil>' on any pod without (init) containers (the common case), feeding defect #1. Coalesce to an empty array across all 22 affected component health checks. 3. recipes/checks/dynamo-platform: validate-deployment-exists asserted a Deployment named 'dynamo-operator', but the chart prefixes it with the release name (dynamo-platform-dynamo-operator-controller-manager), so the assert never matched and retried to the deadline. Match by the stable app.kubernetes.io/name=dynamo-operator label instead. Verified on an EKS H100 cluster: deployment phase goes from an 8m timeout (status=other) to PASSED in ~21s; full deployment 4/4 and conformance 11/11 green.
yuanchen8911
added a commit
that referenced
this pull request
Jun 9, 2026
Deployment-phase health-check validation regressed in the recent in-process executor + deepened-check stack (NVIDIA#1235/NVIDIA#1245/NVIDIA#1252): 1. inprocess.go: runAssertWithRetry/runErrorWithRetry retried ANY non-nil error, so a permanent JMESPath evaluation error was retried for the entire assert window (6m) instead of failing fast like the old chainsaw binary did. Classify assertion-engine errors as ErrCodeInvalidRequest (terminal) and short-circuit the retry loops on them. Adds a regression test asserting terminal eval errors fail fast (< AssertRetryInterval) rather than retrying to deadline. 2. recipes/checks/*: the (init)containerStatuses[?...] | length(@) projection throws 'invalid type for: <nil>' on any pod without (init) containers (the common case), feeding defect #1. Coalesce to an empty array across all 22 affected component health checks. 3. recipes/checks/dynamo-platform: validate-deployment-exists asserted a Deployment named 'dynamo-operator', but the chart prefixes it with the release name (dynamo-platform-dynamo-operator-controller-manager), so the assert never matched and retried to the deadline. Match by the stable app.kubernetes.io/name=dynamo-operator label instead. Verified on an EKS H100 cluster: deployment phase goes from an 8m timeout (status=other) to PASSED in ~21s; full deployment 4/4 and conformance 11/11 green.
yuanchen8911
added a commit
that referenced
this pull request
Jun 9, 2026
Deployment-phase health-check validation regressed in the recent in-process executor + deepened-check stack (NVIDIA#1235/NVIDIA#1245/NVIDIA#1252): 1. inprocess.go: runAssertWithRetry/runErrorWithRetry retried ANY non-nil error, so a permanent JMESPath evaluation error was retried for the entire assert window (6m) instead of failing fast like the old chainsaw binary did. Classify assertion-engine errors as ErrCodeInvalidRequest (terminal) and short-circuit the retry loops on them. Adds a regression test asserting terminal eval errors fail fast (< AssertRetryInterval) rather than retrying to deadline. 2. recipes/checks/*: the (init)containerStatuses[?...] | length(@) projection throws 'invalid type for: <nil>' on any pod without (init) containers (the common case), feeding defect #1. Coalesce to an empty array across all 22 affected component health checks. 3. recipes/checks/dynamo-platform: validate-deployment-exists asserted a Deployment named 'dynamo-operator', but the chart prefixes it with the release name (dynamo-platform-dynamo-operator-controller-manager), so the assert never matched and retried to the deadline. Match by the stable app.kubernetes.io/name=dynamo-operator label instead. Verified on an EKS H100 cluster: deployment phase goes from an 8m timeout (status=other) to PASSED in ~21s; full deployment 4/4 and conformance 11/11 green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Dependency: This PR is intentionally stacked on top of NVIDIA#577 and should be reviewed/merged after NVIDIA#577.
Make the H100 training and inference GPU workflows symmetric by moving training to a kind+Kubeflow recipe, moving inference to H100 x2, and aligning their shared conformance coverage while keeping the inference smoke-test tail.
Motivation / Context
This is the follow-up to the GPU conformance dedup work in NVIDIA#577.
It removes the remaining drift between training and inference GPU CI so both workflows exercise the same core conformance checks, with only controller/gateway coverage and the inference smoke test differing.
Fixes: N/A
Related: NVIDIA#554, NVIDIA#577
Type of Change
Component(s) Affected
cmd/aicr,pkg/cli)cmd/aicrd,pkg/api,pkg/server)pkg/recipe)pkg/bundler,pkg/component/*)pkg/collector,pkg/snapshotter)pkg/validator)pkg/errors,pkg/k8s)docs/,examples/)Implementation Notes
recipes/overlays/h100-kind-training-kubeflow.yamland updates the training workflow to installplatform: kubeflow.robust-controllerandsecure-accelerator-access, and adds Kubeflow chainsaw/assert coverage for the trainer controller, webhook, andTrainJobCRD.H100 x2, addsgang-schedulingto the kind inference recipes, and removes deaddeploymentphase plumbing from the inference workflow.mainafter fix(ci): deduplicate conformance coverage in GPU CI NVIDIA/aicr#577 merges.Testing
go test ./pkg/recipe/... git diff --check -- .github/workflows/gpu-h100-training-test.yaml .github/workflows/gpu-h100-inference-test.yaml pkg/recipe/conformance_test.go pkg/recipe/deployment_order_guard_test.go recipes/overlays/h100-kind-training.yaml recipes/overlays/h100-kind-training-kubeflow.yaml recipes/overlays/h100-kind-inference.yaml recipes/overlays/h100-kind-inference-dynamo.yaml tests/chainsaw/ai-conformance/cluster/assert-kubeflow-trainer.yaml tests/chainsaw/ai-conformance/kind-training-kubeflow make lintgo test ./pkg/recipe/...passed.git diff --check -- ...passed.make lintwas green on this branch before the local Homebrew Go toolchain upgrade, but current reruns are blocked locally by a stuck parallelgolangci-lintlock after that environment change.make qualifyis not reliable locally right now because the upgraded Homebrew Go 1.26.2 install has a separate stdlib resolution issue (go list syncfails).Risk Assessment
Rollout notes: This changes the inference GPU job to require
H100 x2and assumes the corresponding runner class remains available. Retarget the PR tomainafter NVIDIA#577 merges.Checklist
make testwith-race)make lint)git commit -S) — GPG signing info