cozystack

0 Fork 0

mirror of https://github.com/cozystack/cozystack.git synced 2026-10-30 08:37:31 +09:00

Author	SHA1	Message	Date
Aleksei Sviridkin	1f30ac4598	fix(tests): let the mariadb waits cover a first boot (#3769 ) ## What this PR does Closes #3741 Several waits for a database HelmRelease to become Ready gave up before the release could: the mariadb, mongodb, rabbitmq and clickhouse backup walkthroughs waited 300 seconds, and both mariadb Chainsaw suites gave their HelmRelease-ready assert 6 minutes. A generated release carries a 10-minute install timeout (`pkg/cmd/server/start.go:91`), so a failed install was reported as a bare timeout, because the release had not yet recorded why it failed, and for mariadb a first boot the startup probe was still willing to wait for failed the harness instead. A wait on such a release has two floors and has to beat the larger. The first is the app's own first boot: cozystack-api leaves `spec.waitStrategy` unset and helm-controller defaults it to `poller`, so the install waits through kstatus, and kstatus maps the operator CR's `Ready=False` onto `InProgress`. For mariadb that boot may take the chart's 310-second startup budget (`startupProbe.failureThreshold: 30` in `packages/apps/mariadb/templates/mariadb.yaml` with the operator's probe defaults of 20 and 10); mongodb, rabbitmq and clickhouse set no startup probe. The second is the install timeout. At or below it the waiter is the one that expires, because its clock starts when the step runs and the install's only once the Application has been reconciled and the chart artifact fetched. The install timeout binds in every case here, so the walkthrough waits go to 660 seconds and the mariadb asserts to 11 minutes. Raising a wait inside a walkthrough spends the Chainsaw op that runs the script, and an op that runs out SIGKILLs the process group and loses the script's error. Each op therefore grows by exactly what the waits it reaches gained: mariadb's round-trip from 30 to 42 minutes, rabbitmq's from 30 to 42, mongodb's from 30 to 36 (its round-trip runs with `SKIP_RESTORE`, so only the source wait counts), and clickhouse's from 60 to 72, which still sits above the sum of every inner ceiling. That bounds a failure at a single wait, not a run that is slow at every stage and then fails late; bounding that needs a phase budget, not a larger literal. postgres keeps its 360-second waits and every walkthrough keeps its 300-second bucket wait: whether a suite pays the extra minutes on every red run is a per-suite call. The mariadb endpoint asserts keep their job. A freshly applied CR carries no conditions and kstatus reads it as current, so the install wait can end before any pod is up; an endpoint address closes that window. That is also why the `wait_for_field` on the CR stays in the walkthrough. `wait_hr_ready` now dumps every condition and the release history on timeout and on `Stalled=True`, instead of the single Ready message a scheduled retry can overwrite before anyone reads it. It also stops printing one `(none recorded)` line for different answers: a release that never appeared, a release deleted during the wait, a lookup kubectl could not answer, a failed history read and an empty history each say so, with kubectl's own stderr kept, and a release seen earlier still gets its dump when only the last lookup failed. The change goes to every byte-identical copy of the helper, in `examples/backups/{clickhouse,mariadb,mongodb,postgres,rabbitmq}/00-helpers.sh`, and they stay identical. `hack/mariadb-first-boot-waits.bats` pins the mariadb budgets against the tree. It derives both floors from their sources instead of pinning literals, so raising `failureThreshold` past the install timeout, or changing the install-timeout default, reds it; each source has a test that refuses to proceed when it cannot be read, including the ApplicationDefinition, whose two override annotations would each invalidate the derivation. It holds the round-trip op at a fixed remainder over the ceilings up to and including the source wait, bucket stage and S3 preflight included, and at a fixed slack once both mariadb waits are paid, so raising a wait without the op reds. The extractors are also tested against fixtures, because the tree cannot tell a correct extractor from a broken one. Beyond mariadb, the file runs every exit branch of `wait_hr_ready` in each copy against a stubbed kubectl, and pins the copies byte-identical. The population is every definition in the tree found by grep, minus two named exceptions held to still differ, so a new copy joins without editing the test and an exception whose copy converges goes red: - `examples/backups/redis/00-helpers.sh` is written in the compact one-line style of the rest of that file, so converging it rewrites its shape rather than applying the shared body. - `examples/backups/kafka-metadata/00-helpers.sh` is a helper of its own with the same diagnostic gap on timeout (it prints no reason at all), and closing that is a change to kafka-metadata. `docs/agents/e2e-testing.md` replaces the reviewer checklist's 5-6m standard and its list of exceptions, which the tree had outgrown, with the floor a wait has to beat and the rule that a longer wait carries its reason in-line. The derivation gets a section of its own, opening with the rule in one line. For a release an app chart renders itself the floor is `spec.install.timeout`, then `spec.timeout`, then helm-controller's 5m default, and the nested `<name>-system` releases never take cozystack-api's 10m. The harbor comments name the floor each of their asserts sits against, and the opensearch comment stops citing the removed standard. Adding `hack/mariadb-first-boot-waits.bats` does not escalate the E2E selection: `hack/[^/]+\.bats$` sits in `inert_config_pattern` in `hack/select-e2e.sh`, and `full_suite_pattern` escalates only `hack/e2e-.bats`. On this changeset `hack/select-e2e.sh` selects `clickhouse harbor mariadb mongodb opensearch postgres rabbitmq`. What this costs is paid only on a failing run; the raised numbers are ceilings. Summing the timeouts stated on each suite's own steps, mariadb goes from 92m to 114m, clickhouse from 116m to 128m, rabbitmq from 67m to 79m and mongodb from 95m to 101m, before the untimed steps that fall back to the defaults in `.chainsaw.yaml`. clickhouse is the most expensive suite by that sum. All of them compete for the 215m `timeout-minutes` of the whole `e2e` job, which also covers image download, sandbox preparation and the Cozystack install, and a job cancelled at the cap loses both its log and `cozyreport.tgz`. This does not fix the failure that led here. In the run that prompted the investigation the mariadb replica never opened port 3306 at all, spending its whole 310-second budget inside `mariadb-install-db` before being killed, and no wait budget would have saved it. That defect is #3744. What this fixes is a second, independent defect found while reading the same code. One thing here is argued rather than measured: the mechanism is checked against helm-controller and kstatus, and every number is derived from the tree, but no run ID pins that it is what any particular red run hit. Both floors justify 660s whatever the cause was, so being wrong about the cause leaves slack rather than breakage. ### Not in this PR - This PR retires the mariadb, mongodb, rabbitmq and clickhouse walkthrough waits and the two mariadb asserts. The rest is tracked in #4404: the other Chainsaw HR-Ready asserts under their floor, the harbor asserts, the 300-second bucket waits, the postgres waits, and the redis and kafka-metadata diagnostic gap. - The clickhouse round-trip comment says every wait fails fast, `wait_hr_ready` on `Stalled=True`, "so a genuinely broken backup reports in seconds". A release whose install is still running is neither Ready nor Stalled, so that holds for a stalled release and not for an install that has not finished. The sentence predates this change. - `hack/mariadb-first-boot-waits.bats` is large for what it guards (two floors, two ratchets on the op, and the helper identity pin), with many fixture tests on its own extractors. Trimming it to that core is a separate decision. - Two comments in that file state facts about the mariadb suite that are true today and that nothing checks: three script ops carry a minute timeout, and the only one ahead of the round-trip is the 8m pre-clean. - The mariadb op comment says a bound on the whole flow "needs a phase budget (docs/agents/e2e-testing.md)"; the doc describes it as bounding the collection phase as a phase (`COZY_DIAG_PHASE_BUDGET`), so the pointer would resolve faster if it named that bullet. - Cosmetic: the behavioural test's loop body is not indented under its `for`, one line of the bats header runs past the width of the rest, and the checklist's sentence under the example assert ("what this block demonstrates least") reads awkwardly. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file, for all eighteen files. The five `examples/backups//00-helpers.sh` files change the diagnostics of `wait_hr_ready`, and the mariadb, mongodb, rabbitmq and clickhouse walkthrough scripts change wait budgets; the manifests the walkthroughs apply and the flows they document are unchanged, so no website page restates anything that moved. Four Chainsaw suites (mariadb, clickhouse, mongodb, rabbitmq) change timeouts and comments, and the harbor and opensearch suites change comments only. `docs/agents/e2e-testing.md` changes the reviewer checklist entry and adds the sizing section. `hack/mariadb-first-boot-waits.bats` is a new file picked up by the existing `BATS_UNIT_FILES` wildcard. `hack/package.mk`, `hack/common-envs.mk` and `hack/update-crd.sh` are untouched, and no `values.schema.json`, `values.yaml` default, version enum, `ApplicationDefinition`, CRD, namespace or release asset is touched. - [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(tests): the MariaDB, MongoDB, RabbitMQ and ClickHouse backup examples and the MariaDB Chainsaw suites now wait for a HelmRelease past the release's own install timeout, so a failed install reports its own reason instead of a bare timeout, and the examples' HelmRelease wait now tells a release that never appeared, one deleted mid-wait and a kubectl that could not answer apart. ```	2026-09-23 18:23:04 +04:00
Aleksei Sviridkin	99de79ff52	fix(cdi): pin the CDI release kubevirt-cdi is reconciled against and stop update overwriting it (#4107 ) ## What this PR does `packages/system/kubevirt-cdi` resolved its CDI version at run time from GitHub's `releases/latest` redirect and wrote the result straight over `templates/cdi-cr.yaml`. That file is not vendored. Upstream ships a bare CDI object; ours adds Helm templating for `cloneStrategyOverride`, `uploadProxyURL` and the worker-pod `podResourceRequirements`, the `ExpandDisks` feature gate, a `CriticalAddonsOnly` toleration, and the two RBAC documents that let a tenant clone from the golden-image namespace. I diffed the two at v1.66.1 to write that list, so it is complete as of that release rather than only hedged: upstream ships 283 bytes with one feature gate, two nodeSelectors and an `imagePullPolicy`, and we are a strict superset of it. Every entry arrived by hand, so `wget -O templates/cdi-cr.yaml` was not refreshing a vendored file, it was overwriting a first-party one. Same loss as the `rm -rf templates` that #4097 removes, arriving through the fetch instead of the wipe. `update` now pins the release the object is reconciled against, and fetches it only to confirm the bytes still hash to a digest I measured. Nothing is written under `templates/`. At the next CDI bump whose asset differs, that check fails, which is the point: upstream moving its own defaults is when the local copy needs a human to reconcile it, not an overwrite. The download is kept on a mismatch, since reconciling one starts by reading it. `RELEASE` is `v1.66.1`, which is the release `kubevirt-cdi-operator` vendors, so the pin and the deployed version agree. What the platform deploys is decided by that package, and it can move to another release without this file changing, so this line is pointed at it by hand. `update` then checks the two agree: it compares `RELEASE` with the `cdi-operator` image tag that package vendors and fails naming both versions when they differ, because otherwise its closing line would report a release the platform may no longer run. That is a check and not a derivation, so `RELEASE` stays a pin a human moves. Upstream publishes no checksum for the asset, so the pin comes from a download rather than from a published file: `a497f90de608c1df26f9ee4095289373f17132d74a2042ca03e00c17c964f8a7`. It catches an asset that moved under a tag that should be immutable. It is not authenticity verification, and the comment beside it says so. The digest command is chosen when `update` expands it, `sha256sum` when it is on PATH and `shasum -a 256` otherwise. Which one ran here is worth stating, because the usual claim about macOS is out of date: this machine picked `sha256sum`, and that binary is `/sbin/sha256sum`, a native Darwin one rather than GNU coreutils. It reports `sha256sum (Darwin) 1.0` and prints the same `<digest> <path>` format. The package already had a `test:` target, from `0418577f5`, so what it gains here is the `.PHONY` line that commit did not add. `hack/package.mk` declares only the nine targets it defines itself, so without it a file or directory named `test` or `update` beside the Makefile makes make consider that target up to date and skip the recipe in silence. The new suite pins the `ExpandDisks` gate, the toleration, `podResourceRequirements`, the `uploadProxyURL` override and the clone RBAC, and renders the upload-proxy Ingress and TLSRoute, each under the `gateway-enabled` value that selects it. The clone-strategy override is pinned by `tests/clone_strategy_test.yaml`, which is already on `main`. One consequence of the pin move is a claim rather than a number, so it gets its own line. Upstream carried `WebhookPvcRendering` from v1.63.0 through v1.65.0 and dropped it by v1.66.0; the local object never carried it, so the divergence ran both ways across those three releases and one way since. The comment says to re-derive the delta at each bump instead of trusting it, because that was a fact about a span of releases and not about CDI. `packages/system/kubevirt-cdi/Makefile` carried two identical `test:` targets, one from `0418577f5` and one appended by a later merge, so make printed `overriding commands for target 'test'` on every invocation in the package. The second sat inside the span this recipe rewrites, so it goes with the rewrite rather than being re-authored into it. The comment kept above it is the reason a reader should not add another. Closes #4157. ### Scope An earlier revision also pinned and tested `kubevirt-cdi-operator`. #3920 has since landed the two real bugs in that recipe, the patch strip level and `patches/tolerations.diff`, so that half is dropped here rather than merged twice. Pinning and testing the operator package is #4190. Named improvements left alone, so they read as choices. - The two upload-proxy tests each render their route under the `gateway-enabled` value that selects it and neither asserts the other is absent; the templates already settle that with `ne "true"` and `eq "true"` on the same variable, and `tests/standalone_render_test.yaml` already asserts each route absent under the values that select the other. So the package does cover it; these two cases just do not, and a third copy would not add anything. - The CDI object carries no version string for a rendered-output assertion, so `RELEASE` is checked by `make update` rather than by `make test`. - Neither route case pins the backend service port, 443 in both templates; that is outside what the suite claims. - The template-loss guard still counts `cdi-cr.yaml` as named, because the download line in `update` mentions the stem, so re-adding a wipe here would have it report two files instead of three; the guard's header already says a mention is not proof of a write, and the case needs a wipe this recipe no longer has. - The suite pins the local additions by design, and most of the upstream-derived half is pinned by nothing: `imagePullPolicy` and the two `nodeSelector` blocks have no assertion anywhere under `tests/`. `HonorWaitForFirstConsumer` is the exception and only as a side effect: `featureGates` is pinned whole, so that gate rides along with `ExpandDisks`. The rest are maintained by the same hand reconciliation the pin forces whenever the upstream asset changes, so a carry-over that drops one of them is green. Three asserts would close what is left. - One more, remote enough to name rather than code around: if the digest tool is missing entirely, `$$actual` comes out empty, the comparison against `CR_SHA256` fails, and the error text diagnoses a moved asset when the real cause is a machine with neither `sha256sum` nor `shasum`. Busybox ships the first and macOS ships the second, so nobody has hit it. Closing it means an executable line, which would then owe mutation coverage of its own, and that is disproportionate to a case with no known instance. The mechanism is written here so the next reader does not have to re-derive it from an error message that points the wrong way. - Separately, the per-package `.PHONY` gap this Makefile's own line argues against is not specific to this package: most package Makefiles do not declare their own `test:` or `update:` phony, and #4214 tracks that. - A key that does not exist yet is not held either: every item in the delta is pinned whole, but a NEW key appearing at `spec` or `spec.config` level passes unnoticed, and such a key can change behaviour rather than sit inert. I confirmed it on a copy extracted from this commit, appending `spec.config.filesystemOverhead.global: "0.99"` and getting a green suite. That is the shape to recognise at the next bump: not a changed value, which is caught, but a new sibling key beside the ones we pin. - And `spec.config.podResourceRequirements` is now pinned whole here and by four leaves in `tests/importer-resources_test.yaml`: the whole-map form is the stronger one and the leaves are a subset of it, so changing the default takes two edits and the second failure reads as a surprise until you know why both exist. - If the operator's image reference ever gains a digest (`:v1.66.1@sha256:...`), `${deployed##:}` takes the digest's hex instead of the tag, and `update` fails with a loud false mismatch rather than passing silently. - Deriving `RELEASE` with `$(shell ...)` from the sibling manifest was suggested and declined on merits: a derived value is not a pin, the point is that a human chooses which CDI release this object is reconciled against, and computing it from whatever the neighbour last vendored hides that choice from the diff. The operator comparison lives in `update` and not in `test:`, even though `kubevirt-operator` ties its own manifest to the tree from `test:`. A mismatch calls for a hand reconciliation, and `update` is the recipe that asks for one; it is also where the success line that would otherwise name the wrong release is printed. I checked it on a copy extracted from this commit. With the operator's image tag moved to v1.67.0, `make update` exits 2 and names both versions. With the tags equal it passes. With the check's `exit 1` removed, the moved tag passes again, so the red comes from the check. Worth knowing when reading the mismatch path: `kubevirt-operator` deletes its download on a hash mismatch and this package keeps it. That is deliberate here, since reconciling a moved asset starts by reading it, but the two packages do read oppositely and neither file says so. ### Base and ordering This stacks on #4097 and should land after it. Both edit `packages/system/kubevirt-cdi/Makefile`, and this change builds directly on that one's edit to the same recipe, so it is based on the head of `fix/cdi-update-keeps-local-templates` rather than on `main`. GitHub computes mergeability against the PR's base branch, which here is that stack branch and not `main`. Both branches are rebased onto current `main`, which has not touched these paths since the previous merge-base. ### Screenshots No UI changes. ### Downstream repositories Walked the trigger map against the diff. Two entries come close and neither fires. The website triggers on developer tooling and package Makefiles, but `content/en/docs/next/development.md` documents `make update` only as "Update Helm chart and versions from the upstream source", which still holds, and its worked example is cilium, which this does not touch. Its `make test` line refers to the e2e sandbox in `packages/core/testing`, not per-package unit tests. `ccp` triggers on `hack/` layout and on what a make target does, and nothing under `hack/` is touched. Its `package-bump` skill already assumes a package's `update` recipe hardcodes the upstream version instead of resolving it at run time, and names cilium, kafka-operator, opensearch-operator and postgres-operator as examples. `kubevirt-cdi` was one of those counterexamples and now is not. Pinning `kubevirt-cdi-operator` the same way is #4190 and out of scope here. The skill handles vendored Helm charts, in-repo image builds and version enums, and a raw manifest download is none of those. Everything else in the map needs an app, a schema, an installer value, a variant or a release asset. - [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(cdi): `make update` in kubevirt-cdi no longer overwrites the hand-maintained CDI object. It pins the CDI release that object is reconciled against instead of resolving GitHub's releases/latest at run time, and verifies the fetched asset against a pinned sha256 without writing under templates/. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit Reliability * Updates now use verified, pinned CDI releases to prevent unintended manifest changes. * Failed or checksum-mismatched downloads no longer overwrite existing configuration. * Bug Fixes * Preserved local CDI customizations during release updates. * Added validation for CDI resources, access controls, upload-proxy routing, and TLS configuration. * Tests * Added automated checks for incomplete template refreshes and generated Kubernetes resources. * Added chart tests covering CDI behavior, clone permissions, and upload-proxy networking. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-23 17:53:23 +04:00
Aleksei Sviridkin	b1519acfe7	fix(e2e): cap the bucket S3 reads so the verify step honors its timeout (#4281 ) ## What this PR does Refs #4268 — a plain reference, not `Fixes`: the issue asks for a `script` op to be cut off at its declared ceiling, and that is Chainsaw behaviour this repo cannot change. Chainsaw runs the op's shell under Go's `exec.CommandContext`, whose default cancel SIGKILLs the shell PID alone and then blocks on the inherited stdout/stderr pipe until an orphaned `mc` closes it, so the op returns only when `mc` exits on its own rather than at the ceiling. This PR mitigates that where it bit — per-command caps so no descendant outlives its own budget — but the op-timeout gap itself lives in Chainsaw, so the link stays a reference rather than a `Fixes`. The sibling step that produced the 606s, `verify-every-bucket-takes-a-write` in the `bucket-1-capacity` Test, is on `main` and already carries its own per-call caps; this PR applies the same shape to `verify-s3-access`. On run 34948279363 the bucket suite's `verify-every-bucket-takes-a-write` step declared `timeout: 5m` but ran 606s and ended on `mc`'s own non-zero status rather than being cut at the ceiling. `verify-s3-access` in `hack/e2e-chainsaw/bucket/chainsaw-test.yaml` is the same-shaped step in this suite. This PR wraps every S3 (`mc`) and apiserver (`kubectl`) read in `verify-s3-access` in its own `timeout`, so a hung read dies at a known budget instead of holding the step open, and corrects the two places (`hack/e2e-chainsaw/.chainsaw.yaml` and `docs/agents/e2e-testing.md`) that explained the op ceiling with a mechanism that does not hold. It caps `verify-s3-access` for sharing the shape, not for producing the 606s number. ### Which of the issue's two candidate mechanisms holds The issue named two candidates and stated the log could not separate them. The Chainsaw v0.2.15 source separates them, and it is the second one, with the exact refinement the issue proposed. Candidate 1 — "an operation `timeout` does not bound a `script` op" — is refuted. The op timeout is applied and does reach the process: `pkg/runner/operations/script.go:24` copies the step's `timeout` into `Timeouts.Exec`, and `:38` wraps the op context with `ctx, cancel := context.WithTimeout(ctx, tc.Timeouts().Exec)` before calling `op.Exec(ctx, …)`. So a 5m step produces a real 300s deadline on the context the command runs under. Candidate 2 — "it bounds it and kills the shell, but Chainsaw waits on a pipe a grandchild (`mc`) still holds open" — is confirmed. In `pkg/engine/operations/script/operation.go` the command is built with `exec.CommandContext(ctx, shell, …)` (`:105`), and in `execute` its stdout/stderr are set to `bytes.Buffer` (`:128`–`:129`, the buffers are `internal.CommandOutput` in `pkg/engine/operations/internal/command_output.go:12`–`13`) and run via `cmd.Run()` (`:130`). Two Go `os/exec` properties then produce the overrun: `exec.CommandContext`'s default cancel is `Process.Kill()` on the shell PID alone — not a process-group kill, and the operation sets no `SysProcAttr.Setpgid` — and because stdout/stderr are not `os.File`, `os/exec` allocates OS pipes and copier goroutines, so `cmd.Wait()` blocks until those pipes reach EOF. EOF needs every inherited write-end closed. When the shell forks `mc`, `mc` inherits those fds; at 300s the deadline SIGKILLs the shell but `mc` is orphaned, keeps retrying the S3 PUT, and holds the pipe, so the op does not return until `mc` exits on its own at 606s. `cmd.Run()` then returns `signal: killed`, which is stored in the `$error` binding and never printed — only the `($error == null): true` check failure is logged — which is why grepping the run for `signal: killed` found nothing. Empirical confirmation. A standalone Go program reproduces it exactly. `exec.CommandContext` with a 1s timeout, `bytes.Buffer` stdout/stderr, running `sh -c 'sleep 2 & sleep 4'` (a descendant that inherits the pipe): `cmd.Run()` returns after ~4s, not 1s, with `err=signal: killed` and `ctx.Err()=context deadline exceeded` — the ceiling fired at 1s, control came back only when the descendant closed the pipe. Redirecting the descendants' stdout/stderr away from the pipe (`… >/dev/null 2>&1`) returns at 1.00s. That is the whole mechanism: the ceiling cuts the shell but not a descendant that still holds the pipe. ### The fix Per-command caps, the remedy the issue itself points to ("every read carries its own cap") and the shape already used elsewhere in the suite. The bucket step now defines a `k()` wrapper (`timeout 30 kubectl`) and an `mc()` wrapper (`timeout 15 mc`) for the reads, and calls `timeout 45 mc cp` directly for the two PUTs; a hung read is killed at its own budget, releasing the inherited pipe promptly, so even when the op ceiling fires the residual is one inner cap rather than `mc`'s full retry exhaustion. The terminal precedents for these budgets are the redis suite's terminal `k()` (30s, `redis/chainsaw-test.yaml:298`) and the sibling `verify-every-bucket-takes-a-write` step's terminal `mc cp` (45s, `:390`, sized for a volume grow); the 15s cap on `mc` in `hack/e2e-chainsaw/_lib/backup-access-preflight.sh` is not a precedent, because it sits inside a `while :;` retry loop where a 124 costs one attempt, whereas here under `set -e` a 124 ends the step. The inner budgets — 2×30 for the two `k` reads + 2×45 for the two `mc cp` PUTs + at most 7×15 for the other `mc` calls (eight other call sites, but the two cleanup `mc rm` are mutually exclusive) + the 30s port-forward readiness wait = at most 285s — sit under the 5m op ceiling, which stays as a backstop, per the "op above the sum of inner budgets" rule in `docs/agents/e2e-testing.md`. The long-lived `kubectl port-forward` keeps no cap by design (it must persist for the whole step, and its stdout/stderr are already redirected to `/dev/null`, so it cannot hold the step's pipe). The `mc()` wrapper is non-recursive because `timeout` execs its argument through a PATH lookup that bypasses shell functions — verified in `dash`, `bash` and `sh`. The `catch` block's diagnostic read gets the same treatment — `timeout 30 kubectl` under a 1m op `timeout` — for the same pipe reason. ### The readonly deny probe fails on a timeout The readonly-upload probe used to branch on `mc` directly (`if mc cp …; then`), which suspends `set -e`, so a `timeout` there (exit 124) was indistinguishable from the access-denied non-zero the probe expects: a 45s hang on the deny path would exit 124, take the false branch, reach cleanup and exit 0 — a long hang turned green, the exact window this PR exists to bound. This is the first `ro-user` write in the step, so no earlier call proved that path healthy. The probe now captures the status: exit 0 is the readonly regression, 124 is a timed-out probe, both fail; any other non-zero is the expected denial. ### No unit surface; why the evidence is the proof This is an e2e-config and doc-correctness change. The embedded Chainsaw script is not extracted into a unit harness anywhere in the tree, so there is no red-to-green unit test to add without inventing a bespoke script-linter (which would be a new checker standing in for the fix). The regression is instead pinned by the mechanism proof above: the source citations show the ceiling cannot cut a descendant that holds the pipe, and the caps remove the only way a descendant outlives its budget. The deny-probe status handling and the pipe-vs-file secret reads were exercised against stub exit codes (0 → regression-fail, 124 → timeout-fail, other non-zero → expected denial) and the whole script passes `sh -n` and `bash -n`. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. The change touches only `hack/e2e-chainsaw/` suite content and `docs/agents/e2e-testing.md`; it moves or renames nothing under `hack/`, and touches no node-prerequisite or `package.mk` path. No downstream repository is reached. - [x] No downstream repository is affected by this change ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Bug Fixes* - Improved end-to-end S3 access verification so stalled Kubernetes API and S3 operations fail promptly. - Correctly distinguishes timed-out access checks from expected permission denials. - Preserves partial command output and allows subsequent test steps to continue after individual timeouts. - Applies separate time limits to bucket reads, uploads, and access checks for more predictable test behavior. - Documentation - Clarified operation timeout behavior and recommended per-command limits within the overall operation limit. - Documented timeout handling for orphaned processes and updated worst-case timing guidance. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-23 17:14:56 +04:00
Aleksei Sviridkin	ec706c49b1	docs(e2e): correct the Chainsaw Test-ordering claim in etcd and clickhouse suites (#4277 ) Fixes #4269 ## What `hack/e2e-chainsaw/etcd/chainsaw-test.yaml` explained its layout with a comment claiming Chainsaw executes Tests in name order, not file order, and used that claim to justify why teardown lives inside each scenario. The clickhouse suite had picked up the same claim. This removes the ordering claim from both — it is false at the pinned version — and re-grounds the etcd teardown justification in a reason that does not depend on order. ## Why the claim was wrong (evidence, Chainsaw v0.2.15 — the pinned version) At v0.2.15 Chainsaw guarantees no order between Tests at all: not file, not document, not name. Restating "name order" as "document order" is wrong for the same reason. Verified against the pinned source: - `pkg/runner/runner.go` — the runner registers each Test through `testing.T.Run` and the per-Test body calls `t.Parallel()` whenever `spec.concurrent` is nil or true. Neither suite sets `concurrent`, so both take that path. Go pauses those subtests and releases them together once the parent returns; they then race for the `-test.parallel` semaphore. - `pkg/runner/flags/flags.go` — `execution.parallel` maps only to `-test.parallel`, a cap on how many Tests run at a time, not an order. - `hack/e2e-chainsaw/.chainsaw.yaml` sets `execution.parallel: 1`, so Tests are serialised — one at a time — but not ordered. The E2E run on this PR's own head demonstrates it: discovery loaded the five selected Tests in document order, then ran `clickhouse-2` before `clickhouse-1` with both etcd Tests interleaved between the clickhouse pair. Serial, non-overlapping, and in an order that is neither file, document, nor name. ## The fix The teardown decision the false claim justified is still correct, for a reason that does not touch order. Each etcd scenario shares the singleton `etcd` release and its cluster-scoped Kamaji `DataStore`, runs with `skipDelete: true` (opting out of Chainsaw auto-cleanup), and v0.2.15 has no test-level `finally` — so a scenario is reclaimed only if it reclaims itself, and that is true of whichever Test runs last regardless of order. The etcd comment now says exactly this, with the ordering clause dropped rather than restated. The clickhouse header's ordering sentence is removed outright: the three Tests use distinct releases and share no state, so nothing there needed an ordering claim. This is a comment-correctness change with no runtime surface, so there is no unit test to add — the load-bearing claim was verified empirically against the pinned Chainsaw source, as above. A repo-wide grep confirms no other ordering claim survives in `hack/e2e-chainsaw/` or `docs/agents/e2e-testing.md`. Per `docs/agents/e2e-testing.md`: comment-only, no new retries, no traps, no assertion or teardown-timing changes. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * Documentation * Corrected end-to-end test comments to drop an incorrect test-ordering claim; Chainsaw v0.2.15 guarantees no order between Tests (not file, not document, not name). * Clarified that teardown behavior is independent of test execution order. * Documented the relationship between ClickHouse backup tests and the corresponding backup test suite. * No test behavior or user-facing functionality changed. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-23 16:40:30 +04:00
Aleksei Sviridkin	b96c822bde	docs(agents): give the HR-Ready budget derivation its own section The checklist entry had grown into a derivation and buried the rule it states. The rule stays in the checklist and the derivation moves to a section of its own. The derivation under-budgeted in two ways. It stated the 10m install timeout as universal, while releases an app chart renders itself take their template's spec.install.timeout, then its spec.timeout, or helm-controller's 5m where neither is set. And it restated which kinds kstatus gates on, which is easy to get wrong, so it now points at the upstream table instead. The harbor comments now name the floor each of their asserts actually sits against, and the opensearch comment stops citing a 5-6m standard the checklist no longer states. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com>	2026-09-23 11:56:12 +03:00
Aleksei Sviridkin	4373c8c0f2	test(e2e): pin the mariadb first-boot wait budgets against the tree The waits are right only relative to numbers that live elsewhere: the chart's failureThreshold, the install timeout cozystack-api stamps, and the Chainsaw op that runs the walkthrough. Deriving the floors from the tree rather than pinning literals means a change to any of them reds the contract instead of silently invalidating the waits. The extractors are tested against fixtures as well, because the tree cannot tell a correct extractor from a broken one: the chart carries a single failureThreshold and every wait clears every floor, so a reader of the wrong operand stays green. The wait_hr_ready exit branches are tested by behaviour against a stubbed kubectl, one stub per answer they have to tell apart, because a static check cannot see what errexit does to a failing read. That test and a byte-identity check cover every definition of the helper in the tree, found by grep rather than listed, so a new copy is picked up without editing the test. The redis and kafka-metadata definitions are named exceptions with their reasons in the header, and each is held to still differ, so an exception whose copy converges goes red instead of lingering. The suite runs under /bin/sh, which is dash on the runner, so the behavioural test runs its body under an explicit bash. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com>	2026-09-23 11:56:11 +03:00
Aleksei Sviridkin	957ddd7f25	fix(tests): raise the mariadb suite HR asserts past the install timeout At 6m the HelmRelease-ready asserts expired before the release's own 600s install timeout, so a failed install was reported as a bare Chainsaw timeout with no reason. 11m lets the release record why it failed first. The round-trip op grows by exactly what the two walkthrough waits gained. Otherwise a failure at either wait could run past the op, which SIGKILLs the process group and loses the script's error. That bounds one failure, not the whole flow: a run slow at every stage that fails late still reaches the op, and bounding that needs a phase budget rather than a larger literal. The endpoint asserts stay. A freshly applied CR has no conditions and kstatus reads it as current, so the HelmRelease wait can end before any pod is up; an endpoint address closes that window. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com>	2026-09-23 11:56:11 +03:00
Aleksei Sviridkin	f629280944	fix(tests): raise the mongodb, rabbitmq and clickhouse HR waits Those walkthroughs gave their database HelmReleases 300s, under the 600s install timeout the generated releases carry, so a failed install surfaced as a bare timeout rather than its own reason. None of these charts sets a startup probe, so 660s clears both floors, as it does for mariadb. Each Chainsaw op that runs one of the scripts grows by exactly what the waits it reaches gained, so it keeps the margin it had. The clickhouse op still sits above the sum of every inner ceiling. The rabbitmq one covers a failed install at the source wait, while the ceilings up to its target wait add up to more than the op, as they did before. The mongodb round-trip runs with SKIP_RESTORE, so only its source wait counts there. postgres keeps its 360s: whether a suite pays the extra minutes on every red run is a per-suite call, and this one is not made here. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-23 11:56:11 +03:00
Aleksei Sviridkin	89da4a121c	fix(cdi): stop make update from deleting first-party templates (#4097 ) ## What this PR does `make update` in `packages/system/kubevirt-cdi` began with `rm -rf templates` and then refetched one file, `cdi-cr.yaml`. Nothing in that directory is purely vendored. Two of the three manifests, the upload-proxy Ingress and TLSRoute from the Gateway API work, are ours and no fetch recreates them, so the next CDI bump would have dropped upload-proxy exposure from the chart and still exited 0. The third is `cdi-cr.yaml` itself, which despite its name carries Helm templating, the `ExpandDisks` gate, a toleration and the two RBAC documents that preauthorize cloning from `cozy-public`; the fetch overwrites it, and removing the wipe does not change that. That recipe now opens with `mkdir -p templates` and deletes nothing, which is where `packages/system/kubevirt-operator` landed at the last KubeVirt bump. The wipe bought nothing there anyway: `wget -O` truncates the file it writes. `kubevirt-cdi-operator` has the same recipe shape, and the same edit there would be a regression rather than a fix. It applies two patches, and since #3920 moved them to `-p4` they resolve, so a hunk upstream has moved out from under leaves `templates/cdi-operator.yaml.rej`; its `awk -i inplace` step leaves `templates/cdi-operator.yaml.gawk.XXXXXX` if gawk dies abnormally. Helm renders every file under `templates/`, and I reproduced both: the reject fails the package with a YAML parse error, and the gawk temp renders a duplicate object at exit 0. The wipe is what takes both, so removing it for symmetry would trade a latent loss for a live one, and the quieter of the two is the one that survives. Deleting the wipe there wants the recipe restructured to transform outside `templates/` and move the result in once, which is what `kubevirt-operator` does today and what its own comment gives this exact reason for. That is a larger change than this one and belongs on its own; #4383 makes it. Meanwhile the scan below covers the package: I added a first-party file to its `templates/` and the guard reported it, so the day that package grows one, this fails rather than the file being lost. `hack/package-update-templates.bats` guards this across the tree rather than in these two packages. It scans every package Makefile and fails when a recipe that clears its own `templates/` leaves behind a tracked file it never names. Put the removal back into `kubevirt-cdi` and it goes red naming both upload-proxy files. A file counts as named only outside the Makefile's own `NAME` and `NAMESPACE`, however the assignment is written. The match is on the variable name rather than on a list of spellings: the tree already carries three of them, `export NAME=`, the bare `NAME=`, and `NAME := ` in `packages/core/testing`, and the hole is identical in each. Without that, a package called `foo-bar` satisfies the check for its own `templates/bar.yaml` through its identity alone. Nothing the scan reaches is exempted that way as the tree stands, so the exclusion is there for the first package that would be: rename the asset in `kubevirt-cdi-operator`'s recipe and put the removal back, and the guard reports the orphaned template only with the exclusion in place. The two variables are matched by name rather than by a `[A-Z_]+=` shape, because a shape that broad is one unspaced assignment away from swallowing a variable that does name templates, which is how `etcd-operator-crds` names all five of its CRDs. A control test runs the same helpers over a fixture broken the same way, over one whose name swallows a template stem, then over that fixture rewritten to spell the removal with a trailing slash, to take one file instead of the directory, and to drop the removal entirely. Both cases pass under `hack/cozytest.sh hack/package-update-templates.bats`. Two limits, both written into the file's header. The check fires on the consequence rather than the shape, so `rm -rf templates` in a package that does refetch everything stays green, which is what keeps `kubevirt-cdi-operator` green for as long as its recipe wipes. And it only looks at a recipe clearing its own directory: `cert-manager` clears `../cert-manager-crds/templates/` across the package boundary, which needs the removal's target resolved rather than the wiping package's own, so that directory is unguarded. The other packages that clear their own `templates/` refetch all of it and are untouched here. Two more limits the header names. The scan reads the text of each package's own `update:` recipe, so a removal made elsewhere is out of range: in a prerequisite target, behind an include, or behind a recursive make. `capi-operator`, `clustersecret-operator`, `openbao` and `reloader` write `update: clean <name>-update` and take both recipes from `hack/package.mk`, which removes nothing under `templates/` today. And the glob stops at `packages///Makefile`. The package Makefiles off that depth are the group aggregators such as `packages/system/Makefile` and the vendored kamaji subcharts, and none of them declares `update`. One behaviour of the matcher worth naming, and deliberately not changed here. `wipe_command` evaluates `/^update:/` ahead of its terminator, so a second `update:` in the same Makefile re-arms the scan instead of ending it, and what that means depends on what sits between the two: adjacent or separated by a blank line, both recipes are read; separated by another target, only the first is. The direction that matters is a false positive rather than a miss. A Makefile whose first `update:` clears `templates/` and whose second does not would be reported by this scan, while make runs only the second, so the package fails over a recipe that never executes. No package in the tree defines `update:` twice today. Three more notes, all non-blocking and none acted on. The header says what the scan leaves uncovered is a recipe clearing someone else's `templates/`; that is the uncovered set for deletion, and `kubevirt-cdi` is green under the scan while still losing `cdi-cr.yaml`'s content to the in-place `wget -O` on every run, which the Makefile comment records but the header does not. The identity strip has a false-positive direction the header does not argue: a wiping package writing `templates/$(NAME).yaml` would be reported even though the recipe writes it, which no package does today. And the fixture stages `.gitkeep` with `git add --all`, so a developer whose global excludes list that name would get a red for environment rather than defect; `--force` would remove the dependency. The stem match is generous, and the helper's comment now names the two live shapes. A sibling name: `templates/crds.yaml` next to the existing `templates/crds-experimental.yaml` on `gateway-api-crds`. A mention from an unrelated context: `templates/sidecar.yaml` on `objectstorage-controller`, whose Makefile says `sidecar` across its image targets while its recipe clears `templates/` and writes only `templates/controller.yaml`. I fed both to the helper and both come back exempted; a stem the Makefile never mentions is still reported. The duplicate `test:` target in this Makefile is #4157 and stays there. It sits outside the hunks this diff touches, so removing it here would be reaching past what this change is about. One more note, not a defect. Where the comment points at `kubevirt-operator` as the shape that closes the transient-file class, `packages/system/kubevirt/Makefile` is the closer analogue for the `cdi-cr.yaml` half specifically: its `update` downloads nothing and re-applies the parameterization onto a hand-maintained CR with a per-marker self-check. And the failure message offers a third option, staging into a tmpdir the way `etcd-operator-crds` does, in a parenthetical that reads as a gloss on the option before it. What this PR does not fix, and what does. `make update` here still overwrites `templates/cdi-cr.yaml`, which the Makefile comment says plainly. A wholesale overwrite is loud: upstream's object carries neither `podResourceRequirements` nor `cloneStrategyOverride`, and `tests/importer-resources_test.yaml` and `tests/clone_strategy_test.yaml` pin both, so `helm unittest` goes red. The silent case is a partial hand reconciliation that keeps those two and drops something else, because nothing here pins the `ExpandDisks` gate, the `CriticalAddonsOnly` toleration, the `uploadProxyURL` override or the two RBAC documents that preauthorize tenant cloning from `cozy-public`. #4107, stacked on this branch, stops `update` writing the file at all and adds `tests/local_templates_test.yaml`, which pins those. One limit the header does not name, left alone here. The comment strip is anchored to the start of a line, so a trailing comment on a recipe line survives into the searched text and counts as naming the file it mentions. I confirmed it by feeding `wget ... -O templates/cdi-cr.yaml # local-extra.yaml is written by hand` through the filter. Nothing in the tree is written that way, and the header only claims comment lines are dropped, which is what it does. `sed 's/#.//'` in place of the `grep -v` closes it if it ever matters. Closes #4028. ### Screenshots No UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. Two entries come close: the website's "developer tooling (the package `Makefile`s)" line, and ccp's "move or rename anything under `hack/`". Neither fires. `make update` still behaves as the website's development page describes, nothing under `hack/` moved or was renamed, and the two files ccp's skills key on (`hack/package.mk`, `hack/common-envs.mk`) are untouched. - [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(cdi): `make update` in kubevirt-cdi no longer deletes its first-party templates ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Bug Fixes - Updated Kubernetes CDI package updates to preserve existing template files while refreshing downloaded manifests. - Prevented update operations from clearing first-party and vendored template content. - Tests - Added safeguards to detect package update recipes that could remove tracked templates without restoring them. - Added Helm unit test support for the Kubernetes CDI package. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-23 12:23:58 +04:00
Aleksei Sviridkin	8d37e0b950	docs(hack): drop a false universal from the nats-rd secrets check (#4390 ) ## What this PR does The header of the nats-rd secrets check explained its assertions with a claim about every `-rd` package: none has a tests directory or the `test` target that the helm-unittest runner looks for. `postgres-rd` has both now, so the claim told the next reader that the helm-unittest route is closed for this class of package when it is open. The claim is removed. The assertions rest only on `nats-rd` itself having neither, which is still true, so they stay as they are. Moving them to helm-unittest is a separate question and is not part of this change. Comment-only change. Closes #4209 ### Screenshots No 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 docs(hack): remove a false claim about -rd package tests from the nats-rd secrets check comment. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * Documentation * Updated a test-file comment to clarify that the inlined ResourceDefinition chart lacks a `tests` directory and the test target used by the Helm unit test script. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-23 11:52:55 +04:00
Aleksei Sviridkin	1071802e3f	test(e2e): pin the release-history read against the upstream type (#3607 ) ## What this PR does `hack/remediation-guard.bats` carried a test called "status.history extraction pins HR v2 status.history shape". It wrote a HelmRelease document containing the field names it expected, then read that same document back with the same jsonpath through `yq`. What it established is that `yq` can read a fixture the test itself composed. Its own comment named the failure it was there to catch: a future Flux release renames the field, the jsonpath matches nothing, the guard reports no remediation cycle, and real cycles pass the e2e assertion. A rename moves neither the fixture nor the expression, so the test stays green through exactly that event. The comment also named where the truth lives, `github.com/fluxcd/helm-controller/api/v2`, and then pinned a copy of the author's expectations instead. `internal/fluxcontract` supplies neither half. The expression is read out of `hack/e2e-chainsaw/_lib/remediation-guard.sh` at test time, so it is the one that ships rather than a copy. The object is a `helmv2.HelmRelease` built from the upstream type at the version `go.mod` holds, serialized and matched through `k8s.io/client-go/util/jsonpath`, which is the engine kubectl's `-o jsonpath` uses. A renamed or retyped field is then a compile error or a failed match, and a guard that stops reading history fails the lookup with a message saying so. That is one test. Two more exist so that it reads the expression kubectl actually receives, because the expression now lives in a named assignment the test sources and the caller expands: between the value checked here and the value that runs there is a shell variable, and each step of that indirection can fail while the suite stays green. The first follows the caller's own call: it reads the local handed to `helmrelease_has_remediation_cycle`, requires that local to be assigned exactly once, and requires that assignment to take the expansion of the shared name rather than the bare name, which matters because kubectl echoes a brace-less template back verbatim, so a read that lost its `$` returns its own text, clears the script's empty-history check, and matches no status. Starting from the call rather than from the text is what makes the pin describe the read that runs: asking only whether a well formed read exists somewhere in the file is satisfied by a second assignment overwriting the same local, and by a read filling a local the guard never consumes. The second holds that no shell library but the one that owns the name assigns it, which does not follow from the first: an assignment beside the read satisfies the first and still replaces the value after this file has approved the one the library exported. It covers every library in that directory rather than the caller alone, because the caller sources three of them and the owning one first, so an assignment in any of the others lands later and wins. Those two match text with a pattern, and each carries both halves of its contract. A refuse table lists the forms the pattern must reject, and an accept table lists the forms it must keep accepting, one case per freedom the code claims: for the call, the `if` prefix, the negation, an expansion with or without braces; for the read, the spelling of `-o`, the quoting of the substitution, an expansion with or without braces, and the local's name, which stays free because the call and the read are renamed together. Without the accept side those freedoms survive only until someone tightens the pattern, and nothing reddens when they go. Two boundaries, named rather than left to be found. The pin holds the shape of the read, not the vocabulary of the answer: the guard greps for `failed` and `uninstalled`, and the test supplies those two strings itself, so a release status renamed upstream leaves the jsonpath returning strings, the test green because it wrote them, and the shell grep silently matching nothing. Those values come from Helm's release status rather than the Flux type, so pinning them reaches past what this change binds, and there is more than one way to do it. Second, a literal history read that fills a local the guard never consumes is not caught, and on its own that is harmless: the guard still runs the shared expression, and the moment it stops doing so the read filling the consumed local no longer matches and the caller test fails. The caller-side half is a heuristic and is meant to be read as one. It decides whether a piece of text is the read by matching it, and a matcher that reasons about shell is wrong at some spelling by construction, which is the same objection the file raises against reading the library's assignment as text instead of sourcing it. Sourcing is available for the library and not for the caller, so what is left is to make the imprecision run one way: the count is of occurrences, so an extra one reddens and a missed one cannot pass, and a form outside the accepted set fails with the pattern printed rather than quietly stopping asserting. That direction is the whole argument for keeping it. A false red is a broken build on a legitimate rewrite, and the failure names what it did not recognise, so it costs a minute. A silent green is a guard that asserts nothing for as long as nobody looks, which is the state this branch exists to end. The ways the caller can break without anyone noticing are worth that trade. A read that lost its `$` hands kubectl a template with no braces, which comes back as the variable's own name; a hand-written path that is valid but names the wrong field comes back as that field's values. Both are non-empty, so the script's own empty-history backstop stays quiet and the cycle grep matches nothing. A deleted call is quieter still, since there is then no assertion to fail. None of the three is visible to the library-side pin, which judges the value the library hands out and never learns whether it reached kubectl. What does fail loudly is an expression kubectl cannot use at all, and that is the one case needing no pin. The shape the fixed test now has already exists in this tree, in `hack/seaweedfs-naming-audit.bats`, under "the audit's reconstruction agrees with the chart helper it mirrors". That one also writes a heredoc, which is what makes the pair worth comparing: it copies the real `_naming.tpl` out of the chart, renders it through `helm`, and checks the audit script's output against the render. The heredoc is a harness for calling the real helper, not a record of what the author expected it to say, and the two sides have different authors. A test that writes both sides proves only that it is self-consistent. Nothing is lost by the deletion. The removed test's tail fed extracted statuses into `helmrelease_has_remediation_cycle`, and the seven tests remaining in the bats file `hack/remediation-guard.bats` still cover that, including `uninstalled then deployed still returns detected`. The extraction half is now pinned more strictly than before: the old test asserted only that the output was non-empty and contained one line, the new one asserts the exact two bare tokens, so dropping the `{"\n"}` separator also goes red. CI runs it. `make test-controllers` is its own step in `.github/workflows/pull-requests.yaml` and runs `go test ./internal/... -count=1`, which picks the new package up without registration. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map against the diff. It adds a Go test package under `internal/`, deletes one test from `hack/remediation-guard.bats`, adds a variable to `hack/e2e-chainsaw/_lib/remediation-guard.sh`, points the kubectl invocation in `hack/e2e-chainsaw/_lib/run-kubernetes.sh` at that variable, adds a sentence to `docs/agents/e2e-testing.md`, and records `make test-controllers` in `docs/agents/overview.md`. No package is added, renamed or removed, and nothing touches a `values.schema.json`, a version enum, a `values.yaml` default, an `ApplicationDefinition`, a CRD, a namespace, an annotation or a telemetry metric. `hack/e2e-prepare-cluster.bats` is untouched, so the node contract that `talm` and `ansible-cozystack` restate by hand does not move. The closest call is `ccp`, whose skills anchor on files under `hack/`. Its trigger is a move, a rename, or a make target that does something different. This changes the contents of three files, not their names, and no target behaves differently; `hack/package.mk` and `hack/common-envs.mk` are untouched. - [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 NONE ```	2026-09-23 11:09:48 +04:00
Aleksei Sviridkin	90a1a947c5	feat(kube-ovn): bump kube-ovn to v1.15.26 (#4331 ) ## What this PR does Bumps kube-ovn from `v1.15.10` to `v1.15.26` (package `cozy-kubeovn` `0.38.0` -> `0.39.0`). The motivation is a cold-start failure mode in the OVN NB/SB raft leader checker. In `v1.15.10`, `checkDBClusterIntegrity` (`pkg/ovn_leader_checker/ovn.go:336`) called `os.Remove` on a healthy local raft DB (`ovn.go:367`) whenever a leader observed fewer members than expected. That count drops below the target transiently during cluster growth, a rolling restart, or recovery after a node address change, so the check could delete a valid DB and leave `ovn-central` unable to elect a leader. `v1.15.26` removes that function entirely, so the DB is no longer deleted. It is a removal of the faulty check, not a grace-period around it. The chart is re-vendored with `make update`, not hand-edited, so the templates match the image: - `charts/kube-ovn` is re-pulled from the `v1.15.26` tag and `patches/mtu.diff` is re-applied; the image build args are regenerated. - This brings the whole v1.15.26 chart: the reworked master-node affinity helpers, the widened kube-ovn CRD schema (`bandwidth.ingress`/`egress` from `integer` to int-or-string, `u2oFeatures` added), and the new `OVN_CENTRAL_MODE` (`cluster`/`single`) support with its `central-svc`, `central-pvc` and `central-hcp-rbac` templates. `OVN_CENTRAL_MODE` defaults to `cluster` (raft on hostPath), so an in-place upgrade keeps the existing topology. - Two vendored patches now included upstream in `v1.15.26` are dropped: `fix-keepvmip-portgroup.diff` (`hasAliveSiblingVMPod`, `pod.go:2437`) and `fix-vmim-scheduling-retry.diff` (scheduling requeue, `kubevirt.go:195`). `fix-vmim-target-pod-retry.diff` is kept: `v1.15.26` still returns nil at `kubevirt.go:215`. The image also picks up `kube-ovn-bfdd-supervisor`, which v1.15.26 adds (`makefiles/build.mk:33`) and upstream's `dist/images/Dockerfile:12` copies into `/kube-ovn`. `pkg/controller/vpc_egress_gateway.go:47` hardcodes that path for the `bfdd` container a `VpcEgressGateway` with `spec.bfd` gets, so a copy that stops at `kube-ovn-controller` ships a container that cannot exec. `hack/check-kubeovn-image-copies.sh` compares what our Dockerfile and upstream's copy into the image, `make update` runs it against the tag it pulls and `hack/check-kubeovn-image-copies.bats` runs it on every CI run, because `make update` otherwise rewrites only the ARG lines and the copy drifts at each bump. The `multus` commit moves the `kube-ovn-cni-plugins-checked-against` marker to `v1.15.26`. `CNI_PLUGINS_VERSION` is `v1.9.1` in kube-ovn's base image at both releases and in multus, so portmap and macvlan stay in lockstep; the marker is the only change. The image ref stays digest-pinned (`repository` + `tag@sha256`), which the promote-retag release contract requires for a cozystack-built image. The digest is carried over from `v1.15.10` as a placeholder and is refreshed to the real `v1.15.26` image by the release-candidate digest-vendoring step; no PR or main job rebuilds it. Verification: `helm unittest .` passes (22 tests), `helm template .` renders clean, the `cni-plugins-staging-contract` and `promote-retag` unit contracts are green, and the image-build `TestVMIMigrationSchedulingRetry` gate passes 6/6 on the patched v1.15.26 tree. > [!NOTE] > `start-db.sh` was substantially reworked upstream between `v1.15.10` and `v1.15.26`. The privileged fork E2E run installs into a sandbox and passes, but the in-tree E2E job is skipped on a fork PR, and a fresh install is not an upgrade. An in-place upgrade on a multi-node control plane (image rebuild plus OVN raft re-convergence) is still untested before merge. ### Screenshots N/A: no UI changes. ### Downstream repositories Walked the trigger map against the diff. The change is confined to `packages/system/kubeovn` (re-vendored chart, image build args, version pins) and a one-line marker in `packages/system/multus`. It does not add, rename or remove a package, does not touch platform values, variants, components, release assets, `ApplicationDefinition` semantics, the Talos version, or any hand-typed CRD schema in a downstream provider. No downstream trigger matches. - [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 feat(kube-ovn): bump kube-ovn to v1.15.26, which removes the OVN leader-checker integrity check that could delete a healthy raft DB and prevent ovn-central from electing a leader after a node address change or rolling restart ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - Added hosted control-plane support for OVN Central with StatefulSet deployment, persistent storage, service exposure, and dedicated RBAC. - Added split-cluster and data-plane-only configuration with external OVN endpoints. - Added configurable TLS, leader election, service, and storage settings. - Enhanced CRDs with overlay-only routing and flexible bandwidth quantities. - Bug Fixes - Improved VM migration retries during scheduling and preserved connectivity during live migration. - Improved upgrade handling and service configuration validation. - Updates - Upgraded Kube-OVN components and images to version 1.15.26. - Documented required OVN IPsec directory preparation for read-only filesystems. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-23 02:42:25 +04:00
Aleksei Sviridkin	0f21d06520	fix(dashboard): raise the console nginx header buffers for large OIDC tokens (#4376 ) ## What this PR does A user whose OIDC access token carries many group claims cannot use the console: nginx answers `400 request header or cookie too large` before the request reaches the apiserver proxy. The config baked into the console image kept the nginx defaults, 1k for `client_header_buffer_size` and four buffers of 8k for `large_client_header_buffers`, while the token arrives as one Cookie header several kilobytes long. A header field has to fit into a single buffer, so the per-buffer size is what decides this. Both directives are now 16k, at the top level of that config rather than inside the server block: a header field read before the `Host` header has selected the virtual server is measured against the default server's buffers, so the same lines inside a server block cover that server only while it stays the default one. Measured in the pinned nginx base image: a 12 KB cookie gets 400 on the old config and 200 on the new one, a 20 KB one still gets 400. `nginx -t` accepts the result. This hop sees more than the browser sent. The gatekeeper in front of it runs with `--pass-access-token=true` and `--pass-authorization-header=true`, so the request arrives carrying the `kc-access` cookie, an `Authorization: Bearer` header and `X-Forwarded-Access-Token`, three separately bounded header fields where the edge saw one. This is the console image only. The platform ingress-nginx controller also runs on the defaults, which is #2853. Worth reading that trace again before fixing it there: the controller logged `upstream_status 400`, so it forwarded the request and the 400 came back from behind the gatekeeper, which is this layer. The check in `hack/console-nginx-header-buffers.bats` reads the config out of the Containerfile heredoc and fails when either directive is missing, below 16k, or no longer ahead of the first server block. That config lives inside an image build, so no chart template and no helm-unittest case can reach it. `make unit-tests` picks the file up on its own. No image reference changes here. CI stamps the console digest. Closes #3113 ### Screenshots No UI change. This is the nginx config in front of the SPA. ### Downstream repositories The diff is the console image's nginx config plus one new `hack/.bats` file. `cozystack-ui` is archived and the console lives in this repo, so a console change belongs in this PR. Nothing under `hack/` moved or was renamed, and no make target changed behaviour. - [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(dashboard): the console no longer answers `400 request header or cookie too large` to users whose OIDC access token carries many group claims ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit Bug Fixes * Increased the console’s NGINX request-header buffer capacity to support larger header fields and request headers. * Improved handling of requests with headers that previously exceeded the default buffer limits. * Tests * Added automated checks to verify the console’s NGINX header-buffer settings remain correctly configured. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-23 01:31:45 +04:00
Aleksei Sviridkin	cc602f0dca	docs(hack): drop a false universal from the nats-rd secrets check The justification for the two nats-rd assertions ended with a claim about every -rd package: none carries a tests directory or the test target the helm-unittest runner looks for. At least one does now, so a reader deciding where to put the next -rd assertion was told the helm-unittest route is closed when it is open. The assertions rest only on nats-rd lacking both, which still holds, so the claim goes and the assertions stay as they are. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-23 00:10:54 +03:00
Aleksei Sviridkin	225b639c18	fix(cdi): stop make update from overwriting the first-party CDI object templates/cdi-cr.yaml is ours. Against upstream's bare CDI object it adds Helm templating for cloneStrategyOverride, uploadProxyURL and the worker-pod podResourceRequirements, the ExpandDisks feature gate, a CriticalAddonsOnly toleration, and the two RBAC documents that let a tenant clone from the golden-image namespace. Five of those arrived by hand. The toleration did not: it came in with the first commit, which was byte-identical to upstream v1.58.0, and upstream dropped it at v1.64.0 while this object kept it. That is the same divergence `WebhookPvcRendering` records, running the other way. Either way `wget -O templates/cdi-cr.yaml` was not refreshing a vendored file, it was overwriting a first-party one -- the same loss as the `rm -rf templates` this recipe stopped doing, arriving through the fetch instead of the wipe. What replaces it: the release the object is derived from is pinned rather than resolved from GitHub's releases/latest redirect, and `update` fetches it only to confirm the bytes still hash to a measured sha256. Nothing is written under templates/. The next CDI bump that changes the asset fails that check, which is the point -- upstream moving its own defaults is when the local copy needs a human to reconcile it, not an overwrite. The download survives a mismatch, because reconciling one starts by reading it. RELEASE is v1.66.1, which is the release the operator manifest vendors. ../kubevirt-cdi-operator decides what the platform deploys, and it can move to another release without this file changing. RELEASE is still set by hand rather than derived, because a derived value is not a pin. What `update` adds is a comparison with the cdi-operator image tag that package vendors, failing when the two differ; without it the recipe's closing line would name a release the platform may no longer run. The difference from upstream is stated against v1.66.1 rather than as a standing fact: upstream carried WebhookPvcRendering from v1.63.0 through v1.65.0 and dropped it by v1.66.0, and this object never carried it, so the difference ran both ways for three releases and one way after. The comment says to re-derive it at each bump for that reason. The `test:` target this package needed arrived separately, so what it gains here is the `.PHONY` line that came without: hack/package.mk declares only the targets it defines itself, so a file or directory named test or update beside this Makefile would otherwise make that target up to date and skip the recipe. The package also carried a second, identical `test:` target, so make warned on every invocation in it; that one is gone, and the comment above the remaining target is what should stop another being added. The suite is what makes the local delta checkable rather than described, which is why the comment above `update` no longer lists what the object adds: two copies of that list would diverge, and only one of them can fail. hack/package-update-templates.bats attributed `@#` recipe comments to one named package. The tree writes them in more than one, and this adds another, so the attribution goes; the case below it exercises both spellings without naming who writes which. The header in that file also lost its two-item example of first-party manifests. The list named the two the recipe never wrote, and this change stops it writing `cdi-cr.yaml` as well, so the example would have been short by one the moment it landed. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-23 00:08:41 +03:00
Aleksei Sviridkin	2f5625f0dc	fix(cdi): stop make update from deleting first-party templates The update recipe in kubevirt-cdi opened with `rm -rf templates` and then refetched one upstream manifest. The directory keeps two more files -- the upload-proxy Ingress and TLSRoute -- that are written by hand and never refetched, so the next CDI bump would drop upload-proxy exposure from the chart while `make update` still exited 0. The clean slate bought nothing either way: wget -O truncates the file it writes. The recipe now creates the directory with `mkdir -p` and leaves the rest of it alone, which is what kubevirt-operator moved to when it was last bumped. kubevirt-cdi-operator keeps its wipe. The same change there would be for symmetry rather than for a loss, and it is not free: both its patch calls resolve, so a hunk upstream has moved out from under leaves templates/cdi-operator.yaml.rej, and gawk's inplace edit leaves templates/cdi-operator.yaml.gawk.XXXXXX if it dies abnormally. Helm renders every file under templates/, so the first fails the package loudly and the second renders a duplicate object at exit 0. The wipe is what takes both. Removing it wants that recipe restructured to transform outside templates/ the way kubevirt-operator now does, which is its own change. Until then the scan below is what watches that package, with the limit the scan already documents: it reports a tracked template whose stem the Makefile never mentions, so a name that collides with something already written there, templates/ tolerations.yaml against the patches/tolerations.diff line, is exempted. It catches the loss it was written for, not every loss. The bats case scans every package Makefile rather than a pinned list, so a first-party template added under a recipe that clears templates/ fails the commit that adds it instead of vanishing at the next bump. A package's own NAME and NAMESPACE do not count as naming a file, because a package called foo-bar would otherwise vouch for its templates/bar.yaml with nothing writing it. A removal reaching into another package is outside the scan's range, and the header records the case in the tree that this leaves uncovered. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-22 23:53:49 +03:00
Aleksei Sviridkin	97cb2781db	test(e2e): pin the release-history read against the upstream type The test that claimed to pin the HelmRelease v2 history shape wrote a HelmRelease document containing the field names it expected, then read that same document back with the same jsonpath. It established that yq can read a fixture the test itself composed. Its own comment described the failure it was meant to catch: a future Flux release renames the field, the jsonpath matches nothing, the guard reports no remediation cycle, and real cycles pass the e2e assertion. A rename moves neither the fixture nor the expression, so the test stays green through exactly that event. The comment also named where the truth lives, github.com/fluxcd/helm-controller/api/v2, and pinned a copy of the author's expectations instead. Replace it with a Go test that supplies neither half. The expression is read out of the shell library that ships it, and the object is built from the upstream type at the version go.mod holds, then serialized and matched the way kubectl does. A renamed or retyped field is a compile error or a failed match; a guard that stops reading history fails the lookup with a message saying so. Both directions were confirmed by mutation. Locating the expression by searching the file is not enough on its own. Inline in a long script, locating it means deciding which occurrences are code and which are prose, and every way of getting that wrong fails green: an example quoted near a broken read satisfies the pin while the guard reads nothing. So the expression moves into a named assignment in remediation-guard.sh, which the guard already sources, and the test sources that library and takes the value the shell is left holding. Deciding which text is the assignment is then not the test's problem: an `export`, an indent, a second assignment later in the file all resolve the way the guard will resolve them, because the thing resolving them is the shell. Sourcing borrows the environment along with the shell, so the snippet unsets the name before it sources. `go test` hands the child its own environment, and a shell that had exported this name would answer in place of a library that stopped assigning it: the value comes back non-empty, the empty check stays quiet, and the pin passes over a guard that reads nothing. That is this file's own subject reaching it through the environment rather than through the source. Moving the expression out of the call site puts a step between what is pinned and what runs, so the call site is checked too, and the check starts from the call rather than from the text: the local the guard is handed must be assigned exactly once, by an anchored assignment whose jsonpath is the expansion of the shared name. That a well-formed read exists somewhere in the file is a weaker claim than it reads as, since a second assignment to the same local, or a read filling another one, satisfies it while the guard runs a literal nothing here pins. The expansion and not the name: kubectl echoes a braceless template back verbatim, so a read that dropped the `$` would return its own text, clear the script's empty-history backstop, and carry no status the cycle check matches. The library is checked by sourcing rather than by reading, so an assignment the shell would run is resolved by the shell instead of by a guess about which text the shell would pick. Each rule is pinned by its own test, so a regression names which one it opened rather than only that extraction broke. How kubectl's printer treats an absent path is its own configuration rather than part of this contract, so it is left unasserted: guessing at it would put the expectation and the fixture back in the same hands. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-22 23:41:35 +03:00
Aleksei Sviridkin	4475fb4c17	test(e2e): assert the System-mode OIDC uid claim mapping The System lane read the rendered AuthenticationConfiguration for the issuer URL and the audience but not for the mapping that gives the user a UID. A configuration missing that mapping is still valid, so the tenant control plane starts and the suite passes while every OIDC user authenticates with an empty UID. The unit twin mocks the Secret without the mapping, so dropping the grep from the helper reddens it rather than leaving it unpinned. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-22 23:04:28 +03:00
Aleksei Sviridkin	4bf95815a8	fix(kubernetes): render the aggregation-layer UID header for tenant OIDC (#3539 ) ## What this PR does Tenant clusters with `spec.oidc.mode: System` or `CustomConfig` authenticate users fine, but a UID does not survive the hop to an aggregated API server. The tenant kube-apiserver publishes `requestheader-uid-headers` into the `extension-apiserver-authentication` ConfigMap only when the flag is set, and the chart never set it. The aggregation layer still sends `X-Remote-Uid`, but the extension server trusts only the header names published in that ConfigMap, so it ignores the one it gets and the call arrives without a UID. The field documentation even told operators to add the flag by hand via `controlPlane.apiServer.extraArgs`. It forwards a UID rather than creating one. ServiceAccount tokens carry one, and for those the aggregation hop starts working. Users authenticated by `spec.oidc.mode: System` still have none, because the generated AuthenticationConfiguration maps `username` and `groups` only; giving them one means mapping a claim, which changes the identity of users who have none today, and is tracked separately. The chart now renders `--requestheader-uid-headers=X-Remote-Uid` on v1.32 and newer, and adds `--feature-gates=RemoteRequestHeaderUID=true` on v1.32, where the gate is still alpha and the apiserver rejects the flag without it. From v1.33 the gate is beta and on by default, so only the header flag is rendered. v1.31 gets nothing: the flag does not exist there, and writing it by hand stops the apiserver from starting. `--emulated-version` in the same passthrough moves that decision, because the gate resolves through the versioned spec at the emulated version. Rendering the gate on every version from v1.32 rather than on v1.32 alone was the alternative, and it would remove the migration step entirely, because an opt-out would then drop two entries instead of one and move the entry count by itself. It loses on what it leaves behind: a redundant gate term on every cluster, which stops the apiserver from starting on the day the gate is retired from the known list. The design also prefers rendering over failing for the same reason the fail branches are narrow, since a cluster stranded on a HelmRelease that no longer converges is a worse outcome than one that never gets the UID fix. The chart reads an operator entry the way the apiserver reads it: header list elements matched whole and untrimmed and folded as `EqualFold` folds them, gate names compared exactly, gate values resolved as `strconv.ParseBool` resolves them, `AllAlpha` and `AllBeta` blankets honoured, an empty value treated as no entry. On v1.32 an entry that says nothing about `RemoteRequestHeaderUID` gets a second one carrying the operator's own terms plus `RemoteRequestHeaderUID=true`. Carrying those terms into the merged copy is what makes it land correctly whether the tenant control plane collapses duplicate flags into the last one or hands both to the apiserver, which merges them in the order it reads them. Tests cover the render matrix over mode, version and operator input: every fail path with its exact message, the ownership and merge paths, duplicate entries in both orders, blanket gates, empty values, and case variants on the header name and on the gate value. The latest-version tenant lane creates its tenant with `spec.oidc.mode: System` and asserts the flag on the rendered KamajiControlPlane, so the tenant apiserver starts with the flag in CI. Nothing asserts the UID arriving at an aggregated apiserver; that needs an extension server inside the tenant and is not in this change. The layer where the count-keyed reset actually bites is the tenant Deployment's arguments, and asserting there needs a live tenant control plane, so it is not in this change either. The rule this follows is that a configuration which renders and boots today keeps rendering. Both flags are new, so an `extraArgs` shape reaching this code was rendering before it existed, and stranding a running tenant on a HelmRelease that no longer converges is a worse outcome than one that never gets the fix. A few shapes do stop rendering, and reaching any of them takes writing `--requestheader-uid-headers` into the passthrough by hand, attempting exactly what the chart now does and getting it wrong; in each the kube-apiserver fails option validation and never starts, so nothing that was working is lost. `docs/oidc-tenant.md` covers these shapes. One known limitation is in there and worth naming here: on v1.33 and newer, adding the documented opt-out is count-neutral in `extraArgs`, and so is removing it again, and on v1.33 and v1.34 adding or removing `--emulated-version=1.31`. The pinned Kamaji build rewrites a running apiserver's arguments only when that count changes and otherwise keeps every flag that has left the list, so either the header flag stays beside a now-disabled gate or the disabled gate stays beside the header flag, and the apiserver refuses to start. Changing the entry count in the same edit is the mitigation, and the fix belongs to the pin rather than to this chart, tracked in #3541. The paths that need no operator edit are clear: turning the feature on for a default tenant moves the count, and a tenant already carrying the opt-out renders byte-identically before and after. Out of scope: `mode: None` still publishes no UID header even though ServiceAccount tokens carry UIDs. That is pre-existing, and widening the condition changes the render for every default tenant. Tracked separately, along with the claim mapping above. Closes #3377 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change Checked the trigger map against the diff: no package added or removed, no schema field, enum or default changed (description text only), no ApplicationDefinition surface touched, no hack/ layout moved. The managed-apps reference page picks up the new field description through release-time README regeneration. ### Release note ```release-note fix(kubernetes): tenant clusters with `spec.oidc.mode` set to `System` or `CustomConfig` now render `--requestheader-uid-headers=X-Remote-Uid` on the tenant kube-apiserver, plus the `RemoteRequestHeaderUID` feature gate on v1.32, so that an aggregated API server can read the caller's UID out of the request headers. Without that flag the tenant apiserver never publishes `requestheader-uid-headers`, and an aggregated API server ignores the UID header even for callers that have one, ServiceAccount tokens among them. It forwards a UID rather than creating one: users authenticated by `spec.oidc.mode: System` still have none, because the generated AuthenticationConfiguration maps `username` and `groups` only, and that is tracked separately. Adding the flag by hand through `controlPlane.apiServer.extraArgs` is no longer necessary, and an entry of your own is respected: the chart steps aside when yours already carries `X-Remote-Uid` or already enables the gate, merges `RemoteRequestHeaderUID=true` into a copy of a v1.32 `--feature-gates` entry that says nothing about the gate, and renders neither flag when you switch the gate off. The render fails where a `--requestheader-uid-headers` list of your own would stop the kube-apiserver from starting anyway: one that omits `X-Remote-Uid`, one with an empty element, and a list of your own while the gate is off. On v1.33 and newer there is a migration step, and it applies in both directions: the pinned Kamaji build rewrites a running apiserver's arguments only when the number of `extraArgs` entries changes, and otherwise keeps every flag that has left the list. Adding the documented opt-out keeps that number the same, so the header flag already on the Deployment stays beside the now-disabled gate. Removing the opt-out later keeps it the same too, so the disabled gate stays on the Deployment beside the header flag the chart renders again, although your spec no longer shows it. Adding or removing `--emulated-version=1.31` has the same shape both ways on v1.33 and v1.34. The apiserver refuses to start in each case, so change the entry count in the same edit. ```	2026-09-23 00:03:20 +04:00
Aleksei Sviridkin	7d53f7af22	fix(dashboard): raise the console nginx header buffers for large OIDC tokens The nginx config baked into the console image kept the nginx defaults for client_header_buffer_size (1k) and large_client_header_buffers (4 8k). An OIDC access token carrying many group claims arrives as a single Cookie header of several kilobytes, which no longer fits into one buffer, so nginx answered 400 "request header or cookie too large" before the request reached the /apis, /api or /k8s/ upstream. Raise both to 16k, at the top level of that config rather than inside the server block: a header field read before the Host header has selected the virtual server is measured against the default server's buffers, so the same lines inside a server block cover that server only while it stays the default one. Measured against the pinned nginx base image, a 12 KB cookie gets 400 on the old config and 200 on the new one. The ceiling moves rather than disappears: a 20 KB cookie is still rejected. This is the console hop alone. The platform ingress-nginx controller in front of it runs on the same defaults, so where the dashboard is published through an Ingress a token large enough to exceed them is still rejected at the edge; where it is published through the gateway, whose edge has no per-header ceiling of this kind, the console was the hop returning the 400. Raising the controller is a separate change in a separate package. The accompanying check extracts the config from the Containerfile heredoc and fails if either directive is missing, below 16k, or no longer ahead of the first server block: the config lives inside an image build, where no chart template and no helm-unittest case can reach it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-22 21:43:23 +03:00
scooby87	7d07bbe3ce	[kafka] Migrate from ZooKeeper to KRaft mode (#2352 ) ## Summary - Replace ZooKeeper-based Kafka deployments with KRaft (Kafka Raft) metadata mode - Existing ZooKeeper instances migrate automatically and in place via a Helm `pre-install,pre-upgrade` hook Job that drives Strimzi's ZK-to-KRaft migration state machine - New instances deploy directly in KRaft mode with broker and controller `KafkaNodePool`s - The node pools are owned by the migration hook, not by regular chart manifests (see "Why the pools are hook-managed") - Kafka version is user-configurable (v3.9 default, v3.8 available) - Strimzi operator is unchanged in this PR (stays at 0.45.1-rc1); 0.45.x is the last line that supports ZooKeeper→KRaft migration - ZooKeeper removed from the dashboard UI (ApplicationDefinition) ## Migration strategy Both fresh installs and ZooKeeper→KRaft migrations are driven by a single Helm hook Job (`templates/migration-hook.yaml`, `pre-install,pre-upgrade`) that owns the `KafkaNodePool`s. On every install and upgrade the Job: 1. Ensures both node pools exist (idempotent): a broker pool named exactly `kafka` for a migrating cluster — so Strimzi adopts the existing `<cluster>-kafka-N` brokers and their PVCs in place — or `b-<hash>` for a fresh cluster, plus a controller pool `c-<hash>`. The controller pool pins `strimzi.io/next-node-ids` above the broker range so the broker pool keeps node IDs `[0..N-1]` and Strimzi cannot hand the existing broker IDs to the fresh controllers. The shared `kafka` name is claimed atomically (via `kubectl create`) so only one migration per namespace can win it. 2. Inspects the Kafka CR: no CR → fresh install (pools ensured; the main apply then creates the KRaft Kafka CR, nothing to migrate); already `strimzi.io/kraft=enabled` or `status.kafkaMetadataState: KRaft` → done; any other read error → fail closed. Otherwise (a ZooKeeper cluster) it migrates. 3. Annotates `strimzi.io/node-pools=enabled` then `strimzi.io/kraft=migration`, polls `status.kafkaMetadataState` through `ZooKeeper → KRaftMigration → KRaftDualWriting → KRaftPostMigration → PreKRaft → KRaft`, and only once a safe state is reached flips `strimzi.io/kraft=enabled`. If it times out in an intermediate state it aborts without finalising, so Helm fails the upgrade and the next reconcile retries from where it left off. Data is migrated in place: brokers keep their node IDs and reuse their existing PVCs, nothing is copied. Migration is one-way — Strimzi has no supported KRaft→ZooKeeper rollback once complete, so lowering the `version` field or rolling the app back to a pre-KRaft chart is unsupported (take a backup first). ### Why the pools are hook-managed The hook must create the pools and walk the migration state machine before the main release apply. A resource a hook creates cannot also be a regular chart manifest across the ZK→KRaft boundary: Helm's main apply finds the pool live in the cluster but absent from the previous (ZooKeeper) release revision and aborts with `original object KafkaNodePool ... not found`, leaving the release `failed` even though the cluster migrated fine (reproduced under server-side apply, the helm-controller default). Managing the pools only from the hook avoids that, so `templates/kafkanodepools.yaml` and the deployed-version gate ConfigMap (`templates/version.yaml`) are removed and the pools are applied idempotently by the Job on every install and upgrade. ## Bug fixes included - ZooKeeper `metricsConfig` referenced wrong ConfigMap key (`kafka-metrics-config.yml` instead of `zookeeper-metrics-config.yml`) - ZooKeeper `storageClass` used `kafka.storageClass` instead of `zookeeper.storageClass` - WorkloadMonitor used `.Values.replicas` instead of `.Values.kafka.replicas` - `validateNameLength` now caps a Kafka app name at the KRaft controller pod-hostname budget (`<release>-c-<hash>-<id>` must fit 63 chars), instead of admitting names that later fail the chart render forever ## What's next A follow-up PR will upgrade Strimzi to 0.51.0 and Kafka to 4.2 once all instances are confirmed migrated to KRaft. That PR is already prepared on branch `KRaft-upgrade-strimzi-051`. ## Test plan - [x] `helm template` renders correctly for new KRaft instances (no ZooKeeper refs) - [x] `helm template` with custom values (1 replica, external, topics, v3.8) - [x] Version validation rejects unsupported versions via JSON schema - [x] helm-unittest: 86/86 across 9 suites - [x] `hack/kafka-migration-hook.bats`: 12/12 — the real hook script executed against a stub kubectl (resolution branches, fail-closed reads, atomic collision guard, finalize guard) - [x] Operator template renders with the unchanged Strimzi 0.45.1-rc1 - [x] Live migration on a disposable cluster under server-side apply: fresh install → healthy KRaft, release `deployed`; in-place migration of a 20000-message ZooKeeper cluster → release `deployed`, broker PVCs adopted in place (same UIDs), and the KRaft data byte-for-byte identical to the pre-migration ZooKeeper data (matching sha256 over every `partition:offset:key:value`) ### Release note ```release-note [kafka] Enable KRaft metadata mode with automatic, in-place ZooKeeper-to-KRaft migration. Kafka version is now configurable (v3.9, v3.8). The Strimzi operator is unchanged (0.45.1-rc1). ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * New Features * Added chart-level Kafka version selector (default v3.9) and KRaft-aware node pool rendering with controller storage sizing. * Updates * Default Kafka appVersion updated to 3.9.0; operator and image tag defaults aligned to 0.45.0. * Templates now omit ZooKeeper resources and metrics when KRaft is in use. * Tests * New Helm unit tests for Kafka CR, node pools, metrics, topics, dashboard mappings, and workload monitors. * Documentation * Values, schema, and README updated to document new version and controller storage options. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-22 19:19:21 +03:00
Aleksei Sviridkin	187e4ce12b	test(e2e): assert the tenant OIDC UID header on the control plane The render matrix proves the chart emits --requestheader-uid-headers, but nothing checked that the flag survives the path a real tenant takes: aggregated API admission of spec.oidc, the HelmRelease, and the rendered KamajiControlPlane. Assert it in both OIDC helpers, next to the existing --authentication-config check on the same resource. Read the arg list once into a variable and grep the capture twice. The existing check consumed a pipeline, which is single-use, and a second kubectl call would double the management-cluster reads these helpers already make. Only the header flag is asserted. The chart also renders --feature-gates=RemoteRequestHeaderUID=true, but only on v1.32 where the gate is still alpha, while this lane takes the highest version in the map, which is above that. The header flag is the part that holds wherever the flag exists at all. The bats case for the missing flag runs the helper as a standalone subshell under set -e rather than as an if condition. Shells suppress errexit for a command whose status is being tested, and that reaches inside the subshell, so the if spelling reports success and pins nothing. The binding query next to these assertions piped kubectl straight into sort and so returned sort's status. Its callers capture it in a command substitution, where errexit does not reach, so a failed query read as an empty binding list. It now returns kubectl's failure itself. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>	2026-09-22 18:23:48 +03:00
Aleksei Sviridkin	9efe78ce44	test(e2e): hold the tenant-cluster install wait to an ordering rule (#3802 ) A test that creates a tenant `Kubernetes` cluster must not end while that release is still installing. The kind drops the install wait on its main resources, but the chart's blocking post-install hook still holds the install open, so a test that asserts and ends hands the rest of it to teardown. Teardown then reds on a step that tests nothing. Reading the cluster mid-install is fine. Ending mid-install is not. `hack/e2e-chainsaw/_lib/run-kubernetes.sh` already waits for the parent release before its tenant-cluster assertions, and that wait is older than this change. Nothing held it there. This adds the guard that does. The guard reads the script, not Chainsaw fixtures: the cluster comes from a heredoc, so a guard walking `apply.file` would find nothing to check and pass. It requires the CR, then a readiness wait on the parent release, then the assertions the test ends on. The wait has to block, and it has to be unconditional in the body that holds it. One more case holds this script to being the only creator of that kind, since a second one would be governed by no rule here. The ordering is read within one function body, so splitting `run_kubernetes_test` reds the unit lane until the rule is pointed at the new shape. Timeout length stays with review. The floor depends on where the wait sits, and the script text does not carry that. Which spellings each matcher takes, which rewrites it refuses instead of judging, and the blind spots it names are written in the guard's header next to the code, each with its mechanism and whether it fails loudly or quietly. Fixtures mark their own lines, so a probe resolves by name instead of by a line number that drifts. No runtime code here: the guard, its fixtures, and the conventions that state the rule. ```release-note NONE ```	2026-09-22 19:00:52 +04:00
IvanHunters	a4547a6fe0	test(kube-ovn): compare image copies against upstream's Dockerfile packages/system/kubeovn/images/kubeovn/Dockerfile is a hand-kept copy of upstream's dist/images/Dockerfile, and `make update` rewrites only its ARG VERSION and ARG TAG. A binary upstream starts shipping therefore does not reach our image on its own, and nothing read the two files together: kube-ovn-bfdd-supervisor went missing that way with the whole suite green. hack/check-kubeovn-image-copies.sh normalizes the COPY instructions of both files and reports either side's surplus. It compares only the stages built FROM kubeovn/kube-ovn-base, because our builder stage clones and patches kube-ovn and its COPY lines have no upstream counterpart, and it treats an empty result as a broken check rather than agreement. `make update` now runs it against the tag it pulls, so the next bump stops on a mismatch, and hack/check-kubeovn-image-copies.bats runs it on every CI run against a verbatim copy of upstream's Dockerfile at the vendored appVersion. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>	2026-09-22 17:10:05 +03:00
myasnikovdaniil	41ef4a6437	fix(ci): teach the overlay to repair an unstamped images/.tag ref (#4362 ) ## What `hack/overlay-main-images.sh` walks `images/.tag`, but every branch of its line classifier needs either a YAML key or an `@sha256:`. A `.tag` file holds one bare reference and nothing else, so until a build stamps it the line has neither, the drift guard reads it as a non-ref change, and the committed reference survives. For a package onboarded with the `:v0.0.0` placeholder this repo already uses for a new image, that surviving reference is one no registry serves. The overlay is the only thing that would replace it for a PR that does not rebuild the package, so once such a package is on main every full-suite run that skips its build keeps the placeholder. The same placeholder inside a `values.yaml` repairs itself, because there it sits under an `image:` key the classifier recognises. The difference is the file shape, not the placeholder. ## Reproduction A fixture holding a committed bare tag against an artifact copy carrying a stamped one: ``` $ cd fixture/fakerepo && sh ../../hack/overlay-main-images.sh ../mainpkgs '[]' '' overlay: packages/system/x/values.yaml -> current-main drift (non-ref change) in packages/apps/x/images/x.tag -> keeping committed ref ``` Both files differ only in their reference. The `values.yaml` one is repaired; the `.tag` one is not. Adding any digest to the same placeholder makes it pass, which is what narrows this to the classifier rather than the file. ## Why it has not bitten yet 17 of the 19 `.tag` files in the tree carry `@sha256:` and pass on that branch alone. The only other digest-less one is `packages/extra/seaweedfs/images/seaweedfs-cosi-driver.tag`, and it survives only because it is third-party: `build-main` never rebuilds it, the artifact copy is byte-identical, and the classifier is never reached. The first digest-less first-party `.tag` is what turns the gap live, and one is coming in #3426. ## The fix `hack/promote-rewrite-tags.sh` already carries a second match branch for exactly this shape, with a comment saying why ("the second branch is what catches a bare `.tag` file, whose content has no YAML key at all"). This gives the overlay the same one, scoped to `.tag` so an unkeyed line in a `values.yaml` still counts as drift, because only in a `.tag` file is a whole line known to be a reference and nothing else. Two tests in `hack/overlay-main-images_test.bats`: the unstamped placeholder is overlaid, and a `.tag` whose non-reference line differs still counts as drift, so the rule cannot quietly widen into "every line in a `.tag` file is a reference". Removing the fix fails the first one. ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Bug Fixes* - Fixed image overlay handling for `.tag` files containing bare image references, allowing current references to be applied correctly. - Preserved drift detection when `.tag` files also contain unrelated, non-reference changes. - Documentation - Clarified how image-reference tooling handles bare references and package-tree processing. - Tests - Added coverage for bare-reference overlays and mixed reference/comment changes. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-22 18:53:28 +05:00
Aleksei Sviridkin	c0ff783026	fix(hack): make bats unit tests pass on a BSD/macOS checkout (#4284 ) ## What this PR does Refs #4272 (Defects 1 and 2). On a clean checkout of `main`, `make unit-tests` is red out of the box on macOS for reasons that have nothing to do with the change under test: the BSD userland shipped with macOS differs from the GNU tools on the Linux CI runners, and CI never sees any of it. This PR closes the two worst of those, both scoped to `hack/`. Defect 1 — the runner cannot parse a non-ASCII `@test` title. `hack/cozytest.sh` rebuilds each test's function name by walking its title one character at a time with awk `substr()`. BSD awk aborts that walk on the first multibyte byte (`towc: multibyte conversion failure`) rather than returning it, so a file carrying a non-ASCII `@test` title never parses and not one of its tests runs — silently, because the suite only reports that the runner errored. Four `hack/.bats` files carry such a title today. The fix pins `LC_ALL=C` on all three awk passes (title parse, run selection, and the exit-code scan of the captured log) so each reads the input byte-wise; the parse and selection passes must share the ctype or they would derive mismatched function names for one title. A new regression test, `hack/cozytest-nonascii-title.bats`, builds a multibyte-titled fixture at runtime and asserts the runner actually runs it; it is red against the pre-fix runner (mutation-verified) and green after. Defect 2 — GNU-only `sed -i`.* `hack/nightly-mirror.sh`, `hack/promote-rewrite-tags.sh`, and the fixture builder in `hack/promote-rewrite-tags_test.bats` edit files in place with `sed -i`. BSD sed reads the argument after `-i` as a mandatory backup suffix, so `sed -i -e` and `sed -i -E` swallow the following flag as that suffix and the edit either errors or silently no-ops. Each is rerouted through a temp file, portable to both seds. The `nightly-mirror.sh` rewrite additionally relied on the GNU `\?` optional quantifier, which BSD BRE reads as a literal `?` so a bare host was never matched; it is replaced with the POSIX `\{0,1\}`, equivalent on both. All three suites are green on macOS after the change, and behaviour on GNU sed is unchanged. ### Scope This PR is deliberately scoped to Defects 1 and 2. Defect 3 from the issue — a GNU-only `date -d` in `hack/seaweedfs-naming-audit.sh` — is tracked separately as #4273 and is left untouched here. For transparency: with Defect 1 fixed, several `hack/.bats` suites that previously never ran now run, and a handful still fail on macOS because they depend on GNU coreutils `timeout`, which base macOS lacks (`capture-dataplane`, `cozytest-capture-gate`, `run-kubernetes-join-timing_test`, `run-kubernetes-runner-cpu_test`, `runner-identity`). Those failures are pre-existing — identical before and after this change when the runner is swapped in place — and are a distinct defect from the two fixed here, so they are out of this PR's scope. A fully green `make unit-tests` on macOS additionally depends on #4273 and on that separate `timeout` gap. ### Verification All fixes were verified on a genuine macOS host (Darwin, BSD awk 20200816 / BSD sed). `hack/cozytest-nonascii-title.bats`, `hack/nightly-mirror_test.bats` (5 tests), and `hack/promote-rewrite-tags_test.bats` (17 tests) are green; the two files the issue reports as running zero tests (`overlay-main-images_test.bats`, `promote-gate-contract.bats`) now yield 13 and 15 `Test OK`, matching the issue's numbers. Each fix is mutation-proven: reverting it turns the corresponding suite red again. ### Downstream repositories The diff touches only internal `hack/` tooling — the bats test runner and two release scripts. No downstream repository vendors or restates these: `cozystack/ccp` and `cozystack/external-apps-example` gate on `hack/package.mk`, `hack/common-envs.mk` and `hack/update-crd.sh`, none of which this PR changes, and nothing renames or moves a file or alters what a make target does. - [x] No downstream repository is affected by this change ### Release note ```release-note fix(hack): make the bats unit-test runner parse non-ASCII `@test` titles under BSD awk, and route the in-place edits in `nightly-mirror.sh` and `promote-rewrite-tags.sh` through a temp file so they work under BSD sed — closing two of the host-tool gaps that make `make unit-tests` red on a clean macOS checkout ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit Bug Fixes * Improved test runner compatibility with non-ASCII test titles and output. * Improved text-processing and file-rewrite behavior across BSD and GNU environments. * Preserved file permissions and metadata during automated rewrites. * Improved handling of host values and release tags across supported platforms. * Tests * Added coverage for non-ASCII test titles. * Updated rewrite tests for portable BSD and GNU tooling. * Documentation * Clarified supported BSD and GNU command-line tool compatibility. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-22 17:37:41 +04:00
Alexey Artamonov	0b9332877e	test(kafka): drop the false-green ZK->KRaft migration chainsaw suite The kafka-zk-to-kraft-migration suite did not exercise the pre-upgrade hook, it reimplemented a simplified flow beside it: a controller pool named "controller" instead of the hashed c-<hash>, no strimzi.io/next-node-ids pin, and a single broker so the controller-vs-broker node-id collision could never occur. That is exactly the defect the hook exists to prevent, so the suite stayed green on a revision whose in-place migration lost every message — reproduced by hand 2/2 on a live cluster. A suite that cannot fail on this code is worse than no suite: it reads as migration coverage and invites trust it has not earned, while costing ~380s on every Kafka PR and asserting only that a PVC object exists, never reading a record back. The migration hook's bats suite and the live upgrade runs are the real coverage; the Strimzi bump would delete this suite regardless. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>	2026-09-22 16:05:44 +03:00
Aleksei Sviridkin	1c88014661	fix(installer): keep node_modules out of the packages artifact (#4290 ) ## What this PR does `flux push artifact` walks the whole `packages/` tree and reads neither `.gitignore` nor `.sourceignore`, so a working copy where somebody ran `pnpm install` under the vendored console ships the console's dependency tree inside the bundle. Helm then refuses the dashboard chart on every cluster that pulls it, because one dependency file sits above the 5MB per-file limit, and the build itself reports nothing: ```text Could not load chart: decompressed chart file "dashboard/images/console/node_modules/.pnpm/@rolldown+binding-darwin-arm64@1.0.0-rc.15/node_modules/@rolldown/binding-darwin-arm64/rolldown-binding.darwin-arm64.node" is larger than the maximum file size 5242880 ``` Passing `--ignore-paths` replaces flux's built-in list rather than extending it, so the defaults are restated alongside `node_modules/`. `hack/check-packages-artifact-ignores.bats` holds both halves: dropping `node_modules/`, dropping one of the defaults, or declaring the variable without passing it to the recipe each turn the test red. Its second case re-reads the defaults out of the installed flux, so an upstream addition surfaces as a failure rather than as a pattern quietly missing from the artifact. Measured on a tree with the console's dependencies installed: the artifact goes from 80MB carrying 32397 `node_modules` entries to 6.7MB carrying none, with the dashboard chart still in it. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change The two workflows that push the same artifact themselves (`e2e-fork.yaml`, `nightly.yaml`) run from a clean CI checkout, where the tree has no `node_modules` to pick up. ### Release note ```release-note fix(installer): the packages OCI artifact no longer picks up node_modules from a developer's working copy, which made the dashboard chart unloadable on every cluster that pulled the bundle. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * Bug Fixes * Package artifacts now exclude unnecessary files, including version-control metadata, common media and archive formats, and dependency directories. * This helps prevent oversized files from exceeding Helm’s per-file size limit. * Tests * Added validation to ensure package artifact ignore patterns remain complete and aligned with the packaging tool’s defaults. * Optional checks report when the packaging tool is unavailable without failing the test suite. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-22 16:17:52 +04:00
Alexey Artamonov	ac92b6b95c	fix(e2e): cap the bucket verify S3 reads so the step honors its timeout A Chainsaw `script` op runs its shell under exec.CommandContext with stdout and stderr wired to in-process buffers. When the op timeout fires it SIGKILLs the shell process alone, not its descendants, and cmd.Wait() does not return until every process that inherited the pipe fds closes them. An mc that keeps retrying an S3 call after the shell is gone holds the pipe open, so the op runs to mc's own exhaustion instead of stopping at its ceiling. This is why the bucket suite's verify-every-bucket-takes-a-write step ran 606s under a 300s ceiling. verify-s3-access shares that shape, so wrap each S3 and apiserver read in its own `timeout`: a hung read dies at a known budget and the op timeout stays a backstop. The 30s kubectl cap matches the terminal `k` wrapper the redis suite carries. The two `mc cp` PUTs take 45s, the budget verify-every-bucket-takes-a-write already gives a write, because a working PUT can still wait while the master grows a volume for a freshly created bucket; 15s left too little headroom on that grow and could turn a slow-but-healthy write red on an unrelated PR. The 15s cap on the mc reads has no terminal precedent: the backup-access preflight caps `mc cp` at 15s too, but inside a retry loop where a 124 costs one attempt rather than the step, so it is not one. Land each secret's encoded value in a file before base64 -d so a timed-out kubectl read (exit 124) stops the step under set -e rather than masking as an empty-credential mc alias set further down. The readonly-upload deny probe branched on mc directly (if mc cp ...), which suspends set -e, so a timeout there (exit 124) was indistinguishable from the access-denied non-zero the probe expects: a hung deny path would exit 124, take the false branch, and reach a green cleanup, the exact window this change bounds. Capture the status instead: exit 0 is the readonly regression, 124 is a timed-out probe, both fail; any other non-zero is the expected denial. The suite's own catch read (the `kubectl get pods` in namespace cozy-objectstorage-controller) carried neither an op timeout nor an inner cap, so it inherits timeouts.exec (5m) and can hold the pipe the same way; give it a 30s inner cap and a 1m op timeout so the rule the rest of the file follows holds for it too. The .chainsaw.yaml notes still read the op ceiling as a reliable cut, the old SIGKILL as a de-facto cap and a smaller timeout as guaranteeing a kill; correct them, since the shell-only kill plus the pipe wait are why an inner cap on every read is required rather than optional. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>	2026-09-22 15:16:25 +03:00
Aleksei Sviridkin	92d2eede52	test(opensearch): separate the mount-plan rewrite from the roll onto it (#4279 ) ## What this is `Refs #4259`. A diagnosis of the opensearch suite timeout plus the one in-repo change it warrants: making the failing step say which of its two candidates actually happened. It does not claim to fix the root cause — see "What this does not do". ## What the run actually showed The `verify-statefulset-rolled-onto-the-new-mount-plan` assert erred with exactly two field failures, both under `status`: `(currentRevision == updateRevision): false` and `updatedReplicas: field not found`. No volume field appears in the error list, and `readyReplicas: 1` shows as an unchanged line, so the pod was Ready at the deadline and nothing had replaced it. Per e2e-testing.md ("Reading a failure diff: the actual side is a projection"), the real cause is the error line, not the diff — and a Chainsaw diff projects the live object onto the expected fields, so the three `http-ca` / `http-cert` / `http-key` lines showing in the diff are the expected tree, not proof the volumes are absent. Read that way, the volume assertions matched: the three volumes were present on the live template. The "volumes missing" reading is the projection trap the doc warns against. ## Which candidate Candidate (b): the template was rewritten onto the subPath mount plan, and the operator has not restarted the pod onto it. This is consistent with the source, traced against opensearch-k8s-operator v2.8.0 as vendored: - The chart renders the CR correctly — `generate` dropped, `secret` and `caSecret` named — proven by `packages/apps/opensearch/tests/tls_upgrade_test.yaml`. - Given that CR, `pkg/reconcilers/tls.go` `handleHttp` appends exactly the three volumes the step asserts (`http-ca` from `opensearch-test.http-ca`, `http-key` and `http-cert` from `opensearch-test-http-server`), and `pkg/reconcilers/cluster.go` `reconcileNodeStatefulSet` writes that template onto the StatefulSet. Three branches return ahead of that write — no node-pool config hash, the `opensearch.role` recreate, and parallel recovery — but the parallel-recovery gate is `ReadyReplicas < replicas-1`, unreachable at one replica, so the write happens. The reconcilers that run ahead of the cluster reconciler do not stop it either: the TLS reconciler runs first and returns the admin-certificate path's result, empty in the ordinary case, and `securityconfig.Reconcile` short-circuits to nil when the security-config checksum is unchanged, which the move does not change. So the operator does write the volumes. - The nodePool StatefulSet is `OnDelete` (`pkg/builders/cluster.go` sets `UpdateStrategy.Type: OnDelete` alongside `PodManagementPolicy: OrderedReady`), so the StatefulSet controller never deletes the pod to apply a template change: `podManagementPolicy` orders creation and deletion during scaling, `updateStrategy` decides the update. The roll is the operator's rolling-restart reconciler's to drive (`pkg/reconcilers/rollingRestart.go`) — it deletes the pod itself. It needs `status.Initialized`, it needs `ReadyReplicas == replicas` (a Ready pod is a precondition, not the blocker), then it opens a client to the OpenSearch API and calls `CheckClusterStatusForRestart`, which holds the restart while the cluster is red; on yellow the gate now enables shard allocation, waits for every data node to join, waits for shard activity to settle, then proceeds (#4317 deleted the old `.opensearch-observability` carve-out, and this suite builds the operator with that patch). Any of those gates can sit indefinitely. `status.currentRevision` never advances by itself under `OnDelete`; the operator patches it once `updatedReplicas == replicas`, so the second assert's own condition cannot go true unless that reconciler ran to the end. - The pod's startup probe is a TCP check on 9200 (`builders/cluster.go`), so "connection refused" is OpenSearch not listening, not a probe-scheme mismatch. The plaintext `GET / HTTP/1.1` from `curl/8.5.0` on the TLS listener is neither the startup/liveness probe (TCP) nor the readiness probe (`curl -k https://localhost:9200`, localhost, HTTPS) — it is external traffic to the LoadBalancer address, a red herring for the stall. ## What this changes The step now runs two asserts instead of one. The first waits for the operator to rewrite the template onto the three volumes, on a reconcile-scale budget (6m) — a failure here is the operator not writing them (candidate a). The second waits for the roll to complete onto that template, on the pod-lifetime budget (12m) — a failure here with the first green is the roll not completing (candidate b): the operator never restarting the pod, which the rolling-restart reconciler holds while `CheckClusterStatusForRestart` reports the cluster red — on yellow the gate enables shard allocation, waits for every data node to join, waits for shard activity to settle, then proceeds — or a replacement that was deleted but never came Ready. The second assert carries the three volume projections and `observedGeneration == metadata.generation` as well, so the roll is pinned to the template the first assert checked rather than to whatever revision is current when the status converges. The suite `catch` also captures the StatefulSet as YAML and as a describe, and the operator's own log, because `updateRevision`, `currentRevision`, `generation` and `updatedReplicas` are what separate the two halves — describe alone omits them — and the restart decision is logged in the operator, not in the cluster pods. The reconciler logs `Starting rolling restart of the StatefulSet` (`rollingRestart.go:155`) once its early gates (`Initialized`, ready-replica, update-pending) pass; it builds its OpenSearch client just before, at `:138`, and that client pings the cluster — so a run whose log lacks `Starting rolling restart` either had an earlier gate hold it silently or had the client ping fail loudly, not only the former, while that line present followed by a `Couldn't proceed with rolling restart ... because <reason>` line names a health-stage hold (the health gate is `CheckClusterStatusForRestart`, and it is not the last: `PreparePodForDelete` follows it). The operator-log `tail` was raised so it spans the whole assert window rather than a third of it. ## What this does not do It does not fix the stall; #4317 does, and is already merged. The run that captured this (`Run E2E tests`, job 104576486900) shows the rolling-restart reconciler holding on health: the manager logged `Starting rolling restart of the StatefulSet opensearch-test-nodes` (`rollingRestart.go:155`, printed only after the `Initialized` and ready-replica gates passed) and then, every ten seconds to the deadline, `Couldn't proceed with rolling restart ... waiting for health to be green` (`os_data_service.go:180`), which is reached only after `GetFlatClusterSettings` returned 2xx (`os_client.go:224` makes anything else an error). So the operator reached the cluster and the cluster answered; health held the restart. #4259 established why, and the CA move plays no part: on a single data node the bootstrap node leaving is audited into a fresh `security-auditlog-` index with one replica, that replica has nowhere to go, the cluster stays yellow for good, and the v2.8.0 restart gate opens only on green (or on a yellow `.opensearch-observability`) so it never opens. #4317 backports opensearch-project/opensearch-k8s-operator#1569 to fix that gate; this PR is only the diagnostic split that names which half of the roll a failure is. The split content itself is not the failure: on `49f3f059f` both asserts passed, the second after 1m47s, and this change touches only the step's comments and the operator-log tail, not the assertions. With this change in place, a future occurrence names the half, and the catch carries the roll's own fields and the operator log, so the crust-gather can be read against a specific claim rather than an ambiguous one. ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit Tests * Improved OpenSearch StatefulSet failure diagnostics by capturing YAML, detailed resource descriptions, and the latest operator pod logs. * Expanded rolling-restart verification to check volume projections, template updates, rollout readiness, observed and metadata generations, and revision consistency. * Separated template validation from rollout completion checks with dedicated six- and twelve-minute time limits. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-21 20:44:23 +04:00
IvanHunters	c4f72325a6	fix(test): pass over the flux defaults check instead of aborting the suite hack/cozytest.sh is not bats and defines no `skip`, so the guard for a missing flux killed the whole file with `skip: not found` and kept the unit job red. A red checks job also cancels finalize and e2e, so the recipe this suite exists to hold never ran in CI at all. The form the rest of the suite already uses returns 0 instead, as hack/release-changelog-behaviour.bats does for a missing node. Verified by mutation: with flux installed, dropping a pattern from FLUX_DEFAULT_IGNORES still turns the test red, so the early return does not make it vacuous. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>	2026-09-21 19:20:48 +03:00
scooby87	6677c24beb	fix(ci): fail the trailer check on an empty commit range (#4278 ) ## What `hack/check-commit-trailers.sh` printed `Commit trailers OK: 0 commit(s) checked.` and exited 0 when the range it was given resolved to zero commits, so a run that examined nothing was indistinguishable from a clean run. An empty range now prints a message naming the empty case and exits 1. ## Why The documented pre-push recipe in `docs/agents/contributing.md` is `hack/check-commit-trailers.sh origin/main..HEAD`. The range is resolved in the process working directory, so an agent whose shell sits in a different checkout than the branch resolves it against the wrong `HEAD` and gets nothing — and the old green there was a green nobody could tell from a real one, defeating the check precisely when the checkout is misconfigured. The CI path is unaffected: `.github/workflows/pre-commit.yml` builds the range from a pull request's merge base and head SHA, which GitHub refuses to open with no commits between the two, and which goes empty only once the base has absorbed the head's commits and there is nothing left to merge, where a red trailer check is harmless. ## Test TDD, bats: the previously buggy `accepts an empty range` test is inverted into `fails on an empty range instead of reporting OK`, which asserts a non-zero exit, that the message names the empty case, and that the `Commit trailers OK` line is absent. Mutation-verified — removing the guard turns it RED, restoring it turns it GREEN. Full suite: 28 passing, 0 failing. ## Release note ```release-note Local runs of `hack/check-commit-trailers.sh` now fail on an empty commit range instead of reporting OK, so the documented pre-push check no longer passes silently when it examined nothing. ``` Fixes #4263 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Bug Fixes - Commit trailer checks now fail when the specified commit range contains no commits. - Empty ranges no longer report a misleading successful validation. - Error output clearly indicates that no commits were checked. - Tests - Added coverage confirming empty commit ranges return a failure status and appropriate diagnostic output. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-21 16:12:39 +03:00
myasnikovdaniil	a1b32f2e5a	ci(workflows): build Talos only when its sources changed (#4021 ) Stacked on #4020, not independently mergeable. Nothing on the same-repo PR path consumes either half of what `build-talos` produces. `image-talos` pushes `${REGISTRY}/talos:pr-N-sha`, which no chart in the tree references and which is not even version-tagged here (`PUBLISH_VERSIONED` and `PUBLISH_FLOATING` are both 0). `image-matchbox` stamps `packages/extra/bootbox/images/matchbox.tag`, and that one line is the whole content of `pr-patch-fragment-talos` - checked against the artifact from a real run - but bootbox is opt-in and the E2E platform Package does not enable it, so no run pulls the image. Unstamped tag leaves installer artifact pointing at released matchbox, which is a valid ref nothing on this path resolves. So the job ran a privileged siderolabs imager build on every PR for two artifacts with no reader. Now it runs when the PR touched `packages/core/talos`, and `finalize` accepts a skipped result as well as a successful one. Without that second half skipping it would block finalize, and therefore e2e, on nearly every PR. Forks keep the unconditional build, and the reason is not about talos. `e2e-fork.yaml` publish job requires at least one OCI archive to exist, an invariant that holds today only because this job always ran, so a fork PR touching nothing under `packages/` would arrive with an empty export and fail a legitimate run. Teaching that guard what `plan` expected to build is work in the fork allowlist path and belongs in its own change. Saving is about 1.5-2 min of critical path. Main point is removing a privileged image build that nobody reads from every pull request. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - CI Improvements - Pull request validation now runs Talos builds only when relevant changes require them, while retaining full validation for fork-based contributions. - Finalization correctly handles skipped Talos builds and preserves generated boot artifacts when builds succeed. - Changed-file detection now accounts for renames, deletions, and shared build inputs. - Documentation - Updated testing and build guidance to reflect the current validation behavior. - Tests - Added comprehensive coverage for build classification, workflow outputs, skipped jobs, and artifact handling. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-21 14:52:53 +05:00
Myasnikov Daniil	1118b340a7	fix(ci): teach the overlay to repair an unstamped images/.tag ref hack/overlay-main-images.sh walks images/.tag, but every branch of its line classifier needs either a YAML key or an @sha256:. A .tag file holds one bare reference and nothing else, so until a build stamps it the line has neither, the drift guard reads it as a non-ref change, and the committed reference survives. For a package onboarded with the :v0.0.0 placeholder this repo already uses for a new image, that surviving reference is one no registry serves. The overlay is the only thing that would replace it for a PR that does not rebuild the package, so once such a package is on main every full-suite run that skips its build keeps the placeholder. The same placeholder inside a values.yaml repairs itself, because there it sits under an `image:` key the classifier recognises: the difference is the file shape, not the placeholder. The gap is latent rather than new. 17 of the 19 .tag files in the tree carry @sha256: and pass on that branch alone; the only other digest-less one, extra/seaweedfs/images/seaweedfs-cosi-driver.tag, is third-party, so build-main never rebuilds it, the artifact copy is byte-identical and the classifier is never reached. The first digest-less first-party .tag is what turns it live. hack/promote-rewrite-tags.sh already carries a second match branch for this shape and states why. This is that branch, scoped to *.tag so an unkeyed line in a values.yaml still counts as drift. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>	2026-09-21 14:42:41 +05:00
Alexey Artamonov	7fd8376ccb	fix(ci): fail the trailer check on an empty commit range The pre-push recipe resolves origin/main..HEAD in the process working directory, so an agent whose shell sits in a checkout at or behind the base resolves it to zero commits and the check printed "Commit trailers OK" with exit 0. A run that examined nothing was indistinguishable from a clean one, defeating the recipe exactly when the checkout is wrong. Treat an empty range as a named failure so the misconfiguration is loud; the CI path is unaffected because it builds the range from a pull request's merge base and head, which is empty only once nothing is left to merge. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>	2026-09-21 12:20:22 +03:00
Alexey Artamonov	8ef230f0bf	Merge remote-tracking branch 'origin/main' into kafka-multi-instance Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> # Conflicts: # docs/storage-immutability.md # packages/apps/kafka/tests/topics_test.yaml # packages/system/kafka-rd/cozyrds/kafka.yaml	2026-09-21 12:05:33 +03:00
Aleksei Sviridkin	8799a4e4aa	fix(cluster-api): backport live migration before host eviction (#4291 ) ## What this PR does CAPK v0.1.10 handles host eviction by draining and deleting the guest VMI. It also looks up the physical host name in the guest cluster, and a recreated guest can remain cordoned. Backport CAPK #374, #389 and #392 onto the current provider API and runtime, with separate bounded submission, active migration and fallback windows, and ordinary VMIM objects without a priority, a field the pinned `kubevirt.io/api` v1.5.0 does not carry. The submitted VMIM deadline survives reconciliation; expired, invalid or legacy marker values enter the existing fallback path. Add the manager build to the package pipeline, read its digest pin through the provider imageUrl override, and ship VMIM create permission in the compressed provider manifest. Source tests and four packaging checks pass, including failed-build pin preservation and the rendered ConfigMap payload. The regression tests include unprocessed submissions, legacy and invalid deadline values, repeated reconciles, and a fresh active migration window after submission expiry. The updated manager also cross-compiles for linux/amd64 with Go1.26.4. The previous OCI build predates the submission-deadline correction and does not validate an image containing this change. The updated source still needs the maintainer-owned image build described below. The checked-in pin still identifies the original upstream image, so installing this source chart directly does not enable the backport. The regular main and release builds now include this package, stamp the built image digest, and package that exact artifact; the release workflow also commits the generated pin. Pre-merge E2E validation needs a maintainer-owned build because fork publication only allows image repositories already present in the base tree. ### 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(cluster-api): attempt live migration before drain/delete on External eviction, use the guest node name for fallback drain, and uncordon recreated guests. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - Added a Cluster API Provider KubeVirt image pinned to version 0.1.10. - Added migration-first workload eviction with fallback draining and node recovery. - Added permission to create virtual machine migration resources. - Included the provider image in builds and deployment templates. - Bug Fixes - Preserved migration deadlines across retries and improved recovery after interrupted or completed migrations. - Added a bounded wait for migrations before falling back to draining. - Documentation - Documented the provider image version, patches, build process, and release considerations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-18 22:23:10 +04:00
Aleksei Sviridkin	ee09e73343	fix(kafka): make topics[].config optional (#3935 ) ## What this PR does Makes `topics[].config` optional across the Kafka typed API, generated schema, documentation, and chart contract. The KafkaTopic template already omits an empty config, and Strimzi accepts the field as optional; the Cozystack schema was the layer incorrectly rejecting such topics. The Helm test renders a topic without config and asserts `spec.config` is absent. The Chainsaw fixture also omits config and waits for the resulting KafkaTopic to reach Ready. Go API compatibility: `Topic.Config` changes from `runtime.RawExtension` to `runtime.RawExtension`, so Go callers constructing that field must pass a pointer. The Terraform provider intentionally does not model Kafka topic tuning (`config`), so this does not require a provider follow-up. Validation: - `helm unittest packages/apps/kafka`: 36/36 passed. - `go vet ./kafka` in `api/apps/v1alpha1`: clean; the package has no test files, so this shows it compiles. - root `make generate`, which owns `zz_generated.deepcopy.go`, and `make -C packages/apps/kafka generate`: no diff. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map against all nine changed files. The provider's Kafka model deliberately excludes `topics[].config`; no downstream repository mirrors this nested field or its requiredness. - [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(kafka): allow topics to omit the optional Strimzi config map ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features* - Kafka topic configuration is now optional. Topics can be created without explicitly providing configuration settings. - Topic configuration is omitted from serialized resources when not specified. - Bug Fixes - Improved handling of topic resources without configuration, including correct readiness validation and rendering. - Documentation - Updated Kafka configuration documentation and schemas to reflect that topic configuration is optional. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-18 21:29:50 +04:00
Alexey Artamonov	8fbec92411	test(hack): match rewrite diagnostics literally in the mirror test The two assertions checking the dry-run host-rewrite diagnostic used `grep -q` with patterns carrying the registry hostnames `iad.ocir.io` and `ghcr.io`. The dots are BRE wildcards, so the assertions pass on a diagnostic whose host bytes are corrupted (`iadXocir.io` matches), which is exactly the failure the test exists to catch. Switch both to `grep -Fq` so the host strings are compared literally. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>	2026-09-18 17:45:21 +03:00
Alexey Artamonov	a9dd898684	fix(hack): correct the nightly-mirror dry-run diagnostic and BSD-sed comments The --dry-run line advertised `sed -i 's\|...\|g'`, an idiom the real path stopped using once the edit moved through a temp file, and it printed only the first of the two expressions the rewrite applies -- understating what the run touches and re-describing a command the script no longer runs. Report the rewrite by intent instead, covering both the `<host>/` prefix and the bare-host scalar. Its test asserted the old `s\|...\|g` string, so it encoded the same stale idiom; retarget it at the new message and guard against `sed -i` reappearing in the plan. Also tighten two comments that overstate what BSD sed does: the `\?` note claimed the quantifier merely fails to match a bare host, but `["']\?` reads as "a quote then a literal ?" so the whole expression is dead on macOS, matching neither shape; and the `cat`-over-temp-file note now records that `cat` truncates before it writes, an interrupt-window trade taken to avoid `mv` stamping mktemp's 0600 onto rewritten files. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>	2026-09-18 17:45:21 +03:00
Alexey Artamonov	cb08472c98	fix(hack): make in-place sed edits portable to BSD sed nightly-mirror.sh and promote-rewrite-tags.sh, and the fixture builder in promote-rewrite-tags_test.bats, rewrite files with `sed -i`. BSD sed reads the argument after -i as a mandatory backup suffix, so `sed -i -e` and `sed -i -E` consume the following flag as that suffix: the edit either errors or silently no-ops, leaving `make unit-tests` red on macOS while CI stays green on GNU sed. Route each edit through a temp file instead. The nightly-mirror rewrite also relied on the GNU `\?` optional quantifier, which BSD BRE reads as a literal `?` so a bare host never matched; replace it with the POSIX `\{0,1\}`, equivalent on both seds. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>	2026-09-18 17:45:10 +03:00
Alexey Artamonov	0473079448	fix(hack): parse non-ASCII @test titles under BSD awk cozytest.sh rebuilds each test's function name by walking its @test title one character at a time with awk substr(). BSD awk aborts that walk on the first multibyte byte instead of returning it, so a file carrying a non-ASCII title never parses and none of its tests run -- silently, since the suite only reports that the runner errored. Four hack/*.bats files carry such a title today, and the gap is invisible to CI because GNU awk does not abort on the same input. Pin LC_ALL=C on all three awk passes -- the title parse, the run selection, and the exit-code scan of the captured log -- so each reads the input byte-wise. The parse and selection passes must share the ctype or they would derive mismatched function names for one title. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>	2026-09-18 17:45:10 +03:00
Alexey Artamonov	324a342e61	test(seaweedfs): make the BSD-date shim honor the audit's real format string The two end-to-end tests that reproduce issue #4273 on a GNU runner drive the audit through _write_fake_date, a `date` shim that rejects the GNU `-d` spelling so the script must take its BSD `date -j -f` branch. But the shim discarded the format argument the script passed and recomputed the epoch with its own hardcoded `%Y-%m-%dT%H:%M:%S`. So the one string those tests exist to protect -- the audit's BSD format spelling, which is the whole of #4273 -- was never actually exercised: a regression in it left the tests green. The only test that hits a real BSD `date -j -f` short- circuits on the GNU branch on the CI runner, so the gap was invisible exactly where the gate runs. The shim now captures both the format and the value and uses the format the audit passed. On a BSD host it honors it through real `-j -f`, so a wrong format fails as production would; on a GNU-only runner, which has no `-j` to enforce a format, it falls back to `-d` only for the exact format the audit is supposed to pass, so a regressed spelling is not silently rescued and the tests turn red on the very runner they guard. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>	2026-09-18 17:42:45 +03:00
Alexey Artamonov	6aa4ca954a	fix(seaweedfs): parse naming-audit timestamps portably across GNU and BSD date The naming audit read PV and first_deployed timestamps with `date -u -d`, a spelling only GNU date accepts. On a host with BSD date the call failed, its stderr was discarded by the `2>/dev/null` on the same line, and `\|\| return 0` turned that host-tool failure into the audit's "no epoch" signal -- so every tenant read as MIXED with "direction cannot be established", as if the cluster lacked the evidence rather than the host lacking a readable clock. Since the audit gates a destructive runbook step, that indistinguishability is the defect. parse_epoch now tries the GNU spelling then the BSD one (`date -j -f`) and uses whichever the host speaks, so both call sites work on GNU and BSD hosts alike with no behaviour change for a valid timestamp on GNU. A timestamp that is present but readable by neither form is no longer folded into the absent-evidence branch: it is fatal and names the value, keeping a genuine parse failure loud and distinct from the legitimate "no bound PV age" case (unbound claim, NotFound PVC/PV) the audit conservatively degrades on. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>	2026-09-18 17:42:45 +03:00
Yan Bondarenko	0e4d6a305c	test(cluster-api): declare the subshell EXIT traps in the CAPK bats suite hack/bats-no-exit-trap.bats is an exact ratchet, and the new suite installs three traps inside explicit subshells without declaring them, so make unit-tests fails on the merge commit. The header now declares the count and says why the three are not debt. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>	2026-09-18 17:30:13 +04:00
Yan Bondarenko	c8f4906e93	fix(cluster-api): backport live migration before host eviction Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>	2026-09-18 17:30:13 +04:00
Yan Bondarenko	8ec76a3614	fix(kafka): make topics[].config optional Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>	2026-09-18 17:30:06 +04:00
Aleksei Sviridkin	88a7ea0e3c	fix(kubeovn): retry VM migration setup after scheduling (#4253 ) ## What this PR does Kube-OVN can drop VM migration setup when a migration enters `Scheduling` before the target Pod is visible or the source or target node is known. The phase may remain unchanged afterward, leaving the target Pod waiting for network setup. Backport the final fix from kubeovn/kube-ovn#6834 to the pinned v1.15.10 image: return an error for the transient missing-node case so the existing rate-limited queue retries it. Keep the same-node behavior unchanged. That backport can be removed when updating to v1.15.14 or later. Add a separate retry guard for the target-Pod creation race. It returns an error when the informer cache has no target Pod yet, instead of forgetting the migration event. This guard is not part of #6834 and must remain until an equivalent fix is verified in the selected upstream version. Also include the existing kubeovn image target in the root build list. Without it, a kubeovn patch produces an empty PR build matrix and the image is omitted from the regular build and release paths. Validation: - All six regression cases pass on Linux/amd64 using the real controller workqueue. The new missing-target-Pod case fails without its guard and passes after the Pod appears in the informer cache, without changing the VMIM phase. The existing deferred-node, already-scheduled, and same-node cases continue to pass. - All 19 build-matrix tests pass; the full matrix includes kubeovn exactly once. - Both lint steps, pinned golangci-lint v2.10.1 and upstream `modernize -test -skipgenerated ./...`, pass for the Linux/amd64 target using Go 1.26.4. On macOS, modernize is built for the host and run with the Linux target environment, since `go tool` otherwise tries to execute a cross-compiled Linux binary. - All three packaged patches apply cleanly to pinned v1.15.10 with `git apply --check --whitespace=error`; the resulting source matches the tested tree. Formatting and source/build whitespace checks pass. The regression test covers controller queue behavior; the standard container E2E environment does not cover DRBD-backed live migration. The regular image build applies the backport and stamps the resulting digest before packaging. ### Screenshots Not applicable; no 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 change adds an existing image target to the build list; package layout, make target interfaces, chart values, APIs, and published image-reference formats stay the same. CCP discovers package image targets dynamically and does not carry the root build list. The website has no documented workaround for kubeovn/kube-ovn#6823 or #6834 to remove. ### Release note ```release-note fix(kubeovn): retry VM migration network setup when the target Pod or source/target scheduling information is not yet available, and include the Kube-OVN image in the regular build matrix. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Bug Fixes - Improved VM migration handling when source or destination node scheduling information is temporarily unavailable. - Migration events are now retried after scheduling completes instead of being dropped. - Preserved existing behavior when migration source and destination nodes are identical. - Tests - Added coverage for migration scheduling and retry scenarios, including partially assigned and same-node cases. <!-- end of auto-generated comment: release notes by coderabbit.ai -->	2026-09-18 17:29:45 +04:00
Alexey Artamonov	36bbc62445	test(opensearch): separate the mount-plan rewrite from the roll onto it The verify-statefulset-rolled step folded two outcomes with two owners into one assert: that the operator rewrote the nodes template onto the subPath mount plan, and that the StatefulSet finished rolling onto it. When the roll stalled, the timeout could not say which half did not happen. The failure surfaced only as the status mismatch, while the volume projections in the diff read as absent even though they were present -- the projection trap e2e-testing.md documents, where the "actual" side of a Chainsaw diff is a projection and the real cause is the error line above it, not the diff. Split into two asserts so each candidate fails on its own line: the template rewrite on a reconcile-scale budget, the roll on the pod-lifetime one. The nodePool StatefulSet is OnDelete (opensearch-k8s-operator v2.8.0), so the roll is the operator's rolling-restart reconciler's to drive, not the StatefulSet controller's -- it deletes the pod itself, and currentRevision advances only once that reconciler patches it. A failure on the first assert is the operator not writing the volumes; a failure on the second with the first green is the roll not completing. The catch captures the StatefulSet as YAML and as a describe and the operator's own log, because updateRevision, currentRevision, generation and updatedReplicas are what separate the two halves -- describe omits them -- and the restart decision is logged in the operator, not in the cluster pods. The roll-gate comment ties each log signature to a v2.8.0 source line: the reconciler builds its OpenSearch client at rollingRestart.go:138, before it logs "Starting rolling restart" at :155, and that client pings the cluster, so the absence of the log line means an early gate held silently or the client ping failed loudly -- both, not just the first. The stall itself is fixed in #4317: a single-replica cluster is left permanently yellow by an index whose replica cannot be assigned. This change only makes the suite name which half of the roll a future failure stalled on. Refs #4259 Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> Assisted-by: LLM	2026-09-18 14:58:00 +03:00
Timofei Larkin	c398e0e116	Merge branch 'main' into chore/drop-http-cache main took migration 57 and moved migrations.targetVersion to 58 while this branch was open, so that line is the one conflict: 57 at the merge base, 58 on main, 59 here. Resolved as 59, which is what this branch's migration 58 requires, since run-migrations.sh loops up to targetVersion - 1. Merged rather than rebased because the branch carries commits that are not mine to rewrite. Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>	2026-09-18 13:20:03 +03:00

1 2 3 4 5 ...