feat(storcli2): logical volume drive add (expand) and remove

pkg/implementation/logicalvolumemanager/storcli2.go

@@ -22,6 +22,10 @@ const (
 	storcli2CmdVD  = "vd"
 	// storcli2CmdDelete deletes the virtual drive addressed by the selector.
 	storcli2CmdDelete = "delete"
+	// storcli2CmdExpand adds physical drives to a virtual drive (online capacity
+	// expansion). storcli2 dropped "start migrate", so this is the only way to
+	// grow a volume; the RAID level is preserved by the firmware.
+	storcli2CmdExpand = "expand"
 	// storcli2ControllerSelector addresses a whole controller (used by "add vd").
 	storcli2ControllerSelector = "/c%d"
 	// storcli2VolumeSelector addresses a single virtual drive by its number.
@@ -50,7 +54,10 @@ type StorCLI2 struct {
 	StorCLI2 commandrunner.CommandRunner
 }
 
-var _ ports.LVCacheSetter = &StorCLI2{}
+var (
+	_ ports.LVCacheSetter         = &StorCLI2{}
+	_ ports.LogicalVolumesManager = &StorCLI2{}
+)
 
 // NewStorCLI2 returns a logical-volume manager backed by the given storcli2 /
 // perccli2 command runner and physical-drive and logical-volume getters.
@@ -166,6 +173,43 @@ func (s *StorCLI2) DeleteLV(metadata *logicalvolume.Metadata) error {
 	return nil
 }
 
+// AddPDsToLV grows a logical volume with the given physical drives through
+// "expand" (online capacity expansion). storcli2 dropped "start migrate", so
+// expansion is the only supported path; the RAID level is preserved by the
+// firmware. The drives must share a single enclosure.
+func (s *StorCLI2) AddPDsToLV(
+	lvMetadata *logicalvolume.Metadata,
+	pdsMetadata ...*physicaldrive.Metadata,
+) error {
+	drives, err := storcli2FormatDrives(pdsMetadata)
+	if err != nil {
+		return errors.Wrap(err, "failed to format drives")
+	}
+
+	selector := fmt.Sprintf(storcli2VolumeSelector, lvMetadata.CtrlMetadata.ID, lvMetadata.ID)
+
+	output, err := s.StorCLI2.Run([]string{selector, storcli2CmdExpand, drives})
+	if err != nil {
+		return errors.Wrapf(err, "failed to run expand command for logical volume %s", lvMetadata.ID)
+	}
+
+	if _, err := storcli2.Decode(output); err != nil {
+		return errors.Wrapf(err, "failed to expand logical volume %s", lvMetadata.ID)
+	}
+
+	return nil
+}
+
+// DeletePDsFromLV is not supported by storcli2: the storcli-to-storcli2 command
+// map drops "start migrate" with no replacement for removing drives from a
+// volume (see DESIGN.md).
+func (*StorCLI2) DeletePDsFromLV(
+	_ *logicalvolume.Metadata,
+	_ ...*physicaldrive.Metadata,
+) error {
+	return ports.ErrFunctionNotSupportedByImplementation
+}
+
 // findNewLogicalVolume returns the volume whose physical-drive set is exactly
 // the request's. A physical drive belongs to a single virtual drive, so the
 // freshly created volume is the only match.

pkg/implementation/logicalvolumemanager/storcli2_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/scality/raidmgmt/pkg/domain/entities/logicalvolume"
 	"github.com/scality/raidmgmt/pkg/domain/entities/physicaldrive"
 	"github.com/scality/raidmgmt/pkg/domain/entities/raidcontroller"
+	"github.com/scality/raidmgmt/pkg/domain/ports"
 	"github.com/scality/raidmgmt/pkg/implementation/logicalvolumemanager"
 )
 
@@ -202,6 +203,95 @@ func TestStorCLI2DeleteLV(t *testing.T) {
 	}
 }
 
+// TestStorCLI2AddPDsToLV covers the expand happy path: the drives are formatted
+// into a single-enclosure list and submitted through "expand".
+func TestStorCLI2AddPDsToLV(t *testing.T) {
+	t.Parallel()
+
+	ctrl := storcli2Ctrl()
+	metadata := &logicalvolume.Metadata{CtrlMetadata: ctrl, ID: "25"}
+	pds := []*physicaldrive.Metadata{
+		{CtrlMetadata: ctrl, ID: "252:3"},
+		{CtrlMetadata: ctrl, ID: "252:4"},
+	}
+
+	mockRunner := new(MockCommandRunner)
+	mockRunner.On("Run", []string{"/c0/v25", "expand", "drives=252:3,4"}).
+		Return(storcli2Fixture(t, "expand/success.json"), nil)
+
+	manager := logicalvolumemanager.NewStorCLI2(
+		mockRunner, new(MockPhysicalDrivesGetter), new(MockLogicalVolumesGetter),
+	)
+
+	err := manager.AddPDsToLV(metadata, pds...)
+	require.NoError(t, err)
+	mockRunner.AssertExpectations(t)
+}
+
+// TestStorCLI2AddPDsToLVCommandError pins that an "expand" failure payload is
+// surfaced.
+func TestStorCLI2AddPDsToLVCommandError(t *testing.T) {
+	t.Parallel()
+
+	ctrl := storcli2Ctrl()
+	metadata := &logicalvolume.Metadata{CtrlMetadata: ctrl, ID: "25"}
+	pd := &physicaldrive.Metadata{CtrlMetadata: ctrl, ID: "252:3"}
+
+	mockRunner := new(MockCommandRunner)
+	mockRunner.On("Run", []string{"/c0/v25", "expand", "drives=252:3"}).
+		Return(storcli2Fixture(t, "expand/fail.json"), nil)
+
+	manager := logicalvolumemanager.NewStorCLI2(
+		mockRunner, new(MockPhysicalDrivesGetter), new(MockLogicalVolumesGetter),
+	)
+
+	err := manager.AddPDsToLV(metadata, pd)
+	require.Error(t, err)
+}
+
+// TestStorCLI2AddPDsToLVMultipleEnclosures pins that drives spanning two
+// enclosures are rejected before "expand" is run.
+func TestStorCLI2AddPDsToLVMultipleEnclosures(t *testing.T) {
+	t.Parallel()
+
+	ctrl := storcli2Ctrl()
+	metadata := &logicalvolume.Metadata{CtrlMetadata: ctrl, ID: "25"}
+	pds := []*physicaldrive.Metadata{
+		{CtrlMetadata: ctrl, ID: "252:3"},
+		{CtrlMetadata: ctrl, ID: "253:4"},
+	}
+
+	mockRunner := new(MockCommandRunner)
+
+	manager := logicalvolumemanager.NewStorCLI2(
+		mockRunner, new(MockPhysicalDrivesGetter), new(MockLogicalVolumesGetter),
+	)
+
+	err := manager.AddPDsToLV(metadata, pds...)
+	require.Error(t, err)
+	mockRunner.AssertNotCalled(t, "Run")
+}
+
+// TestStorCLI2DeletePDsFromLV pins that drive removal is reported as unsupported:
+// storcli2 has no replacement for storcli's "start migrate option=remove".
+func TestStorCLI2DeletePDsFromLV(t *testing.T) {
+	t.Parallel()
+
+	ctrl := storcli2Ctrl()
+	metadata := &logicalvolume.Metadata{CtrlMetadata: ctrl, ID: "25"}
+	pd := &physicaldrive.Metadata{CtrlMetadata: ctrl, ID: "252:3"}
+
+	mockRunner := new(MockCommandRunner)
+
+	manager := logicalvolumemanager.NewStorCLI2(
+		mockRunner, new(MockPhysicalDrivesGetter), new(MockLogicalVolumesGetter),
+	)
+
+	err := manager.DeletePDsFromLV(metadata, pd)
+	require.ErrorIs(t, err, ports.ErrFunctionNotSupportedByImplementation)
+	mockRunner.AssertNotCalled(t, "Run")
+}
+
 // TestStorCLI2SetLVCacheOptions covers the only-changed-flag behavior: each
 // policy is set through its own command, and an unchanged policy emits no
 // command at all.

pkg/implementation/logicalvolumemanager/testdata/storcli2/expand/fail.json

@@ -0,0 +1,22 @@
+{
+"Controllers":[
+{
+	"Command Status" : {
+		"CLI Version" : "008.0005.0000.0010 Feb 22, 2023",
+		"Operating system" : "Linux4.18.0-553.115.1.el8_10.x86_64",
+		"Controller" : "0",
+		"Status" : "Failure",
+		"Description" : "expand VD failed",
+		"Detailed Status" : [
+			{
+				"VD" : 25,
+				"Status" : "Failed",
+				"ErrType" : "CLI",
+				"ErrCd" : 255,
+				"ErrMsg" : "Expansion not supported on this VD"
+			}
+		]
+	}
+}
+]
+}

pkg/implementation/logicalvolumemanager/testdata/storcli2/expand/success.json

@@ -0,0 +1,13 @@
+{
+"Controllers":[
+{
+	"Command Status" : {
+		"CLI Version" : "008.0005.0000.0010 Feb 22, 2023",
+		"Operating system" : "Linux4.18.0-553.115.1.el8_10.x86_64",
+		"Controller" : "0",
+		"Status" : "Success",
+		"Description" : "expand VD succeeded"
+	}
+}
+]
+}

tests/testdata-tools/README.md

@@ -52,7 +52,7 @@ The script writes each fixture under its owning component package's
 | `logicalvolumegetter` | `testdata/storcli2/show/v999_invalid.json` | VD not found | safe |
 | `logicalvolumemanager` | `testdata/storcli2/create/{success,fail}.json` | `add vd ...` | destructive |
 | `logicalvolumemanager` | `testdata/storcli2/delete/{success,fail_invalid,fail_vdNotExist}.json` | `delete` | destructive |
-| `logicalvolumemanager` | `testdata/storcli2/migrate/fail.json` | `start migrate ...` | destructive |
+| `logicalvolumemanager` | `testdata/storcli2/expand/{success,fail}.json` | `expand drives=...` | destructive |
 | `logicalvolumemanager` | `testdata/storcli2/cacheoptions/success_{wrcache,rdcache}.json` | `set wrcache/rdcache` | destructive |
 | `logicalvolumemanager` | `testdata/storcli2/cacheoptions/combined_syntax_error.json` | v1 combined `set` syntax (rejected) | destructive |
 | `blinker` | `testdata/storcli2/{start,stop}.json` | `start locate` / `stop locate` | destructive |
@@ -68,8 +68,6 @@ storcli v1 (the script still uses the v1 syntax):
 
 - `logicalvolumemanager/testdata/storcli2/cacheoptions/combined_syntax_error.json`
   (`unexpected TOKEN_WRITE_CACHE`)
-- `logicalvolumemanager/testdata/storcli2/migrate/fail.json`
-  (`unexpected TOKEN_MIGRATE`)
 - `physicaldrivegetter/testdata/storcli2/jbod/disable/fail.json`
   (`unexpected TOKEN_JBOD`)
 

tests/testdata-tools/collect_storcli2_testdata.sh

@@ -351,13 +351,14 @@ if [ "${DESTRUCTIVE}" = "true" ]; then
         "${OUTPUT_DIR}/logicalvolumemanager/testdata/storcli2/delete/fail_vdNotExist.json" \
         "${C}/v${INVALID_VD_ID}" delete
 
-    # Migrate - failure case (operation not supported/possible)
-    # Command: storcli2 /c0/v1 start migrate type=raid0 option=add drives=306:99 J
-    # Used by: adapter.migrate() error case
+    # Expand - failure case (invalid drive). storcli2 dropped "start migrate";
+    # AddPDsToLV goes through "expand".
+    # Command: storcli2 /c0/v1 expand drives=306:99 J
+    # Used by: StorCLI2.AddPDsToLV() error case
     run_and_save \
-        "Migrate VD (expected failure)" \
-        "${OUTPUT_DIR}/logicalvolumemanager/testdata/storcli2/migrate/fail.json" \
-        "${C}/v1" start migrate type=raid0 option=add "drives=${FIRST_ENCLOSURE}:${INVALID_SLOT}"
+        "Expand VD (expected failure)" \
+        "${OUTPUT_DIR}/logicalvolumemanager/testdata/storcli2/expand/fail.json" \
+        "${C}/v1" expand "drives=${FIRST_ENCLOSURE}:${INVALID_SLOT}"
 
     # --- Step 0: Auto-detect sacrifice VD if needed ---
     if [ "${SACRIFICE_VD_ID}" = "auto" ]; then