Addresses the branch review on the ZK->KRaft migration path.
CRITICAL — broker node pool adoption. Strimzi derives broker/PVC names as
<cluster>-<pool>-<id>; a pre-node-pool ZooKeeper cluster has brokers
<cluster>-kafka-N, so the adopting broker pool must be named exactly "kafka"
or the operator provisions fresh empty brokers and orphans the tenant's data.
A new kafka.brokerPoolName helper resolves the broker pool to "kafka" for an
existing (migrating) cluster and keeps the release-scoped "<release>-broker"
for fresh installs, so multiple Kafka instances still coexist in one namespace.
The migration hook creates the pool as "kafka" and fails closed if a "kafka"
pool already belongs to another cluster (node pool names are namespace-unique,
so in-place migration of co-namespaced ZK clusters is unsupported). Wired
through kafkanodepools.yaml, workloadmonitor.yaml and the hook.
MAJOR — migration robustness.
- Raise the HelmRelease timeout: the migration Job can run tens of minutes but
the release timeout defaults to 10m, so helm-controller aborted and thrashed
the pre-upgrade hook. Add release.cozystack.io/helm-install-timeout=45m on the
kafka ApplicationDefinition, above the Job's activeDeadlineSeconds (2400s).
- Idempotent migration: do not re-issue kraft=migration once the cluster is past
the ZooKeeper phase (an invalid backward transition Strimzi rejects on retry).
- Fail closed on the decision gates: distinguish a genuine NotFound (skip) from
any other kubectl error (abort), and never treat an empty kafkaMetadataState on
an existing CR as "migrated".
MINOR — version downgrade documented as unsupported; integer (not lexical)
compare in the version gate; deleteClaim/PVC-retention and the one-per-namespace
migration limitation documented in the README; docs/storage-immutability.md
updated for KRaft.
Tests — helm-unittest coverage for the broker pool name, the hook guards and the
idempotent/fail-closed logic; a bats check for the helm-install-timeout budget
ordering; and a Chainsaw ZK->KRaft migration suite that seeds a ZooKeeper
cluster, drives the migration, and asserts the original broker PVC survives (not
recreated) and the cluster reaches KRaft. The migration sequence and the
Chainsaw assertions were validated against a live cluster.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
The dashboard no longer registers a brand icon for the kind, and the
ServicesTab fixture moves to TCPBalancer, which the catalog still ships.
An instance that survives the removal still renders, on the generic
sidebar icon.
api/apps/v1alpha1/httpcache goes too. types.go is generated by
values-gen out of packages/apps/http-cache, which is gone, so the file
could never be regenerated, and no Go file in this repository imports
it. Surviving instances do not depend on it: the aggregated API serves
the schema recorded in the in-cluster ApplicationDefinition. The
terraform provider's cozystack_httpcache resource is hand-written and
does not import it either; only its per-kind schema-guard test does,
and that goes away with the resource.
Assisted-by: LLM
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
## What this PR does
Fixes#2985. Editing a `VMDisk`'s `storageClass` or `source` after its
DataVolume already exists was silently ignored:
`packages/apps/vm-disk/templates/dv.yaml` looks up the existing
DataVolume and, when found, reused its `spec` verbatim (because
`DataVolume.spec` is immutable in CDI). The edit therefore rendered as a
no-op — the typed CR accepted it and the HelmRelease reported success,
while the DataVolume/PVC kept the original spec and the source of truth
silently diverged from the cluster.
The template now builds the desired `source` into a shared
`$desiredSource` variable (used both for rendering and comparison) and,
when a DataVolume already exists, calls `fail()` with an explicit
message if the requested `storageClass` or `source` differs from the
immutable existing spec — mirroring the explicit size handling already
done in `pvc-resize-hook.yaml`. Storage growth is unaffected and still
flows through the resize hook.
`source` is also marked `@immutable` in `values.yaml`, so the generated
schema now carries the same `x-kubernetes-validations` transition rule
(`self == oldSelf`) that `storageClass` already had; `types.go`,
`values.schema.json` and the generated `vm-disk-rd` CRD are regenerated
to match.
### Verification
- `helm unittest` covers both the immutability guard (via a mocked
existing DataVolume through `kubernetesProvider`, since `lookup` needs a
cluster) and initial-creation source rendering for every source type —
12/12 pass.
- Verified live on a dev cluster with `helm install --dry-run=server`
against a real DataVolume: an unchanged edit renders, a `storageClass`
change fails, a `source` change fails, and a storage grow still renders
the resize hook without tripping the guard.
### Scope note
This PR only makes the drift visible and marks `source` immutable. Two
deeper platform findings sit outside its scope: (1) #2657 (pre-existing)
— the `apps.cozystack.io` aggregated apiserver does not evaluate the
`x-kubernetes-validations` CEL rules on update, so `@immutable` is a
no-op at admission for every Application kind, which is exactly why the
chart-level `fail()` — not the CEL rule — is the substantive guard here;
(2) #3971 — that same apiserver does not honour `--dry-run=server` and
persists the change (surfaced while testing this fix).
### Downstream repositories
Walked the trigger map in `docs/agents/contributing.md` against the diff
file by file. The diff touches only `packages/apps/vm-disk/` (template,
`values.yaml`, regenerated `values.schema.json`, tests), the regenerated
`packages/system/vm-disk-rd/cozyrds/vm-disk.yaml`, and
`api/apps/v1alpha1/vmdisk/types.go`. No package was added, renamed or
removed; no `values.schema.json` field was added, renamed or removed
(only a validation rule on the existing `source` field); no default
changed; `README.md` is unchanged; the `cozyrds` file was regenerated,
not renamed. The terraform provider models fields, defaults and enums
but not CEL immutability, so it is unaffected. No downstream repository
is reached by this change.
- [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:
### Release note
```release-note
fix(vm-disk): editing storageClass or source on an existing VMDisk now fails with a clear message instead of being silently ignored; delete and recreate the disk to change these immutable fields.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added support for blank, HTTP, image, disk, and upload VM disk
sources.
* VM disk sources and storage classes are now protected from unintended
changes, while storage expansion remains supported.
* Existing disks preserve their configured source and storage settings.
* **Bug Fixes**
* Invalid or conflicting source configurations are rejected.
* Prevented silent no-op updates and incorrect immutability errors for
existing disks.
* **Documentation**
* Documented source and storage class immutability, including the need
to recreate disks when changing either setting.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Add the Valkey e2e bats test (mirrors redis), build the compat image in the
top-level build target, map the valkey scope to area/database in the PR
labeler, and list valkey among charts with an immutable storageClass.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
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 doc still claimed the kubernetes-nodes per-pool storageClass is intentionally
not immutable and undefaulted. This branch annotated it immutable (self == oldSelf
XValidation, @immutable in values.yaml) and it defaults to replicated, and the
cluster field is immutable too. Rewrite the paragraph to describe both fields,
add kubernetes-nodes to the covered-apps list, and drop the now-stale
top-level-only caveat on the kubernetes entry.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Remove dead nodeGroups blocks from the CI values fixtures and point the
storage-immutability doc at the kubernetes-nodes per-pool storageClass.
Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com>
The console Containerfile ran `corepack prepare pnpm@latest`, so the image build could drift to a newer pnpm major than the `pnpm@11.1.2` pinned in package.json and used by CI. `--frozen-lockfile` guards the lockfile, but pinning the builder makes the image build reproducible and consistent with CI.
Also repoints docs/storage-immutability.md off the now-vendored cozystack-ui#6 link to the in-tree console path (follow-up from #2963).
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
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>
Alexey Artamonov