A Velero restore of a VMDisk or a VMInstance deletes the disk's
DataVolume and restores the PVC under its name, with no owner. The
restored release is in sync, so nothing recreates the DataVolume until
the next Helm upgrade, typically a platform update. That upgrade is
refused by CDI's admission webhook ("Destination PVC ... already
exists") and the release stays failed, followed by the VMInstance
release that looks the disk up.
CDI admits a DataVolume over an existing claim only when the
DataVolume carries cdi.kubevirt.io/allowClaimAdoption or the
DataVolumeClaimAdoption feature gate is on. The restore controller
puts the annotation on the restored PVC, where CDI never reads it, and
the CDI CR does not enable the gate. Setting it on the DataVolume
keeps the scope to vm-disk instead of every DataVolume in the cluster,
and does not depend on a gate CDI intends to remove. A claim CDI
created for this DataVolume carries its uid in createdForDataVolume
and is never treated as adoptable, so a fresh import is unaffected.
The VMDisk backup round-trip now forces an upgrade after the restore,
which is what reaches the failure, and checks that the DataVolume
adopted the restored PVC instead of replacing it.
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
Assisted-by: LLM
## What this PR does
A VM disk created from a golden image into a StorageClass other than the
image's one can hang forever: its temporary clone PVC stays unbound, and
the user sees nothing. With the default `csi-clone` strategy, CDI checks
the source and target classes for a common provisioner, a matching
volume mode and room to expand. It never compares class names. So a
target class backed by the same provisioner gets no host-assisted
fallback, and the clone is left to the CSI driver. LINSTOR creates it
next to the source data, so on a target class that does not allow remote
volume access from every node (a node-local class, for example) the PVC
may never bind.
The vm-disk chart now refuses such a disk at render time and tells the
user which StorageClass to set. The check follows CDI's own clone
planner. The effective strategy comes from the CDI override, or else
from the target StorageProfile. It has to be `csi-clone`, or `snapshot`
when CDI gets a VolumeSnapshotClass of the provisioner. Once the
snapshot path has such a class it runs the same checks as csi-clone, and
LINSTOR ships one. Both classes have to share a driver (the golden's is
read from its PV, as CDI does), the volume modes have to match, and the
target class has to allow expansion. On top of that the chart refuses
only a LINSTOR target whose `allowRemoteVolumeAccess`, read the way
linstor-csi reads it, is not true; without the parameter that means a
layer list with no DRBD or NVMe layer. A remote-accessible LINSTOR class
and other drivers render. When the provisioner or the volume mode
differs, CDI falls back to a host-assisted copy, which completes, so the
disk renders as before. A target class without expansion is left alone
too, see the follow-ups.
The class check compares nothing when a lookup it needs returns nothing:
under `helm template`, before the golden PVC exists, or without the CDI
resource. An empty `storageClass` is compared as the class CDI would
pick for a kubevirt DataVolume, which is the default virt class,
otherwise the Kubernetes default class. Existing disks are exempt,
because their DataVolume spec is immutable. A disk counts as existing
when its DataVolume or its PVC exists, since CDI can garbage-collect a
completed DataVolume. Under `snapshot`, a snapshot class that both
StorageProfiles name but that does not exist is refused too, for any
driver, because CDI then fails to plan the clone instead of falling
back.
This is a breaking change: a new disk in a LINSTOR class without remote
access from every node rendered before and is refused now, and so is one
whose StorageProfiles name a snapshot class that does not exist. The
first case can include a clone that would happen to work: on a class
with `Immediate` binding, which I have not verified, or when the node
the clone is placed on already holds a replica of the golden image. The
way out is the golden image's class or the `copy` clone strategy.
How this covers the three fix directions in the issue:
1. Omitting `cloneStrategyOverride` when it is empty already landed on
main in #4138: the template skips the field for an empty or null value,
the values schema accepts the empty string, and the chart tests pin
both. This PR does not touch it.
2. I kept `csi-clone` as the default on purpose. Same-class clones are
the common case, and `csi-clone` is the fast path for them. Changing the
default is a separate decision, tracked in #4850.
3. vm-disk refuses the mismatch where the clone can hang, as described
above. That is narrower than the issue proposed, which was to refuse
every mismatch: every case CDI falls back on completes, and a clone into
a remote-accessible LINSTOR class should complete too. The last part
comes from reading the CDI and linstor-csi sources, I have not run it on
a cluster.
Follow-ups this PR does not cover:
- When several classes claim the default, CDI picks the newest one. The
chart does not reproduce that tie-break and compares nothing in that
case.
- The chart does not compare sizes. A target class without volume
expansion is never refused for remote access, even when the disk is no
larger than the golden image and CDI would attempt the clone.
- Under `snapshot` on a cluster that does not serve the snapshot API,
StorageProfiles naming a snapshot class still make CDI fail, and the
chart does not look that class up.
- A disk cloned from another disk (`source.disk`) into another class of
the same provisioner can hang the same way, and nothing refuses it yet.
- Whether `csi-clone` should remain the platform default now that it can
be turned off: #4850.
Closes#2908
### Screenshots
Not a UI change.
### 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:
No values, schema or ApplicationDefinition change. Re-check notes for
the next CDI and linstor-csi bumps sit next to the version pins in the
kubevirt-cdi-operator and linstor Makefiles. The new note in the vm-disk
README reaches the website through the regular reference-page
regeneration.
### Release note
```release-note
fix(vm-disk)!: a new disk from a golden image is now refused with a clear error when CDI would CSI-clone it into another LINSTOR StorageClass that does not allow remote volume access from every node. Before, such a disk could stay pending forever with no error. A clone that would have landed on a node already holding a replica of the golden image is refused as well; use the golden image's StorageClass or the copy clone strategy.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Prevented image-backed disks from being rendered when CDI’s selected
clone strategy cannot safely clone across differing storage classes,
including applicable LINSTOR remote-access and snapshot-class checks.
* Existing DataVolumes and same-namespace PVCs are not blocked by these
checks. Disk creation can proceed when required information is
unavailable or the selected strategy supports the operation.
* **Documentation**
* Clarified when image-backed disks may be refused and recommended using
the golden image’s storage class or the copy strategy as alternatives.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
When CDI clones a golden image into a StorageClass other than the one
the golden PVC lives in, its csi-clone path, and its snapshot path once
it gets a VolumeSnapshotClass of the provisioner, check the two classes
for a common driver, a matching volume mode and room to expand, and
never compare the class names. A target class of the same provisioner
therefore gets no host-assisted fallback and the clone is left to the
CSI driver. LINSTOR creates that clone next to the source data, so on a
target class that does not allow remote volume access from every node,
such as a node-local one, the temporary clone PVC stays unbound with
nothing surfaced to the user.
Refuse such a disk at render time, mirroring the planner's inputs: the
CDI override or the target StorageProfile's strategy, the snapshot class
CDI would pick, the golden's driver as its PV records it, the target's
provisioner, the volume modes and the target's expansion setting, and
refuse only a LINSTOR target whose remote access policy, parsed the way
linstor-csi parses it, is not plainly "from anywhere". A snapshot class
both StorageProfiles name but that does not exist is refused as well,
because CDI then fails to plan the clone instead of falling back. An
empty storageClass is resolved the way CDI resolves it for a kubevirt
DataVolume. A disk whose DataVolume or PVC already exists is exempt,
since CDI can garbage-collect a completed DataVolume. Combinations that
fall back to a host-assisted copy, targets reachable from any node, and
anything else the lookups cannot find still render.
This changes behaviour for a new disk in a LINSTOR class without remote
access from every node, and for one whose StorageProfiles name a
snapshot class that does not exist: both rendered before and are refused
now. The way out is the golden image's class, or the copy clone
strategy.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
A disk whose HTTP source answers 404 or does not resolve never leaves
ImportInProgress: CDI fails an import only on a fatal error and restarts
the importer pod otherwise, so the condition only ever said the disk was
still importing. The reason it will not finish was visible only in the
importer logs and on the DataVolume, whose Running condition carries the
error through the back-off between restarts.
Appending that message lets the user see why the disk is stuck from the
application status and the console, which already shows the
WorkloadsReady message for a disk.
Assisted-by: LLM
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
<!-- 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
Stacked on #4392, so the base is `fix/vm-default-images-health-check`.
Retarget to `main` once #4392 merges.
A new vm-disk with `source.image` now fails to render when the golden
image it names does not exist, and the error names the image and
`cozy-public`. Before this the chart cloned
`cozy-public/vm-default-images-<name>` without looking, and CDI waited
for that PVC forever while the VMDisk stayed Ready. That happens on a
typo, and on every cluster where the opt-in
`cozystack.vm-default-images` package is off, so the message also says
to add it to `bundles.enabledPackages`.
The golden counts as missing only when neither its DataVolume nor its
PVC exists. CDI clones the PVC, so a PVC with no DataVolume is still a
usable source, and a DataVolume whose PVC is not created yet is an
import about to start.
Under `helm template` every lookup is empty, so the check runs only when
the release namespace itself is visible. The same trick is already used
in the kubernetes and seaweedfs charts. Existing disks are exempt the
same way #4392 exempts them, because their DataVolume spec is immutable.
A disk refused while the catalog is off renders by itself once the
golden appears, since app HelmReleases retry on failure.
Tests are in `packages/apps/vm-disk/tests/dv_golden_image_test.yaml`:
refusal of a missing golden, a PVC without a DataVolume, a DataVolume
before its PVC, rendering with no cluster, and an existing disk whose
golden was removed.
Closes#4548
### 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. -->
No UI change.
### 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(vm-disk): a new disk with source.image now fails with a clear error when the golden image does not exist in cozy-public, instead of waiting forever for a clone that never starts.
```
## What this PR does
Nothing on a vm-disk application tells whether CDI ever finished
populating the disk. Its WorkloadMonitor has no pods to count, and
`Ready` follows the Helm release. So an import or clone that never
completes can leave a green application over a disk nobody can use.
cozystack/cozystack#2908 is one way to get there: a clone into a
StorageClass that cannot place a replica where the source data sits. The
expensive part there was the silence, not the stuck clone.
The disk state was in the DataVolume phase all along, and nothing read
it. With this change the WorkloadMonitor in cozystack-controller reads
it. The monitor lists the DataVolumes its selector matches and reports
not operational while any of them is in flight, Failed, Unknown or has
no phase. An upload counts as in flight until it completes. That
surfaces on the application as `WorkloadsReady=False` with reason
`DataVolumeNotReady` and a message that names the DataVolume and its
phase, for example `DataVolume vm-disk-x is ImportInProgress`.
`Ready` still follows the HelmRelease, as for any other kind
(`convertHelmReleaseToApplication` in
`pkg/registry/apps/application/rest.go`), and for a vm-disk it no longer
waits on the DataVolume at all. So the disk state has to be visible
outside `Ready`, and this PR puts it where operators look by default:
- `kubectl get` on any application kind gets a `WORKLOADS` column next
to `READY`. It shows the `WorkloadsReady` status, or `<none>` when the
application has no WorkloadMonitor.
- The console badge turns amber and reads `DataVolumeNotReady` while
`Ready` is True and `WorkloadsReady` is not True for that reason. A
failing `Ready` still wins. The console shows only the disk case: other
kinds need a separate decision, because a stopped VM legitimately has no
pod and reports `WorkloadsReady=False` too.
A Helm readiness gate on the DataVolume would be the obvious
alternative, and it is not used on purpose. helm-controller runs that
wait inside its reconcile, so an importing disk would hold one of the
shard's worker slots for the whole install window, and a disk stuck for
good would keep taking the slot back. cozystack/cozystack#4145 has the
details. The monitor costs no slot, keeps watching after install, and
covers existing disks without anyone writing to them.
Without any gate, the install would still wait on the DataVolume.
helm-controller's default poller falls back to a resource's `Ready`
condition for kinds it has no rule for (`checkReadyCondition` in
fluxcd/cli-utils `pkg/kstatus/status/status.go`), so a vm-disk install
could hold a worker slot for the whole install timeout. The
ApplicationDefinition therefore gets a `healthCheckExprs` entry for
DataVolume with `current: "true"`, the same idiom postgres-rd uses for
its ScaledObject. With it Helm never waits on the DataVolume. The entry
is pinned by a helm-unittest in `packages/system/vm-disk-rd`, which gets
a `test` target for it.
Release settings reach a HelmRelease only when the application is
created or updated, so an existing disk gets the entry on its next
update. The reconciler that syncs definitions onto existing releases
copies chartRef, valuesFrom and labels only. That affects any kind with
release settings, so it is filed as cozystack/cozystack#4409 and not
fixed here. `WorkloadsReady` covers every disk right away, since the
monitor needs no write to the application.
CDI copies the DataVolume labels onto the PVC it creates and makes the
DataVolume its controller, so the monitor also publishes a `pvc-<name>`
Workload for the disk. That Workload reads not operational while its
DataVolume is not ready, even though the PVC is already Bound, so
`kubectl get workloads` agrees with `WorkloadsReady`. While the
DataVolumes cannot be read it keeps the verdict it has, like the
monitor.
The WorkloadMonitor CRD now requires at least one selector label. An
empty selector would match every Pod, PVC, Service and DataVolume in the
namespace. No chart renders one.
When an application has several monitors that are not operational and
only some of them name a cause, `WorkloadsReady` carries the named
messages and reason only. A vm-disk has one monitor, so it does not hit
this.
### Which phases count as ready
Settled phases pass: `Succeeded`, `PendingPopulation`,
`WaitForFirstConsumer`, `Paused`. A disk with `bindImmediately: false`
on a `WaitForFirstConsumer` class rests in `PendingPopulation` or
`WaitForFirstConsumer` until a VM consumes it. A multi-stage import
rests in `Paused` until its next checkpoint, and the vm-disk chart
renders no checkpoint source. These report ready while holding no data,
because that is where such a disk is supposed to sit.
`UploadReady` is not one of them. CDI enters it when the upload server
pod is running and ready, and leaves it for `Succeeded` only when that
pod succeeds (`updateStatusPhase` in
`pkg/controller/datavolume/upload-controller.go` at v1.66.1), so the one
phase covers a disk nobody has uploaded to and a half-uploaded one. A
`source: upload` disk therefore reports `WorkloadsReady=False` from
creation until `virtctl image-upload` completes.
A phase the controller does not know also reads as ready, so a CDI
release that adds or renames a phase cannot turn working disks red. The
cost goes the other way: a renamed in-flight phase silently reads as
settled. The list wants a re-read whenever the CDI version moves. It
matches the named phases of the CDI v1.66.1 API, the version vendored in
`packages/system/kubevirt-cdi-operator`.
### Clusters without CDI
CDI comes with the IaaS bundle, while cozystack-controller runs
everywhere, and CDI can be installed while the controller is running. A
watch on a kind whose CRD is absent never syncs, and then the whole
controller does not start. So the watch is not set up at startup. A
runnable adds the DataVolume watch once the kind is served. It waits for
the first reconcile, because only from then on does a new watch start at
once, and then checks discovery right away and every 30s after that. It
installs the DataVolume reader only after the DataVolume informer has
synced within a timeout, because a reconcile context has no deadline and
a List on an informer that never syncs would block every WorkloadMonitor
reconcile. The watch source is started once: a controller-runtime `Kind`
source adds an event handler to the informer and never removes it, so on
a timeout the next check waits on the source it already has instead of
starting another one. RBAC needs no change: the controller's ClusterRole
already has read on everything.
When the DataVolumes cannot be read, because the reader is not installed
yet or the List fails, the monitor still publishes its replica counts
and keeps its last DataVolume verdict, and a List error is returned so
the request is retried and recorded as a `DataVolumesUnavailable`
Warning event on the monitor. The status of a monitor without
DataVolumes is not affected by a failing List. A disk whose verdict is
already recorded does not read as ready in the window after a controller
restart. A monitor with no recorded verdict yet, which is every monitor
right after the upgrade that brings this change and any disk created
right after a restart, reads ready until the DataVolume watch has
synced. Because the watch starts on the first reconcile rather than on
the next discovery tick, that window is the time to sync the DataVolume
informer plus at most one 5s requeue, not a 30s poll interval. The
window is not zero, since a monitor reconciled in it is published before
its DataVolumes can be read. From then on, a reconcile that runs before
the watch has synced is requeued after a few seconds, so the DataVolume
events that queue it are not lost. A pass that changes nothing in the
monitor status does not write it, so that requeue sends no status writes
to the API server. Once the reader is installed, every monitor that
holds a DataVolume verdict is queued once more. So a verdict whose
DataVolume was deleted while the controller was down is recomputed then,
not at the periodic resync.
### Tests
The table test in `workloadmonitor_controller_test.go` covers the named
phases of CDI v1.66.1, plus Failed, Unknown, an empty phase and a
missing status. A second table checks the message and reason for an
importing, a failed, a phaseless and a populated disk, and a third test
lists two stuck DataVolumes in both orders and expects one message. It
also covers a DataVolume outside the selector, the kind not being
served, a List error that still publishes the replica counts, and a
missing reader that keeps the last verdict. The watch tests check that
the watch starts on the first reconcile and not before it. A source that
never syncs leaves no reader and is retried on the same source, with the
reconciles in between still requeued. A List failure records the Warning
event, the sync window records none, and an unchanged pass writes no
status. A manager that stops during the wait gets the cancellation back,
and a reconcile that runs while the watch syncs is requeued without an
error. A table test covers the PVC Workload: importing, populated, a PVC
with no DataVolume controller or a controller of another kind or group,
and the unread cases. A helm-unittest in
`packages/system/cozystack-controller` pins the selector's
`minProperties`. `rest_table_test.go` pins the `WORKLOADS` column,
`rest_workloads_test.go` the condition message and reason, and
`status.test.ts` the console badge, including a halted VM that stays
green.
In `hack/e2e-chainsaw/vminstance` the existing VMDisk scenarios now wait
for the DataVolume to be Ready, then for `Ready` and `WorkloadsReady` on
the VMDisk itself. That replaces the wait on the HelmRelease. Those
asserts would pass even if the monitor ignored DataVolumes, so a new
scenario imports from an `.invalid` URL. It first waits for the importer
to have failed, which CDI reports on the DataVolume as `Running=False`
with reason `Error`, and then for `Ready=True` with
`WorkloadsReady=False` and reason `DataVolumeNotReady` on the VMDisk.
Without the first wait the VMDisk assert passed in about a second, on a
DataVolume that had no phase yet. The e2e runs on a fresh install only,
so nothing here exercises an existing release across an upgrade.
### What it does not do
#2908 lists possible fix directions, and none of them is here. The
cross-class clone still never completes, it just stops being invisible.
Keep the issue open.
While the DataVolume watch syncs, every monitor is still requeued. Only
the ones whose selector reaches a DataVolume need it, but the controller
cannot tell which those are before it can read DataVolumes.
Closes#4449
### Screenshots
There is no screenshot: I had no running console to take one from. The
badge change is pinned by the vitest cases in
`packages/system/dashboard/images/console/apps/console/src/lib/status.test.ts`:
a VMDisk whose `WorkloadsReady` has reason `DataVolumeNotReady` goes
amber, and a halted VMInstance with the generic reason stays green.
### Downstream repositories
I walked the trigger map against the diff.
cozystack/terraform-provider-cozystack is affected: its `wait_for_ready`
reads only `Ready`, so on a `cozystack_vmdisk` it returns before the
disk is populated once this lands. That needs a decision on the provider
side, so it is an issue there. The release-time bot regenerates
`packages/apps/vm-disk/README.md` into the website's virtualization
pages, and `vm-disk` is in the `VMS` list that generator reads, so the
website needs no follow-up. The ApplicationDefinition keeps its kind,
plural, prefix and path.
- [ ] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [x]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
https://github.com/cozystack/terraform-provider-cozystack/issues/59
- [ ]
[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
fix(vm-disk): report whether CDI finished populating a disk in the WorkloadsReady condition, which names the stuck DataVolume and its phase. kubectl get shows WorkloadsReady in a new WORKLOADS column for every application kind, and the console badge turns amber while a disk is not populated, so a disk whose import or clone never completes no longer looks healthy. Ready of a new or updated vm-disk no longer waits for the DataVolume, so wait on WorkloadsReady to know the disk is populated.
```
Ready follows the Helm release; whether CDI finished is in
WorkloadsReady. Say so and where an operator sees it, say that a
disk created before this change keeps a release that may wait on the
DataVolume until its next update, say that an upload disk reads not
ready until the upload completes, and say that a disk waiting for its
first consumer counts as ready, so it is not read as broken.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
A new disk on source.image turns the name into a clone of
cozy-public/vm-default-images-<name> without looking it up. When that
golden does not exist, because of a typo or because the opt-in
vm-default-images package is not enabled, CDI records
CloneWithoutSource on the DataVolume and waits forever, while the
VMDisk stays Ready and the user never sees why.
Fail the render instead, naming the image and cozy-public, the same way
a golden whose import Failed is already refused. The golden counts as
missing only when neither its DataVolume nor its PVC exists: CDI
clones the PVC, so a PVC with no DataVolume is a usable source, and a
DataVolume whose PVC CDI has not created yet is an import about to
start. An empty lookup only means "missing" when the release namespace
itself is visible; under helm template every lookup is empty and the
chart must still render. Existing disks stay exempt, as their
DataVolume spec is immutable.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
A disk with source.image clones a golden from cozy-public without
looking at it. When that golden's import has failed, CDI never
populates it, so the clone waits forever and the new disk sits pending
with nothing pointing at the image.
Fail the render instead when the golden's phase is Failed, naming the
image. A golden still importing is left alone because CDI holds the
clone until the source is populated. An existing disk is exempt: its
DataVolume spec is immutable, so failing the render cannot rescue it
and would only block its upgrades.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Assisted-by: LLM
Immediate binding for every source fixed a real bug: a standalone disk
on a WaitForFirstConsumer class has no consumer to wait for, so it never
populated. It also took away the path that is correct for the other case.
For a disk a VMInstance consumes, KubeVirt renders a temporary launcher
pod carrying the VM's own affinity and nodeSelector, so the volume binds
where the VM can actually run; with the annotation always present the CDI
worker becomes the first consumer instead and the volume lands where that
pod happened to be scheduled. On a node-pinned class the VM is then
unschedulable against a node-affinity conflict, and storageClass is
immutable, so the only exit is deleting the disk.
The two cases want opposite defaults and only the operator knows which
one a given disk is. bindImmediately defaults to true, so nothing moves
for existing disks or for anyone on `replicated`, which binds Immediate
either way. Both branches are pinned in tests, including the
existing-DataVolume path, which renders its own annotation block.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: LLM
Requesting immediate binding for every source, not just `upload`, is
deliberate: a VMDisk is a standalone object populated before any VM
consumes it, so on a WaitForFirstConsumer StorageClass a disk with no VM
attached would never populate at all.
What that costs was recorded nowhere an operator reads. On a node-pinned
class the CDI worker pod chooses the node, so the volume binds where the
importer landed rather than where the VM will run, and a VM constrained
elsewhere -- a nodeSelector, a request only one node satisfies, or the
Windows affinity from _cluster.scheduling.dedicatedNodesForWindowsVMs --
stays unschedulable against a node-affinity conflict. One disk is enough
for that; it is not a multi-disk-only case. storageClass is immutable,
so the exit is delete and recreate. The chart default `replicated` binds
Immediate regardless and does not move.
Also pinned in tests, in both directions the annotation is emitted: per
source type (blank, image, disk) so a regression to the old upload-only
condition reddens, and on the existing-DataVolume path, where `helm
upgrade` retrofits the annotation onto disks created before it was
unconditional -- inert for a volume that has already bound, and now a
recorded decision rather than an unasserted side effect.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: LLM
A VMDisk is a standalone object that is populated before any VM consumes
it, so on a WaitForFirstConsumer StorageClass CDI needs an explicit
signal to schedule its worker and start provisioning. Without it the
DataVolume sits in PendingPopulation indefinitely and the disk is never
filled -- and nothing says so.
This is a consistency fix rather than a new behaviour. The chart default
`replicated` is volumeBindingMode: Immediate, so a disk on it already
bound and populated at creation; `local`, the cluster's default class,
is WaitForFirstConsumer, so the same VMDisk silently did nothing there.
The annotation was previously emitted only for source: upload, which is
the one source that could not work without it, leaving http, image, disk
and blank to differ by class.
Two consequences worth knowing. CDI checks the annotation by presence
and ignores its value, so "" and "true" are equivalent and this is not a
behaviour change on the upload path. And on a node-pinned class each
disk now binds at its own creation rather than at VM scheduling, so a
multi-disk VM on `local` can find its disks on different nodes -- that
is the existing limitation of a node-pinned class, which is why the
chart default is not one, but it is reached earlier now.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Editing a VMDisk's storageClass or source after its DataVolume already
exists was silently ignored: dv.yaml reused the existing DataVolume.spec
verbatim (DataVolume.spec is immutable in CDI), so the edit rendered as a
no-op while the HelmRelease reported success and the CR diverged from the
cluster.
When a DataVolume already exists, fail with a clear message if the
requested storageClass or source differs from what the disk was created
with. Both compare the operator's input recorded in an annotation at
creation (vm-disk.cozystack.io/storage-class and .../source), written only
at creation and preserved verbatim afterwards — never overwritten with an
unapplied edit. Recording the input rather than reading the live object
matters on both axes: the image-clone PVC name carries a chart-version
prefix (vm-image- was renamed to vm-default-images-, commit bec35e3a) that
drifts across upgrades with no edit, and the live PVC storageClassName of an
adopted or foreign DataVolume may legitimately differ from the CR default
without being a change. The annotation key presence (not its value) marks a recorded class, so a disk
created with the cluster default "" is recorded and still caught if later
changed to a concrete class.
Disks with no annotation (created earlier, or out-of-band) are handled
conservatively so a working release never deadlocks: source is compared
against the stored spec.source only when that stored source is a shape the
chart itself produces, so an adopted DataVolume using spec.sourceRef or a
non-chart source (registry, snapshot, http+secretRef, ...) is left alone
rather than deadlocked. The image-clone exemption fires only when the stored
source is a cozy-public pvc AND the request is itself an image, so a change
away from an image source to another type is still caught. storageClass has
no safe live fallback, so an un-annotated disk is not guarded here (apiserver
CEL, #2657, is the eventual guard). An unrecognized source type
now fails explicitly. Storage growth is unaffected and still handled by
pvc-resize-hook.yaml.
Mark source as immutable in values.yaml so the generated schema carries
the same x-kubernetes-validations rule already present on storageClass,
and document the contract, the exemption, and the source-annotation schema
coupling in docs/storage-immutability.md and the chart README.
Add helm-unittest coverage for the immutability guard (rejection incl.
clearing storageClass to empty and changing a non-image disk to image,
no-op reuse with annotations preserved, legacy image-disk drift, annotation
non-poisoning, un-annotated storageClass skip, adopted default-class disk,
unknown source) and initial-creation source rendering.
Fixes#2985
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
The vm-disk pvc-resize, cozystack-api, seaweedfs and vm-instance update
hooks (whose alpine/k8s image this PR refreshes) ran as root with no
securityContext. Add a pod-level runAsNonRoot + runAsUser 65534 +
seccompProfile RuntimeDefault and a container-level
allowPrivilegeEscalation: false + capabilities drop ALL, mirroring the
dashboard adopt hook. kubectl needs no root, so this is behavior-neutral.
Each hook is lookup-gated; pin the securityContext in a per-chart
helm-unittest that mocks the gating object (DaemonSet / PVC / Service) via
kubernetesProvider so the guard renders without a cluster. vm-disk and
vm-instance had no Makefile test target, so add 'test: helm unittest .' to
both so CI runs the new suites.
Scope note: other kubectl hooks on clastix/kubectl (mariadb, keycloak,
kubernetes, tenant) are untouched here — they are not part of this
alpine/k8s refresh and belong to a separate hardening pass.
Also fix the vm-disk resize hook's backoffLimit: it was nested under the
pod spec (silently ignored, leaving the Job at the default 6 retries)
instead of Job.spec. Move it to Job.spec and pin it in the test.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Six further references to the alpine/k8s tools image were still pinned at
1.33.4 across the installer, the cozystack-api post-upgrade hook, the vm-disk
and vm-instance resize hooks, the vertical-pod-autoscaler CRD-install override,
and the e2e cilium leak healer. Move them all to 1.36.2 so the whole repo runs
a single current tools-image version (the two digest-pinned refs get the
matching multi-arch digest sha256:44ef4942...). Same stale-stdlib remediation
as the seaweedfs bump in this PR.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Move the dynamic-dropdown x-cozystack-options vendor extensions from the
generated values.schema.json into the values.yaml source for vm-instance,
kubernetes, vm-disk and bucket so they survive schema regeneration. The new
cozyvalues-gen @x-<keyword> directive emits these extensions from the source
annotations. Annotations on typedef @field entries (Disk, Network, GPU,
NodeGroup) are declared once and cover every place the typedef is reused.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
Mark fields that reference cluster resources with the x-cozystack-options
vendor keyword so the dashboard renders them as dynamic dropdowns:
GPU, instanceType, instanceProfile, network (vm-instance), GPU/
instanceType (kubernetes nodeGroups), image (vm-disk), storagePool
(bucket), storageClass (all apps) and disks[].name (vm-instance). Drop
the stale static instanceProfile enum.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
Annotate storageClass as immutable across every stateful app whose
chart exposes one — clickhouse, foundationdb, harbor, http-cache,
kafka (Kafka + ZooKeeper), kubernetes (top-level only), mariadb,
mongodb, nats, openbao, opensearch, postgres, qdrant, rabbitmq,
redis, vm-disk. Without coverage across the catalogue the UI would
render storageClass read-only on some apps and editable on others —
a worse mental model than uniform behaviour.
The annotation lands at the schema layer (CEL XValidation rule
'self == oldSelf' on values.schema.json / types.go / embedded
openAPISchema in cozyrds/<app>.yaml). The cozystack aggregated
apiserver does not yet evaluate CEL on application-backed resources;
that is tracked at cozystack/cozystack#2657. The UI
(cozystack/cozystack-ui#6) consumes the marker today.
The PVC-pinning argument is identical for every chart: Kubernetes
fixes a PVC's storageClassName at creation time, so editing this
field on an existing resource never migrated data. The schema
annotation makes that contract explicit.
kubernetes.nodeGroups[name].storageClass is intentionally NOT
annotated: the field is optional and undefaulted, so a strict
'self == oldSelf' rule would block any future attempt to set it on
an existing node group. A richer ratcheting form is needed there —
see docs/storage-immutability.md.
A refactor in pkg/registry/apps/application/rest.go extracts the
inline schema-build path into buildSpecSchema so it can be
unit-tested. Three regression tests in rest_spec_schema_test.go pin
the contract that x-kubernetes-validations survives the
v1->internal->structural conversion (loaded from the actual MariaDB
ApplicationDefinition fixture on disk, not a hand-edited mock).
Each per-app README gains a one-line blockquote pointing to a new
docs/storage-immutability.md that explains the contract and which
consumers enforce it today.
Requires cozystack/cozyvalues-gen with @immutable support — bump the
pre-commit pin in .github/workflows/pre-commit.yml once the release
exists.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The \$dv variable assigned by the lookup call on the image PVC was never
read. Remove the dead line to keep the template tidy.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Move common-envs.mk and package.mk from scripts/ to hack/ directory.
Update all Makefile includes to use new paths. Remove unused
issue-flux-certificates.sh script.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
[cozystack-controller] Introduce new dashboard-controller
[dashboard] Introduce new dashboard based on openapi-ui
Co-authored-by: kklinch0 <kklinch0@gmail.com>
Signed-off-by: kklinch0 <kklinch0@gmail.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Mattia Eleuteri