Repository navigation
mtcollins1 boot: every attempt persists a BMC SEL delta, BMC inventory and pre-OS console bundle - #12093
Conversation
…y and pre-OS console bundle Every terminal arm of mtcollins1_boot_wet now writes artifacts/mtcollins1-boot-diagnostics.txt (uploaded by the existing artifacts/mtcollins1-boot-* glob) through one conclude step: - SEL: an authenticated before/after `sel elist` (new diagnostic.ipmi.Tool SelListAuthenticated), delta through sel_stimulus_listener sel_read/sel_after, with the raw records verbatim. The reader now admits ipmitool's seventh "Reading" field on threshold rows, which this BMC emits and which refused the whole log before. - Inventory: new extdeps.bmc.redfish_memory_inventory (DMTF Memory, Processor and Resource schemas) plus readonly GETs in extdeps.bmc.http, credentialed through a per-attempt gunbc.auth.netrc_binding netrc. - Console: retention and a size bound on the SOL capture, the firmware statement, and a new typed receipt of the Ampere DRAM firmware block (gunbc.machine_intake_ampere_dram_console_observation, transcribed from run 35801475989): version, params versus defaults, and the per-socket trained-DIMM roster. Failed reads are typed BmcReadCause values with the endpoint and instant. Decisive findings are appended to a refusal reason; a success is never flipped. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
briansrls
left a comment
There was a problem hiding this comment.
SOURCE HOLD at exact head d1cdb7f.
The conclude funnel, typed read causes, SEL threshold-row repair, netrc cleanup reporting, raw evidence retention, and “diagnostics never flip success” boundary are good. Five walls remain:
-
The Ampere DRAM witness does not preserve the real console grammar. Committed Mt. Collins captures render
MCU param:on one line followed by indentedecc_en = ..., andDRAM populated DIMMs:on one line followed by separate indentedSK0 MC...rows. The parser and witness instead synthesizeMCU param: ecc_en = ...andDRAM populated DIMMs: SK0 ...as single lines. On the real transcript this parser reads zero DIMMs (and generally zero params), then falsely reports every expected socket empty. Build a stateful block reader over the actual header/body lines and use exact committed/live bytes as the positive witness; a synthesized joined line is not an inhabitance control. -
A parameter differing from the firmware-rendered default is an observation, not a decisive finding. The corpus already contains successful sparse boots where
mcu_enable_maskis non-default while both sockets train. Keep name/value/default in the console section, but do not route it throughampere_dram_findingsinto== decisive findings ==or append it to the refusal reason. No meaning or fault relation is established for eithermcu_enable_maskorecc_en. -
The Redfish collection reader is not complete under the Redfish collection contract. It ignores
Members@odata.countandMembers@odata.nextLink, and discards each member's@odata.idto reconstruct a URI from its last segment. Redfish permits partial collections, makes nextLink opaque, and tells clients not to assume member URIs. A first page can therefore become a false “BMC lists fewer members” diagnosis. Carry same-origin member resource refs, enumerate nextLink to exhaustion (or return a typed incomplete-collection cause), and reconcile the enumerated population withMembers@odata.countbefore producing count findings. -
The 300-second diagnostics allowance is not a hard bound over the modeled call graph. With the expected 32 Memory members and 2 Processor members, Systems + the two collections + their members is 37 curl calls; at the declared 30-second per-call maximum, the Memory chain alone can exceed 300 seconds. The collection size is currently uncapped, so the route is not bounded at all. Bound page/member populations before fetching and derive the workflow allowance from those bounds and the service timeouts, or place the whole diagnostic transaction under a typed outer deadline that still writes the bundle. A GitHub step kill before netrc shred/bundle write defeats the brief.
-
BmcSessionUnestablishedis intentionally ambiguous between unreachable and credential-refused, butbmc_read_cause_is_no_answerclassifies it as proof that the BMC did not answer. That reintroduces the guess the type deleted. Render it as ambiguous; emit “BMC did not answer” only when the aggregate causes establish that fact. Add a control with IPMI SessionUnestablished plus Redfish AuthRefused.
For the question in the PR: yes, mcu_enable_mask != printed default should be worded and placed as an observation only. Do not encode the stronger “effective controller mask” interpretation either; the roster correlation is evidence, but no cited authority establishes the parameter's semantics.
The absence of a live MegaRAC inventory fixture is acceptable once the schema route is structurally honest and fail-closed. The first live bundle can remain its inhabitance reading.
…ated exact-link Redfish, derived deadline, honest no-answer, ByteSize, typed attempt id - DRAM: stateful block reader over the firmware's real layout (headers and indented rows on separate lines, CR/NUL stripped); fixtures are verbatim bytes of runs 35801475989 and 35793279839. Params are observations, never findings. - Redfish: follow Members@odata.nextLink, reconcile against Members@odata.count, keep each exact same-origin @odata.id (one GetResourceByPath read), bound pages and members, refuse CollectionIncomplete. - Deadline: every read bounded (coreutils timeout for ipmitool, curl --max-time) and Redfish reads counted against a budget; the step allowance is derived from those bounds; a spent budget yields a partial bundle naming what was not reached. - "The BMC did not answer" only when every cause is a definitive unreachable/timeout. - Console size from stat(1) as ByteSize; missing run id is a typed AttemptIdentityUnavailable. - fleet-converge.yml regenerated through main_wet (43 -> 52). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
# Conflicts: # .github/workflows/fleet-converge.yml
|
Response to GitHub review 5285894704 (five walls) and dashboard review 70302 (two findings). Head (1) DRAM grammar. Rewritten as a stateful block reader.
(2) Params are observations only. Every name, value and default row is kept in the console section, marked "(differs from printed default)" where it does. A param is never a finding and never reaches the refusal. (3) Pagination.
(4) Derived deadline.
(5) No-answer honesty. "The BMC did not answer" now needs every cause to be a definitive (A, review 70302) ByteSize. The bound and both console variants carry (B, review 70302) Attempt identity. A missing or empty Evidence (claim_batch built from this tree, run on the merged head): DRAM 13/13, bundle 37/37, boot_run 16/16, pre_os_observer 17/17, pre_os_replay 16/16. Four production mutations, each red and then restored, as listed above. Still unverified: no live BMC read was made. The Redfish fixtures follow the DMTF schemas (see the declared-limit section in the PR body). |
briansrls
left a comment
There was a problem hiding this comment.
SOURCE HOLD at exact head 4f52682.
The five walls from review 5285894704 are substantially closed, as are review 70302's byte-size and attempt-identity findings: the real DRAM grammar is now exercised over captured bytes; params are observations only; Redfish follows exact same-origin links and reconciles count/nextLink; the request population is bounded; SessionUnestablished is no longer called proof of no answer; stat supplies ByteSize; and a missing run id is typed. Two fail-closed gaps remain.
- A PARTIAL DRAM BLOCK CAN STILL MINT AN EMPTY-SOCKET FINDING. AmpereDramReadState knows whether it is InRoster, but AmpereDramBlock carries neither “roster header observed” nor “roster closed.” closed_blocks appends the current block at EOF regardless of its subsection, and ampere_dram_findings treats every expected socket with zero parsed rows as established empty. Thus both of these can confidently report socket 1 empty even though the capture ended before that fact was available:
NOTICE: DRAM FW version ...\n MCU param: ...\n
and
DRAM populated DIMMs:\n
SK0 MC0 S0: ...\n
<EOF before the unindented line that closes the roster, and potentially before SK1 rows>
This matters specifically on timeout/SOL-loss arms, where the transcript can end mid-block. Carry roster standing explicitly (not observed | open/partial | closed), retain partial rows as observations, and allow “socket N printed no trained DIMM” only from a CLOSED roster. Required REDs: no roster header => no empty-socket finding; roster opened with SK0 then EOF => partial/unterminated, not SK1-empty; the complete real block still yields the SK1-empty finding.
- THE REDFISH COLLECTION IS COUNT-COMPLETE BUT NOT IDENTITY-COMPLETE. redfish_collection_assembly admits when count(member_links) equals Members@odata.count, but it does not require the exact member links to be unique and does not reconcile later pages' present Members@odata.count values with the first. The member folds likewise do not reject duplicate member Ids or bind a returned member identity to the link requested. A response with count=2 and links [A,A] therefore becomes CollectionComplete, reads A twice, and can appear as two inventory members while B is absent—exactly the false-short/false-complete class this repair is meant to remove.
Require a set-complete population before findings: unique exact @odata.id links; every present page count consistent with the declared total; and either response @odata.id joined to the requested link or, at minimum, unique member Ids in the assembled member population. Any duplicate or identity disagreement must be BmcCollectionIncomplete/BmcResponseMalformed and mint no inventory-count finding. Add REDs for duplicate links and two distinct links returning the same member identity.
The derived read budget, partial-bundle rendering, no-answer aggregate, SEL threshold-row repair, verdict-preservation boundary, and regenerated 52-minute workflow projection otherwise read coherently. The absence of a live MegaRAC inventory fixture remains an acceptable declared activation frontier once these two incomplete-observation cases refuse rather than manufacture inventory facts.
Non-blocking metadata: the PR body still describes the pre-repair 22/22 + 9/9 evidence and 43-minute/300-second shape; refresh it to the 37/37 + 13/13 exact-head ledger before landing.
briansrls
left a comment
There was a problem hiding this comment.
SOURCE HOLD at exact head 4f52682.
All five walls from review 5285894704 are closed: the DRAM parser now consumes the real block grammar and exact run bytes; parameter/default differences are observation-only; Redfish follows exact same-origin links and nextLink under page/member/read bounds; the workflow allowance is derived from those bounds; and SessionUnestablished no longer proves that the BMC did not answer. The ByteSize and attempt-identity findings are also closed. The reported mutation controls are appropriately discriminating.
One collection-completeness wall remains. redfish_collection_assembly establishes only count(member_links) == Members@odata.count; it does not establish that those links identify distinct members, and it consults only the first page's count. Thus this response is currently admitted complete:
- page 1: count=2, Members=[CPU0], nextLink=page2
- page 2: count=2 (or a contradictory count), Members=[CPU0], no nextLink
The fold returns two links, reads CPU0 twice, and processor_findings sees two Processor members—the census socket count—so an absent CPU1 can disappear from the diagnostics. The same defect can inflate the Memory population. This directly contradicts the bundle's completeness claim and its fail-closed first-live-reading boundary.
Require the assembled @odata.id population to be unique before CollectionComplete, and refuse duplicate member links with a located BmcCollectionIncomplete. Also reconcile every page's reported Members@odata.count with the first total (or explicitly refuse a missing/contradictory later count under the implementation's chosen strict subset), rather than discarding later page counts.
Required discriminating controls:
- two pages containing the same Processor URI with total=2 -> CollectionRefused, never two members;
- later page count disagrees with the first -> CollectionRefused;
- two distinct Processor URIs with a stable total=2 -> CollectionComplete.
Non-source landing note: the PR body still carries the superseded 22/22, 9/9, 300-second/43-minute account; refresh it to the exact-head 37/37, 13/13, 830-second diagnostics allowance and 52-minute workflow projection after the source repair. GitHub currently reports no exact-head check runs for this merge head, so this review is source-scoped.
…Redfish collections are identity-complete - DRAM: AmpereDramBlock carries RosterNotObserved | RosterOpen | RosterClosed. An unindented line or a new block header closes a roster; the end of the capture never does. Only a closed roster may establish an untrained socket. - Redfish: member links must be unique, every page's Members@odata.count must equal the first, each member's returned @odata.id must equal its link, and returned Ids must be unique. Otherwise the section is unread (CollectionIncomplete / ResponseMalformed) and no count finding is minted. Member assembly is a pure fold over fetched bodies. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
# Conflicts: # .github/workflows/fleet-converge.yml
|
Response to review 5286526214 (confirmed by 5286530473). Head (1) A partial DRAM block no longer fabricates an empty socket.
(2) Redfish collections are now identity-complete as well as count-complete.
(3) PR body refreshed with the current evidence: the 830 s → 52 min derivation, all mutations, and the declared limit. Witness files (run locally with claim_batch):
CI on the previous head |
briansrls
left a comment
There was a problem hiding this comment.
SOURCE HOLD
Exact-head review of a08eec771e4825ff9e1247d444447734e8e9b9a0.
The two prior walls are closed at their stated grains.
- The DRAM block now carries
RosterNotObserved | RosterOpen | RosterClosed; end-of-capture preservesRosterOpen, and onlyRosterClosedcan mint an empty-socket finding. - Redfish collection pages now require one stable total, unique exact same-origin links, full count reconciliation, no pending nextLink, and bounded pages/members. Memory and Processor member bodies must return the requested
@odata.id, and theirIdpopulations must be unique.
One identity-completeness gap remains at the collection root.
The selected ComputerSystem resource is not joined to the Systems member link
read_inventory_with_netrc correctly requires the Systems collection to contain exactly one link, then GETs that link. But the resulting body is passed directly to redfish_system_inventory_links_of; unlike Memory and Processor member bodies, its returned @odata.id is never compared with the requested Systems link.
Counterexample:
Systems collection: [/redfish/v1/Systems/A]
GET /redfish/v1/Systems/A returns:
@odata.id = /redfish/v1/Systems/B
Memory/Processors links for B
All downstream collection/member identity controls can pass, while the bundle diagnoses a different ComputerSystem from the one the Systems collection selected. That contradicts the module's own “the system is read, not assumed” boundary and the PR's identity-complete claim.
Before consuming the system's Memory/Processors links, require:
redfish_resource_odata_id(system_body) == requested_system_link
Otherwise return BmcResponseMalformed/unread inventory and mint no inventory-count finding.
Required RED and control:
- one Systems link A, body answers
@odata.id=B-> unread/refused; - one Systems link A, body answers
@odata.id=A-> the existing inventory route remains complete.
After that source repair, rerun the 17 DRAM, 42 bundle and 16 boot-run claims on the committed merged head. The supplied receipts are honestly labeled pre-merge and therefore do not establish this exact SHA. At review time clippy was green while compiler, floor and emit-build were still running.
…d before its links are followed The system resource must return its requested link as its own @odata.id; otherwise the inventory is an unread BmcResponseMalformed and no count finding is minted (mtcollins1_system_inventory_links). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Response to review 5287148499. Head Fix. Right after the system GET,
Main had not moved since Evidence on the committed head. Each |
briansrls
left a comment
There was a problem hiding this comment.
APPROVE at exact head 058f260.
This supersedes REQUEST_CHANGES review 5287148499. The remaining Systems-identity wall is closed at the correct seam: immediately after GET of the sole Systems member and before its Memory/Processors links are consumed, mtcollins1_system_inventory_links requires the response body's own @odata.id to equal the exact requested Systems link. Missing/malformed or mismatched identity returns ObservationRefused, which read_inventory_with_netrc converts to BmcResponseMalformed through inventory_unread; therefore neither downstream collection is followed and no inventory-count finding can be minted about another ComputerSystem.
The delta from a08eec7 is one semantic commit touching only the diagnostic bundle and its witness. The RED uses requested A with returned B; the positive control uses A/A and retains the exact Memory and Processors links. The reported M11 mutation (forcing the join true) discriminates the RED while leaving the control green.
I accept the clean committed-head claim_batch receipts reported for 058f260: DRAM 17/17, bundle 44/44, boot_run 16/16, pre_os_observer 17/17, and pre_os_replay 16/16. No remaining source finding in the reviewed diagnostics scope.
At review time compiler and clippy are green; floor and emit-build remain in progress. Land after the required exact-head checks finish green. The declared absence of a live MegaRAC Redfish fixture remains an activation/inhabitance frontier, not a source hold.
#12093 (boot diagnostic bundle) was built on the per-run approval flow. Resolved by porting its conclude/bundle structure onto the standing-ruling + unit-hold boot: every terminal arm still uploads a bundle, the before-SEL read is taken after the ruling confirms and before the hold, and the receipt names the standing ruling instead of a per-run escalation id. fleet-converge.yml regenerated from the merged authority. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Brief: node adhoc-bf82bd28-a15 (operator, 2026-09-23). A remote operator must be able to diagnose a failed or degraded mtcollins1 boot from the run's receipts alone. Run 35793279839 came up with socket 1 absent and the receipts said nothing about why.
What every attempt now writes
Every terminal arm of
mtcollins1_boot_wetwritesartifacts/mtcollins1-boot-diagnostics.txt, and the existingartifacts/mtcollins1-boot-*upload carries it. The arms are: accepted, refused, timeout, approval expired, pre-OS refusal, and the early refusals before the credential is read.mtcollins1_boot_conclude. The attempt returnsMtCollins1BootAttempt, so an early refusal writes the BMC sections asNOT TAKEN: <why>.attempt=,diagnostics=andfindings=lines.AttemptIdentityUnavailable, and an unfiled escalation readsnot-filed (...); neither is ever a blank.beforesnapshot is taken after the credential read and before the gate acts, and anaftersnapshot at the end.diagnostic.ipmi.Tool SelListAuthenticated, under coreutilstimeout --kill-afterin the argv shape ofextdeps.tools.gnu_coreutils timeout_command; exit 124 is classified asBmcTimeout.sel_stimulus_listenersel_read/sel_after, covering standard, firmware-progress andOemSelPayloadrecords. OEM records stay opaque and counted, and the raw rows are kept verbatim.extdeps.bmc.redfish_memory_inventoryis built on the DMTF Memory, Processor and Resource schemas, with the real property names. It adds collection pages, the system's inventory links and a resource's own@odata.id.redfish.Http GetResourceByPath, which follows the exact same-origin@odata.id, with curl--max-time. Credentials come from a per-attemptgunbc.auth.netrc_bindingnetrc, shredded on every path.Members@odata.nextLinkup to 8 pages.Members@odata.countmust equal the first page's.@odata.idmust equal the link that fetched it, and returnedIds must be unique.BmcCollectionIncompleteorBmcResponseMalformed, and the section is unread, so no inventory-count finding can be minted from it.stat -c %s(newcoreutils.Stat PathSizeop) as aByteSizeagainst a 16 MiB bound, taken before the capture is loaded.String.length()counts Unicode scalars, not bytes.firmware_console_reportreports the firmware statement.gunbc.machine_intake_ampere_dram_console_observation) is a stateful block reader over the firmware's real layout, transcribed from the bytes of runs 35801475989 and 35793279839. It reads the version, params as observations only, and the per-socket trained-DIMM roster.Failed reads. A failed read is a typed
BmcReadCausewith the endpoint and instant. "The BMC did not answer" is said only when every cause is a definitive unreachable or timeout. Otherwise each distinct kind is named, e.g. an IPMI session failure (unreachable and credential-refused are indistinguishable) beside a Redfish connect failure.The bundle never flips the verdict. Findings are only appended to a refusal reason.
Deadline, derived
Every read has a transport bound, and Redfish reads are counted against a budget of 48 (this board's census inventory is 38 reads). A read the budget can't pay for is refused before any request as
BmcDeadlineReached, which names what was not reached. The shred and the bundle write still run.mtcollins1_boot_diagnostics_allowance()= 2 × (20 s + 5 s kill-after) + 48 × 15 s + 60 s publish = 830 s. The step timeout is 2250 s + 830 s = 3080 s, rounded up to 52 min.fleet-converge.ymlwas regenerated throughgenerated_artifact_gate main_wet --wet, on the merged tree after main's concurrent edit, which the generated-artifact merge driver correctly refused to resolve. The committed file is the generator's output, and it differs from main by that line only.Evidence
All claims were run with
claim_batchbuilt from this tree; CI runs no claims.ampere_dram_console_observation_witness_testmtcollins1_boot_diagnostic_bundle_witness_testmtcollins1_boot_run_witness_testpre_os_observer_witness_test(unchanged consumer of the SEL reader)pre_os_replay_witness_test(unchanged consumer of the SEL reader)gh run download <run> -n mtcollins1-boot-receipts, with the line range and sha256 of the excerpt and the file in each comment.CP: 3ff00100, and SK1 is empty in both.CP:line areRosterOpenwith no empty-socket finding.RosterNotObservedwith no finding.Id;//link.Production mutations, each run, turned red, and restored:
a_capture_cut_mid_roster_is_open_and_mints_no_empty_socketturns FAIL.@odata.idjoin removed → FAIL.Idcheck removed → FAIL.BmcSessionUnestablishedcounted as no-answer → FAIL.Declared limit: no live inhabitance reading yet
The Redfish Memory, Processors and Systems fixtures follow the DMTF schemas, not responses observed from mtcollins1's MegaRAC. No such response is in the corpus, and this lane was not allowed to touch the BMC. The first live
mtcollins1_bootattempt is the inhabitance reading. Its diagnostics file will show which properties the firmware populates, how it names members, whether it pages, and whether empty slots areState=Absentmembers or omitted. Anything this model misreads appears there as a typed cause, never as a silently missing section.Other frontiers
4f52682e13(run 35813304590): clippy, compiler, emit-build, floor and witnesses all passed. CI on the new head is pending.🤖 Generated with Claude Code