docs(platform-package): document kubevirt.migrations for v1.6 - #705
europrinter (yankawai) wants to merge 1 commit into
Conversation
✅ Deploy Preview for cozystack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 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 |
… through platform values (#4402) This reopens a fork backport from a branch in this repository so its CI can run. The commit is the one from #4339 by @yankawai, unchanged. On release-1.6 the pull-request CI pushes the images it builds, and a run from a fork has no registry credentials, so #4339 stopped at the build jobs and never reached e2e. The description below is the author's. ## What this PR does Manual backport of #4254 to `release-1.6`. The automated backport, #4322, stopped with conflict markers in four files as its only commit, so it is red on DCO and unmergeable as it stands. Every conflict has the same cause: the `kubevirt.disabledFeatureGates` work landed on main after 1.6 was cut, and the cherry-pick carried it into hunks this line does not have. Four differences follow: - platform values gains a `kubevirt` block holding `migrations` alone, and the iaas bundle gains only the migrations hop; - the wiring test takes the four migrations cases and the case for a null `kubevirt` block, without the assertions that belong to disabledFeatureGates cases this line does not have; - `update_idempotency_test.sh` gains `run_update_logged`, a four-line helper that arrived on main with that work and the new cases need; - the kubevirt Makefile header counts six sed patches rather than five. Verified on this branch: 9 kubevirt chart tests, 31 platform wiring tests (102 across the platform suite), and `update_idempotency_test` PASS under GNU make and sed. Removing the new awk guard from the Makefile turns that test red naming the migrations block, so the ported guard is doing work. On this line `(.Values.kubevirt).migrations` is the only reader of `.Values.kubevirt`; rewriting it as `.Values.kubevirt.migrations` turns the null-block case red with a nil pointer. Feature summary, unchanged from #4254: cluster-wide migration settings could only be applied by patching the KubeVirt CR, because the kubevirt chart did not render `migrations` and the platform bundle did not forward it. `kubevirt.migrations` now passes through the generated Package to `spec.configuration.migrations`. A hand patch of that Package is undone on the next platform render, so this hop is the only supported setter. ```yaml kubevirt: migrations: bandwidthPerMigration: 625M parallelMigrationsPerCluster: 2 parallelOutboundMigrationsPerNode: 1 ``` ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map against the diff: platform values, the iaas bundle and the kubevirt chart. The v1.6 reference page of the platform values had no `kubevirt` section, so the row goes there as a follow-up that should land with this backport. - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: cozystack/website#705 ### Release note ```release-note feat(kubevirt): expose `kubevirt.migrations` in the platform values and forward it to `spec.configuration.migrations` on the KubeVirt CR, so live-migration bandwidth and parallelism can be set without patching a Package that the platform re-renders. ```
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
europrinter (@yankawai) NOT LGTM. The text matches the shipped code, but the page needs to say which 1.6 patch release has the value, and the backport reference is stale.
Business context: documents the kubevirt.migrations platform value for 1.6 users now that the value is backported to the 1.6 line.
I checked every sentence against the release-1.6 branch and v1.6.4-rc.1. The default {}, the forward into spec.configuration.migrations on the KubeVirt CR, the empty or null map leaving KubeVirt defaults alone (both with guards, in packages/core/platform/templates/bundles/iaas.yaml and packages/system/kubevirt/templates/kubevirt-cr.yaml), the field names in the example and the 625M = 5 Gbit/s arithmetic are all correct.
Blockers
B1: the page does not say the value needs v1.6.4
The v1.6 tree is read by everyone on any 1.6.x release. The backport (cozystack/cozystack#4402) landed after v1.6.3, and git tag --contains on its merge commit returns only v1.6.4-rc.1. At v1.6.3 the platform chart has no kubevirt key and iaas.yaml never reads it, so an operator on 1.6.0 to 1.6.3 who copies this example gets no error and no effect. Please add a short line to the row or under the heading, something like "Available since v1.6.4". Merging before v1.6.4 is released is fine with that line in place.
B2: the PR body and the first commit cite a closed backport
The PR body says cozystack/cozystack#4339 backports the value and asks to merge it first. The first commit message says the same. #4339 was closed without merging; the backport that landed is #4402. This repo merges with merge commits, so both the commit body and the PR body end up in main history as written. Please point both at #4402 and drop the "merge that backport first" sentence, since there is nothing left to wait for.
Placement and markdown are fine. next already has the same row from #697, and there is no newer versioned tree that would also need it. The table and the paragraphs are single lines and the fence has a language.
cozystack/cozystack#4402 backports the kubevirt.migrations platform value to the 1.6 line, and the v1.6 reference had no KubeVirt section at all. This adds the migrations row and its example from the next page, with a line saying the value is available since v1.6.4, because earlier 1.6 releases ignore it. The disabledFeatureGates row stays out, as that value is not on the 1.6 line. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
551235e to
7ab90b4
Compare
|
Added a line under the KubeVirt heading saying that |
What this PR does
Adds a KubeVirt section with the
kubevirt.migrationsrow, its example and the note on bandwidth units to the v1.6 platform-package reference. It documents the value that cozystack/cozystack#4402 backported torelease-1.6. That backport first ships in v1.6.4, so the section says the value is available since Cozystack v1.6.4 and that earlier 1.6 releases ignore it. Thekubevirt.disabledFeatureGatesrow fromnextstays out because that value is not on the 1.6 line. The row and the example are the ones #697 added tonext.Checks run for this change.
bash hack/check-i18n.sh checkpassed with 29 translation keys and 20 tracked translated pages.git diff origin/main...HEAD --checkpassed, and the PR changes one reference file.Screenshots
Not applicable, reference documentation only.
Downstream repositories
This reference-only change does not alter any downstream API, schema, build tooling, installer setting or runtime behavior.
Release note