diff --git a/.golangci.yml b/.golangci.yml index f78ba73c..db096ed0 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -22,6 +22,7 @@ linters: - gofumpt - tenv - durationcheck + - staticcheck issues: exclude-use-default: false diff --git a/cmd/kubectl-directpv/flags.go b/cmd/kubectl-directpv/flags.go index ffede0a2..fe684969 100644 --- a/cmd/kubectl-directpv/flags.go +++ b/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], "=") diff --git a/go.mod b/go.mod index 1bc4a78c..c811b092 100644 --- a/go.mod +++ b/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 diff --git a/pkg/admin/install.go b/pkg/admin/install.go index 74188d5a..7ca80973 100644 --- a/pkg/admin/install.go +++ b/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 diff --git a/pkg/apis/directpv.min.io/types/types.go b/pkg/apis/directpv.min.io/types/types.go index 9f031b78..4fbd88ef 100644 --- a/pkg/apis/directpv.min.io/types/types.go +++ b/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: diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index c8a84e9b..f50ae4f5 100644 --- a/pkg/controller/controller_test.go +++ b/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], diff --git a/pkg/k8s/fake.go b/pkg/k8s/fake.go index c019787e..cd2c9432 100644 --- a/pkg/k8s/fake.go +++ b/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, diff --git a/pkg/legacy/client/client.go b/pkg/legacy/client/client.go index 2629ed0c..8e02deaa 100644 --- a/pkg/legacy/client/client.go +++ b/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", diff --git a/pkg/metrics/collector_test.go b/pkg/metrics/collector_test.go index b30a4295..4ab88418 100644 --- a/pkg/metrics/collector_test.go +++ b/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") + } }