Skip to content

[Backport release-1.5] fix(seaweedfs): close the 4.31 rename fallout on the 1.5.x→1.6 upgrade path - #3370

Merged
myasnikovdaniil merged 4 commits into
release-1.5from
backport-3339-to-release-1.5
Aug 4, 2026
Merged

[Backport release-1.5] fix(seaweedfs): close the 4.31 rename fallout on the 1.5.x→1.6 upgrade path#3370
myasnikovdaniil merged 4 commits into
release-1.5from
backport-3339-to-release-1.5

Conversation

@github-actions

Copy link
Copy Markdown

Description

Backport of #3339 to release-1.5.

@github-actions

Copy link
Copy Markdown
Author

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-3339-to-release-1.5
git worktree add --checkout .worktree/backport-3339-to-release-1.5 backport-3339-to-release-1.5
cd .worktree/backport-3339-to-release-1.5
git reset --hard HEAD^
git cherry-pick -x 8499b5f4a1779c9e4e588863ffadfa5a3b1ab901 ae93356ed179cf46fd6da6cc37c9763ced4d4c2f 47e24f01d1f3fccc8f35e7dd093fafb5d43d9ed2 601f164c5f02c43786c50ef42f36cced97d6fd4e 24bcb0bb6916766cb8cbcf0dc0f31b7a568da11c 95fcc76902b8444395c4f83aec71f96753e05f0a b16a51e60ebee5b751d360f8aae9e874823f9308 b4f98af940029e7e32ddbaaceafc402c7a41c23a a099b7e086c55365155770f2d2e399ad75c62677 8355f3e36fba333d137efdb8481e3e1a1e5c39dc 9b82d1bf36631acede467fa9a6eeb93523e7c285 bd770b2288ec5ac7a764f61708b6bb939e11d1ed
git push --force-with-lease

Upstream chart 4.31 switched resource names from the chart name to the release
name. The data-plane release is <name>-system, so every StatefulSet was renamed
to <name>-system-*. StatefulSet names are immutable, so the upgrade could not
rename in place: Helm 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 S3 Service and one filer metadata
database, so the empty set answered reads and mixed volume IDs from an
independent sequence into shared metadata.

Pin fullnameOverride so the rendered names no longer follow the release name,
matching the convention already used by other system packages (cozy-proxy,
flux-operator, linstor-scheduler, victoria-metrics-operator).

The s3-consumer suite pinned the name/fullname divergence this override removes
by default, so it now re-creates that divergence explicitly and keeps guarding it
regardless of the shipped default.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
(cherry picked from commit 7bcd8e0)
(cherry picked from commit 1da45ead8262334636919c097019afdf71da412e)
With the chart-based names pinned back, an upgrade adopts the running workloads
and their volumes in place. Before 4.31 the chart ignored the release name, so
this holds for an instance running under any name: its data sits on the
chart-named data1-seaweedfs-volume-* PVCs either way.

Two states cannot be adopted and stop the render instead, each pointing the
operator at the matching recovery step:

- Fresh on 1.5.x: only the renamed volumes exist, so adopting the chart-based
  name would rename the workloads away from that data and bring up an empty
  cluster. Helm cannot move data between PVCs, so the operator is sent to the PV
  re-bind procedure.
- D-split: both sets exist AND the renamed volume servers are live, so they may
  hold objects written through the split endpoint. Adopting would strand them, so
  the operator is sent to reconcile the split first. A duplicate that never served
  (zero ready replicas) is safe to adopt and renders through.

The guard mirrors the chart's fullname helper to recognise what 4.31 named this
instance (the helper appends the chart name when the release name does not
contain it, then truncates), and matches the renamed volumes by shape plus the
chart labels on the StatefulSet rather than reconstructing the exact name.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
(cherry picked from commit 052ce2e)
(cherry picked from commit 5a724e0bc0d43c8afb621a6da4aab5b13a86e774)
Classify each tenant from its volume PVCs, re-bind the volumes of a tenant
installed fresh on 1.5.x so the adopted names pick them up, stop a split cluster
before upgrading it, and clear the leftovers the upgrade cannot remove itself.
The recovery steps select the renamed set by a precise filter and derive secret
names from the fullname helper, so they work for an instance running under a
non-default name and for pooled or zoned volumes.

Co-authored-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
(cherry picked from commit 088669fb5ae5106aa1d4d22e3226488f557e4b02)
Backport of #3339 onto release-1.5, on top of the #3282 adopt-in-place
fix (#3326).

Three differences from main, all forced by what release-1.5 does not
carry:

  * The repair migration is renumbered 53 -> 45 and stamps 46, since
    release-1.5 tops out at migration 44 with targetVersion 45.
    platform values.yaml goes to targetVersion 46.

  * Migrations 43 and 45 keep release-1.5's inline labelled stamp
    instead of lib/cozystack-version.sh. That helper is the shared
    stamp refactor (#2980), whose backport (#3065) was closed, so it
    does not exist on this branch. Only lib/seaweedfs-db-adopt.sh --
    the part that actually carries the fix -- is sourced.

  * The extra/seaweedfs post-delete cleanup hook and its three tests
    are dropped. The hook arrived with #3092, a breaking change that
    was never backported, so there is nothing here for #3339's changes
    to it to apply to. Comments in lib/seaweedfs-db-adopt.sh and the
    recovery runbook that promised the hook would reclaim an orphaned
    volume are corrected to say it does not on 1.5.x.

The migration renumbering is carried through the shared helper, the
bats suite (STAMP 54 -> 46) and the runbook prose.

Verified locally: helm unittest green for both seaweedfs charts (51 + 16
assertions) including both blind-upgrade guard canaries, and 34 bats
assertions across migration-seaweedfs-db-adopt, seaweedfs-naming-audit
and seaweedfs-guard-parity.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
(cherry picked from commit 88e9fdcbbbff8aeab317b08557ca6bd6c8b5706d)
@myasnikovdaniil
myasnikovdaniil force-pushed the backport-3339-to-release-1.5 branch from 999ae6a to 33611df Compare July 31, 2026 05:28
@github-actions github-actions Bot added size/XXL This PR changes 1000+ lines, ignoring generated files area/release Issues or PRs related to release tooling (changelog, backport, release pipeline) area/storage Issues or PRs related to storage (linstor, seaweedfs, bucket, velero, harbor) kind/bug Categorizes issue or PR as related to a bug labels Jul 31, 2026
myasnikovdaniil pushed a commit that referenced this pull request Jul 31, 2026
Backfill tenant.cozystack.io/<ancestor> labels on existing tenant
namespaces so clusters heal on upgrade without waiting on each tenant
HelmRelease to re-reconcile (#2810/#2171).

Backport of migration 49 from #2912, adapted for release-1.5:
- migration file 49 -> 46. The existing backport PR #3191 renumbered it to
  45, but the SeaweedFS repair migration backported alongside it (#3339 /
  #3370) claims 45 too, so the two collide. Whichever of those two PRs
  merges second has to take 46; here the SeaweedFS one keeps 45 because it
  is what the 1.5.x line is being cut for.
- targetVersion 45 -> 47, covering both new migrations. #3191 alone bumps it
  to 46, and because the two backports agree on that value git merges them
  without a conflict — leaving targetVersion one short of the highest
  migration, which run-migrations.sh silently skips (its loop stops at
  TARGET-1). release-1.5 has no `migrations-target-check` to catch it; that
  guard arrived on main after v1.5.2 and was never backported.
- stamp via the inline labeled-ConfigMap apply used by release-1.5
  migrations, since lib/cozystack-version.sh does not exist on this branch

Best-effort and idempotent: kubectl label --overwrite adds only the listed
labels, and per-namespace failures are tolerated so a transient apiserver
error cannot abort the platform upgrade.

(cherry picked from commit 50f78ce)

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
(cherry picked from commit aee9fa85b65d96dae1dff7e5044fd26ce710797e)
@myasnikovdaniil
myasnikovdaniil marked this pull request as ready for review August 4, 2026 08:27
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@dosubot dosubot Bot added the backport-previous Backport target — previous release line label Aug 4, 2026
@myasnikovdaniil
myasnikovdaniil merged commit be9094c into release-1.5 Aug 4, 2026
10 checks passed
@myasnikovdaniil
myasnikovdaniil deleted the backport-3339-to-release-1.5 branch August 4, 2026 08:30
myasnikovdaniil added a commit that referenced this pull request Aug 19, 2026
…on and in the chart (#3892)

Migration slot 45 holds a different migration here than on main and
release-1.6. On this branch it is the seaweedfs db-split repair, added
after v1.5.3 by `33611dfe7` (the #3370 backport), which in the same
commit bumped `migrations.targetVersion` from 45 to 46. On main and 1.6
slot 45 is the `helm.sh/resource-policy=keep` pin on
`KubeadmConfigTemplate`, and the seaweedfs repair sits at slot 53
instead.

Nothing is broken today. v1.5.0 through v1.5.3 all ship `targetVersion:
45`, so no released cluster is stamped past it and every one of them
still runs 1.6's slot 45 on the way up. Cutting v1.5.4 is what first
ships 46. After that a 1.5.4 to 1.6 upgrade runs `seq 46 53` and never
executes 1.6's slot 45, so helm prunes `KubeadmConfigTemplate` while the
live kubeadm-backed MachineSet still references it,
`bootstrap.configRef` dangles and controller-manager floods through the
talos worker rollover. Tenant kubernetes only, noisy broken upgrade
rather than data loss.

This folds the keep-pin into release-1.5's slot 45 so it does both jobs,
and puts the annotation on the rendered template in
`packages/apps/kubernetes/templates/cluster.yaml` so templates created
after the upgrade are born pinned. `targetVersion` stays 46.

### Why uninstall does not leak

`keep` suppresses only helm's own delete. CAPI stamps an
`ownerReference` to the `Cluster` on this object, which the chart does
not render (checked on a live v1.5 stand, and `MachineDeployment` and
`KubevirtMachineTemplate` carry the same reference). The `Cluster`
itself is helm-managed with no `keep`, so `helm uninstall` deletes it
and GC reclaims the template through that reference.

Template name is deterministic rather than content-hashed, so `keep`
cannot accumulate a copy per upgrade. One residue is the template of a
removed nodeGroup, which helm no longer prunes. It is inert, re-adopted
in place if the group comes back, and GC takes it with the `Cluster`.

### Tests

Mutation-proven rather than revert-tested. The one worth naming:
swapping the selector to the `meta.helm.sh/release-name` annotation
makes the migration exit 0 reporting `pinned=0 failures=0`, which looks
like a clean run, and the test catches it because the annotation never
reached the object.

Also converts four pre-existing negated-grep assertions in
`hack/seaweedfs-guard-parity.bats`. `! grep -q X f` cannot fail, posix
and bash both exempt a `!`-negated pipeline from errexit, so those were
comments that look like assertions. All of them hold once they are real.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/release Issues or PRs related to release tooling (changelog, backport, release pipeline) area/storage Issues or PRs related to storage (linstor, seaweedfs, bucket, velero, harbor) backport-previous Backport target — previous release line kind/bug Categorizes issue or PR as related to a bug size/XXL This PR changes 1000+ lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants