mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-03 02:27:01 +09:00
docs/agents-dev-principles
9
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
33cf6972bb |
test(qdrant): exercise the tenant trust anchor end to end (#4701)
## What this PR does The qdrant e2e suite now runs with TLS on and checks the tenant trust anchor end to end. Before this, the fixture left `tls` unset with `external: false`, so the chart rendered no CA and no `TenantProjection`, and nothing in e2e reached the controller or `tenantsecrets`. Setting `tls.enabled: true` turns TLS on without the LoadBalancer that `external: true` would allocate. The first new step checks that the anchor is published. The `qdrant-test-ca` sentinel has to report `Ready=True/Projected`, and `qdrant-test.tenant-ca` has to carry both the `tenant-ca` and `tenantresource=true` labels. The projection must hold only `ca.crt` with the same fingerprint as the chart's CA, and `tenantsecrets` must serve it. The key-bearing `qdrant-test-ca` and `qdrant-test-tls` must answer NotFound there, and any other error fails the step. The second step checks that the anchor is what decides trust. It reads the anchor through `tenantsecrets`, port-forwards the qdrant Service and runs `openssl s_client` with that file as the only CA, hostname check included. A CA with the same subject and another key must fail, and so must the runner's default trust store. A handshake failure counts only when the output shows a verification error, so a refused connection does not pass as a rejection. Plaintext HTTP must get no response. Curl exits 7 and 28 fail the step, and a second verified handshake afterwards proves the forward was still alive. TLS does not break the existing steps, because none of them talk to qdrant. `create-qdrant`, `wait-helmreleases-ready`, `verify-statefulset-and-storage` and `verify-pvc-reclaim-on-delete` only read Kubernetes objects: no `http://`, no port 6333 or 6334, no curl or exec, and the suite has no backup round trip. The release now also renders two Issuers, two Certificates and the sentinel. The chart switches its probes to HTTPS with `enable_tls`, so `readyReplicas: 1` now covers a pod serving TLS. Local checks: `chainsaw lint test` (v0.2.15) says the file is valid. `helm template` with the fixture's spec renders 2 Certificates, 2 Issuers and 1 TenantProjection, and none without TLS. I ran the handshake logic against a local `openssl s_server` with a CA and leaf shaped like the chart's. The anchor passes, the wrong CA and the default store fail on verification, a wrong hostname fails, plaintext gets `000`, and a closed port is not counted as a rejection. A sentinel with a missing source (`SourceNotFound`) is not tested here, the reconciler unit tests cover it. Review notes I left as they are: - Through `kubectl port-forward` the local socket is always accepted, so curl exits 7 and 28 only catch a dead forward process. A false pass would need qdrant to answer plaintext and a pod-side blip on that one request at the same time. - qdrant may reset the socket on the plaintext request, and some kubectl versions drop the whole port-forward on a pod-side error. Then the handshake after it turns the step red, not green. If CI shows `lost connection to pod` there, the plaintext check should get its own port-forward. - The suite's `catch` has no `describe` of the `TenantProjection`. The sentinel assert prints its conditions when it fails, but a red Secret assert or script after it would not. - The vendored chart's `servicemonitor.yaml` hardcodes `scheme: http`, so metrics scraping fails for any qdrant with TLS on. That was already the case before this PR, and the suite does not check metrics. Tracked in #4703. cert-manager does not set owner references here, so `qdrant-test-ca` and `qdrant-test-tls` stay in the namespace with their private keys after the app is deleted. That is tracked in #4704. The nats half of #3706 is tracked in #4702. Closes #3706 ### Screenshots No UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff touches only `hack/e2e-chainsaw/qdrant/`, which no entry in the trigger map covers. ### Release note ```release-note test(qdrant): e2e now runs qdrant with TLS on and checks that a client trusting only the tenant-published CA reaches it, while a wrong or missing anchor and plaintext HTTP are refused. ``` |
||
|
Aleksei Sviridkin
|
a7ac37be65 |
test(e2e): let HR-Ready waits outlast the install timeout of the release they wait on (#4471)
## What this PR does Stacked on #4468, which holds the raised backup waits to a start margin; review that one first. This PR's own diff is the six commits on top of it. Most HR-Ready waits in the E2E suites gave up before the release they were waiting on could fail its install. cozystack-api gives every release it generates a 600s install timeout, and the Chainsaw asserts in front of those releases sat at 5 or 6 minutes. When an install genuinely failed, the suite reported a bare assert timeout against a release that was still installing, and the reason the install failed, which the release records when its own timeout fires, was never reached. Red runs failed just as often as they should; they only said less. I raised every HR-Ready wait above the install timeout of its own release, by the 60s start margin #4468 introduces: - Chainsaw asserts on releases cozystack-api generates go to 11m, across bucket, clickhouse, etcd, external-dns, foundationdb, harbor, mongodb, openbao, opensearch, postgres, qdrant, rabbitmq, redis, valkey and vminstance. - Asserts on nested `<name>-system` releases take their floor from the template that renders them: 11m for external-dns, openbao and qdrant (10m `spec.timeout`), 16m for `harbor-test-system` (15m). `harbor-test` sat exactly on its 600s floor, which expires first because the assert's clock starts before the install's; it goes to 11m too. - The kuberture Package release takes its 10m from cozystack-operator and goes to 11m. - In the backup walkthroughs, the bucket waits go from 300s to 660s in clickhouse, kafka, mariadb, mongodb, postgres, rabbitmq and redis, and the two postgres waits go from 360s to 660s. The kafka bucket wait passed no budget and silently took the helper's 300s default, which is why earlier scans for literal budgets missed it. Each Chainsaw op that runs a walkthrough grows by exactly what its waits gained (clickhouse 78m, kafka 46m, mariadb 48m, mongodb 42m, postgres 66m, rabbitmq 48m, redis 36m), so a failure at any later wait keeps the headroom it had and is still reported by the script instead of being cut off by the op. Two waits stay as they are, on purpose: - kafka: `kafka-rd` sets a 7h install timeout, which no budget inside the 215-minute E2E job can clear. The kafka assert and the Kafka waits in the kafka and kafka-metadata walkthroughs keep their numbers, and the assert now says why in-line. - `vm-instance-test`: `vm-instance-rd` disables the install wait, so its release goes Ready as soon as helm has applied and there is no floor to clear. Earlier lists of this problem included it by mistake. The cost falls on red runs only: a suite whose release never goes Ready now spends up to six more minutes before it reports, and in exchange the report carries the release's own reason. Green runs are unchanged. Suites run one after another in a single job capped at 215 minutes, so a systemic failure that reddens many suites now reaches that cap sooner, and a job cancelled at the cap loses its log and `cozyreport.tgz`. I took that trade because a failed install that says why is worth more than a red run that ends a few minutes earlier. `docs/agents/e2e-testing.md` stopped being true in several places once the waits were raised: it described most of the tree as under the floor, mariadb as the only suite raised, the walkthrough bucket waits as under it, and harbor's asserts as sitting on and under theirs. I rewrote those sentences, and the one that pointed at "every wait above the band" of 5-6m, kept the start-margin wording from #4468, and added that a walkthrough wait passing no budget takes the helper's default. The comments in `hack/mariadb-first-boot-waits.bats` that quoted the old op and bucket figures are restated; its existing checks and constants are unchanged, because every op grew by exactly its bucket delta and each remainder still sits at exactly its constant. I also added postgres to the table that file holds the walkthroughs to, so its raised waits and its op cannot drift back apart unnoticed, and a test that holds each table suite's bucket wait to the bucket release's floor, read from the api-server default with `bucket-rd` checked for an override; the op check alone could not catch a bucket wait dropping back, since a lower wait only widens the slack. redis stays out: its walkthrough waits on no redis HelmRelease, only on the bucket and on the sentinel master, so the table's floor check has nothing to read there. How I checked it, since E2E cannot run locally: - I re-ran the enumeration from the sizing section of `docs/agents/e2e-testing.md` over every `hack/e2e-chainsaw/*/chainsaw-test.yaml` and every `wait_hr_ready` call under `examples/backups/`, literal or default, and held each against its own release's floor. Every `Ready=True` assert and every `wait_hr_ready` call a suite drives now clears its floor, except the kafka ones named above. The opt-in etcd round-trip uses bare `kubectl wait hr` calls at 300s instead of that helper; they are listed in #4470. The `Ready=False` assert on the rejected redis upgrade is not a readiness wait and is left out. - `make bats-unit-mariadb-first-boot-waits` passes, 31 of 31. For the new postgres row, putting the op back to 50m fails only the postgres slack test, and a postgres wait at 659s fails only the postgres floor test. Setting any one table suite's bucket wait to 659s fails only the bucket test, naming only that suite, and an install-timeout override on `bucket-rd` fails it too. I ran the mutations under plain `bats` so a fail-fast runner could not hide a second failure. - `hack/select-e2e.sh` over this diff selects bucket, clickhouse, etcd, external-dns, foundationdb, harbor, kafka, kuberture, mariadb, mongodb, openbao, opensearch, postgres, qdrant, rabbitmq, redis, valkey and vminstance. Out of scope, and tracked in #4470: the tenant addon waits in `hack/e2e-chainsaw/_lib/run-kubernetes.sh` (an edit there escalates the whole E2E run), the redis and kafka-metadata `wait_hr_ready` helpers that print no reason on timeout, and the opt-in etcd round-trip's bare `kubectl wait` calls. Known and left for later, none of them changed here: - Only the mariadb Chainsaw asserts and the walkthroughs in the guard's table are held by a test. The other raised asserts, and the redis and kafka bucket waits, rest on literals with in-line reasons; #4470 covers extending the guard. - The Kafka waits in `examples/backups/kafka/04-create-kafka.sh`, `07-restore-to-copy.sh` and `kafka-metadata/run-all.sh` stay at 300s against the 7h floor without an in-line reason at the call site. The doc's rule asks that of Chainsaw asserts only, and the kafka Chainsaw assert carries it. The postgres waits are the ones #3748 is about. Their budget now clears the install-timeout floor and a test holds it there; what a CNPG first boot costs on this platform is still unmeasured, so that issue stays open. Closes #3771 Closes #3753 ### Screenshots Not a UI change. ### Downstream repositories I walked the trigger map in `docs/agents/contributing.md` against the diff. It touches E2E suites, backup walkthrough scripts, a unit-lane bats file and an agent doc, none of which any downstream repository restates. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(tests): HelmRelease readiness waits in the E2E suites and backup walkthroughs now outlast the install timeout of the release they wait on, so a failed install reports its own reason instead of a bare timeout. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Extended readiness and backup round-trip test timeouts across multiple services to accommodate slower startup and provisioning. * Added checks to verify that bucket readiness waits meet configured installation-timeout requirements. * **Documentation** * Updated testing guidance to clarify how readiness waits relate to release-specific timeouts and overall test budgets. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
efb4b1f5aa |
test(qdrant): exercise the tenant trust anchor end to end
The fixture left TLS unset with external off, so the chart rendered no CA and no TenantProjection, and no e2e run walked the chain from the sentinel through the CA-extraction controller to the tenantsecrets read path. Turn TLS on explicitly, without allocating a LoadBalancer, and check both ends: the projection is key-free, carries the chart's CA and is served through tenantsecrets while the key-bearing Secrets are not; and a client trusting only that anchor verifies the listener, while a wrong anchor, no anchor and plaintext HTTP all fail. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
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> |
||
|
Myasnikov Daniil
|
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> |
||
|
Aleksei Sviridkin
|
e9411d6b8c |
fix(apps): make the post-delete cleanup hooks report failure
The qdrant and harbor post-delete hooks guarded every delete with `|| echo "WARNING: ..." >&2`, set no `-e`, and ended on an unconditional completion message, so the container exited 0 on every path. Helm read that as a successful hook and the hook-succeeded delete policy removed the Job together with the pod that carried the warning, leaving an uninstall that reclaimed nothing indistinguishable from one that had nothing to reclaim. Count the failures instead and set the exit code at the end, so a failed delete does not skip the deletes that follow it. An object that is already gone stays a success: every delete keeps --ignore-not-found, which exits 0 in that case. This is the shape packages/apps/mariadb already uses for the same problem. Both hooks also consumed the output of `kubectl get` directly -- harbor by iterating it, qdrant by piping it into grep, whose status the pipeline then reports. A listing that failed therefore arrived as an empty list and the sweep was skipped rather than reported. Each listing is now captured before it is used, so a listing that failed and one that returned nothing are different events. A hook that exits non-zero keeps its Job and pods, which is the point: they hold the log. Give the Job one retry rather than the Kubernetes default of six or none at all. Every delete is idempotent, so a second attempt re-issues an API call rather than re-running any logic, and one short backoff is what separates a transient apiserver error from a real one -- these failures become visible for the first time here, and a signal that fires on noise stops being read, including on the day it is right. Six is the far end of the same mistake: ten minutes of backoff across seven pods outlasts the hook timeout, and Helm reports that timeout instead of the hook's own error. The qdrant Chainsaw suite explained its post-delete pod collector by quoting the guard string removed here and concluding the pod is always collected away. That is now backwards: a failed cleanup is exactly when the pod survives and the collector is the evidence. Rewrite the note, and the sentence in the harbor suite pointing at it. hack/app-cleanup-hooks_test.bats renders both scripts and runs them against a fake kubectl. There are eight failure paths across the two hooks -- six in harbor, two in qdrant -- and each has a case that goes red when that one path alone is reverted to a swallowed warning. Message assertions are anchored on the hook's own `ERROR: ` prefix, because the fake kubectl names the object in its own diagnostic too and an unanchored match is satisfied by the fixture. The helm-unittest suites pin the retry limit and the hook delete policy: adding hook-failed to that annotation hands the Job back to Helm to delete on failure and destroys the evidence this change exists to keep, and until these asserts that reversal left both suites green. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
3fc8e524b2 |
fix(e2e): select catch pod logs by the nested release instance label
The harbor, openbao and qdrant app charts deploy their workload through a nested HelmRelease named `<release>-system`, and pass `fullnameOverride: <release>` down to it. The inner chart therefore stamps app.kubernetes.io/instance from its own release name, with the suffix, while object names come from the override, without it. The catch blocks selected on the outer release name, so podLogs matched no pods and every failed run collected an empty result. A pod name says nothing about its instance label; the two come from different sources on purpose. In qdrant the unsuffixed value is therefore not retired but kept as a collector of its own: the parent chart's PVC-cleanup Job labels its pod template with the parent release name, and that suite deletes the parent inside a `try:`, so the Job is in scope while the test still runs. The comment there is explicit about how narrow that window is -- the hook always exits 0, so Helm calls it succeeded and `hook-succeeded` takes the pod away; what is left is an uninstall that hangs with Helm still waiting on it. Harbor renders the same hook and does not get the same collector. It has no `delete` op, so its parent release is uninstalled only in Chainsaw's cleanup phase, which no catch can observe: catch is a defer inside the step, while the cleaner runs from t.Cleanup, after the test function returns. That collector would be the same permanently-empty capture this commit removes. A comment records the asymmetry so the next reader does not restore it for symmetry. openbao renders no such hook at all. Each workload value is now also pinned on a resource in an assert the suite already runs, so the chart renaming a label fails a step instead of quietly emptying a capture. The comments state what that does and does not cover: it does not catch a wrong value typed into a selector, and it catches nothing when the run fails before the assert. Harbor's database was reached by no collector at all. It is selected on `cnpg.io/cluster`, CNPG's own primary label, rather than on the instance label CNPG merely derives from the Cluster name -- the primary label has nothing to drift from, and it is what the postgres suite and the WorkloadMonitors already use. Each lookup keeps stderr apart from the value it captures. kubectl exits 0 while logging a klog line about an API group it could not reach, and an aggregated APIService in that state is what a failed run usually looks like; folded into the value, that line makes the "nothing found" branch unreachable and prints as though it were a result. Telling those outcomes apart is one helper in hack/e2e-chainsaw/_lib/ rather than a copy per call site, separated from the kubectl call so it can be pinned without a cluster, the way etcd-probe.sh already is. Both the census and the qdrant cleanup listing report through it, differing only in wording, so the discrimination is implemented and tested once. The tests cover each outcome for both wordings, including the zero exit with a klog line on stderr -- the case a folded stream turned into a row. A collector that matches nothing cannot say so: podLogs is silent, and `kubectl get -l` exits zero on an empty match. Each test-wide catch prints the instance label of every pod in the namespace, and the qdrant cleanup catch reports its three outcomes apart from a single call -- printing what that call returned rather than running it again, which could put a healthy listing under a failure heading. That census runs on a degraded cluster, where kubectl otherwise retries discovery against an unreachable apiserver until something kills the script, so every lookup carries `--request-timeout`. Measured against an unreachable apiserver, a bounded call costs about five times the flag, since kubectl retries discovery five times before it gives up. Each script then carries an explicit op timeout sized above its own calls -- 2m for the single-call censuses, 5m for the qdrant cleanup listing, which makes three. An inner budget is what should fire: a SIGKILL at the op boundary yields nothing, where a kubectl that gives up leaves an error to print. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov DaniilandClaude Fable 5
|
6c76a462a0 |
test(e2e): fold remaining post-branch bats drift into the Chainsaw suites
- qdrant: add the post-delete PVC-reclaim guard (#3059) — delete the CR and poll the ordinal-anchored data PVC to absence via the error op (retries transient API errors internally), with a PVC/jobs/pods dump on failure; Chainsaw auto-cleanup alone would never notice a leaked StatefulSet-templated PVC - kuberture: make the split-horizon negative log assertions fail loud — under set -e a pipeline beginning with ! suppresses errexit, so both leak checks silently passed (the same vacuous-negation bug main fixed in |
||
|
Myasnikov DaniilandClaude Opus 4.8
|
e22d84f2c8 |
test(e2e): migrate app suite from BATS to Chainsaw and wire into CI
Replaces the per-app BATS E2E suite (hack/e2e-apps/*.bats) with declarative Kyverno Chainsaw tests under hack/e2e-chainsaw/, extending this branch's pilot (postgres + bucket) to all 18 app suites. - Declarative ports assert on status/conditions and concrete fields; the repeated "timeout ... until kubectl get" + "kubectl wait" boilerplate collapses into a single polling assert with structured failure diffs and automatic events/describe/podLogs capture on failure. - Inherently imperative suites keep their logic in script steps: openbao init/unseal, kuberture split-horizon probes, vminstance, the gateway admission/impersonation cases, and kubernetes-latest/previous, which wrap the relocated hack/e2e-chainsaw/_lib/run-kubernetes.sh verbatim. - Gateway tests derive the tenant apex from the namespace host label at runtime, so they are host-independent. CI: the e2e job now runs "chainsaw test hack/e2e-chainsaw/" via a new test-chainsaw target and uploads the JUnit chainsaw-report.xml; the chainsaw binary is added to the e2e-sandbox image. install-cozystack and test-openapi stay BATS. The per-app 3-retry loop is dropped, since assertion polling replaces the fixed-timeout flakiness it papered over. Verified against a dev cluster (DB/app, storage and VM suites pass). Service port assertions use the ports[*].port projection form because Chainsaw v0.2.15 mis-evaluates JMESPath number-literal port filters. Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |