Repository navigation
fix(monitoring): scope tenant VMAlert to its own rule namespaces - #4091
myasnikovdaniil wants to merge 1 commit into
Conversation
github-actions
Bot
added
area/monitoring
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe VMAlert template preserves cluster-wide rule selection for ChangesVMAlert rule scope
Estimated code review effort: 3 (Moderate) | ~15–30 minutes Merge Risk: ⚪ Minimal · up to Tenant monitoring now excludes infrastructure rules while root monitoring retains them; the scoped rendering behavior is covered for both deployment modes and no merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
myasnikovdaniil NOT LGTM. The diagnosis from #2775 is right and nested tenants work, but the new selector also drops alerts that tenants get today and should keep.
Nested tenants check out on a trial merge with current main. A child tenant without its own monitoring inherits the parent's namespace.cozystack.io/monitoring value, so the parent's VMAlert still sees its rules, and a child with its own stack drops out, same as in vmagent.yaml. helm unittest packages/system/monitoring passes. The new suite goes red if I always render selectAllByDefault: true, if I flip it to true in the tenant branch, or if I hardcode the selector value to tenant-root.
Tenants lose the CloudNativePG alerts. packages/system/postgres-operator/templates/alerts.yaml installs the cnpg-default-alerts PrometheusRule (LongRunningTransaction, BackendsWaiting, PGReplication and others) in cozy-postgres-operator, and the vm-operator converter makes a VMRule in that namespace. The data comes from tenant databases: apps/postgres/templates/db.yaml sets monitoring.enablePodMonitor: true, so the PodMonitor lives in the tenant namespace and the tenant's own vmagent scrapes it. On main the tenant VMAlert picks up this rule, evaluates it against metrics it really has, and the alert reaches the tenant's Alertmanager. With this change cozy-postgres-operator doesn't match the tenant label, so a tenant with Postgres stops getting these alerts. So where the rules live does matter here. Rules for tenant workloads that sit in an operator namespace need another way into the tenant, for example rendered into the tenant namespace by the monitoring chart, before the selector can be narrowed. A test for that would help, the current suite only checks the selector shape.
The cozy-monitoring install path loses its alerts too. packages/core/platform/sources/monitoring.yaml still defines a cozystack.monitoring PackageSource that installs this chart into cozy-monitoring. The rest of the chart treats that namespace as a platform view: vmagent.yaml skips it and dashboards.yaml renders for it. Here it falls into the tenant branch and gets namespace.cozystack.io/monitoring: cozy-monitoring, a value no namespace carries, so that VMAlert selects nothing. Adding cozy-monitoring to the root condition, like the other two templates do, fixes it, and a test case for that namespace keeps it fixed.
The only red check is E2E (in-tree) from 2026-09-05 (attempt 1): chainsaw/kafka timed out deleting the Kafka app in verify-pvc-reclaim-on-delete. I don't see how a VMAlert selector gets there, so I don't count it against this PR.
A tenant's VMAlert was rendered with selectAllByDefault and no rule
selector, which by the operator's rules means "select every VMRule in
the cluster". It then evaluated those rules against the tenant's own
vmselect, which only holds that tenant's metrics.
Infrastructure alerts are written as absent(up{job="kubelet"} == 1) and
friends, so they fired in every tenant precisely because the series is
absent there -- the noise reported in #2775: CDI operator is down,
plus Kubelet / KubeAPI / KubeControllerManager / KubeScheduler
disappeared from target discovery. Those rules live outside any tenant
namespace (cozy-monitoring holds 38 of them, cozy-kubevirt-cdi and
cozy-linstor one each), so no change to where they are rendered can fix
this; the selection is what is too wide.
Scope a tenant VMAlert by the namespace.cozystack.io/monitoring label,
the same way vmagent.yaml already scopes scrapes. The label's value
names the tenant whose monitoring stack covers that namespace, so the
selector matches the tenant's own namespace plus any child tenant
inheriting this stack -- exactly the namespaces whose metrics reach this
vmselect. ruleSelector stays unset, so all VMRules in those namespaces
continue to match, tenant-authored ones included.
Root monitoring keeps selectAllByDefault: the infrastructure namespaces
carry no such label, and a selector would silence the platform's own
alerts. Root behaviour is therefore unchanged.
Assisted-By: LLM
Signed-off-by: Myasnikov Daniil <[email protected]>
1287f29 to
41df418
Compare
2 participants
Tenant vmalert was rendered with
selectAllByDefaultand no rule selector, which for vm-operator means select every VMRule in cluster. It then evaluates them against tenant own vmselect that holds only tenant metrics. Infra alerts are written asabsent(up{job="kubelet"} == 1), so they fire in tenant exactly because series is absent there.This PR scopes tenant vmalert by
namespace.cozystack.io/monitoringlabel, same wayvmagent.yamlalready scopes scrapes. Label value is the tenant whose monitoring stack covers that namespace, so selector matches tenant own namespace plus child tenants inheriting this stack.ruleSelectorstays unset, so all VMRules in those namespaces still match, tenant written ones included.Root keeps
selectAllByDefault. Infra rules live incozy-monitoring,cozy-kubevirt-cdi,cozy-linstorand those namespaces carry no such label, so any selector there would silence platform own alerts. Root behaviour is unchanged.Note that rendering infra alerts conditionally (like dashboards do) does not fix this, because the rules must exist for root anyway and one of the five alerts comes from
cozy-kubevirt-cdi, not from monitoring-agents at all. Too wide selection is the problem, not the place rules are rendered.Checked on a live cluster with two vmalerts in one namespace, one scoped and one not:
All five alerts from the issue (CDIOperatorDown, KubeAPIDown, KubeControllerManagerDown, KubeletDown, KubeSchedulerDown) are in unscoped set and absent in scoped one, tenant own rule survives.
Closes #2775
Summary by CodeRabbit