mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 22:13:18 +09:00
IvanHunters
cbb2692db0
## What this PR does Stacked on #3315. The worker-pool adoption path treats `kubectl` reads as fail-open: `kubectl` exits 1 for NotFound and for transient server failures (timeout, 429, etcd hiccup) alike, and three code paths classified every non-zero read as "object absent". Each sits exactly where absence and failure must not be confused, because "absent" there means "do not pin" or "do not unpin": - **Migration 54, `adopt_one`/`pin_keep`**: a transient error while reading a worker object skipped the keep pin and the ownership rewrite, the migration still completed and stamped 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 `cur=$(... || true)` owner read additionally bypassed the foreign-owner refusal guard. - **Migration 54, KubevirtMachineTemplate listings** (adopt loop and `pin_pool_objects`): masked twice (`2>/dev/null` plus `|| true`), so a failed list adopted or pinned zero KMTs and the parent upgrade would prune a preserved older-revision KMT still referenced by an in-flight MachineSet — permanently, since the child chart only re-emits KMTs already annotated with its own release name. - **Pre-delete unpin hook, existence check**: `kubectl get ... >/dev/null 2>&1 || return 0` four lines below the comment declaring "Failures are NOT swallowed". A timeout during uninstall read as absent, the Job reported success, and Helm 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 fix applies one contract everywhere: absence is detected with `--ignore-not-found` — a missing object is success with empty output — so a non-zero read exit can only be a real failure and aborts (the migration exits 1 before the version stamp so `run-migrations.sh` retries on the next upgrade; the hook Job fails via `set -e` and is retried by `backoffLimit` with logs retained). Two properties fall out of choosing `--ignore-not-found` over classifying stderr text. stderr is never captured into data: kubectl prints warnings on *successful* reads (API deprecation headers — deterministic for a deprecated GVK — and client-side throttling notices), and a warning glued onto a `2>&1`-captured annotation value would trip the foreign-owner refusal or fake a pinned state. And absence is never inferred from message text: a transient conversion-webhook outage fails a read with `... service "capi-webhook-service" not found`, which a `*"not found"*` match would misclassify as absence, silently re-opening the prune hazard. Mechanically: `adopt_one` reads `-o json --ignore-not-found` and extracts the owner annotation with jq (already a dependency), keeping absence distinct from present-but-unannotated; `pin_keep` reads `-o name`; the hook's `unpin()` folds absence and "present but unannotated" into one empty-output no-op, which is exactly its semantics, and the whole error-classification branch disappears. `list_kmts` is the one read that still needs stderr — for the two deterministic missing-CRD messages (migration-47 precedent) — so it captures stderr to a temp file, never into the data variable; KMT listings capture into a variable first because `for x in $(...)` discards the substitution's exit status even under `set -e`, leaving `|| true` to cover only the empty-grep-match case. The one deliberately tolerant read — the child-HR existence check — is annotated as such: its fallback is an idempotent apply that itself fails closed. `adopt_one` also stamps `meta.helm.sh/release-namespace` alongside the release name, closing the corner where an object with out-of-band-stripped Helm annotations would fail the child's ownership validation. Every kubectl invocation in migration 54 and the hook is now either fail-closed or carries a comment stating why fail-open is safe at that call site. Fail-closed reads cut the other way for names that cannot exist: kubectl rejects a name containing `/` client-side and deterministically, so a pin-path read for one would deadlock the migration on every retry. `pin_pool_objects` therefore skips statically when the composed `kubernetes-<cluster>-<group>` is not a valid DNS-1123 subdomain — such a name provably never had an object applied under it, so there is nothing to pin and no read to misclassify; keys that are label-invalid but still valid object names (dotted) keep the pin behavior. The PR also re-homes three control-plane-chart comments that still described the talos-reconcile Job as living in that chart (it runs per-pool in kubernetes-nodes releases since the split, and a pool-less cluster keeps DNS SANs only — a best-effort extra nothing depends on). Tests, each red against the code it guards (verified by running the pre-fix scripts under the same injections): - The migration-54 fake kubectl models absence the way real kubectl behaves — `--ignore-not-found` present: success + empty; absent: `Error from server (NotFound)` + exit 1 — and gains `FAKE_GET_FAIL`/`FAKE_GET_FAIL_MSG` for transient-error injection and `FAKE_WARN_STDERR` for success-path stderr warnings. Six new bats tests pin the contract: an absent object still completes and stamps; a transient error on the adopt path, the KMT listing, or the pin path aborts before the version stamp; a deprecation warning does not break adoption or fake a foreign owner; and a failure whose message contains "not found" still aborts without stamping. - New `hack/kubernetes-nodes-unpin-hook.bats` tests the hook's embedded script behaviorally — renders the chart, extracts the Job's args, executes them against a fake kubectl — since error classification cannot be asserted from the manifest. Seven cases: absent objects no-op and succeed (native pool preserved), a transient named-get error fails the Job (the original script printed "unpin done" and exited 0 under the same injection), a "not found"-worded failure fails the Job, a failed KMT listing fails the Job, a success-path warning does not fake a pinned state (the `2>&1` variant issued three spurious annotate calls), keep-annotated objects are unpinned with the 6-hex hash anchor respected, and an existing-but-unpinned object is untouched. Verified locally: both migration-54 bats suites (14 + 3 tests), the hook suite (7 tests), and `helm unittest` for both charts (kubernetes-nodes 32 tests + 12 snapshots, kubernetes 117 tests) all pass. The slash-key deadlock was reproduced against the pre-fix script (exit 1 at the first pin read, no stamp) before the fix turned it into a warn-and-skip. ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. --> - [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: Walked the trigger map against the diff: a platform migration script, a hook script inside an existing chart template, and unit-test additions under `hack/`. No package added or renamed, no values/schema/API/README change, no `hack/*.mk` or make-target change, no node-prerequisite change — no trigger matches. ### Release note ```release-note fix(platform): the worker-pool adoption migration and the kubernetes-nodes pre-delete unpin hook fail closed on transient apiserver errors instead of misreading them as absent objects, which could otherwise let a platform upgrade prune a live worker pool or let a pool deletion leak its VMs ```
Cozystack
Cozystack is a free platform and framework for building clouds.
Cozystack is a CNCF Sandbox Level Project that was originally built and sponsored by Ænix.
With Cozystack, you can transform a bunch of servers into an intelligent system with a simple REST API for spawning Kubernetes clusters, Database-as-a-Service, virtual machines, load balancers, HTTP caching services, and other services with ease.
Use Cozystack to build your own cloud or provide a cost-effective development environment.
Use-Cases
-
Using Cozystack to build a public cloud
You can use Cozystack as a backend for a public cloud -
Using Cozystack to build a private cloud
You can use Cozystack as a platform to build a private cloud powered by Infrastructure-as-Code approach -
Using Cozystack as a Kubernetes distribution
You can use Cozystack as a Kubernetes distribution for Bare Metal
Documentation
The documentation is located on the cozystack.io website.
Read the Getting Started section for a quick start.
If you encounter any difficulties, start with the troubleshooting guide and work your way through the process that we've outlined.
Versioning
Versioning adheres to the Semantic Versioning principles.
A full list of the available releases is available in the GitHub repository's Release section.
Contributions
Contributions are highly appreciated and very welcomed!
In case of bugs, please check if the issue has already been opened by checking the GitHub Issues section. If it isn't, you can open a new one. A detailed report will help us replicate it, assess it, and work on a fix.
You can express your intention to on the fix on your own. Commits are used to generate the changelog, and their author will be referenced in it.
If you have a concrete feature request, open an issue. For a cross-cutting or architectural change, open a design proposal in the community repo. For questions or early ideas, use GitHub Discussions. See CONTRIBUTING.md for the full routing.
Community
You are welcome to join our Telegram group and come to our weekly community meetings. Add them to your Google Calendar or iCal for convenience.
License
Cozystack is licensed under Apache 2.0.
The code is provided as-is with no warranties.
Commercial Support
A list of companies providing commercial support for this project can be found on official site.