Skip to content

Commit

Permalink
chore: eliminate unreachable error paths in cache
Browse files Browse the repository at this point in the history
Signed-off-by: Markus Lehtonen <[email protected]>
  • Loading branch information
marquiz committed Jan 12, 2024
1 parent 2ae6e1a commit 6e50a6b
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 35 deletions.
4 changes: 1 addition & 3 deletions cmd/cdi/cmd/cdi-api.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,7 @@ func cdiResolveDevices(ociSpecFiles ...string) error {
err error
)

if cache, err = cdi.NewCache(); err != nil {
return fmt.Errorf("failed to create CDI cache instance: %w", err)
}
cache = cdi.NewCache()

for _, ociSpecFile := range ociSpecFiles {
ociSpec, err = readOCISpec(ociSpecFile)
Expand Down
13 changes: 7 additions & 6 deletions pkg/cdi/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func WithAutoRefresh(autoRefresh bool) Option {
// NewCache creates a new CDI Cache. The cache is populated from a set
// of CDI Spec directories. These can be specified using a WithSpecDirs
// option. The default set of directories is exposed in DefaultSpecDirs.
func NewCache(options ...Option) (*Cache, error) {
func NewCache(options ...Option) *Cache {
c := &Cache{
autoRefresh: true,
watch: &watch{},
Expand All @@ -72,7 +72,8 @@ func NewCache(options ...Option) (*Cache, error) {
c.Lock()
defer c.Unlock()

return c, c.configure(options...)
c.configure(options...)
return c
}

// Configure applies options to the Cache. Updates and refreshes the
Expand All @@ -85,12 +86,14 @@ func (c *Cache) Configure(options ...Option) error {
c.Lock()
defer c.Unlock()

return c.configure(options...)
c.configure(options...)

return nil
}

// Configure the Cache. Start/stop CDI Spec directory watch, refresh
// the Cache if necessary.
func (c *Cache) configure(options ...Option) error {
func (c *Cache) configure(options ...Option) {
for _, o := range options {
o(c)
}
Expand All @@ -103,8 +106,6 @@ func (c *Cache) configure(options ...Option) error {
c.watch.start(&c.Mutex, c.refresh, c.dirErrors)
}
c.refresh()

return nil
}

// Refresh rescans the CDI Spec directories and refreshes the Cache.
Expand Down
29 changes: 9 additions & 20 deletions pkg/cdi/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ devices:
}
}

cache, err = NewCache(WithSpecDirs(
cache = NewCache(WithSpecDirs(
filepath.Join(dir, "etc"),
filepath.Join(dir, "run")),
)
Expand All @@ -196,9 +196,6 @@ devices:
}
}

if len(tc.errors) == 0 {
require.Nil(t, err)
}
require.NotNil(t, cache)

for name, dev := range cache.devices {
Expand Down Expand Up @@ -557,8 +554,7 @@ devices:
if !selfRefresh {
opts = append(opts, WithAutoRefresh(false))
}
cache, err = NewCache(opts...)
require.NoError(t, err)
cache = NewCache(opts...)
require.NotNil(t, cache)
} else {
err = updateSpecDirs(t, dir, update.etc, update.run)
Expand Down Expand Up @@ -789,13 +785,12 @@ devices:
dir, err = createSpecDirs(t, tc.updates[0].etc, tc.updates[0].run)
require.NoError(t, err)

cache, err = NewCache(
cache = NewCache(
WithSpecDirs(
filepath.Join(dir, "etc"),
filepath.Join(dir, "run"),
),
)
require.NoError(t, err)
require.NotNil(t, cache)

go injector()
Expand Down Expand Up @@ -1176,13 +1171,12 @@ devices:
t.Errorf("failed to create test directory: %v", err)
return
}
cache, err = NewCache(
cache = NewCache(
WithSpecDirs(
filepath.Join(dir, "etc"),
filepath.Join(dir, "run"),
),
)
require.Nil(t, err)
require.NotNil(t, cache)

unresolved, err := cache.InjectDevices(tc.ociSpec, tc.devices...)
Expand Down Expand Up @@ -1415,13 +1409,12 @@ devices:
t.Errorf("failed to create test directory: %v", err)
return
}
cache, err = NewCache(
cache = NewCache(
WithSpecDirs(
filepath.Join(dir, "etc"),
filepath.Join(dir, "run"),
),
)
require.Nil(t, err)
require.NotNil(t, cache)

vendors := cache.ListVendors()
Expand Down Expand Up @@ -1571,15 +1564,14 @@ containerEdits:
if len(tc.invalid) != 0 {
dir, err = createSpecDirs(t, nil, nil)
require.NoError(t, err)
cache, err = NewCache(
cache = NewCache(
WithSpecDirs(
filepath.Join(dir, "etc"),
filepath.Join(dir, "run"),
),
WithAutoRefresh(false),
)

require.NoError(t, err)
require.NotNil(t, cache)

etc = map[string]string{}
Expand All @@ -1604,21 +1596,19 @@ containerEdits:
dir, err = createSpecDirs(t, etc, nil)
require.NoError(t, err)

cache, err = NewCache(
cache = NewCache(
WithSpecDirs(
filepath.Join(dir, "etc"),
),
)
require.NoError(t, err)
require.NotNil(t, cache)

other, err = NewCache(
other = NewCache(
WithSpecDirs(
filepath.Join(dir, "run"),
),
WithAutoRefresh(false),
)
require.NoError(t, err)
require.NotNil(t, other)

cSpecs := map[string]*cdi.Spec{}
Expand Down Expand Up @@ -1796,15 +1786,14 @@ devices:

dir, err = createSpecDirs(t, nil, nil)
require.NoError(t, err)
cache, err = NewCache(
cache = NewCache(
WithSpecDirs(
filepath.Join(dir, "etc"),
filepath.Join(dir, "run"),
),
WithAutoRefresh(false),
)

require.NoError(t, err)
require.NotNil(t, cache)

for idx, data := range tc.specs {
Expand Down
7 changes: 1 addition & 6 deletions pkg/cdi/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ var (
func GetRegistry(options ...Option) Registry {
var new bool
initOnce.Do(func() {
reg, _ = getRegistry(options...)
reg = &registry{NewCache(options...)}
new = true
})
if !new && len(options) > 0 {
Expand All @@ -144,8 +144,3 @@ func (r *registry) DeviceDB() RegistryDeviceDB {
func (r *registry) SpecDB() RegistrySpecDB {
return r
}

func getRegistry(options ...Option) (*registry, error) {
c, err := NewCache(options...)
return &registry{c}, err
}

0 comments on commit 6e50a6b

Please sign in to comment.