Skip to content

Commit beef916

Browse files
committed
Cleanup small sections of code
1 parent 7bda0c0 commit beef916

File tree

10 files changed

+35
-67
lines changed

10 files changed

+35
-67
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ require (
88
github.com/IBM/platform-services-go-sdk v0.62.2
99
github.com/container-storage-interface/spec v1.9.0
1010
github.com/davecgh/go-spew v1.1.1
11+
github.com/google/go-cmp v0.6.0
1112
github.com/kubernetes-csi/csi-test v2.2.0+incompatible
1213
github.com/onsi/ginkgo/v2 v2.17.1
1314
github.com/onsi/gomega v1.32.0
@@ -66,7 +67,6 @@ require (
6667
github.com/golang/protobuf v1.5.4 // indirect
6768
github.com/google/cel-go v0.17.7 // indirect
6869
github.com/google/gnostic-models v0.6.8 // indirect
69-
github.com/google/go-cmp v0.6.0 // indirect
7070
github.com/google/gofuzz v1.2.0 // indirect
7171
github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1 // indirect
7272
github.com/google/uuid v1.6.0 // indirect

pkg/cloud/metadata.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func (m *Metadata) GetPvmInstanceId() string {
5757
return m.pvmInstanceId
5858
}
5959

60-
// TokenizeProviderID tokenize the provider id into Metadata structure
60+
// TokenizeProviderID tokenizes the provider id into Metadata structure
6161
// ProviderID format: ibmpowervs://<region>/<zone>/<service_instance_id>/<powervs_machine_id>
6262
func TokenizeProviderID(providerID string) (*Metadata, error) {
6363
data := strings.Split(providerID, "/")
@@ -87,13 +87,12 @@ func TokenizeProviderID(providerID string) (*Metadata, error) {
8787

8888
// Get New Metadata Service
8989
func NewMetadataService(k8sAPIClient KubernetesAPIClient, kubeconfig string) (MetadataService, error) {
90-
klog.Infof("retrieving instance data from kubernetes api")
90+
klog.Infof("retrieving instance data from kubernetes API")
9191
clientset, err := k8sAPIClient(kubeconfig)
9292
if err != nil {
9393
klog.Warningf("error creating kubernetes api client: %v", err)
94-
} else {
95-
klog.Infof("kubernetes api is available")
96-
return KubernetesAPIInstanceInfo(clientset)
94+
return nil, fmt.Errorf("an error occured during creation of k8s API client: %w", err)
9795
}
98-
return nil, fmt.Errorf("error getting instance data from ec2 metadata or kubernetes api")
96+
klog.Info("kubernetes API is available")
97+
return KubernetesAPIInstanceInfo(clientset)
9998
}

pkg/cloud/metadata_k8s.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,7 @@ var DefaultKubernetesAPIClient = func(kubeconfig string) (kubernetes.Interface,
3636
return nil, err
3737
}
3838
// creates the clientset
39-
clientset, err := kubernetes.NewForConfig(config)
40-
if err != nil {
41-
return nil, err
42-
}
43-
return clientset, nil
39+
return kubernetes.NewForConfig(config)
4440
}
4541

4642
// Get instance info from kubernetes API

pkg/cloud/powervs.go

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"github.com/davecgh/go-spew/spew"
3232
"k8s.io/apimachinery/pkg/util/wait"
3333
"k8s.io/utils/ptr"
34+
3435
"sigs.k8s.io/ibm-powervs-block-csi-driver/pkg/util"
3536
)
3637

@@ -39,14 +40,10 @@ var _ Cloud = &powerVSCloud{}
3940
const (
4041
PollTimeout = 120 * time.Second
4142
PollInterval = 5 * time.Second
42-
TIMEOUT = 60 * time.Minute
4343
VolumeInUseState = "in-use"
4444
VolumeAvailableState = "available"
4545
)
4646

47-
type PowerVSClient interface {
48-
}
49-
5047
type powerVSCloud struct {
5148
pvmInstancesClient *instance.IBMPIInstanceClient
5249
volClient *instance.IBMPIVolumeClient
@@ -176,24 +173,16 @@ func (p *powerVSCloud) AttachDisk(volumeID string, nodeID string) (err error) {
176173
if err != nil {
177174
return err
178175
}
176+
return p.WaitForVolumeState(volumeID, VolumeInUseState)
179177

180-
err = p.WaitForVolumeState(volumeID, VolumeInUseState)
181-
if err != nil {
182-
return err
183-
}
184-
return nil
185178
}
186179

187180
func (p *powerVSCloud) DetachDisk(volumeID string, nodeID string) (err error) {
188181
err = p.volClient.Detach(nodeID, volumeID)
189182
if err != nil {
190183
return err
191184
}
192-
err = p.WaitForVolumeState(volumeID, VolumeAvailableState)
193-
if err != nil {
194-
return err
195-
}
196-
return nil
185+
return p.WaitForVolumeState(volumeID, VolumeAvailableState)
197186
}
198187

199188
func (p *powerVSCloud) IsAttached(volumeID string, nodeID string) (attached bool, err error) {
@@ -210,11 +199,9 @@ func (p *powerVSCloud) ResizeDisk(volumeID string, reqSize int64) (newSize int64
210199
return 0, err
211200
}
212201

213-
capacityGiB := util.BytesToGiB(reqSize)
214-
215202
dataVolume := &models.UpdateVolume{
216203
Name: &disk.Name,
217-
Size: float64(capacityGiB),
204+
Size: float64(util.BytesToGiB(reqSize)),
218205
Shareable: &disk.Shareable,
219206
}
220207

pkg/cloud/powervs_node.go

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -31,66 +31,54 @@ const (
3131
)
3232

3333
type NodeUpdateScopeParams struct {
34-
ServiceInstanceId string
3534
InstanceId string
35+
ServiceInstanceId string
3636
Zone string
3737
}
3838

3939
type NodeUpdateScope struct {
4040
Cloud
41-
ServiceInstanceId string
4241
InstanceId string
42+
ServiceInstanceId string
4343
Zone string
4444
}
4545

4646
func NewNodeUpdateScope(params NodeUpdateScopeParams) (scope *NodeUpdateScope, err error) {
47-
scope = &NodeUpdateScope{}
48-
4947
if params.ServiceInstanceId == "" {
5048
err = errors.New("ServiceInstanceId is required when creating a NodeUpdateScope")
5149
return
5250
}
53-
scope.ServiceInstanceId = params.ServiceInstanceId
54-
5551
if params.InstanceId == "" {
5652
err = errors.New("InstanceId is required when creating a NodeUpdateScope")
5753
return
5854
}
59-
scope.InstanceId = params.InstanceId
60-
6155
if params.Zone == "" {
6256
err = errors.New("zone is required when creating a NodeUpdateScope")
6357
return
6458
}
65-
scope.Zone = params.Zone
66-
67-
c, err := NewPowerVSCloud(scope.ServiceInstanceId, scope.Zone, false)
59+
c, err := NewPowerVSCloud(params.ServiceInstanceId, params.Zone, false)
6860
if err != nil {
6961
klog.Errorf("Failed to get powervs cloud: %v", err)
7062
return
7163
}
72-
scope.Cloud = c
7364

74-
return scope, nil
65+
return &NodeUpdateScope{
66+
ServiceInstanceId: params.ServiceInstanceId,
67+
InstanceId: params.InstanceId,
68+
Zone: params.Zone,
69+
Cloud: c,
70+
}, nil
7571
}
7672

7773
func (p *powerVSCloud) GetPVMInstanceDetails(instanceID string) (*models.PVMInstance, error) {
78-
insDetails, err := p.pvmInstancesClient.Get(instanceID)
79-
if err != nil {
80-
return nil, err
81-
}
82-
return insDetails, nil
74+
return p.pvmInstancesClient.Get(instanceID)
75+
8376
}
8477

8578
func (p *powerVSCloud) UpdateStoragePoolAffinity(instanceID string) error {
86-
87-
body := &models.PVMInstanceUpdate{
88-
StoragePoolAffinity: ptr.To[bool](StoragePoolAffinity),
89-
}
90-
91-
_, err := p.pvmInstancesClient.Update(instanceID, body)
92-
if err != nil {
93-
return err
94-
}
95-
return nil
79+
_, err := p.pvmInstancesClient.Update(instanceID,
80+
&models.PVMInstanceUpdate{
81+
StoragePoolAffinity: ptr.To[bool](StoragePoolAffinity),
82+
})
83+
return err
9684
}

pkg/driver/node.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ func (d *nodeService) stageVolume(wwn string, req *csi.NodeStageVolumeRequest) e
224224
// If the volume corresponding to the volume_id is already staged to the staging_target_path,
225225
// and is identical to the specified volume_capability the Plugin MUST reply 0 OK.
226226
source := (*dev).GetMapper()
227-
if err == nil && deviceFromMount == source {
227+
if deviceFromMount == source {
228228
klog.V(4).Infof("Volume %s is already staged", req.VolumeId)
229229
return nil
230230
}

pkg/driver/validation.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222

2323
func ValidateDriverOptions(options *Options) error {
2424
if err := validateMode(options.mode); err != nil {
25-
return fmt.Errorf("Invalid mode: %v", err)
25+
return fmt.Errorf("invalid mode: %v", err)
2626
}
2727
return nil
2828
}
@@ -31,6 +31,5 @@ func validateMode(mode Mode) error {
3131
if mode != AllMode && mode != ControllerMode && mode != NodeMode {
3232
return fmt.Errorf("Mode is not supported (actual: %s, supported: %v)", mode, []Mode{AllMode, ControllerMode, NodeMode})
3333
}
34-
3534
return nil
3635
}

pkg/driver/validation_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func TestValidateDriverOptions(t *testing.T) {
7272
{
7373
name: "fail because validateMode fails",
7474
mode: Mode("unknown"),
75-
expErr: fmt.Errorf("Invalid mode: Mode is not supported (actual: unknown, supported: %v)", []Mode{AllMode, ControllerMode, NodeMode}),
75+
expErr: fmt.Errorf("invalid mode: Mode is not supported (actual: unknown, supported: %v)", []Mode{AllMode, ControllerMode, NodeMode}),
7676
},
7777
}
7878

pkg/driver/version.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,5 @@ func GetVersion() VersionInfo {
5252
func GetVersionJSON() (string, error) {
5353
info := GetVersion()
5454
marshalled, err := json.MarshalIndent(&info, "", " ")
55-
if err != nil {
56-
return "", err
57-
}
58-
return string(marshalled), nil
55+
return string(marshalled), err
5956
}

pkg/driver/version_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import (
1818
"reflect"
1919
"runtime"
2020
"testing"
21+
22+
"github.com/google/go-cmp/cmp"
2123
)
2224

2325
func TestGetVersion(t *testing.T) {
@@ -33,7 +35,7 @@ func TestGetVersion(t *testing.T) {
3335
}
3436

3537
if !reflect.DeepEqual(version, expected) {
36-
t.Fatalf("structs not equall\ngot:\n%+v\nexpected:\n%+v", version, expected)
38+
t.Fatalf("structs not equal, diff(- want, + got): \n%v", cmp.Diff(version, expected))
3739
}
3840
}
3941

@@ -53,6 +55,6 @@ func TestGetVersionJSON(t *testing.T) {
5355
}`, runtime.Version(), runtime.Compiler, fmt.Sprintf("%s/%s", runtime.GOOS, runtime.GOARCH))
5456

5557
if version != expected {
56-
t.Fatalf("json not equall\ngot:\n%s\nexpected:\n%s", version, expected)
58+
t.Fatalf("structs not equal, diff(- want, + got): \n%v", cmp.Diff(version, expected))
5759
}
5860
}

0 commit comments

Comments
 (0)