feat: add blocked nodes metric to scrape-time collector by rawadhossain · Pull Request #431 · kubernetes-sigs/node-readiness-controller

rawadhossain · 2026-08-19T12:59:08Z

Description

Adds node_readiness_blocked_nodes{rule, condition} for tracking the number of currently-held nodes blocked by each unsatisfied condition for each NodeReadinessRule. Ref. New Scrape-Time Collector Surface

The metric is collected directly from the controller runtime cache on each Prometheus scrape, sharing the same Node snapshot with node_readiness_rule_nodes.

Changes

Added node_readiness_blocked_nodes metric with rule and condition labels.
Added ListBlockedNodes to calculate blocked nodes from the live Node state.
Uses the rule's DefaultStatus when a condition is missing from the Node, same as the controller.
Shares the ListNodes with node_readiness_rule_nodes during each scrape.

Related to #397

Type of Change

/kind feature

Testing

make test, make lint, go test ./... -race all pass
Verified against a live cluster, including live condition changes and DefaultStatus behavior.

Checklist

make test passes
make lint passes

Signed-off-by: Rawad Hossain <rawad.hossain00@gmail.com>

netlify · 2026-08-19T12:59:15Z

✅ Deploy Preview for node-readiness-controller canceled.

Name	Link
🔨 Latest commit	`2745dc4`
🔍 Latest deploy log	https://app.netlify.com/projects/node-readiness-controller/deploys/6a8d14454a1dea0008cda7cc

kubernetes-prow · 2026-08-19T12:59:17Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rawadhossain
Once this PR has been reviewed and has the lgtm label, please assign mrunalp 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

kubernetes-prow · 2026-08-19T12:59:20Z

Hi @rawadhossain. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

rawadhossain · 2026-08-19T13:13:41Z

/cc @AvineshTripathi @ajaysundark

AvineshTripathi · 2026-08-19T20:36:15Z

/ok-to-test

AvineshTripathi

did my initial round of reviewing, please have a look at the comments!

AvineshTripathi · 2026-08-20T06:35:56Z


 // Collect implements prometheus.Collector.
 func (c *ReadinessCollector) Collect(ch chan<- prometheus.Metric) {
 	ctx, cancel := context.WithTimeout(context.Background(), collectTimeout)


5sec to do ListNodes, ListRuleNodeStates and ListBlockedNodes can feel little less. wdyt?

I wasn't fully sure about this. Kept it at 5s since it's below Prometheus's default scrape timeout (10s), so we can fail and log before the scrape times out. I also tested it with larger node counts and it seemed fine. Do you think it'd be better to increase?

ajaysundark · 2026-08-23T06:21:00Z

 	defer cancel()

-	counts, err := c.lister.ListRuleNodeStates(ctx)
+	nodes, err := c.lister.ListNodes(ctx)


I have a high level suggestion. Why does this collector list nodes twice? If the node list results have different snapshots (in the single scrape) between the two list time. This could result confusing results

Can your forEachRuleNode be a single list computation for all needs?

yeah missed on that one, actually it wasn't the node list, it was rule list being fetched twice, which had the same risk you described. Fixed that now so the rules are fetched once per scrape and the same snapshot is passed to both computations.

and for forEachRuleNode, I did try combining the two forEachRuleNode passes into a single computation and benchmarked it with different node/rule counts, improvement was pretty small, but it made the deletion case a bit harder to handle correctly. Like a deleting rule still needs to count for held/released nodes but not for blocked nodes. So kept the two computations separate for now to keep logic simpler.

DsThakurRawat · 2026-08-23T11:05:42Z

Looked through the collector changes and noticed two small items in internal/controller/nodereadinessrule_controller.go:

Double pass on scrape: Collect() in internal/metrics/collector.go calls ListRuleNodeStates and then ListBlockedNodes. Both functions call r.List(ctx, ruleList) and run forEachRuleNode over the full node slice. On large node counts, this does two full O(rules * nodes) passes per scrape. We could combine them into a single pass that collects both RuleNodeCounts and RuleBlockedConditions together.
Deletion timestamp filter mismatch: In ListBlockedNodes (line 580), rules undergoing deletion are skipped (!rule.DeletionTimestamp.IsZero()), but ListRuleNodeStates (line 565) includes them (return false). While a deleted rule is waiting on finalizer cleanup, node_readiness_rule_nodes will continue emitting held/released gauges for that rule, while node_readiness_blocked_nodes drops all condition metrics for it. We should keep the deletion filter consistent across both metric collectors.

rawadhossain · 2026-08-23T13:07:42Z

Looked through the collector changes and noticed two small items in internal/controller/nodereadinessrule_controller.go:

Double pass on scrape: Collect() in internal/metrics/collector.go calls ListRuleNodeStates and then ListBlockedNodes. Both functions call r.List(ctx, ruleList) and run forEachRuleNode over the full node slice. On large node counts, this does two full O(rules * nodes) passes per scrape. We could combine them into a single pass that collects both RuleNodeCounts and RuleBlockedConditions together.

Deletion timestamp filter mismatch: In ListBlockedNodes (line 580), rules undergoing deletion are skipped (!rule.DeletionTimestamp.IsZero()), but ListRuleNodeStates (line 565) includes them (return false). While a deleted rule is waiting on finalizer cleanup, node_readiness_rule_nodes will continue emitting held/released gauges for that rule, while node_readiness_blocked_nodes drops all condition metrics for it. We should keep the deletion filter consistent across both metric collectors.

already addressed.
good catch, fixed the inconsistency.

AvineshTripathi

I tried this locally, didn't see any issue there for now. Left some minor comments. PHAL

AvineshTripathi · 2026-08-24T20:17:49Z

 	var errors []string
 	for _, node := range nodeList.Items {
-		if !r.ruleAppliesTo(ctx, rule, &node) {
+		applies, held := r.ruleAppliesToWithTaint(ctx, rule, &node)


Internally this parses nodeselector for every node which can be avoided.

right, didn't notice that. Moved the parsing outside the loop so it only happens once per rule now, and removed ruleAppliesToWithTaint since it's not needed anymore

AvineshTripathi · 2026-08-24T20:27:15Z

-	for rule, rc := range counts {
-		ch <- prometheus.MustNewConstMetric(ruleNodesDesc, prometheus.GaugeValue, rc.Held, rule, string(RuleNodeStateHeld))
-		ch <- prometheus.MustNewConstMetric(ruleNodesDesc, prometheus.GaugeValue, rc.Released, rule, string(RuleNodeStateReleased))
+	counts, err := c.lister.ListRuleNodeStates(ctx, nodes, rules)


I think we should rename counts to nodeStatesByRule or something intuitive

yes renamed to nodeStatesByRule

AvineshTripathi · 2026-08-24T20:31:56Z

-	nodeList := &corev1.NodeList{}
-	if err := r.List(ctx, nodeList); err != nil {
+// ListRules returns the current list of NodeReadinessRules.
+func (r *RuleReadinessController) ListRules(ctx context.Context) ([]*readinessv1alpha1.NodeReadinessRule, error) {


why ListRule is returning []*readinessv1alpha1.NodeReadinessRule while ListNode returns []corev1.NodeList. One is a pointer while other is a copy

yeah, actually followed the existing pattern. Rules are used as pointers everywhere else in this file, so ListRules follows the same pattern. Whereas nodes are only read here and handled as values, so I left this as is to stay consistent with the existing usage.

add blocked nodes metric

0a8a7ff

Signed-off-by: Rawad Hossain <rawad.hossain00@gmail.com>

kubernetes-prow Bot added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 19, 2026

kubernetes-prow Bot requested review from dchen1107 and mrunalp August 19, 2026 12:59

kubernetes-prow Bot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 19, 2026

kubernetes-prow Bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Aug 19, 2026

rawadhossain mentioned this pull request Aug 19, 2026

[FEATURE] Scrape-Time Metrics Collector #397

Closed

3 tasks

kubernetes-prow Bot requested review from AvineshTripathi and ajaysundark August 19, 2026 13:13

kubernetes-prow Bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 19, 2026

rawadhossain mentioned this pull request Aug 19, 2026

feat: Add selector and bootstrap observability metrics #286

Open

2 tasks

AvineshTripathi reviewed Aug 20, 2026

View reviewed changes

simplify node rule checks

367985d

rawadhossain force-pushed the blocked-nodes branch from e72d1ac to 367985d Compare August 20, 2026 19:32

ajaysundark reviewed Aug 23, 2026

View reviewed changes

This was referenced Aug 23, 2026

REQUEST: New membership for rawadhossain kubernetes/org#6576

Closed

[TRACKING] Update Existing Metrics #446

Open

AvineshTripathi reviewed Aug 24, 2026

View reviewed changes

reuse rule snapshot in collector

2745dc4

rawadhossain force-pushed the blocked-nodes branch from 844bd82 to 2745dc4 Compare August 25, 2026 04:04

Uh oh!

Conversation

rawadhossain commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Related to #397

Type of Change

Testing

Checklist

Uh oh!

netlify Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for node-readiness-controller canceled.

Uh oh!

kubernetes-prow Bot commented Aug 19, 2026

Uh oh!

kubernetes-prow Bot commented Aug 19, 2026

Uh oh!

rawadhossain commented Aug 19, 2026

Uh oh!

AvineshTripathi commented Aug 19, 2026

Uh oh!

AvineshTripathi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ajaysundark Aug 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DsThakurRawat commented Aug 23, 2026

Uh oh!

rawadhossain commented Aug 23, 2026

Uh oh!

AvineshTripathi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rawadhossain commented Aug 19, 2026 •

edited

Loading

netlify Bot commented Aug 19, 2026 •

edited

Loading

ajaysundark Aug 23, 2026 •

edited

Loading