Repository: kubernetes-sigs/aws-ebs-csi-driver
Pull request #405 (2019-11-10 10:39:55)

Normalize variable function and method naming
* Fix golint error and normalize variable or function/method naming

* ~If block ends with a return statement, so drop this else and outdent its block~

k8s-ci-robot: Hi @sivanzcw. Thanks for your PR.

I'm waiting for a [kubernetes-sigs](https://github.com/orgs/kubernetes-sigs/people) member to verify that this patch is reasonable to test. If it is, they should reply with `/ok-to-test` on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should [join the org](https://git.k8s.io/community/community-membership.md#member) to skip this step.

Once the patch is verified, the new status will be reflected by the `ok-to-test` label.

I understand the commands that are listed [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Faws-ebs-csi-driver).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/aws-ebs-csi-driver/pull/405#" title="Author self-approved">sivanzcw</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **ddebroy**
You can assign the PR to them by writing `/assign @ddebroy` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Faws-ebs-csi-driver).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes-sigs/aws-ebs-csi-driver/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["ddebroy"]} -->
k8s-ci-robot: Welcome @sivanzcw! <br><br>It looks like this is your first PR to <a href='https://github.com/kubernetes-sigs/aws-ebs-csi-driver'>kubernetes-sigs/aws-ebs-csi-driver</a> 🎉. Please refer to our [pull request process documentation](https://git.k8s.io/community/contributors/guide/pull-requests.md) to help your PR have a smooth ride to approval. <br><br>You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. [Here is the bot commands documentation](https://go.k8s.io/bot-commands). <br><br>You can also check if kubernetes-sigs/aws-ebs-csi-driver has [its own contribution guidelines](https://github.com/kubernetes-sigs/aws-ebs-csi-driver/tree/master/CONTRIBUTING.md). <br><br>You may want to refer to our [testing guide](https://git.k8s.io/community/contributors/devel/sig-testing/testing.md) if you run into trouble with your tests not passing. <br><br>If you are having difficulty getting your pull request seen, please follow the [recommended escalation practices](https://github.com/kubernetes/community/blob/master/contributors/guide/pull-requests.md#why-is-my-pull-request-not-getting-reviewed). Also, for tips and tricks in the contribution process you may want to read the [Kubernetes contributor cheat sheet](https://git.k8s.io/community/contributors/guide/contributor-cheatsheet/README.md). We want to make sure your contribution gets all the attention it needs! <br><br>Thank you, and welcome to Kubernetes. :smiley:
coveralls: ## Pull Request Test Coverage Report for [Build 931](https://coveralls.io/builds/26886529)

* **13** of **14**   **(92.86%)**  changed or added relevant lines in **3** files are covered.
* **2** unchanged lines in **1** file lost coverage.
* Overall coverage decreased (**-0.08%**) to **70.605%**

---

|  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [pkg/driver/node.go](https://coveralls.io/builds/26886529/source?filename=pkg%2Fdriver%2Fnode.go#L477) | 1 | 2 | 50.0%
<!-- | **Total:** | **13** | **14** | **92.86%** | -->

|  Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [pkg/driver/controller.go](https://coveralls.io/builds/26886529/source?filename=pkg%2Fdriver%2Fcontroller.go#L449) | 2 | 71.63% |
<!-- | **Total:** | **2** |  | -->

|  Totals | [![Coverage Status](https://coveralls.io/builds/26886529/badge)](https://coveralls.io/builds/26886529) |
| :-- | --: |
| Change from base [Build 928](https://coveralls.io/builds/26869687): |  -0.08% |
| Covered Lines: | 1213 |
| Relevant Lines: | 1718 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go
index 7ee1aa76..fc80f2a2 100644
--- a/pkg/cloud/cloud.go
+++ b/pkg/cloud/cloud.go
@@ -195,7 +195,7 @@ type Cloud interface {
 	CreateSnapshot(ctx context.Context, volumeID string, snapshotOptions *SnapshotOptions) (snapshot *Snapshot, err error)
 	DeleteSnapshot(ctx context.Context, snapshotID string) (success bool, err error)
 	GetSnapshotByName(ctx context.Context, name string) (snapshot *Snapshot, err error)
-	GetSnapshotById(ctx context.Context, snapshotID string) (snapshot *Snapshot, err error)
+	GetSnapshotByID(ctx context.Context, snapshotID string) (snapshot *Snapshot, err error)
 	ListSnapshots(ctx context.Context, volumeID string, maxResults int64, nextToken string) (listSnapshotsResponse *ListSnapshotsResponse, err error)
 }
 
@@ -570,7 +570,7 @@ func (c *cloud) GetSnapshotByName(ctx context.Context, name string) (snapshot *S
 	return c.ec2SnapshotResponseToStruct(ec2snapshot), nil
 }
 
-func (c *cloud) GetSnapshotById(ctx context.Context, snapshotID string) (snapshot *Snapshot, err error) {
+func (c *cloud) GetSnapshotByID(ctx context.Context, snapshotID string) (snapshot *Snapshot, err error) {
 	request := &ec2.DescribeSnapshotsInput{
 		SnapshotIds: []*string{
 			aws.String(snapshotID),
diff --git a/pkg/driver/constants.go b/pkg/driver/constants.go
index 689157b2..f8b02ef3 100644
--- a/pkg/driver/constants.go
+++ b/pkg/driver/constants.go
@@ -35,7 +35,7 @@ const (
 	EncryptedKey = "encrypted"
 
 	// KmsKeyId represents key for KMS encryption key
-	KmsKeyIdKey = "kmskeyid"
+	KmsKeyIDKey = "kmskeyid"
 )
 
 // constants for default command line flag values
diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go
index 5e0c1ce7..47d9f287 100644
--- a/pkg/driver/controller.go
+++ b/pkg/driver/controller.go
@@ -113,7 +113,7 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
 		volumeType  string
 		iopsPerGB   int
 		isEncrypted bool
-		kmsKeyId    string
+		kmsKeyID    string
 	)
 
 	for key, value := range req.GetParameters() {
@@ -133,8 +133,8 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
 			} else {
 				isEncrypted = false
 			}
-		case KmsKeyIdKey:
-			kmsKeyId = value
+		case KmsKeyIDKey:
+			kmsKeyID = value
 		default:
 			return nil, status.Errorf(codes.InvalidArgument, "Invalid parameter key %s for CreateVolume", key)
 		}
@@ -162,7 +162,7 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
 		IOPSPerGB:        iopsPerGB,
 		AvailabilityZone: zone,
 		Encrypted:        isEncrypted,
-		KmsKeyID:         kmsKeyId,
+		KmsKeyID:         kmsKeyID,
 	}
 
 	volumeSource := req.GetVolumeContentSource()
@@ -433,7 +433,7 @@ func (d *controllerService) ListSnapshots(ctx context.Context, req *csi.ListSnap
 
 	snapshotID := req.GetSnapshotId()
 	if len(snapshotID) != 0 {
-		snapshot, err := d.cloud.GetSnapshotById(ctx, snapshotID)
+		snapshot, err := d.cloud.GetSnapshotByID(ctx, snapshotID)
 		if err != nil {
 			if err == cloud.ErrNotFound {
 				klog.V(4).Info("ListSnapshots: snapshot not found, returning with success")
diff --git a/pkg/driver/mocks/mock_cloud.go b/pkg/driver/mocks/mock_cloud.go
index c87c8713..9dc3858f 100644
--- a/pkg/driver/mocks/mock_cloud.go
+++ b/pkg/driver/mocks/mock_cloud.go
@@ -137,17 +137,17 @@ func (mr *MockCloudMockRecorder) GetDiskByName(arg0, arg1, arg2 interface{}) *go
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDiskByName", reflect.TypeOf((*MockCloud)(nil).GetDiskByName), arg0, arg1, arg2)
 }
 
-// GetSnapshotById mocks base method
-func (m *MockCloud) GetSnapshotById(arg0 context.Context, arg1 string) (*cloud.Snapshot, error) {
-	ret := m.ctrl.Call(m, "GetSnapshotById", arg0, arg1)
+// GetSnapshotByID mocks base method
+func (m *MockCloud) GetSnapshotByID(arg0 context.Context, arg1 string) (*cloud.Snapshot, error) {
+	ret := m.ctrl.Call(m, "GetSnapshotByID", arg0, arg1)
 	ret0, _ := ret[0].(*cloud.Snapshot)
 	ret1, _ := ret[1].(error)
 	return ret0, ret1
 }
 
-// GetSnapshotById indicates an expected call of GetSnapshotById
-func (mr *MockCloudMockRecorder) GetSnapshotById(arg0, arg1 interface{}) *gomock.Call {
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSnapshotById", reflect.TypeOf((*MockCloud)(nil).GetSnapshotById), arg0, arg1)
+// GetSnapshotByID indicates an expected call of GetSnapshotByID
+func (mr *MockCloudMockRecorder) GetSnapshotByID(arg0, arg1 interface{}) *gomock.Call {
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSnapshotByID", reflect.TypeOf((*MockCloud)(nil).GetSnapshotByID), arg0, arg1)
 }
 
 // GetSnapshotByName mocks base method
diff --git a/pkg/driver/node.go b/pkg/driver/node.go
index 650b4fb5..abcf9de9 100644
--- a/pkg/driver/node.go
+++ b/pkg/driver/node.go
@@ -459,7 +459,7 @@ func (d *nodeService) nodePublishVolumeForFileSystem(req *csi.NodePublishVolumeR
 // findDevicePath finds path of device and verifies its existence
 // if the device is not nvme, return the path directly
 // if the device is nvme, finds and returns the nvme device path eg. /dev/nvme1n1
-func (d *nodeService) findDevicePath(devicePath, volumeId string) (string, error) {
+func (d *nodeService) findDevicePath(devicePath, volumeID string) (string, error) {
 	exists, err := d.mounter.ExistsPath(devicePath)
 	if err != nil {
 		return "", err
@@ -474,7 +474,7 @@ func (d *nodeService) findDevicePath(devicePath, volumeId string) (string, error
 	// This is the magic name on which AWS presents NVME devices under /dev/disk/by-id/
 	// For example, vol-0fab1d5e3f72a5e23 creates a symlink at
 	// /dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol0fab1d5e3f72a5e23
-	nvmeName := "nvme-Amazon_Elastic_Block_Store_" + strings.Replace(volumeId, "-", "", -1)
+	nvmeName := "nvme-Amazon_Elastic_Block_Store_" + strings.Replace(volumeID, "-", "", -1)
 
 	return findNvmeVolume(nvmeName)
 }


Test patch:
diff --git a/pkg/cloud/cloud_test.go b/pkg/cloud/cloud_test.go
index 09647d4f..f9bb813a 100644
--- a/pkg/cloud/cloud_test.go
+++ b/pkg/cloud/cloud_test.go
@@ -783,7 +783,7 @@ func TestGetSnapshotByName(t *testing.T) {
 	}
 }
 
-func TestGetSnapshotById(t *testing.T) {
+func TestGetSnapshotByID(t *testing.T) {
 	testCases := []struct {
 		name            string
 		snapshotName    string
@@ -821,7 +821,7 @@ func TestGetSnapshotById(t *testing.T) {
 			ctx := context.Background()
 			mockEC2.EXPECT().DescribeSnapshotsWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DescribeSnapshotsOutput{Snapshots: []*ec2.Snapshot{ec2snapshot}}, nil)
 
-			_, err := c.GetSnapshotById(ctx, tc.snapshotOptions.Tags[SnapshotNameTagKey])
+			_, err := c.GetSnapshotByID(ctx, tc.snapshotOptions.Tags[SnapshotNameTagKey])
 			if err != nil {
 				if tc.expErr == nil {
 					t.Fatalf("GetSnapshotByName() failed: expected no error, got: %v", err)
diff --git a/pkg/driver/controller_test.go b/pkg/driver/controller_test.go
index b97f3f8d..290fdfa8 100644
--- a/pkg/driver/controller_test.go
+++ b/pkg/driver/controller_test.go
@@ -35,7 +35,7 @@ import (
 
 const (
 	expZone       = "us-west-2b"
-	expInstanceId = "i-123456789abcdef01"
+	expInstanceID = "i-123456789abcdef01"
 )
 
 func TestCreateVolume(t *testing.T) {
@@ -575,7 +575,7 @@ func TestCreateVolume(t *testing.T) {
 					VolumeCapabilities: stdVolCap,
 					Parameters: map[string]string{
 						EncryptedKey: "true",
-						KmsKeyIdKey:  "arn:aws:kms:us-east-1:012345678910:key/abcd1234-a123-456a-a12b-a123b4cd56ef",
+						KmsKeyIDKey:  "arn:aws:kms:us-east-1:012345678910:key/abcd1234-a123-456a-a12b-a123b4cd56ef",
 					},
 				}
 
@@ -1328,7 +1328,7 @@ func TestListSnapshots(t *testing.T) {
 				defer mockCtl.Finish()
 
 				mockCloud := mocks.NewMockCloud(mockCtl)
-				mockCloud.EXPECT().GetSnapshotById(gomock.Eq(ctx), gomock.Eq("snapshot-1")).Return(mockCloudSnapshotsResponse, nil)
+				mockCloud.EXPECT().GetSnapshotByID(gomock.Eq(ctx), gomock.Eq("snapshot-1")).Return(mockCloudSnapshotsResponse, nil)
 
 				awsDriver := controllerService{
 					cloud:         mockCloud,
@@ -1357,7 +1357,7 @@ func TestListSnapshots(t *testing.T) {
 				defer mockCtl.Finish()
 
 				mockCloud := mocks.NewMockCloud(mockCtl)
-				mockCloud.EXPECT().GetSnapshotById(gomock.Eq(ctx), gomock.Eq("snapshot-1")).Return(nil, cloud.ErrNotFound)
+				mockCloud.EXPECT().GetSnapshotByID(gomock.Eq(ctx), gomock.Eq("snapshot-1")).Return(nil, cloud.ErrNotFound)
 
 				awsDriver := controllerService{
 					cloud:         mockCloud,
@@ -1386,7 +1386,7 @@ func TestListSnapshots(t *testing.T) {
 				defer mockCtl.Finish()
 
 				mockCloud := mocks.NewMockCloud(mockCtl)
-				mockCloud.EXPECT().GetSnapshotById(gomock.Eq(ctx), gomock.Eq("snapshot-1")).Return(nil, cloud.ErrMultiSnapshots)
+				mockCloud.EXPECT().GetSnapshotByID(gomock.Eq(ctx), gomock.Eq("snapshot-1")).Return(nil, cloud.ErrMultiSnapshots)
 
 				awsDriver := controllerService{
 					cloud:         mockCloud,
@@ -1463,7 +1463,7 @@ func TestControllerPublishVolume(t *testing.T) {
 			name: "success normal",
 			testFunc: func(t *testing.T) {
 				req := &csi.ControllerPublishVolumeRequest{
-					NodeId:           expInstanceId,
+					NodeId:           expInstanceID,
 					VolumeCapability: stdVolCap,
 					VolumeId:         "vol-test",
 				}
@@ -1562,7 +1562,7 @@ func TestControllerPublishVolume(t *testing.T) {
 			name: "fail no VolumeCapability",
 			testFunc: func(t *testing.T) {
 				req := &csi.ControllerPublishVolumeRequest{
-					NodeId:   expInstanceId,
+					NodeId:   expInstanceID,
 					VolumeId: "vol-test",
 				}
 
@@ -1595,7 +1595,7 @@ func TestControllerPublishVolume(t *testing.T) {
 			name: "fail invalid VolumeCapability",
 			testFunc: func(t *testing.T) {
 				req := &csi.ControllerPublishVolumeRequest{
-					NodeId: expInstanceId,
+					NodeId: expInstanceID,
 					VolumeCapability: &csi.VolumeCapability{
 						AccessMode: &csi.VolumeCapability_AccessMode{
 							Mode: csi.VolumeCapability_AccessMode_UNKNOWN,
@@ -1669,7 +1669,7 @@ func TestControllerPublishVolume(t *testing.T) {
 			testFunc: func(t *testing.T) {
 				req := &csi.ControllerPublishVolumeRequest{
 					VolumeId:         "does-not-exist",
-					NodeId:           expInstanceId,
+					NodeId:           expInstanceID,
 					VolumeCapability: stdVolCap,
 				}
 
@@ -1705,7 +1705,7 @@ func TestControllerPublishVolume(t *testing.T) {
 			testFunc: func(t *testing.T) {
 				req := &csi.ControllerPublishVolumeRequest{
 					VolumeId:         "does-not-exist",
-					NodeId:           expInstanceId,
+					NodeId:           expInstanceID,
 					VolumeCapability: stdVolCap,
 				}
 
@@ -1753,7 +1753,7 @@ func TestControllerUnpublishVolume(t *testing.T) {
 			name: "success normal",
 			testFunc: func(t *testing.T) {
 				req := &csi.ControllerUnpublishVolumeRequest{
-					NodeId:   expInstanceId,
+					NodeId:   expInstanceID,
 					VolumeId: "vol-test",
 				}
 				expResp := &csi.ControllerUnpublishVolumeResponse{}
diff --git a/tests/sanity/fake_cloud_provider.go b/tests/sanity/fake_cloud_provider.go
index 2e74c08b..7f3fee21 100644
--- a/tests/sanity/fake_cloud_provider.go
+++ b/tests/sanity/fake_cloud_provider.go
@@ -178,7 +178,7 @@ func (c *fakeCloudProvider) GetSnapshotByName(ctx context.Context, name string)
 	return snapshots[0].Snapshot, nil
 }
 
-func (c *fakeCloudProvider) GetSnapshotById(ctx context.Context, snapshotID string) (snapshot *cloud.Snapshot, err error) {
+func (c *fakeCloudProvider) GetSnapshotByID(ctx context.Context, snapshotID string) (snapshot *cloud.Snapshot, err error) {
 	ret, exists := c.snapshots[snapshotID]
 	if !exists {
 		return nil, cloud.ErrNotFound