Add cache for mounts

This change adds a cache to the mounts type. This means that if called to get
a list of folders, for example, the result is reused instead of recalculated.
This also avoids duplicate logging.

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

internal/discover/csv.go

@@ -119,7 +119,7 @@ func newFromMountSpecs(logger *logrus.Logger, locators map[csv.MountSpecType]loo
 
 // Mounts returns the discovered mounts for the csvDiscoverer.
 // Note that if the discoverer is for the device MountSpecType, the list of mounts is empty.
-func (d csvDiscoverer) Mounts() ([]Mount, error) {
+func (d *csvDiscoverer) Mounts() ([]Mount, error) {
 	if d.mountType == csv.MountSpecDev {
 		return d.None.Mounts()
 	}
@@ -129,7 +129,7 @@ func (d csvDiscoverer) Mounts() ([]Mount, error) {
 
 // Devices returns the discovered devices for the csvDiscoverer.
 // Note that if the discoverer is not for the device MountSpecType, the list of devices is empty.
-func (d csvDiscoverer) Devices() ([]Device, error) {
+func (d *csvDiscoverer) Devices() ([]Device, error) {
 	if d.mountType != csv.MountSpecDev {
 		return d.None.Devices()
 	}

internal/discover/csv_test.go

@@ -31,7 +31,7 @@ func TestCSVDiscoverer(t *testing.T) {
 
 	testCases := []struct {
 		description          string
-		input                csvDiscoverer
+		input                *csvDiscoverer
 		expectedMounts       []Mount
 		expectedMountsError  error
 		expectedDevicesError error
@@ -39,7 +39,7 @@ func TestCSVDiscoverer(t *testing.T) {
 	}{
 		{
 			description: "dev mounts are empty",
-			input: csvDiscoverer{
+			input: &csvDiscoverer{
 				mounts: mounts{
 					lookup: &lookup.LocatorMock{
 						LocateFunc: func(string) ([]string, error) {
@@ -54,14 +54,14 @@ func TestCSVDiscoverer(t *testing.T) {
 		},
 		{
 			description: "dev devices returns error for nil lookup",
-			input: csvDiscoverer{
+			input: &csvDiscoverer{
 				mountType: "dev",
 			},
 			expectedDevicesError: fmt.Errorf("no lookup defined"),
 		},
 		{
 			description: "lib devices are empty",
-			input: csvDiscoverer{
+			input: &csvDiscoverer{
 				mounts: mounts{
 					lookup: &lookup.LocatorMock{
 						LocateFunc: func(string) ([]string, error) {
@@ -76,7 +76,7 @@ func TestCSVDiscoverer(t *testing.T) {
 		},
 		{
 			description: "lib mounts returns error for nil lookup",
-			input: csvDiscoverer{
+			input: &csvDiscoverer{
 				mountType: "lib",
 			},
 			expectedMountsError: fmt.Errorf("no lookup defined"),

internal/discover/mounts.go

@@ -18,6 +18,7 @@ package discover
 
 import (
 	"fmt"
+	"sync"
 
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/lookup"
 	"github.com/sirupsen/logrus"
@@ -31,15 +32,25 @@ type mounts struct {
 	logger   *logrus.Logger
 	lookup   lookup.Locator
 	required []string
+	sync.Mutex
+	cache []Mount
 }
 
 var _ Discover = (*mounts)(nil)
 
-func (d mounts) Mounts() ([]Mount, error) {
+func (d *mounts) Mounts() ([]Mount, error) {
 	if d.lookup == nil {
 		return nil, fmt.Errorf("no lookup defined")
 	}
 
+	if d.cache != nil {
+		d.logger.Debugf("returning cached mounts")
+		return d.cache, nil
+	}
+
+	d.Lock()
+	defer d.Unlock()
+
 	paths := make(map[string]bool)
 
 	for _, candidate := range d.required {
@@ -68,5 +79,7 @@ func (d mounts) Mounts() ([]Mount, error) {
 		mounts = append(mounts, mount)
 	}
 
+	d.cache = mounts
+
 	return mounts, nil
 }

internal/discover/mounts_test.go

@@ -41,16 +41,17 @@ func TestMounts(t *testing.T) {
 		description    string
 		expectedError  error
 		expectedMounts []Mount
-		input          mounts
+		input          *mounts
 	}{
 		{
 			description:   "nill lookup returns error",
 			expectedError: fmt.Errorf("no lookup defined"),
+			input:         &mounts{},
 		},
 		{
 			description:   "empty required returns no mounts",
 			expectedError: nil,
-			input: mounts{
+			input: &mounts{
 				lookup: &lookup.LocatorMock{
 					LocateFunc: func(string) ([]string, error) {
 						return []string{"located"}, nil
@@ -61,7 +62,7 @@ func TestMounts(t *testing.T) {
 		{
 			description:   "required returns located",
 			expectedError: nil,
-			input: mounts{
+			input: &mounts{
 				lookup: &lookup.LocatorMock{
 					LocateFunc: func(string) ([]string, error) {
 						return []string{"located"}, nil
@@ -74,7 +75,7 @@ func TestMounts(t *testing.T) {
 		{
 			description:   "mounts removes located duplicates",
 			expectedError: nil,
-			input: mounts{
+			input: &mounts{
 				lookup: &lookup.LocatorMock{
 					LocateFunc: func(string) ([]string, error) {
 						return []string{"located"}, nil
@@ -86,7 +87,7 @@ func TestMounts(t *testing.T) {
 		},
 		{
 			description: "mounts skips located errors",
-			input: mounts{
+			input: &mounts{
 				lookup: &lookup.LocatorMock{
 					LocateFunc: func(s string) ([]string, error) {
 						if s == "error" {
@@ -101,7 +102,7 @@ func TestMounts(t *testing.T) {
 		},
 		{
 			description: "mounts skips unlocated",
-			input: mounts{
+			input: &mounts{
 				lookup: &lookup.LocatorMock{
 					LocateFunc: func(s string) ([]string, error) {
 						if s == "empty" {
@@ -116,7 +117,7 @@ func TestMounts(t *testing.T) {
 		},
 		{
 			description: "mounts skips unlocated",
-			input: mounts{
+			input: &mounts{
 				lookup: &lookup.LocatorMock{
 					LocateFunc: func(s string) ([]string, error) {
 						if s == "multiple" {