mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-28 23:57:08 +09:00
main
6
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
d9ed566326 |
fix(migration): restore the class reclaim policy on imported volumes (#4480)
<!-- 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 VM import handoff sets the transferred `PersistentVolume` to `Retain` so the data survives the swap from the Forklift-populated claim to the VMDisk's claim, and then never gives the policy back. Every imported disk therefore ends up with `persistentVolumeReclaimPolicy: Retain`, while its StorageClass says `Delete` and every VMDisk created directly carries `Delete`. When the tenant later deletes an imported VMDisk through the normal path, its DataVolume and claim go away, but the volume stays `Released` with its backing storage: an orphaned, billed volume that no tenant can see or remove. #### How it was measured On a lab cluster with a LINSTOR-backed `replicated` class (`reclaimPolicy: Delete`): a 16 GiB VM imported from vSphere produced a VMDisk whose PV was `Retain`, while every non-imported VMDisk's PV in the cluster was `Delete`. Deleting the VMInstance and then the VMDisk removed the DataVolume and the claim and left the PV `Released`, still `Retain`, with its LINSTOR resource in place. Patching that PV back to `Delete` made the CSI provisioner delete both the PV and the LINSTOR resource, which confirms the reclaim policy is the only thing keeping it. #### The fix `Retain` is now held only for the window it protects. A new step, `settleReclaimPolicy`, restores the policy the volume's StorageClass declares (`Delete` when the class names none or no longer exists) once the handoff is provably over, and until then reports "not yet" and touches nothing: - the replacement claim is `Bound` and the PV is `Bound` with a `claimRef` naming that claim by UID, as the PV controller reports it rather than as the controller wrote it, so no release is in flight; - the transferred claim is gone; - the claim's one and only owner is the VMDisk's DataVolume, as a controller reference whose UID matches the live DataVolume (CDI's adoption), so the only deletion that can reach the volume is the VMDisk's own; - the VMDisk exists and carries this task's output markers. `fulfill` runs this step after adopting every disk and before creating the VMInstance and deleting the scaffolding VM, so a VM is reported `Succeeded` only once its volumes carry their class's policy, and an import that fails after this point still leaves disks that clean up like any other. The step is a compare-and-set, so repeated passes and restarts are no-ops once settled. It also runs on the path that recovers outputs whose status was never written, for tasks caught mid-handoff by an upgrade. The comment that called permanent `Retain` "part of the contract" and the matching paragraph in `packages/system/migration-controller/README.md` are updated; the README also says how to fix volumes left on `Retain` by imports completed before this change, since those tasks are terminal and are not reconciled again. In practice a VM now stays in `Creating` for one more requeue (about 20 s) after the swap, while the PV controller rebinds and CDI adopts the claim. #### Tests - `TestSettleRestoresTheClassReclaimPolicy`: a finished handoff gets the class's policy back: `Delete`, `Retain` when the class asks for it, and `Delete` when the class names none or is gone. - `TestSettleIsIdempotent`: a second pass over a settled volume reports settled and does not rewrite the PV. - `TestSettleKeepsRetainWhileTheHandoffIsOpen`: thirteen cases, each breaking one condition (claim not created by this import, claim not bound, PV still `Released`, `claimRef` still on the old claim, `claimRef` on the new claim with a stale UID, transferred claim still present, claim not yet adopted by CDI, an extra owner, a DataVolume owner that is not the controller, an owner reference to a previous DataVolume, DataVolume missing, VMDisk missing, VMDisk not created by this import). Every one keeps `Retain`. - `TestFulfillRestoresTheReclaimPolicyBeforeFinishing`: behavioural regression over `fulfill`. The first pass swaps the volume, keeps `Retain`, creates no VMInstance and keeps the scaffolding VM. After simulating the PV controller and CDI, the second pass restores `Delete`, creates the VMInstance and removes the scaffolding VM. With the new step turned into a no-op, this test fails. - `TestFulfillSettlesReclaimedOutputs`: the recovery path for outputs whose status was never written. With the handoff still open the volume keeps `Retain`; once it is finished the policy goes back to `Delete`. - The existing `adoptVolume` tests still assert `Retain` during the swap. `go test ./internal/migrationcontroller/...` and `make test-controllers` pass, `go vet` is clean, and `make generate` leaves the tree unchanged. ### 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: The diff touches only `internal/migrationcontroller/`, `packages/system/migration-controller/README.md` and a comment in `packages/system/migration-controller/templates/rbac.yaml`, which match no entry of the trigger map; the website's VMware migration page does not describe the reclaim policy, so it needs no change. ### 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(migration): disks imported from VMware no longer keep a Retain reclaim policy after the import. The volume gets its StorageClass policy back once it is bound to its VMDisk, so deleting an imported VMDisk deletes its volume instead of leaving an orphaned Released PersistentVolume. Volumes imported before this fix keep Retain and have to be patched back by hand. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Imported volumes are protected during claim transfers, then returned to their StorageClass reclaim policy after the replacement claim and volume are safely adopted. If the class or policy is unavailable, the policy defaults to `Delete`. * VM imports wait for volume adoption and reclaim-policy settlement before completing, helping prevent premature volume deletion and incomplete imports. Previously unrecorded handoff outputs are also settled before being returned. * **Documentation** * Clarified reclaim-policy behavior during imports and how to restore cleanup behavior for volumes from older imports. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
2c2c0301f7 |
chore(deps): roll up pending Renovate updates and move to Go 1.27.1 (#4447)
## What this PR does This replaces the open Renovate PRs with one rollup. Go modules move to their newest releases that keep the Kubernetes libraries and controller-runtime where they are, the toolchain and builder images move to Go 1.27.1, and the first-party code gets the Go 1.27 modernizers. Tests for the code the bumps touch come first, in their own commits, and pass on both the old and the new dependencies. Before the bumps: - The e2e sandbox image builds again: dl.min.io answers 410 for mc since MinIO archived it, so mc now comes from its last GitHub release with a pinned checksum. - Renovate stops offering tags of the apimachinery fork. They mirror upstream, so #2999 is a downgrade that drops our patch. - `make unit-tests` now runs the tests of the nested Go modules (kubevirt-csi-driver, token-proxy, kubeovn-webhook, api/apps/v1alpha1). They never ran in CI before. - New tests: JWT verification in the dashboard token proxy, the kube-ovn plunger metrics, cert-manager object idempotency in the tenant gateway, values-file layering in the operator, and the kubevirt CSI driver wrapper. - One real fix. The lineage webhook decoded admitted objects with encoding/json, so an int64 above 2^53 came back rounded. jsonpatch v2.5 compares numbers exactly and would emit a replace that rewrites such a field in the pod. The webhook now decodes with apimachinery's json. Held back on purpose, with the reasons in the commit messages: - Kubernetes libraries stay on v0.35. #3001 needs the apimachinery fork rebased onto release-1.37 first. - cel-go v0.29.2 and etcd v3.6.15, the lines k8s.io/apiserver v0.35 is built and tested with. - grpc v1.83.2. v1.84.0 is affected by GO-2026-6443 and no v1.84 patch release carries the fix yet. - kubevirt.io/api and the CDI API stay on what upstream csi-driver builds against. From CDI v1.60 it no longer compiles. - api/apps/v1alpha1 keeps go 1.26.4, so consumers of the published types are not forced onto Go 1.27. - go fix runs without embedlit (Go 1.27-only syntax, breaks backports to release branches on Go 1.26) and omitzero (would make metadata, spec and status required in the published OpenAPI schemas), and one errorsastype rewrite that did not compile is left out. govulncheck reports nothing reachable in any of the five modules, standard library included. Supersedes #4311, #4310, #4309, #3452, #3993, #3927, #3902, #3805, #3451, #3415, #3414, #3413, #3302, #4050 and #3363. #3001 stays open. Not checked locally: the images that build upstream projects (kube-ovn, kamaji, cilium, linstor-csi and others) with the Go 1.27 builder. The image build jobs of this PR cover them. Closes #4487. ### Screenshots No UI changes. ### Downstream repositories - [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 ```release-note chore(deps): move the toolchain and builder images to Go 1.27.1 and raise Go dependencies to their latest compatible releases, closing the published advisories; the lineage webhook no longer rewrites int64 fields above 2^53 in admitted objects. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a restore resource policy option for Velero strategies. * Added a test target to run Go tests across tracked modules. * **Bug Fixes** * Updated the end-to-end test environment to download and verify the MinIO client from its current release location. * Added coverage for token validation, webhook patching, controller behavior, and reconciliation. * **Maintenance** * Updated the Go toolchain, dependencies, and container image versions. * Refreshed pinned workflow actions and base image digests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Mattia Eleuteri
|
325feec805 |
fix(migration): restore the class reclaim policy on imported volumes
The handoff sets the transferred PersistentVolume to Retain so the data survives the window in which the Forklift claim is deleted and claimRef is re-pointed at the VMDisk's claim. It never gave the policy back, so every imported disk kept Retain for good while its StorageClass says Delete. Deleting such a VMDisk removes its DataVolume and claim through the normal path and strands the volume Released with its backing storage, billed and out of any tenant's reach. Retain is now held only while it protects something. Once the new claim and the volume both report Bound to each other, the transferred claim is gone, and CDI has adopted the claim into the VMDisk DataVolume as its only owner, the volume gets its class's policy back (Delete when the class names none or no longer exists). The VMInstance is created and the VM reported Succeeded only after that, so a task that fails later still leaves disks that clean up like any other. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Aleksei Sviridkin
|
047c715775 |
chore: clean up stray console file and test fixtures
Drop a local kubeconfig that was committed by accident alongside the console sources and ignore the filename so it cannot come back. Replace environment-specific values in the vSphere migration fixtures with synthetic ones: the tests only care about the shape of the inventory record, not where it was captured. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
7a0aab2bdc |
refactor: apply the Go 1.27 modernizers
Run go fix over every first-party module: interface{} becomes any,
hand-written loops become slices, maps, min/max and range-over-int,
errors.As becomes errors.AsType, pointer helpers become new(expr) and
are inlined at their call sites, and so on. Every rewrite keeps
behaviour.
Three rewrites stay out. embedlit flattens embedded-field literals
such as ObjectMeta, syntax only Go 1.27 accepts, which would break
backports to release branches still on Go 1.26. omitzero drops
omitempty from struct fields, and openapi-gen reads omitempty as
optional, so metadata, spec and status would turn required in the
published schemas. And errorsastype rewrote an errors.As whose
target, apierrors.APIStatus, does not implement error, which
errors.AsType rejects at compile time.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
||
|
Andrei Kvapil
|
54621bc046 |
feat(migration): tenant self-service VM import from VMware vSphere
Hidora's customers are leaving VMware now, and a tenant has to be able to import their own machines without a platform administrator. The chart approach could not express that: a tenant can neither create the Secret holding vCenter credentials nor name the VDDK image, so both had to come from somewhere they cannot reach. Two CRDs in a new `forklift.cozystack.io` group, split by lifecycle. A VMImportSource is a long-lived connection -- type, endpoint, credentials, readiness -- and a VMImportTask is the one-shot operation that names machines and produces VMDisks and VMInstances. The outputs carry no owner reference back to the task, so deleting a finished import leaves the machines alone. Konveyor Forklift does the transfer underneath; the controller renders its objects, mirrors its verdicts, and hands each finished volume into a VMDisk without copying it. What a live vCenter migration forced out, and no amount of source reading would have: - A guest copied verbatim keeps the drivers it had under VMware and no virtio, so on a virtio disk it does not fail the import -- it succeeds and then does not boot. Imported disks go on SATA. - VDDK connects straight to the ESXi host at the address vCenter advertises, which is routinely unreachable, and on the first cluster collided with the Service CIDR. `spec.hosts` redirects per host. - Firmware must be carried across or a UEFI guest imports as BIOS and never boots (stacked on #3002). - Deleting a task mid-transfer orphaned its DataVolume; a finalizer now removes the engine's volumes and leaves the outputs. The console gains a Migration section: connections are editable, and the VM field is a picker built from a machine list the controller publishes, since the aggregated API cannot reach the inventory and should not hold a source's credentials to try. Two general mechanisms fall out of that -- an option source may take an argument, and one named in a CRD annotation may reference a sibling field. Design: cozystack/community#62. Docs: cozystack/website#673. Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |