Skip to content

Commit

Permalink
Add staticcheck linter
Browse files Browse the repository at this point in the history
Signed-off-by: Bala.FA <bala@minio.io>
  • Loading branch information
balamurugana committed Nov 27, 2024
1 parent 5b49369 commit c073f97
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 49 deletions.
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ linters:
- gofumpt
- tenv
- durationcheck
- staticcheck

issues:
exclude-use-default: false
Expand Down
31 changes: 15 additions & 16 deletions cmd/kubectl-directpv/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"strings"

"github.com/minio/directpv/pkg/apis/directpv.min.io/types"
directpvtypes "github.com/minio/directpv/pkg/apis/directpv.min.io/types"
"github.com/minio/directpv/pkg/ellipsis"
"github.com/minio/directpv/pkg/utils"
"github.com/spf13/cobra"
Expand All @@ -31,16 +30,16 @@ import (
var errInvalidLabel = errors.New("invalid label")

var driveStatusValues = []string{
strings.ToLower(string(directpvtypes.DriveStatusError)),
strings.ToLower(string(directpvtypes.DriveStatusLost)),
strings.ToLower(string(directpvtypes.DriveStatusMoving)),
strings.ToLower(string(directpvtypes.DriveStatusReady)),
strings.ToLower(string(directpvtypes.DriveStatusRemoved)),
strings.ToLower(string(types.DriveStatusError)),
strings.ToLower(string(types.DriveStatusLost)),
strings.ToLower(string(types.DriveStatusMoving)),
strings.ToLower(string(types.DriveStatusReady)),
strings.ToLower(string(types.DriveStatusRemoved)),
}

var volumeStatusValues = []string{
strings.ToLower(string(directpvtypes.VolumeStatusPending)),
strings.ToLower(string(directpvtypes.VolumeStatusReady)),
strings.ToLower(string(types.VolumeStatusPending)),
strings.ToLower(string(types.VolumeStatusReady)),
}

var (
Expand Down Expand Up @@ -132,10 +131,10 @@ func setFlagOpts(cmd *cobra.Command) {
var (
wideOutput bool

driveStatusSelectors []directpvtypes.DriveStatus
driveIDSelectors []directpvtypes.DriveID
volumeStatusSelectors []directpvtypes.VolumeStatus
labelSelectors map[directpvtypes.LabelKey]directpvtypes.LabelValue
driveStatusSelectors []types.DriveStatus
driveIDSelectors []types.DriveID
volumeStatusSelectors []types.VolumeStatus
labelSelectors map[types.LabelKey]types.LabelValue

dryRunPrinter func(interface{})
)
Expand Down Expand Up @@ -181,7 +180,7 @@ func validateDriveNameArgs() error {
func validateDriveStatusArgs() error {
for i := range driveStatusArgs {
driveStatusArgs[i] = strings.TrimSpace(driveStatusArgs[i])
status, err := directpvtypes.ToDriveStatus(driveStatusArgs[i])
status, err := types.ToDriveStatus(driveStatusArgs[i])
if err != nil {
return err
}
Expand All @@ -199,7 +198,7 @@ func validateDriveIDArgs() error {
if !utils.IsUUID(driveIDArgs[i]) {
return fmt.Errorf("invalid drive ID %v", driveIDArgs[i])
}
driveIDSelectors = append(driveIDSelectors, directpvtypes.DriveID(driveIDArgs[i]))
driveIDSelectors = append(driveIDSelectors, types.DriveID(driveIDArgs[i]))
}
return nil
}
Expand Down Expand Up @@ -255,7 +254,7 @@ func validateVolumeNameArgs() error {
func validateVolumeStatusArgs() error {
for i := range volumeStatusArgs {
volumeStatusArgs[i] = strings.TrimSpace(volumeStatusArgs[i])
status, err := directpvtypes.ToVolumeStatus(volumeStatusArgs[i])
status, err := types.ToVolumeStatus(volumeStatusArgs[i])
if err != nil {
return err
}
Expand All @@ -266,7 +265,7 @@ func validateVolumeStatusArgs() error {

func validateLabelArgs() error {
if labelSelectors == nil {
labelSelectors = make(map[directpvtypes.LabelKey]directpvtypes.LabelValue)
labelSelectors = make(map[types.LabelKey]types.LabelValue)
}
for i := range labelArgs {
tokens := strings.Split(labelArgs[i], "=")
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ require (
github.com/prometheus/client_model v0.6.1
github.com/spf13/cobra v1.8.1
github.com/spf13/viper v1.19.0
golang.org/x/text v0.20.0
golang.org/x/time v0.8.0
google.golang.org/grpc v1.68.0
gopkg.in/yaml.v3 v3.0.1
Expand Down Expand Up @@ -96,7 +97,6 @@ require (
golang.org/x/sync v0.9.0 // indirect
golang.org/x/sys v0.27.0 // indirect
golang.org/x/term v0.26.0 // indirect
golang.org/x/text v0.20.0 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20241118233622-e639e219e697 // indirect
google.golang.org/protobuf v1.35.2 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
Expand Down
18 changes: 8 additions & 10 deletions pkg/admin/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,12 @@ func (client Client) isLegacyEnabled(ctx context.Context, args InstallArgs) bool
).
IgnoreNotFound(true).
List(ctx)
for result := range resultCh {
if result.Err != nil {
utils.Eprintf(args.Quiet, true, "unable to get volumes; %v", result.Err)
break
if result, ok := <-resultCh; ok {
if result.Err == nil {
return true
}

return true
utils.Eprintf(args.Quiet, true, "unable to get volumes; %v", result.Err)
}

legacyClient, err := legacyclient.NewClient(client.K8s())
Expand All @@ -177,13 +176,12 @@ func (client Client) isLegacyEnabled(ctx context.Context, args InstallArgs) bool
return false
}

for result := range legacyClient.ListVolumes(ctx) {
if result.Err != nil {
utils.Eprintf(args.Quiet, true, "unable to get legacy volumes; %v", result.Err)
break
if result, ok := <-legacyClient.ListVolumes(ctx); ok {
if result.Err == nil {
return true
}

return true
utils.Eprintf(args.Quiet, true, "unable to get legacy volumes; %v", result.Err)
}

return false
Expand Down
10 changes: 6 additions & 4 deletions pkg/apis/directpv.min.io/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ package types

import (
"fmt"
"strings"

"golang.org/x/text/cases"
"golang.org/x/text/language"
)

// DriveName is drive name type.
Expand Down Expand Up @@ -55,7 +57,7 @@ const (

// ToDriveStatus converts string value to DriveStatus.
func ToDriveStatus(value string) (status DriveStatus, err error) {
status = DriveStatus(strings.Title(value))
status = DriveStatus(cases.Title(language.Und).String(value))
switch status {
case DriveStatusReady, DriveStatusLost, DriveStatusError, DriveStatusRemoved, DriveStatusMoving:
return status, nil
Expand All @@ -76,7 +78,7 @@ const (

// ToVolumeStatus converts string value to VolumeStatus.
func ToVolumeStatus(value string) (status VolumeStatus, err error) {
status = VolumeStatus(strings.Title(value))
status = VolumeStatus(cases.Title(language.Und).String(value))
switch status {
case VolumeStatusReady, VolumeStatusPending:
return status, nil
Expand All @@ -100,7 +102,7 @@ const (
// StringsToAccessTiers converts strings to access tiers.
func StringsToAccessTiers(values ...string) (accessTiers []AccessTier, err error) {
for _, value := range values {
switch at := AccessTier(strings.Title(value)); at {
switch at := AccessTier(cases.Title(language.Und).String(value)); at {
case AccessTierDefault, AccessTierWarm, AccessTierHot, AccessTierCold:
accessTiers = append(accessTiers, at)
default:
Expand Down
16 changes: 8 additions & 8 deletions pkg/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"testing"
"time"

"github.com/dustin/go-humanize"
directpvtypes "github.com/minio/directpv/pkg/apis/directpv.min.io/types"
"github.com/minio/directpv/pkg/client"
clientsetfake "github.com/minio/directpv/pkg/clientset/fake"
Expand All @@ -37,7 +38,6 @@ import (

const (
nodeID = "test-node"
GiB = 1024 * 1024 * 1024
)

func init() {
Expand Down Expand Up @@ -133,8 +133,8 @@ func TestController(t *testing.T) {
}

volumes := []*pkgtypes.Volume{
newVolume("test-volume-1", "1", 2*GiB),
newVolume("test-volume-2", "2", 3*GiB),
newVolume("test-volume-1", "1", 2*humanize.GiByte),
newVolume("test-volume-2", "2", 3*humanize.GiByte),
}

volumesMap := map[string]*pkgtypes.Volume{
Expand All @@ -151,8 +151,8 @@ func TestController(t *testing.T) {
<-doneCh

volumes = []*pkgtypes.Volume{
newVolume("test-volume-1", "1", 4*GiB),
newVolume("test-volume-2", "1", 6*GiB),
newVolume("test-volume-1", "1", 4*humanize.GiByte),
newVolume("test-volume-2", "1", 6*humanize.GiByte),
}
volumesMap = map[string]*pkgtypes.Volume{
"test-volume-1": volumes[0],
Expand All @@ -176,7 +176,7 @@ func TestController(t *testing.T) {

// Retry on error
volumes = []*pkgtypes.Volume{
newVolume("test-volume-1", "1", 4*GiB),
newVolume("test-volume-1", "1", 4*humanize.GiByte),
}
volumesMap = map[string]*pkgtypes.Volume{
"test-volume-1": volumes[0],
Expand Down Expand Up @@ -217,8 +217,8 @@ func TestController(t *testing.T) {

// Delete
volumes = []*pkgtypes.Volume{
newVolume("test-volume-1", "1", 4*GiB),
newVolume("test-volume-2", "1", 6*GiB),
newVolume("test-volume-1", "1", 4*humanize.GiByte),
newVolume("test-volume-2", "1", 6*humanize.GiByte),
}
volumesMap = map[string]*pkgtypes.Volume{
"test-volume-1": volumes[0],
Expand Down
3 changes: 1 addition & 2 deletions pkg/k8s/fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ func (fd *FakeDiscovery) ServerGroupsAndResources() ([]*metav1.APIGroup, []*meta

// FakeInit initializes fake clients.
func FakeInit() {
var kubeClient kubernetes.Interface
kubeClient = kubernetesfake.NewSimpleClientset()
var kubeClient kubernetes.Interface = kubernetesfake.NewSimpleClientset()
crdClient := &apiextensionsv1fake.FakeCustomResourceDefinitions{
Fake: &apiextensionsv1fake.FakeApiextensionsV1{
Fake: &kubeClient.(*kubernetesfake.Clientset).Fake,
Expand Down
9 changes: 4 additions & 5 deletions pkg/legacy/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (

"github.com/minio/directpv/pkg/k8s"
directcsi "github.com/minio/directpv/pkg/legacy/apis/direct.csi.min.io/v1beta5"
directv1beta5 "github.com/minio/directpv/pkg/legacy/apis/direct.csi.min.io/v1beta5"
typeddirectcsi "github.com/minio/directpv/pkg/legacy/clientset/typed/direct.csi.min.io/v1beta5"
"github.com/minio/directpv/pkg/utils"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -99,7 +98,7 @@ func GetClient() *Client {

// RemoveAllDrives removes legacy drive CRDs.
func (client Client) RemoveAllDrives(ctx context.Context, backupFile string) (backupCreated bool, err error) {
var drives []directv1beta5.DirectCSIDrive
var drives []directcsi.DirectCSIDrive
for result := range client.ListDrives(ctx) {
if result.Err != nil {
return false, fmt.Errorf("unable to get legacy drives; %w", result.Err)
Expand All @@ -110,7 +109,7 @@ func (client Client) RemoveAllDrives(ctx context.Context, backupFile string) (ba
return false, nil
}

data, err := utils.ToYAML(directv1beta5.DirectCSIDriveList{
data, err := utils.ToYAML(directcsi.DirectCSIDriveList{
TypeMeta: metav1.TypeMeta{
Kind: "List",
APIVersion: "v1",
Expand Down Expand Up @@ -140,7 +139,7 @@ func (client Client) RemoveAllDrives(ctx context.Context, backupFile string) (ba

// RemoveAllVolumes removes legacy volume CRDs.
func (client Client) RemoveAllVolumes(ctx context.Context, backupFile string) (backupCreated bool, err error) {
var volumes []directv1beta5.DirectCSIVolume
var volumes []directcsi.DirectCSIVolume
for result := range client.ListVolumes(ctx) {
if result.Err != nil {
return false, fmt.Errorf("unable to get legacy volumes; %w", result.Err)
Expand All @@ -151,7 +150,7 @@ func (client Client) RemoveAllVolumes(ctx context.Context, backupFile string) (b
return false, nil
}

data, err := utils.ToYAML(directv1beta5.DirectCSIVolumeList{
data, err := utils.ToYAML(directcsi.DirectCSIVolumeList{
TypeMeta: metav1.TypeMeta{
Kind: "List",
APIVersion: "v1",
Expand Down
22 changes: 19 additions & 3 deletions pkg/metrics/collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ func TestVolumeStatsEmitter(t *testing.T) {
noOfMetricsExposedPerVolume := 2
expectedNoOfMetrics := len(testObjects) * noOfMetricsExposedPerVolume
noOfMetricsReceived := 0
var failed bool
wg.Add(1)
go func() {
defer wg.Done()
Expand All @@ -126,7 +127,9 @@ func TestVolumeStatsEmitter(t *testing.T) {
}
metricOut := clientmodelgo.Metric{}
if err := metric.Write(&metricOut); err != nil {
(*t).Fatal(err)
t.Errorf("metric write failed; %v", err)
failed = true
return
}
volumeName := getVolumeNameFromLabelPair(metricOut.GetLabel())
mt := metricType(getFQNameFromDesc(metric.Desc().String()))
Expand All @@ -136,7 +139,9 @@ func TestVolumeStatsEmitter(t *testing.T) {
TypeMeta: types.NewVolumeTypeMeta(),
})
if gErr != nil {
(*t).Fatalf("[%s] Volume (%s) not found. Error: %v", volumeName, volumeName, gErr)
t.Errorf("[%s] Volume (%s) not found. Error: %v", volumeName, volumeName, gErr)
failed = true
return
}
if volObj.Status.UsedCapacity != int64(*metricOut.Gauge.Value) {
t.Errorf("Expected Used capacity: %v But got %v", volObj.Status.UsedCapacity, *metricOut.Gauge.Value)
Expand All @@ -146,7 +151,9 @@ func TestVolumeStatsEmitter(t *testing.T) {
TypeMeta: types.NewVolumeTypeMeta(),
})
if gErr != nil {
(*t).Fatalf("[%s] Volume (%s) not found. Error: %v", volumeName, volumeName, gErr)
t.Errorf("[%s] Volume (%s) not found. Error: %v", volumeName, volumeName, gErr)
failed = true
return
}
if volObj.Status.TotalCapacity != int64(*metricOut.Gauge.Value) {
t.Errorf("Expected Total capacity: %v But got %v", volObj.Status.TotalCapacity, *metricOut.Gauge.Value)
Expand All @@ -163,7 +170,16 @@ func TestVolumeStatsEmitter(t *testing.T) {
}()

fmc.publishVolumeStats(ctx, &volumes[0], metricChan)
if failed {
t.Fatalf("publish volume stats failed for %v", volumes[0].Name)
}
fmc.publishVolumeStats(ctx, &volumes[1], metricChan)
if failed {
t.Fatalf("publish volume stats failed for %v", volumes[1].Name)
}

wg.Wait()
if failed {
t.Fatalf("test failed")
}
}

0 comments on commit c073f97

Please sign in to comment.