DAOS-17693 control: Correctly copy mkfs options (#17498)

Options were not being copied properly into the mkfs argument
slice. This PR fixes the issue and improves unit testing.

Signed-off-by: Kris Jacque <kris.jacque@hpe.com>

Author: kjacque

src/control/provider/system/system_linux.go

@@ -1,6 +1,6 @@
 //
 // (C) Copyright 2019-2024 Intel Corporation.
-// (C) Copyright 2025 Hewlett Packard Enterprise Development LP
+// (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP
 //
 // SPDX-License-Identifier: BSD-2-Clause-Patent
 //
@@ -56,12 +56,18 @@ var magicToStr = map[int64]string{
 
 // DefaultProvider returns the package-default provider implementation.
 func DefaultProvider() *LinuxProvider {
-	return &LinuxProvider{}
+	return &LinuxProvider{
+		runCommand: func(name string, args ...string) ([]byte, error) {
+			return exec.Command(name, args...).Output()
+		},
+	}
 }
 
 // LinuxProvider encapsulates Linux-specific implementations of system
 // interfaces.
-type LinuxProvider struct{}
+type LinuxProvider struct {
+	runCommand func(string, ...string) ([]byte, error)
+}
 
 // mountId,parentId,major:minor,root,mountPoint
 const (
@@ -254,6 +260,10 @@ type MkfsReq struct {
 // Mkfs attempts to create a filesystem of the supplied type, on the
 // supplied device.
 func (s LinuxProvider) Mkfs(req MkfsReq) error {
+	if req.Filesystem == "" {
+		return errors.New("no filesystem type specified")
+	}
+
 	cmdPath, err := exec.LookPath(fmt.Sprintf("mkfs.%s", req.Filesystem))
 	if err != nil {
 		return errors.Wrapf(err, "unable to find mkfs.%s", req.Filesystem)
@@ -263,7 +273,7 @@ func (s LinuxProvider) Mkfs(req MkfsReq) error {
 		return err
 	}
 
-	args := make([]string, 0, len(req.Options))
+	args := make([]string, len(req.Options))
 	_ = copy(args, req.Options)
 	// TODO: Think about a way to allow for some kind of progress
 	// callback so that the user has some visibility into long-running
@@ -274,7 +284,7 @@ func (s LinuxProvider) Mkfs(req MkfsReq) error {
 	if req.Force {
 		args = append([]string{"-F"}, args...)
 	}
-	out, err := exec.Command(cmdPath, args...).Output()
+	out, err := s.runCommand(cmdPath, args...)
 	if err != nil {
 		return &RunCmdError{
 			Wrapped: err,
@@ -301,7 +311,7 @@ func (s LinuxProvider) GetDeviceLabel(device string) (string, error) {
 	}
 
 	args := []string{"-o", "label", "--noheadings", device}
-	out, err := exec.Command(cmdPath, args...).Output()
+	out, err := s.runCommand(cmdPath, args...)
 	if err != nil {
 		return "", &RunCmdError{
 			Wrapped: err,
@@ -325,7 +335,7 @@ func (s LinuxProvider) Getfs(device string) (string, error) {
 	}
 
 	args := []string{"-s", device}
-	out, err := exec.Command(cmdPath, args...).Output()
+	out, err := s.runCommand(cmdPath, args...)
 	if err != nil {
 		return FsTypeNone, &RunCmdError{
 			Wrapped: err,

src/control/provider/system/system_linux_test.go

@@ -1,6 +1,6 @@
 //
 // (C) Copyright 2019-2024 Intel Corporation.
-// (C) Copyright 2025 Hewlett Packard Enterprise Development LP
+// (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP
 //
 // SPDX-License-Identifier: BSD-2-Clause-Patent
 //
@@ -10,6 +10,7 @@ package system
 import (
 	"errors"
 	"os"
+	"path/filepath"
 	"regexp"
 	"strings"
 	"syscall"
@@ -202,39 +203,39 @@ func TestSystemLinux_GetfsType(t *testing.T) {
 	}
 }
 
-func TestSystemLinux_GetDeviceLabel(t *testing.T) {
-	validDev := func(t *testing.T) string {
-		t.Helper()
+func validDev(t *testing.T) string {
+	t.Helper()
 
-		// Only want numbered partitions, not whole disks
-		re := regexp.MustCompile(`^[a-zA-Z]+[0-9]+$`)
+	// Only want numbered partitions, not whole disks
+	re := regexp.MustCompile(`^[a-zA-Z]+[0-9]+$`)
 
-		sysRoot := "/sys/class/block/"
-		entries, err := os.ReadDir(sysRoot)
-		if err != nil {
-			t.Fatalf("unable to read %q: %v", sysRoot, err)
-		}
-
-		for _, entry := range entries {
-			if !re.MatchString(entry.Name()) {
-				continue
-			}
+	sysRoot := "/sys/class/block/"
+	entries, err := os.ReadDir(sysRoot)
+	if err != nil {
+		t.Fatalf("unable to read %q: %v", sysRoot, err)
+	}
 
-			devPath := "/dev/" + entry.Name()
-			info, err := os.Stat(devPath)
-			if err != nil {
-				continue
-			}
-			if (info.Mode()&os.ModeDevice) != 0 && (info.Mode()&os.ModeCharDevice) == 0 {
-				t.Logf("using block device %q for test", devPath)
-				return devPath
-			}
+	for _, entry := range entries {
+		if !re.MatchString(entry.Name()) {
+			continue
 		}
 
-		t.Fatal("no valid block device found for test")
-		return ""
+		devPath := "/dev/" + entry.Name()
+		info, err := os.Stat(devPath)
+		if err != nil {
+			continue
+		}
+		if (info.Mode()&os.ModeDevice) != 0 && (info.Mode()&os.ModeCharDevice) == 0 {
+			t.Logf("using block device %q for test", devPath)
+			return devPath
+		}
 	}
 
+	t.Fatal("no valid block device found for test")
+	return ""
+}
+
+func TestSystemLinux_GetDeviceLabel(t *testing.T) {
 	for name, tc := range map[string]struct {
 		path   string
 		expErr error
@@ -312,3 +313,79 @@ func TestSystemLinux_fsStrFromMagic(t *testing.T) {
 		})
 	}
 }
+
+func TestSystemLinux_Mkfs(t *testing.T) {
+	for name, tc := range map[string]struct {
+		req        MkfsReq
+		expErr     error
+		expCmdName string
+		expCmdArgs []string
+	}{
+		"empty": {
+			req:    MkfsReq{},
+			expErr: errors.New("no filesystem"),
+		},
+		"bad filesystem": {
+			req: MkfsReq{
+				Filesystem: "moo",
+			},
+			expErr: errors.New("unable to find mkfs.moo"),
+		},
+		"bad device": {
+			req: MkfsReq{
+				Filesystem: "ext4",
+				Device:     "/notreal",
+			},
+			expErr: syscall.ENOENT,
+		},
+		"success": {
+			req: MkfsReq{
+				Filesystem: "ext4",
+				Device:     validDev(t), // real device, but actual mkfs command is mocked
+			},
+			expCmdName: "mkfs.ext4",
+			expCmdArgs: []string{validDev(t)},
+		},
+		"force": {
+			req: MkfsReq{
+				Filesystem: "ext4",
+				Device:     validDev(t),
+				Force:      true,
+			},
+			expCmdName: "mkfs.ext4",
+			expCmdArgs: []string{"-F", validDev(t)},
+		},
+		"options": {
+			req: MkfsReq{
+				Filesystem: "ext4",
+				Device:     validDev(t),
+				Options:    []string{"-L", "my_device"},
+			},
+			expCmdName: "mkfs.ext4",
+			expCmdArgs: []string{"-L", "my_device", validDev(t)},
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			p := DefaultProvider()
+
+			var seenName string
+			var seenArgs []string
+			p.runCommand = func(name string, args ...string) ([]byte, error) {
+				seenName = name
+				seenArgs = args
+				return []byte{}, nil
+			}
+
+			err := p.Mkfs(tc.req)
+
+			test.CmpErr(t, tc.expErr, err)
+
+			if seenName != "" {
+				// don't care where the binary was found, just that it was
+				seenName = filepath.Base(seenName)
+			}
+			test.AssertEqual(t, tc.expCmdName, seenName, "mkfs command name")
+			test.AssertEqual(t, tc.expCmdArgs, seenArgs, "mkfs args")
+		})
+	}
+}

src/control/server/storage/metadata/provider.go

@@ -1,6 +1,6 @@
 //
 // (C) Copyright 2022-2024 Intel Corporation.
-// (C) Copyright 2025 Hewlett Packard Enterprise Development LP
+// (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP
 //
 // SPDX-License-Identifier: BSD-2-Clause-Patent
 //
@@ -107,13 +107,16 @@ func (p *Provider) setupMountPoint(req storage.MetadataFormatRequest) error {
 		return errors.Wrap(err, "checking existing device label")
 	}
 
-	var opts []string
+	opts := []string{
+		// Quiet mode
+		"-q",
+	}
 	if label != "" {
 		p.log.Debugf("preserving existing device label %q for %q", label, req.Device)
 		opts = append(opts, "-L", label)
 	}
 
-	p.log.Debugf("formatting device %q", req.Device)
+	p.log.Debugf("mkfs.%s %q with options: %s", defaultDevFS, req.Device, strings.Join(opts, " "))
 	if err := p.sys.Mkfs(system.MkfsReq{
 		Filesystem: defaultDevFS,
 		Device:     req.Device,

src/control/server/storage/metadata/provider_test.go

@@ -1,6 +1,6 @@
 //
 // (C) Copyright 2022-2024 Intel Corporation.
-// (C) Copyright 2025 Hewlett Packard Enterprise Development LP
+// (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP
 //
 // SPDX-License-Identifier: BSD-2-Clause-Patent
 //
@@ -110,7 +110,8 @@ func TestMetadata_Provider_Format(t *testing.T) {
 			sysCfg: &system.MockSysConfig{
 				GetfsTypeErr: []error{errors.New("mock GetfsType")},
 			},
-			expMkfs: true,
+			expMkfsOpts: []string{"-q"},
+			expMkfs:     true,
 		},
 		"GetfsType retries with parent if dir doesn't exist": {
 			req: pathReq,
@@ -145,16 +146,18 @@ func TestMetadata_Provider_Format(t *testing.T) {
 			sysCfg: &system.MockSysConfig{
 				MkfsErr: errors.New("mock mkfs"),
 			},
-			expErr:  errors.New("mock mkfs"),
-			expMkfs: true,
+			expErr:      errors.New("mock mkfs"),
+			expMkfsOpts: []string{"-q"},
+			expMkfs:     true,
 		},
 		"Mount fails": {
 			req: deviceReq,
 			mountCfg: &storage.MockMountProviderConfig{
 				MountErr: errors.New("mock Mount"),
 			},
-			expErr:  errors.New("mock Mount"),
-			expMkfs: true,
+			expErr:      errors.New("mock Mount"),
+			expMkfsOpts: []string{"-q"},
+			expMkfs:     true,
 		},
 		"remove old data dir fails": {
 			req: deviceReq,
@@ -172,8 +175,9 @@ func TestMetadata_Provider_Format(t *testing.T) {
 					}
 				}
 			},
-			expErr:  errors.New("removing old control metadata subdirectory"),
-			expMkfs: true,
+			expErr:      errors.New("removing old control metadata subdirectory"),
+			expMkfsOpts: []string{"-q"},
+			expMkfs:     true,
 		},
 		"create data dir fails": {
 			req: deviceReq,
@@ -191,36 +195,40 @@ func TestMetadata_Provider_Format(t *testing.T) {
 					}
 				}
 			},
-			expErr:  errors.New("creating control metadata subdirectory"),
-			expMkfs: true,
+			expErr:      errors.New("creating control metadata subdirectory"),
+			expMkfsOpts: []string{"-q"},
+			expMkfs:     true,
 		},
 		"chown data dir fails": {
 			req: deviceReq,
 			sysCfg: &system.MockSysConfig{
 				ChownErr: errors.New("mock chown"),
 			},
-			expErr:  errors.New("mock chown"),
-			expMkfs: true,
+			expErr:      errors.New("mock chown"),
+			expMkfsOpts: []string{"-q"},
+			expMkfs:     true,
 		},
 		"Unmount fails": {
 			req: deviceReq,
 			mountCfg: &storage.MockMountProviderConfig{
 				IsMountedRes: true,
 				UnmountErr:   errors.New("mock Unmount"),
 			},
-			expErr:  errors.New("mock Unmount"),
-			expMkfs: true,
+			expErr:      errors.New("mock Unmount"),
+			expMkfsOpts: []string{"-q"},
+			expMkfs:     true,
 		},
 		"device success": {
-			req:     deviceReq,
-			expMkfs: true,
+			req:         deviceReq,
+			expMkfsOpts: []string{"-q"},
+			expMkfs:     true,
 		},
 		"preserve existing label": {
 			req: deviceReq,
 			sysCfg: &system.MockSysConfig{
 				GetDeviceLabelRes: "old_label",
 			},
-			expMkfsOpts: []string{"-L", "old_label"},
+			expMkfsOpts: []string{"-q", "-L", "old_label"},
 			expMkfs:     true,
 		},
 		"path only doesn't attempt device format": {