Merge pull request #631 from kishen-v/refactor

Clean up code.

Author: k8s-ci-robot

go.mod

@@ -8,6 +8,7 @@ require (
 	github.com/IBM/platform-services-go-sdk v0.62.2
 	github.com/container-storage-interface/spec v1.9.0
 	github.com/davecgh/go-spew v1.1.1
+	github.com/google/go-cmp v0.6.0
 	github.com/kubernetes-csi/csi-test v2.2.0+incompatible
 	github.com/onsi/ginkgo/v2 v2.17.1
 	github.com/onsi/gomega v1.32.0
@@ -66,7 +67,6 @@ require (
 	github.com/golang/protobuf v1.5.4 // indirect
 	github.com/google/cel-go v0.17.7 // indirect
 	github.com/google/gnostic-models v0.6.8 // indirect
-	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/google/gofuzz v1.2.0 // indirect
 	github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1 // indirect
 	github.com/google/uuid v1.6.0 // indirect

pkg/cloud/metadata.go

@@ -57,7 +57,7 @@ func (m *Metadata) GetPvmInstanceId() string {
 	return m.pvmInstanceId
 }
 
-// TokenizeProviderID tokenize the provider id into Metadata structure
+// TokenizeProviderID tokenizes the provider id into Metadata structure
 // ProviderID format: ibmpowervs://<region>/<zone>/<service_instance_id>/<powervs_machine_id>
 func TokenizeProviderID(providerID string) (*Metadata, error) {
 	data := strings.Split(providerID, "/")
@@ -87,13 +87,12 @@ func TokenizeProviderID(providerID string) (*Metadata, error) {
 
 // Get New Metadata Service
 func NewMetadataService(k8sAPIClient KubernetesAPIClient, kubeconfig string) (MetadataService, error) {
-	klog.Infof("retrieving instance data from kubernetes api")
+	klog.Infof("retrieving instance data from kubernetes API")
 	clientset, err := k8sAPIClient(kubeconfig)
 	if err != nil {
 		klog.Warningf("error creating kubernetes api client: %v", err)
-	} else {
-		klog.Infof("kubernetes api is available")
-		return KubernetesAPIInstanceInfo(clientset)
+		return nil, fmt.Errorf("an error occured during creation of k8s API client: %w", err)
 	}
-	return nil, fmt.Errorf("error getting instance data from ec2 metadata or kubernetes api")
+	klog.Info("kubernetes API is available")
+	return KubernetesAPIInstanceInfo(clientset)
 }

pkg/cloud/metadata_k8s.go

@@ -36,11 +36,7 @@ var DefaultKubernetesAPIClient = func(kubeconfig string) (kubernetes.Interface,
 		return nil, err
 	}
 	// creates the clientset
-	clientset, err := kubernetes.NewForConfig(config)
-	if err != nil {
-		return nil, err
-	}
-	return clientset, nil
+	return kubernetes.NewForConfig(config)
 }
 
 // Get instance info from kubernetes API

pkg/cloud/powervs.go

@@ -31,6 +31,7 @@ import (
 	"github.com/davecgh/go-spew/spew"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/utils/ptr"
+
 	"sigs.k8s.io/ibm-powervs-block-csi-driver/pkg/util"
 )
 
@@ -39,14 +40,10 @@ var _ Cloud = &powerVSCloud{}
 const (
 	PollTimeout          = 120 * time.Second
 	PollInterval         = 5 * time.Second
-	TIMEOUT              = 60 * time.Minute
 	VolumeInUseState     = "in-use"
 	VolumeAvailableState = "available"
 )
 
-type PowerVSClient interface {
-}
-
 type powerVSCloud struct {
 	pvmInstancesClient *instance.IBMPIInstanceClient
 	volClient          *instance.IBMPIVolumeClient
@@ -176,24 +173,16 @@ func (p *powerVSCloud) AttachDisk(volumeID string, nodeID string) (err error) {
 	if err != nil {
 		return err
 	}
+	return p.WaitForVolumeState(volumeID, VolumeInUseState)
 
-	err = p.WaitForVolumeState(volumeID, VolumeInUseState)
-	if err != nil {
-		return err
-	}
-	return nil
 }
 
 func (p *powerVSCloud) DetachDisk(volumeID string, nodeID string) (err error) {
 	err = p.volClient.Detach(nodeID, volumeID)
 	if err != nil {
 		return err
 	}
-	err = p.WaitForVolumeState(volumeID, VolumeAvailableState)
-	if err != nil {
-		return err
-	}
-	return nil
+	return p.WaitForVolumeState(volumeID, VolumeAvailableState)
 }
 
 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
 		return 0, err
 	}
 
-	capacityGiB := util.BytesToGiB(reqSize)
-
 	dataVolume := &models.UpdateVolume{
 		Name:      &disk.Name,
-		Size:      float64(capacityGiB),
+		Size:      float64(util.BytesToGiB(reqSize)),
 		Shareable: &disk.Shareable,
 	}
 

pkg/cloud/powervs_node.go

@@ -31,66 +31,54 @@ const (
 )
 
 type NodeUpdateScopeParams struct {
-	ServiceInstanceId string
 	InstanceId        string
+	ServiceInstanceId string
 	Zone              string
 }
 
 type NodeUpdateScope struct {
 	Cloud
-	ServiceInstanceId string
 	InstanceId        string
+	ServiceInstanceId string
 	Zone              string
 }
 
 func NewNodeUpdateScope(params NodeUpdateScopeParams) (scope *NodeUpdateScope, err error) {
-	scope = &NodeUpdateScope{}
-
 	if params.ServiceInstanceId == "" {
 		err = errors.New("ServiceInstanceId is required when creating a NodeUpdateScope")
 		return
 	}
-	scope.ServiceInstanceId = params.ServiceInstanceId
-
 	if params.InstanceId == "" {
 		err = errors.New("InstanceId is required when creating a NodeUpdateScope")
 		return
 	}
-	scope.InstanceId = params.InstanceId
-
 	if params.Zone == "" {
 		err = errors.New("zone is required when creating a NodeUpdateScope")
 		return
 	}
-	scope.Zone = params.Zone
-
-	c, err := NewPowerVSCloud(scope.ServiceInstanceId, scope.Zone, false)
+	c, err := NewPowerVSCloud(params.ServiceInstanceId, params.Zone, false)
 	if err != nil {
 		klog.Errorf("Failed to get powervs cloud: %v", err)
 		return
 	}
-	scope.Cloud = c
 
-	return scope, nil
+	return &NodeUpdateScope{
+		ServiceInstanceId: params.ServiceInstanceId,
+		InstanceId:        params.InstanceId,
+		Zone:              params.Zone,
+		Cloud:             c,
+	}, nil
 }
 
 func (p *powerVSCloud) GetPVMInstanceDetails(instanceID string) (*models.PVMInstance, error) {
-	insDetails, err := p.pvmInstancesClient.Get(instanceID)
-	if err != nil {
-		return nil, err
-	}
-	return insDetails, nil
+	return p.pvmInstancesClient.Get(instanceID)
+
 }
 
 func (p *powerVSCloud) UpdateStoragePoolAffinity(instanceID string) error {
-
-	body := &models.PVMInstanceUpdate{
-		StoragePoolAffinity: ptr.To[bool](StoragePoolAffinity),
-	}
-
-	_, err := p.pvmInstancesClient.Update(instanceID, body)
-	if err != nil {
-		return err
-	}
-	return nil
+	_, err := p.pvmInstancesClient.Update(instanceID,
+		&models.PVMInstanceUpdate{
+			StoragePoolAffinity: ptr.To[bool](StoragePoolAffinity),
+		})
+	return err
 }

pkg/driver/node.go

@@ -224,7 +224,7 @@ func (d *nodeService) stageVolume(wwn string, req *csi.NodeStageVolumeRequest) e
 	// If the volume corresponding to the volume_id is already staged to the staging_target_path,
 	// and is identical to the specified volume_capability the Plugin MUST reply 0 OK.
 	source := (*dev).GetMapper()
-	if err == nil && deviceFromMount == source {
+	if deviceFromMount == source {
 		klog.V(4).Infof("Volume %s is already staged", req.VolumeId)
 		return nil
 	}

pkg/driver/validation.go

@@ -22,7 +22,7 @@ import (
 
 func ValidateDriverOptions(options *Options) error {
 	if err := validateMode(options.mode); err != nil {
-		return fmt.Errorf("Invalid mode: %v", err)
+		return fmt.Errorf("invalid mode: %v", err)
 	}
 	return nil
 }
@@ -31,6 +31,5 @@ func validateMode(mode Mode) error {
 	if mode != AllMode && mode != ControllerMode && mode != NodeMode {
 		return fmt.Errorf("Mode is not supported (actual: %s, supported: %v)", mode, []Mode{AllMode, ControllerMode, NodeMode})
 	}
-
 	return nil
 }

pkg/driver/validation_test.go

@@ -72,7 +72,7 @@ func TestValidateDriverOptions(t *testing.T) {
 		{
 			name:   "fail because validateMode fails",
 			mode:   Mode("unknown"),
-			expErr: fmt.Errorf("Invalid mode: Mode is not supported (actual: unknown, supported: %v)", []Mode{AllMode, ControllerMode, NodeMode}),
+			expErr: fmt.Errorf("invalid mode: Mode is not supported (actual: unknown, supported: %v)", []Mode{AllMode, ControllerMode, NodeMode}),
 		},
 	}
 

pkg/driver/version.go

@@ -52,8 +52,5 @@ func GetVersion() VersionInfo {
 func GetVersionJSON() (string, error) {
 	info := GetVersion()
 	marshalled, err := json.MarshalIndent(&info, "", "  ")
-	if err != nil {
-		return "", err
-	}
-	return string(marshalled), nil
+	return string(marshalled), err
 }

pkg/driver/version_test.go

@@ -18,6 +18,8 @@ import (
 	"reflect"
 	"runtime"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
 )
 
 func TestGetVersion(t *testing.T) {
@@ -33,7 +35,7 @@ func TestGetVersion(t *testing.T) {
 	}
 
 	if !reflect.DeepEqual(version, expected) {
-		t.Fatalf("structs not equall\ngot:\n%+v\nexpected:\n%+v", version, expected)
+		t.Fatalf("structs not equal, diff(- want, + got): \n%v", cmp.Diff(version, expected))
 	}
 }
 
@@ -53,6 +55,6 @@ func TestGetVersionJSON(t *testing.T) {
 }`, runtime.Version(), runtime.Compiler, fmt.Sprintf("%s/%s", runtime.GOOS, runtime.GOARCH))
 
 	if version != expected {
-		t.Fatalf("json not equall\ngot:\n%s\nexpected:\n%s", version, expected)
+		t.Fatalf("structs not equal, diff(- want, + got): \n%v", cmp.Diff(version, expected))
 	}
 }