Wait for current DaemonSet status before reporting readiness

[sylvesterkaczmarek]

Description

Fixes #2968.

Require the DaemonSet controller to have observed the current generation before nonzero desired/available/updated counts can establish readiness. Previously, generation 3 with observedGeneration 2 and matching counts of 2 was accepted as ready even though those counts describe the previous template.

The zero-node path already checks the generation, as does Deployment readiness. This adds the same requirement to the nonzero DaemonSet path without changing pod-count checks, rollout strategy, reconciliation writes, or zero-node behavior. Kubernetes defines the status field in its DaemonSet API reference.

The existing TestIsDaemonSetReadyStaleGeneration deliberately characterized this known gap by expecting true. It now requires false. New cases cover initial/stale/current/newer observed generations, incomplete counts, zero-node compatibility, and a real state-aggregation transition after a fake-client status update. No existing test is removed or skipped.

Newly reproduced on main 60526e35 and reported on 25 September 2026. Issue #2968 was rechecked for assignments and competing work. The open Service/ServiceAccount reconciliation patches touch other methods in the file and are independent of this correction.

Testing

• The nine new regression scenarios pass; three failed before the production fix. The corrected existing characterization also fails on unchanged production code.
• Full make unit-test, including its build prerequisite, passes on macOS arm64 with Go 1.27.1. make fmt reports no formatting changes.
• The complete internal/state suite passes with the race detector on both macOS and Linux arm64: 280 passing test/subtest events, no failures or skips. Linux go vet also passes.
• Full-tree golangci-lint 2.13.2 passes in Linux (0 issues). The changed state package also passes lint on macOS. Full-tree macOS lint reports three SA4023 diagnostics in the unchanged non-Linux validator path; no suppression or unrelated validator change is included.
• Linux validation ran in an isolated Go container with networking disabled. No Kubernetes cluster, GPU, driver installation, or deployment was modified.
• git diff --check passes. The commit is signed off and its signature is verified by GitHub.

make fmt
make unit-test
go test -race -count=1 ./internal/state
golangci-lint run ./...

The last command passes on Linux as described above. Live-cluster and GPU integration were not run. API definitions, generated assets, module files and vendor contents are unchanged; their regeneration targets were not run for this implementation-only correction.

Checklist

• No secrets, sensitive information, or unrelated changes.
• Full-tree lint passes on Linux; macOS limitation documented.
• Regression tests cover the corrected behavior and compatibility cases.
• Generated assets and Go module artifacts are unchanged.

---

[copy-pr-bot]

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]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: NVIDIA/gpu-operator/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 11513737-813f-484d-8181-bf3f80665550

📥 Commits

Reviewing files that changed from the base of the PR and between 60526e3 and 87dadc6.

📒 Files selected for processing (3)

• internal/state/state_skel.go
• internal/state/state_skel_reconcile_test.go
• internal/state/state_skel_test.go

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

---

📝 Walkthrough

Walkthrough

For DaemonSets with nonzero desired nodes, readiness now requires ObservedGeneration to be at least Generation, in addition to the existing pod-count checks. The zero-desired-node condition remains unchanged. Tests cover stale generations, zero desired pods, and the getSyncState transition from not ready to ready.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 87dad

The change waits for the DaemonSet controller to observe the current generation before reporting nonzero workloads ready, while preserving the reported zero-node behavior. No merge-blocking issue is identified.

---

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

[kvnloo]

I checked this against the existing readiness flow and the scope looks appropriately narrow.

Putting the ObservedGeneration >= Generation guard directly on the nonzero DaemonSet-ready branch fixes the stale-status case without changing the existing pod-count semantics. It also stays consistent with the zero-node path: status from an older generation cannot establish readiness in either branch.

The getSyncState regression is especially useful here because it verifies the real aggregation path transitions from notReady to ready after the status update, rather than only testing isDaemonSetReady in isolation.

I would also keep the comparison as >= rather than tightening it to ==; the contract we care about is that observed status is not stale.

I don't see a blocking scope issue in this diff.

[kvalliyurnatt]

/ok to test 87dadc6

[kvalliyurnatt]

Thanks for your contribution. LGTM

[rajathagasthya]

@sylvesterkaczmarek Please squash your commits into a single commit and make sure you --signoff.

[sylvesterkaczmarek]

Done. I rebased onto current main and squashed the branch to a single commit, aeb59ce, with the required Signed-off-by trailer. The commit is GitHub Verified, DCO is green, the PR remains approved, and the full internal/state package passes locally. Ready for the remaining maintainer/runner checks.

[rajathagasthya]

/ok-to-test aeb59ce

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.