Commit Graph
4 Commits
Author SHA1 Message Date
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>
2026-08-12 18:43:02 +03:00
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>
2026-08-05 16:46:50 +03:00
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 12fe3256f)
- foundationdb: retry the one-shot generations.reconciled read for up
  to 2m — the operator stamps it after the post-bounce uptime gate, so
  it can trail the fdbcli-derived health fields the previous assert
  converged on (a79f8009f)
- kubernetes-latest: add the version-independent SC-fallback-default
  Test (#2872 B1) calling verify_storageclass_fallback_default from the
  shared lib; a separate Test doc so it runs regardless of the heavy
  bringup Test's outcome, safe under the suite-wide parallel: 1 while
  it temporarily mutates cluster-scoped StorageClasses

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
2026-07-08 18:17:19 +05:00
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>
2026-06-25 13:42:34 +05:00