Repository navigation
fix(monitoring-agents): use global.clusterDomain for FQDN resolution - #2086
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdded a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
Summary of ChangesHello Andrei Kvapil (@kvaps), I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug that prevented monitoring-agents from installing correctly in tenant clusters. The core problem stemmed from Helm subcharts being unable to access cluster-specific configuration values. The solution involves refactoring how cluster domain information is passed, introducing a new global variable that ensures consistent and correct FQDN resolution across different cluster types, thereby restoring proper cross-cluster service discovery functionality. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Andrei Kvapil (kvaps)
marked this pull request as ready for review
February 23, 2026 15:03
Andrei Kvapil (kvaps)
requested review from
IvanHunters,
Andrey Kolkov (androndo),
Aleksei Sviridkin (lexfrei) and
Timofei Larkin (lllamnyp)
as code owners
February 23, 2026 15:03
dosubot
Bot
added
size/S
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request correctly addresses the issue of accessing _cluster values from a Helm subchart by introducing a global.clusterDomain variable. The changes in packages/system/monitoring-agents/values.yaml correctly use a with block to conditionally append the cluster domain. However, the changes in packages/apps/kubernetes/templates/helmreleases/monitoring-agents.yaml use direct concatenation, which is less robust. I've suggested using the with block there as well for consistency and to prevent potential issues with empty domain values.
| tenant: {{ .Release.Namespace }} | ||
| remoteWrite: | ||
| url: http://vminsert-shortterm.{{ $targetTenant }}.svc:8480/insert/0/prometheus | ||
| url: http://vminsert-shortterm.{{ $targetTenant }}.svc.{{ $clusterDomain }}:8480/insert/0/prometheus |
Contributor
There was a problem hiding this comment.
For consistency and robustness, it's better to use the with block to conditionally append the cluster domain, similar to how it's done in packages/system/monitoring-agents/values.yaml. This will prevent issues if $clusterDomain could be empty in some scenarios, which would result in a trailing dot in the hostname.
url: http://vminsert-shortterm.{{ $targetTenant }}.svc{{ with $clusterDomain }}.{{ . }}{{ end }}:8480/insert/0/prometheus| Name http | ||
| Match kube.* | ||
| Host vlogs-generic.{{ $targetTenant }}.svc | ||
| Host vlogs-generic.{{ $targetTenant }}.svc.{{ $clusterDomain }} |
Contributor
There was a problem hiding this comment.
For consistency and robustness, it's better to use the with block to conditionally append the cluster domain, similar to how it's done in packages/system/monitoring-agents/values.yaml. This will prevent issues if $clusterDomain could be empty in some scenarios, which would result in a trailing dot in the hostname.
Host vlogs-generic.{{ $targetTenant }}.svc{{ with $clusterDomain }}.{{ . }}{{ end }}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/apps/kubernetes/templates/helmreleases/monitoring-agents.yaml (1)
48-49: Consider quoting theclusterDomainvalue for defensive YAML encoding.
cozy.localis unambiguous as a bare YAML scalar today, but if the cluster domain ever contains a colon, leading/trailing whitespace, or other special characters, the unquoted form would produce invalid YAML. Quoting is zero-cost and eliminates the risk.🛡️ Proposed fix
global: - clusterDomain: {{ $clusterDomain }} + clusterDomain: "{{ $clusterDomain }}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/apps/kubernetes/templates/helmreleases/monitoring-agents.yaml` around lines 48 - 49, The global.clusterDomain value in the Helm template should be rendered as a quoted YAML scalar to prevent invalid YAML when the domain contains colons, whitespace, or special characters; update the template expression that outputs {{ $clusterDomain }} so it emits a quoted string (e.g., wrap the rendered value in double quotes) where the symbol to change is the clusterDomain key in the monitoring-agents.yaml Helm template.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/apps/kubernetes/templates/helmreleases/monitoring-agents.yaml`:
- Line 55: The override uses the singular key remoteWrite.url but the
vmagent.yaml template expects remoteWrite.urls (plural), so change the
HelmRelease override to use remoteWrite.urls and supply a YAML list value (e.g.,
a single-item list with the shortterm insert URL) so it matches the format from
values.yaml and will be picked up by the template; update the entry that
currently sets url: http://vminsert-shortterm.{{ $targetTenant }}.svc.{{
$clusterDomain }}:8480/insert/0/prometheus to use urls: -
http://vminsert-shortterm.{{ $targetTenant }}.svc.{{ $clusterDomain
}}:8480/insert/0/prometheus so vmagent's remoteWrite.urls is correctly
overridden.
---
Nitpick comments:
In `@packages/apps/kubernetes/templates/helmreleases/monitoring-agents.yaml`:
- Around line 48-49: The global.clusterDomain value in the Helm template should
be rendered as a quoted YAML scalar to prevent invalid YAML when the domain
contains colons, whitespace, or special characters; update the template
expression that outputs {{ $clusterDomain }} so it emits a quoted string (e.g.,
wrap the rendered value in double quotes) where the symbol to change is the
clusterDomain key in the monitoring-agents.yaml Helm template.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/apps/kubernetes/templates/helmreleases/monitoring-agents.yamlpackages/system/monitoring-agents/values.yaml
…n tenant HelmRelease PR #2075 added _cluster.cluster-domain references to monitoring-agents values.yaml for FQDN resolution. This broke fluent-bit because it is a subchart where _cluster values are not accessible (only global values are shared with subcharts), causing "index of untyped nil". Revert values.yaml to short DNS names (.svc) for the management cluster where they resolve locally, and add FQDN with cluster-domain suffix in the tenant kubernetes HelmRelease where _cluster values are available and cross-cluster DNS resolution is needed. Closes #2084 Co-Authored-By: Claude <[email protected]> Signed-off-by: Andrei Kvapil <[email protected]>
Andrei Kvapil (kvaps)
force-pushed
the
fix/monitoring-agents-cluster-domain
branch
from
February 23, 2026 15:15
76d9576 to
d8bb352
Compare
5 tasks done
Andrei Kvapil (kvaps)
deleted the
fix/monitoring-agents-cluster-domain
branch
February 23, 2026 19:16
1 of 12 tasks
1 of 12 tasks
Aleksei Sviridkin (lexfrei)
added a commit
that referenced
this pull request
Nov 10, 2026
…t'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 -->
myasnikovdaniil
added a commit
that referenced
this pull request
Nov 10, 2026
…t'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 --> Backported-from: c117420
cozystack-ci Bot
added a commit
that referenced
this pull request
Nov 12, 2026
…t'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 --> Backported-from: c117420
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
PR #2075 introduced
_cluster.cluster-domainreferences in monitoring-agentsvalues.yamlfor FQDN resolution in tenant clusters. This broke the fluent-bit subchart because_clustervalues are not accessible from the Helm subchart context — onlyglobalvalues are shared with subcharts.This PR replaces
_clusterreferences with a newglobal.clusterDomainvariable:service.namespace.svc)cozy.local) for tenant clusters, enabling FQDN resolution for cross-cluster service discoveryFixes #2084
Release note
Summary by CodeRabbit