Design Reviews & ADRs

Deliverable 2 of Phase 12. A worked ADR, an ADR template, and a design-review feedback example on the JD's four axes (feasibility/performance/reliability/maintainability) — WARMUP Ch. 4.


Part A — Worked ADR (example)

ADR-007: Redfish-first device driver abstraction with IPMI/SNMP fallback

  • Status: Accepted (2026-06-13)
  • Context: Our fleet is heterogeneous — new accelerator nodes speak Redfish, the rack PDU speaks SNMP, and some peer BMCs are IPMI-only. We need one control plane that manages all of them, is testable without hardware, and lets a device class migrate protocols without rewrites. (Phase 03.)
  • Decision: Define a single DeviceDriver interface (power_state, set_power, read_sensors, …) with RedfishDriver (preferred), IpmiDriver, and SnmpDriver backends and a capability-detecting factory; normalize all telemetry into one Reading model. All rack logic programs against the interface.
  • Alternatives considered:
    • Redfish-only: simplest, but strands the IPMI/SNMP gear we must manage today.
    • Per-protocol code paths in callers: no abstraction tax, but protocol quirks leak everywhere and every caller must change to migrate a device. Rejected (unmaintainable at fleet scale).
    • A heavyweight third-party DCIM: faster initially, but loses control over the integration depth and the conformance/feedback loop with the ODM. Rejected for the core path.
  • Consequences:
    • (+) One model above; protocol quirks contained; hermetic tests via a FakeDriver (Phase 02/11); a device can move IPMI→Redfish without touching callers.
    • (+) Reused by the exporter (Phase 07) and the capstone (Phase 13).
    • (−) An abstraction layer to maintain; risk of a lowest-common-denominator API — mitigated by capability flags and OEM passthrough where needed.
    • (−) Defensive coding required for vendor variance (Phase 03 Ch. 9); a conformance suite is therefore mandatory (Phase 11).

Part B — ADR template (copy per decision)

# ADR-NNN: <short decision title>
- Status: Proposed | Accepted | Superseded by ADR-MMM  (date)
- Context: <the forces — technical, partner, timeline — that make this a real decision>
- Decision: <what we will do, specifically>
- Alternatives considered: <each option + why accepted/rejected>
- Consequences: <the good, the bad we accept, and what it enables/blocks downstream>

Write an ADR for any decision that is costly to reverse or that someone will later ask "why is it this way?" — e.g., "A/B firmware banks" (Phase 08), "operator-based lifecycle" (Phase 06), "management VLAN as a hard boundary" (Phase 09).


Part C — Design-review feedback (example, implementation-focused)

Review a change on the four axes; separate must-fix from opinion; critique the design, not the person; bring alternatives (WARMUP Ch. 4).

Change under review: a new BMC telemetry module that hardcodes Redfish paths and queries the BMC synchronously on every Prometheus scrape.

  • Maintainability / correctness (must-fix): hardcoded /redfish/v1/Systems/1/... breaks across vendors (IDs differ — Phase 03 Ch. 9). Follow @odata.id links instead. Alternative: use the existing RedfishDriver (ADR-007) which already does link-following.
  • Reliability (must-fix): synchronous BMC calls on the scrape path couple scrape latency to BMC health and hammer fragile BMCs; one slow BMC stalls all scrapes. Alternative: poll on the exporter's own schedule with caching + retries/timeout (Phase 02/07) and serve cached values.
  • Performance (consider): add a scrape_duration_seconds histogram so we can SLO it (Phase 07).
  • Security (must-fix): TLS verification is disabled (ssl_insecure=true). Validate the cert against the management CA (Phase 03 Ch. 7).
  • Testability (must-fix): no seam — can't unit-test without a BMC. Inject the DeviceDriver so a FakeDriver covers failure paths (Phase 02/11).
  • Operability (nit): switch print to structured logging with node/rack context (Phase 07).
  • Style (linter): defer to ruff/black; not blocking.

Decision capture: agreed to cached polling + link-following + cert validation + the seam; opened ADR-012 ("no synchronous device I/O on the scrape path") and added both rules to the review checklist so the standard sticks (Phase 11 continuous improvement).


How to use this deliverable

  1. Write one real ADR for a curriculum decision using Part B.
  2. Do a Part-C-style review of one of your own labs' code on the four axes.
  3. Keep an adr/ directory in your projects; number ADRs; link them from runbooks and the EBOM.