From 156a36f035e14eb32afb4801cd46c5409d649b08 Mon Sep 17 00:00:00 2001 From: ianp94 Date: Mon, 20 Jul 2026 17:25:36 -0400 Subject: [PATCH 1/2] load(PR1): persist the interesting corpus as a campaign-owned ConfigMap (DD-026) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Producer half of load mode. An explore run's interesting inputs (the "replay corpus") are now persisted so they can be replayed under load (PR 2), viewed on the dashboard, and re-run for reproducibility. Transport: reuse the termination-message channel, NOT a sidecar (DD-026 §3 flagged the sidecar + a new driver-pod identity as the costly path). The driver splices a byte-capped (~3 KB, ≈top-N) replayCorpus array into the summary it already writes to /dev/termination-log; the operator — which now holds configmaps create;update — reads it back and materializes a campaign-owned -corpus-out ConfigMap, setting status.corpusConfigMap. The driver stays credential-less. The cap is also the right load-replay semantics (hammer the best states); a bigger transport is a future extension. - runner: CoverageGuidedRun publishes its live corpus + splices replayCorpus into writeSummary; replayCorpusJson (dedup + JSON-escape + byte budget) unit tested. - operator: driverSummary.ReplayCorpus; emitCorpusConfigMap (create-or-update, owner-ref'd); status.corpusConfigMap; configmaps create;update;patch RBAC (regenerated role + synced chart RBAC/CRD). envtest spec added. - e2e: assert a completed campaign emits status.corpusConfigMap with route entries, owned by the campaign. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JuGqspSB9zpA5AyvHJBiDK --- deploy/e2e/e2e.sh | 12 +++ .../closurejvm.dev_closurejvmcampaigns.yaml | 6 ++ .../closurejvm-operator/templates/rbac.yaml | 2 +- docs/LOAD-MODE-DESIGN.md | 11 ++- .../api/v1alpha1/closurejvmcampaign_types.go | 5 ++ .../closurejvm.dev_closurejvmcampaigns.yaml | 6 ++ operator/config/rbac/role.yaml | 3 + .../closurejvmcampaign_controller.go | 52 +++++++++++- .../closurejvmcampaign_controller_test.go | 39 +++++++++ runner/coverage/CoverageGuidedRun.java | 80 ++++++++++++++++++- .../runner/coverage/ReplayCorpusJsonTest.java | 53 ++++++++++++ 11 files changed, 260 insertions(+), 9 deletions(-) create mode 100644 test/runner/coverage/ReplayCorpusJsonTest.java diff --git a/deploy/e2e/e2e.sh b/deploy/e2e/e2e.sh index 4a18497..3c8297b 100644 --- a/deploy/e2e/e2e.sh +++ b/deploy/e2e/e2e.sh @@ -320,6 +320,18 @@ YAML echo " (campaign coveragePct=${cpct:-}; $(grep -c 'via corpusDir' "$dlog") value-file(s) resolved from corpus)" rm -f "$dlog" + # --- DD-026 PR 1: the run emits its interesting "replay corpus" as a campaign-owned ConfigMap --- + ccorpus="$($K -n "$NS" get closurejvmcampaign jpetstore-campaign -o jsonpath='{.status.corpusConfigMap}' 2>/dev/null || true)" + ccount=0; cowner2="" + if [ -n "$ccorpus" ]; then + ccount="$($K -n "$NS" get configmap "$ccorpus" -o jsonpath='{.data.corpus\.txt}' 2>/dev/null | grep -c '^/' || true)" + cowner2="$($K -n "$NS" get configmap "$ccorpus" -o jsonpath='{.metadata.ownerReferences[0].kind}' 2>/dev/null || true)" + fi + check "campaign emitted status.corpusConfigMap" "echo '$ccorpus' | grep -q 'jpetstore-campaign-corpus-out'" + check "replay-corpus ConfigMap has route entries" "[ '${ccount:-0}' -ge 1 ]" + check "replay-corpus ConfigMap owned by the campaign (GC)" "[ '$cowner2' = 'ClosureJVMCampaign' ]" + echo " (corpusConfigMap=${ccorpus:-}; ${ccount} route(s))" + # --- P5b: the operator brought up a per-campaign dashboard and the driver pushed to it --------- say "Assert the per-campaign dashboard (P5b)" durl="$($K -n "$NS" get closurejvmcampaign jpetstore-campaign -o jsonpath='{.status.dashboardURL}' 2>/dev/null || true)" diff --git a/deploy/helm/closurejvm-operator/crds/closurejvm.dev_closurejvmcampaigns.yaml b/deploy/helm/closurejvm-operator/crds/closurejvm.dev_closurejvmcampaigns.yaml index 5203d56..1214e7b 100644 --- a/deploy/helm/closurejvm-operator/crds/closurejvm.dev_closurejvmcampaigns.yaml +++ b/deploy/helm/closurejvm-operator/crds/closurejvm.dev_closurejvmcampaigns.yaml @@ -229,6 +229,12 @@ spec: x-kubernetes-list-map-keys: - type x-kubernetes-list-type: map + corpusConfigMap: + description: |- + CorpusConfigMap names a ConfigMap the operator emits at end-of-run holding the run's interesting + "replay corpus" (the inputs that reached new coverage), for reproducibility, the dashboard corpus + view, and load-mode replay (DD-026 PR 1). Owner-referenced to the campaign, so it GCs with it. + type: string coveragePct: description: CoveragePct/Findings are read from the driver's end-of-run summary (DD-025 §7a). diff --git a/deploy/helm/closurejvm-operator/templates/rbac.yaml b/deploy/helm/closurejvm-operator/templates/rbac.yaml index 0d40d10..836c0fb 100644 --- a/deploy/helm/closurejvm-operator/templates/rbac.yaml +++ b/deploy/helm/closurejvm-operator/templates/rbac.yaml @@ -20,7 +20,7 @@ rules: verbs: ["create", "delete", "get", "list", "watch"] - apiGroups: [""] resources: ["configmaps"] - verbs: ["get", "list", "watch"] + verbs: ["get", "list", "watch", "create", "update", "patch"] - apiGroups: [""] resources: ["pods"] verbs: ["get", "list", "watch"] diff --git a/docs/LOAD-MODE-DESIGN.md b/docs/LOAD-MODE-DESIGN.md index 06261aa..87c0d70 100644 --- a/docs/LOAD-MODE-DESIGN.md +++ b/docs/LOAD-MODE-DESIGN.md @@ -162,9 +162,14 @@ status: ## 6. Phased plan - [x] **PR 0 — this design note (DD-026).** -- [ ] **PR 1 — producer.** Runner writes the interesting corpus out; operator captures it into a - campaign-owned ConfigMap and sets `status.corpusConfigMap`. Ships on its own (reproducibility + - dashboard corpus view). Adds no `load` behavior yet. +- [x] **PR 1 — producer.** **Chosen transport: reuse the termination-message channel, not a sidecar.** + The driver splices a byte-capped (~3 KB, ≈top-N) `replayCorpus` array into the summary it already + writes to `/dev/termination-log`; the operator (which now has `configmaps: create;update`) reads it + back and materializes a campaign-owned `-corpus-out` ConfigMap, setting + `status.corpusConfigMap`. This keeps the driver **credential-less** (no new pod SA/identity — the + costly path §3 flagged) and the top-N cap is the right load-replay semantics anyway. Trade-off: the + emitted corpus is bounded to what fits the ~4 KiB termination message; a larger transport + (sidecar/PVC) is a future extension if corpora need to be bigger. - [ ] **PR 2 — consumer.** `mode: load` + `driver.concurrency`; the load driver; `status.load` metrics + dashboard wiring. diff --git a/operator/api/v1alpha1/closurejvmcampaign_types.go b/operator/api/v1alpha1/closurejvmcampaign_types.go index 562f122..a40123e 100644 --- a/operator/api/v1alpha1/closurejvmcampaign_types.go +++ b/operator/api/v1alpha1/closurejvmcampaign_types.go @@ -119,6 +119,11 @@ type ClosureJVMCampaignStatus struct { CoveragePct string `json:"coveragePct,omitempty"` // +optional Findings int32 `json:"findings,omitempty"` + // CorpusConfigMap names a ConfigMap the operator emits at end-of-run holding the run's interesting + // "replay corpus" (the inputs that reached new coverage), for reproducibility, the dashboard corpus + // view, and load-mode replay (DD-026 PR 1). Owner-referenced to the campaign, so it GCs with it. + // +optional + CorpusConfigMap string `json:"corpusConfigMap,omitempty"` // +optional StartTime *metav1.Time `json:"startTime,omitempty"` // +optional diff --git a/operator/config/crd/bases/closurejvm.dev_closurejvmcampaigns.yaml b/operator/config/crd/bases/closurejvm.dev_closurejvmcampaigns.yaml index 5203d56..1214e7b 100644 --- a/operator/config/crd/bases/closurejvm.dev_closurejvmcampaigns.yaml +++ b/operator/config/crd/bases/closurejvm.dev_closurejvmcampaigns.yaml @@ -229,6 +229,12 @@ spec: x-kubernetes-list-map-keys: - type x-kubernetes-list-type: map + corpusConfigMap: + description: |- + CorpusConfigMap names a ConfigMap the operator emits at end-of-run holding the run's interesting + "replay corpus" (the inputs that reached new coverage), for reproducibility, the dashboard corpus + view, and load-mode replay (DD-026 PR 1). Owner-referenced to the campaign, so it GCs with it. + type: string coveragePct: description: CoveragePct/Findings are read from the driver's end-of-run summary (DD-025 §7a). diff --git a/operator/config/rbac/role.yaml b/operator/config/rbac/role.yaml index 63f5814..0f9677b 100644 --- a/operator/config/rbac/role.yaml +++ b/operator/config/rbac/role.yaml @@ -79,8 +79,11 @@ rules: resources: - configmaps verbs: + - create - get - list + - patch + - update - watch - apiGroups: - "" diff --git a/operator/internal/controller/closurejvmcampaign_controller.go b/operator/internal/controller/closurejvmcampaign_controller.go index 43d41d9..6c5a6bb 100644 --- a/operator/internal/controller/closurejvmcampaign_controller.go +++ b/operator/internal/controller/closurejvmcampaign_controller.go @@ -22,6 +22,7 @@ import ( "encoding/hex" "encoding/json" "fmt" + "strings" "time" appsv1 "k8s.io/api/apps/v1" @@ -62,7 +63,7 @@ type ClosureJVMCampaignReconciler struct { //+kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;delete //+kubebuilder:rbac:groups=core,resources=services,verbs=get;list;watch;create;delete //+kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;watch -//+kubebuilder:rbac:groups=core,resources=configmaps,verbs=get;list;watch +//+kubebuilder:rbac:groups=core,resources=configmaps,verbs=get;list;watch;create;update;patch func (r *ClosureJVMCampaignReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { l := log.FromContext(ctx) @@ -204,6 +205,15 @@ func (r *ClosureJVMCampaignReconciler) Reconcile(ctx context.Context, req ctrl.R if s := r.readDriverSummary(ctx, &campaign); s != nil { campaign.Status.CoveragePct = fmt.Sprintf("%.1f", s.Exploration.Coverage.Pct) campaign.Status.Findings = s.Exploration.Corpus + // Persist the interesting "replay corpus" as a campaign-owned ConfigMap (DD-026 PR 1) — + // for reproducibility, the dashboard corpus view, and load-mode replay. The driver stays + // credential-less: it wrote the corpus into its summary (termination message); the operator, + // which already holds the RBAC, materializes the ConfigMap here. + if len(s.ReplayCorpus) > 0 { + if err := r.emitCorpusConfigMap(ctx, &campaign, s.ReplayCorpus); err != nil { + l.Error(err, "failed to emit replay-corpus ConfigMap") // non-fatal: the run still Completed + } + } } now := metav1.Now() campaign.Status.CompletionTime = &now @@ -231,7 +241,7 @@ func (r *ClosureJVMCampaignReconciler) Reconcile(ctx context.Context, req ctrl.R return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } -// driverSummary is the subset of StatusReporter.snapshotJson the campaign surfaces. +// driverSummary is the subset of the driver's end-of-run summary the campaign surfaces. type driverSummary struct { Exploration struct { Corpus int32 `json:"corpus"` @@ -239,6 +249,44 @@ type driverSummary struct { Pct float64 `json:"pct"` } `json:"coverage"` } `json:"exploration"` + // ReplayCorpus is the capped set of interesting inputs the run fired (DD-026 PR 1); the operator + // materializes it into status.corpusConfigMap. + ReplayCorpus []string `json:"replayCorpus"` +} + +const corpusOutKey = "corpus.txt" + +func corpusConfigMapName(c *closurejvmv1alpha1.ClosureJVMCampaign) string { + return c.Name + "-corpus-out" +} + +// emitCorpusConfigMap materializes the run's replay corpus into a campaign-owned ConfigMap (one key, +// newline-joined) and records it in status.corpusConfigMap. Create-or-update (idempotent on requeue). +func (r *ClosureJVMCampaignReconciler) emitCorpusConfigMap(ctx context.Context, c *closurejvmv1alpha1.ClosureJVMCampaign, corpus []string) error { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: corpusConfigMapName(c), Namespace: c.Namespace}, + Data: map[string]string{corpusOutKey: strings.Join(corpus, "\n") + "\n"}, + } + if err := controllerutil.SetControllerReference(c, cm, r.Scheme); err != nil { + return err + } + var existing corev1.ConfigMap + switch err := r.Get(ctx, types.NamespacedName{Namespace: cm.Namespace, Name: cm.Name}, &existing); { + case apierrors.IsNotFound(err): + if cerr := r.Create(ctx, cm); cerr != nil && !apierrors.IsAlreadyExists(cerr) { + return cerr + } + case err != nil: + return err + default: + existing.Data = cm.Data + existing.OwnerReferences = cm.OwnerReferences + if uerr := r.Update(ctx, &existing); uerr != nil { + return uerr + } + } + c.Status.CorpusConfigMap = cm.Name + return nil } // readDriverSummary finds the driver Job's pod and parses the summary the runner wrote to its diff --git a/operator/internal/controller/closurejvmcampaign_controller_test.go b/operator/internal/controller/closurejvmcampaign_controller_test.go index b253778..0db784f 100644 --- a/operator/internal/controller/closurejvmcampaign_controller_test.go +++ b/operator/internal/controller/closurejvmcampaign_controller_test.go @@ -120,6 +120,7 @@ var _ = Describe("ClosureJVMCampaign Controller (P5a)", func() { // remove it explicitly to keep specs isolated. &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: campaignName + "-dashboard", Namespace: namespace}}, &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: campaignName + "-dashboard", Namespace: namespace}}, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: campaignName + "-corpus-out", Namespace: namespace}}, } { _ = k8sClient.Delete(ctx, o) } @@ -220,6 +221,44 @@ var _ = Describe("ClosureJVMCampaign Controller (P5a)", func() { Expect(got.Status.CompletionTime).NotTo(BeNil()) }) + It("emits the replay corpus as a campaign-owned ConfigMap on completion (DD-026 PR 1)", func() { + Expect(k8sClient.Create(ctx, newTargetDeploy())).To(Succeed()) + makeInjectedTarget() + Expect(k8sClient.Create(ctx, newCampaign())).To(Succeed()) + _, err := reconcileOnce() + Expect(err).NotTo(HaveOccurred()) + + job := &batchv1.Job{} + Expect(k8sClient.Get(ctx, jobKey, job)).To(Succeed()) + job.Status.Succeeded = 1 + Expect(k8sClient.Status().Update(ctx, job)).To(Succeed()) + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: campaignName + "-driver-c", Namespace: namespace, + Labels: map[string]string{"closurejvm.dev/campaign": campaignName}}, + Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "driver", Image: runnerImg}}}, + } + Expect(k8sClient.Create(ctx, pod)).To(Succeed()) + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: pod.Name, Namespace: namespace}, pod)).To(Succeed()) + pod.Status.ContainerStatuses = []corev1.ContainerStatus{{ + Name: "driver", + State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ + Message: `{"exploration":{"corpus":2,"coverage":{"pct":11.0}},"replayCorpus":["/actions/Catalog.action","/actions/Cart.action?add=1"]}`}}, + }} + Expect(k8sClient.Status().Update(ctx, pod)).To(Succeed()) + + _, err = reconcileOnce() + Expect(err).NotTo(HaveOccurred()) + + got := &closurejvmv1alpha1.ClosureJVMCampaign{} + Expect(k8sClient.Get(ctx, campaignKey, got)).To(Succeed()) + Expect(got.Status.CorpusConfigMap).To(Equal(campaignName + "-corpus-out")) + + cm := &corev1.ConfigMap{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: campaignName + "-corpus-out", Namespace: namespace}, cm)).To(Succeed()) + Expect(cm.Data["corpus.txt"]).To(Equal("/actions/Catalog.action\n/actions/Cart.action?add=1\n")) + Expect(cm.OwnerReferences).To(ContainElement(HaveField("Name", campaignName))) + }) + It("surfaces a failed initContainer's reason in campaign status (review #24)", func() { Expect(k8sClient.Create(ctx, newTargetDeploy())).To(Succeed()) makeInjectedTarget() diff --git a/runner/coverage/CoverageGuidedRun.java b/runner/coverage/CoverageGuidedRun.java index 175163d..d9d0494 100644 --- a/runner/coverage/CoverageGuidedRun.java +++ b/runner/coverage/CoverageGuidedRun.java @@ -139,6 +139,9 @@ public static void main(String[] args) throws Exception { // Start the corpus as the full seed set so every seeded endpoint is exercised, rather than // relying on random discovery to stumble onto them. List corpus = new ArrayList<>(seeds); + // Publish the live corpus so the end-of-run summary can emit a capped "replay corpus" the + // operator persists (DD-026 PR 1). Same list object, populated as the run finds new coverage. + lastCorpus = corpus; long best = 0, total = 0; // Alternate session epochs: sign on for a stretch (so account/cart/order handlers run @@ -236,16 +239,87 @@ static long parseDurationMillis(String s) { return (long) (Double.parseDouble(num.trim()) * mult); } - /** Write the end-of-run metrics (StatusReporter's snapshot JSON) to {@code path} for the operator. */ + /** The run's live corpus, published for the end-of-run summary's replay-corpus emission (DD-026). */ + private static volatile List lastCorpus; + + /** + * Max bytes of the emitted replay corpus. The summary is written to the pod's termination message + * (operator reads it back), which Kubernetes caps at ~4 KiB total; the metrics JSON is a few + * hundred bytes, so keep the corpus well under the remainder. This bounds the replay corpus to the + * top interesting inputs — which is the right load-replay semantics anyway (hammer the best states, + * not every input). Overridable for tests / a future larger-transport path. + */ + static final int REPLAY_CORPUS_MAX_BYTES = + Integer.getInteger("closurejvm.corpus.out.maxBytes", 3000); + + /** + * Write the end-of-run summary (StatusReporter's metrics JSON, plus a capped {@code replayCorpus} + * array of the interesting inputs) to {@code path} for the operator to read back (DD-025 §7a, + * DD-026 PR 1). + */ private static void writeSummary(String path) { try { - java.nio.file.Files.write(Paths.get(path), - StatusReporter.snapshotJson().getBytes(StandardCharsets.UTF_8)); + String snap = StatusReporter.snapshotJson(); // a complete JSON object: {...} + String corpusJson = replayCorpusJson(lastCorpus, REPLAY_CORPUS_MAX_BYTES); + // Splice "replayCorpus":[...] in before the closing brace; the operator ignores it if it + // doesn't parse the field, and parses it into status.corpusConfigMap if it does. + String merged = snap.endsWith("}") + ? snap.substring(0, snap.length() - 1) + ",\"replayCorpus\":" + corpusJson + "}" + : snap; + java.nio.file.Files.write(Paths.get(path), merged.getBytes(StandardCharsets.UTF_8)); } catch (Exception ignored) { // never let summary-writing break the run's exit } } + /** + * A JSON array of distinct corpus inputs, added in order until the encoded size would exceed + * {@code maxBytes}. Package-private for testing. + */ + static String replayCorpusJson(List corpus, int maxBytes) { + StringBuilder sb = new StringBuilder("["); + if (corpus != null) { + java.util.LinkedHashSet seen = new java.util.LinkedHashSet<>(corpus); + boolean first = true; + for (String entry : seen) { + String enc = jsonString(entry); + // +1 for a leading comma once we're past the first element. + int projected = sb.length() + enc.length() + (first ? 0 : 1) + 1 /* closing ] */; + if (projected > maxBytes) { + break; + } + if (!first) { + sb.append(','); + } + sb.append(enc); + first = false; + } + } + return sb.append(']').toString(); + } + + /** Minimal JSON string encoder (quotes + escapes) — avoids a JSON dependency for one field. */ + private static String jsonString(String s) { + StringBuilder b = new StringBuilder("\""); + for (int i = 0; i < s.length(); i++) { + char c = s.charAt(i); + switch (c) { + case '"': b.append("\\\""); break; + case '\\': b.append("\\\\"); break; + case '\n': b.append("\\n"); break; + case '\r': b.append("\\r"); break; + case '\t': b.append("\\t"); break; + default: + if (c < 0x20) { + b.append(String.format("\\u%04x", (int) c)); + } else { + b.append(c); + } + } + } + return b.append('"').toString(); + } + /** * Cookie jar for the current session "epoch". JPetStore keeps the signed-on account in the * HTTP session, so without carrying JSESSIONID every authenticated handler sees a null account diff --git a/test/runner/coverage/ReplayCorpusJsonTest.java b/test/runner/coverage/ReplayCorpusJsonTest.java new file mode 100644 index 0000000..81d6312 --- /dev/null +++ b/test/runner/coverage/ReplayCorpusJsonTest.java @@ -0,0 +1,53 @@ +package runner.coverage; + +import org.junit.Test; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * The end-of-run "replay corpus" is spliced into the summary the operator reads back (DD-026 PR 1), + * which rides the pod termination message (~4 KiB cap). These pin the size-capping, dedup, and JSON + * escaping so a change can't silently blow the cap or emit malformed JSON. Package {@code runner.coverage} + * to reach the package-private method. + */ +public class ReplayCorpusJsonTest { + + @Test + public void emptyOrNullIsAnEmptyArray() { + assertEquals("[]", CoverageGuidedRun.replayCorpusJson(null, 3000)); + assertEquals("[]", CoverageGuidedRun.replayCorpusJson(Collections.emptyList(), 3000)); + } + + @Test + public void encodesAndDeduplicates() { + List corpus = Arrays.asList("/a", "/b", "/a"); + String json = CoverageGuidedRun.replayCorpusJson(corpus, 3000); + assertEquals("[\"/a\",\"/b\"]", json); + } + + @Test + public void escapesJsonMetacharacters() { + String json = CoverageGuidedRun.replayCorpusJson(Arrays.asList("/x?q=\"a\"\n"), 3000); + assertTrue(json, json.contains("\\\"a\\\"")); + assertTrue(json, json.contains("\\n")); + } + + @Test + public void capsAtTheByteBudget() { + // 200 distinct routes = ~4 KB of content; a 500-byte budget must truncate well under it. + java.util.List big = new java.util.ArrayList<>(); + for (int i = 0; i < 200; i++) { + big.add(String.format("/actions/x?id=%06d", i)); + } + String json = CoverageGuidedRun.replayCorpusJson(big, 500); + assertTrue("must stay within budget: " + json.length(), json.length() <= 500); + assertTrue("valid JSON array", json.startsWith("[") && json.endsWith("]")); + assertFalse("must have truncated, not emitted all 200", json.contains("000199")); + } +} From 45deb1ebb8241fa8016b51d2e98d6ebfceff7e94 Mon Sep 17 00:00:00 2001 From: ianp94 Date: Mon, 20 Jul 2026 17:36:42 -0400 Subject: [PATCH 2/2] load(PR1): fix corpus/shutdown race + dynamic byte budget; retry emit; trim RBAC (review #33) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four review fixes: - (race, blocking) The summary shutdown hook could iterate `corpus` while the main loop mutated it on an external SIGTERM — CME risk, and the broad catch in writeSummary would then drop the ENTIRE summary (metrics too). Synchronize the corpus add() sites, take a defensive copy under the same monitor, and split writeSummary's try/catch so a corpus failure never takes down the metrics. - (byte budget) Compute the corpus budget as (4 KiB − actual metrics size), not a fixed 3000, so a future metrics field can't push the merged write past the cap; count UTF-8 bytes, not UTF-16 chars. Added a combined-size test that writes a 500-entry corpus through the real snapshotJson and asserts < 4096. - (retryable) Emit the corpus BEFORE flipping to a terminal phase; on failure stay Running and retry (terminal campaigns never reconcile again, so the old path permanently forfeited the corpus on a transient error). - (RBAC) Drop the unused `patch` verb — emitCorpusConfigMap only Creates/Updates. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JuGqspSB9zpA5AyvHJBiDK --- .../closurejvm-operator/templates/rbac.yaml | 2 +- operator/config/rbac/role.yaml | 1 - .../closurejvmcampaign_controller.go | 16 +++-- runner/coverage/CoverageGuidedRun.java | 68 +++++++++++++------ .../runner/coverage/ReplayCorpusJsonTest.java | 23 +++++++ 5 files changed, 83 insertions(+), 27 deletions(-) diff --git a/deploy/helm/closurejvm-operator/templates/rbac.yaml b/deploy/helm/closurejvm-operator/templates/rbac.yaml index 836c0fb..44989a4 100644 --- a/deploy/helm/closurejvm-operator/templates/rbac.yaml +++ b/deploy/helm/closurejvm-operator/templates/rbac.yaml @@ -20,7 +20,7 @@ rules: verbs: ["create", "delete", "get", "list", "watch"] - apiGroups: [""] resources: ["configmaps"] - verbs: ["get", "list", "watch", "create", "update", "patch"] + verbs: ["get", "list", "watch", "create", "update"] - apiGroups: [""] resources: ["pods"] verbs: ["get", "list", "watch"] diff --git a/operator/config/rbac/role.yaml b/operator/config/rbac/role.yaml index 0f9677b..77d4b43 100644 --- a/operator/config/rbac/role.yaml +++ b/operator/config/rbac/role.yaml @@ -82,7 +82,6 @@ rules: - create - get - list - - patch - update - watch - apiGroups: diff --git a/operator/internal/controller/closurejvmcampaign_controller.go b/operator/internal/controller/closurejvmcampaign_controller.go index 6c5a6bb..607c06a 100644 --- a/operator/internal/controller/closurejvmcampaign_controller.go +++ b/operator/internal/controller/closurejvmcampaign_controller.go @@ -63,7 +63,7 @@ type ClosureJVMCampaignReconciler struct { //+kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;delete //+kubebuilder:rbac:groups=core,resources=services,verbs=get;list;watch;create;delete //+kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;watch -//+kubebuilder:rbac:groups=core,resources=configmaps,verbs=get;list;watch;create;update;patch +//+kubebuilder:rbac:groups=core,resources=configmaps,verbs=get;list;watch;create;update func (r *ClosureJVMCampaignReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { l := log.FromContext(ctx) @@ -201,20 +201,28 @@ func (r *ClosureJVMCampaignReconciler) Reconcile(ctx context.Context, req ctrl.R // --- aggregate the Job's outcome ----------------------------------------------------------- if job.Status.Succeeded > 0 { - campaign.Status.Phase = closurejvmv1alpha1.CampaignCompleted if s := r.readDriverSummary(ctx, &campaign); s != nil { campaign.Status.CoveragePct = fmt.Sprintf("%.1f", s.Exploration.Coverage.Pct) campaign.Status.Findings = s.Exploration.Corpus // Persist the interesting "replay corpus" as a campaign-owned ConfigMap (DD-026 PR 1) — // for reproducibility, the dashboard corpus view, and load-mode replay. The driver stays // credential-less: it wrote the corpus into its summary (termination message); the operator, - // which already holds the RBAC, materializes the ConfigMap here. + // which already holds the RBAC, materializes the ConfigMap here. Emit BEFORE flipping to a + // terminal phase: a terminal campaign never reconciles again (top-of-func guard), so a + // transient failure here would otherwise permanently forfeit the corpus. On failure, stay + // Running and retry next reconcile (the create-or-update is idempotent). if len(s.ReplayCorpus) > 0 { if err := r.emitCorpusConfigMap(ctx, &campaign, s.ReplayCorpus); err != nil { - l.Error(err, "failed to emit replay-corpus ConfigMap") // non-fatal: the run still Completed + l.Error(err, "emitting replay-corpus ConfigMap; will retry") + campaign.Status.Phase = closurejvmv1alpha1.CampaignRunning + if uerr := r.Status().Update(ctx, &campaign); uerr != nil { + return ctrl.Result{}, uerr + } + return ctrl.Result{RequeueAfter: 5 * time.Second}, nil } } } + campaign.Status.Phase = closurejvmv1alpha1.CampaignCompleted now := metav1.Now() campaign.Status.CompletionTime = &now meta.SetStatusCondition(&campaign.Status.Conditions, metav1.Condition{ diff --git a/runner/coverage/CoverageGuidedRun.java b/runner/coverage/CoverageGuidedRun.java index d9d0494..65da49f 100644 --- a/runner/coverage/CoverageGuidedRun.java +++ b/runner/coverage/CoverageGuidedRun.java @@ -177,7 +177,9 @@ public static void main(String[] args) throws Exception { total = lastCoverageTotal; // else a sequence-only run reports coverage=N/0 if (coveredAfterSeq > best) { best = coveredAfterSeq; - corpus.add(sequence.get(sequence.size() - 1)); + // Synchronized: the summary shutdown hook may snapshot this list concurrently on an + // external SIGTERM (kubectl delete pod / eviction) before the deadline is hit. + synchronized (corpus) { corpus.add(sequence.get(sequence.size() - 1)); } StatusReporter.recordSaved("Coverage"); } continue; @@ -215,7 +217,7 @@ public static void main(String[] args) throws Exception { total = lastCoverageTotal; if (covered > best) { best = covered; - corpus.add(input); + synchronized (corpus) { corpus.add(input); } // see the sequence branch above StatusReporter.recordSaved("Coverage"); } } @@ -240,29 +242,51 @@ static long parseDurationMillis(String s) { } /** The run's live corpus, published for the end-of-run summary's replay-corpus emission (DD-026). */ - private static volatile List lastCorpus; + static volatile List lastCorpus; // package-private for the combined-size test - /** - * Max bytes of the emitted replay corpus. The summary is written to the pod's termination message - * (operator reads it back), which Kubernetes caps at ~4 KiB total; the metrics JSON is a few - * hundred bytes, so keep the corpus well under the remainder. This bounds the replay corpus to the - * top interesting inputs — which is the right load-replay semantics anyway (hammer the best states, - * not every input). Overridable for tests / a future larger-transport path. - */ + /** Absolute upper bound on the replay-corpus bytes (also overridable); the effective budget is the + * smaller of this and what's left of the ~4 KiB termination message after the metrics JSON. */ static final int REPLAY_CORPUS_MAX_BYTES = Integer.getInteger("closurejvm.corpus.out.maxBytes", 3000); + /** Keep the whole summary (metrics + corpus) safely under kubelet's 4096-byte termination cap. */ + private static final int TERMINATION_MSG_BUDGET = 3900; + /** * Write the end-of-run summary (StatusReporter's metrics JSON, plus a capped {@code replayCorpus} * array of the interesting inputs) to {@code path} for the operator to read back (DD-025 §7a, - * DD-026 PR 1). + * DD-026 PR 1). The corpus is best-effort and is built in a separate try so a corpus failure (e.g. + * a shutdown-time hiccup) can never take down the metrics summary the operator depends on. */ - private static void writeSummary(String path) { + static void writeSummary(String path) { // package-private for testing + String snap; + try { + snap = StatusReporter.snapshotJson(); // a complete JSON object: {...} + } catch (Exception e) { + return; // no metrics to write + } + String corpusJson = "[]"; + try { + // Defensive copy under the list's own monitor — the main loop's synchronized add()s and this + // snapshot can't interleave, so no ConcurrentModificationException on a SIGTERM race. + List lc = lastCorpus; + List snapshot; + if (lc != null) { + synchronized (lc) { snapshot = new ArrayList<>(lc); } + } else { + snapshot = java.util.Collections.emptyList(); + } + // Budget the corpus to what fits the termination message alongside the actual metrics size. + int snapBytes = snap.getBytes(StandardCharsets.UTF_8).length; + int overhead = ",\"replayCorpus\":".length(); // the closing '}' is already counted in snap + int budget = Math.min(REPLAY_CORPUS_MAX_BYTES, TERMINATION_MSG_BUDGET - snapBytes - overhead); + corpusJson = replayCorpusJson(snapshot, Math.max(2, budget)); // >=2 so "[]" always fits + } catch (Exception ignored) { + corpusJson = "[]"; // corpus is best-effort; fall through and still write the metrics + } try { - String snap = StatusReporter.snapshotJson(); // a complete JSON object: {...} - String corpusJson = replayCorpusJson(lastCorpus, REPLAY_CORPUS_MAX_BYTES); - // Splice "replayCorpus":[...] in before the closing brace; the operator ignores it if it - // doesn't parse the field, and parses it into status.corpusConfigMap if it does. + // Splice "replayCorpus":[...] before the closing brace; the operator ignores the field if it + // doesn't parse it, and materializes status.corpusConfigMap if it does. String merged = snap.endsWith("}") ? snap.substring(0, snap.length() - 1) + ",\"replayCorpus\":" + corpusJson + "}" : snap; @@ -273,25 +297,27 @@ private static void writeSummary(String path) { } /** - * A JSON array of distinct corpus inputs, added in order until the encoded size would exceed - * {@code maxBytes}. Package-private for testing. + * A JSON array of distinct corpus inputs, added in order until the encoded UTF-8 size would exceed + * {@code maxBytes} (a true byte budget — the termination message is byte-limited). Package-private + * for testing. */ static String replayCorpusJson(List corpus, int maxBytes) { StringBuilder sb = new StringBuilder("["); + int bytes = 1 + 1; // '[' + ']' if (corpus != null) { java.util.LinkedHashSet seen = new java.util.LinkedHashSet<>(corpus); boolean first = true; for (String entry : seen) { String enc = jsonString(entry); - // +1 for a leading comma once we're past the first element. - int projected = sb.length() + enc.length() + (first ? 0 : 1) + 1 /* closing ] */; - if (projected > maxBytes) { + int encBytes = enc.getBytes(StandardCharsets.UTF_8).length + (first ? 0 : 1); // + comma + if (bytes + encBytes > maxBytes) { break; } if (!first) { sb.append(','); } sb.append(enc); + bytes += encBytes; first = false; } } diff --git a/test/runner/coverage/ReplayCorpusJsonTest.java b/test/runner/coverage/ReplayCorpusJsonTest.java index 81d6312..5fa573b 100644 --- a/test/runner/coverage/ReplayCorpusJsonTest.java +++ b/test/runner/coverage/ReplayCorpusJsonTest.java @@ -38,6 +38,29 @@ public void escapesJsonMetacharacters() { assertTrue(json, json.contains("\\n")); } + @Test + public void combinedSummaryStaysUnderTheTerminationCap() throws Exception { + // writeSummary splices the corpus into the real metrics JSON; the whole thing rides the pod + // termination message (kubelet caps it at 4096 bytes). A huge corpus must be budgeted down so + // the merged write stays under the cap — otherwise kubelet truncation makes it un-parseable and + // the operator loses BOTH the corpus and the coverage/findings summary. + java.util.List big = new java.util.ArrayList<>(); + for (int i = 0; i < 500; i++) { + big.add(String.format("/actions/Catalog.action?categoryId=CAT%04d&productId=FI-SW-%04d", i, i)); + } + CoverageGuidedRun.lastCorpus = big; + java.nio.file.Path tmp = java.nio.file.Files.createTempFile("summary", ".json"); + tmp.toFile().deleteOnExit(); + CoverageGuidedRun.writeSummary(tmp.toString()); + + byte[] written = java.nio.file.Files.readAllBytes(tmp); + assertTrue("summary must stay under kubelet's 4096-byte cap, was " + written.length, written.length < 4096); + String s = new String(written, java.nio.charset.StandardCharsets.UTF_8); + assertTrue("single JSON object", s.startsWith("{") && s.endsWith("}")); + assertTrue("carries the metrics", s.contains("\"exploration\":")); + assertTrue("carries the replay corpus", s.contains("\"replayCorpus\":[")); + } + @Test public void capsAtTheByteBudget() { // 200 distinct routes = ~4 KB of content; a 500-byte budget must truncate well under it.