Add staticcheck linter (#977)

Signed-off-by: Bala.FA <[email protected]>

.golangci.yml

@@ -22,6 +22,7 @@ linters:
     - gofumpt
     - tenv
     - durationcheck
+    - staticcheck
 
 issues:
   exclude-use-default: false

cmd/kubectl-directpv/flags.go

@@ -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"
@@ -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 (
@@ -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{})
 )
@@ -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
 		}
@@ -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
 }
@@ -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
 		}
@@ -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], "=")

go.mod

@@ -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
@@ -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

pkg/admin/install.go

@@ -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())
@@ -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

pkg/apis/directpv.min.io/types/types.go

@@ -18,7 +18,9 @@ package types
 
 import (
 	"fmt"
-	"strings"
+
+	"golang.org/x/text/cases"
+	"golang.org/x/text/language"
 )
 
 // DriveName is drive name type.
@@ -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
@@ -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
@@ -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:

pkg/controller/controller_test.go

@@ -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"
@@ -37,7 +38,6 @@ import (
 
 const (
 	nodeID = "test-node"
-	GiB    = 1024 * 1024 * 1024
 )
 
 func init() {
@@ -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{
@@ -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],
@@ -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],
@@ -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],

pkg/k8s/fake.go

@@ -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,

pkg/legacy/client/client.go

@@ -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"
@@ -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)
@@ -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",
@@ -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)
@@ -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",

pkg/metrics/collector_test.go

@@ -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()
@@ -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()))
@@ -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)
@@ -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)
@@ -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")
+	}
 }