Skip to content

Commit e0d9577

Browse files
authored
Add staticcheck linter (#974)
Signed-off-by: Bala.FA <bala@minio.io>
1 parent 8906683 commit e0d9577

File tree

10 files changed

+69
-60
lines changed

10 files changed

+69
-60
lines changed

.golangci.yml

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ linters:
2222
- gofumpt
2323
- tenv
2424
- durationcheck
25+
- staticcheck
2526

2627
issues:
2728
exclude-use-default: false

cmd/kubectl-directpv/flags.go

+15-16
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"strings"
2323

2424
"github.com/minio/directpv/pkg/apis/directpv.min.io/types"
25-
directpvtypes "github.com/minio/directpv/pkg/apis/directpv.min.io/types"
2625
"github.com/minio/directpv/pkg/ellipsis"
2726
"github.com/minio/directpv/pkg/utils"
2827
"github.com/spf13/cobra"
@@ -31,16 +30,16 @@ import (
3130
var errInvalidLabel = errors.New("invalid label")
3231

3332
var driveStatusValues = []string{
34-
strings.ToLower(string(directpvtypes.DriveStatusError)),
35-
strings.ToLower(string(directpvtypes.DriveStatusLost)),
36-
strings.ToLower(string(directpvtypes.DriveStatusMoving)),
37-
strings.ToLower(string(directpvtypes.DriveStatusReady)),
38-
strings.ToLower(string(directpvtypes.DriveStatusRemoved)),
33+
strings.ToLower(string(types.DriveStatusError)),
34+
strings.ToLower(string(types.DriveStatusLost)),
35+
strings.ToLower(string(types.DriveStatusMoving)),
36+
strings.ToLower(string(types.DriveStatusReady)),
37+
strings.ToLower(string(types.DriveStatusRemoved)),
3938
}
4039

4140
var volumeStatusValues = []string{
42-
strings.ToLower(string(directpvtypes.VolumeStatusPending)),
43-
strings.ToLower(string(directpvtypes.VolumeStatusReady)),
41+
strings.ToLower(string(types.VolumeStatusPending)),
42+
strings.ToLower(string(types.VolumeStatusReady)),
4443
}
4544

4645
var (
@@ -132,10 +131,10 @@ func setFlagOpts(cmd *cobra.Command) {
132131
var (
133132
wideOutput bool
134133

135-
driveStatusSelectors []directpvtypes.DriveStatus
136-
driveIDSelectors []directpvtypes.DriveID
137-
volumeStatusSelectors []directpvtypes.VolumeStatus
138-
labelSelectors map[directpvtypes.LabelKey]directpvtypes.LabelValue
134+
driveStatusSelectors []types.DriveStatus
135+
driveIDSelectors []types.DriveID
136+
volumeStatusSelectors []types.VolumeStatus
137+
labelSelectors map[types.LabelKey]types.LabelValue
139138

140139
dryRunPrinter func(interface{})
141140
)
@@ -181,7 +180,7 @@ func validateDriveNameArgs() error {
181180
func validateDriveStatusArgs() error {
182181
for i := range driveStatusArgs {
183182
driveStatusArgs[i] = strings.TrimSpace(driveStatusArgs[i])
184-
status, err := directpvtypes.ToDriveStatus(driveStatusArgs[i])
183+
status, err := types.ToDriveStatus(driveStatusArgs[i])
185184
if err != nil {
186185
return err
187186
}
@@ -199,7 +198,7 @@ func validateDriveIDArgs() error {
199198
if !utils.IsUUID(driveIDArgs[i]) {
200199
return fmt.Errorf("invalid drive ID %v", driveIDArgs[i])
201200
}
202-
driveIDSelectors = append(driveIDSelectors, directpvtypes.DriveID(driveIDArgs[i]))
201+
driveIDSelectors = append(driveIDSelectors, types.DriveID(driveIDArgs[i]))
203202
}
204203
return nil
205204
}
@@ -255,7 +254,7 @@ func validateVolumeNameArgs() error {
255254
func validateVolumeStatusArgs() error {
256255
for i := range volumeStatusArgs {
257256
volumeStatusArgs[i] = strings.TrimSpace(volumeStatusArgs[i])
258-
status, err := directpvtypes.ToVolumeStatus(volumeStatusArgs[i])
257+
status, err := types.ToVolumeStatus(volumeStatusArgs[i])
259258
if err != nil {
260259
return err
261260
}
@@ -266,7 +265,7 @@ func validateVolumeStatusArgs() error {
266265

267266
func validateLabelArgs() error {
268267
if labelSelectors == nil {
269-
labelSelectors = make(map[directpvtypes.LabelKey]directpvtypes.LabelValue)
268+
labelSelectors = make(map[types.LabelKey]types.LabelValue)
270269
}
271270
for i := range labelArgs {
272271
tokens := strings.Split(labelArgs[i], "=")

go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ require (
2121
github.com/prometheus/client_model v0.6.1
2222
github.com/spf13/cobra v1.8.1
2323
github.com/spf13/viper v1.19.0
24+
golang.org/x/text v0.20.0
2425
golang.org/x/time v0.8.0
2526
google.golang.org/grpc v1.68.0
2627
gopkg.in/yaml.v3 v3.0.1
@@ -97,7 +98,6 @@ require (
9798
golang.org/x/sync v0.9.0 // indirect
9899
golang.org/x/sys v0.27.0 // indirect
99100
golang.org/x/term v0.26.0 // indirect
100-
golang.org/x/text v0.20.0 // indirect
101101
google.golang.org/genproto/googleapis/rpc v0.0.0-20241118233622-e639e219e697 // indirect
102102
google.golang.org/protobuf v1.35.2 // indirect
103103
gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect

pkg/admin/install.go

+8-10
Original file line numberDiff line numberDiff line change
@@ -162,13 +162,12 @@ func (client Client) isLegacyEnabled(ctx context.Context, args InstallArgs) bool
162162
).
163163
IgnoreNotFound(true).
164164
List(ctx)
165-
for result := range resultCh {
166-
if result.Err != nil {
167-
utils.Eprintf(args.Quiet, true, "unable to get volumes; %v", result.Err)
168-
break
165+
if result, ok := <-resultCh; ok {
166+
if result.Err == nil {
167+
return true
169168
}
170169

171-
return true
170+
utils.Eprintf(args.Quiet, true, "unable to get volumes; %v", result.Err)
172171
}
173172

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

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

186-
return true
184+
utils.Eprintf(args.Quiet, true, "unable to get legacy volumes; %v", result.Err)
187185
}
188186

189187
return false

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

+6-4
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ package types
1818

1919
import (
2020
"fmt"
21-
"strings"
21+
22+
"golang.org/x/text/cases"
23+
"golang.org/x/text/language"
2224
)
2325

2426
// DriveName is drive name type.
@@ -55,7 +57,7 @@ const (
5557

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

7779
// ToVolumeStatus converts string value to VolumeStatus.
7880
func ToVolumeStatus(value string) (status VolumeStatus, err error) {
79-
status = VolumeStatus(strings.Title(value))
81+
status = VolumeStatus(cases.Title(language.Und).String(value))
8082
switch status {
8183
case VolumeStatusReady, VolumeStatusPending:
8284
return status, nil
@@ -100,7 +102,7 @@ const (
100102
// StringsToAccessTiers converts strings to access tiers.
101103
func StringsToAccessTiers(values ...string) (accessTiers []AccessTier, err error) {
102104
for _, value := range values {
103-
switch at := AccessTier(strings.Title(value)); at {
105+
switch at := AccessTier(cases.Title(language.Und).String(value)); at {
104106
case AccessTierDefault, AccessTierWarm, AccessTierHot, AccessTierCold:
105107
accessTiers = append(accessTiers, at)
106108
default:

pkg/controller/controller.go

+6-11
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,6 @@ import (
3232
"k8s.io/klog/v2"
3333
)
3434

35-
const (
36-
queueBaseDelay = 100 * time.Millisecond
37-
queueMaxDelay = 10 * time.Minute
38-
)
39-
4035
// Event represents a controller event.
4136
type Event struct {
4237
Type EventType
@@ -65,7 +60,7 @@ const (
6560
type Controller struct {
6661
name string
6762
handler EventHandler
68-
queue workqueue.RateLimitingInterface
63+
queue workqueue.TypedRateLimitingInterface[Event]
6964
informer cache.SharedIndexInformer
7065
workerThreads int
7166
// locking
@@ -82,10 +77,10 @@ func New(name string, handler EventHandler, workers int, resyncPeriod time.Durat
8277
cache.Indexers{},
8378
)
8479

85-
queue := workqueue.NewRateLimitingQueue(
86-
workqueue.NewMaxOfRateLimiter(
87-
workqueue.NewItemExponentialFailureRateLimiter(queueBaseDelay, queueMaxDelay),
88-
&workqueue.BucketRateLimiter{Limiter: rate.NewLimiter(rate.Limit(10), 100)},
80+
queue := workqueue.NewTypedRateLimitingQueue(
81+
workqueue.NewTypedMaxOfRateLimiter(
82+
workqueue.NewTypedItemExponentialFailureRateLimiter[Event](100*time.Millisecond, 10*time.Minute),
83+
&workqueue.TypedBucketRateLimiter[Event]{Limiter: rate.NewLimiter(rate.Limit(10), 100)},
8984
),
9085
)
9186

@@ -168,7 +163,7 @@ func (c *Controller) processNextItem(ctx context.Context) bool {
168163

169164
defer c.queue.Done(event)
170165

171-
if err := c.processItem(ctx, event.(Event)); err != nil {
166+
if err := c.processItem(ctx, event); err != nil {
172167
c.queue.AddRateLimited(event)
173168
utilruntime.HandleError(err)
174169
} else {

pkg/controller/controller_test.go

+8-8
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"testing"
2525
"time"
2626

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

3839
const (
3940
nodeID = "test-node"
40-
GiB = 1024 * 1024 * 1024
4141
)
4242

4343
func init() {
@@ -133,8 +133,8 @@ func TestController(t *testing.T) {
133133
}
134134

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

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

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

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

218218
// Delete
219219
volumes = []*pkgtypes.Volume{
220-
newVolume("test-volume-1", "1", 4*GiB),
221-
newVolume("test-volume-2", "1", 6*GiB),
220+
newVolume("test-volume-1", "1", 4*humanize.GiByte),
221+
newVolume("test-volume-2", "1", 6*humanize.GiByte),
222222
}
223223
volumesMap = map[string]*pkgtypes.Volume{
224224
"test-volume-1": volumes[0],

pkg/k8s/fake.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,7 @@ func (fd *FakeDiscovery) ServerGroupsAndResources() ([]*metav1.APIGroup, []*meta
4747

4848
// FakeInit initializes fake clients.
4949
func FakeInit() {
50-
var kubeClient kubernetes.Interface
51-
kubeClient = kubernetesfake.NewSimpleClientset()
50+
var kubeClient kubernetes.Interface = kubernetesfake.NewSimpleClientset()
5251
crdClient := &apiextensionsv1fake.FakeCustomResourceDefinitions{
5352
Fake: &apiextensionsv1fake.FakeApiextensionsV1{
5453
Fake: &kubeClient.(*kubernetesfake.Clientset).Fake,

pkg/legacy/client/client.go

+4-5
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323

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

10099
// RemoveAllDrives removes legacy drive CRDs.
101100
func (client Client) RemoveAllDrives(ctx context.Context, backupFile string) (backupCreated bool, err error) {
102-
var drives []directv1beta5.DirectCSIDrive
101+
var drives []directcsi.DirectCSIDrive
103102
for result := range client.ListDrives(ctx) {
104103
if result.Err != nil {
105104
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
110109
return false, nil
111110
}
112111

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

141140
// RemoveAllVolumes removes legacy volume CRDs.
142141
func (client Client) RemoveAllVolumes(ctx context.Context, backupFile string) (backupCreated bool, err error) {
143-
var volumes []directv1beta5.DirectCSIVolume
142+
var volumes []directcsi.DirectCSIVolume
144143
for result := range client.ListVolumes(ctx) {
145144
if result.Err != nil {
146145
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
151150
return false, nil
152151
}
153152

154-
data, err := utils.ToYAML(directv1beta5.DirectCSIVolumeList{
153+
data, err := utils.ToYAML(directcsi.DirectCSIVolumeList{
155154
TypeMeta: metav1.TypeMeta{
156155
Kind: "List",
157156
APIVersion: "v1",

pkg/metrics/collector_test.go

+19-3
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ func TestVolumeStatsEmitter(t *testing.T) {
112112
noOfMetricsExposedPerVolume := 2
113113
expectedNoOfMetrics := len(testObjects) * noOfMetricsExposedPerVolume
114114
noOfMetricsReceived := 0
115+
var failed bool
115116
wg.Add(1)
116117
go func() {
117118
defer wg.Done()
@@ -126,7 +127,9 @@ func TestVolumeStatsEmitter(t *testing.T) {
126127
}
127128
metricOut := clientmodelgo.Metric{}
128129
if err := metric.Write(&metricOut); err != nil {
129-
(*t).Fatal(err)
130+
t.Errorf("metric write failed; %v", err)
131+
failed = true
132+
return
130133
}
131134
volumeName := getVolumeNameFromLabelPair(metricOut.GetLabel())
132135
mt := metricType(getFQNameFromDesc(metric.Desc().String()))
@@ -136,7 +139,9 @@ func TestVolumeStatsEmitter(t *testing.T) {
136139
TypeMeta: types.NewVolumeTypeMeta(),
137140
})
138141
if gErr != nil {
139-
(*t).Fatalf("[%s] Volume (%s) not found. Error: %v", volumeName, volumeName, gErr)
142+
t.Errorf("[%s] Volume (%s) not found. Error: %v", volumeName, volumeName, gErr)
143+
failed = true
144+
return
140145
}
141146
if volObj.Status.UsedCapacity != int64(*metricOut.Gauge.Value) {
142147
t.Errorf("Expected Used capacity: %v But got %v", volObj.Status.UsedCapacity, *metricOut.Gauge.Value)
@@ -146,7 +151,9 @@ func TestVolumeStatsEmitter(t *testing.T) {
146151
TypeMeta: types.NewVolumeTypeMeta(),
147152
})
148153
if gErr != nil {
149-
(*t).Fatalf("[%s] Volume (%s) not found. Error: %v", volumeName, volumeName, gErr)
154+
t.Errorf("[%s] Volume (%s) not found. Error: %v", volumeName, volumeName, gErr)
155+
failed = true
156+
return
150157
}
151158
if volObj.Status.TotalCapacity != int64(*metricOut.Gauge.Value) {
152159
t.Errorf("Expected Total capacity: %v But got %v", volObj.Status.TotalCapacity, *metricOut.Gauge.Value)
@@ -163,7 +170,16 @@ func TestVolumeStatsEmitter(t *testing.T) {
163170
}()
164171

165172
fmc.publishVolumeStats(ctx, &volumes[0], metricChan)
173+
if failed {
174+
t.Fatalf("publish volume stats failed for %v", volumes[0].Name)
175+
}
166176
fmc.publishVolumeStats(ctx, &volumes[1], metricChan)
177+
if failed {
178+
t.Fatalf("publish volume stats failed for %v", volumes[1].Name)
179+
}
167180

168181
wg.Wait()
182+
if failed {
183+
t.Fatalf("test failed")
184+
}
169185
}

0 commit comments

Comments
 (0)