docs: add nodepool rollout status RFC - #3216
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: cnmcavoy The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
kubernetes-prow
Bot
added
cncf-cla: yes
| generation: 12 | ||
| status: | ||
| observedGeneration: 12 # NEW: the generation the counts below were derived from | ||
| nodeClaims: 20 # NEW: NodeClaims owned by this NodePool |
There was a problem hiding this comment.
Been thinking about this, I think we want this to be status.nodes, status.upToDateNodes, status.readyNodes, etc. The end user doesn't care about Nodeclaims
| nodes: 20 | ||
| resources: {...} | ||
| conditions: | ||
| - type: NodeClaimsUpToDate # NEW |
There was a problem hiding this comment.
I kinda like this mirroring the drift condition on the nodeclaim, I know @jmdeal disagrees
|
|
||
| Core workload controllers solve this by reporting rollout accounting on the parent: a Deployment reports `replicas`/`updatedReplicas`/`readyReplicas` plus `observedGeneration`, a DaemonSet reports `desiredNumberScheduled`/`updatedNumberScheduled`, and Cluster API reports `upToDateReplicas` on MachineDeployments alongside an `UpToDate` condition on Machines. `kubectl rollout status` and essentially all GitOps tooling are built on that convention. NodePool already aggregates `status.nodes` and `status.resources`; this RFC extends that aggregation to rollout progress. | ||
|
|
||
| Earlier attempts at this are [#3108](https://github.com/kubernetes-sigs/karpenter/pull/3108) and [#3177](https://github.com/kubernetes-sigs/karpenter/pull/3177). The difference from these is that we do not propose surfacing "drift" on the NodePool. We propose surfacing *how many NodeClaims were provisioned from the NodePool's current spec revision*. Drift is the mechanism that eventually makes those numbers converge; the revision is the contract consumers gate on. |
There was a problem hiding this comment.
Love this approach, I think this is a much more user oriented design than surfacing too much of the drift internals
| conditions: | ||
| - type: NodeClaimsUpToDate # NEW | ||
| status: "False" | ||
| reason: RolloutInProgress |
There was a problem hiding this comment.
I am curious what other reasons there would be besides rollout in progress
| name: default | ||
| generation: 12 | ||
| status: | ||
| observedGeneration: 12 # NEW: the generation the counts below were derived from |
There was a problem hiding this comment.
Do we bump this when the nodeclass changes too?
2 participants
Fixes #3071
Description
Karpenter does not exposes any aggregate signal on the NodePool that answers the question an operator or an external orchestrator asks after pushing a change: "has this NodePool finished rolling out the spec I just applied?"
We propose surfacing how many NodeClaims were provisioned from the NodePool's current spec revision. Drift is the mechanism that eventually makes those numbers converge; the revision is the contract consumers gate on.
How was this change tested?
Documentation-only change. Follows the RFC template defined in the /designs dir.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.