[no-relnote] Refactor CDI ContainerEdit creation

This change refactors the creation of contianer edits to make
provision for injecting options that affect the generated CDI
specifications.

Signed-off-by: Evan Lezar <elezar@nvidia.com>

internal/edits/api.go

@@ -0,0 +1,29 @@
+/**
+# Copyright 2024 NVIDIA CORPORATION
+#
+# 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 edits
+
+import (
+	"tags.cncf.io/container-device-interface/pkg/cdi"
+
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/discover"
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/oci"
+)
+
+type Interface interface {
+	EditsFromDiscoverer(discover.Discover) (*cdi.ContainerEdits, error)
+	SpecModifierFromDiscoverer(discover.Discover) (oci.SpecModifier, error)
+}

internal/edits/edits.go

@@ -17,15 +17,11 @@
 package edits
 
 import (
-	"fmt"
-
 	ociSpecs "github.com/opencontainers/runtime-spec/specs-go"
 	"tags.cncf.io/container-device-interface/pkg/cdi"
 	"tags.cncf.io/container-device-interface/specs-go"
 
-	"github.com/NVIDIA/nvidia-container-toolkit/internal/discover"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
-	"github.com/NVIDIA/nvidia-container-toolkit/internal/oci"
 )
 
 type edits struct {
@@ -33,58 +29,6 @@ type edits struct {
 	logger logger.Interface
 }
 
-// NewSpecEdits creates a SpecModifier that defines the required OCI spec edits (as CDI ContainerEdits) from the specified
-// discoverer.
-func NewSpecEdits(logger logger.Interface, d discover.Discover) (oci.SpecModifier, error) {
-	c, err := FromDiscoverer(d)
-	if err != nil {
-		return nil, fmt.Errorf("error constructing container edits: %v", err)
-	}
-	e := edits{
-		ContainerEdits: *c,
-		logger:         logger,
-	}
-
-	return &e, nil
-}
-
-// FromDiscoverer creates CDI container edits for the specified discoverer.
-func FromDiscoverer(d discover.Discover) (*cdi.ContainerEdits, error) {
-	devices, err := d.Devices()
-	if err != nil {
-		return nil, fmt.Errorf("failed to discover devices: %v", err)
-	}
-
-	mounts, err := d.Mounts()
-	if err != nil {
-		return nil, fmt.Errorf("failed to discover mounts: %v", err)
-	}
-
-	hooks, err := d.Hooks()
-	if err != nil {
-		return nil, fmt.Errorf("failed to discover hooks: %v", err)
-	}
-
-	c := NewContainerEdits()
-	for _, d := range devices {
-		edits, err := device(d).toEdits()
-		if err != nil {
-			return nil, fmt.Errorf("failed to created container edits for device: %v", err)
-		}
-		c.Append(edits)
-	}
-
-	for _, m := range mounts {
-		c.Append(mount(m).toEdits())
-	}
-
-	for _, h := range hooks {
-		c.Append(hook(h).toEdits())
-	}
-
-	return c, nil
-}
-
 // NewContainerEdits is a utility function to create a CDI ContainerEdits struct.
 func NewContainerEdits() *cdi.ContainerEdits {
 	c := cdi.ContainerEdits{

internal/edits/edits_test.go

@@ -25,7 +25,7 @@ import (
 )
 
 func TestFromDiscovererAllowsMountsToIterate(t *testing.T) {
-	edits, err := FromDiscoverer(discover.None{})
+	edits, err := New().EditsFromDiscoverer(discover.None{})
 	require.NoError(t, err)
 
 	require.Empty(t, edits.Mounts)

internal/edits/lib.go

@@ -0,0 +1,92 @@
+/**
+# Copyright 2024 NVIDIA CORPORATION
+#
+# 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 edits
+
+import (
+	"fmt"
+
+	"tags.cncf.io/container-device-interface/pkg/cdi"
+
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/discover"
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/oci"
+)
+
+// New creates an Interface from the supplied options.
+func New(opts ...Option) Interface {
+	o := &options{}
+	for _, opt := range opts {
+		opt(o)
+	}
+	if o.logger == nil {
+		o.logger = logger.New()
+	}
+
+	return o
+}
+
+// EditsFromDiscoverer creates CDI container edits for the specified discoverer.
+func (o *options) EditsFromDiscoverer(d discover.Discover) (*cdi.ContainerEdits, error) {
+	devices, err := d.Devices()
+	if err != nil {
+		return nil, fmt.Errorf("failed to discover devices: %w", err)
+	}
+
+	mounts, err := d.Mounts()
+	if err != nil {
+		return nil, fmt.Errorf("failed to discover mounts: %w", err)
+	}
+
+	hooks, err := d.Hooks()
+	if err != nil {
+		return nil, fmt.Errorf("failed to discover hooks: %w", err)
+	}
+
+	c := NewContainerEdits()
+	for _, d := range devices {
+		edits, err := device(d).toEdits()
+		if err != nil {
+			return nil, fmt.Errorf("failed to create container edits for device: %w", err)
+		}
+		c.Append(edits)
+	}
+
+	for _, m := range mounts {
+		c.Append(mount(m).toEdits())
+	}
+
+	for _, h := range hooks {
+		c.Append(hook(h).toEdits())
+	}
+
+	return c, nil
+}
+
+// SpecModifierFromDiscoverer creates a SpecModifier that defines the required OCI spec edits (as CDI ContainerEdits) from the specified
+// discoverer.
+func (o *options) SpecModifierFromDiscoverer(d discover.Discover) (oci.SpecModifier, error) {
+	c, err := o.EditsFromDiscoverer(d)
+	if err != nil {
+		return nil, fmt.Errorf("error constructing container edits: %w", err)
+	}
+	e := edits{
+		ContainerEdits: *c,
+		logger:         o.logger,
+	}
+
+	return &e, nil
+}

internal/edits/options.go

@@ -0,0 +1,31 @@
+/**
+# Copyright 2024 NVIDIA CORPORATION
+#
+# 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 edits
+
+import "github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
+
+type options struct {
+	logger logger.Interface
+}
+
+type Option func(*options)
+
+func WithLogger(logger logger.Interface) Option {
+	return func(o *options) {
+		o.logger = logger
+	}
+}

internal/edits/resource.go

@@ -0,0 +1,30 @@
+/**
+# Copyright 2024 NVIDIA CORPORATION
+#
+# 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 edits
+
+import (
+	"tags.cncf.io/container-device-interface/pkg/cdi"
+	"tags.cncf.io/container-device-interface/specs-go"
+)
+
+// NewResource creates a CDI resource (Device) with the specified name.
+func NewResource(name string, edits *cdi.ContainerEdits) specs.Device {
+	return specs.Device{
+		Name:           name,
+		ContainerEdits: *edits.ContainerEdits,
+	}
+}

internal/modifier/discover.go

@@ -45,7 +45,11 @@ func NewModifierFromDiscoverer(logger logger.Interface, d discover.Discover) (oc
 // Modify applies the modifications required by discoverer to the incomming OCI spec.
 // These modifications are applied in-place.
 func (m discoverModifier) Modify(spec *specs.Spec) error {
-	specEdits, err := edits.NewSpecEdits(m.logger, m.discoverer)
+	e := edits.New(
+		edits.WithLogger(m.logger),
+	)
+
+	specEdits, err := e.SpecModifierFromDiscoverer(m.discoverer)
 	if err != nil {
 		return fmt.Errorf("failed to get required container edits: %v", err)
 	}

pkg/nvcdi/full-gpu-nvml.go

@@ -30,7 +30,7 @@ import (
 
 // GetGPUDeviceSpecs returns the CDI device specs for the full GPU represented by 'device'.
 func (l *nvmllib) GetGPUDeviceSpecs(i int, d device.Device) ([]specs.Device, error) {
-	edits, err := l.GetGPUDeviceEdits(d)
+	e, err := l.GetGPUDeviceEdits(d)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get edits for device: %v", err)
 	}
@@ -41,10 +41,7 @@ func (l *nvmllib) GetGPUDeviceSpecs(i int, d device.Device) ([]specs.Device, err
 		return nil, fmt.Errorf("failed to get device name: %v", err)
 	}
 	for _, name := range names {
-		spec := specs.Device{
+		spec := edits.NewResource(name, e)
-			Name:           name,
-			ContainerEdits: *edits.ContainerEdits,
-		}
 		deviceSpecs = append(deviceSpecs, spec)
 	}
 
@@ -58,12 +55,7 @@ func (l *nvmllib) GetGPUDeviceEdits(d device.Device) (*cdi.ContainerEdits, error
 		return nil, fmt.Errorf("failed to create device discoverer: %v", err)
 	}
 
-	editsForDevice, err := edits.FromDiscoverer(device)
+	return (*nvcdilib)(l).editsFromDiscoverer(device)
-	if err != nil {
-		return nil, fmt.Errorf("failed to create container edits for device: %v", err)
-	}
-
-	return editsForDevice, nil
 }
 
 // newFullGPUDiscoverer creates a discoverer for the full GPU defined by the specified device.

pkg/nvcdi/gds.go

@@ -38,22 +38,19 @@ func (l *gdslib) GetAllDeviceSpecs() ([]specs.Device, error) {
 	if err != nil {
 		return nil, fmt.Errorf("failed to create GPUDirect Storage discoverer: %v", err)
 	}
-	edits, err := edits.FromDiscoverer(discoverer)
+	e, err := (*nvcdilib)(l).editsFromDiscoverer(discoverer)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create container edits for GPUDirect Storage: %v", err)
 	}
 
-	deviceSpec := specs.Device{
+	deviceSpec := edits.NewResource("all", e)
-		Name:           "all",
-		ContainerEdits: *edits.ContainerEdits,
-	}
 
 	return []specs.Device{deviceSpec}, nil
 }
 
 // GetCommonEdits generates a CDI specification that can be used for ANY devices
 func (l *gdslib) GetCommonEdits() (*cdi.ContainerEdits, error) {
-	return edits.FromDiscoverer(discover.None{})
+	return edits.NewContainerEdits(), nil
 }
 
 // GetSpec is unsppported for the gdslib specs.

pkg/nvcdi/lib-csv.go

@@ -23,7 +23,6 @@ import (
 	"tags.cncf.io/container-device-interface/pkg/cdi"
 	"tags.cncf.io/container-device-interface/specs-go"
 
-	"github.com/NVIDIA/nvidia-container-toolkit/internal/discover"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/edits"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/platform-support/tegra"
 	"github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/spec"
@@ -53,7 +52,7 @@ func (l *csvlib) GetAllDeviceSpecs() ([]specs.Device, error) {
 	if err != nil {
 		return nil, fmt.Errorf("failed to create discoverer for CSV files: %v", err)
 	}
-	e, err := edits.FromDiscoverer(d)
+	e, err := (*nvcdilib)(l).editsFromDiscoverer(d)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create container edits for CSV files: %v", err)
 	}
@@ -64,10 +63,7 @@ func (l *csvlib) GetAllDeviceSpecs() ([]specs.Device, error) {
 	}
 	var deviceSpecs []specs.Device
 	for _, name := range names {
-		deviceSpec := specs.Device{
+		deviceSpec := edits.NewResource(name, e)
-			Name:           name,
-			ContainerEdits: *e.ContainerEdits,
-		}
 		deviceSpecs = append(deviceSpecs, deviceSpec)
 	}
 
@@ -76,8 +72,7 @@ func (l *csvlib) GetAllDeviceSpecs() ([]specs.Device, error) {
 
 // GetCommonEdits generates a CDI specification that can be used for ANY devices
 func (l *csvlib) GetCommonEdits() (*cdi.ContainerEdits, error) {
-	d := discover.None{}
+	return edits.NewContainerEdits(), nil
-	return edits.FromDiscoverer(d)
 }
 
 // GetGPUDeviceEdits generates a CDI specification that can be used for GPU devices

pkg/nvcdi/lib-nvml.go

@@ -26,7 +26,6 @@ import (
 	"tags.cncf.io/container-device-interface/pkg/cdi"
 	"tags.cncf.io/container-device-interface/specs-go"
 
-	"github.com/NVIDIA/nvidia-container-toolkit/internal/edits"
 	"github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/spec"
 )
 
@@ -74,7 +73,7 @@ func (l *nvmllib) GetCommonEdits() (*cdi.ContainerEdits, error) {
 		return nil, fmt.Errorf("failed to create discoverer for common entities: %v", err)
 	}
 
-	return edits.FromDiscoverer(common)
+	return (*nvcdilib)(l).editsFromDiscoverer(common)
 }
 
 // GetDeviceSpecsByID returns the CDI device specs for the GPU(s) represented by

pkg/nvcdi/lib-wsl.go

@@ -39,15 +39,12 @@ func (l *wsllib) GetSpec() (spec.Interface, error) {
 // GetAllDeviceSpecs returns the device specs for all available devices.
 func (l *wsllib) GetAllDeviceSpecs() ([]specs.Device, error) {
 	device := newDXGDeviceDiscoverer(l.logger, l.devRoot)
-	deviceEdits, err := edits.FromDiscoverer(device)
+	e, err := (*nvcdilib)(l).editsFromDiscoverer(device)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create container edits for DXG device: %v", err)
 	}
 
-	deviceSpec := specs.Device{
+	deviceSpec := edits.NewResource("all", e)
-		Name:           "all",
-		ContainerEdits: *deviceEdits.ContainerEdits,
-	}
 
 	return []specs.Device{deviceSpec}, nil
 }
@@ -59,7 +56,7 @@ func (l *wsllib) GetCommonEdits() (*cdi.ContainerEdits, error) {
 		return nil, fmt.Errorf("failed to create discoverer for WSL driver: %v", err)
 	}
 
-	return edits.FromDiscoverer(driver)
+	return (*nvcdilib)(l).editsFromDiscoverer(driver)
 }
 
 // GetGPUDeviceEdits generates a CDI specification that can be used for GPU devices

pkg/nvcdi/lib.go

@@ -24,6 +24,8 @@ import (
 	"github.com/NVIDIA/go-nvml/pkg/nvml"
 	"tags.cncf.io/container-device-interface/pkg/cdi"
 
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/discover"
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/edits"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/lookup/root"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/platform-support/tegra/csv"
@@ -190,6 +192,14 @@ func (m *wrapper) GetCommonEdits() (*cdi.ContainerEdits, error) {
 	return edits, nil
 }
 
+// editsFromDiscoverer
+func (l *nvcdilib) editsFromDiscoverer(d discover.Discover) (*cdi.ContainerEdits, error) {
+	e := edits.New(
+		edits.WithLogger(l.logger),
+	)
+	return e.EditsFromDiscoverer(d)
+}
+
 // resolveMode resolves the mode for CDI spec generation based on the current system.
 func (l *nvcdilib) resolveMode() (rmode string) {
 	if l.mode != ModeAuto {

pkg/nvcdi/management.go

@@ -43,19 +43,16 @@ func (m *managementlib) GetAllDeviceSpecs() ([]specs.Device, error) {
 		return nil, fmt.Errorf("failed to create device discoverer: %v", err)
 	}
 
-	edits, err := edits.FromDiscoverer(devices)
+	e, err := (*nvcdilib)(m).editsFromDiscoverer(devices)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create edits from discoverer: %v", err)
 	}
 
-	if len(edits.DeviceNodes) == 0 {
+	if len(e.DeviceNodes) == 0 {
 		return nil, fmt.Errorf("no NVIDIA device nodes found")
 	}
 
-	device := specs.Device{
+	device := edits.NewResource("all", e)
-		Name:           "all",
-		ContainerEdits: *edits.ContainerEdits,
-	}
 	return []specs.Device{device}, nil
 }
 
@@ -71,7 +68,7 @@ func (m *managementlib) GetCommonEdits() (*cdi.ContainerEdits, error) {
 		return nil, fmt.Errorf("failed to create driver library discoverer: %v", err)
 	}
 
-	edits, err := edits.FromDiscoverer(driver)
+	edits, err := (*nvcdilib)(m).editsFromDiscoverer(driver)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create edits from discoverer: %v", err)
 	}

pkg/nvcdi/mig-device-nvml.go

@@ -29,7 +29,7 @@ import (
 
 // GetMIGDeviceSpecs returns the CDI device specs for the full GPU represented by 'device'.
 func (l *nvmllib) GetMIGDeviceSpecs(i int, d device.Device, j int, mig device.MigDevice) ([]specs.Device, error) {
-	edits, err := l.GetMIGDeviceEdits(d, mig)
+	e, err := l.GetMIGDeviceEdits(d, mig)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get edits for device: %v", err)
 	}
@@ -40,10 +40,7 @@ func (l *nvmllib) GetMIGDeviceSpecs(i int, d device.Device, j int, mig device.Mi
 	}
 	var deviceSpecs []specs.Device
 	for _, name := range names {
-		spec := specs.Device{
+		spec := edits.NewResource(name, e)
-			Name:           name,
-			ContainerEdits: *edits.ContainerEdits,
-		}
 		deviceSpecs = append(deviceSpecs, spec)
 	}
 	return deviceSpecs, nil
@@ -60,7 +57,7 @@ func (l *nvmllib) GetMIGDeviceEdits(parent device.Device, mig device.MigDevice)
 		return nil, fmt.Errorf("failed to create device discoverer: %v", err)
 	}
 
-	editsForDevice, err := edits.FromDiscoverer(deviceNodes)
+	editsForDevice, err := (*nvcdilib)(l).editsFromDiscoverer(deviceNodes)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create container edits for Compute Instance: %v", err)
 	}

pkg/nvcdi/mofed.go

@@ -38,22 +38,19 @@ func (l *mofedlib) GetAllDeviceSpecs() ([]specs.Device, error) {
 	if err != nil {
 		return nil, fmt.Errorf("failed to create MOFED discoverer: %v", err)
 	}
-	edits, err := edits.FromDiscoverer(discoverer)
+	e, err := (*nvcdilib)(l).editsFromDiscoverer(discoverer)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create container edits for MOFED devices: %v", err)
 	}
 
-	deviceSpec := specs.Device{
+	deviceSpec := edits.NewResource("all", e)
-		Name:           "all",
-		ContainerEdits: *edits.ContainerEdits,
-	}
 
 	return []specs.Device{deviceSpec}, nil
 }
 
 // GetCommonEdits generates a CDI specification that can be used for ANY devices
 func (l *mofedlib) GetCommonEdits() (*cdi.ContainerEdits, error) {
-	return edits.FromDiscoverer(discover.None{})
+	return edits.NewContainerEdits(), nil
 }
 
 // GetSpec is unsppported for the mofedlib specs.

pkg/nvcdi/transform/merged-device.go

@@ -109,7 +109,6 @@ func mergeDeviceSpecs(deviceSpecs []specs.Device, mergedDeviceName string) (*spe
 	}
 
 	mergedEdits := edits.NewContainerEdits()
-
 	for _, d := range deviceSpecs {
 		d := d
 		edit := cdi.ContainerEdits{
@@ -118,9 +117,6 @@ func mergeDeviceSpecs(deviceSpecs []specs.Device, mergedDeviceName string) (*spe
 		mergedEdits.Append(&edit)
 	}
 
-	merged := specs.Device{
+	merged := edits.NewResource(mergedDeviceName, mergedEdits)
-		Name:           mergedDeviceName,
-		ContainerEdits: *mergedEdits.ContainerEdits,
-	}
 	return &merged, nil
 }