From 3eca7dfd7b266be551a77140d2f4b341edcb88ac Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 25 Aug 2023 16:09:43 +0200 Subject: [PATCH 01/19] Replace check targets with golangci-lint Signed-off-by: Evan Lezar --- .gitlab-ci.yml | 36 ++++-------------------------------- Makefile | 30 +++++------------------------- docker/Dockerfile.devel | 2 ++ 3 files changed, 11 insertions(+), 57 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ee74ce1d..2b2b5dfe 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -25,43 +25,15 @@ build-dev-image: .requires-build-image: image: "${BUILDIMAGE}" + needs: + - job: build-dev-image -.go-check: +check: extends: - .requires-build-image stage: go-checks - -fmt: - extends: - - .go-check script: - - make assert-fmt - -vet: - extends: - - .go-check - script: - - make vet - -lint: - extends: - - .go-check - script: - - make lint - allow_failure: true - -ineffassign: - extends: - - .go-check - script: - - make ineffassign - allow_failure: true - -misspell: - extends: - - .go-check - script: - - make misspell + - make check go-build: extends: diff --git a/Makefile b/Makefile index 7f7d375b..420b0a5a 100644 --- a/Makefile +++ b/Makefile @@ -38,7 +38,7 @@ EXAMPLE_TARGETS := $(patsubst %,example-%, $(EXAMPLES)) CMDS := $(patsubst ./cmd/%/,%,$(sort $(dir $(wildcard ./cmd/*/)))) CMD_TARGETS := $(patsubst %,cmd-%, $(CMDS)) -CHECK_TARGETS := assert-fmt vet lint ineffassign misspell +CHECK_TARGETS := golangci-lint MAKE_TARGETS := binaries build check fmt lint-internal test examples cmds coverage generate licenses $(CHECK_TARGETS) TARGETS := $(MAKE_TARGETS) $(EXAMPLE_TARGETS) $(CMD_TARGETS) @@ -78,30 +78,8 @@ fmt: go list -f '{{.Dir}}' $(MODULE)/... \ | xargs gofmt -s -l -w -assert-fmt: - go list -f '{{.Dir}}' $(MODULE)/... \ - | xargs gofmt -s -l > fmt.out - @if [ -s fmt.out ]; then \ - echo "\nERROR: The following files are not formatted:\n"; \ - cat fmt.out; \ - rm fmt.out; \ - exit 1; \ - else \ - rm fmt.out; \ - fi - -ineffassign: - ineffassign $(MODULE)/... - -lint: -# We use `go list -f '{{.Dir}}' $(MODULE)/...` to skip the `vendor` folder. - go list -f '{{.Dir}}' $(MODULE)/... | xargs golint -set_exit_status - -misspell: - misspell $(MODULE)/... - -vet: - go vet $(MODULE)/... +golangci-lint: + golangci-lint run ./... licenses: go-licenses csv $(MODULE)/... @@ -141,6 +119,7 @@ $(DOCKER_TARGETS): docker-%: .build-image $(DOCKER) run \ --rm \ -e GOCACHE=/tmp/.cache \ + -e GOLANGCI_LINT_CACHE=/tmp/.cache \ -v $(PWD):$(PWD) \ -w $(PWD) \ --user $$(id -u):$$(id -g) \ @@ -154,6 +133,7 @@ PHONY: .shell --rm \ -ti \ -e GOCACHE=/tmp/.cache \ + -e GOLANGCI_LINT_CACHE=/tmp/.cache \ -v $(PWD):$(PWD) \ -w $(PWD) \ --user $$(id -u):$$(id -g) \ diff --git a/docker/Dockerfile.devel b/docker/Dockerfile.devel index 20d09b60..a3aa25d0 100644 --- a/docker/Dockerfile.devel +++ b/docker/Dockerfile.devel @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. ARG GOLANG_VERSION=x.x.x +ARG GOLANGCI_LINT_VERSION=v1.54.1 FROM golang:${GOLANG_VERSION} RUN go install golang.org/x/lint/golint@6edffad5e6160f5949cdefc81710b2706fbcd4f6 @@ -19,3 +20,4 @@ RUN go install github.com/matryer/moq@latest RUN go install github.com/gordonklaus/ineffassign@d2c82e48359b033cde9cf1307f6d5550b8d61321 RUN go install github.com/client9/misspell/cmd/misspell@latest RUN go install github.com/google/go-licenses@latest +RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin ${GOLANGCI_LINT_VERSION} \ No newline at end of file From 7f610d19edde49cc725daac63c67a0cd1f745a7d Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 25 Aug 2023 16:09:58 +0200 Subject: [PATCH 02/19] Add .golangci.yml config Signed-off-by: Evan Lezar --- .golangci.yml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..88bc08e4 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,16 @@ +run: + deadline: 10m + +linters: + enable: + - contextcheck + - gocritic + - gofmt + - goimports + - gosec + - gosimple + - govet + - ineffassign + - misspell + - staticcheck + - unconvert From d8d56e18f9eb885b4b20bade935d84ab2fc0b3a7 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Mon, 28 Aug 2023 11:08:36 +0200 Subject: [PATCH 03/19] Add nolint for dxcore Signed-off-by: Evan Lezar --- .golangci.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.golangci.yml b/.golangci.yml index 88bc08e4..80492d3e 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -14,3 +14,11 @@ linters: - misspell - staticcheck - unconvert + +issues: + exclude-rules: + # Exclude the gocritic dupSubExpr issue for cgo files. + - path: internal/dxcore/dxcore.go + linters: + - gocritic + text: dupSubExpr From 49dbae5c325a6c231325869d131530ca2317b78c Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Wed, 30 Aug 2023 16:08:10 +0200 Subject: [PATCH 04/19] Use .golangci config for toml.Delete issues Signed-off-by: Evan Lezar --- .golangci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.golangci.yml b/.golangci.yml index 80492d3e..cd7efc23 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -22,3 +22,8 @@ issues: linters: - gocritic text: dupSubExpr + # Exclude the checks for usage of returns to config.Delete(Path) in the crio and containerd config packages. + - path: pkg/config/engine/ + linters: + - errcheck + text: config.Delete From 73749285d5212ad99dcdc3d466412a95a36ad546 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 25 Aug 2023 16:11:24 +0200 Subject: [PATCH 05/19] Remove unused loadSaver interface Signed-off-by: Evan Lezar --- cmd/nvidia-container-runtime-hook/hook_config.go | 2 -- cmd/nvidia-ctk/cdi/transform/root/root.go | 5 ----- internal/dxcore/api.go | 9 --------- internal/modifier/cdi.go | 6 ------ internal/modifier/csv.go | 7 ------- internal/requirements/constraints/constants.go | 14 -------------- tools/container/containerd/config_v2_test.go | 1 - tools/container/toolkit/runtime.go | 13 ------------- 8 files changed, 57 deletions(-) diff --git a/cmd/nvidia-container-runtime-hook/hook_config.go b/cmd/nvidia-container-runtime-hook/hook_config.go index 3bd0828f..afdb8bbc 100644 --- a/cmd/nvidia-container-runtime-hook/hook_config.go +++ b/cmd/nvidia-container-runtime-hook/hook_config.go @@ -17,8 +17,6 @@ const ( driverPath = "/run/nvidia/driver" ) -var defaultPaths = [...]string{} - // HookConfig : options for the nvidia-container-runtime-hook. type HookConfig config.Config diff --git a/cmd/nvidia-ctk/cdi/transform/root/root.go b/cmd/nvidia-ctk/cdi/transform/root/root.go index 191eb8be..ad9d2e22 100644 --- a/cmd/nvidia-ctk/cdi/transform/root/root.go +++ b/cmd/nvidia-ctk/cdi/transform/root/root.go @@ -28,11 +28,6 @@ import ( "github.com/urfave/cli/v2" ) -type loadSaver interface { - Load() (spec.Interface, error) - Save(spec.Interface) error -} - type command struct { logger logger.Interface } diff --git a/internal/dxcore/api.go b/internal/dxcore/api.go index fd63d5a6..ed70bb4f 100644 --- a/internal/dxcore/api.go +++ b/internal/dxcore/api.go @@ -16,15 +16,6 @@ package dxcore -import ( - "github.com/NVIDIA/go-nvml/pkg/dl" -) - -const ( - libraryName = "libdxcore.so" - libraryLoadFlags = dl.RTLD_LAZY | dl.RTLD_GLOBAL -) - // dxcore stores a reference the dxcore dynamic library var dxcore *context diff --git a/internal/modifier/cdi.go b/internal/modifier/cdi.go index 69e2a9fa..3fb44221 100644 --- a/internal/modifier/cdi.go +++ b/internal/modifier/cdi.go @@ -28,12 +28,6 @@ import ( "github.com/container-orchestrated-devices/container-device-interface/pkg/parser" ) -type cdiModifier struct { - logger logger.Interface - specDirs []string - devices []string -} - // NewCDIModifier creates an OCI spec modifier that determines the modifications to make based on the // CDI specifications available on the system. The NVIDIA_VISIBLE_DEVICES enviroment variable is // used to select the devices to include. diff --git a/internal/modifier/csv.go b/internal/modifier/csv.go index 56adcdc7..22688e20 100644 --- a/internal/modifier/csv.go +++ b/internal/modifier/csv.go @@ -22,7 +22,6 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/internal/config" "github.com/NVIDIA/nvidia-container-toolkit/internal/config/image" "github.com/NVIDIA/nvidia-container-toolkit/internal/cuda" - "github.com/NVIDIA/nvidia-container-toolkit/internal/discover" "github.com/NVIDIA/nvidia-container-toolkit/internal/logger" "github.com/NVIDIA/nvidia-container-toolkit/internal/modifier/cdi" "github.com/NVIDIA/nvidia-container-toolkit/internal/oci" @@ -31,12 +30,6 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi" ) -// csvMode represents the modifications as performed by the csv runtime mode -type csvMode struct { - logger logger.Interface - discoverer discover.Discover -} - const ( visibleDevicesEnvvar = "NVIDIA_VISIBLE_DEVICES" visibleDevicesVoid = "void" diff --git a/internal/requirements/constraints/constants.go b/internal/requirements/constraints/constants.go index 9e515214..24f93353 100644 --- a/internal/requirements/constraints/constants.go +++ b/internal/requirements/constraints/constants.go @@ -16,8 +16,6 @@ package constraints -import "fmt" - const ( equal = "=" notEqual = "!=" @@ -37,15 +35,3 @@ func (c always) Assert() error { func (c always) String() string { return "true" } - -// invalid is an invalid constraint and can never be met -type invalid string - -func (c invalid) Assert() error { - return fmt.Errorf("invalid constraint: %v", c.String()) -} - -// String returns the string representation of the contraint -func (c invalid) String() string { - return string(c) -} diff --git a/tools/container/containerd/config_v2_test.go b/tools/container/containerd/config_v2_test.go index 3081b8a8..a1b7413e 100644 --- a/tools/container/containerd/config_v2_test.go +++ b/tools/container/containerd/config_v2_test.go @@ -90,7 +90,6 @@ func TestUpdateV2ConfigDefaultRuntime(t *testing.T) { func TestUpdateV2Config(t *testing.T) { const runtimeDir = "/test/runtime/dir" - const expectedVersion = int64(2) testCases := []struct { runtimeName string diff --git a/tools/container/toolkit/runtime.go b/tools/container/toolkit/runtime.go index 220d7ff5..92195975 100644 --- a/tools/container/toolkit/runtime.go +++ b/tools/container/toolkit/runtime.go @@ -82,16 +82,3 @@ func newRuntimeInstaller(source string, target executableTarget, env map[string] return &r } - -func findLibraryRoot(root string) (string, error) { - libnvidiamlPath, err := findManagementLibrary(root) - if err != nil { - return "", fmt.Errorf("error locating NVIDIA management library: %v", err) - } - - return filepath.Dir(libnvidiamlPath), nil -} - -func findManagementLibrary(root string) (string, error) { - return findLibrary(root, "libnvidia-ml.so") -} From 2fad708556cdb96279ab06e44ff068818f5ef60c Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 25 Aug 2023 16:12:54 +0200 Subject: [PATCH 06/19] Address ioutil deprecation Signed-off-by: Evan Lezar --- cmd/nvidia-container-runtime/main_test.go | 4 ++-- pkg/config/engine/docker/option.go | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/cmd/nvidia-container-runtime/main_test.go b/cmd/nvidia-container-runtime/main_test.go index 4260559b..dad9ac79 100644 --- a/cmd/nvidia-container-runtime/main_test.go +++ b/cmd/nvidia-container-runtime/main_test.go @@ -3,7 +3,7 @@ package main import ( "bytes" "encoding/json" - "io/ioutil" + "io" "log" "os" "os/exec" @@ -188,7 +188,7 @@ func (c testConfig) getRuntimeSpec() (specs.Spec, error) { } defer jsonFile.Close() - jsonContent, err := ioutil.ReadAll(jsonFile) + jsonContent, err := io.ReadAll(jsonFile) if err != nil { return spec, err } else if json.Valid(jsonContent) { diff --git a/pkg/config/engine/docker/option.go b/pkg/config/engine/docker/option.go index 4f64382f..dfb67499 100644 --- a/pkg/config/engine/docker/option.go +++ b/pkg/config/engine/docker/option.go @@ -20,7 +20,6 @@ import ( "bytes" "encoding/json" "fmt" - "io/ioutil" "os" "github.com/NVIDIA/nvidia-container-toolkit/internal/logger" @@ -72,7 +71,7 @@ func (b *builder) loadConfig(config string) (*Config, error) { } b.logger.Infof("Loading config from %v", config) - readBytes, err := ioutil.ReadFile(config) + readBytes, err := os.ReadFile(config) if err != nil { return nil, fmt.Errorf("unable to read config: %v", err) } From 12dc12ce097831cf82a504bd437a5232de103d93 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 25 Aug 2023 16:14:06 +0200 Subject: [PATCH 07/19] Fix misspellings Signed-off-by: Evan Lezar --- cmd/nvidia-container-runtime-hook/container_config_test.go | 6 +++--- cmd/nvidia-ctk/hook/chmod/chmod.go | 2 +- cmd/nvidia-ctk/hook/create-symlinks/create-symlinks.go | 2 +- cmd/nvidia-ctk/hook/update-ldcache/update-ldcache.go | 2 +- internal/config/image/capabilities.go | 2 +- internal/config/image/cuda_image.go | 6 +++--- internal/discover/list.go | 2 +- internal/info/proc/information_files.go | 2 +- internal/ldcache/ldcache.go | 2 +- internal/modifier/cdi.go | 2 +- internal/oci/runtime_path.go | 2 +- internal/oci/spec.go | 2 +- internal/oci/spec_file.go | 2 +- pkg/nvcdi/driver-nvml.go | 2 +- pkg/nvcdi/lib-nvml.go | 2 +- pkg/nvcdi/spec/builder.go | 2 +- tools/container/nvidia-toolkit/run.go | 2 +- tools/container/toolkit/runtime.go | 2 +- 18 files changed, 22 insertions(+), 22 deletions(-) diff --git a/cmd/nvidia-container-runtime-hook/container_config_test.go b/cmd/nvidia-container-runtime-hook/container_config_test.go index c918bf06..2e5ec98b 100644 --- a/cmd/nvidia-container-runtime-hook/container_config_test.go +++ b/cmd/nvidia-container-runtime-hook/container_config_test.go @@ -1015,14 +1015,14 @@ func TestGetDriverCapabilities(t *testing.T) { for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { - var capabilites string + var capabilities string c := HookConfig{ SupportedDriverCapabilities: tc.supportedCapabilities, } getDriverCapabilities := func() { - capabilites = c.getDriverCapabilities(tc.env, tc.legacyImage).String() + capabilities = c.getDriverCapabilities(tc.env, tc.legacyImage).String() } if tc.expectedPanic { @@ -1031,7 +1031,7 @@ func TestGetDriverCapabilities(t *testing.T) { } getDriverCapabilities() - require.EqualValues(t, tc.expectedCapabilities, capabilites) + require.EqualValues(t, tc.expectedCapabilities, capabilities) }) } } diff --git a/cmd/nvidia-ctk/hook/chmod/chmod.go b/cmd/nvidia-ctk/hook/chmod/chmod.go index 4f2f28e1..1276f7fa 100644 --- a/cmd/nvidia-ctk/hook/chmod/chmod.go +++ b/cmd/nvidia-ctk/hook/chmod/chmod.go @@ -66,7 +66,7 @@ func (m command) build() *cli.Command { c.Flags = []cli.Flag{ &cli.StringSliceFlag{ Name: "path", - Usage: "Specifiy a path to apply the specified mode to", + Usage: "Specify a path to apply the specified mode to", Destination: &cfg.paths, }, &cli.StringFlag{ diff --git a/cmd/nvidia-ctk/hook/create-symlinks/create-symlinks.go b/cmd/nvidia-ctk/hook/create-symlinks/create-symlinks.go index 16ef3ed5..b57b4cd8 100644 --- a/cmd/nvidia-ctk/hook/create-symlinks/create-symlinks.go +++ b/cmd/nvidia-ctk/hook/create-symlinks/create-symlinks.go @@ -56,7 +56,7 @@ func (m command) build() *cli.Command { // Create the '' command c := cli.Command{ Name: "create-symlinks", - Usage: "A hook to create symlinks in the container. This can be used to proces CSV mount specs", + Usage: "A hook to create symlinks in the container. This can be used to process CSV mount specs", Action: func(c *cli.Context) error { return m.run(c, &cfg) }, diff --git a/cmd/nvidia-ctk/hook/update-ldcache/update-ldcache.go b/cmd/nvidia-ctk/hook/update-ldcache/update-ldcache.go index a0267297..f9719e64 100644 --- a/cmd/nvidia-ctk/hook/update-ldcache/update-ldcache.go +++ b/cmd/nvidia-ctk/hook/update-ldcache/update-ldcache.go @@ -60,7 +60,7 @@ func (m command) build() *cli.Command { c.Flags = []cli.Flag{ &cli.StringSliceFlag{ Name: "folder", - Usage: "Specifiy a folder to add to /etc/ld.so.conf before updating the ld cache", + Usage: "Specify a folder to add to /etc/ld.so.conf before updating the ld cache", Destination: &cfg.folders, }, &cli.StringFlag{ diff --git a/internal/config/image/capabilities.go b/internal/config/image/capabilities.go index 9c05acc0..824a6db9 100644 --- a/internal/config/image/capabilities.go +++ b/internal/config/image/capabilities.go @@ -73,7 +73,7 @@ func (c DriverCapabilities) Has(capability DriverCapability) bool { return c[capability] } -// Any checks whether any of the specified capabilites are set +// Any checks whether any of the specified capabilities are set func (c DriverCapabilities) Any(capabilities ...DriverCapability) bool { if c.IsAll() { return true diff --git a/internal/config/image/cuda_image.go b/internal/config/image/cuda_image.go index f7a6713b..ee264001 100644 --- a/internal/config/image/cuda_image.go +++ b/internal/config/image/cuda_image.go @@ -139,12 +139,12 @@ func (i CUDA) DevicesFromEnvvars(envVars ...string) VisibleDevices { func (i CUDA) GetDriverCapabilities() DriverCapabilities { env := i[envNVDriverCapabilities] - capabilites := make(DriverCapabilities) + capabilities := make(DriverCapabilities) for _, c := range strings.Split(env, ",") { - capabilites[DriverCapability(c)] = true + capabilities[DriverCapability(c)] = true } - return capabilites + return capabilities } func (i CUDA) legacyVersion() (string, error) { diff --git a/internal/discover/list.go b/internal/discover/list.go index aa0c5910..8fbf6e75 100644 --- a/internal/discover/list.go +++ b/internal/discover/list.go @@ -27,7 +27,7 @@ type list struct { var _ Discover = (*list)(nil) -// Merge creates a discoverer that is the composite of a list of discoveres. +// Merge creates a discoverer that is the composite of a list of discoverers. func Merge(d ...Discover) Discover { l := list{ discoverers: d, diff --git a/internal/info/proc/information_files.go b/internal/info/proc/information_files.go index dde77416..f84b76a6 100644 --- a/internal/info/proc/information_files.go +++ b/internal/info/proc/information_files.go @@ -56,7 +56,7 @@ func ParseGPUInformationFile(path string) (GPUInfo, error) { } // gpuInfoFrom parses a GPUInfo struct from the specified reader -// An information file has the following strucutre: +// An information file has the following structure: // $ cat /proc/driver/nvidia/gpus/0000\:06\:00.0/information // Model: Tesla V100-SXM2-16GB // IRQ: 408 diff --git a/internal/ldcache/ldcache.go b/internal/ldcache/ldcache.go index 7673a49a..0547dee4 100644 --- a/internal/ldcache/ldcache.go +++ b/internal/ldcache/ldcache.go @@ -234,7 +234,7 @@ func (c *ldcache) getEntries(selected func(string) bool) []entry { return entries } -// List creates a list of libraires in the ldcache. +// List creates a list of libraries in the ldcache. // The 32-bit and 64-bit libraries are returned separately. func (c *ldcache) List() ([]string, []string) { all := func(s string) bool { return true } diff --git a/internal/modifier/cdi.go b/internal/modifier/cdi.go index 3fb44221..7cef3672 100644 --- a/internal/modifier/cdi.go +++ b/internal/modifier/cdi.go @@ -29,7 +29,7 @@ import ( ) // NewCDIModifier creates an OCI spec modifier that determines the modifications to make based on the -// CDI specifications available on the system. The NVIDIA_VISIBLE_DEVICES enviroment variable is +// CDI specifications available on the system. The NVIDIA_VISIBLE_DEVICES environment variable is // used to select the devices to include. func NewCDIModifier(logger logger.Interface, cfg *config.Config, ociSpec oci.Spec) (oci.SpecModifier, error) { devices, err := getDevicesFromSpec(logger, ociSpec, cfg) diff --git a/internal/oci/runtime_path.go b/internal/oci/runtime_path.go index 948312f3..3c43f31d 100644 --- a/internal/oci/runtime_path.go +++ b/internal/oci/runtime_path.go @@ -23,7 +23,7 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/internal/logger" ) -// pathRuntime wraps the path that a binary and defines the semanitcs for how to exec into it. +// pathRuntime wraps the path that a binary and defines the semantics for how to exec into it. // This can be used to wrap an OCI-compliant low-level runtime binary, allowing it to be used through the // Runtime internface. type pathRuntime struct { diff --git a/internal/oci/spec.go b/internal/oci/spec.go index dedf794c..f433e1c4 100644 --- a/internal/oci/spec.go +++ b/internal/oci/spec.go @@ -23,7 +23,7 @@ import ( "github.com/opencontainers/runtime-spec/specs-go" ) -// SpecModifier defines an interace for modifying a (raw) OCI spec +// SpecModifier defines an interface for modifying a (raw) OCI spec type SpecModifier interface { // Modify is a method that accepts a pointer to an OCI Srec and returns an // error. The intention is that the function would modify the spec in-place. diff --git a/internal/oci/spec_file.go b/internal/oci/spec_file.go index 7fba130d..8784ae92 100644 --- a/internal/oci/spec_file.go +++ b/internal/oci/spec_file.go @@ -79,7 +79,7 @@ func (s *fileSpec) Modify(m SpecModifier) error { return s.memorySpec.Modify(m) } -// Flush writes the stored OCI specification to the filepath specifed by the path member. +// Flush writes the stored OCI specification to the filepath specified by the path member. // The file is truncated upon opening, overwriting any existing contents. func (s fileSpec) Flush() error { if s.Spec == nil { diff --git a/pkg/nvcdi/driver-nvml.go b/pkg/nvcdi/driver-nvml.go index bc8171a3..89349b27 100644 --- a/pkg/nvcdi/driver-nvml.go +++ b/pkg/nvcdi/driver-nvml.go @@ -34,7 +34,7 @@ import ( // The supplied NVML Library is used to query the expected driver version. func NewDriverDiscoverer(logger logger.Interface, driverRoot string, nvidiaCTKPath string, nvmllib nvml.Interface) (discover.Discover, error) { if r := nvmllib.Init(); r != nvml.SUCCESS { - return nil, fmt.Errorf("failed to initalize NVML: %v", r) + return nil, fmt.Errorf("failed to initialize NVML: %v", r) } defer nvmllib.Shutdown() diff --git a/pkg/nvcdi/lib-nvml.go b/pkg/nvcdi/lib-nvml.go index 95ccf66b..1a43cdb7 100644 --- a/pkg/nvcdi/lib-nvml.go +++ b/pkg/nvcdi/lib-nvml.go @@ -41,7 +41,7 @@ func (l *nvmllib) GetAllDeviceSpecs() ([]specs.Device, error) { var deviceSpecs []specs.Device if r := l.nvmllib.Init(); r != nvml.SUCCESS { - return nil, fmt.Errorf("failed to initalize NVML: %v", r) + return nil, fmt.Errorf("failed to initialize NVML: %v", r) } defer l.nvmllib.Shutdown() diff --git a/pkg/nvcdi/spec/builder.go b/pkg/nvcdi/spec/builder.go index 0a867a55..bfefff93 100644 --- a/pkg/nvcdi/spec/builder.go +++ b/pkg/nvcdi/spec/builder.go @@ -85,7 +85,7 @@ func (o *builder) Build() (*spec, error) { if raw.Version == DetectMinimumVersion { minVersion, err := cdi.MinimumRequiredVersion(raw) if err != nil { - return nil, fmt.Errorf("failed to get minumum required CDI spec version: %v", err) + return nil, fmt.Errorf("failed to get minimum required CDI spec version: %v", err) } raw.Version = minVersion } diff --git a/tools/container/nvidia-toolkit/run.go b/tools/container/nvidia-toolkit/run.go index 78efd9a1..740835b3 100644 --- a/tools/container/nvidia-toolkit/run.go +++ b/tools/container/nvidia-toolkit/run.go @@ -65,7 +65,7 @@ func main() { &cli.BoolFlag{ Name: "no-daemon", Aliases: []string{"n"}, - Usage: "terminate immediatly after setting up the runtime. Note that no cleanup will be performed", + Usage: "terminate immediately after setting up the runtime. Note that no cleanup will be performed", Destination: &options.noDaemon, EnvVars: []string{"NO_DAEMON"}, }, diff --git a/tools/container/toolkit/runtime.go b/tools/container/toolkit/runtime.go index 92195975..d2e0b69f 100644 --- a/tools/container/toolkit/runtime.go +++ b/tools/container/toolkit/runtime.go @@ -43,7 +43,7 @@ func installContainerRuntimes(toolkitDir string, driverRoot string) error { } // newNVidiaContainerRuntimeInstaller returns a new executable installer for the NVIDIA container runtime. -// This installer will copy the specified source exectuable to the toolkit directory. +// This installer will copy the specified source executable to the toolkit directory. // The executable is copied to a file with the same name as the source, but with a ".real" suffix and a wrapper is // created to allow for the configuration of the runtime environment. func newNvidiaContainerRuntimeInstaller(source string) *executable { From f2c9937ca8775df2cdbe8a8c2fce5db585e0db25 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 25 Aug 2023 16:15:30 +0200 Subject: [PATCH 08/19] Use cdi parser package Signed-off-by: Evan Lezar --- cmd/nvidia-ctk/cdi/generate/generate.go | 2 +- pkg/nvcdi/spec/builder.go | 3 ++- pkg/nvcdi/transform/merged-device.go | 4 +++- tools/container/toolkit/toolkit.go | 7 ++++--- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/cmd/nvidia-ctk/cdi/generate/generate.go b/cmd/nvidia-ctk/cdi/generate/generate.go index 6adb0936..47b6c711 100644 --- a/cmd/nvidia-ctk/cdi/generate/generate.go +++ b/cmd/nvidia-ctk/cdi/generate/generate.go @@ -28,7 +28,7 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi" "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/spec" "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/transform" - "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" + cdi "github.com/container-orchestrated-devices/container-device-interface/pkg/parser" "github.com/urfave/cli/v2" ) diff --git a/pkg/nvcdi/spec/builder.go b/pkg/nvcdi/spec/builder.go index bfefff93..610cb8ce 100644 --- a/pkg/nvcdi/spec/builder.go +++ b/pkg/nvcdi/spec/builder.go @@ -22,6 +22,7 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/transform" "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" + "github.com/container-orchestrated-devices/container-device-interface/pkg/parser" "github.com/container-orchestrated-devices/container-device-interface/specs-go" ) @@ -47,7 +48,7 @@ func newBuilder(opts ...Option) *builder { } if s.raw != nil { s.noSimplify = true - vendor, class := cdi.ParseQualifier(s.raw.Kind) + vendor, class := parser.ParseQualifier(s.raw.Kind) s.vendor = vendor s.class = class } diff --git a/pkg/nvcdi/transform/merged-device.go b/pkg/nvcdi/transform/merged-device.go index 4b744155..6730afeb 100644 --- a/pkg/nvcdi/transform/merged-device.go +++ b/pkg/nvcdi/transform/merged-device.go @@ -20,7 +20,9 @@ import ( "fmt" "github.com/NVIDIA/nvidia-container-toolkit/internal/edits" + "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" + "github.com/container-orchestrated-devices/container-device-interface/pkg/parser" "github.com/container-orchestrated-devices/container-device-interface/specs-go" ) @@ -64,7 +66,7 @@ func NewMergedDevice(opts ...MergedDeviceOption) (Transformer, error) { } m.simplifier = NewSimplifier() - if err := cdi.ValidateDeviceName(m.name); err != nil { + if err := parser.ValidateDeviceName(m.name); err != nil { return nil, fmt.Errorf("invalid device name %q: %v", m.name, err) } diff --git a/tools/container/toolkit/toolkit.go b/tools/container/toolkit/toolkit.go index b05062f3..971916ae 100644 --- a/tools/container/toolkit/toolkit.go +++ b/tools/container/toolkit/toolkit.go @@ -27,6 +27,7 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi" "github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/transform" "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" + "github.com/container-orchestrated-devices/container-device-interface/pkg/parser" toml "github.com/pelletier/go-toml" log "github.com/sirupsen/logrus" "github.com/urfave/cli/v2" @@ -238,11 +239,11 @@ func validateOptions(c *cli.Context, opts *options) error { return fmt.Errorf("invalid --toolkit-root option: %v", opts.toolkitRoot) } - vendor, class := cdi.ParseQualifier(opts.cdiKind) - if err := cdi.ValidateVendorName(vendor); err != nil { + vendor, class := parser.ParseQualifier(opts.cdiKind) + if err := parser.ValidateVendorName(vendor); err != nil { return fmt.Errorf("invalid CDI vendor name: %v", err) } - if err := cdi.ValidateClassName(class); err != nil { + if err := parser.ValidateClassName(class); err != nil { return fmt.Errorf("invalid CDI class name: %v", err) } opts.cdiVendor = vendor From e0df157f70936873b4064af152009e7421d8ae18 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 25 Aug 2023 16:30:37 +0200 Subject: [PATCH 09/19] Remove unnecessary assignment to the blank identifier Signed-off-by: Evan Lezar --- internal/modifier/csv.go | 2 +- internal/modifier/gds.go | 2 +- internal/modifier/mofed.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/modifier/csv.go b/internal/modifier/csv.go index 22688e20..53579268 100644 --- a/internal/modifier/csv.go +++ b/internal/modifier/csv.go @@ -55,7 +55,7 @@ func NewCSVModifier(logger logger.Interface, cfg *config.Config, image image.CUD return nil, fmt.Errorf("failed to get list of CSV files: %v", err) } - if nvidiaRequireJetpack, _ := image[nvidiaRequireJetpackEnvvar]; nvidiaRequireJetpack != "csv-mounts=all" { + if nvidiaRequireJetpack := image[nvidiaRequireJetpackEnvvar]; nvidiaRequireJetpack != "csv-mounts=all" { csvFiles = csv.BaseFilesOnly(csvFiles) } diff --git a/internal/modifier/gds.go b/internal/modifier/gds.go index 9ef15992..510051ab 100644 --- a/internal/modifier/gds.go +++ b/internal/modifier/gds.go @@ -38,7 +38,7 @@ func NewGDSModifier(logger logger.Interface, cfg *config.Config, image image.CUD return nil, nil } - if gds, _ := image[nvidiaGDSEnvvar]; gds != "enabled" { + if gds := image[nvidiaGDSEnvvar]; gds != "enabled" { return nil, nil } diff --git a/internal/modifier/mofed.go b/internal/modifier/mofed.go index 5cc69169..505c2db0 100644 --- a/internal/modifier/mofed.go +++ b/internal/modifier/mofed.go @@ -38,7 +38,7 @@ func NewMOFEDModifier(logger logger.Interface, cfg *config.Config, image image.C return nil, nil } - if mofed, _ := image[nvidiaMOFEDEnvvar]; mofed != "enabled" { + if mofed := image[nvidiaMOFEDEnvvar]; mofed != "enabled" { return nil, nil } From 8a9f367067999dc1121948810488f92c7e707626 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 25 Aug 2023 16:48:11 +0200 Subject: [PATCH 10/19] Check returned error values Signed-off-by: Evan Lezar --- cmd/nvidia-ctk/config/config.go | 6 +++--- cmd/nvidia-ctk/config/create-default/create-default.go | 3 +-- internal/info/additional_info.go | 4 +++- internal/oci/spec_test.go | 3 ++- internal/runtime/runtime.go | 5 ++++- pkg/nvcdi/driver-nvml.go | 6 +++++- pkg/nvcdi/driver-wsl.go | 6 +++++- pkg/nvcdi/lib-nvml.go | 6 +++++- pkg/nvcdi/lib.go | 6 +++++- tools/container/toolkit/toolkit.go | 7 ++++--- 10 files changed, 37 insertions(+), 15 deletions(-) diff --git a/cmd/nvidia-ctk/config/config.go b/cmd/nvidia-ctk/config/config.go index ee5832b6..f6d636fc 100644 --- a/cmd/nvidia-ctk/config/config.go +++ b/cmd/nvidia-ctk/config/config.go @@ -119,10 +119,10 @@ func run(c *cli.Context, opts *options) error { } defer output.Close() - if err != nil { - return err + if _, err := cfgToml.Save(output); err != nil { + return fmt.Errorf("failed to save config: %v", err) } - cfgToml.Save(output) + return nil } diff --git a/cmd/nvidia-ctk/config/create-default/create-default.go b/cmd/nvidia-ctk/config/create-default/create-default.go index d8c8b00f..e16eb855 100644 --- a/cmd/nvidia-ctk/config/create-default/create-default.go +++ b/cmd/nvidia-ctk/config/create-default/create-default.go @@ -85,8 +85,7 @@ func (m command) run(c *cli.Context, opts *flags.Options) error { } defer output.Close() - _, err = cfgToml.Save(output) - if err != nil { + if _, err = cfgToml.Save(output); err != nil { return fmt.Errorf("failed to write output: %v", err) } diff --git a/internal/info/additional_info.go b/internal/info/additional_info.go index fbb9ed23..9fffad78 100644 --- a/internal/info/additional_info.go +++ b/internal/info/additional_info.go @@ -52,7 +52,9 @@ func (i additionalInfo) UsesNVGPUModule() (uses bool, reason string) { if ret != nvml.SUCCESS { return false, fmt.Sprintf("failed to initialize nvml: %v", ret) } - defer i.nvmllib.Shutdown() + defer func() { + _ = i.nvmllib.Shutdown() + }() var names []string diff --git a/internal/oci/spec_test.go b/internal/oci/spec_test.go index f17f3d3c..1acf1728 100644 --- a/internal/oci/spec_test.go +++ b/internal/oci/spec_test.go @@ -22,7 +22,8 @@ func TestMaintainSpec(t *testing.T) { spec := NewFileSpec(inputSpecPath).(*fileSpec) - spec.Load() + _, err := spec.Load() + require.NoError(t, err) outputSpecPath := filepath.Join(moduleRoot, "test/output", f) spec.path = outputSpecPath diff --git a/internal/runtime/runtime.go b/internal/runtime/runtime.go index e43c778a..87a11f6a 100644 --- a/internal/runtime/runtime.go +++ b/internal/runtime/runtime.go @@ -18,6 +18,7 @@ package runtime import ( "encoding/json" + "errors" "fmt" "strings" @@ -53,7 +54,9 @@ func (r rt) Run(argv []string) (rerr error) { if rerr != nil { r.logger.Errorf("%v", rerr) } - r.logger.Reset() + if err := r.logger.Reset(); err != nil { + rerr = errors.Join(rerr, fmt.Errorf("failed to reset logger: %v", err)) + } }() // We apply some config updates here to ensure that the config is valid in diff --git a/pkg/nvcdi/driver-nvml.go b/pkg/nvcdi/driver-nvml.go index 89349b27..28dacae3 100644 --- a/pkg/nvcdi/driver-nvml.go +++ b/pkg/nvcdi/driver-nvml.go @@ -36,7 +36,11 @@ func NewDriverDiscoverer(logger logger.Interface, driverRoot string, nvidiaCTKPa if r := nvmllib.Init(); r != nvml.SUCCESS { return nil, fmt.Errorf("failed to initialize NVML: %v", r) } - defer nvmllib.Shutdown() + defer func() { + if r := nvmllib.Shutdown(); r != nvml.SUCCESS { + logger.Warningf("failed to shutdown NVML: %v", r) + } + }() version, r := nvmllib.SystemGetDriverVersion() if r != nvml.SUCCESS { diff --git a/pkg/nvcdi/driver-wsl.go b/pkg/nvcdi/driver-wsl.go index 1aa02b8e..c49ef4ad 100644 --- a/pkg/nvcdi/driver-wsl.go +++ b/pkg/nvcdi/driver-wsl.go @@ -43,7 +43,11 @@ func newWSLDriverDiscoverer(logger logger.Interface, driverRoot string, nvidiaCT if err != nil { return nil, fmt.Errorf("failed to initialize dxcore: %v", err) } - defer dxcore.Shutdown() + defer func() { + if err := dxcore.Shutdown(); err != nil { + logger.Warningf("failed to shutdown dxcore: %v", err) + } + }() driverStorePaths := dxcore.GetDriverStorePaths() if len(driverStorePaths) == 0 { diff --git a/pkg/nvcdi/lib-nvml.go b/pkg/nvcdi/lib-nvml.go index 1a43cdb7..db6cabab 100644 --- a/pkg/nvcdi/lib-nvml.go +++ b/pkg/nvcdi/lib-nvml.go @@ -43,7 +43,11 @@ func (l *nvmllib) GetAllDeviceSpecs() ([]specs.Device, error) { if r := l.nvmllib.Init(); r != nvml.SUCCESS { return nil, fmt.Errorf("failed to initialize NVML: %v", r) } - defer l.nvmllib.Shutdown() + defer func() { + if r := l.nvmllib.Shutdown(); r != nvml.SUCCESS { + l.logger.Warningf("failed to shutdown NVML: %v", r) + } + }() gpuDeviceSpecs, err := l.getGPUDeviceSpecs() if err != nil { diff --git a/pkg/nvcdi/lib.go b/pkg/nvcdi/lib.go index f8a52c1b..be4e62af 100644 --- a/pkg/nvcdi/lib.go +++ b/pkg/nvcdi/lib.go @@ -191,7 +191,11 @@ func (l *nvcdilib) getCudaVersion() (string, error) { if r != nvml.SUCCESS { return "", fmt.Errorf("failed to initialize nvml: %v", r) } - defer l.nvmllib.Shutdown() + defer func() { + if r := l.nvmllib.Shutdown(); r != nvml.SUCCESS { + l.logger.Warningf("failed to shutdown NVML: %v", r) + } + }() version, r := l.nvmllib.SystemGetDriverVersion() if r != nvml.SUCCESS { diff --git a/tools/container/toolkit/toolkit.go b/tools/container/toolkit/toolkit.go index 971916ae..93766b23 100644 --- a/tools/container/toolkit/toolkit.go +++ b/tools/container/toolkit/toolkit.go @@ -455,13 +455,14 @@ func installToolkitConfig(c *cli.Context, toolkitConfigPath string, nvidiaContai config.Set(key, value) } - _, err = config.WriteTo(targetConfig) - if err != nil { + if _, err := config.WriteTo(targetConfig); err != nil { return fmt.Errorf("error writing config: %v", err) } os.Stdout.WriteString("Using config:\n") - config.WriteTo(os.Stdout) + if _, err = config.WriteTo(os.Stdout); err != nil { + log.Warningf("Failed to output config to STDOUT: %v", err) + } return nil } From dd2f218226ee189bd086b420eae9344293569ac8 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 25 Aug 2023 16:49:31 +0200 Subject: [PATCH 11/19] Use MustCompile for static regexp Signed-off-by: Evan Lezar --- internal/config/toml.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/internal/config/toml.go b/internal/config/toml.go index 8c931675..6561ab1c 100644 --- a/internal/config/toml.go +++ b/internal/config/toml.go @@ -154,10 +154,7 @@ func (t Toml) contents() ([]byte, error) { // format fixes the comments for the config to ensure that they start in column // 1 and are not followed by a space. func (t Toml) format(contents []byte) ([]byte, error) { - r, err := regexp.Compile(`(\n*)\s*?#\s*(\S.*)`) - if err != nil { - return nil, fmt.Errorf("unable to compile regexp: %v", err) - } + r := regexp.MustCompile(`(\n*)\s*?#\s*(\S.*)`) replaced := r.ReplaceAll(contents, []byte("$1#$2")) return replaced, nil From 73857eb8e3ab8ba8f4d6c59f7df4d953a0bdbeb7 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 25 Aug 2023 16:50:40 +0200 Subject: [PATCH 12/19] Fix unnecessary conversion Signed-off-by: Evan Lezar --- cmd/nvidia-ctk/cdi/generate/generate.go | 2 +- .../create-dev-char-symlinks/existing.go | 9 +----- .../existing_linux.go | 28 +++++++++++++++++ .../existing_other.go | 30 +++++++++++++++++++ internal/ldcache/ldcache.go | 2 +- internal/requirements/constraints/binary.go | 2 +- pkg/nvcdi/lib_test.go | 6 ++-- 7 files changed, 65 insertions(+), 14 deletions(-) create mode 100644 cmd/nvidia-ctk/system/create-dev-char-symlinks/existing_linux.go create mode 100644 cmd/nvidia-ctk/system/create-dev-char-symlinks/existing_other.go diff --git a/cmd/nvidia-ctk/cdi/generate/generate.go b/cmd/nvidia-ctk/cdi/generate/generate.go index 47b6c711..830437ca 100644 --- a/cmd/nvidia-ctk/cdi/generate/generate.go +++ b/cmd/nvidia-ctk/cdi/generate/generate.go @@ -238,7 +238,7 @@ func (m command) generateSpec(opts *options) (spec.Interface, error) { nvcdi.WithDriverRoot(opts.driverRoot), nvcdi.WithNVIDIACTKPath(opts.nvidiaCTKPath), nvcdi.WithDeviceNamer(deviceNamer), - nvcdi.WithMode(string(opts.mode)), + nvcdi.WithMode(opts.mode), nvcdi.WithLibrarySearchPaths(opts.librarySearchPaths.Value()), nvcdi.WithCSVFiles(opts.csv.files.Value()), nvcdi.WithCSVIgnorePatterns(opts.csv.ignorePatterns.Value()), diff --git a/cmd/nvidia-ctk/system/create-dev-char-symlinks/existing.go b/cmd/nvidia-ctk/system/create-dev-char-symlinks/existing.go index a1af8b20..946f42b0 100644 --- a/cmd/nvidia-ctk/system/create-dev-char-symlinks/existing.go +++ b/cmd/nvidia-ctk/system/create-dev-char-symlinks/existing.go @@ -63,20 +63,13 @@ func (m existing) DeviceNodes() ([]deviceNode, error) { if m.nodeIsBlocked(d) { continue } - var stat unix.Stat_t err := unix.Stat(d, &stat) if err != nil { m.logger.Warningf("Could not stat device: %v", err) continue } - deviceNode := deviceNode{ - path: d, - major: unix.Major(uint64(stat.Rdev)), - minor: unix.Minor(uint64(stat.Rdev)), - } - - deviceNodes = append(deviceNodes, deviceNode) + deviceNodes = append(deviceNodes, newDeviceNode(d, stat)) } return deviceNodes, nil diff --git a/cmd/nvidia-ctk/system/create-dev-char-symlinks/existing_linux.go b/cmd/nvidia-ctk/system/create-dev-char-symlinks/existing_linux.go new file mode 100644 index 00000000..4aab942a --- /dev/null +++ b/cmd/nvidia-ctk/system/create-dev-char-symlinks/existing_linux.go @@ -0,0 +1,28 @@ +/** +# Copyright (c) NVIDIA CORPORATION. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +**/ + +package devchar + +import "golang.org/x/sys/unix" + +func newDeviceNode(d string, stat unix.Stat_t) deviceNode { + deviceNode := deviceNode{ + path: d, + major: unix.Major(stat.Rdev), + minor: unix.Minor(stat.Rdev), + } + return deviceNode +} diff --git a/cmd/nvidia-ctk/system/create-dev-char-symlinks/existing_other.go b/cmd/nvidia-ctk/system/create-dev-char-symlinks/existing_other.go new file mode 100644 index 00000000..9be96294 --- /dev/null +++ b/cmd/nvidia-ctk/system/create-dev-char-symlinks/existing_other.go @@ -0,0 +1,30 @@ +//go:build !linux + +/** +# Copyright (c) NVIDIA CORPORATION. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +**/ + +package devchar + +import "golang.org/x/sys/unix" + +func newDeviceNode(d string, stat unix.Stat_t) deviceNode { + deviceNode := deviceNode{ + path: d, + major: unix.Major(uint64(stat.Rdev)), + minor: unix.Minor(uint64(stat.Rdev)), + } + return deviceNode +} diff --git a/internal/ldcache/ldcache.go b/internal/ldcache/ldcache.go index 0547dee4..5ea263c1 100644 --- a/internal/ldcache/ldcache.go +++ b/internal/ldcache/ldcache.go @@ -287,7 +287,7 @@ func (c *ldcache) resolveSelected(selected func(string) bool) ([]string, []strin func (c *ldcache) resolve(target string) (string, error) { name := filepath.Join(c.root, target) - c.logger.Debugf("checking %v", string(name)) + c.logger.Debugf("checking %v", name) link, err := symlinks.Resolve(name) if err != nil { diff --git a/internal/requirements/constraints/binary.go b/internal/requirements/constraints/binary.go index 5ca37408..be2667f7 100644 --- a/internal/requirements/constraints/binary.go +++ b/internal/requirements/constraints/binary.go @@ -57,7 +57,7 @@ func (c binary) eval() (bool, error) { return false, err } - switch string(c.operator) { + switch c.operator { case equal: return compare == 0, nil case notEqual: diff --git a/pkg/nvcdi/lib_test.go b/pkg/nvcdi/lib_test.go index 77ead658..b467ee5b 100644 --- a/pkg/nvcdi/lib_test.go +++ b/pkg/nvcdi/lib_test.go @@ -104,13 +104,13 @@ type infoMock struct { } func (i infoMock) HasDXCore() (bool, string) { - return bool(i.hasDXCore), "" + return i.hasDXCore, "" } func (i infoMock) HasNvml() (bool, string) { - return bool(i.hasNVML), "" + return i.hasNVML, "" } func (i infoMock) IsTegraSystem() (bool, string) { - return bool(i.isTegra), "" + return i.isTegra, "" } From f8870b31be31187472aafd49bc98774d3166cace Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 25 Aug 2023 16:55:04 +0200 Subject: [PATCH 13/19] Fix filepath.Join with single arg Signed-off-by: Evan Lezar --- internal/discover/graphics.go | 2 +- pkg/nvcdi/driver-nvml.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/discover/graphics.go b/internal/discover/graphics.go index 0bc9451e..b52eb563 100644 --- a/internal/discover/graphics.go +++ b/internal/discover/graphics.go @@ -239,7 +239,7 @@ func newDRMDeviceFilter(logger logger.Interface, devices image.VisibleDevices, d return nil, fmt.Errorf("failed to determine DRM devices for %v: %v", busID, err) } for _, drmDeviceNode := range drmDeviceNodes { - filter[filepath.Join(drmDeviceNode)] = true + filter[drmDeviceNode] = true } } diff --git a/pkg/nvcdi/driver-nvml.go b/pkg/nvcdi/driver-nvml.go index 28dacae3..a2a4a41c 100644 --- a/pkg/nvcdi/driver-nvml.go +++ b/pkg/nvcdi/driver-nvml.go @@ -128,9 +128,9 @@ func getFirmwareSearchPaths(logger logger.Interface) ([]string, error) { standardPaths := []string{ filepath.Join("/lib/firmware/updates/", utsRelease), - filepath.Join("/lib/firmware/updates/"), + "/lib/firmware/updates/", filepath.Join("/lib/firmware/", utsRelease), - filepath.Join("/lib/firmware/"), + "/lib/firmware/", } return append(firmwarePaths, standardPaths...), nil From 2f48ab99c33d58c944806b34ba5f1e51c210d1e5 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 25 Aug 2023 17:34:30 +0200 Subject: [PATCH 14/19] Address singleCaseSwitch errors Signed-off-by: Evan Lezar --- cmd/nvidia-ctk/config/config.go | 3 +-- pkg/config/engine/containerd/config_v1.go | 3 +-- pkg/config/engine/containerd/config_v2.go | 3 +-- pkg/config/engine/crio/crio.go | 3 +-- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/cmd/nvidia-ctk/config/config.go b/cmd/nvidia-ctk/config/config.go index f6d636fc..17cf1e06 100644 --- a/cmd/nvidia-ctk/config/config.go +++ b/cmd/nvidia-ctk/config/config.go @@ -146,8 +146,7 @@ func (c *configToml) setFlagToKeyValue(setFlag string) (string, interface{}, err if v == nil { return key, nil, errInvalidConfigOption } - switch v.(type) { - case bool: + if _, ok := v.(bool); ok { if len(setParts) == 1 { return key, true, nil } diff --git a/pkg/config/engine/containerd/config_v1.go b/pkg/config/engine/containerd/config_v1.go index b432f60b..c2343941 100644 --- a/pkg/config/engine/containerd/config_v1.go +++ b/pkg/config/engine/containerd/config_v1.go @@ -38,8 +38,7 @@ func (c *ConfigV1) AddRuntime(name string, path string, setAsDefault bool) error config.Set("version", int64(1)) - switch runc := config.GetPath([]string{"plugins", "cri", "containerd", "runtimes", "runc"}).(type) { - case *toml.Tree: + if runc, ok := config.GetPath([]string{"plugins", "cri", "containerd", "runtimes", "runc"}).(*toml.Tree); ok { runc, _ = toml.Load(runc.String()) config.SetPath([]string{"plugins", "cri", "containerd", "runtimes", name}, runc) } diff --git a/pkg/config/engine/containerd/config_v2.go b/pkg/config/engine/containerd/config_v2.go index 3fcdb9c5..35205446 100644 --- a/pkg/config/engine/containerd/config_v2.go +++ b/pkg/config/engine/containerd/config_v2.go @@ -32,8 +32,7 @@ func (c *Config) AddRuntime(name string, path string, setAsDefault bool) error { config.Set("version", int64(2)) - switch runc := config.GetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", "runc"}).(type) { - case *toml.Tree: + if runc, ok := config.GetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", "runc"}).(*toml.Tree); ok { runc, _ = toml.Load(runc.String()) config.SetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name}, runc) } diff --git a/pkg/config/engine/crio/crio.go b/pkg/config/engine/crio/crio.go index 03db8bff..dcf8572a 100644 --- a/pkg/config/engine/crio/crio.go +++ b/pkg/config/engine/crio/crio.go @@ -44,8 +44,7 @@ func (c *Config) AddRuntime(name string, path string, setAsDefault bool) error { config := (toml.Tree)(*c) - switch runc := config.Get("crio.runtime.runtimes.runc").(type) { - case *toml.Tree: + if runc, ok := config.Get("crio.runtime.runtimes.runc").(*toml.Tree); ok { runc, _ = toml.Load(runc.String()) config.SetPath([]string{"crio", "runtime", "runtimes", name}, runc) } From 2e1f94aedfcde95102732c26d4aaaa88b092a6aa Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 25 Aug 2023 17:38:36 +0200 Subject: [PATCH 15/19] Fix append assignments Signed-off-by: Evan Lezar --- internal/discover/ipc.go | 2 +- internal/platform-support/tegra/tegra.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/discover/ipc.go b/internal/discover/ipc.go index 9b0fc11c..f636290f 100644 --- a/internal/discover/ipc.go +++ b/internal/discover/ipc.go @@ -70,7 +70,7 @@ func (d *ipcMounts) Mounts() ([]Mount, error) { var modifiedMounts []Mount for _, m := range mounts { mount := m - mount.Options = append(m.Options, "noexec") + mount.Options = append(mount.Options, "noexec") modifiedMounts = append(modifiedMounts, mount) } diff --git a/internal/platform-support/tegra/tegra.go b/internal/platform-support/tegra/tegra.go index ad5c5b6a..563c6e28 100644 --- a/internal/platform-support/tegra/tegra.go +++ b/internal/platform-support/tegra/tegra.go @@ -51,11 +51,10 @@ func New(opts ...Option) (discover.Discover, error) { } if o.symlinkLocator == nil { - searchPaths := append(o.librarySearchPaths, "/") o.symlinkLocator = lookup.NewSymlinkLocator( lookup.WithLogger(o.logger), lookup.WithRoot(o.driverRoot), - lookup.WithSearchPaths(searchPaths...), + lookup.WithSearchPaths(append(o.librarySearchPaths, "/")...), ) } From 1b16b341dddd5256d40249cad78fae0350bed3b6 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 25 Aug 2023 17:43:31 +0200 Subject: [PATCH 16/19] Fix default permissions Signed-off-by: Evan Lezar --- internal/config/config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index f842bbb3..ba03716b 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -35,7 +35,7 @@ func TestGetConfigWithCustomConfig(t *testing.T) { contents := []byte("[nvidia-container-runtime]\ndebug = \"/nvidia-container-toolkit.log\"") require.NoError(t, os.MkdirAll(filepath.Dir(filename), 0766)) - require.NoError(t, os.WriteFile(filename, contents, 0766)) + require.NoError(t, os.WriteFile(filename, contents, 0600)) cfg, err := GetConfig() require.NoError(t, err) From 709e27bf4bb67e90b6888556e9185228dec38200 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 25 Aug 2023 17:50:43 +0200 Subject: [PATCH 17/19] Fix implicit memory aliasing in for loop Signed-off-by: Evan Lezar --- internal/modifier/cdi/spec.go | 1 + internal/modifier/hook_remover.go | 1 + internal/modifier/stable.go | 1 + internal/modifier/stable_test.go | 2 ++ internal/platform-support/tegra/csv/mount_spec_test.go | 1 + pkg/nvcdi/transform/deduplicate.go | 1 + pkg/nvcdi/transform/merged-device.go | 1 + pkg/nvcdi/transform/remove.go | 1 + pkg/nvcdi/transform/root.go | 1 + pkg/nvcdi/transform/sorter.go | 1 + tools/container/docker/docker_test.go | 3 +++ 11 files changed, 14 insertions(+) diff --git a/internal/modifier/cdi/spec.go b/internal/modifier/cdi/spec.go index 57c65e9a..8c54c54a 100644 --- a/internal/modifier/cdi/spec.go +++ b/internal/modifier/cdi/spec.go @@ -34,6 +34,7 @@ var _ oci.SpecModifier = (*fromCDISpec)(nil) // Modify applies the mofiications defined by the raw CDI spec to the incomming OCI spec. func (m fromCDISpec) Modify(spec *specs.Spec) error { for _, device := range m.cdiSpec.Devices { + device := device cdiDevice := cdi.Device{ Device: &device, } diff --git a/internal/modifier/hook_remover.go b/internal/modifier/hook_remover.go index f92011fd..f9af14ba 100644 --- a/internal/modifier/hook_remover.go +++ b/internal/modifier/hook_remover.go @@ -49,6 +49,7 @@ func (m nvidiaContainerRuntimeHookRemover) Modify(spec *specs.Spec) error { var newPrestart []specs.Hook for _, hook := range spec.Hooks.Prestart { + hook := hook if isNVIDIAContainerRuntimeHook(&hook) { m.logger.Debugf("Removing hook %v", hook) continue diff --git a/internal/modifier/stable.go b/internal/modifier/stable.go index 44dbc094..c72a94c3 100644 --- a/internal/modifier/stable.go +++ b/internal/modifier/stable.go @@ -48,6 +48,7 @@ func (m stableRuntimeModifier) Modify(spec *specs.Spec) error { // If an NVIDIA Container Runtime Hook already exists, we don't make any modifications to the spec. if spec.Hooks != nil { for _, hook := range spec.Hooks.Prestart { + hook := hook if isNVIDIAContainerRuntimeHook(&hook) { m.logger.Infof("Existing nvidia prestart hook (%v) found in OCI spec", hook.Path) return nil diff --git a/internal/modifier/stable_test.go b/internal/modifier/stable_test.go index 283b9e8b..586bdbb2 100644 --- a/internal/modifier/stable_test.go +++ b/internal/modifier/stable_test.go @@ -150,6 +150,8 @@ func TestAddHookModifier(t *testing.T) { } for _, tc := range testCases { + tc := tc + logHook.Reset() t.Run(tc.description, func(t *testing.T) { diff --git a/internal/platform-support/tegra/csv/mount_spec_test.go b/internal/platform-support/tegra/csv/mount_spec_test.go index 110ca8c1..c7162d1a 100644 --- a/internal/platform-support/tegra/csv/mount_spec_test.go +++ b/internal/platform-support/tegra/csv/mount_spec_test.go @@ -70,6 +70,7 @@ func TestNewMountSpecFromLine(t *testing.T) { for i, tc := range testCases { t.Run(fmt.Sprintf("test case %d", i), func(t *testing.T) { + tc := tc target, err := NewMountSpecFromLine(tc.line) if tc.expectedError != nil { require.Error(t, err) diff --git a/pkg/nvcdi/transform/deduplicate.go b/pkg/nvcdi/transform/deduplicate.go index e53ff594..f9d6db28 100644 --- a/pkg/nvcdi/transform/deduplicate.go +++ b/pkg/nvcdi/transform/deduplicate.go @@ -39,6 +39,7 @@ func (d dedupe) Transform(spec *specs.Spec) error { } var updatedDevices []specs.Device for _, device := range spec.Devices { + device := device if err := d.transformEdits(&device.ContainerEdits); err != nil { return err } diff --git a/pkg/nvcdi/transform/merged-device.go b/pkg/nvcdi/transform/merged-device.go index 6730afeb..9bd6b99d 100644 --- a/pkg/nvcdi/transform/merged-device.go +++ b/pkg/nvcdi/transform/merged-device.go @@ -111,6 +111,7 @@ func mergeDeviceSpecs(deviceSpecs []specs.Device, mergedDeviceName string) (*spe mergedEdits := edits.NewContainerEdits() for _, d := range deviceSpecs { + d := d edit := cdi.ContainerEdits{ ContainerEdits: &d.ContainerEdits, } diff --git a/pkg/nvcdi/transform/remove.go b/pkg/nvcdi/transform/remove.go index bf49660a..06c8500d 100644 --- a/pkg/nvcdi/transform/remove.go +++ b/pkg/nvcdi/transform/remove.go @@ -39,6 +39,7 @@ func (r remove) Transform(spec *specs.Spec) error { } for _, device := range spec.Devices { + device := device if err := r.transformEdits(&device.ContainerEdits); err != nil { return fmt.Errorf("failed to remove edits from device %q: %w", device.Name, err) } diff --git a/pkg/nvcdi/transform/root.go b/pkg/nvcdi/transform/root.go index 53bcf2a4..6ff9c75e 100644 --- a/pkg/nvcdi/transform/root.go +++ b/pkg/nvcdi/transform/root.go @@ -54,6 +54,7 @@ func (t rootTransformer) Transform(spec *specs.Spec) error { } for _, d := range spec.Devices { + d := d if err := t.applyToEdits(&d.ContainerEdits); err != nil { return fmt.Errorf("failed to apply root transform to device %s: %w", d.Name, err) } diff --git a/pkg/nvcdi/transform/sorter.go b/pkg/nvcdi/transform/sorter.go index e64de305..573fcac6 100644 --- a/pkg/nvcdi/transform/sorter.go +++ b/pkg/nvcdi/transform/sorter.go @@ -44,6 +44,7 @@ func (d sorter) Transform(spec *specs.Spec) error { } var updatedDevices []specs.Device for _, device := range spec.Devices { + device := device if err := d.transformEdits(&device.ContainerEdits); err != nil { return err } diff --git a/tools/container/docker/docker_test.go b/tools/container/docker/docker_test.go index d43c7203..83aa33a4 100644 --- a/tools/container/docker/docker_test.go +++ b/tools/container/docker/docker_test.go @@ -235,6 +235,8 @@ func TestUpdateConfig(t *testing.T) { } for i, tc := range testCases { + tc := tc + o := &options{ Options: container.Options{ RuntimeName: tc.runtimeName, @@ -361,6 +363,7 @@ func TestRevertConfig(t *testing.T) { } for i, tc := range testCases { + tc := tc o := &options{} err := o.RevertConfig(&tc.config) From 48d68e4eff356dc5c8cc52f2e6a2f1a5c0323439 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Mon, 28 Aug 2023 11:07:04 +0200 Subject: [PATCH 18/19] Add nolint for exec calls Signed-off-by: Evan Lezar --- cmd/nvidia-container-runtime-hook/main.go | 1 + cmd/nvidia-container-runtime/main_test.go | 5 +++++ cmd/nvidia-ctk/hook/chmod/chmod.go | 1 + cmd/nvidia-ctk/hook/update-ldcache/update-ldcache.go | 1 + internal/oci/runtime_syscall_exec.go | 1 + tools/container/container.go | 1 + tools/container/nvidia-toolkit/run.go | 3 +++ 7 files changed, 13 insertions(+) diff --git a/cmd/nvidia-container-runtime-hook/main.go b/cmd/nvidia-container-runtime-hook/main.go index 30aad846..0c61e558 100644 --- a/cmd/nvidia-container-runtime-hook/main.go +++ b/cmd/nvidia-container-runtime-hook/main.go @@ -142,6 +142,7 @@ func doPrestart() { args = append(args, rootfs) env := append(os.Environ(), cli.Environment...) + //nolint:gosec // TODO: Can we harden this so that there is less risk of command injection? err = syscall.Exec(args[0], args, env) log.Panicln("exec failed:", err) } diff --git a/cmd/nvidia-container-runtime/main_test.go b/cmd/nvidia-container-runtime/main_test.go index dad9ac79..185c3378 100644 --- a/cmd/nvidia-container-runtime/main_test.go +++ b/cmd/nvidia-container-runtime/main_test.go @@ -86,6 +86,7 @@ func TestBadInput(t *testing.T) { t.Fatal(err) } + //nolint:gosec // TODO: Can we harden this so that there is less risk of command injection cmdCreate := exec.Command(nvidiaRuntime, "create", "--bundle") t.Logf("executing: %s\n", strings.Join(cmdCreate.Args, " ")) err = cmdCreate.Run() @@ -103,6 +104,7 @@ func TestGoodInput(t *testing.T) { t.Fatalf("error generating runtime spec: %v", err) } + //nolint:gosec // TODO: Can we harden this so that there is less risk of command injection cmdRun := exec.Command(nvidiaRuntime, "run", "--bundle", cfg.bundlePath(), "testcontainer") t.Logf("executing: %s\n", strings.Join(cmdRun.Args, " ")) output, err := cmdRun.CombinedOutput() @@ -113,6 +115,7 @@ func TestGoodInput(t *testing.T) { require.NoError(t, err, "should be no errors when reading and parsing spec from config.json") require.Empty(t, spec.Hooks, "there should be no hooks in config.json") + //nolint:gosec // TODO: Can we harden this so that there is less risk of command injection cmdCreate := exec.Command(nvidiaRuntime, "create", "--bundle", cfg.bundlePath(), "testcontainer") t.Logf("executing: %s\n", strings.Join(cmdCreate.Args, " ")) err = cmdCreate.Run() @@ -158,6 +161,7 @@ func TestDuplicateHook(t *testing.T) { } // Test how runtime handles already existing prestart hook in config.json + //nolint:gosec // TODO: Can we harden this so that there is less risk of command injection cmdCreate := exec.Command(nvidiaRuntime, "create", "--bundle", cfg.bundlePath(), "testcontainer") t.Logf("executing: %s\n", strings.Join(cmdCreate.Args, " ")) output, err := cmdCreate.CombinedOutput() @@ -226,6 +230,7 @@ func (c testConfig) generateNewRuntimeSpec() error { return err } + //nolint:gosec // TODO: Can we harden this so that there is less risk of command injection cmd := exec.Command("cp", c.unmodifiedSpecFile(), c.specFilePath()) err = cmd.Run() if err != nil { diff --git a/cmd/nvidia-ctk/hook/chmod/chmod.go b/cmd/nvidia-ctk/hook/chmod/chmod.go index 1276f7fa..90ea8107 100644 --- a/cmd/nvidia-ctk/hook/chmod/chmod.go +++ b/cmd/nvidia-ctk/hook/chmod/chmod.go @@ -127,6 +127,7 @@ func (m command) run(c *cli.Context, cfg *config) error { args := append([]string{filepath.Base(chmodPath), cfg.mode}, paths...) + //nolint:gosec // TODO: Can we harden this so that there is less risk of command injection return syscall.Exec(chmodPath, args, nil) } diff --git a/cmd/nvidia-ctk/hook/update-ldcache/update-ldcache.go b/cmd/nvidia-ctk/hook/update-ldcache/update-ldcache.go index f9719e64..db5ae266 100644 --- a/cmd/nvidia-ctk/hook/update-ldcache/update-ldcache.go +++ b/cmd/nvidia-ctk/hook/update-ldcache/update-ldcache.go @@ -100,6 +100,7 @@ func (m command) run(c *cli.Context, cfg *config) error { args = append(args, "-r", containerRoot) } + //nolint:gosec // TODO: Can we harden this so that there is less risk of command injection return syscall.Exec(args[0], args, nil) } diff --git a/internal/oci/runtime_syscall_exec.go b/internal/oci/runtime_syscall_exec.go index d752776a..6820e3a1 100644 --- a/internal/oci/runtime_syscall_exec.go +++ b/internal/oci/runtime_syscall_exec.go @@ -27,6 +27,7 @@ type syscallExec struct{} var _ Runtime = (*syscallExec)(nil) func (r syscallExec) Exec(args []string) error { + //nolint:gosec // TODO: Can we harden this so that there is less risk of command injection err := syscall.Exec(args[0], args, os.Environ()) if err != nil { return fmt.Errorf("could not exec '%v': %v", args[0], err) diff --git a/tools/container/container.go b/tools/container/container.go index 85547019..4cba08f0 100644 --- a/tools/container/container.go +++ b/tools/container/container.go @@ -157,6 +157,7 @@ func (o Options) SystemdRestart(service string) error { logrus.Infof("Restarting %v%v using systemd: %v", service, msg, args) + //nolint:gosec // TODO: Can we harden this so that there is less risk of command injection cmd := exec.Command(args[0], args[1:]...) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr diff --git a/tools/container/nvidia-toolkit/run.go b/tools/container/nvidia-toolkit/run.go index 740835b3..87d5b111 100644 --- a/tools/container/nvidia-toolkit/run.go +++ b/tools/container/nvidia-toolkit/run.go @@ -229,6 +229,7 @@ func installToolkit(o *options) error { filepath.Join(o.root, toolkitSubDir), } + //nolint:gosec // TODO: Can we harden this so that there is less risk of command injection cmd := exec.Command("sh", "-c", strings.Join(cmdline, " ")) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr @@ -247,6 +248,7 @@ func setupRuntime(o *options) error { cmdline := fmt.Sprintf("%v setup %v %v\n", o.runtime, o.runtimeArgs, toolkitDir) + //nolint:gosec // TODO: Can we harden this so that there is less risk of command injection cmd := exec.Command("sh", "-c", cmdline) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr @@ -272,6 +274,7 @@ func cleanupRuntime(o *options) error { cmdline := fmt.Sprintf("%v cleanup %v %v\n", o.runtime, o.runtimeArgs, toolkitDir) + //nolint:gosec // TODO: Can we harden this so that there is less risk of command injection cmd := exec.Command("sh", "-c", cmdline) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr From acc50969dc6f0e879021c10df69f1867cc47e12b Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Tue, 29 Aug 2023 10:12:45 +0200 Subject: [PATCH 19/19] Fix ifElseChain lint errors Signed-off-by: Evan Lezar --- cmd/nvidia-container-runtime/main_test.go | 7 ++++--- internal/oci/spec_memory_test.go | 7 ++++--- internal/platform-support/tegra/symlinks.go | 9 +++++---- internal/runtime/logger.go | 14 ++++++++------ 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/cmd/nvidia-container-runtime/main_test.go b/cmd/nvidia-container-runtime/main_test.go index 185c3378..90bfbea7 100644 --- a/cmd/nvidia-container-runtime/main_test.go +++ b/cmd/nvidia-container-runtime/main_test.go @@ -193,14 +193,15 @@ func (c testConfig) getRuntimeSpec() (specs.Spec, error) { defer jsonFile.Close() jsonContent, err := io.ReadAll(jsonFile) - if err != nil { + switch { + case err != nil: return spec, err - } else if json.Valid(jsonContent) { + case json.Valid(jsonContent): err = json.Unmarshal(jsonContent, &spec) if err != nil { return spec, err } - } else { + default: err = json.NewDecoder(bytes.NewReader(jsonContent)).Decode(&spec) if err != nil { return spec, err diff --git a/internal/oci/spec_memory_test.go b/internal/oci/spec_memory_test.go index a43944a4..3e8ba0ed 100644 --- a/internal/oci/spec_memory_test.go +++ b/internal/oci/spec_memory_test.go @@ -152,12 +152,13 @@ func TestModify(t *testing.T) { err := spec.Modify(modifier{tc.modifierError}) - if tc.spec == nil { + switch { + case tc.spec == nil: require.Error(t, err, "%d: %v", i, tc) - } else if tc.modifierError != nil { + case tc.modifierError != nil: require.EqualError(t, err, tc.modifierError.Error(), "%d: %v", i, tc) require.EqualValues(t, &specs.Spec{}, spec.Spec, "%d: %v", i, tc) - } else { + default: require.NoError(t, err, "%d: %v", i, tc) require.Equal(t, "updated", spec.Spec.Version, "%d: %v", i, tc) } diff --git a/internal/platform-support/tegra/symlinks.go b/internal/platform-support/tegra/symlinks.go index c64138db..283b2f4d 100644 --- a/internal/platform-support/tegra/symlinks.go +++ b/internal/platform-support/tegra/symlinks.go @@ -80,19 +80,20 @@ func (d symlinkHook) getSpecificLinks() ([]string, error) { lib := filepath.Base(m.Path) - if strings.HasPrefix(lib, "libcuda.so") { + switch { + case strings.HasPrefix(lib, "libcuda.so"): // XXX Many applications wrongly assume that libcuda.so exists (e.g. with dlopen). target = "libcuda.so.1" link = "libcuda.so" - } else if strings.HasPrefix(lib, "libGLX_nvidia.so") { + case strings.HasPrefix(lib, "libGLX_nvidia.so"): // XXX GLVND requires this symlink for indirect GLX support. target = lib link = "libGLX_indirect.so.0" - } else if strings.HasPrefix(lib, "libnvidia-opticalflow.so") { + case strings.HasPrefix(lib, "libnvidia-opticalflow.so"): // XXX Fix missing symlink for libnvidia-opticalflow.so. target = "libnvidia-opticalflow.so.1" link = "libnvidia-opticalflow.so" - } else { + default: continue } if linkProcessed[link] { diff --git a/internal/runtime/logger.go b/internal/runtime/logger.go index 2cf2c953..a23702a4 100644 --- a/internal/runtime/logger.go +++ b/internal/runtime/logger.go @@ -104,11 +104,12 @@ func (l *Logger) Update(filename string, logLevel string, argv []string) { newLogger.SetFormatter(new(logrus.JSONFormatter)) } - if len(logFiles) == 0 { + switch len(logFiles) { + case 0: newLogger.SetOutput(io.Discard) - } else if len(logFiles) == 1 { + case 1: newLogger.SetOutput(logFiles[0]) - } else if len(logFiles) > 1 { + default: var writers []io.Writer for _, f := range logFiles { writers = append(writers, f) @@ -234,12 +235,13 @@ func parseArgs(args []string) loggerConfig { } var value string - if len(parts) == 2 { + switch { + case len(parts) == 2: value = parts[2] - } else if i+1 < len(args) { + case i+1 < len(args): value = args[i+1] i++ - } else { + default: continue }