Skip to content

feat(monitoring): alert when pods of one workload share a node - #4993

Merged
Aleksei Sviridkin (lexfrei) merged 2 commits into
cozystack:mainfrom
yankawai:feat/pod-replicas-colocated-alert
Nov 13, 2026
Merged

Aleksei Sviridkin (lexfrei) merged 2 commits into
cozystack:mainfrom
yankawai:feat/pod-replicas-colocated-alert

Conversation

@yankawai

@yankawai europrinter (yankawai) commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

What this PR does

On three 3-node clusters running v1.6.4 and v1.6.5 we found a tenant etcd with two of its three members on one node, a tenant Redis with two of its three Sentinels on the node that also ran the master, and several VictoriaMetrics and VictoriaLogs pairs (vmstorage, vmselect, vmalertmanager, vlstorage) sharing a node. Nothing alerted. Losing one of those nodes would have cost the etcd or Sentinel quorum, or both copies of a storage tier. Spread constraints keep new pods apart once they are in place (#4986 for etcd, #4989 for monitoring, #4990 for Redis), but a placement that already exists, or that comes from a soft rule or a drain, stays as it is, because the scheduler does not move running pods.

This adds PodReplicasColocated, a warning in a new workload-placement group under packages/system/monitoring-agents/alerts/. It fires when two or more running pods of one StatefulSet, ReplicaSet, CNPG Cluster or StrimziPodSet have been on one node for an hour while the cluster has at least as many schedulable nodes as the workload has running pods, so five replicas on three nodes stay quiet. The alert carries cluster, namespace, owner_kind, owner_name and node, plus the severity, exported_instance and service labels the other platform alerts set, and its value is the number of pods on that node. DaemonSets, Jobs and every other owner kind are not in the list, so a DaemonSet surge pod next to its predecessor does not count.

etcd-operator gives every member pod its own EtcdMember as controller owner (etcdmember_controller.go#L546), so grouping by owner would see one pod per group. Members get their names from GenerateName: <cluster>- (etcdcluster_controller.go#L434, #L839), and members adopted from the old StatefulSet keep names like etcd-0 (adopt.go#L103). The rule drops the last dash-separated part of the member name and reports the group as EtcdCluster <name>. With the etcd package that is one group per tenant namespace, and two etcd clusters in one namespace would still be told apart. CNPG instance pods carry a controller reference to their Cluster (cluster_create.go#L1580), and Strimzi pods one to their StrimziPodSet (StrimziPodSetController.java#L369).

Owner and node both come from kube_pod_info. kube-state-metrics fills created_by_kind and created_by_name from the controller owner reference and node from spec.nodeName (pod.go#L638-L653). kube_pod_owner and kube_pod_status_phase have no node label of their own, and the kube-state-metrics scrape adds node from __meta_kubernetes_pod_node_name with honorLabels: true (templates/kube-state-metrics-scrape.yaml), so on those series node is the node of the kube-state-metrics pod. The test data keeps that label on the phase series. A pod counts while kube_pod_status_phase{phase="Running"} is 1, so a Failed or Pending pod on the same node does not. kube_pod_labels does carry every pod label here (--metric-labels-allowlist=pods=[*]), but the rule does not depend on it.

A node counts as schedulable unless it has a NoSchedule or NoExecute taint. A cordon shows up as the node.kubernetes.io/unschedulable:NoSchedule taint (node_lifecycle_controller.go#L549-L555), and not ready and unreachable nodes get theirs from their conditions (#L87-L99), so one clause covers all three, and a tainted control plane does not count either. Pods that a drain had to put together on the remaining nodes therefore stay quiet while the node is out, and the hour starts once it is back.

The alert waits an hour. A replacement pod runs next to its terminating predecessor only for the predecessor's grace period, and node maintenance takes the drained node out of the count, while a placement that has held for an hour does not fix itself. The alert is about a risk rather than an outage, so the wait costs nothing. The VMAlert sets both remoteWrite and remoteRead, so the pending state survives a vmalert restart (vmalert docs).

Not covered are workloads that run one pod per owner, such as ClickHouse with one StatefulSet per replica (patterns.go#L45-L53) and the KubeVirt VMs behind a tenant Kubernetes node group. They would need grouping by pod labels, which this rule avoids. The node count is cluster-wide, so a workload that a node selector or affinity restricts to fewer nodes than it has pods fires too.

hack/workload-placement-alerts.bats takes .spec of the VMRule and replays it through vmalert-tool v1.136.0 pinned by digest, the release vm-operator v0.68.4 deploys by default (config.go#L38) and the same image #4963 and #4985 use. hack/testdata/workload-placement-alerts/unittest.yaml has 13 cases with 24 checks. Two of three StatefulSet pods on one node fire at 61 minutes and not at 59, with labels and annotations pinned. Spread pods, five ReplicaSet pods on three nodes, a DaemonSet surge pod, two pods of one Job, a Failed pod next to a running one, and one pod each in two clusters that share workload and node names stay quiet. etcd members fire as one EtcdCluster per namespace, for generated and adopted member names. A replacement pod next to its terminating predecessor for twenty minutes stays quiet, and the same overlap held past the hour fires and resolves when it ends. A cordoned node keeps the alert quiet until it is uncordoned, and a node with unreachable taints keeps it quiet throughout. ReplicaSet, CNPG Cluster and StrimziPodSet pods fire with their owner kind.

$ bats hack/workload-placement-alerts.bats
1..1
ok 1 workload placement alerts pass their vmalert-tool cases

With the expression replaced by one that never fires, the suite prints not ok 1 and vmalert-tool reports 5 of the 24 checks, which are all the checks that expect an alert. On main, where the rule file does not exist, the suite fails as well. Each of these changes to the rule fails at least one check, with the number of failing checks in brackets. for: 15m (5), no phase filter (1), no node count (6), no cluster label (6), DaemonSet and Job in the kind list (3), no taint clause (4), < instead of <= (5), etcd members grouped per member (1).

With Bats 1.14.0 at the commit CI pins, the suite passes together with alert-severity-contract, bats-strict-setup, bats-no-negated-assert, bats-no-exit-trap, bats-runner-coverage, bats-runner-contract, image-pin-consistency and check-commit-trailers, 109 of 109, and the severity contract lists the new alert as warning. helm-unittest 1.1.2, the version CI pins, passes the monitoring-agents chart with 19 of 19 tests in 6 suites, and helm template with Helm 3.17.3 renders the VMRule the same as the file. Like the migration suites the new suite needs docker, and only the image pull touches the network. I have not run the rule in vmalert on a live cluster.

On release-1.6 kube-state-metrics 2.15.0, its scrape config, the alerts template, the VMAlert and vm-operator v0.68.4 are the same, and etcd-operator v0.5.5 and CNPG 1.28.2 set the same owners and member names. The new files apply there without changes. That backport is a separate PR with one more commit that drops the load test_helper line, because release-1.6 runs hack/*.bats through hack/cozytest.sh.

Screenshots

Not a UI change.

Downstream repositories

A new alert file under packages/system/monitoring-agents/alerts/, a Bats suite under hack/ and its test data. No values, schema, README, image or ApplicationDefinition change, and the website does not list platform alerts by name, so no trigger map entry applies.

Release note

feat(monitoring): new warning alert PodReplicasColocated fires when two or more running pods of one StatefulSet, ReplicaSet, CNPG Cluster, StrimziPodSet or etcd cluster have shared a node for an hour while the cluster has a schedulable node for each of them.

Summary by CodeRabbit

  • New Features

    • Added monitoring for workloads whose running pod replicas are colocated on the same node.
    • Alerts include workload, node, cluster, and severity details, along with quorum-loss guidance.
    • Coverage includes StatefulSets, ReplicaSets, Clusters, StrimziPodSets, and EtcdMembers.
  • Tests

    • Added validation for workload-placement alerts across scheduling, availability, cluster-isolation, and pod-replacement scenarios.

Two etcd members, two Redis Sentinels or both replicas of a storage
tier can end up on one node through a soft spread rule, a drain, or a
placement made before a spread rule existed. Nothing reported it, and
the scheduler does not move running pods, so it stays like that until
a pod is recreated. One node failure then costs the quorum or every
copy at once.

etcd-operator owns each member pod by its own EtcdMember, so members
are grouped by the EtcdCluster name their names start with. Owner and
node are read from kube_pod_info because kube_pod_owner and
kube_pod_status_phase get the node of the kube-state-metrics pod as a
target label. Nodes with a NoSchedule or NoExecute taint do not count,
which covers cordoned, not ready and unreachable nodes. The alert waits
an hour, longer than a replacement pod runs next to its terminating
predecessor or a node takes to drain and come back.

Assisted-by: LLM
Signed-off-by: Yan Bondarenko <[email protected]>
@github-actions github-actions Bot added size/XL This PR changes 500-999 lines, ignoring generated files area/monitoring Issues or PRs related to the monitoring stack (vlogs, vmstack, grafana, workloadmonitor) kind/feature Categorizes issue or PR as related to a new feature labels Oct 20, 2026
@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 5632596 and 054852c.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

Adds a PodReplicasColocated warning alert for supported workloads and tests its firing conditions, labels, and annotations. The tests also cover workload placement, pod replacement overlap, node availability, running status, and cluster isolation.

Changes: Workload placement alert

Layer / File(s) Summary
Define the colocated-pod alert
packages/system/monitoring-agents/alerts/workload-placement.yaml
Adds a VMRule that identifies running pods by workload owner, detects colocated replicas, and filters based on schedulable-node count. The alert requires the condition to persist for one hour and includes warning labels and quorum-impact annotations.
Test workload coverage and alert output
hack/testdata/workload-placement-alerts/unittest.yaml
Adds rule tests for StatefulSet, ReplicaSet, DaemonSet, Job, EtcdMember, Cluster, and StrimziPodSet workloads. Tests check alert output and cases where the alert does not fire.
Test replacement and node conditions
hack/testdata/workload-placement-alerts/unittest.yaml, hack/workload-placement-alerts.bats
Tests replacement overlap, cordoned and unreachable nodes, non-running pods, and cluster isolation. Adds a Bats test that runs the fixture with vmalert-tool in a network-disabled Docker container.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Suggested reviewers: ivanhunters

Merge Risk: Minimal · previous review 56325

This change adds a new warning alert for pods of the same replicated workload that have shared a node for an hour while other schedulable nodes are available, along with tests for the supported workload types and edge cases. It changes no workload behavior and only adds alerting. No outstanding issues block merging, though the alert's noise level on real clusters should be watched after rollout.

Pre-merge checks: Passed 5

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: adding a monitoring alert for pods from one workload sharing a node.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. (2 skipped: 2 unsupported.)

Finishing Touches

🧪 Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

ℹ️ Recent review info

⚙️ Run configuration

  • Configuration used: Repository: cozystack/cozystack/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b883bc2e-b690-43c9-8c00-b7c069ea7975
📥 Commits

Reviewing files that changed from the base of the PR and between d1d4e92 and 5632596.

📒 Files selected for processing: 3

  • hack/testdata/workload-placement-alerts/unittest.yaml
  • hack/workload-placement-alerts.bats
  • packages/system/monitoring-agents/alerts/workload-placement.yaml

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 4 remain after this review.

@IvanHunters IvanHunters left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran the new vmalert-tool suite (13 cases) plus three independent mutations (>1 to >9999, for:1h to 15m, node-gate <= to <), all caught, so the tests are not vacuous. The fire/no-fire table checks out: DaemonSet and Job kinds are excluded, etcd members collapse into one EtcdCluster, and two clusters with the same workload and node names do not cross-group. The phase join on (cluster,namespace,pod) correctly excludes node given KSM honorLabels. Rule name and group are unique. No blocking issues.

Non-blocking:

  • workload-placement.yaml:17,43: created_by_kind="Cluster" is generic. Today only CNPG pods carry that owner kind, but a future operator whose pods are owned by a Kind=Cluster controller would be grouped under the same alert. Isolated per (namespace, owner_name), so blast radius is small.
  • workload-placement.yaml:63-68: the schedulable-node count is cluster-wide, so a workload that a node selector or affinity restricts to fewer nodes than it has replicas will fire a false PodReplicasColocated. Already acknowledged in the PR body.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <[email protected]>
@lexfrei

Copy link
Copy Markdown
Contributor

europrinter (@yankawai) I merged main into your branch (054852c) so CI runs against the current base. Your commits are unchanged.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

europrinter (@yankawai) LGTM, CI and E2E are green on this head.

@lexfrei
Aleksei Sviridkin (lexfrei) merged commit 4fa6bfe into cozystack:main Nov 13, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/monitoring Issues or PRs related to the monitoring stack (vlogs, vmstack, grafana, workloadmonitor) kind/feature Categorizes issue or PR as related to a new feature size/XL This PR changes 500-999 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants