fix(seaweedfs): close the 4.31 rename fallout on the 1.5.x→1.6 upgrade path - #3339

… 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>

…st 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>

…n 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>

…clock window

The audit named the candidate duplicate by measuring each generation's oldest
PV against the release's first_deployed, with an absolute 120-second window
for the mid-rebind case. The shipped StorageClasses are WaitForFirstConsumer,
so PVs are provisioned at pod-SCHEDULE time — on a cold cluster minutes after
first_deployed — and a tenant interrupted mid-way through Step 2's re-bind
(both generations sitting on original-vintage PVs) fell out of the window and
into a branch decided by which volume index happened to provision first. The
advice printed there routes the operator to a step that deletes the claims
Step 2 had not yet re-bound.

Replace the window with the relative rule runbook Step 2a already enforces: a
generation is the candidate duplicate only when EVERY one of its bound PVs is
strictly newer than every bound PV of the other generation. Overlapping or
tying vintages name no candidate — finish Step 2 or escalate. first_deployed
is still printed, as context only.

The direction decision is now a pure function (classify_mixed_direction) so
the bats suite drives it directly: clean duplicate both ways, interrupted
re-bind, tie, and a no-clock case pinning the WaitForFirstConsumer regression.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

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>

…sification

MultiZone (and Simple-with-pools) tenants render no plain `volume` component —
extra/seaweedfs sets volume.enabled=false and the vendored chart emits one
component per zone key with suffix `volume-<key>`, truncated at
(62 - len(suffix)), which is SHORTER than the 56-char cut the guard's
renamedVolumePrefix replays. The final review pass flagged this divergence as a
guard bypass. For the supported instance the prefixes cannot diverge: the
tenant module hardcodes the name `seaweedfs`, the fullname is 16 chars, and
nothing truncates until a zone/pool key of ~40+ characters. Three new cases pin
that supported path — legacy-only zones adopt, release-named-only zones refuse
as class S, both generations refuse — since the suite previously had no zone
shapes at all and the zone objects are the ONLY evidence on a MultiZone tenant.

The divergence beyond that boundary (long instance names or absurd keys) is an
accepted, documented limit, not a fix: decided in the 1.6 triage, recorded in
_naming.tpl (both copies, kept in lockstep) and the runbook's Scope section.
Instance names are fixed by the tenant module, so absurd keys are the only
reachable trigger; revisit if instance naming is ever opened up.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

…andling

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>

The post-delete cleanup hook read Cluster/seaweedfs-db's Helm owner with
`|| true`, so a Forbidden, a timeout or any API error collapsed to an
empty owner, took the "nothing to reclaim" branch, and let the Job
complete -- permanently orphaning the keep-protected Cluster the hook
exists to reclaim, with nothing else that will ever collect it.

Only a genuine NotFound now counts as absence; every other error prints
the failure and exits non-zero so the Job retries. This matches the
fail-closed handling the delete path already had. Add a regression
assertion that the ownership lookup no longer swallows failures.

Address review feedback from coderabbitai on
packages/extra/seaweedfs/templates/hooks/cleanup.yaml.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

The naming guard now refuses every two-generation state, but two
comments in extra/seaweedfs still described a D-wedged duplicate (zero
ready replicas) as safe to adopt and rendering through. readyReplicas is
a snapshot, not proof the duplicate never served, so it cannot be told
apart from D-split and the guard refuses it too.

Align the state summary in the seaweedfs template comment and the test
header with the presence-only refusal behaviour. No classifier change --
comments only.

Address review feedback from coderabbitai on
packages/extra/seaweedfs/tests/fullname_override_test.yaml.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

Address review feedback from IvanHunters on
packages/core/platform/images/migrations/migrations/43:23:

The suite invoked the migrations as `sh "$MIG_DIR/<n>"`. On the CI runner
/bin/sh is dash, which has no `set -o pipefail`, so both scripts aborted
on their `set` line with "set: Illegal option -o pipefail" (exit 2)
before doing any work, and the job went red. Production was unaffected —
the migrations image is FROM alpine, so /bin/sh there is busybox ash —
which is exactly why checking the runtime image missed it.

Rather than change a shebang or drop pipefail, run the migrations the way
run-migrations.sh does: by path, inside the image's own pinned alpine
base, read out of the migrations Dockerfile so the interpreter under test
cannot drift from the one they ship on. pipefail is load-bearing here —
lib/cozystack-version.sh pipes the rendered manifest into `kubectl
apply`, so without it a failed render stamps the version from empty
input — and a shell that lacks it can prove nothing about that path.

run_migration() returns the container's status explicitly: cozytest.sh's
awk generator rewrites every bare `}` in column 0 into `return 0` + `}`,
so a helper that falls off its own end reports success no matter what it
ran, and every fail-closed assertion would have passed vacuously.

15/15 pass. Verified the interpreter under test is BusyBox v1.37.0 ash
with pipefail supported, and that a fail-open mutation in
lib/seaweedfs-db-adopt.sh turns the suite red.

Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

hack/seaweedfs-naming-audit.bats sources the audit script at top level,
and cozytest.sh sources the converted test file into its own /bin/sh --
dash 0.5.12 on the Ubuntu CI runner. dash has no `pipefail`, so
`set -uo pipefail` aborted the runner before the first @test and took
the whole file down with it:

    hack/cozytest.sh: 48: set: Illegal option -o pipefail

The bash shebang could not prevent this: `.` executes the file's
contents in the caller's shell, so the shebang is only a comment there.

The script was already POSIX apart from those two lines -- no arrays,
no [[, no local, no bashisms at all -- so the script becomes POSIX sh
rather than the runner becoming bash. That keeps the shell under test
and the shell at run time the same one, instead of testing under dash
while shipping under bash.

Dropping `pipefail` changes no behaviour: the script never sets -e and
never reads a pipeline's exit status, reporting failure as empty output
checked with [ -n ... ].

Verified under dash (ubuntu:24.04): 11/11 tests pass, and the script
runs clean against a live cluster, classifying five seaweedfs tenants
with the revision-1 scheme and the PV ages agreeing on every MIXED
case. shellcheck -s sh reports nothing.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>