mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-29 16:17:20 +09:00
feat/vm-disk-http-auth
28
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 IvanHunters
|
3d1a83f419 |
fix(migrations): make the unadopted-pools record publish best-effort
The durable-record ConfigMap write sat under set -euo pipefail right before the version stamp, so a permanent failure (e.g. the record exceeding etcd's object size limit at a very large skip count) would abort the migration and deadlock the fleet upgrade with no path out -- even though every skipped pool was already pinned prune-proof earlier in the sweep, so the workers are safe regardless of whether the record lands. The record is a convenience artifact, not a safety mechanism, so swallow its failure with a warning instead of failing closed. Covered by a bats case that fails the publish and asserts the run still stamps. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
952901776a |
fix(migrations): adopt unlabelled parent kubernetes releases and record unadopted pools in migration 56
The worker-adoption sweep selected parents by the apps.cozystack.io/ application.kind label, which postdates v1.2. A parent kubernetes HelmRelease older than that, or one restored by hand, lacks the label: the sweep skipped it, stamped, and the parent's control-plane-only re-render then pruned its un-pinned workers with no pin and no warning. Discover such releases by their chartRef instead and restamp the three application labels before the sweep (additive, cannot corrupt a release, and repairs it for cozystack-api too). Every warn+pin+skip branch leaves a pool pinned prune-proof but unadopted, recorded only in the Job log, which the next upgrade reaps; the migration does not re-run, so an operator cannot find those pools afterward. Accumulate the skips and publish them as configmap/cozystack-migration-56-unadopted before the stamp. Extend the fake kubectl and add bats coverage for the restamp (positive and negative) and the unadopted-ConfigMap paths. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
05c967bcaa |
test(migrations): move the worker-pool adoption bats to migration 56
The main merge renumbered the Phase 2 worker-pool adoption migration from 54
to 56 (behind main's redis-adopt 54 and fluxcd-orphan 55), but its bats and
fakebin did not move: hack/migration-54-adopt-{path,values}.bats still pinned
MIG at migrations/54, which is now the redis-operator adoption, so both went
red on "RedisFailover CRD not present; nothing to adopt".
Rename the two bats and hack/testdata/migration-54 to 56, repoint MIG/FAKEBIN
at migration 56, and update the version references (stamp 55 -> 57). The bare
migration-54 testdata name also collided with the migration-54-redis and
migration-55-fluxcd siblings; 56 disambiguates it.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
f035da8a0f |
Merge branch 'main' into feat/kubernetes-nodes-phase2b
Resolve conflicts introduced by main: - Drop the FluxCD tenant addon (API type, deepcopy, values schema, RD) in favour of the GPU type main added; main removed the fluxcd addon wholesale. - Renumber the Phase 2 worker-pool adoption migration 54 -> 56, behind main's redis-operator (54) and fluxcd-removal (55) migrations; targetVersion 55 -> 57. - Accept the branch's removal of the in-chart talos-reconcile Job; it moved to the kubernetes-nodes chart, main only touched its comment. - Regenerate kubernetes values.schema.json / README and the kubernetes-rd ApplicationDefinition schema. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Andrei Kvapil
|
851a86dcf1 |
feat(platform): orphan tenant FluxCD HelmReleases on upgrade
Add migration 54 for the removed addons.fluxcd feature. For every tenant that had it enabled, the FluxCD HelmReleases in the management cluster are suspended, stripped of their Flux finalizer and deleted, so the delete does not trigger a Helm uninstall inside the tenant cluster. The Flux release already running in the tenant keeps running untouched. Runs as a pre-upgrade hook, before the addon-less Kubernetes chart reaches the tenant HelmReleases. Mirrors the orphan idiom of migration 29. Bumps migrations.targetVersion to 55. Fail closed. Dropping the finalizer is the only thing that keeps the delete inert, and migrations never re-run, so the script runs under `set -euo pipefail` with no `|| true` on the critical path and gates the delete on the finalizer actually being gone. The verify read uses `--ignore-not-found` rather than `2>/dev/null || true`, so a failed read aborts instead of reading as an empty finalizer list that passes the gate on an object whose state was never checked. A migration that stops is recoverable; one that half-orphans a fleet is not. Cover it with a docker + fake-kubectl unit test running the real script by path inside the migrations image's own alpine base (busybox ash, jq baked on), so it exercises the shipped interpreter and load-bearing pipefail. The suite pins ordering and selection (only the addon HelmReleases are touched, each suspend -> finalizer-drop -> delete in order) and the fail-closed paths (a surviving finalizer, a failing verify read, and a failing fleet scan each abort before deleting and before stamping). Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Andrei Kvapil
|
3197c48bc5 |
test(migrations): cover migration 54 redis adopt (label+pin, CRD probe fail-closed)
Drive migration 54 end-to-end against a fake kubectl in a jq-enabled build of
the migrations image's alpine base (busybox ash, the production interpreter),
mirroring the docker+fake-kubectl migration-test harness.
Pinned properties:
- an absent CRD stamps 55 without patching anything;
- a failing CRD probe aborts under set -euo pipefail before stamping (a
one-off apiserver blip must not read as "absent" and skip adoption);
- every RedisFailover across namespaces is labelled onto the freshworks
operator-group, while the engine is pinned to redis:6.2.6-alpine only where
the image is empty (both roles for Harbor; sentinel only where redis.image
is preset, which is left untouched; label-only when both are preset);
- a failed adoption patch aborts before stamping so the Job retries.
Auto-discovered by the Makefile hack/*.bats wildcard; no registration needed.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
|
||
|
IvanHunters
|
c010745e8e |
Merge branch 'main' into feat/kubernetes-nodes-phase2b
Bring the worker-pool split branch up to date with main (111 commits), picking up the node-join and CI fixes it was missing: the 18m tenant node-join deadline, guest-fsync-off on ephemeral CI disks (#3455), kubevirt-csi client rate-limit raise (#3428), the Cilium ingress-IP race guard (#3430), and etcd-operator v0.5.4. Conflict resolution: main's #3535 (render the talos-reconcile Job for the default md0 group) modified packages/apps/kubernetes/templates/talos/ talos-reconcile-job.yaml, which this branch deletes because the worker split moves that Job into the kubernetes-nodes chart. Kept the parent template deleted and dropped the parent-scoped test talos_reconcile_nodegroups_test.yaml: the empty-nodeGroups md0-default gap it guards cannot occur in the per-pool child chart, and migration 54 materialises the implicit md0 into an explicit child HelmRelease. targetVersion resolves to 55 (migration 54 on top of main's 53). Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Timofei Larkin
|
f0b99da43a |
test(kubernetes): make kubectl_wait_retry set -e safe
The chainsaw scripts source run-kubernetes.sh under `set -eu`, where the wrapper's bare `_out=$(kubectl wait ... 2>&1)` on a failing wait aborts the whole sourcing script at the assignment -- before `_rc=$?`, before the retry loop, before anything prints, with the error text captured into the substitution and lost. Every `kubectl wait` failure therefore killed the suite instantly and silently, and the transient-retry machinery the wrapper exists for (etcd leader flap et al.) has never been reachable. This is how the kubernetes-latest suite died in CI with an empty stderr: the wait it was executing failed, and the script simply stopped. Guard the assignment (`_rc=0; _out=$(...) || _rc=$?`) so the wrapper's own logic decides: allowlisted transient errors retry with a visible notice, everything else prints and propagates, exactly the documented contract. New unit suite executes the wrapper in a `sh -eu` subshell against a fake kubectl, the same way chainsaw runs it: a transient etcd error must be retried with the script surviving (against the previous lib this exits 1 after one call with empty output), and NotFound must not be retried, must fail, and must be printed. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Myasnikov Daniil
|
1eded27900 |
test(release): scope the trim pin to the parse step, fix two stale references
Three review nits, all verified against the tree, plus a tighter scope than the first attempt at them. The behaviour suite asserted "promote-rc does not trim the rc tag" against the whole file. promote-rc.yaml has no .trim() today, so it passed, but any unrelated .trim() added anywhere in the workflow would have failed a test about tag policy. Scoping it to the parse JOB was not enough either: that job is ~180 lines, most of them the e2e gate's github-script, whose string handling has nothing to do with tag policy. Both greps are now scoped to the parse STEP, and run through a code_lines filter copied from the contract suite so a YAML comment mentioning .trim() cannot fail them. Mutation-checked in both directions: a .trim() injected into the parse step fails the test, one injected into the e2e-gate step of the same job does not. - The Step 2 checklist entry still said "must be main" while Step 2 itself now lists three valid starting points (main, detached at the release tag, detached at the rc tag being promoted). The checklist is what an agent ticks off, so it was the half that mattered; it now points at Step 2 rather than re-encoding the count, so a fourth configuration cannot desync it. The heading said "Checking current branch", which two of the three states do not have. - hack/testdata/needs-semantics.yaml named release-changelog-contract.bats as its consumer; the only consumer is release-changelog-behaviour.bats. 50/50 tests green across the three suites (11 + 24 + 15). Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Timofei Larkin
|
730d006ce8 |
fix(platform): skip pinning names no object can have
The pin path composed "kubernetes-<cluster>-<group>" and read it back, but a group key that cannot form a valid object name (a slash, a space, an underscore...) provably never had objects applied under it -- the old chart's apply would have been rejected by the apiserver. Worse, kubectl rejects a name containing '/' client-side, deterministically, so pin_keep's fail-closed read classified it as a real failure and the migration exited 1 on every retry: a permanent fleet-wide pre-upgrade deadlock over a pool that cannot exist. Lean on the invariant instead of the read: pin_pool_objects statically checks the composed name against the DNS-1123 subdomain shape and skips with a note when it cannot name an object -- nothing to pin, no kubectl call to misread. Keys that are label-invalid but still valid object names (e.g. dotted) may have live objects and keep today's pin behavior; the pin-path transient-error test now uses a dotted key so it still exercises a genuine read. The fake kubectl reproduces real kubectl's client-side slash rejection, and a new bats case pins the skip: a "pool/a" key must warn, pin nothing, apply nothing, and still reach the version stamp. Red against the previous script, which deadlocked at the first pin read. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Timofei Larkin
|
a54d5ca4a5 |
fix(kubernetes-nodes): unpin absence via --ignore-not-found
Same two holes as the migration's read classification, in the unpin hook. Capturing stderr into $pol (2>&1) contaminated the success path: kubectl warns on successful reads, so an unannotated object read as "pinned" and drew a spurious annotate call -- one per named object. And the *"not found"* pattern classified a transient conversion-webhook failure as absence, letting the Job report success while the keep-annotated objects leaked past the uninstall. --ignore-not-found makes the whole classification unnecessary here: absence folds into the same empty-output no-op branch as "present but unannotated", which is exactly the semantics unpin() wants, and any non-zero exit is a real failure that set -e turns into a failed Job. The case/esac disappears; stderr stays on the Job log where it is visible instead of parsed. This also avoids tempfile juggling under the container's readOnlyRootFilesystem. The bats suite gains a previously missing failure-path case and regression tests for both holes, red against the previous commit: a failed KMT listing must fail the Job (FAKE_LIST_FAIL), a read failure whose message contains "not found" must fail the Job, and a success-path stderr warning must not fake a pinned state (FAKE_WARN_STDERR). Absence is now modeled the way real kubectl behaves under --ignore-not-found: success with empty output. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Timofei Larkin
|
bdadb3ff04 |
fix(platform): detect absence via --ignore-not-found
The stderr-based classification this branch introduced had two holes of its own. Capturing stderr into the data variable (2>&1) contaminates the SUCCESS path: kubectl prints warnings on successful reads (API deprecation headers -- deterministic for a deprecated GVK, and CAPI v1beta1 is one -- plus client-side throttling notices), so the warning glued onto the owner annotation would trip adopt_one's foreign-owner refusal on every tenant and permanently deadlock the pre-upgrade hook. And the *"not found"* pattern over-matches: a transient conversion-webhook outage fails a read with '... service "capi-webhook-service" not found', which the classifier would treat as absence -- skipping the keep pin and re-opening the exact fail-open prune hazard this branch exists to close. Detect absence with --ignore-not-found instead: a missing object is SUCCESS with empty output, so a non-zero exit can only be a real failure and aborts before the version stamp, with no stderr parsing at all. adopt_one reads -o json and extracts the owner annotation with jq (already a dependency), keeping absence distinct from present-but- unannotated; pin_keep reads -o name. list_kmts still needs stderr for the one legitimate classification -- the two deterministic missing-CRD messages -- so it captures stderr to a temp file, never into data, and no longer accepts NotFound-shaped text for a list. The fake kubectl honors --ignore-not-found the way real kubectl does, gains FAKE_WARN_STDERR (success-path stderr warnings) and FAKE_GET_FAIL_MSG (transient error with arbitrary text). Two new bats tests, each red against the previous commit: a deprecation warning must not break adoption or fake a foreign owner, and a failure whose message contains "not found" must still abort without stamping. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Timofei Larkin
|
1ca947e2fb |
fix(kubernetes-nodes): fail unpin reads closed
The pre-delete unpin hook's existence check swallowed every kubectl get failure (`... >/dev/null 2>&1 || return 0`), four lines below the comment declaring "Failures are NOT swallowed". kubectl exits 1 for NotFound and for a transient apiserver error alike, so a timeout during uninstall read as "object absent": the unpin was skipped, the Job succeeded, and Helm then skipped the still-keep-annotated MachineDeployment -- leaking the adopted pool's worker VMs into the tenant namespace, running and unmanaged, after the pool was deleted. The KMT-listing path was already hardened for exactly this class; the existence check was the remaining instance. Classify by stderr: NotFound (native pool, or already gone) stays a no-op; any other failure fails the Job, bounded by backoffLimit and activeDeadlineSeconds, with logs retained. The existence check and the annotation read fold into a single request. The embedded script's behavior cannot be asserted from the manifest, so a new bats suite renders the chart, extracts the Job's script, and executes it against a fake kubectl: NotFound no-ops and succeeds, a transient named-get error fails the Job (red against the previous script, which printed "unpin done" and exited 0), keep-annotated objects are unpinned with the 6-hex hash anchor respected, and an unpinned existing object is left untouched. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Timofei Larkin
|
f8258c8859 |
fix(platform): fail migration 54 reads closed
kubectl exits 1 for NotFound and for transient server failures (timeout, 429, etcd hiccup) alike, and migration 54 treated every non-zero read as "object absent". A transient error while reading a worker object would skip the keep pin and the ownership rewrite, let the migration complete and stamp version 55 (so it never re-runs), and the parent's control-plane-only upgrade would then prune the un-pinned live MachineDeployment -- deleting every running worker VM in the pool. The doubly-masked KubevirtMachineTemplate listings (2>/dev/null plus `|| true`) had the same shape: a failed list adopted or pinned zero KMTs, permanently stranding a preserved older-revision KMT still referenced by an in-flight MachineSet, since the child chart only re-emits KMTs already annotated with its own release name. The `cur=$(... || true)` owner read additionally bypassed the foreign-owner refusal guard. Classify reads by stderr, the migration-47 precedent: NotFound means genuinely absent and is skipped; anything else exits 1 so run-migrations.sh leaves CURRENT_VERSION at 54 and retries on the next upgrade. adopt_one and pin_keep fold the existence check into the annotation read (one request instead of two); KMT listings go through a capture-first list_kmts helper because `for x in $(...)` discards the substitution's exit status even under set -e, leaving `|| true` to cover only the empty-grep-match case, mirroring the unpin hook. The child-HR existence check stays deliberately tolerant: its fallback is an idempotent apply that itself fails closed. adopt_one now also stamps meta.helm.sh/release-namespace so an object whose Helm annotations were stripped out-of-band cannot fail the child's ownership validation. The fake kubectl now reports absent objects the way the real apiserver does (NotFound on stderr, exit 1) and gains FAKE_GET_FAIL to inject transient errors; four new bats tests pin the classification: NotFound still completes and stamps, while a transient error on the adopt path, the KMT listing, or the pin path aborts before the version stamp. Each red against the previous script. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Myasnikov Daniil
|
dd308644c8 |
ci(release): execute the release-changelog logic in tests, not just grep it
The existing suite asserts on workflow YAML. That catches a rename or a deleted `if:` and nothing else: it cannot tell whether the shell inside a `run:` block does what the step name claims, and it stayed green while the logic underneath was wrong — twice, in the two preceding rounds. Add hack/release-changelog-behaviour.bats, which runs things for real. The GitHub Actions semantics the whole non-blocking design rests on are now verified by executing them. hack/testdata/needs-semantics.yaml mirrors the promote / changelog / open-pr shapes with a fatal (non-AI) step in the changelog job, and `act` runs it. Result: open-pr DOES run when a job it needs has failed, `needs.changelog.result` is `failure`, and — the part that could not be read off the YAML — a failed job's outputs come back EMPTY, while a successful job's propagate. That empty-output behaviour is precisely why open-pr re-establishes changelog presence from disk instead of trusting needs.changelog.outputs. Deleting the `if:` from the fixture makes open-pr get skipped, so the gate is demonstrably load-bearing. The git logic moves into hack/changelog-preserve.sh and is exercised against real bare remotes: a branch that exists, one that does not, one with no changelog, one with an empty changelog, and the full preserve/checkout -B/restore/force-push cycle promote actually performs. The last of these directly reproduces the regression the preserve step exists to prevent. An empty changelog is deliberately NOT preserved — it would suppress generation and then publish as blank notes. finalize's release-body selection runs under node across all three inputs (good, whitespace-only, absent) and the resulting decision table is compared as a whole. Replace the shell overlay with a sparse second actions/checkout of the dispatch ref at .release-tooling. The overlay had a bootstrap problem I introduced: the script that fetches missing tooling was itself missing from the rc tree, so it could never have worked for exactly the trees that need it. actions/checkout comes from the runner, not the repo, so it has no such circularity. .release-tooling is gitignored because promote runs `git add -A`, and the contract suite pins that. All 21 tests mutation-checked. Breaking changelog-preserve.sh, making it return an empty file, removing the fixture's `if:`, dropping the tooling checkout, and renaming its path each fail the suite and pass on restore. Not covered, and stated in the suite header rather than implied: the AI generation step, the real GitHub API calls, and whether the sparse checkout resolves on GitHub's runners. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
IvanHunters
|
c8ba404646 |
fix(platform): guard migration 54 against RFC-1123-invalid pool names
The 53-char overflow branch exists because exiting the migration deadlocks the platform pre-upgrade hook for every tenant. The equivalent deadlock via name validity was unguarded: the old nodeGroups map key was never schema-constrained (map[string]NodeGroup, no propertyNames pattern), so a stored parent HR can carry a key like 'My_Pool' whose own render already failed at apply time. The migration then built child_name=kubernetes-nodes-<cluster>-My_Pool, kubectl apply rejected the RFC-1123-invalid HelmRelease name, the script exited 1 under set -e, and every subsequent platform upgrade re-ran and re-failed. Validate child_name against the RFC-1123 label pattern alongside the length check and route failures through the same warn + pin + skip path. Factor the pin-and-skip logic shared by both guards into pin_pool_objects. Cover the new branch in migration-54-adopt-path.bats (the fake kubectl apply now rejects an invalid metadata.name, so the case fails without the guard). Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
8f24cfb56c |
fix(platform): point migration fixtures and docs at renumbered migration 54
The Phase-2 worker-pool adoption migration was renumbered 52 -> 54 during the rebase onto main (main added migrations 52 linstor-scheduler and 53 seaweedfs). Rename the adoption bats tests and their testdata directory, and update the kubernetes-nodes pre-delete hook comment and the kubernetes README so they reference migration 54. The bats MIG path pointed at migrations/52 (now the linstor migration), so without this the adoption unit tests would exercise the wrong migration. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Ivan Okhotnikov
|
386053ba8f |
fix(platform): harden migration 52 (name overflow, images, adoption tests)
Address review on #3315: - Name-length overflow no longer exits 1 (which would deadlock the platform pre-upgrade hook for every tenant). Instead pin the pool's worker objects with resource-policy=keep, skip the child HR, warn loudly, and continue. - Copy only images.kubectl into the child values, not the parent's whole images map (waitForKubeconfig/talosCsrSigner are undeclared in the schema). - Note that the pre-upgrade window is not quiet (a parent HR edit mid-upgrade renders the old chart and self-heals once the new artifact lands). - Cover the adoption path directly: annotate keep + release-name, idempotent skip, unexpected-owner refusal, and the 6-hex KMT anchor rejecting a sibling pool's template. Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com> |
||
|
Ivan Okhotnikov
|
c11e3d6682 |
test(platform): guard migration 52 copies version/talos/images into child HR
Drives migration 52 against a fake kubectl and asserts the adopted child HelmRelease carries the tenant's non-default talos.version, version and images (plus group fields and storageClass fallback). Fails against the pick(o;...) filter-parameter bug that dropped those keys and rolled live worker VMs; passes against the pick($o;...) fix. Signed-off-by: Ivan Okhotnikov <xorokhotnikov@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
|
0ba5337526 |
fix(migrations): key the etcd-adoption endpoint scheme off the claimed bucket
Deriving the safety-snapshot target from the projected cozy-backups-creds Secret is right, but detecting the admin-managed external-S3 case (provisionBucket: false) by "the projected endpoint is absent" is not: that state does not exist. The projector substitutes BACKUP_STORAGE_ENDPOINT and fails ReasonSourceMalformed when both sources are empty, so a projected Secret always carries a non-empty, scheme-stripped endpoint. So for provisionBucket: false against a plaintext external S3 the projected endpoint is a bare host, the script saw it non-empty, forced https:// and never consulted the Strategy CR — a regression against main, which read the CR verbatim and passed http:// through. Being a pre-upgrade hook with no ETCD_ADOPT_SKIP_BACKUP plumbed through migration-hook.yaml, that is a total upgrade block with no operator escape. Classify on data instead: match the projected bucketName against the .status.bucketName of any BucketClaim. The COSI driver assigns that name and the projector republishes exactly it (packages/system/bucket user-credentials.yaml reads BucketInfo spec.bucketName), so a match means the creds describe a COSI bucket => the host is the always-TLS S3 ingress => force https, as the 1.6 chart helper does. No match means external S3. Keying on data rather than on the claim's name and namespace matters twice. backupStorage.namespace/.bucketName are supported Package-CR overrides that this hook cannot see, so a name lookup would miss a renamed bucket and fall back to the v1.5.x plaintext CR — the original P0. And a BucketClaim wedged Terminating (its cosi bucketclaim-protection finalizer outlives an uninstalled COSI controller) keeps answering a name lookup while its status names some other bucket. Split the two paths by which source is authoritative. On COSI the projected coordinates win: the bucket is the COSI-assigned name the CR can only reproduce through a live lookup that may never have run. On external S3 the Strategy CR wins WHOLESALE, not just for the endpoint — the CR renders backupStorage.* and points at this same cozy-backups-creds, so CR coordinates plus these credentials is exactly what the cluster's own BackupJobs already use, and the bucket an operator will look in. Keeping the Secret's bucket while taking the CR's endpoint assembled a pairing nothing else produces: the projector copies bucketName straight from the admin-managed source Secret and never from backupStorage.bucketName, so the two can disagree outright and the snapshot could land in a bucket the platform never writes. Relying on the CR is safe on this path alone, because bucketName short-circuits to values before the BucketClaim lookup that races on the COSI path, so the CR is guaranteed present. Match with a single jq `any`, never `jq -r | grep -q`: grep exits on the first match and SIGPIPEs jq, and pipefail reports jq's 141 rather than grep's 0, so a MATCH returns as a failure and the cluster is misclassified external. It is a race on the list outgrowing one stdio write (~8KB) with only a last-line match safe, so it strikes exactly the big multi-tenant clusters that can least afford it, and backoffLimit 3 can decide differently on each retry. Measured with the match first, 20 runs each: 300 claims went 0/20 correct (rc=141 every time) and 2000 likewise, while `any` is 20/20 at 3, 300 and 2000. Refuse rather than guess when the answer is not knowable. Three states qualify: the BucketClaim API is unreadable; the list is not interpretable (no producer emits that, but without a shape gate it reads as "no claim" = external = the plaintext P0, silently); or the bucket is claimed ONLY by a Terminating claim. That last one is ambiguous — an admin who moved COSI -> external S3 reusing the bucket name, versus a still-COSI cluster whose claim is mid-recreate — and both readings fail as the same confusing handshake error when wrong, so a guess buys no reliability while reading it as "external" specifically resurrects the plaintext-:8333 P0. A live claim still wins over an unrelated Terminating one, so healthy clusters are unaffected. Each refusal names the condition and the operator action; the migration is idempotent, so re-running after resolving it is the remedy. Behaviour change worth noting: a cluster on external S3 that still has COSI installed now refuses on a sustained BucketClaim-API failure where main would have proceeded off the Strategy CR. That is the intended trade — main proceeded only because it never had to tell the two apart — but it does mean an apiserver outage can now block this hook where it previously did not. ETCD_ADOPT_SKIP_BACKUP is still not plumbed through migration-hook.yaml, so the messages deliberately point at resolving the condition rather than at an escape hatch the operator cannot reach. Force the scheme only onto a non-empty host: an absent endpoint must degrade to the CR as main did, not to a literal "https://" that would pass the final non-empty guard and reach etcd-migrate as a destination. The old test modelled external S3 by dropping the projected endpoint entirely — a state the projector cannot produce — so it green-lit the regression. It now models what the projector really writes, and the suite pins the stale-claim, overridden-coordinates, empty-endpoint, 300-claim-multi-tenant, Terminating-match, hybrid-coordinate and unreadable/uninterpretable API cases. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
9ceccd2b07 |
fix(migrations): derive the etcd-adoption snapshot target from the projected bucket creds
Migration 50 is a pre-upgrade hook, so every chart-rendered resource it reads still belongs to the version being upgraded FROM. It read the snapshot target from the live cozy-default-etcd Strategy CR, which on a v1.5.x cluster is broken in one of two ways — and the platform upgrade is gated behind this snapshot, so neither can self-heal first: 1. Absent. strategy-etcd-default.yaml is guarded by `if $bucketName`, whose helper looks up the BucketClaim status. That BucketClaim is created by the same chart, so on first render the lookup is empty and the Strategy is silently skipped. Helm lookup only runs at install/upgrade time, so a cluster that reached 1.5.x in one hop never grows the CR on reconcile. 2. Present, carrying the static v1.5.x default http://seaweedfs-s3.tenant-root.svc.cozy.local:8333. Cozystack ships SeaweedFS with enableSecurity=true, so :8333 is a TLS listener and plaintext against it fails the handshake. Fixing the scheme alone is not enough: the Etcd Strategy's S3 schema has no caCert/insecureSkipVerify, so the self-signed in-cluster endpoint is unusable at any scheme and a trusted-cert endpoint is required. Either way migration 50 hard-fails and blocks the upgrade, and the documented ETCD_ADOPT_SKIP_BACKUP escape hatch cannot be reached — migration-hook.yaml passes only NAMESPACE/CURRENT_VERSION/TARGET_VERSION. Take the coordinates from cozy-backups-creds instead. The projector writes it from the COSI-provisioned bucket's system credentials — bucket provisioning, not chart values — so it describes the live bucket regardless of which version rendered the charts, and it carries endpoint, bucketName, region and forcePathStyle alongside the AWS keys. resolve_platform_backup_args already read this Secret to validate those keys, so preferring it adds no new dependency. The endpoint is a bare host fronted by the always-TLS S3 ingress, so force https://, exactly as the 1.6 chart helper backupstrategy-controller.endpoint does. The Strategy CR remains the fallback for the admin-managed external-S3 case (provisionBucket: false), where .Values.backupStorage.endpoint is authoritative and may legitimately be plaintext against a private store; that path is detected by the absence of a projected endpoint and its scheme is preserved verbatim. The bats fixture modelled cozy-backups-creds as carrying only the AWS keys, and test 1 asserted the broken plaintext endpoint as expected output — the test was written against the implementation rather than the requirement, which is why the defect shipped green. The fixture now carries the coordinates the real projected Secret has, test 1 asserts the derived https endpoint, and two cases are added: an absent Strategy CR still resolving, and external S3 keeping the CR endpoint verbatim. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Andrey Kolkov
|
47d239c519 |
fix(platform): address review on migration 50 (exact SAN match, Secret-gated wait, IPv6 kubeconfig)
Review feedback on #3261 (gemini-code-assist, coderabbitai, myasnikovdaniil): - Exact SAN match. _san_present read the Certificate SANs as {.spec.dnsNames} (a bracketed JSON blob that `tr ','` cannot split) and matched with a substring `grep -qF`, so a wildcard that is a substring of a longer SAN (e.g. *.etcd.<ns>.svc inside *.etcd.<ns>.svc.cluster.local) could false-positive and skip the re-issue. Read {.spec.dnsNames[*]} (space separated) and match exactly with `grep -qxF`, matching the Secret-annotation branch. - Wait on the re-issued Secret, not the patched spec. The post-patch wait loop called _san_present, which returns as soon as the Certificate spec.dnsNames contains the wildcard -- but the patch just added it there, so the loop broke on the first iteration before cert-manager re-issued the Secret, and the "re-issued" log printed unconfirmed. Add _secret_has_san (Secret alt-names annotation only, same empty-read retry + exact match) and gate the wait on it. - IPv6-safe kubeconfig server. The synthesized kubeconfig used https://$KUBERNETES_SERVICE_HOST:$KUBERNETES_SERVICE_PORT, which is an invalid URL on an IPv6-only cluster (a bare IPv6 host must be bracketed). Use https://kubernetes.default.svc -- IP-family-agnostic, validated by the mounted SA CA, and the same server URL kubectl synthesizes in-cluster. Tests (the review blocker): extend the bats suite (and its fake kubectl) with two cases pinning these contracts -- a superstring dnsName must NOT skip the re-issue patch, and etcd-migrate must be invoked with a synthesized in-cluster kubeconfig whose server is kubernetes.default.svc (not the bare IPv6 host), authenticating via the SA token file + CA. Both the kubeconfig path and the SA directory are now env-overridable (ETCD_MIGRATE_KUBECONFIG, ETCD_ADOPT_SA_DIR) so the synthesis is exercisable off-cluster; production defaults are unchanged. Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
d0740ed8c7 |
test(etcd): gate S3 backup round-trip behind ETCD_E2E_S3_ROUNDTRIP
Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey Kolkov
|
cc5d688e15 |
feat(platform): migrate to etcd-operator v1alpha2
Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrei Kvapil
|
5d7e56bffe |
[tests] refactor tests and remove e2e.applications
Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
klinch0
|
b9e80b9a91 | add endpoint for http checks (#469) | ||
|
klinch0
|
f06f653744 |
add values and checks (#454)
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a new configuration file for Kubernetes deployments, enhancing clarity on parameters and settings. - Added common parameters for NATS, including external access and persistent volume settings. - **Bug Fixes** - Improved error handling and feedback in Helm release management scripts. - **Chores** - Reduced verbosity in test output by removing unnecessary echo statements in the testing Makefile. - Added success return statements in various check scripts to ensure proper termination. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |