Skip to content

Commit

Permalink
Add golanci-lint config file and address lints
Browse files Browse the repository at this point in the history
Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>
  • Loading branch information
ArangoGutierrez committed Mar 13, 2024
1 parent 63b3ea6 commit 1de4eb7
Show file tree
Hide file tree
Showing 10 changed files with 67 additions and 23 deletions.
2 changes: 0 additions & 2 deletions .github/workflows/golang.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ jobs:
version: latest
args: -v --timeout 5m
skip-cache: true
- name: Check golang modules
run: make check-vendor
test:
name: Unit test
runs-on: ubuntu-latest
Expand Down
14 changes: 9 additions & 5 deletions .github/workflows/images.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,17 @@ jobs:
REPO_FULL_NAME="${{ github.event.pull_request.head.repo.full_name }}"
echo "${REPO_FULL_NAME}"
echo "LABEL_IMAGE_SOURCE=https://github.com/${REPO_FULL_NAME}" >> $GITHUB_ENV
GENERATE_ARTIFACTS="false"
if [[ "${{ github.actor }}" == "dependabot[bot]" ]]; then
echo "PUSH_ON_BUILD=false" >> $GITHUB_ENV
else
echo "PUSH_ON_BUILD=true" >> $GITHUB_ENV
GENERATE_ARTIFACTS="false"
elif [[ "${{ github.event_name }}" == "pull_request" && "${{ github.event.pull_request.head.repo.full_name }}" == "${{ github.repository }}" ]]; then
GENERATE_ARTIFACTS="true"
elif [[ "${{ github.event_name }}" == "push" ]]; then
GENERATE_ARTIFACTS="true"
fi
echo "PUSH_ON_BUILD=${GENERATE_ARTIFACTS}" >> $GITHUB_ENV
echo "BUILD_MULTI_ARCH_IMAGES=${GENERATE_ARTIFACTS}" >> $GITHUB_ENV
- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v3
Expand All @@ -69,8 +75,6 @@ jobs:
env:
IMAGE_NAME: ghcr.io/${LOWERCASE_REPO_OWNER}/vgpu-device-manager
VERSION: ${COMMIT_SHORT_SHA}
# TODO: For now we only build single-arch images to speed up development.
BUILD_MULTI_ARCH_IMAGES: "false"
run: |
echo "${VERSION}"
make -f deployments/container/Makefile build-${{ matrix.dist }}
35 changes: 35 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
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/vgpu-device-manager

issues:
exclude-rules:
# We disable the memory aliasing checks in tests
- path: ".*_test.go"
linters:
- gosec
text: "G601: Implicit memory aliasing in for loop"
# We create world-readable files in tests.
- path: ".*_test.go"
linters:
- gosec
text: "G306: Expect WriteFile permissions to be 0600 or less"
7 changes: 3 additions & 4 deletions api/spec/v1/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,22 +48,21 @@ func (vs *VGPUConfigSpec) MatchesDeviceFilter(deviceID types.DeviceID) bool {

// MatchesAllDevices checks a 'VGPUConfigSpec' to see if it matches on 'all' devices.
func (vs *VGPUConfigSpec) MatchesAllDevices() bool {
switch devices := vs.Devices.(type) {
case string:
if devices, ok := vs.Devices.(string); ok {
return devices == "all"
}
return false
}

// MatchesDevices checks a 'VGPUConfigSpec' to see if it matches on a device at the specified 'index'.
func (vs *VGPUConfigSpec) MatchesDevices(index int) bool {
switch devices := vs.Devices.(type) {
case []int:
if devices, ok := vs.Devices.([]int); ok {
for _, d := range devices {
if index == d {
return true
}
}
}

return vs.MatchesAllDevices()
}
3 changes: 2 additions & 1 deletion api/spec/v1/spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@
package v1

import (
"testing"

"github.com/stretchr/testify/require"
"sigs.k8s.io/yaml"
"testing"
)

func TestSpec(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion cmd/nvidia-vgpu-dm/apply/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (c *Context) ApplyVGPUConfig() error {
func applyWrapper(c *cli.Context, f *Flags) error {
err := CheckFlags(f)
if err != nil {
cli.ShowSubcommandHelp(c)
_ = cli.ShowSubcommandHelp(c)
return err
}

Expand Down
3 changes: 2 additions & 1 deletion cmd/nvidia-vgpu-dm/assert/assert.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,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
}

Expand Down Expand Up @@ -217,6 +217,7 @@ func WalkSelectedVGPUConfigForEachGPU(vgpuConfig v1.VGPUConfigSpecSlice, f func(

log.Debugf(" GPU %v: %v", i, deviceID)

// nolint: gosec
err = f(&vc, i, deviceID)
if err != nil {
return err
Expand Down
19 changes: 12 additions & 7 deletions deployments/container/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,24 @@ package main

import (
"fmt"
"os"
"os/exec"

log "github.com/sirupsen/logrus"
cli "github.com/urfave/cli/v2"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/tools/clientcmd"
"os"
"os/exec"

"context"
"sync"
"time"

corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/util/wait"
"sync"
"time"
)

const (
Expand Down Expand Up @@ -217,7 +219,7 @@ func start(c *cli.Context) error {
log.Errorf("ERROR: %v", err)
continue
}
log.Infof("Successfuly updated to vGPU config: %s", value)
log.Infof("Successfully updated to vGPU config: %s", value)
}
}

Expand Down Expand Up @@ -251,13 +253,16 @@ func continuouslySyncVGPUConfigChanges(clientset *kubernetes.Clientset, vGPUConf
}

func updateConfig(clientset *kubernetes.Clientset, selectedConfig string) error {
defer setVGPUConfigStateLabel(clientset)
defer func() {
_ = setVGPUConfigStateLabel(clientset)
}()

vGPUConfigState = "failed"

log.Info("Asserting that the requested configuration is present in the configuration file")
err := assertValidConfig(selectedConfig)
if err != nil {
return fmt.Errorf("Unable to validate the selected vGPU configuration")
return fmt.Errorf("unable to validate the selected vGPU configuration")
}

log.Info("Checking if the selected vGPU device configuration is currently applied or not")
Expand Down
3 changes: 2 additions & 1 deletion pkg/types/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@
package types

import (
"github.com/stretchr/testify/require"
"testing"

"github.com/stretchr/testify/require"
)

func TestParseVGPUType(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/types/vgpu_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (v VGPUConfig) AssertValid() error {
return fmt.Errorf("cannot mix time-sliced and MIG-backed vGPU devices on the same GPU")
}
migBacked = true
} else {
} else if vgpuType.G <= 0 {
if idx > 0 && migBacked {
return fmt.Errorf("cannot mix time-sliced and MIG-backed vGPU devices on the same GPU")
}
Expand Down

0 comments on commit 1de4eb7

Please sign in to comment.