mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-30 16:47:20 +09:00
release-1.6
266
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 myasnikovdaniil
|
2f820f7e6a |
[Backport release-1.6] fix(tenant): skip Cilium policies on isp-hosted without Cilium (#5080)
## What this PR does Backport of #4710 to release-1.6. - The traces-central templates do not exist on release-1.6, so their hunks and the matching unit-test assertions are dropped, a test case that relied on a main-only cozy-lib change uses `_cluster: {}` instead. ### 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: ### Release note ```release-note fix(tenant): the tenant chart no longer fails to install on the isp-hosted variants when the host has no Cilium. There, the Cilium network policies are created only if the host serves the Cilium policy API; otherwise tenants get no network isolation from Cozystack. Other variants are unchanged. ``` |
||
|
Aleksei Sviridkin
|
4f8c0a6fda |
fix(tenant): skip Cilium policies on isp-hosted without Cilium
The isp-hosted variants install no CNI of their own, so on a host
without Cilium the cilium.io/v2 kinds do not exist and the tenant
install fails, taking the root tenant and everything that depends on
cozystack-basics down with it.
The policies now render on isp-hosted only when the host serves both
Cilium policy kinds. On every other variant Cozystack installs Cilium
itself, so they keep rendering unconditionally: a CRD that is missing
at render time there fails the install loudly instead of leaving the
tenant without network isolation, because helm-controller does not
re-render a release when only API discovery changes.
On isp-hosted without Cilium the tenant gets no network isolation from
Cozystack; isolating tenants is then up to the host's CNI.
release-1.6 has no shared-central tracing templates, so their hunks,
the tenant.tracingCentralEgress helper beside the new one and the
tracing cases of the test suite are left out. cozy-lib on this branch
fails on an absent _cluster, as the rest of the chart already does,
so the no-_cluster case runs with an empty _cluster instead.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
(cherry picked from commit
|
||
|
Aleksei Sviridkin
|
a08713ef7e |
fix(platform): accept the container gpu-operator variant in iaas
The cozystack.gpu-operator PackageSource ships three variants: default,
vgpu and container. The iaas bundle's variant guard accepted only the
first two, so selecting container failed the render on a value the
package ships, values-container.yaml configures deliberately, and the
variant bats suite already covers.
The exclusion was coherent when it was written and stopped being so
immediately. The container variant and the guard landed within minutes
of each other from parallel branches, neither an ancestor of the other,
variant first. Until the guard, the bundle hardcoded the default variant
and container was selected by hand-crafting a Package CR, so leaving it
out of a bundle-side list cost nothing. The guard then made the bundle
the variant owner and closed that route without noticing it was one; its
comment describes itself as typo protection and names passthrough as the
example typo.
The KubeVirt side is not a bare allowlist edit. The container variant
serves GPUs to pods through the device plugin and runs no VM-facing
GPU stack: sandboxWorkloads and vfioManager are both off and the host
driver stays bound. Both KubeVirt-side pieces the bundle wires are
VM-facing — the HostDevices feature gate advertises a capability such
a node cannot serve, and permittedHostDevices lists resources no
sandbox device plugin registers there. Both are now emitted only for
default and vgpu, and container yields the gpu-operator Package with
the kubevirt component untouched.
Comments and docs that described the KubeVirt mirroring as
unconditional on variant, or the container variant as unmerged and
unavailable, are corrected to match.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
(cherry picked from commit
|
||
|
cozystack-ci[bot]
|
7dfc094583 |
docs: add changelog for v1.6.5
Signed-off-by: cozystack-ci[bot] <274107086+cozystack-ci[bot]@users.noreply.github.com> |
||
|
Aleksei Sviridkin
|
f3223c33d3 |
ci(e2e): run nightly and release-tag e2e on the container lane
Nightly and the rc gate still booted the management nodes as QEMU
guests, the substrate the merge-gating lanes left behind. On it the
tenant Kubernetes suites have missed their node-join deadline on most
nights since 2026-10-02, each failure spends the whole deadline, and
the job reaches its timeout before the alphabetically later suites
run. Over the same days those suites passed on every PR run that
selected them, where the nodes are Talos containers and a tenant
worker sits one virtualization level shallower.
Both lanes now use the PR lanes' container substrate. Nightly's e2e no
longer waits for the nocloud disk, which it does not boot; the disk is
still built and published. A manual e2e-tag dispatch against a tag
that predates the container lane fails early and names the ref to
dispatch from instead.
DRBD, the replicated StorageClass and the Cozystack Talos node image
lose their last e2e coverage with this. The docs and comments that
pointed at nightly and the rc gate as that coverage now say so.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
(cherry picked from commit
|
||
|
cozystack-ci[bot]
|
de56e7b5b8 |
docs: add changelog for v1.6.4
Signed-off-by: cozystack-ci[bot] <274107086+cozystack-ci[bot]@users.noreply.github.com> |
||
|
Andrei Kvapil
|
cb67da1016 |
fix(nightly): rewrite an image host that is the whole scalar value
The nightly host rewrite is a literal "<src-registry>/" substring
replace, so it only reaches a host sitting contiguously in front of the
repository. kubeovn keeps its host in global.registry.address with the
bare repository in a sibling key: no trailing slash, no match. The
image was mirrored to the public registry and the tree kept pointing at
the private build registry.
The gap was dormant while kubeovn's image came from a separate
repository, because the baked ref carried the public host and the
ownership filter skipped it. Building the image in-tree stamps the
build-registry host into that key and turns it live, so fix it here.
Add an end-of-line-anchored expression that fires only when the source
registry is the complete value; a contiguous ref continues past the
host, so the two expressions cannot both fire on one reference.
keycloak-operator splits at a different boundary and stays unfixed --
neither of its keys holds the source registry as a whole. Documented,
and dormant while no CI path rebuilds it.
Assisted-By: Claude
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
(cherry picked from commit
|
||
|
Myasnikov Daniil
|
1a0a4dd952 |
docs(agents): describe the container lane as the merged standard
The container lane moves out of "In-flight direction (not yet the merged
standard)" and into the body, because it is now what both merge-gating
lanes run. The node-join section is rewritten the other way round: it
described a deadline that failed the suite without blocking the lane,
and there is no longer any such carve-out to describe.
What the section gains is the part that was missing while the lane was a
spike -- what moves off the per-PR path with the substrate, and where it
still gets covered. DRBD and the replicated StorageClass were already
named; Immediate binding, tenant StorageClass propagation and the
Cozystack Talos node image were not, and all three now say nightly and
release-candidate rather than reading as lost. The host CPU and memory
visibility gap is stated too, since it is what a reader meets first on a
red run and it looks like a product bug.
Adapted for release-1.6: this branch never had the node-join soft-red
carve-out, so there is no section to rewrite for it, and nightly.yaml
does not run from here, so the QEMU coverage the section points to is
e2e-tag.yaml alone. The section also describes the e2e-side settings
that stand in for chart values this branch does not have -- the
no-drbd satellite configuration, CSI strict topology, the CDI worker
memory ceiling and immediate binding for VM disks -- and it lands after
them for that reason.
(cherry picked from commit
|
||
|
cozystack-ci[bot]
|
c2baf16255 |
docs: add changelog for v1.6.3
Signed-off-by: cozystack-ci[bot] <274107086+cozystack-ci[bot]@users.noreply.github.com> |
||
|
cozystack-ci[bot]
|
269d339c0c |
Prepare release v1.6.2 (promoted from v1.6.2-rc.1)
Signed-off-by: cozystack-ci[bot] <274107086+cozystack-ci[bot]@users.noreply.github.com> |
||
|
Mattia Eleuteri
|
c6fc034fed |
fix(backupstrategy-controller): repair the bucket credentials Secret from the gate, not a chart fail
Addresses the four findings on #3524.
**The `fail` in the bucket chart (MAJOR).** The <bucket>-<user>-credentials
Secret was made to `fail` the render while its COSI lookup was empty, so the
release would retry and converge instead of skipping the Secret permanently.
Helm cannot render a partial set, so that fail aborts the WHOLE
<bucket>-system release: the other users' Secrets, and the bucket UI
Deployment/Service/Ingress/HTTPRoute with them. One declared user whose
BucketAccess never provisions takes down every other user of that bucket,
and on an already-installed release parks it in Failed, blocking every later
upgrade.
**No per-bucket escape (MAJOR).** Rather than add one, remove the need for
it. The gate already forces the release that renders the lookup-gated
Strategy CRs; the credentials Secret is the same trap one release earlier,
and the gate reads that Secret to resolve the bucket name in the first
place. So while it is absent, or carries no bucket name, force the bucket's
<bucket>-system release instead of doing nothing. That cannot deadlock its
own precondition: the BucketClaim and the BucketAccess whose COSI Secret the
lookup reads are rendered unconditionally by the parent release, not by the
one being forced.
The two releases are throttled independently — they are forced in sequence
on a bootstrap, so a shared timestamp would delay the second by a full
MinForceInterval for no reason. Coordinates come from the chart under
provisionBucket, and are omitted on external S3 where the Secret is
admin-managed and no release renders it. No RBAC delta: patch on
helmreleases is already cluster-scoped.
The chart goes back to a partial render, with the comment now explaining why
the skip is not self-healing and why failing is the wrong lever, and a test
that renders the UI with every user unresolvable so a reintroduced `fail`
fails the suite. Tenant buckets keep their pre-existing behaviour;
generalising the repair to every bucket needs a controller that owns them
and is tracked separately.
**Stale gauge (MINOR).** cozystack_backup_default_objects_missing was only
written on the happy path, so the state the gate exists to catch reported 0
and the documented alert never fired. It is now written on the unresolved
path too — the credentials Secret counts as one of the objects the default
backups depend on, which is what it always was. On an API error the gauge is
deliberately left alone rather than flapping, so a new
cozystack_backup_default_objects_check_errors_total marks it stale, and the
doc pairs the two instead of overstating the gauge.
**Suspended releases (MINOR).** forceHelmRelease now does a point Get and
skips a release with spec.suspend: true. helm-controller ignores forceAt and
requestedAt while suspended (`cozyhr suspend` sets exactly that), so the gate
was re-stamping every MinForceInterval for the whole suspension and climbing
the force counter — which the runbook attributes to a render that is not
producing the objects, the wrong diagnosis. The skip is logged and not
counted, so a climbing counter keeps its documented meaning.
Reported-by: IvanHunters <xorokhotnikov@gmail.com>
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
Assisted-By: Claude <noreply@anthropic.com>
(cherry picked from commit
|
||
|
Mattia Eleuteri
|
9244be9aa0 |
fix(backupstrategy-controller): repair lookup-gated backup objects
The default Strategy CRs and the Velero BackupStorageLocation are
Helm-templated behind a `lookup` of the BucketClaim the same chart
creates, and the <bucket>-<user>-credentials Secret behind a `lookup` of
the COSI Secret. When those lookups are empty at install time the objects
were silently skipped, and helm-controller does not re-render a release
whose chart and values did not change (drift detection is off on
operator-generated HelmReleases), so the skip was permanent: clusters ran
for months with only BackupClass cozy-default and no Strategy CRs at all.
That also fail-closes the pre-adoption snapshot in the v1.6.0 etcd
migration, which reads the projected credentials Secret.
Add a DefaultObjectsGate runnable to backupstrategy-controller. Once the
bucket name is resolvable from the projector's source Secret, it checks
that every object cozy-default routes to exists and forces one real Helm
upgrade (reconcile.fluxcd.io/forceAt + requestedAt) when any is missing.
Helm remains the objects' only author; the gate only makes sure the render
that produces them actually happens. It cannot be a render-time `fail` in
this chart: the chart is the producer of the Bucket its own lookup reads,
so a failed render would deadlock the condition.
The bucket chart's user-credentials template CAN fail safely — the
BucketAccess that produces the COSI Secret belongs to the parent release
and that release retries forever — so make it fail loudly instead of
skipping, with requireUserCredentials=false as the offline-render escape.
Also correct the chart and doc comments that claimed Flux re-renders on
its interval, and document the two-release manual recovery for clusters
already affected.
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
(cherry picked from commit
|
||
|
cozystack-ci[bot]
|
781328ebed |
docs: add changelog for v1.6.1
Signed-off-by: cozystack-ci[bot] <274107086+cozystack-ci[bot]@users.noreply.github.com> |
||
|
Myasnikov Daniil
|
a4d73b2346 |
fix(seaweedfs): read the chart name first-match, fail the fake on unmodelled calls
Review findings from #3436, plus what an adversarial re-check of the first
attempt at them turned up.
The chart-name extraction was a greedy sed, i.e. LAST match. Helm marshals
"config" (the release`s values) after "chart", so a values subtree that spells
chart.metadata.name shadowed the real chart name: the release then read as
non-SeaweedFS, the tenant silently vanished from the report, and the script
exited 0 -- the precise false clean this PR exists to prevent, reachable
without any corruption at all. It now takes the FIRST match. The path stays
adjacent on purpose: a looser "any name after metadata" matches
chart.templates[].name, which Helm emits immediately after metadata on every
healthy release, so the review`s suggested relaxation would have returned a
template path for every tenant. Over-strictness fails loudly and recoverably;
over-looseness deletes data quietly.
Newlines are now folded before matching, so a pretty-printed payload parses at
all rather than aborting, and whitespace around the punctuation is tolerated.
first_deployed got the same treatment -- it had the identical greedy, single-
line-only shape, and a spaced payload silently dropped the PV-vintage row from
the report.
The FATAL message names the path it read and the shape it expected, so a future
Helm format change is diagnosable instead of looking like real corruption.
Tests: three new payload shapes (spaced, pretty multi-line, and a values decoy)
each byte-compared against the same golden as the compact payload, so the shape
must make no difference to the report. Mutation-checked -- restoring last-match
extraction fails the decoy test on its own, restoring the single-line matcher
fails the pretty test.
The test fake answered any unmodelled kubectl invocation with exit 0 and empty
stdout, the fail-open shape this script was rewritten to reject, and enough to
let a newly added query pass the goldens unnoticed. Unmodelled calls now exit 97
naming the invocation. Verified inert first by instrumenting the fake: 88
invocations across the suite, none unmodelled. Also added the missing test for
the by-name PVC GET fatal path, whose FAIL mode existed with nothing behind it.
The runbook now tells the operator to read the exit code: a non-zero exit means
the table is incomplete and no step may be taken on it. That contract lived only
in the script header, while the runbook is what the operator follows.
29/29 tests green; sh -n clean.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
(cherry picked from commit
|
||
|
myasnikovdaniil
|
55195ded0c |
test(e2e): capture previous container logs and stop killing the snapshot (#3346)
## What this PR does Root-causing a recent E2E failure was nearly blocked because the diagnostics captured on failure had collected almost nothing. Two independent mechanisms were responsible, and both had been silently in place for a long time. **The failing container's previous logs were never captured.** The pod had crash-looped, so the decisive evidence lived in the first container instance, reachable only through `kubectl logs --previous`. Chainsaw's `podLogs` action has no `previous` field, so this could not be expressed per-suite; the conclusion had to be reconstructed from indirect evidence instead. A new capture runs from the global catch and from the BATS failure hook, selecting containers at runtime by `restartCount > 0` across both init and regular containers, so a healthy run stays quiet and future suites are covered without per-suite wiring. **The catch was structurally guaranteed to be killed.** A Chainsaw `script` op is bounded by `timeouts.exec`, which is 5m, while the catch's own inner budgets summed to roughly 960s. It reported `SCRIPT | ERROR | signal: killed` and produced nothing — and its second leg, the data-plane capture, had therefore never run at all. The kill signature was itself misleading: `timeout` exits 124 and cannot produce that message, which comes from Go's `exec.Cmd` killing the process group. Each catch op now carries an explicit timeout above the budgets it contains, so the inner one fires and leaves a partial, usable artifact instead of losing everything. **The snapshot was truncated at 60 seconds.** `crust-gather --duration` defaults to 60s and was never passed, so every snapshot ever taken was cut short, with nothing in the artifact saying so. The API discovery it performs first is not covered by that budget at all, which is where the outer wall-clock was going on a cluster with a degraded aggregated APIService — precisely the state a failed run is in. Both budgets are now set explicitly, and the collector's output goes to a log inside the snapshot with a completeness verdict, so "is this snapshot complete?" is answerable from the artifact. Failure paths are reported rather than swallowed: an unreachable API says so instead of implying nothing had restarted, a truncated capture is kept and marked, and every dropped item is logged, since silent truncation reads as full coverage. ### What is not verified here The mechanism claims are read from the pinned versions of Chainsaw and crust-gather. Everything cluster-facing is not verified by this PR: the go-template's output shape against a live API server, `--previous` returning the first instance for a real crash-looper, and whether the new collection budgets suffice on a degraded cluster. The bats suite covers the selection helpers only, and those cases are mutation-tested. ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Diagnostics** * Improved end-to-end failure evidence by capturing “previous instance” logs for crash-looping pods (including restarted init containers) and prioritizing relevant namespaces. * Added time-bounded diagnostics collection with explicit outer timeouts; collector output is saved to per-snapshot log files. * Classifies collector outcomes as complete, timeout-truncated, or failed, while preserving the original test failure signal. * **Documentation** * Expanded E2E failure-diagnostics guidance with new timeout and evidence-capture conventions. * **Tests** * Added unit-style tests for previous-log selection, prioritization, row limiting, and deterministic log artifact naming. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
myasnikovdaniil
|
d0c0f399f8 |
fix(ci): cover every image-ref storage shape in promote, retag and mirror (#3404)
## The bug Promotion rewrites the rc version substring to the stable version across the vendored image references. That rewrite lived inline in `promote-rc.yaml` and globbed the depth-2 package `values.yaml` plus `packages/apps/kubernetes/images/*.tag` **only**, on the premise that the kubernetes app was the only one whose `.tag` files carry the cozystack version. Nine other `.tag` files do, and `system/multus` seds its reference straight into a vendored upstream daemonset manifest. #3397 (promote `v1.6.0-rc.4` → `v1.6.0`) is therefore staged with **33 of 55 first-party references still reading `v1.6.0-rc.4`**. `hack/promote-retag.sh` and `hack/nightly-mirror.sh` had the same blind spot from the other direction — both scanned the depth-2 `values.yaml` alone, so those images were never retagged to the stable version (**30 references selected before, 42 after**) and never mirrored to the public registry for a nightly. A retag miss leaves an image short of a tag, since the digest still resolves inside one registry. A mirror miss is worse: the host rewrite walks the same file list, so a reference that is neither mirrored nor rewritten leaves the published tree pointing at the private build registry. `v1.6.0` is the first release to go through promotion — every earlier release was a full rebuild from the tag, which stamped every file uniformly. The glob has always been wrong; the old path just made it unobservable. ## Why it could not be caught before The rewrite was workflow-inline, so there was nothing to unit test. The only way to observe the miss was to cut a release. It is a script now — `hack/promote-rewrite-tags.sh` — and `hack/promote-rewrite-tags_test.bats` round-trips it against the real tree under `make unit-tests`, with no cluster, registry or release involved. ## The fix The three call sites each carried their own idea of where a reference can live, which is how they drifted apart. That knowledge moves into `hack/lib/image-refs.sh` as the single enumeration — `image_ref_files()` for the files, `collect_image_refs()` for the references inside them — and all three source it, so a new storage location is declared once and reaches every consumer. The rewrite also asserts its own postcondition: it scans **wider** than it rewrites and fails the promotion when a reference survives in a location the enumeration does not know about, rather than shipping it quietly. The wide scan is shape-filtered to image-reference position, because `X.Y.Z-rc.N` is an ordinary version string that other things legitimately contain — `kubevirt-csi-driver/go.sum` pins `github.com/golang/protobuf v1.4.0-rc.2`, and `v1.4.0-rc.2` is a cozystack rc that was actually cut. Narrowing it to `${RC}@sha256:` instead would have been wrong: `system/cilium/values.yaml` keeps `tag:` and `digest:` under separate keys. `docs/agents/image-refs.md` records the contract — three tag classes, three storage shapes, the invariants, and which consumer does what. ## Not a bug: kamaji `cluster-api-control-plane-provider-kamaji` stays at `v0.19.0-cozystack.0` across the promotion. It is a first-party rebuild of an upstream component, versioned by that component rather than by the cozystack version line, so no rewrite is owed. A test pins this so a future fix here cannot over-reach. ## Documented known gaps, deliberately not fixed here Both are pre-existing and in the **host** dimension; the version dimension this PR owns is complete, because the version always lives in a `tag` key regardless of where the host sits. - **The nightly host rewrite cannot reach a split host.** It is a literal `<src-registry>/` substring replace, so `keycloak-operator` (host in a sibling `registry:` key) and `kubeovn` (`global.registry.address`, no trailing slash) are mirrored but keep pointing at the build registry in the published tree. Fixing it needs structure-aware rewriting. - **A reference inside a gzip is invisible.** `capi-providers-cpprovider` ships `files/components.gz`, and the promote postcondition cannot catch this class at all since `grep -rIl` skips binaries. Nothing is owed today — kamaji is component-versioned and the image is still mirrored and retagged via its `.tag` — so the residual is that a nightly's kamaji ConfigMap keeps the build-registry host. ## Testing - 17 tests in the new suite; `make bats-unit-tests` green (312 tests, exit 0); `shellcheck -x` clean; `zizmor` clean on the workflow. - Verified against four real rc versions (`1.4.0-rc.2`, `1.4.0-rc.4`, `1.5.0-rc.2`, `1.6.0-rc.4`) in both directions — no false positives, no false negatives. - Every test is mutation-proven. Reverting the enumeration to the old glob, deleting any of the five YAML shapes, corrupting emitted digests, reverting extras to parse-only, dropping the unreadable-file guard, broadening shape 4 beyond `global.images`, or planting an unenumerated first-party reference in any storage shape each fail a named test. ## Reviewer note, out of scope `platform-migrations` is pinned at two different digests — `core/platform/values.yaml` at `v1.5.0` and `backupstrategy-controller/values.yaml` at `v1.4.0-rc.2` (an unowned pin; no Makefile writes that key, tracked in #3143). A `promote-retag.sh` dry-run emits **two copies to the same destination tag**, so the write-once guard will fail the promotion. This is independent of this PR and needs a decision about the backup-client image version, so it is left alone here — but it blocks the 1.6.0 promote and wants fixing before the re-cut. ## Release note ```release-note Promotion, retagging and nightly mirroring now cover every image reference the build stamps, including those stored in `images/*.tag` files and in vendored manifests. Previously a promoted release could ship images still tagged with the release-candidate version, and those images received no stable tag in the registry. ``` Fixes the promote half of #3143. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Image promotion and nightly mirroring now detect digest-pinned image references across values files, standalone tag files, and supported templates. - Stable release preparation supports RC→stable tag rewriting via a dedicated promotion script, with broader verification to ensure no RC references remain. - **Bug Fixes** - Improved completeness of ref discovery and scanning, including previously missed template/tag locations. - **Documentation** - Added and linked guidance documenting image-reference invariants and how tooling enumerates supported storage shapes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
1f302d0d78 |
docs(agents): correct issue ref, mirror claim and example in image-refs
The build-list violations bullet pointed at #3143, which tracks the adjacent gap (packages in the list going stale between releases) and never covers packages absent from the list; they are now tracked in the newly filed #3416. The nightly split-host paragraph claimed both affected images are mirrored while the published tree points at the build registry — today neither ref carries the source-registry host (keycloak-operator is never rebuilt by CI, kubeovn is built externally by cozystack/kubeovn-chart), so the ownership filter skips them and the gap is latent, not live; reworded to say exactly that. The opening example paired a v1.6.0 tag with the real v1.5.0 grafana digest — a ref that will never exist — and grafana is slated to stop being built; replaced with the literal committed cozystack-api ref. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Andrey Kolkov
|
aef9e201c7 |
fix(ci): make workflow cancellation effective (#3401)
## What this PR does - allows superseded PR workflow runs to cancel `finalize` and `E2E Tests` instead of continuing because of job-level `always()` - makes `update-website-docs` cancellation-responsive while preserving its behavior when changelog generation fails or is skipped - retains step-level `always()` conditions for reports and sandbox cleanup ### Screenshots Not applicable. ### Release note ```release-note fix(ci): ensure superseded and manually canceled workflows stop long-running jobs ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Prevented release and validation-related jobs from running when a workflow is cancelled. * Updated pull request check, end-to-end testing, and stable documentation update conditions to avoid executing after cancellation. * Kept existing success-gating so follow-up tasks still require prerequisite steps to complete successfully. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
76f7267ff2 |
test(e2e): describe --previous as the preceding instance, not the first
kubectl logs --previous returns the immediately preceding terminated container instance, not the original first one — after multiple restarts the first instance is already gone. Reword the helper, the BATS and Chainsaw failure handlers, and the diagnostics doc to match the actual --previous semantics. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov Daniil
|
9bd8c17971 |
fix(ci): shape-filter the promote postcondition and close test gaps
Review of the previous commit found the postcondition itself could abort
a release. It grepped the bare "X.Y.Z-rc.N" substring across the tree, and
three live files carry such strings without being image refs:
kubevirt-csi-driver's go.sum pins github.com/golang/protobuf v1.4.0-rc.2
-- a version cozystack actually cut, so promoting v1.4.0 would have
failed on a Go checksum line -- metallb_test.yaml names v1.5.0-rc.2 in a
comment, and the console pnpm-lock.yaml carries rolldown@1.0.0-rc.15. The
remediation the error message gave would have made things worse: declaring
go.sum as a ref source would sed a checksum file.
Match the version only in image-reference position now: after an
image/repository/tag key with no intervening '#', or immediately before an
@sha256 digest. Narrowing it to "${RC}@sha256:" alone would have been
wrong -- cilium keeps `tag: v1.5.0` and `digest:` under separate keys, so
the version is not always adjacent to the digest. Verified against four
real rc versions that previously aborted.
The suite also only tested the fail-closed direction, which is why that
defect survived, and the collector had no completeness guard at all: a
first-party ref added at an unenumerated path on a `latest@` tag was
mirrored by nothing and passed all eighteen tests. Since the mirror's
failure mode is the worst of the three -- the published nightly points at
the private build registry -- that gap mattered most where coverage was
thinnest.
Added: a fail-open test using the three real rc versions above; a
collector completeness diff that discovers refs by content and fails on
any the enumeration misses; coverage for the OCI-artifact shape, whose
deletion previously left every suite green; and an unreadable-file test,
since a read error was being treated as "no match" and silently skipped.
The component-versioned test was vacuous -- its rc version shared no
substring with the kamaji tag it claimed to protect, so it passed by
construction. It now uses an rc version sharing kamaji's 0.19.0 prefix,
which a loosely-anchored implementation would corrupt.
Round-trip fixture discovery matched a combined 'ghcr.io/cozystack/
cozystack/' prefix, which skips keycloak-operator (host in a sibling
registry: key) and kubeovn (host in global.registry.address). It now
matches 'cozystack/cozystack' and stamps split `tag:` keys too.
Also documented two pre-existing gaps rather than half-fixing them: the
nightly host rewrite is a contiguous-substring replace and cannot reach a
split host, and a ref inside a gzip is invisible to every tool including
the postcondition, which skips binaries. Both are host-dimension defects;
the version dimension this change owns is unaffected, because the version
always lives in a tag key regardless of where the host sits.
Assisted-By: Claude <noreply@anthropic.com>
Assisted-By: GPT-5 <noreply@openai.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
||
|
Aleksei Sviridkin
|
07c67a8913 |
test(e2e): capture previous container logs and stop killing the snapshot
Root-causing a crash-loop failure was blocked twice over by the diagnostics collected on failure. `kubectl logs` shows only the current container instance, so for a crash-looping pod the suites' `podLogs` collector captured the Nth restart -- a fresh replay of the same startup path -- while the evidence that explained the failure sat in the first, interrupted instance. Chainsaw's podLogs collector has no `previous` option, so add e2e-capture-previous-logs.sh and drive it from the global catch and from the BATS failure hook. It discovers containers with restartCount > 0 at runtime (init containers included, where an interrupted bootstrap lives), orders the app suites' namespace first, and dumps each to the job log and the snapshot tree. Containers that never restarted are skipped, so a healthy run emits one line rather than a page of "previous terminated container not found". Every way that capture can come up short is named rather than inferred, because kubectl exits 1 for a garbage-collected container, an RBAC denial and an unreachable node alike: the container cap, a pod list that never returned (which is not the same as nothing having restarted), a read that timed out versus one kubectl refused and the reason it gave, and a previous instance that produced no output. A partial capture is kept and marked truncated rather than deleted -- it is the only copy of a log already gone from the cluster. The crust-gather snapshot was killed by the Chainsaw op containing it. A script op is bounded by timeouts.exec (5m), and the catch's own wall-clock wrappers summed to 960s, so the script was SIGKILLed partway through -- taking out the process group mid-write and skipping the data-plane capture entirely. Give each catch op an explicit timeout above the budgets it contains, so an inner timeout fires instead and leaves a partial, still-servable snapshot behind. The data-plane leg drops from 600s to 300s at that call site so both legs fit one envelope; the BATS hook keeps the wider budget, having no containing op. Set crust-gather --duration explicitly as well. It is the collection phase's own budget, it defaults to 60s, and on elapse it abandons the collection and skips its finish step, leaving a truncated tree with no indication anything is missing. The outer wall-clock has to exceed it because the API discovery that runs first is not covered by that budget at all -- with an unhealthy aggregated APIService, the state a failed run is usually in, discovery alone can consume the whole timeout before a single object is written. Send each collector's output to a log beside its snapshot instead of /dev/null, with a complete/truncated/failed verdict, so that question is answerable from the artifact. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov Daniil
|
09323fff13 |
fix(ci): cover every image-ref storage shape in promote, retag and mirror
Promotion rewrote the rc version substring across the depth-2 package values.yaml plus packages/apps/kubernetes/images/*.tag alone, on the premise that the kubernetes app was the only one whose .tag files carry the cozystack version. Nine other .tag files do, and system/multus seds its ref straight into a vendored upstream manifest. v1.6.0 was therefore staged with 33 of 55 first-party refs still reading v1.6.0-rc.4. promote-retag.sh and nightly-mirror.sh had the same blind spot from the other direction: both scanned only the depth-2 values.yaml, so those images were never retagged to the stable version (30 refs selected before, 42 after) and never mirrored to the public registry for a nightly. A retag miss merely leaves an image short of a tag, since the digest still resolves inside one registry; a mirror miss is worse, because the host rewrite walks the same file list and a ref that is neither mirrored nor rewritten leaves the published tree pointing at the private build registry. The three call sites each carried their own idea of where a ref can live, which is how they drifted apart. Move that knowledge into hack/lib/image-refs.sh as the single enumeration -- image_ref_files() for the files, collect_image_refs() for the refs inside them -- and have all three source it, so a new storage location is declared once and reaches every consumer. The rewrite itself moves out of promote-rc.yaml into hack/promote-rewrite-tags.sh. Being workflow-inline is why this was only observable by cutting a release: there was nothing to unit test. It now also asserts its own postcondition, scanning wider than it rewrites and failing the promotion when any ref survives in a location the enumeration does not know about, rather than shipping it quietly. Component-versioned images (kamaji's v0.19.0-cozystack.N) and third-party pass-through images do not ride the cozystack version line and are deliberately left alone; a test pins that so a future fix here cannot over-reach. capi-providers-cpprovider is a documented known gap: its ref is also embedded in a gzipped copy that the chart actually ships, and rewriting only the readable copy would diverge the two. docs/agents/image-refs.md records the contract -- three tag classes, three storage shapes, the invariants, and which consumer does what. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
a4b5b9d57b |
docs(release): update workflow cancellation condition
Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Andrey KolkovandClaude Fable 5
|
7d117679d4 |
docs(backups): document bucketName's two modes, cover isp-hosted in the wiring test
Branch-review on this PR: for provisionBucket: false the backupstrategy-controller.bucketName helper returns .Values.backupStorage.bucketName verbatim as the real S3 bucket name and every strategy CR plus the Velero BSL bakes it in, so the external-S3 section must require updating bucketName (the Secret's bucketName key alone is not read by the strategies), and the knob table must state the two-mode semantics instead of the COSI-only one. Also route the external-S3 instruction through the platform component override instead of naming raw chart values, and add the isp-hosted variant case to the forwarding test (92 tests green). Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey KolkovandClaude Fable 5
|
0cd3fe20b8 |
docs(backups): credentials are projected for RestoreJob reconciles too
The projector runs from both the BackupJob and RestoreJob controllers (restorejob_controller.go), not only before BackupJobs — keep the monorepo doc aligned with the website copy (cozystack/website#622). Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey KolkovandClaude Fable 5
|
93de6adc04 |
docs(backups): fix HelmRelease interval, FDB API group, BSL wait syntax, leader-election phrasing
Same corrections as the website copy (cozystack/website#622 round 4): the re-render interval is 5m via the operator's helmrelease-interval flag (not a 10-minute Flux default), the FoundationDB backup CR group is apps.foundationdb.org, the BSL readiness check uses valid kubectl wait --for=jsonpath syntax, and the projector's first synchronous round is gated on leader election rather than pod readiness. Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey KolkovandClaude Fable 5
|
a5dff22c51 |
docs(backups): fix PromQL examples and bucket-name phrasing, document namespace knob and bslEnabled gap
Keep the monorepo doc aligned with the website copy (cozystack/website#622 branch-review round 3): full cozystack_backup_credentials_projection_* metric names with range vectors in the alerting examples, the intro no longer conflates the Bucket CR name with the COSI-assigned S3 bucket name, the knob table gains the backupStorage.namespace row, and the external-S3 section states that the Velero BSL derives from the same backupStorage block while velero.bslEnabled is not carried by the override path. Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey KolkovandClaude Fable 5
|
b4c1e44fd5 |
docs(backups): namespace the flux reconcile example
The backupstrategy-controller HelmRelease lives in cozy-backup-controller (packages/core/platform/sources/ backupstrategy-controller.yaml), so the un-namespaced command fails out of the box. Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey KolkovandClaude Fable 5
|
e74c8cb459 |
docs(backups): pin the placeholder-credentials upgrade note to v1.5
Same note as the website copy (cozystack/website#622 review round 2): name the explicit version boundary instead of 'pre-PR / After this PR', so the two documents cannot diverge — the empty-string defaults and useSystemBucket shipped in release-1.5. Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey KolkovandClaude Fable 5
|
1422f8b78a |
fix(platform): forward backupStorage overrides to the backupstrategy-controller Package (#3245)
The documented admin override for the cozy-default backup S3 coordinates (spec.components.backupstrategy-controller.values on the cozystack.cozystack-platform Package) had no effect: that PackageSource exposes only the platform component, so the operator silently ignored the block, and nothing in the platform chart forwarded backupStorage to the child Package. Wire it the same way bundles/iaas.yaml forwards kubevirt / gpu-operator values: a new top-level backupStorage value on the platform chart is rendered into the emitted cozystack.backupstrategy-controller Package CR as components.backupstrategy-controller.values.backupStorage, from where the cozystack operator merges it into the backupstrategy-controller HelmRelease over the chart defaults. When backupStorage is unset the emitted Package is unchanged. Update docs/operations/backup-classes.md to the working override path (spec.components.platform.values.backupStorage) and call out the two plausible-looking paths that do not work. Add a helm-unittest suite covering the no-op default and the forwarding on isp-full and isp-full-generic. Fixes #3245 Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
e73ad68a85 |
fix(backups): support cnpg-operator with barman plugin (#3313)
<!-- 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 `backport` if it's a bugfix that needs to be backported
to a previous version.
-->
## What this PR does
Fixes #3300. Migrates CNPG Postgres backups off the deprecated native
`spec.backup.barmanObjectStore` (removed in CNPG 1.29; the
`barman-cloud-*` binaries it needs are absent from the `standard` image
variant keycloak-db pins, and the bare system-flavor tags apps/postgres
pins are deprecated upstream) onto the [CloudNativePG Barman Cloud
plugin](https://github.com/cloudnative-pg/plugin-barman-cloud), across
every Postgres backup surface: `apps/postgres`, `system/keycloak`, and
the `cozy-default-cnpg` BackupClass strategy driver. The chart renders
`spec.plugins` + a `barmancloud.cnpg.io/ObjectStore` CR; the
backup-controller driver SSA-applies an `ObjectStore` and patches
`spec.plugins`; `Backup`/`ScheduledBackup` use `method: plugin`. The
`postgres-operator` chart vendors and co-locates the
`plugin-barman-cloud` operator (with its cert-manager-issued mTLS) so
CNPG can discover it.
One platform-level change is required for the plugin to run and ships
with this PR (see the upgrade notes in
`docs/operations/backup-classes.md` and the release note below): the
CloudNativePG operator is bumped to 1.28.1 (also fixing an operator/CRD
version skew that broke all backups). The other prerequisite — the
LINSTOR scheduler admission webhook stripping
`initContainers[].restartPolicy` cluster-wide, which broke the plugin's
restartable sidecar — is already fixed on `main` (
|
||
|
Myasnikov Daniil
|
b4f98af940 |
docs(seaweedfs): fail closed in the rename runbook's reclaim-policy handling
The 1.5.x->1.6 rename recovery runbook deletes each volume PVC after switching its PV to Retain, but the switch was never gated on success. With no `set -e`, a failed `kubectl patch ... Retain` left the PV at Delete and the following `delete pvc` took the data with the claim. Gate the delete on both the annotation record and the Retain patch succeeding, aborting the re-bind loop otherwise, so no PVC is deleted while its PV is unprotected. In the Step 5 restore loop, chain the annotation removal after the restore patch with `&&` so a failed patch keeps the saved policy for a retry. Address review feedback from coderabbitai on docs/operations/seaweedfs-431-rename-recovery.md. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
95fcc76902 |
docs(seaweedfs): scope the rename runbook to the supported instance name
The tenant module is the supported way to run SeaweedFS and it hardcodes the instance name: packages/apps/tenant/templates/seaweedfs.yaml creates the HelmRelease as `seaweedfs`, and a tenant can only enable or disable it. Every shell selector in this runbook assumes that name, so say so up front, and route instances created directly against the API under other names to escalation instead of through loops whose name filters provably cannot see truncated claims. The ~40-char zone/pool key limit of the guard's reconstruction is called out as an accepted limit for default-named instances too. Two content fixes ride along, both for findings the final review pass raised: - The emptiness check told the operator to kubectl exec the candidate's own volume pod — which for a D-wedged duplicate can never start; that is the definition of the class, so the check was unexecutable exactly where it matters most and every D-wedged tenant funneled to escalation. Mount each bound candidate claim read-only in a scratch pod instead (a Pending claim has no PV and is empty by construction). The weed shell cross-check stays on the authoritative master, which is running. - Step 1's description of the audit now matches the audit itself: direction is decided by the relative strictly-newer PV rule (the same precondition Step 2a enforces), and an interrupted re-bind reads as overlapping vintages with no candidate — not by a clock window anchored on first_deployed. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
601f164c5f |
fix(seaweedfs): stop the naming guard guessing which volume generation holds the data
The guard told a class-S tenant from a D tenant by comparing PVC creationTimestamps,
on the premise that "PVCs are never recreated in place, so the older generation is
where the data was born". The premise is false, and this runbook is the counter-
example: Step 2's re-bind DELETES each release-named claim and recreates it under
the chart name against the same PV.
So a tenant interrupted part-way through Step 2 — after volume-0, before volume-1 —
has data1-seaweedfs-volume-0 created SECONDS ago holding real data, beside
data1-seaweedfs-system-volume-1 with the original timestamp also holding real data.
That is the exact inversion of the rule. Reproduced against the old template:
keeps its data on the release-named volume PVCs (created 2026-06-20T07:01:59Z,
OLDER than the chart-named data1-seaweedfs-volume-* claims created
2026-07-17T09:30:00Z) ... Remove the empty chart-named set ... (Step 2a, then Step 2)
Step 2a then runs `grep -E '^data1-seaweedfs-volume' | xargs kubectl delete pvc` —
deleting the claim Step 2 had just re-bound — and Step 2 had already restored the
PV's original reclaim policy, so on any Delete-policy StorageClass the volume and
the bytes go with it. The guard's own recovery path was a data-loss instruction.
Two more holes in the same classifier:
- The S-damaged branch was gated on $systemPVC, which requires the PVC name to
contain "seaweedfs". seaweedfs.componentName truncates the fullname to 56 chars
before appending -volume, so an instance name >= ~40 chars drops the chart name
off the PVC name — the very case fullname_override_named_test.yaml exists for.
Probed with `archive-of-quarterly-financial-statements-x1`: damaged + renamed set
Ready fell through to the D-split branch ("is running two live SeaweedFS sets ...
Stop the split"), sending the operator to quiesce the set holding ALL the data —
the exact misclassification the parent commit exists to remove. Damaged + not
Ready fired no branch at all and rendered through unguarded.
- readyReplicas == 0 was read as proof the duplicate never served. It is a
snapshot: a duplicate that crashed, was scaled down, or lost readiness AFTER
serving writes reads identically, and adoption strands whatever it wrote.
There is no sound signal here. Claim timestamps are mutable, StatefulSets are
recreated by the adoption hook, and liveness is a snapshot — nothing durable in the
object graph distinguishes "empty set a bad upgrade created" from "the data,
mid-re-bind". Rather than add a fourth heuristic, the guard now decides only what
is decidable:
legacy generation only (or nothing) -> render; adoption is correct
release-named generation only -> refuse, class S (Step 2)
BOTH generations -> refuse; an operator classifies
Both generation flags now OR the PVC and StatefulSet evidence, so the truncated
long-name tenant is seen via its label-matched StatefulSet.
The cost is bounded and lands only where it should. A duplicate exists only on a
tenant that passed through the broken 4.31 rename in 1.5.x — already damaged,
already needing an operator. A 1.4.x tenant upgrading straight to 1.6 never
renames, so it has one generation and is untouched. Confirmed on the stand:
tenant-l (1.4.5-era, never upgraded) is legacy-only and dry-runs clean; the four
both-generation tenants all already need an operator. What is lost is D-wedged
auto-adoption, which was resting on the readyReplicas assumption above.
Classification moves to the runbook, where a human has signals a template does not:
Step 1 now reads the ages of the PVs (durable — they survive the re-bind) via
.spec.volumeName instead of the claims, reports MIXED instead of picking, and walks
the operator through interrupted-Step-2 first, then strict age separation, then
verifying emptiness directly, then escalating rather than guessing. Step 2 stashes
the PV's original reclaim policy ON THE PV and leaves it Retain until Step 5
verifies the tenant, so a mis-step during recovery is survivable. Step 2a grows a
precondition that refuses unless every chart-named claim is bound to a strictly
newer PV.
Tests: 7 of 14 in the enforcing suite fail against the old template, including the
interrupted-Step-2 tenant and both truncated-long-name cases. hack/seaweedfs-guard-
parity.bats pins the two copies (system/ enforcing, extra/ sibling) against silent
drift — they cannot share a library, neither chart depends on cozy-lib, and "keep
these in sync" as a comment is the failure class this whole branch is about; it
also pins that creationTimestamp and readyReplicas never come back.
`make test` in both charts now runs helm template --is-upgrade and greps for the
refusal. The fail-closed canary — the headline safety property — had ZERO automated
coverage: guard_fail_closed_test.yaml passes with AND without the fix, because
helm-unittest 1.0.3 ignores release.isUpgrade. Verified the new target fails when
the canary is removed.
Review round 3 — the refusal was right; the justification was false and the
procedure that replaced the chart's judgement was broken on both axes.
THE COMMENT WAS WRONG. It said "nothing durable in the object graph distinguishes
the empty generation from the data mid-re-bind". A durable signal does exist, and
the reviewers were right to call that out: the Helm release history. Verified on
the stand — sh.helm.release.v1.<name>-system.v1 records the scheme the tenant was
BORN under (tenant-root/l/dsplit/named rev 1 render seaweedfs-master => pre-4.31;
tenant-fresh renders seaweedfs-system-master => 1.5.x), and info.first_deployed is
on every retained revision, so anchoring PV creationTimestamps against it survives
history pruning and the Step 2 re-bind (tenant-root: chart-named +10s vs
release-named +2084s; tenant-fresh: +5225s vs +6s).
BUT IT DOES NOT UNLOCK AUTO-ADOPTION, and that is the substantive disagreement
with the round-3 direction. Birth order answers "which generation is ORIGINAL".
Adoption needs "is the OTHER one EMPTY". Those come apart exactly where it
matters: D-wedged (duplicate never scheduled, safe to adopt) and D-split
(duplicate served writes, adopting strands them) are BOTH born pre-4.31 and are
therefore identical on every durable signal. Verified: tenant-root and
tenant-dsplit have the same rev-1 scheme and the same first_deployed deltas, yet
one duplicate served and the other did not. The only thing that ever separated
them was readyReplicas, which is a snapshot and not evidence. So the render keeps
refusing on presence, and the comment now says why — naming the signal, so nobody
re-derives the broken one.
The signal IS used where it belongs, in the audit.
- naming-guard.yaml / seaweedfs.yaml: match the release-named generation by
RECONSTRUCTING the name 4.31 gives this release's volume component
(templates/_naming.tpl, replaying seaweedfs.fullname + componentName), tested
FIRST because the prefixes nest. Fixes a blocker: an instance legitimately named
`seaweedfs-volume` renders seaweedfs-volume-system-volume and
data1-seaweedfs-volume-system-volume-0, which BOTH satisfy the chart-named
prefixes — a prefix-only guard read its live storage as legacy and would render
onto the empty chart-named claims, stranding the real generation. The parity
bats passed straight through it, because parity pins sameness, not correctness.
Reconstruction also removes the name-match blind spot at source: a truncated long
instance name now matches by PVC, not only by StatefulSet.
- hack/seaweedfs-naming-audit.sh + .bats: the classifier is a tested script now.
Two revisions of it shipped as an untested snippet inside the runbook and both
were wrong in ways that routed a live tenant into the step that strands its data:
a selector `^data1-(.*seaweedfs.*)-volume` that also matched the chart-named
claims, so the release-named range spanned BOTH generations, every tenant read as
"overlapping / mid-rebind", and a genuine S-damaged tenant was routed AWAY from
Step 2a into Step 2 — which deletes its data claim, fails to recreate it, then
deletes its StatefulSets; and a `grep seaweedfs` name match that reported
"L — nothing to do" for a long-named tenant the chart refuses. Both are covered.
The audit reports L/S/MIXED matching the guard, names the original generation
from both durable signals, detects MID-REBIND (both generations at
first_deployed), and states plainly that "original" is not "empty" and that
emptiness must be established from the duplicate's volume files before deleting.
A bats case renders _naming.tpl through helm and compares it to the script's
reconstruction, so the two implementations cannot drift.
- Runbook: Step 1 calls the script. Steps 3/4 were circular — Step 3 ended in an
upgrade the presence-gated guard refuses, and Step 4 told the operator to clear
duplicates AFTER an upgrade that cannot run until they are gone. Step 3 now
removes the duplicate BEFORE the upgrade and re-runs the audit to prove it; Step 4
starts from "every tenant reads L or S". D-wedged no longer claims to need
nothing. Step 3's "enumerate fids that resolve only on the duplicate" was not an
executable procedure and is now an explicit escalation: two masters allocate
volume IDs from independent sequences into one shared seaweedfs-db, so there is
no supported tool that reconciles two volume-ID spaces against one metadata
store. Step 2's re-bind restores the cleanup hook's labels on the recreated
claims, without which a recovered tenant leaks its volumes on a later delete.
Fixture corrected: data1-...-x1-system-volume-0 is not a name the chart renders;
componentName gives ...-x1-system-seaw-volume-0.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
||
|
Myasnikov Daniil
|
47e24f01d1 |
fix(platform): hand seaweedfs-db over for every instance name, not just the default
Migration 43 performs the v1.5.0 db-split hand-over: it re-owns Cluster/seaweedfs-db from <name>-system to the new <name>-db release and stamps helm.sh/resource-policy: keep, so the <name>-system upgrade — whose post-split chart no longer renders the Cluster — does not prune it as a removed resource. It compared the owning release name against the literal "seaweedfs-system". SeaweedFS is a user-creatable kind, so an instance named `foo` is owned by `foo-system` and was silently skipped: no re-own, no keep. The <name>-system upgrade then deleted the Cluster, CNPG took its PVC with it, and the filer metadata — the index for every object in that tenant's S3 — was destroyed. Data loss, not an outage: the volume PVCs still hold the bytes, nothing can find them. Verified live on the upgrade stand, which is a natural control group. All five tenants sit in the SAME failed-upgrade loop (<name>-system last-deployed is rev 1, a pre-split revision whose manifest still contains the Cluster, so every retry recomputes the same deletion): tenant-root/dsplit/l instance `seaweedfs` owner=seaweedfs-db keep Cluster alive tenant-named instance `foo` no Cluster at all Cluster GONE The only difference is that migration 43 fired for the default-named instances and skipped `foo`. That also answers whether `keep` is sufficient: three tenants survive the identical prune loop with it. Correction to the reported mechanism: this is the 1.4->1.5 db split (PR #2601, v1.5.0), not a 1.5->1.6 change — v1.5.3 and main are identical on every db-split file. And the split does protect the object; it just only protected the instances someone happened to name `seaweedfs`. - lib/seaweedfs-db-adopt.sh: shared hand-over matching the `-system` SUFFIX, so `foo-system` -> `foo-db` exactly as `seaweedfs-system` -> `seaweedfs-db`. Idempotent, and refuses a release named literally `-system` rather than annotating an owner of `-db` that no release would claim. - Migration 43 sources it: covers clusters upgrading from before 43 (1.4.x). - Migration 53 (new) re-runs it: migrations never re-run, so any cluster already at >= 44 ran the hardcoded version and is still exposed. As a pre-upgrade hook it lands before <name>-system re-renders, closing the window on THIS upgrade. This is the one that matters for 1.5.x -> 1.6. - targetVersion 53 -> 54. Tests fail on unfixed code: the non-default-name case produces no ANNOTATE at all against the hardcoded 43, while the default-name case passes — which is precisely the bug's shape. They drive the real migration scripts against a fake kubectl, following hack/migration-50-etcd-adopt.bats. LIMIT, stated plainly: a Cluster that is already deleted cannot be recovered by either migration — there is nothing left to re-annotate and the PVC went with it. Such a tenant needs seaweedfs-db restored from a backup. Runbook Step 0 adds the ownership audit that distinguishes at-risk from already-lost, and records that <name>-db can report Ready while its Cluster is gone (it rendered fine; a later <name>-system prune removed it and Flux has not re-checked) — so the audit trusts `kubectl get cluster`, not the HelmRelease status. Review round 2 — two ways this still lost the database: - OWNERSHIP IS NOT SAFETY. The helper skipped any Cluster already owned by <name>-db, treating ownership as proof the hand-over was done. It is not: where the hand-over was skipped, <name>-system prunes the Cluster and <name>-db RECREATES it under its own ownership with NO keep, while <name>-system's prune baseline still lists it — so the next reconcile deletes it again. That is the observed delete/recreate loop, and the skip walked straight past it. Verified on the stand: tenant-l and tenant-root are seaweedfs-db-owned AND their <name>-system deployed revision (rev 1) still contains the Cluster, so only keep saves them; tenant-fresh is seaweedfs-db-owned without keep but was installed after the split, so its baseline never had one. Telling those apart needs the release's deployed manifest, which this script cannot read cheaply or reliably. The costs are asymmetric — a needless keep leaves an orphan on delete (now reclaimed by the cleanup hook), a missing one loses the database — so keep is stamped on every Cluster owned by either side of the split. - FAIL OPEN, THEN STAMP. `set -euo pipefail` does not abort on a failing command substitution in a `for` word-list: the loop ran zero times, the script continued, and stamp_cozystack_version ran anyway. Migrations never re-run, so one throttle, RBAC hiccup, or not-yet-established CNPG apiservice behind the pre-upgrade hook permanently left every at-risk tenant exposed with no later migration to catch them. Reproduced: against the old helper a failing fleet scan still logged STAMP 54. Now every kubectl failure is fatal EXCEPT the two that genuinely mean "nothing to do" — the CNPG resource type not being served (the fail-open that IS load-bearing: a cluster without CNPG must still upgrade) and a Cluster vanishing between scan and read. An unreadable annotation is no longer indistinguishable from an absent one, and the unowned case now warns instead of passing silently. - ORPHAN ON DELETE. `keep` is permanent (removal deferred to the 1.7 batch migrations) and also survives the <name>-db release's own uninstall, so `kubectl delete seaweedfs <name>` left the CNPG Cluster, its Postgres pods and its PVCs behind — the cleanup hook selects app.kubernetes.io/instance=<name>-system, which the db PVCs (cnpg.io/cluster=seaweedfs-db) do not carry. Migration 53 widened that from pre-1.5 default-named tenants to every instance name, so the hook now deletes the Cluster explicitly; its PVCs carry ownerReferences to it and follow. - Tests assert the stamped VERSION (44 / 54), not a bare "STAMP": a wrong number would loop run-migrations.sh forever and the old assertion could not tell. Review round 3 — the reclaim introduced a data-loss path of its own: - DELETING ONE APP DELETED ANOTHER APP'S DATABASE. The db chart hardcodes the Cluster name `seaweedfs-db` for every instance, and the reclaim was scoped by that name in the namespace rather than by what the release owns. templates/seaweedfs-db.yaml gates the <name>-db HelmRelease on `topology != "Client"`; the cleanup hook had no such gate. Verified by render: `helm template s3-remote . --set topology=Client` emits ZERO s3-remote-db HelmReleases yet still emitted the Role granting delete on clusters/seaweedfs-db AND `kubectl delete cluster.postgresql.cnpg.io -n tenant-root seaweedfs-db`. So in a namespace running a server instance plus a Client, deleting the CLIENT destroyed the SERVER's filer metadata — CNPG takes the PVC via ownerReferences — for an app the operator never touched. Reachable the same way via a second Simple instance, whose <name>-db loses Helm's ownership check but which still installs and still carries the hook. main's hook was instance-scoped; this was new, and mine. Now gated exactly as seaweedfs-db.yaml is, and the Job re-checks meta.helm.sh/release-name == <release>-db at run time before deleting. The gate stops the permission being granted where it can never be legitimate; the runtime check stops the delete where it can. - `|| echo` turned any API/authz/admission failure into exit zero, so backoffLimit never retried and hook-delete-policy removed the only cleanup Job while the keep-protected Cluster stayed orphaned. A failed reclaim now fails the Job. - TEST SHELL. The migrations run under /bin/sh = busybox ash (the image is FROM alpine, run-migrations.sh is #!/bin/sh), but the tests drove them with bash — so the fail-closed guarantees were asserted in a shell that never runs them, which matters more than usual given the bug they cover was a fail-open. The scripts are now invoked via `sh` and the fake kubectl is POSIX sh. Verified directly against the production base image (busybox 1.37.0 / alpine:3.24): `set -o pipefail` is supported, every script passes `ash -n`, and the failing-fleet-scan path exits non-zero without stamping under real ash. The recipe is in the bats header. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
ae93356ed1 |
fix(seaweedfs): name cluster-scoped RBAC per namespace, not after the pinned fullname
ClusterRoles and ClusterRoleBindings are cluster-scoped: one object per name, cluster-wide. This chart installs once per tenant, so every cluster-scoped name it renders must be unique per install. Four of them were not. Upstream 4.31 switched them from global.serviceAccountName to seaweedfs.fullname. That is fatal in combination with the #3282 pin: fullnameOverride: seaweedfs is what adopts the running NAMESPACED workloads across the rename, so fullname is by construction the SAME string in every tenant. Every SeaweedFS instance in the fleet therefore renders seaweedfs-objectstorage-provisioner, seaweedfs-rw-cr and their bindings — one object, five claimants. NOT a #3282 regression. Rendering v1.4.5, v1.5.3 and main for the same tenants: v1.4.5 tenant-root-seaweedfs-* tenant-named-seaweedfs-* unique v1.5.3 seaweedfs-system-* foo-system-seaweedfs-* COLLIDES (identical for every default-named instance; a non-default instance name was the only thing that escaped) main seaweedfs-* seaweedfs-* COLLIDES (nothing escapes: the pin removes the last distinguishing input) So the collision shipped in 1.5.0 with the 4.31 bump; #3282 widens it from "every default-named instance" to "every instance". Observed live on the upgrade stand: one ClusterRole/seaweedfs-objectstorage-provisioner annotated to a single owning tenant, four other tenants' COSI provisioners left with no RBAC, and tenant-dsplit's helm history recording Upgrade failed: original object ClusterRole "seaweedfs-objectstorage-provisioner" not found The binding is the sharp end: its subject names a ServiceAccount in the release namespace, so the single live ClusterRoleBinding grants cluster-wide secrets RBAC to whichever tenant reconciled last, and silently revokes it from the rest. Fix: put the four names back on global.seaweedfs.serviceAccountName, which extra/seaweedfs already sets to <namespace>-seaweedfs and which cosi-provisioner-sa-name.patch already uses for the ServiceAccount and the binding's subject — the cluster-scoped names were simply left behind. Three of the four come out byte-identical to pre-4.31, so a 1.4.x tenant adopts them in place; the fourth (rw-crb) replaces upstream's username-shaped system:serviceaccount:<sa>:default and is pruned from the release's own manifest. The namespaced pin is untouched: workloads still render seaweedfs-master/-filer/ -volume, so #3282's adoption is intact. Uniqueness now rests on a value, so assert it rather than trust it: templates/cluster-scoped-rbac-guard.yaml refuses to render when the name would not carry the release namespace. It sits behind the same namespace canary as the naming guard, because system/seaweedfs's own values.yaml ships a placeholder that only extra/seaweedfs overrides — a client-side render must not trip on it. Tests fail on unfixed code: cluster_scoped_names_test asserts two tenants get two names, and unpatched both render seaweedfs-objectstorage-provisioner (5 failed, 0 passed). They test the vendored chart templates directly, which is where the rename happens. The guard includes seaweedfs.compat before reading. extra/seaweedfs sets the OLD flat key global.serviceAccountName, and the shim is what folds it into the canonical global.seaweedfs.serviceAccountName; without it the guard compares against this chart's placeholder default and refuses every real upgrade. Caught by helm upgrade --dry-run=server against the stand, not by unit tests — the mocks never carry the flat key. Verified live with --dry-run=server on the stand: tenant-l (installed pre-4.31, never upgraded) already owns tenant-l-seaweedfs-objectstorage-provisioner and tenant-l-seaweedfs-rw-cr, and the fixed chart renders exactly those — adopted in place, no ownership error. tenant-root renders tenant-root-seaweedfs-* and also dry-runs clean. Leftovers on the release-based names that a 1.5.x tenant passed through are inert once every tenant is upgraded (no release renders them any more) but are not always pruned, since more than one release claimed them; the runbook adds the sweep. Review round 2: - The uniqueness guard used `contains .Release.Namespace $sa`, a substring test that accepts namespace `tenant-a` for service account `tenant-ab-seaweedfs` — a different namespace's. Anchored to hasPrefix "<namespace>-". It still asserts the VALUE the names are built from, not the rendered names, so it cannot catch the chart being reverted to name them after the pinned fullname; cluster_scoped_names_test.yaml covers that axis and is what actually bites. - Added the case that would have caught the compat bug: every test set the CANONICAL global.seaweedfs.serviceAccountName, while production sets the OLD FLAT global.serviceAccountName that only seaweedfs.compat folds in. Deleting the include left the suite passing 5/5 while every real upgrade refused. The new case sets the flat key; with the include removed it now fails with the production error. - Runbook sweep missed the fourth pre-4.31 name: 4.31 renamed the master-rw binding from upstream's username-shaped system:serviceaccount:<sa>:default to <sa>-rw-crb, and the old one carries neither suffix nor a tenant prefix, so the grep could not see it (live on the stand as system:serviceaccount:tenant-l-seaweedfs:default). Also documents the rolling- upgrade window: Helm prunes by name without checking ownership, so the first tenant onto the fixed chart deletes shared objects a not-yet-upgraded tenant is still bound through — COSI provisioning 403s until they reconcile, S3 traffic unaffected. Review round 3: - hasPrefix "<namespace>-" was still too loose. It accepts `tenant-a-b-seaweedfs` for namespace `tenant-a` — which is ALSO the natural value for namespace `tenant-a-b`, so two namespaces would accept one string and collide on the very object this guard exists to keep unique. extra/seaweedfs sets exactly `<namespace>-seaweedfs`, so assert equality with it and nothing looser. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
8499b5f4a1 |
fix(seaweedfs): move the naming-migration guard into the render that actually renames
The class-S guard from #3282 lives in extra/seaweedfs, but that render is not in the path of a platform upgrade: the <name>-system HelmRelease pulls system/seaweedfs from a platform-managed ExternalArtifact, so bumping the platform upgrades the release directly from the new artifact and applies fullnameOverride: seaweedfs without extra/seaweedfs ever re-rendering. On a tenant installed fresh on 1.5.x that stood up an empty chart-named set beside the live release-named data, deleted the renamed s3 Service, and left S3 readable only because the empty set's pods cannot start. Worse, the damage flips the guard's own classification: creating the chart-named artifacts makes $legacyData true, so the guard then reports D-split and sends the operator to runbook Step 3 — which quiesces the renamed set, i.e. the one holding ALL the data. - Add the ENFORCING guard to system/seaweedfs (templates/naming-guard.yaml): the only render guaranteed to sit between a platform upgrade and the tenant's workloads. It also refuses to render if the fullnameOverride pin is ever lifted. - Tell S from D by PVC AGE when both generations exist: PVCs are never recreated in place (StatefulSets are, by the adoption hook), so the older volume-PVC generation is where the data was born. Renamed-older ⇒ class S (damaged variant, new Step 2a), never D-split. - Fail closed: the release namespace is a lookup canary; an upgrade that cannot see the cluster refuses instead of classifying blind. Client-side installs (CI lint/unittest) still render — they never touch live data. - Mirror the same classification in extra/seaweedfs so the operator-facing SeaweedFS application reports the correct class. - Tests now model the ExternalArtifact upgrade path in the chart it actually runs through (the old ones mocked lookups only in extra/seaweedfs, which is why this shipped green), including the damaged-S tenant observed live. - Runbook: add S-damaged audit classification (PVC age), Step 2a cleanup of the empty chart-named set, and the D-wedged anti-affinity stall workaround observed on 3-node clusters. helm-unittest 1.0.3 ignores release.isUpgrade, so the canary refusal is verified with 'helm template --is-upgrade' instead of a unit test. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Andrey KolkovandClaude Fable 5
|
f1627354b4 |
docs(contributing): point the ansible migration note at loadBalancerClass
The downstream-coordination checklist still directed contributors at an
"externalIPs to exposureClass" migration; exposureClass was removed in
|
||
|
Andrey KolkovandClaude Fable 5
|
572492c276 |
docs(backups): scope the barman-binaries claim, drop the stale linstor webhook note
The db.yaml plugins-block comment still carried the over-generalized claim that the pinned postgresql image variants no longer ship the barman-cloud-* binaries; mirror the scoped wording from docs/operations/backup-classes.md (only the standard variant keycloak-db pins lacks them; this chart's bare system-flavor tags still ship them but the flavor is deprecated upstream). After the rebase onto main this PR no longer touches linstor-scheduler: main re-vendored it to upstream chart 0.3.1 / extender v0.3.6, which keeps the admission webhook and stops stripping unknown pod fields (including initContainers[].restartPolicy the barman sidecar needs). The upgrade note claiming the webhook is removed and locality scheduling becomes advisory is rewritten as a hold-back warning. Also: afterwards -> afterward (American English, review nit). Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey KolkovandClaude Fable 5
|
1ea7ef54c0 |
docs(backups): correct the CRD-upgrade and barman-binaries claims
Review follow-ups: the vendored cloudnative-pg chart ships the CNPG CRDs as ordinary release manifests (crds.create), so helm upgrade updates them with the operator — the out-of-band kubectl apply claim was wrong (the historical skew came from the image.tag pin, which this PR removes). And the missing barman-cloud-* binaries only affect the standard image variant keycloak-db pins; apps/postgres pins bare system-flavor tags that still ship them — its breakage was the operator/CRD skew. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey KolkovandClaude Fable 5
|
8967205353 |
docs(backups): document the mixed-version BackupJob window caught by the e2e
Running the gated postgres backup e2e against a cluster whose apps still
render the pre-upgrade chart surfaced the CNPG webhook rejection
("Cannot enable a WAL archiver plugin when barmanObjectStore is
configured"): the driver cannot attach spec.plugins while the native
config is still present. Document the window and its resolution in the
upgrade notes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
|
||
|
Andrey KolkovandClaude Opus 4.8
|
604e79437b |
docs(backups): disclose CNPG-bump + LINSTOR scheduler change; fix stale e2e fixture
Review follow-up (cozystack#3300): - docs/operations/backup-classes.md: add an operator-facing upgrade note for the two platform-wide changes this migration carries — the CloudNativePG operator bump to 1.28.1 (with the CRD-skew fix and the Helm-CRD-upgrade caveat) and the LINSTOR scheduler admission-webhook removal, which makes data-locality scheduling advisory cluster-wide (spec.schedulerName no longer auto-assigned; opt in via explicit schedulerName). These affect more than Postgres backups and were previously only noted in a chart values.yaml comment. - hack/e2e-chainsaw/postgres/postgres.yaml: drop stale restic-era backup fields (s3Region, s3Bucket, cleanupStrategy, resticPassword) that no longer exist in the Postgres backup schema; use current fields (destinationPath, endpointURL, retentionPolicy, 6-field schedule). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey KolkovandClaude Opus 4.8
|
302a0193f1 |
docs+test(backups): update upgrade notes for the plugin flow + cover recovery rendering
Review follow-ups for the CNPG barman-cloud plugin migration (cozystack#3300): - docs/operations/backup-classes.md: the 'Upgrade notes from chart-managed backups' section still described the removed native spec.backup.barmanObjectStore behavior. Rewrite it for the plugin flow (chart renders spec.plugins + a barmancloud.cnpg.io/ObjectStore; the driver SSA-applies an ObjectStore and patches spec.plugins), and document the real upgrade hazard: the plugin-barman-cloud operator + cert-manager must be present in cozy-postgres-operator or the Cluster references an unregistered plugin and backups/WAL archiving stall. - apps/postgres backup_storage_test.yaml: add a positive bootstrap-recovery rendering test. Asserts externalClusters[].plugin references barman-cloud.cloudnative-pg.io with barmanObjectName=<release>-recovery and serverName on the plugin parameter, and that the -recovery ObjectStore carries spec.configuration but NOT serverName (the plugin forbids it there). Previously the recovery path had only fail-fast guard tests, no positive coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
myasnikovdaniil
|
4a00b502d7 |
fix(seaweedfs): adopt legacy workloads in place across the 4.31 rename (#3282)
## What this PR does Fixes a v1.5.0 upgrade regression where a tenant ends up with **two** SeaweedFS workload sets. **Root cause.** v1.5.0 bumped the vendored SeaweedFS chart `4.0.405 → 4.31.0`. Upstream changed resource naming: before 4.31 the chart named workloads after the **chart** (`seaweedfs-*`, ignoring the release name), and 4.31 names them after the **release**. The data-plane release is `<name>-system`, so every StatefulSet wanted to become `seaweedfs-system-*`. StatefulSet names are immutable, so Helm could not rename in place — it stood up a second, empty set beside the running one while the data stayed on the original `data1-seaweedfs-volume-*` PVCs. Where the node count let both sets run, they shared one `seaweedfs-s3` Service (identical pod labels) and one `seaweedfs-db` filer metadata database, so the empty set answered reads and the two master sets mixed volume IDs from independent sequences into one metadata table. **Fix.** Pin `fullnameOverride` in `system/seaweedfs` values so the rendered names stop following the release name, which is the convention already used by `cozy-proxy`, `flux-operator`, `grafana-operator`, `linstor-scheduler` and `victoria-metrics-operator`. The names go back to what every pre-1.5 cluster already runs, so the upgrade adopts the running workloads **and their volumes** in place. No data migration, no platform migration. **The one case that cannot be adopted.** A tenant installed *fresh* on 1.5.x wrote its data under the renamed workloads, so its volumes are not the chart-named ones. Pinning the chart name there would rename the workloads *away* from that data and bring up an empty cluster beside it. Helm cannot move data between PVCs, so `extra/seaweedfs` refuses to render for such a tenant and points at the recovery runbook, which re-binds the existing PVs onto the adopted PVC names (no copying — the claim is renamed, the volume is not touched). The guard is fail-closed: it fires only when renamed volumes are present and no adopted ones are. Two naming subtleties the guard has to respect, both verified by rendering the shipped 1.5 chart: - Before 4.31 the chart ignored the release name, so an instance running under a non-default name (`SeaweedFS` is a user-creatable kind) *also* stored its data on `data1-seaweedfs-volume-*`. Such a tenant is adopted, not blocked. - On 4.31 the fullname helper appends the chart name when the release name does not contain it, and the component helper truncates to 56 characters. A fresh 1.5.x instance named `archive` therefore wrote to `data1-archive-system-seaweedfs-volume-*`, and a long name loses the chart name from its tail entirely. The guard matches volumes by shape and by the chart labels on the StatefulSet instead of reconstructing those names, so both are caught. **On the duplicate set.** Once the fix reconciles, the `seaweedfs-system-*` StatefulSets and Deployments are no longer part of the release and Helm removes them on its own. What it cannot remove — PVCs it did not template, and cert-manager Secrets that carry no owner reference — is inert, and the runbook clears it explicitly. A pre-upgrade migration was considered and dropped: it would duplicate what Helm already does, while its only unique action (deleting PVCs) cannot distinguish "the duplicate never served" from "it served and is currently down", and any transient delete failure under `set -e` would fail the platform's pre-upgrade hook and block delivery of this very fix. ## Testing `helm unittest` — `system/seaweedfs` 21 tests, `extra/seaweedfs` 17 tests. The guard's `lookup` branches are covered by mocking the Kubernetes provider (the pattern already used in `core/platform` and `system/cozystack-api`): adopted-only, renamed-only, both sets, zoned volumes, a non-default instance name, a truncated long name, and an unrelated `data1-*-volume-*` PVC that must not trip the guard. The s3-consumer suite now re-creates the name/fullname divergence explicitly, so it keeps guarding that regression regardless of the shipped default. The remaining regression test for this class of bug is an upgrade E2E (seed a pre-4.31 named set, upgrade, assert a single set with data intact) — no fresh-install E2E or `helm template` can reproduce an upgrade over pre-existing state. That belongs in the chainsaw upgrade lane and is a follow-up. ### Screenshots N/A — no UI changes. ### Release note ```release-note fix(seaweedfs): adopt existing SeaweedFS workloads and volumes in place when upgrading past the 4.31 chart rename, instead of standing up a second, empty cluster beside them ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an audit & recovery runbook for SeaweedFS 4.31 rename impacts. * **Bug Fixes** * Stabilized SeaweedFS workload/PVC naming across upgrades, including non-default instance names. * Added preflight safeguards to prevent upgrade scenarios that could split or wedge existing data. * Updated post-delete cleanup to remove secrets from both legacy and current naming schemes. * **Tests** * Expanded Helm unit coverage for naming guards, cleanup behavior, and S3 service name consumers. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
myasnikovdaniil
|
f6a03e93f8 |
fix(etcd): complete v1alpha2 transition for in-cluster 1.5→1.6 upgrades (#3270)
Consolidates the etcd `v1alpha2` transition fix for **in-cluster 1.5 → 1.6 upgrades** into a single PR, rebased on current `main`. Supersedes #3265 and #3261 — their commits are carried here (authorship preserved), so those PRs can be closed once this lands. Fresh installs use the new shapes directly and are unaffected, so fresh-install CI does not catch these; the 1.5 → 1.6 e2e upgrade path is the authoritative regression guard. ## What this PR does ### 1. Keep the legacy `etcd-headless` Service alive during adoption — `packages/extra/etcd` `etcd-migrate` adopts legacy clusters **in place**: the Pods keep their original `spec.subdomain: etcd-headless` and are dialed at `etcd-<i>.etcd-headless.<ns>.svc` until they roll onto the operator's native `<member>.etcd.<ns>.svc` domain. The v1alpha2 operator only creates the native `etcd` Service and the legacy `etcd-headless` Service is pruned, so those per-pod names stop resolving (`no such host`), `MemberList` fails, and `status.readyMembers` never populates — the `EtcdCluster` never goes `Ready` even though etcd is healthy and in quorum. We ship a chart-managed transitional headless `etcd-headless` Service (selector mirrors the operator's native `etcd` Service via `etcd-operator.cozystack.io/cluster`, `publishNotReadyAddresses: true`) — the DNS counterpart of the legacy `*.etcd-headless.<ns>.svc` SAN already kept for this window. Removable together with that SAN once members roll onto the native subdomain. ### 2. Survive the immutable controller-Deployment selector on upgrade — `packages/system/etcd-operator` (#3242) #2859 replaced the upstream etcd-operator chart with the cozystack-authored one, changing `Deployment.spec.selector.matchLabels`. `spec.selector` is immutable, so `helm upgrade` cannot patch the existing Deployment and the whole HelmRelease upgrade fails (`field is immutable`). A **pre-upgrade hook** (`templates/pre-upgrade-selector-fix.yaml`: ServiceAccount + Role + RoleBinding + Job) deletes the Deployment **only** when its live selector is the pre-1.6 one, so Helm recreates it cleanly. No-op when the selector already matches, never runs on fresh install. ### 3. Raise the operator's memory cold-start floor — `packages/system/etcd-operator` Steady-state working set is ~250Mi; the static `limits.memory: 128Mi` OOMKills a Pod that starts before the VPA admission webhook rewrites it. Raise the floor to `256Mi` (and VPA `minAllowed` to match) as defense in depth so the operator never depends on VPA timing to avoid crashing. ### 4. Make migration 50 (etcd adoption) robust in-cluster — `packages/core/platform/images/migrations/migrations/50` (was #3261) Exact server/peer cert-SAN match, Secret-gated wait on the adoption Secret, and in-cluster (IPv6-safe) kubeconfig handling, so the migration script drives the adoption reliably from inside the cluster. Covered by `hack/migration-50-etcd-adopt.bats`. ### 5. Hardening / review fixes (this PR's original scope) - **Hook `runAsUser: 65532`.** `pre-upgrade-selector-fix.yaml` set `runAsNonRoot: true` but no numeric `runAsUser`; `clastix/kubectl`'s image user is the non-numeric name `nonroot`, which the kubelet cannot verify against `runAsNonRoot` — so the hook Pod fails admission and silently blocks the very upgrade it exists to unblock. Adds `runAsUser: 65532`, matching the postgres-operator webhook-ready hook that runs the same image. - **Digest-pin the kubectl image.** The values comment claimed digest-pinning but shipped a floating `v1.32` tag. Reuses the digest postgres-operator vendors, adds the `renovate` annotation, and templates `repo:tag@digest`. - **Tests.** `etcd-operator/tests/selector-fix-hook_test.yaml` (hook wiring, weight ordering, namespaced least-privilege RBAC, numeric-non-root security context, digest-pinned image), `etcd-operator/tests/deployment_test.yaml` (256Mi cold-start floor), `extra/etcd/tests/etcd-cluster_test.yaml` (transitional `etcd-headless` Service). Each assertion was mutation-tested. ### 6. Derive the default S3 endpoint from the provisioned bucket — `packages/system/backupstrategy-controller` Derive the default S3 endpoint (and per-driver scheme / TLS / `secure_connection`) from the provisioned bucket Secret instead of requiring it to be hand-set, so etcd (and other) backup strategies get a working endpoint by default. Docs in `docs/operations/backup-classes.md`; covered by `tests/endpoint_form_test.yaml`. ### Verification - `helm unittest` green on current `main`: etcd-operator **18/18**, extra/etcd **18/18**, backupstrategy-controller **11/11**. - Behaviours 1 & 2 were reproduced and confirmed live on a 1.5.2 → 1.6.0-rc.1 adoption (3-node cluster) — recreating the `etcd-headless` Service took the adopted cluster to `readyMembers=3 / Available=True`. ```release-note fix(etcd): complete the v1alpha2 transition on in-cluster 1.5→1.6 upgrades — keep the legacy etcd-headless Service alive so adopted members stay resolvable, delete the pre-1.6 operator Deployment via a pre-upgrade hook to get past the immutable selector, raise the operator's memory floor so it does not OOM before the VPA scales it, and make the etcd adoption migration robust in-cluster. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved backup storage endpoint handling across supported backup drivers, including provisioned and external S3 storage. * Added compatibility support for legacy etcd pod discovery during migration. * Added an automated upgrade safeguard for etcd operator deployments. * **Bug Fixes** * Improved certificate SAN detection and etcd migration authentication. * Increased the etcd operator’s minimum startup memory to prevent early restarts. * **Documentation** * Clarified backup endpoint, TLS, and driver-specific behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov DaniilandClaude Opus 4.8
|
7e34e87cd9 |
docs(backups): document derive-from-Secret S3 endpoint behavior
The `backupstrategy-controller.endpoint` helper changed how the S3 endpoint is resolved (for a provisioned bucket it derives from the COSI system Secret and forces https://, with `backupStorage.endpoint` demoted to a fallback), but docs/operations/backup-classes.md still described the old single-verbatim-value behavior and directly contradicted the code: - the "Endpoint format per driver" intro said the templates adapt "the single backupStorage.endpoint value"; - it claimed drivers "pull from backupStorage.endpoint in chart values, not from the Secret" — now precisely backwards for provisioned buckets; - the values-table `endpoint` row omitted the fallback/derive semantics. Rewrite those to match the new helper + the values.yaml comment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Ivan Okhotnikov
|
0164a6afa4 |
fix(monitoring): set groups_attribute_path=groups so allowed_groups actually works on Grafana v11.x
Verified end-to-end on dev3 with Grafana v11.6.15: OAuth flow reaches Grafana, id_token and userinfo response both carry the correct `groups` claim (`[cozystack-cluster-admin, tenant-root-super-admin]` for kvaps), `allowed_groups` in grafana.ini lists `tenant-root-super-admin`, and login still fails with `user not a member of one of the required groups`. Grafana debug log shows `Attributes: map[]` — the extracted user's Groups slice is empty. On v11.x+ Grafana does not auto-map the `groups` claim onto user.Groups unless `groups_attribute_path` is set explicitly (the field is documented as optional but the gate is a silent no-op without it — a claim-name-string is not enough). Fix: hard-wire `groups_attribute_path=groups` in the chart-forced System-mode block alongside `allowed_groups`. "groups" points at the top-level `groups` array from userinfo, matching the shape Keycloak's oidc-group-membership-mapper emits (verified on the same dev3 run). Two helm-unittest cases updated to assert `groups_attribute_path` lives next to `allowed_groups` in both the users-map-active and empty-users branches (rejecting the silent no-op regression). Docs also updated to name the requirement. Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
874ca9427f |
fix(apps/kubernetes): close cross-tenant kube-apiserver OIDC hole via CEL claim gate
Symmetric with the Grafana-side allowed_groups gate — round-3 review from lllamnyp on PR#3176 covered both surfaces, and #3044's original System-mode wiring shipped without this guard. The rationale is identical: audience binding stops REPLAY of a minted token across tenant kube-apiservers, but nothing stopped Keycloak from ISSUING a usable token to a caller outside the tenant. Absent a claim-side gate, any authenticated cozy-realm user could `kubectl oidc-login` against another tenant's cluster and land at `system:authenticated`. RBAC default-denies named-resource access, but `system:authenticated` still leaks discovery (kubectl api-resources, OpenAPI schemata) — tenant-alice could enumerate tenant-bob's cluster's CRD shape and built-in resource surface. Fix: add a CEL `claimValidationRules` entry to the rendered AuthenticationConfiguration (System mode only) that requires at least one of the release's four tenant-scoped Keycloak groups (`<namespace>-{view,use,admin,super-admin}`) to be present in the token's `groups` claim. The `has(claims.groups) &&` guard is required — CEL evaluation on a missing claim surfaces as HTTP 500 from the authenticator instead of a clean 401. CustomConfig mode is not touched — the tenant's own AuthenticationConfiguration is authoritative and the tenant is responsible for claim-side guards on their own issuer. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
b002c967ac |
fix(monitoring): close cross-tenant Grafana OIDC hole via tenant-group gate
Round-3 review from lllamnyp: any user in the shared `cozy` Keycloak
realm could sign into any tenant's Grafana. The audience mapper stops
replay of a minted token across releases, but does not gate who may
mint one — tenant-alice could complete the OAuth flow against
tenant-bob's Grafana. With `spec.oidc.users` non-empty the earlier
`allow_sign_up=false` fix mostly closed the door (though tenant-alice
still hit `error 401` after landing in Grafana, revealing the release
exists); with empty users under mode: System — a config the schema
permits and the docs describe as hands-off — the login succeeded and
minted a Viewer account for every cozy-realm identity.
The fix uses the KeycloakRealmGroups the tenant chart already
provisions (`<namespace>-{view,use,admin,super-admin}` in
packages/apps/tenant/templates/keycloakgroups.yaml) — no new
directory objects owned by the Monitoring release. Two chart edits:
1. oidc-keycloak.yaml: add the platform-managed realm-default `groups`
scope to `defaultClientScopes` explicitly. EDP's KeycloakClient
does NOT merge realm-level defaults into `defaultClientScopes`, so
the scope must be listed here or the `groups` claim would be
missing from the token.
2. grafana.yaml: in System mode, unconditionally emit
`allowed_groups: <ns>-view <ns>-use <ns>-admin <ns>-super-admin`
in `auth.generic_oauth` and add `groups` to `scopes`. Grafana
rejects any login whose token does not carry at least one of the
groups.
The gate is chart-owned and does not depend on the users-map — with
`users: []` tenant members still get a self-service Viewer through
`auto_assign_org_role`, cross-tenant users get 401 in Keycloak's OAuth
callback. Docs updated to reflect the two-layered contract
(tenant-membership gate + optional users-map).
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
f64ba91872 |
fix(monitoring): do not render users-Job when spec.oidc.users is empty
Address PR#3176 review round-2 gap-2: the users-Job's prune pass deletes every non-admin Main-Org member whose email is not in the desired list. When the desired list is empty every helm-upgrade wiped the whole org. Three empty-users states used to fall into this trap: * customConfig.secretRef: users[] is forbidden by assertCustomSecretRefUsersEmpty, so the desired list was structurally always empty and the chart wiped an org whose auth.ini fragment the operator advertised as authoritative. * System with users[] unset: the operator opted into OIDC but not into the chart-managed users-map, yet the Job still pruned every Main-Org member on their behalf. * CustomConfig-inline with users[] unset: same shape as System. The mode=None guard (from an earlier round) already had the same shape; extend the render gate to cover the other empty-list doors — now the Job renders only when `mode != None AND users non-empty`. The trade-off: taking `users:` back to `[]` no longer prunes the last entry — matching the already-documented `System → None` posture — called out explicitly in the docs. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |