Skip to content

Commit 50278cb

Browse files
elezarelezar
committed
[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 <[email protected]>
1 parent c3c0cdc commit 50278cb

File tree

17 files changed

+222
-115
lines changed

17 files changed

+222
-115
lines changed

‎internal/edits/api.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
# Copyright 2024 NVIDIA CORPORATION
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
**/
16+
17+
package edits
18+
19+
import (
20+
"tags.cncf.io/container-device-interface/pkg/cdi"
21+
22+
"github.com/NVIDIA/nvidia-container-toolkit/internal/discover"
23+
"github.com/NVIDIA/nvidia-container-toolkit/internal/oci"
24+
)
25+
26+
type Interface interface {
27+
EditsFromDiscoverer(discover.Discover) (*cdi.ContainerEdits, error)
28+
SpecModifierFromDiscoverer(discover.Discover) (oci.SpecModifier, error)
29+
}

‎internal/edits/edits.go‎

Lines changed: 0 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -17,74 +17,18 @@
1717
package edits
1818

1919
import (
20-
"fmt"
21-
2220
ociSpecs "github.com/opencontainers/runtime-spec/specs-go"
2321
"tags.cncf.io/container-device-interface/pkg/cdi"
2422
"tags.cncf.io/container-device-interface/specs-go"
2523

26-
"github.com/NVIDIA/nvidia-container-toolkit/internal/discover"
2724
"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
28-
"github.com/NVIDIA/nvidia-container-toolkit/internal/oci"
2925
)
3026

3127
type edits struct {
3228
cdi.ContainerEdits
3329
logger logger.Interface
3430
}
3531

36-
// NewSpecEdits creates a SpecModifier that defines the required OCI spec edits (as CDI ContainerEdits) from the specified
37-
// discoverer.
38-
func NewSpecEdits(logger logger.Interface, d discover.Discover) (oci.SpecModifier, error) {
39-
c, err := FromDiscoverer(d)
40-
if err != nil {
41-
return nil, fmt.Errorf("error constructing container edits: %v", err)
42-
}
43-
e := edits{
44-
ContainerEdits: *c,
45-
logger: logger,
46-
}
47-
48-
return &e, nil
49-
}
50-
51-
// FromDiscoverer creates CDI container edits for the specified discoverer.
52-
func FromDiscoverer(d discover.Discover) (*cdi.ContainerEdits, error) {
53-
devices, err := d.Devices()
54-
if err != nil {
55-
return nil, fmt.Errorf("failed to discover devices: %v", err)
56-
}
57-
58-
mounts, err := d.Mounts()
59-
if err != nil {
60-
return nil, fmt.Errorf("failed to discover mounts: %v", err)
61-
}
62-
63-
hooks, err := d.Hooks()
64-
if err != nil {
65-
return nil, fmt.Errorf("failed to discover hooks: %v", err)
66-
}
67-
68-
c := NewContainerEdits()
69-
for _, d := range devices {
70-
edits, err := device(d).toEdits()
71-
if err != nil {
72-
return nil, fmt.Errorf("failed to created container edits for device: %v", err)
73-
}
74-
c.Append(edits)
75-
}
76-
77-
for _, m := range mounts {
78-
c.Append(mount(m).toEdits())
79-
}
80-
81-
for _, h := range hooks {
82-
c.Append(hook(h).toEdits())
83-
}
84-
85-
return c, nil
86-
}
87-
8832
// NewContainerEdits is a utility function to create a CDI ContainerEdits struct.
8933
func NewContainerEdits() *cdi.ContainerEdits {
9034
c := cdi.ContainerEdits{

‎internal/edits/edits_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import (
2525
)
2626

2727
func TestFromDiscovererAllowsMountsToIterate(t *testing.T) {
28-
edits, err := FromDiscoverer(discover.None{})
28+
edits, err := New().EditsFromDiscoverer(discover.None{})
2929
require.NoError(t, err)
3030

3131
require.Empty(t, edits.Mounts)

‎internal/edits/lib.go‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
/**
2+
# Copyright 2024 NVIDIA CORPORATION
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
**/
16+
17+
package edits
18+
19+
import (
20+
"fmt"
21+
22+
"tags.cncf.io/container-device-interface/pkg/cdi"
23+
24+
"github.com/NVIDIA/nvidia-container-toolkit/internal/discover"
25+
"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
26+
"github.com/NVIDIA/nvidia-container-toolkit/internal/oci"
27+
)
28+
29+
// New creates an Interface from the supplied options.
30+
func New(opts ...Option) Interface {
31+
o := &options{}
32+
for _, opt := range opts {
33+
opt(o)
34+
}
35+
if o.logger == nil {
36+
o.logger = logger.New()
37+
}
38+
39+
return o
40+
}
41+
42+
// EditsFromDiscoverer creates CDI container edits for the specified discoverer.
43+
func (o *options) EditsFromDiscoverer(d discover.Discover) (*cdi.ContainerEdits, error) {
44+
devices, err := d.Devices()
45+
if err != nil {
46+
return nil, fmt.Errorf("failed to discover devices: %w", err)
47+
}
48+
49+
mounts, err := d.Mounts()
50+
if err != nil {
51+
return nil, fmt.Errorf("failed to discover mounts: %w", err)
52+
}
53+
54+
hooks, err := d.Hooks()
55+
if err != nil {
56+
return nil, fmt.Errorf("failed to discover hooks: %w", err)
57+
}
58+
59+
c := NewContainerEdits()
60+
for _, d := range devices {
61+
edits, err := device(d).toEdits()
62+
if err != nil {
63+
return nil, fmt.Errorf("failed to create container edits for device: %w", err)
64+
}
65+
c.Append(edits)
66+
}
67+
68+
for _, m := range mounts {
69+
c.Append(mount(m).toEdits())
70+
}
71+
72+
for _, h := range hooks {
73+
c.Append(hook(h).toEdits())
74+
}
75+
76+
return c, nil
77+
}
78+
79+
// SpecModifierFromDiscoverer creates a SpecModifier that defines the required OCI spec edits (as CDI ContainerEdits) from the specified
80+
// discoverer.
81+
func (o *options) SpecModifierFromDiscoverer(d discover.Discover) (oci.SpecModifier, error) {
82+
c, err := o.EditsFromDiscoverer(d)
83+
if err != nil {
84+
return nil, fmt.Errorf("error constructing container edits: %w", err)
85+
}
86+
e := edits{
87+
ContainerEdits: *c,
88+
logger: o.logger,
89+
}
90+
91+
return &e, nil
92+
}

‎internal/edits/options.go‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/**
2+
# Copyright 2024 NVIDIA CORPORATION
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
**/
16+
17+
package edits
18+
19+
import "github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
20+
21+
type options struct {
22+
logger logger.Interface
23+
}
24+
25+
type Option func(*options)
26+
27+
func WithLogger(logger logger.Interface) Option {
28+
return func(o *options) {
29+
o.logger = logger
30+
}
31+
}

‎internal/edits/resource.go‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
# Copyright 2024 NVIDIA CORPORATION
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
**/
16+
17+
package edits
18+
19+
import (
20+
"tags.cncf.io/container-device-interface/pkg/cdi"
21+
"tags.cncf.io/container-device-interface/specs-go"
22+
)
23+
24+
// NewResource creates a CDI resource (Device) with the specified name.
25+
func NewResource(name string, edits *cdi.ContainerEdits) specs.Device {
26+
return specs.Device{
27+
Name: name,
28+
ContainerEdits: *edits.ContainerEdits,
29+
}
30+
}

‎internal/modifier/discover.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ func NewModifierFromDiscoverer(logger logger.Interface, d discover.Discover) (oc
4545
// Modify applies the modifications required by discoverer to the incomming OCI spec.
4646
// These modifications are applied in-place.
4747
func (m discoverModifier) Modify(spec *specs.Spec) error {
48-
specEdits, err := edits.NewSpecEdits(m.logger, m.discoverer)
48+
e := edits.New(
49+
edits.WithLogger(m.logger),
50+
)
51+
52+
specEdits, err := e.SpecModifierFromDiscoverer(m.discoverer)
4953
if err != nil {
5054
return fmt.Errorf("failed to get required container edits: %v", err)
5155
}

‎pkg/nvcdi/full-gpu-nvml.go‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import (
3030

3131
// GetGPUDeviceSpecs returns the CDI device specs for the full GPU represented by 'device'.
3232
func (l *nvmllib) GetGPUDeviceSpecs(i int, d device.Device) ([]specs.Device, error) {
33-
edits, err := l.GetGPUDeviceEdits(d)
33+
e, err := l.GetGPUDeviceEdits(d)
3434
if err != nil {
3535
return nil, fmt.Errorf("failed to get edits for device: %v", err)
3636
}
@@ -41,10 +41,7 @@ func (l *nvmllib) GetGPUDeviceSpecs(i int, d device.Device) ([]specs.Device, err
4141
return nil, fmt.Errorf("failed to get device name: %v", err)
4242
}
4343
for _, name := range names {
44-
spec := specs.Device{
45-
Name: name,
46-
ContainerEdits: *edits.ContainerEdits,
47-
}
44+
spec := edits.NewResource(name, e)
4845
deviceSpecs = append(deviceSpecs, spec)
4946
}
5047

@@ -58,12 +55,7 @@ func (l *nvmllib) GetGPUDeviceEdits(d device.Device) (*cdi.ContainerEdits, error
5855
return nil, fmt.Errorf("failed to create device discoverer: %v", err)
5956
}
6057

61-
editsForDevice, err := edits.FromDiscoverer(device)
62-
if err != nil {
63-
return nil, fmt.Errorf("failed to create container edits for device: %v", err)
64-
}
65-
66-
return editsForDevice, nil
58+
return (*nvcdilib)(l).editsFromDiscoverer(device)
6759
}
6860

6961
// newFullGPUDiscoverer creates a discoverer for the full GPU defined by the specified device.

‎pkg/nvcdi/gds.go‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,22 +38,19 @@ func (l *gdslib) GetAllDeviceSpecs() ([]specs.Device, error) {
3838
if err != nil {
3939
return nil, fmt.Errorf("failed to create GPUDirect Storage discoverer: %v", err)
4040
}
41-
edits, err := edits.FromDiscoverer(discoverer)
41+
e, err := (*nvcdilib)(l).editsFromDiscoverer(discoverer)
4242
if err != nil {
4343
return nil, fmt.Errorf("failed to create container edits for GPUDirect Storage: %v", err)
4444
}
4545

46-
deviceSpec := specs.Device{
47-
Name: "all",
48-
ContainerEdits: *edits.ContainerEdits,
49-
}
46+
deviceSpec := edits.NewResource("all", e)
5047

5148
return []specs.Device{deviceSpec}, nil
5249
}
5350

5451
// GetCommonEdits generates a CDI specification that can be used for ANY devices
5552
func (l *gdslib) GetCommonEdits() (*cdi.ContainerEdits, error) {
56-
return edits.FromDiscoverer(discover.None{})
53+
return edits.NewContainerEdits(), nil
5754
}
5855

5956
// GetSpec is unsppported for the gdslib specs.

‎pkg/nvcdi/lib-csv.go‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323
"tags.cncf.io/container-device-interface/pkg/cdi"
2424
"tags.cncf.io/container-device-interface/specs-go"
2525

26-
"github.com/NVIDIA/nvidia-container-toolkit/internal/discover"
2726
"github.com/NVIDIA/nvidia-container-toolkit/internal/edits"
2827
"github.com/NVIDIA/nvidia-container-toolkit/internal/platform-support/tegra"
2928
"github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/spec"
@@ -53,7 +52,7 @@ func (l *csvlib) GetAllDeviceSpecs() ([]specs.Device, error) {
5352
if err != nil {
5453
return nil, fmt.Errorf("failed to create discoverer for CSV files: %v", err)
5554
}
56-
e, err := edits.FromDiscoverer(d)
55+
e, err := (*nvcdilib)(l).editsFromDiscoverer(d)
5756
if err != nil {
5857
return nil, fmt.Errorf("failed to create container edits for CSV files: %v", err)
5958
}
@@ -64,10 +63,7 @@ func (l *csvlib) GetAllDeviceSpecs() ([]specs.Device, error) {
6463
}
6564
var deviceSpecs []specs.Device
6665
for _, name := range names {
67-
deviceSpec := specs.Device{
68-
Name: name,
69-
ContainerEdits: *e.ContainerEdits,
70-
}
66+
deviceSpec := edits.NewResource(name, e)
7167
deviceSpecs = append(deviceSpecs, deviceSpec)
7268
}
7369

@@ -76,8 +72,7 @@ func (l *csvlib) GetAllDeviceSpecs() ([]specs.Device, error) {
7672

7773
// GetCommonEdits generates a CDI specification that can be used for ANY devices
7874
func (l *csvlib) GetCommonEdits() (*cdi.ContainerEdits, error) {
79-
d := discover.None{}
80-
return edits.FromDiscoverer(d)
75+
return edits.NewContainerEdits(), nil
8176
}
8277

8378
// GetGPUDeviceEdits generates a CDI specification that can be used for GPU devices

0 commit comments

Comments
 (0)