mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 22:13:18 +09:00
3630842172efffa0ff4a44fcbb7c373da753a89b
915
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 IvanHunters
|
c0fcdd767c |
fix(migrations): discover pre-chartRef parents via inline spec.chart in the restamp
The restamp discovered unlabelled parent kubernetes releases by spec.chartRef only, but that misses the population it exists for. The apps.cozystack.io/ application.kind label was introduced 2025-12-19 ( |
||
|
IvanHunters
|
c6b6755eec |
fix(migrations): record foreign-owner skips, harden record_skip, scope restamp to kubernetes- names
Three hardening fixes on migration 56's new code: - The foreign-owner branch in adopt_one pins the object prune-proof but does not adopt it (the child release can then stay Failed on the ownership conflict), yet it was the one warn+pin+skip path that did not record itself in the durable unadopted ConfigMap -- so an operator relying on that record to find stuck pools would miss it. Record it like every sibling branch. - record_skip ran a bare append under set -e inside skip branches that are designed never to abort; a failed write (e.g. /tmp full) could deadlock the fleet upgrade for one tenant. Make it fail-open, matching the branches' intent. - The restamp selector matched any release by chartRef; scope it to names starting with kubernetes- so it cannot mislabel (and then have the sweep skip) a non-standard release, keeping restamp and the sweep's contract in sync. Also fix the previously-vacuous negative restamp test (its fixture lacked the chartRef, so it never reached the label guard it claimed to check). Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
3d1a83f419 |
fix(migrations): make the unadopted-pools record publish best-effort
The durable-record ConfigMap write sat under set -euo pipefail right before the version stamp, so a permanent failure (e.g. the record exceeding etcd's object size limit at a very large skip count) would abort the migration and deadlock the fleet upgrade with no path out -- even though every skipped pool was already pinned prune-proof earlier in the sweep, so the workers are safe regardless of whether the record lands. The record is a convenience artifact, not a safety mechanism, so swallow its failure with a warning instead of failing closed. Covered by a bats case that fails the publish and asserts the run still stamps. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
952901776a |
fix(migrations): adopt unlabelled parent kubernetes releases and record unadopted pools in migration 56
The worker-adoption sweep selected parents by the apps.cozystack.io/ application.kind label, which postdates v1.2. A parent kubernetes HelmRelease older than that, or one restored by hand, lacks the label: the sweep skipped it, stamped, and the parent's control-plane-only re-render then pruned its un-pinned workers with no pin and no warning. Discover such releases by their chartRef instead and restamp the three application labels before the sweep (additive, cannot corrupt a release, and repairs it for cozystack-api too). Every warn+pin+skip branch leaves a pool pinned prune-proof but unadopted, recorded only in the Job log, which the next upgrade reaps; the migration does not re-run, so an operator cannot find those pools afterward. Accumulate the skips and publish them as configmap/cozystack-migration-56-unadopted before the stamp. Extend the fake kubectl and add bats coverage for the restamp (positive and negative) and the unadopted-ConfigMap paths. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
05c967bcaa |
test(migrations): move the worker-pool adoption bats to migration 56
The main merge renumbered the Phase 2 worker-pool adoption migration from 54
to 56 (behind main's redis-adopt 54 and fluxcd-orphan 55), but its bats and
fakebin did not move: hack/migration-54-adopt-{path,values}.bats still pinned
MIG at migrations/54, which is now the redis-operator adoption, so both went
red on "RedisFailover CRD not present; nothing to adopt".
Rename the two bats and hack/testdata/migration-54 to 56, repoint MIG/FAKEBIN
at migration 56, and update the version references (stamp 55 -> 57). The bare
migration-54 testdata name also collided with the migration-54-redis and
migration-55-fluxcd siblings; 56 disambiguates it.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
f035da8a0f |
Merge branch 'main' into feat/kubernetes-nodes-phase2b
Resolve conflicts introduced by main: - Drop the FluxCD tenant addon (API type, deepcopy, values schema, RD) in favour of the GPU type main added; main removed the fluxcd addon wholesale. - Renumber the Phase 2 worker-pool adoption migration 54 -> 56, behind main's redis-operator (54) and fluxcd-removal (55) migrations; targetVersion 55 -> 57. - Accept the branch's removal of the in-chart talos-reconcile Job; it moved to the kubernetes-nodes chart, main only touched its comment. - Regenerate kubernetes values.schema.json / README and the kubernetes-rd ApplicationDefinition schema. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Andrei Kvapil
|
851a86dcf1 |
feat(platform): orphan tenant FluxCD HelmReleases on upgrade
Add migration 54 for the removed addons.fluxcd feature. For every tenant that had it enabled, the FluxCD HelmReleases in the management cluster are suspended, stripped of their Flux finalizer and deleted, so the delete does not trigger a Helm uninstall inside the tenant cluster. The Flux release already running in the tenant keeps running untouched. Runs as a pre-upgrade hook, before the addon-less Kubernetes chart reaches the tenant HelmReleases. Mirrors the orphan idiom of migration 29. Bumps migrations.targetVersion to 55. Fail closed. Dropping the finalizer is the only thing that keeps the delete inert, and migrations never re-run, so the script runs under `set -euo pipefail` with no `|| true` on the critical path and gates the delete on the finalizer actually being gone. The verify read uses `--ignore-not-found` rather than `2>/dev/null || true`, so a failed read aborts instead of reading as an empty finalizer list that passes the gate on an object whose state was never checked. A migration that stops is recoverable; one that half-orphans a fleet is not. Cover it with a docker + fake-kubectl unit test running the real script by path inside the migrations image's own alpine base (busybox ash, jq baked on), so it exercises the shipped interpreter and load-bearing pipefail. The suite pins ordering and selection (only the addon HelmReleases are touched, each suspend -> finalizer-drop -> delete in order) and the fail-closed paths (a surviving finalizer, a failing verify read, and a failing fleet scan each abort before deleting and before stamping). Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Andrei Kvapil
|
7cca3d75e7 |
feat(kubernetes)!: remove the FluxCD tenant addon
The optional `addons.fluxcd` toggle of the Kubernetes app deployed a Flux Operator + FluxInstance into a tenant cluster. It was served by two vendored charts, packages/system/fluxcd-operator and packages/system/ fluxcd, which were the last AGPL-3.0 content in this Apache-2.0 repository and a governance blocker for CNCF Incubation. The management cluster does not use these charts (it runs classic Flux via internal/fluxinstall; migration 21 already removed the operator from it), and the addon defaulted to disabled. Rather than maintain an AGPL payload for an unused feature, drop it entirely: - delete both system packages and their platform source entries - remove the addons.fluxcd HelmReleases, values and generated schema from the Kubernetes app - drop the addons.fluxcd block still submitted by the e2e harness - drop the removed fluxcd package entries from CODEOWNERS BREAKING CHANGE: the addons.fluxcd option of the Kubernetes app is removed. Tenant clusters that had it enabled keep their running Flux release; see the companion migration for the upgrade path. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Aleksei Sviridkin
|
4385990869 |
feat(redis-operator): migrate from archived spotahome to freshworks-oss fork (#3406)
## What this PR does `spotahome/redis-operator` was archived on 2026-06-11 (read-only; last release v1.2.4, 2022). This migrates the operator to the actively-maintained **freshworks-oss/redis-operator** fork (v3.3.5, Apache-2.0). ### Why freshworks-oss It keeps the same `RedisFailover` CRD and API group (`databases.spotahome.com`) — so existing managed Redis is unaffected — and adds a native `spec.engine: Redis|Valkey` switch, letting managed Valkey drop its Redis-compatibility shim image and run the official `valkey/valkey` image directly. ### Changes - Re-vendor the chart from `oci://ghcr.io/freshworks-oss/charts/redis-operator` (v3.3.5), which carries the `engine`-aware CRD. - Build the operator image from freshworks-oss v3.3.5 source. - Re-target the label/annotation-preservation patch onto the freshworks Go module (`github.com/freshworks/redis-operator`). ### Verification - freshworks-oss source + our `labels.diff` patch compiles natively with Go 1.26.3 (`service` and `cmd` packages). - `git apply --check` of the re-targeted patch against v3.3.5 succeeds. - The operator chart renders with the cozystack-registry image override. - The image is built (freshworks-oss v3.3.5 source + our patch, multi-arch amd64/arm64), pushed to `ghcr.io/cozystack/cozystack/redis-operator`, and pinned by digest in `values.yaml`. - `hack/promote-retag_test.bats` and `hack/nightly-mirror_test.bats` pass with the pinned digest. ### Note for maintainers `packages/system/redis-operator` is not part of the automated `make build`; its image is built out-of-band and the digest is pinned in `values.yaml`. That image is already built and pinned in this PR; re-run `make -C packages/system/redis-operator image` only if you bump the operator version. ### Follow-up The cozystack/redis-operator fork carries cert-manager TLS work (cozystack/cozystack#2729) that freshworks-oss does not have yet; the plan is to upstream it to freshworks-oss so it lands in the shared maintained fork rather than a cozystack-only one. ### Release note ```release-note feat(redis-operator): migrate from the archived spotahome/redis-operator to the actively-maintained freshworks-oss/redis-operator fork (same RedisFailover CRD, adds native spec.engine: Redis|Valkey) ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for injecting additional environment variables into the Redis Operator (including a default operator group id). * Added default configurable labels for the Redis Operator. * **Bug Fixes** * Preserves and merges existing Service labels and annotations during Redis Operator updates. * **Chores** * Updated the Redis Operator and Helm chart to the Freshworks-maintained 3.3.5 release, aligned chart/image sources, and simplified Helm chart retrieval. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
68d83cb686 |
test(e2e): add a runner-layer fixed-work canary around the node-join wait (#3919)
## What this PR does The diagnostics around the node-join wait already carry counter pairs that report time in the CPU rows, events in the KVM exits, and instantaneous values and running maxima in the files beside those. None of them is a fixed quantity of work, so none can separate a machine that was given fewer turns from a machine that took its turns and got less done with them. This adds a collector that runs two fixed amounts of work on the runner VM and times them: an integer loop and a streaming write, each under a ceiling of its own. It takes one sample before the node-join wait and one after it, on the failing and the passing path alike. Each sample is an absolute reading rather than half a difference, so one alone is still readable against the expected ranges the capture prints, and the pair says whether a slowdown began inside the interval it brackets or was already there going into it. The collector sits outside the three counter pairs that bracket the same wait rather than among them, because it occupies a core while it runs, and a burn placed inside one of those intervals would be charged to the join window it is supposed to measure. A new bats suite of 57 tests covers the collector. The existing node-join guards were extended to price it against the phase budget and to keep the spend order documented in both kubernetes suites in step with the order the block actually runs. On the budget: the sample taken before the wait is bounded at two arms of the canary ceiling plus grace, so it can add up to fifty seconds ahead of the wait, and the guard prices it at exactly that, while the guard's total stays below an honest worst case because collectors it already priced can spend more than the figures it prices them at. This branch was reviewed extensively before it was opened, and what is known to be incomplete is written down rather than left implied: the words describing how each floor sits against its healthy band are marked in the code as written rather than derived, so they are the part that can go stale when a constant moves, and the phase budget residuals are tracked in the issue the comments beside them cite. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file. The diff is five files: the e2e diagnostics collector library, its two new and extended bats suites, and the two chainsaw suite definitions that document the spend order. The closest call is `cozystack/ccp`, whose skills gate on `hack/package.mk` and `hack/common-envs.mk` and drive `make generate`. Neither file is touched, nothing under `hack/` is moved or renamed, and no make target changes what it does: the new suite is picked up because `BATS_UNIT_FILES` already globs `hack/*.bats`, so the file list grows and the target's contract does not. Nothing else matches. No package is added, renamed or removed, so the website app lists are unaffected. No `values.schema.json`, version enum, default or `ApplicationDefinition` changes, so the Terraform provider is unaffected. `hack/e2e-prepare-cluster.bats` is untouched, so the node prerequisites that `ansible-cozystack` and `talm` restate are unchanged. `hack/update-crd.sh`, the `release.labels` convention and the chart source kinds are untouched, so `external-apps-example` and `cozyhr` are unaffected. No label, annotation or metric name changes, so `cozy-proxy` and `cozystack-telemetry-server` are unaffected. Minimum node and network requirements are unchanged, so `examples` is unaffected. - [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 test(e2e): the node-join diagnostics now time two fixed amounts of work on the runner before and after the wait, so a run that spent its wall clock without completing work is visible in the report. No user-facing change. ``` |
||
|
Aleksei Sviridkin
|
7ec5dff93a |
test(e2e): add a runner-layer fixed-work canary around the node-join wait
The counters the node-join failure path collects are read in whatever unit their source publishes: time in the CPU rows, events in the KVM exits, instantaneous values and running maxima in the files beside those. Not one of them carries a unit of work of its own, so a machine that spent every tick and got a fraction of the work done reads healthy in all of them. Add a collector that carries its own unit of work. It runs two fixed amounts of work at the runner layer and times each: a loop over a handful of integer-valued scalars, which pressure on the shared cache or the memory controller moves little, and a store stream through blocks larger than the last-level cache one core can allocate into, which is dominated by it. Fixed work gives each reading a scale of its own, so a red run can be read with no green one beside it, and the capture states the expected range and where the estimate comes from. Sampled where the three counter pairs already are, before the wait on every run and after it on both paths, but outside their brackets: it occupies a core rather than reading a file, and a burn inside one of those intervals would land in the interval each pair divides by: runner-kernel CPU time charged to the join window, and for the two counting the guests, a core taken away from what they count. Two arms, each under a twenty-second ceiling with the five-second kill grace behind it, so fifty seconds at worst against the 420s diagnostics budget. The budget guard derives that from the ceiling the source declares, and the phase-warning guard names the canary among the collectors that keep running when timeout is missing. An exit shaped like the ceiling is attributed to it only when the whole bound was on the clock: an outside kill produces the same status, and read by status alone it would publish exactly the finding this collector exists to detect. A reading below the range the legend states, and an arm the ceiling stopped before it could produce one, reach the job log from all three reporting sites, each naming where it was taken, the way a missing reading does from the two outside the diagnostics block. On the passing path the report that holds the figure is the artifact nobody downloads. Each floor is printed in the legend it comes from and sits where a bounded arm crosses it before the ceiling stops the arm, because a floor the ceiling reaches first never fires: the arm is cut off before its rate can fall that far, and every alert then arrives as a bound rather than as the slowdown the floor was meant to name. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
IvanHunters
|
cd72d81bb4 |
Merge branch 'main' into feat/kubernetes-nodes-phase2b
Reconcile the phase-2b split with main's configurable worker-health feature (MHC condition timeouts, maxNodeProvisionTime, podCpuLimit/ podCpuRequest), which landed on both charts after this branch forked: - kubernetes (parent): pool rendering stays removed; main's parent worker-pool additions do not apply post-split. - kubernetes-nodes (pool): absorb the feature; keep the directional parent-version skew guard and the immutable cluster/storageClass rules. KubevirtMachineTemplate content hashes are unchanged (render snapshot). - migration 54: carry podCpuLimit/podCpuRequest (KMT-hash inputs) and the cluster-wide readyUnknownTimeout/readyFalseTimeout/maxNodeProvisionTime into adopted pools, so adoption neither rolls nor resets a tuned pool. Covered by a new adopt-values bats case. - drop the parent chart's obsolete MHC-timeout suites; the kubernetes-nodes twins cover the feature. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Aleksei Sviridkin
|
4b50a3ceb4 |
fix(ingress-nginx): pull the protobuf-exporter from the project namespace (#3783)
## What this PR does
Points the `protobuf-exporter` sidecar at the project's registry.
The sidecar was still pulled from `ghcr.io/kvaps/...` while the
controller beside it — built from the same repository, pinned to the
same `v1.11.2` — already came from `ghcr.io/cozystack/...`. The
repository moved to the organisation some time ago and the image
reference did not follow. It is the last thing in the tree that ships a
Cozystack-owned component out of an individual's namespace, which is a
Required item on the Incubation checklist ("All project metadata and
resources are vendor-neutral").
The image is the same bytes: `v1.11.2` was copied into the organisation
namespace with the manifest digest preserved, so `sha256:6d9235a9…` is
unchanged here. Nothing is rebuilt and nothing is upgraded.
The version is deliberately not bumped. `v1.11.5` exists in the
organisation namespace, but the tag tracks the upstream ingress-nginx
version and the controller sits on `v1.11.2`; bumping only the sidecar
would split the pair.
The comment in `hack/promote-retag.sh` loses its `ghcr.io/kvaps/...`
entry, because that path now falls under the bullet below it — a
cozystack-org repository outside `$REGISTRY`
(`ghcr.io/cozystack/cozystack`). Selector behaviour is unchanged;
`hack/promote-retag_test.bats` passes, 11/11.
### Downstream repositories
Walked the trigger map in `docs/agents/contributing.md` against the
diff. The two files touched are a package `values.yaml` image reference
and a comment in `hack/promote-retag.sh` — no trigger matches. In
particular the `ccp` trigger is a move, a rename, or a change to what a
make target does; this is neither, and its anchors (`hack/package.mk`,
`hack/common-envs.mk`) are untouched.
- [x] No downstream repository is affected by this change
### Release note
```release-note
fix(ingress-nginx): the protobuf-exporter sidecar is now pulled from ghcr.io/cozystack instead of a personal namespace. Same image and digest; no version change.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Chores**
* Updated the protobuf exporter image source to the new container
registry location.
* Refreshed image promotion guidance to remove an outdated third-party
registry example.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
Andrei Kvapil
|
3197c48bc5 |
test(migrations): cover migration 54 redis adopt (label+pin, CRD probe fail-closed)
Drive migration 54 end-to-end against a fake kubectl in a jq-enabled build of
the migrations image's alpine base (busybox ash, the production interpreter),
mirroring the docker+fake-kubectl migration-test harness.
Pinned properties:
- an absent CRD stamps 55 without patching anything;
- a failing CRD probe aborts under set -euo pipefail before stamping (a
one-off apiserver blip must not read as "absent" and skip adoption);
- every RedisFailover across namespaces is labelled onto the freshworks
operator-group, while the engine is pinned to redis:6.2.6-alpine only where
the image is empty (both roles for Harbor; sentinel only where redis.image
is preset, which is left untouched; label-only when both are preset);
- a failed adoption patch aborts before stamping so the Job retries.
Auto-discovered by the Makefile hack/*.bats wildcard; no registration needed.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
|
||
|
Andrei Kvapil
|
caf9511365 |
fix(platform): CreateReplace the redis-operator CRDs on upgrade
helm-controller skips CRD upgrades by default, so an upgraded cluster would keep the old spotahome schema and prune the fork's spec.engine field on admission. The schema diff is additive, so CreateReplace is safe. Also refresh the harbor e2e wait comment for the freshworks operator. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Aleksei Sviridkin
|
ca058ad964 |
test(e2e): anchor the phase-warning guard on its own unique phrase
The guard located the missing-timeout warning by the first match of "timeout is not on PATH" in the shared library. The bracket-pair captures now write [bounds] notes carrying that same phrase earlier in the file, so the guard read a note as the warning and failed on every run, holding the unit gate red for all pull requests. The node-join copy of this guard was moved to the warning's own phrase for the same reason; this aligns the remaining copy. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
b1b8cbecc2 |
test(e2e): price the layers above the sandbox on the node-join window
Every CPU reading on this failure path has a subject inside the sandbox: a cgroup, a tenant worker's threads, a sandbox node's own /proc/stat. Each of them can say a guest was given its ticks while getting no work done, and none can say what the layers above spent. That is where this suite's own tax is paid: the sandbox nodes' guest time is charged to the runner kernel as user time, and whatever the runner VM loses to the machine hosting it is charged to nobody the sandbox can see. Two readings close that. /proc/stat is not namespaced, so reading it from the sandbox container reads the runner VM's kernel -- the only place steal on that layer appears, which is what separates a slow suite from a machine sharing a core with somebody else. The sandbox VMs are QEMU processes in this container's own PID namespace, so their threads are a local read too, and they say which node spent the kernel's user time and whether the three spent it evenly. Both are taken at the same two points as the KVM counters and for the same reason: the values are cumulative, so a rate needs a pair, and the interval worth measuring is the window the guest is failing to boot in. The thread reading runs the probe the worker capture already uses rather than a copy of it, since a second copy would drift exactly at the failure paths. Each carries the layer it describes in its own legend, because three kernels in this report publish a /proc/stat and two captures read QEMU threads, one layer apart in each direction -- and a reader who takes one for the other compares a hypervisor against its own guest. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
b76cb2091c |
test(e2e): record the runner VM shape and region in the e2e report
An e2e lane asks for a runner class, not for a machine. A flex shape is filled from whatever capacity the pool has at the time, so one lane can be served by two processor generations, and the region is a preference with a fallback. None of that reached the report, so a red run and a green one could not be compared by the hardware they ran on. The read is taken on the runner VM, before the sandbox container is created, because the metadata service answers on a link-local address the container has no guaranteed route to: an "unreachable" answered from in there would look exactly like a capture that had tried. It is wired into the sandbox apply target rather than into a workflow step, so every lane picks it up through `make prepare-env` on both the passing and the failing path, and cozyreport folds the file into the artifact from inside without going near the endpoint. The keys are fetched one at a time and the root instance document never is. That document carries the instance and compartment OCIDs, the image OCID, the display name, the hostname and the user metadata bag, and the report is a public artifact, so asking per key makes that property something the wire settles rather than something a filter has to be trusted for. It also matches where the shape sizing is published: it has its own subpath and is not in the root document at all. From the sizing object only the numbers are kept, so the rule holds for keys nobody has seen yet, and an answer that is not one scalar is named rather than printed. Every read carries a wall-clock ceiling, since a read that never returns costs the step its whole budget without reaching any exit status, and the first key doubles as the probe so a machine that is not an OCI instance pays one ceiling rather than one per key. Nothing here is fatal: this runs inside the one retried step in the job, so a runner with no metadata service leaves an "imds unreachable" line and an exit status of 0. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
0a1d2673d0 |
test(e2e): capture worker block IO and volume state on node-join failure
A worker that burns its whole vCPU while making no progress is either computing without result or waiting on blocks, and every counter this path already collects is blind to the second: the CPU families say what the group was allowed and what it got, the byte counters say what crossed the network, and neither counts a read. Read the Pod's block IO counters from the same kubelet endpoint the CPU ones come from, once rather than twice, because they accumulate from the container starting and a worker container starts with the guest it carries. Read the state of the replicated volumes behind the worker disks as well. A replica that is SyncTarget, Inconsistent or Diskless serves the guest's system disk from a peer over the network rather than locally, and the report collects the same tables only after the suite, by which time cleanup has deleted the volumes a red run would need them for. The guest's own /proc/diskstats would answer this from inside and cannot be reached: the credential minted for the guest carries the os:reader role, which is not allowed to read file contents, and Talos publishes no resource carrying disk statistics. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
c3247c0ca0 |
test(e2e): capture the sandbox kernel KVM exit counters across the node-join wait (#3886)
## What this PR does Every CPU reading this suite takes on a failing node-join describes a subject inside the sandbox: a cgroup, a QEMU thread, a sandbox node's own `/proc/stat`. All of them can report a guest that was handed its ticks while the guest gets no work done, and none of them can say what a tick cost. That is the state the node-join failures are in now, with quota untouched, throttling near zero, steal at zero and the sandbox host between a third and two thirds idle, while the guest does several times less work per core than the same image does on a run that passes. The price is paid one layer down. A tenant worker is a guest of a sandbox node, which is itself a guest of the runner, and the runner's kernel absorbs the exits, nested page faults and TLB flushes that the arrangement provokes. That kernel publishes the totals in debugfs, and the suite runs in a container sharing it, so this is a local read rather than a walk over nodes. Being local is what makes it affordable. The budget guard in `hack/run-kubernetes-node-join_test.bats` computes both sides of its own inequality, and on this tree the sampling pair can overshoot by 662s against the 680s the derivation allows, which leaves 18 seconds of room. A per-node reading costs two samples times three nodes times the read bound and its kill grace, or 150 seconds at today's knobs, and no transport changes that: the cost sits in the number of per-node calls rather than in the number of counters. Reading the layer the sandbox shares with the runner costs one bounded read instead, and takes nothing away from the collectors already there. debugfs is mounted in a mount namespace belonging to the call. The suite runs for another twenty minutes after this returns, and a mount left behind would be a diagnostic editing the environment the rest of the run measures. The pair is taken across the node-join wait rather than inside the diagnostics block. A counter here accumulates over the life of the VM it belongs to, so one reading is an average over that life rather than a rate over anything; taking the first before the wait and the second after it makes the interval a difference divides by the window the guest was failing to boot in, instead of a few seconds after the deadline had already expired. The kernel-wide files add a wrinkle the artifact states beside the numbers: the kernel answers them by summing over the VMs alive at the moment of the read, so a VM that went away between the samples takes its whole contribution with it while the survivors keep accumulating. The difference is then understated by a whole VM life and its sign proves nothing when positive; when it does come out negative, that is a finding about the sandbox rather than a rate. The second reading is taken on the passing path too, so a green run leaves a baseline to read the failing ones against, and it is taken immediately after the wait rather than at the end of the run. That placement is the measurement: the happy-path tail is another ten to fifteen minutes of a tenant cluster doing storage and LoadBalancer work, so a reading taken after it would price a different window over a different workload while the capture's legend says it priced the join, and the green-versus-red comparison the collector exists for would be gone. A guard in `hack/run-kubernetes-node-join_test.bats` pins both calls to either side of the wait and fails if either drifts. What the capture cannot answer is written into the artifact rather than left for a reader to assume. The counters price the sandbox VMs' own execution and not what runs inside them. A per-VM directory is named after a pid in the kernel's initial namespace, which the container cannot map to its own processes, so the split says how evenly the sandbox VMs paid and not which node paid what. Several counters sitting in that same directory are gauges or running maxima rather than cumulative, so a difference of those is a number with no meaning. And there is no `ept_violation` counter to look for: the nested-paging faults are counted as `pf_taken` and `pf_fixed`, under those names whichever vendor the runner turns out to be. Every branch that can come up short is named where a reader will look for it: no `unshare` or no `mount` on PATH, a mount the sandbox refused, a kernel whose debugfs holds no `kvm` directory at all (which would be a finding about how the sandbox VMs are being run, not a collector that failed), a `kvm` directory holding nothing readable, a walk the wrapper cut short, and a report directory that could not be created. The pairing instruction rides only on the arm that holds a whole reading, so the artifact never invites a difference against a file that has no counters in it. A capture holding no counter answers its caller non-zero, the two findings included, so the passing path writes the shortfall into the job log instead of leaving it in a report nobody downloads. A walk cut short after it wrote counters does not: it left a usable partial reading, marked as partial, and telling the caller nothing was collected would contradict the file sitting beside it. That distinction is decided by whether a counter reached the file rather than by whether the file has bytes in it, because the walk writes a heading before it reads anything. Covered by `hack/run-kubernetes-kvm-exits_test.bats`, which runs under `make unit-tests`. The guards in `hack/run-kubernetes-node-join_test.bats` that enumerate the collectors were updated rather than worked around: the new capture gets a cost arm in the budget guard, an entry in the missing-timeout warning, and an explicit exemption from the spend-order list, because it is not behind the phase gate. It is not behind that gate deliberately: every other collector the phase declines loses only itself, while declining this one would orphan a reading already taken eighteen minutes earlier. Two guards there are new: one pins where the two readings sit rather than what they contain, the other pins that both call sites consume the collector's status, since this is the only collector in that file whose return value is read at all and it is read so a passing run's shortfall reaches the job log. The collector is also stubbed in that suite, at the shared helper and at each of the five inline stub lists inside its `bash -c` bodies. Left out of them the node-join unit tests call it for real, which on Linux means the unit suite runs `unshare --mount` and `mount -t debugfs` with whatever privileges the host gives it, while on macOS the collector stops at its own `command -v unshare` precondition and the path is never exercised at all: a unit test whose result is set by the machine rather than by the code. Measured with a recording fake on `PATH`, the suite makes 21 real invocations with no stub, 9 with the shared helper alone, and none with every site covered. Stubbing costs the block-level read audit nothing, because that audit works by letting real collector bodies reach the kubectl mock and this collector issues no kubectl read. Context for the measurement is cozystack/cozystack#3513. ### Screenshots Not a UI change. ### Downstream repositories I walked the trigger map in `docs/agents/contributing.md` against the diff. The diff adds one collector to `hack/e2e-chainsaw/_lib/run-kubernetes.sh`, adds one BATS suite under `hack/`, and edits guards in an existing BATS suite. Nothing under `hack/` is moved or renamed, no make target changes what it does, and `hack/e2e-prepare-cluster.bats` is not touched, so neither the `ccp` trigger on `hack/` layout nor the `ansible-cozystack` and `talm` triggers on node prerequisites fire. - [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 test(e2e): capture the runner kernel's KVM exit counters on either side of the tenant node-join wait, so a node-join failure can be told apart from a nested-virtualisation slowdown that no counter inside the sandbox can see ``` |
||
|
Aleksei Sviridkin
|
827f4e7e10 |
test(e2e): measure the sandbox kernel KVM exit counters across the node-join wait
Every CPU reading on this failure path describes a subject inside the sandbox -- a cgroup, a QEMU thread, a node's own /proc/stat -- and all of them can report a guest that was handed its ticks while the guest gets no work done. None of them can say what a tick cost. That price is paid one layer down: a tenant worker is a guest of a sandbox node, which is itself a guest of the runner, and the runner's kernel absorbs the exits, nested page faults and TLB flushes the arrangement provokes. Read those counters from the debugfs that kernel publishes them in. The suite runs in a container sharing it, so this is a local read rather than a walk over nodes, which is what makes it affordable at all: the phase budget has no room left for a further per-node read. debugfs is mounted in a mount namespace belonging to the call, so nothing is left behind for the twenty minutes of suite that follow it. The pair is taken across the node-join wait rather than inside the diagnostics block: once before the wait on every run, once after it on the failing path and on the passing one. The counters are running totals -- per-VM since that VM was created, kernel-wide summed over the VMs alive at the read -- so the interval a difference divides by becomes the window the guest was failing to boot in, and a passing run leaves a baseline to read the failing ones against. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
IvanHunters
|
5297df5cf3 |
Merge remote-tracking branch 'origin/main' into pr-3315-lgtm
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> # Conflicts: # api/apps/v1alpha1/kubernetes/types.go # api/apps/v1alpha1/kubernetes/zz_generated.deepcopy.go # hack/e2e-chainsaw/_lib/run-kubernetes.sh # packages/apps/kubernetes-nodes/tests/render-parity.sh # packages/apps/kubernetes/README.md # packages/apps/kubernetes/templates/cluster.yaml # packages/apps/kubernetes/values.schema.json # packages/apps/kubernetes/values.yaml # packages/system/kubernetes-nodes-rd/cozyrds/kubernetes-nodes.yaml # packages/system/kubernetes-rd/cozyrds/kubernetes.yaml |
||
|
IvanHunters
|
6839f3474c |
test(migrations): pin implicit-md0 KMT-feeding fields against DEFAULT_MD0 drift
The implicit-md0 case only asserted the child HR name, minReplicas and the role, none of which enter the content-hashed KubevirtMachineTemplate. An edit to DEFAULT_MD0's instanceType, diskSize, storageClass or container fields would re-hash the KMT and silently roll every implicit-md0 pool on upgrade while every test stayed green. Pin the pre-split md0 field values so such an edit reds this case. Mutation-verified: changing instanceType/diskSize in DEFAULT_MD0 fails it. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
myasnikovdaniil
|
9f5550abdd |
fix(release): publish rewritten packages during RC promotion (#3795)
## What this PR does Fixes #3477. - Publishes a new packages OCI candidate after rewriting RC image tags to stable tags. - Pins the candidate digest in the stable installer without rebuilding container images. - Verifies the candidate against the prospective merge and original RC artifact before creating stable names. - Refuses promotion when the target maintenance branch lacks the candidate-aware pipeline. - Protects candidates referenced by open release PRs from retention cleanup. - Leaves already published v1.6.0 and v1.6.1 unchanged. ### Screenshots Not applicable. ### 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: ### Testing - `bats --no-tempdir-cleanup hack/promote-packages-artifact_test.bats hack/verify-promoted-packages_test.bats hack/promote-packages-contract.bats hack/promotion-retention-contract.bats` - `actionlint .github/workflows/promote-rc.yaml .github/workflows/pull-requests.yaml .github/workflows/pull-requests-release.yaml .github/workflows/retention.yaml` - `sh -n hack/promote-packages-artifact.sh hack/verify-promoted-packages.sh` - `git diff --check origin/main...HEAD` ### Release note ```release-note fix(release): publish the rewritten stable packages artifact when promoting a release candidate ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Release candidates now use temporary, immutable package artifacts verified before stable publication. - Stable promotions validate package contents, installer references, container digests, and release metadata before tagging. - Temporary promotion artifacts are retained while needed and cleaned up after expiration. - **Bug Fixes** - Improved handling of release labels, fork pull requests, and promotion pull requests. - Added end-to-end verification before release promotion. - Prevented invalid, ambiguous, or untrusted package references from being promoted. - **Documentation** - Updated release and promotion guidance for candidate verification, digest pinning, and transactional promotion. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
73b8afc256 |
test(e2e): run tenant workers on one vCPU with CPU headroom above it
Under a two-vCPU worker both vCPU threads burn 1.4 to 1.8 cores between them while the guest kernel never gets past SMP bringup, and raising the CFS ceiling from two cores to three changed neither the burn nor the progress, so the spin is between the two vCPUs, not against the quota. A single-vCPU guest has no sibling to spin on. The ceiling of two keeps the QEMU emulator and IO threads from eating into the one core the guest computes with, and the request keeps scheduling where the workers always were. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
b8964c8233 |
test(e2e): give tenant workers CPU headroom with a request to match
Restores the ceiling the kubernetes suites briefly carried, now that a node group can declare the CPU request that goes with it. The limit on its own was defaulted into the request by KubeVirt, so every worker asked the scheduler for the whole ceiling and sat Pending on Insufficient cpu before a guest existed. The request is set to what KubeVirt derived for these workers before, which leaves scheduling where it was and leaves the ceiling as the only variable the run changes. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
80e2753bf4 |
fix(e2e): drop the worker CPU limit until it can carry its own request
A container that sets a CPU limit and no request is given a request equal to the limit, so each tenant worker asked the scheduler for its whole three-CPU ceiling and every kubernetes suite worker sat Pending on Insufficient cpu across all three sandbox nodes, before any guest booted. The headroom experiment comes back when the chart can pair the limit with an explicit request. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
c2c50b4119 |
test(e2e): give tenant workers CPU headroom above their vCPU count
Workers that fail to join have been measured burning their vCPU threads flat out under a ceiling equal to their vCPU count while the guest kernel recorded no progress at all, with the emulator, IO, vhost and vnc threads flat at zero. That is the shape of a vCPU spinning on a lock whose holder is the other vCPU, preempted out of the quota the two share, and it predicts that a ceiling with room above the vCPU count lets the preempted one back in. What the next runs answer is whether the join failure rate moves. If it does not, the lever is elsewhere and the value goes back. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
cf2d9c6d13 |
test(e2e): split the tenant worker CPU time across QEMU threads
The node-join diagnostics phase samples the tenant worker's CPU counters twice on failure, but it reads them from the compute container's cgroup, and that cgroup holds every thread QEMU runs: the guest's vCPUs, QEMU's main thread, and its IO and worker threads. A container burning most of its quota while the guest reports no progress is two incompatible findings wearing one number, and no container-level counter separates them. Add a collector that execs into each Running worker's compute container, finds the QEMU process and copies every /proc/<pid>/task/<tid>/stat line into the artifact verbatim, with a legend beside them. Nothing is computed here. The legend names where utime and stime sit and why counting fields over whitespace reads the wrong column for exactly the threads the split is about: a KVM vCPU thread is named "CPU 0/KVM", that space is inside the comm parentheses, and every field after it shifts by one. It runs inside the sampling loop the two existing subjects already share, so it costs no second wait: both readings ride the interval already being paid, and the rate comes from the two files plus the stamps each one carries. Every way the read can come up short is a named outcome rather than an empty directory, and the legend and the pairing instruction are withheld from each of them: telling a reader to subtract two files asserts that both hold a whole reading. The probe shells out to nothing but getconf, whose absence it names, because a probe that needed cat would report a container missing coreutils as a container whose QEMU had exited, and it counts what it managed to read so that having read no process at all stays separable from having read them all without finding QEMU. Its stderr is suppressed on the walk rather than on either read: redirections are applied left to right, so a 2>/dev/null after an input redirect is not in place yet when opening the input fails, and a thread that exits between the glob and the read would otherwise put a shell complaint in the artifact beside a healthy capture. It also reports a QEMU that was named and then gone before its task files could be opened, because a process heading alone is enough to make the capture non-empty and would otherwise carry a legend describing columns the file does not have. The Pod listing the cAdvisor captures already share now takes its whole query as a parameter, so this collector reuses that read rather than carrying a third copy of it, and asks for the Running Pods only: a launcher left over from an earlier attempt costs the sibling caller nothing and would cost this one a bounded exec and a slot under its cap. The guard that holds the sampling group against the room the tenant snapshot needs now counts every capped walk in the loop rather than the two spelled the same way, so the new subject is inside the arithmetic rather than beside it. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
959e90c9ab |
test(e2e): time the guest console capture and baseline it on a green run
A tenant worker's guest console is stamped in seconds since that guest's own kernel started, so a log ending mid-boot says how far the guest got and nothing about when. Whether it ends there because the guest started late or because the guest stopped printing decides which failure is being looked at, and neither instant was recorded: the read's own instant was nowhere in the file, surviving only as the artifact's mtime, which is metadata rather than content and so says nothing to anyone reading the log; and the guest's start only inside a describe dump that a capture with no silent console never triggers. Across three collected runs every capture exited zero and every log began at the first kernel line, so the reads were complete at both ends and the logs end where the guests stopped -- and which of the two happened could not be read off any of them. Each console log now carries the instant its own read started and returned, and CAPTURE-TIMELINE.txt beside it carries each Pod's phase, its start time and the start of its compute and console containers, from one bounded read taken once for the whole selector. Those subtract to how long a guest had been alive when it was read, which is what the log's last kernel stamp is then compared against. The console container is asked for in both status lists, since KubeVirt runs it as an init container and reading only one of them would report a container that started fine as one that never ran. A read that failed, a read cut off part way and a read that answered with nothing are recorded as themselves rather than left as a missing file, and the note names the error log only when there is one. The same capture also runs on the passing path, before the tenant is torn down and the Pod holding the stream goes with it. Every console this tree has collected came from a failure, so what a healthy boot looks like has never been measured, while "slower than usual" is the claim the failure keeps resting on. That caller asks for the pool's minimum rather than its maximum, since two workers booting is the same measurement as ten, and it warns in the job log rather than failing a suite that has already proved everything it exists to prove. Both paths write the same directory, so the capture also records which of them produced it. That capture costs real wall clock at the tail of an operation with a fixed ceiling, which is a way for a run that passed to be killed while collecting a debugging aid. So it is declined out loud when the ceiling no longer has room for it, against a reserve derived from its own cost and held in step with the timeout both suites give the operation. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
2b3c163d15 |
fix(e2e): size tenant workers by resources, not by u1.large
Sizing the workers with u1.large doubled their memory along with the vCPUs, and four 8Gi workers do not fit on the sandbox beside the rest of the suite: the second worker of each cluster stayed Pending with Insufficient memory on all three nodes, so the node group could never reach two Ready nodes and node-join failed for a reason that has nothing to do with the CPU ceiling it was meant to test. Setting resources.cpu and resources.memory keeps the two vCPUs at the memory the workers had before. The chart drops the instancetype from the VM when a group sets both, and the values schema still requires the field, so it stays in the values as the sizing fallback it now is. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
3b49be07fa |
test(e2e): give tenant workers two vCPUs
The tenant worker boots Talos and unpacks the kubelet image on a single vCPU whose CFS quota also covers the QEMU emulator and IO threads, and measurements on both quiet and loaded runs show the compute container pinned at 93-98% of that one-core ceiling for the whole node-Ready budget. The join deadline is then decided entirely by how much CPU work the boot happens to need, and under parallel e2e load it does not fit. Doubling the worker to u1.large raises the ceiling instead of the weight, which measurements showed changes nothing since the workers never lose a scheduling fight. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
ef96292b64 |
fix(ci): give the nightly e2e runner headroom over its guest vCPUs (#3833)
## What this PR does The e2e sandbox boots three QEMU nodes at `-smp 8`, so 24 guest vCPUs are committed before any tenant VM exists. Four jobs boot that sandbox. Three run on the 32-vCPU class; nightly ran on the 24-vCPU one, where 24 guests against 24 host vCPUs leaves nothing for QEMU's own emulation and IO threads, containerd, the kernel and the runner agent. The other lanes moved to the larger class at some point and nightly was not moved with them. The sandbox keeps its 24 vCPUs. That guest count is part of what e2e claims to test, and a smaller nightly cluster stops matching the one PRs run against. This is hygiene, not a fix. Measured boot-phase data across 18 runs says the load a workflow puts on the host dominates guest starvation, not the runner class: the lanes already on 32 vCPU boot guests slower than nightly did on 24, and class and workflow kind are fully confounded there, so neither separates out. Do not read this as making anything faster or as addressing a particular red run. What is left is the arithmetic. A 1:1 commit has no headroom at all, and two lanes running the identical sandbox on different classes is drift nothing was watching. The rest is a check, because the guest demand and the runner classes live in different files and nothing compared them. `hack/sandbox-runner-headroom.bats` reads `-smp` and `-m` times the loop count out of `hack/e2e-prepare-cluster.bats`, reads the runner class of every job that calls the sandbox make targets, and wants the runner strictly larger on both axes. Neither side is a hardcoded list, so bumping `-smp`, adding a management node, or adding a fifth lane on a smaller runner turns it red on the same commit. Memory is in there because the same argument applies to it and `-m 24576` sits on the same line as `-smp 8`, not because a class that violates it exists today. It stops instead of guessing when it cannot read something, since a guess comes out smaller than the real demand and passes a runner that cannot hold the sandbox. Refused: a missing or non-literal `-smp` / `-m`, a launch written outside any `@test` block or inside one left open at end of file, a `for` opener the strict pattern does not match, a `for` list holding anything but literal words, a `while` sharing the block, two launches or two loops in one block, and a `runs-on` with no `oracle-vm-<n>cpu-<m>gb` in it. Unparsed loop openers are counted separately from parsed ones, so moving `do` to the next line is refused rather than silently multiplied by one. There is also a cross-check that every workflow calling the sandbox make targets yields at least one matched job, so a renamed target turns the parser red instead of quietly shrinking what it covers. The derivation runs against the live tree, which passes whether or not those refusals still work, so each one is pinned by a fixture that feeds a synthetic file to the same program. That matters more than it sounds: two of the refusals above exist because the first version of this file did not have them, and a launch moved into a file-level helper would have halved the measured demand and stayed green. The numbers come from: ```bash sed -n '98,107p' hack/e2e-prepare-cluster.bats # three nodes, -smp 8, -m 24576 grep -rn 'prepare-env' .github/workflows/ # the four jobs that boot it grep -rn 'runs-on: oracle-vm-' .github/workflows/ # their runner classes ``` The rule also goes into `docs/agents/e2e-testing.md` next to the other environment invariants, so whoever adds the fifth lane picks its runner class from the docs instead of from a red unit suite. Checked that it bites, twice over. Fourteen mutations of the inputs: reverting the runner class, `-smp 11`, a fourth and fifth management node, `-smp cpus=8`, `-m size=24576`, `do` on the next line, `$(seq 1 3)` as the list, a `while` loop, the launch moved into a file-level helper, renaming the make target, `ubuntu-latest`, a matrix expression in `runs-on`, a hypothetical `32cpu-64gb` class, and a new lane on a 4-vCPU runner. Then eleven mutations of the parser itself, one per refusal, each of which kills exactly the fixture that names it and nothing else. Green on this tree, and the same numbers out of gawk, mawk and BSD awk. Closes #3831. ### Screenshots No UI change. ### Downstream repositories Walked the trigger map against the diff. It changes one `runs-on` line plus a comment in `.github/workflows/nightly.yaml` and adds one file under `hack/`. The `ccp` trigger is moving or renaming something under `hack/`, or changing what a make target does; `hack/package.mk` and `hack/common-envs.mk` are untouched, and the new test is picked up by the existing `hack/*.bats` wildcard without the target changing. The `ansible-cozystack` and `talm` triggers are node prerequisites in `hack/e2e-prepare-cluster.bats`, which this reads and does not modify. - [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(ci): run the nightly e2e suite on a runner class larger than the sandbox's guest vCPU demand, and add a unit check that fails when an e2e lane loses that headroom ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Updated nightly end-to-end testing to run on a higher-capacity runner, improving support for multiple QEMU guests and required resource headroom. * Added validation to prevent sandbox runners from being undersized or oversubscribed. * **Documentation** * Documented the resource requirements and deterministic conventions for end-to-end sandbox testing. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
527d4ea00f |
test(e2e): measure tenant worker CPU as a rate, and the sandbox nodes under it
Two instruments for the node-join failure the kubernetes-* suites keep hitting. Neither changes what the suites assert. The worker CPU capture now reads container_cpu_usage_seconds_total beside the CFS counters, and reads them twice a fixed interval apart instead of once. The throttled counters say a container met its ceiling and cannot say how much CPU it got, which are the two candidate explanations for a stalled worker: a guest held at a one-core ceiling, and a guest whose vCPU thread is rarely scheduled at all. Both leave throttled periods behind. Every counter involved accumulates from container start, so a single reading is an average over the whole uptime and no rate at all, and the profile under investigation is a guest that freezes for tens of seconds and then runs flat out: the average comes out low, the instantaneous figure comes out at the cap, and only the second separates it from a guest that is simply capped. The sandbox nodes are a new subject. The worker captures describe a cgroup, so a guest that is runnable and never scheduled looks from there like a guest that asked for nothing. Whether the sandbox node itself was given its turn is recorded only in the steal column of its own /proc/stat: no kubelet endpoint carries it, and crust-gather cannot reach it because PodSecurity refuses its node-shell Pod here, so the read goes over the Talos API, which is already how the report reaches those nodes for dmesg. The rows are captured verbatim with their column legend beside them rather than reduced to a percentage, because /proc/stat has no header, steal and guest sit next to each other, and guest is large on a node running VMs and already counted inside user. Both subjects are read on either side of one shared wait, so the pair pays it once. The wait is not the interval either subject's counters span, though: each pass walks its own nodes and the other subject's pass falls between a subject's two readings, and on the run this exists for those passes are the slow part. Each capture therefore stamps both ends of its read, so a rate comes from subtracting stamps rather than from assuming the knob, and the width of those brackets is the uncertainty. That wait is a measurement interval rather than a stand-in for a condition nobody wrote a wait for, so the e2e conventions carve it out by name instead of marking it as a fixed timeout to be replaced. The pair is gated below the serial console rather than above it. Its ceiling is sixteen bounded reads plus the interval, which is most of the phase budget, and admission gates only when a collector may start, so above the console a run that hit those bounds would leave the console never started rather than merely cut short. The console is the only capture that survives a worker which never reached apid, which is the shape this failure usually takes. That overrun is inside the term the budget derivation already allows for one late admission, and a guard holds the pair's ceiling against it. No collector here may leave an empty directory, since an empty directory reads as a healthy node: a missing binary, a missing talosconfig, a listing that never answered, a node with no address and a read that was cut off each land as a named note or a COLLECTION-FAILED.txt. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
3dd142a718 |
fix(e2e): capture both ends of the worker byte path on node-join failure (#3804)
## What this PR does A tenant worker that misses the node-Ready deadline while its kubelet image pull crawls leaves two mechanisms behind one symptom, and the failure artifact could not tell them apart: either the link is slow, or the pull keeps restarting and discarding what it fetched. From inside the guest both are a progress line that barely moves. This adds the host-side counters that separate them, and the guest-side reads that explain a kubelet which never got as far as pulling at all. **Host side.** A per-worker network capture reads the eight `container_network_*` families from the same kubelet cAdvisor endpoint the CPU throttling capture already uses. cAdvisor builds them from the interface counters inside the Pod's own network namespace and publishes one row per interface, so what arrived at the worker is counted there in full, including the bytes a restart threw away, set against the progress the guest reported, that separates a pull which kept restarting from a slow link. Which row answers is not obvious, and getting it wrong inverts the reading, so the collector states it rather than leaving it to the reader. A bridge-bound worker presents four interfaces in its Pod netns: the one still named `eth0` is a dummy that holds the Pod IP so the kubelet's check passes and carries no traffic, so it reports zero, and it is the row a reader reaches for first, by name, where zero reads as a worker that received nothing. The number that answers is the renamed NIC's receive; the bridge counts the same bytes again, and the tap's counters are named from the host's end. The filter deliberately carries no interface predicate, so all four reach the artifact and can be told apart. Two further properties of the family fail by capturing nothing rather than loudly, so both are stated where the filter is: the dropped-packet series are spelled `packets_dropped` and there is no `*_drops_total`, and every row carries an empty container label because cAdvisor publishes them on the Pod sandbox, which means a filter borrowed from the CPU counters beside it matches nothing at all. The fixture's row shape and values are real kubelet output with the identifying labels rewritten. Its four interface names are derived rather than observed: no virt-launcher Pod was available to sample, so they are read from the name generators in the KubeVirt version this repo ships, and the counters attached to them illustrate direction rather than magnitude. **Guest side.** The Talos capture gains the service state machine and the link table. Talos creates a service's log buffer when its runner opens the log writer, as it starts the process, so a kubelet still waiting on its volumes, on time sync, or on the container runtime reporting healthy never reached its runner and has no log at all, and `talosctl logs kubelet` then fails naming a log that was never registered, which is true about the log and silent about the service. `talosctl services` answers with the state and the condition being waited on, in one call for every service, so the runtime a kubelet is blocked behind arrives in the same file. The link table is read as YAML because MTU lives in the `LinkStatus` spec and not among the resource's printed columns, and it is the value that decides whether a path-MTU reading of a stalled transfer survives. Both new reads are made with the short-lived `os:reader` credential the capture already mints; neither needs a wider role. **A worker that answered nothing now says so once, for the worker.** Each read already carried its own error and exit code, which is accurate per file and adds up to a directory indistinguishable from a worker that simply had little to report. ### What this does not do This collects evidence; it fixes nothing and is not expected to change any suite's outcome. The comparison it enables is not symmetric either, and the source says so: the host side reports bytes on an interface, while the guest side reports an image pull's own progress, which is unpacking rather than arrival. They bound each other; they are not two measurements of one quantity. ### Cost The diagnostics phase budget drops from 8m to 7m. That is forced rather than chosen: the inequality that keeps the tenant crust-gather snapshot reachable had ten seconds of slack, and the two added guest reads cost sixty across the minimum worker pool. A shorter budget declines more, and what it declines first is what is gated last: the mirror capture, whose subject is partly recoverable elsewhere, and the image-cache re-probe, which has no substitute and was already first to go. The new network capture also spends a listing and one read per node ahead of the guest captures, so those reach their gate with less headroom than before. All of it is priced where the number lives, as a ceiling the read bounds impose rather than a duration measured on a live cluster, and a collector the budget declines says so in the log rather than going quiet. The remaining slack is ten seconds, which is worth knowing before the next collector is added rather than after: it buys no bounded read at any bound this file uses, so adding one means re-deriving the budget again. ### Testing `hack/run-kubernetes-network-counters_test.bats` is new (15 tests). The node-join, Talos-diagnostics and CPU-throttle suites gain coverage for the phase ordering, the per-worker marker boundary, the spend-order comments in both Chainsaw suites, and the enumeration of collectors that survive a missing `timeout`. Every claim the two Chainsaw comment blocks make about spend order is derived from the source rather than restated, so a collector added without a line there fails. The two cAdvisor captures now run one shared body. They previously carried near-identical copies, and the copies cost more than duplication normally does: the block-level bound audit pins each collector by something only it produces, and two collectors issuing identical reads left neither with anything of its own, so removing one from the audit went unnoticed. Each is now pinned by the report directory it writes. Verified with `hack/cozytest.sh` across eleven suites, checking that the number of tests executed matches the number declared, because an exit code answers "did anything fail", never "did anything run". Each behavioural change was confirmed by mutating the source and observing the intended test fail. ### Screenshots No UI changes. ### Downstream repositories Walked the trigger map file by file against the diff. All seven changed files live under `hack/`, and the change adds one file and modifies six; no repository file is moved, renamed or deleted. The `ccp` entry is the only one that keys on `hack/` at all, and it keys on `hack/package.mk`, `hack/common-envs.mk` and `hack/update-crd.sh` as anchors for `make generate`; none of those is touched, and no make target changes behaviour. Nothing here is a package, a chart value, an `ApplicationDefinition`, a CRD, a release asset, a namespace, a variant, or a node prerequisite, so no other entry applies. - [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 ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added worker network-counter diagnostics with interface-level and cumulative traffic data. * Expanded Talos diagnostics with service-state and link-state information. * **Bug Fixes** * Improved reporting for partial captures, timeouts, unavailable data, and zero-traffic workers. * Clarified failure reporting and fallback messages when timeout controls are unavailable. * **Documentation** * Updated diagnostic timing and collector documentation for the seven-minute budget and new metrics. * **Tests** * Added comprehensive coverage for network counters, filtering, truncation, and expanded Talos diagnostic scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
db2ddfab2b |
fix(ci): give the nightly e2e runner headroom over its guest vCPUs
The e2e sandbox boots three QEMU management nodes at `-smp 8`, so 24 guest vCPUs are committed before any tenant VM starts. The nightly lane ran that sandbox on a 24-vCPU runner, a 1:1 commit that leaves the host nothing for QEMU's emulation and IO threads, containerd, the kernel and the runner agent. The tag, fork and pull-request lanes already run the same sandbox on the 32-vCPU class, so move nightly there too. This aligns headroom and claims no performance effect. Measured boot data shows the load a workflow puts on the host, not the runner class, dominates guest starvation, and the two are confounded in that data; no red run is attributed to the class here. The case is the arithmetic: a 1:1 commit leaves no headroom, and two lanes running the identical sandbox on different classes is drift nothing was watching. Add hack/sandbox-runner-headroom.bats to keep the two sides paired. It derives the guest vCPU and memory demand from the sandbox bats file, and the runner class of every job that drives the sandbox make targets, then requires the runner to be strictly larger on both axes. Neither side is a literal list, so raising `-smp`, adding a management node or adding a lane on a smaller runner turns it red instead of drifting apart silently. Input the derivation cannot read in full is refused rather than guessed: a guess produces a demand smaller than the real one, which passes a runner that cannot hold the sandbox, while a refusal names the line. That covers a launch written outside any `@test` block, a block left open at end of file, a `for` opener the strict pattern misses, a non-literal loop list, and a `while` loop sharing the block. The derivation runs against the live tree, which passes whether or not those refusals still work, so each one is pinned by a fixture feeding a synthetic file to the same program. Loosening the parser turns a fixture red rather than quietly widening what the check accepts. Record the rule alongside the other environment invariants in docs/agents/e2e-testing.md, so a new lane picks its runner class from the documentation rather than from a failing unit suite. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
e441edc2aa |
test(e2e): stop the mongodb backup suite before the restore leg
A to-copy restore replays the source cluster's system users into the target, so the target's own generated credentials stop authenticating and the harness cannot read the sentinel back out of the restored copy. The suite has never passed for that reason, holding the in-tree e2e run red on every branch that selects it. Drive run-all.sh with SKIP_RESTORE=1 so the suite still proves the backup half end to end, and pin the skip by asserting the skip branch's own log line: exit zero alone is also what a completed round-trip returns, so without that assertion the step could not tell a skipped flow from a full one, nor notice SKIP_RESTORE failing to reach the harness. The to-copy manifest and the example README both stated that CI covers the restore, which stops being true here, so both now say where coverage ends. The asserted log line is annotated at its source, since rewording it would red the suite with a bare match failure. Coverage returns by dropping SKIP_RESTORE together with the assertion on that line, once the credential handling behind the restore is fixed. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
IvanHunters
|
b1ce856c47 |
test(platform): cover the 53-char child-name overflow skip branch
Every warn-pin-skip hardening branch in migration 54 had a regression fixture except the >53-char child-name overflow guard. Add one: a 33-char cluster name plus pool md0 yields a 54-char child release name, so the overflow branch must warn, pin the live pool objects with resource-policy=keep, skip adoption, and let the run stamp. Without the guard the flow reaches kubectl apply of an over-long HelmRelease name, the apiserver rejects it, and set -e exits before the version stamp, deadlocking every tenant's platform upgrade. Verified non-vacuous: neutralising the length guard reddens this fixture. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
c7d3d40810 |
test(platform): assert the non-scalar-field skip branch pins live pool objects
The existing bad-scalar test ran with the pool objects absent, so it pinned the warn+skip+no-child-HR contract but not the pin itself. Add a companion that presents the live MachineDeployment, MachineHealthCheck, WorkloadMonitor and KubevirtMachineTemplate and asserts each is annotated resource-policy=keep with no ownership transfer, mirroring the non-object-nodeGroups companion. Verified non-vacuous: removing the branch's pin_pool_objects call reddens it. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Myasnikov Daniil
|
ffae003d6b |
fix(release): stop a failed org package listing from exiting green
The nightly sweep's own listing shared one `|| true` with the grep that filters it, and grep legitimately exits 1 when no cozystack/* package matches. A failed /orgs/.../packages read therefore arrived as an empty list, took the "No cozystack/* container packages found" branch and exited 0 -- below the nightly sweep but ABOVE the deferred guard, so a protection set that could not be resolved reported success having swept nothing. That bypasses the guard rather than varying it: replaying the step against a stub gh, a failed /pulls combined with a legitimately empty org listing exited 0 after printing its own ::error::. List and filter as two statements so one `|| true` stops meaning two things, handle the listing failure like the other two (::error::, blocked flag, emptied list), and consult the flag before the early exit. That exit is non-zero, matching the other two listing failures: no sweep is left to carry the run down to the guard, so this is the last place the flag can be honoured, and an unresolved protection set is exactly what fail-closed means. The legitimate empty-org case still prints the same line and exits 0. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
a37a3178e5 |
docs(release): name all four sourcers of the image-ref enumeration
The library header still claimed three call sites. There are four: this PR made hack/verify-promoted-packages.sh source it to compare the candidate's refs against the rc artifact's. The historical drift story stays with the original three, since the verifier has only ever read the shared enumeration. The consumer table gained hack/promote-packages-artifact.sh under a bullet requiring every consumer of references to read all three shapes through the library, while the exception below it still named only hack/overlay-main-images.sh. The publisher sources nothing and needs nothing: it hands the whole tree to `flux push artifact` and touches one reference, the installer's own self-pin, by key with yq. Say that, rather than implying a gap it does not have -- the real exception remains overlay-main-images.sh, which does read references and does walk the tree itself. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
dee398f3b3 |
fix(release): defer a retention listing failure to the promotion guard
Both candidate-protection sources open with a `gh api --paginate` listing and neither handled a failure. Under `set -euo pipefail` a transient /pulls or /branches blip therefore exited the step before the unrelated nightly GHCR sweep had run -- the exact coupling the deferred guard at the bottom exists to remove, and which hack/promotion-retention-contract.bats already asserts positionally. Route both listings into that guard instead: raise the blocked flag and empty the list, so the promotion sweep is skipped and the job still fails while the nightly sweep finishes. Emptying is not cosmetic -- a half-paginated list left in place is a smaller protected set, and a smaller protected set is the one shape that can authorise a delete. Replaying the step's real shell against a stub gh: with /pulls failing it exited 5 and never reached the nightly line; it now exits 1 with the nightly sweep done and the promotion sweep skipped. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
b65b6aa3ae |
test(release): pin every requiredBaseFiles entry, not just the list
The promote gate refuses a target base that lacks any of five [path, marker] pairs, and the contract test asserted the list as a block: deleting the whole thing turned it red, deleting the hack/lib/image-refs.sh entry alone did not. Four of five pairs were therefore unpinned individually. Pin the missing one. The verifier sources that library through the same `dirname $0` as lib/promoted-packages.sh, so a base carrying the script without it fails at exactly the same point -- after the candidate, the staging branch and the draft release exist. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
acd0a9e0a2 |
test(release): cover the verifier's rc-reference scan guard
The scan treats any grep diagnostic as a failed scan, because `find ... -exec ... +` reports the legitimate "nothing matched" as a failure too and the `|| true` absorbing that also absorbs a file that went unscanned. Deleting the whole guard left the suite green: its two siblings -- a failing collector and an empty collection -- each have a dedicated test, and this one had none. Reach the state the guard exists for by putting a grep on PATH that answers the scan's invocation the way the real one answers a file it cannot open. A chmod 000 fixture would stop proving anything wherever the suite runs as root. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
04fa8471fe |
docs(release): say what the gates actually guarantee
Three claims that were stronger than the code, plus one legibility fix. The PR gate's comment said a `packages/` edit "on the PR or its base" fails the status before merge. The base half is not true: GitHub fires no `pull_request` event when the base branch advances, it only recomputes refs/pull/N/merge, so a change landing on the base after this went green is never re-verified and the PR merges on the stale result. Checking the merge tree does make base drift visible WHEN the job runs, which is worth keeping — but finalize is what actually catches the drift case, after the merge and before any stable name. Say both, in the workflow and in the reviewer checklist, so nobody reads the green as covering more than it does. "Transactional at every consumer-visible name" is a judgement call dressed as an absolute, inherited from the "this dispatch performs NO registry mutation" it replaced. The dispatch now does write to the registry: the `promotion-*` candidate is a real, publicly listable tag. What it creates is no *stable* name — no vX.Y.Z, no :latest, no stable installer. Stated that way the sentence stops needing the reader to already know which writes were meant. The retention paragraph listed two of the conditions the workflow enforces. It now lists all of them, including the ones the workflow gained here: same-repository, bot-authored, `release-X.Y.Z` branch, the `release` label, exactly one valid digest pin, and protection from a base-branch tip as well as an open PR. Also: `flux pull artifact` reads its credentials from ~/.docker/config.json, which is why finalize now runs `docker login` — but its runner class is not one of the docker-using build ones and the setup step installs everything except docker. Check for it there, where the message can name what is missing, instead of failing with "command not found" halfway down the release path. And the verifier's exclusion list is a restatement of flux's `excludeOCI`, not a derivation — the CLI does not expose it — so the coupling and the pinned version are now written down next to the list. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
820e72d572 |
fix(release): protect a candidate until its retag, not until PR merge
Retention protected a packages candidate only while an OPEN bot release PR pinned its digest. The digest stops being eligible for deletion only when finalize's retag adds a stable tag to the manifest, because that is what makes the `all(tags match promotion-*)` predicate stop matching — and finalize runs after the merge that closes the PR. The window between them is where a half-finished release sits. Finalize creates the write-once stable git tag, then dies before the retag: a lost runner, a 5xx publishing the draft, a non-fast-forward on the maintenance branch. What is left pins the candidate from a merge commit and a tag nobody can move, with no open PR and only a temporary tag on the manifest. Thirty days later retention deletes it, after which re-running finalize 404s at the candidate check and re-dispatching promote is refused because the stable tag exists. The release can then only be finished by hand — the exact class of manual cleanup this design exists to remove. Protect the tips of the branches a promote PR can merge into as well. The two sources are read differently on purpose. An open promote PR's head is supposed to carry exactly one candidate pin, so anything else is an anomaly and blocks. A branch tip is an additional source: every digest pin found is protected and a branch without one contributes nothing, because a maintenance line cut before this pipeline existed cannot pin a candidate and must not wedge retention forever. Blocking now stops the promotion sweep alone. It used to `exit 1` inside the resolution loop, which also killed the unrelated nightly prune further down the same step — so a WIP commit adding a trailing comment to a pin on a release branch would silently stop GHCR nightly pruning. The job still fails; it fails after the nightly sweep has run. Verified by running the step's script against a stubbed `gh`: a digest pinned by main's tip and referenced by no open PR is now excluded while a genuinely abandoned one is still listed; a manifest that also carries a stable tag stays excluded by the existing predicate; and a malformed pin on a PR head leaves the nightly sweep running, skips the promotion sweep and exits 1. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
05b2ce90cf |
fix(release): close the verifier's fail-open paths and cover them
Three gaps in the guards that decide whether the published candidate is still the tree being tagged. All three passed silently rather than failing, which is the worst shape for a check that runs immediately before a write-once name is created. `normalized_refs` piped `collect_image_refs` straight into its parsing loop, so the pipeline reported `sort -u`'s status and the collector's was discarded. POSIX sh has no `pipefail` and this script has to stay POSIX, so `set -e` never saw it: a failed or truncated collection became a smaller set and the rc-vs-candidate comparison matched it happily. The verifier then reported that no container bytes changed across promotion having proven nothing — on the guard that carries that entire claim. Collect into a file first, so a nonzero collector aborts before anything is compared. An empty collection is the same failure wearing a zero exit status and needs its own check: two empty sets compare equal, so the proof passes having examined nothing, and every packages tree carries image references — none means the collector understood nothing it was handed. The rc-leftovers scan swallowed grep's stderr, turning "could not read this file" into "this file is clean" — the silent-skip shape hack/promote-rewrite-tags.sh refuses by name. The `|| true` has to stay, because `-exec … +` reports the legitimate "nothing matched" as a failure too, so the diagnostics are the only remaining signal that a file went unscanned. Fail on any of them. Defence in depth rather than a live hole: a surviving rc string would also fail the per-file compare, since the release tree has none. The per-file content comparison had no test at all. It is the only leg that catches a file present on BOTH sides with different bytes — exactly the threat the job exists for, a maintainer editing packages/ on the release PR after promotion published the candidate. The file-set compare sees the same names, the installer compare reads one file, and the rc-refs compare never looks at the release tree. Replacing the comparison with a no-op left all nine other tests green. Red phases, all observed before the fixes. The content-compare case changes one non-image line on one side, so no other leg can fire and claim the credit; it fails with the comparison no-op'd and passes with it restored. The two collector cases run the verifier from a copy whose sibling lib/ holds a stub — which is how the workflows already invoke it, `.release-tooling/hack/…` with libraries resolved next to the script, so the failure is reached the way production would reach it. Against the piped version both fail on `[ "$rc" -ne 0 ]`: the verifier exits 0 and prints its success line whether the collector returns 3 or returns nothing. Also pins `e2e`'s dependency on the candidate gate. The contract pinned `e2e-report`'s, so removing `verify-release-candidate` from `e2e`'s `needs` together with the `success` clause on its release arm left all six tests green — and running a full-e2e promotion suite against a candidate that never verified is the worse of the two failures. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: Claude <noreply@anthropic.com> |
||
|
Myasnikov Daniil
|
9fa8eeeda2 |
ci(release): pin the promotion toolchain and cap the candidate gate
Three review findings on the new promotion steps.
`yq` was fetched from `releases/latest/download` in all three steps this
pipeline installs it in — the promote dispatch, the promote PR gate and
finalize. That makes an unpinned moving dependency part of what decides
whether a stable release may be created, and part of what rewrites the
installer pin: two promotions of the same rc can be served different
binaries. Pin it the way FLUX_VERSION already is, version-gated so a
matching preinstalled yq is reused, on the version the e2e sandbox image
already tracks.
A version authenticates nothing about the bytes it names, though, and
these are the workflows that mint write-once stable names. yq is
therefore also pinned BY CONTENT: its SHA-256 is committed beside its
version and verified before install.
Scoped to yq deliberately. It is the one downloaded binary with write
authority over what a release contains — it stamps the candidate digest
into installer values at promote time and platformVersion into the
stable chart at finalize. skopeo, helm and oras only move bytes that are
already digest-addressed, so a substituted one is caught downstream; a
substituted yq writes the wrong pin and every later check verifies that
pin faithfully.
The Flux installer is left as it is, on purpose. It fetches
flux_${FLUX_VERSION}_checksums.txt from the pinned release and verifies
the binary against it, so the binary is not the exposure — the ~6 KB
bootstrap script from fluxcd.io is, and it is mutable by design and
fetched identically in ten other workflows here. Pinning its bytes in
these three files would break on the next regeneration of a script the
project does not control while leaving the same fetch unpinned
everywhere else. The comment names the residual risk instead.
No precedent existed either way: nothing under .github/workflows/ or
hack/ verified a downloaded tool before this, and every version-pinned
download in the tree (oras 1.2.0, cozyvalues-gen 1.6.0) is unverified.
This is a new convention rather than an application of one, confined to
the release path and justified by write authority, and the comment says
so where the next reader will be standing. The three unpinned fetches
outside this pipeline (nightly.yaml, e2e-fork.yaml) are left alone.
The checksum is trust-on-first-use, which is the honest description: it
fixes the bytes to what was checked when the version was chosen and
makes any later substitution loud. Provenance is the release's own
published checksums list cross-checked against the downloaded asset;
both agree on
fa52a4e758c63d38299163fbdd1edfb4c4963247918bf9c1c5d31d84789eded4, and
that binary self-reports v4.53.3.
Verified locally against the real asset: the version gate skips a
matching preinstalled yq and re-installs a mismatched one, the checksum
line reports OK for the genuine binary and exits 1 for a tampered copy,
which aborts the step under the runner's `bash -e`.
`verify-release-candidate` had no `timeout-minutes`, so it inherited the
6-hour default while `e2e-report` waits on it through `needs` — a
stalled `flux pull artifact` would hold the required `E2E Tests` status
pending for the whole of it. 30 minutes, matching `checks` and
`finalize`.
The contract pins that all three install steps carry both versions and
the checksum, run the checksum check, name no `latest/download`, and
agree on one yq version and one checksum. So the unpinned one-line idiom
cannot come back into a release step by copy-paste from the workflows
that still use it, and bumping the version in one file without its
checksum cannot pass. Both mutations are caught.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: Claude <noreply@anthropic.com>
|
||
|
Myasnikov Daniil
|
2d734fb229 |
fix(release): preflight the rc tree's own pin before publishing
hack/promote-packages-artifact.sh pushes the candidate BEFORE rewriting installer values, so the artifact carries the rc tree's platformSourceUrl/platformSourceRef verbatim — and hack/verify-promoted-packages.sh reads exactly those two fields back out of it as the candidate's rc baseline, requiring a trusted repository and an immutable `digest=sha256:<64 hex>` pin. The publisher checked neither. A tag-form ref or an untrusted URL in the rc tree published a candidate and pushed two commits regardless; the refusal arrived later, in the promote PR gate or in finalize, once the staging branch and the draft release already existed. Check both at dispatch time instead, while nothing has been written. A copied regex would drift from the guard it is bringing forward and quietly restore the late failure, so the trusted repository and the digest pattern move into hack/lib/promoted-packages.sh and both halves source it. That makes the library part of what the verifier needs at run time, so promote's base-branch precondition gains it: the list exists because the base's own workflows run after merge, and a base carrying the verifier without its library fails at the same two late points. Tests: both preflights are covered, each asserting the mock registry client was never invoked — "exited non-zero" alone does not say nothing was written. Both fail with the preflight removed. The publisher fixtures now pin the trusted repository the way a real rc tree does, and still send the candidate elsewhere via REGISTRY so the rewrite assertions keep their teeth. Also covers a malformed PROMOTION_ID. It is not attacker-influenced, but it is the run-uniqueness suffix of the candidate tag and retention.yaml deletes a candidate only when every tag on the manifest matches `promotion-v…-run-[0-9]+-[0-9]+`; a value outside that shape publishes an artifact no cleanup pass can ever collect. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
54e8533a9d |
fix(release): compare executable bits unambiguously in the verifier
The mode check was written as a single chain:
if { [ -x "$a" ] && [ ! -x "$r" ] || [ ! -x "$a" ] && [ -x "$r" ]; }
which reads like a symmetric difference and is not one. POSIX gives
`&&` and `||` equal precedence and left associativity, so it groups as
`((A && B) || C) && D` with `D = [ -x "$r" ]`. A file executable in the
candidate and not in the release tree makes `A && B` true but `D`
false, so the whole condition is false and the mismatch is accepted.
Only the mirror case was rejected — and it is the benign one. The
direction that got through is content the merge reviewer never saw made
runnable inside the very artifact the operator installs, which cmp(1)
cannot see because it compares bytes.
Resolve each side to a value and compare those, so no precedence rule
is load-bearing.
Both directions now have coverage. Before the fix the candidate-only
test failed against the shipped condition and the release-only test
passed, which is what makes the pair meaningful rather than a check
that only goes red when the whole block is deleted.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|