Skip to content

fix(chaos): compound runs and resilience faults go through the safety guard and the disruption lease - #375

Merged
bmscomp merged 1 commit into
mainfrom
fix/audit-06-guard-lease
Oct 17, 2026
Merged

bmscomp merged 1 commit into
mainfrom
fix/audit-06-guard-lease

Conversation

@bmscomp

@bmscomp bmscomp commented Oct 17, 2026

Copy link
Copy Markdown
Owner

Why

A compound run and a resilience run reach the chaos providers past the two checks a disruption plan gets: the safety guard, which counts the brokers the faults hit, and the lease that keeps disruptions one at a time per cluster. Checked in the code:

  • Compound. POST /api/disruptions/compound (DisruptionAnalysisResource.executeCompound) ran only CompoundChaosOrchestrator.checkLimits, whose javadoc called it "the only check its faults get". A chaos:run caller could post {"faults":[{"faultSpec":{"experimentName":"x","disruptionType":"POD_KILL","targetAll":true}}]}. providerName defaults to kubernetes, whose POD_KILL force-deletes every pod with strimzi.io/component-type=kafka at once, every broker and every KRaft controller. The same fault as a plan step is REJECTED with "Plan would affect ALL 3 brokers".
  • Resilience. ResilienceOrchestrator.execute calls chaosCoordinator.triggerFault with neither the guard nor the lease. A CI resilience run could kill broker 1 while an engineer's leader-cascade plan killed broker 0. Each passes alone; together two of three brokers are down under min.insync.replicas=2, and every acks=all producer on the cluster fails.
  • Each undoes the other. A compound or resilience NETWORK_PARTITION on the kubernetes provider ends with cleanup(), which deletes every managed-by=kates NetworkPolicy, so it cut short a running plan's partition. A plan's SCALE_DOWN rollback restores every snapshot in the namespace, a resilience run's scale-down included.
  • The concurrent compound wait. executeConcurrent waited for each fault only timeoutSec, 120 s by default (DisruptionDtos.java:15). A longer fault was reported "Timed out after 120s" while it still ran, so a lease released in a finally would go back with the fault still in.
  • The javadoc. DisruptionOrchestrator.execute said the lease "cannot be bypassed by adding a caller". The DisruptionLauncher 409 and the REJECTED warning say the holder is a disruption plan, which after this PR it need not be.

Changes

DisruptionAnalysisResource.executeCompound (POST /api/disruptions/compound), in this order:

  1. compoundOrchestrator.checkLimits(faults), now public, runs first. A fault with no faultSpec is still refused by its place (faults[i].faultSpec); the guard reads every spec and would throw on a null one.
  2. The faults are counted as a plan named compound, one step faults[i] per fault, each with targetTopic blanked. A compound fault is never aimed at a topic's leader (PodTargets ignores it, and the Litmus provider uses it as the DNS TARGET_HOSTNAMES), and the guard's targeted() would count the leader instead. A plan validatePlan refuses is a 400: No fault was triggered: and its errors, joined by ; .
  3. tryAcquire(currentTarget(), "compound#<uuid>"). While another disruption holds the lease: 409 Conflict, "Another disruption is running against , so no fault was triggered".
  4. The faults run inside try { … } finally { release }.

The OpenAPI 400 text names the guard's refusal, and a 409 response is documented.

CompoundChaosOrchestrator

  • executeConcurrent waits for each fault max(timeoutSec, delayBeforeSec + chaosDurationSec + 120) seconds, the wait executeSequential already gives it. A timeout names that wait.
  • checkLimits is public. Its javadoc, in the same number of lines, says the resource asks it first, then the guard, and holds the lease.

ResilienceResource (POST /api/resilience and POST /api/resilience/scenarios/{id})

  • New private unsafeOrBusy(FaultSpec). It answers 409 while concurrencyGuard.isBusy(currentTarget()), as DisruptionLauncher does for a plan. Otherwise it validates the fault as a one-step plan (resilience, step chaosSpec, targetTopic blanked) and answers 400, No fault was injected: and its errors. Both use ApiError.of(...) with Bad Request/Conflict and no fieldErrors.
  • Both endpoints call it after every other check, just before the stream starts, so the cheaper 400s still answer without a cluster. Their OpenAPI texts name the guard's 400, and a 409 is documented on each.

ResilienceOrchestrator.execute

  • Takes the lease (resilience:<runId>#<uuid>) just before step 4 and releases it in a finally on the outer try, so it is held through the fault, the recovery wait and the report. A run that finds the lease taken ends ERROR, "Another disruption is running against , so no fault was injected", and no fault goes in.
  • The lease is taken ahead of the continuous probes, not right at triggerFault: refused after the probes had started, their thread would never stop.
  • The class javadoc's step 4 says the lease is taken there.

Messages and javadoc, each edited in place in the same number of lines, so no CLI ref below them moves:

  • DisruptionLauncher 409: "Another disruption (a plan, a compound run or a resilience run's fault) is running against . Wait for it to finish before starting another."
  • DisruptionOrchestrator's REJECTED warning: "another disruption is already running against …". Its execute javadoc drops "cannot be bypassed by adding a caller": plans take the lease there, and compound runs and resilience faults take the same lease themselves.
  • DisruptionConcurrencyGuard's class javadoc: the lease serialises plans, compound runs and resilience faults per target.

kates mcp: four anchored caveat refs re-pointed, anchors unchanged, no caveat text changed.

  • mcp_tools_runs.go: DisruptionAnalysisResource.java:110-141 → 118-192, ResilienceResource.java:97-127 → 107-137.
  • mcp_tools_scenario.go: ResilienceOrchestrator.java:144-145 → 158-159.
  • mcp_tools_chaos.go: CompoundChaosOrchestrator.java:150-169 → 155-174.

mcpCaveatActivityDisruptionRows ("resilience runs and compound chaos write no row") stays true.

Docs

  • Book 07: the plan-versus-resilience table's "Safety guard" row, and "Another plan running", now "Another disruption running". The callout "A Resilience Run Has No Safety Guard" is now "A Resilience Run Has No Rollback". The one-at-a-time rule covers compound runs and resilience faults; the fault-limits paragraph says compound and resilience runs are also counted by the guard and get a 400; the Limits of the Guard bullet and the resilience section's intro say what a resilience run gets.
  • Book 11: the 409 on plans also comes while a compound run or a resilience fault holds the cluster. The resilience section says what the guard checks, adds the 400 and the 409 to both endpoints, and the ERROR when the lease is taken at injection.
  • Books 01, 02, 05 and 10, the glossary's resilience run and safety guard entries, and Part III's intro.
  • Stretched-cluster guide 04: what the guard checks of a resilience run. Its A6 site-B transient hits every broker, so the guard now refuses it as a resilience run too. The guide runs it outside Kates instead, with kubectl delete pod -l as make gameday does, while a REST INTEGRITY run writes (so no rpoMs).

What callers see. Runs that used to start are now refused: on a 1-broker cluster ("Plan would affect ALL 1 brokers"), with a kates.chaos.kafka.namespace or label that matches no broker ("No broker pods found"), with targetAll on the brokers, or with a drain of the node holding every broker. Parallel resilience runs now take turns: the second gets a 409 before it starts, or ends ERROR with no fault if it finds the lease taken when its steadyStateSec is up.

Tests

New:

  • DisruptionAnalysisResourceCompoundTest (plain JUnit, the guard and orchestrator mocked, a real DisruptionConcurrencyGuard):
    • anUnsafeCompoundRunIsRefusedBeforeAnyFault: 400 naming the guard's error, neither executeConcurrent nor executeSequential called;
    • aCompoundRunWhileTheLeaseIsHeldIsAConflict: 409, no fault;
    • theLeaseIsHeldWhileTheFaultsRunAndReleasedAfter: busy inside the orchestrator call, free after a 200 and after the orchestrator throws;
    • eachFaultIsAStepCountedWithoutItsTargetTopic: the plan validatePlan gets.
  • DisruptionResourceTest.aCompoundRunWithNoBrokerToCountIsRefused: the guard wired end to end, with a targetAll POD_KILL. It accepts either "No fault was triggered: No broker pods found" or "No fault was triggered: Plan would affect ALL ". @QuarkusTest's Kubernetes client reads the local kubeconfig, which on the machine this was built on reached a kind cluster with 3 brokers. With targetAll the run is refused either way and nothing is killed. aCompoundRunWithAFaultOutsideTheChaosLimitsIsRefused is unchanged and still passes, so the limits are still asked first.
  • ResilienceOrchestratorLeaseTest:
    • aFaultWhileAnotherDisruptionHoldsTheLeaseIsNotInjected: ERROR, triggerFault never called;
    • theLeaseIsHeldFromTheFaultThroughRecovery: busy inside triggerFault and inside the probe evaluation;
    • theLeaseIsReleasedWhenTheRunEndsOrFails: after COMPLETED, and after triggerFault throws.
  • ResilienceResourceTest: anUnsafeFaultIsRefusedBeforeTheStream (400) and aRunWhileADisruptionHoldsTheLeaseIsAConflict (409), each on POST /api/resilience and /api/resilience/scenarios/broker-crash, with the orchestrator never called.
  • CompoundChaosOrchestratorTest.aConcurrentFaultLongerThanTimeoutSecIsWaitedFor: timeoutSec 0 and a fault that ends after about 200 ms. Before the fix it was "Timed out after 0s".

Changed:

  • ResilienceResourceTest and ResilienceResourceStreamTest mock DisruptionSafetyGuard with a safe answer in @BeforeEach. A bare mock returns null, and the real guard finds no broker in CI.
  • ResilienceOrchestratorBenchmarkStatusTest, ResilienceOrchestratorChaosStartTest, ResilienceOrchestratorRecoveryTest and ResilienceOrchestratorTestRunIdTest set the new concurrencyGuard field.

Run locally, all passing:

  • cd kates && ./mvnw -o test -Dquarkus.grpc.codegen.skip=true -Dquarkus.http.test-port=0: 1,477 tests, 0 failures, 0 errors, 0 skipped. The "Cleanup failed" WARN lines come from mock-server 500s the tests inject on purpose.
  • spotless:check on JDK 25: clean.
  • cd cli && go test ./..., including the caveat-ref anchor tests against the kates/ edits; go vet ./...; golangci-lint run --new-from-rev=origin/main ./...: 0 issues; govulncheck ./...: no vulnerabilities.
  • scripts/check-cli-style.sh, scripts/check-cli-compat.sh.
  • scripts/check-book-style.sh: the structure ratchet holds for all 38 pages.
  • scripts/gen-version-matrix.sh --check, scripts/gen-chart-table.sh --check, which also cover docs/stretched-kafka.
  • scripts/check-book-contrast.py: all 170 colour pairs meet WCAG AA.
  • scripts/check-mermaid.sh: all 156 diagrams parse.
  • A scratch-copy HTML render of the book: 39 chapters prepared, 38 pages rendered, 0 warnings. check-book-fragments.py finds all 7,460 fragment links landing on an id.
  • Not run: the charts set (helm lint, helm template, check-versions.sh, yamllint), make check-help, actionlint and shellcheck. No chart, Makefile, .github or shell file changed.

Left to CI:

  • Book PDF render (book.yml, TeX Live with ebgaramond);
  • native image build and smoke test (native.yml, triggered by kates/**);
  • the *IT integration tests (Testcontainers, Docker);
  • Kind cluster tests;
  • Docker image builds;
  • the github-advanced-security AI scan, which is red on every PR (a GitHub-side error).

Decisions

  • D1: an unsafe compound or resilience fault is a 400, as these endpoints' other "no fault was triggered/injected" refusals are, not the 422 a REJECTED plan gets.
  • D2 (a): a concurrent compound run waits for each fault at least delayBeforeSec + chaosDurationSec + 120 s.
  • D3: a resilience run holds the lease from just before triggerFault through the recovery wait and the report, with one finally.
  • D4: one PR for compound and resilience, not split. They share the Launcher and Orchestrator message edits and the book 07 and 11 rows.
  • D5: the guard's warnings ("Only 1 broker would remain", the SCALE_DOWN note) are not sent back in the compound 200 body or the resilience report.
  • D6: the lease is keyed on the configured kates.chaos.kafka.namespace/cluster, as for plans, not on the spec's targetNamespace. Resilience runs aimed at other namespaces now take turns too.

Deliberately not done

  • A resilience run still gets no rollback and no check that the Kafka pods are Ready before its fault, and no maxAffectedBrokers. The docs say so.
  • Known window: a resilience fault whose chaosFuture.get times out (delayBeforeSec + chaosDurationSec + 120 s) keeps the lease until the run's finally, but litmus-crd may still be running the fault after that. Plans have the same window.
  • The checks before the stream and the lease at injection are not atomic. A run that loses the lease during steadyStateSec ends ERROR after its benchmark has run.
  • The lease lives in memory, so it holds only with one Kates API pod. The chart enforces that.
  • The "one-plan rule" wording at book 07:661 and 12-deployment.md:648 is left alone: it is out of scope, and fix(chaos): orphan recovery undoes every fault left from before the Kates API started, and NetworkPolicies in any namespace #374 edits next to it.
  • No href diff of the book render against main. The only structural change is a callout title, the callout has no id, and nothing links to it.

Landing

Lands after #373 (plan PR 4) and #374 (plan PR 5), which edit files this PR also edits. No Java file is shared.

Once each has merged, rebase this branch on main. Where a landed change moved lines in a Java file a caveat ref cites, re-point the ref from a diff line map of that file, then rerun cd cli && go test ./cmd -run TestMCPCaveat.

… guard and the disruption lease

A compound run and a resilience run called the chaos providers past the
safety guard and past the lease that keeps plans one at a time. A
chaos:run caller could post a compound POD_KILL with targetAll, which on
the kubernetes provider force-deletes every broker and KRaft controller
at once, while the same fault as a plan step is refused with "Plan would
affect ALL 3 brokers". A CI resilience run could kill broker 1 while a
leader-cascade plan killed broker 0, leaving two of three brokers down
under min.insync.replicas=2, and either one's cleanup or rollback undid
the other's fault.

POST /api/disruptions/compound now checks the chaos limits first, so a
fault with no spec is still refused by its place, then counts its faults
as the steps of a plan, each without its targetTopic: a compound fault
is never aimed at a topic's leader, and the guard would count the leader
instead. A plan the guard refuses is a 400, "No fault was triggered:"
and its reasons. The run then takes the lease, or answers 409 while
another disruption holds it, and gives it back when its faults end. A
concurrent run waits for each fault at least delayBeforeSec +
chaosDurationSec + 120 s, as a sequential run does: with only
timeoutSec, 120 s by default, a longer fault was reported timed out and
the lease went back while the fault still ran.

POST /api/resilience and /api/resilience/scenarios/{id} ask the same
guard of the fault, as a one-step plan, after their other checks and
before the stream starts: a 409 while the cluster is taken, a 400 when
the guard refuses. The run itself takes the lease just before its fault
goes in and holds it through the recovery wait and the report; one that
finds the cluster taken then ends ERROR with no fault injected. A
resilience run still gets no rollback and no check of the Kafka pods
before its fault.

The lease's 409 and REJECTED messages no longer say the holder is a
plan, and the DisruptionOrchestrator javadoc no longer says the lease
cannot be bypassed. Book chapters 01, 02, 05, 07, 10, 11, the
glossary, Part III's intro and the stretched-cluster guide say what a
resilience run now gets. The guide's A6 site-B transient hits every
broker, so the guard refuses it as a resilience run too, and the guide
now has it run outside Kates with kubectl. Four anchored caveat refs
are re-pointed; no caveat text changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant