Repository navigation
bugfix/fix-longterm - #697
Conversation
|
Caution Review failedThe pull request is closed. """ WalkthroughThis pull request updates the configuration for the vmagent to support multiple remote write endpoints. In the template file, a loop is introduced to generate a URL entry for each element in a list. The values file is modified to replace the single URL entry with a list containing two URLs, accommodating both short-term and long-term insert services. Changes
Sequence Diagram(s)sequenceDiagram
participant H as Helm Template Engine
participant V as vmagent Deployment
participant R as RemoteWrite Endpoints
H->>V: Render vmagent.yaml with list of URLs
V->>H: Loop over each URL in remoteWrite.urls
H->>R: Generate configuration entry for each URL
V->>R: Send data to each remote endpoint
R-->>V: Acknowledge data reception
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/system/monitoring-agents/templates/vmagent.yaml(1 hunks)packages/system/monitoring-agents/values.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
packages/system/monitoring-agents/templates/vmagent.yaml
[error] 14-14: syntax error: could not find expected ':'
(syntax)
🔇 Additional comments (3)
packages/system/monitoring-agents/templates/vmagent.yaml (1)
1-12: Overall YAML StructureThe rest of the file’s configuration (API version, extraArgs, scrape settings, etc.) remains unchanged and appears structurally sound. Ensure that any downstream processes or tools consuming this configuration are aware of the templated changes.
Also applies to: 16-35
packages/system/monitoring-agents/values.yaml (2)
304-312: Update RemoteWrite Configuration with Multiple URLsThe remoteWrite configuration has been updated to use a
urlslist instead of a singleurl. This now clearly provides endpoints for both short-term and long-term insert services, meeting the PR objectives. The indentation and list structure look correct.
1-303: Remaining Configuration UnchangedAll other sections of the values file remain unaffected. It’s a good idea to double-check that components referencing the remoteWrite configuration (for example, in your deployment or in related templates) have been updated accordingly to reference the new
urlsfield.Also applies to: 313-370
| {{- range .Values.vmagent.remoteWrite.urls }} | ||
| - url: {{ . | quote }} | ||
| {{- end }} |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
YAML Indentation in the Templated RemoteWrite Loop
The new templating block that generates multiple remoteWrite URLs may produce invalid YAML due to its current indentation. The YAML linter error on line 14 ("could not find expected ':'") suggests that the list item (- url: {{ . | quote }}) isn’t indented enough relative to its parent key. To ensure that the rendered YAML is valid, consider increasing the indentation of the list items inside the range loop. For example:
- {{- range .Values.vmagent.remoteWrite.urls }}
- - url: {{ . | quote }}
- {{- end }}
+ {{- range .Values.vmagent.remoteWrite.urls }}
+ - url: {{ . | quote }}
+ {{- end }}This diff ensures that the list items are properly nested under the remoteWrite key. Please verify the generated YAML (e.g. using helm template) to confirm that the syntax is correct.
Action Required: Correct YAML Indentation for RemoteWrite URLs
The YAML linter error ("could not find expected ':'") indicates that the list items generated by the range loop are not indented sufficiently within their parent key. To fix this, please update the indentation by adding extra spaces to nest the list items correctly. For example, modify the code as follows:
- {{- range .Values.vmagent.remoteWrite.urls }}
- - url: {{ . | quote }}
- {{- end }}
+ {{- range .Values.vmagent.remoteWrite.urls }}
+ - url: {{ . | quote }}
+ {{- end }}After applying this change, verify the output with helm template to ensure that the rendered YAML is valid.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{- range .Values.vmagent.remoteWrite.urls }} | |
| - url: {{ . | quote }} | |
| {{- end }} | |
| {{- range .Values.vmagent.remoteWrite.urls }} | |
| - url: {{ . | quote }} | |
| {{- end }} |
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 14-14: syntax error: could not find expected ':'
(syntax)
Signed-off-by: kklinch0 <[email protected]>
5fed2e2 to
f0ffe2a
Compare
#4254) ## What this PR does Cluster-wide migration settings could only be applied by patching the KubeVirt CR: the kubevirt chart did not render `migrations`, and the platform bundle did not forward it. Add the optional `kubevirt.migrations` platform value and pass it through the generated Package to `spec.configuration.migrations`. For example, set the following under `spec.components.platform.values` on the `cozystack.cozystack-platform` Package: ```yaml kubevirt: migrations: bandwidthPerMigration: 625M parallelMigrationsPerCluster: 2 parallelOutboundMigrationsPerNode: 1 ``` `bandwidthPerMigration` is a quantity in bytes per second. Empty or null settings leave the field unset; existing KubeVirt defaults are unchanged. The map is forwarded intact, including explicit `false` and `0` values. `make update` restores the new parameterization independently after a hand-merge. Validation: - All 207 Helm tests in `packages/system/kubevirt` and `packages/core/platform` passed (42 suites). - Before the template changes, the new limit and false/zero cases failed at both forwarding stages (four expected regression failures). - The GNU sed / make update-idempotency suite passed, including restoration of a missing migrations block, repeated runs, and rejection of a missing insertion anchor. - Standalone KubeVirt rendering and `helm lint` passed. Neither modified chart defines a `make generate` target. Full platform rendering requires its live source repository lookup; the platform template behavior was checked through its existing Helm unit-test fixtures. This is separate from #3989, which exposes `vmStateStorageClass` in the same chart and bundle files. ### Screenshots Not applicable; no UI changes. ### Downstream repositories - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: [#697](cozystack/website#697) - [ ] [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: The website follow-up [#697](cozystack/website#697) documents this new platform value in the unreleased `next` reference and should be merged after this implementation. The merged website PR [#678](cozystack/website#678) documents `kubevirt.disabledFeatureGates` only. The other listed repositories do not need a coordinated change: this adds an optional platform value without changing app schemas, required installer values, CRDs, package layout, source kinds, shared tooling contracts, or node prerequisites. ### Release note ```release-note feat(kubevirt): Configure cluster-wide KubeVirt migration settings through the platform Package value kubevirt.migrations. Empty settings preserve existing defaults. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added optional KubeVirt migration configuration. * Supports migration bandwidth, concurrency limits, auto-convergence, and completion timeout settings. * Migration bandwidth can be specified in bytes per second. * Empty or unset migration settings preserve KubeVirt defaults. * **Bug Fixes** * Migration values now preserve valid `false` and `0` settings when provided. * **Tests** * Added coverage for default, empty, null, configured, and repeated-update scenarios. * Added validation to prevent incomplete or duplicated migration configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…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 -->
…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
…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
Summary by CodeRabbit