fix(sku): fix v5 SKU upgrade regressions that wedged pre-existing machines

[wminckler]

PR #3717 introduced three bugs that broke machines on upgrade to v2.1.

Description

Bug 1 — v5 generation hard-failed on pre-existing hardware, wedging machines at MatchingSku

generate_sku_from_machine_at_version_5 rejected NVMe drives missing size_mb or pci_path, which are absent on hardware_info records discovered before #3717. Any unassigned machine hit this on upgrade, propagated the error as a StateHandlerError, and wedged permanently in MatchingSku with no recovery path short of a reboot. Generation now includes the drive with whatever fields are present; machines self-heal once hardware re-enumeration populates the new fields.

Bug 2 — diff_storage_by_model clobbered duplicate models from v5 actual SKUs

diff_storage_by_model (used when the expected SKU is v4) built its map with a plain HashMap collect, silently dropping duplicate model keys. A v5 actual SKU has one entry per drive (count=1 each), so two drives of the same model left only one entry with count=1. Compared against a v4 expected with count=2, this produced a false diff and prevented find_matching from ever finding a match for those machines. Fixed by aggregating counts via and_modify.

Bug 3 — graceful degradation allowed unconstrained v5 entries to be auto-created as expected SKUs

The fix for Bug 1 meant generate_missing_sku_for_machine could now generate and persist a v5 expected SKU where every storage entry had no pci_patterns and no size bounds — silently accepting any drive at any location, weaker than v4 model matching. validate_storage_for_create now rejects v5 entries with no constraints. Also adds a min_size_mb > max_size_mb inversion check (caught by CodeRabbit review).

The VerifyingSku path was unaffected by all three bugs — it generates at expected_sku.schema_version, so v4 expected SKUs always generated v4 actuals.

Related issues

Fixes #6520848

Type of Change

• Add - New feature or capability
• Change - Changes in existing functionality
• Fix - Bug fixes
• Remove - Removed features or deprecated functionality
• Internal - Internal changes (refactoring, tests, docs, etc.)

Breaking Changes

• This PR contains breaking changes

Testing

• Unit tests added/updated
• Integration tests added/updated
• Manual testing performed
• No testing required (docs, internal refactor, etc.)

Regression test v5_actual_matches_v4_expected_by_model covers Bug 2. Existing 15 SKU unit tests in crates/api-model/src/sku.rs cover Bug 1 and Bug 3 indirectly (generation no longer errors; unconstrained entries are rejected at the create layer). All 16 tests pass.

Additional Notes

The three bugs form a chain: Bug 1 wedges unassigned machines; Bug 2 prevents assigned machines from re-matching after upgrade; Bug 3 would have allowed a degraded auto-generated expected SKU to persist once Bug 1 was fixed. All three need to land together.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: fd2eef08-785f-4a58-8d6f-16661b19fbab

📥 Commits

Reviewing files that changed from the base of the PR and between 221726a and bbbc772.

📒 Files selected for processing (2)

• crates/api-core/src/tests/sku.rs
• crates/api-db/src/sku.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• crates/api-db/src/sku.rs

---

Summary by CodeRabbit

• Bug Fixes
  • Improved SKU create/replace validation for storage entries by enforcing consistent min/max size ranges and rejecting missing PCI/size constraints for certain v5 storage configurations.
  • v5 SKU generation is now more resilient to NVMe devices that lack size or PCI location details.
  • Corrected storage comparisons so multiple drives with the same storage model are counted accurately.
• Tests
  • Added a regression test for matching v5 actual SKUs to v4 expected SKUs by model.
  • Updated SKU test fixtures to include additional storage constraint details.

Walkthrough

V5 NVMe SKU generation now retains incomplete discovery data for downstream validation. SKU persistence validates storage bounds, PCI patterns, and unconstrained entries. Legacy storage comparison aggregates duplicate models, with regression coverage and updated fixture data.

Changes

SKU storage handling

Layer / File(s)	Summary
Storage generation and persistence validation crates/api-db/src/sku.rs, crates/api-core/src/tests/sku.rs	V5 NVMe generation preserves missing size or PCI data, while create and replace validation enforce size ordering, PCI regex validity, and constraints on unconstrained storage entries; the SKU fixture includes the corresponding storage bounds and PCI patterns.
Legacy storage comparison aggregation crates/api-model/src/sku.rs	Legacy comparisons now sum counts for duplicate storage models, with regression coverage for v5 actual and v4 expected SKUs.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MachineHardware
  participant SKUGenerator
  participant SKUPersistence
  MachineHardware->>SKUGenerator: provide discovered NVMe fields
  SKUGenerator->>SKUPersistence: submit storage entry with optional fields
  SKUPersistence->>SKUPersistence: validate storage constraints

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main SKU v5 upgrade regression fixes addressed by the change.
Description check	✅ Passed	The description is directly aligned with the storage validation, SKU generation, and diffing fixes in the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}