mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-13 08:05:19 +09:00
fix(kubernetes, monitoring): send tenant cluster metrics to the tenant's vminsert (#5024)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does The vmagent that `packages/apps/kubernetes` deploys into a tenant Kubernetes cluster has not been shipping metrics anywhere, because the app and the `monitoring-agents` chart disagree on two values keys, and Helm drops a key nobody reads without a word. - **Remote write URL.** #697 changed the chart to read `vmagent.remoteWrite.urls` as a list, but the app still passed a single `vmagent.remoteWrite.url` (the cluster domain was later added to that ignored key in #2086). The chart then applied its own defaults, `vminsert-{shortterm,longterm}.cozy-monitoring.svc`, which do not exist inside a tenant cluster. vmagent fails with `no such host` and queues the failed writes on the node disk, up to `-remoteWrite.maxDiskUsagePerURL` (1 GiB) per URL, which is enough to push a small node into DiskPressure. - **Tenant label.** The chart writes `tenant: {{ .Values.global.target }}`, which only the root bundle sets, so every tenant cluster series was labelled `tenant="cozy-monitoring"` and the `vmagent.externalLabels.tenant` the app passes was ignored. The fix: - the app passes `remoteWrite.urls` with the tenant's shortterm vminsert; - the chart uses `vmagent.externalLabels.tenant` when set, and falls back to `global.target`. The label is deliberately not fixed by passing `global.target` from the app, because the tenant label must name the tenant that owns the cluster (`.Release.Namespace`), while the monitoring target can be a parent tenant. The root install sets neither key: `helm template` of `templates/vmagent.yaml` with `global.target=tenant-root` renders the same URLs and label values before and after; the only textual change is that the `tenant` label is now quoted, so a tenant name YAML would read as a boolean or number still renders as a string. Tests: `packages/apps/kubernetes/tests/monitoring_agents_test.yaml` pins the keys the app sends, and `packages/system/monitoring-agents/tests/vmagent_remote_write_test.yaml` pins how the chart reads them, for both the root install and a tenant cluster. Reverting either template change fails its suite. `helm unittest` passes on both packages (358 and 23 tests). Upgrade note for operators: on tenant clusters the `tenant` label of vmagent series moves from `cozy-monitoring` to the owning tenant's namespace, so dashboards or alerts that filtered on `tenant="cozy-monitoring"` for those series need updating. Before this change those series never reached the tenant's Monitoring in the first place. Left out on purpose: the shortterm store stays hardcoded, as it was in the app before; deriving it from the target Monitoring's `metricsStorages`, and bounding vmagent's on-disk buffer, are separate changes. ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(kubernetes, monitoring): the vmagent of tenant Kubernetes clusters now writes to the tenant's vminsert and labels series with the owning tenant instead of `cozy-monitoring`. Previously no tenant cluster metric reached the tenant's Monitoring. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit - **Bug Fixes** - Monitoring agents now configure remote-write endpoints using a URL list. - Tenant labels use explicitly configured values when available, with a fallback to the global target. - Tenant and cluster labels, remote-write URLs, and external labels are applied consistently across root and tenant-cluster deployments. - **Tests** - Added coverage for root and tenant-cluster monitoring configurations, including remote-write settings and metric labels. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
4 files changed
+110
-2
No files matched your search
@@ -45,7 +45,8 @@ spec:
|
||||
cluster: {{ .Release.Name }}
|
||||
tenant: {{ .Release.Namespace }}
|
||||
remoteWrite:
|
||||
url: http://vminsert-shortterm.{{ $targetTenant }}.svc.{{ $clusterDomain }}:8480/insert/0/prometheus
|
||||
urls:
|
||||
- http://vminsert-shortterm.{{ $targetTenant }}.svc.{{ $clusterDomain }}:8480/insert/0/prometheus
|
||||
fluent-bit:
|
||||
readinessProbe:
|
||||
httpGet:
|
||||
|
||||
@@ -0,0 +1,42 @@
|
||||
suite: monitoring-agents HelmRelease values match the monitoring-agents chart
|
||||
|
||||
# The values below are consumed by packages/system/monitoring-agents, which
|
||||
# reads vmagent.remoteWrite.urls as a list and the tenant label from
|
||||
# vmagent.externalLabels.tenant. A key the chart does not read is dropped in
|
||||
# silence, and the chart then falls back to cozy-monitoring defaults that do
|
||||
# not resolve inside a tenant cluster. Its counterpart is
|
||||
# packages/system/monitoring-agents/tests/vmagent_remote_write_test.yaml.
|
||||
templates:
|
||||
- templates/helmreleases/monitoring-agents.yaml
|
||||
|
||||
values:
|
||||
- values/common.yaml
|
||||
|
||||
release:
|
||||
name: kubernetes-clusterprod
|
||||
namespace: tenant-a
|
||||
|
||||
set:
|
||||
addons.monitoringAgents.enabled: true
|
||||
_namespace.monitoring: tenant-root
|
||||
_cluster:
|
||||
cluster-domain: cozy.local
|
||||
|
||||
tests:
|
||||
- it: sends metrics to the shortterm vminsert of the monitoring tenant as a urls list
|
||||
asserts:
|
||||
- equal:
|
||||
path: spec.values.vmagent.remoteWrite
|
||||
value:
|
||||
urls:
|
||||
- http://vminsert-shortterm.tenant-root.svc.cozy.local:8480/insert/0/prometheus
|
||||
|
||||
- it: labels series with the tenant that owns the cluster, not the monitoring tenant
|
||||
asserts:
|
||||
- equal:
|
||||
path: spec.values.vmagent.externalLabels
|
||||
value:
|
||||
cluster: kubernetes-clusterprod
|
||||
tenant: tenant-a
|
||||
- notExists:
|
||||
path: spec.values.global
|
||||
@@ -10,7 +10,7 @@ spec:
|
||||
shardCount: 1
|
||||
externalLabels:
|
||||
cluster: {{ .Values.vmagent.externalLabels.cluster }}
|
||||
tenant: {{ .Values.global.target }}
|
||||
tenant: {{ .Values.vmagent.externalLabels.tenant | default .Values.global.target | quote }}
|
||||
extraArgs:
|
||||
{{- toYaml (deepCopy .Values.vmagent.extraArgs | mergeOverwrite (fromYaml (include "monitoring-agents.vmagent.defaultExtraArgs" .))) | nindent 4 }}
|
||||
remoteWrite:
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
suite: vmagent remote write targets and tenant label
|
||||
|
||||
# The root bundle sets only global.target, and the chart defaults derive both
|
||||
# the remote write URLs and the tenant label from it. A tenant Kubernetes
|
||||
# cluster gets this chart through packages/apps/kubernetes instead, which
|
||||
# passes vmagent.remoteWrite.urls and vmagent.externalLabels.tenant and leaves
|
||||
# global.target at its default. If either value is dropped, vmagent writes to
|
||||
# cozy-monitoring services that do not exist in the tenant cluster, or labels
|
||||
# every series tenant="cozy-monitoring".
|
||||
templates:
|
||||
- templates/vmagent.yaml
|
||||
release:
|
||||
name: cozy-monitoring-agents
|
||||
namespace: cozy-monitoring
|
||||
tests:
|
||||
- it: root install writes to both stores of global.target and labels series with it
|
||||
set:
|
||||
global.target: tenant-root
|
||||
asserts:
|
||||
- equal:
|
||||
path: spec.remoteWrite
|
||||
value:
|
||||
- url: http://vminsert-shortterm.tenant-root.svc:8480/insert/0/prometheus
|
||||
- url: http://vminsert-longterm.tenant-root.svc:8480/insert/0/prometheus
|
||||
- equal:
|
||||
path: spec.externalLabels.tenant
|
||||
value: tenant-root
|
||||
|
||||
- it: tenant cluster writes only to the URLs its parent app passes
|
||||
set:
|
||||
vmagent:
|
||||
externalLabels:
|
||||
cluster: kubernetes-clusterprod
|
||||
tenant: tenant-a
|
||||
remoteWrite:
|
||||
urls:
|
||||
- http://vminsert-shortterm.tenant-a.svc.cozy.local:8480/insert/0/prometheus
|
||||
asserts:
|
||||
- equal:
|
||||
path: spec.remoteWrite
|
||||
value:
|
||||
- url: http://vminsert-shortterm.tenant-a.svc.cozy.local:8480/insert/0/prometheus
|
||||
|
||||
- it: tenant cluster labels series with the owning tenant, not global.target
|
||||
set:
|
||||
vmagent:
|
||||
externalLabels:
|
||||
cluster: kubernetes-clusterprod
|
||||
tenant: tenant-a
|
||||
asserts:
|
||||
- equal:
|
||||
path: spec.externalLabels
|
||||
value:
|
||||
cluster: kubernetes-clusterprod
|
||||
tenant: tenant-a
|
||||
|
||||
- it: tenant label stays a string when the value reads as a YAML scalar
|
||||
set:
|
||||
vmagent:
|
||||
externalLabels:
|
||||
tenant: "true"
|
||||
asserts:
|
||||
- equal:
|
||||
path: spec.externalLabels.tenant
|
||||
value: "true"
|
||||
Reference in new issue
Block a user