diff --git a/bible-local/10-decisions.md b/bible-local/10-decisions.md index b9ef10f..3eb957e 100644 --- a/bible-local/10-decisions.md +++ b/bible-local/10-decisions.md @@ -1270,3 +1270,43 @@ of raw Redfish walk is picked up automatically without a dedicated parser. `redfishtree.FindCandidateArchives` + `redfishtree.Build`, not re-implement tar/zip walking. - `redfishwalk` is a genuine fallback: it only wins `Detect()` when no dedicated vendor parser scores higher on the same archive, per the registry's highest-confidence-wins rule. + +--- + +## ADL-049 — Reanimator export/re-import round trip silently dropped Memory and PSUs + +**Date:** 2026-08-11 +**Context:** After ADL-048 fixed Dell iDRAC10 inventory parsing, the same PowerEdge R7715 archive +still showed no Memory or Power Supplies sections in the `/chart/current` web view, even though +`internal/exporter.ConvertToReanimator` produced both correctly from a fresh TSR upload (verified via +direct API calls against the running server). The web view *did* break identically after re-uploading +a previously downloaded `reanimator.json` export back into LOGPile (the "Reanimator" round trip: export, +then re-import the same file to inspect/verify it) — reproduced via `POST /api/upload` with that file. +Root cause: `ReanimatorMemory.Present` and `ReanimatorPSU.Present` (`internal/exporter/reanimator_models.go`) +were already declared as `*bool` `json:"present,omitempty"`, matching `ReanimatorStorage.Present`, but +unlike `convertStorageFromDevices` (which sets `Present: &presentValue` on every emitted item), +`convertMemoryFromDevices` and `convertPSUsFromDevices` never populated that field on the structs they +built — a plain omission, not a deliberate contract choice. So exported JSON always had `"present": true` +for storage but no `present` key at all for memory/PSU items. `parseUploadedSnapshot` (handlers.go) +re-imports a reanimator export via a direct `json.Unmarshal` straight into `models.AnalysisResult` +(the internal shape, not a dedicated import mapper); with no `present` key to unmarshal, Go's zero +value left `MemoryDIMM.Present` / `PSU.Present` as `false`. On the next `ConvertToReanimator` call (every +`/chart/current` render re-converts from the current in-memory result), `MemoryDIMM.IsInstalledInventory()` +requires `Present == true`, and `convertPSUsFromDevices` has an explicit `if !present { continue }` — so +every memory/PSU entry, despite carrying full data, got filtered out as "not installed". +**Decision:** Populate `Present` on `ReanimatorMemory`/`ReanimatorPSU` in `convertMemoryFromDevices`/ +`convertPSUsFromDevices`, mirroring what `convertStorageFromDevices` already does. No changes needed to +the import path or the `Present`-based filtering — the filtering itself is correct, it was just never +given the field it needs from a re-imported export. +**Consequences:** +- Re-uploading a LOGPile-exported `reanimator.json` now preserves Memory and Power Supplies through the + round trip, verified against the live server: upload TSR → export reanimator.json → re-upload it → + `/chart/current` still shows both sections. +- Regression test: `TestConvertToReanimator_MemoryAndPSURoundTripSurvivesReimport` in + `internal/exporter/reanimator_converter_test.go` — converts a minimal result, marshals it, unmarshals + back into `models.AnalysisResult` (simulating `parseUploadedSnapshot`), and asserts the reconverted + output still contains both entries. +- General lesson for this converter: any `Reanimator*` struct field meant to round-trip through + `parseUploadedSnapshot` must actually be populated by its `convert*FromDevices` function — the struct + declaring the field is not enough. `Storage` was the only category doing this correctly before this fix; + worth auditing PCIe/NIC/GPU conversion the same way if a similar round-trip gap is reported for them. diff --git a/internal/exporter/reanimator_converter.go b/internal/exporter/reanimator_converter.go index 329102c..d8d5898 100644 --- a/internal/exporter/reanimator_converter.go +++ b/internal/exporter/reanimator_converter.go @@ -43,12 +43,12 @@ func ConvertToReanimator(result *models.AnalysisResult) (*ReanimatorExport, erro TargetHost: targetHost, CollectedAt: collectedAt, Hardware: ReanimatorHardware{ - Board: convertBoard(result.Hardware.BoardInfo), - Firmware: dedupeFirmware(convertFirmware(result.Hardware.Firmware)), - CPUs: dedupeCPUs(convertCPUsFromDevices(devices, collectedAt, result.Hardware.BoardInfo.SerialNumber, buildCPUMicrocodeBySocket(result.Hardware.Firmware))), - Memory: dedupeMemory(convertMemoryFromDevices(devices, collectedAt)), - Storage: dedupeStorage(convertStorageFromDevices(devices, collectedAt)), - PCIeDevices: dedupePCIe(convertPCIeFromDevices(devices, collectedAt)), + Board: convertBoard(result.Hardware.BoardInfo), + Firmware: dedupeFirmware(convertFirmware(result.Hardware.Firmware)), + CPUs: dedupeCPUs(convertCPUsFromDevices(devices, collectedAt, result.Hardware.BoardInfo.SerialNumber, buildCPUMicrocodeBySocket(result.Hardware.Firmware))), + Memory: dedupeMemory(convertMemoryFromDevices(devices, collectedAt)), + Storage: dedupeStorage(convertStorageFromDevices(devices, collectedAt)), + PCIeDevices: dedupePCIe(convertPCIeFromDevices(devices, collectedAt)), PowerSupplies: dedupePSUs(convertPSUsFromDevices(devices, collectedAt)), Sensors: convertSensors(result.Sensors), BMCEventSummary: buildBMCEventSummary(result.Events, collectedAt), @@ -716,9 +716,11 @@ func convertMemoryFromDevices(devices []models.HardwareDevice, collectedAt strin continue } meta := buildStatusMeta(status, d.StatusCheckedAt, d.StatusChangedAt, d.StatusHistory, d.ErrorDescription, collectedAt) + presentValue := present result = append(result, ReanimatorMemory{ Slot: d.Slot, Location: d.Location, + Present: &presentValue, SizeMB: d.SizeMB, Type: d.Type, MaxSpeedMHz: intFromDetailMap(d.Details, "max_speed_mhz"), @@ -990,8 +992,10 @@ func convertPSUsFromDevices(devices []models.HardwareDevice, collectedAt string) } status := normalizeStatus(d.Status, false) meta := buildStatusMeta(status, d.StatusCheckedAt, d.StatusChangedAt, d.StatusHistory, d.ErrorDescription, collectedAt) + presentValue := present result = append(result, ReanimatorPSU{ Slot: d.Slot, + Present: &presentValue, Model: d.Model, Vendor: d.Manufacturer, WattageW: d.WattageW, diff --git a/internal/exporter/reanimator_converter_test.go b/internal/exporter/reanimator_converter_test.go index 7d7fc2c..4e3be5a 100644 --- a/internal/exporter/reanimator_converter_test.go +++ b/internal/exporter/reanimator_converter_test.go @@ -2013,3 +2013,67 @@ func TestIsDeviceBoundFirmwareFQDD(t *testing.T) { } } } + +// TestConvertToReanimator_MemoryAndPSURoundTripSurvivesReimport is a regression guard +// for a bug where re-uploading a previously exported reanimator.json (the "Reanimator" +// round-trip: export, then re-import the same file) silently dropped Memory and Power +// Supplies from the chart view. convertMemoryFromDevices/convertPSUsFromDevices never +// set the already-declared ReanimatorMemory.Present / ReanimatorPSU.Present output +// field, so the exported JSON omitted "present" for those two sections (unlike +// Storage, which did set it). Re-importing that JSON via parseUploadedSnapshot +// (a direct json.Unmarshal into models.AnalysisResult) left Present=false on the +// round-tripped items, and IsInstalledInventory()/the PSU present check then dropped +// them on the next ConvertToReanimator call. (2026-08-11) +func TestConvertToReanimator_MemoryAndPSURoundTripSurvivesReimport(t *testing.T) { + original := &models.AnalysisResult{ + Filename: "test.zip", + CollectedAt: time.Date(2026, 7, 22, 4, 16, 0, 0, time.UTC), + Hardware: &models.HardwareConfig{ + BoardInfo: models.BoardInfo{ + Manufacturer: "Dell Inc.", + ProductName: "PowerEdge R7715", + SerialNumber: "1TVFYL4", + }, + Memory: []models.MemoryDIMM{ + {Slot: "A1", Present: true, SizeMB: 32768, SerialNumber: "37CC7641", Status: "OK"}, + }, + PowerSupply: []models.PSU{ + {Slot: "PSU.Slot.1", Present: true, Model: "PWR SPLY,1500W", SerialNumber: "CNDED0064K2RKW", Status: "OK"}, + }, + }, + } + + exported, err := ConvertToReanimator(original) + if err != nil { + t.Fatalf("ConvertToReanimator() failed: %v", err) + } + if len(exported.Hardware.Memory) != 1 || exported.Hardware.Memory[0].Present == nil || !*exported.Hardware.Memory[0].Present { + t.Fatalf("expected exported memory to carry present=true, got %+v", exported.Hardware.Memory) + } + if len(exported.Hardware.PowerSupplies) != 1 || exported.Hardware.PowerSupplies[0].Present == nil || !*exported.Hardware.PowerSupplies[0].Present { + t.Fatalf("expected exported PSU to carry present=true, got %+v", exported.Hardware.PowerSupplies) + } + + // Simulate the round trip: marshal the exported reanimator JSON, then unmarshal + // it straight into models.AnalysisResult the way parseUploadedSnapshot does when + // a user re-uploads a previously downloaded reanimator.json. + raw, err := json.Marshal(exported) + if err != nil { + t.Fatalf("marshal exported: %v", err) + } + var reimported models.AnalysisResult + if err := json.Unmarshal(raw, &reimported); err != nil { + t.Fatalf("unmarshal into AnalysisResult: %v", err) + } + + reconverted, err := ConvertToReanimator(&reimported) + if err != nil { + t.Fatalf("ConvertToReanimator() on reimported result failed: %v", err) + } + if len(reconverted.Hardware.Memory) != 1 { + t.Fatalf("memory did not survive reanimator round trip, got %+v", reconverted.Hardware.Memory) + } + if len(reconverted.Hardware.PowerSupplies) != 1 { + t.Fatalf("power supplies did not survive reanimator round trip, got %+v", reconverted.Hardware.PowerSupplies) + } +} diff --git a/internal/exporter/reanimator_models.go b/internal/exporter/reanimator_models.go index 2c11899..3052961 100644 --- a/internal/exporter/reanimator_models.go +++ b/internal/exporter/reanimator_models.go @@ -12,28 +12,28 @@ type ReanimatorExport struct { // ReanimatorHardware contains all hardware components type ReanimatorHardware struct { - Board ReanimatorBoard `json:"board"` - Firmware []ReanimatorFirmware `json:"firmware,omitempty"` - CPUs []ReanimatorCPU `json:"cpus,omitempty"` - Memory []ReanimatorMemory `json:"memory,omitempty"` - Storage []ReanimatorStorage `json:"storage,omitempty"` - PCIeDevices []ReanimatorPCIe `json:"pcie_devices,omitempty"` - PowerSupplies []ReanimatorPSU `json:"power_supplies,omitempty"` - Sensors *ReanimatorSensors `json:"sensors,omitempty"` - BMCEventSummary []ReanimatorBMCEventRow `json:"bmc_event_summary,omitempty"` - EventLogs []ReanimatorEventLog `json:"event_logs,omitempty"` - PlatformConfig map[string]any `json:"platform_config,omitempty"` + Board ReanimatorBoard `json:"board"` + Firmware []ReanimatorFirmware `json:"firmware,omitempty"` + CPUs []ReanimatorCPU `json:"cpus,omitempty"` + Memory []ReanimatorMemory `json:"memory,omitempty"` + Storage []ReanimatorStorage `json:"storage,omitempty"` + PCIeDevices []ReanimatorPCIe `json:"pcie_devices,omitempty"` + PowerSupplies []ReanimatorPSU `json:"power_supplies,omitempty"` + Sensors *ReanimatorSensors `json:"sensors,omitempty"` + BMCEventSummary []ReanimatorBMCEventRow `json:"bmc_event_summary,omitempty"` + EventLogs []ReanimatorEventLog `json:"event_logs,omitempty"` + PlatformConfig map[string]any `json:"platform_config,omitempty"` } // ReanimatorBMCEventRow is one row in the BMC critical/warning event summary table. type ReanimatorBMCEventRow struct { - Severity string `json:"severity"` - Component string `json:"component"` - MessageID string `json:"message_id"` - Timestamp string `json:"timestamp"` + Severity string `json:"severity"` + Component string `json:"component"` + MessageID string `json:"message_id"` + Timestamp string `json:"timestamp"` Description string `json:"description"` - Status string `json:"status"` - ResolvedAt string `json:"resolved_at,omitempty"` + Status string `json:"status"` + ResolvedAt string `json:"resolved_at,omitempty"` } // ReanimatorBoard represents motherboard/server information @@ -128,25 +128,25 @@ type ReanimatorStorage struct { MetadataBytesPerBlock int64 `json:"metadata_bytes_per_block,omitempty"` TemperatureC float64 `json:"temperature_c,omitempty"` PowerOnHours int64 `json:"power_on_hours,omitempty"` - PowerCycles int64 `json:"power_cycles,omitempty"` - UnsafeShutdowns int64 `json:"unsafe_shutdowns,omitempty"` - MediaErrors int64 `json:"media_errors,omitempty"` - ErrorLogEntries int64 `json:"error_log_entries,omitempty"` - WrittenBytes int64 `json:"written_bytes,omitempty"` - ReadBytes int64 `json:"read_bytes,omitempty"` - LifeUsedPct float64 `json:"life_used_pct,omitempty"` - RemainingEndurancePct *int `json:"remaining_endurance_pct,omitempty"` - LifeRemainingPct float64 `json:"life_remaining_pct,omitempty"` - AvailableSparePct float64 `json:"available_spare_pct,omitempty"` - ReallocatedSectors int64 `json:"reallocated_sectors,omitempty"` - CurrentPendingSectors int64 `json:"current_pending_sectors,omitempty"` - OfflineUncorrectable int64 `json:"offline_uncorrectable,omitempty"` - Status string `json:"status,omitempty"` - StatusCheckedAt string `json:"status_checked_at,omitempty"` - StatusChangedAt string `json:"status_changed_at,omitempty"` - ManufacturedYearWeek string `json:"manufactured_year_week,omitempty"` - StatusHistory []ReanimatorStatusHistoryEntry `json:"status_history,omitempty"` - ErrorDescription string `json:"error_description,omitempty"` + PowerCycles int64 `json:"power_cycles,omitempty"` + UnsafeShutdowns int64 `json:"unsafe_shutdowns,omitempty"` + MediaErrors int64 `json:"media_errors,omitempty"` + ErrorLogEntries int64 `json:"error_log_entries,omitempty"` + WrittenBytes int64 `json:"written_bytes,omitempty"` + ReadBytes int64 `json:"read_bytes,omitempty"` + LifeUsedPct float64 `json:"life_used_pct,omitempty"` + RemainingEndurancePct *int `json:"remaining_endurance_pct,omitempty"` + LifeRemainingPct float64 `json:"life_remaining_pct,omitempty"` + AvailableSparePct float64 `json:"available_spare_pct,omitempty"` + ReallocatedSectors int64 `json:"reallocated_sectors,omitempty"` + CurrentPendingSectors int64 `json:"current_pending_sectors,omitempty"` + OfflineUncorrectable int64 `json:"offline_uncorrectable,omitempty"` + Status string `json:"status,omitempty"` + StatusCheckedAt string `json:"status_checked_at,omitempty"` + StatusChangedAt string `json:"status_changed_at,omitempty"` + ManufacturedYearWeek string `json:"manufactured_year_week,omitempty"` + StatusHistory []ReanimatorStatusHistoryEntry `json:"status_history,omitempty"` + ErrorDescription string `json:"error_description,omitempty"` } // ReanimatorPCIe represents a PCIe device