DAOS-15841 control: Improve coverage and fs mocking in storage packag…

…es (#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 <[email protected]>

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{}

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)
+}

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 != "" {

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)

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
 	}