Add reconfigure api #218

elezar · 2025-07-10T20:50:06Z

This change migrates the mig-reconfigure implementation from NVIDIA/vgpu-device-manager#93 to a package in mig-parted so that it can be shared between the mig-manager and the vgpu-device-manager.

We will extend the API to support the mig-manager use case as a follow-up.

See #216

Copilot

Pull Request Overview

This PR migrates the mig-reconfigure implementation from the vgpu-device-manager repository to a shared package in mig-parted, enabling both mig-manager and vgpu-device-manager to utilize the same functionality.

Creates a new pkg/mig/reconfigure package with configuration, validation, and execution logic
Implements validation for systemd service names using custom validators
Provides comprehensive test coverage using mocked interfaces

Reviewed Changes

Copilot reviewed 9 out of 194 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pkg/mig/reconfigure/reconfigure.go	Core implementation of MIG reconfiguration logic with systemd service management
pkg/mig/reconfigure/options.go	Configuration options and functional option pattern for API construction
pkg/mig/reconfigure/validate.go	Validation logic including custom systemd service name validator
pkg/mig/reconfigure/api.go	Public API interfaces and constants
pkg/mig/reconfigure/reconfigure_test.go	Comprehensive test suite with mocked dependencies
pkg/mig/reconfigure/*_mock.go	Generated mock implementations for testing
go.mod	Added validator dependency

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.}

Copilot · 2025-08-14T09:21:27Z

pkg/mig/reconfigure/validate.go

+
+	err := validate.RegisterValidation("systemd_service_name", validateSystemdServiceName)
+	if err != nil {
+		return fmt.Errorf("unable to registre systemd service name validator: %w", err)


The word 'registre' is misspelled. It should be 'register'.

Suggested change

return fmt.Errorf("unable to registre systemd service name validator: %w", err)

return fmt.Errorf("unable to register systemd service name validator: %w", err)

Copilot · 2025-08-14T09:21:27Z

pkg/mig/reconfigure/reconfigure_test.go

+			expectedError: nil,
+		},
+		{
+			description: "reconfigure exists if config is applied",


The word 'exists' should be 'exits' to match the similar test case on line 131.

Suggested change

description: "reconfigure exists if config is applied",

description: "reconfigure exits if config is applied",

Copilot · 2025-08-14T09:21:28Z

pkg/mig/reconfigure/options.go

+	// TODO: Define the validation schema.
+	SelectedMIGConfig string `validate:"required"`
+
+	// DriverLibrayPath is the path to libnvidia-ml.so.1 in the container.


The word 'DriverLibrayPath' is misspelled. It should be 'DriverLibraryPath'.

Suggested change

// DriverLibrayPath is the path to libnvidia-ml.so.1 in the container.

// DriverLibraryPath is the path to libnvidia-ml.so.1 in the container.

Copilot · 2025-08-14T09:21:28Z

pkg/mig/reconfigure/reconfigure.go

+	}
+
+	cmd = exec.Command("chroot", opts.HostRootMount, "systemctl", "-q", "is-enabled", service) // #nosec G204 -- HostRootMount validated via dirpath, service validated via systemd_service_name.
+	if err := cmd.Run(); err != nil {


This is a duplicate check of the same command executed on line 331. Consider extracting this repeated logic into a helper function to reduce code duplication.

Suggested change

if err := cmd.Run(); err != nil {

if err := runSystemctlIsEnabled(opts.HostRootMount, service); err != nil {

ArangoGutierrez

2 Nits

ArangoGutierrez · 2025-08-14T09:25:50Z

pkg/mig/reconfigure/api.go

@@ -0,0 +1,40 @@
+package reconfigure


nit: License header

ArangoGutierrez · 2025-08-14T09:26:03Z

pkg/mig/reconfigure/options.go

@@ -0,0 +1,132 @@
+package reconfigure


nit: License header

This change adds a mig/reconfigure package that implements the functionality added to the vgpu-device-manager. Signed-off-by: Evan Lezar <[email protected]>

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

tariq1890 · 2025-09-23T01:59:55Z

pkg/mig/reconfigure/reconfigure.go

+		return false
+	}
+
+	cmd = exec.Command("chroot", opts.HostRootMount, "systemctl", "-q", "is-enabled", service) // #nosec G204 -- HostRootMount validated via dirpath, service validated via systemd_service_name.


thoughts on using this library to invoke systemctl functions?

https://github.com/taigrr/systemctl

Please disregard the above library, they too are just shelling out and invoking the systemctl command

Looks like we will need to use coreos/go-systemd to replace the shell invocations here - https://github.com/coreos/go-systemd

I found this blog quite helpul - https://blog.gripdev.xyz/2024/09/27/golang-restart-systemd-unit/

This was referenced Jul 10, 2025

Migrate to mig parted reconfigure NVIDIA/vgpu-device-manager#101

Draft

Add MIG config support when MIG-backed vGPU type NVIDIA/vgpu-device-manager#93

Merged

elezar force-pushed the add-reconfigure-api branch from 648ffdd to e71d1cc Compare July 11, 2025 08:46

elezar force-pushed the add-reconfigure-api branch from e71d1cc to 2dcb50b Compare July 30, 2025 12:43

elezar self-assigned this Jul 30, 2025

elezar requested review from cdesiniotis and klueska July 30, 2025 13:22

elezar marked this pull request as ready for review July 30, 2025 13:22

elezar force-pushed the add-reconfigure-api branch from 7ba94e5 to e9f7a10 Compare July 31, 2025 11:13

elezar requested review from ArangoGutierrez and tariq1890 August 11, 2025 15:05

ArangoGutierrez requested a review from Copilot August 14, 2025 09:19

Copilot AI reviewed Aug 14, 2025

View reviewed changes

ArangoGutierrez reviewed Aug 14, 2025

View reviewed changes

elezar added 7 commits September 16, 2025 14:26

Add MIG Reconfigure API for vgpu-device-manager

5ea34a6

This change adds a mig/reconfigure package that implements the functionality added to the vgpu-device-manager. Signed-off-by: Evan Lezar <[email protected]>

Return formatted errors instead of logging

89e8bfc

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

Add commandRunnner interface and a basic test

d4ed26a

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

Update vendoring

f793f1a

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

Add migParted interface for testing

6544a19

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

Add nodeLabeller interface for testing

36f8eef

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

Add additional test cases

9cf3052

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

elezar force-pushed the add-reconfigure-api branch from e9f7a10 to 9cf3052 Compare September 16, 2025 12:27

tariq1890 reviewed Sep 23, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add reconfigure api #218

Add reconfigure api #218

Uh oh!

elezar commented Jul 10, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Aug 14, 2025

Uh oh!

Copilot AI Aug 14, 2025

Uh oh!

Copilot AI Aug 14, 2025

Uh oh!

Copilot AI Aug 14, 2025

Uh oh!

ArangoGutierrez left a comment

Uh oh!

ArangoGutierrez Aug 14, 2025

Uh oh!

ArangoGutierrez Aug 14, 2025

Uh oh!

tariq1890 Sep 23, 2025

Uh oh!

tariq1890 Sep 23, 2025

Uh oh!

tariq1890 Sep 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	return fmt.Errorf("unable to registre systemd service name validator: %w", err)
	return fmt.Errorf("unable to register systemd service name validator: %w", err)

	description: "reconfigure exists if config is applied",
	description: "reconfigure exits if config is applied",

	// DriverLibrayPath is the path to libnvidia-ml.so.1 in the container.
	// DriverLibraryPath is the path to libnvidia-ml.so.1 in the container.

	if err := cmd.Run(); err != nil {
	if err := runSystemctlIsEnabled(opts.HostRootMount, service); err != nil {

Add reconfigure api #218

Are you sure you want to change the base?

Add reconfigure api #218

Uh oh!

Conversation

elezar commented Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

ArangoGutierrez left a comment

Choose a reason for hiding this comment

Uh oh!

ArangoGutierrez Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

ArangoGutierrez Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

tariq1890 Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

tariq1890 Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

tariq1890 Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

elezar commented Jul 10, 2025 •

edited

Loading