[praveen9354]

Reopens #3012 — closed on 2026-05-11 with the question "does pods_drained_total satisfy this?" After comparing both metric definitions, the two are adjacent rather than overlapping.

Why pods_disrupted_total is not a duplicate of pods_drained_total

Aspect	pods_drained_total (existing)	pods_disrupted_total (this PR)
Fires at	Each successful pod eviction	NodeClaim-disruption decision time
Increment shape	+1 per pod (eviction.go:215)	+N per disruption (N = reschedulable)
Labels	reason	reason, nodepool, capacity_type
Pod filter	All evicted pods	Reschedulable only (excludes DS, mirror)
Counts pods that fail to evict?	No (200 path only)	Yes (decision-time count)
Counts pods lost to ungraceful node	No (terminator never runs)	Yes (via node/health controller)
Increment sites	1 (terminator/eviction.go)	4 (queue, expiration, health, GC)

Three concrete operator questions only pods_disrupted_total answers:

1. Pods that fail to evict (PDB violations, repeated 429s, ungraceful terminations) — pods_drained_total only increments on the 200 path in terminator/eviction.go:215, so the denominator of intended disruption is invisible. pods_drained_total / pods_disrupted_total is the eviction success rate per nodepool/reason; neither metric alone expresses this.

2. Ungraceful node loss (node/health/controller.go GC path) — when an unhealthy node's NodeClaim is GC'd, the terminator never runs, so pods_drained_total is structurally blind to those pod disruptions. pods_disrupted_total catches them because it increments in the same 4 controllers that already increment nodeclaims_disrupted_total.

3. Label cardinality parity with nodeclaims_disrupted_total — that metric already exists at pkg/metrics/metrics.go:60 with (reason, nodepool, capacity_type). Operators can answer "how many NodeClaims were disrupted per nodepool" but not the corresponding pod-level question with matching labels.

Open to adjustments

If cardinality is a concern, happy to drop the capacity_type label.

If the name suggests overlap with pods_drained_total, happy to rename to pods_disruption_initiated_total to make the timing difference explicit.

Implementation

Mirrors karpenter_nodeclaims_disrupted_total but counts the reschedulable pods that were on each disrupted node, across the four NodeClaim disruption paths where pod records exist:

• pkg/controllers/disruption/queue.go uses Candidate.reschedulablePods directly (already cached on the Candidate)
• pkg/controllers/nodeclaim/expiration/controller.go, pkg/controllers/node/health/controller.go, and pkg/controllers/nodeclaim/garbagecollection/controller.go list pods via the new node.CountReschedulablePodsOnNode helper, which applies the same pod.IsReschedulable filter (active or stateful-set terminating; not DaemonSet- or Node-owned)

Skipped: nodeclaim/lifecycle/launch.go and liveness.go disrupt NodeClaims that never bound pods (launch failure, liveness watchdog), so a pods counter on those paths would always be 0.

A list error during pod counting is logged at V(1) and the metric reports 0 for that disruption — never blocks the disruption itself.

Refs 2020

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: praveen9354
Once this PR has been reviewed and has the lgtm label, please assign jonathan-innis for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[praveen9354]

Friendly ping @DerekFrank when you have a moment — this is the pods_disrupted_total counter PR that pairs with the existing karpenter_nodeclaims_disrupted_total. The PR body lays out the operator-question motivation in detail, including:

1. Different timing from pods_drained_total — disrupted_total fires at NodeClaim-disruption decision time, while drained_total only fires on successful 200 eviction. Their ratio is the per-nodepool eviction success rate.
2. Ungraceful node loss is uncounted by drained_total — when node/health/controller.go GCs an unhealthy node, the terminator never runs, so drained_total is structurally blind. disrupted_total catches it because it increments in the same four controllers as nodeclaims_disrupted_total.
3. Label cardinality parity with nodeclaims_disrupted_total — (reason, nodepool, capacity_type) — operators can answer "how many NodeClaims by nodepool" but not the pod-level equivalent.

Happy to drop the capacity_type label if cardinality is the concern, or rename to pods_disruption_initiated_total for timing clarity — both adjustments are explicit in the PR description.

Presubmit matrix (1.29.x → 1.35.x) was green; one flake on node/termination (untouched by this PR) is the only red. Whenever you have bandwidth, would appreciate a look. Thanks!

[praveen9354]

@DerekFrank gentle re-ping when you have a moment — this is the pods_disrupted_total counter PR pairing with the existing karpenter_nodeclaims_disrupted_total. CI is green (13/13 checks passing). Happy to address any clarifications on naming, label cardinality, or the disrupted-vs-drained timing distinction. Thanks!

[DerekFrank]

/assign @DerekFrank

[DerekFrank]

I think I misunderstood the initial PR. I'll try to take a deeper look next week

[praveen9354]

@MaciekPytel @tallaxes — gentle ping. A duplicate PR (#3087) for the same karpenter_pods_disrupted_total counter was opened and then closed by its author on 2026-06-12 in favor of this PR, explicitly noting the broader scope here (closing comment):

"PR #3015 by @praveen9354 already covers this feature with broader scope — incrementing PodsDisruptedTotal at all four relevant controller sites (disruption queue, expiration, node health, GC), not just the queue."

CI checks here are green (14 ok). Happy to address any review feedback — let me know if there's a scope or design concern blocking the review queue.

[DerekFrank]

Okay, some minor code architecture points but I'm aligned on the need for this metric

Separate issue, not this PR: putting nodepool on pods_drained_total gets us
per-nodepool eviction success rate (drained / disrupted). Different site
(terminator/eviction.go) and it bumps cardinality on an existing metric, so it
wants its own PR.

[DerekFrank]

This is GetPods + an IsReschedulable filter, which is the same thing
NewCandidate already does inline. GetPods only uses node.Name, so I'd make
it take nodeNames ...string (the existing callers all have a node, so it's a
mechanical .Name) and add a ReschedulablePods(...) helper on top. Then the
metric sites just do len(...) and this function goes away.

[DerekFrank]

I think I prefer disruption_initiated_total

[praveen9354]

Thanks for the review. Addressed both points in split commits:

1. Helper refactor — GetPods now takes nodeNames ...string (existing callers become mechanical .Name). Added ReschedulablePods(...) composing GetPods + IsReschedulable; the three emit sites (health/expiration/gc) call it and len(...) at the emit line. CountReschedulablePodsOnNode is gone. Moved the empty-nodename guard into GetPods so future callers inherit it.

2. Metric rename — PodsDisruptedTotal → PodsDisruptionInitiatedTotal (karpenter_pods_disruption_initiated_total), help string updated. Left NodeClaimsDisruptedTotal alone since the per-nodeclaim semantic is unambiguous.

Rebased onto latest main (main had moved significantly since I opened this, including your #3009 and #3045 clock refactors — my test now uses env.Clock). All 7 presubmit lanes green.

pods_drained_total + nodepool label saved for a follow-up PR.

[praveen9354]

Gentle ping @DerekFrank on whether the addressed points look good, and @MaciekPytel / @tallaxes for the maintainer approval when you have a moment. Happy to rebase or address any additional feedback.

[DerekFrank]

/lgtm

[DerekFrank]

/lgtm

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: DerekFrank, praveen9354

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [DerekFrank]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment