Skip to content

Commit

Permalink
Add root to mounts type
Browse files Browse the repository at this point in the history
This change adds a root member to the mounts type that is used to
perform most of the lookups for files and devices. This allows
for consistent handling of relative paths.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
  • Loading branch information
elezar committed Jul 18, 2022
1 parent acc0afb commit 34e80ab
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 57 deletions.
12 changes: 5 additions & 7 deletions internal/discover/char_devices.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,14 @@ var _ Discover = (*charDevices)(nil)
func NewCharDeviceDiscoverer(logger *logrus.Logger, devices []string, root string) Discover {
locator := lookup.NewCharDeviceLocator(logger, root)

return NewDeviceDiscoverer(logger, locator, devices)
return NewDeviceDiscoverer(logger, locator, root, devices)
}

// NewDeviceDiscoverer creates a discoverer which locates the specified set of device nodes using the specified locator.
func NewDeviceDiscoverer(logger *logrus.Logger, locator lookup.Locator, devices []string) Discover {
return &charDevices{
logger: logger,
lookup: locator,
required: devices,
}
func NewDeviceDiscoverer(logger *logrus.Logger, locator lookup.Locator, root string, devices []string) Discover {
m := NewMounts(logger, locator, root, devices).(*mounts)

return (*charDevices)(m)
}

// Mounts returns the discovered mounts for the charDevices.
Expand Down
12 changes: 4 additions & 8 deletions internal/discover/csv.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func NewFromCSVFiles(logger *logrus.Logger, files []string, root string) (Discov
mountSpecs = append(mountSpecs, targets...)
}

return newFromMountSpecs(logger, locators, mountSpecs)
return newFromMountSpecs(logger, locators, root, mountSpecs)
}

// loadCSVFile loads the specified CSV file and returns the list of mount specs
Expand All @@ -71,7 +71,7 @@ func loadCSVFile(logger *logrus.Logger, filename string) ([]*csv.MountSpec, erro

// newFromMountSpecs creates a discoverer for the CSV file. A logger is also supplied.
// A list of csvDiscoverers is returned, with each being associated with a single MountSpecType.
func newFromMountSpecs(logger *logrus.Logger, locators map[csv.MountSpecType]lookup.Locator, targets []*csv.MountSpec) (Discover, error) {
func newFromMountSpecs(logger *logrus.Logger, locators map[csv.MountSpecType]lookup.Locator, root string, targets []*csv.MountSpec) (Discover, error) {
if len(targets) == 0 {
return &None{}, nil
}
Expand All @@ -95,13 +95,9 @@ func newFromMountSpecs(logger *logrus.Logger, locators map[csv.MountSpecType]loo
var m Discover
switch t {
case csv.MountSpecDev:
m = NewDeviceDiscoverer(logger, locator, candidatesByType[t])
m = NewDeviceDiscoverer(logger, locator, root, candidatesByType[t])
default:
m = &mounts{
logger: logger,
lookup: locator,
required: candidatesByType[t],
}
m = NewMounts(logger, locator, root, candidatesByType[t])
}
discoverers = append(discoverers, m)

Expand Down
41 changes: 40 additions & 1 deletion internal/discover/csv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func TestNewFromMountSpec(t *testing.T) {

testCases := []struct {
description string
root string
targets []*csv.MountSpec
expectedError error
expectedDiscoverer Discover
Expand Down Expand Up @@ -76,12 +77,50 @@ func TestNewFromMountSpec(t *testing.T) {
&mounts{
logger: logger,
lookup: locators["dev"],
root: "/",
required: []string{"dev0", "dev1"},
},
),
&mounts{
logger: logger,
lookup: locators["lib"],
root: "/",
required: []string{"lib0"},
},
},
},
},
{
description: "sets root",
targets: []*csv.MountSpec{
{
Type: "dev",
Path: "dev0",
},
{
Type: "lib",
Path: "lib0",
},
{
Type: "dev",
Path: "dev1",
},
},
root: "/some/root",
expectedDiscoverer: &list{
discoverers: []Discover{
(*charDevices)(
&mounts{
logger: logger,
lookup: locators["dev"],
root: "/some/root",
required: []string{"dev0", "dev1"},
},
),
&mounts{
logger: logger,
lookup: locators["lib"],
root: "/some/root",
required: []string{"lib0"},
},
},
Expand All @@ -91,7 +130,7 @@ func TestNewFromMountSpec(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.description, func(t *testing.T) {
discoverer, err := newFromMountSpecs(logger, locators, tc.targets)
discoverer, err := newFromMountSpecs(logger, locators, tc.root, tc.targets)
if tc.expectedError != nil {
require.Error(t, err)
return
Expand Down
22 changes: 12 additions & 10 deletions internal/discover/gds.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,19 @@ func NewGDSDiscoverer(logger *logrus.Logger, root string) (Discover, error) {
root,
)

udev := &mounts{
logger: logger,
lookup: lookup.NewDirectoryLocator(logger, root),
required: []string{"/run/udev"},
}
udev := NewMounts(
logger,
lookup.NewDirectoryLocator(logger, root),
root,
[]string{"/run/udev"},
)

cufile := &mounts{
logger: logger,
lookup: lookup.NewFileLocator(logger, root),
required: []string{"/etc/cufile.json"},
}
cufile := NewMounts(
logger,
lookup.NewFileLocator(logger, root),
root,
[]string{"/etc/cufile.json"},
)

d := gdsDeviceDiscoverer{
logger: logger,
Expand Down
28 changes: 23 additions & 5 deletions internal/discover/mounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package discover

import (
"fmt"
"path/filepath"
"strings"
"sync"

"github.com/NVIDIA/nvidia-container-toolkit/internal/lookup"
Expand All @@ -31,13 +33,24 @@ type mounts struct {
None
logger *logrus.Logger
lookup lookup.Locator
root string
required []string
sync.Mutex
cache []Mount
}

var _ Discover = (*mounts)(nil)

// NewMounts creates a discoverer for the required mounts using the specified locator.
func NewMounts(logger *logrus.Logger, lookup lookup.Locator, root string, required []string) Discover {
return &mounts{
logger: logger,
lookup: lookup,
root: filepath.Join("/", root),
required: required,
}
}

func (d *mounts) Mounts() ([]Mount, error) {
if d.lookup == nil {
return nil, fmt.Errorf("no lookup defined")
Expand Down Expand Up @@ -71,11 +84,7 @@ func (d *mounts) Mounts() ([]Mount, error) {
continue
}

r, err := d.lookup.Relative(p)
if err != nil {
d.logger.Warnf("Failed to get relative path of %v: %v", p, err)
continue
}
r := d.relativeTo(p)
if r == "" {
r = p
}
Expand All @@ -97,3 +106,12 @@ func (d *mounts) Mounts() ([]Mount, error) {

return d.cache, nil
}

// relativeTo returns the path relative to the root for the file locator
func (d *mounts) relativeTo(path string) string {
if d.root == "/" {
return path
}

return strings.TrimPrefix(path, d.root)
}
29 changes: 3 additions & 26 deletions internal/discover/mounts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,37 +140,14 @@ func TestMounts(t *testing.T) {
input: &mounts{
lookup: &lookup.LocatorMock{
LocateFunc: func(s string) ([]string, error) {
return []string{"located"}, nil
},
RelativeFunc: func(s string) (string, error) {
return "relative", nil
return []string{"/some/root/located"}, nil
},
},
root: "/some/root",
required: []string{"required0", "multiple", "required1"},
},
expectedMounts: []Mount{
{Path: "relative", HostPath: "located"},
},
},
{
description: "mounts skips relative error",
input: &mounts{
lookup: &lookup.LocatorMock{
LocateFunc: func(s string) ([]string, error) {
return []string{s}, nil
},
RelativeFunc: func(s string) (string, error) {
if s == "error" {
return "", fmt.Errorf("no relative path")
}
return "relative" + s, nil
},
},
required: []string{"required0", "error", "required1"},
},
expectedMounts: []Mount{
{Path: "relativerequired0", HostPath: "required0"},
{Path: "relativerequired1", HostPath: "required1"},
{Path: "/located", HostPath: "/some/root/located"},
},
},
}
Expand Down

0 comments on commit 34e80ab

Please sign in to comment.