mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-30 00:27:04 +09:00
docs/agents-dev-principles
158
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
c1fe1e9cb2 |
ci(e2e): say what the 215-minute e2e budget actually covers
The comment on every lane claimed the budget fits two tenant Kubernetes suites failing at their node-join deadline. A green full suite on the container lane already spends most of it, and a suite that runs into the deadline costs about 25 minutes more than one that passes, so the claim does not hold. State the real bound so a cancelled run is read as a red suite, not as a budget to raise. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
da28ea7ee8 |
ci(e2e): run nightly and release-tag e2e on the container lane
Nightly and the rc gate still booted the management nodes as QEMU guests, the substrate the merge-gating lanes left behind. On it the tenant Kubernetes suites have missed their node-join deadline on most nights since 2026-10-02, each failure spends the whole deadline, and the job reaches its timeout before the alphabetically later suites run. Over the same days those suites passed on every PR run that selected them, where the nodes are Talos containers and a tenant worker sits one virtualization level shallower. Both lanes now use the PR lanes' container substrate. Nightly's e2e no longer waits for the nocloud disk, which it does not boot; the disk is still built and published. A manual e2e-tag dispatch against a tag that predates the container lane fails early and names the ref to dispatch from instead. DRBD, the replicated StorageClass and the Cozystack Talos node image lose their last e2e coverage with this. The docs and comments that pointed at nightly and the rc gate as that coverage now say so. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
387760b94e |
test(tests): run unit suites with Bats and retain POSIX coverage (#3849)
## What this PR does This runs the unit suite with Bats and keeps a separate `dash` pass for tests that source production shell helpers. Unit files are discovered from the filesystem, and each loads the audited helper that restores `set -u` in test bodies without changing subprocess shell options. CI pins Bats 1.14.0, keeps JUnit reports, and runs Bats for docs-only changes too. Code changes also run Helm, Go, controller and POSIX compatibility tests. Per-file Make targets and the `--keep-going` sub-make are removed because one `bats -j` run goes through all files anyway, so a red file doesn't stop the others, and `hack/bats-unit-keep-going.bats` checks this with real bats. `-k` stays in the workflow because without it a red Bats file keeps `test-controllers` from starting. Local tests require bats-core 1.5 or newer. GNU parallel is optional, and the complete local hook runs with `pre-commit run bats-unit-tests --hook-stage manual --all-files`. Live-cluster suites keep their existing runner. ### Validation `make unit-tests test-controllers -j4 -k --output-sync=target BATS_JOBS=4 BATS_POSIX_SHELL=dash` passed, including every discovered Bats file and the POSIX pass. Mutation checks reject omitted files, disabled or exported nounset, missing CI steps and missing or excessive POSIX coverage. ### Downstream repositories The developer guide update is tracked separately in [cozystack/website](https://github.com/cozystack/website/issues/717). CCP has no caller of the changed targets. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - documentation follow-up: [issue](https://github.com/cozystack/website/issues/717) - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Testing & Reliability** * Bats unit tests run as a dedicated suite, with optional parallel execution and JUnit reports. * Pull request checks run Bats tests for documentation-only changes, and unit and controller tests for code changes. * POSIX-shell compatibility tests are included in the standard unit-test run and continue after individual failures. * Test reports are uploaded even when checks fail. * Added a manual local test hook for running Bats and POSIX compatibility tests. * Shared strict setup is applied across Bats unit tests. * **Documentation** * Updated testing guidance with current commands, runner compatibility, and CI coverage. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
3e078b79a9 |
ci(e2e): refuse a new fork image before the build starts (#4767)
## What this PR does A fork PR that adds a first-party image cannot pass the fork e2e lane, and today it finds that out only after the whole build. `e2e-fork.yaml` pushes the fork's image archives only under names the base tree already exports, so a new image is refused at the push with `image '<name>' is not in the base-tree allowlist`. Nothing the contributor does on the PR changes that. This PR keeps the push allowlist as strict as it is and moves the failure to the start of the run. On fork PRs, `plan` in `pull-requests.yaml` now runs `hack/fork-new-images.sh`. The script derives the image names of the PR tree and of its base parent with the same `make -n` derivation the push step uses. If the PR tree exports a name the base does not, it fails before any build, names the images and says what has to happen: a maintainer lands the image on the base branch first, or the change runs from a branch in this repository. The check runs from the fork's own tree, so a fork can remove it. That is fine, because it only saves an honest contributor a wasted build. The push allowlist in `e2e-fork.yaml` is still the boundary. That workflow cannot call the script, since it runs from the default branch over an untrusted checkout. So `hack/fork-new-images.bats` pins the two derivations to each other, and also covers the script's behaviour and the fork-only gating of the step. I replayed the script against the merge commit of #4291 and its base parent. It fails with `cluster-api-provider-kubevirt`, the same name the push step refused there. The workflow_run lane itself cannot be exercised locally. This does not lift the limitation itself: a fork still cannot get a green run for a new image. Part of #4329. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note ci(e2e): a fork PR that adds a new first-party image now fails in the plan job, naming the image, instead of after the whole build at the image push. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Fork pull requests that add image names not present in the base branch are now rejected during planning, with the new names reported before builds begin. * **Documentation** * Updated guidance describes the early check and clarifies how it relates to the image allowlist used during publishing. * **Tests** * Added coverage for new, unchanged, reduced, and empty image sets, along with checks that the pull request workflow applies the check only to fork submissions. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
cc942fed68 |
test(tests): run unit suites with Bats and retain POSIX coverage
The shell translator rewrites test bodies and loses Bats reporting. The unit lane needs complete TAP results and JUnit reports without changing how live-cluster suites run. Sourced production helpers still need a dash pass, and unit test bodies need nounset without exporting shell options into subprocesses. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
bcfa50d7e9 |
test(e2e): install only the packages the selected suites need
The E2E job installs the whole platform on a fresh cluster before it knows which suites will run, so every PR pays install time and flake surface for packages its suites never use. Select the suites first and pass the complement computed by select-install.sh to the platform Package as disabledPackages. The full-e2e label and an empty selection keep the full install, and the nightly and tag lanes pass no list, so they are unchanged. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
80eb2e5037 |
ci(e2e): refuse a new fork image before the build starts
The fork e2e lane pushes a fork's image archives only under names the base tree already exports, which keeps a fork from minting repositories under the CI registry. An image the PR itself adds is therefore refused, but only at the push, after the whole build has run, and nothing the contributor does on the PR can clear it. Compare the image names of the PR tree and its base in the plan job of fork runs, with the same make -n derivation the push allowlist uses, and fail there naming the new images and what has to happen first. The push allowlist is unchanged and remains the boundary: this check runs from the fork's own tree, so it only saves an honest contributor the wasted build. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2af8b0c229 |
ci(build): fall back to direct, retry and cache Go module downloads (#4660)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does One transient error from proxy.golang.org is enough to fail an image build or the `Unit & controller tests` job today. The error is an HTTP/2 stream reset (`stream error ... INTERNAL_ERROR`), and nothing retries or falls back after it. This PR adds three things: a direct fallback, a bounded retry and a module cache. Image builds now fall back to fetching modules from their origin when the proxy fails. They get `GOPROXY=https://proxy.golang.org|direct` as a build arg from `hack/common-envs.mk`, and every Go builder stage declares `ARG GOPROXY`. With a pipe Go goes direct on any proxy error. With Go's default comma it does that only on a 404 or 410. The value comes from `IMAGE_GOPROXY` and not from the host's `GOPROXY`, because buildkit can't reach a mirror on localhost or a `file://` proxy. Every workflow job that runs Go on the runner sets the same value as env. Explicit module downloads in Dockerfiles are retried three times with a growing pause. The loop is inline, in one form that the contract test pins. I tried a shared script from a named build context first, but buildx refuses any `--build-context` for the four Dockerfiles still pinned to the `dockerfile:1.2` frontend. The images also build from different contexts, so no single file can be copied in. Raising those pins changes how those images build and needs a real build to check, so I left it for a follow-up. Two alpine stages, token-proxy and shadowbox's `ss-server`, left the download to `go build`. The alpine golang images have no git, so the direct fallback can't fetch for them, and they now get the same download step. The runner jobs use `hack/go-mod-download.sh` with the same bound. The unit test job now starts from a module cache instead of downloading everything. It restores `GOMODCACHE` with `actions/cache`, keyed on every `go.sum` in the tree, and then prefetches all tracked modules through the retry, so a cache miss still works. A cache saved by a PR run is visible only to that PR, but one saved on the default branch can be restored by any PR ([docs](https://docs.github.com/en/actions/reference/workflows-and-actions/dependency-caching#restrictions-for-accessing-a-cache)). That is why the build cache warmer on main writes the same key before its image build. This way the first run of a PR, which is the one that flakes, starts warm. The warmer restores only an exact key, so old module versions don't pile up, and its Go steps can't fail the image build. Both runner labels are Linux, so the key matches. Both also check out under `/home/ubuntu/_work`, so `go env GOMODCACHE` should give the same cache path, but I have not seen it on the warmer's runner yet. The release jobs in `tags.yaml` and `promote-rc.yaml` get the GOPROXY env only, no cache. `hack/go-module-fetch-contract.bats` covers all three. Each test is red on main, and I checked each guard with a mutation. Closes #4591 ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note ci(build): Go module downloads in image builds and CI jobs now fall back to direct fetching and retry on transient proxy.golang.org errors. The unit test job reuses a module cache written from main. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Reliability** - Go module downloads now fall back to direct access when the public proxy encounters errors. - Builds and automated checks retry failed module downloads, helping them recover from temporary network issues. - **Performance** - CI caches Go modules to reduce repeated downloads and speed up builds and tests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
6df5d076b1 |
ci(workflows): pin flux, yq and helm-unittest installs
Every flux install fetched fluxcd.io/install.sh and ran it as root, so whatever that host served at the moment executed on runners that later hold registry credentials. Use the upstream setup action instead, pinned by commit and given an explicit version: it checks the release tarball against that release's checksums file and runs no script fetched at run time. nightly and the fork e2e lane still took yq from the floating latest release. They now use the pinned, checksum-verified install the promotion steps already have. The helm-unittest plugin embeds its own Helm and was installed at whatever version was newest, so a plugin release could change chart test results on a PR that touched no chart. Pin it, and replace a different version a persistent runner may carry. A contract test scans every workflow for these three tools, so an unpinned flux, yq or helm-unittest install copied from an older step fails the unit lane. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2d0c359f4c |
test(e2e): stop bringing up the monitoring instance nothing looks at (#4023)
## What this PR does This PR turns off monitoring instance of root tenant in e2e install and removes waits on it. Install patched root tenant with `monitoring: true` and then waited up to 15m each on vmalert and vmalertmanager, `vlclusters/generic`, shortterm and longterm vmclusters, `grafana-db` CNPG cluster and grafana deployment, but nothing in the run looks at any of it. No chainsaw suite touches tenant-root monitoring objects. Only one that mentions monitoring is `foundationdb`, it asserts `WorkloadMonitor` whose CRD comes from `cozystack-controller` and exists without monitoring stack. Monitoring OIDC wiring is covered by `packages/system/monitoring/tests/oidc_test.yaml` since #4019 removed orphaned BATS suites. Kubernetes suites run with `monitoringAgents.enabled: false` and tracing is off by default (`tracingStorages` is empty), so they don't depend on this instance either. Those waits were not free. Monitoring stack competes with rest of install for node capacity, and removed block own comment said slow victorialogs start can fail install on PR that never touched monitoring - that's why it was moved to 15m budget. ### What stays Monitoring package stays installed, because `tenant-application` has `dependsOn cozystack.monitoring-application` and without the package platform release is never resolved. Only tenant-root instance is off. Container lane `logsStorages` override for victorialogs also stays. It does nothing while monitoring is off, but keeps a run that turns monitoring back on from creating immutable replicated PVC on lane that has only `local`. Its comment, paragraph in `docs/agents/e2e-testing.md` and comment on Install step env in `pull-requests.yaml` now say that instead of calling root monitoring install-time workload. ### What is lost `curl` through `root-ingress-controller` with `Host: grafana.example.org`, it was the only check that root ingress actually routes to backend. Grafana was the target just because it was installed. Moving it to seaweedfs host needs expected response checked on live stand first, so it's tracked in #4615. ### Savings Not measured. The happy path never spent the 15m budgets, so what this returns is real bring-up time of the stack plus tail risk, and only CI runs can show how much it is. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note NONE ``` |
||
|
Aleksei Sviridkin
|
5eee22fe28 |
ci(workflows): cache Go modules and fall back to direct in Go jobs
The unit and controller test job has no module cache and runs on an ephemeral runner, so every run fetches every module from proxy.golang.org, and one transient stream reset fails the job. It now restores the module cache keyed on every go.sum in the tree and prefetches the modules of every tracked go.mod through a bounded retry, hack/go-mod-download.sh. The cache is written from main by the build cache warmer under the same key, because a cache saved by a PR run is visible to that PR only, while one saved on the default branch is readable from every PR. Every job that runs Go on the runner sets GOPROXY=https://proxy.golang.org|direct so any proxy error falls back to the origin. The release jobs get only that, not a cache. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
312409ecd2 |
test(e2e): fail the suite whose pods outlive its cleanup (#4492)
## What this PR does A Chainsaw suite could pass while the workloads it created kept running after its own cleanup, and nothing noticed until a later suite failed. On one E2E run the brokers of four Kafka releases from suites that had passed and deleted their applications (#4488), plus a TenantControlPlane sitting on its finalizer with two apiserver pods (#4489), pushed pod memory requests to the node limit. Harbor then failed on `Insufficient memory` two suites later, under the wrong name. This makes the suite that leaked fail, and name what it left behind. - Chainsaw v0.2.15 has no hook that runs after a test's cleanup: its `Configuration` carries `error.catch`, which runs only on failure. So `make test-chainsaw` now runs `hack/e2e-chainsaw-run.sh`, which invokes `chainsaw test` once per suite directory and checks the cluster between invocations with `hack/e2e-chainsaw/_lib/leak-guard.sh`. - Attribution: a pod is charged to a suite when all three hold. It is not terminal. Its UID was not in the pod list taken before the suite. The root of its ownerReferences chain was created at or after the suite started. A platform pod replaced mid-suite by a rollout, an eviction or a crash traces back to an object the install created, so it is not charged. A pre-existing pod adopted by a controller recreated mid-suite is excluded by the UID baseline. A leak is charged once, to its own suite. - The suite start is read off an apiserver, from the `creationTimestamp` of a server-side dry-run create, rather than off the sandbox. With several control-plane nodes the reading and a root's timestamp can come from different apiservers, so the rule assumes their clocks agree more closely than the gap between the reading and the suite's first object; no skew margin is added. Source for that behaviour: the generic registry `Store.create` stamps it through `rest.FillObjectMetaSystemFields`, then `DryRunnableStorage.Create` copies it back without persisting (`staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go` and `dryrun.go` at Kubernetes v1.33.0). - Budget: charged pods get 300s to disappear, the default `timeouts.cleanup` in `.chainsaw.yaml`; a unit test keeps the two equal. A suite that overrides its own cleanup timeout (kafka sets 8m) is covered the same way, because the check runs only after `chainsaw test` has returned, which is after every cleanup in the suite has finished under its own timeout; the 300s start from there. The leaks this targets sat about 110 minutes (Kafka brokers) and 30+ minutes (the TenantControlPlane) past their suites. Every green suite logs `leak-guard: suite=<dir> clear after Ns`, so the number can be checked against later runs. - Past the budget the run prints `LEAKED-PODS suite=<dir> budget=300s count=N`, then one line per pod with its phase, deletion state, root and `app.kubernetes.io/instance`. It raises a `leaked-pods` error annotation and adds a failed `leak-guard` test case to `chainsaw-report.xml`. The run continues through the remaining suites and exits non-zero at the end, as the single invocation did on a red test. - A suite that already failed on its own is checked once without the wait. Its leftovers are reported as a `::warning` with `LEAKED-PODS suite=<dir> checked-once=chainsaw-failed`, since some may still be tearing down. - Each pod gets one `kubectl wait` inside the remaining budget, and nothing is retried. A timeout or a spent budget gives the verdict from the last good pod list, also when the re-list after it fails, so a leak fails the suite. A wait that fails only after running for its whole `--timeout` counts as a timeout whatever it printed: kubectl maps an expired context to "timed out waiting for the condition" only inside the watch, so a deadline that passes during its initial lookup surfaces as "context deadline exceeded" (`wait.go` and `delete.go` in kubectl v1.33.0). A spent budget is caught before calling kubectl and never passed as `--timeout=0`, which means "check once". - Any other error on a wait or a re-list, before the budget is spent and before any timeout, stops the waiting and is reported as `LEAKED-PODS suite=<dir> count=N inconclusive="<error>"`. That is a warning, not a leak failure, so an apiserver blip cannot become a leak red. - A failed first reading after a suite, including a pod list that cannot be parsed, leaves the suite unchecked. It fails with a `leak-guard` annotation and a report case saying the cluster could not be read, not that it leaked. Errors that are not leaks carry the annotation title `leak-guard`, not `leaked-pods`. - Known leaks are listed in `hack/e2e-chainsaw/_lib/leak-allowlist.txt` as `<suite-dir> <issue-ref>`: `kubernetes-latest` and `kubernetes-previous` against #4489 (both suites create a TenantControlPlane through `_lib/run-kubernetes.sh`). `kafka` and `kafka-metadata` were listed against #4488 until #4490 fixed it; those entries are gone. An allowlisted suite prints its leak as a `::warning` and does not fail. An entry that names a directory that is not a suite stops the run before the first suite, so a rename cannot keep an exemption alive. The PR fixing each issue deletes its lines. Suite selection is unchanged. The runner runs the named directories when there are any, and otherwise every directory directly under `hack/e2e-chainsaw/`, each handed to Chainsaw's own discovery; it refuses a tree with a test at its top level, where that split would stop being exact. I verified this with Chainsaw v0.2.15's own `discovery.DiscoverTests`, built from source at that tag. For the full tree, for `kafka redis harbor` and for `kubernetes-latest`, the union of the per-suite discoveries equals what a single `chainsaw test [suites]` discovers: 57, 7 and 2 tests, identical lists. 57 matches the test count in an earlier full run's report. A unit test pins that the runner passes chainsaw the suites and flags it was given, in order. The order changes too: suites used to run in Chainsaw's own order and now run one directory at a time in alphabetical order, which moves the kubernetes suites after the kafka ones. Before #4490 fixed #4488 this starved them (see the E2E criterion below). With the fix merged the order has no effect, and a future leak that lands before a memory-heavy suite is named at the suite that leaked. One behaviour does change: a selected name that is not a directory used to fail the single invocation before any test ran; now only that suite fails and the others run. `chainsaw-report.xml` keeps its path and shape: one `<testsuites>` root with the summed counts, one `<testsuite>` per suite directory. Its consumers are the `Collect chainsaw report` / `Upload chainsaw report` steps in `pull-requests.yaml`, `e2e-fork.yaml`, `nightly.yaml` and `e2e-tag.yaml`, which `docker cp` the file and upload it as an artifact. Nothing in the repository parses it. The merge is unit-tested on two per-suite reports in Chainsaw's real shape, one of them failed, and the result is checked to parse as XML. Tests: `hack/chainsaw-leak-guard_test.bats` (31 tests, in `make unit-tests`; named without the `e2e-` prefix, which the unit lane excludes) covers the attribution rule, the budget, the allowlist, the merge and the runner, all against stub `kubectl` and `chainsaw`. Every rule was mutation-checked: removing the guard call, having the Makefile bypass the runner, inverting the allowlist match, flipping or tightening the root-timestamp comparison, removing the UID baseline, removing the terminal-phase filter, skipping the budget wait, changing the budget, lowering or raising the depth cap, accepting a stale allowlist entry, reading an unreadable pod list as empty, dropping the leak case from the report, skipping directories in the selection, treating any failed owner read as a missing owner, dropping the separate report for a suite that already failed, dropping the not-run reason from the report, reading any wait error as a timeout, removing the inconclusive branch, letting a failed re-list after a timeout turn a leak inconclusive, ignoring a failed re-list, reporting an unreadable cluster as a leak, reading an unparsable pod list as clean, reporting a kubectl warning instead of the error, passing the report name without its extension, wording a failed re-list as a failed wait, dropping an allowlist's last line when it has no newline, dropping the check that `--set-string` carries a value, and reading a wait that ran its whole timeout but printed a context error as inconclusive. Each one turns its named test red, and restoring turns it green. E2E criterion, registered before the run: in the `Run E2E tests` log of this PR, every suite that ran prints `leak-guard: suite=<dir> clear after`, a `LEAKED-PODS suite=<dir>` line carrying `allowlisted=`, or one carrying `checked-once=` (a suite that failed on its own) or `inconclusive=` (waiting stopped on a failed read), and no line matches `LEAKED-PODS suite=.* budget=`. With #4490 merged, `kafka` and `kafka-metadata` are no longer allowlisted and must print `leak-guard: suite=kafka clear after` and `leak-guard: suite=kafka-metadata clear after`, which is the live check that #4490 fixed #4488. The first run of this PR (before #4490, run 36223896793) is the positive control: it printed `LEAKED-PODS suite=kafka count=15` and `suite=kafka-metadata count=8`, allowlisted, and every other suite cleared in 0-37s. That run also went red on both kubernetes suites, because those 23 leaked brokers left no memory for the second tenant worker VM (`0/3 nodes are available: 3 Insufficient memory`), which the order change described above made possible. The guard has not yet run against a real cluster, so the first run of this PR should carry the `full-e2e` label; its `clear after` lines are the first measured teardown times, and any unexpected `LEAKED-PODS` line is either a real leak or a false positive to understand before merge. Follow-ups, known and out of scope here: - The allowlist exempts a whole suite, so a new leak in an allowlisted suite stays hidden until the issue's fix removes the line. - Attribution is per suite directory, not per Chainsaw `Test`; the leak lines carry the release instance, which usually names the test. - Only pods are checked. A leaked PVC, LoadBalancer IP or bucket holds no node memory and is not seen. - The 300s budget rests on Chainsaw's cleanup timeout, not on a measured pod-gone latency; the `clear after` lines are where that measurement will come from. - A platform object recreated mid-suite (for example a HelmRelease uninstall/reinstall remediation, or a force-replace on an immutable field) has a root newer than the suite start, and its pods would be charged to whichever suite was running as a leak; the first CI runs are where to look for this. - The per-pod wait recomputes the remaining budget before each pod, and no unit test pins that. - `kubernetes-previous` is allowlisted for #4489 although the stuck finalizer was seen only in `kubernetes-latest`; both suites create a TenantControlPlane the same way, and the exemption hides any new leak there until #4489 is fixed. - Each clean suite now waits for its own pods to finish terminating before the next starts, where the tail used to overlap the next suite. The first full run's `clear after` lines say what that costs against the E2E job's 215-minute cap. - After a timeout followed by a failed re-list, the leak lines come from the list taken before the wait, so a pod whose own wait had already succeeded is still named and the count can be too high; the verdict is unaffected. - Likewise, when every wait succeeded and the re-list then fails, the inconclusive report still lists the pods whose deletion was just confirmed. In the boundary case where the budget runs out during the last successful wait and the re-list then fails, the result is inconclusive rather than a verdict from the last good list, because the guard only treats the budget as spent when it finds it exhausted at a check. - A mistyped `COZY_LEAK_ALLOWLIST` path passes allowlist validation; it fails closed, since no suite is then exempt. - An allowlisted suite is not waited on, so `kubernetes-latest` and `kubernetes-previous` will print `LEAKED-PODS ... allowlisted=` on most runs even for pods still in normal teardown; waiting out the budget first and applying the allowlist only to what remains would stop that noise. - A slow teardown in each of several suites can add up to 300s apiece, against the E2E job's 215-minute cap. On the first full run of this PR (run 36223896793) the `clear after` values of the suites that were waited on summed to 210s; the last full run before this change took 158 of the 215 minutes. - A pod whose owner's resource type is no longer served gets "the server doesn't have a resource type" rather than NotFound, so its suite is reported unchecked instead of leaked; it still fails. - `kubectl wait --for=delete` waits for deletion, while the guard skips pods in `Succeeded` or `Failed`; a pod that finishes but is never deleted (a Job under a leaked root) spends the rest of the budget before the suite shows `clear`. The verdict is right; only job time is lost. - The unit tests do not pin the one-line empty report Chainsaw writes for a directory with no tests (`_lib`), nor the branch that catches a spent budget before calling `kubectl wait`; both are verified against the Chainsaw and kubectl sources rather than by a test. - The runner's per-suite report directory from `mktemp -d` is not removed; the sandbox is discarded after the job. - Each non-suite directory under `hack/e2e-chainsaw/` (today `_lib`) costs one empty Chainsaw invocation, because the runner leaves the decision of what is a test to Chainsaw. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff adds files under `hack/`, changes the `test-chainsaw` target, and rewords one comment and the failure line in the four E2E workflows. The trigger map's `hack/` entry is for cozystack/ccp, whose skills anchor on `hack/package.mk`, `hack/common-envs.mk` and `make generate`, none of which change here. A GitHub code search for `chainsaw` across the cozystack organisation finds hits only in this repository. ### Release note ```release-note test(e2e): the E2E app suite now fails the suite whose pods outlive its own cleanup by more than 300s, naming each leftover pod and its owner; suites with a filed leak are listed in hack/e2e-chainsaw/_lib/leak-allowlist.txt and only warn. ``` |
||
|
Aleksei Sviridkin
|
4d7f9e485a |
docs(ci): say why the overlay's ancestry tie goes to main (#4469)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does This fixes the comment on the overlay's ancestry tie, which described the wrong reason for the tie. No behaviour changes. The comment said a line fresh off main is the same as main. That holds only for the graph. `cozystack-packages:main` is built from main's tip, so once main moves past the cut, a stack on such a line gets newer images than the line. The comment now says so. It also says why the tie still goes to main: a branch forked from main before the cut ties the same way, and that case is far more common. This follows up on a review note in #4460 and is stacked on it. ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note docs(ci): explain why the base-branch overlay sends an ancestry tie to main. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified how artifact selection works for stacked pull requests on release lines without line-specific commits, including why `main` is preferred in a tie and how that may affect image freshness. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
71a943277f |
build(make): keep running bats unit files after one fails
Each bats file is its own make target, so a plain `make bats-unit-tests` stopped scheduling files at the first red one and never ran the rest, while the output looked like a finished suite. CI passes -k and was not affected; a local run was. The target now runs the files in a --keep-going sub-make, which still exits non-zero when any file fails and shares the caller's job slots and output sync. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov Daniil
|
57bf10acae |
test(e2e): stop bringing up the monitoring instance nothing looks at
The install turned monitoring on in the root Tenant and then waited up to 15m each on vmalert and vmalertmanager, vlclusters/generic, the shortterm and longterm vmclusters, the grafana-db CNPG cluster and the Grafana Deployment. No suite inspects any of it. The one Chainsaw suite that mentions monitoring, foundationdb, asserts a WorkloadMonitor, whose CRD ships with cozystack-controller and exists without the monitoring stack. The OIDC wiring of the monitoring chart is covered by its helm-unittest suite rather than by the e2e run. Those waits were not idle. The stack competes with the rest of the install for node capacity, and their own comment recorded that a slow VictoriaLogs start could fail the install on a pull request that never touched monitoring. The run was paying bring-up time and flake risk for a component it never looked at. The monitoring package stays installed: tenant-application declares dependsOn cozystack.monitoring-application, so dropping the package would leave the platform release unresolved. Only the tenant-root instance is turned off. The container lane's VictoriaLogs storage override stays. It does nothing while monitoring is off, and it keeps a run that turns monitoring back on from creating a replicated PVC on a lane that only has local storage. One check goes with the Grafana waits: the curl through root-ingress-controller at grafana.example.org, the only assertion that the root ingress routes to a backend. Grafana was its target because it happened to be installed, and pointing it at a seaweedfs host needs the expected response checked on a live stand first. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
941101b65e |
ci: drop unused packages:write from the pull-request workflow
The build, build-talos and finalize jobs each requested `packages: write`, but none of them writes to GHCR. They authenticate to OCIR with OCIR_USER/OCIR_TOKEN and push to REGISTRY, which the workflow sets to OCIR. The only GHCR login in the file is in verify-release-candidate, and that job holds `packages: read`. GHCR pushes elsewhere in CI authenticate with GITHUB_TOKEN plus packages:write rather than a stored secret, so on those paths the scope is the whole credential. Granting it on the pull_request lane hands every same-repo PR run a token with GHCR write authority that the lane has no use for. Whether a package honours that write depends on its Actions access setting, which lives outside this repo, so this is least-privilege hygiene rather than a fix for a demonstrated exploit. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
e126158404 |
fix(ci): keep the controller Go tests running when a unit test fails first (#4233)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does The `checks` job in `.github/workflows/pull-requests.yaml` runs `make unit-tests test-controllers -j4 --output-sync=target` as a single step. `unit-tests` and `test-controllers` are independent goals on that command line, and without `-k` GNU Make stops handing out new work the instant any earlier-processed goal's prerequisite fails — a goal that had not yet been scheduled never starts. So whenever a BATS file inside `unit-tests` fails before `test-controllers` gets a job slot, the entire Go controller suite (`go test ./internal/...`) silently never runs, and the PR page shows only the unit-test failure with no signal at all about the controllers. This adds `-k` so already-schedulable goals keep running after an earlier one fails; the job still exits non-zero and still fails whenever anything failed, `-k` only stops make from abandoning independent work. This addresses the second half of https://github.com/cozystack/cozystack/issues/3776. The first half of that report — whether the job's comment overclaims that it gates the merge, since `Unit & controller tests` is not currently in `main`'s required status checks — needs a maintainer's call between two mutually exclusive fixes (weaken the comment, or add the job to branch protection), and is deliberately left untouched here; the issue itself frames the two as independent and separately useful. Note: `hack/unit-test-parallelism.bats`, which pins the literal invocation string in a test, was updated to match. ### Screenshots N/A — CI workflow and test-script change only, no UI. ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: I walked the trigger map: this PR only touches CI scheduling flags inside this repo's own `.github/workflows/pull-requests.yaml` and a `hack/*.bats` meta-test that asserts against that same file. Nothing in the trigger map names either file, and the change has no effect on any package, API, chart, or contract another repository consumes. ### Validation Reproduced the exact scheduling gap in an isolated Makefile mirroring this repo's real dependency shape (a `unit-tests`-like goal depending on several independent prerequisites, one of which fails immediately, plus a sibling goal with no dependency on it), under the same GNU Make 4.4.1 `-j4 --output-sync=target` invocation used here. Without `-k` the sibling goal never ran, across 5 repeated trials; with `-k` added it ran to completion every time across 3 repeated trials, and the overall exit code stayed non-zero either way. `bats hack/unit-test-parallelism.bats` passes locally against the real files. I also ran `make unit-tests test-controllers -j4 -k --output-sync=target` against this repo directly: local disk space (a limit of my sandbox, unrelated to this change) made `go test ./internal/...` fail to compile with "no space left on device", so I could not confirm a clean end-to-end pass of `test-controllers` itself in this environment — but the run still exercised the fix: after that build failure, unrelated `hack/*.bats` prerequisites kept being scheduled and ran to completion instead of the whole invocation aborting, which is exactly the behavior `-k` adds over the prior command line. I could not run this repo's actual CI runners to confirm disk is not similarly constrained there; nothing in the workflow or runner spec suggests it would be. ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(ci): the `Unit & controller tests` PR check no longer silently skips the Go controller test suite when a Helm-unittest or BATS unit test fails first; both suites now always run and report. ``` --- Part of #3776 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Updated automated test execution so unit and controller test suites continue running when one test target fails, while the job still reports failures. * Updated workflow validation to reflect the revised test command. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Pujitha Paladugu
|
65b02bf5ac |
fix(ci): keep the controller Go tests running when a unit test fails first
The `checks` job runs `make unit-tests test-controllers -j4` as one step. Without -k, GNU Make stops scheduling new work as soon as any prerequisite fails, so when a BATS file in unit-tests fails first, the independent test-controllers goal never starts and the PR page shows no signal about the Go controller suite at all. With -k make keeps running every goal it still can, and the job stays red on any failure. Report: https://github.com/cozystack/cozystack/issues/3776 Assisted-by: LLM Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> |
||
|
Aleksei Sviridkin
|
2f08832392 |
Merge remote-tracking branch 'origin/feat/talos-arm64' into feat/arm64-image-builds
Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
6d759cc73e |
feat(build): build package images for amd64 and arm64 (#4498)
## What this PR does
Package images can now be built for amd64 and arm64 in one `make image`,
and a published build is multi-arch unless `PLATFORM` says otherwise.
This is the build half of arm64 support from #1961 and #903. CI still
publishes amd64 only until it gets a native arm64 leg, which comes in a
separate PR.
Builder stages now run on the build platform and compile for the target
one, so Go never runs under emulation. Under emulation it is slow and it
crashes: the migration-controller Dockerfile already records a "bad
pointer in Go heap" from exactly that. On top of this, `PLATFORM`
defaults to `linux/amd64,linux/arm64`. A build on an arm64 workstation
can't publish an arm64-only digest any more. That happened once already,
the cilium pin was arm64-only until
|
||
|
Aleksei Sviridkin
|
2df6015414 |
ci(talos): keep the Talos builds in CI amd64
The talos package now builds for every arch in PLATFORM, and the jobs that build it without setting PLATFORM would pick up arm64 as well: the PR Talos build, the nightly disk and the release assets. None of them has a consumer for arm64 output, since e2e boots amd64 and the release publishes the amd64 assets only, so each one pins amd64 the way the image builds already do. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
b6a30bd504 |
docs(e2e): describe the per-suite runner and the pod-leak guard
The agent guide, the suite README and a workflow comment said the E2E step is a single chainsaw invocation; it is now one per suite, and a suite whose pods outlive its cleanup fails. Say how a pod is charged to a suite, what the budget is, and how the allowlist is kept honest. The workflows' failure line now points at LEAKED-PODS as well, since a run that failed only on a leak has no assertion diff to point at. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
084ac47237 |
feat(build): build published images for amd64 and arm64 by default
PLATFORM defaulted to empty, so buildx built for the host alone. On an arm64 workstation `make image` published an arm64-only digest, which amd64 nodes then fail to pull; the cilium pin shipped exactly that until it was rebuilt by hand. A build now targets linux/amd64 and linux/arm64 unless PLATFORM is set, and the digest a package stamps is the index digest. LOAD=1 keeps building for the host only. The classic docker image store cannot load a multi-platform index, and loading is the documented local workflow. This changes local pushes: a `make image` on the default docker driver without the containerd image store now fails with "Multi-platform build is not supported for the docker driver", where it used to build for the host. Such a build needs a docker-container builder (BUILDER), or LOAD=1 or an explicit PLATFORM for a single architecture. talos and testing pin linux/amd64 with `override`, placed before the include because common-envs.mk expands BUILDX_ARGS immediately. The Talos assets are amd64: matchbox bakes the kernel and initramfs into its image, and the e2e sandbox boots the disk under qemu-system-x86_64. An arm64 variant of either would still serve or boot amd64 files. CI pins linux/amd64 on every image build. Nothing there sets up QEMU, fork builds export a single-platform archive, and tags.yaml builds on the default docker driver, so the default would break or emulate those jobs. Native arm64 builds belong in a leg of their own. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
IvanHunters
|
95e68bad86 |
ci(workflows): use a literal match in the flux version guards
The guard ran `grep -qx "flux: v${FLUX_VERSION}"`, where the dots in the
version are regex metacharacters, so a client printing `flux: v2X8X6`
would satisfy it. Use `grep -Fqx` for a fixed-string match in the build
toolchain guards this change added.
Also correct the step comments that still read "install if absent
(idempotent)": the step now installs whenever the pinned version is not
the one present, not only when flux is missing.
Assisted-by: LLM
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
edbc4f1b3e |
ci(workflows): pin flux version in build toolchain steps
The "Set up build toolchain" steps installed flux with a bare `curl https://fluxcd.io/install.sh | sudo bash`, letting the installer resolve its floating "latest". latest had advanced to v2.9.5, and a matrix job on PR #4429 failed when that release's checksums transiently 4xx'd during install. Pin FLUX_VERSION to 2.8.6 and guard on the installed client version, the same shape the "Set up promotion toolchain" steps already use, so the flux CLI is fixed and reproducible across CI instead of following latest. This does not remove the checksum host's transient errors; it removes the uncontrolled version bump. Related to the yq floating-latest pin in #3640. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Aleksei Sviridkin
|
2c2c0301f7 |
chore(deps): roll up pending Renovate updates and move to Go 1.27.1 (#4447)
## What this PR does This replaces the open Renovate PRs with one rollup. Go modules move to their newest releases that keep the Kubernetes libraries and controller-runtime where they are, the toolchain and builder images move to Go 1.27.1, and the first-party code gets the Go 1.27 modernizers. Tests for the code the bumps touch come first, in their own commits, and pass on both the old and the new dependencies. Before the bumps: - The e2e sandbox image builds again: dl.min.io answers 410 for mc since MinIO archived it, so mc now comes from its last GitHub release with a pinned checksum. - Renovate stops offering tags of the apimachinery fork. They mirror upstream, so #2999 is a downgrade that drops our patch. - `make unit-tests` now runs the tests of the nested Go modules (kubevirt-csi-driver, token-proxy, kubeovn-webhook, api/apps/v1alpha1). They never ran in CI before. - New tests: JWT verification in the dashboard token proxy, the kube-ovn plunger metrics, cert-manager object idempotency in the tenant gateway, values-file layering in the operator, and the kubevirt CSI driver wrapper. - One real fix. The lineage webhook decoded admitted objects with encoding/json, so an int64 above 2^53 came back rounded. jsonpatch v2.5 compares numbers exactly and would emit a replace that rewrites such a field in the pod. The webhook now decodes with apimachinery's json. Held back on purpose, with the reasons in the commit messages: - Kubernetes libraries stay on v0.35. #3001 needs the apimachinery fork rebased onto release-1.37 first. - cel-go v0.29.2 and etcd v3.6.15, the lines k8s.io/apiserver v0.35 is built and tested with. - grpc v1.83.2. v1.84.0 is affected by GO-2026-6443 and no v1.84 patch release carries the fix yet. - kubevirt.io/api and the CDI API stay on what upstream csi-driver builds against. From CDI v1.60 it no longer compiles. - api/apps/v1alpha1 keeps go 1.26.4, so consumers of the published types are not forced onto Go 1.27. - go fix runs without embedlit (Go 1.27-only syntax, breaks backports to release branches on Go 1.26) and omitzero (would make metadata, spec and status required in the published OpenAPI schemas), and one errorsastype rewrite that did not compile is left out. govulncheck reports nothing reachable in any of the five modules, standard library included. Supersedes #4311, #4310, #4309, #3452, #3993, #3927, #3902, #3805, #3451, #3415, #3414, #3413, #3302, #4050 and #3363. #3001 stays open. Not checked locally: the images that build upstream projects (kube-ovn, kamaji, cilium, linstor-csi and others) with the Go 1.27 builder. The image build jobs of this PR cover them. Closes #4487. ### Screenshots No UI changes. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note chore(deps): move the toolchain and builder images to Go 1.27.1 and raise Go dependencies to their latest compatible releases, closing the published advisories; the lineage webhook no longer rewrites int64 fields above 2^53 in admitted objects. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a restore resource policy option for Velero strategies. * Added a test target to run Go tests across tracked modules. * **Bug Fixes** * Updated the end-to-end test environment to download and verify the MinIO client from its current release location. * Added coverage for token validation, webhook patching, controller behavior, and reconciliation. * **Maintenance** * Updated the Go toolchain, dependencies, and container image versions. * Refreshed pinned workflow actions and base image digests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
cce3387f8d |
docs(ci): say why the overlay's ancestry tie goes to main
The comment claimed a line fresh off main is the same as main, but the cozystack-packages:main artifact is built from main's tip, not from the cut commit. Once main moves past the cut, a stack on such a line gets newer images than the line's, until the stack holds a commit of the line's own. State that limit, and why the tie still goes to main: a branch forked from main before a cut ties the same way and is far more common. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
9c5c5f9d4e |
fix(ci): overlay the right line's refs for a PR on a feature branch
The base-branch overlay pulled cozystack-packages:<base_ref>. A stacked PR's base is a feature branch such as fix/foo: the slash is not valid in an OCI tag and no artifact is published for it, so the pull failed, the step only warned, and every package the PR did not rebuild ran its committed release ref. With a committed ref older than its chart, that broke E2E (Grafana plugin 404). main and release-X.Y still read their own artifact. Any other base reads the artifact of whichever of main and the release lines it has the fewest commits on top of. Always picking main would put main's images under a release line's charts for a stack rooted on that line, the cross-generation mix that fails install. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
renovate[bot]
|
a7dfbe6f8c |
chore(deps): update container image & action digests
Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov Daniil
|
975ea6acb7 |
ci(workflows): gate Talos builds on their inputs
Classify the rename-blind changed-path list against direct Talos sources and shared build inputs. This lets same-repository pull requests skip the heavy build while removals, renames, and build configuration changes still select it. Fork builds remain unconditional. Preserve the matchbox digest produced by a successful Talos build when finalize overlays base-branch image references. When the job is skipped, bootbox continues to receive the current base reference. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
62f2234aea |
fix(e2e): stop a failed teardown posting a failed test verdict
The teardown ends both merge-gating lanes under `if: always()`, and this branch made it `exit "$cleanup_rc"`. So a `docker compose down` hiccup, a busy zpool or a failed rmdir after a fully passing suite posts the required "E2E Tests" status as a failure, and nothing downstream can tell that apart from a test that actually broke. The runner is discarded when the job ends, so there is no next run for the strictness to protect; a reused host is a developer machine, and an annotation serves it. Loud, not fatal. Two comments described mechanisms that are gone. The breakpoint step still explained reading a soft-red output that this branch removed, so a maintainer would go looking for it; the node-join deadline now fails its suite like any other, which is what makes `failure()` sufficient. The build-talos header still described building the nocloud disk and e2e needing it, when the job builds neither and the container lane downloads no disk -- the profile is compiled by nightly and by the tag build's `make assets` instead, and what a PR still compiles is installer.yaml, which shares every input with it bar the platform and output format. And the platform-package override now states its consequence for both packages it replaces, not only for CDI: the merge-gating lane exercises neither the shipped CDI default nor `drbd.enabled: true`. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
Myasnikov Daniil
|
ad00f35f2d |
test(e2e): correct two comments that describe the opposite of the code
pull-requests.yaml said the QEMU substrate "is not wired into any workflow now" and that prepare-cluster and hack/e2e-prepare-cluster.bats stay in the tree for a nightly lane that will carry the coverage. Both halves are wrong in the direction that costs the most: nightly.yaml and e2e-tag.yaml already run `make prepare-env`, which calls prepare-cluster, so anyone trusting that comment deletes live code and takes nightly and every release-candidate run with it. docs/agents/e2e-testing.md states it correctly, so the tree disagreed with itself. hack/e2e-platform-packages.sh said the lane replaces "only LINSTOR with an otherwise-identical Package". It replaces two, and the kubevirt-cdi one is not identical: it raises the importer memory limit from CDI's 600M to 4Gi and the request from 60M to 256Mi. The consequence belongs next to the override -- the merge-gating lane does not exercise the shipped CDI default, and whether a tenant worker disk imports at 600M is answered by the QEMU lanes, which take no override. Also drops the dead half of `resolve_assets`. It resolved nocloud-amd64.raw.xz and exported disk_id; the container lane boots no disk and nothing in this workflow reads that output any more, so all the lookup could still do was fail a release-labelled PR over an asset it does not use. The job stays -- e2e's release arm gates on its result -- and now proves what is still worth proving, that the draft release exists. e2e-tag.yaml keeps resolving the asset, because it consumes it. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: LLM |
||
|
Myasnikov Daniil
|
186547d835 |
test(e2e): run the merge-gating lanes on Talos containers
The srv1-srv3 nodes of both lanes that gate a merge -- the same-repo one in pull-requests.yaml and the fork one in e2e-fork.yaml -- run as Talos containers instead of QEMU guests, so a tenant worker sits at L2 rather than L3. Measured in CI on commits where both substrates ran the same tree: whole-job wall clock down 24-49 minutes, essentially all of it in Chainsaw, while the two tenant-Kubernetes suites go from 1143-2040s to 466-809s and stop depending on the runner's vgif flag. The node-join soft-red gate goes with the substrate, which is what it was for. It existed because nested virtualisation on the shared runners degraded far enough that a worker registering in minutes elsewhere could miss any deadline this test could afford; #3513 established that the discriminator was the host's kvm_amd vgif flag, and removing a nesting level removes what it acted on. So hack/e2e-node-join-soft-red.sh, the SOFT-RED-node-join.txt marker and the soft_red job outputs are gone from all four workflows, and a missed deadline is an ordinary red again. The ::warning annotation survives, because the reason a run went red should be legible without opening the log, and so does the 124-versus-everything -else test, because "the workers were slow" and "the wait never ran" send a reader to different places. Done by swapping the substrate inside the existing e2e jobs rather than by promoting a separate container job. Those jobs carry more than a substrate -- the GitHub App token, the report-overrun guard, the images list, the SSH breakpoint -- and promoting a job that never had them would have dropped four features without saying so. Three properties of container mode drive the design, and each fails quietly rather than loudly. machine.kernel.modules is a silent no-op -- kernel_module_spec.go returns early on ModeContainer with no error and no event -- so the host loads openvswitch and zfs before the nodes start and the workflow step asserts both rather than trusting them; a missing module surfaces an hour later looking like a CNI or a storage regression. Docker gives a privileged container its own tmpfs /dev seeded once at creation, so /dev/kvm works but ZFS zvols created later never appear and linstor-csi fails ControllerPublishVolume; the compose file binds the host devtmpfs, which is also why talosctl cluster create docker can never carry LINSTOR. And there is no ARP VIP, so 192.168.123.11 stands in for the QEMU lane's .10. The QEMU machinery stays in the tree and keeps running: nightly.yaml and e2e-tag.yaml still call `make prepare-env`, so DRBD, the replicated StorageClass, its Immediate binding mode, the tenant StorageClass propagation that only applies to remotely-accessible classes, and the Cozystack Talos node image with its extensions all keep nightly and release-candidate coverage. None of them is exercised per pull request any more. That is the trade. Nothing on the PR path builds or downloads a nocloud disk now, so build-talos drops the talos-nocloud step and the talos-image artifact. build-talos itself stays: finalize consumes its digest fragment. One property this does not fix, recorded beside the runner class because it will be read off a red run eventually: a container node reports the HOST's CPU and memory as capacity, and hack/e2e-container-up.sh counters that with kubelet systemReserved. That corrects the scheduler and not what a pod reads from /proc, so a workload sizing itself from /proc/cpuinfo or /proc/meminfo is configured for the whole runner. Also carried here because the same files carry them: the ghcr.io pull-through mirror removal (see #4007 -- whichever lands first makes the other a no-op), the authoritative HelmRelease readiness gate, the backup-credential preflight, the prepull guard against an image-less container fragment, and folding the two standalone OIDC suites into the latest-version tenant cluster. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
e49633208c |
ci(tests): parallelize unit test targets
The BATS half of `make unit-tests` ran as one shell loop over ~60 files, which is a nine-minute serial tail no amount of runner CPU could touch. Each file becomes its own Make prerequisite instead, so `-j4` schedules them, and the workflow asks for `make unit-tests test-controllers -j4 --output-sync=target` in a single step: one make process owns all four runner CPUs, the Helm and Go targets share the same bounded pool rather than waiting for each other, and output stays grouped by target instead of four logs interleaving. The existence check moves into its own prerequisite rather than disappearing. It guards against the wildcard matching nothing, which would otherwise make the whole target succeed by having no work. COZYTEST_TRACE rides per target. It arrived with the serial loop it now replaces, and a target that dropped it would go back to streaming full xtrace for every suite -- so the parallelism guard requires the prefix rather than tolerating it. 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
|
61f7939ab7 |
ci(workflows): make E2E breakpoint opt-in via labels only
Drop the author-association branch from the "Breakpoint on E2E failure" step condition, so a red end-to-end run only opens an SSH breakpoint when the pull request carries the debug or release label. The dropped branch held an ephemeral CI runner open for up to twenty minutes on every red maintainer-authored pull request, whether or not anyone intended to attach. Trim the step's explanatory comment to match: drop the paragraph about the removed authorship path, including its stale claim that only the debug label affects runner selection, since this workflow has no conditional runs-on to begin with. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Myasnikov Daniil
|
04fa8471fe |
docs(release): say what the gates actually guarantee
Three claims that were stronger than the code, plus one legibility fix. The PR gate's comment said a `packages/` edit "on the PR or its base" fails the status before merge. The base half is not true: GitHub fires no `pull_request` event when the base branch advances, it only recomputes refs/pull/N/merge, so a change landing on the base after this went green is never re-verified and the PR merges on the stale result. Checking the merge tree does make base drift visible WHEN the job runs, which is worth keeping — but finalize is what actually catches the drift case, after the merge and before any stable name. Say both, in the workflow and in the reviewer checklist, so nobody reads the green as covering more than it does. "Transactional at every consumer-visible name" is a judgement call dressed as an absolute, inherited from the "this dispatch performs NO registry mutation" it replaced. The dispatch now does write to the registry: the `promotion-*` candidate is a real, publicly listable tag. What it creates is no *stable* name — no vX.Y.Z, no :latest, no stable installer. Stated that way the sentence stops needing the reader to already know which writes were meant. The retention paragraph listed two of the conditions the workflow enforces. It now lists all of them, including the ones the workflow gained here: same-repository, bot-authored, `release-X.Y.Z` branch, the `release` label, exactly one valid digest pin, and protection from a base-branch tip as well as an open PR. Also: `flux pull artifact` reads its credentials from ~/.docker/config.json, which is why finalize now runs `docker login` — but its runner class is not one of the docker-using build ones and the setup step installs everything except docker. Check for it there, where the message can name what is missing, instead of failing with "command not found" halfway down the release path. And the verifier's exclusion list is a restatement of flux's `excludeOCI`, not a derivation — the CLI does not expose it — so the coupling and the pinned version are now written down next to the list. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
9fa8eeeda2 |
ci(release): pin the promotion toolchain and cap the candidate gate
Three review findings on the new promotion steps.
`yq` was fetched from `releases/latest/download` in all three steps this
pipeline installs it in — the promote dispatch, the promote PR gate and
finalize. That makes an unpinned moving dependency part of what decides
whether a stable release may be created, and part of what rewrites the
installer pin: two promotions of the same rc can be served different
binaries. Pin it the way FLUX_VERSION already is, version-gated so a
matching preinstalled yq is reused, on the version the e2e sandbox image
already tracks.
A version authenticates nothing about the bytes it names, though, and
these are the workflows that mint write-once stable names. yq is
therefore also pinned BY CONTENT: its SHA-256 is committed beside its
version and verified before install.
Scoped to yq deliberately. It is the one downloaded binary with write
authority over what a release contains — it stamps the candidate digest
into installer values at promote time and platformVersion into the
stable chart at finalize. skopeo, helm and oras only move bytes that are
already digest-addressed, so a substituted one is caught downstream; a
substituted yq writes the wrong pin and every later check verifies that
pin faithfully.
The Flux installer is left as it is, on purpose. It fetches
flux_${FLUX_VERSION}_checksums.txt from the pinned release and verifies
the binary against it, so the binary is not the exposure — the ~6 KB
bootstrap script from fluxcd.io is, and it is mutable by design and
fetched identically in ten other workflows here. Pinning its bytes in
these three files would break on the next regeneration of a script the
project does not control while leaving the same fetch unpinned
everywhere else. The comment names the residual risk instead.
No precedent existed either way: nothing under .github/workflows/ or
hack/ verified a downloaded tool before this, and every version-pinned
download in the tree (oras 1.2.0, cozyvalues-gen 1.6.0) is unverified.
This is a new convention rather than an application of one, confined to
the release path and justified by write authority, and the comment says
so where the next reader will be standing. The three unpinned fetches
outside this pipeline (nightly.yaml, e2e-fork.yaml) are left alone.
The checksum is trust-on-first-use, which is the honest description: it
fixes the bytes to what was checked when the version was chosen and
makes any later substitution loud. Provenance is the release's own
published checksums list cross-checked against the downloaded asset;
both agree on
fa52a4e758c63d38299163fbdd1edfb4c4963247918bf9c1c5d31d84789eded4, and
that binary self-reports v4.53.3.
Verified locally against the real asset: the version gate skips a
matching preinstalled yq and re-installs a mismatched one, the checksum
line reports OK for the genuine binary and exits 1 for a tampered copy,
which aborts the step under the runner's `bash -e`.
`verify-release-candidate` had no `timeout-minutes`, so it inherited the
6-hour default while `e2e-report` waits on it through `needs` — a
stalled `flux pull artifact` would hold the required `E2E Tests` status
pending for the whole of it. 30 minutes, matching `checks` and
`finalize`.
The contract pins that all three install steps carry both versions and
the checksum, run the checksum check, name no `latest/download`, and
agree on one yq version and one checksum. So the unpinned one-line idiom
cannot come back into a release step by copy-paste from the workflows
that still use it, and bumping the version in one file without its
checksum cannot pass. Both mutations are caught.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: Claude <noreply@anthropic.com>
|
||
|
Myasnikov Daniil
|
5d56681f8c |
fix(ci): put every label guard in lockstep, forks included
Promotion needs the `release` label to start work, so this PR added it to the set of labels whose `labeled` event is not discarded. That set is stated in four places and was updated in three. `gate-concurrency-contract.bats` caught the shape of the problem and then became the first casualty of it. It asserted that the concurrency key excludes exactly ONE label from the `-label` group, by counting occurrences of `github.event.label.name != '`. A second publishing label made the count 2, turning the suite red — and with it `checks`, then `finalize`, then `e2e`, then the required `E2E Tests` status. Raising the constant to 2 would re-hardcode the same brittleness one label later and throw away the invariant the test exists for: the set of labels excluded from the `-label` suffix is exactly the set `plan` runs for. Both sets are now extracted from the workflow and compared, so the assertion holds at any number of labels. `resolve_assets` was the guard that got missed. It sits on `e2e`'s release arm, so a label event `plan` runs for and it skips for leaves `e2e` with both arms false — `finalize` is force-skipped for release PRs — while `e2e-report` still runs. On a promote PR already carrying `full-e2e`, re-applying `release` therefore superseded the green run and posted `E2E Tests` = failure, because `full-e2e` bypasses the "e2e not repeated" branch and `E2E_RESULT` is `skipped`. `unlabeled` is not a trigger, so removing the label could not clear it; only a push could. `release` must also not publish on the fork lane. e2e-fork.yaml's `resolve` decides "this run built nothing, so it is not a verdict" by asking whether `Plan build` skipped. Admitting `release` at `plan` made that false: a fork run reaches `plan`, has `build`/`finalize` gated off by its own label, succeeds having produced no pr-patch, and the fork lane fails closed on the missing artifact — red on a suite that was never going to run, on a PR whose only fault is a mislabel. `plan` and `resolve_assets` now admit `release` for same-repo PRs only, and the concurrency key carries the complementary term, so NOT(A OR B OR (C AND NOT F)) stays exactly (NOT A) AND (NOT B) AND (NOT C OR F). For a same-repo PR F is false and every one of them reduces to what it was, so none of this changes non-fork behaviour. The same shape still reaches the fork lane from a PUSH to a fork PR somebody mislabelled `release` — `build` is gated on the label, not on the event, so this predates promotion using the label. Left alone; the comment in e2e-fork.yaml now records it instead of asserting an invariant this PR would have made false. The contract enumerated `plan` and `e2e-report` by name, which is why it did not catch `resolve_assets`: a named-job loop cannot notice a job nobody thought to name. It now derives the job list from the file and requires every job that decides from a label name to decide from the same set, and pins the fork term on both halves of the complement. The file's existing properties are kept — extraction runs over executable-only lines, so a commented-out key cannot satisfy the contract, and every comparison asserts a non-empty input first, so an empty extraction fails closed under both bats and cozytest.sh. Verified by mutation, each caught: excluding a label `plan` skips for, admitting one the key does not exclude, reverting `resolve_assets`, deleting `e2e-report`'s guard, narrowing `verify-release-candidate`'s, renaming the label in one guard only, commenting out the group key, and dropping the fork term from either side alone. Deleting `plan`'s step-level fork guard is caught too, by an assertion that had to stop matching on the bare identifier once `plan`'s own `if:` named it. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
c2e145709b |
fix(release): publish stable packages candidate
Serialize the tag-rewritten packages tree during RC promotion and pin its immutable digest in the installer. Verify the candidate against the prospective merge and the original RC artifact before creating stable names. Assisted-By: Claude <noreply@anthropic.com> Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
1abc653cfb |
fix(ci): treat a disabled chainsaw suite as inert in test selection
Editing hack/e2e-chainsaw/backup/chainsaw-test.yaml.disabled selects the whole Chainsaw suite. The per-suite rule reads the suite name off the directory and ignores the suffix, so the change selects "backup"; the final intersection against the suites that actually exist finds no such suite and empties; and the safety net for an empty selection escalates to the full run. A disabled suite is registered nowhere and executed by nothing, which makes its edits the one file class that provably cannot regress a test. The mapping had it backwards: they bought the most expensive outcome the selector has. Rare in practice, and paid on every commit by anyone grooming or re-enabling such a suite. Classify *.disabled as inert ahead of the per-suite rule, and record it alongside the other rules that select nothing so the header stays an accurate enumeration. The safety net keeps its job for paths no rule has looked at, which is what it was added for. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
a6c94c4528 |
fix(ci): correct an inverted caveat and clean up after the assertion
Two defects introduced by the previous two commits. The in-tree lane's comment said `skip=true` cannot mean "a workflow nobody classified", and then explained, after a colon, exactly how it can: `.github/` is inert as a directory and the e2e workflows are exempted from it by name, so a new one reads as inert. The clause meant to warn a reader instead told them the one unsafe case is safe, in the copy consulted when deciding whether to trust a green gate. The other three copies of the caveat state it correctly. The scratch-directory cleanup landed before the last assertion in every test added here. That passes the EXIT-trap guard and inverts the reason the guard exists: both runners set `-e`, so cleanup placed last is unreachable on failure and the fixture survives for inspection, which `docs/agents/e2e-testing.md` §3 states outright and the already-converted `hack/select-install_test.bats` follows. Placed early, a failing test deletes the evidence of its own failure, and one test had the call between two assertions, so which of them failed decided whether the directory survived. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
55f3c79e9f |
docs(ci): stop pointing at the inert list for paths it does not hold
Four places said an empty selection means the inert pattern matched, and sent the reader to `inert_config_pattern` to see which paths those are. Documentation, dashboards and `*.md` are dropped before that pattern is ever consulted, and a backups example whose app has no suite is dropped where backup examples are matched. A reader checking the pattern for any of them finds nothing and has to reconstruct the order by reading the script. Name the rules that produce an empty selection instead, and keep the claim to the one the lanes rely on: empty is a decision some rule reached, not a path nothing looked at. The same bullet claimed every path is classified. One is not. The inert pattern makes `.github/` inert as a whole directory and the workflows that run the suite are exempted from it by name, so a workflow added under `.github/workflows/` reads as inert and the fall-through never sees it. Today's enumeration is correct and I checked it against `rg -l test-chainsaw`, so this is not a live hole, but the sentence saying it cannot exist is what makes the reminder next to that list look decorative. Record it as the guard it is. Separately, the fork lane's rationale for appending a newline described a mechanism this branch removes: the reader now guards its own loop, so the terminator is no longer what keeps the last path alive. Say that it is now the text-file contract and a second line of defence across two files, which is a reason to keep it and a different reason than the one written there. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov Daniil
|
ee4a48278f |
refactor(ci): drop the unreachable empty-selection branch in both lanes
Both lanes ran `if [ "$FULL_E2E" = "true" ] || [ -z "$SELECTED_APPS" ]` under a step gated on `steps.select.outputs.skip != 'true'`. When `select` runs, an empty result sets `skip=true` and the step never starts, so the only way to arrive with no selection is the `full-e2e` label — which sets `FULL_E2E=true`. The second test could never decide anything the first had not, while the comment above it advertised "full suite when the selection is empty" in the file whose whole point is that a green status means tests ran. Testing `FULL_E2E` alone keeps the real invariant visible. The comments now say what an empty selection means after the preceding commit: every changed path was explicitly classified inert, not that the selector had no idea. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
myasnikovdaniil
|
7dca642894 |
fix(ci): restore e2e for fork PRs and close the merge-gate bypass (#3262)
## What this PR does Fixes #3257 — fork PRs never ran e2e, and the skipped required check let them merge anyway. Fork PRs get no secrets on `pull_request`, so `make image` pushed anonymously and died with `denied`. That failed every build, collapsed the `build → finalize → e2e` chain, and left the required **E2E Tests** job *skipped* — which branch protection treats as satisfied, so an approved fork PR could merge with e2e never having run. This implements **Option A** from the issue (privileged `workflow_run` split), pushing fork e2e images to the existing CI registry (OCIR): **Build primitive (`hack/common-envs.mk`)** - New `OCI_EXPORT_DIR` mode: build each image to a per-image OCI archive instead of pushing, and force `PUSH`/`LOAD` off. The digest is captured via `--metadata-file` regardless of output type, so the refs baked into `pr.patch` match what the privileged run later pushes. Threaded through the `image-tags` macro every package already uses — no per-package Makefile change. **`pull-requests.yaml` (unprivileged, fork branch)** - `build` / `build-talos` / `finalize` export images to OCI archives and upload them as artifacts instead of pushing → the red `denied` build wall disappears and the chain no longer collapses. - The in-tree e2e job is renamed off `"E2E Tests"` and guarded to same-repo PRs. A new `e2e-report` job concludes the required `"E2E Tests"` context as an **explicit check-run** (success on a docs-only PR or when e2e passed, failure otherwise). A skipped job can no longer satisfy the required check — this also closes the same hole for same-repo PRs whose build fails. **`e2e-fork.yaml` (new, privileged `workflow_run` from the default branch)** - Resolves the fork PR, opens a pending `"E2E Tests"` check-run, and fails-closed if the fork build failed. - Pushes the pre-built image archives to OCIR **by digest** and flux-pushes the digest-pinned packages tree. The OCIR token is live only for trusted binaries (`git`/`skopeo`/`flux`/`yq`) over data — never for `make` or any fork-authored script. - Runs the e2e suite against the pushed closure with **no push token** and concludes the check-run on the PR head SHA. Fork build code never receives registry credentials; the fork's images and test scripts run only in the credential-less, ephemeral e2e job — a stronger posture than a `pull_request_target` run that would hand fork code the secrets directly. ### Validation Local: buildx OCI-archive output + digest capture, macro expansion for single- and multi-image packages, non-fork push path unchanged, `actionlint` clean. Needs live validation (why this is a **draft**): `workflow_run` artifact download across runs; that the `"E2E Tests"` commit status satisfies the required context and keeps a fork PR non-mergeable until the privileged run concludes it; `refs/pull/<N>/merge` fetch + `git apply --3way`; `skopeo --preserve-digests` end-to-end. `workflow_run` only runs from the default branch, so the privileged half is exercised by mirroring the workflow onto a personal fork's default branch and opening a throwaway fork PR into it — not by merging first. _Since review_: the fork `e2e` job ran the pre-#2826 BATS app loop that the merged Chainsaw migration deleted (already broken); it now runs the same Chainsaw + TIA path as the in-tree job, with suite selection and the `full-e2e` override sourced from `resolve`'s trusted base-repo file list / labels (not a merge-ref diff). Publish job hardened per the non-blocking follow-ups: base-tree image allowlist, `skopeo copy --all`, pinned flux CLI. The required `E2E Tests` gate is now published as a commit **status** (default token + `statuses:write`), not a COZYSTACK_CI app check-run — the app lacked `checks:write` (`checks.create` 403'd) and a default-token check-run floated under a labeler suite; a commit status has neither problem, and forks can't post one (their `pull_request` token is read-only). Two review-bot findings fixed too: fork `OCI_EXPORT_DIR` is now absolute (a relative path misplaced archives under `make -C <pkg>` and every fork build would fail at upload), and the "≥1 archive" guard no longer no-ops under `nullglob`. ### Repo-admin prerequisites One repository setting lives outside this diff: 1. **Branch protection: keep `E2E Tests` required.** The gate is now a commit **status** named `E2E Tests` (posted with the default `GITHUB_TOKEN` — see `docs/agents/e2e-testing.md` §10), not a job or an app check-run. Branch protection already requires `E2E Tests` (currently the e2e job's name, renamed here to `E2E (in-tree)`), so the required context now resolves to the status — **no GitHub App, no `checks:write` grant, no app-id pin needed**. Just don't pin the context to a specific app. 2. **Actions → "Require approval for all outside collaborators".** Each fork run must be a deliberate maintainer click before it executes, since the fork's unprivileged run produces the artifacts the privileged `e2e-fork.yaml` consumes. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff (7 files: two CI workflows, `docs/agents/e2e-testing.md`, `hack/common-envs.mk` + its bats suite, and two package `Makefile`s). The one trigger worth naming is "change developer tooling (`cozyvalues-gen`, `cozypkg`, the package `Makefile`s) → `development.md`" on the website. It does not fire here: `OCI_EXPORT_DIR` is a CI-only build mode that defaults off, and the local developer workflow is unchanged — `make image` still builds and pushes exactly as before, which the new default-path case in `hack/common-envs_test.bats` now pins. No chart values, no API surface, no app list, no platform variant, no Talos version bump and no release asset rename are touched. - [x] No downstream repository is affected by this change ### Release note ```release-note fix(ci): fork pull requests now run e2e via a privileged workflow, and a fork PR can no longer merge on a skipped (never-run) required E2E Tests check ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added a dedicated fork E2E workflow that runs after the base PR and reports results via the required **E2E Tests** commit status. - Fork PRs now export build images as OCI archive artifacts for later publishing. - Added a same-repo E2E reporting job and renamed the in-tree E2E flow to **E2E (in-tree)**. - **Bug Fixes** - Added tighter, fail-closed gating to ensure merge blocking when **E2E Tests** isn’t correctly finalized. - **Documentation** - Documented fork vs same-repo E2E split, OCI export behavior, and required fork security settings. - **Tests** - Added Bats coverage validating `OCI_EXPORT_DIR` and preventing unintended registry pushes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
cc8b01d5cd |
docs(ci): say when each lane's status opener actually lands
The reference section claimed both lanes open the required context when their run starts, and enumerated the triggers it protects against: a re-run, a reopen, a `full-e2e` label. Every one of those starts a Pull Request run, and on a fork that run cannot open the status at all. The opener is guarded same-repo only, because a fork's `pull_request` token is read-only, and the fork's opener lives in the privileged workflow, which starts only once the unprivileged build has finished. So on the lane the sentence was most likely to be read about, an already-green head SHA keeps reading green for the whole build. The gap is structural. A read-only token cannot post a status at any point in the run, and that same read-only token is what makes publishing the gate with the default token safe in the first place. It is bounded by the build rather than open-ended. What was wrong was only the claim that it does not exist. Say so in all three places that carried the overstatement: the section itself, the opener's own comment, and the contract test, which pins that each lane has an opener and cannot express when it lands. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
c45179a9d0 |
docs(ci): correct the reason the build matrix keeps the rename-detected list
The comment justified the split by saying a path that no longer exists has nothing to build. That is only half the picture: on a rename between packages the source package is still there and still needs rebuilding, and rename detection drops it from the list the matrix reads, so the matrix under-builds. The behaviour predates this change and is out of scope for a docs-only decision, but the comment presented it as deliberate and correct rather than as inherited and left alone. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
9c68b1f83e |
fix(ci): stop a cancelled fork run from publishing a verdict
The invariant that a cancelled run publishes nothing was written down in three places and held in two of them. The fork lane's terminal reporter ran under `always()`, which includes cancellation. When a second privileged run supersedes the first, the dying one still posted `failure` on a head SHA it no longer owned, for a suite that never finished. It self-corrected once the superseding run reported, but that can be two hours later, and until then a legitimate pull request reads red. The same-repo lane was moved off `always()` for exactly this reason. Move this one too: a failed publish or e2e still reports, since neither is a cancellation, and a cancelled run now leaves the `pending` it opened standing until its successor concludes. The same-repo guard's rationale cited a scenario that no longer exists. It described a label event cancelling the run that opened the pull request, which the concurrency split made impossible for every label the workflow discards. The guard is still needed, for a push, for a `full-e2e` label superseding the head run, and for a manual cancel, so the comment now names those instead of an extinct one. The reference section listed the pieces this invariant rests on and omitted the reporters, which is the half a reader is most likely to touch, and counted them, which was already wrong and would go wrong again on the next addition. It now enumerates without counting. The contract pins both reporters, because both lanes have had this defect at different times in the same shape. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
8a21085324 |
fix(ci): decide docs-only from both sides of a rename
The two lanes answered "is this docs-only" from different sources and could disagree. `plan` reads `git diff --name-only`, which applies rename detection and reports a rename as its new path alone, so moving a code file into docs/ read as docs-only. The fork lane reads the PR file list, where a rename carries `previous_filename`, and reads the same change as code. Disagreement is not a near-miss here. The same-repo lane builds nothing and uploads no patch artifact; the fork lane concludes e2e is required and then fails closed because that artifact is missing, reporting that the fork build did not complete. It completed — it correctly built nothing. There is no push the contributor can make to clear it and no label that overrides it, so a legitimate contribution is permanently red. Point the docs-only decision at the rename-blind list, which the job already computes for the downstream-trigger-map flag and whose comment names this exact trap. Both lanes now agree, and the same-repo side stops shipping a removed code path untested — until now such a change skipped every build and reported e2e as not required. The build matrix keeps reading the rename-detected list: it picks packages to rebuild, and a path that no longer exists has nothing to build. The contract file gains the agreement as a pin, and grows a header to match what it now covers. The fork half is pinned on the docs-only expression rather than on `previous_filename`, which also appears in the file list a few lines down and would have kept the assertion satisfied with the check deleted. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
d3515509c8 |
fix(ci): let a status-API blip skip the opener instead of failing the run
The step that claims the required context is the first in `plan`, and `checks`, `build`, `build-talos`, `finalize` and `e2e` all hang off that job. A transient failure from the status API would therefore discard a full build and suite for a call that is an optimisation, not a gate. Mark it `continue-on-error`. Losing the opener degrades exactly to the behaviour that preceded it — the head SHA keeps whatever status it had until `e2e-report` concludes — while a run that dies before reporting leaves the merge blocked either way. The terminal post keeps its own error handling, because that one failing silently is what would let a run finish with no verdict at all. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |