mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-05 19:47:37 +09:00
b4fbaf00a23691fee2fcc09638fbeca4584fae3c
1340
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
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 --> |
||
|
|
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. ``` |
||
|
|
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 --> |
||
|
|
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 --> |
||
|
|
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 --> |
||
|
|
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
|
||
|
|
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> |
||
|
|
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> |
||
|
|
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 --> |
||
|
|
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. ``` |
||
|
|
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> |
||
|
|
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> |
||
|
|
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>
|
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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>
|
||
|
|
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 --> |
||
|
|
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> |
||
|
|
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 --> |
||
|
|
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. ``` |
||
|
|
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 ``` |
||
|
|
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 --> |
||
|
|
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. ``` |
||
|
|
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> |
||
|
|
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> |
||
|
|
b306abde76 |
test(e2e): wait for the S3 gateway to accept a key before using it
BucketAccess turns accessGranted as soon as the COSI driver has written the new key into the filer's IAM file. The S3 gateway loads that file later, from a filer metadata subscription, and with two filer replicas the change can reach the filer a gateway follows only after the other replica passes it on. Both bucket suites call mc alias set right after the assert, so a key the gateway has not loaded yet is rejected with InvalidAccessKeyId and the suite goes red. Probe the key through the same port-forward the writes use. Only the InvalidAccessKeyId text is waited on, for at most 60s; any other error or a timed-out attempt fails at once, and the window expiring fails with a message naming the BucketAccess. The two steps get an 8m op ceiling so the inner budgets still sit under it. Signed-off-by: Aleksei Sviridkin <f@lex.la> Assisted-by: LLM |
||
|
|
200a7fff66 |
test(e2e): read the kafka round-trip cleanup order without yq if/else
The check on kafka-2-backup-roundtrip's finally block classified each op with if/then/elif/else, which yq only gained in v4.54.1. The CI runner's yq predates that and rejects the whole expression with a lexer error, so the test failed there while passing locally. Have yq emit the plain facts per op and let awk name it. The round-trip document is cut out before that query, because in a multi-document file yq also evaluates an array literal against the documents a select drops and prints a line for each. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
cb7a579065 |
fix(e2e): compare LINSTOR pool datasets, not free bytes, at cleanup
For a node with only thick ZFS pools, `linstor sp l` reports the free capacity cached at the satellite's last report rather than asking the satellite, and that report can land while `zfs destroy` is still freeing blocks in the background. A cleanup that had removed every volume it created could then wait out its whole budget against a figure that never moved. That residue has reached about 0.9 GiB, with no known upper bound, against 1 GiB for the smallest volume a suite is known to have leaked, so a byte tolerance has no safe value between the two. Record the datasets under each node's pool before the suite, read with `zfs list` on that node's satellite, and wait until no dataset outside that set remains. A leaked volume is then reported by name and node, and a destroy still freeing space in the background does not count. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
30689f8109 |
fix(ci): map the recurring unmapped scopes and add area/dx
Over the full PR history a set of scopes recurring three or more times still fell through to area/uncategorized. Most of them have an obvious home and were only missing a row. Subsystem names such as `networking` or `storage` are now accepted as the scope of their own area, since contributors reach for those as often as for a package name. `dx`, `hack` and `cozypkg` describe developer tooling, which no existing area covers: area/build is image builds, area/testing is tests. They get a new area/dx rather than being filed under either. The contract test also pins that every map target exists in labels.yml, since the label sync workflow creates only what that file defines, and that no key is written twice, since a repeated key silently overrides the earlier one. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
a8c3eb5260 |
fix(ci): strip every leading backport prefix from a PR title
A backport of a backport is titled with two `[Backport ...]` prefixes. Only the first was stripped, so the second was parsed as a bracket scope: the real scope and the kind were lost, and the area/release added by the strip suppressed the area/uncategorized fallback, so the PR looked labelled correctly. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
cd6476ccb7 |
fix(ci): look up the labeler's type and scope maps by own key only
Both maps are plain object literals, so a title typed or scoped `constructor`, `toString` or `__proto__` resolved to an Object.prototype member. That value is truthy, landed in the label set and killed the step on `l.startsWith`, leaving the PR with no labels at all. The new contract test runs the workflow's own script under node with stubbed context, so it exercises the code as shipped rather than a copy of it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
5fde704a30 |
fix(ci): skip directories named like Chainsaw step files
Chainsaw builds step files only from directory entries that are not directories, so a directory named like 01-x.yaml makes nothing a suite. Both selectors listed it as one, and select-install demanded a mapping for it. A symlink still counts, as it does for Chainsaw. A directory named chainsaw-test.yaml keeps its suite listed: Chainsaw reads that name whatever its type and fails loudly on a directory, and an unlisted suite would hide that failure. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
e82e5baf65 |
fix(ci): discover Chainsaw tests nested below a suite directory
Chainsaw walks every subdirectory of the path it is handed, and the runner hands it each top-level directory under hack/e2e-chainsaw/, so a test in <dir>/<sub>/ runs as part of <dir>. Both selectors read only one level down, so a directory whose tests all sit deeper was never listed: no selection named it, the full-suite list left it out too, and only a run with no selection at all executed it. Both scripts now take the top-level directory of a test file found in that directory or anywhere below it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
c1b99df98d |
fix(ci): escalate a file placed directly under examples/backups
A file beside the per-app backup directories rather than inside one belongs to no single walkthrough, but it matched neither backup rule and fell to examples/ in the inert list, so an edit to it selected nothing and both lanes skipped Chainsaw. It is now treated as shared by all of them and escalates the same way examples/backups/_lib/ does. README.md there stays inert through the *.md rule. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
6913a04622 |
fix(ci): discover Chainsaw suites made only of numbered step files
With no chainsaw-test.* in a directory, Chainsaw v0.2.15 builds a Test from files named like 01-install.yaml. Such a directory runs whenever the whole tree is handed to chainsaw, but neither selector listed it, so no escalation carried it and no edit to it could select it. Both scripts now derive the suite list from the file names Chainsaw itself accepts. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
a0af2fd2f8 |
test(e2e): hold the postgres and bucket walkthrough waits to the floor
The postgres backup walkthrough's HelmRelease waits and every table walkthrough's bucket wait now clear the install-timeout floor, and the Chainsaw ops grew to contain them, but nothing checked it: a wait dropped back under the floor, or raised without its op, would pass. The op check cannot catch the bucket case on its own, since a lower wait only widens the slack. Add postgres to the table the other walkthroughs are held by, with the remainder its op kept through the raise, and hold 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. This holds the budgets only; what a CNPG first boot costs on this platform is still unmeasured. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
4386014cf4 |
test(e2e): restate the wait guard's figures for the raised bucket waits
The mariadb op and its bucket wait both grew by six minutes, so the worked arithmetic in the remainder test and the note that the bucket wait sits under its floor no longer describe the tree. The header also said the other suites' HR-Ready asserts sit under the floor by choice, which stopped being true once they were raised. The checks and their constants are unchanged: every remainder still sits at exactly its constant. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
04d469fc85 |
fix(tests): let the backup walkthrough waits outlive their installs
The backup round-trips drive examples/backups, whose bucket waits gave up at 300s and whose postgres waits at 360s, under the 600s install timeout cozystack-api gives each of those releases. The kafka example's bucket wait passed no budget and took the helper's 300s default. A failed install there reported only that the wait ran out. Raise each to 660s, and grow the Chainsaw op around every walkthrough by what its waits gained, so a failure at any later wait keeps the headroom it had and is still reported by the script rather than cut off by the op. The kafka and kafka-metadata Kafka waits stay: their 7h install timeout cannot be cleared inside the E2E job cap. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
49cefe0dff |
fix(tests): size HR-Ready asserts on nested and Package releases
A nested <name>-system release takes its install timeout from the template that renders it, not from cozystack-api, and a Package release takes it from cozystack-operator. The external-dns, openbao and qdrant templates set 10m and harbor's sets 15m; the operator gives 10m. Their asserts sat at 5m or 10m, so a failed install reported as a bare timeout. Raise each above its own floor: 11m, and 16m for harbor-test-system. harbor-test was on its 600s floor exactly, which expires first because its clock starts before the install's; it goes to 11m with the generated parents here. kafka keeps its 6m: kafka-rd sets a 7h install timeout that no budget inside the E2E job cap can clear, and the assert now says so. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
9e6cb7d0cc |
fix(tests): let HR-Ready asserts outlive the install they wait on
cozystack-api gives every release it generates a 600s install timeout, and these asserts gave up at 5-6m. A failed install was therefore reported as a bare assert timeout while the release was still installing and had not yet recorded why it failed. Raise them to 11m, above that floor, so the release's own reason is on record before Chainsaw gives up. The cost is up to six more minutes on a red run of each suite; a green run is unchanged. vm-instance-test stays: vm-instance-rd disables the install wait, so its release goes Ready as soon as helm has applied. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
94b328c6eb |
test(e2e): hold every raised backup wait to a start margin
The wait-budget guard accepted a wait one second over the install timeout, although the wait's clock starts before the install's by the Application reconcile, the chart fetch and the helm-controller queue. A wait lowered from 660 to 601 passed every test and brought back the bare assert timeout the budgets exist to avoid. A wait now has to clear its floor by a named 60s margin. Nothing in the tree bounds those three delays, so the value is policy: the headroom every current wait already carries. The sizing guide in docs/agents/e2e-testing.md asked only for "strictly above", which the guard now rejects; it points at the margin instead of restating a number. The mongodb, rabbitmq and clickhouse walkthrough waits were raised for the same reason as mariadb's, and their Chainsaw ops grown with them, but nothing held either. Each suite is now a row in one table and gets its own floor and op tests, so a regression reds only its own suite. Floors come from the api-server install-timeout default and, for mariadb, the chart's startup probe; each suite's RD is checked for an override instead of being assumed not to carry one. Each op keeps the remainder it had before the raise over every HelmRelease wait it reaches, the bucket one included, so raising a wait and its op together keeps the remainder instead of widening the slack. The mongodb target behind SKIP_RESTORE=1 is not reached and is left out until the op stops passing it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
7fe713580d |
test(e2e): drop readiness asserts that HelmRelease Ready already covers
The qdrant and external-dns apps render their workload in a nested -system HelmRelease, and both suites already assert that HelmRelease Ready, which helm-controller reports only once the workload is up. Asserting readyReplicas on the same Deployment or StatefulSet again adds nothing, so those asserts keep only the instance-label pin. The vminstance PVC asserts follow a DataVolume Ready assert, and a DataVolume is not Ready before its PVC is Bound. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
|
3303cc59c7 |
fix(e2e): delete the kafka suite's broker and controller PVCs
The kafka chart sets deleteClaim: false on both node pools, so deleting an app leaves its volumes behind on purpose. The suite deleted its apps and never the volumes. The 35 GiB a run leaves in tenant-test, 20 GiB of it on one node, kept that LINSTOR node short of space for the kubernetes suites that run after it. The backup round-trip leaves its two clusters' volumes in tenant-root the same way. Each test now deletes the PVCs by strimzi.io/cluster once its apps are gone, and waits for them to go. The kafka test does it from a step cleanup, so a failed run reclaims them too. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
64b3c9619f |
fix(e2e): hold replicated-lane LINSTOR cleanup to a pre-suite baseline
The kubernetes suites' cleanup in the replicated lane demanded 90 GiB free on every LINSTOR node. Volumes that earlier suites and the platform legitimately keep can hold a node under that figure even when the suite reclaimed everything it used, so the cleanup failed suites that had leaked nothing. Both lanes now record the per-node baseline before the suite creates anything and compare against it on cleanup. The 90 GiB figure is what a replicated suite needs to place its workers and CDI scratch volumes, so it stays as a precondition checked before the baseline, and a shortfall there is reported as the sandbox's state rather than the suite's. A run that fails before recording its baseline used to leave the previous suite's file in place for its cleanup to compare against; the file is now removed when the suite starts. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
|
54159736bc |
feat(talos)!: bump Talos to v1.14.2 (#4598)
## What this PR does Bumps the Talos image Cozystack builds from v1.13.6 to v1.14.2 to get a newer DRBD. v1.13.6 ships DRBD 9.3.2, v1.13.7 and later and v1.14.0/v1.14.1 ship 9.3.3, and v1.14.2 is the first release with 9.3.4. ZFS moves from 2.4.3 to 2.4.4. On 9.3.2 I've seen a peer reboot leave the DRBD sender thread spinning on a failed send (EPIPE). What followed was RCU stalls, processes stuck in D-state, a hanging `drbdsetup disconnect` and two-phase-commit timeouts. The [9.3.4 ChangeLog](https://github.com/LINBIT/drbd/blob/drbd-9.3.4/ChangeLog) has fixes for a sender thread pinning a CPU while its connection is down, for two-phase-commit issues and a hanging `drbdadm disconnect`, and for two nodes ending UpToDate with different data after a reconnect. None of them are in 9.3.3. I matched the trace to those entries by their descriptions only, not against the commits. The base installer image had to change. Talos v1.14 no longer publishes `ghcr.io/siderolabs/installer`, so `gen-profiles.sh` now uses `ghcr.io/siderolabs/installer-base`, same as upstream's own v1.14 imager profiles. I built the amd64 installer from the new profile with `imager:v1.14.2` locally and it builds without errors. PR CI will not boot the new image. It builds the installer and matchbox images, but the e2e runs on the upstream `talos:v1.13.5` container from `hack/e2e-compose.yaml`. The first run that boots the new kernel, DRBD and ZFS is the nightly after merge, which builds the nocloud disk from these profiles and runs it in QEMU. On Talos v1.14.2 the `drbd_transport_tcp` module is no longer loaded on demand. After upgrading a node from v1.13.6, `drbd` was loaded and the transport module was not, `drbdsetup new-peer` failed with "Failed to create transport (drbd_transport_xxx module missing?)", and every DRBD resource on the node stayed in Connecting. Listing the module in `machine.kernel.modules` fixed it without a reboot. The cause is [siderolabs/pkgs#1565](https://github.com/siderolabs/pkgs/pull/1565), in Talos since v1.14.0: the kernel is built with an empty modprobe path, so it no longer loads modules on `request_module()`, which is how DRBD asks for its transport. Any module that used to load that way now has to be listed explicitly (reported upstream as siderolabs/talos#14501). Outside DRBD this includes `dm-thin-pool` and `dm-multipath`, which the v1.14.2 kernel builds as modules, so LVM-thin or multipath users on Talos need them listed too. The e2e node config now loads `drbd_transport_tcp` explicitly, and the talm preset (cozystack/talm#252) and the install docs (cozystack/website#720) get the same line. The module ships in the drbd extension on v1.13 too, so the extra line is safe before the upgrade. The order matters for host nodes. Either update the cozystack preset in the talm project to a version with cozystack/talm#252 and re-render and apply the node config, or add `drbd_transport_tcp` to `machine.kernel.modules` by hand. Only then upgrade the node to v1.14.2. A node upgraded first loses DRBD replication until the module is added. The host network policy now also denies port 2383 to `world`. Talos v1.14 moved etcd's `/metrics`, `/health` and gRPC-gateway JSON API from 2379 to a separate listener on 2383, and the policy only blocked 2379 and 2380. Without this, the etcd JSON API on v1.14 control-plane nodes would be reachable from outside, protected only by client mTLS. Two smaller changes come with the bump. The multus `talos-cni-plugins-checked-against` marker moves to v1.14.2, because the pkgs release behind Talos v1.14.2 still pins CNI plugins v1.9.1, same as multus. The system memory limits doc now describes the v1.14 OOM trigger, since v1.14.0 dropped its global memory PSI clause ([siderolabs/talos#13895](https://github.com/siderolabs/talos/pull/13895)). Before upgrading hosts, operators should know a few things from the [v1.14.0 release notes](https://github.com/siderolabs/talos/releases/tag/v1.14.0) and the Talos compatibility code: - Talos v1.14 accepts a host upgrade from v1.12.0 or later. It supports Kubernetes 1.32 through 1.37, while v1.13 still accepted 1.31, so a cluster on Kubernetes 1.31 has to upgrade Kubernetes first. - etcd's HTTP endpoints (`/metrics`, `/health`) moved from port 2379 to 2383. The Cozystack etcd scrape proxy reads a separate metrics listener on `127.0.0.1:2381`, which this change does not move. Anything else that scraped or health-checked etcd on 2379 has to switch to 2383, and a firewall outside the cluster that blocked 2379 should block 2383 too. - etcd and kube-apiserver now require TLS 1.3, so a client that can only speak TLS 1.2 stops connecting. - Workload isolation (`SecurityProfileConfig`) stays off on upgraded clusters, but `talosctl gen config` turns it on for new ones. I haven't tested Cozystack with it enabled. - `talosctl apply-config --mode=reboot` is gone. ### Screenshots Not a UI change. ### Downstream repositories - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: cozystack/website#720 - [ ] [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: - [x] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: cozystack/talm#252 - [ ] [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 PR refreshes the `next` version pins for v1.14.2 and adds the module to the Talos install pages. The talm PR adds it to the cozystack preset. ### Release note ```release-note feat(talos): Bump Talos to v1.14.2, which ships DRBD 9.3.4 and ZFS 2.4.4. DRBD 9.3.4 fixes a sender thread spinning on a dead connection, two-phase-commit hangs and a data divergence after a reconnect. Before upgrading a node, add `drbd_transport_tcp` to its Talos `machine.kernel.modules` next to `drbd`: Talos v1.14.2 no longer loads it on demand, and without it DRBD cannot connect to any peer. We recommend skipping the v1.13 line and upgrading nodes straight to v1.14.2. Talos accepts a direct upgrade from v1.12.0 or later. Talos v1.14 moves etcd's HTTP metrics and health endpoints from port 2379 to 2383, and the Cozystack host network policy now blocks 2383 from outside the cluster. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Updates** * Upgraded host node support to Talos v1.14.2 across machine images, with refreshed firmware and storage extensions. * Updated memory-pressure guidance for Talos v1.14, including version-specific behavior and configuration recommendations. * Added protection for Talos v1.14’s etcd HTTP endpoint on port 2383. * Updated cluster preparation to load the TCP transport module required for DRBD. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
312409ecd2 |
test(e2e): fail the suite whose pods outlive its cleanup (#4492)
## What this PR does A Chainsaw suite could pass while the workloads it created kept running after its own cleanup, and nothing noticed until a later suite failed. On one E2E run the brokers of four Kafka releases from suites that had passed and deleted their applications (#4488), plus a TenantControlPlane sitting on its finalizer with two apiserver pods (#4489), pushed pod memory requests to the node limit. Harbor then failed on `Insufficient memory` two suites later, under the wrong name. This makes the suite that leaked fail, and name what it left behind. - Chainsaw v0.2.15 has no hook that runs after a test's cleanup: its `Configuration` carries `error.catch`, which runs only on failure. So `make test-chainsaw` now runs `hack/e2e-chainsaw-run.sh`, which invokes `chainsaw test` once per suite directory and checks the cluster between invocations with `hack/e2e-chainsaw/_lib/leak-guard.sh`. - Attribution: a pod is charged to a suite when all three hold. It is not terminal. Its UID was not in the pod list taken before the suite. The root of its ownerReferences chain was created at or after the suite started. A platform pod replaced mid-suite by a rollout, an eviction or a crash traces back to an object the install created, so it is not charged. A pre-existing pod adopted by a controller recreated mid-suite is excluded by the UID baseline. A leak is charged once, to its own suite. - The suite start is read off an apiserver, from the `creationTimestamp` of a server-side dry-run create, rather than off the sandbox. With several control-plane nodes the reading and a root's timestamp can come from different apiservers, so the rule assumes their clocks agree more closely than the gap between the reading and the suite's first object; no skew margin is added. Source for that behaviour: the generic registry `Store.create` stamps it through `rest.FillObjectMetaSystemFields`, then `DryRunnableStorage.Create` copies it back without persisting (`staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go` and `dryrun.go` at Kubernetes v1.33.0). - Budget: charged pods get 300s to disappear, the default `timeouts.cleanup` in `.chainsaw.yaml`; a unit test keeps the two equal. A suite that overrides its own cleanup timeout (kafka sets 8m) is covered the same way, because the check runs only after `chainsaw test` has returned, which is after every cleanup in the suite has finished under its own timeout; the 300s start from there. The leaks this targets sat about 110 minutes (Kafka brokers) and 30+ minutes (the TenantControlPlane) past their suites. Every green suite logs `leak-guard: suite=<dir> clear after Ns`, so the number can be checked against later runs. - Past the budget the run prints `LEAKED-PODS suite=<dir> budget=300s count=N`, then one line per pod with its phase, deletion state, root and `app.kubernetes.io/instance`. It raises a `leaked-pods` error annotation and adds a failed `leak-guard` test case to `chainsaw-report.xml`. The run continues through the remaining suites and exits non-zero at the end, as the single invocation did on a red test. - A suite that already failed on its own is checked once without the wait. Its leftovers are reported as a `::warning` with `LEAKED-PODS suite=<dir> checked-once=chainsaw-failed`, since some may still be tearing down. - Each pod gets one `kubectl wait` inside the remaining budget, and nothing is retried. A timeout or a spent budget gives the verdict from the last good pod list, also when the re-list after it fails, so a leak fails the suite. A wait that fails only after running for its whole `--timeout` counts as a timeout whatever it printed: kubectl maps an expired context to "timed out waiting for the condition" only inside the watch, so a deadline that passes during its initial lookup surfaces as "context deadline exceeded" (`wait.go` and `delete.go` in kubectl v1.33.0). A spent budget is caught before calling kubectl and never passed as `--timeout=0`, which means "check once". - Any other error on a wait or a re-list, before the budget is spent and before any timeout, stops the waiting and is reported as `LEAKED-PODS suite=<dir> count=N inconclusive="<error>"`. That is a warning, not a leak failure, so an apiserver blip cannot become a leak red. - A failed first reading after a suite, including a pod list that cannot be parsed, leaves the suite unchecked. It fails with a `leak-guard` annotation and a report case saying the cluster could not be read, not that it leaked. Errors that are not leaks carry the annotation title `leak-guard`, not `leaked-pods`. - Known leaks are listed in `hack/e2e-chainsaw/_lib/leak-allowlist.txt` as `<suite-dir> <issue-ref>`: `kubernetes-latest` and `kubernetes-previous` against #4489 (both suites create a TenantControlPlane through `_lib/run-kubernetes.sh`). `kafka` and `kafka-metadata` were listed against #4488 until #4490 fixed it; those entries are gone. An allowlisted suite prints its leak as a `::warning` and does not fail. An entry that names a directory that is not a suite stops the run before the first suite, so a rename cannot keep an exemption alive. The PR fixing each issue deletes its lines. Suite selection is unchanged. The runner runs the named directories when there are any, and otherwise every directory directly under `hack/e2e-chainsaw/`, each handed to Chainsaw's own discovery; it refuses a tree with a test at its top level, where that split would stop being exact. I verified this with Chainsaw v0.2.15's own `discovery.DiscoverTests`, built from source at that tag. For the full tree, for `kafka redis harbor` and for `kubernetes-latest`, the union of the per-suite discoveries equals what a single `chainsaw test [suites]` discovers: 57, 7 and 2 tests, identical lists. 57 matches the test count in an earlier full run's report. A unit test pins that the runner passes chainsaw the suites and flags it was given, in order. The order changes too: suites used to run in Chainsaw's own order and now run one directory at a time in alphabetical order, which moves the kubernetes suites after the kafka ones. Before #4490 fixed #4488 this starved them (see the E2E criterion below). With the fix merged the order has no effect, and a future leak that lands before a memory-heavy suite is named at the suite that leaked. One behaviour does change: a selected name that is not a directory used to fail the single invocation before any test ran; now only that suite fails and the others run. `chainsaw-report.xml` keeps its path and shape: one `<testsuites>` root with the summed counts, one `<testsuite>` per suite directory. Its consumers are the `Collect chainsaw report` / `Upload chainsaw report` steps in `pull-requests.yaml`, `e2e-fork.yaml`, `nightly.yaml` and `e2e-tag.yaml`, which `docker cp` the file and upload it as an artifact. Nothing in the repository parses it. The merge is unit-tested on two per-suite reports in Chainsaw's real shape, one of them failed, and the result is checked to parse as XML. Tests: `hack/chainsaw-leak-guard_test.bats` (31 tests, in `make unit-tests`; named without the `e2e-` prefix, which the unit lane excludes) covers the attribution rule, the budget, the allowlist, the merge and the runner, all against stub `kubectl` and `chainsaw`. Every rule was mutation-checked: removing the guard call, having the Makefile bypass the runner, inverting the allowlist match, flipping or tightening the root-timestamp comparison, removing the UID baseline, removing the terminal-phase filter, skipping the budget wait, changing the budget, lowering or raising the depth cap, accepting a stale allowlist entry, reading an unreadable pod list as empty, dropping the leak case from the report, skipping directories in the selection, treating any failed owner read as a missing owner, dropping the separate report for a suite that already failed, dropping the not-run reason from the report, reading any wait error as a timeout, removing the inconclusive branch, letting a failed re-list after a timeout turn a leak inconclusive, ignoring a failed re-list, reporting an unreadable cluster as a leak, reading an unparsable pod list as clean, reporting a kubectl warning instead of the error, passing the report name without its extension, wording a failed re-list as a failed wait, dropping an allowlist's last line when it has no newline, dropping the check that `--set-string` carries a value, and reading a wait that ran its whole timeout but printed a context error as inconclusive. Each one turns its named test red, and restoring turns it green. E2E criterion, registered before the run: in the `Run E2E tests` log of this PR, every suite that ran prints `leak-guard: suite=<dir> clear after`, a `LEAKED-PODS suite=<dir>` line carrying `allowlisted=`, or one carrying `checked-once=` (a suite that failed on its own) or `inconclusive=` (waiting stopped on a failed read), and no line matches `LEAKED-PODS suite=.* budget=`. With #4490 merged, `kafka` and `kafka-metadata` are no longer allowlisted and must print `leak-guard: suite=kafka clear after` and `leak-guard: suite=kafka-metadata clear after`, which is the live check that #4490 fixed #4488. The first run of this PR (before #4490, run 36223896793) is the positive control: it printed `LEAKED-PODS suite=kafka count=15` and `suite=kafka-metadata count=8`, allowlisted, and every other suite cleared in 0-37s. That run also went red on both kubernetes suites, because those 23 leaked brokers left no memory for the second tenant worker VM (`0/3 nodes are available: 3 Insufficient memory`), which the order change described above made possible. The guard has not yet run against a real cluster, so the first run of this PR should carry the `full-e2e` label; its `clear after` lines are the first measured teardown times, and any unexpected `LEAKED-PODS` line is either a real leak or a false positive to understand before merge. Follow-ups, known and out of scope here: - The allowlist exempts a whole suite, so a new leak in an allowlisted suite stays hidden until the issue's fix removes the line. - Attribution is per suite directory, not per Chainsaw `Test`; the leak lines carry the release instance, which usually names the test. - Only pods are checked. A leaked PVC, LoadBalancer IP or bucket holds no node memory and is not seen. - The 300s budget rests on Chainsaw's cleanup timeout, not on a measured pod-gone latency; the `clear after` lines are where that measurement will come from. - A platform object recreated mid-suite (for example a HelmRelease uninstall/reinstall remediation, or a force-replace on an immutable field) has a root newer than the suite start, and its pods would be charged to whichever suite was running as a leak; the first CI runs are where to look for this. - The per-pod wait recomputes the remaining budget before each pod, and no unit test pins that. - `kubernetes-previous` is allowlisted for #4489 although the stuck finalizer was seen only in `kubernetes-latest`; both suites create a TenantControlPlane the same way, and the exemption hides any new leak there until #4489 is fixed. - Each clean suite now waits for its own pods to finish terminating before the next starts, where the tail used to overlap the next suite. The first full run's `clear after` lines say what that costs against the E2E job's 215-minute cap. - After a timeout followed by a failed re-list, the leak lines come from the list taken before the wait, so a pod whose own wait had already succeeded is still named and the count can be too high; the verdict is unaffected. - Likewise, when every wait succeeded and the re-list then fails, the inconclusive report still lists the pods whose deletion was just confirmed. In the boundary case where the budget runs out during the last successful wait and the re-list then fails, the result is inconclusive rather than a verdict from the last good list, because the guard only treats the budget as spent when it finds it exhausted at a check. - A mistyped `COZY_LEAK_ALLOWLIST` path passes allowlist validation; it fails closed, since no suite is then exempt. - An allowlisted suite is not waited on, so `kubernetes-latest` and `kubernetes-previous` will print `LEAKED-PODS ... allowlisted=` on most runs even for pods still in normal teardown; waiting out the budget first and applying the allowlist only to what remains would stop that noise. - A slow teardown in each of several suites can add up to 300s apiece, against the E2E job's 215-minute cap. On the first full run of this PR (run 36223896793) the `clear after` values of the suites that were waited on summed to 210s; the last full run before this change took 158 of the 215 minutes. - A pod whose owner's resource type is no longer served gets "the server doesn't have a resource type" rather than NotFound, so its suite is reported unchecked instead of leaked; it still fails. - `kubectl wait --for=delete` waits for deletion, while the guard skips pods in `Succeeded` or `Failed`; a pod that finishes but is never deleted (a Job under a leaked root) spends the rest of the budget before the suite shows `clear`. The verdict is right; only job time is lost. - The unit tests do not pin the one-line empty report Chainsaw writes for a directory with no tests (`_lib`), nor the branch that catches a spent budget before calling `kubectl wait`; both are verified against the Chainsaw and kubectl sources rather than by a test. - The runner's per-suite report directory from `mktemp -d` is not removed; the sandbox is discarded after the job. - Each non-suite directory under `hack/e2e-chainsaw/` (today `_lib`) costs one empty Chainsaw invocation, because the runner leaves the decision of what is a test to Chainsaw. ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff adds files under `hack/`, changes the `test-chainsaw` target, and rewords one comment and the failure line in the four E2E workflows. The trigger map's `hack/` entry is for cozystack/ccp, whose skills anchor on `hack/package.mk`, `hack/common-envs.mk` and `make generate`, none of which change here. A GitHub code search for `chainsaw` across the cozystack organisation finds hits only in this repository. ### Release note ```release-note test(e2e): the E2E app suite now fails the suite whose pods outlive its own cleanup by more than 300s, naming each leftover pod and its owner; suites with a filed leak are listed in hack/e2e-chainsaw/_lib/leak-allowlist.txt and only warn. ``` |
||
|
|
42b7239198 |
test(e2e): wait for the opensearch handoff before the first API call (#4462)
<!-- 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 sends its first OpenSearch API call, the index create in `make-the-cluster-permanently-yellow`, as soon as the nodes StatefulSet reports one ready replica. Two operator actions are released at that moment and neither has finished: the bootstrap pod handing over the cluster-manager role, and a rolling restart of the only data node. #4412 is the first of them hitting the create with a 503. This PR makes the step wait for both before its first call: an `error:` assert that `opensearch-test-bootstrap-0` no longer exists, then an assert that the nodes StatefulSet has settled on its update revision, in the same form the suite already uses after the listener move. Anchors below are opensearch-k8s-operator v2.8.0 with the in-tree patches applied, paths under `opensearch-operator/`. **Bootstrap handoff.** - The operator starts retiring the bootstrap pod only once the cluster is `Initialized` (`pkg/reconcilers/cluster.go:110-111`), and that flag is set as soon as every cluster-manager pool has all its replicas ready (`controllers/opensearchController.go:257`, `pkg/builders/cluster.go:1146-1164`). A Ready data node therefore starts the handoff and does not finish it. - The voting exclusion is a cluster-manager action, so it goes through the bootstrap pod and fails while that pod's security is not initialized. The operator then keeps the pod and requeues after 10s (`pkg/reconcilers/cluster.go:36`, `:186-190`). - The pod is deleted only after the bootstrap node has left the last committed voting configuration (`pkg/reconcilers/cluster.go:195-205`, `opensearch-gateway/services/os_client.go:760-771`). Once the pod object is gone, including its Terminating phase, the bootstrap process has exited and holds no vote, and the only cluster-manager-eligible node left is `opensearch-test-nodes-0`. - The assert cannot pass vacuously. The cluster forms around the bootstrap pod (`cluster.initial_master_nodes` names only that pod, `pkg/builders/cluster.go:807`), and the pod is created unconditionally while the cluster is not initialized (`pkg/reconcilers/cluster.go:112-113`). **Rolling restart on first boot.** The failing run's own operator log and StatefulSet dump show it: - `10:27:52.850` the reconcile in which `Initialized` became true. - `10:27:53.251` "Failed to exclude the bootstrap node from the voting configuration". - `10:27:53.981` "Starting rolling restart of the StatefulSet opensearch-test-nodes" (`pkg/reconcilers/rollingRestart.go:155`). - `10:27:54.079` "Couldn't proceed with rolling restart … because enabled shard allocation" (`:182`), the first step of the yellow path, after which the next 10s requeue moves on to `PreparePodForDelete` (`:195`) and deletes the pod. The StatefulSet in the same dump is at generation 2 with `currentRevision` `…744cc7b944` and `updateRevision` `…677c65d7fb`: the pod template was rewritten after `opensearch-test-nodes-0` was created, and the restart onto it waits only on `Initialized` (`rollingRestart.go:79`, `:123`). With one replica that restart takes down the node the step's requests go to, so a create that got past the handoff could still land on a terminating or restarting node. Once the StatefulSet reports `currentRevision == updateRevision` with its one replica updated and ready, the node the step talks to is on its final template and the operator has nothing pending. The suite runs one topology: one replica in a single pool with the cluster_manager, data and ingest roles, on a PVC. The bootstrap pod comes back only through the emptyDir recovery path (`pkg/reconcilers/cluster.go:452-478`), which a PVC-backed cluster never takes, and a restart that was not pending leaves the second assert true immediately. I checked the rest of the suite for the same window. Apart from this step, only the catch script (diagnostics, `|| true`) calls the OpenSearch HTTP API. `verify-endpoint-serves-the-chart-chain` makes a TLS handshake only, and it runs after the suite has already waited for the listener roll to complete, on a single node that is its own cluster-manager. Budgets: 3m for the handoff, which took about two seconds in the failing run; that leaves room for several 10s retire retries, a 30s requeue and the default 30s termination grace period the bootstrap pod runs with. 6m for the restart, shorter than the 12m the suite gives the listener roll because nothing is pulled here: the health steps requeue every 10s and the pod then restarts on an image the node already holds, where the first boot, bootstrap included, took under two and a half minutes. Residual risk, unproven: if the pod template were rewritten again after the rollout assert passed, that assert would have passed on a template that is not the final one. I found no mechanism for it. The StatefulSet builder reads no cluster status (the only status read in `pkg/builders/cluster.go` is the replica check in `AllMastersReady`), so the transition to `Initialized` cannot rewrite the template by itself; a rewrite would need a configuration or secret input to change after that point. Which input changed on first boot in the failing run I could not tell from the log, because the ControllerRevisions were not captured. What I ran: `chainsaw lint test` with Chainsaw v0.2.15, the version the e2e sandbox pins, passes on the changed suite. As a positive control, lint rejects each new operation when I rename one of its keys. `hack/select-e2e.sh` on this diff selects `opensearch` only. What I did not run: the E2E suite itself. The race is timing-dependent (one hit in six failed in-tree runs checked, per the issue), so a single green run would not prove the fix either. The argument rests on the operator code and the failing run's log cited above. Closes #4412 ### 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: The only file changed is `hack/e2e-chainsaw/opensearch/chainsaw-test.yaml`, which no downstream repository consumes. ### 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 NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Cluster health checks now wait for bootstrap completion and verify that all nodes are ready and up to date before proceeding. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
a66907f535 |
fix(backups): share the walkthrough wait helpers and fix the S3 preflight cleanup (#4635)
## What this PR does This fixes the S3 access preflight and a few backup walkthroughs, and moves the walkthrough wait helpers into one shared file. The access preflight now keeps its evidence and cleans up after itself. When `mc alias set` failed, it dropped the port-forward log, and now it prints it like the other exits do. The exit trap used to kill the port-forward before `mc rm`, so the delete always hit a closed port and the warning gave no reason. The delete now runs first, and the warning carries mc's own error, or says the delete timed out when mc was killed before writing one. A PUT that timed out (rc 124) may have landed, so it counts as uploaded. A PUT the server refused does not, so access that never went live gets no delete and no warning. The wait helpers were copied into every walkthrough and had drifted. `wait_for_field`, `wait_hr_ready` and `wait_deleted` had 12, 8 and 2 copies with 5, 3 and 2 different bodies. They now live in `examples/backups/_lib/wait-helpers.sh`, and each `00-helpers.sh` sources it. Call sites and wait budgets are unchanged. Where copies differed, the shared body keeps every diagnostic any copy had. What changes per walkthrough: - `wait_for_field` in every walkthrough but redis now shows kubectl's stderr, so a Forbidden or transport error is visible instead of reading as an empty field. `--ignore-not-found` keeps a resource that does not exist yet from printing an error on every poll. Redis already kept stderr. - `wait_for_field` in foundationdb, nats and vmi gains the optional 7th terminal-value argument. None of their callers pass it, so nothing changes at runtime. - vmi loses its progress dots. - `wait_hr_ready` in redis and kafka-metadata now prints every condition and the release history on Stalled or timeout. It also says whether the release never appeared, was deleted, or could not be looked up. - The kafka `wait_deleted` timeout message reads "it is still present" instead of "still present". `hack/backup-examples-shared-helpers.bats` goes red if a walkthrough defines one of these helpers again or stops providing them, and it pins both sides of the `wait_for_field` read: kubectl's reason for a failed read is shown, a missing resource prints nothing. The byte-identity pin between copies in `hack/mariadb-first-boot-waits.bats` is gone, and its errexit test runs through the mariadb walkthrough's helpers. The kafka topic-data demo had the empty-read race from #4400. `kafka_run` read topic state through a throwaway `kubectl run -i` Pod. It now execs into a long-lived CLI Pod that keeps the restricted security context, and `cleanup.sh` removes it by label. The Pod name comes from its own `KAFKA_TOPIC_CLI_POD`, so setting the kafka-metadata demo's `KAFKA_CLI_POD` does not move it. #4400 itself was already fixed on main for nats, kafka-metadata and rabbitmq. The nats and kafka-metadata demos could exec into a Pod they had just refused. They read CLI output inside `$(...)`, where bash does not apply errexit, so the ownership refusal was ignored. A failed read of the Pod's phase was treated as no Pod, and the demo went on to delete and replace it. Every setup step now returns on failure. Their "exec fails with kubectl's own error" tests could never fail: the runner's trace wrote the expected message into the same stderr the test grepped. The stub now prints the message itself. Each fix has a test that fails on main. For each fix I also reverted it and checked that only its own test goes red. One gap stays open outside this PR. `hack/select-e2e.sh` maps `examples/backups/<app>/` paths by app name, so a later PR that changes only `examples/backups/_lib/wait-helpers.sh` would select no suite. This PR is fine because it touches every walkthrough. The rule belongs to the select-e2e owners and is tracked in #4626. While reviewing I also found that the kafka README puts steps that create and exec into Pods under the tenant persona, and the tenant role can do neither. That is #4625, left for its own change. Left out on purpose, because they are outside what this diff changes: - The kafka topic demo and the kafka-metadata demo each carry a CLI Pod helper, and the kafka-metadata one runs without the restricted security context. - The walkthrough READMEs do not say that a directory now needs `../_lib` next to it. - The errexit test in `hack/mariadb-first-boot-waits.bats` loops over one file. I kept it so the rebase against #4471 stays mechanical. Closes #3757, closes #4243, closes #4244 ### 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 fix(backups): the backup walkthroughs share one set of wait helpers that keep kubectl errors visible, the kafka topic-data demo no longer reads empty replies from a throwaway Pod, the nats and kafka-metadata demos no longer exec into a CLI Pod they refused, and the e2e S3 access preflight says why client setup or cleanup failed and deletes its test object reliably. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Kafka topic backup walkthroughs now reuse a named CLI Pod, with restricted security settings and checks to prevent using a Pod owned by another demo. * Backup walkthroughs can wait for resources to reach a desired state, Helm releases to become ready, or resources to be deleted. * **Bug Fixes** * Kafka and NATS walkthroughs now stop and report errors when CLI Pod lookups, creation, deletion, or command execution fail. * S3 preflight cleanup now attempts removal after a timed-out upload and reports cleanup failures, including timeouts. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
|
a9dc106a81 |
fix(e2e): bound post-install-prep calls and make its failures name their cause (#4632)
## What this PR does The post-install-prep step of the e2e install could hang without failing, and several of its failure paths gave up without saying why. The script runs in the background of the install test, which waits on it, so a kubectl call that never returns keeps the install open until the job's own timeout. This PR fixes both problems in `hack/e2e-post-install-prep.sh`. - Every call that does work now has a wall-clock bound: the pool listing and creates, both applies, the CDI patch, the reads in the LINSTOR node loop, and the diagnostic reads on every failure path. Plain requests also get a smaller `--request-timeout`, so kubectl can say why a request failed before the outer timeout kills it. When a bound fires, the log names the call. - `kubectl wait --timeout` turned out to be no bound at all for the GET it runs first, so each wait attempt got an outer timeout too. If that timeout cuts an attempt, the timeout line says so. - The wait loop keeps kubectl's stderr now. An expired credential or an RBAC denial used to look the same as "not created yet" and ended in a timeout on the object. Each error is printed once when it changes, and the timeout line repeats the last one. - When the linstor wait times out, the script lists every HelmRelease first. The linstor release can be just waiting for a release further up the dependency chain, and the Ready messages name which one. - The MetalLB CRD wait and the CDI StorageProfile wait used to exit silently. Now they print a timeout line and one last read of the missing object with its error. The MetalLB wait also prints the metallb HelmRelease. - A failed or killed storage pool listing stops the script before any pool is created. Before, it read as "no pools exist", and the creates that followed failed with an unrelated error. All of this is tested in `hack/post-install-prep.bats` with the existing stub PATH and virtual clock. One test checks that every kubectl call on the success path and on each timeout path runs under a bound. The literal grep in `hack/linstor-storage-mode_test.bats` is replaced with a test of the behaviour. The rest of #3680 lives outside this script and moves to #4628: the empty `{"error":{}}` in helm-controller's Helm log, previous-container logs on passing runs, and node and pod resource state during install. Closes #3708, closes #3707, closes #3713, closes #3714, closes #3680. ### 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 every call in the post-install-prep step and make its failures name their cause, so a hung call fails the install instead of holding it until the job timeout ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Reliability** * Cluster operations now have bounded wait times, with clearer diagnostics when resources or endpoints are delayed or unavailable. * Storage setup stops when pool listings fail instead of masking the error. * **Tests** * Expanded coverage for timeout handling, error reporting, and failures during storage and resource setup. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |