mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-03 10:37:21 +09:00
feat/upgrade-e2e-chainsaw
902
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Myasnikov Daniil
|
c7dca5f1da |
test(e2e): record the upgrade lane's known failure instead of staying red
The lane is advisory, and an advisory check that is red for a reason everyone already knows trains reviewers to ignore it — then it stops working on the day it finds something new. It currently has exactly one such reason: tenant-test/vm-instance-test does not return to Ready across an upgrade (#3984), while the same workload passes on a fresh install in the very same run. Give the lane a ledger rather than a longer timeout or a suite exclusion. hack/e2e-wait-hr-ready.sh learns an optional EXPECTED_NOT_READY_FILE, and only hack/e2e-upgrade-apply.bats sets it; unset, the script behaves as it always did, down to making no second listing call on the happy path, so the install gate is untouched. The entries have to expire on their own or the file becomes the thing it was meant to prevent, a stale explanation outliving its defect. Three rules do that, all failures rather than warnings: - a not-Ready release that is NOT listed still fails the gate; - a listed release that is Ready again fails until its line is deleted; - a line naming no release in the cluster fails too. So the fastest way to remove a line is to fix the defect, and the file cannot be wrong about the present without saying so. A named file that cannot be read is also a failure: treating it as "nothing expected" would turn a typo into a gate that quietly accepts more than it claims. hack/wait-hr-ready_test.bats drives the real script through a kubectl stub over all of it. It is deliberately not named hack/e2e-*.bats: the unit target is $(filter-out hack/e2e-%.bats,...), so a file named after the script it tests would be excluded from the suite and never run. Verified non-vacuous by mutation — disabling each of the three rules in turn turns exactly one case red, and the missing-file refusal likewise. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
2cb2c3ae6c |
test(e2e): follow a composite action when sizing the sandbox lanes
The headroom check derives its job list by matching workflow jobs on the prepare-env / prepare-cluster make targets, and its coverage cross-check demands that every workflow file naming packages/core/testing yield at least one such job. Moving the sandbox bring-up into .github/actions/e2e-prepare left the target inside the action and only runs-on in the workflow, so the sweep matched neither the e2e nor the upgrade-e2e job and the cross-check turned red — exactly the case the file's header named as its floor and declined to cover. Both legs now follow a local `uses: ./…` into the action file it names, and into a local action that action uses in turn, so a job is matched wherever the make target actually sits and a workflow is read together with the actions it names. A `uses:` naming a reusable workflow is deliberately not followed: .github/workflows is already swept file by file, and the calling job carries no runs-on for this check to size. The sweep now emits JOB / REF / ERROR records so both legs read one derivation instead of two that can disagree. A local reference the strict path pattern cannot read, one that resolves to no readable action.yaml, and an action that reaches itself are all refusals rather than silent drops — a marker this cannot open is a lane it cannot see. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
f269e63331 |
test(e2e): fail where the baseline capture fails, not where it is read
The seed side carried the same fail-open the platform verify just lost: `kubectl get pods -A | awk ... > file || true` takes the exit status of the pipeline's LAST command, and the `|| true` discarded even that. A failed listing therefore wrote an EMPTY baseline and passed. An empty baseline is not silent — verify/platform reads every currently-unbound PV as newly unbound — but it fails forty minutes later, in the wrong suite, naming resources that were unbound before the upgrade ever started. Take both listings out of their pipelines so `set -e` sees them, and drop the `2>/dev/null` that was hiding the reason. The two intermediates go to /tmp rather than the baseline directory, so what the verify phase reads there is still exactly the two snapshots this suite documents. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
44feb45722 |
test(e2e): fail the platform verify when it has no baseline to compare
`comm -13 "$dir/unbound-pv.txt" ... || true` turns a missing baseline
file into an empty diff, and an empty diff reads as "nothing newly
broken". A verify phase run without its seed phase therefore reported
the platform healthy having compared nothing at all — the most expensive
kind of green. The sibling suites already guard theirs: verify/vm and
verify/redis fail fast with `[ -f "$before" ] || { ...; exit 1; }`.
Assert both baselines exist before either diff, and drop the `|| true`
that was swallowing the missing-file error along with everything else.
The two `kubectl get` calls had the same shape of hole one layer down: a
pipeline takes the exit status of its LAST command, so `kubectl get pv |
awk ... > file` left an empty list and a passing diff whenever the
kubectl call failed. Write the listing to a file first, where `set -e`
sees the failure.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
||
|
Myasnikov Daniil
|
f11bd3ba4b |
test(e2e): pin what the line-match case can actually catch
Review found the case named "line match is dot-anchored (1.5 does not match v155.x)" does not test that: removing the `sed 's/\./\\./g'` escaping from latest_on_line leaves all ten cases green. Checked it, and the reason is narrower than the report. Over the tags stable_desc emits — `^v[0-9]+\.[0-9]+\.[0-9]+$` and nothing else — the escaping, the trailing `$` and the `^` are each unfalsifiable: such a tag carries exactly one `v` and it is at the front, so no fixture can be built that the escaped and unescaped patterns disagree about. v155.0.0 in particular matches neither reading, so the old fixture pinned nothing at all. One piece of the anchoring a fixture CAN reach is the leading `^v` pair. v11.5.0 ends in "1.5.0" and `sort -rV` puts it above v1.5.3, so a pattern that has lost that prefix resolves the wrong line entirely. Swap the fixture to it, rename the case to the property it now pins, and record in both files what is and is not falsifiable here, so the next reader does not have to redo the experiment. Verified non-vacuous: dropping `^v` from latest_on_line's pattern turns exactly this case red; the other ten stay green. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
a9e39c91e0 |
test(e2e): clean up the resolver suite without EXIT traps
hack/bats-no-exit-trap.bats, added to main after this branch opened, is red on this file: eleven tests install `trap 'rm -rf "$tmp"' EXIT` and the file declares no debt, so `bats-unit-tests` fails, which skips `finalize` and with it both the required `E2E Tests` check and this PR's own lane. Convert rather than declare the debt. A handler installed inside an @test replaces the one the bats binary keeps for its own bookkeeping, and a test that then fails prints no TAP line at all — not `not ok`, nothing — so the suite reads green while a case is failing. Removing the scratch directory as the last statement of the body is the documented alternative: both runners set -e, so on failure the removal is unreachable and the directory survives for inspection, which is what a failed test wants anyway. The file header now says so, next to the cleanup it explains, so the next person adding a test here copies the right shape. Verified: hack/bats-no-exit-trap.bats green, and all eleven cases in this suite still pass. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
980d397b64 |
test(e2e): give the major-only target case a tag set the bug can pass
The case I just added was vacuous. _fixture ships no v1.0.x or v1.1.x tag, so the buggy reading of "v1" as v1.1 walked down to an empty v1.0 line and exited non-zero for the wrong reason — the assertion held whether or not the defect was present, which the mutation run showed: reverting -s left all eleven green. Give the case its own tag set containing v1.0.9, so the buggy path has a real baseline to return with exit 0 and the case can only pass because the target is refused. Reverting -s now fails this case and only this case. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
0c969ffcfb |
fix(e2e): refuse a target naming only a major version
`cut -f2` prints the whole line when that line carries no delimiter, so a single-component target like "v1" came back with min=1 and was resolved as if it were v1.1 — walking down to the v1.0 line and printing a real-looking baseline with exit 0. The lane would then install that baseline and report on an upgrade path nobody asked for, which is worse than failing. Pass -s so the delimiter-less line is suppressed, min comes back empty, and the existing parse guard rejects it. Two-component targets are unaffected: the guard rejects a missing minor, not a missing patch, and v1.6 still resolves. In practice the lane only ever passes "" or a target derived from a release-X.Y.Z branch, so this was latent rather than live. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
2bd363ccbe |
test(ci): keep the label-gate contract in step with the broadened gate
The previous commit broadened the plan and resolve_assets label gates to admit upgrade-e2e, but hack/promote-gate-contract.bats pins those two expressions as exact grep -cF literals. Both counts fell to zero, bats-unit-tests failed, and because "Unit & controller tests" gates finalize, the required E2E Tests check and this PR's own upgrade lane were both skipped — the commit meant to make the label start a run stopped every run instead. I broke this by running the unit suite before writing that commit rather than after; pre-commit does not cover bats-unit-tests, so it went unnoticed. Update both assertions to the broadened expressions, and add a case pinning the label end to end: admitted by the `labeled` allow-list in both plan and resolve_assets, honoured by the job's own condition, and named "Upgrade E2E Test" so it cannot collide with the required "E2E Tests" context and stop being advisory. Note the label is double-quoted inside the JSON allow-list — matching the single-quoted form counts zero and asserts nothing. Verified non-vacuous: narrowing the plan gate back to the full-e2e-only literal turns exactly one test red. Also correct the MariaDB seed header, which still prescribed the 127.0.0.1 connection that commit f598c253 replaced — the same file documents that path as OOM-killing the nano server container. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov DaniilandClaude Opus 4.8
|
c9d3e4a88c |
test(e2e): skip etcd-adoption backup in the upgrade lane, keep seaweedfs baseline
The etcd v1alpha2 adoption snapshot cannot succeed in an e2e sandbox: it needs a trusted-cert (ACME) external S3 endpoint, which a sandbox (example.org, no DNS/ACME, self-signed in-cluster cert) cannot provide — matching the dev10 findings. Set migrations.etcdAdoptSkipBackup=true in the platform Package so migration 50 runs the real adoption without the infeasible snapshot. Keep seaweedfs on the root tenant (the historical default install-cozystack.bats also uses) for a realistic baseline, but drop the fragile cozy-backups-creds wait — the adoption no longer depends on the seaweedfs S3/bucket/creds chain. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov DaniilandClaude Opus 4.8
|
8e2543758e |
test(e2e): fix upgrade canary DB access (verified on a live cluster)
Dev-cluster validation of the seed/verify canary commands surfaced two real bugs, now fixed and re-verified (postgres=3, mariadb=3 rows): - postgres: the seed writes SQL to psql over a heredoc on stdin, but `kubectl exec` without `-i` does not forward stdin, so CREATE/INSERT silently never ran. Add `-i`. - mariadb: the nano server container ships the `mariadb` client (MariaDB 11 dropped `mysql`) and does not bind 127.0.0.1 — connecting there fails and its retry OOM-kills the container. Use the `mariadb` client against the mariadb-test-primary Service (writable primary) from mariadb-test-0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov DaniilandClaude Opus 4.8
|
1f6e59d19a |
test(e2e): add chainsaw-native release upgrade testing lane
Reimplements the intent of #2401 (previous-minor-stable -> current upgrade test) over the post-Chainsaw-migration codebase. The old #2401 was a single 895-line BATS file, now obsolete; this builds a two-phase Chainsaw lane bracketing an external helm upgrade, reusing existing files throughout. Flow (make upgrade-cozystack, in the e2e sandbox): install-previous (bats, prev OCI chart) -> seed (chainsaw, skipDelete) -> helm upgrade to in-tree chart (bats) -> verify (chainsaw, assert-only). DRY over existing files: - 4 composite actions (.github/actions/e2e-{download-assets,prepare,collect, teardown}) shared by both the e2e and new upgrade-e2e jobs; the e2e job is refactored onto them (behavior-preserving). - hack/e2e-wait-hr-ready.sh: the all-HR-Ready gate extracted from e2e-install-cozystack.bats so install and upgrade share one legible gate. - hack/upgrade-prev-version.sh (+ unit test) resolves the baseline version. - seed suites apply the existing app manifests by relative path (no new CRs). Trigger: an upgrade-e2e job in pull-requests.yaml, parallel to e2e, gated on the release label or a maintainer upgrade-e2e label. Advisory / non-blocking. Coverage: postgres + mariadb canary rows, redis/VM volume survival, migration stamp advance, all-HRs-Ready + no-new-unbound-PV. The tenant Kubernetes survival suites (#2931 path) are env-gated OFF (UPGRADE_E2E_TENANT_K8S) as the flakiest surface, pending dev-stand validation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
a94abe0971 |
test(e2e): record the runner's SVM feature flags in the identity capture
Adds one line to the runner identity record, taken from the lscpu read it
already does:
svm features: svm npt nrip_save vgif
vgif decides whether a guest that is itself running a hypervisor pays a VM exit
on every CLGI/STGI. Here that is the sandbox Talos nodes hosting the tenant
KubeVirt VMs, so it only bites at double nesting, and the sandbox nodes' own
boot is unaffected.
Across the archived reports no run missing the flag has passed a
tenant-Kubernetes suite: 0 of 62, against 11 of 12 with it. Exposure varies
between hosts of the same shape, so it is not a property of the lane and cannot
be recovered once the runner is recycled. Recording it on every run, red and
green, is what makes the correlation countable rather than reconstructed by hand.
No new bounded read, no compiler on the runner, no sysfs. Gated on svm so an
Intel host's vnmi is not reported as an SVM feature.
Five tests for the reader. Identity suite 62 assertions, hack/ sweep 732.
No behaviour change and no gating on the flag.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
||
|
Aleksei Sviridkin
|
ad62a8d62b |
test(e2e): stop blocking merges on the tenant node-join deadline
The tenant node-join wait no longer discriminates. Nested virtualisation on the shared runners degrades far enough that a guest which registers in minutes on real hardware can miss any deadline this test can afford, so a red on that one wait says as much about the machine the run landed on as about the product, and it teaches a reader to re-run the job. The suite still fails on it. That is what keeps Chainsaw's catch running: the previous-instance container logs, the host snapshot, the data-plane capture and the event dump are all keyed on the test failing, and the report stays honest. What changes is one layer up. The wait writes a marker beside its diagnostics, and a new lane-side gate reads that marker to decide whether the job blocks. It opens only when every failed test in the run carries one, and blocks on anything it cannot attribute: a report it could not read, a failed test the report does not name, a failure with no marker. Only the lanes that block a merge ask it. The two pull-request lanes do; nightly and the tag lane run the same suites and stay hard. A soft pass on the tag lane would ship a release candidate whose tenant clusters brought up no worker nodes, and nightly is the only place an e2e failure reaches anyone who did not open a pull request, since no lane in this tree notifies anywhere. A softened red finishes the job green, so the job's result stops answering whether the suite passed, and both kinds of consumer are handed the decision instead of inferring it. A step gated on the job failing would silently stop running: the SSH breakpoint is that case, and it still opens for a pull request marked for debugging. A job that reports the result would silently start reporting a pass: the required commit status now says the deadline was missed and that the merge is not blocked on it, rather than that the suite passed. The deadline goes to 29m. That is the test's own budget and nothing more: its clock starts after the machinedeployment wait, the LoadBalancer assignment and the healthz probe, while the chart's autoscaler and health-check timers have been running since the Machine was created, so no ordering against them is claimed. The chart's 20m and 30m give a scale to pick against, and a budget over the first leaves room for a remediation cycle without promising that one fits. The operation ceiling both suites give the script rises with the deadline, and the diagnostics phase budget rises inside it, because the collectors gated ahead of the serial console cost more between them than the old budget covered. Both budget guards read their terms from the sources that set them. The e2e job cap rises with them, and its guard holds what a guard can hold: the pair's operation and catch ceilings, and one figure across every lane that runs the suite. It also records that the suites' full summed ceilings have never fit any cap in that file, so the margin above the bound is taken from a measured run instead. The prose that restates the cap elsewhere moves with it, and the sweep that holds the deadline's restatements now reads the figure spelled in words as well as in minutes. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
e6be333c06 |
test(e2e): capture CertificateSigningRequests in cozyreport
hack/cozyreport.sh never captured CertificateSigningRequests, leaving the hypothesis that a node join hangs on kubelet certificate issuance (bootstrap token, CSR, approval) unverifiable from the collected artifact. Add a bounded CSR list and a per object yaml capture for the host cluster, next to the existing node walk, and the same capture for every tenant Kubernetes cluster whose kubeconfig a failed kubernetes-* Chainsaw test left on disk. Both walks reuse the existing bounded read helpers, so a cut off or refused read produces the same truncation and failure markers every other capture in the file already writes, and an empty CSR list is not treated as an error. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
9ccb133b66 |
fix(ci): de-escalate select-e2e and name every escalation (#3817)
## Why
I replayed the 150 most recently merged pull requests through
`hack/select-e2e.sh` with every escalation branch instrumented. 118 of
them (78.7%) ran the full 21-suite Chainsaw run, and the causes, counted
once per escalating PR with the count where that cause was the only one
in brackets, were `FULL_PATTERN` 88 [68], `NO_SUITE_FOR_GROUP` 24 [24],
`CHAINSAW_SHARED` 18 [1] and `UNCLASSIFIED` 7 [4]. `YQ_BROKEN`,
`NO_GRAPH_OWNER` and `BACKSTOP_EMPTY` never fired.
Two problems come out of that. Most of the escalation is real but
unnecessary. Whole classes of path that provably cannot affect a
Chainsaw suite were escalating because no rule claimed them. And the
commonest cause of a full run was absent from the log, so "why did this
pull request run everything" could only be answered by re-deriving the
selection by hand.
## What changes
**Every escalation now names its cause on stderr.** Seven branches could
reach the full suite and four did it in silence, `full_suite_pattern`
among them, the commonest cause by a wide margin, so the usual answer
was the one the log never carried. The lines go to stderr and must stay
there: stdout is the suite list and both e2e lanes parse it, so a reason
line on stdout would be read as a suite name. A unit test asserts every
message, verified by muting each of the eleven in turn and confirming
the file goes red for all of them. A reason that regresses to silence
changes no selection and is otherwise invisible.
**`cozystack.etcd-application` gains its missing `dependsOn:
cozystack.etcd-operator`.** `packages/extra/etcd` renders `kind:
EtcdCluster` from `etcd-operator.cozystack.io/v1alpha2`, and this was
the only operator-backed application source in the tree with no edge to
its operator. It is a latent install bug as much as a selection gap:
without the edge `etcd-rd` registers the ApplicationDefinition as soon
as the engine is up, so a tenant can create an `Etcd` before the
operator exists and its HelmRelease fails on `no matches for kind
"EtcdCluster"`, and `hack/select-install.sh`'s forward closure for the
`etcd` suite omitted the operator for the same reason. On the selection
side `cozystack.etcd-operator` reached no runnable suite, so every
change to the operator ran all 21.
**`vm-disk-application` maps to the `vminstance` suite.** It resolved to
a `vm-disk` suite that does not exist, `intersect_suites()` dropped the
name, and the source reached nothing runnable. Neither did
`cozystack.kubevirt-cdi` above it, whose only other dependents are
`vm-default-images` and `vm-disk-application` itself, so every CDI
change ran all 21. The coverage was there and the table did not know it:
the `vminstance` suite creates a `VMDisk` and asserts the `DataVolume`
behind it (`hack/e2e-chainsaw/vminstance/vmdisk.yaml`,
`vmdisk-vmi.yaml`). Same defect class as the etcd edge above, on the
mapping side rather than the graph side, and with no production blast
radius of its own, since `src_to_suites` is read by the selector and by
nothing else.
**`cozystack.cozystack-basics` joins `cozystack.cozystack-engine` as a
propagation hub excluded from the reverse-dependency walk.** Every edge
into it exists so a namespace or a platform-wide policy is in place
before the dependent installs, which `kubevirt-cdi` states in its own
source ("Depend on cozystack-basics so the target namespace exists
first"), and that is install ordering rather than behaviour. The damage
runs opposite to the engine's: the engine fans one change out to every
app, basics narrows instead. It reaches no suite today so it escalates,
but it sits upstream of `kubevirt-cdi`, so the first suite to land under
CDI silently converts the platform's namespace-and-policy package from
the full run to that one suite. Not hypothetical — on #3426 enabling the
site-router suite takes `cozystack-basics` from 22 suites to
`site-router` alone, and nothing in the output says coverage was lost.
Dropping the reverse edges keeps the package reachable, stops it
propagating, and leaves a change to it running everything through the
per-path escalation.
**`hack/e2e-apps/<name>.bats` maps to the `<name>` suite**, off the
basename, exactly as the per-suite rule takes the name off a
`hack/e2e-chainsaw/<app>/` directory. Deliberately mapped rather than
marked inert: what remains there is wired to nothing after the Chainsaw
migration, and inert would bake that orphan status into the rule and go
quietly wrong the day a lane runs those files again. The basename is
membership-tested against the suite list on the spot, so an unmatched
one escalates there and then rather than having the verdict deferred to
the final intersection, where the rest of the diff would decide it.
**`packages/tests/` and three `.gitattributes` join
`inert_config_pattern`,** which is what the script's own header says to
do with a genuinely inert path instead of widening the fall-through.
`packages/tests/` is a helm-unittest fixture chart, and changing a test
*of* `cozy-lib` does not change `cozy-lib`, no PackageSource lists those
paths as a component, and nothing installs them, while a change to the
library itself still escalates through `packages/library/`. The
`.gitattributes` entries are enumerated rather than matched by filename,
because the justification is what those files contain (only
`linguist-generated` markers) and the name does not carry it:
`.gitattributes` can also set `filter`, `eol`, `working-tree-encoding`
and `export-subst`, each of which changes what lands in the working tree
and therefore what gets built.
**`full_suite_pattern` escalates only `hack/e2e-*.bats`, not every
`hack/*.bats`.** The root `Makefile` is the authority on the split and
draws it at exactly that prefix, `BATS_UNIT_FILES := $(filter-out
hack/e2e-%.bats,$(wildcard hack/*.bats))`, so the 60 files it keeps are
the unit lane and the e2e sandbox runs none of them, while the three it
filters out are what `packages/core/testing`'s recipes execute and those
still escalate. Being inert here does not leave them untested: `make
unit-tests` is gated on the `plan` job's `code` output, which
`pull-requests.yaml` computes as "any changed path outside `docs/`" and
never from `select-e2e.sh`, so a bats-only pull request still runs the
whole unit lane, plus install and the OpenAPI tests, since the `e2e` job
reads the same output rather than the selection.
## Measured effect
The before-and-after comparison below is a second measurement over a
slightly different population, stated separately because the two are not
interchangeable: it replays the last 150 first-parent merge commits on
`main` (`git diff --name-only $sha^1 $sha`) through the base selector
with base sources versus this branch with its own, where the histogram
above walks the 150 merged pull requests as GitHub lists them. The
populations overlap heavily and the direction is the same, and the
totals differ by two commits.
| | full suite | scoped | nothing |
|---|---|---|---|
| before | 120 | 20 | 10 |
| after | 106 | 25 | 19 |
Full-suite rate 80.0% → 70.7%, verdict changed on 15 commits. Fourteen
de-escalated and I read every one of their file lists: they are
unit-lane bats files, non-e2e workflows, `docs/`, `CODEOWNERS`,
helm-unittest fixtures and linguist markers. The fifteenth gets one
suite *wider*: a `cert-manager` change now also selects `etcd`, which is
correct, since `cert-manager` is a dependency of `etcd-operator` and the
etcd app now genuinely reaches it through the new edge.
That replay predates the two reachability rules, and replaying the same
150 first-parent merges across those two on their own — the selector at
`73bc4c0ae` against the selector at `adf231513`, same sources on both
sides — changes no verdict at all. Two of the 150 touch
`cozystack-basics` and both ran the full suite before and after, which
is the hub exclusion keeping something true rather than failing to do
anything, and none of the 150 touch `kubevirt-cdi` or `vm-disk`, so the
mapping shows up only in a direct selection.
A few individual selections, before → after:
| changed files | before | after |
|---|---|---|
| `hack/select-e2e_test.bats` | 21 suites | *nothing* |
| `hack/select-e2e_test.bats` +
`packages/apps/redis/templates/redis.yaml` | 21 suites | `redis` |
| `hack/e2e-install-cozystack.bats` | 21 suites | 21 suites |
| `packages/tests/cozy-lib-tests/**` | 21 suites | *nothing* |
| `packages/system/.gitattributes` | 21 suites | *nothing* |
| `hack/e2e-apps/postgres.bats` | 21 suites | `postgres` |
| `packages/system/etcd-operator/values.yaml` | 21 suites | `etcd` |
| `packages/system/kubevirt-cdi/values.yaml` | 21 suites | `vminstance`
|
| `packages/apps/vm-disk/values.yaml` | 21 suites | `vminstance` |
| `packages/system/cozystack-basics/values.yaml` | 21 suites | 21 suites
|
## Blast radius of the `dependsOn` edge
A `dependsOn` on a PackageSource is an install-ordering edge in
production, not only a test-selection hint, so this is the part worth
reviewing hardest. Both Packages are emitted by
`packages/core/platform/templates/bundles/system.yaml` under the single
`bundles.system.enabled` guard with no intervening conditional between
them, in every variant, so no supported configuration has the app
without the operator. `hack/select-install.sh --validate` reports the
graph still has no cycle and nothing dangling. The one new exposure is
an admin who lists `cozystack.etcd-operator` in
`bundles.disabledPackages` while keeping the app, which now leaves it
`DependenciesNotReady`. That is the same exposure postgres, mariadb,
kafka and redis already carry, with `Package.spec.ignoreDependencies` as
the escape hatch. On upgrade `etcd-rd` waits for the operator's two
HelmReleases, and an unhealthy `etcd-operator` already means broken etcd
apps.
## Testing
Every rule added or changed has a test in `hack/select-e2e_test.bats`,
the install-side change has one in `hack/select-install_test.bats`, and
the new graph edge has a helm-unittest guard at
`packages/core/platform/tests/sources_etcd_application_dependson_test.yaml`.
The round-trip test that walks every suite through both mapping tables
stays green, `vminstance` included now that two sources map onto it.
The two reachability rules were each checked by removing the rule and
confirming its own test reddens: without the mapping the CDI test falls
back to the full-suite escalation, and without the hub exclusion
`cozystack-basics` selects `redis vminstance` instead of everything. The
hub test has to seed its own hazard, since no suite sits downstream of
basics in this tree yet, so it gives `redis-application` an edge onto
basics in a copied sources directory and pins that a redis change still
selects redis, or the assertion would pass against a graph that resolved
nothing.
`make bats-unit-tests` aborts on the first failing file (`for f in …; do
… || exit 1; done`), and `hack/ghcr-mirror_test.bats` fails on `main`
today and sorts before `select-e2e_test.bats`, so the aggregate target
never reaches these tests. I ran the 60 unit files individually instead:
59 pass, and the one failure is that pre-existing one, byte-identical to
the base commit and referencing nothing this branch touches. Also green:
`make helm-unit-tests`, `make rd-presets-check migrations-target-check
test-check-readiness`, `hack/select-install.sh --validate`, and
`pre-commit run --all-files` leaving a clean tree.
## Two things a reviewer should know, neither addressed here
`assert_selection` in `hack/select-e2e_test.bats` aborts the whole file
with `exit 1` rather than failing a single test. That is the only thing
that works under `hack/cozytest.sh`, which provides no real `run`,
`$status` or `skip`, and it is the idiom the existing tests in that file
already use. It becomes wrong when #3497 and #3498 flatten these onto
real `bats(1)`, where a failing assert should end one test and let the
rest report. Worth converting with that work rather than ahead of it.
`cozystack.mongodb-application` carries the identical missing operator
edge: `packages/apps/mongodb` renders `psmdb.percona.com/v1` while the
source depends only on `cozystack.networking` and
`cozystack.cozystack-engine`, so `cozystack.mongodb-operator` still
reaches no runnable suite and every change to it escalates to all 21.
Same defect class and same fix shape as the etcd edge, but it is a
second production install-ordering change and belongs in its own pull
request.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **New Features**
- The default etcd application now includes its operator and required
platform dependencies, ensuring the EtcdCluster provider is available.
- **Bug Fixes**
- Improved end-to-end test selection for application-specific changes,
shared components, inert paths, and unresolved or unclassified files.
- Full-suite escalations now provide clear reasons through standard
error.
- **Tests**
- Expanded coverage for test selection, dependency reachability, and
etcd application dependency declarations.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
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> |
||
|
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> |
||
|
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> |
||
|
Myasnikov Daniil
|
adf231513f |
fix(ci): correct two reachability defects in the selection graph
Both are cases of the walk reaching the wrong set of suites for a package, and both were found by enabling the site-router suite on #3426, which is the first suite in the tree to land downstream of kubevirt-cdi. `vm-disk-application` now maps to the `vminstance` suite. It mapped to a `vm-disk` suite that does not exist, which intersect_suites() drops, so the source reached nothing runnable and neither did kubevirt-cdi above it: every CDI change ran all 21 suites. The vminstance suite creates a VMDisk and asserts the DataVolume behind it (hack/e2e-chainsaw/vminstance/vmdisk.yaml, vmdisk-vmi.yaml), so it is the suite that covers both. A CDI change now selects `vminstance` alone. `cozystack.cozystack-basics` joins `cozystack.cozystack-engine` as a propagation hub excluded from the reverse-dependency walk. Every edge into it exists so a namespace or a platform-wide policy is in place before the dependent installs -- kubevirt-cdi says exactly that in its own source -- which is install ordering, not behaviour. The damage runs opposite to the engine's: the engine fans one change out to every app, while basics narrows down instead. It reaches no suite today so it escalates, but it sits upstream of kubevirt-cdi, so the first suite to land under CDI silently converts the platform's namespace-and-policy package from the full run to that one suite. On #3426 that is 22 suites down to `site-router` alone, with nothing in the output to say coverage was lost. Dropping the reverse edges keeps the package reachable, stops it propagating, and leaves a change to it running everything through the per-path escalation. Both rules have a test, each verified to redden with its rule removed: the mapping test falls back to the full-suite escalation, and the hub test -- which seeds the hazard by giving redis-application an edge onto basics, since no suite sits downstream of it in this tree yet -- selects `redis vminstance` instead of everything. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@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> |
||
|
Myasnikov Daniil
|
73bc4c0ae3 |
fix(ci): build the backstop's reason list without a blind pipeline
`unmatched=$(echo … | tr … | sort -u | grep -v '^$' | paste -sd ' ' -)` takes its exit status from paste, so a failure in tr, sort or grep is invisible under set -e. Demonstrated with a sort stub that exits 1: the pipeline reports rc=0 and yields an empty string, so the line becomes `no runnable suite is named by ''`. That is the same last-command blindness this script already handles twice -- for the suite-list find and for the two yq indexes -- and the comments there explain why each was a bug. Selection stays fail-safe because the escalation is decided before the message is built, so the only casualty is the reason line; but the reason line is a contract as of this change set, and a contract that degrades in silence is what the change set exists to remove. Replaced with the shell's own split plus a `case` membership test: no external command, nothing that can half-succeed, dedup preserved, and the same idiom resolve_suites already uses a few lines up. A test feeds two unresolved directories with one repeated and pins the whole line, so a regression to a form that can emit a partial or empty name turns the file red rather than printing a worse message. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
c28d7804af |
fix(ci): enumerate the inert .gitattributes instead of matching by name
The rule was broader than its justification. It read `(^|/)\.gitattributes$`, so it claimed every present and future .gitattributes anywhere, while the argument for it was about content: every one in this tree holds nothing but `linguist-generated` markers, which reach no build, chart or test. The filename does not carry that argument. .gitattributes can also set `filter`, `eol`, `working-tree-encoding` and `export-subst`, each of which changes what lands in the working tree and therefore what gets built, so a by-name rule would eventually make a live file inert with nothing to notice it. Nothing is lost by narrowing, because only three files ever reached the rule: api/.gitattributes and internal/crdinstall/manifests/.gitattributes are escalated by full_suite_pattern first, so entries for them would be dead. Measured against the base selector, those three are exactly the paths whose classification this branch changes, and they still select nothing. Same trade as the workflow enumeration a few lines up, and the same upkeep, which the comment now states: after adding or moving one, re-check with `git ls-files '*.gitattributes' '.gitattributes'` and confirm the contents are still only linguist markers. A .gitattributes not on the list is classified by whatever rule its path falls under -- the graph inside a package, the unclassified fall-through at the root -- and both fail safe. A test pins that root case, so a future widening back to a by-name match turns the file red. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
2d6a7ec777 |
test(ci): pin the unclassified-path reason line
The header claims "a test asserts each line" about the reason-line contract. It was false for one branch: deleting the `unclassified path` echo at the fall-through left `hack/cozytest.sh hack/select-e2e_test.bats` exiting 0. That is the worst one to leave unpinned. It is #3392's own guard, its cause is the one a reader cannot infer from the selection ("a path nobody has classified" is not expressible as a suite list), and the only action it asks for is to classify the path it names. The word "unclassified" appears in seven comments in the test file while pinning it zero times, so the branch read as covered. Verified by mutation both ways: with the assertion added, deleting the echo turns the file red, and a sweep that mutes each of the eleven `select-e2e:` messages in turn now reddens the file for every one of them, so the header's claim holds for the whole set rather than for most of it. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
b2702621d3 |
fix(ci): decide the per-app bats rule per path, not after the merge
Two defects in the hack/e2e-apps/ arm added by the previous commit. The arm set trigger_any=1 for whatever basename it derived and left the verdict to the final intersection. That makes the escalation depend on the REST of the diff: hack/e2e-apps/monitoring-oidc-system.bats alone emptied the selection and the backstop escalated, but the same file beside packages/apps/redis/templates/redis.yaml selected `redis` with nothing on stderr, where the pre-change selector ran all 21 suites. That is the merge-before-escalate shape #3330 removed from the graph walk, reintroduced one rule over. The basename is now membership-tested against the suite list on the spot and an unmatched one sets trigger_full=1 and names the file. POSIX case matches `/` with `*`, so the arm also sees nested paths, and the unanchored capture turned hack/e2e-apps/fixtures/postgres.bats into the "suite" fixtures/postgres -- right outcome by accident, reason line printing a path where a suite name belongs. The capture is anchored to one segment, and what the arm does with anything that is not a top-level <name>.bats is now stated rather than left to fall out of a regex: it is shared material for those suites the way hack/e2e-chainsaw/_lib/ is for the Chainsaw ones, so it escalates for the same reason. Both claims that said otherwise are corrected -- the arm's own comment and docs/agents/e2e-testing.md both asserted the deferred escalation was equivalent to the old fall-through, which held only for a single-file diff. The backstop comment drops the e2e-apps clause it gained last commit, since that route no longer reaches it. The header's "four of the seven" becomes count-free: this commit adds two escalating branches, so a hardcoded total was already stale. Tests: the mixed-diff case is the regression pin (the isolated case passes with the escalation deferred and cannot see the bug), plus a nested-path case and a non-.bats case. Both fixes are mutation-verified -- restoring either defect turns the file red. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
4d4f5edf92 |
fix(ci): escalate only the e2e bats files, not the whole unit lane
full_suite_pattern took every hack/*.bats, so all 63 of them ran the whole Chainsaw suite -- including the 60 that are the unit lane and that the e2e sandbox never executes. The root Makefile is the authority on the split and draws it at exactly the prefix this narrows to: BATS_UNIT_FILES := $(filter-out hack/e2e-%.bats,$(wildcard hack/*.bats)) The 60 files it keeps run under `make unit-tests`; the three it filters out -- e2e-prepare-cluster, e2e-install-cozystack, e2e-test-openapi -- are the ones packages/core/testing's recipes execute inside the sandbox, and those still escalate. So no Chainsaw suite can regress from an edit to one of the 60. Inert here is not a green gate with nothing behind it. `make unit-tests` is gated on pull-requests.yaml's `code` output, computed in the workflow as "any changed path outside docs/" and never from select-e2e.sh, so a bats-only pull request still runs the whole unit lane. The `e2e` job reads the same output, so it still installs the platform and runs the OpenAPI tests before skipping Chainsaw. Two tests: one pins the rule on both sides of the prefix and asserts an inert bats file beside an app path does not mask the selection, and one walks every hack/*.bats in the tree and requires the selector's verdict to agree with the Makefile's split, so a file named to fit neither lane surfaces here rather than on the next PR that touches it. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
33428671fa |
fix(ci): classify hack/e2e-apps/ and packages/tests/ in select-e2e
Three path classes matched no rule in hack/select-e2e.sh and escalated to all 21 Chainsaw suites through the unclassified fall-through -- 7 of the last 150 merged pull requests between them. hack/e2e-apps/<name>.bats now maps to the <name> suite, off the basename exactly as the per-suite rule takes the name off a hack/e2e-chainsaw/<app>/ directory. Not marked inert: what is left in that directory after the Chainsaw migration is wired to nothing, and inert would bake that orphan status into the rule and go quietly wrong the day a lane runs those files again. A basename no suite carries drops out of the intersection at the bottom of the script and escalates through the backstop, which is the outcome those paths already had, so mapping optimistically loses nothing and gains the correct answer for every name that is a suite. packages/tests/ and every .gitattributes join inert_config_pattern, which is what the script's own header says to do with a genuinely inert path rather than widening the fall-through. packages/tests/ is a helm-unittest fixture chart: changing a test OF cozy-lib does not change cozy-lib, no PackageSource lists those paths as a component, and nothing installs them -- a change to the library itself still escalates through packages/library/. .gitattributes is matched by name wherever it lives because every one in this tree is a linguist-generated marker, which reaches no build, chart or test; one under a tree full_suite_pattern already covers still escalates, since that pattern is checked first. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
42bd45a87c |
fix(platform): make etcd-application depend on etcd-operator
extra/etcd renders kind: EtcdCluster from etcd-operator.cozystack.io/v1alpha2, but cozystack.etcd-application declared no dependsOn edge to cozystack.etcd-operator -- the only operator-backed application source in the tree that did not. The edge has been absent since the sources were first added. Two consequences, one per side of the graph. On install this is a latent ordering bug. Without the edge the etcd-rd HelmRelease registers the ApplicationDefinition as soon as cozystack-engine is up, so a tenant can create an Etcd before the operator exists and its HelmRelease then fails with `no matches for kind "EtcdCluster"`. hack/select-install.sh's forward closure for the etcd suite omitted the operator for the same reason, which would install the suite against a cluster with no CRD to apply to. On test selection cozystack.etcd-operator reached no runnable suite, so every change to the operator escalated to all 21 suites -- 5 of the last 150 merged pull requests. The etcd suite exists; only the edge that makes it reachable from the operator was missing. Scope of the ordering change: both Packages are emitted unconditionally by the same bundle (templates/bundles/system.yaml, under the single bundles.system.enabled guard, in every variant), so no supported configuration has one without the other. An admin who lists cozystack.etcd-operator in bundles.disabledPackages while keeping the app now leaves it DependenciesNotReady -- the same exposure postgres, mariadb, kafka and redis already carry, with Package.spec.ignoreDependencies as the escape hatch. select-install.sh --validate confirms the edge adds no cycle and nothing dangling. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
3c3f9450ae |
fix(ci): name the cause on every select-e2e escalation
Seven branches in hack/select-e2e.sh escalate to the full Chainsaw suite and four of them did it in silence: the shared hack/e2e-chainsaw/_lib/ and .chainsaw.yaml case, the full_suite_pattern match, a packages/ path no PackageSource claims, and the empty-selection backstop. "Run everything" is the same 21 suite names whichever rule produced it, so a reason line is the only thing that tells the seven apart. full_suite_pattern is the commonest cause of a full run by a wide margin -- 88 of the last 150 merged pull requests, 68 of them with no other cause -- so the usual answer to "why did this pull request run the whole suite" was the one the log never carried, recoverable only by re-deriving the selection by hand. Each new line names the path that caused it rather than only the rule, and the backstop names the directly-selected names it could not resolve. They go to stderr, never stdout: stdout is the suite list and both e2e lanes parse it, so a reason line there would be read as a suite name. A unit test asserts each of the four messages. A reason that regresses to silence changes no selection, so it is invisible to every other test in the file. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@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> |