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: 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..20e4d944 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,30 @@ +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: + # 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" 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 \ diff --git a/api/hooks/v1/hooks.go b/api/hooks/v1/hooks.go index 3e109e2a..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 { @@ -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/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/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 { diff --git a/api/spec/v1/spec_test.go b/api/spec/v1/spec_test.go index 45dffd86..2680c3ae 100644 --- a/api/spec/v1/spec_test.go +++ b/api/spec/v1/spec_test.go @@ -19,16 +19,17 @@ 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) { 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..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" @@ -275,7 +276,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) } } diff --git a/cmd/nvidia-mig-parted/apply/apply.go b/cmd/nvidia-mig-parted/apply/apply.go index fffaeb40..b0558d97 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" ) @@ -183,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 404f21a1..d480fdf9 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" ) @@ -103,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 } @@ -233,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/cmd/nvidia-mig-parted/checkpoint/checkpoint.go b/cmd/nvidia-mig-parted/checkpoint/checkpoint.go index bd421169..ab5529c3 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() @@ -81,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 } @@ -107,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) } diff --git a/cmd/nvidia-mig-parted/export/export.go b/cmd/nvidia-mig-parted/export/export.go index aece81fb..eb033a6d 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" ) @@ -91,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 } @@ -131,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/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..82f88344 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() @@ -154,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 } 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 } 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/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} } 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..27ceb047 100644 --- a/pkg/mig/config/config_test.go +++ b/pkg/mig/config/config_test.go @@ -20,12 +20,12 @@ import ( "fmt" "math/rand" "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 { @@ -142,7 +142,6 @@ func TestIteratePermutationsUntilSuccess(t *testing.T) { return perms } - rand.Seed(time.Now().UnixNano()) mcg := NewA100_SXM4_40GB_MigConfigGroup() type testCase struct { 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..b485be3c 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. @@ -215,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) } 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 {