Merge pull request #69387 from mcrute/master

hack/.golint_failures

@@ -89,7 +89,6 @@ pkg/apis/storage/v1beta1
 pkg/apis/storage/v1beta1/util
 pkg/auth/authorizer/abac
 pkg/capabilities
-pkg/cloudprovider/providers/aws
 pkg/cloudprovider/providers/fake
 pkg/cloudprovider/providers/gce
 pkg/cloudprovider/providers/gce/cloud

pkg/cloudprovider/providers/aws/aws.go

@@ -428,7 +428,7 @@ type VolumeOptions struct {
 	Encrypted bool
 	// fully qualified resource name to the key to use for encryption.
 	// example: arn:aws:kms:us-east-1:012345678910:key/abcd1234-a123-456a-a12b-a123b4cd56ef
-	KmsKeyId string
+	KmsKeyID string
 }
 
 // Volumes is an interface for managing cloud-provisioned volumes
@@ -511,7 +511,7 @@ type Cloud struct {
 	// attached, to avoid a race condition where we assign a device mapping
 	// and then get a second request before we attach the volume
 	attachingMutex sync.Mutex
-	attaching      map[types.NodeName]map[mountDevice]awsVolumeID
+	attaching      map[types.NodeName]map[mountDevice]EBSVolumeID
 
 	// state of our device allocator for each node
 	deviceAllocators map[types.NodeName]DeviceAllocator
@@ -1110,7 +1110,7 @@ func newAWSCloud(cfg CloudConfig, awsServices Services) (*Cloud, error) {
 		cfg:      &cfg,
 		region:   regionName,
 
-		attaching:        make(map[types.NodeName]map[mountDevice]awsVolumeID),
+		attaching:        make(map[types.NodeName]map[mountDevice]EBSVolumeID),
 		deviceAllocators: make(map[types.NodeName]DeviceAllocator),
 	}
 	awsCloud.instanceCache.cloud = awsCloud
@@ -1613,14 +1613,14 @@ func (i *awsInstance) describeInstance() (*ec2.Instance, error) {
 func (c *Cloud) getMountDevice(
 	i *awsInstance,
 	info *ec2.Instance,
-	volumeID awsVolumeID,
+	volumeID EBSVolumeID,
 	assign bool) (assigned mountDevice, alreadyAttached bool, err error) {
 	instanceType := i.getInstanceType()
 	if instanceType == nil {
 		return "", false, fmt.Errorf("could not get instance type for instance: %s", i.awsID)
 	}
 
-	deviceMappings := map[mountDevice]awsVolumeID{}
+	deviceMappings := map[mountDevice]EBSVolumeID{}
 	for _, blockDevice := range info.BlockDeviceMappings {
 		name := aws.StringValue(blockDevice.DeviceName)
 		if strings.HasPrefix(name, "/dev/sd") {
@@ -1632,7 +1632,7 @@ func (c *Cloud) getMountDevice(
 		if len(name) < 1 || len(name) > 2 {
 			glog.Warningf("Unexpected EBS DeviceName: %q", aws.StringValue(blockDevice.DeviceName))
 		}
-		deviceMappings[mountDevice(name)] = awsVolumeID(aws.StringValue(blockDevice.Ebs.VolumeId))
+		deviceMappings[mountDevice(name)] = EBSVolumeID(aws.StringValue(blockDevice.Ebs.VolumeId))
 	}
 
 	// We lock to prevent concurrent mounts from conflicting
@@ -1675,12 +1675,12 @@ func (c *Cloud) getMountDevice(
 	chosen, err := deviceAllocator.GetNext(deviceMappings)
 	if err != nil {
 		glog.Warningf("Could not assign a mount device.  mappings=%v, error: %v", deviceMappings, err)
-		return "", false, fmt.Errorf("Too many EBS volumes attached to node %s.", i.nodeName)
+		return "", false, fmt.Errorf("too many EBS volumes attached to node %s", i.nodeName)
 	}
 
 	attaching := c.attaching[i.nodeName]
 	if attaching == nil {
-		attaching = make(map[mountDevice]awsVolumeID)
+		attaching = make(map[mountDevice]EBSVolumeID)
 		c.attaching[i.nodeName] = attaching
 	}
 	attaching[chosen] = volumeID
@@ -1691,7 +1691,7 @@ func (c *Cloud) getMountDevice(
 
 // endAttaching removes the entry from the "attachments in progress" map
 // It returns true if it was found (and removed), false otherwise
-func (c *Cloud) endAttaching(i *awsInstance, volumeID awsVolumeID, mountDevice mountDevice) bool {
+func (c *Cloud) endAttaching(i *awsInstance, volumeID EBSVolumeID, mountDevice mountDevice) bool {
 	c.attachingMutex.Lock()
 	defer c.attachingMutex.Unlock()
 
@@ -1718,7 +1718,7 @@ type awsDisk struct {
 	// Name in k8s
 	name KubernetesVolumeID
 	// id in AWS
-	awsID awsVolumeID
+	awsID EBSVolumeID
 }
 
 func newAWSDisk(aws *Cloud, name KubernetesVolumeID) (*awsDisk, error) {
@@ -1887,13 +1887,14 @@ func (d *awsDisk) waitForAttachmentStatus(status string) (*ec2.VolumeAttachment,
 			if describeErrorCount > volumeAttachmentStatusConsecutiveErrorLimit {
 				// report the error
 				return false, err
-			} else {
+			}
+
 			glog.Warningf("Ignoring error from describe volume for volume %q; will retry: %q", d.awsID, err)
 			return false, nil
 		}
-		} else {
+
 		describeErrorCount = 0
-		}
+
 		if len(info.Attachments) > 1 {
 			// Shouldn't happen; log so we know if it is
 			glog.Warningf("Found multiple attachments for volume %q: %v", d.awsID, info)
@@ -1981,7 +1982,7 @@ func wrapAttachError(err error, disk *awsDisk, instance string) error {
 				glog.Errorf("Error describing volume %q: %q", disk.awsID, err)
 			} else {
 				for _, a := range info.Attachments {
-					if disk.awsID != awsVolumeID(aws.StringValue(a.VolumeId)) {
+					if disk.awsID != EBSVolumeID(aws.StringValue(a.VolumeId)) {
 						glog.Warningf("Expected to get attachment info of volume %q but instead got info of %q", disk.awsID, aws.StringValue(a.VolumeId))
 					} else if aws.StringValue(a.State) == "attached" {
 						return fmt.Errorf("Error attaching EBS volume %q to instance %q: %q. The volume is currently attached to instance %q", disk.awsID, instance, awsError, aws.StringValue(a.InstanceId))
@@ -2097,9 +2098,9 @@ func (c *Cloud) DetachDisk(diskName KubernetesVolumeID, nodeName types.NodeName)
 			// Someone deleted the volume being detached; complain, but do nothing else and return success
 			glog.Warningf("DetachDisk %s called for node %s but volume does not exist; assuming the volume is detached", diskName, nodeName)
 			return "", nil
-		} else {
-			return "", err
 		}
+
+		return "", err
 	}
 
 	if !attached && diskInfo.ec2Instance != nil {
@@ -2196,8 +2197,8 @@ func (c *Cloud) CreateDisk(volumeOptions *VolumeOptions) (KubernetesVolumeID, er
 	request.Size = aws.Int64(int64(volumeOptions.CapacityGB))
 	request.VolumeType = aws.String(createType)
 	request.Encrypted = aws.Bool(volumeOptions.Encrypted)
-	if len(volumeOptions.KmsKeyId) > 0 {
-		request.KmsKeyId = aws.String(volumeOptions.KmsKeyId)
+	if len(volumeOptions.KmsKeyID) > 0 {
+		request.KmsKeyId = aws.String(volumeOptions.KmsKeyID)
 		request.Encrypted = aws.Bool(true)
 	}
 	if iops > 0 {
@@ -2208,7 +2209,7 @@ func (c *Cloud) CreateDisk(volumeOptions *VolumeOptions) (KubernetesVolumeID, er
 		return "", err
 	}
 
-	awsID := awsVolumeID(aws.StringValue(response.VolumeId))
+	awsID := EBSVolumeID(aws.StringValue(response.VolumeId))
 	if awsID == "" {
 		return "", fmt.Errorf("VolumeID was not returned by CreateVolume")
 	}
@@ -2230,7 +2231,7 @@ func (c *Cloud) CreateDisk(volumeOptions *VolumeOptions) (KubernetesVolumeID, er
 	// Such volume lives for couple of seconds and then it's silently deleted
 	// by AWS. There is no other check to ensure that given KMS key is correct,
 	// because Kubernetes may have limited permissions to the key.
-	if len(volumeOptions.KmsKeyId) > 0 {
+	if len(volumeOptions.KmsKeyID) > 0 {
 		err := c.waitUntilVolumeAvailable(volumeName)
 		if err != nil {
 			if isAWSErrorVolumeNotFound(err) {
@@ -2313,9 +2314,9 @@ func (c *Cloud) checkIfAvailable(disk *awsDisk, opName string, instance string)
 		// Volume is attached somewhere else and we can not attach it here
 		if len(info.Attachments) > 0 {
 			attachment := info.Attachments[0]
-			instanceId := aws.StringValue(attachment.InstanceId)
-			attachedInstance, ierr := c.getInstanceByID(instanceId)
-			attachErr := fmt.Sprintf("%s since volume is currently attached to %q", opError, instanceId)
+			instanceID := aws.StringValue(attachment.InstanceId)
+			attachedInstance, ierr := c.getInstanceByID(instanceID)
+			attachErr := fmt.Sprintf("%s since volume is currently attached to %q", opError, instanceID)
 			if ierr != nil {
 				glog.Error(attachErr)
 				return false, errors.New(attachErr)
@@ -2334,6 +2335,7 @@ func (c *Cloud) checkIfAvailable(disk *awsDisk, opName string, instance string)
 	return true, nil
 }
 
+// GetLabelsForVolume gets the volume labels for a volume
 func (c *Cloud) GetLabelsForVolume(ctx context.Context, pv *v1.PersistentVolume) (map[string]string, error) {
 	// Ignore any volumes that are being provisioned
 	if pv.Spec.AWSElasticBlockStore.VolumeID == volume.ProvisionedVolumeName {
@@ -2399,14 +2401,16 @@ func (c *Cloud) DiskIsAttached(diskName KubernetesVolumeID, nodeName types.NodeN
 			// The disk doesn't exist, can't be attached
 			glog.Warningf("DiskIsAttached called for volume %s on node %s but the volume does not exist", diskName, nodeName)
 			return false, nil
-		} else {
-			return true, err
 		}
+
+		return true, err
 	}
 
 	return attached, nil
 }
 
+// DisksAreAttached returns a map of nodes and Kubernetes volume IDs indicating
+// if the volumes are attached to the node
 func (c *Cloud) DisksAreAttached(nodeDisks map[types.NodeName][]KubernetesVolumeID) (map[types.NodeName]map[KubernetesVolumeID]bool, error) {
 	attached := make(map[types.NodeName]map[KubernetesVolumeID]bool)
 
@@ -2455,7 +2459,7 @@ func (c *Cloud) DisksAreAttached(nodeDisks map[types.NodeName][]KubernetesVolume
 			continue
 		}
 
-		idToDiskName := make(map[awsVolumeID]KubernetesVolumeID)
+		idToDiskName := make(map[EBSVolumeID]KubernetesVolumeID)
 		for _, diskName := range diskNames {
 			volumeID, err := diskName.MapToAWSVolumeID()
 			if err != nil {
@@ -2465,7 +2469,7 @@ func (c *Cloud) DisksAreAttached(nodeDisks map[types.NodeName][]KubernetesVolume
 		}
 
 		for _, blockDevice := range awsInstance.BlockDeviceMappings {
-			volumeID := awsVolumeID(aws.StringValue(blockDevice.Ebs.VolumeId))
+			volumeID := EBSVolumeID(aws.StringValue(blockDevice.Ebs.VolumeId))
 			diskName, found := idToDiskName[volumeID]
 			if found {
 				// Disk is still attached to node
@@ -2477,6 +2481,8 @@ func (c *Cloud) DisksAreAttached(nodeDisks map[types.NodeName][]KubernetesVolume
 	return attached, nil
 }
 
+// ResizeDisk resizes an EBS volume in GiB increments, it will round up to the
+// next GiB if arguments are not provided in even GiB increments
 func (c *Cloud) ResizeDisk(
 	diskName KubernetesVolumeID,
 	oldSize resource.Quantity,

pkg/cloudprovider/providers/aws/aws_instancegroups.go

@@ -42,7 +42,7 @@ func ResizeInstanceGroup(asg ASG, instanceGroupName string, size int) error {
 	return nil
 }
 
-// Implement InstanceGroups.ResizeInstanceGroup
+// ResizeInstanceGroup implements InstanceGroups.ResizeInstanceGroup
 // Set the size to the fixed size
 func (c *Cloud) ResizeInstanceGroup(instanceGroupName string, size int) error {
 	return ResizeInstanceGroup(c.asg, instanceGroupName, size)
@@ -70,7 +70,7 @@ func DescribeInstanceGroup(asg ASG, instanceGroupName string) (InstanceGroupInfo
 	return &awsInstanceGroup{group: group}, nil
 }
 
-// Implement InstanceGroups.DescribeInstanceGroup
+// DescribeInstanceGroup implements InstanceGroups.DescribeInstanceGroup
 // Queries the cloud provider for information about the specified instance group
 func (c *Cloud) DescribeInstanceGroup(instanceGroupName string) (InstanceGroupInfo, error) {
 	return DescribeInstanceGroup(c.asg, instanceGroupName)

pkg/cloudprovider/providers/aws/aws_loadbalancer.go

@@ -35,8 +35,12 @@ import (
 )
 
 const (
+	// ProxyProtocolPolicyName is the tag named used for the proxy protocol
+	// policy
 	ProxyProtocolPolicyName = "k8s-proxyprotocol-enabled"
 
+	// SSLNegotiationPolicyNameFormat is a format string used for the SSL
+	// negotiation policy tag name
 	SSLNegotiationPolicyNameFormat = "k8s-SSLNegotiationPolicy-%s"
 )
 
@@ -1326,16 +1330,16 @@ func (c *Cloud) ensureLoadBalancerInstances(loadBalancerName string, lbInstances
 	removals := actual.Difference(expected)
 
 	addInstances := []*elb.Instance{}
-	for _, instanceId := range additions.List() {
+	for _, instanceID := range additions.List() {
 		addInstance := &elb.Instance{}
-		addInstance.InstanceId = aws.String(instanceId)
+		addInstance.InstanceId = aws.String(instanceID)
 		addInstances = append(addInstances, addInstance)
 	}
 
 	removeInstances := []*elb.Instance{}
-	for _, instanceId := range removals.List() {
+	for _, instanceID := range removals.List() {
 		removeInstance := &elb.Instance{}
-		removeInstance.InstanceId = aws.String(instanceId)
+		removeInstance.InstanceId = aws.String(instanceID)
 		removeInstances = append(removeInstances, removeInstance)
 	}
 

pkg/cloudprovider/providers/aws/device_allocator.go

@@ -27,18 +27,20 @@ import (
 // can be used for anything that DeviceAllocator user wants.
 // Only the relevant part of device name should be in the map, e.g. "ba" for
 // "/dev/xvdba".
-type ExistingDevices map[mountDevice]awsVolumeID
+type ExistingDevices map[mountDevice]EBSVolumeID
 
-// On AWS, we should assign new (not yet used) device names to attached volumes.
-// If we reuse a previously used name, we may get the volume "attaching" forever,
-// see https://aws.amazon.com/premiumsupport/knowledge-center/ebs-stuck-attaching/.
 // DeviceAllocator finds available device name, taking into account already
 // assigned device names from ExistingDevices map. It tries to find the next
 // device name to the previously assigned one (from previous DeviceAllocator
 // call), so all available device names are used eventually and it minimizes
 // device name reuse.
+//
 // All these allocations are in-memory, nothing is written to / read from
 // /dev directory.
+//
+// On AWS, we should assign new (not yet used) device names to attached volumes.
+// If we reuse a previously used name, we may get the volume "attaching" forever,
+// see https://aws.amazon.com/premiumsupport/knowledge-center/ebs-stuck-attaching/.
 type DeviceAllocator interface {
 	// GetNext returns a free device name or error when there is no free device
 	// name. Only the device suffix is returned, e.g. "ba" for "/dev/xvdba".
@@ -74,9 +76,9 @@ func (p devicePairList) Len() int           { return len(p) }
 func (p devicePairList) Less(i, j int) bool { return p[i].deviceIndex < p[j].deviceIndex }
 func (p devicePairList) Swap(i, j int)      { p[i], p[j] = p[j], p[i] }
 
-// Allocates device names according to scheme ba..bz, ca..cz
-// it moves along the ring and always picks next device until
-// device list is exhausted.
+// NewDeviceAllocator allocates device names according to scheme ba..bz, ca..cz
+// it moves along the ring and always picks next device until device list is
+// exhausted.
 func NewDeviceAllocator() DeviceAllocator {
 	possibleDevices := make(map[mountDevice]int)
 	for _, firstChar := range []rune{'b', 'c'} {

pkg/cloudprovider/providers/aws/regions_test.go

@@ -73,7 +73,7 @@ func TestRecognizesNewRegion(t *testing.T) {
 		t.Fatalf("region already valid: %q", region)
 	}
 
-	awsServices := NewFakeAWSServices(TestClusterId).WithAz(region + "a")
+	awsServices := NewFakeAWSServices(TestClusterID).WithAz(region + "a")
 	_, err := newAWSCloud(CloudConfig{}, awsServices)
 	if err != nil {
 		t.Errorf("error building AWS cloud: %v", err)

pkg/cloudprovider/providers/aws/retry_handler.go

@@ -40,14 +40,14 @@ type CrossRequestRetryDelay struct {
 	backoff Backoff
 }
 
-// Create a new CrossRequestRetryDelay
+// NewCrossRequestRetryDelay creates a new CrossRequestRetryDelay
 func NewCrossRequestRetryDelay() *CrossRequestRetryDelay {
 	c := &CrossRequestRetryDelay{}
 	c.backoff.init(decayIntervalSeconds, decayFraction, maxDelay)
 	return c
 }
 
-// Added to the Sign chain; called before each request
+// BeforeSign is added to the Sign chain; called before each request
 func (c *CrossRequestRetryDelay) BeforeSign(r *request.Request) {
 	now := time.Now()
 	delay := c.backoff.ComputeDelayForRequest(now)
@@ -84,7 +84,7 @@ func describeRequest(r *request.Request) string {
 	return service + "::" + operationName(r)
 }
 
-// Added to the AfterRetry chain; called after any error
+// AfterRetry is added to the AfterRetry chain; called after any error
 func (c *CrossRequestRetryDelay) AfterRetry(r *request.Request) {
 	if r.Error == nil {
 		return
@@ -126,7 +126,8 @@ func (b *Backoff) init(decayIntervalSeconds int, decayFraction float64, maxDelay
 	b.maxDelay = maxDelay
 }
 
-// Computes the delay required for a request, also updating internal state to count this request
+// ComputeDelayForRequest computes the delay required for a request, also
+// updates internal state to count this request
 func (b *Backoff) ComputeDelayForRequest(now time.Time) time.Duration {
 	b.mutex.Lock()
 	defer b.mutex.Unlock()
@@ -165,7 +166,7 @@ func (b *Backoff) ComputeDelayForRequest(now time.Time) time.Duration {
 	return time.Second * time.Duration(int(delay.Seconds()))
 }
 
-// Called when we observe a throttling error
+// ReportError is called when we observe a throttling error
 func (b *Backoff) ReportError() {
 	b.mutex.Lock()
 	defer b.mutex.Unlock()

pkg/cloudprovider/providers/aws/sets_ippermissions.go

@@ -23,8 +23,10 @@ import (
 	"github.com/aws/aws-sdk-go/service/ec2"
 )
 
+// IPPermissionSet maps IP strings of strings to EC2 IpPermissions
 type IPPermissionSet map[string]*ec2.IpPermission
 
+// NewIPPermissionSet creates a new IPPermissionSet
 func NewIPPermissionSet(items ...*ec2.IpPermission) IPPermissionSet {
 	s := make(IPPermissionSet)
 	s.Insert(items...)
@@ -97,10 +99,10 @@ func (s IPPermissionSet) List() []*ec2.IpPermission {
 	return res
 }
 
-// IsSuperset returns true if and only if s1 is a superset of s2.
-func (s1 IPPermissionSet) IsSuperset(s2 IPPermissionSet) bool {
+// IsSuperset returns true if and only if s is a superset of s2.
+func (s IPPermissionSet) IsSuperset(s2 IPPermissionSet) bool {
 	for k := range s2 {
-		_, found := s1[k]
+		_, found := s[k]
 		if !found {
 			return false
 		}
@@ -108,11 +110,11 @@ func (s1 IPPermissionSet) IsSuperset(s2 IPPermissionSet) bool {
 	return true
 }
 
-// Equal returns true if and only if s1 is equal (as a set) to s2.
+// Equal returns true if and only if s is equal (as a set) to s2.
 // Two sets are equal if their membership is identical.
 // (In practice, this means same elements, order doesn't matter)
-func (s1 IPPermissionSet) Equal(s2 IPPermissionSet) bool {
-	return len(s1) == len(s2) && s1.IsSuperset(s2)
+func (s IPPermissionSet) Equal(s2 IPPermissionSet) bool {
+	return len(s) == len(s2) && s.IsSuperset(s2)
 }
 
 // Difference returns a set of objects that are not in s2

pkg/cloudprovider/providers/aws/tags.go

@@ -38,6 +38,7 @@ const TagNameKubernetesClusterPrefix = "kubernetes.io/cluster/"
 // did not allow shared resources.
 const TagNameKubernetesClusterLegacy = "KubernetesCluster"
 
+// ResourceLifecycle is the cluster lifecycle state used in tagging
 type ResourceLifecycle string
 
 const (
@@ -152,13 +153,13 @@ func (t *awsTagging) hasClusterTag(tags []*ec2.Tag) bool {
 // Ensure that a resource has the correct tags
 // If it has no tags, we assume that this was a problem caused by an error in between creation and tagging,
 // and we add the tags.  If it has a different cluster's tags, that is an error.
-func (c *awsTagging) readRepairClusterTags(client EC2, resourceID string, lifecycle ResourceLifecycle, additionalTags map[string]string, observedTags []*ec2.Tag) error {
+func (t *awsTagging) readRepairClusterTags(client EC2, resourceID string, lifecycle ResourceLifecycle, additionalTags map[string]string, observedTags []*ec2.Tag) error {
 	actualTagMap := make(map[string]string)
 	for _, tag := range observedTags {
 		actualTagMap[aws.StringValue(tag.Key)] = aws.StringValue(tag.Value)
 	}
 
-	expectedTags := c.buildTags(lifecycle, additionalTags)
+	expectedTags := t.buildTags(lifecycle, additionalTags)
 
 	addTags := make(map[string]string)
 	for k, expected := range expectedTags {
@@ -178,7 +179,7 @@ func (c *awsTagging) readRepairClusterTags(client EC2, resourceID string, lifecy
 		return nil
 	}
 
-	if err := c.createTags(client, resourceID, lifecycle, addTags); err != nil {
+	if err := t.createTags(client, resourceID, lifecycle, addTags); err != nil {
 		return fmt.Errorf("error adding missing tags to resource %q: %q", resourceID, err)
 	}
 

pkg/cloudprovider/providers/aws/tags_test.go

@@ -23,14 +23,14 @@ import (
 )
 
 func TestFilterTags(t *testing.T) {
-	awsServices := NewFakeAWSServices(TestClusterId)
+	awsServices := NewFakeAWSServices(TestClusterID)
 	c, err := newAWSCloud(CloudConfig{}, awsServices)
 	if err != nil {
 		t.Errorf("Error building aws cloud: %v", err)
 		return
 	}
 
-	if c.tagging.ClusterID != TestClusterId {
+	if c.tagging.ClusterID != TestClusterID {
 		t.Errorf("unexpected ClusterID: %v", c.tagging.ClusterID)
 	}
 }
@@ -116,7 +116,7 @@ func TestFindClusterID(t *testing.T) {
 }
 
 func TestHasClusterTag(t *testing.T) {
-	awsServices := NewFakeAWSServices(TestClusterId)
+	awsServices := NewFakeAWSServices(TestClusterID)
 	c, err := newAWSCloud(CloudConfig{}, awsServices)
 	if err != nil {
 		t.Errorf("Error building aws cloud: %v", err)
@@ -131,7 +131,7 @@ func TestHasClusterTag(t *testing.T) {
 		},
 		{
 			Tags: map[string]string{
-				TagNameKubernetesClusterLegacy: TestClusterId,
+				TagNameKubernetesClusterLegacy: TestClusterID,
 			},
 			Expected: true,
 		},
@@ -143,26 +143,26 @@ func TestHasClusterTag(t *testing.T) {
 		},
 		{
 			Tags: map[string]string{
-				TagNameKubernetesClusterPrefix + TestClusterId: "owned",
+				TagNameKubernetesClusterPrefix + TestClusterID: "owned",
 			},
 			Expected: true,
 		},
 		{
 			Tags: map[string]string{
-				TagNameKubernetesClusterPrefix + TestClusterId: "",
+				TagNameKubernetesClusterPrefix + TestClusterID: "",
 			},
 			Expected: true,
 		},
 		{
 			Tags: map[string]string{
 				TagNameKubernetesClusterLegacy:                 "a",
-				TagNameKubernetesClusterPrefix + TestClusterId: "shared",
+				TagNameKubernetesClusterPrefix + TestClusterID: "shared",
 			},
 			Expected: true,
 		},
 		{
 			Tags: map[string]string{
-				TagNameKubernetesClusterPrefix + TestClusterId: "shared",
+				TagNameKubernetesClusterPrefix + TestClusterID: "shared",
 				TagNameKubernetesClusterPrefix + "b":           "shared",
 			},
 			Expected: true,

pkg/cloudprovider/providers/aws/volumes.go

@@ -31,14 +31,14 @@ import (
 // awsVolumeRegMatch represents Regex Match for AWS volume.
 var awsVolumeRegMatch = regexp.MustCompile("^vol-[^/]*$")
 
-// awsVolumeID represents the ID of the volume in the AWS API, e.g. vol-12345678
-// The "traditional" format is "vol-12345678"
-// A new longer format is also being introduced: "vol-12345678abcdef01"
-// We should not assume anything about the length or format, though it seems
-// reasonable to assume that volumes will continue to start with "vol-".
-type awsVolumeID string
-
-func (i awsVolumeID) awsString() *string {
+// EBSVolumeID represents the ID of the volume in the AWS API, e.g.
+// vol-12345678 The "traditional" format is "vol-12345678" A new longer format
+// is also being introduced: "vol-12345678abcdef01" We should not assume
+// anything about the length or format, though it seems reasonable to assume
+// that volumes will continue to start with "vol-".
+type EBSVolumeID string
+
+func (i EBSVolumeID) awsString() *string {
 	return aws.String(string(i))
 }
 
@@ -59,8 +59,8 @@ type diskInfo struct {
 	disk            *awsDisk
 }
 
-// MapToAWSVolumeID extracts the awsVolumeID from the KubernetesVolumeID
-func (name KubernetesVolumeID) MapToAWSVolumeID() (awsVolumeID, error) {
+// MapToAWSVolumeID extracts the EBSVolumeID from the KubernetesVolumeID
+func (name KubernetesVolumeID) MapToAWSVolumeID() (EBSVolumeID, error) {
 	// name looks like aws://availability-zone/awsVolumeId
 
 	// The original idea of the URL-style name was to put the AZ into the
@@ -96,9 +96,10 @@ func (name KubernetesVolumeID) MapToAWSVolumeID() (awsVolumeID, error) {
 		return "", fmt.Errorf("Invalid format for AWS volume (%s)", name)
 	}
 
-	return awsVolumeID(awsID), nil
+	return EBSVolumeID(awsID), nil
 }
 
+// GetAWSVolumeID converts a Kubernetes volume ID to an AWS volume ID
 func GetAWSVolumeID(kubeVolumeID string) (string, error) {
 	kid := KubernetesVolumeID(kubeVolumeID)
 	awsID, err := kid.MapToAWSVolumeID()

pkg/volume/awsebs/aws_util.go

@@ -168,7 +168,7 @@ func populateVolumeOptions(pluginName, pvcName string, capacityGB resource.Quant
 				return nil, fmt.Errorf("invalid encrypted boolean value %q, must be true or false: %v", v, err)
 			}
 		case "kmskeyid":
-			volumeOptions.KmsKeyId = v
+			volumeOptions.KmsKeyID = v
 		case volume.VolumeParameterFSType:
 			// Do nothing but don't make this fail
 		default: