Commit Graph
7 Commits
Author SHA1 Message Date
Aleksei Sviridkin 548d33f01f build(images): fall back to direct and retry Go module downloads
A transient proxy.golang.org HTTP/2 stream reset fails an image build
outright: Go's default GOPROXY falls back to direct only on a 404 or
410. Image builds now get GOPROXY=https://proxy.golang.org|direct as a
build arg from hack/common-envs.mk, declared by every Go builder stage,
so any proxy error falls back to fetching from the origin. The value
comes from IMAGE_GOPROXY rather than the host's GOPROXY, which may
name a mirror that buildkit cannot reach. The alpine golang images
carry no git, so in their stages a module hosted on GitHub cannot be
fetched directly and the retry is the cover.

Explicit `go mod download` steps retry three times with a growing
pause. The loop is inline, in one form the contract test pins: the
images build from different contexts, so no shared script can be
copied in, and a named build context in the shared buildx arguments
makes buildx refuse every Dockerfile pinned to a frontend older than
dockerfile:1.4.

The two alpine stages that left the download to `go build` get the
same download step, because without git neither the fallback nor a
retry covered them.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-10-01 19:37:00 +03:00
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 -->
2026-09-29 14:41:04 +03:00
Mattia Eleuteri c6a4933cf6 test(migration): cover the recovery settle and the handoff guards
Recovering outputs that no status recorded goes through fulfill's
vm == nil branch, which settles the reclaim policy of volumes an older
controller left on Retain. Nothing drove that branch, so the write path
could be removed with the suite still green. A fulfill test now runs it
with the handoff open and finished.

The guards that stop the controller from rewriting the policy of a
volume it did not produce, or before CDI controls the claim, get one
case each: a claim or VMDisk marked by another import, a non-controller
DataVolume owner reference, and a claimRef naming the replacement claim
under a stale UID.

The storageclasses RBAC comment now also names the reclaim policy read.

Assisted-by: LLM
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
2026-09-29 09:37:25 +02:00
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>
2026-09-25 12:07:23 +02:00
Aleksei Sviridkin ec7387109e build(go): move the toolchain and builder images to Go 1.27.1
Go 1.26.4 carries standard-library advisories in net/http, net/url,
crypto/tls, encoding/asn1 and html/template that govulncheck reports
as reachable from the first-party binaries. The shipped stdlib comes
from the builder image, so the builder tags move together with the
go directive; the digests are the multi-arch indexes that golang:1.27,
1.27-alpine and 1.27-bookworm resolve to at 1.27.1.

api/apps/v1alpha1 keeps go 1.26.4. It is a published types-only
module, no binary here links its stdlib, and raising its directive
would force every consumer onto Go 1.27.

linstor-csi v1.11.2 pins grpc v1.81.0, which no longer builds on
Go 1.27: golang.org/x/net drops http2.TrailerPrefix there. Its image
now raises grpc to v1.83.2 before building, as the kamaji image does
for its advisories; the upstream unit tests pass on it.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-24 17:23:18 +03:00
renovate[bot] a7dfbe6f8c chore(deps): update container image & action digests
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-09-24 16:55:04 +03:00
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>
2026-09-07 17:03:44 +02:00