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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 -->
2 participants
Summary by CodeRabbit