Repository navigation
mtcollins1 boot: select the §28.3 chassis action from observed power state, and read the transition back - #12247
Conversation
…ower state and read the transition back A cycle on an off host is a no-op per IPMI v2.0 §28.3; the Mt. Collins MegaRAC exited 0 and did nothing (run 36023602469), and BootControlIssued was minted on that exit alone. The handoff now reads §28.2 chassis status first (off -> power up, on -> the host's running-restart verb, unread/undecodable -> refuse before any verb), then bounded-polls chassis status until the transition is observed (cycle needs off then on), else PowerCommandAcceptedNoTransition. - extdeps.bmc.ipmi_chassis_control: typed IpmiChassisControlAction and the one System Power decoder / ChassisPowerObservation (moved from the fan lane). - All callers migrated to the typed action; no string power action remains. - Read-back allowance joins the boot-step timeout (57 -> 60 min). - mtcollins1: PSU2 AC loss carries the 2026-09-24 operator report (PSU1 only). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Regenerated with |
…h pattern selection) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ance joins the boot step timeout Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Merged main (#12101) and resolved the conflicts:
Regenerated all generated artifacts with Run locally (claim_batch --wet, head-built), 64 of 64 PASS:
— sent from fierce-cat-441 |
briansrls
left a comment
There was a problem hiding this comment.
SOURCE HOLD at exact head 57e74a1a3c944820510652d0203d5afc43eba38c (the requested f33687fa042ff8caf04fe83d946ca807eb72612b was superseded by a main merge and regenerated workflow).
The incident repair is correctly aimed: the Mt. Collins row is cycle; an OFF pre-read selects on; an ON pre-read selects the row's restart action; unreadable/undecodable state refuses before a power write; and success is no longer derived from the ChassisPowerControl exit status alone. Moving the §28.2 decoder to one extdeps authority, deriving the readback allowance, regenerating the workflow, and typing the PSU2 report are all accepted.
Two source seams remain.
1. running_restart still admits actions that are not a running restart
oob_power_action_selection accepts running_restart: IpmiChassisControlAction, the full four-arm §28.3 type. On an already-ON host it passes that value through unchanged. This makes three invalid paths writable:
running_restart = IpmiChassisPowerUp:chassis power oncan exit 0 on an already-on host; the first readback is still ON,ExpectPowerOnimmediately succeeds, and the handoff mintsBootControlIssued { power: PoweredOn }although no restart or state transition occurred and the one-shot override may remain unconsumed. This recreates the exact accepted-command/pre-existing-state class the PR is meant to eliminate.running_restart = IpmiChassisPowerDown: an OFF readback mintsBootControlIssued { power: PoweredOff }, so a boot handoff can report success while deliberately leaving the host off.running_restart = IpmiChassisHardReset: the code knows power state cannot witness this action, but sends the reset anyway and only then spends the bounded poll returning failure. An effect known to be unverifiable should refuse before the write.
The PR body says the success carrier is PoweredOn | PowerCycled, but the source also exposes PoweredOff | HostReset.
Required repair: give the handoff a narrower running-restart type (currently only the observed PowerCycle policy is justified), or validate the broad action before any power write and refuse every non-restart/unwitnessable arm. PowerDown, already-on PowerUp, and HardReset must be unable to reach ChassisPowerControl through this boot-handoff route. Add discriminating controls for those three cases. The generic extdeps §28.3 action type may remain broad; the workflow policy type must not be.
2. Exhaustion erases a partially observed cycle
The poll carries ExpectPowerOffThenOn { off_seen }, but PowerReadbackExhausted retains only the last ChassisPowerObservation. Thus a sequence such as ON -> OFF -> unreadable... reaches exhaustion after having established the OFF half, yet becomes PowerCommandAcceptedNoTransition { observed_state: ChassisUnreadable }. The renderers then say no transition was observed. That is false: a transition to OFF was observed; only return to ON was unestablished.
This distinction is operationally load-bearing: "never left ON", "reached OFF but did not return ON", and "readback became unavailable" describe different chassis states and retry evidence. Carry the final expectation/progress into the exhausted outcome (or split the arms) and render it accurately. Add a control that observes OFF and then exhausts without ON, proving that the partial transition survives. Also correct the source comment claiming one-second spacing samples every spec-admitted off window: with a one-second sleep plus a status call budgeted at two seconds, a conforming one-second off interval can be missed; that conservative false refusal is acceptable, but it is not guaranteed coverage.
No objection remains to the current Mt. Collins cycle policy, the off->on selection, decoder move, timeout integration, or the accepted-command-without-readback refusal once these state-space holes are closed.
…ts progress (review 5310234917) - The power write takes OobPowerAction (OobPowerUp | OobPowerCycle), chosen by the §28.2 selection alone: up only from an OFF read, cycle only from an ON read. The caller-supplied running restart is removed; with one evidenced verb it carried no choice and let the broad §28.3 type through (already-on PowerUp, PowerDown, HardReset). Success carries only PoweredOn | PowerCycled. - An exhausted read-back names how far the transition got: never left on, reached off but not back on, never reached on, or read-back unavailable. The observed OFF half is no longer lost. - The bound's comment states that a 1 s off window can fall between samples and that the resulting false refusal is accepted. - Controls: three compile-time refusals (DeclaredTypeNotInhabited) for PowerDown, HardReset and a §28.3 PowerUp at the route's action type, with a clean control; on-host selects cycle only; ON -> OFF -> unreadable exhausts as reached-off-not-returned. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
briansrls
left a comment
There was a problem hiding this comment.
HOLD — exact current head 5e0bee9, not the rollup's 6985674.
[P1] The carried decoder blocker is still present in extdeps.bmc.ipmi_chassis_control chassis_power_line. The fold replaces its accumulated answer whenever another System Power line appears. Thus System Power : on\nSystem Power : off becomes an admitted off reading and selects OobPowerUp; reversing the lines selects OobPowerCycle. Duplicate identical lines also pass, and System Power : on : garbage is accepted because only the second split field is inspected. A malformed external response is selecting an effect, not refusing.
Repair at this shared decoder: admit exactly one well-formed System Power field with exactly one recognized on/off value. Zero, multiple matching fields (including identical duplicates), conflicting values and malformed/trailing fields must yield ChassisStatusUndecodable. Preserve the offending evidence. Add REDs for both conflict orders, identical duplicates and malformed field cardinality, plus the ordinary on/off controls. The fan consumer and boot action selector must consume that same result, with no power write on undecodable input.
The narrowed OobPowerAction and admit_callers boundary are otherwise a substantial improvement: only the observed-state selector reaches power-up/cycle, and exhausted readback preserves an observed off half. I did not carry forward the obsolete concern about callers selecting power-down or hard reset; those constructors do not inhabit the new route.
Exact-head workflow 36228771813 is completed/success; green CI does not discriminate the malformed decoder population above. No hardware action was performed. Re-review the repaired head before queueing.
… state; ChassisStatus/ChassisBootParamGet bounded in place so the read-back budget bounds its reads (side chat at 6985674) - extdeps.bmc.ipmi_chassis_control: ChassisPowerLineRepeated -> ChassisStatusUndecodable. - extdeps.bmc.ipmi: ChassisStatus and ChassisBootParamGet run under coreutils timeout with a caller-supplied deadline/kill_after; no unbounded form. The handoff's read ceiling is derived from its own 5 s + 1 s bound; the fan series passes 20 s + 5 s. - Control: two System Power lines (agreeing or not) select no power action. - Regenerated (main_wet): boot step 37 -> 41 min, job 87 -> 91 min. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
briansrls
left a comment
There was a problem hiding this comment.
HOLD — exact head 2fc6622f25270672f6748501ad997b8a5077f11e.
The requested repairs are present and accepted: a second System Power line now reaches ChassisStatusUndecodable; ChassisStatus and ChassisBootParamGet have one bounded operation each with caller-supplied deadline/kill-after; the handoff and fan caller provide those bounds; and the derived workflow budget moves 37→41 minutes.
One carried decoder condition from the latest hold remains. chassis_power_line rejects fewer than two colon-separated fields, but then reads only fields.skip(1).first(). Therefore malformed output such as System Power : on : trailing still decodes as ON and selects OobPowerCycle; System Power : off : trailing decodes as OFF and selects OobPowerUp. A malformed external response is still able to select an effect.
Small required repair: admit only exactly one well-formed System Power key/value field with exactly the literal on or off; extra/trailing fields must produce an undecodable/malformed arm, carrying the offending line or fields for diagnosis. Add REDs for trailing-field ON and OFF, alongside the duplicate-line and ordinary ON/OFF controls. The duplicate-line construction and bounded-read changes need no redesign.
At review time clippy was green; compiler, floor, and emit-build were still in progress. This hold is source-scoped.
…alue; trailing fields are malformed (review 5326561042) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
briansrls
left a comment
There was a problem hiding this comment.
APPROVE-MERGE — exact head bd381207dfb8674acf8b125b43bb27796b6c4415.
The residual decoder blocker from review 5326561042 is closed at the shared §28.2 authority. A line whose trimmed key is System Power now admits a power state only when the colon split has exactly two fields and the sole value is literally on or off. Missing delimiters, trailing fields and unrecognized values cannot select a power action; the malformed arm retains the offending line. Multiple System Power lines remain undecodable rather than first/last-wins.
The new controls distinguish on : trailing, off : trailing, and the no-colon System Power line, while the ordinary on/off, repeated-line, narrowed-action and transition-readback controls remain. The in-place timeout bounds for ChassisStatus and ChassisBootParamGet are retained.
At review time clippy, compiler, and emit-build were completed/success; floor was still in progress. Queue once the exact-head required checks are green.
…2247; standing ruling removed from the hold acquire) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Defect. Run 36023602469 never powered mtcollins1 on. The host was off.
mtcollins1_power_action = "cycle"sentipmitool chassis power cycle. IPMI v2.0 §28.3 says a power cycle should do nothing on a host in S4/S5 (D5h recommended). The AMI MegaRAC exited 0 and did nothing, and the handoff mintedBootControlIssuedon that exit code alone.Fix at the authority (§6b).
extdeps.bmc.ipmi_chassis_control(new) models the §28.3 Chassis Control action as a typed choice:IpmiChassisPowerDown | PowerUp | PowerCycle | HardReset, plus its ipmitool verb. That extdeps type stays broad. The boot handoff's power write takes only the narrowerOobPowerAction = OobPowerUp | OobPowerCycle, so power-down, hard reset and an already-on power-up can't reach it on this route (review 5310234917).System Powerdecoder andChassisPowerObservation, moved out of the fan-observation module so the fan lane and the boot handoff share one decoder.oob_boot_handoffreads chassis status before choosing a verb:PowerStateReadFailed; no power command is sentPowerCommandAcceptedNoTransition { action, observed_state }. Exit 0 alone is never success.PowerStateReadFailed,PowerCommandRefused,PowerCommandAcceptedNoTransition. The success armBootControlIssuednow carriespower: PoweredOn | PowerCycled. It is still a control-plane fact, not proof of what booted.mtcollins1_actuate,mtcollins1_boot_authorization,host_reset_subject_rosterandhost_reset_return_run.cycle).ipmi.chassis.power.on-or-cycle.oob_power_readback_allowance()(182 s) joinsmtcollins1_boot_step_timeout_minutes. The generated step timeout goes 57 → 60 min.gunbc runrefused for lack of a cgroup memory limit. The arithmetic reproduces the old 57 min exactly (3380 s → 57; 3562 s → 60). The generated-artifact drift check on this PR is the confirmation.SupplyUncabledOperatorReportedcause: the 2026-09-24 operator report that only PSU1 is connected. That report explains the recurringPSU2_Status Power Supply AC lostSEL entries.Controls (
test.claim.machine_intake.oob_power_action_witness, 9 tests, all PASS on a remote claim_batch run):unknownpower state selects nothingfan_policy_standing_witness: 36/36 PASS after the decoder move.Not verified locally:
mtcollins1_boot_run_witness(only change: the covered sum includes the read-back allowance). It was OOM-killed (rc=137) during type-checking under a 12 GiB bound, and the larger runner had no writable cgroup, so it never ran. Leaving it to CI.Known risk: a cycle whose off window is shorter than one poll (about 1 s sleep plus one chassis-status call) would refuse as not observed. That failure is loud, not silent. §28.3 sets a power-off interval of at least 1 s for a cycle.
Split (approved by eager-owl-205). These go in a follow-up after #12101 lands, because all three edit
mtcollins1_boot_run:🤖 Generated with Claude Code
Review 5310234917 (head 6985674):
power_actionrow and thepower_actionfields on the reset roster and the boot subject are removed.PoweredOn | PowerCycled.PowerCommandAcceptedNoTransitionnow carriesprogress: never left on, reached off but did not come back on, never reached on, or read-back unavailable. An observed off half is no longer lost.DeclaredTypeNotInhabited) forIpmiChassisPowerDown,IpmiChassisHardResetandIpmiChassisPowerUppassed as the handoff's action type, plus a clean control.main_wetregen: no generated file changed.