What
nodewright.nvidia.com/nodeState_<name> is a single annotation whose value is one JSON document covering every package on the node:
type NodeState map[string]PackageStatus // api/nodewright/v1alpha1
Three controllers write it, each on its own workqueue: the heavy pass (SaveNodesAndSkyhook), JobReconciler, and PodReconciler. Because the contention is inside one annotation value, none of Kubernetes' merge machinery helps — strategic merge treats the value as an opaque string, and server-side apply tracks ownership at the annotation-key level, not within the value. Every writer must therefore hand-roll a read-merge-write.
That is what patchNodeState (job_controller.go) and saveNodeChanges (skyhook_controller.go) both are: optimistic-locked read-modify-write loops with uncached retry reads, plus a delta type (nodeStateDelta, computeNodeStateDelta, apply) so the pass can express "only the entries I actually changed" instead of restamping the whole document.
Proposal
Key the annotation per package: nodewright.nvidia.com/nodeState_<skyhook>_<package>, one PackageStatus per key.
Writers then touch disjoint annotation keys, and a plain strategic merge patch is correct with no coordination — the apiserver merges the map natively. The delta machinery, the optimistic locks, the retry loops and the uncached-reader plumbing on all three reconcilers can go.
Cost
- Node-state schema migration. Existing nodes carry the single-blob form; needs a
zz.migration.<version>.go shim to split it, and the rollback-safe converge/prune pattern already used for the skyhook -> nodewright prefix rename is the precedent.
- Every reader changes:
wrapper/node.go (State, SetState, Upsert, RemoveState, Reset, CleanupSCRMetadata), the CLI, chainsaw assertions, docs.
- Annotation count grows by packages-per-skyhook; total metadata stays bounded well under the 256KB limit (the per-key overhead roughly offsets the removed JSON nesting).
Why file it rather than do it
The immediate lost-update bug is fixed in #413 by having the pass apply only its delta. That is the right size of change for a bug fix on a feature branch. This issue is the structural version — it deletes the bug class rather than patching an instance, and it is worth doing on its own schedule with a proper migration.
Related: #411, #413, #382.
What
nodewright.nvidia.com/nodeState_<name>is a single annotation whose value is one JSON document covering every package on the node:Three controllers write it, each on its own workqueue: the heavy pass (
SaveNodesAndSkyhook),JobReconciler, andPodReconciler. Because the contention is inside one annotation value, none of Kubernetes' merge machinery helps — strategic merge treats the value as an opaque string, and server-side apply tracks ownership at the annotation-key level, not within the value. Every writer must therefore hand-roll a read-merge-write.That is what
patchNodeState(job_controller.go) andsaveNodeChanges(skyhook_controller.go) both are: optimistic-locked read-modify-write loops with uncached retry reads, plus a delta type (nodeStateDelta,computeNodeStateDelta,apply) so the pass can express "only the entries I actually changed" instead of restamping the whole document.Proposal
Key the annotation per package:
nodewright.nvidia.com/nodeState_<skyhook>_<package>, onePackageStatusper key.Writers then touch disjoint annotation keys, and a plain strategic merge patch is correct with no coordination — the apiserver merges the map natively. The delta machinery, the optimistic locks, the retry loops and the uncached-reader plumbing on all three reconcilers can go.
Cost
zz.migration.<version>.goshim to split it, and the rollback-safe converge/prune pattern already used for the skyhook -> nodewright prefix rename is the precedent.wrapper/node.go(State,SetState,Upsert,RemoveState,Reset,CleanupSCRMetadata), the CLI, chainsaw assertions, docs.Why file it rather than do it
The immediate lost-update bug is fixed in #413 by having the pass apply only its delta. That is the right size of change for a bug fix on a feature branch. This issue is the structural version — it deletes the bug class rather than patching an instance, and it is worth doing on its own schedule with a proper migration.
Related: #411, #413, #382.