Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/cdi/go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module tags.cncf.io/container-device-interface/cmd/cdi

go 1.21
go 1.23

require (
github.com/fsnotify/fsnotify v1.5.1
Expand Down
2 changes: 1 addition & 1 deletion cmd/validate/go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module tags.cncf.io/container-device-interface/cmd/validate

go 1.21
go 1.23

require tags.cncf.io/container-device-interface/schema v0.0.0

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module tags.cncf.io/container-device-interface

go 1.21
go 1.23

require (
github.com/fsnotify/fsnotify v1.5.1
Expand Down
105 changes: 27 additions & 78 deletions pkg/cdi/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ import (
"errors"
"fmt"
"io/fs"
"maps"
"os"
"path/filepath"
"runtime"
"slices"
"sort"
"strings"
"sync"

Expand Down Expand Up @@ -297,25 +297,17 @@ func (c *Cache) highestPrioritySpecDir() (string, int) {
// priority Spec directory. If name has a "json" or "yaml" extension it
// choses the encoding. Otherwise the default YAML encoding is used.
func (c *Cache) WriteSpec(raw *cdi.Spec, name string) error {
var (
specDir string
path string
prio int
spec *Spec
err error
)

specDir, prio = c.highestPrioritySpecDir()
specDir, prio := c.highestPrioritySpecDir()
if specDir == "" {
return errors.New("no Spec directories to write to")
}

path = filepath.Join(specDir, name)
path := filepath.Join(specDir, name)
if ext := filepath.Ext(path); ext != ".json" && ext != ".yaml" {
path += defaultSpecExt
}

spec, err = newSpec(raw, path, prio)
spec, err := newSpec(raw, path, prio)
if err != nil {
return err
}
Expand All @@ -328,25 +320,19 @@ func (c *Cache) WriteSpec(raw *cdi.Spec, name string) error {
// Spec previously written by WriteSpec(). If the file exists and
// its removal fails RemoveSpec returns an error.
func (c *Cache) RemoveSpec(name string) error {
var (
specDir string
path string
err error
)

specDir, _ = c.highestPrioritySpecDir()
specDir, _ := c.highestPrioritySpecDir()
if specDir == "" {
return errors.New("no Spec directories to remove from")
}

path = filepath.Join(specDir, name)
path := filepath.Join(specDir, name)
if ext := filepath.Ext(path); ext != ".json" && ext != ".yaml" {
path += defaultSpecExt
}

err = os.Remove(path)
if err != nil && errors.Is(err, fs.ErrNotExist) {
err = nil
err := os.Remove(path)
if errors.Is(err, fs.ErrNotExist) {
return nil
}

return err
Expand All @@ -367,61 +353,46 @@ func (c *Cache) GetDevice(device string) *Device {
// ListDevices lists all cached devices by qualified name. Might trigger a cache
// refresh, in which case any errors encountered can be obtained using GetErrors().
func (c *Cache) ListDevices() []string {
var devices []string

c.mu.Lock()
defer c.mu.Unlock()

_, _ = c.refreshIfRequired(false) // we record but ignore errors

for name := range c.devices {
devices = append(devices, name)
}
sort.Strings(devices)

return devices
return slices.Sorted(maps.Keys(c.devices))
Comment thread
klihub marked this conversation as resolved.
}

// ListVendors lists all vendors known to the cache. Might trigger a cache refresh,
// in which case any errors encountered can be obtained using GetErrors().
func (c *Cache) ListVendors() []string {
var vendors []string

c.mu.Lock()
defer c.mu.Unlock()

_, _ = c.refreshIfRequired(false) // we record but ignore errors

for vendor := range c.specs {
vendors = append(vendors, vendor)
}
sort.Strings(vendors)

return vendors
return slices.Sorted(maps.Keys(c.specs))
}

// ListClasses lists all device classes known to the cache. Might trigger a cache
// refresh, in which case any errors encountered can be obtained using GetErrors().
func (c *Cache) ListClasses() []string {
var (
cmap = map[string]struct{}{}
classes []string
)

c.mu.Lock()
defer c.mu.Unlock()

_, _ = c.refreshIfRequired(false) // we record but ignore errors

var classes []string
seen := map[string]struct{}{}
for _, specs := range c.specs {
for _, spec := range specs {
cmap[spec.GetClass()] = struct{}{}
class := spec.GetClass()
if _, ok := seen[class]; ok {
continue
}
seen[class] = struct{}{}
classes = append(classes, class)
}
}
for class := range cmap {
classes = append(classes, class)
}
sort.Strings(classes)
slices.Sort(classes)

return classes
}
Expand All @@ -442,12 +413,7 @@ func (c *Cache) GetVendorSpecs(vendor string) []*Spec {
func (c *Cache) GetSpecErrors(spec *Spec) []error {
c.mu.RLock()
defer c.mu.RUnlock()
var errs []error
if e, ok := c.errors[spec.GetPath()]; ok {
errs = make([]error, len(e))
copy(errs, e)
}
return errs
return slices.Clone(c.errors[spec.GetPath()])
}

// GetErrors returns all errors encountered during the last
Expand All @@ -471,25 +437,14 @@ func (c *Cache) GetErrors() map[string][]error {
func (c *Cache) GetSpecDirectories() []string {
c.mu.RLock()
defer c.mu.RUnlock()

dirs := make([]string, len(c.specDirs))
copy(dirs, c.specDirs)
return dirs
return slices.Clone(c.specDirs)
}

// GetSpecDirErrors returns any errors related to configured Spec directories.
func (c *Cache) GetSpecDirErrors() map[string]error {
c.mu.RLock()
defer c.mu.RUnlock()
if c.dirErrors == nil {
return nil
}

errs := make(map[string]error)
for dir, err := range c.dirErrors {
errs[dir] = err
}
return errs
return maps.Clone(c.dirErrors)
}

// Our fsnotify helper wrapper.
Expand Down Expand Up @@ -583,13 +538,6 @@ func (w *watch) watch(fsw *fsnotify.Watcher, m sync.Locker, refresh func() error

// Update watch with pending/missing or removed directories.
func (w *watch) update(dirErrors map[string]error, removed ...string) bool {
var (
dir string
ok bool
err error
update bool
)

// If we failed to create an fsnotify.Watcher we have a nil watcher here
// (but with autoRefresh left on). One known case when this can happen is
// if we have too many open files. In that case we always return true and
Expand All @@ -598,12 +546,13 @@ func (w *watch) update(dirErrors map[string]error, removed ...string) bool {
return true
}

for dir, ok = range w.tracked {
var update bool
for dir, ok := range w.tracked {
if ok {
continue
}

err = w.watcher.Add(dir)
err := w.watcher.Add(dir)
if err == nil {
w.tracked[dir] = true
delete(dirErrors, dir)
Expand All @@ -614,7 +563,7 @@ func (w *watch) update(dirErrors map[string]error, removed ...string) bool {
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to your changes, but still: modifying a map while iterating it looks fragile.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updating a map in-place is probably OK, but I have some follow-ups that slightly cleanup this code;

}

for _, dir = range removed {
for _, dir := range removed {
w.tracked[dir] = false
dirErrors[dir] = errors.New("directory removed")
update = true
Expand Down
4 changes: 2 additions & 2 deletions pkg/cdi/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1888,7 +1888,7 @@ func TestCacheConcurrentConfigure(t *testing.T) {

go func() {
defer wg.Done()
for i := 0; i < iterations; i++ {
for i := range iterations {
if i%2 == 0 {
_ = cache.Configure(WithSpecDirs(dir1))
} else {
Expand All @@ -1899,7 +1899,7 @@ func TestCacheConcurrentConfigure(t *testing.T) {

go func() {
defer wg.Done()
for i := 0; i < iterations; i++ {
for range iterations {
cache.highestPrioritySpecDir()
}
}()
Expand Down
2 changes: 1 addition & 1 deletion schema/go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module tags.cncf.io/container-device-interface/schema

go 1.21
go 1.23

require (
github.com/santhosh-tekuri/jsonschema/v6 v6.0.3
Expand Down
Loading