operator(campaign): fail loud on empty extracted classes

[ianp94]

Addresses the follow-up from the #20 review: a driver run that extracts no .class files (the classic case: a war-only target image, whose WEB-INF/classes only materializes at runtime, not in the image layers — DD-025 §7b) currently lets the driver run and report a silent coveragePct=0, indistinguishable from a genuinely low run.

Change

A second initContainer verify-classes runs after extract-classes and fails the pod (→ BackoffLimit exhausted → campaign Failed) when no .class files landed in the extract dir, with a message pointing at spec.driver.classesPath and the war-only cause:

closurejvm: no .class files extracted into /closurejvm-classes — check spec.driver.classesPath
(war-only images expose classes only at runtime, not in the image)

It references only the fixed dest dir (campaignClassesDir) — no user input in the shell string, so the injection-safety property from #19 is preserved (unlike the free-form classesPath, which is still exec'd without a shell in extract-classes).

Scope note

This is the fail-loud half of the war-only follow-up. Full war-only support (extracting classes from ROOT.war/lib jars so such images work without the e2e's explode-the-war workaround) remains a separate, larger backlog item. With this, a war-only image fails clearly instead of silently — a strict improvement.

Validation

envtest 29/29 — the launch test now asserts both initContainers and that verify-classes runs sh -c checking campaignClassesDir for .class files. The e2e (which explodes the war) is unaffected: classes are present, so verify-classes passes.

@claude please review — focus on: (1) the verify-classes shell command correctness + injection-safety (only the fixed dest dir is interpolated; find ... -name '*.class' -print -quit); (2) whether failing the whole Job (generic DriverFailed campaign reason) is the right UX vs. a distinct status condition; (3) initContainer ordering / image choice (reuses the runner image).

🤖 Generated with Claude Code

[ianp94]

@claude please review. Focus: (1) verify-classes shell correctness + injection-safety (only the fixed dest dir interpolated; find -name '*.class' -print -quit); (2) failing the whole Job (generic DriverFailed reason) vs a distinct status condition for empty-classes; (3) initContainer ordering + reusing the runner image. envtest 29/29.

[claude]

Claude finished @ianp94's task in 2m 3s —— View job

---

Review

• Gather context (diff, related files, DD-025, prior PR operator(P5a): runner image + in-cluster campaign e2e #20/operator(P5a): ClosureJVMCampaign CRD + driver-Job reconciler #19 discussion)
• Review verify-classes shell command correctness + injection-safety
• Assess DriverFailed generic reason vs distinct status condition for empty-classes
• Review initContainer ordering + image choice
• Post final review

Reviewed operator/internal/controller/campaign_resources.go (lines 155-169) and the accompanying test.

1. verify-classes shell correctness + injection-safety ✅

The generated command:

if [ -z "$(find /closurejvm-classes -name '*.class' -print -quit 2>/dev/null)" ]; then
  echo 'closurejvm: no .class files extracted into /closurejvm-classes — check spec.driver.classesPath (war-only images expose classes only at runtime, not in the image)';
  exit 1;
fi

• campaignClassesDir is a Go const ("/closurejvm-classes", campaign_resources.go:42), never derived from spec.driver.classesPath or any other user input — the injection-safety property from operator(P5a): ClosureJVMCampaign CRD + driver-Job reconciler #19 holds. Good contrast with extract-classes, which correctly keeps classesPath out of a shell string entirely (cmd/exec, not sh -c).
• -print -quit is the right choice — stops at the first match instead of walking the whole tree, keeping this cheap even for large extracted trees.
• POSIX semantics check out: when the if condition is false (classes exist) and there's no else, sh -c exits with status of the last executed command — per POSIX, that's 0 when no branch body ran — so a successful extract passes through cleanly. Confirmed correct.
• 2>/dev/null on find correctly fails safe: if find errors for any reason (e.g. mount issue), $(...) is empty and the container fails loud rather than silently exiting 0.

No injection or correctness issues found here.

2. Generic DriverFailed vs a distinct condition — worth reconsidering

This is the one substantive gap. verify-classes's helpful message (pointing at spec.driver.classesPath, naming the war-only cause) only ever reaches echo — i.e. it lands in kubectl logs <pod> -c verify-classes, not in the CR. The reconcile path that observes the failed Job (closurejvmcampaign_controller.go:216-217) is:

if job.Status.Failed > 0 && jobBackoffExhausted(&job) {
    return r.fail(ctx, &campaign, "DriverFailed", "the driver Job failed")
}

This is the same generic reason/message used for every other driver failure mode (bad classesPath in extract-classes, a driver JVM crash, etc.) — verify-classes failing produces a campaign status indistinguishable from any other DriverFailed. Given the PR's stated goal is to fail loud, right now it's only loud to someone who thinks to kubectl logs the init container; kubectl get closurejvmcampaign / the dashboard still shows the same opaque message as before this PR.

Given readDriverSummary (line 240) already has a pattern for finding the driver Job's pod and reading a container's terminated state/message, a low-cost improvement would be: in the Failed branch, look up the pod's init container statuses, and if verify-classes (or any init container) terminated non-zero, surface its name/message in the condition — e.g. Reason: "ClassesExtractionFailed", Message pulled from the container's exit reason — rather than adding an entirely separate status field. That would preserve the actionable message (pointing at classesPath) all the way to campaign status instead of leaving it two kubectl hops away. Not a blocker, but worth a follow-up given this PR is specifically about making the failure legible.

3. initContainer ordering + image choice ✅

• Ordering is correct: extract-classes (index 0) runs first, verify-classes (index 1) second — Kubernetes runs initContainers sequentially and only proceeds to the next on success, so verify-classes never runs against a dir an incomplete/failed extract left in an inconsistent state.
• Reusing runnerImage (eclipse-temurin:17-jre, a full Debian/Ubuntu-based image — see deploy/runner-image/Dockerfile) for verify-classes is a reasonable pragmatic choice: sh and find are both present, and since the driver container already requires pulling this image onto the node, verify-classes adds no extra image pull. A minimal busybox/alpine image would start marginally faster but isn't worth the extra image reference for one cheap find.
• One minor, non-blocking note: with BackoffLimit: 2 (line 118), a systematically misconfigured classesPath (e.g. a genuinely war-only target) will now deterministically fail verify-classes on all 3 pod attempts before the Job/campaign is marked Failed — each attempt fails fast during init, so this is cheap, but it's 3 guaranteed-identical failures instead of failing after the first. Pre-existing behavior for any init failure, not new to this PR, but slightly more visible now that this failure mode is intentionally triggered.

Test coverage

The envtest update (closurejvmcampaign_controller_test.go:156-168) correctly asserts both initContainers exist in order, that verify-classes uses sh and that its command references both campaignClassesDir and .class. That's an appropriate level of assertion for envtest (no real pod execution) — the actual shell logic is exercised by the e2e run, which the PR notes still passes since it explodes the war before this init container runs.

Summary

No correctness or security issues — the shell interpolation is safe and the find/if logic is sound. The main thing worth a follow-up (possibly out of scope for this PR) is #2: right now the campaign's Ready condition doesn't distinguish "no classes extracted, check classesPath" from any other driver failure, so the loud failure is only loud in pod logs, not in campaign status.

[ianp94]

Great point — a fail-loud that's only loud in pod logs is half the win. Implemented your suggestion in `7fb91cc`:

• `verify-classes` now also writes its reason to `/dev/termination-log` (not just stdout), with `TerminationMessageReadFile`.
• On a backoff-exhausted Job, the reconciler looks up the driver pod's `InitContainerStatuses`, and if one terminated non-zero, surfaces its name + message as the campaign condition — `Reason=InitContainerFailed`, `Message="verify-classes: no .class files extracted … — check spec.driver.classesPath"` — falling back to `DriverFailed` for a driver-container crash. So `kubectl get closurejvmcampaign` / the dashboard now show the actionable reason, not the opaque one.
• envtest 30/30 (new spec asserts the verify-classes reason reaches `status.conditions`).

On your other notes: the `sh -c` correctness/injection-safety, ordering, and image reuse you confirmed — no changes there. The 3×-identical-fast-failure under `BackoffLimit` is pre-existing init behavior; cheap since it fails during init. Merging.