From b414729b8b1b2af8690b688c70eeafffa7a330c6 Mon Sep 17 00:00:00 2001 From: Krisztian Litkey Date: Tue, 15 Oct 2024 18:13:18 +0300 Subject: [PATCH 1/4] Makefile: add [golang]ci-lint target. And hook it into [pre-]pr-checks. Signed-off-by: Krisztian Litkey --- Makefile | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 27d40615..bc06c927 100644 --- a/Makefile +++ b/Makefile @@ -16,6 +16,7 @@ GO_TEST := $(GO_CMD) test -race -v -cover GO_LINT := golint -set_exit_status GO_FMT := gofmt GO_VET := $(GO_CMD) vet +CI_LINT := golangci-lint CDI_PKG := $(shell grep ^module go.mod | sed 's/^module *//g') @@ -43,7 +44,7 @@ test: test-gopkgs test-schema # validation targets # -pre-pr-checks pr-checks: test fmt lint vet +pre-pr-checks pr-checks: test fmt lint vet ci-lint fmt format: $(Q)$(GO_FMT) -s -d -w -e . @@ -52,6 +53,9 @@ lint: $(Q)$(GO_LINT) -set_exit_status ./... vet: $(Q)$(GO_VET) ./... +golangci-lint ci-lint: + $(Q)$(CI_LINT) run + # # build targets From 78c60415044fcf336fc39516cb3c8b6a544b90fc Mon Sep 17 00:00:00 2001 From: Krisztian Litkey Date: Tue, 15 Oct 2024 18:14:34 +0300 Subject: [PATCH 2/4] pkg/cdi: address complaints by golangci-lint. Signed-off-by: Krisztian Litkey --- pkg/cdi/cache.go | 16 ++++++++-------- pkg/cdi/default-cache_test.go | 9 ++++++--- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/pkg/cdi/cache.go b/pkg/cdi/cache.go index c2f7fe34..a1fb84ed 100644 --- a/pkg/cdi/cache.go +++ b/pkg/cdi/cache.go @@ -116,7 +116,7 @@ func (c *Cache) configure(options ...Option) { c.watch.setup(c.specDirs, c.dirErrors) c.watch.start(&c.Mutex, c.refresh, c.dirErrors) } - c.refresh() + _ = c.refresh() // we record but ignore errors } // Refresh rescans the CDI Spec directories and refreshes the Cache. @@ -233,7 +233,7 @@ func (c *Cache) InjectDevices(ociSpec *oci.Spec, devices ...string) ([]string, e c.Lock() defer c.Unlock() - c.refreshIfRequired(false) + _, _ = c.refreshIfRequired(false) // we record but ignore errors edits := &ContainerEdits{} specs := map[*Spec]struct{}{} @@ -340,7 +340,7 @@ func (c *Cache) GetDevice(device string) *Device { c.Lock() defer c.Unlock() - c.refreshIfRequired(false) + _, _ = c.refreshIfRequired(false) // we record but ignore errors return c.devices[device] } @@ -352,7 +352,7 @@ func (c *Cache) ListDevices() []string { c.Lock() defer c.Unlock() - c.refreshIfRequired(false) + _, _ = c.refreshIfRequired(false) // we record but ignore errors for name := range c.devices { devices = append(devices, name) @@ -369,7 +369,7 @@ func (c *Cache) ListVendors() []string { c.Lock() defer c.Unlock() - c.refreshIfRequired(false) + _, _ = c.refreshIfRequired(false) // we record but ignore errors for vendor := range c.specs { vendors = append(vendors, vendor) @@ -389,7 +389,7 @@ func (c *Cache) ListClasses() []string { c.Lock() defer c.Unlock() - c.refreshIfRequired(false) + _, _ = c.refreshIfRequired(false) // we record but ignore errors for _, specs := range c.specs { for _, spec := range specs { @@ -409,7 +409,7 @@ func (c *Cache) GetVendorSpecs(vendor string) []*Spec { c.Lock() defer c.Unlock() - c.refreshIfRequired(false) + _, _ = c.refreshIfRequired(false) // we record but ignore errors return c.specs[vendor] } @@ -544,7 +544,7 @@ func (w *watch) watch(fsw *fsnotify.Watcher, m *sync.Mutex, refresh func() error } else { w.update(dirErrors) } - refresh() + _ = refresh() m.Unlock() case _, ok := <-watch.Errors: diff --git a/pkg/cdi/default-cache_test.go b/pkg/cdi/default-cache_test.go index 1b3de6a9..3b4e6351 100644 --- a/pkg/cdi/default-cache_test.go +++ b/pkg/cdi/default-cache_test.go @@ -115,7 +115,8 @@ devices: filepath.Join(dir, "run"), ), } - Configure(opts...) + err = Configure(opts...) + require.NoError(t, err, "unexpected Configure() error") devices := cache.ListDevices() if len(tc.devices) == 0 { @@ -261,7 +262,8 @@ devices: filepath.Join(dir, "run"), ), } - Configure(opts...) + err = Configure(opts...) + require.NoError(t, err, "unexpected Configure() error") } else { err = updateSpecDirs(dir, update.etc, update.run) if err != nil { @@ -395,12 +397,13 @@ devices: return } - Configure( + err = Configure( WithSpecDirs( filepath.Join(dir, "etc"), filepath.Join(dir, "run"), ), ) + require.NoError(t, err, "unexpected Configure() error") unresolved, err := InjectDevices(tc.ociSpec, tc.devices...) if len(tc.unresolved) != 0 { From 9927475639279985604347802150ce73665bebcd Mon Sep 17 00:00:00 2001 From: Krisztian Litkey Date: Tue, 15 Oct 2024 18:23:01 +0300 Subject: [PATCH 3/4] .github: add golangci-lint workflow. Signed-off-by: Krisztian Litkey --- .github/workflows/golangci-lint.yml | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 .github/workflows/golangci-lint.yml diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml new file mode 100644 index 00000000..cbb8e32f --- /dev/null +++ b/.github/workflows/golangci-lint.yml @@ -0,0 +1,27 @@ +name: CI +on: + push: + branches: + - main + pull_request: + branches: + - main + +permissions: + contents: read + # Optional: allow read access to pull request. Use with `only-new-issues` option. + # pull-requests: read + +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: stable + - name: golangci-lint + uses: golangci/golangci-lint-action@v6 + with: + version: v1.61.0 From 84ea77f793cab61626c7c9ec13bcbe0fe875b628 Mon Sep 17 00:00:00 2001 From: Krisztian Litkey Date: Wed, 16 Oct 2024 14:08:02 +0300 Subject: [PATCH 4/4] pkg/cdi: document possible implicit cache refresh. Update public API documentation to point out a possible implicit cache refresh and how to query any errors encountered during it. Signed-off-by: Krisztian Litkey --- pkg/cdi/cache.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/pkg/cdi/cache.go b/pkg/cdi/cache.go index a1fb84ed..04f15e02 100644 --- a/pkg/cdi/cache.go +++ b/pkg/cdi/cache.go @@ -222,7 +222,8 @@ func (c *Cache) refreshIfRequired(force bool) (bool, error) { // InjectDevices injects the given qualified devices to an OCI Spec. It // returns any unresolvable devices and an error if injection fails for -// any of the devices. +// any of the devices. Might trigger a cache refresh, in which case any +// errors encountered can be obtained using GetErrors(). func (c *Cache) InjectDevices(ociSpec *oci.Spec, devices ...string) ([]string, error) { var unresolved []string @@ -335,7 +336,9 @@ func (c *Cache) RemoveSpec(name string) error { return err } -// GetDevice returns the cached device for the given qualified name. +// GetDevice returns the cached device for the given qualified name. Might trigger +// a cache refresh, in which case any errors encountered can be obtained using +// GetErrors(). func (c *Cache) GetDevice(device string) *Device { c.Lock() defer c.Unlock() @@ -345,7 +348,8 @@ func (c *Cache) GetDevice(device string) *Device { return c.devices[device] } -// ListDevices lists all cached devices by qualified name. +// 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 @@ -362,7 +366,8 @@ func (c *Cache) ListDevices() []string { return devices } -// ListVendors lists all vendors known to the cache. +// 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 @@ -379,7 +384,8 @@ func (c *Cache) ListVendors() []string { return vendors } -// ListClasses lists all device classes known to the cache. +// 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{}{} @@ -404,7 +410,8 @@ func (c *Cache) ListClasses() []string { return classes } -// GetVendorSpecs returns all specs for the given vendor. +// GetVendorSpecs returns all specs for the given vendor. Might trigger a cache +// refresh, in which case any errors encountered can be obtained using GetErrors(). func (c *Cache) GetVendorSpecs(vendor string) []*Spec { c.Lock() defer c.Unlock()