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

pkg/implementation/blinker/storcli2.go

@@ -0,0 +1,96 @@
+package blinker
+
+import (
+	"fmt"
+
+	"github.com/pkg/errors"
+
+	"github.com/scality/raidmgmt/pkg/domain/entities/physicaldrive"
+	"github.com/scality/raidmgmt/pkg/domain/ports"
+	"github.com/scality/raidmgmt/pkg/implementation/commandrunner"
+	"github.com/scality/raidmgmt/pkg/implementation/storcli2"
+)
+
+const (
+	// storcli2CmdStart and storcli2CmdStop begin and end a drive locate.
+	storcli2CmdStart = "start"
+	storcli2CmdStop  = "stop"
+	// storcli2CmdLocate is the locate (blink) operation token.
+	storcli2CmdLocate = "locate"
+	// storcli2EnclosureSelector and storcli2NoEnclosureSelector address a single
+	// drive, with or without an enclosure component.
+	storcli2EnclosureSelector   = "/c%d/e%s/s%s"
+	storcli2NoEnclosureSelector = "/c%d/s%s"
+)
+
+// StorCLI2 blinks physical drives through a storcli2/perccli2 command runner. A
+// single implementation serves both binaries; the concrete runner is injected
+// at construction time.
+type StorCLI2 struct {
+	runner commandrunner.CommandRunner
+}
+
+var _ ports.Blinker = &StorCLI2{}
+
+// NewStorCLI2 returns a blinker backed by the given storcli2 / perccli2 command
+// runner.
+func NewStorCLI2(runner commandrunner.CommandRunner) *StorCLI2 {
+	return &StorCLI2{
+		runner: runner,
+	}
+}
+
+// StartBlink starts locating (blinking) a physical drive.
+func (s *StorCLI2) StartBlink(metadata *physicaldrive.Metadata) error {
+	if err := s.locate(metadata, storcli2CmdStart); err != nil {
+		return errors.Wrap(err, "failed to start blinking physical drive")
+	}
+
+	return nil
+}
+
+// StopBlink stops locating (blinking) a physical drive.
+func (s *StorCLI2) StopBlink(metadata *physicaldrive.Metadata) error {
+	if err := s.locate(metadata, storcli2CmdStop); err != nil {
+		return errors.Wrap(err, "failed to stop blinking physical drive")
+	}
+
+	return nil
+}
+
+// locate runs "<start|stop> locate" on a drive selector and surfaces the
+// in-JSON failure that storcli2 may report regardless of its exit code.
+func (s *StorCLI2) locate(metadata *physicaldrive.Metadata, action string) error {
+	selector, err := storcli2SelectorPD(metadata)
+	if err != nil {
+		return errors.Wrap(err, "failed to build drive selector")
+	}
+
+	output, err := s.runner.Run([]string{selector, action, storcli2CmdLocate})
+	if err != nil {
+		return errors.Wrapf(err, "failed to run %s locate command", action)
+	}
+
+	if _, err := storcli2.Decode(output); err != nil {
+		return errors.Wrapf(err, "%s locate command failed", action)
+	}
+
+	return nil
+}
+
+// storcli2SelectorPD builds the storcli2 selector for a drive, choosing the
+// enclosure or no-enclosure form from its parsed slot.
+func storcli2SelectorPD(metadata *physicaldrive.Metadata) (string, error) {
+	slot, err := physicaldrive.ParseSlot(metadata.ID)
+	if err != nil {
+		return "", errors.Wrapf(err, "failed to parse slot %s", metadata.ID)
+	}
+
+	if slot.Enclosure != "" {
+		return fmt.Sprintf(
+			storcli2EnclosureSelector, metadata.CtrlMetadata.ID, slot.Enclosure, slot.Bay,
+		), nil
+	}
+
+	return fmt.Sprintf(storcli2NoEnclosureSelector, metadata.CtrlMetadata.ID, slot.Bay), nil
+}

pkg/implementation/blinker/storcli2_test.go

@@ -0,0 +1,111 @@
+package blinker_test
+
+import (
+	"os"
+	"testing"
+
+	"github.com/pkg/errors"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+
+	"github.com/scality/raidmgmt/pkg/domain/entities/physicaldrive"
+	"github.com/scality/raidmgmt/pkg/domain/entities/raidcontroller"
+	"github.com/scality/raidmgmt/pkg/implementation/blinker"
+)
+
+type MockCommandRunner struct {
+	mock.Mock
+}
+
+func (m *MockCommandRunner) Run(args []string) ([]byte, error) {
+	arguments := m.Called(args)
+
+	return arguments.Get(0).([]byte), arguments.Error(1)
+}
+
+// storcli2Fixture reads a storcli2 JSON fixture from the package testdata.
+func storcli2Fixture(t *testing.T, name string) []byte {
+	t.Helper()
+
+	data, err := os.ReadFile("testdata/storcli2/" + name)
+	require.NoError(t, err)
+
+	return data
+}
+
+// storcli2PDMetadata builds physical-drive metadata with the given EID:Slt id.
+func storcli2PDMetadata(id string) *physicaldrive.Metadata {
+	return &physicaldrive.Metadata{
+		CtrlMetadata: &raidcontroller.Metadata{ID: 0},
+		ID:           id,
+	}
+}
+
+// TestStorCLI2Blink covers the start/stop locate happy paths, including the
+// enclosure and no-enclosure selector forms.
+func TestStorCLI2Blink(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		id       string
+		selector string
+		action   string
+		fixture  string
+		start    bool
+	}{
+		{name: "start with enclosure", id: "252:0", selector: "/c0/e252/s0", action: "start", fixture: "start.json", start: true},
+		{name: "stop with enclosure", id: "252:0", selector: "/c0/e252/s0", action: "stop", fixture: "stop.json", start: false},
+		{name: "start without enclosure", id: "5", selector: "/c0/s5", action: "start", fixture: "start.json", start: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			mockRunner := new(MockCommandRunner)
+			mockRunner.On("Run", []string{tt.selector, tt.action, "locate"}).
+				Return(storcli2Fixture(t, tt.fixture), nil)
+
+			b := blinker.NewStorCLI2(mockRunner)
+
+			var err error
+			if tt.start {
+				err = b.StartBlink(storcli2PDMetadata(tt.id))
+			} else {
+				err = b.StopBlink(storcli2PDMetadata(tt.id))
+			}
+
+			require.NoError(t, err)
+			mockRunner.AssertExpectations(t)
+		})
+	}
+}
+
+// TestStorCLI2BlinkCommandError pins that a runner failure is surfaced.
+func TestStorCLI2BlinkCommandError(t *testing.T) {
+	t.Parallel()
+
+	mockRunner := new(MockCommandRunner)
+	mockRunner.On("Run", []string{"/c0/e252/s0", "start", "locate"}).
+		Return([]byte(nil), errors.New("boom"))
+
+	b := blinker.NewStorCLI2(mockRunner)
+
+	err := b.StartBlink(storcli2PDMetadata("252:0"))
+	require.Error(t, err)
+}
+
+// TestStorCLI2BlinkInvalidSlot pins that an unparseable drive id is rejected
+// before any command is run.
+func TestStorCLI2BlinkInvalidSlot(t *testing.T) {
+	t.Parallel()
+
+	mockRunner := new(MockCommandRunner)
+
+	b := blinker.NewStorCLI2(mockRunner)
+
+	err := b.StopBlink(storcli2PDMetadata(""))
+	require.Error(t, err)
+	mockRunner.AssertNotCalled(t, "Run")
+}

pkg/implementation/logicalvolumemanager/storcli2.go

@@ -20,6 +20,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.
@@ -43,6 +47,8 @@ type StorCLI2 struct {
 	runner commandrunner.CommandRunner
 }
 
+var _ ports.LogicalVolumesManager = &StorCLI2{}
+
 // NewStorCLI2 returns a logical-volume manager backed by the given storcli2 /
 // perccli2 command runner and physical-drive and logical-volume getters.
 func NewStorCLI2(
@@ -132,6 +138,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.runner.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

@@ -9,6 +9,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"
 )
 
@@ -211,3 +212,92 @@ 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")
+}

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"
+			}
+		]
+	}
+}
+]
+}