From 19104d315b5f89f1d7abd754dc31a2e85181c1a6 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Tue, 18 Aug 2026 13:50:37 +0000 Subject: [PATCH] test(resource-value-verification): full live e2e validation run (task 8.1) Validation-only, no code changes. Confirmed the complete spec end-to-end against a freshly built controller+scheduler+apiserver: default 9-resource set + one deliberately-unregistered name -> VERDICT resources=10 rejected=1 result=FAIL (exit 1), attributable specifically to the rejected name, not a false PASS; positive control (defaults only) -> VERDICT resources=9 rejected=0 result=PASS (exit 0). All 27 individual layer-checks (9 resources x 3 layers) MATCH; raw scheduler.log/apiserver.log trace lines independently verified against tasks 4.1/4.2's documented format; rejection confirmed non-silent across 4 independent log sources; cleanup and log preservation confirmed. Surfaced (but correctly did not fix, out of boundary) bug-1583: the apiserver stores resource-location updates with zero name validation, even for names the controller/scheduler both reject. Confirmed this does not affect this feature's own correctness - accept/reject is determined entirely by cmd_resupdate()'s controller-side follow-up query, before any layer is checked. Co-Authored-By: Claude Sonnet 5 --- .kiro/specs/resource-value-verification/tasks.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.kiro/specs/resource-value-verification/tasks.md b/.kiro/specs/resource-value-verification/tasks.md index 14b0b46e..d7bdfe0a 100644 --- a/.kiro/specs/resource-value-verification/tasks.md +++ b/.kiro/specs/resource-value-verification/tasks.md @@ -103,7 +103,7 @@ - _Depends: 6.2, 6.3, 6.4_ - [ ] 8. Validation: end-to-end live verification runs -- [ ] 8.1 Run the full flow live, including one deliberately-unregistered resource name +- [x] 8.1 Run the full flow live, including one deliberately-unregistered resource name - Execute the skill end-to-end against a freshly deployed controller/scheduler/apiserver, using the default resource set (task 2.1's previously-missing names) plus one resource name that does not exist in the cluster's registered set - Observable completion: every genuinely-registered resource individually reports matching values at all three layers (confirming tasks 4.1/4.2's trace-log lines are correctly formatted and greppable, not just present), AND the unregistered name is reported as a clear, specifically-identified rejection (not a silent omission) with correct per-name accept/reject accounting across the whole batch; per design.md's Error Handling (an unrecognized name is a run-level failure), the run's single overall VERDICT is FAIL, attributable specifically to the unregistered name — not a false overall PASS - _Requirements: 1.1, 1.2, 1.3, 1.4, 2.1, 2.2, 3.1, 3.2, 4.1, 4.2, 4.3, 5.1, 5.2, 5.3, 5.4, 6.1, 6.2, 6.4_ @@ -116,6 +116,7 @@ - _Depends: 7.1_ ## Implementation Notes +- Task 8.1 (validation-only, no code changes) confirmed the full spec end-to-end live: `VERDICT resources=10 layers_checked=3 mismatches=0 unreachable=0 rejected=1 result=FAIL` (default 9 + 1 fake name) and `VERDICT resources=9 ... rejected=0 result=PASS` (defaults only, positive control). Found (but correctly did not fix, out of boundary) **bug-1583**: the apiserver's `OnResourceLocationUpdated` performs zero name validation and stores ANY resource-location update pushed to it — including a genuinely-unregistered name the controller/scheduler both reject — because `do_update_resvals()` fires the `HOOK_DETAIL_RESOURCE_UPDATE` hook with the raw, unfiltered submitted list; the scheduler happens to independently re-validate and reject, the apiserver does not. Confirmed this does NOT affect this feature's own correctness (accept/reject is determined entirely by `cmd_resupdate()`'s own controller-side follow-up query, before any layer is checked) — but it's a real, previously-undocumented data-integrity gap in production code, worth a dedicated follow-up outside this feature. - Task 7.1 (round 1 REJECTED, round 2 fixed): `aggregate_verdict()`/`build_verdict_record()` (VERDICT_FIELD_SEP=`$'\x1f'`, ASCII Unit Separator — bash's IFS tab-splitting silently collapses adjacent empty fields even with IFS set to only a tab, bug-1581) are the self-contained, synthetic-testable comparison step task 8.2 needs. `main()`'s live-data wiring is `build_verdict_records_from_tsv()` — derive a layer's "ok" status from `SCHEDULER_CHECK_RESULTS[$name]`/`APISERVER_CHECK_RESULTS[$name]`'s STRING CONTENT (`UNREACHABLE:*` prefix check), never from array-key existence — tasks 6.3/6.4 always set the key, even on a genuine timeout (just with an empty value), so key-existence can never distinguish UNREACHABLE from MISMATCH (bug-1582). `CONTROLLER_OBSERVED`'s key-existence check IS correct for the controller layer specifically (task 6.2 has no separate UNREACHABLE concept — a query failure there aborts the whole run). VERDICT line format: `[xce-test-resources] VERDICT resources= layers_checked=3 mismatches= unreachable= rejected= result=PASS|FAIL` — `rejected=` is an intentional addition beyond design.md's illustrative template (Requirement 6.3 needs rejected names attributable in the machine-parseable line itself); `layers_checked=3` is a fixed constant, not a per-run computed count. SKILL.md fully rewritten to describe the now-complete, functional tool. - Task 6.4: `poll_apiserver_log()`/`query_apiserver_resvals()` mirror task 6.3's scheduler-check pattern exactly (same bounded-wait/batched-polling shape). The apiserver's zap `console` encoder renders structured fields as a trailing JSON object (`resource-location stored {"resource": "r15m", "host": "...", "value": 1.5}`), not classic `key=value` text — grep for the message string, then parse the trailing JSON block. All three layers (controller/scheduler/apiserver) confirmed simultaneously MATCH on the full default set including `cpuf`, proving the bug-1579 fix holds end-to-end. - **bug-1579 is now FULLY fixed** (retrofit on top of tasks 6.1/6.2, not a numbered task — user-requested follow-up). `inject_resources()`'s worker-context `xce_test` script now inserts a `!` line between `resupdate` and `quit`: `! bash