From 3a11c8659cfb1dafc721e8091512ae7bee4d7b62 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Sun, 28 Jan 2024 13:53:41 +0100 Subject: [PATCH 01/12] Use golangci-lint for checks Signed-off-by: Evan Lezar --- .github/workflows/golang.yaml | 3 --- .golangci.yml | 25 +++++++++++++++++++++++ Makefile | 37 +++++++++-------------------------- 3 files changed, 34 insertions(+), 31 deletions(-) create mode 100644 .golangci.yml diff --git a/.github/workflows/golang.yaml b/.github/workflows/golang.yaml index 0b28a486..f29ecfcd 100644 --- a/.github/workflows/golang.yaml +++ b/.github/workflows/golang.yaml @@ -26,9 +26,6 @@ on: jobs: check: - # TODO: Disable golangci-lint until the repo has been onboarded. - # See https://github.com/NVIDIA/mig-parted/issues/34 - if: false runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..77d30532 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,25 @@ +run: + deadline: 10m + +linters: + enable: + - contextcheck + - errcheck + - gocritic + - gofmt + - goimports + - gosec + - gosimple + - govet + - ineffassign + - misspell + - staticcheck + - unconvert + disable: [] + +linters-settings: + goimports: + local-prefixes: github.com/NVIDIA/mig-parted + +issues: + exclude-rules: [] diff --git a/Makefile b/Makefile index 79beea90..bf3f991b 100644 --- a/Makefile +++ b/Makefile @@ -27,7 +27,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 := lint MAKE_TARGETS := binaries build check fmt lint-internal test examples cmds coverage generate $(CHECK_TARGETS) TARGETS := $(MAKE_TARGETS) $(EXAMPLE_TARGETS) $(CMD_TARGETS) @@ -66,43 +66,24 @@ 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)/... +goimports: + go list -f {{.Dir}} $(MODULE)/... \ + | xargs goimports -local $(MODULE) -w lint: -# We use `go list -f '{{.Dir}}' $(MODULE)/...` to skip the `vendor` folder. - go list -f '{{.Dir}}' $(MODULE)/... | grep -v /internal/ | xargs golint -set_exit_status - -lint-internal: -# We use `go list -f '{{.Dir}}' $(MODULE)/...` to skip the `vendor` folder. - go list -f '{{.Dir}}' $(MODULE)/internal/... | xargs golint -set_exit_status - -misspell: - misspell $(MODULE)/... - -vet: - go vet $(MODULE)/... + golangci-lint run ./... COVERAGE_FILE := coverage.out test: build cmds - go test -v -coverprofile=$(COVERAGE_FILE) $(MODULE)/... + go test -coverprofile=$(COVERAGE_FILE) $(MODULE)/cmd/... $(MODULE)/internal/... $(MODULE)/api/... coverage: test cat $(COVERAGE_FILE) | grep -v "_mock.go" > $(COVERAGE_FILE).no-mocks go tool cover -func=$(COVERAGE_FILE).no-mocks +generate: + go generate $(MODULE)/... + $(DOCKER_TARGETS): docker-%: @echo "Running 'make $(*)' in container image $(BUILDIMAGE)" $(DOCKER) run \ From 8b82aa91198b1bed10911540d8b76e15216a4bea Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Sun, 28 Jan 2024 13:56:28 +0100 Subject: [PATCH 02/12] Address misspell issues Signed-off-by: Evan Lezar --- api/hooks/v1/hooks.go | 2 +- api/spec/v1/spec_test.go | 2 +- cmd/nvidia-mig-manager/main.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/hooks/v1/hooks.go b/api/hooks/v1/hooks.go index 3e109e2a..c09f1754 100644 --- a/api/hooks/v1/hooks.go +++ b/api/hooks/v1/hooks.go @@ -77,7 +77,7 @@ func (h *HookSpec) Run(envs EnvsMap, output bool) error { } // Combine merges to EnvMaps together -// Overlapping enviroment variables from e2 take precendence over those in e. +// Overlapping environment variables from e2 take precedence over those in e. func (e EnvsMap) Combine(e2 EnvsMap) EnvsMap { combined := make(EnvsMap) for k, v := range e { diff --git a/api/spec/v1/spec_test.go b/api/spec/v1/spec_test.go index 45dffd86..12a1bd38 100644 --- a/api/spec/v1/spec_test.go +++ b/api/spec/v1/spec_test.go @@ -28,7 +28,7 @@ func TestMarshallUnmarshall(t *testing.T) { spec := Spec{ Version: "v1", MigConfigs: map[string]MigConfigSpecSlice{ - "valid-format-non-existant-devices": []MigConfigSpec{ + "valid-format-non-existent-devices": []MigConfigSpec{ { Devices: "all", MigEnabled: true, diff --git a/cmd/nvidia-mig-manager/main.go b/cmd/nvidia-mig-manager/main.go index b4df8c13..0120aee7 100644 --- a/cmd/nvidia-mig-manager/main.go +++ b/cmd/nvidia-mig-manager/main.go @@ -275,7 +275,7 @@ func start(c *cli.Context) error { log.Errorf("Error: %s", err) continue } - log.Infof("Successfuly updated to MIG config: %s", value) + log.Infof("Successfully updated to MIG config: %s", value) } } From 2c4957c2c6123b3e11d6af569032bc991f5cc55a Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Sun, 28 Jan 2024 13:58:20 +0100 Subject: [PATCH 03/12] Run goimports -l Signed-off-by: Evan Lezar --- api/spec/v1/spec_test.go | 3 ++- cmd/nvidia-mig-manager/main.go | 3 ++- cmd/nvidia-mig-parted/apply/apply.go | 5 +++-- cmd/nvidia-mig-parted/assert/assert.go | 5 +++-- cmd/nvidia-mig-parted/checkpoint/checkpoint.go | 5 +++-- cmd/nvidia-mig-parted/export/export.go | 5 +++-- cmd/nvidia-mig-parted/export/export_test.go | 3 ++- cmd/nvidia-mig-parted/main.go | 5 +++-- cmd/nvidia-mig-parted/restore/restore.go | 5 +++-- cmd/nvidia-mig-parted/util/nvml.go | 3 ++- pkg/mig/config/config.go | 3 ++- pkg/mig/config/config_test.go | 3 ++- pkg/mig/config/known_configs_test.go | 3 ++- pkg/mig/mode/nvml.go | 3 ++- pkg/mig/mode/nvml_test.go | 3 ++- pkg/mig/state/state.go | 3 ++- pkg/mig/state/state_test.go | 3 ++- 17 files changed, 40 insertions(+), 23 deletions(-) diff --git a/api/spec/v1/spec_test.go b/api/spec/v1/spec_test.go index 12a1bd38..2680c3ae 100644 --- a/api/spec/v1/spec_test.go +++ b/api/spec/v1/spec_test.go @@ -19,9 +19,10 @@ package v1 import ( "testing" - "github.com/NVIDIA/mig-parted/pkg/types" "github.com/stretchr/testify/require" "sigs.k8s.io/yaml" + + "github.com/NVIDIA/mig-parted/pkg/types" ) func TestMarshallUnmarshall(t *testing.T) { diff --git a/cmd/nvidia-mig-manager/main.go b/cmd/nvidia-mig-manager/main.go index 0120aee7..2fc861e8 100644 --- a/cmd/nvidia-mig-manager/main.go +++ b/cmd/nvidia-mig-manager/main.go @@ -23,10 +23,11 @@ import ( "strings" "sync" - "github.com/NVIDIA/mig-parted/internal/info" log "github.com/sirupsen/logrus" cli "github.com/urfave/cli/v2" + "github.com/NVIDIA/mig-parted/internal/info" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/client-go/kubernetes" diff --git a/cmd/nvidia-mig-parted/apply/apply.go b/cmd/nvidia-mig-parted/apply/apply.go index fffaeb40..082f6739 100644 --- a/cmd/nvidia-mig-parted/apply/apply.go +++ b/cmd/nvidia-mig-parted/apply/apply.go @@ -21,11 +21,12 @@ import ( "os" "reflect" + "github.com/sirupsen/logrus" + cli "github.com/urfave/cli/v2" + hooks "github.com/NVIDIA/mig-parted/api/hooks/v1" "github.com/NVIDIA/mig-parted/cmd/nvidia-mig-parted/assert" "github.com/NVIDIA/mig-parted/internal/nvml" - "github.com/sirupsen/logrus" - cli "github.com/urfave/cli/v2" "sigs.k8s.io/yaml" ) diff --git a/cmd/nvidia-mig-parted/assert/assert.go b/cmd/nvidia-mig-parted/assert/assert.go index 404f21a1..c9ad00ee 100644 --- a/cmd/nvidia-mig-parted/assert/assert.go +++ b/cmd/nvidia-mig-parted/assert/assert.go @@ -22,12 +22,13 @@ import ( "os" "strings" + "github.com/sirupsen/logrus" + cli "github.com/urfave/cli/v2" + v1 "github.com/NVIDIA/mig-parted/api/spec/v1" "github.com/NVIDIA/mig-parted/cmd/nvidia-mig-parted/util" "github.com/NVIDIA/mig-parted/internal/nvml" "github.com/NVIDIA/mig-parted/pkg/types" - "github.com/sirupsen/logrus" - cli "github.com/urfave/cli/v2" "sigs.k8s.io/yaml" ) diff --git a/cmd/nvidia-mig-parted/checkpoint/checkpoint.go b/cmd/nvidia-mig-parted/checkpoint/checkpoint.go index bd421169..57539211 100644 --- a/cmd/nvidia-mig-parted/checkpoint/checkpoint.go +++ b/cmd/nvidia-mig-parted/checkpoint/checkpoint.go @@ -22,12 +22,13 @@ import ( "os" "strings" + "github.com/sirupsen/logrus" + cli "github.com/urfave/cli/v2" + checkpoint "github.com/NVIDIA/mig-parted/api/checkpoint/v1" "github.com/NVIDIA/mig-parted/cmd/nvidia-mig-parted/util" "github.com/NVIDIA/mig-parted/internal/nvml" "github.com/NVIDIA/mig-parted/pkg/mig/state" - "github.com/sirupsen/logrus" - cli "github.com/urfave/cli/v2" ) var log = logrus.New() diff --git a/cmd/nvidia-mig-parted/export/export.go b/cmd/nvidia-mig-parted/export/export.go index aece81fb..8dd47f4a 100644 --- a/cmd/nvidia-mig-parted/export/export.go +++ b/cmd/nvidia-mig-parted/export/export.go @@ -22,11 +22,12 @@ import ( "io" "os" - "github.com/NVIDIA/mig-parted/api/spec/v1" - "github.com/NVIDIA/mig-parted/internal/nvml" "github.com/sirupsen/logrus" cli "github.com/urfave/cli/v2" + v1 "github.com/NVIDIA/mig-parted/api/spec/v1" + "github.com/NVIDIA/mig-parted/internal/nvml" + yaml "gopkg.in/yaml.v2" ) diff --git a/cmd/nvidia-mig-parted/export/export_test.go b/cmd/nvidia-mig-parted/export/export_test.go index 6bb3c446..e3819b2d 100644 --- a/cmd/nvidia-mig-parted/export/export_test.go +++ b/cmd/nvidia-mig-parted/export/export_test.go @@ -19,8 +19,9 @@ package export import ( "testing" - "github.com/NVIDIA/mig-parted/api/spec/v1" "github.com/stretchr/testify/require" + + v1 "github.com/NVIDIA/mig-parted/api/spec/v1" ) func TestMergeConfigSpecs(t *testing.T) { diff --git a/cmd/nvidia-mig-parted/main.go b/cmd/nvidia-mig-parted/main.go index c58c8ff5..b560e64a 100644 --- a/cmd/nvidia-mig-parted/main.go +++ b/cmd/nvidia-mig-parted/main.go @@ -19,6 +19,9 @@ package main import ( "os" + log "github.com/sirupsen/logrus" + cli "github.com/urfave/cli/v2" + "github.com/NVIDIA/mig-parted/cmd/nvidia-mig-parted/apply" "github.com/NVIDIA/mig-parted/cmd/nvidia-mig-parted/assert" "github.com/NVIDIA/mig-parted/cmd/nvidia-mig-parted/checkpoint" @@ -26,8 +29,6 @@ import ( "github.com/NVIDIA/mig-parted/cmd/nvidia-mig-parted/restore" "github.com/NVIDIA/mig-parted/cmd/nvidia-mig-parted/util" "github.com/NVIDIA/mig-parted/internal/info" - log "github.com/sirupsen/logrus" - cli "github.com/urfave/cli/v2" ) // Flags holds variables that represent the set of top level flags that can be passed to the mig-parted CLI. diff --git a/cmd/nvidia-mig-parted/restore/restore.go b/cmd/nvidia-mig-parted/restore/restore.go index 2827fb77..761b3555 100644 --- a/cmd/nvidia-mig-parted/restore/restore.go +++ b/cmd/nvidia-mig-parted/restore/restore.go @@ -23,13 +23,14 @@ import ( "reflect" "strings" + "github.com/sirupsen/logrus" + cli "github.com/urfave/cli/v2" + checkpoint "github.com/NVIDIA/mig-parted/api/checkpoint/v1" hooks "github.com/NVIDIA/mig-parted/api/hooks/v1" "github.com/NVIDIA/mig-parted/cmd/nvidia-mig-parted/apply" "github.com/NVIDIA/mig-parted/pkg/mig/state" "github.com/NVIDIA/mig-parted/pkg/types" - "github.com/sirupsen/logrus" - cli "github.com/urfave/cli/v2" ) var log = logrus.New() diff --git a/cmd/nvidia-mig-parted/util/nvml.go b/cmd/nvidia-mig-parted/util/nvml.go index 5b3a4090..c0e69f47 100644 --- a/cmd/nvidia-mig-parted/util/nvml.go +++ b/cmd/nvidia-mig-parted/util/nvml.go @@ -22,8 +22,9 @@ import ( "strconv" "strings" - "github.com/NVIDIA/mig-parted/internal/nvml" log "github.com/sirupsen/logrus" + + "github.com/NVIDIA/mig-parted/internal/nvml" ) const ( diff --git a/pkg/mig/config/config.go b/pkg/mig/config/config.go index a076df29..278621b2 100644 --- a/pkg/mig/config/config.go +++ b/pkg/mig/config/config.go @@ -20,10 +20,11 @@ import ( "fmt" "strings" + log "github.com/sirupsen/logrus" + "github.com/NVIDIA/mig-parted/internal/nvlib" "github.com/NVIDIA/mig-parted/internal/nvml" "github.com/NVIDIA/mig-parted/pkg/types" - log "github.com/sirupsen/logrus" ) type Manager interface { diff --git a/pkg/mig/config/config_test.go b/pkg/mig/config/config_test.go index 13642bbe..26c9929f 100644 --- a/pkg/mig/config/config_test.go +++ b/pkg/mig/config/config_test.go @@ -22,10 +22,11 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" + "github.com/NVIDIA/mig-parted/internal/nvlib" "github.com/NVIDIA/mig-parted/internal/nvml" "github.com/NVIDIA/mig-parted/pkg/types" - "github.com/stretchr/testify/require" ) func NewMockLunaServerMigConfigManager() Manager { diff --git a/pkg/mig/config/known_configs_test.go b/pkg/mig/config/known_configs_test.go index 19978f5c..33695ff5 100644 --- a/pkg/mig/config/known_configs_test.go +++ b/pkg/mig/config/known_configs_test.go @@ -19,8 +19,9 @@ package config import ( "testing" - "github.com/NVIDIA/mig-parted/pkg/types" "github.com/stretchr/testify/require" + + "github.com/NVIDIA/mig-parted/pkg/types" ) func TestValidConfiguration(t *testing.T) { diff --git a/pkg/mig/mode/nvml.go b/pkg/mig/mode/nvml.go index a9b78c01..68d64b77 100644 --- a/pkg/mig/mode/nvml.go +++ b/pkg/mig/mode/nvml.go @@ -19,8 +19,9 @@ package mode import ( "fmt" - "github.com/NVIDIA/mig-parted/internal/nvml" log "github.com/sirupsen/logrus" + + "github.com/NVIDIA/mig-parted/internal/nvml" ) type nvmlMigModeManager struct { diff --git a/pkg/mig/mode/nvml_test.go b/pkg/mig/mode/nvml_test.go index 006ac99f..38e815d2 100644 --- a/pkg/mig/mode/nvml_test.go +++ b/pkg/mig/mode/nvml_test.go @@ -20,8 +20,9 @@ import ( "fmt" "testing" - "github.com/NVIDIA/mig-parted/internal/nvml" "github.com/stretchr/testify/require" + + "github.com/NVIDIA/mig-parted/internal/nvml" ) type Return = nvml.Return diff --git a/pkg/mig/state/state.go b/pkg/mig/state/state.go index f0d0ced9..1426ce15 100644 --- a/pkg/mig/state/state.go +++ b/pkg/mig/state/state.go @@ -19,12 +19,13 @@ package state import ( "fmt" + log "github.com/sirupsen/logrus" + "github.com/NVIDIA/mig-parted/internal/nvlib" "github.com/NVIDIA/mig-parted/internal/nvml" "github.com/NVIDIA/mig-parted/pkg/mig/config" "github.com/NVIDIA/mig-parted/pkg/mig/mode" "github.com/NVIDIA/mig-parted/pkg/types" - log "github.com/sirupsen/logrus" ) // Manager represents the set of operations for fetching / restoring the full MIG state of all GPUs on a node. diff --git a/pkg/mig/state/state_test.go b/pkg/mig/state/state_test.go index cbfbb961..6ff75642 100644 --- a/pkg/mig/state/state_test.go +++ b/pkg/mig/state/state_test.go @@ -20,11 +20,12 @@ import ( "fmt" "testing" + "github.com/stretchr/testify/require" + "github.com/NVIDIA/mig-parted/internal/nvml" "github.com/NVIDIA/mig-parted/pkg/mig/config" "github.com/NVIDIA/mig-parted/pkg/mig/mode" "github.com/NVIDIA/mig-parted/pkg/types" - "github.com/stretchr/testify/require" ) func newMockMigStateManagerOnLunaServer() *migStateManager { From 300e2d56eeac6a0ad9a178cf9a1d7acab7457f8e Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Sun, 28 Jan 2024 14:01:02 +0100 Subject: [PATCH 04/12] Add exception for random number generator in test Signed-off-by: Evan Lezar --- .golangci.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 77d30532..20e4d944 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -22,4 +22,9 @@ linters-settings: local-prefixes: github.com/NVIDIA/mig-parted issues: - exclude-rules: [] + exclude-rules: + # We use math/rand instead of crypto/rand for unique names in config tests. + - path: pkg/mig/config/config_test.go + linters: + - gosec + text: "G404" From 404149d8585dc9afbcb3a76747ba89fc9c2e5319 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Sun, 28 Jan 2024 14:02:47 +0100 Subject: [PATCH 05/12] Address gocritic error: capitalized local variables Signed-off-by: Evan Lezar --- internal/nvlib/mig/mig.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/nvlib/mig/mig.go b/internal/nvlib/mig/mig.go index e8efcfdc..658f981d 100644 --- a/internal/nvlib/mig/mig.go +++ b/internal/nvlib/mig/mig.go @@ -42,11 +42,11 @@ func NewMock(nvml nvml.Interface) Interface { return Interface{nvml} } -func (I Interface) Device(d nvml.Device) Device { +func (i Interface) Device(d nvml.Device) Device { return Device{d} } -func (I Interface) GpuInstance(gi nvml.GpuInstance) GpuInstance { +func (i Interface) GpuInstance(gi nvml.GpuInstance) GpuInstance { return GpuInstance{gi} } From 495d563e4a22f31e2ed1aded1bca6cd05696ebcc Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Sun, 28 Jan 2024 14:06:40 +0100 Subject: [PATCH 06/12] Fix checking of returned error values Signed-off-by: Evan Lezar --- api/hooks/v1/hooks_test.go | 2 +- cmd/nvidia-mig-parted/apply/apply.go | 2 +- cmd/nvidia-mig-parted/assert/assert.go | 2 +- cmd/nvidia-mig-parted/checkpoint/checkpoint.go | 2 +- cmd/nvidia-mig-parted/export/export.go | 10 +++++++--- cmd/nvidia-mig-parted/restore/restore.go | 2 +- 6 files changed, 12 insertions(+), 8 deletions(-) diff --git a/api/hooks/v1/hooks_test.go b/api/hooks/v1/hooks_test.go index 13658170..fd3dec2f 100644 --- a/api/hooks/v1/hooks_test.go +++ b/api/hooks/v1/hooks_test.go @@ -49,7 +49,7 @@ func captureOutput(f func() error) (string, error) { go func() { var buf bytes.Buffer wg.Done() - io.Copy(&buf, reader) + _, _ = io.Copy(&buf, reader) out <- buf.String() }() diff --git a/cmd/nvidia-mig-parted/apply/apply.go b/cmd/nvidia-mig-parted/apply/apply.go index 082f6739..b0558d97 100644 --- a/cmd/nvidia-mig-parted/apply/apply.go +++ b/cmd/nvidia-mig-parted/apply/apply.go @@ -184,7 +184,7 @@ func (c *Context) ApplyMigConfig() error { func applyWrapper(c *cli.Context, f *Flags) error { err := CheckFlags(f) if err != nil { - cli.ShowSubcommandHelp(c) + _ = cli.ShowSubcommandHelp(c) return err } diff --git a/cmd/nvidia-mig-parted/assert/assert.go b/cmd/nvidia-mig-parted/assert/assert.go index c9ad00ee..34276fa2 100644 --- a/cmd/nvidia-mig-parted/assert/assert.go +++ b/cmd/nvidia-mig-parted/assert/assert.go @@ -104,7 +104,7 @@ func BuildCommand() *cli.Command { func assertWrapper(c *cli.Context, f *Flags) error { err := CheckFlags(f) if err != nil { - cli.ShowSubcommandHelp(c) + _ = cli.ShowSubcommandHelp(c) return err } diff --git a/cmd/nvidia-mig-parted/checkpoint/checkpoint.go b/cmd/nvidia-mig-parted/checkpoint/checkpoint.go index 57539211..65c222a8 100644 --- a/cmd/nvidia-mig-parted/checkpoint/checkpoint.go +++ b/cmd/nvidia-mig-parted/checkpoint/checkpoint.go @@ -82,7 +82,7 @@ func CheckFlags(f *Flags) error { func checkpointWrapper(c *cli.Context, f *Flags) error { err := CheckFlags(f) if err != nil { - cli.ShowSubcommandHelp(c) + _ = cli.ShowSubcommandHelp(c) return err } diff --git a/cmd/nvidia-mig-parted/export/export.go b/cmd/nvidia-mig-parted/export/export.go index 8dd47f4a..eb033a6d 100644 --- a/cmd/nvidia-mig-parted/export/export.go +++ b/cmd/nvidia-mig-parted/export/export.go @@ -92,7 +92,7 @@ func BuildCommand() *cli.Command { func exportWrapper(c *cli.Context, f *Flags) error { err := CheckFlags(f) if err != nil { - cli.ShowSubcommandHelp(c) + _ = cli.ShowSubcommandHelp(c) return err } @@ -132,13 +132,17 @@ func WriteOutput(w io.Writer, spec *v1.Spec, f *Flags) error { if err != nil { return fmt.Errorf("error unmarshaling MIG config to YAML: %v", err) } - w.Write(output) + if _, err := w.Write(output); err != nil { + return fmt.Errorf("error writing YAML output: %w", err) + } case JSONFormat: output, err := json.MarshalIndent(spec, "", " ") if err != nil { return fmt.Errorf("error unmarshaling MIG config to JSON: %v", err) } - w.Write(output) + if _, err := w.Write(output); err != nil { + return fmt.Errorf("error writing JSON output: %w", err) + } } return nil } diff --git a/cmd/nvidia-mig-parted/restore/restore.go b/cmd/nvidia-mig-parted/restore/restore.go index 761b3555..82f88344 100644 --- a/cmd/nvidia-mig-parted/restore/restore.go +++ b/cmd/nvidia-mig-parted/restore/restore.go @@ -155,7 +155,7 @@ func (c *Context) ApplyMigConfig() error { func restoreWrapper(c *cli.Context, f *Flags) error { err := CheckFlags(f) if err != nil { - cli.ShowSubcommandHelp(c) + _ = cli.ShowSubcommandHelp(c) return err } From 28f9599079a99135a96413ef63137ca84a6895f0 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Sun, 28 Jan 2024 14:11:26 +0100 Subject: [PATCH 07/12] Rewrite single case switch statements Signed-off-by: Evan Lezar --- api/spec/v1/helpers.go | 6 ++---- api/spec/v1/spec.go | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/api/spec/v1/helpers.go b/api/spec/v1/helpers.go index 6f14a2c8..4263c82d 100644 --- a/api/spec/v1/helpers.go +++ b/api/spec/v1/helpers.go @@ -48,8 +48,7 @@ func (ms *MigConfigSpec) MatchesDeviceFilter(deviceID types.DeviceID) bool { // MatchesAllDevices checks a 'MigConfigSpec' to see if it matches on 'all' devices. func (ms *MigConfigSpec) MatchesAllDevices() bool { - switch devices := ms.Devices.(type) { - case string: + if devices, ok := ms.Devices.(string); ok { return devices == "all" } return false @@ -57,8 +56,7 @@ func (ms *MigConfigSpec) MatchesAllDevices() bool { // MatchesDevices checks a 'MigConfigSpec' to see if it matches on a device at the specified 'index'. func (ms *MigConfigSpec) MatchesDevices(index int) bool { - switch devices := ms.Devices.(type) { - case []int: + if devices, ok := ms.Devices.([]int); ok { for _, d := range devices { if index == d { return true diff --git a/api/spec/v1/spec.go b/api/spec/v1/spec.go index 96f8eae7..ed87fc3e 100644 --- a/api/spec/v1/spec.go +++ b/api/spec/v1/spec.go @@ -57,8 +57,7 @@ func (s *Spec) UnmarshalJSON(b []byte) error { result := Spec{} for k, v := range spec { - switch k { - case "version": + if k == "version" { var version string err := json.Unmarshal(v, &version) if err != nil { From d547743d27f65932595a3589a6aebd2a4751ef3a Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Sun, 28 Jan 2024 14:13:55 +0100 Subject: [PATCH 08/12] Remove deprecated call to rand.Seed Signed-off-by: Evan Lezar --- pkg/mig/config/config_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/mig/config/config_test.go b/pkg/mig/config/config_test.go index 26c9929f..27ceb047 100644 --- a/pkg/mig/config/config_test.go +++ b/pkg/mig/config/config_test.go @@ -20,7 +20,6 @@ import ( "fmt" "math/rand" "testing" - "time" "github.com/stretchr/testify/require" @@ -143,7 +142,6 @@ func TestIteratePermutationsUntilSuccess(t *testing.T) { return perms } - rand.Seed(time.Now().UnixNano()) mcg := NewA100_SXM4_40GB_MigConfigGroup() type testCase struct { From 8bbce502a6b2d5a95ad23ec904184a6d6bc702cf Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Sun, 28 Jan 2024 14:18:02 +0100 Subject: [PATCH 09/12] Use os.Create instead of creating file with 0666 permissions Signed-off-by: Evan Lezar --- cmd/nvidia-mig-parted/checkpoint/checkpoint.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cmd/nvidia-mig-parted/checkpoint/checkpoint.go b/cmd/nvidia-mig-parted/checkpoint/checkpoint.go index 65c222a8..ab5529c3 100644 --- a/cmd/nvidia-mig-parted/checkpoint/checkpoint.go +++ b/cmd/nvidia-mig-parted/checkpoint/checkpoint.go @@ -108,8 +108,12 @@ func checkpointWrapper(c *cli.Context, f *Flags) error { return fmt.Errorf("error marshalling MIG state to json: %v", err) } - err = os.WriteFile(f.CheckpointFile, []byte(j), 0666) + checkpointFile, err := os.Create(f.CheckpointFile) if err != nil { + return fmt.Errorf("error creating checkpoint file: %w", err) + } + defer checkpointFile.Close() + if _, err := checkpointFile.Write(j); err != nil { return fmt.Errorf("error writing checkpoint file: %v", err) } From 3cba2c1ea887afb800d4865b8d77dac94e9f9762 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Sun, 28 Jan 2024 14:21:19 +0100 Subject: [PATCH 10/12] Fix implicit memory aliasing in loop Signed-off-by: Evan Lezar --- cmd/nvidia-mig-parted/assert/assert.go | 3 ++- pkg/mig/state/state.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/nvidia-mig-parted/assert/assert.go b/cmd/nvidia-mig-parted/assert/assert.go index 34276fa2..d480fdf9 100644 --- a/cmd/nvidia-mig-parted/assert/assert.go +++ b/cmd/nvidia-mig-parted/assert/assert.go @@ -234,7 +234,8 @@ func WalkSelectedMigConfigForEachGPU(migConfig v1.MigConfigSpecSlice, f func(*v1 log.Debugf(" GPU %v: %v", i, deviceID) - err = f(&mc, i, deviceID) + migConfigSpec := mc + err = f(&migConfigSpec, i, deviceID) if err != nil { return err } diff --git a/pkg/mig/state/state.go b/pkg/mig/state/state.go index 1426ce15..b485be3c 100644 --- a/pkg/mig/state/state.go +++ b/pkg/mig/state/state.go @@ -216,7 +216,8 @@ func (m *migStateManager) RestoreConfig(state *types.MigState) error { return fmt.Errorf("error getting GPU instance profile info for '%v': %v", giState.ProfileID, ret) } - gi, ret := device.CreateGpuInstanceWithPlacement(&giProfileInfo, &giState.Placement) + placement := giState.Placement + gi, ret := device.CreateGpuInstanceWithPlacement(&giProfileInfo, &placement) if ret.Value() != nvml.SUCCESS { return fmt.Errorf("error creating GPU instance for '%v': %v", giState.ProfileID, ret) } From 0513cf4f1639fec0bfe102fd6b4e0de331eac340 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Sun, 28 Jan 2024 14:24:25 +0100 Subject: [PATCH 11/12] Add nolint for gosec G204 Signed-off-by: Evan Lezar --- api/hooks/v1/hooks.go | 2 +- cmd/nvidia-mig-parted/util/device.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/hooks/v1/hooks.go b/api/hooks/v1/hooks.go index c09f1754..e9e74df4 100644 --- a/api/hooks/v1/hooks.go +++ b/api/hooks/v1/hooks.go @@ -66,7 +66,7 @@ func (h HooksMap) Run(name string, envs EnvsMap, output bool) error { // It injects the environment variables associated with the provided EnvMap, // and optionally prints the output for each hook to stdout and stderr. func (h *HookSpec) Run(envs EnvsMap, output bool) error { - cmd := exec.Command(h.Command, h.Args...) + cmd := exec.Command(h.Command, h.Args...) //nolint:gosec cmd.Env = h.Envs.Combine(envs).Format() cmd.Dir = h.Workdir if output { diff --git a/cmd/nvidia-mig-parted/util/device.go b/cmd/nvidia-mig-parted/util/device.go index 5b835bf8..0e5eb3d8 100644 --- a/cmd/nvidia-mig-parted/util/device.go +++ b/cmd/nvidia-mig-parted/util/device.go @@ -150,7 +150,7 @@ func nvmlResetAllGPUs() (string, error) { return "No GPUs to reset...", nil } - cmd := exec.Command("nvidia-smi", "-r", "-i", strings.Join(pciBusIDs, ",")) + cmd := exec.Command("nvidia-smi", "-r", "-i", strings.Join(pciBusIDs, ",")) //nolint:gosec output, err := cmd.CombinedOutput() return string(output), err } From 0c7baea6878def1e0c9c6682994527859113d7e3 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Sun, 28 Jan 2024 15:54:27 +0100 Subject: [PATCH 12/12] Remove unneeded gitlab pipeline var Signed-off-by: Evan Lezar --- .common-ci.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.common-ci.yml b/.common-ci.yml index 910e5c3e..c6e1faf4 100644 --- a/.common-ci.yml +++ b/.common-ci.yml @@ -19,7 +19,6 @@ default: variables: GIT_SUBMODULE_STRATEGY: recursive - BUILDIMAGE: "${CI_REGISTRY_IMAGE}/build:${CI_COMMIT_SHORT_SHA}" BUILD_MULTI_ARCH_IMAGES: "true" stages: