feat(storcli2): logical volume create and delete

[g-carre]

Summary

Implements the create/delete part of ports.LogicalVolumesManager on the storcli2/perccli2 logical-volume manager (ARTESCA-17647).

• CreateLV — fills the request drives through the physical-drive getter, validates the RAID creation, formats a single-enclosure drives=e:s,... list (rejecting multi-enclosure requests), issues add vd with the bare RAID-level and cache tokens, and rediscovers the new volume by its physical-drive set.
• DeleteLV — issues /cx/vx delete and surfaces the in-JSON failure payload.
• The manager now also takes a PhysicalDrivesGetter; command verbs follow the storcli2 command map documented in DESIGN.md.

Testing

go build, go vet, gofmt, and the package test suite pass. New tests cover the create happy path, command-failure, multi-enclosure rejection, and a table-driven delete (success / invalid VD / nonexistent VD).

Note: This PR targets improvement/storcli2-cache-jbod-setter (its base), not main.

Issue: ARTESCA-17647

🤖 Generated with Claude Code

[claude]

• pkg/implementation/logicalvolumemanager/storcli2.go:42 — StorCLI2 does not satisfy ports.LogicalVolumesManager (missing AddPDsToLV / DeletePDsFromLV). Add stubs returning ErrFunctionNotSupportedByImplementation and a compile-time assertion, matching the MDADM pattern.
  
  The rest of the implementation is solid: error wrapping is consistent, the single-enclosure validation in storcli2FormatDrives is correct, the PD-set matching for volume rediscovery is sound, and the test coverage (happy path, command failure, multi-enclosure rejection, table-driven delete) is good.
  
  Review by Claude Code

[claude]

• pkg/implementation/logicalvolumemanager/storcli2.go:41 — StorCLI2 is missing AddPDsToLV and DeletePDsFromLV from ports.LogicalVolumesManager. Add stub methods returning ports.ErrFunctionNotSupportedByImplementation and a compile-time interface assertion (var _ ports.LogicalVolumesManager = &StorCLI2{}) to match the MDADM adapter pattern.
  
  Review by Claude Code

[claude]

• pkg/implementation/logicalvolumemanager/storcli2.go:44 — StorCLI2 does not satisfy ports.LogicalVolumesManager (missing AddPDsToLV / DeletePDsFromLV). Add a compile-time interface check and stub the unsupported methods with ErrFunctionNotSupportedByImplementation, matching the MDADM implementation in this package.
  
  Review by Claude Code

[claude]

1 issue found:

• pkg/implementation/logicalvolumemanager/storcli2.go:42 — StorCLI2 is missing AddPDsToLV/DeletePDsFromLV stubs and the var _ ports.LogicalVolumesManager compile-time assertion that both existing adapters carry. Without these, the type cannot be used as a LogicalVolumesManager.

Looks good otherwise: error handling follows the errors.Wrap convention, cache-token extraction is correctly shared and fails closed, the findNewLogicalVolume rediscovery approach is sound, tests cover the happy path plus three failure modes (command error, multi-enclosure, unsettable policy, and table-driven delete), and fixtures match the code paths they exercise.

Review by Claude Code

[claude]

• pkg/implementation/logicalvolumemanager/storcli2.go:44 — StorCLI2 implements CreateLV/DeleteLV but not AddPDsToLV/DeletePDsFromLV from LogicalVolumesManager. Consider adding stubs that return ErrFunctionNotSupportedByImplementation.
  
  Otherwise clean: error handling uses errors.Wrap consistently, hexagonal boundaries are respected, cache token refactoring to the shared storcli2 package is well-motivated, drive formatting correctly validates single-enclosure constraint before running the command, and the test suite covers happy path plus key failure modes (command error, multi-enclosure rejection, unsettable cache policy, invalid/nonexistent VD deletion) with realistic fixtures.
  
  Review by Claude Code

[claude]

• pkg/implementation/logicalvolumemanager/storcli2.go:44 — Missing compile-time interface assertion (var _ ports.LogicalVolumesManager = &StorCLI2{}) and stub implementations for AddPDsToLV/DeletePDsFromLV returning ErrFunctionNotSupportedByImplementation. Every other adapter in the codebase has this assertion; without it, StorCLI2 cannot be used as a LogicalVolumesManager.
  
  The rest of the PR — cache-token extraction into storcli2/cache.go, the create/delete logic, drive-list formatting, multi-enclosure rejection, and tests — looks solid.
  
  Review by Claude Code

[ezekiel-alexrod]

LGTM

[claude]

The ports.LogicalVolumesManager interface does not embed PhysicalDrivesGetter or LogicalVolumesGetter, so the compile-time assertion on this line only checks the four manager methods. But the struct embeds both getter interfaces, which means it also satisfies PhysicalDrivesGetter and LogicalVolumesGetter — and its PhysicalDrives method (from the embedded getter) is callable by external callers even though it's not part of the manager's contract.

This is consistent with how SSACLI and MDADM do it in the same package, so it's not a regression — just noting that the embedded getter interfaces leak as public API. If that's intentional (e.g. a composite RAIDController wires these together), it's fine.

— Claude Code

[claude]

storcli2VolumeSelector is defined identically in both this file and lvcachesetter/storcli2.go (line 18). Consider moving it to the shared storcli2 package alongside Decode and the cache-token helpers, since it's a common storcli2 addressing convention reused across adapters.

— Claude Code

[claude]

LGTM — clean implementation. Two minor observations posted as inline comments:

• pkg/implementation/logicalvolumemanager/storcli2.go:30 — storcli2VolumeSelector is duplicated between this file and lvcachesetter/storcli2.go; could live in the shared storcli2 package.
• pkg/implementation/logicalvolumemanager/storcli2.go:50 — Informational note that the embedded getter interfaces leak as public API on the struct (consistent with SSACLI/MDADM, not a regression).

Error handling is thorough (all paths wrapped with context), test coverage is solid across create/delete/expand/unsupported-ops, cache-token extraction was cleanly refactored into the shared storcli2 package, hexagonal boundaries are respected, and DeletePDsFromLV correctly returns ErrFunctionNotSupportedByImplementation. The blinker adapter is well-structured with enclosure/no-enclosure selector handling. All referenced testdata fixtures exist and match the code paths they exercise.

Review by Claude Code