The shell translator rewrites test bodies and loses Bats reporting.
The unit lane needs complete TAP results and JUnit reports without
changing how live-cluster suites run.
Sourced production helpers still need a dash pass, and unit test bodies
need nounset without exporting shell options into subprocesses.
Assisted-by: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The restore path wraps the topic in \Q...\E on both the --describe and the
--alter calls so a name with a regex-special character cannot match a
colliding sibling. Only the --alter guard was pinned: the colliding pair in
the fixture gave both topics the same partition count and replication
factor, so stripping \Q on the --describe read the sibling's identical
numbers and nothing downstream diverged.
Give the sibling a distinct, larger partition count. A stripped --describe
\Q now matches it first (glob order places desc.pay-events before
desc.pay.events), reads 9 > the backup's 7, and aborts with "cannot
decrease partitions" so no alter is recorded and the assertion reddens.
Also point the suite's scratch file at a per-test path via METADATA_FILE so
runs cannot read back a prior test's records.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
The Kafka restore path rejects a recorded replication factor that is not
numeric, but only the partition-count guard had its own test. Dropping
both guards reddened that test, so the pair looked covered, while dropping
the replication-factor guard alone left the suite green. Add a case that
drives the create path (topic absent) with a valid recorded partition
count and a non-numeric replication factor, so the guard is pinned on its
own and a future edit that removes it fails loudly.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
The restore path validated only the shape parsed from the live cluster,
not the partition count and replication factor read from the stored
object. A non-numeric recorded partition count made `[ "${lp}" -lt
"${parts}" ]` and its `elif` exit 2; an `if` condition is exempt from
errexit, so both read as false, the topic was skipped, and the loop still
printed its per-topic success line - so the RestoreJob reached Succeeded
without ever touching the topic. A well-formed but wrong-content object,
which curl -f cannot catch, must fail loudly like the unrecognised-record
guard.
Validate the recorded parts and rf are numeric before the comparisons and
--create, with a bats case that reddens when the guard is dropped.
Also correct a stale to-copy example comment: the strategy reads the exact
object the source backup recorded in status.artifact.uri (injected as
ARTIFACT_URI), not a key derived from .Backup.ApplicationRef.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
The restore half of the driver - the half
that mutates a live Kafka cluster - had no test reaching its happy path,
so three reversions of it left the suite green.
- Add restore reconcile tests mirroring the Rabbitmq driver this was
copied from: ToCopy (pins the targetApplicationRef.Name merge - a
to-copy restore must resolve the TARGET, not write the source's topics
back into the source), NoRecordedArtifactURI, TargetClusterNotReady and
TargetNotFound. Verified non-vacuous: neutralising the name merge reddens
ToCopy, and reverting the fail-fast ordering below reddens
NoRecordedArtifactURI.
- Fail fast on a Backup with no status.artifact.uri: move the check ahead
of the readiness gate so it terminates immediately instead of waiting out
the whole deadline and then reporting a misleading "timed out waiting for
Kafka cluster to become Ready" (reconcileRabbitmqRestore checks it first;
same order here).
- Add bats cases for the fail-closed guards that no test reddened: MODE
validation, the s3:// URI prefix, cleanup treating 404 as success, an
unparsable backup/live describe, a kafka-configs describe error, and an
unrecognised record kind (plus a CFG_FAIL knob in the configs stub). The
describe-parse guards abort via `set -euo pipefail` at the
`X=$(describe_field ...)` assignment, so these pin the fail-closed
behaviour; the explicit guards remain as belt-and-braces.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
- delete_topic waited with `grep -qx` (BRE), so deleting `audit.events`
matched the live `audit-events` and spun to its timeout — the e2e
failed at step 25 on every run that reached it. Use `grep -qxF`.
- The two -k/--connect-to guards were written as `! grep`, which cannot
fail under hack/cozytest.sh (it wraps each @test in `set -e` + a
trailing `return 0`, and bash exempts a `!`-negated command from
errexit). Rewrite them as positive `if grep ...; then return 1; fi`;
verified the -k reinstatement now reddens the suite under cozytest.
- Cover the restore-side literal-topic guard that was unpinned at every
level: a bats case runs the existing-topic branch with a colliding
sibling and asserts the `--alter` argv carries `\Q…\E`, and run-all.sh
now leaves `audit.events` live so the e2e exercises that branch instead
of only `--create`. Add bats cases for the partition-shrink refusal, the
`-f` upload guard, and `--exclude-internal`.
- Bound a running Job: reconcileKafka / reconcileKafkaRestore now fail the
BackupJob/RestoreJob once kafkaDefaultBackupDeadline elapses in the
Job-running branch (buildJobStrategyBatchJob sets no
activeDeadlineSeconds), so an unschedulable pod no longer pins it in
Running forever. Pinned by TestReconcileKafka_FailsWhenJobNeverCompletes.
- Fail closed on an unknown record kind and an unknown MODE, and validate
the live partition/RF parsed on restore (an empty value silently skipped
reconciliation). Correct the stale "image pinned in the strategy" comment
in the example helpers and the chainsaw test (the controller now resolves
the target broker's image as .ClientImage).
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
The suite was written for upstream bats and passed there, but CI runs
hack/cozytest.sh, which knows only @test + bash: it never calls setup()
and provides no $BATS_TEST_TMPDIR / run / $status / $output. So setup()
never ran and the first test aborted with "STATE: parameter not set"
under set -u. Move the stub setup into an init_stubs helper each test
calls, drop the bats builtins for plain exit-code and $(...) assertions,
and mktemp per test instead of $BATS_TEST_TMPDIR. Verified with
hack/cozytest.sh (11/11), the runner make bats-unit-tests uses.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
The comment cited "the 160-line driver script"; a count about the file
decays on the next edit and nothing pins it. State the invariant instead.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
The port workaround built --connect-to's HOST1 from ${host}, but the
virtual-hosted branch (forcePathStyle:false) sends to ${bucket}.${host}.
curl matches HOST1 against the request host, so on a virtual-hosted AND
non-default-port endpoint the redirect never fired and the request hit the
wrong port. Derive HOST1 from the host the URL actually uses; the real
connection target stays ${host}:${port}. Path-style is unaffected. Add a
bats case for the virtual-hosted ported mode, which the suite did not cover.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
- Regression fix: an earlier change dropped the curl --connect-to port workaround along
with -k, but curl 7.76.1 (the image's) 403s on ANY ported endpoint,
which is the shape backupStorage.endpoint ships (:8333) and what
provisionBucket:false and s3.ingress.enabled:false produce. Restore
--connect-to (sign a portless URL, redirect to the real port), keeping
-k off so TLS is still verified. IPv6-safe host/port split, path prefix
preserved. The port-less ingress passed; the ported case is the one that regressed.
- Correct the S3 envelope claim in values.yaml and backup-classes.md: the
403 is decided by the port, not a body-hash the backend recomputes, and
--connect-to handles it on SeaweedFS and MinIO alike.
- Hold the script fixes at the layer the contract lives at. Externalise
the driver to files/kafka-backup.sh (embedded verbatim via .Files.Get,
render byte-identical) and add hack/kafka-backup-script.bats, which
EXECUTES it against stubbed kafka CLIs + curl and asserts behaviour: \Q
literal match on the colliding pair, --connect-to with no -k on a ported
endpoint, fail-closed --list, comma-value bracket grouping, RF-divergence
abort, and the cleanup delete's HTTP handling. Not a template grep.
- Pin the Backup-delete dispatch and the readiness deadline in Go:
TestCleanupOnDelete_Kafka_RoutesToKafkaCleanup and
TestReconcileKafka_FailsAfterReadinessDeadline.
- Docs: config restore is additive (no --delete-config); export time
scales with topic count and the Job has no wall clock (as its siblings).
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
Myasnikov Daniil