mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-04 11:07:03 +09:00
fix/select-e2e-find-not
1389
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
ceb1afefc4 |
test(e2e): spell the find negation as -not in select-e2e_test
The negated-assertion audit reads a continued line that starts with `!` as a bash statement, so the `! -type d` operand inside the find expression on the continuation line turned the unit lane red on main. `-not` is the same find predicate and is not a statement prefix. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
IvanHunters
|
50dec5d46a |
fix(platform): resume a retried migration Job from the stamped version (#4583)
## What this PR does A retried pod of the platform migration Job no longer runs migrations that already finished in the same upgrade. The Job gets `CURRENT_VERSION` once, at render time, and a failed pod is replaced by one with the same environment. By then every finished migration has stamped the `cozystack-version` ConfigMap, but the new pod ignored that and started from the rendered version again. `run-migrations.sh` now starts from the version stamped in the ConfigMap. If reading it fails, the pod fails and the Job retries it, because falling back to the rendered version is exactly the re-run this fixes. The rendered value is used only when the ConfigMap holds no integer version, which is also when the chart itself could not get a real version from it. Without that fallback a value like `v58` gave a green Job that ran nothing. The new `hack/run-migrations.bats` runs the script in the migrations image's alpine base against a fake kubectl that keeps the stamp in a file, so one pod's stamps are what the next pod reads. It needs docker, like the other migration suites. I also found that the ConfigMap existence check at the top of the script treats any API error as a missing ConfigMap and skips every pending migration. That is older than this change and is filed as #4579. Closes #4555 ### Screenshots ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(platform): a retried platform migration Job now resumes from the version stamped in the cozystack-version ConfigMap instead of re-running migrations that already finished in the same upgrade. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Migration startup now resumes from a valid stamped version, avoiding reruns of migrations that have already completed. * Empty or non-numeric stamps fall back to the rendered version. Out-of-range stamps or failed stamp reads stop migrations before they run. * If the migration ConfigMap is missing, migrations do not run and the target version is stamped. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
2ddd55ce5b |
fix(build): honour PUSH and LOAD from the environment and in image-talos (#4654)
## What this PR does A local build that asks to load images into docker no longer pushes them. `hack/common-envs.mk` set `PUSH` and `LOAD` with plain `:=`, and a makefile assignment beats the environment, so `LOAD=1 PUSH=0 make image` quietly pushed a multi-arch build. Both now take the environment value, and an empty one falls back to the default. I also made `PUSH` default to 0 when `LOAD=1`, so `LOAD=1` alone only loads. Before this, `make image LOAD=1` pushed and loaded at once. An explicit `PUSH` from the environment or the command line still wins, and `OCI_EXPORT_DIR` still forces both off. Every buildx target gets the values through `BUILDX_ARGS`, so the fix lives in that one file. The Talos installer was the one image that ignored `PUSH`. It is published with skopeo, not buildx, so it never saw `--push=$(PUSH)`. Its registry copies now run only when `PUSH=1`. A build that pushes nothing no longer writes `PUSHED_TAGS_LOG`. That log tells the multi-arch stitch which tags to move, so it must hold only tags that were really pushed. The cache helper lost an argument nobody used. No caller passes the second argument of `cache-args`, and an explicit tag there would bypass `CACHE_TAG`, so the amd64 and arm64 builds of that image would share one cache ref. CI sets neither `PUSH` nor `LOAD`, so CI builds do the same thing as before. Each fix has a `make --dry-run` test in `hack/common-envs_test.bats`. Closes #4499 Closes #4504 Closes #4551 ### 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 ccp skills pass `PUSH=1` explicitly, which still works. The website docs do not mention `PUSH`, `LOAD` or `cache-args`. ### Release note ```release-note fix(build): `LOAD` and `PUSH` set in the environment are now honoured, `LOAD=1` no longer pushes unless `PUSH=1` is also set, and the Talos installer is no longer published when `PUSH=0`. ``` |
||
|
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
|
9c025fa2b1 |
fix(installer): skip the packages artifact push and stamp with PUSH=0 (#4663)
## What this PR does A local build with `PUSH=0` no longer publishes the packages artifact. Before this change, `make image-packages` in `packages/core/installer` ran `flux push artifact` no matter what `PUSH` was set to, and then wrote the pushed digest into `values.yaml`. The `chart` target in the same Makefile already respects `PUSH`. With `PUSH` set to anything other than 1, the target now skips both the push and the digest stamp. It prints one line saying so and leaves the committed `platformSourceUrl` and `platformSourceRef` as they are. It still writes `packages/core/platform/.build-revision`, which is gitignored. I chose to skip the stamp instead of computing a local digest, because every reader of that value needs a digest that exists in the registry. The chart passes `platformSourceRef` to the operator as `--platform-source-ref`, and Flux then pulls `cozystack-packages@<digest>` from the registry. The release scripts `verify-promoted-packages.sh`, `promote-packages-artifact.sh`, `stitch-multiarch.sh` and `nightly-mirror.sh` also treat it as a registry digest. A digest of an artifact that was never pushed points at nothing. On main an unpushed run would not even get that far, since the recipe dies at `test -n "$DIGEST"`. The gate is a make-level `ifeq ($(PUSH),1)`, so it reads whatever `PUSH` ends up as after `hack/common-envs.mk`. On main that means `PUSH=0` on the command line. I also applied the `common-envs.mk` change from #4654 locally, which takes `PUSH` from the environment and makes `LOAD=1` default it to 0. With that applied, a dry run shows no push and no stamp for `PUSH=0` in the environment, for `LOAD=1`, and for `PUSH=0` on the command line, while `LOAD=1 PUSH=1` and an empty `PUSH` still push. CI is not affected, because no workflow sets `PUSH` or `LOAD` and `PUSH` stays at 1. I dry-ran the three CI invocations of this Makefile before and after the change, on main and again with #4654 applied: - `tags.yaml`, release stitch step: `make -C packages/core/installer image-packages chart` with `REGISTRY`, a release tag as `IMAGE_TAG`, and `PUSHED_TAGS_LOG` - `pull-requests.yaml`, installer step of a same-repo PR: `make -C packages/core/installer image` with a `pr-N-sha` tag, `PLATFORM=linux/amd64`, `BUILDER`, and an empty `OCI_EXPORT_DIR` - `build-main.yaml` and `build-release.yaml`: `make build`, which runs the installer `image` target with defaults ```text $ diff --recursive before/ after/ # tags.txt pr.txt build.txt $ echo $? 0 ``` There is one side effect. `OCI_EXPORT_DIR` forces `PUSH := 0`, so running `image-packages` with it set no longer pushes, where before it would have tried an anonymous push. No CI path does that. The fork PR build already runs only `pre-checks image-operator` for the installer, and the `make -n ... image OCI_EXPORT_DIR=/oci` scan in `e2e-fork.yaml` greps only the `.oci.tar` names, which this change does not touch. The new test dry-runs the target in both modes. With `PUSH=0` there must be no `flux push artifact` and no `platformSourceRef` stamp; with `PUSH` at its default, both must be present. The first test fails on main. I mutated the guard three ways, and each time only the matching test went red: - condition forced true: the `PUSH=0` test fails - compared against 2: the default-path test fails - `endif` moved up so only the push is gated: the `PUSH=0` test fails on the stamp check One limitation is left as a follow-up. The test's `env -u PUSH -u LOAD` only isolates it once #4654 lands, because on main a file assignment wins over the environment anyway. It does not cover `PUSH=0` coming in through `MAKEFLAGS`, so `make bats-unit-tests PUSH=0` would fail the default-path test here and the ordering check in `hack/stitch-multiarch_test.bats`. Nothing runs the suite that way today. If that changes, passing an explicit `PUSH=1` on the default-path `make` lines, or unsetting `MAKEFLAGS` the way `hack/bats-unit-keep-going.bats` does, would fix it. ### 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 website's `development.md` tells developers to run `make image-packages` with `REGISTRY` set to their own registry. That is the default `PUSH=1` path, and it works as before. Neither the website nor ccp mentions `PUSH=0` for this target. Closes #4656 ### Release note ```release-note fix(installer): `make image-packages` no longer pushes the cozystack-packages artifact or stamps its digest into values.yaml when PUSH=0. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * You can run `image-packages` with `PUSH=0` to skip publishing the artifact and keep the committed platform source reference unchanged. * When pushing is enabled, the command publishes the artifact and updates the platform source URL and digest-based reference. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
33cf6972bb |
test(qdrant): exercise the tenant trust anchor end to end (#4701)
## What this PR does The qdrant e2e suite now runs with TLS on and checks the tenant trust anchor end to end. Before this, the fixture left `tls` unset with `external: false`, so the chart rendered no CA and no `TenantProjection`, and nothing in e2e reached the controller or `tenantsecrets`. Setting `tls.enabled: true` turns TLS on without the LoadBalancer that `external: true` would allocate. The first new step checks that the anchor is published. The `qdrant-test-ca` sentinel has to report `Ready=True/Projected`, and `qdrant-test.tenant-ca` has to carry both the `tenant-ca` and `tenantresource=true` labels. The projection must hold only `ca.crt` with the same fingerprint as the chart's CA, and `tenantsecrets` must serve it. The key-bearing `qdrant-test-ca` and `qdrant-test-tls` must answer NotFound there, and any other error fails the step. The second step checks that the anchor is what decides trust. It reads the anchor through `tenantsecrets`, port-forwards the qdrant Service and runs `openssl s_client` with that file as the only CA, hostname check included. A CA with the same subject and another key must fail, and so must the runner's default trust store. A handshake failure counts only when the output shows a verification error, so a refused connection does not pass as a rejection. Plaintext HTTP must get no response. Curl exits 7 and 28 fail the step, and a second verified handshake afterwards proves the forward was still alive. TLS does not break the existing steps, because none of them talk to qdrant. `create-qdrant`, `wait-helmreleases-ready`, `verify-statefulset-and-storage` and `verify-pvc-reclaim-on-delete` only read Kubernetes objects: no `http://`, no port 6333 or 6334, no curl or exec, and the suite has no backup round trip. The release now also renders two Issuers, two Certificates and the sentinel. The chart switches its probes to HTTPS with `enable_tls`, so `readyReplicas: 1` now covers a pod serving TLS. Local checks: `chainsaw lint test` (v0.2.15) says the file is valid. `helm template` with the fixture's spec renders 2 Certificates, 2 Issuers and 1 TenantProjection, and none without TLS. I ran the handshake logic against a local `openssl s_server` with a CA and leaf shaped like the chart's. The anchor passes, the wrong CA and the default store fail on verification, a wrong hostname fails, plaintext gets `000`, and a closed port is not counted as a rejection. A sentinel with a missing source (`SourceNotFound`) is not tested here, the reconciler unit tests cover it. Review notes I left as they are: - Through `kubectl port-forward` the local socket is always accepted, so curl exits 7 and 28 only catch a dead forward process. A false pass would need qdrant to answer plaintext and a pod-side blip on that one request at the same time. - qdrant may reset the socket on the plaintext request, and some kubectl versions drop the whole port-forward on a pod-side error. Then the handshake after it turns the step red, not green. If CI shows `lost connection to pod` there, the plaintext check should get its own port-forward. - The suite's `catch` has no `describe` of the `TenantProjection`. The sentinel assert prints its conditions when it fails, but a red Secret assert or script after it would not. - The vendored chart's `servicemonitor.yaml` hardcodes `scheme: http`, so metrics scraping fails for any qdrant with TLS on. That was already the case before this PR, and the suite does not check metrics. Tracked in #4703. cert-manager does not set owner references here, so `qdrant-test-ca` and `qdrant-test-tls` stay in the namespace with their private keys after the app is deleted. That is tracked in #4704. The nats half of #3706 is tracked in #4702. Closes #3706 ### Screenshots No UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff touches only `hack/e2e-chainsaw/qdrant/`, which no entry in the trigger map covers. ### Release note ```release-note test(qdrant): e2e now runs qdrant with TLS on and checks that a client trusting only the tenant-published CA reaches it, while a wrong or missing anchor and plaintext HTTP are refused. ``` |
||
|
Aleksei Sviridkin
|
7ea71042b7 |
ci(e2e): add a daily isp-hosted install lane on kind (#4712)
## What this PR does
This adds a daily CI lane that installs the `isp-hosted` platform
variant through the `hosted` installer variant. No lane ran either of
them before. The PR, nightly and tag lanes all install `isp-full` on
Talos, where Cozystack brings its own CNI and storage, and that is how
#3376 shipped unnoticed.
What `isp-hosted` changes is what it leaves to the host. The system
bundle swaps kube-ovn and Cilium for a `noop` networking Package and
drops multus, LINSTOR, MetalLB, cozy-proxy, snapshot-controller and the
rest of the shared data-plane set. The iaas bundle refuses to render.
Migrations are not a difference, `isp-full` enables them too.
So the lane runs on kind. kindnet, kube-proxy and local-path come from
the cluster, and there is no Cilium, no LINSTOR and no `replicated` or
`local` StorageClass. It installs the latest GHCR nightly instead of
building anything, at 05:00 UTC after the nightly mirror publishes, and
on `workflow_dispatch`. There is no pull request trigger, because a run
would install the nightly images and not the PR's.
A run passes when every HelmRelease is Ready, `cozystack-platform` and
`tenant-root` included. It also checks that the platform emitted the
`noop` networking Package and none of LINSTOR, MetalLB, multus or
KubeVirt, and that the cluster has no Cilium CRD and no `replicated` or
`local` class.
The lane reuses the e2e sandbox container for tooling and
`collect-report`, attached to the kind network.
`hack/isp-hosted-lane_test.bats` pins the wiring: the Package selects
`isp-hosted`, the install uses `variant=hosted`, the kind DNS domain
matches `networking.clusterDomain`, and kind is pinned by checksum and
digest. Six of its seven tests were red before the implementation
existed.
`hack/sandbox-runner-headroom.bats` gets a named exemption. That check
treats any workflow touching `packages/core/testing` as a lane that
boots the QEMU guests, and this one boots none. The exemption is checked
after the job match, and removing it turns the test red again.
The pins come from the kind v0.32.0 release: the binary sha256
`50030de2…` from its `kind-linux-amd64.sha256sum`, and
`kindest/node:v1.33.12@sha256:3f5c8443…` from its release notes.
v1.33.12 is the Kubernetes version the Talos lanes use, so the variant
is the only thing that differs. All three are plain `env` values in the
workflow, where a Renovate custom manager can match them.
### Real runs
The first real run on this branch,
[36961209755](https://github.com/cozystack/cozystack/actions/runs/36961209755),
went red on `tenant-root`:
```text
tenant-root False Helm install failed for release tenant-root/tenant-root with chart tenant@0.0.0+629b61300902: unable to build kubernetes objects from release manifest: resource mapping not found for name: "allow-external-communication" namespace: "tenant-root" from "": no matches for kind "CiliumNetworkPolicy" in version "cilium.io/v2"
```
This is #3387 reproduced live: `packages/apps/tenant` renders Cilium
policies on a cluster without Cilium. The other releases went Ready,
except `cozystack-basics`, which was still in an upgrade at the last
listing. The fix landed in #4710.
The run after it,
[37089699259](https://github.com/cozystack/cozystack/actions/runs/37089699259),
is green on the first attempt. It installed nightly
`0.0.0-nightly.20261003.37089458387.1`, built from
|
||
|
IvanHunters
|
14eab3cfe3 |
fix(backups): restrict the kafka-metadata CLI Pod and correct demo personas (#4695)
## What this PR does
This fixes two problems in the Kafka and NATS backup demos under
`examples/backups/`.
The Kafka metadata demo ran its CLI Pod without any securityContext, so
a namespace that enforces the restricted Pod Security level would reject
it. The Pod now gets the same restricted context as the CLI Pod in the
topic-data demo: `runAsNonRoot`, the `RuntimeDefault` seccomp profile,
no privilege escalation, all capabilities dropped. The new bats case
reads the overrides passed to `kubectl run` and fails if any of those
fields is missing or the patch is not sent as `--override-type=json`. It
is red on main, and it goes red on each of seven single-field mutations
of the patch while the other five cases stay green.
One known limit: `runAsNonRoot` needs an image that runs as a non-root
user. The default Strimzi image runs as UID 1001 and is fine. If
`KAFKA_IMAGE` is overridden with an image that runs as root, the kubelet
refuses to start the container, and the helper fails only when its 5
minute `kubectl wait` times out.
The kafka and nats walkthroughs told a tenant to run scripts that the
tenant roles forbid. Steps 03, 04, 06, 07 and `cleanup.sh` were in the
tenant column, and the scenario docs and several script comments had the
tenant create the backup Secret. Those rows now say admin and name the
part a tenant admin can do on its own, and the scenario docs say the
same. After this change the persona column in both tables matches the
tenant roles. kafka-metadata has no persona table, so I didn't change
anything there.
These are the tenant rules on pods and backup objects, plus one example
of the per-app secret rules. Each app chart's dashboard Role allows
`get`, `list` and `watch` on a few named Secrets and no other verb on
secrets. Nothing aggregated into the tenant roles grants `pods/exec`,
`pods/log`, `helmreleases`, `objectstorage.k8s.io`, `statefulsets` or
`kafkas.kafka.strimzi.io`.
```yaml
# packages/system/cozystack-basics/templates/clusterroles.yaml:100-112 (cozy:tenant:view:base, every tenant level)
- apiGroups:
- ""
resources:
- pods
- services
- persistentvolumes
- endpoints
- events
- resourcequotas
verbs:
- get
- list
- watch
# packages/system/cozystack-basics/templates/clusterroles.yaml:238-246 (cozy:tenant:admin:base)
- apiGroups: [""]
resources:
- pods
- services
- persistentvolumes
- endpoints
- events
verbs:
- delete
# packages/system/backup-controller/templates/tenant-clusterroles.yaml:35-44 (cozy:backups:admin)
- apiGroups: ["backups.cozystack.io"]
resources:
- plans
- backupjobs
- restorejobs
verbs:
- create
- update
- patch
- delete
# packages/apps/bucket/templates/dashboard-resourcemap.yaml:6-14 (bound at the tenant "use" level)
- apiGroups:
- ""
resources:
- secrets
resourceNames:
{{- range $name, $user := .Values.users }}
- {{ $.Release.Name }}-{{ $name }}-credentials
{{- end }}
verbs: ["get", "list", "watch"]
```
How each changed row runs into those rules:
| Row | What the script needs | Why a tenant can't |
| --- | --- | --- |
| 03 (kafka, nats) | waits on the `HelmRelease` and COSI objects, `get`
on the `bucket-<name>-backup` Secret, kafka also reads the CA Secret in
`tenant-root` | no `helmreleases` or `objectstorage.k8s.io` rule; the
bucket Role covers only `<release>-<user>-credentials` |
| 04 (kafka, nats) | waits on the `HelmRelease` and the Strimzi `Kafka`
or the `StatefulSet`, `create` on `secrets`, `create` on `pods`,
`pods/exec` | pods are `get`/`list`/`watch`/`delete` only, and nothing
grants `create` on secrets |
| 06 (kafka, nats) | `create` on `pods` and `pods/exec` for the CLI Pod,
kafka also `get` on `pods/log` for the refused restore | same pod rules,
and nothing grants `pods/log` |
| 07 (kafka, nats) | same as 04 | same as 04 |
| `cleanup.sh` (kafka, nats) | `delete` on `secrets`, on `backups`, and
on the cluster-scoped `BackupClass` and strategy | `cozy:backups:admin`
deletes only plans, backupjobs and restorejobs, and no tenant rule
covers secrets or cluster-scoped objects |
Step 05 stays a tenant step. Its `BackupJob` comes from
`cozy:backups:admin`, so in practice that means a tenant admin. The nats
demo's CLI Pod has the same missing securityContext as the
kafka-metadata one, and I left it for a separate issue.
Closes #4625
Closes #4636
### Screenshots
<!-- REQUIRED for UI changes: attach screenshots or screen recordings
demonstrating
the visual impact of your changes. PRs with UI changes without
screenshots will not be merged. -->
Not a UI change.
### Downstream repositories
<!--
Cozystack is upstream for repositories that are NOT kept in sync with it
automatically, and nothing in CI compares the two sides, so a change
here can break them silently.
Which change forces what, and which file to touch, is the trigger map in
docs/agents/contributing.md under "Downstream Repositories". Walk it
before you tick anything.
IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM
TO FILL IN:
1. Walk the trigger map against the actual diff of this PR, file by
file. Do not decide from the PR title or from your memory of what you
changed.
2. For every repository you tick, open the follow-up PR there and link
it on the same line. A ticked box with no link claims work that does not
exist, which is worse than an empty box. Search that repository for an
open PR or issue covering it first, and link that instead of filing a
duplicate.
3. When the follow-up is out of scope, or needs a decision that is not
yours to make, open an issue in that repository instead, link that, and
say so in the PR body. Do not push a speculative PR into another
repository just to fill a line here.
4. Do not tick "No downstream repository is affected" to make the
checklist go away. If you are unsure, leave every box empty and say so
in the PR body, so a human decides.
-->
- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:
### 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(backups): the kafka-metadata backup demo now runs its CLI Pod with the restricted Pod Security context, and the kafka and nats backup demos now list steps 03, 04, 06, 07 and cleanup as admin steps, because they need Pod, exec, Secret and cluster-scoped permissions that tenant roles do not grant.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Security**
* Kafka metadata backup CLI Pods are recreated with restricted security
settings when they are not running or lack the required non-root
configuration.
* **Documentation**
* Kafka and NATS guides clarify which setup, restore, and cleanup steps
require administrator access and which job actions tenants can perform.
* Updated examples clarify that the demo provides required storage
Secrets and certificates.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
IvanHunters
|
9239555e6a |
ci(workflows): pin flux, yq and helm-unittest installs (#4645)
## What this PR does Workflows no longer run the flux install script as root. Every step that needed the flux CLI downloaded `https://fluxcd.io/install.sh` and ran it with `sudo`, so whatever that host served at the moment ran on runners that later get registry credentials. PR #4442 pinned the version, but the script was still fetched and executed. All 12 of those steps now use `fluxcd/flux2/action`, pinned to the v2.8.6 commit with `version: "2.8.6"`. The action downloads the release tarball, checks it against that release's checksums file, and adds the binary to `PATH`. Two workflows still took yq from the floating latest release, nightly and the fork e2e lane. They now use the same install the promotion steps already have: yq v4.53.3 checked against a committed SHA-256. I verified that hash against the release's `checksums` file and against the binary itself. The helm-unittest plugin was installed at whatever version was newest. The plugin embeds its own Helm, so a plugin release could change chart test results on a PR that touched no chart. It's pinned to v1.1.2 now, the version CI already runs. A runner that carries a different version gets it replaced. A new contract test scans every workflow for these three tools, because an unpinned install usually comes back by copying an older step. On main it fails on the install script, the missing setup action, the latest-release yq and the unpinned plugin. I ran one mutation per guard, and each one failed only the test meant to catch it. The existing contract tests that checked `FLUX_VERSION` inside the old steps now check for the action step. The e2e sandbox image still runs `install.sh` in its Dockerfile. That's an image build, not a workflow step, so I left it for a separate change. Closes #3619 Closes #3640 Closes #4510 ### Screenshots Not applicable, 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: The diff touches only workflow tool-install steps and `hack/*.bats` contract tests. Nothing in the trigger map reaches them. ### Release note ```release-note ci(workflows): install flux through the commit-pinned upstream setup action instead of running fluxcd.io/install.sh as root, pin yq with checksum verification in the nightly and fork e2e workflows, and pin the helm-unittest plugin version. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Reliability** * CI workflows now use a consistent, pinned Flux version. The yq and Helm plugin setup also verifies or enforces specific versions. * **Tests** * Added checks to ensure workflow tool versions are pinned and consistent, and that downloaded yq releases are checksum-verified. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
40638429bd |
fix(release): collect refs stored as registry/image/tag/digest keys (#4697)
## What this PR does The shared image-ref enumeration skipped refs stored the way upstream ingress-nginx stores them: `registry`, `image`, `tag` and `digest` as separate keys, with the repository in `image` and a digest that has no `@`. Two refs in the tree use this layout, the ingress-nginx controller and its default backend, both in `packages/system/ingress-nginx/values.yaml`. Every tool that reads refs through `hack/lib/image-refs.sh` missed both of them: the promote retag, the nightly mirror, the multi-arch gate, the stitch step and the promoted-packages verifier. I added a sixth YAML shape that joins `<registry>/<image>@<digest>`. It fires only on a complete `sha256:` digest, so upstream's unpinned default (`digest: ""`) and image maps without a digest emit nothing. It checks that `digest` is a string before `test()`, same as shape 3 does. The library header and `docs/agents/image-refs.md` now say "the YAML sub-shapes listed" instead of a number, so the next shape doesn't make the count wrong again. The real tree now gives 93 refs instead of 91, and the two new ones are exactly the ingress-nginx pair. The retag and mirror output does not change today, because both refs sit outside the owned registry and the ownership filters drop them. Both digests are multi-arch indexes, so `hack/verify-multiarch.sh --refs-file` with just those two passes and the release gate stays green. To show the consumers really pick the shape up, I ran both dry-runs on a copy of the tree with the controller moved under the build registry, the host being the whole `registry` value. In that copy the controller is the only build-registry ref. Before: ```text $ REGISTRY=iad.ocir.io/idyksih5sir9/cozystack hack/promote-retag.sh 1.99.0 --dry-run No cozystack-owned digest-pinned image refs found under iad.ocir.io/idyksih5sir9/cozystack/ — is this the rc's baked tree? exit 1 $ hack/nightly-mirror.sh 1.99.0 <tree>/packages --dry-run No cozystack-owned digest-pinned image refs found under iad.ocir.io/idyksih5sir9/cozystack/ in '<tree>/packages' — is this the baked main tree? exit 1 ``` After: ```text $ REGISTRY=iad.ocir.io/idyksih5sir9/cozystack hack/promote-retag.sh 1.99.0 --dry-run ▸ iad.ocir.io/idyksih5sir9/cozystack/ingress-nginx-controller sha256:a8ddcee59b63f6cbf93f4d3c9ffa1d1dda3c1d39497748d4484655356c68b3f6 DRY-RUN skopeo copy --multi-arch all docker://iad.ocir.io/idyksih5sir9/cozystack/ingress-nginx-controller@sha256:a8ddcee5… docker://iad.ocir.io/idyksih5sir9/cozystack/ingress-nginx-controller:1.99.0 exit 0 $ hack/nightly-mirror.sh 1.99.0 <tree>/packages --dry-run ▸ iad.ocir.io/idyksih5sir9/cozystack/ingress-nginx-controller -> ghcr.io/cozystack/cozystack/ingress-nginx-controller sha256:a8ddcee5… DRY-RUN skopeo copy --multi-arch all docker://iad.ocir.io/…/ingress-nginx-controller@sha256:a8ddcee5… docker://ghcr.io/cozystack/cozystack/ingress-nginx-controller:1.99.0 DRY-RUN skopeo copy --multi-arch all docker://iad.ocir.io/…/ingress-nginx-controller@sha256:a8ddcee5… docker://ghcr.io/cozystack/cozystack/ingress-nginx-controller:nightly exit 0 ``` The red test is a new case in `collect_refs_from_file understands every YAML shape`. It has a fixture in the ingress-nginx layout, with a non-string digest in front of it, and three near-misses that must emit nothing: no digest, an empty digest, and a digest without `image`. On main the positive case fails with `got: <nothing>`. Removing the rule, the registry join, the full-digest check, or the type guard on `image` or on `digest` turns that test red, and only that one. The bats suites of all library consumers pass with both the old and the new library. Two gaps remain in the tests and I left them out on purpose. No fixture covers the new rule without a `registry` key, and none pins its type guard on `registry`. Shapes 2 and 3 have the same unpinned `registry` guard on main. The overlay script has no such gap. `hack/overlay-main-images.sh` does not use the library, and its line classifier already accepts `image:`, `registry:`, `tag:` and `digest:` lines. Closes #4518 ### Screenshots Not a UI change. ### Downstream repositories I walked the trigger map against the diff. None of the changed files are in it. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(release): the image-ref enumeration used by promotion, the nightly mirror and the multi-arch gate now reads refs stored as separate registry/image/tag/digest keys, the layout of the ingress-nginx controller and default backend. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Image references can now be read from configurations that store the image name and SHA-256 digest separately, with an optional registry. * **Bug Fixes** * Incomplete or invalid image and digest values are ignored rather than producing malformed references. * **Documentation** * Documented the additional image storage format. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
9196b9798e |
fix(e2e): bound the dataplane capture collector by wall clock (#4640)
## What this PR does The e2e data-plane collector limited itself by counting pods and LoadBalancers, but both of its callers stop it by the clock. `hack/cozytest.sh` gives it 600s and the Chainsaw catch gives it 300s. One affected pod could already run past the smaller limit, and the LoadBalancer capture, which runs last, was the part that got lost. I moved the collector to a wall-clock budget and fixed the other issues filed against it. - Every kubectl call now goes through `dp_run`. It cuts the call's timeout to what is left of `COZY_DATAPLANE_BUDGET` and starts nothing once the budget is spent. The default is 270s, and `cozytest.sh` passes 570s. The pod section stops 90s early, so the LoadBalancer section always gets time to start. A call the deadline cut says so, a call cut by its own bound names that bound, and a refused call says it was not started and names its section; exec blocks carry that line inside the block. The node capture and the LoadBalancer probe used to blame an unanswered lookup or a missing cni-server when the budget stopped a call. Now they name the budget (#3671). - A static test fails on any kubectl call that bypasses `dp_run` and lists the lines. It matches by command position, so a call inside a double-quoted `$(...)` is still checked (#3694). - Per-pod and per-LoadBalancer capture files join namespace and name with `_`, which neither can contain. A second run into the same directory appends, with the run separator `capture-notes.txt` already uses (#3679). - `capture_node` and `capture_lb_node` no longer write their callers' loop variables. A static test fails when a function assigns a name that a `read -r` loop or another scope owns (#3675). - The tests that shrank every timeout to one second now cut only the read under test. A `timeout` shim decides by argv, so a slow machine can't cut a read the test needs answered (#3685). - `cilium-dbg bpf lb list` runs unfiltered in every affected node's capture, not only for a failing LoadBalancer (#3689). The call-site comment in `.chainsaw.yaml` and the collector paragraph in `docs/agents/e2e-testing.md` now match the code. Every behaviour change has a test in `hack/capture-dataplane.bats` that failed on the code before it. Two parts of the issues live elsewhere. The runner stopping a bats file at its first failing test is #4620, and a conntrack read filtered by service VIP is #4623. From #4629 I fixed the items on lines this PR already changes: the node capture now gets the run separator, a failed speaker log read writes a note, the endpoint tcpdump says when it falls back to the geneve interface, and the per-read timeout knobs are validated (a zero there would otherwise look like a spent budget). The rest of #4629 stays open: `ovn-lflows.txt` and `lb-speaker-logs.txt` are still replaced on each run, and two comments still describe older behaviour. - Closes #3694 - Closes #3671 - Closes #3679 - Closes #3675 - Closes #3685 - Closes #3689 - Refs #4629 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(e2e): bound the failure-path data-plane collector by a wall-clock budget, keep its per-pod and per-LoadBalancer captures from overwriting each other, and capture the cilium service map for ClusterIP failures. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Data-plane diagnostics now follow a configurable time budget, reserving time for LoadBalancer captures and stopping operations when the budget is spent. * Capture reports distinguish failed, successful, unknown, and budget-limited checks. LoadBalancers with successful probes are skipped for heavy capture. * Repeated captures are preserved together, and limits help control LoadBalancer diagnostic collection. * Diagnostics include additional network details from relevant nodes during probe replay. * **Documentation** * Updated guidance covers capture time limits and how unanswered EndpointSlice reads are reported. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
f209c477e2 |
test(e2e): read the opensearch release history through the pinned name (#4689)
<!-- Thank you for making a contribution! Here are some tips for you:
- Use Conventional Commits for the PR title: `type(scope): description`
- Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore
- Scopes are not an exhaustive list — pick the most specific scope for
the change and extend the list when a genuinely new area appears.
Examples:
- System components: dashboard, platform, operator, cilium, kube-ovn,
linstor, fluxcd, cluster-api
- Managed apps: postgres, mariadb, redis, kafka, clickhouse,
virtual-machine, kubernetes
- Development and maintenance: api, hack, tests, ci, docs, maintenance
- Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or
add a `BREAKING CHANGE:` footer
- If it's a work in progress, consider creating this PR as a draft.
- Don't hesistate to ask for opinion and review in the community chats,
even if it's still a draft.
- Add the label `kind/backport` if it's a bugfix that needs to be
backported to a previous version.
-->
## What this PR does
The opensearch suite now reads HelmRelease history through the shared
jsonpath, and a unit test fails if any Chainsaw suite goes back to a
copied literal. Before this, the suite carried its own copy of the
expression, and the upstream contract test in `internal/fluxcontract`
only checks `HELMRELEASE_HISTORY_JSONPATH`. I checked the copy against
helm-controller `api` v1.5.1, the version go.mod pins: the history field
is `history` and each snapshot has `status`. So the read works today, it
was just not pinned.
The suite passes `-o "jsonpath=${HELMRELEASE_HISTORY_JSONPATH}"` now,
which gives kubectl exactly the same argument as before. The new test
reads every suite under `hack/e2e-chainsaw/`. A suite that calls
`helmrelease_has_remediation_cycle` must fill the local it hands the
guard from the shared name, and no suite may assign that name itself. It
reuses the matchers that already pin `run-kubernetes.sh`. The test is
red on main and green with the fix.
One known limit. The second-assignment check counts over the whole file,
which fits `run-kubernetes.sh` because it runs as one shell. A Chainsaw
suite runs each `script` step in its own shell, so an unrelated
`statuses=` in another opensearch step would turn the test red with
nothing broken. That fails toward red, never toward a silent pass, so I
left it.
Closes #4550
### 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
test(e2e): the opensearch suite reads HelmRelease history through the shared jsonpath, which is now pinned against the upstream Flux type for every Chainsaw suite that runs the remediation guard.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Tests**
* Expanded validation across Chainsaw end-to-end suites to check
consistent HelmRelease history handling in remediation checks, including
the order and isolation of script steps.
* The OpenSearch suite now uses the shared history lookup. Empty-history
checks and remediation-cycle failure handling are unchanged, and there
is no change to end-user behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
IvanHunters
|
b4fbaf00a2 |
test(e2e): keep gateway and redis-tls op ceilings above their own waits (#4643)
<!-- 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 Two Chainsaw script ops had a timeout smaller than the waits inside them. When the op ceiling fires first, Chainsaw kills the shell: the script never prints its own error, and a cleanup trap never runs. The gateway listener-set step could poll for 660s under a 6m op. Its cleanup trap also deleted two tenants with no limit on the wait. Each delete now runs under an outer `timeout`, because kubectl's own `--timeout` does not cover discovery or a stalled request. A tenant delete gets 300s, the same 5m that `.chainsaw.yaml` gives a delete op. The parent is deleted only after the child is gone, because deleting a parent while its child is still uninstalling wedges the parent's cleanup. A failed tenant delete fails the step with kubectl's own error, so a leaked tenant shows up here and not in a later suite. The op is now 24m, which covers the polls plus those bounds. The redis-tls CA-projection step has 30s read caps that add up to well over its 5m op. Raising the op would not hold, because several reads run in loops. So all reads now share a 240s step budget. No read starts after the budget runs out, and a read that is still running gets cut off when it does. A read that hits its bound, or finds the budget already spent, says which read it was. Two exec checks in the same step blamed the 15s redis-cli cap for any failure, including a refused exec or a missing container. They now print the exit status and what the exec printed. The step also runs under bash now, for pipefail. The sandbox's `/bin/sh` is dash, which rejects `set -o pipefail`. Chainsaw 0.2.15, the version the sandbox image installs, supports a `shell:` field on script ops. Without pipefail, a failed Secret read took base64's zero exit status and was reported as a wrong source name. The private-key check read the projection a second time, so a failed second read made it pass. It now checks the value the step already fetched. I added `hack/chainsaw-op-budgets.bats` to guard all of this. It holds the gateway op above its polls plus its delete bounds, keeps the child-before-parent order, holds the redis budget under its op with every kubectl call going through the clamped helper, and runs the redis step's script against a stubbed kubectl to check each failure message and the clamp. On main, 11 of its 12 tests fail. The one that passes is the healthy-stub control. Closes #4114 Closes #4250 Closes #4246 ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> Not a UI change. ### Downstream repositories - [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 touches two Chainsaw suites and adds one unit test file under `hack/`. It moves or renames nothing a downstream repository reads. ### Release note ```release-note test(e2e): the gateway listener-set and redis-tls projection steps now fail on their own errors instead of being killed by the op timeout, and redis-tls reports the real cause of a failed read or exec. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added automated checks to verify gateway operations have sufficient time to complete and cleanup deletions are bounded and correctly ordered. * Expanded gateway scenarios to cover cleanup failures: if child-tenant deletion fails or times out, the parent is left in place; deletion failures are reported. * Updated Redis TLS checks with bounded read times and a shared execution limit. Failure reports include command output and status, and checks cover private-key material in the decoded projection. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
1ca603a7b2 |
test(e2e): wait for the S3 gateway to accept a key before using it (#4694)
## What this PR does The bucket e2e suite fails now and then because the S3 gateway rejects a key that COSI already reports as granted. The error is `The access key ID you provided does not exist in our records.` from `mc alias set`. In job 109130131391 the BucketAccess assert finished at 21:23:43.57 and the first `mc alias set` failed at 21:23:44.82. In job 109396174094 cap1 and cap2 were accepted, and cap3 was rejected two seconds into the step. Granted access is not the same as a usable key. In seaweedfs-cosi-driver v0.3.1, `DriverGrantBucketAccess` writes the new key into `/etc/iam/identity.json` on the filer and returns. The S3 gateway in SeaweedFS 4.45 reloads that file later, from a filer metadata subscription (`weed/s3api/auth_credentials_subscribe.go`). In our chart the driver writes through `seaweedfs-filer` and the two gateways follow `seaweedfs-filer-client`, so the change may have to pass through the other filer replica first. The suite now waits until the gateway accepts the key before it writes. Every alias in both write steps goes through `await_key`, which uses the same port-forward as the writes and so asks the same gateway pod. It waits only while the answer is the InvalidAccessKeyId text, for at most 60s. Any other error fails at once, and so does an attempt cut off at its 15s cap. When the window runs out, the step fails with a message that names the BucketAccess. One probe costs at most 77s, so both steps go from a 5m to an 8m op timeout. The inner budgets now sum to 409s and 441s, and the budget comments say so. I checked the probe locally against a fake `mc`: ```text case=ok rc=0 attempts=1 elapsed=0s Added `a` successfully. case=lag rc=0 attempts=3 elapsed=5s Added `a` successfully. case=other rc=1 attempts=1 elapsed=0s FAIL: mc alias set for BucketAccess bucket-test-admin failed (exit 1) with an error other than the key not being loaded yet case=never rc=1 attempts=31 elapsed=60s FAIL: S3 gateway still rejects the key of BucketAccess bucket-test-admin after 60s of probing (the gateway never loaded it, or the driver never wrote it) ``` A hanging `mc` fails on its first attempt after 15s with exit 124. `chainsaw lint test` from v0.2.15 accepts the file. The read-only upload check gets fixed along the way. Before, an unloaded `ro-user` key also made the upload fail, and that failure passed as the expected denial. One thing I left as it is: `await_key` is copied into both steps, because chainsaw steps share no shell, and moving it to `_lib/` would run every suite on each edit. Closes #4580 ### Screenshots Not applicable, no UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note test(e2e): the bucket suite waits for the S3 gateway to accept a new key before writing with it, instead of trusting BucketAccess readiness. ``` |
||
|
IvanHunters
|
858ca4d0a3 |
test(e2e): cover monitoring OIDC modes with a Chainsaw suite (#4678)
## What this PR does Monitoring OIDC gets live-cluster e2e coverage again. It had none since the two pre-Chainsaw BATS suites were removed without a port. This adds a Chainsaw suite in `hack/e2e-chainsaw/monitoring/` that ports what they asserted, plus the in-place switch from an inline config to a `secretRef` that nothing covered. The suite takes one Monitoring release in `tenant-test` through three modes in a row: System, CustomConfig with an inline config, and CustomConfig with a `secretRef`. It creates that release itself and never reads the tenant-root monitoring instance, so it keeps working if #4023 or #4479 turns root monitoring off. What each phase checks: - System mode: cozystack-api admits `spec.oidc` and both HelmReleases go Ready. The Grafana CR points `auth.generic_oauth` at the cozy realm, has no `role_attribute_path`, and has `skip_org_role_sync` and `oauth_allow_insecure_email_lookup` forced. The KeycloakClient is confidential and references the generated Secret, which holds 32 alphanumeric characters. The audience scope is bound to this release's clientId. There is no chart-owned `KeycloakRealmGroup`. The users-Job carries the users and the mode, and `grafana-deployment` reads the client secret from the Secret. - Switch to inline: the Grafana CR takes the tenant's map, keeps the forced keys and drops every System-only key. The users-Job reruns with the new list. The KeycloakClient, the audience scope and the client Secret are pruned, and the Deployment stops referencing that Secret. - Switch to `secretRef`: the inline `auth.generic_oauth` block is gone from the Grafana CR. The operator Secret is mounted at `/etc/grafana/oidc` on both the CR and the Deployment. This checks the mount only and does not prove Grafana loads the file, see #4679. - After the install and after each switch, neither release may show a remediation cycle. That means no `failed` or `uninstalled` snapshot, and the latest revision must be exactly 1, then 2, then 3. The revision check is there because helm-controller truncates history after the previous deployed snapshot. An upgrade that failed, rolled back and then passed on retry leaves no failed entry, only extra revisions. I did not assert `GF_PATHS_CUSTOM_INI`. I could not find anything in Grafana that reads it (#4679), and pinning it would make possibly dead wiring part of the contract. The coverage split with the render tests is this. `packages/system/monitoring/tests/oidc_test.yaml` keeps the fresh-install render of each mode, including the CustomConfig negatives (no Keycloak objects, no client Secret). This suite covers what a render cannot see: admission, the Deployment the Grafana operator builds, and what a switch between modes prunes or leaves behind. The Keycloak objects are checked as written, so creating the client in Keycloak is not asserted. Removing it is, because the operator's finalizer holds the KeycloakClient until the client is gone from Keycloak. Before the first apply the suite also waits until the tenant's `cozystack-values` carries `oidc-enabled: "true"`. The outer release copies `_cluster` into the inner release once, so a Monitoring applied right after the tenant is created can render from stale values and fail the System-mode guard for the full 15m. That is what the first CI run on this PR hit. The Keycloak CRDs are required, not probed for. The e2e install always enables platform OIDC, so a missing CRD is a broken install. The removed suites skipped in that case, and their probe could not tell an absent API group from a present one. The selector picks the suite up by name from `cozystack.monitoring-application` and `cozystack.monitoring`. A change to `system/monitoring`, `extra/monitoring`, `monitoring-rd` or `grafana-operator` now runs only this suite instead of the full run. The tenant-root Grafana bring-up stays covered by the install BATS while it exists. One selector unit test relied on `cozystack.monitoring` reaching no suite, so it now renames that source in its copy of the graph. The second commit keeps Keycloak changes on every suite that drives a Keycloak client. Before this PR a change to `system/keycloak` or `system/keycloak-operator` escalated to the full run. With the monitoring suite it would select monitoring alone, because `monitoring-application` is the only source that depends on `keycloak-operator`, and `kubernetes-latest` with its OIDC lifecycle would drop out. `src_to_suites()` now maps both to `kubernetes-latest monitoring`. The mapping has to be explicit because `kubernetes-application` has no `dependsOn` on `keycloak-operator` (#4680). A `postgres-operator` change now also reaches `kubernetes-latest`, through Keycloak's database. The selector comment and the e2e doc no longer use `system/postgres-operator` as the example of a path with a source that reaches no suite. Cost: one monitoring stack bring-up, two in-place upgrades and teardown. I expect 10 to 20 minutes on every full run. The selector change makes the CI run on this PR a full run, so the suite's own timing is in the chainsaw report. A red run can spend up to 16m on each of four waits, since each one sits a minute above the 15m timeout of the release it waits on. The suite cannot run locally, so I checked it this way: - `chainsaw lint test` at v0.2.15, the sandbox version, passes. - I ran the suite's own steps against a local envtest apiserver. Chainsaw applies the real manifests, and a stand-in for helm and the operators renders the chart from the stored Monitoring spec. With the chart intact all nine steps pass. Each of 30 single breakages of the chart or of the simulated controllers turns the expected step red. - `hack/select-e2e_test.bats` and `hack/select-install_test.bats` pass. The new Keycloak selection case fails without the mapping and when `kubernetes-latest` is dropped from it. A browser login through Keycloak is out of scope, same as in the removed suites. Left for later: - KeycloakClient and scope `status` after the operator creates them in Keycloak is not checked. - `hack/e2e-chainsaw/README.md` lists suites by hand. I added `monitoring`, but `backup`, `kafka-metadata` and `kubernetes-proxmox` were already missing. - The `operator diff selects all dependent app suites` selector test could also check `monitoring`. - The comment on the Keycloak mapping says both suites wait for the finalizer to remove the client. The monitoring suite does wait. `kubernetes-latest` only reads once after its upgrade. - The ready checks compare `observedGeneration` with `metadata.generation` through `contains()`. Both are fields of the same object, not a number literal, so the v0.2.15 literal gotcha should not apply, but no live run has passed this suite yet. - The inline fixture carries no `enabled: "true"`, so that phase checks a disabled OAuth block. The asserts do not depend on it. Closes #4051 Closes #3785 ### 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 ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added end-to-end coverage for Monitoring installations using system OIDC, inline custom configuration, and Secret-based configuration. The tests verify readiness, rendered resources, configuration cleanup, and upgrade history. * Updated test selection so Keycloak-related changes also run the Monitoring suite. * **Documentation** * Added Monitoring to the documented Chainsaw suite list and clarified how changed paths determine test coverage. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
6e4c9c4b6a |
fix(ci): harden the PR labeler's title parsing and map recurring scopes (#4666)
## What this PR does Fixes three problems in `.github/workflows/pr-labeler.yaml`. Each one either killed the job or put a PR in the wrong area, and none of them showed up anywhere. - Both maps are now looked up with `Object.hasOwn`. A title typed or scoped `constructor`, `toString`, `hasOwnProperty` or `__proto__` used to resolve to an `Object.prototype` member and crash the step on `l.startsWith`, which left the PR with no labels at all. - Every leading `[Backport ...]` prefix is now stripped, not only the first one. With two prefixes, the second was read as a bracket scope, so the real scope and the `kind/*` label were both lost. The `area/release` label hid the problem by keeping `area/uncategorized` from firing. - Scopes that recur three or more times over the full PR history now map to an area. So does each area's own name (`networking`, `storage`, `database`, `virtualization`, `testing`, `dependencies`). Scope-to-area decisions: - `area/database`: `database`, `etcd-operator`, `opensearch`, `opensearch-operator` - `area/networking`: `networking`, `kubeovn`, `multus`, `kilo`, `hetzner-robotlb`, `ouroboros` (it rewrites the CoreDNS Corefile to fix hairpin NAT, so it is networking, not platform) - `area/virtualization`: `virtualization`, `vm`, `kubevirt-operator`, `kubevirt-cdi`, `kubevirt-instancetypes` - `area/platform`: `operator`, `cozystack-operator`, `controller`, `cozystack-controller`, `cozystack-basics`, `flux-shard-operator`, `migrations`, `scheduler`, `cozystack-scheduler`, `reloader`, `bootbox`, and `lineage`. I put `lineage` here on purpose: no existing label covers it more precisely, and it is too narrow for an area of its own. - `area/storage`: `storage`, `piraeus-operator`, `backupstrategy-controller` - `area/release`: `changelog`, `changelogs` - `area/ci`: `workflows`, `labeler` - `area/dependencies`: `dependencies`, `renovate` - `area/api`: `registry` (the cozystack-api storage layer) - `area/dashboard`: `dashboard-controller` - `area/monitoring`: `monitoring-agents` - `area/keycloak`: `keycloak-configure` - `area/testing`: `testing` - New `area/dx`, added to `.github/labels.yml`: `dx`, `hack`, `cozypkg`. These are developer tooling (hack/ scripts, package Makefiles, cozypkg). `area/build` covers image builds and `area/testing` covers tests, and neither describes this work. Left unmapped on purpose: - `http-cache`: the application was removed from the tree. - `openbao`: already mapped on main. - `apps`, `system`, `docs`: catch-all scopes that don't point to one area. - `fix`, `bugfix`: malformed titles. - `security`, `rbac`, `governance`, `codeowners`, `maintenance`, `images`, `readme`, `telemetry`, `cluster-autoscaler`: no single area fits any of them. I did not add spelling normalisation. `kubeovn` got its own row, since it is the only alternate spelling that recurs. A new contract test, `hack/pr-labeler-contract.bats`, pulls the workflow's own `script:` block out of the file and runs it under node with stubbed `context`, `core` and `github`, so it tests the shipped code, not a copy. On main it is red on the prototype titles (`THROW: l.startsWith is not a function`), on the double backport (only `area/release`), on the newly mapped scopes and on the area-name scopes. It also checks that every map target exists in `labels.yml` and that no key is written twice. The suite fails rather than skips when node is missing. The older node-based suites skip, so this is the first one that needs node on the unit-test runner. Some things stay out of this PR. The scope table in `docs/agents/contributing.md` is a summary. It has no row for `area/dx` and does not mention that an area's own name works as a scope. Separately, the singular `migration` still maps to `area/virtualization` as it did before, while the new `migrations` maps to `area/platform`. A backport PR whose scope is unmapped still never gets `area/uncategorized`, even with a single prefix. The `area/release` added by the strip satisfies the fallback check. This behaviour is the same on main and is not changed here. The test that checks map targets against `labels.yml` passes by itself if the map literal cannot be extracted. The duplicate-key and area-name tests go red in that case, so the suite as a whole still fails. Closes #4125, closes #4124, closes #4122 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(ci): the PR labeler no longer crashes on titles scoped like Object.prototype members, handles nested backport prefixes, and maps more recurring scopes, including a new area/dx. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added the `area/dx` label for issues and pull requests related to developer tooling. * **Improvements** * Pull requests can now be categorized using a broader set of scope aliases. * Repeated leading backport prefixes are recognized and route the pull request to the release area. * Label mapping now avoids treating inherited property names as valid mappings. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
02fbe76418 |
fix(ci): close three blind spots in the e2e test selectors (#4658)
<!-- 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 e2e selectors miss Chainsaw suites that have no `chainsaw-test.yaml` or `.yml` directly in their directory. Chainsaw v0.2.15 also runs a directory that holds only numbered step files such as `01-install.yaml`, and its discovery walks every subdirectory of the path the runner hands it. A suite in either layout ran only when nothing was selected. No narrowed selection named it, the full-suite list left it out, and `select-install.sh --validate` never asked for its mapping. Now `hack/select-e2e.sh` and `hack/select-install.sh` list a top-level directory as a suite when Chainsaw would load a test from it or from anywhere below it. No suite in the current tree uses either layout, so today's suite list stays the same. A file placed directly in `examples/backups/` (a shared `common.sh`, for example) selected nothing, because it fell through to the inert `examples/` rule. It now escalates to the full suite, same as `examples/backups/_lib/`. `README.md` there stays inert through the `*.md` rule. The `test-chainsaw` comment in `packages/core/testing/Makefile` said Chainsaw discovers every `chainsaw-test.yaml`, which was too narrow. I fixed it, and the matching sentences in `docs/agents/e2e-testing.md` too. Every behaviour change has a bats test that fails against the scripts on main. Known gaps I left for a follow-up, none of them a wrong answer today: - The `full_suite_list` helper in `hack/select-e2e_test.bats` repeats the selector's `find`/`sed`, so a bug in that shared pattern would pass every full-suite assertion. The step-file and nested tests compare against literal lists, which covers the new layouts. - The selectors are a little wider than Chainsaw: a broken `chainsaw-test.y*ml` symlink, or one with no Test in it, still lists its suite. That errs toward running more. - A few test comments describe what used to happen rather than why the rule exists. Closes #4630 ### 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 fix(ci): the e2e test selectors now see Chainsaw suites made of numbered step files or nested below a suite directory, and run the full suite for an edit to a file placed directly in examples/backups/. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * E2E suite discovery recognizes supported manifest and numbered-step files at any depth and maps them to the top-level suite. * Changes to files directly under `examples/backups/` trigger full-suite selection. * Suite selection reports an error if discovery fails or finds no suites. * **Documentation** * Clarified supported suite layouts, disabled suite-file handling, discovery and escalation rules, and per-path coverage guidance. * **Tests** * Added coverage for nested suites, numbered steps, unmapped suite layouts, and backup-path selection. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
9485f9118b |
test(tests): make negated BATS assertions able to fail (#3790)
## What this PR does
A hundred and twelve assertions across twelve BATS suites begin with
`!`. A hundred and seven of them pass no matter what they find. POSIX
exempts a command whose return value is inverted with `!` from `set -e`,
so the failing status is dropped where it is produced, and
`hack/cozytest.sh` closes each test function with `return 0`, so it
never resurfaces as the test's own status either. The other five are
followed by `|| { ... exit 1; }`, which reads the inverted status and
does work; they are rewritten too, because telling the two apart needs
the tail of every statement parsed, and a guard that guesses wrong there
is green on a real one.
Switching to the `bats` binary does not fix the hundred and seven. Its
ERR trap carries the identical exemption: `bash -c 'set -eE; trap "echo
TRAP" ERR; ! true; echo REACHED'` prints `REACHED`, runs no trap and
exits zero. The only negated assertion `bats` reports is one standing
last in the test body, where the inverted status happens to be what the
function returns. Two of the hundred and seven stood last; the rest did
not.
What was lost is specific rather than diffuse. `!` is how you spell
"this must not appear", and an absence is what no other assertion in a
test covers. The affected suites assert that a migration ran nothing
destructive, that no image was copied, that a flag was not passed, that
a backup cleanup never flushed the master, that an object does not
exist. On that half of their contract they held for any input.
Each site is now written as `if cmd; then echo "FAIL: ..."; false; fi`,
the form already used in the files where this had been noticed one at a
time, and each carries a message naming what must not have happened.
`hack/bats-no-negated-assert.bats` scans every `hack/**/*.bats` and
refuses the old form, so a new one cannot be added silently. Inverted
conditions (`if !`, `while !`, `until !`) are untouched, because `set
-e` is not what decides a condition. `[ ! -f x ]` is untouched for a
different reason and remains an ordinary failing assertion: the `!`
there is an operator of `test`, not negation of a command, so nothing
exempts its status.
The guard pins its own premise instead of describing it, on both sides.
One test asserts the runner still passes a bare negated assertion; a
second, that neither `set -e` nor the ERR trap sees an inverted status;
a third, that the prescribed `if cmd; then ...; false; fi` does fail,
under both runners, and *at* the form rather than somewhere before it,
since a run that died early would satisfy a bare status check and pin
nothing.
Every rewrite here rests on the third test. If `false` inside a `then`
body ever stopped propagating, the first test would still pass, because
a negated assertion would still be vacuous, while every converted
absence went silently green. A ban that prescribes a replacement has to
test the replacement too.
Every claim the guard makes is pinned by removing the thing it claims.
The recursion is exercised *through* the audit function against a
fixture in a subdirectory, not through a second copy of its `find`
expression: a copy keeps passing while the function loses its recursion,
and an audit that returns empty reads as a clean tree. Recursion matters
because `BATS_UNIT_FILES` is a non-recursive `wildcard`, so `make
unit-tests` never runs a suite placed in a subdirectory.
`hack/e2e-apps/` held such suites until they moved to Chainsaw and is
gone now, which is why the test pins against a fixture and not a real
directory.
The rule's surface was checked in the other direction too. The shape
appears in no `hack/**/*.sh` script as an assertion: the one hit,
`hack/testdata/install-wait/call-shapes.sh`, is fixture text that
`hack/run-kubernetes-install-wait_test.bats` parses, not a command
anything runs. It appears in no Chainsaw `script:` step either, so
`.bats` is the whole surface rather than the part that was convenient to
guard.
The scan is lexical, so its header names the statement boundaries it
owns rather than implying it understands shell: `;` `&` `&&` `||` `{`
`(`, the `)` closing a case pattern, and the words `then` `do` `else`,
each measured against the runner before being added. A keyword may
legally be preceded only by whitespace or by `;`, since every other
separator in front of `then`, `do` or `else` is a shell syntax error,
checked with `sh -n` rather than recalled. The scan therefore admits
exactly those two, and that part of the class is closed rather than
patched one spelling at a time. All three keywords are vacuous after
either spelling, so missing one is a silent miss, the single direction
this scan must not err in, given that its whole design trades noise for
safety. For the same reason it takes `!(cmd)` with no space before the
subshell, which is vacuous on dash and bash alike, and reads through a
`time` or `time -p` prefix, which is vacuous under `bats` and under
cozytest wherever `/bin/sh` is bash.
Five over-counts remain and are deliberate, each named in the header: `(
! cmd )` does propagate its inverted status and is reported anyway; so
would a `!` opening a statement after a redirection `&`; `x=$(true || !
false)` is reported although the `||` reads the inverted status; a
condition continued by a trailing `&&` instead of a backslash is
reported, because the strip only recognises a condition whose end it can
see; and `time ! cmd` is reported although on a dash `/bin/sh`, which
has no `time` keyword, it runs an external `time` that fails. Each costs
one rewritten line, where the opposite error costs a silent pass. They
are enumerated rather than engineered away on purpose, because a removed
error leaves no trace while a listed one tells the next reader what they
are looking at. The scan also errs *quiet* in at least two places, named
here rather than in the header. A file cut off midway through a line
continuation loses its last buffered statement, but that input is not a
runnable test, since `sh -n` rejects it with `unexpected end of file`.
And `time -- ! cmd`, which bash accepts, is vacuous under `bats` and not
peeled by the scan; nothing in the tree uses it, and I stopped adding
spellings there rather than chase every prefix bash allows. The test
naming the shapes claims only what its fixture holds. A lexical scan
over shell cannot promise it has enumerated the language, and a name
that promised it would be false whether or not a gap existed today.
The branch name is narrower than the subject and I am leaving it as it
is rather than churn it. This is not a defect in `hack/cozytest.sh` that
moving to the `bats` binary would fix. It is a POSIX rule both runners
inherit, and moving to `bats` would close two of the hundred and seven
sites.
One suite went red for real once its assertions could fail.
`seaweedfs-guard-parity` forbids four mutable discriminators by grepping
each chart, and both charts carry a template comment block arguing why
those signals were rejected, so the grep matched the argument and would
have reported a chart for documenting the rule it follows. The charts
are correct; the assertion was not. It now strips `{{/* ... */}}` before
matching. Two details there are load-bearing and neither is obvious. The
strip removes the comment *span*, not the line that opens it: dropping
the line is shorter and silently loses a discriminator sharing a line
with a comment, which is a chart classifying on a forbidden signal and a
test reporting that it does not. And the strip reports when the file
ended inside a comment, and the checks refuse to read its output when it
did, because every `{{/*` is read as an opener, so one inside a quoted
string (these charts carry long `fail (printf ...)` messages) would
swallow the rest of the file and leave the checks reading nothing.
One commit widens six comments that explained this trap as applying to a
"negated pipeline". It is not about pipelines, and the narrower wording
invites the conclusion that the other shapes are safe. Comment text
only; each of those files already uses the working form.
The last commit makes the one absence check in
`hack/e2e-install-cozystack.bats` that already used the `if` form tell a
missing object from a failed read. It asserted that the version
configmap was gone on `kubectl get`'s exit status alone, and that status
is 1 both for a NotFound and for an api-server the test cannot reach.
### Verification
Every unit suite this branch touches was run through `hack/cozytest.sh`
on the current base and is green. `migration-seaweedfs-db-adopt` passes
15 of 15 on macOS only when its temp directory sits under `$HOME`. Its
container cannot write into a bind mount under `/var/folders`, and
`/usr/bin/mktemp -d` ignores `TMPDIR`, so the run needs GNU `mktemp`
first on `PATH` as well. Measured on Linux before the rebase, it passed
15 of 15 as is.
The six sites that reached main after this branch was first written, in
`redis-backup-strategy` and `proxmox-e2e-fixture_test`, were each shown
to be able to fail: with the condition inverted, the suite goes red at
that assertion and prints its own `FAIL:` message. The nine sites in
`migration-seaweedfs-db-adopt` carry a two-sided check as well: with the
pattern swapped for one the same test proves is present, the rewritten
form fails the suite and the original `!` form passes it.
The guard itself is green under cozytest with bash and with dash as
`/bin/sh`, under the `bats` binary, and with mawk, gawk and BSD awk.
Each branch of its separator class and of the `time` and `!(cmd)`
handling was removed in turn, and each removal turns the shapes test
red.
`e2e-install-cozystack.bats` is an e2e suite and runs only in the E2E
workflow, so its one changed absence check is not exercised locally.
### Screenshots
Not a UI change.
### Downstream repositories
Walked the trigger map against the diff file by file. The change touches
eighteen existing BATS files, adds one, and adds a convention to
`docs/agents/e2e-testing.md`. It does not touch
`hack/e2e-prepare-cluster.bats`, which is the node contract
`ansible-cozystack` and `talm` restate; it moves and renames nothing
under `hack/` and changes no make target's behaviour, which is what
`ccp` gates on; and it touches no package, schema, enum, default, CRD,
label, annotation or metric.
- [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
```
Closes #3565
|
||
|
Aleksei Sviridkin
|
2df578bffd |
test(e2e): read the opensearch release history through the pinned name
The opensearch suite wrote the history jsonpath out as a literal, so the contract test that runs HELMRELEASE_HISTORY_JSONPATH against the upstream Flux type never saw the expression this suite hands kubectl. A field rename upstream would have reached it only as an empty read. The suite now takes the shared expression, and the contract test reads every Chainsaw suite that calls the guard, so a suite copying the literal, or assigning the shared name after sourcing the library, fails in the unit lane instead of going unchecked. The read is matched inside the script step that calls the guard, because every step runs in a shell of its own and a read in another step never reaches the call. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
874e1e44b7 |
fix(backups): run the kafka-metadata demo CLI Pod as restricted
The kafka-metadata walkthrough started its long-lived CLI Pod with a bare kubectl run, so the Pod carried no securityContext and a namespace enforcing the restricted Pod Security level would reject it. The kafka topic-data walkthrough already layers the restricted context onto its CLI Pod; the metadata demo now applies the same JSON patch. A running CLI Pod left by an earlier run without that context is replaced rather than reused, since a Pod's securityContext cannot change in place. The new bats cases read the overrides kubectl run receives and fail unless both the Pod-level and the container-level fields are present and the patch is sent as --override-type=json, and check that an owned Pod without the restricted context is recreated. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
cc731e3105 |
test(e2e): keep redis-tls failures legible under the op ceiling
The CA-projection step caps each kubectl at 30s, but more capped reads run than its 5m op can cover, so a degraded apiserver could spend the ceiling and leave nothing but "signal: killed". The reads now share a step budget held under the op, and a read that hits its bound or finds the budget spent says which read it was. Two exec checks reported the 15s redis-cli cap for any failure, sending a reader after a slow redis when the exec was refused or the container was missing. They now report the exit status and what the exec printed. The step also runs under bash with pipefail: under dash a failed Secret read took base64's zero status and surfaced as a wrong source name. The private-key check reads the projection already fetched, since a second read that failed made it pass. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
a057d738d1 |
test(e2e): give the gateway listener-set op room for its own waits
The step's polls can wait 660s under a 6m op, and its cleanup trap deletes two tenants with no bound on the wait. When the op ceiling fires first, Chainsaw kills the shell: the poll that was running never reports, the trap never runs, and both tenants stay behind for every later suite. The deletes keep waiting, child before parent, since a parent deleted while its child is still uninstalling wedges its own cleanup. Each delete runs under an outer timeout with a kill grace, because kubectl's --timeout bounds only the wait after the request and not discovery or a stalled request. The parent is deleted only once the child is gone. A failed tenant delete fails the step with kubectl's error, its exit status and the tenant left behind, rather than passing with the leak. The op is sized above the polls plus those bounds, and a unit guard holds it to that sum and the cleanup to that order. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
b1085782c7 |
test(e2e): fail the isp-hosted no-Cilium check on any cilium.io CRD
The check looked for one CRD by name, so a cluster carrying only part of Cilium's CRDs passed as a cluster without Cilium, and a failed lookup read as an absent CRD and passed too. It now lists the CRD groups once, fails if that list cannot be read, and fails on any CRD in cilium.io. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
bd504e0b4e |
ci(e2e): run the isp-hosted install daily against the nightly
The lane installs the latest GHCR nightly on a kind cluster once a day, after the nightly has published. It takes no pull request trigger: a run would install the nightly's images, not the pull request's, and report on code the pull request did not change. The unit test holds the lane's wiring in place, since a lane that quietly installed another variant would test something other than its name says. The sandbox headroom check reads any workflow touching packages/core/testing as one that boots the QEMU guests; this lane starts only the sandbox container, so it is exempted by name, after the job match, and is sized like the others if it ever starts calling the guest targets. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
3e36b35795 |
test(e2e): add an install suite for the isp-hosted variant on kind
isp-hosted shipped unable to converge and nothing noticed, because no lane installs it: every e2e lane runs isp-full on Talos, where Cozystack provides the CNI and storage itself. This suite installs it the way it is meant to run, through the hosted installer variant on a cluster that brings its own CNI, kube-proxy and default StorageClass, and passes only when every HelmRelease is Ready and the variant really left out the data plane it delegates to the host. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
3a07885b70 |
fix(platform): resume a retried migration Job from the stamped version
The migration hook Job gets CURRENT_VERSION once, at render time, and a failed pod is retried with that same environment. Each migration that finished has stamped the cozystack-version ConfigMap by then, yet the retry started from the rendered version and ran those migrations again. Migrations are written to run once, so a re-run is not safe in general. The runner now starts from the version stamped in the ConfigMap. A failed read aborts the pod, since falling back would bring the re-run back and the Job retries anyway. The rendered value is kept only when the ConfigMap holds no integer version, which is also the case where the chart could not derive a real one from it. An all-digit stamp past the shell's integer range aborts too: its comparison fails inside an if, out of reach of set -e, and the Job would go green having run nothing, while the chart renders that stamp as 0. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
bf03c84711 |
chore(deps): move to the Kubernetes 1.37 libraries without the apimachinery fork (#4473)
## What this PR does This moves the root module's `k8s.io/*` from v0.35 to v0.37.1 and controller-runtime from v0.23 to v0.25.1, and drops the `k8s.io/apimachinery` fork on the way. Supersedes #3001, #4467 and #4472; those were a stack of the same commits. The four commits are meant to be read one by one, and each builds and passes the tests on its own: 1. **Plain watches stop getting the initial-events-end bookmark.** The apiserver defaults `sendInitialEvents=true` onto any watch with an empty or `0` resourceVersion and leaves bookmarks off, and our aggregated storages took that flag alone as a WatchList request. From `k8s.io/apiserver` v0.36 the watch handler compiled into cozystack-api panics on that bookmark, so `kubectl get -w` on any cozystack-api resource would die after the bump. The marker now needs both flags, the same condition the watch cache uses. There are tests per storage and one through the real handler chain. 2. **The apimachinery fork goes.** Every apps kind shares the `Application` Go type, and stock `Scheme.ConvertToVersion` picks the first kind registered for it. The fork patched that; the apps group now encodes through a serializer that keeps the kind the object carries. A handler-chain test covers create, get, list, update, watch, apply and table for two kinds and fails without it. 3. **MutatingAdmissionPolicy stays out of admission.** From v0.36 the plugin is on by default and refuses every write until it has synced `mutatingadmissionpolicies`, which cozystack-api has no RBAC to list and which clusters before 1.36 do not serve. Without this, every create, update and delete would fail after the bump. It is also re-applied in `Complete`, since `--disable-admission-plugins` replaces the list. 4. **The bump itself**, with the regenerated clients, openapi and strategy CRDs. Commits 2 and 4 are where the review effort goes; 1 and 3 are small. Known changes in behaviour: - Without the fork, the row objects of a Table requested with `includeObject=Object` carry the first registered app kind. kubectl asks for that only with `--sort-by`, and it sorts on fields, not on the kind. Nothing at this layer can fix it, because the handler converts through `APIGroupInfo.Scheme`, a concrete `*runtime.Scheme`. Admission webhooks and ValidatingAdmissionPolicy still see the right kind. - The alpha PodSpec field `workloadRef` became `schedulingGroup` upstream. Once the new strategy CRDs are applied, the apiserver prunes `workloadRef` from stored strategies it serves, and their next write erases it. It only ever worked behind an alpha feature gate. The branch is rebased onto the Renovate rollup (#4447), with `go mod tidy` and codegen rerun on top of it. ### 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): cozystack-api and the controllers build against the Kubernetes 1.37 libraries without the apimachinery fork. Plain watches no longer receive the WatchList initial-events-end bookmark, and objects in Table rows requested with includeObject=Object may carry another app's kind. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Application API responses now preserve the requested resource kind across operations, including lists and watches. * Backup strategy resources support additional Kubernetes pod settings, including probe protocols, volume ownership options, and scheduling groups. * Backup strategy schemas include updated Kubernetes pod fields and descriptions. * **Bug Fixes** * Initial-event watch bookmarks are sent only when requested; plain watches continue without an ending bookmark. * Mutating Admission Policy is disabled by default, including when admission plugin options are customized. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
9809b22280 |
fix(vm-disk): report DataVolume readiness through WorkloadsReady (#4380)
## What this PR does Nothing on a vm-disk application tells whether CDI ever finished populating the disk. Its WorkloadMonitor has no pods to count, and `Ready` follows the Helm release. So an import or clone that never completes can leave a green application over a disk nobody can use. cozystack/cozystack#2908 is one way to get there: a clone into a StorageClass that cannot place a replica where the source data sits. The expensive part there was the silence, not the stuck clone. The disk state was in the DataVolume phase all along, and nothing read it. With this change the WorkloadMonitor in cozystack-controller reads it. The monitor lists the DataVolumes its selector matches and reports not operational while any of them is in flight, Failed, Unknown or has no phase. An upload counts as in flight until it completes. That surfaces on the application as `WorkloadsReady=False` with reason `DataVolumeNotReady` and a message that names the DataVolume and its phase, for example `DataVolume vm-disk-x is ImportInProgress`. `Ready` still follows the HelmRelease, as for any other kind (`convertHelmReleaseToApplication` in `pkg/registry/apps/application/rest.go`), and for a vm-disk it no longer waits on the DataVolume at all. So the disk state has to be visible outside `Ready`, and this PR puts it where operators look by default: - `kubectl get` on any application kind gets a `WORKLOADS` column next to `READY`. It shows the `WorkloadsReady` status, or `<none>` when the application has no WorkloadMonitor. - The console badge turns amber and reads `DataVolumeNotReady` while `Ready` is True and `WorkloadsReady` is not True for that reason. A failing `Ready` still wins. The console shows only the disk case: other kinds need a separate decision, because a stopped VM legitimately has no pod and reports `WorkloadsReady=False` too. A Helm readiness gate on the DataVolume would be the obvious alternative, and it is not used on purpose. helm-controller runs that wait inside its reconcile, so an importing disk would hold one of the shard's worker slots for the whole install window, and a disk stuck for good would keep taking the slot back. cozystack/cozystack#4145 has the details. The monitor costs no slot, keeps watching after install, and covers existing disks without anyone writing to them. Without any gate, the install would still wait on the DataVolume. helm-controller's default poller falls back to a resource's `Ready` condition for kinds it has no rule for (`checkReadyCondition` in fluxcd/cli-utils `pkg/kstatus/status/status.go`), so a vm-disk install could hold a worker slot for the whole install timeout. The ApplicationDefinition therefore gets a `healthCheckExprs` entry for DataVolume with `current: "true"`, the same idiom postgres-rd uses for its ScaledObject. With it Helm never waits on the DataVolume. The entry is pinned by a helm-unittest in `packages/system/vm-disk-rd`, which gets a `test` target for it. Release settings reach a HelmRelease only when the application is created or updated, so an existing disk gets the entry on its next update. The reconciler that syncs definitions onto existing releases copies chartRef, valuesFrom and labels only. That affects any kind with release settings, so it is filed as cozystack/cozystack#4409 and not fixed here. `WorkloadsReady` covers every disk right away, since the monitor needs no write to the application. CDI copies the DataVolume labels onto the PVC it creates and makes the DataVolume its controller, so the monitor also publishes a `pvc-<name>` Workload for the disk. That Workload reads not operational while its DataVolume is not ready, even though the PVC is already Bound, so `kubectl get workloads` agrees with `WorkloadsReady`. While the DataVolumes cannot be read it keeps the verdict it has, like the monitor. The WorkloadMonitor CRD now requires at least one selector label. An empty selector would match every Pod, PVC, Service and DataVolume in the namespace. No chart renders one. When an application has several monitors that are not operational and only some of them name a cause, `WorkloadsReady` carries the named messages and reason only. A vm-disk has one monitor, so it does not hit this. ### Which phases count as ready Settled phases pass: `Succeeded`, `PendingPopulation`, `WaitForFirstConsumer`, `Paused`. A disk with `bindImmediately: false` on a `WaitForFirstConsumer` class rests in `PendingPopulation` or `WaitForFirstConsumer` until a VM consumes it. A multi-stage import rests in `Paused` until its next checkpoint, and the vm-disk chart renders no checkpoint source. These report ready while holding no data, because that is where such a disk is supposed to sit. `UploadReady` is not one of them. CDI enters it when the upload server pod is running and ready, and leaves it for `Succeeded` only when that pod succeeds (`updateStatusPhase` in `pkg/controller/datavolume/upload-controller.go` at v1.66.1), so the one phase covers a disk nobody has uploaded to and a half-uploaded one. A `source: upload` disk therefore reports `WorkloadsReady=False` from creation until `virtctl image-upload` completes. A phase the controller does not know also reads as ready, so a CDI release that adds or renames a phase cannot turn working disks red. The cost goes the other way: a renamed in-flight phase silently reads as settled. The list wants a re-read whenever the CDI version moves. It matches the named phases of the CDI v1.66.1 API, the version vendored in `packages/system/kubevirt-cdi-operator`. ### Clusters without CDI CDI comes with the IaaS bundle, while cozystack-controller runs everywhere, and CDI can be installed while the controller is running. A watch on a kind whose CRD is absent never syncs, and then the whole controller does not start. So the watch is not set up at startup. A runnable adds the DataVolume watch once the kind is served. It waits for the first reconcile, because only from then on does a new watch start at once, and then checks discovery right away and every 30s after that. It installs the DataVolume reader only after the DataVolume informer has synced within a timeout, because a reconcile context has no deadline and a List on an informer that never syncs would block every WorkloadMonitor reconcile. The watch source is started once: a controller-runtime `Kind` source adds an event handler to the informer and never removes it, so on a timeout the next check waits on the source it already has instead of starting another one. RBAC needs no change: the controller's ClusterRole already has read on everything. When the DataVolumes cannot be read, because the reader is not installed yet or the List fails, the monitor still publishes its replica counts and keeps its last DataVolume verdict, and a List error is returned so the request is retried and recorded as a `DataVolumesUnavailable` Warning event on the monitor. The status of a monitor without DataVolumes is not affected by a failing List. A disk whose verdict is already recorded does not read as ready in the window after a controller restart. A monitor with no recorded verdict yet, which is every monitor right after the upgrade that brings this change and any disk created right after a restart, reads ready until the DataVolume watch has synced. Because the watch starts on the first reconcile rather than on the next discovery tick, that window is the time to sync the DataVolume informer plus at most one 5s requeue, not a 30s poll interval. The window is not zero, since a monitor reconciled in it is published before its DataVolumes can be read. From then on, a reconcile that runs before the watch has synced is requeued after a few seconds, so the DataVolume events that queue it are not lost. A pass that changes nothing in the monitor status does not write it, so that requeue sends no status writes to the API server. Once the reader is installed, every monitor that holds a DataVolume verdict is queued once more. So a verdict whose DataVolume was deleted while the controller was down is recomputed then, not at the periodic resync. ### Tests The table test in `workloadmonitor_controller_test.go` covers the named phases of CDI v1.66.1, plus Failed, Unknown, an empty phase and a missing status. A second table checks the message and reason for an importing, a failed, a phaseless and a populated disk, and a third test lists two stuck DataVolumes in both orders and expects one message. It also covers a DataVolume outside the selector, the kind not being served, a List error that still publishes the replica counts, and a missing reader that keeps the last verdict. The watch tests check that the watch starts on the first reconcile and not before it. A source that never syncs leaves no reader and is retried on the same source, with the reconciles in between still requeued. A List failure records the Warning event, the sync window records none, and an unchanged pass writes no status. A manager that stops during the wait gets the cancellation back, and a reconcile that runs while the watch syncs is requeued without an error. A table test covers the PVC Workload: importing, populated, a PVC with no DataVolume controller or a controller of another kind or group, and the unread cases. A helm-unittest in `packages/system/cozystack-controller` pins the selector's `minProperties`. `rest_table_test.go` pins the `WORKLOADS` column, `rest_workloads_test.go` the condition message and reason, and `status.test.ts` the console badge, including a halted VM that stays green. In `hack/e2e-chainsaw/vminstance` the existing VMDisk scenarios now wait for the DataVolume to be Ready, then for `Ready` and `WorkloadsReady` on the VMDisk itself. That replaces the wait on the HelmRelease. Those asserts would pass even if the monitor ignored DataVolumes, so a new scenario imports from an `.invalid` URL. It first waits for the importer to have failed, which CDI reports on the DataVolume as `Running=False` with reason `Error`, and then for `Ready=True` with `WorkloadsReady=False` and reason `DataVolumeNotReady` on the VMDisk. Without the first wait the VMDisk assert passed in about a second, on a DataVolume that had no phase yet. The e2e runs on a fresh install only, so nothing here exercises an existing release across an upgrade. ### What it does not do #2908 lists possible fix directions, and none of them is here. The cross-class clone still never completes, it just stops being invisible. Keep the issue open. While the DataVolume watch syncs, every monitor is still requeued. Only the ones whose selector reaches a DataVolume need it, but the controller cannot tell which those are before it can read DataVolumes. Closes #4449 ### Screenshots There is no screenshot: I had no running console to take one from. The badge change is pinned by the vitest cases in `packages/system/dashboard/images/console/apps/console/src/lib/status.test.ts`: a VMDisk whose `WorkloadsReady` has reason `DataVolumeNotReady` goes amber, and a halted VMInstance with the generic reason stays green. ### Downstream repositories I walked the trigger map against the diff. cozystack/terraform-provider-cozystack is affected: its `wait_for_ready` reads only `Ready`, so on a `cozystack_vmdisk` it returns before the disk is populated once this lands. That needs a decision on the provider side, so it is an issue there. The release-time bot regenerates `packages/apps/vm-disk/README.md` into the website's virtualization pages, and `vm-disk` is in the `VMS` list that generator reads, so the website needs no follow-up. The ApplicationDefinition keeps its kind, plural, prefix and path. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: https://github.com/cozystack/terraform-provider-cozystack/issues/59 - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(vm-disk): report whether CDI finished populating a disk in the WorkloadsReady condition, which names the stuck DataVolume and its phase. kubectl get shows WorkloadsReady in a new WORKLOADS column for every application kind, and the console badge turns amber while a disk is not populated, so a disk whose import or clone never completes no longer looks healthy. Ready of a new or updated vm-disk no longer waits for the DataVolume, so wait on WorkloadsReady to know the disk is populated. ``` |
||
|
Aleksei Sviridkin
|
0a3546e126 |
test(e2e): tell a missing object from a failed read
The assertion that the no-delete guardrail can be bypassed decides on `kubectl get`'s exit status alone. That status is 1 for a NotFound and 1 for a connection, auth or discovery failure, so an api-server the test cannot reach reads as proof the configmap is gone. `--ignore-not-found` collapses the NotFound alone into success with empty output, which leaves the status free to mean a failed read. Read the two separately: a non-zero status fails the test naming the read that did not answer, non-empty output fails it naming the object that must not be there. Capture stdout only. kubectl writes deprecation and throttling notices to stderr on a call that succeeded, and folding those into the value under test would report an object that is not there. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
cd448fdc04 |
test(bats): widen the comments describing the errexit exemption
Six suites explain why they avoid `! cmd` by saying errexit is suppressed for a "negated pipeline". The exemption is not about pipelines: a bare `! true`, a negated `[ ... ]`, one inside a `for` body or a brace group, one in a case branch, and one following `;`, `&`, `&&` or `||` are all exempt too. Anyone reading the narrower wording could conclude the other shapes are safe. One of the six carries the sentence twice, and the second copy is split across two physical lines -- `` `!` `` ending one and `pipeline)` opening the next -- so a line-oriented search for the phrase does not see it. Prose wraps; a grep for it has to fold the lines first. Comment text only; every one of these files already uses the working form the comment recommends. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
5137d98e71 |
test(bats): refuse a negated assertion across every suite
A negated assertion is the natural way to spell "this must not
appear", and an absence is what nothing else in a test covers, so the
errexit exemption lands where it is least likely to be noticed and
invisible in a green run. Switching runners does not lift it: the ERR
trap the bats binary reports through carries the same exemption, so
neither runner sees a negated assertion that has any statement below
it.
Add a suite that scans every hack/**/*.bats, subdirectories included,
and fails on a statement whose first token is `!` -- naming the file,
the line the statement opens on, and the form to use instead.
Statement boundaries are the POSIX separators, each measured against
the runner before being admitted: `;` `&` `&&` `||` `{` `(`, the `)`
closing a case pattern, and the words `then` `do` `else`. A keyword may
legally be preceded only by whitespace or by `;` -- every other
separator in front of one is a shell syntax error, checked with `sh -n`
rather than recalled -- so the scan admits exactly those two and that
part of the class is closed rather than patched a spelling at a time.
Both spellings are vacuous under the runner, so missing either is a
silent miss, the one direction this scan must not err in. For the same
reason the scan takes a `!` with no space before its subshell,
`!(cmd)`, which is vacuous on dash and bash alike, and reads through a
`time` or `time -p` prefix, which is vacuous under bats and under
cozytest where /bin/sh is bash. A scan that wants `!` and then
whitespace as the first token misses both. On a dash /bin/sh `time` is
not a keyword and `time ! cmd` fails, so there it is an over-count.
Five over-counts remain, all erring loud, each named in the header:
they are enumerated rather than removed, because a removed error
leaves no trace while a named one tells the next reader what they are
looking at.
Inverted conditions are untouched because errexit is not what decides
a condition. `[ ! -f x ]` is untouched for a different reason and
stays a normal failing assertion: the `!` there is an operator of
`test`, not negation of a command, so nothing exempts its status.
Each claim the suite makes is pinned against removing the thing it
claims, and the separator class is measured branch by branch rather than
only at its newest entries. The recursion is exercised through the audit
function on a fixture in a subdirectory rather than through a second
copy of its find expression, because a copy passes while the function
loses its recursion -- and an audit that returns empty reads as a clean
tree. The condition-strip carries fixtures that put the negation after
an `&&` inside the condition, where only the strip can save it. The
continuation fold is pinned by a fixture whose negation sits on the
continued line, so folded and unfolded name different lines. Both
keyword spellings, the `time` prefix and `!(cmd)` have fixtures of
their own. The subshell over-count carries a fixture for both of its
halves -- that the scan reports the shape, and that the shape really
does fail.
Both halves of the premise are pinned, not just the half about the
banned form. A test asserts the runner still passes a bare negated
assertion; a second asserts the prescribed `if cmd; then ...; false; fi`
does fail, under both runners, and fails AT the form rather than
somewhere before it. Every converted absence rests on that second
half, and its failure mode is asymmetric: were `false` in a `then`
body to stop propagating, the first test would still pass while every
conversion went silently green.
The rule joins the E2E conventions doc as its own section, and the
inline list in AGENTS.md that an agent may read instead of the doc --
it belongs beside the EXIT-trap ban for the same reason, both leaving
a green suite that checks nothing.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
||
|
Aleksei Sviridkin
|
e5a1ca853f |
fix(ci): run every Keycloak client suite on a Keycloak change
With a monitoring suite in the tree, a change to system/keycloak or system/keycloak-operator stopped escalating to the full run and selected monitoring alone, because monitoring-application is the only source that declares the keycloak-operator edge. That dropped kubernetes-latest, whose OIDC lifecycle also creates a KeycloakClient and waits on the operator's finalizer to remove it. The mapping is explicit because kubernetes-application has no dependsOn on keycloak-operator. A postgres-operator change now reaches kubernetes-latest too, through Keycloak's database. The selector comment and the e2e doc no longer name system/postgres-operator as a path with a source that reaches no suite, which stopped being true when monitoring got its suite. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
472f962eff |
test(e2e): cover monitoring OIDC modes with a Chainsaw suite
Monitoring OIDC lost its live-cluster coverage when the two pre-Chainsaw BATS suites were removed without a port. Render tests pin each mode on a fresh install, but not admission by cozystack-api, the Deployment the Grafana operator builds from the CR, or what an in-place mode change leaves behind on a running release. The suite drives one tenant-test Monitoring release through System, CustomConfig with an inline config, and CustomConfig with a secretRef, so a single stack bring-up covers all three and the upgrades between them: the cozy-realm client, scope and client Secret are pruned when System mode is left, and the inline generic_oauth block is gone once the secretRef mount arrives. The monitoring package sources now map to this suite, so a change to system/monitoring, extra/monitoring, monitoring-rd or grafana-operator selects it alone instead of escalating to the full run, and the selector test that relied on cozystack.monitoring reaching no suite seeds a source that does not. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
40dac09aaa |
test(bats): spell absences so they can fail
An assertion written `! cmd` cannot fail a test. POSIX exempts a command whose return value is inverted with `!` from errexit, so the status is dropped where it is produced. hack/cozytest.sh then closes each test function with `return 0`, and the bats binary's ERR trap carries the same exemption -- it reports only a negated assertion standing last in the body, where the inverted status happens to be what the function returns. Every absence these suites assert was written that way: a command that must not reach the log, an image that must not be copied, a flag that must not be passed, an object that must not exist. On that half of each suite's contract the tests held for any input. Rewrite each as `if cmd; then echo "FAIL: ..."; false; fi`, the form already used where the trap had been noticed one file at a time, and give every one a message naming what must not have happened. The message states the property, not the pattern: where the forbidden string is a substring of a required value -- an https endpoint the test requires in its http form -- naming the string would describe the opposite of what is asserted. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
a72236687c |
test(seaweedfs): read the naming guard's logic, not the prose about it
The parity suite forbids four mutable discriminators -- claim
timestamps, ready-replica counts and the two birth-order flags -- by
grepping each chart's whole file. Both charts carry a template comment
block arguing why those signals were rejected, so the grep matched the
argument and would have reported a chart for documenting the rule it
follows. It never did report: the checks were spelled `! grep`, a form
POSIX exempts from errexit, so they could not fail whatever they found.
Strip the {{/* ... */}} spans before matching and let the loop fail
through `false`, naming the signal it matched.
The span and not the line. Dropping every line that opens a comment is
shorter and silently loses a discriminator that shares its line with
one -- `{{- $x := .creationTimestamp }} {{/* why */}}` leaves with the
comment and the checks find nothing. The two spellings agree on
today's charts, so a fixture pins the difference rather than trusting
it.
Have the strip say when it ended inside a comment, and ask before
reading what it returned. Every `{{/*` is read as an opener, including
one inside a quoted string -- these charts carry long `fail (printf
...)` messages -- and a lone opener swallows the rest of the file,
leaving the checks reading nothing. A canary token cannot cover that:
it only proves whatever precedes IT survived, and the block most
likely to swallow is the last one in the file.
It says so on stdout rather than through an exit status, because
hack/cozytest.sh rewrites every `}` at column 0 into `return 0; }` --
the strip's own closing brace included -- so a status it returned
would never reach the caller.
Counting delimiters cannot stand in for the state the strip already
carries. Go template comments do not nest, so the first `*/}}` closes
the block and `{{/* the opener is spelled {{/* */}}` is a comment Helm
renders, while its two openers against one closer read as an imbalance
that is not one.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
||
|
Aleksei Sviridkin
|
9c9a8c225d |
test(seaweedfs): lower the volume size limit on a live instance and test the chart hand-off (#4707)
## What this PR does This adds an e2e case that lowers the SeaweedFS volume size limit on a live instance and checks that the bucket still takes writes. It also adds a unit test for the hand-off between the SeaweedFS app chart and the data-plane chart. The new `seaweedfs` suite creates its own instance in `tenant-test`, directly through the API, so the limit can change without racing the tenant module. It installs at 200 MB and writes into one bucket until every volume of the bucket sits between 20 and 200 MB. Then it patches the resource to 20 and waits for both releases and the master rollout. After that a write to the same bucket has to land, and the bucket has to have a new volume. If the master never picks up the new limit, the write goes to an old volume and the second check fails. 200 and 20 stand in for the shipped 1000 and 400, because filling a bucket into that band at real sizes would cost gigabytes in the sandbox. The suite also checks the live master StatefulSet. The limit from the resource has to be in the master command before and after the change, and all four `WEED_MASTER_VOLUME_GROWTH_COPY_*` counts have to stay. The same hand-off is checked on every PR by `hack/seaweedfs-chart-seam.bats`. It renders the app chart, takes `spec.values` from the `seaweedfs-system` HelmRelease and renders the data-plane chart with them. The expected default comes from the app chart's values, so this test does not pin 400 a second time. I checked in a scratch copy that it goes red when the app chart's `volumeSizeLimitMB` key or its `master:` block is renamed. A new suite directory broke two cases in `hack/select-e2e_test.bats`, so `seaweedfs-application` in `hack/select-e2e.sh` now maps to the new suite too. That is one line plus its comment. About the suite order question in #4302: this suite runs on its own instance, so it takes nothing from tenant-root's capacity and does not move the margin of `bucket-1-capacity`. Left for later: - A generic helper for the two-chart shape (render a chart, take `spec.values` from a HelmRelease, render the target). This PR covers only the SeaweedFS pair. - The `status.history` remediation guard on the upgrade the suite triggers. A failed upgrade that is retried and then succeeds would pass here without notice. The suite has not run on a cluster yet. Locally `chainsaw lint` passes. I also ran the StatefulSet and HelmRelease asserts with `chainsaw assert` against rendered objects. They pass on the expected state and fail on a wrong limit, a missing `COPY_2`, a rollout in progress and a stale generation, both on the StatefulSet and on the HelmRelease. Closes #4302 Closes #4285 ### 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 test(e2e): add a seaweedfs suite that lowers the volume size limit on a live instance and checks that the bucket still takes writes, and a unit test for the hand-off between the SeaweedFS app and data-plane charts. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added end-to-end coverage for SeaweedFS bucket resizing, including verifying that writes succeed after the volume-size limit changes. - Added SeaweedFS to the available and automatically selected end-to-end test suites. - **Documentation** - Updated local test prerequisites to note the required tools for SeaweedFS tests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
d1aea3f40e |
test(e2e): assert vm-disk readiness through WorkloadsReady
The VMDisk scenarios waited on the HelmRelease, which does not cover the import behind it. They now wait for the DataVolume to be Ready and then for Ready and WorkloadsReady on the VMDisk itself. The DataVolume wait keeps the combined budget the two old waits had. That alone would pass with the monitor ignoring DataVolumes, since an empty monitor reports operational. A new scenario imports from an unresolvable URL and waits for Ready True with WorkloadsReady False, which only a monitor that reads the DataVolume phase can produce. A DataVolume that has not started is not ready either, so the scenario first waits for the importer to fail, which CDI reports as Running False with reason Error, and only then asserts the VMDisk. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
a7ac37be65 |
test(e2e): let HR-Ready waits outlast the install timeout of the release they wait on (#4471)
## What this PR does Stacked on #4468, which holds the raised backup waits to a start margin; review that one first. This PR's own diff is the six commits on top of it. Most HR-Ready waits in the E2E suites gave up before the release they were waiting on could fail its install. cozystack-api gives every release it generates a 600s install timeout, and the Chainsaw asserts in front of those releases sat at 5 or 6 minutes. When an install genuinely failed, the suite reported a bare assert timeout against a release that was still installing, and the reason the install failed, which the release records when its own timeout fires, was never reached. Red runs failed just as often as they should; they only said less. I raised every HR-Ready wait above the install timeout of its own release, by the 60s start margin #4468 introduces: - Chainsaw asserts on releases cozystack-api generates go to 11m, across bucket, clickhouse, etcd, external-dns, foundationdb, harbor, mongodb, openbao, opensearch, postgres, qdrant, rabbitmq, redis, valkey and vminstance. - Asserts on nested `<name>-system` releases take their floor from the template that renders them: 11m for external-dns, openbao and qdrant (10m `spec.timeout`), 16m for `harbor-test-system` (15m). `harbor-test` sat exactly on its 600s floor, which expires first because the assert's clock starts before the install's; it goes to 11m too. - The kuberture Package release takes its 10m from cozystack-operator and goes to 11m. - In the backup walkthroughs, the bucket waits go from 300s to 660s in clickhouse, kafka, mariadb, mongodb, postgres, rabbitmq and redis, and the two postgres waits go from 360s to 660s. The kafka bucket wait passed no budget and silently took the helper's 300s default, which is why earlier scans for literal budgets missed it. Each Chainsaw op that runs a walkthrough grows by exactly what its waits gained (clickhouse 78m, kafka 46m, mariadb 48m, mongodb 42m, postgres 66m, rabbitmq 48m, redis 36m), so a failure at any later wait keeps the headroom it had and is still reported by the script instead of being cut off by the op. Two waits stay as they are, on purpose: - kafka: `kafka-rd` sets a 7h install timeout, which no budget inside the 215-minute E2E job can clear. The kafka assert and the Kafka waits in the kafka and kafka-metadata walkthroughs keep their numbers, and the assert now says why in-line. - `vm-instance-test`: `vm-instance-rd` disables the install wait, so its release goes Ready as soon as helm has applied and there is no floor to clear. Earlier lists of this problem included it by mistake. The cost falls on red runs only: a suite whose release never goes Ready now spends up to six more minutes before it reports, and in exchange the report carries the release's own reason. Green runs are unchanged. Suites run one after another in a single job capped at 215 minutes, so a systemic failure that reddens many suites now reaches that cap sooner, and a job cancelled at the cap loses its log and `cozyreport.tgz`. I took that trade because a failed install that says why is worth more than a red run that ends a few minutes earlier. `docs/agents/e2e-testing.md` stopped being true in several places once the waits were raised: it described most of the tree as under the floor, mariadb as the only suite raised, the walkthrough bucket waits as under it, and harbor's asserts as sitting on and under theirs. I rewrote those sentences, and the one that pointed at "every wait above the band" of 5-6m, kept the start-margin wording from #4468, and added that a walkthrough wait passing no budget takes the helper's default. The comments in `hack/mariadb-first-boot-waits.bats` that quoted the old op and bucket figures are restated; its existing checks and constants are unchanged, because every op grew by exactly its bucket delta and each remainder still sits at exactly its constant. I also added postgres to the table that file holds the walkthroughs to, so its raised waits and its op cannot drift back apart unnoticed, and a test that holds each table suite's bucket wait to the bucket release's floor, read from the api-server default with `bucket-rd` checked for an override; the op check alone could not catch a bucket wait dropping back, since a lower wait only widens the slack. redis stays out: its walkthrough waits on no redis HelmRelease, only on the bucket and on the sentinel master, so the table's floor check has nothing to read there. How I checked it, since E2E cannot run locally: - I re-ran the enumeration from the sizing section of `docs/agents/e2e-testing.md` over every `hack/e2e-chainsaw/*/chainsaw-test.yaml` and every `wait_hr_ready` call under `examples/backups/`, literal or default, and held each against its own release's floor. Every `Ready=True` assert and every `wait_hr_ready` call a suite drives now clears its floor, except the kafka ones named above. The opt-in etcd round-trip uses bare `kubectl wait hr` calls at 300s instead of that helper; they are listed in #4470. The `Ready=False` assert on the rejected redis upgrade is not a readiness wait and is left out. - `make bats-unit-mariadb-first-boot-waits` passes, 31 of 31. For the new postgres row, putting the op back to 50m fails only the postgres slack test, and a postgres wait at 659s fails only the postgres floor test. Setting any one table suite's bucket wait to 659s fails only the bucket test, naming only that suite, and an install-timeout override on `bucket-rd` fails it too. I ran the mutations under plain `bats` so a fail-fast runner could not hide a second failure. - `hack/select-e2e.sh` over this diff selects bucket, clickhouse, etcd, external-dns, foundationdb, harbor, kafka, kuberture, mariadb, mongodb, openbao, opensearch, postgres, qdrant, rabbitmq, redis, valkey and vminstance. Out of scope, and tracked in #4470: the tenant addon waits in `hack/e2e-chainsaw/_lib/run-kubernetes.sh` (an edit there escalates the whole E2E run), the redis and kafka-metadata `wait_hr_ready` helpers that print no reason on timeout, and the opt-in etcd round-trip's bare `kubectl wait` calls. Known and left for later, none of them changed here: - Only the mariadb Chainsaw asserts and the walkthroughs in the guard's table are held by a test. The other raised asserts, and the redis and kafka bucket waits, rest on literals with in-line reasons; #4470 covers extending the guard. - The Kafka waits in `examples/backups/kafka/04-create-kafka.sh`, `07-restore-to-copy.sh` and `kafka-metadata/run-all.sh` stay at 300s against the 7h floor without an in-line reason at the call site. The doc's rule asks that of Chainsaw asserts only, and the kafka Chainsaw assert carries it. The postgres waits are the ones #3748 is about. Their budget now clears the install-timeout floor and a test holds it there; what a CNPG first boot costs on this platform is still unmeasured, so that issue stays open. Closes #3771 Closes #3753 ### Screenshots Not a UI change. ### Downstream repositories I walked the trigger map in `docs/agents/contributing.md` against the diff. It touches E2E suites, backup walkthrough scripts, a unit-lane bats file and an agent doc, none of which any downstream repository restates. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(tests): HelmRelease readiness waits in the E2E suites and backup walkthroughs now outlast the install timeout of the release they wait on, so a failed install reports its own reason instead of a bare timeout. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Extended readiness and backup round-trip test timeouts across multiple services to accommodate slower startup and provisioning. * Added checks to verify that bucket readiness waits meet configured installation-timeout requirements. * **Documentation** * Updated testing guidance to clarify how readiness waits relate to release-specific timeouts and overall test budgets. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
c60332707d |
fix(build): log only the tags a build actually pushes
PUSHED_TAGS_LOG fed every tag to hack/stitch-multiarch.sh whether or not the build pushed it, so a PUSH=0 build, and now a plain LOAD=1 build, handed the stitch tags that were never published. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
c0cfc07a91 |
fix(talos): skip the registry push of the installer when PUSH=0
image-talos publishes with skopeo instead of buildx, so it never saw --push=$(PUSH) and a local PUSH=0 build still overwrote talos tags in REGISTRY. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
0ce0dbe757 |
fix(build): honour LOAD and PUSH from the environment
A plain := assignment beats an environment variable, so `LOAD=1 PUSH=0 make image` pushed a multi-arch build instead of loading one. An empty value from the environment falls back to the default rather than reaching buildx as `--push=`. PUSH now also defaults to 0 under LOAD=1, so a local load never publishes unless PUSH=1 is asked for. The OCI_EXPORT_DIR block still overrides an environment PUSH or LOAD. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
7a083aee50 |
fix(build): drop the per-call cache tag from cache-args
No caller passes the second argument any more, and an explicit tag would bypass CACHE_TAG, so the amd64 and arm64 legs of such an image would write the same mode=max cache ref and evict each other's layers. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
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
|
360f03bb8b |
feat(monitoring,tenant): shared-central tracing topology (phase 4) (#3775)
## What this PR does Adds the opt-in shared-central traces topology from the distributed tracing design (cozystack/community `design-proposals/distributed-tracing`, rollout item 4): a tenant can send its spans to one VictoriaTraces store in tenant-root instead of running its own, with both write and read isolation between the tenants that share it. **Switching it on.** A tenant opts in with the new Tenant field `tracingCentral`, which needs the tenant's own Monitoring. The tenant chart publishes it to the tenant's monitoring charts as `_namespace.tracingCentral` in the `cozystack-values` Secret, which the apps API does not let an application write, and renders the two network paths the topology needs under the same condition. tenant-root decides whether it hosts the store, with the new Monitoring field `tracingCentralHost` next to a cluster-mode `tracingStorages` entry named `generic`. **Isolation.** VictoriaTraces takes the tenant account from the `AccountID`/`ProjectID` headers and does no authorization of its own, so other tenants reach the shared store only through a vmauth in tenant-root. Each central tenant gets a VMUser that pins an account derived from its namespace name and UID, for both its collector's writes and its Grafana reads, and vmauth overrides any account a client sends. The collector and Grafana log in with credentials the chart generates and keeps. The tenant egress rules allow only pods carrying the collector's or Grafana's label to reach the vmauth proxy port, and a pod that carries such a label without the tenant's credentials gets only 401 from it; the vmauth internal routes, whose `/metrics` lists every tenant's user, are served on a port tenants cannot reach. The tenant-root namespace itself is inside the trust boundary, as it already is for the metrics of tenants that rely on a parent's monitoring: its workloads reach the store directly, and one that learns a tenant's account ID can read and write that account, and whoever tenant-root admits to its Grafana reads the store through a datasource that bypasses vmauth (with the System oidc mode, tenant-root's own groups, which already hold the same access level in every tenant under it); the `tracingCentralHost` description says so. **Sharing the store.** Each tenant's VMUser is capped at four requests in flight per vmauth replica, reads and writes together, and each collector replica exports with two senders, so more collector replicas do not raise a tenant's throughput; above the cap vmauth queues for up to 10s and answers 429, which the exporter retries. The store has one disk and one retention for all of its tenants and nothing reserves space per tenant; `tracingCentralHost` states that, since hosting is tenant-root's decision. The vmauth runs two replicas spread across nodes under a PodDisruptionBudget, so draining one node does not stop every central tenant at once. Every traces store is also disk-bounded now, as the design requires: without `retentionDiskUsageBytes` it drops its oldest day once its filesystem is 80% full. That alone does not keep one tenant from filling the shared disk, since VictoriaTraces always keeps the last two days. **States that would drop spans silently are refused, one race aside.** A live render fails, with a message naming the prerequisite, when a tenant turns central on while tenant-root hosts nothing, and when tenant-root stops hosting while central tenants remain; tenant-root's render also refuses `tracingCentralHost` without a store to host. The second check does not wait on a VMUser that is already being deleted, since its tenant is leaving. Each check reads the other side's objects when it renders, so a tenant turning central on while tenant-root stops hosting can pass both and export to a vmauth that is gone; the `tracingCentralHost` description names this race and its recovery. At runtime, tenant-root's Monitoring release is not Ready while the vmauth has failed. These checks run on a render, so removing the tenant-root Monitoring altogether is not refused, and from then on every Monitoring render of a central tenant fails until it turns central off; the `tracingCentralHost` description says to turn central off on the tenants first. Turning central on keeps the tenant's own local stores and their datasources, so traces stored before the switch stay readable until the tenant removes them; turning it off again leaves the spans written meanwhile in the shared store, unreadable from the tenant until it turns central on again, and spans sent while the collector switches back may be dropped; a deleted tenant's spans likewise stay until retention drops them. Right after a tenant turns central on, or after its credentials Secret is re-created, spans its collector sends before the vmauth has loaded the new login get a 401, which the exporter does not retry, so they are dropped; the tracing guide in #3772 covers it. **Where this departs from the design.** The design has the per-tenant collector inject the account headers and adds exactly one egress rule. Here the account is pinned by the tenant's VMUser at the vmauth and the collector sends none, because the collector runs in the tenant namespace and whatever runs with its pod label could otherwise choose an account; vmauth is also what the design already requires for reads, so both directions go through it. That needs a second egress rule, for the tenant's Grafana to reach the vmauth, beside the collector's. The per-tenant write rate the design gives the collector is not in this PR: no OpenTelemetry distribution has a rate-limiting processor, so #4648 builds the collector with one and holds each central tenant to a rate tenant-root publishes. Until it lands, a tenant writing faster than the disk cap allows over two days can fill the disk the other tenants share. **Consent is per host, not per tenant.** `tracingCentralHost` lets every tenant that sets `tracingCentral` in, nested tenants included, and each gets its own VMUser and its own request cap; there is no allow-list. A tenant-root that needs to limit who shares its store keeps `tracingCentralHost` off. **Upgrade.** Tenants that do not opt in render the same objects as before, and their `cozystack-values` Secret is unchanged. The monitoring charts render the same objects too, with one exception: a VTSingle or VTCluster without `retentionDiskUsageBytes` gains `retention.maxDiskUsagePercent: "80"` in its `extraArgs`, so its spec changes and its storage pods restart once, and a store already more than 80% full drops its oldest days when it starts. The inner monitoring HelmRelease's values gain the new `tracingCentralHost: false` default, as with any new field, and its `healthCheckExprs` gain a `VMAuth` entry, which matches nothing outside a hosting tenant-root. Both change the HelmRelease spec, so every tenant's monitoring release upgrades once, rendering the same objects. A tenant-root that has a cluster-mode `generic` store gets no vmauth until it sets `tracingCentralHost`. **Testing.** helm-unittest covers the three charts the change touches (tenant, monitoring, extra monitoring) and every guard was checked by mutation; a bats test covers what helm-unittest cannot, that the generated password and the collector's rollout checksum agree on a first install. On a test cluster: a spoofed `AccountID` from a tenant is overridden by vmauth, one tenant cannot read another's spans, the vmauth starts under the real tenant network policies (and stays in init without its apiserver label), the guards refuse as described, and the rendered collector config passes `otelcol validate` in the pinned image. There is no end-to-end test yet; one that covers cross-tenant isolation is planned as a follow-up. ### Screenshots No UI changes. ### Downstream repositories - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: covered by cozystack/cozystack#3772, the tracing guide - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: cozystack/terraform-provider-cozystack#64 - [ ] [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 Tenant gains the user-facing field `tracingCentral` (bool, default false), which the trigger map sends to terraform-provider-cozystack: its hand-written Tenant schema and model need the field. The follow-up is cozystack/terraform-provider-cozystack#64. The Monitoring gains `tracingCentralHost` (bool, default false); the provider's `cozystack_tenant_module` is a marker without values, so it needs no change. The website reference pages for both are generated from the READMEs on release. The tracing user guide in #3772 covers the per-tenant backend only. A central-mode section in the tracing guide of #3772 (hosting and opting in, the shared disk and retention, turning central off on the tenants before tenant-root stops hosting, and the recovery from the opt-in/stop-hosting race) also takes the lifecycle details the `tracingCentral` description no longer carries. ### Release note ```release-note feat(monitoring,tenant): add an opt-in shared-central traces store. A tenant sets tracingCentral to send its spans to one VictoriaTraces store in tenant-root instead of its own, and tenant-root opts in to hosting it with tracingCentralHost on its Monitoring. Tenants sharing the store are isolated on write and read through a vmauth in tenant-root, and each is capped in the requests it can hold in flight. ``` |
||
|
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
|
42faf54932 |
test(e2e): drop readiness asserts that HelmRelease Ready already covers (#4652)
## What this PR does Removes e2e asserts that wait for something an earlier assert in the same test already proves. First step of [#2653](https://github.com/cozystack/cozystack/issues/2653), not the whole issue. qdrant and external-dns render the workload in a nested `-system` HelmRelease, and both suites already assert that HelmRelease Ready. The `readyReplicas: 1` on the StatefulSet and the Deployments goes, the instance-label pin stays because the catch collectors select on it. I checked on a dev cluster that a parent HelmRelease waits for its nested one (a Qdrant with a broken `storageClass` kept the parent in `Progressing` until its install timeout, then it failed naming the nested HelmRelease), details in [this comment](https://github.com/cozystack/cozystack/issues/2653#issuecomment-5929644802). vminstance: both PVC `Bound` asserts follow a DataVolume `Ready` assert, and a DataVolume is not Ready before its PVC is Bound. Left alone on purpose: the qdrant PVC assert (the reclaim-on-delete step only proves anything if the PVC was seen first, same in kafka), openbao sts and PVC (they run after unseal), operator-created workloads, and the Service and Endpoints shape checks. Chainsaw isn't installed on my machine, so the suites are checked by CI only. ### Screenshots No UI changes. ### Downstream repositories The diff only edits `hack/e2e-chainsaw/*/chainsaw-test.yaml`. 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. No box ticked. - [ ] 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 test(e2e): the qdrant, external-dns and vminstance suites no longer repeat readiness checks that the HelmRelease Ready assert already covers. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Updated end-to-end checks to use Helm release readiness for Deployment and StatefulSet scenarios, rather than separately asserting ready replica counts. * Removed the requirement for PersistentVolumeClaims to reach `Bound` in the VMDisk and VMInstance scenarios; DataVolume readiness checks remain. * Retained instance-label checks and clarified that PVC reclaim validation requires the claim to be observed first. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
89c7ea0b46 |
fix(e2e): hold replicated-lane LINSTOR cleanup to a pre-suite baseline and delete kafka suite PVCs (#4651)
## What this PR does This fixes two cleanup failures in the kubernetes suites. The nightly runs failed `kubernetes-previous` and `kubernetes-latest` in the replicated storage lane. PR runs fail `kubernetes-latest` with `LINSTOR pools did not return to their pre-suite baseline within 300s`. The replicated lane asked for too much free space at cleanup. It wanted 90 GiB free on every LINSTOR node, and volumes from earlier suites kept srv2 at 83 GiB, so both suites failed without leaking anything (nightly run 36659663201). A replicated suite still needs those 90 GiB for its workers and CDI scratch volumes, so the floor is now checked before the suite starts. If a node is short there, the suite fails with the per-node table and an error that blames the sandbox, not the suite. Cleanup in both lanes now compares against a baseline recorded before the suite creates anything, and that baseline is no longer free bytes. On a node whose pools are all thick ZFS, `linstor sp l` does not ask the satellite for free space. The controller fetches it live only for nodes with thin pools and otherwise returns the value cached at the satellite's last report. In LINSTOR v1.33.3 that is `FreeCapacityFetcherProto.assembleRequests` filtering on `hasThinPools`, with `CtrlStorPoolListApiCallHandler` falling back to `getFreeCapacityLastUpdated`. The last report can be taken while `zfs destroy` is still freeing blocks in the background. A cleanup that had removed everything then kept polling a frozen figure 0.4 to 0.9 GiB short and failed after 300s. A byte tolerance does not fix that, because the residue is already close to the smallest volume a suite is known to have leaked (1 GiB). The baseline is now the set of ZFS datasets under each node's LINSTOR pool. It is read with `zfs list` inside that node's `linstor-satellite` container. The zpool name comes from the pool's `StorDriver/StorPoolName` property, because it is `data` on QEMU nodes and `data-srvN` in container mode. Cleanup waits until no dataset outside the baseline remains. A destroyed dataset leaves the namespace when `zfs destroy` returns, whether or not its blocks are still being freed, so background freeing no longer fails the check. A real leak, like the 21 GiB scratch volume left on srv3 in an earlier run, is reported by node and dataset name instead of a KiB difference. A node whose pool cannot be read fails the check. A run that fails before it records a baseline no longer compares against the previous suite's file. Every read the new probe makes runs under `timeout -k 5 30`, so a hung controller or satellite fails one poll and the wait still ends with its failure message. The kafka suite deleted its apps but never their PVCs. The chart keeps them on purpose with `deleteClaim: false`. One run left 35 GiB in `tenant-test`, 20 GiB of it on srv2, and the backup round trip left its two clusters' volumes in `tenant-root` the same way. Each test now deletes its clusters' PVCs by `strimzi.io/cluster` after the apps are gone and waits for them. The `kafka` test does it from a step cleanup, so it also runs on a failed run. The tests cover both fixes. `hack/linstor-storage-mode_test.bats` has cases for a clean cleanup, a volume still listed on the first poll and gone on the next while the cached free capacity stays short, a leaked volume, a node that cannot be read, a hung satellite read, a missing or stale baseline, and the replicated-lane floor. Its fake runs the real controller-side `jq` filter against a LINSTOR-shaped listing. `hack/kafka-suite-pvc-cleanup_test.bats` checks that each cluster's PVC delete is in the suite and runs after the app cleanup. One stub line goes into `hack/run-kubernetes-talos-diagnostics_test.bats`, because that test calls `cozy_cleanup` with no baseline and only checks the order of the deletes. The new guards were mutation-tested. Some limits stay as they are. The local lane waits for dataset names to disappear, not for the freed space, and has no free-space floor, so a background destroy can still be freeing blocks when the next suite starts. The 90 GiB precondition in the replicated lane still reads the cached free capacity, and its comment says so. The two `kubectl exec` reads in `cozy_wait_linstor_pool_free` were there before this PR and still have no time limit of their own. Two related problems stay out of this PR. The `kafka-metadata` suite leaks PVCs the same way, tracked in #4641. The stale comment in `examples/backups/kafka/cleanup.sh` and the missing PVC pre-clean in the round trip are in #4649. Closes #4617, closes #4616, closes #4653 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(e2e): the kubernetes suites' LINSTOR cleanup compares each node's ZFS datasets against a pre-suite baseline instead of cached free capacity, the replicated lane checks its 90 GiB floor before the suite starts, and the kafka suite deletes the PVCs its clusters keep. ``` |
||
|
Aleksei Sviridkin
|
393592a308 |
test(e2e): lower the seaweedfs volume size limit on a live instance
Every existing case installs SeaweedFS at one limit and stays there, so nothing walks the transition an operator makes when lowering it: each volume at or above the new value leaves the writable set and the bucket writes again only after a grow. The failure is a PUT that times out while every release stays Ready, so the suite ends on a write into a bucket filled between the two limits. The suite runs its own instance, created directly so the limit can move without racing the tenant module, and asserts on the live master that the value set on the resource arrives with the growth counts intact. seaweedfs changes now select the suite, which the selector's mapping and its tests are updated for. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
0c8a266a57 |
test(seaweedfs): render the app chart's release values through the data-plane chart
Each SeaweedFS chart has its own suite and neither feeds the other, so a key the app chart writes and the data-plane chart stops reading, or one a chart bump moves, leaves both suites green. Render the hand-off the way helm-controller performs it and assert the master's volume size limit and growth counts on the result. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
efb4b1f5aa |
test(qdrant): exercise the tenant trust anchor end to end
The fixture left TLS unset with external off, so the chart rendered no CA and no TenantProjection, and no e2e run walked the chain from the sentinel through the CA-extraction controller to the tenantsecrets read path. Turn TLS on explicitly, without allocating a LoadBalancer, and check both ends: the projection is key-free, carries the chart's CA and is served through tenantsecrets while the key-bearing Secrets are not; and a client trusting only that anchor verifies the listener, while a wrong anchor, no anchor and plaintext HTTP all fail. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |