From ed21e304b890101827b4dddea2be059ef25ec2b6 Mon Sep 17 00:00:00 2001 From: Tom Nabarro Date: Mon, 20 May 2024 17:16:15 +0100 Subject: [PATCH] DAOS-15841 control: Improve coverage and fs mocking in storage packages (#14379) Some control-plane storage unit tests are inadvertently calling into test runner host OS filesystem. Fix by consolidating SystemProvider interface and applying storage subsystem provider stubs by default in the unit test framework. As a result coverage can be improved by exercising a greater number of code paths. Signed-off-by: Tom Nabarro --- src/control/provider/system/mocks.go | 73 ++++++- src/control/provider/system/system_linux.go | 32 ++- .../provider/system/system_linux_test.go | 6 +- src/control/server/ctl_storage_rpc_test.go | 1 + src/control/server/instance_storage.go | 19 +- src/control/server/instance_storage_test.go | 197 ++++++++++++++---- src/control/server/instance_superblock.go | 37 ++-- .../server/instance_superblock_test.go | 7 +- src/control/server/instance_test.go | 34 ++- .../server/storage/metadata/provider.go | 49 +++-- .../server/storage/metadata/provider_test.go | 41 ++-- src/control/server/storage/mount/provider.go | 18 +- .../server/storage/mount/provider_test.go | 8 +- src/control/server/storage/provider.go | 3 +- src/control/server/storage/scm/mocks.go | 4 + src/control/server/storage/scm/provider.go | 25 +-- .../server/storage/scm/provider_test.go | 2 +- 17 files changed, 390 insertions(+), 166 deletions(-) diff --git a/src/control/provider/system/mocks.go b/src/control/provider/system/mocks.go index ab927b760c4..dc52d7b3b96 100644 --- a/src/control/provider/system/mocks.go +++ b/src/control/provider/system/mocks.go @@ -1,5 +1,5 @@ // -// (C) Copyright 2022 Intel Corporation. +// (C) Copyright 2022-2024 Intel Corporation. // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -44,10 +44,19 @@ type ( SourceToTarget map[string]string GetfsIndex int GetfsUsageResps []GetfsUsageRetval - GetFsTypeRes *FsType - GetFsTypeErr []error + GetfsTypeRes *FsType + GetfsTypeErr []error StatErrors map[string]error RealStat bool + ReadFileResults map[string][]byte + ReadFileErrors map[string]error + RealReadFile bool + GeteuidRes int + GetegidRes int + MkdirErr error + RealMkdir bool + RemoveAllErr error + RealRemoveAll bool } // MockSysProvider gives a mock SystemProvider implementation. @@ -57,7 +66,7 @@ type ( cfg MockSysConfig isMounted MountMap IsMountedInputs []string - GetFsTypeCount int + GetfsTypeCount int } ) @@ -162,17 +171,17 @@ func (msp *MockSysProvider) GetfsUsage(_ string) (uint64, uint64, error) { return resp.Total, resp.Avail, resp.Err } -func (msp *MockSysProvider) GetFsType(path string) (*FsType, error) { - idx := msp.GetFsTypeCount - msp.GetFsTypeCount++ +func (msp *MockSysProvider) GetfsType(path string) (*FsType, error) { + idx := msp.GetfsTypeCount + msp.GetfsTypeCount++ var err error var result *FsType - if idx < len(msp.cfg.GetFsTypeErr) { - err = msp.cfg.GetFsTypeErr[idx] + if idx < len(msp.cfg.GetfsTypeErr) { + err = msp.cfg.GetfsTypeErr[idx] } if err == nil { - result = msp.cfg.GetFsTypeRes + result = msp.cfg.GetfsTypeRes } return result, err @@ -191,6 +200,50 @@ func (msp *MockSysProvider) Stat(path string) (os.FileInfo, error) { return nil, msp.cfg.StatErrors[path] } +func (msp *MockSysProvider) ReadFile(path string) ([]byte, error) { + msp.RLock() + defer msp.RUnlock() + + if msp.cfg.RealReadFile { + return os.ReadFile(path) + } + + // default return value for missing key is nil so + // add entries to indicate path failure e.g. perms or not-exist + if msp.cfg.ReadFileErrors[path] != nil { + return nil, msp.cfg.ReadFileErrors[path] + } + + return msp.cfg.ReadFileResults[path], nil +} + +// Geteuid returns the numeric effective user id of the caller. +func (msp *MockSysProvider) Geteuid() int { + return msp.cfg.GeteuidRes +} + +// Getegid returns the numeric effective group id of the caller. +func (msp *MockSysProvider) Getegid() int { + return msp.cfg.GetegidRes +} + +// Mkdir creates a new directory with the specified name and permission +// bits (before umask). +func (msp *MockSysProvider) Mkdir(path string, flags os.FileMode) error { + if msp.cfg.RealMkdir { + return os.Mkdir(path, flags) + } + return msp.cfg.MkdirErr +} + +// RemoveAll removes path and any children it contains. +func (msp *MockSysProvider) RemoveAll(path string) error { + if msp.cfg.RealRemoveAll { + return os.RemoveAll(path) + } + return msp.cfg.RemoveAllErr +} + func NewMockSysProvider(log logging.Logger, cfg *MockSysConfig) *MockSysProvider { if cfg == nil { cfg = &MockSysConfig{} diff --git a/src/control/provider/system/system_linux.go b/src/control/provider/system/system_linux.go index d5986ace89a..e3fb439c0d8 100644 --- a/src/control/provider/system/system_linux.go +++ b/src/control/provider/system/system_linux.go @@ -1,5 +1,5 @@ // -// (C) Copyright 2019-2022 Intel Corporation. +// (C) Copyright 2019-2024 Intel Corporation. // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -205,8 +205,8 @@ type FsType struct { NoSUID bool } -// GetFsType retrieves the filesystem type for a path. -func (s LinuxProvider) GetFsType(path string) (*FsType, error) { +// GetfsType retrieves the filesystem type for a path. +func (s LinuxProvider) GetfsType(path string) (*FsType, error) { stBuf := new(unix.Statfs_t) if err := unix.Statfs(path, stBuf); err != nil { @@ -313,6 +313,11 @@ func (s LinuxProvider) Stat(path string) (os.FileInfo, error) { return os.Stat(path) } +// ReadFile reads the named file and returns the contents. +func (s LinuxProvider) ReadFile(path string) ([]byte, error) { + return os.ReadFile(path) +} + // Chmod changes the mode of the specified path. func (s LinuxProvider) Chmod(path string, mode os.FileMode) error { return os.Chmod(path, mode) @@ -337,3 +342,24 @@ func parseFsType(input string) string { return FsTypeUnknown } } + +// Geteuid returns the numeric effective user id of the caller. +func (s LinuxProvider) Geteuid() int { + return os.Geteuid() +} + +// Getegid returns the numeric effective group id of the caller. +func (s LinuxProvider) Getegid() int { + return os.Getegid() +} + +// Mkdir creates a new directory with the specified name and permission +// bits (before umask). +func (s LinuxProvider) Mkdir(name string, perm os.FileMode) error { + return os.Mkdir(name, perm) +} + +// RemoveAll removes path and any children it contains. +func (s LinuxProvider) RemoveAll(path string) error { + return os.RemoveAll(path) +} diff --git a/src/control/provider/system/system_linux_test.go b/src/control/provider/system/system_linux_test.go index 4a1b0c0d3eb..dc9e6b21a04 100644 --- a/src/control/provider/system/system_linux_test.go +++ b/src/control/provider/system/system_linux_test.go @@ -1,5 +1,5 @@ // -// (C) Copyright 2019-2022 Intel Corporation. +// (C) Copyright 2019-2024 Intel Corporation. // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -159,7 +159,7 @@ func TestParseFsType(t *testing.T) { } } -func TestSystemLinux_GetFsType(t *testing.T) { +func TestSystemLinux_GetfsType(t *testing.T) { for name, tc := range map[string]struct { path string expResult *FsType @@ -181,7 +181,7 @@ func TestSystemLinux_GetFsType(t *testing.T) { }, } { t.Run(name, func(t *testing.T) { - result, err := DefaultProvider().GetFsType(tc.path) + result, err := DefaultProvider().GetfsType(tc.path) test.CmpErr(t, tc.expErr, err) if diff := cmp.Diff(tc.expResult, result); diff != "" { diff --git a/src/control/server/ctl_storage_rpc_test.go b/src/control/server/ctl_storage_rpc_test.go index 4df8b2b0e65..42d817802c9 100644 --- a/src/control/server/ctl_storage_rpc_test.go +++ b/src/control/server/ctl_storage_rpc_test.go @@ -1659,6 +1659,7 @@ func TestServer_CtlSvc_StorageFormat(t *testing.T) { IsMountedBool: tc.scmMounted, GetfsStr: getFsRetStr, SourceToTarget: devToMount, + RealReadFile: true, } if tc.sClass == storage.ClassRam { total := uint64(1234) diff --git a/src/control/server/instance_storage.go b/src/control/server/instance_storage.go index 00691ae69e4..71dc0ca181e 100644 --- a/src/control/server/instance_storage.go +++ b/src/control/server/instance_storage.go @@ -29,13 +29,17 @@ func (ei *EngineInstance) GetStorage() *storage.Provider { // MountMetadata mounts the configured control metadata location. func (ei *EngineInstance) MountMetadata() error { - ei.log.Debug("checking if metadata is mounted") + msgIdx := fmt.Sprintf("instance %d", ei.Index()) + + msgCheck := fmt.Sprintf("%s: checking if metadata is mounted", msgIdx) + ei.log.Trace(msgCheck) + isMounted, err := ei.storage.ControlMetadataIsMounted() if err != nil { - return errors.Wrap(err, "checking if metadata is mounted") + return errors.WithMessage(err, msgCheck) } - ei.log.Debugf("IsMounted: %v", isMounted) + ei.log.Debugf("%s: IsMounted: %v", msgIdx, isMounted) if isMounted { return nil } @@ -47,10 +51,17 @@ func (ei *EngineInstance) MountMetadata() error { // at the mountpoint specified in the configuration. If the device is already // mounted, the function returns nil, indicating success. func (ei *EngineInstance) MountScm() error { + msgIdx := fmt.Sprintf("instance %d", ei.Index()) + + msgCheck := fmt.Sprintf("%s: checking if scm is mounted", msgIdx) + ei.log.Trace(msgCheck) + isMounted, err := ei.storage.ScmIsMounted() if err != nil && !os.IsNotExist(errors.Cause(err)) { - return errors.WithMessage(err, "failed to check SCM mount") + return errors.WithMessage(err, msgCheck) } + + ei.log.Debugf("%s: IsMounted: %v", msgIdx, isMounted) if isMounted { return nil } diff --git a/src/control/server/instance_storage_test.go b/src/control/server/instance_storage_test.go index ac7ec5dd7af..3685ae14d8c 100644 --- a/src/control/server/instance_storage_test.go +++ b/src/control/server/instance_storage_test.go @@ -10,6 +10,7 @@ import ( "context" "fmt" "os" + "path/filepath" "sync" "testing" "time" @@ -27,23 +28,30 @@ import ( "github.com/daos-stack/daos/src/control/server/storage/scm" ) -func TestIOEngineInstance_MountControlMetadata(t *testing.T) { - cfg := &storage.Config{ - ControlMetadata: storage.ControlMetadata{ - Path: "/dontcare", - DevicePath: "/dev/dontcare", - }, - Tiers: storage.TierConfigs{ - { - Class: storage.ClassRam, - Scm: storage.ScmConfig{ - MountPoint: defaultStoragePath, - RamdiskSize: 1, - }, +var mockRamCfg = storage.Config{ + ControlMetadata: storage.ControlMetadata{ + Path: "/dontcare", + DevicePath: "/dev/dontcare", + }, + Tiers: storage.TierConfigs{ + { + Class: storage.ClassRam, + Scm: storage.ScmConfig{ + MountPoint: defaultStoragePath, + RamdiskSize: 1, }, }, - } + { + Class: storage.ClassNvme, + Bdev: storage.BdevConfig{ + DeviceList: new(storage.BdevDeviceList), + DeviceRoles: storage.BdevRoles{storage.BdevRoleAll}, + }, + }, + }, +} +func TestIOEngineInstance_MountControlMetadata(t *testing.T) { for name, tc := range map[string]struct { meta *storage.MockMetadataProvider sysCfg *system.MockSysConfig @@ -84,11 +92,12 @@ func TestIOEngineInstance_MountControlMetadata(t *testing.T) { } ec := engine.MockConfig(). - WithStorageControlMetadataPath(cfg.ControlMetadata.Path). - WithStorageControlMetadataDevice(cfg.ControlMetadata.DevicePath) + WithStorageControlMetadataPath(mockRamCfg.ControlMetadata.Path). + WithStorageControlMetadataDevice(mockRamCfg.ControlMetadata.DevicePath) runner := engine.NewRunner(log, ec) sysProv := system.NewMockSysProvider(log, tc.sysCfg) - provider := storage.MockProvider(log, 0, cfg, sysProv, nil, nil, tc.meta) + provider := storage.MockProvider(log, 0, &mockRamCfg, sysProv, nil, nil, + tc.meta) instance := NewEngineInstance(log, provider, nil, runner) gotErr := instance.MountMetadata() @@ -352,39 +361,122 @@ func (tly *tally) fakePublish(evt *events.RASEvent) { func TestIOEngineInstance_awaitStorageReady(t *testing.T) { errStarted := errors.New("already started") + dev := "/dev/foo" + mnt := "/mnt/test" dcpmCfg := engine.MockConfig().WithStorage( storage.NewTierConfig(). WithStorageClass(storage.ClassDcpm.String()). - WithScmMountPoint("/mnt/test"). - WithScmDeviceList("/dev/foo"), + WithScmMountPoint(mnt). + WithScmDeviceList(dev), ) for name, tc := range map[string]struct { - engineStarted bool - needsScmFormat bool - hasSB bool - engineIndex uint32 - expFmtType string - expErr error + engineStarted bool + storageCfg *storage.Config + fsStr string + fsErr error + sbSet bool + engineIndex uint32 + isMounted bool + isMountedErr error + mountErr error + unmountErr error + readErr error + metaNeedsFmt bool + metaNeedsFmtErr error + expNoWait bool + expFmtType string + expErr error }{ "already started": { engineStarted: true, + fsStr: "ext4", expErr: errStarted, }, - "no need to format and existing superblock": { - hasSB: true, + "no need to format and superblock set": { + sbSet: true, + fsStr: "ext4", + isMounted: true, + expNoWait: true, }, - "needs scm format": { - needsScmFormat: true, - expFmtType: "SCM", + "no need to format and superblock set; mount fails": { + fsStr: "ext4", + mountErr: errors.New("bad mount"), + expFmtType: "Metadata", + }, + "filesystem exists and superblock set; mountpoint missing": { + sbSet: true, + fsStr: "ext4", + isMountedErr: os.ErrNotExist, + expErr: storage.FaultDeviceWithFsNoMountpoint(dev, mnt), + }, + "mount check fails": { + sbSet: true, + fsStr: "ext4", + isMountedErr: errors.New("fail"), + expFmtType: "SCM", + }, + "no need to format: superblock exists but not yet set": { + fsStr: "ext4", + isMounted: true, + expNoWait: true, }, - "needs metadata format": { + "superblock not found; needs metadata format": { + fsStr: "ext4", + readErr: os.ErrNotExist, expFmtType: "Metadata", }, - "engine index 1": { + "needs scm format": { + fsStr: "none", + expFmtType: "SCM", + }, + "engine index 1; needs metadata format": { engineIndex: 1, + fsStr: "ext4", + readErr: os.ErrNotExist, expFmtType: "Metadata", }, + "metadata path with dcpm": { + storageCfg: &storage.Config{ + ControlMetadata: storage.ControlMetadata{ + Path: "/dontcare", + DevicePath: "/dev/dontcare", + }, + Tiers: storage.TierConfigs{ + {Class: storage.ClassDcpm}, + }, + }, + expErr: storage.FaultBdevConfigRolesWithDCPM, + }, + "no device roles with metadata path configured": { + storageCfg: &storage.Config{ + ControlMetadata: storage.ControlMetadata{ + Path: "/dontcare", + DevicePath: "/dev/dontcare", + }, + Tiers: storage.TierConfigs{ + {Class: storage.ClassRam}, + }, + }, + expErr: storage.FaultBdevConfigControlMetadataNoRoles, + }, + "metadata path; no metadata format needed; scm format fails": { + storageCfg: &mockRamCfg, + isMounted: true, + unmountErr: errors.New("ramdisk busy"), + expErr: errors.New("format ramdisk: failed to clear existing mount: failed to unmount"), + }, + "metadata path; no metadata format needed; scm format succeeds; superblock intact": { + storageCfg: &mockRamCfg, + isMounted: true, + expNoWait: true, + }, + "metadata path; no metadata format needed; scm format succeeds; no superblock": { + storageCfg: &mockRamCfg, + isMounted: true, + readErr: os.ErrNotExist, + expFmtType: "Metadata", + }, } { t.Run(name, func(t *testing.T) { log, buf := logging.NewTestLogger(t.Name()) @@ -396,22 +488,43 @@ func TestIOEngineInstance_awaitStorageReady(t *testing.T) { } runner := engine.NewTestRunner(trc, dcpmCfg) - fs := "none" - if !tc.needsScmFormat { - fs = "ext4" + if tc.storageCfg == nil { + tc.storageCfg = &dcpmCfg.Storage + } + + msc := system.MockSysConfig{ + IsMountedBool: tc.isMounted, + IsMountedErr: tc.isMountedErr, + MountErr: tc.mountErr, + UnmountErr: tc.unmountErr, + GetfsStr: tc.fsStr, + GetfsErr: tc.fsErr, + } + if tc.readErr != nil { + storagePath := mnt + ctlMD := tc.storageCfg.ControlMetadata + if ctlMD.HasPath() { + storagePath = ctlMD.EngineDirectory(uint(tc.engineIndex)) + } + sbPath := filepath.Join(storagePath, "superblock") + t.Logf("setting readfile err %s for path %s", tc.readErr.Error(), sbPath) + msc.ReadFileErrors = map[string]error{sbPath: tc.readErr} + } + smbc := scm.MockBackendConfig{} + mmp := &storage.MockMetadataProvider{ + NeedsFormatRes: tc.metaNeedsFmt, + NeedsFormatErr: tc.metaNeedsFmtErr, } - msc := system.MockSysConfig{GetfsStr: fs} - mbc := scm.MockBackendConfig{} - mp := storage.NewProvider(log, 0, &dcpmCfg.Storage, + mp := storage.NewProvider(log, 0, tc.storageCfg, system.NewMockSysProvider(log, &msc), - scm.NewMockProvider(log, &mbc, &msc), - nil, nil) + scm.NewMockProvider(log, &smbc, &msc), + nil, mmp) engine := NewEngineInstance(log, mp, nil, runner) engine.setIndex(tc.engineIndex) - if tc.hasSB { + if tc.sbSet { engine.setSuperblock(&Superblock{ Rank: ranklist.NewRankPtr(0), ValidRank: true, }) @@ -427,7 +540,7 @@ func TestIOEngineInstance_awaitStorageReady(t *testing.T) { gotErr := engine.awaitStorageReady(ctx) test.CmpErr(t, tc.expErr, gotErr) - if tc.expErr == errStarted || tc.hasSB == true { + if tc.expErr != nil || tc.expNoWait == true { return } diff --git a/src/control/server/instance_superblock.go b/src/control/server/instance_superblock.go index cf4432969bb..624d2e40613 100644 --- a/src/control/server/instance_superblock.go +++ b/src/control/server/instance_superblock.go @@ -1,5 +1,5 @@ // -// (C) Copyright 2019-2023 Intel Corporation. +// (C) Copyright 2019-2024 Intel Corporation. // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -7,7 +7,7 @@ package server import ( - "io/ioutil" + "fmt" "os" "path/filepath" @@ -163,18 +163,26 @@ func (ei *EngineInstance) WriteSuperblock() error { return WriteSuperblock(ei.superblockPath(), ei.getSuperblock()) } -// ReadSuperblock reads the instance's superblock -// from storage. +// ReadSuperblock reads the instance's superblock from storage. func (ei *EngineInstance) ReadSuperblock() error { if err := ei.MountMetadata(); err != nil { return errors.Wrap(err, "failed to mount control metadata device") } - sb, err := ReadSuperblock(ei.superblockPath()) + msgIdx := fmt.Sprintf("instance %d", ei.Index()) + sbPath := ei.superblockPath() + ei.log.Tracef("%s: read sb: %q", msgIdx, sbPath) + + data, err := ei.storage.Sys.ReadFile(sbPath) if err != nil { - ei.log.Debugf("instance %d: failed to read superblock at %s: %s", ei.Index(), ei.superblockPath(), err) - return errors.Wrap(err, "failed to read instance superblock") + return errors.Wrapf(err, "%s: failed to read Superblock from %s", msgIdx, sbPath) + } + + sb := &Superblock{} + if err := sb.Unmarshal(data); err != nil { + return err } + ei.setSuperblock(sb) return nil @@ -198,18 +206,3 @@ func WriteSuperblock(sbPath string, sb *Superblock) error { return errors.Wrapf(common.WriteFileAtomic(sbPath, data, 0600), "Failed to write Superblock to %s", sbPath) } - -// ReadSuperblock reads a Superblock from storage. -func ReadSuperblock(sbPath string) (*Superblock, error) { - data, err := ioutil.ReadFile(sbPath) - if err != nil { - return nil, errors.Wrapf(err, "Failed to read Superblock from %s", sbPath) - } - - sb := &Superblock{} - if err := sb.Unmarshal(data); err != nil { - return nil, err - } - - return sb, nil -} diff --git a/src/control/server/instance_superblock_test.go b/src/control/server/instance_superblock_test.go index c01196e1a69..4dee3eceefb 100644 --- a/src/control/server/instance_superblock_test.go +++ b/src/control/server/instance_superblock_test.go @@ -1,5 +1,5 @@ // -// (C) Copyright 2020-2023 Intel Corporation. +// (C) Copyright 2020-2024 Intel Corporation. // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -44,9 +44,12 @@ func TestServer_Instance_createSuperblock(t *testing.T) { r := engine.NewRunner(log, cfg) msc := &sysprov.MockSysConfig{ IsMountedBool: true, + RealReadFile: true, } mbc := &scm.MockBackendConfig{} - mp := storage.NewProvider(log, 0, &cfg.Storage, sysprov.NewMockSysProvider(log, msc), scm.NewMockProvider(log, mbc, msc), nil, nil) + mp := storage.NewProvider(log, 0, &cfg.Storage, + sysprov.NewMockSysProvider(log, msc), + scm.NewMockProvider(log, mbc, msc), nil, nil) ei := NewEngineInstance(log, mp, nil, r). WithHostFaultDomain(system.MustCreateFaultDomainFromString("/host1")) ei.fsRoot = testDir diff --git a/src/control/server/instance_test.go b/src/control/server/instance_test.go index e4ec0ad84fb..d4eb86cce75 100644 --- a/src/control/server/instance_test.go +++ b/src/control/server/instance_test.go @@ -1,5 +1,5 @@ // -// (C) Copyright 2019-2023 Intel Corporation. +// (C) Copyright 2019-2024 Intel Corporation. // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -25,8 +25,10 @@ import ( "github.com/daos-stack/daos/src/control/lib/atm" "github.com/daos-stack/daos/src/control/lib/ranklist" "github.com/daos-stack/daos/src/control/logging" + sysprov "github.com/daos-stack/daos/src/control/provider/system" "github.com/daos-stack/daos/src/control/server/engine" "github.com/daos-stack/daos/src/control/server/storage" + "github.com/daos-stack/daos/src/control/server/storage/scm" "github.com/daos-stack/daos/src/control/system" ) @@ -113,26 +115,40 @@ func TestServer_Instance_updateFaultDomainInSuperblock(t *testing.T) { testDir, cleanupDir := test.CreateTestDir(t) defer cleanupDir() - inst := getTestEngineInstance(log).WithHostFaultDomain(tc.newDomain) - inst.fsRoot = testDir - inst._superblock = tc.superblock - - sbPath := inst.superblockPath() + // Use real os.ReadFile in MockSysProvider to test superblock logic. + cfg := engine.MockConfig().WithStorage( + storage.NewTierConfig(). + WithStorageClass("ram"). + WithScmMountPoint("/foo/bar"), + ) + runner := engine.NewRunner(log, cfg) + sysCfg := sysprov.MockSysConfig{RealReadFile: true} + sysProv := sysprov.NewMockSysProvider(log, &sysCfg) + scmProv := scm.NewMockProvider(log, &scm.MockBackendConfig{}, &sysCfg) + storage := storage.MockProvider(log, 0, &cfg.Storage, sysProv, scmProv, + nil, nil) + + ei := NewEngineInstance(log, storage, nil, runner). + WithHostFaultDomain(tc.newDomain) + ei.fsRoot = testDir + ei._superblock = tc.superblock + + sbPath := ei.superblockPath() if err := os.MkdirAll(filepath.Dir(sbPath), 0755); err != nil { t.Fatalf("failed to make test superblock dir: %s", err.Error()) } - err := inst.updateFaultDomainInSuperblock() - + err := ei.updateFaultDomainInSuperblock() test.CmpErr(t, tc.expErr, err) // Ensure the newer value in the instance was written to the superblock - newSB, err := ReadSuperblock(sbPath) + err = ei.ReadSuperblock() if tc.expWritten { if err != nil { t.Fatalf("can't read expected superblock: %s", err.Error()) } + newSB := ei.getSuperblock() if newSB == nil { t.Fatalf("expected non-nil superblock") } diff --git a/src/control/server/storage/metadata/provider.go b/src/control/server/storage/metadata/provider.go index e060604e529..ba3b3110ee8 100644 --- a/src/control/server/storage/metadata/provider.go +++ b/src/control/server/storage/metadata/provider.go @@ -1,5 +1,5 @@ // -// (C) Copyright 2022-2023 Intel Corporation. +// (C) Copyright 2022-2024 Intel Corporation. // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -11,32 +11,35 @@ import ( "path/filepath" "strings" + "github.com/pkg/errors" + "github.com/daos-stack/daos/src/control/logging" "github.com/daos-stack/daos/src/control/provider/system" "github.com/daos-stack/daos/src/control/server/storage" "github.com/daos-stack/daos/src/control/server/storage/mount" - "github.com/pkg/errors" ) -// SystemProvider provides operating system capabilities. -type SystemProvider interface { - system.IsMountedProvider - system.MountProvider - Mkfs(req system.MkfsReq) error - Getfs(device string) (string, error) - Chown(string, int, int) error - Stat(string) (os.FileInfo, error) - GetFsType(path string) (*system.FsType, error) -} - const defaultDevFS = "ext4" -// Provider provides management functionality for metadata storage. -type Provider struct { - log logging.Logger - sys SystemProvider - mounter storage.MountProvider -} +type ( + // SystemProvider provides operating system capabilities. + SystemProvider interface { + Chown(string, int, int) error + Getfs(device string) (string, error) + GetfsType(path string) (*system.FsType, error) + Mkdir(string, os.FileMode) error + Mkfs(req system.MkfsReq) error + RemoveAll(string) error + Stat(string) (os.FileInfo, error) + } + + // Provider provides management functionality for metadata storage. + Provider struct { + log logging.Logger + sys SystemProvider + mounter storage.MountProvider + } +) // Format formats the storage used for control metadata, if it is a separate device. // If the storage location is on an existing partition, the format of the existing filesystem is @@ -119,7 +122,7 @@ func (p *Provider) setupMountPoint(req storage.MetadataFormatRequest) error { func (p *Provider) setupRootDir(req storage.MetadataFormatRequest) error { fsPath := req.RootPath for { - fsType, err := p.sys.GetFsType(fsPath) + fsType, err := p.sys.GetfsType(fsPath) if err == nil { if !p.isUsableFS(fsType, fsPath) { return FaultBadFilesystem(fsType) @@ -175,11 +178,11 @@ func (p *Provider) isUsableFS(fs *system.FsType, path string) bool { } func (p *Provider) setupDataDir(req storage.MetadataFormatRequest) error { - if err := os.RemoveAll(req.DataPath); err != nil { + if err := p.sys.RemoveAll(req.DataPath); err != nil { return errors.Wrap(err, "removing old control metadata subdirectory") } - if err := os.Mkdir(req.DataPath, 0755); err != nil { + if err := p.sys.Mkdir(req.DataPath, 0755); err != nil { return errors.Wrap(err, "creating control metadata subdirectory") } @@ -189,7 +192,7 @@ func (p *Provider) setupDataDir(req storage.MetadataFormatRequest) error { for _, idx := range req.EngineIdxs { engPath := storage.ControlMetadataEngineDir(req.DataPath, idx) - if err := os.Mkdir(engPath, 0755); err != nil { + if err := p.sys.Mkdir(engPath, 0755); err != nil { return errors.Wrapf(err, "creating control metadata engine %d subdirectory", idx) } diff --git a/src/control/server/storage/metadata/provider_test.go b/src/control/server/storage/metadata/provider_test.go index 645fad9fbbf..a3262f78f41 100644 --- a/src/control/server/storage/metadata/provider_test.go +++ b/src/control/server/storage/metadata/provider_test.go @@ -1,5 +1,5 @@ // -// (C) Copyright 2022 Intel Corporation. +// (C) Copyright 2022-2024 Intel Corporation. // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -69,17 +69,17 @@ func TestMetadata_Provider_Format(t *testing.T) { }, expErr: errors.New("not a subdirectory"), }, - "GetFsType fails": { + "GetfsType fails": { req: pathReq, sysCfg: &system.MockSysConfig{ - GetFsTypeErr: []error{errors.New("mock GetFsType")}, + GetfsTypeErr: []error{errors.New("mock GetfsType")}, }, - expErr: errors.New("mock GetFsType"), + expErr: errors.New("mock GetfsType"), }, - "GetFsType returns nosuid flag": { + "GetfsType returns nosuid flag": { req: pathReq, sysCfg: &system.MockSysConfig{ - GetFsTypeRes: &system.FsType{ + GetfsTypeRes: &system.FsType{ Name: system.FsTypeExt4, NoSUID: true, }, @@ -89,30 +89,30 @@ func TestMetadata_Provider_Format(t *testing.T) { NoSUID: true, }), }, - "GetFsType returns nfs": { + "GetfsType returns nfs": { req: pathReq, sysCfg: &system.MockSysConfig{ - GetFsTypeRes: &system.FsType{Name: system.FsTypeNfs}, + GetfsTypeRes: &system.FsType{Name: system.FsTypeNfs}, }, expErr: FaultBadFilesystem(&system.FsType{Name: system.FsTypeNfs}), }, - "GetFsType returns unknown": { + "GetfsType returns unknown": { req: pathReq, sysCfg: &system.MockSysConfig{ - GetFsTypeRes: &system.FsType{Name: system.FsTypeUnknown}, + GetfsTypeRes: &system.FsType{Name: system.FsTypeUnknown}, }, }, - "GetFsType skipped with device": { + "GetfsType skipped with device": { req: deviceReq, sysCfg: &system.MockSysConfig{ - GetFsTypeErr: []error{errors.New("mock GetFsType")}, + GetfsTypeErr: []error{errors.New("mock GetfsType")}, }, }, - "GetFsType retries with parent if dir doesn't exist": { + "GetfsType retries with parent if dir doesn't exist": { req: pathReq, sysCfg: &system.MockSysConfig{ - GetFsTypeRes: &system.FsType{Name: system.FsTypeExt4}, - GetFsTypeErr: []error{os.ErrNotExist, os.ErrNotExist, nil}, + GetfsTypeRes: &system.FsType{Name: system.FsTypeExt4}, + GetfsTypeErr: []error{os.ErrNotExist, os.ErrNotExist, nil}, }, }, "ClearMountpoint fails": { @@ -201,7 +201,7 @@ func TestMetadata_Provider_Format(t *testing.T) { req: pathReq, sysCfg: &system.MockSysConfig{ MkfsErr: errors.New("mkfs was called!"), - GetFsTypeRes: &system.FsType{Name: system.FsTypeExt4}, + GetfsTypeRes: &system.FsType{Name: system.FsTypeExt4}, }, mountCfg: &storage.MockMountProviderConfig{ MountErr: errors.New("mount was called!"), @@ -215,6 +215,12 @@ func TestMetadata_Provider_Format(t *testing.T) { testDir, cleanupTestDir := test.CreateTestDir(t) defer cleanupTestDir() + if tc.sysCfg == nil { + tc.sysCfg = new(system.MockSysConfig) + } + tc.sysCfg.RealMkdir = true + tc.sysCfg.RealRemoveAll = true + // Point the paths at the testdir if tc.req.RootPath != "" { tc.req.RootPath = filepath.Join(testDir, tc.req.RootPath) @@ -239,7 +245,8 @@ func TestMetadata_Provider_Format(t *testing.T) { var p *Provider if !tc.nilProv { - p = NewProvider(log, system.NewMockSysProvider(log, tc.sysCfg), storage.NewMockMountProvider(tc.mountCfg)) + p = NewProvider(log, system.NewMockSysProvider(log, tc.sysCfg), + storage.NewMockMountProvider(tc.mountCfg)) } err := p.Format(tc.req) diff --git a/src/control/server/storage/mount/provider.go b/src/control/server/storage/mount/provider.go index d27d1bf17c7..c14c7485ff2 100644 --- a/src/control/server/storage/mount/provider.go +++ b/src/control/server/storage/mount/provider.go @@ -1,5 +1,5 @@ // -// (C) Copyright 2022 Intel Corporation. +// (C) Copyright 2022-2024 Intel Corporation. // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -24,13 +24,17 @@ const ( ) type ( - // SystemProvider defines a set of methods to be implemented by a provider - // of system capabilities. + // SystemProvider provides operating system capabilities. SystemProvider interface { system.IsMountedProvider system.MountProvider system.UnmountProvider Chmod(string, os.FileMode) error + Chown(string, int, int) error + Getegid() int + Geteuid() int + Mkdir(string, os.FileMode) error + RemoveAll(string) error Stat(string) (os.FileInfo, error) } @@ -140,7 +144,7 @@ func (p *Provider) ClearMountpoint(mntpt string) error { } } - if err := os.RemoveAll(mntpt); err != nil && !os.IsNotExist(err) { + if err := p.sys.RemoveAll(mntpt); err != nil && !os.IsNotExist(err) { return errors.Wrapf(err, "failed to remove %s", mntpt) } @@ -169,12 +173,12 @@ func (p *Provider) MakeMountPath(path string, tgtUID, tgtGID int) error { switch { case os.IsNotExist(err): // subdir missing, attempt to create and chown - if err := os.Mkdir(ps, defaultMountPointPerms); err != nil { + if err := p.sys.Mkdir(ps, defaultMountPointPerms); err != nil { return errors.Wrapf(err, "failed to create directory %q", ps) } - if err := os.Chown(ps, tgtUID, tgtGID); err != nil { + if err := p.sys.Chown(ps, tgtUID, tgtGID); err != nil { return errors.Wrapf(err, "failed to set ownership of %s to %d.%d from %d.%d", - ps, tgtUID, tgtGID, os.Geteuid(), os.Getegid()) + ps, tgtUID, tgtGID, p.sys.Geteuid(), p.sys.Getegid()) } case err != nil: return errors.Wrapf(err, "unable to stat %q", ps) diff --git a/src/control/server/storage/mount/provider_test.go b/src/control/server/storage/mount/provider_test.go index df5d8cf3c16..952b5229c0a 100644 --- a/src/control/server/storage/mount/provider_test.go +++ b/src/control/server/storage/mount/provider_test.go @@ -1,5 +1,5 @@ // -// (C) Copyright 2022 Intel Corporation. +// (C) Copyright 2022-2024 Intel Corporation. // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -270,6 +270,11 @@ func TestProvider_ClearMountpoint(t *testing.T) { testDir, cleanupDir := test.CreateTestDir(t) defer cleanupDir() + if tc.msc == nil { + tc.msc = new(system.MockSysConfig) + } + tc.msc.RealRemoveAll = true + if tc.setup == nil { tc.setup = func(t *testing.T, testDir string) func(*testing.T) { t.Helper() @@ -374,6 +379,7 @@ func TestProvider_MakeMountPath(t *testing.T) { msc := &system.MockSysConfig{ StatErrors: make(map[string]error), + RealMkdir: true, } for mp, err := range tc.statErrs { // mocked stat return errors updated for paths diff --git a/src/control/server/storage/provider.go b/src/control/server/storage/provider.go index 1609af84ff4..ecfa0535a2f 100644 --- a/src/control/server/storage/provider.go +++ b/src/control/server/storage/provider.go @@ -26,9 +26,8 @@ const defaultMetadataPath = "/mnt/daos" // SystemProvider provides operating system capabilities. type SystemProvider interface { system.IsMountedProvider - system.MountProvider GetfsUsage(string) (uint64, uint64, error) - Mkfs(system.MkfsReq) error + ReadFile(string) ([]byte, error) } // Provider provides storage specific capabilities. diff --git a/src/control/server/storage/scm/mocks.go b/src/control/server/storage/scm/mocks.go index c452f110676..627f6272cd0 100644 --- a/src/control/server/storage/scm/mocks.go +++ b/src/control/server/storage/scm/mocks.go @@ -428,12 +428,16 @@ func DefaultMockBackend() *MockBackend { return NewMockBackend(nil) } +// NewMockProvider stubs os calls by mocking system and mount providers. scm provider functions +// call into system and mount providers for any os access. func NewMockProvider(log logging.Logger, mbc *MockBackendConfig, msc *system.MockSysConfig) *Provider { sysProv := system.NewMockSysProvider(log, msc) mountProv := mount.NewProvider(log, sysProv) return NewProvider(log, NewMockBackend(mbc), sysProv, mountProv) } +// DefaultMockProvider stubs os calls by mocking system and mount providers. scm provider functions +// call into system and mount providers for any os access. func DefaultMockProvider(log logging.Logger) *Provider { sysProv := system.DefaultMockSysProvider(log) mountProv := mount.NewProvider(log, sysProv) diff --git a/src/control/server/storage/scm/provider.go b/src/control/server/storage/scm/provider.go index 18e2c6b42b6..aac7d9ea5da 100644 --- a/src/control/server/storage/scm/provider.go +++ b/src/control/server/storage/scm/provider.go @@ -1,4 +1,3 @@ -// // (C) Copyright 2019-2024 Intel Corporation. // // SPDX-License-Identifier: BSD-2-Clause-Patent @@ -44,14 +43,11 @@ type ( UpdateFirmware(deviceUID string, firmwarePath string) error } - // SystemProvider defines a set of methods to be implemented by a provider - // of SCM-specific system capabilities. + // SystemProvider provides operating system capabilities. SystemProvider interface { - Mkfs(system.MkfsReq) error - Getfs(device string) (string, error) - Stat(string) (os.FileInfo, error) - Chmod(string, os.FileMode) error Chown(string, int, int) error + Getfs(string) (string, error) + Mkfs(system.MkfsReq) error } // Provider encapsulates configuration and logic for @@ -173,7 +169,7 @@ func (p *Provider) prepare(req storage.ScmPrepareRequest, scan scanFn) (*storage if len(scanResp.Namespaces) > 0 { for _, ns := range scanResp.Namespaces { nsDev := "/dev/" + ns.BlockDevice - isMounted, err := p.IsMounted(nsDev) + isMounted, err := p.mounter.IsMounted(nsDev) if err != nil { if os.IsNotExist(errors.Cause(err)) { continue @@ -221,7 +217,7 @@ func (p *Provider) CheckFormat(req storage.ScmFormatRequest) (*storage.ScmFormat mntptMissing := false - isMounted, err := p.IsMounted(req.Mountpoint) + isMounted, err := p.mounter.IsMounted(req.Mountpoint) if err != nil { if !os.IsNotExist(err) { return nil, err @@ -477,14 +473,3 @@ func (p *Provider) Unmount(req storage.ScmMountRequest) (*storage.MountResponse, Target: req.Target, }) } - -// Stat probes the specified path and returns os level file info. -func (p *Provider) Stat(path string) (os.FileInfo, error) { - return p.sys.Stat(path) -} - -// IsMounted checks to see if the target device or directory is mounted and -// returns flag to specify whether mounted or a relevant fault. -func (p *Provider) IsMounted(target string) (bool, error) { - return p.mounter.IsMounted(target) -} diff --git a/src/control/server/storage/scm/provider_test.go b/src/control/server/storage/scm/provider_test.go index 712db029c12..dd6e473a96a 100644 --- a/src/control/server/storage/scm/provider_test.go +++ b/src/control/server/storage/scm/provider_test.go @@ -233,7 +233,7 @@ func TestProvider_Prepare(t *testing.T) { // Verify namespaces get unmounted on reset. for _, ns := range tc.scanResp.Namespaces { - isMounted, err := p.IsMounted("/dev/" + ns.BlockDevice) + isMounted, err := p.mounter.IsMounted("/dev/" + ns.BlockDevice) if err != nil { t.Fatal(err) }