diff --git a/pkg/implementation/logicalvolumemanager/storcli2.go b/pkg/implementation/logicalvolumemanager/storcli2.go new file mode 100644 index 0000000..9fbe75a --- /dev/null +++ b/pkg/implementation/logicalvolumemanager/storcli2.go @@ -0,0 +1,382 @@ +package logicalvolumemanager + +import ( + "fmt" + "strings" + + "github.com/pkg/errors" + + "github.com/scality/raidmgmt/pkg/domain/entities/logicalvolume" + "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 ( + // storcli2CmdSet is the storcli2 "set" command token. + storcli2CmdSet = "set" + // storcli2CmdAdd and storcli2CmdVD compose the "add vd" command that creates + // a virtual drive. + storcli2CmdAdd = "add" + 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. + storcli2VolumeSelector = "/c%d/v%s" + // storcli2RdCacheFlag and storcli2WrCacheFlag are the read/write cache flags + // of the "set" command. storcli2 has no IO policy flag. + storcli2RdCacheFlag = "rdcache=" + storcli2WrCacheFlag = "wrcache=" + // storcli2DrivesFlag prefixes the "e:s,s,..." drive list of "add vd". + storcli2DrivesFlag = "drives=" + // storcli2RAIDLevelFormat builds the bare RAID-level token of "add vd" + // (e.g. "r0"); storcli's "type=raidN" form is gone in storcli2. + storcli2RAIDLevelFormat = "r%d" +) + +// StorCLI2 manages logical volumes through a storcli2/perccli2 command runner. +// A single implementation serves both binaries; the concrete runner is injected +// at construction time. The current state is read through injected getters: the +// physical-drive getter fills the request drives so creation can be validated, +// and the logical-volume getter both feeds the cache setter and discovers the +// virtual drive created by "add vd". +type StorCLI2 struct { + ports.PhysicalDrivesGetter + ports.LogicalVolumesGetter + + StorCLI2 commandrunner.CommandRunner +} + +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. +func NewStorCLI2( + runner commandrunner.CommandRunner, + physicalDrivesGetter ports.PhysicalDrivesGetter, + logicalVolumesGetter ports.LogicalVolumesGetter, +) *StorCLI2 { + return &StorCLI2{ + PhysicalDrivesGetter: physicalDrivesGetter, + LogicalVolumesGetter: logicalVolumesGetter, + StorCLI2: runner, + } +} + +// SetLVCacheOptions applies the desired cache options to a logical volume, +// emitting only the flags that differ from the current state. storcli2 rejects +// the combined cache syntax and dropped the IO policy, so the read and write +// policies are set through two independent "set" commands and the IO policy is +// ignored. +func (s *StorCLI2) SetLVCacheOptions( + metadata *logicalvolume.Metadata, + desired *logicalvolume.CacheOptions, +) error { + current, err := s.LogicalVolume(metadata) + if err != nil { + return errors.Wrapf(err, "failed to get logical volume %s", metadata.ID) + } + + options, err := storcli2CacheOptions(current.CacheOptions, desired) + if err != nil { + return errors.Wrap(err, "failed to resolve cache options") + } + + selector := fmt.Sprintf(storcli2VolumeSelector, metadata.CtrlMetadata.ID, metadata.ID) + + for _, option := range options { + if err := s.set(selector, option); err != nil { + return errors.Wrapf(err, "failed to set %s", option) + } + } + + return nil +} + +// CreateLV creates a logical volume from a request through "add vd". The +// request drives are filled via the physical-drive getter so the RAID creation +// can be validated, formatted into storcli2's "e:s,s,..." drive list (a single +// enclosure only) and submitted with the bare RAID-level and cache tokens. +// storcli2 reports the new virtual drive's number but not its full state, so +// the volume is rediscovered by matching the request's physical-drive set. +func (s *StorCLI2) CreateLV(request *logicalvolume.Request) (*logicalvolume.LogicalVolume, error) { + physicalDrives := make([]*physicaldrive.PhysicalDrive, 0, len(request.PDrivesMetadata)) + + for _, pdMetadata := range request.PDrivesMetadata { + pd, err := s.PhysicalDrive(pdMetadata) + if err != nil { + return nil, errors.Wrapf(err, "failed to get physical drive %s", pdMetadata.ID) + } + + physicalDrives = append(physicalDrives, pd) + } + + if err := logicalvolume.ValidateRAIDCreation(physicalDrives, request.RAIDLevel); err != nil { + return nil, errors.Wrap(err, "failed to validate RAID creation") + } + + drives, err := storcli2FormatDrives(request.PDrivesMetadata) + if err != nil { + return nil, errors.Wrap(err, "failed to format drives") + } + + args := []string{ + fmt.Sprintf(storcli2ControllerSelector, request.CtrlMetadata.ID), + storcli2CmdAdd, + storcli2CmdVD, + fmt.Sprintf(storcli2RAIDLevelFormat, request.RAIDLevel.Level()), + drives, + } + args = append(args, storcli2CreateCacheFlags(request.CacheOptions)...) + + output, err := s.StorCLI2.Run(args) + if err != nil { + return nil, errors.Wrap(err, "failed to run add vd command") + } + + if _, err := storcli2.Decode(output); err != nil { + return nil, errors.Wrap(err, "add vd command failed") + } + + newLV, err := s.findNewLogicalVolume(request) + if err != nil { + return nil, errors.Wrap(err, "failed to find new logical volume") + } + + return newLV, nil +} + +// DeleteLV deletes the logical volume addressed by the given metadata. A +// nonexistent virtual drive yields a failure payload surfaced by Decode. +func (s *StorCLI2) DeleteLV(metadata *logicalvolume.Metadata) error { + selector := fmt.Sprintf(storcli2VolumeSelector, metadata.CtrlMetadata.ID, metadata.ID) + + output, err := s.StorCLI2.Run([]string{selector, storcli2CmdDelete}) + if err != nil { + return errors.Wrapf(err, "failed to run delete command for logical volume %s", metadata.ID) + } + + if _, err := storcli2.Decode(output); err != nil { + return errors.Wrapf(err, "failed to delete logical volume %s", metadata.ID) + } + + 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. +func (s *StorCLI2) findNewLogicalVolume(request *logicalvolume.Request) ( + *logicalvolume.LogicalVolume, + error, +) { + logicalVolumes, err := s.LogicalVolumes(request.CtrlMetadata) + if err != nil { + return nil, errors.Wrap(err, "failed to list logical volumes") + } + + wanted := make(map[string]struct{}, len(request.PDrivesMetadata)) + for _, pdMetadata := range request.PDrivesMetadata { + wanted[pdMetadata.ID] = struct{}{} + } + + for _, lv := range logicalVolumes { + if storcli2MatchesPDSet(lv.PDrivesMetadata, wanted) { + return lv, nil + } + } + + return nil, errors.New("no logical volume matches the requested physical drives") +} + +// storcli2MatchesPDSet reports whether the physical drives of a volume are +// exactly the wanted set. +func storcli2MatchesPDSet(lvPDs []*physicaldrive.Metadata, wanted map[string]struct{}) bool { + if len(lvPDs) != len(wanted) { + return false + } + + for _, pd := range lvPDs { + if _, ok := wanted[pd.ID]; !ok { + return false + } + } + + return true +} + +// storcli2FormatDrives renders physical-drive metadata into the "drives=e:s,..." +// argument of "add vd". storcli2 names every drive by its "EID:Slt" pair and +// accepts a single shared enclosure, so drives spanning multiple enclosures are +// rejected. +func storcli2FormatDrives(pdsMetadata []*physicaldrive.Metadata) (string, error) { + if len(pdsMetadata) == 0 { + return "", errors.New("no physical drives") + } + + var enclosure string + + slots := make([]string, 0, len(pdsMetadata)) + + for _, pdMetadata := range pdsMetadata { + slot, err := physicaldrive.ParseSlot(pdMetadata.ID) + if err != nil { + return "", errors.Wrapf(err, "failed to parse slot %s", pdMetadata.ID) + } + + if slot.Enclosure == "" { + return "", errors.Errorf("missing enclosure in drive %s", pdMetadata.ID) + } + + switch enclosure { + case "": + enclosure = slot.Enclosure + case slot.Enclosure: + default: + return "", errors.New("multiple enclosures not supported") + } + + slots = append(slots, slot.Bay) + } + + return fmt.Sprintf("%s%s:%s", storcli2DrivesFlag, enclosure, strings.Join(slots, ",")), nil +} + +// storcli2CreateCacheFlags returns the bare write- and read-policy tokens of +// "add vd" for the policies that are known. storcli2 takes the policy as a bare +// token at creation time (the "wrcache="/"rdpolicy=" forms are gone) and has no +// IO policy. +func storcli2CreateCacheFlags(cache *logicalvolume.CacheOptions) []string { + if cache == nil { + return nil + } + + var flags []string + + if cache.WritePolicy != logicalvolume.WritePolicyUnknown { + flags = append(flags, string(cache.WritePolicy)) + } + + if cache.ReadPolicy != logicalvolume.ReadPolicyUnknown { + flags = append(flags, string(cache.ReadPolicy)) + } + + return flags +} + +// storcli2CacheOptions returns the "set" options for the policies that differ +// between current and desired, one per command (storcli2 rejects the combined +// syntax). A changed but unsettable (unknown) policy is an error. +func storcli2CacheOptions(current, desired *logicalvolume.CacheOptions) ([]string, error) { + var options []string + + if desired.ReadPolicy != current.ReadPolicy { + token, ok := storcli2ReadCacheToken(desired.ReadPolicy) + if !ok { + return nil, errors.Errorf("unsettable read policy %q", desired.ReadPolicy) + } + + options = append(options, storcli2RdCacheFlag+token) + } + + if desired.WritePolicy != current.WritePolicy { + token, ok := storcli2WriteCacheToken(desired.WritePolicy) + if !ok { + return nil, errors.Errorf("unsettable write policy %q", desired.WritePolicy) + } + + options = append(options, storcli2WrCacheFlag+token) + } + + return options, nil +} + +// set runs a single "set" command on a volume selector and surfaces the in-JSON +// failure that storcli2 may report regardless of its exit code. +func (s *StorCLI2) set(selector, option string) error { + output, err := s.StorCLI2.Run([]string{selector, storcli2CmdSet, option}) + if err != nil { + return errors.Wrap(err, "failed to run set command") + } + + if _, err := storcli2.Decode(output); err != nil { + return errors.Wrap(err, "set command failed") + } + + return nil +} + +// storcli2ReadCacheToken maps a read policy to its "rdcache" token. An unknown +// policy is not settable and yields ok=false. +func storcli2ReadCacheToken(policy logicalvolume.ReadPolicy) (string, bool) { + switch policy { //nolint:exhaustive // unknown handled by the default + case logicalvolume.ReadPolicyReadAhead: + return "RA", true + case logicalvolume.ReadPolicyNoReadAhead: + return "NoRA", true + default: + return "", false + } +} + +// storcli2WriteCacheToken maps a write policy to its "wrcache" token. An unknown +// policy is not settable and yields ok=false. +func storcli2WriteCacheToken(policy logicalvolume.WritePolicy) (string, bool) { + switch policy { //nolint:exhaustive // unknown handled by the default + case logicalvolume.WritePolicyWriteThrough: + return "WT", true + case logicalvolume.WritePolicyWriteBack: + return "WB", true + case logicalvolume.WritePolicyAlwaysWriteBack: + return "AWB", true + default: + return "", false + } +} diff --git a/pkg/implementation/logicalvolumemanager/storcli2_test.go b/pkg/implementation/logicalvolumemanager/storcli2_test.go new file mode 100644 index 0000000..07dc623 --- /dev/null +++ b/pkg/implementation/logicalvolumemanager/storcli2_test.go @@ -0,0 +1,454 @@ +package logicalvolumemanager_test + +import ( + "os" + "testing" + + "github.com/pkg/errors" + "github.com/stretchr/testify/require" + + "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" +) + +// 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 +} + +func storcli2Metadata() *logicalvolume.Metadata { + return &logicalvolume.Metadata{ + CtrlMetadata: &raidcontroller.Metadata{ID: 0}, + ID: "25", + } +} + +func newStorCLI2LV(cache *logicalvolume.CacheOptions) *logicalvolume.LogicalVolume { + return &logicalvolume.LogicalVolume{ + Metadata: storcli2Metadata(), + CacheOptions: cache, + } +} + +// storcli2Ctrl is the controller every storcli2 manager fixture is scoped to. +func storcli2Ctrl() *raidcontroller.Metadata { + return &raidcontroller.Metadata{ID: 0} +} + +// storcli2CreateRequest builds a two-drive RAID1 create request on the given +// controller; both drives share enclosure 252. +func storcli2CreateRequest(ctrl *raidcontroller.Metadata) *logicalvolume.Request { + return &logicalvolume.Request{ + CtrlMetadata: ctrl, + RAIDLevel: logicalvolume.RAIDLevel1, + PDrivesMetadata: []*physicaldrive.Metadata{ + {CtrlMetadata: ctrl, ID: "252:0"}, + {CtrlMetadata: ctrl, ID: "252:1"}, + }, + CacheOptions: &logicalvolume.CacheOptions{ + WritePolicy: logicalvolume.WritePolicyWriteBack, + ReadPolicy: logicalvolume.ReadPolicyReadAhead, + }, + } +} + +// storcli2AvailablePD returns an unassigned-good drive of uniform size so +// ValidateRAIDCreation accepts it. +func storcli2AvailablePD(metadata *physicaldrive.Metadata) *physicaldrive.PhysicalDrive { + return &physicaldrive.PhysicalDrive{ + Metadata: metadata, + Size: 1000, + Status: physicaldrive.PDStatusUnassignedGood, + } +} + +// TestStorCLI2CreateLV covers the happy path: each request drive is filled and +// validated, "add vd" is issued with the bare RAID-level and cache tokens and a +// single-enclosure drive list, and the new volume is rediscovered by its +// physical-drive set. +func TestStorCLI2CreateLV(t *testing.T) { + t.Parallel() + + ctrl := storcli2Ctrl() + request := storcli2CreateRequest(ctrl) + + mockRunner := new(MockCommandRunner) + mockPDGetter := new(MockPhysicalDrivesGetter) + mockLVGetter := new(MockLogicalVolumesGetter) + + for _, pdMetadata := range request.PDrivesMetadata { + mockPDGetter.On("PhysicalDrive", pdMetadata).Return(storcli2AvailablePD(pdMetadata), nil) + } + + mockRunner.On("Run", []string{"/c0", "add", "vd", "r1", "drives=252:0,1", "wb", "ra"}). + Return(storcli2Fixture(t, "create/success.json"), nil) + + newLV := &logicalvolume.LogicalVolume{ + Metadata: &logicalvolume.Metadata{CtrlMetadata: ctrl, ID: "25"}, + PDrivesMetadata: request.PDrivesMetadata, + } + other := &logicalvolume.LogicalVolume{ + Metadata: &logicalvolume.Metadata{CtrlMetadata: ctrl, ID: "7"}, + PDrivesMetadata: []*physicaldrive.Metadata{{CtrlMetadata: ctrl, ID: "252:9"}}, + } + mockLVGetter.On("LogicalVolumes", ctrl). + Return([]*logicalvolume.LogicalVolume{other, newLV}, nil) + + manager := logicalvolumemanager.NewStorCLI2(mockRunner, mockPDGetter, mockLVGetter) + + lv, err := manager.CreateLV(request) + require.NoError(t, err) + require.Equal(t, "25", lv.ID) + mockRunner.AssertExpectations(t) +} + +// TestStorCLI2CreateLVCommandError pins that an "add vd" failure payload aborts +// before any volume discovery. +func TestStorCLI2CreateLVCommandError(t *testing.T) { + t.Parallel() + + ctrl := storcli2Ctrl() + request := storcli2CreateRequest(ctrl) + + mockRunner := new(MockCommandRunner) + mockPDGetter := new(MockPhysicalDrivesGetter) + mockLVGetter := new(MockLogicalVolumesGetter) + + for _, pdMetadata := range request.PDrivesMetadata { + mockPDGetter.On("PhysicalDrive", pdMetadata).Return(storcli2AvailablePD(pdMetadata), nil) + } + + mockRunner.On("Run", []string{"/c0", "add", "vd", "r1", "drives=252:0,1", "wb", "ra"}). + Return(storcli2Fixture(t, "create/fail.json"), nil) + + manager := logicalvolumemanager.NewStorCLI2(mockRunner, mockPDGetter, mockLVGetter) + + _, err := manager.CreateLV(request) + require.Error(t, err) + mockLVGetter.AssertNotCalled(t, "LogicalVolumes") +} + +// TestStorCLI2CreateLVMultipleEnclosures pins that a request spanning two +// enclosures is rejected before "add vd" is run. +func TestStorCLI2CreateLVMultipleEnclosures(t *testing.T) { + t.Parallel() + + ctrl := storcli2Ctrl() + request := storcli2CreateRequest(ctrl) + request.PDrivesMetadata[1].ID = "253:1" + + mockRunner := new(MockCommandRunner) + mockPDGetter := new(MockPhysicalDrivesGetter) + mockLVGetter := new(MockLogicalVolumesGetter) + + for _, pdMetadata := range request.PDrivesMetadata { + mockPDGetter.On("PhysicalDrive", pdMetadata).Return(storcli2AvailablePD(pdMetadata), nil) + } + + manager := logicalvolumemanager.NewStorCLI2(mockRunner, mockPDGetter, mockLVGetter) + + _, err := manager.CreateLV(request) + require.Error(t, err) + mockRunner.AssertNotCalled(t, "Run") +} + +// TestStorCLI2DeleteLV covers the delete happy path and the two documented +// failure payloads (an invalid VD number and a nonexistent VD). +func TestStorCLI2DeleteLV(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + id string + fixture string + wantErr bool + }{ + {name: "success", id: "25", fixture: "delete/success.json", wantErr: false}, + {name: "invalid", id: "299", fixture: "delete/fail_invalid.json", wantErr: true}, + {name: "not exist", id: "999", fixture: "delete/fail_vdNotExist.json", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctrl := storcli2Ctrl() + metadata := &logicalvolume.Metadata{CtrlMetadata: ctrl, ID: tt.id} + + mockRunner := new(MockCommandRunner) + mockRunner.On("Run", []string{"/c0/v" + tt.id, "delete"}). + Return(storcli2Fixture(t, tt.fixture), nil) + + manager := logicalvolumemanager.NewStorCLI2( + mockRunner, new(MockPhysicalDrivesGetter), new(MockLogicalVolumesGetter), + ) + + err := manager.DeleteLV(metadata) + if tt.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + mockRunner.AssertExpectations(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. +func TestStorCLI2SetLVCacheOptions(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + current *logicalvolume.CacheOptions + desired *logicalvolume.CacheOptions + // calls maps the expected "set" option to the fixture it returns. + calls map[string]string + }{ + { + name: "only read changed", + current: &logicalvolume.CacheOptions{ + ReadPolicy: logicalvolume.ReadPolicyNoReadAhead, + WritePolicy: logicalvolume.WritePolicyWriteBack, + }, + desired: &logicalvolume.CacheOptions{ + ReadPolicy: logicalvolume.ReadPolicyReadAhead, + WritePolicy: logicalvolume.WritePolicyWriteBack, + }, + calls: map[string]string{"rdcache=RA": "cacheoptions/success_rdcache.json"}, + }, + { + name: "only write changed", + current: &logicalvolume.CacheOptions{ + ReadPolicy: logicalvolume.ReadPolicyReadAhead, + WritePolicy: logicalvolume.WritePolicyWriteBack, + }, + desired: &logicalvolume.CacheOptions{ + ReadPolicy: logicalvolume.ReadPolicyReadAhead, + WritePolicy: logicalvolume.WritePolicyWriteThrough, + }, + calls: map[string]string{"wrcache=WT": "cacheoptions/success_wrcache.json"}, + }, + { + name: "both changed", + current: &logicalvolume.CacheOptions{ + ReadPolicy: logicalvolume.ReadPolicyNoReadAhead, + WritePolicy: logicalvolume.WritePolicyWriteBack, + }, + desired: &logicalvolume.CacheOptions{ + ReadPolicy: logicalvolume.ReadPolicyReadAhead, + WritePolicy: logicalvolume.WritePolicyWriteThrough, + }, + calls: map[string]string{ + "rdcache=RA": "cacheoptions/success_rdcache.json", + "wrcache=WT": "cacheoptions/success_wrcache.json", + }, + }, + { + name: "nothing changed", + current: &logicalvolume.CacheOptions{ + ReadPolicy: logicalvolume.ReadPolicyReadAhead, + WritePolicy: logicalvolume.WritePolicyWriteThrough, + }, + desired: &logicalvolume.CacheOptions{ + ReadPolicy: logicalvolume.ReadPolicyReadAhead, + WritePolicy: logicalvolume.WritePolicyWriteThrough, + }, + calls: map[string]string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + mockRunner := new(MockCommandRunner) + mockGetter := new(MockLogicalVolumesGetter) + + metadata := storcli2Metadata() + mockGetter.On("LogicalVolume", metadata).Return(newStorCLI2LV(tt.current), nil) + + for option, fixture := range tt.calls { + mockRunner.On("Run", []string{"/c0/v25", "set", option}). + Return(storcli2Fixture(t, fixture), nil) + } + + manager := logicalvolumemanager.NewStorCLI2(mockRunner, new(MockPhysicalDrivesGetter), mockGetter) + + err := manager.SetLVCacheOptions(metadata, tt.desired) + require.NoError(t, err) + + mockRunner.AssertExpectations(t) + mockRunner.AssertNumberOfCalls(t, "Run", len(tt.calls)) + }) + } +} + +// TestStorCLI2SetLVCacheOptionsGetterError pins that a failure to read the +// current state aborts before any command is run. +func TestStorCLI2SetLVCacheOptionsGetterError(t *testing.T) { + t.Parallel() + + mockRunner := new(MockCommandRunner) + mockGetter := new(MockLogicalVolumesGetter) + + metadata := storcli2Metadata() + mockGetter.On("LogicalVolume", metadata). + Return((*logicalvolume.LogicalVolume)(nil), errors.New("boom")) + + manager := logicalvolumemanager.NewStorCLI2(mockRunner, new(MockPhysicalDrivesGetter), mockGetter) + + err := manager.SetLVCacheOptions(metadata, &logicalvolume.CacheOptions{ + ReadPolicy: logicalvolume.ReadPolicyReadAhead, + }) + require.Error(t, err) + mockRunner.AssertNotCalled(t, "Run") +} + +// TestStorCLI2SetLVCacheOptionsCommandError pins that an in-JSON command +// failure (here storcli's rejected combined syntax, kept as a plain-text +// failure fixture) is surfaced. +func TestStorCLI2SetLVCacheOptionsCommandError(t *testing.T) { + t.Parallel() + + mockRunner := new(MockCommandRunner) + mockGetter := new(MockLogicalVolumesGetter) + + metadata := storcli2Metadata() + mockGetter.On("LogicalVolume", metadata).Return(newStorCLI2LV(&logicalvolume.CacheOptions{ + ReadPolicy: logicalvolume.ReadPolicyNoReadAhead, + WritePolicy: logicalvolume.WritePolicyWriteBack, + }), nil) + mockRunner.On("Run", []string{"/c0/v25", "set", "rdcache=RA"}). + Return(storcli2Fixture(t, "cacheoptions/combined_syntax_error.json"), nil) + + manager := logicalvolumemanager.NewStorCLI2(mockRunner, new(MockPhysicalDrivesGetter), mockGetter) + + err := manager.SetLVCacheOptions(metadata, &logicalvolume.CacheOptions{ + ReadPolicy: logicalvolume.ReadPolicyReadAhead, + WritePolicy: logicalvolume.WritePolicyWriteBack, + }) + require.Error(t, err) +} + +// TestStorCLI2SetLVCacheOptionsUnsettable pins that an unknown desired policy +// (e.g. round-tripped from getter output) is rejected rather than emitted. +func TestStorCLI2SetLVCacheOptionsUnsettable(t *testing.T) { + t.Parallel() + + mockRunner := new(MockCommandRunner) + mockGetter := new(MockLogicalVolumesGetter) + + metadata := storcli2Metadata() + mockGetter.On("LogicalVolume", metadata).Return(newStorCLI2LV(&logicalvolume.CacheOptions{ + ReadPolicy: logicalvolume.ReadPolicyReadAhead, + }), nil) + + manager := logicalvolumemanager.NewStorCLI2(mockRunner, new(MockPhysicalDrivesGetter), mockGetter) + + err := manager.SetLVCacheOptions(metadata, &logicalvolume.CacheOptions{ + ReadPolicy: logicalvolume.ReadPolicyUnknown, + }) + require.Error(t, err) + mockRunner.AssertNotCalled(t, "Run") +} diff --git a/pkg/implementation/logicalvolumemanager/testdata/storcli2/expand/fail.json b/pkg/implementation/logicalvolumemanager/testdata/storcli2/expand/fail.json new file mode 100644 index 0000000..0b7f2e3 --- /dev/null +++ b/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" + } + ] + } +} +] +} diff --git a/pkg/implementation/logicalvolumemanager/testdata/storcli2/expand/success.json b/pkg/implementation/logicalvolumemanager/testdata/storcli2/expand/success.json new file mode 100644 index 0000000..94fa7a6 --- /dev/null +++ b/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" + } +} +] +} diff --git a/pkg/implementation/logicalvolumemanager/testdata/storcli2/migrate/fail.json b/pkg/implementation/logicalvolumemanager/testdata/storcli2/migrate/fail.json deleted file mode 100644 index ffa24c5..0000000 --- a/pkg/implementation/logicalvolumemanager/testdata/storcli2/migrate/fail.json +++ /dev/null @@ -1,5 +0,0 @@ -CLI Version = 008.0005.0000.0010 Feb 22, 2023 -Operating system = Linux4.18.0-553.115.1.el8_10.x86_64 -Status = Failure -Description = syntax error, unexpected TOKEN_MIGRATE, expecting TOKEN_ERASE or TOKEN_INIT or TOKEN_CC or TOKEN_LOCATE - diff --git a/pkg/implementation/physicaldrivegetter/storcli2_jbod.go b/pkg/implementation/physicaldrivegetter/storcli2_jbod.go new file mode 100644 index 0000000..d3b5197 --- /dev/null +++ b/pkg/implementation/physicaldrivegetter/storcli2_jbod.go @@ -0,0 +1,61 @@ +package physicaldrivegetter + +import ( + "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/storcli2" +) + +const ( + // storcli2CmdSet is the storcli2 "set" command token. + storcli2CmdSet = "set" + // storcli2JBODOption converts a drive to the JBOD state. + storcli2JBODOption = "jbod" + // storcli2UConfOption converts a drive back to the unconfigured state; + // storcli2 dropped storcli's "delete jbod". + storcli2UConfOption = "uconf" +) + +var _ ports.JBODSetter = &StorCLI2{} + +// EnableJBOD converts a drive to the JBOD state ("set jbod"). It changes only +// the drive state, not its status. +func (s *StorCLI2) EnableJBOD(metadata *physicaldrive.Metadata) error { + if err := s.setDriveState(metadata, storcli2JBODOption); err != nil { + return errors.Wrap(err, "failed to enable JBOD") + } + + return nil +} + +// DisableJBOD converts a JBOD drive back to the unconfigured state +// ("set uconf"); storcli's "delete jbod" no longer parses. +func (s *StorCLI2) DisableJBOD(metadata *physicaldrive.Metadata) error { + if err := s.setDriveState(metadata, storcli2UConfOption); err != nil { + return errors.Wrap(err, "failed to disable JBOD") + } + + return nil +} + +// setDriveState runs "set " on a drive selector and surfaces the in-JSON +// failure that storcli2 may report regardless of its exit code. +func (s *StorCLI2) setDriveState(metadata *physicaldrive.Metadata, state 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, storcli2CmdSet, state}) + if err != nil { + return errors.Wrapf(err, "failed to run set %s command", state) + } + + if _, err := storcli2.Decode(output); err != nil { + return errors.Wrapf(err, "set %s command failed", state) + } + + return nil +} diff --git a/pkg/implementation/physicaldrivegetter/storcli2_jbod_test.go b/pkg/implementation/physicaldrivegetter/storcli2_jbod_test.go new file mode 100644 index 0000000..600ddfd --- /dev/null +++ b/pkg/implementation/physicaldrivegetter/storcli2_jbod_test.go @@ -0,0 +1,79 @@ +package physicaldrivegetter + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/scality/raidmgmt/pkg/domain/entities/physicaldrive" + "github.com/scality/raidmgmt/pkg/domain/entities/raidcontroller" +) + +// storcli2Success is a minimal success envelope; the captured JBOD fixtures are +// failures only (a success needs hardware, see ARTESCA-17649). +const storcli2Success = `{"Controllers":[{"Command Status":{"Status":"Success"}}]}` + +func storcli2PDMetadata() *physicaldrive.Metadata { + return &physicaldrive.Metadata{ + CtrlMetadata: &raidcontroller.Metadata{ID: 0}, + ID: "306:0", + } +} + +func TestStorCLI2EnableJBOD(t *testing.T) { + t.Parallel() + + mockRunner := new(MockCommandRunner) + mockRunner.On("Run", []string{"/c0/e306/s0", "set", "jbod"}). + Return([]byte(storcli2Success), nil) + + s := NewStorCLI2(mockRunner) + + require.NoError(t, s.EnableJBOD(storcli2PDMetadata())) + mockRunner.AssertExpectations(t) +} + +func TestStorCLI2DisableJBOD(t *testing.T) { + t.Parallel() + + mockRunner := new(MockCommandRunner) + mockRunner.On("Run", []string{"/c0/e306/s0", "set", "uconf"}). + Return([]byte(storcli2Success), nil) + + s := NewStorCLI2(mockRunner) + + require.NoError(t, s.DisableJBOD(storcli2PDMetadata())) + mockRunner.AssertExpectations(t) +} + +// TestStorCLI2EnableJBODFailure pins that the in-JSON failure payload reported +// regardless of exit code is surfaced as an error. +func TestStorCLI2EnableJBODFailure(t *testing.T) { + t.Parallel() + + mockRunner := new(MockCommandRunner) + mockRunner.On("Run", []string{"/c0/e306/s0", "set", "jbod"}). + Return(storcli2Fixture(t, "jbod/enable/fail.json"), nil) + + s := NewStorCLI2(mockRunner) + + err := s.EnableJBOD(storcli2PDMetadata()) + require.Error(t, err) + require.ErrorContains(t, err, "wrong state") +} + +// TestStorCLI2JBODSelectorError pins that an unparseable slot aborts before any +// command is run. +func TestStorCLI2JBODSelectorError(t *testing.T) { + t.Parallel() + + mockRunner := new(MockCommandRunner) + s := NewStorCLI2(mockRunner) + + err := s.EnableJBOD(&physicaldrive.Metadata{ + CtrlMetadata: &raidcontroller.Metadata{ID: 0}, + ID: "", + }) + require.Error(t, err) + mockRunner.AssertNotCalled(t, "Run") +} diff --git a/pkg/implementation/storcli2/envelope.go b/pkg/implementation/storcli2/envelope.go index 68e92e1..9ba3bd8 100644 --- a/pkg/implementation/storcli2/envelope.go +++ b/pkg/implementation/storcli2/envelope.go @@ -39,13 +39,15 @@ type ( // DetailedStatus carries per-target error details. storcli2 adds an // "ErrType" field and identifies the target either by "VD" or by // "EID:Slt"/"PID" (PID may be the int persistent id or the string "-"). + // ErrCd is likewise "any": an int error code on failure, the string "-" on + // success. DetailedStatus struct { VD any `json:"VD,omitempty"` EIDSlot string `json:"EID:Slt,omitempty"` PID any `json:"PID,omitempty"` Status string `json:"Status"` ErrType string `json:"ErrType,omitempty"` - ErrCd int `json:"ErrCd"` + ErrCd any `json:"ErrCd"` ErrMsg string `json:"ErrMsg"` } ) diff --git a/tests/testdata-tools/README.md b/tests/testdata-tools/README.md index 1b27580..cf03698 100644 --- a/tests/testdata-tools/README.md +++ b/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`) diff --git a/tests/testdata-tools/collect_storcli2_testdata.sh b/tests/testdata-tools/collect_storcli2_testdata.sh index 687b9f9..657d514 100755 --- a/tests/testdata-tools/collect_storcli2_testdata.sh +++ b/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