Skip to content

feat(recipes): backfill CRD Established=True for 6 operator-owning health checks#1919

Open
mohityadav8 wants to merge 2 commits into
NVIDIA:mainfrom
mohityadav8:feat/1246-backfill-crd-established
Open

feat(recipes): backfill CRD Established=True for 6 operator-owning health checks#1919
mohityadav8 wants to merge 2 commits into
NVIDIA:mainfrom
mohityadav8:feat/1246-backfill-crd-established

Conversation

@mohityadav8

@mohityadav8 mohityadav8 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Backfills CRD Established=True assertions for the 6 operator-owning health checks that PR #1245 deferred pending chart verification.

Motivation / Context

PR #1245 added CRD Established=True checks for 4 components whose CRD names were verifiable in-tree (gpu-operator, dynamo-platform, nodewright-operator, cert-manager). It explicitly deferred gatekeeper, kueue, kubeflow-trainer, network-operator, k8s-nim-operator, and slinky-slurm-operator because their CRD names weren't enumerated in-tree at the pinned chart version — bundling them blind risked typo'd names failing on live clusters.

Related: #1245

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)
  • Build/CI/tooling

Component(s) Affected

  • CLI (cmd/aicr, pkg/cli)
  • API server (cmd/aicrd, pkg/server)
  • Recipe engine / data (pkg/recipe)
  • Bundlers (pkg/bundler, pkg/component/*)
  • Collectors / snapshotter (pkg/collector, pkg/snapshotter)
  • Validator (pkg/validator)
  • Core libraries (pkg/errors, pkg/k8s)
  • Docs/examples (docs/, examples/)
  • Other: ____________

Implementation Notes

Per component, in recipes/checks/<comp>/health-check.yaml:

Component CRD(s) asserted Verification source
kueue resourceflavors/clusterqueues/localqueues.kueue.x-k8s.io recipes/components/kueue/manifests/*.yaml already ship CRs at this apiVersion — group is load-bearing today, not a guess
kubeflow-trainer trainjobs.trainer.kubeflow.org tests/chainsaw/ai-conformance/kind-training-kubeflow/assert-crds.yaml — asserted on a live kind cluster
network-operator nicclusterpolicies.mellanox.com recipes/components/network-operator/manifests/nic-cluster-policy-aks.yaml ships an actual NicClusterPolicy CR at mellanox.com/v1alpha1
k8s-nim-operator nimservices/nimcaches/nimpipelines.apps.nvidia.com docs/conformance/cncf/v1.35/nim-eks/evidence/robust-operator.md — live kubectl get crds output from an EKS run of chart 3.1.0
gatekeeper constrainttemplates.templates.gatekeeper.sh, configs.config.gatekeeper.sh Not yet verified in-tree. Limited to the two CRDs gatekeeper cannot function without, regardless of feature flags (mutation/expansion/external-data CRDs are feature-conditional and deliberately excluded to avoid false negatives). Flagged inline with a TODO to confirm via helm template gatekeeper/gatekeeper --version 3.22.2 before this is trusted at face value.
slinky-slurm-operator none — negative finding Chart is deployed with crds.enabled: false (recipes/registry.yaml); all 6 slinky.slurm.net CRDs are owned exclusively by the sibling slinky-slurm-operator-crds chart, already covered by its own health check. Comment added explaining the omission per #1246's acceptance criteria.

For kubeflow-trainer, only TrainJob is verified — TrainingRuntime/ClusterTrainingRuntime/JobSet from the issue's guess table are left out pending confirmation. Same caveat for network-operator's MacvlanNetwork/IPoIBNetwork/HostDeviceNetwork.

Testing

go test -race -count=1 ./validators/chainsaw/...
python -m yamllint -c .yamllint.yaml recipes/checks/gatekeeper/health-check.yaml recipes/checks/k8s-nim-operator/health-check.yaml recipes/checks/kubeflow-trainer/health-check.yaml recipes/checks/kueue/health-check.yaml recipes/checks/network-operator/health-check.yaml recipes/checks/slinky-slurm-operator/health-check.yaml
  • TestValidateTestReadOnly_RegistryContent: pass
  • TestRunChainsawTestInProcess_RegistryCorpusParses: pass
  • yamllint: clean, no output
  • Live-cluster validation not yet performed for any of the 6 — see Risk Assessment.

Risk Assessment

  • Medium — Touches multiple components; CRD names for kueue/kubeflow-trainer/network-operator/k8s-nim-operator are verified against real in-tree evidence, but gatekeeper's two CRD names are still unconfirmed against the live chart, and none of the 6 have been validated on a live cluster yet.

Rollout notes: Per-component revert is trivial (each is an isolated try step). Before merge, gatekeeper's constrainttemplates.templates.gatekeeper.sh / configs.config.gatekeeper.sh should be confirmed via helm template against chart 3.22.2 — same live-verification gate PR #1245 used for its 4 components. Recommend @mchmarny or whoever owns cluster access for #1246 do this pass, same as #1245's agreement.

Checklist

  • Tests pass locally (make test with -race)
  • Linter passes (make lint)
  • I did not skip/disable tests to make CI green
  • I added/updated tests for new functionality — no new tests added; existing TestComponentRegistry_RequiresHealthCheck + TestValidateTestReadOnly_RegistryContent lint guards already cover the registry content this PR touches, same as feat(recipes): deepen 21 chainsaw health checks; close epic #660 #1245
  • I updated docs if user-facing behavior changed — N/A, no user-facing CLI/API change
  • Changes follow existing patterns in the codebase
  • Commits are cryptographically signed (git commit -S)

…alth checks

Closes NVIDIA#1246.

- kueue: resourceflavors/clusterqueues/localqueues.kueue.x-k8s.io (verified via in-tree manifests)
- kubeflow-trainer: trainjobs.trainer.kubeflow.org (verified via tests/chainsaw/ai-conformance/kind-training-kubeflow/assert-crds.yaml)
- network-operator: nicclusterpolicies.mellanox.com (verified via components/network-operator/manifests/nic-cluster-policy-aks.yaml)
- k8s-nim-operator: nimservices/nimcaches/nimpipelines.apps.nvidia.com (verified via docs/conformance/cncf/v1.35/nim-eks/evidence/robust-operator.md)
- gatekeeper: constrainttemplates.templates.gatekeeper.sh + configs.config.gatekeeper.sh (needs live-cluster confirmation against chart 3.22.2, flagged inline)
- slinky-slurm-operator: negative finding documented - crds.enabled: false, CRDs owned entirely by sibling slinky-slurm-operator-crds chart

Signed-off-by: Mohit Yadav <ymohit799057@gmail.com>
@mohityadav8
mohityadav8 requested a review from a team as a code owner July 26, 2026 11:03
@copy-pr-bot

copy-pr-bot Bot commented Jul 26, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 1c027db1-21c1-41cc-81f5-212d03d098d1

📥 Commits

Reviewing files that changed from the base of the PR and between 7e0f4a1 and 553d236.

📒 Files selected for processing (6)
  • recipes/checks/gatekeeper/health-check.yaml
  • recipes/checks/k8s-nim-operator/health-check.yaml
  • recipes/checks/kubeflow-trainer/health-check.yaml
  • recipes/checks/kueue/health-check.yaml
  • recipes/checks/network-operator/health-check.yaml
  • recipes/checks/slinky-slurm-operator/health-check.yaml

📝 Walkthrough

Walkthrough

Five operator health checks now verify required CustomResourceDefinitions have reached Established=True before continuing. The Kueue check retains its LocalQueue condition assertion with formatting adjustment. The Slinky health check documents sibling-chart CRD ownership and detects known init-container waiting failures.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: mchmarny

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding CRD Established=True checks for six operator health checks.
Description check ✅ Passed The description directly explains the same CRD backfill work and its context, so it is clearly related.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

…gative finding

Signed-off-by: Mohit Yadav <ymohit799057@gmail.com>
@mohityadav8

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant