mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-06 03:57:52 +09:00
77f6cd1af892004fed62fcdd960f189f920ca86d
1293
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
94b328c6eb |
test(e2e): hold every raised backup wait to a start margin
The wait-budget guard accepted a wait one second over the install timeout, although the wait's clock starts before the install's by the Application reconcile, the chart fetch and the helm-controller queue. A wait lowered from 660 to 601 passed every test and brought back the bare assert timeout the budgets exist to avoid. A wait now has to clear its floor by a named 60s margin. Nothing in the tree bounds those three delays, so the value is policy: the headroom every current wait already carries. The sizing guide in docs/agents/e2e-testing.md asked only for "strictly above", which the guard now rejects; it points at the margin instead of restating a number. The mongodb, rabbitmq and clickhouse walkthrough waits were raised for the same reason as mariadb's, and their Chainsaw ops grown with them, but nothing held either. Each suite is now a row in one table and gets its own floor and op tests, so a regression reds only its own suite. Floors come from the api-server install-timeout default and, for mariadb, the chart's startup probe; each suite's RD is checked for an override instead of being assumed not to carry one. Each op keeps the remainder it had before the raise over every HelmRelease wait it reaches, the bucket one included, so raising a wait and its op together keeps the remainder instead of widening the slack. The mongodb target behind SKIP_RESTORE=1 is not reached and is left out until the op stops passing it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
54159736bc |
feat(talos)!: bump Talos to v1.14.2 (#4598)
## What this PR does Bumps the Talos image Cozystack builds from v1.13.6 to v1.14.2 to get a newer DRBD. v1.13.6 ships DRBD 9.3.2, v1.13.7 and later and v1.14.0/v1.14.1 ship 9.3.3, and v1.14.2 is the first release with 9.3.4. ZFS moves from 2.4.3 to 2.4.4. On 9.3.2 I've seen a peer reboot leave the DRBD sender thread spinning on a failed send (EPIPE). What followed was RCU stalls, processes stuck in D-state, a hanging `drbdsetup disconnect` and two-phase-commit timeouts. The [9.3.4 ChangeLog](https://github.com/LINBIT/drbd/blob/drbd-9.3.4/ChangeLog) has fixes for a sender thread pinning a CPU while its connection is down, for two-phase-commit issues and a hanging `drbdadm disconnect`, and for two nodes ending UpToDate with different data after a reconnect. None of them are in 9.3.3. I matched the trace to those entries by their descriptions only, not against the commits. The base installer image had to change. Talos v1.14 no longer publishes `ghcr.io/siderolabs/installer`, so `gen-profiles.sh` now uses `ghcr.io/siderolabs/installer-base`, same as upstream's own v1.14 imager profiles. I built the amd64 installer from the new profile with `imager:v1.14.2` locally and it builds without errors. PR CI will not boot the new image. It builds the installer and matchbox images, but the e2e runs on the upstream `talos:v1.13.5` container from `hack/e2e-compose.yaml`. The first run that boots the new kernel, DRBD and ZFS is the nightly after merge, which builds the nocloud disk from these profiles and runs it in QEMU. On Talos v1.14.2 the `drbd_transport_tcp` module is no longer loaded on demand. After upgrading a node from v1.13.6, `drbd` was loaded and the transport module was not, `drbdsetup new-peer` failed with "Failed to create transport (drbd_transport_xxx module missing?)", and every DRBD resource on the node stayed in Connecting. Listing the module in `machine.kernel.modules` fixed it without a reboot. The cause is [siderolabs/pkgs#1565](https://github.com/siderolabs/pkgs/pull/1565), in Talos since v1.14.0: the kernel is built with an empty modprobe path, so it no longer loads modules on `request_module()`, which is how DRBD asks for its transport. Any module that used to load that way now has to be listed explicitly (reported upstream as siderolabs/talos#14501). Outside DRBD this includes `dm-thin-pool` and `dm-multipath`, which the v1.14.2 kernel builds as modules, so LVM-thin or multipath users on Talos need them listed too. The e2e node config now loads `drbd_transport_tcp` explicitly, and the talm preset (cozystack/talm#252) and the install docs (cozystack/website#720) get the same line. The module ships in the drbd extension on v1.13 too, so the extra line is safe before the upgrade. The order matters for host nodes. Either update the cozystack preset in the talm project to a version with cozystack/talm#252 and re-render and apply the node config, or add `drbd_transport_tcp` to `machine.kernel.modules` by hand. Only then upgrade the node to v1.14.2. A node upgraded first loses DRBD replication until the module is added. The host network policy now also denies port 2383 to `world`. Talos v1.14 moved etcd's `/metrics`, `/health` and gRPC-gateway JSON API from 2379 to a separate listener on 2383, and the policy only blocked 2379 and 2380. Without this, the etcd JSON API on v1.14 control-plane nodes would be reachable from outside, protected only by client mTLS. Two smaller changes come with the bump. The multus `talos-cni-plugins-checked-against` marker moves to v1.14.2, because the pkgs release behind Talos v1.14.2 still pins CNI plugins v1.9.1, same as multus. The system memory limits doc now describes the v1.14 OOM trigger, since v1.14.0 dropped its global memory PSI clause ([siderolabs/talos#13895](https://github.com/siderolabs/talos/pull/13895)). Before upgrading hosts, operators should know a few things from the [v1.14.0 release notes](https://github.com/siderolabs/talos/releases/tag/v1.14.0) and the Talos compatibility code: - Talos v1.14 accepts a host upgrade from v1.12.0 or later. It supports Kubernetes 1.32 through 1.37, while v1.13 still accepted 1.31, so a cluster on Kubernetes 1.31 has to upgrade Kubernetes first. - etcd's HTTP endpoints (`/metrics`, `/health`) moved from port 2379 to 2383. The Cozystack etcd scrape proxy reads a separate metrics listener on `127.0.0.1:2381`, which this change does not move. Anything else that scraped or health-checked etcd on 2379 has to switch to 2383, and a firewall outside the cluster that blocked 2379 should block 2383 too. - etcd and kube-apiserver now require TLS 1.3, so a client that can only speak TLS 1.2 stops connecting. - Workload isolation (`SecurityProfileConfig`) stays off on upgraded clusters, but `talosctl gen config` turns it on for new ones. I haven't tested Cozystack with it enabled. - `talosctl apply-config --mode=reboot` is gone. ### Screenshots Not a UI change. ### Downstream repositories - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: cozystack/website#720 - [ ] [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: - [x] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: cozystack/talm#252 - [ ] [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: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The website PR refreshes the `next` version pins for v1.14.2 and adds the module to the Talos install pages. The talm PR adds it to the cozystack preset. ### Release note ```release-note feat(talos): Bump Talos to v1.14.2, which ships DRBD 9.3.4 and ZFS 2.4.4. DRBD 9.3.4 fixes a sender thread spinning on a dead connection, two-phase-commit hangs and a data divergence after a reconnect. Before upgrading a node, add `drbd_transport_tcp` to its Talos `machine.kernel.modules` next to `drbd`: Talos v1.14.2 no longer loads it on demand, and without it DRBD cannot connect to any peer. We recommend skipping the v1.13 line and upgrading nodes straight to v1.14.2. Talos accepts a direct upgrade from v1.12.0 or later. Talos v1.14 moves etcd's HTTP metrics and health endpoints from port 2379 to 2383, and the Cozystack host network policy now blocks 2383 from outside the cluster. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Updates** * Upgraded host node support to Talos v1.14.2 across machine images, with refreshed firmware and storage extensions. * Updated memory-pressure guidance for Talos v1.14, including version-specific behavior and configuration recommendations. * Added protection for Talos v1.14’s etcd HTTP endpoint on port 2383. * Updated cluster preparation to load the TCP transport module required for DRBD. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
312409ecd2 |
test(e2e): fail the suite whose pods outlive its cleanup (#4492)
## What this PR does A Chainsaw suite could pass while the workloads it created kept running after its own cleanup, and nothing noticed until a later suite failed. On one E2E run the brokers of four Kafka releases from suites that had passed and deleted their applications (#4488), plus a TenantControlPlane sitting on its finalizer with two apiserver pods (#4489), pushed pod memory requests to the node limit. Harbor then failed on `Insufficient memory` two suites later, under the wrong name. This makes the suite that leaked fail, and name what it left behind. - Chainsaw v0.2.15 has no hook that runs after a test's cleanup: its `Configuration` carries `error.catch`, which runs only on failure. So `make test-chainsaw` now runs `hack/e2e-chainsaw-run.sh`, which invokes `chainsaw test` once per suite directory and checks the cluster between invocations with `hack/e2e-chainsaw/_lib/leak-guard.sh`. - Attribution: a pod is charged to a suite when all three hold. It is not terminal. Its UID was not in the pod list taken before the suite. The root of its ownerReferences chain was created at or after the suite started. A platform pod replaced mid-suite by a rollout, an eviction or a crash traces back to an object the install created, so it is not charged. A pre-existing pod adopted by a controller recreated mid-suite is excluded by the UID baseline. A leak is charged once, to its own suite. - The suite start is read off an apiserver, from the `creationTimestamp` of a server-side dry-run create, rather than off the sandbox. With several control-plane nodes the reading and a root's timestamp can come from different apiservers, so the rule assumes their clocks agree more closely than the gap between the reading and the suite's first object; no skew margin is added. Source for that behaviour: the generic registry `Store.create` stamps it through `rest.FillObjectMetaSystemFields`, then `DryRunnableStorage.Create` copies it back without persisting (`staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go` and `dryrun.go` at Kubernetes v1.33.0). - Budget: charged pods get 300s to disappear, the default `timeouts.cleanup` in `.chainsaw.yaml`; a unit test keeps the two equal. A suite that overrides its own cleanup timeout (kafka sets 8m) is covered the same way, because the check runs only after `chainsaw test` has returned, which is after every cleanup in the suite has finished under its own timeout; the 300s start from there. The leaks this targets sat about 110 minutes (Kafka brokers) and 30+ minutes (the TenantControlPlane) past their suites. Every green suite logs `leak-guard: suite=<dir> clear after Ns`, so the number can be checked against later runs. - Past the budget the run prints `LEAKED-PODS suite=<dir> budget=300s count=N`, then one line per pod with its phase, deletion state, root and `app.kubernetes.io/instance`. It raises a `leaked-pods` error annotation and adds a failed `leak-guard` test case to `chainsaw-report.xml`. The run continues through the remaining suites and exits non-zero at the end, as the single invocation did on a red test. - A suite that already failed on its own is checked once without the wait. Its leftovers are reported as a `::warning` with `LEAKED-PODS suite=<dir> checked-once=chainsaw-failed`, since some may still be tearing down. - Each pod gets one `kubectl wait` inside the remaining budget, and nothing is retried. A timeout or a spent budget gives the verdict from the last good pod list, also when the re-list after it fails, so a leak fails the suite. A wait that fails only after running for its whole `--timeout` counts as a timeout whatever it printed: kubectl maps an expired context to "timed out waiting for the condition" only inside the watch, so a deadline that passes during its initial lookup surfaces as "context deadline exceeded" (`wait.go` and `delete.go` in kubectl v1.33.0). A spent budget is caught before calling kubectl and never passed as `--timeout=0`, which means "check once". - Any other error on a wait or a re-list, before the budget is spent and before any timeout, stops the waiting and is reported as `LEAKED-PODS suite=<dir> count=N inconclusive="<error>"`. That is a warning, not a leak failure, so an apiserver blip cannot become a leak red. - A failed first reading after a suite, including a pod list that cannot be parsed, leaves the suite unchecked. It fails with a `leak-guard` annotation and a report case saying the cluster could not be read, not that it leaked. Errors that are not leaks carry the annotation title `leak-guard`, not `leaked-pods`. - Known leaks are listed in `hack/e2e-chainsaw/_lib/leak-allowlist.txt` as `<suite-dir> <issue-ref>`: `kubernetes-latest` and `kubernetes-previous` against #4489 (both suites create a TenantControlPlane through `_lib/run-kubernetes.sh`). `kafka` and `kafka-metadata` were listed against #4488 until #4490 fixed it; those entries are gone. An allowlisted suite prints its leak as a `::warning` and does not fail. An entry that names a directory that is not a suite stops the run before the first suite, so a rename cannot keep an exemption alive. The PR fixing each issue deletes its lines. Suite selection is unchanged. The runner runs the named directories when there are any, and otherwise every directory directly under `hack/e2e-chainsaw/`, each handed to Chainsaw's own discovery; it refuses a tree with a test at its top level, where that split would stop being exact. I verified this with Chainsaw v0.2.15's own `discovery.DiscoverTests`, built from source at that tag. For the full tree, for `kafka redis harbor` and for `kubernetes-latest`, the union of the per-suite discoveries equals what a single `chainsaw test [suites]` discovers: 57, 7 and 2 tests, identical lists. 57 matches the test count in an earlier full run's report. A unit test pins that the runner passes chainsaw the suites and flags it was given, in order. The order changes too: suites used to run in Chainsaw's own order and now run one directory at a time in alphabetical order, which moves the kubernetes suites after the kafka ones. Before #4490 fixed #4488 this starved them (see the E2E criterion below). With the fix merged the order has no effect, and a future leak that lands before a memory-heavy suite is named at the suite that leaked. One behaviour does change: a selected name that is not a directory used to fail the single invocation before any test ran; now only that suite fails and the others run. `chainsaw-report.xml` keeps its path and shape: one `<testsuites>` root with the summed counts, one `<testsuite>` per suite directory. Its consumers are the `Collect chainsaw report` / `Upload chainsaw report` steps in `pull-requests.yaml`, `e2e-fork.yaml`, `nightly.yaml` and `e2e-tag.yaml`, which `docker cp` the file and upload it as an artifact. Nothing in the repository parses it. The merge is unit-tested on two per-suite reports in Chainsaw's real shape, one of them failed, and the result is checked to parse as XML. Tests: `hack/chainsaw-leak-guard_test.bats` (31 tests, in `make unit-tests`; named without the `e2e-` prefix, which the unit lane excludes) covers the attribution rule, the budget, the allowlist, the merge and the runner, all against stub `kubectl` and `chainsaw`. Every rule was mutation-checked: removing the guard call, having the Makefile bypass the runner, inverting the allowlist match, flipping or tightening the root-timestamp comparison, removing the UID baseline, removing the terminal-phase filter, skipping the budget wait, changing the budget, lowering or raising the depth cap, accepting a stale allowlist entry, reading an unreadable pod list as empty, dropping the leak case from the report, skipping directories in the selection, treating any failed owner read as a missing owner, dropping the separate report for a suite that already failed, dropping the not-run reason from the report, reading any wait error as a timeout, removing the inconclusive branch, letting a failed re-list after a timeout turn a leak inconclusive, ignoring a failed re-list, reporting an unreadable cluster as a leak, reading an unparsable pod list as clean, reporting a kubectl warning instead of the error, passing the report name without its extension, wording a failed re-list as a failed wait, dropping an allowlist's last line when it has no newline, dropping the check that `--set-string` carries a value, and reading a wait that ran its whole timeout but printed a context error as inconclusive. Each one turns its named test red, and restoring turns it green. E2E criterion, registered before the run: in the `Run E2E tests` log of this PR, every suite that ran prints `leak-guard: suite=<dir> clear after`, a `LEAKED-PODS suite=<dir>` line carrying `allowlisted=`, or one carrying `checked-once=` (a suite that failed on its own) or `inconclusive=` (waiting stopped on a failed read), and no line matches `LEAKED-PODS suite=.* budget=`. With #4490 merged, `kafka` and `kafka-metadata` are no longer allowlisted and must print `leak-guard: suite=kafka clear after` and `leak-guard: suite=kafka-metadata clear after`, which is the live check that #4490 fixed #4488. The first run of this PR (before #4490, run 36223896793) is the positive control: it printed `LEAKED-PODS suite=kafka count=15` and `suite=kafka-metadata count=8`, allowlisted, and every other suite cleared in 0-37s. That run also went red on both kubernetes suites, because those 23 leaked brokers left no memory for the second tenant worker VM (`0/3 nodes are available: 3 Insufficient memory`), which the order change described above made possible. The guard has not yet run against a real cluster, so the first run of this PR should carry the `full-e2e` label; its `clear after` lines are the first measured teardown times, and any unexpected `LEAKED-PODS` line is either a real leak or a false positive to understand before merge. Follow-ups, known and out of scope here: - The allowlist exempts a whole suite, so a new leak in an allowlisted suite stays hidden until the issue's fix removes the line. - Attribution is per suite directory, not per Chainsaw `Test`; the leak lines carry the release instance, which usually names the test. - Only pods are checked. A leaked PVC, LoadBalancer IP or bucket holds no node memory and is not seen. - The 300s budget rests on Chainsaw's cleanup timeout, not on a measured pod-gone latency; the `clear after` lines are where that measurement will come from. - A platform object recreated mid-suite (for example a HelmRelease uninstall/reinstall remediation, or a force-replace on an immutable field) has a root newer than the suite start, and its pods would be charged to whichever suite was running as a leak; the first CI runs are where to look for this. - The per-pod wait recomputes the remaining budget before each pod, and no unit test pins that. - `kubernetes-previous` is allowlisted for #4489 although the stuck finalizer was seen only in `kubernetes-latest`; both suites create a TenantControlPlane the same way, and the exemption hides any new leak there until #4489 is fixed. - Each clean suite now waits for its own pods to finish terminating before the next starts, where the tail used to overlap the next suite. The first full run's `clear after` lines say what that costs against the E2E job's 215-minute cap. - After a timeout followed by a failed re-list, the leak lines come from the list taken before the wait, so a pod whose own wait had already succeeded is still named and the count can be too high; the verdict is unaffected. - Likewise, when every wait succeeded and the re-list then fails, the inconclusive report still lists the pods whose deletion was just confirmed. In the boundary case where the budget runs out during the last successful wait and the re-list then fails, the result is inconclusive rather than a verdict from the last good list, because the guard only treats the budget as spent when it finds it exhausted at a check. - A mistyped `COZY_LEAK_ALLOWLIST` path passes allowlist validation; it fails closed, since no suite is then exempt. - An allowlisted suite is not waited on, so `kubernetes-latest` and `kubernetes-previous` will print `LEAKED-PODS ... allowlisted=` on most runs even for pods still in normal teardown; waiting out the budget first and applying the allowlist only to what remains would stop that noise. - A slow teardown in each of several suites can add up to 300s apiece, against the E2E job's 215-minute cap. On the first full run of this PR (run 36223896793) the `clear after` values of the suites that were waited on summed to 210s; the last full run before this change took 158 of the 215 minutes. - A pod whose owner's resource type is no longer served gets "the server doesn't have a resource type" rather than NotFound, so its suite is reported unchecked instead of leaked; it still fails. - `kubectl wait --for=delete` waits for deletion, while the guard skips pods in `Succeeded` or `Failed`; a pod that finishes but is never deleted (a Job under a leaked root) spends the rest of the budget before the suite shows `clear`. The verdict is right; only job time is lost. - The unit tests do not pin the one-line empty report Chainsaw writes for a directory with no tests (`_lib`), nor the branch that catches a spent budget before calling `kubectl wait`; both are verified against the Chainsaw and kubectl sources rather than by a test. - The runner's per-suite report directory from `mktemp -d` is not removed; the sandbox is discarded after the job. - Each non-suite directory under `hack/e2e-chainsaw/` (today `_lib`) costs one empty Chainsaw invocation, because the runner leaves the decision of what is a test to Chainsaw. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff adds files under `hack/`, changes the `test-chainsaw` target, and rewords one comment and the failure line in the four E2E workflows. The trigger map's `hack/` entry is for cozystack/ccp, whose skills anchor on `hack/package.mk`, `hack/common-envs.mk` and `make generate`, none of which change here. A GitHub code search for `chainsaw` across the cozystack organisation finds hits only in this repository. ### Release note ```release-note test(e2e): the E2E app suite now fails the suite whose pods outlive its own cleanup by more than 300s, naming each leftover pod and its owner; suites with a filed leak are listed in hack/e2e-chainsaw/_lib/leak-allowlist.txt and only warn. ``` |
||
|
Aleksei Sviridkin
|
42b7239198 |
test(e2e): wait for the opensearch handoff before the first API call (#4462)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does The opensearch suite sends its first OpenSearch API call, the index create in `make-the-cluster-permanently-yellow`, as soon as the nodes StatefulSet reports one ready replica. Two operator actions are released at that moment and neither has finished: the bootstrap pod handing over the cluster-manager role, and a rolling restart of the only data node. #4412 is the first of them hitting the create with a 503. This PR makes the step wait for both before its first call: an `error:` assert that `opensearch-test-bootstrap-0` no longer exists, then an assert that the nodes StatefulSet has settled on its update revision, in the same form the suite already uses after the listener move. Anchors below are opensearch-k8s-operator v2.8.0 with the in-tree patches applied, paths under `opensearch-operator/`. **Bootstrap handoff.** - The operator starts retiring the bootstrap pod only once the cluster is `Initialized` (`pkg/reconcilers/cluster.go:110-111`), and that flag is set as soon as every cluster-manager pool has all its replicas ready (`controllers/opensearchController.go:257`, `pkg/builders/cluster.go:1146-1164`). A Ready data node therefore starts the handoff and does not finish it. - The voting exclusion is a cluster-manager action, so it goes through the bootstrap pod and fails while that pod's security is not initialized. The operator then keeps the pod and requeues after 10s (`pkg/reconcilers/cluster.go:36`, `:186-190`). - The pod is deleted only after the bootstrap node has left the last committed voting configuration (`pkg/reconcilers/cluster.go:195-205`, `opensearch-gateway/services/os_client.go:760-771`). Once the pod object is gone, including its Terminating phase, the bootstrap process has exited and holds no vote, and the only cluster-manager-eligible node left is `opensearch-test-nodes-0`. - The assert cannot pass vacuously. The cluster forms around the bootstrap pod (`cluster.initial_master_nodes` names only that pod, `pkg/builders/cluster.go:807`), and the pod is created unconditionally while the cluster is not initialized (`pkg/reconcilers/cluster.go:112-113`). **Rolling restart on first boot.** The failing run's own operator log and StatefulSet dump show it: - `10:27:52.850` the reconcile in which `Initialized` became true. - `10:27:53.251` "Failed to exclude the bootstrap node from the voting configuration". - `10:27:53.981` "Starting rolling restart of the StatefulSet opensearch-test-nodes" (`pkg/reconcilers/rollingRestart.go:155`). - `10:27:54.079` "Couldn't proceed with rolling restart … because enabled shard allocation" (`:182`), the first step of the yellow path, after which the next 10s requeue moves on to `PreparePodForDelete` (`:195`) and deletes the pod. The StatefulSet in the same dump is at generation 2 with `currentRevision` `…744cc7b944` and `updateRevision` `…677c65d7fb`: the pod template was rewritten after `opensearch-test-nodes-0` was created, and the restart onto it waits only on `Initialized` (`rollingRestart.go:79`, `:123`). With one replica that restart takes down the node the step's requests go to, so a create that got past the handoff could still land on a terminating or restarting node. Once the StatefulSet reports `currentRevision == updateRevision` with its one replica updated and ready, the node the step talks to is on its final template and the operator has nothing pending. The suite runs one topology: one replica in a single pool with the cluster_manager, data and ingest roles, on a PVC. The bootstrap pod comes back only through the emptyDir recovery path (`pkg/reconcilers/cluster.go:452-478`), which a PVC-backed cluster never takes, and a restart that was not pending leaves the second assert true immediately. I checked the rest of the suite for the same window. Apart from this step, only the catch script (diagnostics, `|| true`) calls the OpenSearch HTTP API. `verify-endpoint-serves-the-chart-chain` makes a TLS handshake only, and it runs after the suite has already waited for the listener roll to complete, on a single node that is its own cluster-manager. Budgets: 3m for the handoff, which took about two seconds in the failing run; that leaves room for several 10s retire retries, a 30s requeue and the default 30s termination grace period the bootstrap pod runs with. 6m for the restart, shorter than the 12m the suite gives the listener roll because nothing is pulled here: the health steps requeue every 10s and the pod then restarts on an image the node already holds, where the first boot, bootstrap included, took under two and a half minutes. Residual risk, unproven: if the pod template were rewritten again after the rollout assert passed, that assert would have passed on a template that is not the final one. I found no mechanism for it. The StatefulSet builder reads no cluster status (the only status read in `pkg/builders/cluster.go` is the replica check in `AllMastersReady`), so the transition to `Initialized` cannot rewrite the template by itself; a rewrite would need a configuration or secret input to change after that point. Which input changed on first boot in the failing run I could not tell from the log, because the ControllerRevisions were not captured. What I ran: `chainsaw lint test` with Chainsaw v0.2.15, the version the e2e sandbox pins, passes on the changed suite. As a positive control, lint rejects each new operation when I rename one of its keys. `hack/select-e2e.sh` on this diff selects `opensearch` only. What I did not run: the E2E suite itself. The race is timing-dependent (one hit in six failed in-tree runs checked, per the issue), so a single green run would not prove the fix either. The argument rests on the operator code and the failing run's log cited above. Closes #4412 ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> Not a UI change. ### 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. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [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: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The only file changed is `hack/e2e-chainsaw/opensearch/chainsaw-test.yaml`, which no downstream repository consumes. ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Cluster health checks now wait for bootstrap completion and verify that all nodes are ready and up to date before proceeding. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
a66907f535 |
fix(backups): share the walkthrough wait helpers and fix the S3 preflight cleanup (#4635)
## What this PR does This fixes the S3 access preflight and a few backup walkthroughs, and moves the walkthrough wait helpers into one shared file. The access preflight now keeps its evidence and cleans up after itself. When `mc alias set` failed, it dropped the port-forward log, and now it prints it like the other exits do. The exit trap used to kill the port-forward before `mc rm`, so the delete always hit a closed port and the warning gave no reason. The delete now runs first, and the warning carries mc's own error, or says the delete timed out when mc was killed before writing one. A PUT that timed out (rc 124) may have landed, so it counts as uploaded. A PUT the server refused does not, so access that never went live gets no delete and no warning. The wait helpers were copied into every walkthrough and had drifted. `wait_for_field`, `wait_hr_ready` and `wait_deleted` had 12, 8 and 2 copies with 5, 3 and 2 different bodies. They now live in `examples/backups/_lib/wait-helpers.sh`, and each `00-helpers.sh` sources it. Call sites and wait budgets are unchanged. Where copies differed, the shared body keeps every diagnostic any copy had. What changes per walkthrough: - `wait_for_field` in every walkthrough but redis now shows kubectl's stderr, so a Forbidden or transport error is visible instead of reading as an empty field. `--ignore-not-found` keeps a resource that does not exist yet from printing an error on every poll. Redis already kept stderr. - `wait_for_field` in foundationdb, nats and vmi gains the optional 7th terminal-value argument. None of their callers pass it, so nothing changes at runtime. - vmi loses its progress dots. - `wait_hr_ready` in redis and kafka-metadata now prints every condition and the release history on Stalled or timeout. It also says whether the release never appeared, was deleted, or could not be looked up. - The kafka `wait_deleted` timeout message reads "it is still present" instead of "still present". `hack/backup-examples-shared-helpers.bats` goes red if a walkthrough defines one of these helpers again or stops providing them, and it pins both sides of the `wait_for_field` read: kubectl's reason for a failed read is shown, a missing resource prints nothing. The byte-identity pin between copies in `hack/mariadb-first-boot-waits.bats` is gone, and its errexit test runs through the mariadb walkthrough's helpers. The kafka topic-data demo had the empty-read race from #4400. `kafka_run` read topic state through a throwaway `kubectl run -i` Pod. It now execs into a long-lived CLI Pod that keeps the restricted security context, and `cleanup.sh` removes it by label. The Pod name comes from its own `KAFKA_TOPIC_CLI_POD`, so setting the kafka-metadata demo's `KAFKA_CLI_POD` does not move it. #4400 itself was already fixed on main for nats, kafka-metadata and rabbitmq. The nats and kafka-metadata demos could exec into a Pod they had just refused. They read CLI output inside `$(...)`, where bash does not apply errexit, so the ownership refusal was ignored. A failed read of the Pod's phase was treated as no Pod, and the demo went on to delete and replace it. Every setup step now returns on failure. Their "exec fails with kubectl's own error" tests could never fail: the runner's trace wrote the expected message into the same stderr the test grepped. The stub now prints the message itself. Each fix has a test that fails on main. For each fix I also reverted it and checked that only its own test goes red. One gap stays open outside this PR. `hack/select-e2e.sh` maps `examples/backups/<app>/` paths by app name, so a later PR that changes only `examples/backups/_lib/wait-helpers.sh` would select no suite. This PR is fine because it touches every walkthrough. The rule belongs to the select-e2e owners and is tracked in #4626. While reviewing I also found that the kafka README puts steps that create and exec into Pods under the tenant persona, and the tenant role can do neither. That is #4625, left for its own change. Left out on purpose, because they are outside what this diff changes: - The kafka topic demo and the kafka-metadata demo each carry a CLI Pod helper, and the kafka-metadata one runs without the restricted security context. - The walkthrough READMEs do not say that a directory now needs `../_lib` next to it. - The errexit test in `hack/mariadb-first-boot-waits.bats` loops over one file. I kept it so the rebase against #4471 stays mechanical. Closes #3757, closes #4243, closes #4244 ### Screenshots No UI changes. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(backups): the backup walkthroughs share one set of wait helpers that keep kubectl errors visible, the kafka topic-data demo no longer reads empty replies from a throwaway Pod, the nats and kafka-metadata demos no longer exec into a CLI Pod they refused, and the e2e S3 access preflight says why client setup or cleanup failed and deletes its test object reliably. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Kafka topic backup walkthroughs now reuse a named CLI Pod, with restricted security settings and checks to prevent using a Pod owned by another demo. * Backup walkthroughs can wait for resources to reach a desired state, Helm releases to become ready, or resources to be deleted. * **Bug Fixes** * Kafka and NATS walkthroughs now stop and report errors when CLI Pod lookups, creation, deletion, or command execution fail. * S3 preflight cleanup now attempts removal after a timed-out upload and reports cleanup failures, including timeouts. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
a9dc106a81 |
fix(e2e): bound post-install-prep calls and make its failures name their cause (#4632)
## What this PR does The post-install-prep step of the e2e install could hang without failing, and several of its failure paths gave up without saying why. The script runs in the background of the install test, which waits on it, so a kubectl call that never returns keeps the install open until the job's own timeout. This PR fixes both problems in `hack/e2e-post-install-prep.sh`. - Every call that does work now has a wall-clock bound: the pool listing and creates, both applies, the CDI patch, the reads in the LINSTOR node loop, and the diagnostic reads on every failure path. Plain requests also get a smaller `--request-timeout`, so kubectl can say why a request failed before the outer timeout kills it. When a bound fires, the log names the call. - `kubectl wait --timeout` turned out to be no bound at all for the GET it runs first, so each wait attempt got an outer timeout too. If that timeout cuts an attempt, the timeout line says so. - The wait loop keeps kubectl's stderr now. An expired credential or an RBAC denial used to look the same as "not created yet" and ended in a timeout on the object. Each error is printed once when it changes, and the timeout line repeats the last one. - When the linstor wait times out, the script lists every HelmRelease first. The linstor release can be just waiting for a release further up the dependency chain, and the Ready messages name which one. - The MetalLB CRD wait and the CDI StorageProfile wait used to exit silently. Now they print a timeout line and one last read of the missing object with its error. The MetalLB wait also prints the metallb HelmRelease. - A failed or killed storage pool listing stops the script before any pool is created. Before, it read as "no pools exist", and the creates that followed failed with an unrelated error. All of this is tested in `hack/post-install-prep.bats` with the existing stub PATH and virtual clock. One test checks that every kubectl call on the success path and on each timeout path runs under a bound. The literal grep in `hack/linstor-storage-mode_test.bats` is replaced with a test of the behaviour. The rest of #3680 lives outside this script and moves to #4628: the empty `{"error":{}}` in helm-controller's Helm log, previous-container logs on passing runs, and node and pod resource state during install. Closes #3708, closes #3707, closes #3713, closes #3714, closes #3680. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(e2e): bound every call in the post-install-prep step and make its failures name their cause, so a hung call fails the install instead of holding it until the job timeout ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Reliability** * Cluster operations now have bounded wait times, with clearer diagnostics when resources or endpoints are delayed or unavailable. * Storage setup stops when pool listings fail instead of masking the error. * **Tests** * Expanded coverage for timeout handling, error reporting, and failures during storage and resource setup. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
d8ec8f0af1 |
fix(ci): run every bats unit file after a failure and cover build-*.sh in the cross-compile check (#4633)
## What this PR does Two fixes to the tooling of the unit lane. A local run of the bats unit tests stopped at the first red file and never ran the files after it, while the output looked like a finished suite. Each bats file is its own make target, and without `--keep-going` make stops scheduling targets after the first failure. CI passes `-k`, so it was not affected. The target now runs the files in a `--keep-going` sub-make. That sub-make still exits non-zero if any file failed, and it shares the caller's `-j` slots and `--output-sync`. The new `hack/bats-unit-keep-going.bats` drives the real Makefile against two fixture files. It checks that both run and the target fails when the first one fails, that the target exits 0 when both pass, and that the sub-make still finds the root Makefile when `MAKEFILES` names another file. I split the exact-line check on this target in `hack/bats-runner-coverage.bats` in two to match the new recipe. A workflow comment and a test name that still called each file a prerequisite now call it a target. The cross-compile check never looked at the multus-cni builder. It treated only a script named exactly `*build.sh` as a compile step, and multus runs upstream `./hack/build-go.sh`. The pattern now allows a suffix after `build` but no `/`, and each case has a fixture test. Among the Dockerfiles the check scans under `packages/`, this matches exactly one new line, the multus stage, and that stage passes. Closes #3692 for the make-level truncation. The in-file half, where `hack/cozytest.sh` ends a file at its first failing test, is tracked in #4620. Closes #4500 Things I left out: - The multus stage passes because a `curl ... TARGETARCH` step sits in the same stage. The check does not see that `build-go.sh` reads `TARGETPLATFORM`. The test header already documents this per-stage limit. - The sub-make reads only the root Makefile, so an extra `make -f other.mk` given to the parent no longer reaches the bats targets. Command-line variables and `MAKEFILES` still do. Nothing in the repo calls it that way. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(ci): `make bats-unit-tests` runs every bats file after a failing one and still fails at the end, and the image cross-compile check now covers build scripts such as `build-go.sh`. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Unit tests now continue running across individual test-file failures while still reporting an overall failure if any file fails. * Build-stage detection now recognizes build scripts with names such as `build-go.sh`. * **Tests** * Added coverage for unit-test results and build-script detection. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
873d9f4c8e |
fix(backups): stop the nats and kafka-metadata demos using a Pod they refused
Both demos read their CLI output through command substitutions, where bash does not apply errexit. When the ownership check refused a same-named Pod another party owns, the refusal was ignored and the exec went into that very Pod. A failed read of the Pod's phase or owner was likewise read as "no Pod", so the helper went on to delete and replace a Pod whose owner it had not seen. Every step of the Pod setup now returns on failure, and the phase and owner reads tell a missing Pod (--ignore-not-found) from a failed read. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
a3caa62e47 |
test(backups): make the nats and kafka-metadata exec-error cases able to fail
Both cases passed the expected kubectl error to the stub through an environment variable named on the command that runs the helper. The unit runner traces that command, with the value expanded, into the very stderr the case then greps, so the case stayed green with the helper's stderr sent to /dev/null. The stub now prints the message itself and the variable is only a flag. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
ead7048b35 |
refactor(backups): define the walkthrough wait helpers once
Every backup walkthrough carried its own wait_for_field, and most their own wait_hr_ready and wait_deleted: twelve, eight and two copies, which had already drifted into five, three and two bodies. A fix made in one copy did not reach the others, and a diagnostics change carried by hand between two of them once went into the wrong function and was caught only by reading the diff. The helpers now live in examples/backups/_lib/wait-helpers.sh, sourced by each walkthrough's 00-helpers.sh. Where copies differed, the shared body keeps the diagnostics any of them had: wait_for_field leaves kubectl's stderr attached, and wait_hr_ready prints the conditions and history dump on Stalled or timeout. Call sites and budgets are unchanged. A new unit test fails when any walkthrough defines one of these helpers again, which replaces the byte-identity pin between copies. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
5318b5e507 |
fix(backups): exec into a CLI Pod in the kafka topic-data demo
kafka_run seeded and read topics through a throwaway `kubectl run -i` Pod per call. That Pod's stdout returns over an attach that carries only what the container writes after the attach registers, so a CLI that finishes first reads as an empty reply with exit 0 -- the empty-read defect already fixed in the redis, nats and kafka-metadata demos. The topic count, dump and replication-factor checks all read that reply. Run one long-lived CLI Pod, keeping the restricted security context, and exec into it. It carries its own name and owner label so it cannot collide with the kafka-metadata demo's Pod in a shared namespace, and cleanup.sh removes it by that label. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
735486fc36 |
fix(e2e): say when a post-install-prep wait attempt hit its bound
When the outer timeout cut a `kubectl wait` attempt, nothing was printed: `timeout` is silent and kubectl dies on the signal. The timeout line then read "last kubectl error: none", which says the client had nothing to report rather than that it was killed mid-request. The attempt's exit status now names that case. The errors the loop prints also carry the script's prefix. The first NotFound shows up on every green run, since the release appears about a minute in, and without the prefix it reads like a failure when the log is scanned after one. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
5b4380dd3b |
fix(e2e): delete the failed S3 preflight object through a live tunnel
The exit trap stopped the port-forward before running mc rm, so the delete went to a closed port and failed every time, leaving the object behind and printing a warning that said nothing about why. The delete now runs first and the warning carries mc's own error, or says the delete timed out when mc was killed before it could write one. A PUT that timed out is now also treated as uploaded: it can land and still outlive its time bound, and the trap never tried to remove such an object. A PUT the server refused is not, so the common case of access that never went live does not end in a delete doomed to the same refusal. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
b6cd6ab682 |
fix(e2e): print the port-forward log when the S3 preflight client fails
mc alias set reaches the endpoint through the port-forward, so a tunnel that came up and then broke is a likely cause of that failure, and its log is the only record of it. The readiness and retry-timeout exits already print it; this one threw it away. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
71a943277f |
build(make): keep running bats unit files after one fails
Each bats file is its own make target, so a plain `make bats-unit-tests` stopped scheduling files at the first red one and never ran the rest, while the output looked like a finished suite. CI passes -k and was not affected; a local run was. The target now runs the files in a --keep-going sub-make, which still exits non-zero when any file fails and shares the caller's job slots and output sync. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
f023e2fa38 |
test(build): count build-*.sh scripts as compiling in the cross-compile check
The compile pattern only matched a script named exactly *build.sh, so the multus-cni builder, which compiles through upstream ./hack/build-go.sh, was never classified as compiling and the check skipped the stage while appearing to cover it. Allow a suffix after "build" in the script name, but not a directory separator, so a path through a build/ directory is not mistaken for a build script. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
c96dcaa11a |
fix(e2e): bound the lookup behind each kubectl wait in post-install-prep
`kubectl wait --timeout` looked like a bound on the wait loop's calls, but it covers only the watch. The command first resolves the object with a plain GET that has no deadline, since no --request-timeout is set, so an apiserver that accepts the connection and never answers holds the wait loop, the most frequently repeated call in the script, just like the calls that were unbounded outright. Each attempt now runs under an outer timeout set a little above the attempt's own --timeout, so the wait still owns its normal expiry. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
08011cef09 |
fix(ci): escalate an edit to the shared backup walkthrough helpers
examples/backups/<app>/ selects that app's suite, and a directory whose name is no suite selects nothing. A helpers directory shared by every walkthrough, examples/backups/_lib/, falls into the second case: an edit to it alone produces an empty selection, which both lanes read as "skip Chainsaw", so a change reaching every backup round-trip would run none. It now escalates to the full suite like hack/e2e-chainsaw/_lib/. The set of suites that run a walkthrough is spread across the suites' own scripts, and a derived subset that misses one would fail open. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
603ff2871e |
docs(hack): drop file and branch counts from the selector comments
The comments explaining the unit/e2e bats split quoted how many files each side held, and the escalation helper how many branches call it. Every figure had gone stale: nothing checks them, and they move each time a bats file or an escalation is added. The Makefile expression quoted beside the bats claim is what defines the set, so the explanation stands without the number. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
a6ec30d56f |
fix(ci): discover Chainsaw suites written as chainsaw-test.yml
Chainsaw loads chainsaw-test.yml when a directory has no chainsaw-test.yaml, but select-e2e and select-install enumerated the .yaml name only. A suite spelled that way ran whenever the whole directory was handed to chainsaw, yet no escalation listed it and no edit to it could select it, so it silently dropped out of every narrowed run. select-install also sent find's errors to /dev/null inside a pipeline that reported sort's status, so a failed listing validated no suite at all and passed. It is now captured and fails validation. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
d777f2a0b3 |
fix(ci): select the suites that use seaweedfs instead of all of them
cozystack.seaweedfs-application mapped to a seaweedfs suite that does not exist, so every seaweedfs change reached nothing runnable and paid for the whole Chainsaw run. The suites that can regress from it are bucket, the backup round-trips that write to the tenant-root instance through its in-cluster S3 endpoint, and harbor, whose registry storage is a BucketClaim served by the seaweedfs COSI driver with no graph edge between the two. bucket alone would miss the CA and service-name paths the round-trips depend on. The test derives the expected set from the suites' own yaml, so a new suite that names the endpoint or a BucketClaim there fails it until it is mapped. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
e070bc7d1d |
fix(ci): escalate a per-suite Chainsaw path that names no suite
A path under hack/e2e-chainsaw/<dir>/ put <dir> into the selection without checking that it is a suite, leaving the final intersection to drop it. Alone the path escalated through the empty-selection backstop, but beside any path that contributed a suite the escalation vanished and the run narrowed to that suite with nothing on stderr. Shared material next to _lib/ or a suite nested deeper than discovery looks then went untested in exactly the pull requests that also touch an app. The directory name is now membership-tested where the path is classified, as the hack/e2e-apps/ arm already does, so the decision is a property of the path and names it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
7e0bd9c403 |
fix(e2e): stop post-install-prep on a failed or killed pool listing
The storage pool listing was piped straight into awk, so its exit status was lost: a listing that failed, or one killed by its time bound, read as "no pools exist yet". The script then sent a create for every node, and on the nodes whose pool did exist the create failed with a reason unrelated to the actual failure. The listing now runs on its own, so a failure stops the script before any pool is created. A bound that fires leaves no text of its own: `timeout` prints nothing and kubectl dies on the signal before it can. The bounded calls therefore go through a wrapper that names the call it killed, so the log does not end on the step's announcement line. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
ff2a677fc0 |
fix(e2e): say why the MetalLB CRD and CDI StorageProfile waits failed
Both waits ran as `timeout N sh -ec 'until kubectl get ...'` with the read's output discarded. On elapse `timeout` exits 124 and `set -e` ends the script, so the last line in the log is the one announcing the wait: CRDs that are late and an operator that never got far enough to install them read the same. A shared deadline loop now polls quietly and, on the way out, reads the objects once more with stderr kept, so the log names which object is missing. The MetalLB wait also lists the metallb HelmRelease, whose state separates those two cases. The loop runs on the script's own clock, which lets the unit suite drive it with the virtual one. The unit suite's runaway guard in the sleep stub now kills its caller as well, because a failing exit status does not stop a loop that runs where set -e is off, and a guard that cannot stop the loop turns a broken deadline into a hung test. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
b9a3fd0e8b |
fix(e2e): name the blocking release when post-install-prep times out
When the linstor HelmRelease never goes Ready, the script reported a timeout on linstor and dumped the cozy-linstor pods and events. In two failing install runs the linstor release only sat behind a dependency chain (kubeovn -> piraeus-operator-crds -> piraeus-operator -> linstor), so the evidence it printed described the wrong release and said nothing about the one that was stuck. The timeout now lists every HelmRelease first. Each Ready message names the dependency it waits on, so the post-install-prep log names the release at the head of the chain. Signed-off-by: Aleksei Sviridkin <f@lex.la> Assisted-by: LLM |
||
|
Aleksei Sviridkin
|
3378711fb9 |
fix(e2e): keep kubectl stderr in the post-install-prep wait loop
The loop discarded stderr to stay quiet while an object is not created yet, since `kubectl wait` exits non-zero at once on NotFound. An expired credential or an RBAC denial exits the same way, so the loop retried a client that could never succeed for the whole budget and then reported a timeout on the object, pointing at the cluster while the cause sat in the client. Each attempt's stderr is now kept and printed whenever it changes, and the timeout line repeats the last one. A NotFound repeated every poll still costs a single line. The text is printed rather than matched to fail fast: apiserver restarts during install produce the same connection and auth errors transiently, and a match list would turn a slow link into a false failure. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
d07099f13c |
fix(e2e): bound the calls that do work in post-install-prep
The install step blocks in `wait` on this script, so a kubectl call that never returns does not fail the step: it holds it open until the job's own ceiling cancels the run, which leaves different evidence behind than a failed test. Only the waits carried a budget. The execs that create storage pools, the applies, the CDI patch, and the reads inside the LINSTOR node loop did not, and the loop's deadline is checked between iterations, on the other side of a call that hangs. The reads that explain a node-loop timeout were unbounded too, on the one path that runs only when the cluster is already unhealthy. Each call now runs under an outer timeout, and plain requests carry a client timeout strictly inside it so kubectl names the reason before the kill lands. The helpers take the bound as a prefix that stays empty when unit tests source them, since `timeout` would bypass the kubectl function those tests substitute. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
9ac67d76a2 |
test(tests): preserve Bats failure reports during fixture cleanup (#3848)
## What this PR does Test-level `EXIT` cleanup overrides the Bats TAP handler, so a failed assertion can disappear from test results. This moves cleanup in eight unit files after their assertions. Failed tests keep scratch files for inspection. Prerequisite for [the Bats runner change](https://github.com/cozystack/cozystack/pull/3849). Existing subshell traps in the OpenAPI and CAPK tests remain supported. Closes #3623 ### Validation `make unit-tests test-controllers -j4 --output-sync=target` passed. The converted files passed under Bats 1.14.0 and `dash`. A deliberately failed assertion produces `not ok` after this change, and the trap audit rejects an added test-level handler. ### Downstream repositories Test cleanup and its documentation only. No downstream consumer changes. - [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: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note NONE ``` |
||
|
Aleksei Sviridkin
|
c983b5c404 |
fix(linstor): use severity warning instead of warn in piraeus-datastore alerts (#3799)
## What this PR does
Seven of the piraeus-datastore alert rules label themselves `severity:
warn`. Alerta, which the monitoring package ships as the alert sink,
validates that value and `warn` is not on its list. Every webhook
delivery for these alerts dies with a 500.
```
Severity (warn) is not one of security, critical, major, minor, warning,
indeterminate, informational, normal, ok, cleared, debug, trace, unknown
```
Alertmanager retries seven times and drops the notification, and the
alert never shows up anywhere. The affected rules are the LINSTOR/DRBD
ones, such as storage pool at capacity, lost quorum, unintentional
diskless and stuck resync. These are exactly the alerts you least want
silently dropped.
Found this on a v1.4.6 cluster where `linstorStoragePoolAtCapacity` was
genuinely firing. The cluster had 153 dropped notifications in 40
minutes and nothing in Alerta. Renaming the label to `warning`, which
the other three rules in the same file already use, stopped the failures
immediately and the alerts came through.
The same labels sit on `main` and in `v1.6.1`, so a backport to
`release-1.6` would be appreciated.
The test commit adds a bats guard so the next one gets caught in CI. It
walks every `PrometheusRule` and `VMRule` under `packages/` outside
vendored `charts/`, helm templates included, and fails on a severity
outside Alerta's default alarm model, which is exactly where `warn`,
`info`, `error` and `fatal` fall. Helm actions are stripped before the
file is read, so a rule behind a `{{- if }}` or an unquoted `{{
.Release.Namespace }}` is checked rather than skipped, and a file that
still does not parse fails the contract instead of thinning it out. On
the current tree that is 194 alerting rules in 41 scanned files.
The guard found one more on `main`. `DatabaseAutoscalerAtMax` in
`packages/system/keda/templates/alerts.yaml` carries `severity: info`,
which Alerta refuses the same way. It is renamed to `informational`, the
value the model has for that level and the one seven other rules in the
tree already use.
Moving that alert off `info` also stops `InfoInhibitor` in
`packages/system/monitoring-agents/alerts/general.rules.yaml` from
firing. That rule matches any `severity="info"` alert and carries
`severity: major`, so on `main` it usually reached Alerta whenever
`DatabaseAutoscalerAtMax` fired, even though the keda alert itself was
refused. After this change no rule carries `info`, so operators no
longer see that `InfoInhibitor` notification.
Fixes #2411.
### Screenshots
Not a UI change.
### Downstream repositories
Walked the trigger map against the diff. It touches severity labels
inside `packages/system/piraeus-operator/alerts/` and
`packages/system/keda/templates/`, plus a bats file under `hack/`. No
package is added or renamed, and there is no values schema, CRD, tooling
the satellites gate on or node contract change. Nothing downstream
restates these labels.
- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
### Release note
```release-note
fix(linstor): LINSTOR/DRBD alerts carried severity `warn`, which Alerta rejects, so they were never delivered. They now use the standard `warning` severity, and the keda at-max autoscaler alert moves from `info` to `informational` for the same reason.
```
|
||
|
Yan Bondarenko
|
bd2cc0a749 |
test(monitoring): guard alert severities against the Alerta alarm model
Alerta refuses an alert whose severity is not in its alarm model, so a rule with such a label is dropped every time it fires and the alert never reaches anyone. Seven piraeus-datastore rules used warn and the keda at-max rule used info, and nothing in the tree caught either. The contract collects every PrometheusRule and VMRule document under packages, in alerts files and in chart templates, and fails on any alerting rule whose labels.severity is missing or not in the accepted list. Helm actions are stripped before parsing, so a templated rule is still checked, and a file that cannot be parsed fails the run instead of being skipped. Values are compared as whole words. none stays out of the list because the Alertmanager config routes it to a blackhole receiver. Vendored charts under packages/**/charts/** are out of scope because they are re-vendored rather than edited. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> Assisted-by: LLM |
||
|
Mattia Eleuteri
|
9afe7f1293 |
fix(kubernetes): assemble the node group once in kubernetes-nodes
The TalosConfigTemplate name is a hash of the machine-config spec, and two
templates have to agree on it: talos-reconcile-job.yaml creates the object,
nodegroup.yaml points MachineDeployment.bootstrap.configRef at it. Sharing the
rendering was not enough — each file also assembled its own $group dict, one
carrying every pool value and one only the few the spec reads, so the two
names agreed only by coincidence of the keys the spec helper happens to read.
Dropping "gpus" from the Job's dict diverged them on a GPU pool with the whole
suite still green, which is the "templates do not exist" deadlock this fix
exists to make impossible.
Move the assembly into a single kubernetes-nodes.group helper, read back with
fromYaml at both call sites, so there is one input as well as one rendering.
Cover it too, since the seam can be reintroduced. The chart's helm-unittest
suites pin each name against its own literal in its own file, which catches a
divergence but reports it as one literal having moved — indistinguishable from
a stale fixture, so re-pasting the new value is the obvious and wrong repair.
hack/check-talosconfigtemplate-name-agreement.bats renders the chart once and
compares the two values against each other, so the failure states the invariant
instead of a number. It asserts on a GPU-shaped pool as well as the default
one: the default renders gpus [], kubelet {} and instanceType "" as falsy, so a
consumer reading a subset of the group is indistinguishable there from one
reading all of it. kernelModules gets a case of its own, because it has no
default and stays unset on both of those shapes. A last case pins that the
shapes hash differently, so the others cannot hold vacuously.
The same guard compares the talos-reconcile Role's resourceNames on
TalosConfigTemplates against that name. The Job applies server-side, and
that is a PATCH on the name even when it creates the object, so a Role
naming anything else leaves the Job Forbidden and the pool without a
template, with the same silent outcome.
Document that talos.registryMirrors and kernelModules sit inside the hashed
spec, so changing either rolls the pool's existing workers instead of only
affecting nodes created later. The kernelModules text described the
fixed-name template as the present state and the content-hash name as a
future one.
No rendered output changes: the golden render snapshots are unchanged.
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
Assisted-by: LLM
|
||
|
Aleksei Sviridkin
|
458827ccdd |
test(e2e): load drbd_transport_tcp explicitly on Talos nodes
Since v1.14.0 Talos builds the kernel with an empty modprobe path (siderolabs/pkgs#1565), so request_module() no longer loads anything. DRBD asks for its transport that way on the first new-peer, and without the module drbdsetup new-peer fails with "Failed to create transport (drbd_transport_xxx module missing?)" and every resource on the node stays in Connecting. The module ships in the drbd extension on v1.13 and v1.14 alike, so listing it next to drbd is safe on both. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Marian Koreniuk
|
e5dc086338 |
fix(kubernetes-nodes): replace the TalosConfigTemplate CABPT's webhook will not let us patch
CABPT's validating webhook rejects any change to TalosConfigTemplate.spec
(TalosConfigTemplate.Spec is immutable), so the worker reconcile Job's
`kubectl apply` fails the moment any input the render depends on genuinely
changes. The apiserver address is the one that moves in practice: the Job
writes it into certSANs and into the worker's endpoint, and a cluster that
gets a new address leaves the Job dead with
TalosConfigTemplate.bootstrap.cluster.x-k8s.io "…" is invalid:
spec: Invalid value: …: Spec is immutable
from then on, until somebody deletes the object by hand. Nothing in the
cluster says that is what has to happen.
The Job now recognises that one failure and replaces the object: delete, then
apply the same manifest. Every other apply error still stops the Job, because a
delete followed by a create that fails for the same reason — an apply rejected
for its content, say — would leave the pool with no template, unable to scale
until the render is fixed. The replace itself does not roll the pool: Machines
reference the template by name, and the name does not change. The match is on
the message for that reason, and the Role's new `delete` is bound to this pool's
own template by name, so the Job cannot reach a sibling pool's.
While the Role is being touched anyway, its other TalosConfigTemplate rights are
pinned to the same name. `get`, `patch` and `update` were unnamed, and several
clusters can share a tenant namespace: a Job that can patch a sibling pool's
template can rewrite the machineconfig that pool's next workers boot. `create`
stays unnamed because RBAC matches resourceNames only on verbs that name an
object.
The manifest is captured into a variable rather than written to a file: the
container runs with readOnlyRootFilesystem, and one variable also means the
replace applies exactly what the first attempt did.
Both applies stay server-side, as the Job's single apply already is: a
client-side merge pulls the full OpenAPI schema and runs the Job out of its
256Mi. The webhook rejects a spec change the same way under server-side apply,
so the `is immutable` match still fires.
UPGRADE NOTE. This changes the KubeVirt path as well, and the render with it:
the talos-reconcile Role gains the `delete` rule, its TalosConfigTemplate rights
are narrowed to this pool's own object, and the Job's script changes,
which moves the Job's content-hash name. Every existing worker pool therefore
gets a new reconcile Job on upgrade. On a pool whose live TalosConfigTemplate
no longer matches the current render, that Job will now delete and recreate it
instead of failing. Machines already running are unaffected — their bootstrap
data was rendered when they joined — but Machines created afterwards boot the
new machineconfig. On a pool where template and render agree, nothing happens.
Both new asserts were checked against the mutation they exist for: deleting the
`exit 1` from the `*)` branch and pointing the delete at `${RELEASE}` each turn
the matching test red. A bare `exit 1` pattern does not — the input-timeout path
near the top of the script already has one.
Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua>
Assisted-by: LLM
|
||
|
Marian Koreniuk
|
7500a9fbae |
test(kubernetes): add the Proxmox e2e suite, parked disabled
Upstream CI has no hypervisor, so this suite cannot run there; it ships
`.disabled` and is run by a lane that owns a Proxmox stand.
Named for what the chart actually creates. The resource definition prefixes the
release, so a Kubernetes CR called k8s-e2e produces
ProxmoxCluster/kubernetes-k8s-e2e in the tenant's own namespace, and the admin
kubeconfig Secret is kubernetes-k8s-e2e-admin-kubeconfig.
Stand-specific values are variables rather than hardcoded. Every one of them is
a bare ${COZY_PVE_*} that run-proxmox.sh expands with envsubst, supplying the
defaults itself — GNU envsubst leaves a ${VAR:-default} in the text exactly as
it is, set or not, and that reaches the apiserver as a string where the schema
wants an integer — and refusing a fixture with a variable still in it. The
StorageClass and the PVC probe read the same COZY_PROXMOX_STORAGE, so the volume
lands where the probe looks. The runner reads the tenant kubeconfig's .conf key,
the address a runner outside the management cluster can reach.
The stand's resource pool comes in through COZY_PVE_POOL, empty by default.
hack/proxmox-e2e-fixture_test.bats pins the shape of the fixture and the render.
The PVC probe reads the storage again after the claim is deleted: a deleted
claim whose disk is still there is the leak the pre-delete hook's Step 1b
exists to prevent. keep_tenant is honoured by the suite's own teardown, not
only by the workflow's. The workflow carries no fallback for the stand's
variables: an unset one fails the job before a key is sent anywhere. The
schedule runs only where vars.PROXMOX_NIGHTLY is true: the job targets a
self-hosted runner with the `proxmox` label, and a scheduled run on a repository
without one would sit in the queue until GitHub cancels it.
The suite also asserts the half of the deployment that lives in the management
cluster — the CSI controller and the cloud-controller-manager — because that is
the property the pair is designed around and a tenant-only assertion would pass
with the token in the wrong place.
Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua>
Assisted-by: LLM
|
||
|
Marian Koreniuk
|
5ab36023e0 |
fix(kubernetes): release Proxmox volumes before the driver that frees them goes
Deleting a tenant cluster left its PVC disks on the hypervisor. The pre-delete
hook deletes every HelmRelease labelled with the cluster in Step 2, the CSI
driver among them, and nothing before that deleted the tenant's PVCs — a volume
is freed only by the driver answering DeleteVolume. capmox then destroys the
worker VMs, and Proxmox frees only what the destroyed VMID owns, which these
volumes are not. They stayed on storage with the tenant's data in them, and the
PV objects that could have named their owner went with the control plane.
Step 1b deletes the claims while the driver is still up, and it deletes the
claims before the Pods that hold them: a claim being deleted is one the
scheduler places no new Pod on, so a Pod that a StatefulSet or Deployment brings
back cannot mount it again and hold it to the backstop. The Pods go second: a
claim in use carries the kubernetes.io/pvc-protection finalizer, so it clears
only once nothing mounts it, and the Pods are going with the cluster in any
case. Then it waits for the PVs rather than the PVCs, because the claim
disappears as soon as its finalizer clears while the disk is freed only when the
PV is gone.
The Pods holding a claim are found by listing every Pod with the claims it
mounts and matching in the shell. A JSONPath filter on the claim name cannot do
it: with a Pod that mounts two claims its left side yields two values, client-go
refuses the whole expression ("can only compare one element at a time"), and no
Pod in that namespace would be found for any claim.
A kubeconfig that exists is not an apiserver that answers, and listing claims
against one that does not comes back empty — the clean case. Step 1b asks
/readyz first and sends an apiserver that stays silent through the backstop, so
the summary carries it. The probe is bounded by `timeout` rather than by
--request-timeout: a non-zero --request-timeout on an in-cluster kubectl call
diverts it to localhost:8080, which is why hack/check-hook-request-timeout.bats
refuses the flag in hook templates, and the check cannot tell this call, which
carries its own kubeconfig, from one that does not.
Best effort, and loud about it: a tenant that never came up has no apiserver to
ask, and a teardown must not wedge on that, but a timeout is reported through
the same backstop the other steps use.
The hook's Role gains one Secret by name — the tenant's admin kubeconfig, which
this step needs. Without it the step takes its no-kubeconfig branch and reports
that there is nothing to release while the volumes stay behind.
Step 5 no longer fails the whole hook where CDI is not installed. It deletes
DataVolumes, which are a KubeVirt-path component, and kubectl exits non-zero on
an unknown resource type; under set -e that failed the teardown after every
earlier step had already run, leaving the cluster half removed and the hook
reporting failure.
Verified on a throwaway tenant: a PVC bound and mounted by a running Pod, then
`helm uninstall`, and the volume freed by the driver rather than by hand.
delete_hook_test.yaml pins Step 1b and the Role's admin-kubeconfig entry on
proxmox and their absence on kubevirt, the /readyz probe, and the order of the
claim and Pod deletes.
Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua>
Assisted-by: LLM
|
||
|
Aleksei Sviridkin
|
3e4030d4d9 |
feat(kubernetes): per-pool kernel modules for Talos worker nodes (#3571)
## What this PR does Implements gap 1 of #3563: a pool-scoped `kernelModules` field on `KubernetesNodes`, emitted as Talos `machine.kernel.modules`. A Talos system extension installs a kernel module but does not load it — loading is `machine.kernel.modules`' job. The worker machine config had no such block and no values key could add one, so a GPU pool on Talos workers was unusable as shipped: the extension supplied the NVIDIA module, nothing loaded it, and the failure was silent end to end. The VM held the PCI device, the node advertised no GPU, and no component logged an error. Found in a production 1.6.0 environment, where the worker rollover took the 4 GPUs of a tenant cluster out of service with no error anywhere. ### Design: three states, and no default The shape was arbitrated in #3563 (option 3 of the three proposed there). - **absent** — the chart chooses. A pool holding at least one `nvidia.com/*` GPU gets `nvidia` (with `NVreg_NvLinkDisable=1`), `nvidia_uvm`, `nvidia_drm`, `nvidia_modeset`; any other pool gets no `kernel` block at all. - **non-empty list** — taken verbatim, replacing the chart's choice outright. - **`[]`** — explicit opt-out: no modules even on a GPU pool. The order is not cosmetic: Talos loads the list in sequence, `nvidia` has to come first because the other three depend on it, and `nvidia_uvm` is what CUDA unified memory needs. This is the order validated against a production GB202 passthrough node. `NVreg_NvLinkDisable=1` is what the gpu-operator driver container applies on non-Talos workers; on Talos that container has to be off, so nothing else would carry it, and without it the driver waits forever for an NVLink fabric that the passed-through GPU does not have. The three states are distinguishable only because the field carries no default — no entry in `values.yaml`, no `default` in `values.schema.json`. A default of `[]` would collapse "absent" into "opted out" for every pool and make the automatic set unreachable. The generated parameter table prints `[]` in the Default column for any array with no default; the field description says so explicitly. For the same reason `keysOrder` does not list the field, so the dashboard renders it at the bottom of the form. Automatic emission is keyed on the `nvidia.com/` resource prefix, not on the mere presence of `gpus`, so an AMD pool is not given NVIDIA modules on a guess. The field loads modules; it does not put them in the image. Which extensions the image carries is the pool's schematic, so a GPU pool points `osImage.factory.schematicID` (or `osImage.builtin.schematicID`) at a schematic carrying the NVIDIA extensions. Module names and parameters reach the reconcile Job's unquoted heredoc, so both are validated at render, and the emitted list is rebuilt from those two fields alone: the schema does not prune unknown keys, and any other key passed through `toYaml` would reach the shell unchecked. A parameter must be printable ASCII with no whitespace, and must not contain `$`, a backtick, a backslash or quotes. The ASCII half is there because RE2's `\s` and `[:cntrl:]` are ASCII-only: without it U+2028 and U+2029 pass, `toYaml` writes them raw, and Helm's YAML parser reads them as line breaks that end the Job's `command` block scalar early. Inside ASCII the guard denies what the shell expands rather than allowing a fixed set, because NVIDIA's documented multi-value parameter is semicolon-separated. ### Rollout, and #3523 A pool that resolves to no modules renders the machine config it rendered before, byte for byte: `tests/render_snapshot_test.yaml` and its committed golden snapshot pass unmodified, including the `talos-reconcile` Job's content-hash name. For a pool that does get modules, what happens depends on how its `TalosConfigTemplate` is named, and the field description states both cases: - **On main today** the template has a fixed name and an immutable spec, so the reconcile Job's apply of a changed machine config is refused (#3515). Running workers keep what they booted with, and new workers boot from the old template too. This covers the upgrade itself: every existing pool holding an `nvidia.com/*` GPU with the field unset gets the automatic set, and its apply is refused the same way. Only pools created afterwards pick the modules up. - **Once #3523 lands** the template is named by its content, so the same change renames it and CAPI rolls the pool. On the upgrade that brings both, every existing pool holding an `nvidia.com/*` GPU with the field unset rolls onto the automatic set. That roll is the intended outcome — those pools have no working driver without it — but it is a roll, and the release note says so. ### Tests - `packages/apps/kubernetes-nodes/tests/kernel_modules_test.yaml`, 12 cases: the NVIDIA set and its order for an `nvidia.com/*` pool, no `kernel` block for a pool with neither GPUs nor modules, an explicit list taken verbatim including `parameters`, an explicit list replacing the NVIDIA default, `[]` opting out while leaving the `gpu=on` label intact, no NVIDIA assumption for another vendor, a semicolon-separated parameter accepted, a hostile module name and a hostile parameter each rejected, a parameter carrying U+2028 and one carrying U+2029 each rejected with the kernelModules message rather than a YAML parse error, and an unvalidated extra key on an item dropped. The patterns pin absolute indentation, because `modules:` one level out is still valid YAML and silently loads nothing. - `hack/talos-reconcile-heredoc_test.bats` lists `kernelModules` in its validated arm and gains two execution-level cases: a hostile name and a hostile parameter each refused at render, and a semicolon-separated parameter surviving the real shell literally. Loosening the guards to `^.+$` makes the refusal case fail. - `helm unittest packages/apps/kubernetes-nodes`: 31 suites, 174 tests, 12 snapshots, all passing. `hack/cozytest.sh hack/talos-reconcile-heredoc_test.bats`: 8 passed. - `make generate` in the package, and `controller-gen object` for the `api/apps/v1alpha1` submodule. ### Not in scope The rest of #3563: the `nvidia-operator-validator` → device-plugin chain that cannot validate an OS-provided driver, CDI spec generation and the `nvidia` containerd runtime default, and the `maxSurge` / `nodeStartupTimeout` defaults. ### Screenshots No UI changes. ### Downstream repositories - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: https://github.com/cozystack/website/issues/643 - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: https://github.com/cozystack/terraform-provider-cozystack/issues/20 - [ ] [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: Both follow-ups are issues rather than PRs. `terraform-provider-cozystack` is hand-written, so the field needs a schema entry, a model entry and an expand/flatten pair on `KubernetesNodes`; the undefaulted three-state behaviour is exactly where that provider's habit of sending its own defaults would break users silently, since materialising `kernel_modules = []` would disable the automatic NVIDIA set for every GPU pool managed through Terraform. `website` needs the three-state behaviour written down, the GPU pool example pointed at `osImage.factory.schematicID`, and the Blackwell constraint documented: on GB202 the schematic must carry `siderolabs/nvidia-open-gpu-kernel-modules-production`, because with the proprietary extension the module loads, `/dev/nvidia0` appears, and `nvidia-smi -L` then reports `No devices found`. The `hack/talos-reconcile-heredoc_test.bats` change adds test cases and does not change node prerequisites, so `ansible-cozystack` and `talm` are not reached. No other repository in the map is touched: no package, platform component, bundle, variant or release asset is added, and `ApplicationDefinition` semantics are unchanged. ### Release note ```release-note feat(kubernetes): Worker node pools (`KubernetesNodes`) accept a `kernelModules` field, emitted as Talos `machine.kernel.modules`. A Talos system extension installs a kernel module without loading it, so a GPU pool previously came up with no working driver and no way to fix it through values. Left unset, a pool holding at least one `nvidia.com/*` GPU now loads `nvidia` (with `NVreg_NvLinkDisable=1`), `nvidia_uvm`, `nvidia_drm` and `nvidia_modeset` automatically; an explicit list replaces that set, and an explicit `[]` opts out. The modules still have to be in the image: point the GPU pool's `osImage.factory.schematicID` at a schematic carrying the NVIDIA extensions, and on Blackwell (GB202) use the open kernel modules extension, not the proprietary one. Pools without GPUs and without the field render exactly what they rendered before. NOTE: while the worker `TalosConfigTemplate` keeps a fixed name (#3515), a changed module list does not reach a pool that already exists — including the automatic set on existing GPU pools — and only pools created afterwards pick it up. Once that template is named by its content (#3523), the same change rolls the pool, so existing GPU pools that leave the field unset are rolled onto the automatic set. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Worker node pools can now configure Talos kernel modules, including optional module parameters. * When no list is specified, NVIDIA GPU pools automatically receive NVIDIA modules; other pools receive none. A custom list replaces this selection, while an empty list disables modules. * Invalid module names or parameters are rejected during configuration rendering. * **Documentation** * Added guidance on module selection, parameters, image-extension requirements, and pool rollout behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
6ab3e0be78 |
feat(postgres): allow preloading an allowlist of shipped modules (#3506)
## What this PR does - Adds `postgresql.sharedPreloadLibraries`, a closed enum of four module names that already ship in the CloudNativePG PostgreSQL image, rendered onto the CNPG `Cluster` field of the same meaning. - Leaves the `shared_preload_libraries` entry in the `postgresql.parameters` blocklist exactly as it is. - Adds `tests/shared_preload_libraries_test.yaml`: absent by default, single and multiple libraries, no interference with `parameters`, schema rejection of a non-allowlisted name and of an absolute path, and the parameter blocklist still firing when the allowlist is in use. - Adds `hack/check-postgres-preload-allowlist.bats`, which renders the chart with `--skip-schema-validation` to exercise the template-side allowlist that helm-unittest never reaches, and checks that it matches the schema enum. `shared_preload_libraries` is blocked in `postgresql.parameters`, and rightly so: the parameter is free text and accepts a path, so it can point at a writable location such as the PGDATA volume. The block does not make every preloadable module unreachable, though. CloudNativePG carries a managed extensions list (`ManagedExtensions` in `pkg/postgres/configuration.go`, checked on v1.30.0, the operator version in this repo): as soon as a parameter under the `pg_stat_statements.`, `auto_explain.` or `pgaudit.` prefix is set, the operator adds that library to `shared_preload_libraries` itself, and for `pg_stat_statements` and `pgaudit` it also runs `CREATE EXTENSION`. The chart already passes such keys through, so `postgresql.parameters."pg_stat_statements.track": all` works on main today. What this PR adds is narrower than a missing capability. `pg_prewarm` has no route at all, and its autoprewarm worker only runs when the module is preloaded. For the other three, the field is an explicit switch that does not require setting one of their tuning parameters, even to its default value, just to get the library loaded. The two routes combine rather than override: the operator merges this list with the libraries it preloads on its own, so removing a module from `sharedPreloadLibraries` does not unload it while a parameter under its prefix is still set. The field description says so. The distinction this PR draws is between a library *name* and a library *path*. Nothing needs to be injected to preload these four: `pg_stat_statements.so`, `auto_explain.so`, `pgaudit.so` and `pg_prewarm.so` are already in `$libdir` in the image, and that directory is on the read-only root filesystem, so it cannot be written to. `dynamic_library_path` stays blocked, so `$libdir` remains the only search path. A bare name from a fixed enum therefore has no way to reach code that is not already in the image, which is the property the blanket block was protecting. The enum in `values.schema.json` is the primary guard and rejects a bad value before rendering starts. The template carries the same list as a backstop for renders that bypass schema validation, using the existing fail-fast `SECURITY:` idiom from the parameter blocklist a few lines below it. helm-unittest validates the schema on every render, so it never reaches that backstop; the bats suite renders with `--skip-schema-validation` instead. I checked that it fails when the template condition is replaced with `false`, and when the template list gains a name the enum does not have. I verified the mechanism end to end on a live CloudNativePG 1.27.3 cluster rather than assuming it: with `spec.postgresql.shared_preload_libraries` set, the operator writes `shared_preload_libraries = 'pg_stat_statements'` into `postgresql.conf`, `SHOW shared_preload_libraries` reports it, and `SELECT ... FROM pg_stat_statements` returns rows. Adding a second module to the list took effect too, and the CNPG instance manager restarts PostgreSQL in place for it, with no pod restart. That restart covers every instance, the primary included, so writes pause briefly. The CRD documents the field as additive to the operator's own defaults, so this does not clobber anything CNPG needs to preload itself. Scope note: the four names are the modules in the image that are useful to a tenant and safe to preload. Anything requiring a background worker or a shipped-but-privileged module was deliberately left out, and the enum is the single place to extend if that changes. ### Screenshots No UI change. ### Downstream repositories `packages/apps/postgres/values.schema.json` gains a field, which per the trigger map reaches `cozystack/terraform-provider-cozystack`: its postgres resource carries a hand-maintained schema and its own expand/flatten pair, with no codegen from `values.schema.json`, so `sharedPreloadLibraries` will be invisible to Terraform users until it is added there by hand. The enum matters twice over there, because the provider restates enums as `stringvalidator.OneOf` lists. I searched that repository and found no open PR or issue covering it (the only open PR is a dependabot Go module bump). I have not filed the follow-up, so I am leaving the box unticked rather than claiming work that does not exist. The same gap applies to `users.<name>.createRole` in #3505; one follow-up there can cover both fields. I walked the rest of the map against the diff and found nothing else: no package is added, renamed or removed, no existing version enum changes, no default changes, `release.prefix` and the output Secret and Service names are untouched, and no CRD outside the regenerated `packages/system/postgres-rd/cozyrds/postgres.yaml` is affected. The `README.md` parameter table is regenerated here and postgres is already in the website generator's app list, so the website path stays automated. - [ ] 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: not filed yet, see the note above - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note feat(postgres): add `postgresql.sharedPreloadLibraries`, a closed allowlist of modules shipped in the PostgreSQL image (`pg_stat_statements`, `auto_explain`, `pgaudit`, `pg_prewarm`) that are preloaded at server start. It is the first way to preload `pg_prewarm`, and lets the other three be preloaded without setting one of their parameters, which CloudNativePG already treats as a request to preload them. The `shared_preload_libraries` parameter stays blocked: only bare names from the allowlist are accepted, never a path. Changing the list restarts PostgreSQL on every instance, the primary included. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added optional PostgreSQL shared preload library configuration for `pg_stat_statements`, `auto_explain`, `pgaudit`, and `pg_prewarm`. * Preserves the configured library order. CloudNativePG may automatically preload the first three libraries when their related parameters are set; `pg_prewarm` must be enabled through this list. * Changing the list restarts every PostgreSQL instance, including the primary. * **Validation** * Rejects unsupported modules, absolute paths, and attempts to set preload libraries through free-form parameters. * **Documentation** * Added configuration guidance covering supported libraries and restart behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
e126158404 |
fix(ci): keep the controller Go tests running when a unit test fails first (#4233)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does The `checks` job in `.github/workflows/pull-requests.yaml` runs `make unit-tests test-controllers -j4 --output-sync=target` as a single step. `unit-tests` and `test-controllers` are independent goals on that command line, and without `-k` GNU Make stops handing out new work the instant any earlier-processed goal's prerequisite fails — a goal that had not yet been scheduled never starts. So whenever a BATS file inside `unit-tests` fails before `test-controllers` gets a job slot, the entire Go controller suite (`go test ./internal/...`) silently never runs, and the PR page shows only the unit-test failure with no signal at all about the controllers. This adds `-k` so already-schedulable goals keep running after an earlier one fails; the job still exits non-zero and still fails whenever anything failed, `-k` only stops make from abandoning independent work. This addresses the second half of https://github.com/cozystack/cozystack/issues/3776. The first half of that report — whether the job's comment overclaims that it gates the merge, since `Unit & controller tests` is not currently in `main`'s required status checks — needs a maintainer's call between two mutually exclusive fixes (weaken the comment, or add the job to branch protection), and is deliberately left untouched here; the issue itself frames the two as independent and separately useful. Note: `hack/unit-test-parallelism.bats`, which pins the literal invocation string in a test, was updated to match. ### Screenshots N/A — CI workflow and test-script change only, no UI. ### 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. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [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: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: I walked the trigger map: this PR only touches CI scheduling flags inside this repo's own `.github/workflows/pull-requests.yaml` and a `hack/*.bats` meta-test that asserts against that same file. Nothing in the trigger map names either file, and the change has no effect on any package, API, chart, or contract another repository consumes. ### Validation Reproduced the exact scheduling gap in an isolated Makefile mirroring this repo's real dependency shape (a `unit-tests`-like goal depending on several independent prerequisites, one of which fails immediately, plus a sibling goal with no dependency on it), under the same GNU Make 4.4.1 `-j4 --output-sync=target` invocation used here. Without `-k` the sibling goal never ran, across 5 repeated trials; with `-k` added it ran to completion every time across 3 repeated trials, and the overall exit code stayed non-zero either way. `bats hack/unit-test-parallelism.bats` passes locally against the real files. I also ran `make unit-tests test-controllers -j4 -k --output-sync=target` against this repo directly: local disk space (a limit of my sandbox, unrelated to this change) made `go test ./internal/...` fail to compile with "no space left on device", so I could not confirm a clean end-to-end pass of `test-controllers` itself in this environment — but the run still exercised the fix: after that build failure, unrelated `hack/*.bats` prerequisites kept being scheduled and ran to completion instead of the whole invocation aborting, which is exactly the behavior `-k` adds over the prior command line. I could not run this repo's actual CI runners to confirm disk is not similarly constrained there; nothing in the workflow or runner spec suggests it would be. ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(ci): the `Unit & controller tests` PR check no longer silently skips the Go controller test suite when a Helm-unittest or BATS unit test fails first; both suites now always run and report. ``` --- Part of #3776 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Updated automated test execution so unit and controller test suites continue running when one test target fails, while the job still reports failures. * Updated workflow validation to reflect the revised test command. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Mattia Eleuteri
|
f7c04b7a39 |
feat(kubernetes): expose kernel modules for Talos worker pools
A Talos system extension installs a kernel module but does not load it; loading is machine.kernel.modules' job. The worker machine config had no such block and no values key could add one, so a GPU pool on Talos workers was unusable as shipped: the extension supplied the NVIDIA module, nothing loaded it, and the failure was silent end to end. The VM held the PCI device, the node advertised no GPU, and no component logged an error. The field is three-state, and the states are distinguishable because it carries no default: absent lets the chart choose (a pool holding at least one nvidia.com/* GPU gets nvidia, nvidia_uvm, nvidia_drm, nvidia_modeset, in that order, since Talos loads the list in sequence and the last three depend on the first; any other pool gets nothing), a non-empty list replaces that choice outright, and an explicit [] opts out even on a GPU pool. A default of [] would collapse absent into opted-out for every pool and make the automatic set unreachable. The automatic nvidia entry carries NVreg_NvLinkDisable=1. Individual GPUs are passed into worker VMs without their NVSwitches, and on non-Talos workers the gpu-operator driver container applies that parameter; on Talos that container has to be off, so nothing else would. Names and parameters reach the reconcile Job's unquoted heredoc, so they are validated at render and the emitted list is rebuilt from those two fields alone: the schema does not prune unknown keys, and any other key passed through toYaml would reach the shell unchecked. A pool that resolves to no modules renders the machine config it rendered before, byte for byte, so its content-hash Job name does not rotate. Refs: #3563 Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
bb3ddee233 |
fix(rabbitmq): bound the topology operator Secret cache
messaging-topology-operator 1.14.2 caches every Secret in the cluster: its ClusterRole grants secrets list/watch cluster-wide with no namespace or label restriction, so the controller-runtime cache holds the whole set. Against the vendored 500Mi limit this OOMKills on any cluster that has grown enough Secrets, and it never recovers, because the initial LIST alone no longer fits. While it is down no topology CR reconciles, for any tenant. Observed on a cluster with 8211 Secrets across 291 namespaces (293 MB, 87% of it Helm release history): a flat 333Mi working set for days, then the ceiling, then a permanent crashloop. The operator was caching 8211 Secrets to read 2. Upstream fixed this by gating the Secret informer on a label, released in v1.20.0, and by bypassing the cache for Service reads, released in v1.20.1. Refresh the vendored manifest to v1.20.2, the current patch release carrying both, through the package's existing make update pipeline. The bump alone would break every install: the app chart's credentials Secret is referenced by User.spec.importCredentialsSecret, and from 1.20 the admission webhook rejects a user-provided Secret that does not carry rabbitmq.com/topology-operator=true. Label it in the same change, pin the label in the chart's unit tests, and give the e2e RabbitMQ a user so the webhook and the User reconcile are exercised against the real operator. The RabbitmqCluster default-user Secret needs no label, it is read uncached via apiReader, and the Status.Binding contract is unchanged since 1.14.2, so the vendored cluster-operator 2.9.0 stays compatible and does not move here. Refs #4128 Signed-off-by: Mattia Eleuteri <mattia@hidora.io> Assisted-by: LLM |
||
|
Mattia Eleuteri
|
25cdd51881 |
feat(postgres): allow preloading an allowlist of shipped modules
shared_preload_libraries is blocked in postgresql.parameters, and rightly so: the parameter is free text and accepts a path, so it can point at a writable location such as the PGDATA volume. What the block leaves is an indirect route. CloudNativePG preloads pg_stat_statements, auto_explain and pgaudit by itself as soon as a parameter under their prefix is set, such as pg_stat_statements.track, so enabling one of them today means setting one of its tuning parameters, even to its default value. pg_prewarm has no route at all, and its autoprewarm worker only runs when the module is preloaded. postgresql.sharedPreloadLibraries is the explicit switch. It is a closed enum of four module names that already ship in the CloudNativePG PostgreSQL image, rendered onto the CNPG Cluster field of the same meaning, which the operator merges with the libraries it preloads on its own. A bare name can only resolve inside $libdir, which sits on the read-only image filesystem, and dynamic_library_path stays blocked, so no path can be introduced. The blocklist on postgresql.parameters is left untouched. The template repeats the enum as a backstop for renders that skip schema validation. helm-unittest always validates the schema and never reaches it, so a bats suite renders with --skip-schema-validation to keep the backstop, and its agreement with the enum, under test. Verified on a live CloudNativePG 1.27.3 cluster: with the field set, the operator writes shared_preload_libraries into postgresql.conf, SHOW reports the modules, and pg_stat_statements returns rows. Changing the list restarts PostgreSQL on every instance, the primary included, through the instance manager and without restarting the pod. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Aleksei Sviridkin
|
afe23a2c3f |
ci(release): pin flux where the release pushes the packages artifact
prepare-release now runs flux push artifact to publish the rc's packages artifact, and it installed whatever flux was latest, while build-amd64 and the candidate checks pin one version. It installs that version now. The arm64 leg and the nightly never run flux, so they no longer install it. The rc-e2e comment gave a stable tag as the reason for its !cancelled(), but the job only runs for an rc; it now says it follows the rule every job below build-arm64 has. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
71144d4fd6 |
ci(release): pin the matchbox rebuild in the release job
tags.yaml only runs on tags, so no pull request exercises it. The matchbox rebuild is the only thing that makes its ref an index, and losing the step, moving it past the tree capture or letting it switch the default builder would surface as a failed gate on the next rc. The comments that called the talos images amd64 only now say they are built for both arches in the amd64 job. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
6ef23da6e7 |
ci(release): publish matchbox as an amd64 and arm64 index
matchbox already carries the kernel and initramfs of both arches, but the release built it for amd64 only: the arm64 leg skips the talos package, so there is no arm64 half to stitch, and the gate let it through on an allowlist entry. Its Dockerfile only copies files, so the amd64 job now rebuilds it for both platforms on a docker-container builder and the stamped ref pins the index. The allowlist entry goes. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
c740031453 |
Merge remote-tracking branch 'origin/main' into ci/build-arm64-leg
Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov Daniil
|
10727c0391 |
test(tests): preserve Bats failure reports during fixture cleanup
Test-level EXIT handlers replace the handler Bats uses for TAP output, so a failed assertion can disappear from the reported test results. Cleanup must preserve those failures before the unit runner changes. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
5e32ef35b0 |
feat(build): build Talos, Harbor, the Velero KubeVirt plugin and the unlisted images for arm64 (#4552)
## What this PR does This brings together four approved PRs that give the remaining first-party images an arm64 build. Each of them lost its approval when #4498 merged and GitHub retargeted it to main, so I merged them into one branch to review and approve once. Each original PR has the full description and review history, and I'll close them in favour of this one. - #4507 builds the Talos installer and matchbox for arm64. Releases also publish the arm64 ISO, disk images, kernel and initramfs. - #4515 builds the KubeVirt Velero plugin in-tree for amd64 and arm64. - #4525 adds flux-plunger, keycloak-operator, kilo and migration-controller to the root `build:` list, so CI rebuilds them like every other image. A test keeps any new image target from being left out. - #4528 rebuilds the Harbor v2.15.1 images for amd64 and arm64, because upstream publishes v2.15 for amd64 only. Release builds still run with `PLATFORM=linux/amd64`, except the Talos installer, so these images ship arm64 only once #4505 adds the arm64 leg. This PR makes that leg buildable. A few commits are new on top of the originals. The velero test no longer pins the plugin's image string, because the release build stamps its own rebuild over it. It now checks the plugin init container by repository name. A promote-retag test comment now says what it guards, and the image-refs doc no longer calls its fixed gaps open. The merge with main keeps the Harbor portal in the list of stages whose output has no architecture. ### Screenshots Not a UI change. ### Downstream repositories I left every box empty and opened nothing in other repositories. Two repositories are touched, and neither follow-up is needed before this merges: - cozystack/website names the amd64 assets in `content/en/docs/next/install/talos/iso.md` and `install/providers/oracle-cloud.md`. The install pages could offer the arm64 downloads once a release carries them. No asset is renamed, so the current links keep working. - cozystack/talm points at `ghcr.io/cozystack/cozystack/talos:<version>` in `charts/cozystack/values.yaml`. Once a release publishes that tag as a two-arch index, the same reference works on arm64 nodes, so nothing there has to change. - [ ] 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: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note feat(talos): the Talos installer image is built for amd64 and arm64, with arm64 profiles that carry drbd and zfs, and the bootbox matchbox image network-boots arm64 machines as well as amd64 ones. Releases publish the arm64 Talos ISO, disk images, kernel and initramfs. feat(velero): the KubeVirt Velero plugin is now built in-tree for amd64 and arm64, so the velero server starts on arm64 clusters. build: flux-plunger, keycloak-operator, kilo and migration-controller are now rebuilt by CI like every other first-party image, so their pinned images pick up source patches and base-image updates and are published for amd64 and arm64. feat(harbor): the Harbor application now runs on arm64 nodes. Its component images are rebuilt from the pinned upstream v2.15.1 release for amd64 and arm64, because upstream publishes v2.15 images for amd64 only. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Talos images and boot assets now support both AMD64 and ARM64, including architecture-specific installation and cloud images. * Added build support for Harbor components and the KubeVirt Velero plugin. * Nightly builds now include ARM64 release assets. * **Improvements** * Harbor uses pinned chart and image versions, with built image references recorded automatically. * Added checks to catch image packages missing from the main build and to verify multi-architecture installer output. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Pujitha Paladugu
|
65b02bf5ac |
fix(ci): keep the controller Go tests running when a unit test fails first
The `checks` job runs `make unit-tests test-controllers -j4` as one step. Without -k, GNU Make stops scheduling new work as soon as any prerequisite fails, so when a BATS file in unit-tests fails first, the independent test-controllers goal never starts and the PR page shows no signal about the Go controller suite at all. With -k make keeps running every goal it still can, and the job stays red on any failure. Report: https://github.com/cozystack/cozystack/issues/3776 Assisted-by: LLM Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> |
||
|
Aleksei Sviridkin
|
8d547be7fe |
fix(tests): reject unreadable RD preset schemas (#4191)
## What this PR does `rd-presets-check` discards `jq` errors, so malformed schema can pass the check or appear as missing presets. It now fails with the schema path and `jq` diagnostic. An absent schema or a readable schema without `resourcesPreset` is still skipped. Regression tests use real `jq` for malformed input and partial output. They also cover the canonical preset set, exact matching and the long-list case fixed by [#4204](https://github.com/cozystack/cozystack/pull/4204). ### Testing - `make unit-tests -j4 --output-sync=target` passed. - Nine cases in `hack/check-rd-presets_test.bats`, run with `bats` and `dash hack/cozytest.sh`. - Both read-failure cases fail against `main`. Eleven deliberate code mutations also fail the expected test. - `shellcheck hack/check-rd-presets.sh hack/check-rd-presets_test.bats`. ### Screenshots Not applicable. ### Downstream repositories - [x] No downstream repository is affected by this change No files or make targets were moved or renamed. The change is limited to the RD checker and its tests. ### Release note ```release-note fix(tests): Fail the RD preset check when a schema cannot be read, with the schema path and jq diagnostic. ``` |
||
|
Aleksei Sviridkin
|
029707f0b2 |
test(hack): say what the promote-retag ownership check guards
The comment above the ownership loop described how both images used to be dropped. Describe the reason the check matters instead: neither image has a release tag to fall back on, so a miss would go unnoticed. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |