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

@@ -22,6 +22,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{}
@@ -34,15 +45,20 @@ 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)
@@ -124,7 +140,7 @@ func (opts *reconfigureMIGOptions) Reconfigure() error {
 	return nil
 }
 
-func (opts *reconfigureMIGOptions) assertValidMIGConfig() error {
+func (opts *reconfigurer) assertValidMIGConfig() error {
 	args := []string{
 		"--debug",
 		"assert",
@@ -135,7 +151,7 @@ func (opts *reconfigureMIGOptions) assertValidMIGConfig() error {
 	return opts.runMigParted(args...)
 }
 
-func (opts *reconfigureMIGOptions) assertMIGConfig() error {
+func (opts *reconfigurer) assertMIGConfig() error {
 	args := []string{
 		"--debug",
 		"assert",
@@ -145,7 +161,7 @@ func (opts *reconfigureMIGOptions) assertMIGConfig() error {
 	return opts.runMigParted(args...)
 }
 
-func (opts *reconfigureMIGOptions) assertMIGModeOnly() error {
+func (opts *reconfigurer) assertMIGModeOnly() error {
 	args := []string{
 		"--debug",
 		"assert",
@@ -156,7 +172,7 @@ func (opts *reconfigureMIGOptions) assertMIGModeOnly() error {
 	return opts.runMigParted(args...)
 }
 
-func (opts *reconfigureMIGOptions) applyMIGModeOnly() error {
+func (opts *reconfigurer) applyMIGModeOnly() error {
 	args := []string{
 		"--debug",
 		"apply",
@@ -167,7 +183,7 @@ func (opts *reconfigureMIGOptions) applyMIGModeOnly() error {
 	return opts.runMigParted(args...)
 }
 
-func (opts *reconfigureMIGOptions) applyMIGConfig() error {
+func (opts *reconfigurer) applyMIGConfig() error {
 	args := []string{
 		"--debug",
 		"apply",
@@ -177,7 +193,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)
@@ -189,7 +205,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 {
@@ -217,7 +233,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)
@@ -315,15 +331,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.commandRunner.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{
+				[]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)
+
+		})
+	}
+}