Add commandRunnner interface and a basic test

Signed-off-by: Evan Lezar <[email protected]>

Author: elezar

pkg/mig/reconfigure/api.go

@@ -1,5 +1,7 @@
 package reconfigure
 
+import "os/exec"
+
 const (
 	MIGConfigStateLabel  = "nvidia.com/mig.config.state"
 	VGPUConfigStateLabel = "nvidia.com/vgpu.config.state"
@@ -9,3 +11,11 @@ const (
 type Reconfigurer interface {
 	Reconfigure() error
 }
+
+// A commandRunner is used to run a constructed command.
+// This interface allows us to inject a runner for testing.
+//
+//go:generate moq -rm -fmt=goimports -out command-runner_mock.go . commandRunner
+type commandRunner interface {
+	Run(*exec.Cmd) error
+}

pkg/mig/reconfigure/command-runner_mock.go

@@ -39,6 +39,17 @@ const (
 	ldPreloadEnvVar = "LD_PRELOAD"
 )
 
+type reconfigurer struct {
+	*reconfigureMIGOptions
+	commandRunner
+}
+
+// A commandWithOutput runs a command and ensures that STDERR and STDOUT are
+// set.
+type commandWithOutput struct{}
+
+var _ commandRunner = (*commandWithOutput)(nil)
+
 // New creates a MIG Reconfigurer with the supplied options.
 func New(opts ...Option) (Reconfigurer, error) {
 	o := &reconfigureMIGOptions{}
@@ -51,26 +62,31 @@ func New(opts ...Option) (Reconfigurer, error) {
 		return nil, err
 	}
 
-	return o, nil
+	r := &reconfigurer{
+		reconfigureMIGOptions: o,
+		commandRunner:         &commandWithOutput{},
+	}
+
+	return r, nil
 }
 
 // Reconfigure configures MIG (Multi-Instance GPU) settings on a Kubernetes
 // node. It validates the requested configuration, checks the current state,
 // applies MIG mode changes, manages host GPU client services, and handles
 // reboots when necessary. The function ensures that MIG configurations are
 // applied safely with proper service lifecycle management.
-func (opts *reconfigureMIGOptions) Reconfigure() error {
+func (opts *reconfigurer) Reconfigure() error {
 	log.Info("Asserting that the requested configuration is present in the configuration file")
 	if err := opts.assertValidMIGConfig(); err != nil {
 		return fmt.Errorf("error validating the selected MIG configuration: %w", err)
 	}
 
-	log.Infof("Getting current value of the '%s' node label", opts.configStateLabel)
-	state, err := opts.getNodeLabelValue(opts.configStateLabel)
+	log.Infof("Getting current value of the '%s' node label", opts.ConfigStateLabel)
+	state, err := opts.getNodeLabelValue(opts.ConfigStateLabel)
 	if err != nil {
-		return fmt.Errorf("unable to get the value of the %q label: %w", opts.configStateLabel, err)
+		return fmt.Errorf("unable to get the value of the %q label: %w", opts.ConfigStateLabel, err)
 	}
-	log.Infof("Current value of '%s=%s'", opts.configStateLabel, state)
+	log.Infof("Current value of '%s=%s'", opts.ConfigStateLabel, state)
 
 	log.Info("Checking if the selected MIG config is currently applied or not")
 	if err := opts.assertMIGConfig(); err == nil {
@@ -116,11 +132,11 @@ func (opts *reconfigureMIGOptions) Reconfigure() error {
 	log.Info("If the -r option was passed, the node will be automatically rebooted if this is not successful")
 	if err := opts.applyMIGModeOnly(); err != nil || opts.assertMIGModeOnly() != nil {
 		if opts.WithReboot {
-			log.Infof("Changing the '%s' node label to '%s'", opts.configStateLabel, configStateRebooting)
-			if err := opts.setNodeLabelValue(opts.configStateLabel, configStateRebooting); err != nil {
-				log.Errorf("Unable to set the value of '%s' to '%s'", opts.configStateLabel, configStateRebooting)
+			log.Infof("Changing the '%s' node label to '%s'", opts.ConfigStateLabel, configStateRebooting)
+			if err := opts.setNodeLabelValue(opts.ConfigStateLabel, configStateRebooting); err != nil {
+				log.Errorf("Unable to set the value of '%s' to '%s'", opts.ConfigStateLabel, configStateRebooting)
 				log.Error("Exiting so as not to reboot multiple times unexpectedly")
-				return fmt.Errorf("unable to set the value of %q to %q: %w", opts.configStateLabel, configStateRebooting, err)
+				return fmt.Errorf("unable to set the value of %q to %q: %w", opts.ConfigStateLabel, configStateRebooting, err)
 			}
 			return rebootHost(opts.HostRootMount)
 		}
@@ -141,7 +157,7 @@ func (opts *reconfigureMIGOptions) Reconfigure() error {
 	return nil
 }
 
-func (opts *reconfigureMIGOptions) assertValidMIGConfig() error {
+func (opts *reconfigurer) assertValidMIGConfig() error {
 	args := []string{
 		"--debug",
 		"assert",
@@ -152,7 +168,7 @@ func (opts *reconfigureMIGOptions) assertValidMIGConfig() error {
 	return opts.runMigParted(args...)
 }
 
-func (opts *reconfigureMIGOptions) assertMIGConfig() error {
+func (opts *reconfigurer) assertMIGConfig() error {
 	args := []string{
 		"--debug",
 		"assert",
@@ -162,7 +178,7 @@ func (opts *reconfigureMIGOptions) assertMIGConfig() error {
 	return opts.runMigParted(args...)
 }
 
-func (opts *reconfigureMIGOptions) assertMIGModeOnly() error {
+func (opts *reconfigurer) assertMIGModeOnly() error {
 	args := []string{
 		"--debug",
 		"assert",
@@ -173,7 +189,7 @@ func (opts *reconfigureMIGOptions) assertMIGModeOnly() error {
 	return opts.runMigParted(args...)
 }
 
-func (opts *reconfigureMIGOptions) applyMIGModeOnly() error {
+func (opts *reconfigurer) applyMIGModeOnly() error {
 	args := []string{
 		"--debug",
 		"apply",
@@ -184,7 +200,7 @@ func (opts *reconfigureMIGOptions) applyMIGModeOnly() error {
 	return opts.runMigParted(args...)
 }
 
-func (opts *reconfigureMIGOptions) applyMIGConfig() error {
+func (opts *reconfigurer) applyMIGConfig() error {
 	args := []string{
 		"--debug",
 		"apply",
@@ -194,7 +210,7 @@ func (opts *reconfigureMIGOptions) applyMIGConfig() error {
 	return opts.runMigParted(args...)
 }
 
-func (opts *reconfigureMIGOptions) hostPersistConfig() error {
+func (opts *reconfigurer) hostPersistConfig() error {
 	config := fmt.Sprintf(`[Service]
 Environment="MIG_PARTED_SELECTED_CONFIG=%s"
 `, opts.SelectedMIGConfig)
@@ -206,7 +222,7 @@ Environment="MIG_PARTED_SELECTED_CONFIG=%s"
 	}
 
 	cmd := exec.Command("chroot", opts.HostRootMount, "systemctl", "daemon-reload") // #nosec G204 -- HostRootMount is validated via dirpath validator.
-	return runCommandWithOutput(cmd)
+	return opts.Run(cmd)
 }
 
 func (opts *reconfigureMIGOptions) hostStopSystemdServices() error {
@@ -234,7 +250,7 @@ func (opts *reconfigureMIGOptions) hostStartSystemdServices() error {
 		log.Infof("Starting %s", service)
 		cmd := exec.Command("chroot", opts.HostRootMount, "systemctl", "start", service) // #nosec G204 -- HostRootMount validated via dirpath, service validated via systemd_service_name.
 		if err := cmd.Run(); err != nil {
-			serviceError := fmt.Errorf("error starting %q: %w", err)
+			serviceError := fmt.Errorf("error starting %q: %w", service, err)
 			log.Errorf("%v; skipping, but continuing...", serviceError)
 
 			errs = errors.Join(errs, serviceError)
@@ -332,15 +348,15 @@ func shouldRestartService(opts *reconfigureMIGOptions, service string) bool {
 	return true
 }
 
-func runCommandWithOutput(cmd *exec.Cmd) error {
+func (c *commandWithOutput) Run(cmd *exec.Cmd) error {
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
 	return cmd.Run()
 }
 
-func (opts *reconfigureMIGOptions) runMigParted(args ...string) error {
+func (opts *reconfigurer) runMigParted(args ...string) error {
 	cmd := opts.migPartedCmd(args...)
-	return runCommandWithOutput(cmd)
+	return opts.Run(cmd)
 }
 
 func (opts *reconfigureMIGOptions) migPartedCmd(args ...string) *exec.Cmd {

pkg/mig/reconfigure/reconfigure.go

@@ -0,0 +1,92 @@
+/**
+# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+**/
+
+package reconfigure
+
+import (
+	"fmt"
+	"os/exec"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+type commandRunnerWithCLI struct {
+	mock  *commandRunnerMock
+	calls [][]string
+}
+
+func (c *commandRunnerWithCLI) Run(cmd *exec.Cmd) error {
+	c.calls = append(c.calls, append([]string{cmd.Path}, cmd.Args...))
+	return c.mock.Run(cmd)
+}
+
+func TestReconfigure(t *testing.T) {
+	testCases := []struct {
+		description   string
+		options       reconfigureMIGOptions
+		commandRunner *commandRunnerWithCLI
+		expectedError error
+		expectedCalls [][]string
+	}{
+		{
+			description: "test command failure",
+			options: reconfigureMIGOptions{
+				NodeName:            "NodeName",
+				MIGPartedConfigFile: "/path/to/config/file.yaml",
+				SelectedMIGConfig:   "selected-mig-config",
+				DriverLibraryPath:   "/path/to/libnvidia-ml.so.1",
+				HostRootMount:       "/host/",
+			},
+			commandRunner: &commandRunnerWithCLI{
+				mock: &commandRunnerMock{
+					RunFunc: func(cmd *exec.Cmd) error {
+						return fmt.Errorf("error running command %v", cmd.Path)
+					},
+				},
+			},
+			expectedError: fmt.Errorf("error validating the selected MIG configuration: error running command nvidia-mig-parted"),
+			expectedCalls: [][]string{
+				{"nvidia-mig-parted", "nvidia-mig-parted", "--debug", "assert", "--valid-config", "--config-file", "/path/to/config/file.yaml", "--selected-config", "selected-mig-config"},
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			// TODO: Once we have better mocks in place for the following
+			// functionality, we can update this.
+			require.False(t, tc.options.WithReboot)
+			require.False(t, tc.options.WithShutdownHostGPUClients)
+
+			// We test explicit validation in a separate test.
+			// For now we only ensure that the options are valid.
+			require.NoError(t, tc.options.Validate())
+
+			r := &reconfigurer{
+				reconfigureMIGOptions: &tc.options,
+				commandRunner:         tc.commandRunner,
+			}
+
+			err := r.Reconfigure()
+			require.EqualValues(t, tc.expectedError.Error(), err.Error())
+
+			require.EqualValues(t, tc.expectedCalls, tc.commandRunner.calls)
+
+		})
+	}
+}