From 4d65f20ff6745d140be46ef7c8f942db9049abe6 Mon Sep 17 00:00:00 2001 From: Mike Crute Date: Tue, 2 Oct 2018 16:11:18 -0700 Subject: [PATCH] fix golint for pkg/cloudprovider/providers/aws --- hack/.golint_failures | 1 - pkg/cloudprovider/providers/aws/aws.go | 64 +++-- pkg/cloudprovider/providers/aws/aws_fakes.go | 263 +++++++++++++----- .../providers/aws/aws_instancegroups.go | 4 +- .../providers/aws/aws_loadbalancer.go | 12 +- pkg/cloudprovider/providers/aws/aws_test.go | 146 +++++----- .../providers/aws/device_allocator.go | 16 +- .../providers/aws/regions_test.go | 2 +- .../providers/aws/retry_handler.go | 11 +- .../providers/aws/sets_ippermissions.go | 14 +- pkg/cloudprovider/providers/aws/tags.go | 7 +- pkg/cloudprovider/providers/aws/tags_test.go | 16 +- pkg/cloudprovider/providers/aws/volumes.go | 21 +- pkg/volume/awsebs/aws_util.go | 2 +- 14 files changed, 366 insertions(+), 213 deletions(-) diff --git a/hack/.golint_failures b/hack/.golint_failures index d3fb821d71..39cf363631 100644 --- a/hack/.golint_failures +++ b/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 diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index 24c8aba533..f628340bf3 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/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 + + glog.Warningf("Ignoring error from describe volume for volume %q; will retry: %q", d.awsID, err) + return false, nil } + + 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, diff --git a/pkg/cloudprovider/providers/aws/aws_fakes.go b/pkg/cloudprovider/providers/aws/aws_fakes.go index 7743632723..bc35818d2e 100644 --- a/pkg/cloudprovider/providers/aws/aws_fakes.go +++ b/pkg/cloudprovider/providers/aws/aws_fakes.go @@ -29,6 +29,7 @@ import ( "github.com/golang/glog" ) +// FakeAWSServices is an fake AWS session used for testing type FakeAWSServices struct { region string instances []*ec2.Instance @@ -45,7 +46,8 @@ type FakeAWSServices struct { kms *FakeKMS } -func NewFakeAWSServices(clusterId string) *FakeAWSServices { +// NewFakeAWSServices creates a new FakeAWSServices +func NewFakeAWSServices(clusterID string) *FakeAWSServices { s := &FakeAWSServices{} s.region = "us-east-1" s.ec2 = &FakeEC2Impl{aws: s} @@ -71,12 +73,13 @@ func NewFakeAWSServices(clusterId string) *FakeAWSServices { var tag ec2.Tag tag.Key = aws.String(TagNameKubernetesClusterLegacy) - tag.Value = aws.String(clusterId) + tag.Value = aws.String(clusterID) selfInstance.Tags = []*ec2.Tag{&tag} return s } +// WithAz sets the ec2 placement availability zone func (s *FakeAWSServices) WithAz(az string) *FakeAWSServices { if s.selfInstance.Placement == nil { s.selfInstance.Placement = &ec2.Placement{} @@ -85,30 +88,37 @@ func (s *FakeAWSServices) WithAz(az string) *FakeAWSServices { return s } +// Compute returns a fake EC2 client func (s *FakeAWSServices) Compute(region string) (EC2, error) { return s.ec2, nil } +// LoadBalancing returns a fake ELB client func (s *FakeAWSServices) LoadBalancing(region string) (ELB, error) { return s.elb, nil } +// LoadBalancingV2 returns a fake ELBV2 client func (s *FakeAWSServices) LoadBalancingV2(region string) (ELBV2, error) { return s.elbv2, nil } +// Autoscaling returns a fake ASG client func (s *FakeAWSServices) Autoscaling(region string) (ASG, error) { return s.asg, nil } +// Metadata returns a fake EC2Metadata client func (s *FakeAWSServices) Metadata() (EC2Metadata, error) { return s.metadata, nil } +// KeyManagement returns a fake KMS client func (s *FakeAWSServices) KeyManagement(region string) (KMS, error) { return s.kms, nil } +// FakeEC2 is a fake EC2 client used for testing type FakeEC2 interface { EC2 CreateSubnet(*ec2.Subnet) (*ec2.CreateSubnetOutput, error) @@ -117,6 +127,7 @@ type FakeEC2 interface { RemoveRouteTables() } +// FakeEC2Impl is an implementation of the FakeEC2 interface used for testing type FakeEC2Impl struct { aws *FakeAWSServices Subnets []*ec2.Subnet @@ -125,6 +136,7 @@ type FakeEC2Impl struct { DescribeRouteTablesInput *ec2.DescribeRouteTablesInput } +// DescribeInstances returns fake instance descriptions func (ec2i *FakeEC2Impl) DescribeInstances(request *ec2.DescribeInstancesInput) ([]*ec2.Instance, error) { matches := []*ec2.Instance{} for _, instance := range ec2i.aws.instances { @@ -163,54 +175,73 @@ func (ec2i *FakeEC2Impl) DescribeInstances(request *ec2.DescribeInstancesInput) return matches, nil } +// AttachVolume is not implemented but is required for interface conformance func (ec2i *FakeEC2Impl) AttachVolume(request *ec2.AttachVolumeInput) (resp *ec2.VolumeAttachment, err error) { panic("Not implemented") } +// DetachVolume is not implemented but is required for interface conformance func (ec2i *FakeEC2Impl) DetachVolume(request *ec2.DetachVolumeInput) (resp *ec2.VolumeAttachment, err error) { panic("Not implemented") } +// DescribeVolumes is not implemented but is required for interface conformance func (ec2i *FakeEC2Impl) DescribeVolumes(request *ec2.DescribeVolumesInput) ([]*ec2.Volume, error) { panic("Not implemented") } +// CreateVolume is not implemented but is required for interface conformance func (ec2i *FakeEC2Impl) CreateVolume(request *ec2.CreateVolumeInput) (resp *ec2.Volume, err error) { panic("Not implemented") } +// DeleteVolume is not implemented but is required for interface conformance func (ec2i *FakeEC2Impl) DeleteVolume(request *ec2.DeleteVolumeInput) (resp *ec2.DeleteVolumeOutput, err error) { panic("Not implemented") } +// DescribeSecurityGroups is not implemented but is required for interface +// conformance func (ec2i *FakeEC2Impl) DescribeSecurityGroups(request *ec2.DescribeSecurityGroupsInput) ([]*ec2.SecurityGroup, error) { panic("Not implemented") } +// CreateSecurityGroup is not implemented but is required for interface +// conformance func (ec2i *FakeEC2Impl) CreateSecurityGroup(*ec2.CreateSecurityGroupInput) (*ec2.CreateSecurityGroupOutput, error) { panic("Not implemented") } +// DeleteSecurityGroup is not implemented but is required for interface +// conformance func (ec2i *FakeEC2Impl) DeleteSecurityGroup(*ec2.DeleteSecurityGroupInput) (*ec2.DeleteSecurityGroupOutput, error) { panic("Not implemented") } +// AuthorizeSecurityGroupIngress is not implemented but is required for +// interface conformance func (ec2i *FakeEC2Impl) AuthorizeSecurityGroupIngress(*ec2.AuthorizeSecurityGroupIngressInput) (*ec2.AuthorizeSecurityGroupIngressOutput, error) { panic("Not implemented") } +// RevokeSecurityGroupIngress is not implemented but is required for interface +// conformance func (ec2i *FakeEC2Impl) RevokeSecurityGroupIngress(*ec2.RevokeSecurityGroupIngressInput) (*ec2.RevokeSecurityGroupIngressOutput, error) { panic("Not implemented") } +// DescribeVolumeModifications is not implemented but is required for interface +// conformance func (ec2i *FakeEC2Impl) DescribeVolumeModifications(*ec2.DescribeVolumesModificationsInput) ([]*ec2.VolumeModification, error) { panic("Not implemented") } +// ModifyVolume is not implemented but is required for interface conformance func (ec2i *FakeEC2Impl) ModifyVolume(*ec2.ModifyVolumeInput) (*ec2.ModifyVolumeOutput, error) { panic("Not implemented") } +// CreateSubnet creates fake subnets func (ec2i *FakeEC2Impl) CreateSubnet(request *ec2.Subnet) (*ec2.CreateSubnetOutput, error) { ec2i.Subnets = append(ec2i.Subnets, request) response := &ec2.CreateSubnetOutput{ @@ -219,24 +250,29 @@ func (ec2i *FakeEC2Impl) CreateSubnet(request *ec2.Subnet) (*ec2.CreateSubnetOut return response, nil } +// DescribeSubnets returns fake subnet descriptions func (ec2i *FakeEC2Impl) DescribeSubnets(request *ec2.DescribeSubnetsInput) ([]*ec2.Subnet, error) { ec2i.DescribeSubnetsInput = request return ec2i.Subnets, nil } +// RemoveSubnets clears subnets on client func (ec2i *FakeEC2Impl) RemoveSubnets() { ec2i.Subnets = ec2i.Subnets[:0] } +// CreateTags is not implemented but is required for interface conformance func (ec2i *FakeEC2Impl) CreateTags(*ec2.CreateTagsInput) (*ec2.CreateTagsOutput, error) { panic("Not implemented") } +// DescribeRouteTables returns fake route table descriptions func (ec2i *FakeEC2Impl) DescribeRouteTables(request *ec2.DescribeRouteTablesInput) ([]*ec2.RouteTable, error) { ec2i.DescribeRouteTablesInput = request return ec2i.RouteTables, nil } +// CreateRouteTable creates fake route tables func (ec2i *FakeEC2Impl) CreateRouteTable(request *ec2.RouteTable) (*ec2.CreateRouteTableOutput, error) { ec2i.RouteTables = append(ec2i.RouteTables, request) response := &ec2.CreateRouteTableOutput{ @@ -245,30 +281,38 @@ func (ec2i *FakeEC2Impl) CreateRouteTable(request *ec2.RouteTable) (*ec2.CreateR return response, nil } +// RemoveRouteTables clears route tables on client func (ec2i *FakeEC2Impl) RemoveRouteTables() { ec2i.RouteTables = ec2i.RouteTables[:0] } +// CreateRoute is not implemented but is required for interface conformance func (ec2i *FakeEC2Impl) CreateRoute(request *ec2.CreateRouteInput) (*ec2.CreateRouteOutput, error) { panic("Not implemented") } +// DeleteRoute is not implemented but is required for interface conformance func (ec2i *FakeEC2Impl) DeleteRoute(request *ec2.DeleteRouteInput) (*ec2.DeleteRouteOutput, error) { panic("Not implemented") } +// ModifyInstanceAttribute is not implemented but is required for interface +// conformance func (ec2i *FakeEC2Impl) ModifyInstanceAttribute(request *ec2.ModifyInstanceAttributeInput) (*ec2.ModifyInstanceAttributeOutput, error) { panic("Not implemented") } +// DescribeVpcs returns fake VPC descriptions func (ec2i *FakeEC2Impl) DescribeVpcs(request *ec2.DescribeVpcsInput) (*ec2.DescribeVpcsOutput, error) { return &ec2.DescribeVpcsOutput{Vpcs: []*ec2.Vpc{{CidrBlock: aws.String("172.20.0.0/16")}}}, nil } +// FakeMetadata is a fake EC2 metadata service client used for testing type FakeMetadata struct { aws *FakeAWSServices } +// GetMetadata returns fake EC2 metadata for testing func (m *FakeMetadata) GetMetadata(key string) (string, error) { networkInterfacesPrefix := "network/interfaces/macs/" i := m.aws.selfInstance @@ -291,199 +335,292 @@ func (m *FakeMetadata) GetMetadata(key string) (string, error) { } else if strings.HasPrefix(key, networkInterfacesPrefix) { if key == networkInterfacesPrefix { return strings.Join(m.aws.networkInterfacesMacs, "/\n") + "/\n", nil - } else { - keySplit := strings.Split(key, "/") - macParam := keySplit[3] - if len(keySplit) == 5 && keySplit[4] == "vpc-id" { - for i, macElem := range m.aws.networkInterfacesMacs { - if macParam == macElem { - return m.aws.networkInterfacesVpcIDs[i], nil - } - } - } - if len(keySplit) == 5 && keySplit[4] == "local-ipv4s" { - for i, macElem := range m.aws.networkInterfacesMacs { - if macParam == macElem { - return strings.Join(m.aws.networkInterfacesPrivateIPs[i], "/\n"), nil - } - } - } - return "", nil } + + keySplit := strings.Split(key, "/") + macParam := keySplit[3] + if len(keySplit) == 5 && keySplit[4] == "vpc-id" { + for i, macElem := range m.aws.networkInterfacesMacs { + if macParam == macElem { + return m.aws.networkInterfacesVpcIDs[i], nil + } + } + } + if len(keySplit) == 5 && keySplit[4] == "local-ipv4s" { + for i, macElem := range m.aws.networkInterfacesMacs { + if macParam == macElem { + return strings.Join(m.aws.networkInterfacesPrivateIPs[i], "/\n"), nil + } + } + } + + return "", nil } else { return "", nil } } +// FakeELB is a fake ELB client used for testing type FakeELB struct { aws *FakeAWSServices } +// CreateLoadBalancer is not implemented but is required for interface +// conformance func (elb *FakeELB) CreateLoadBalancer(*elb.CreateLoadBalancerInput) (*elb.CreateLoadBalancerOutput, error) { panic("Not implemented") } +// DeleteLoadBalancer is not implemented but is required for interface +// conformance func (elb *FakeELB) DeleteLoadBalancer(input *elb.DeleteLoadBalancerInput) (*elb.DeleteLoadBalancerOutput, error) { panic("Not implemented") } +// DescribeLoadBalancers is not implemented but is required for interface +// conformance func (elb *FakeELB) DescribeLoadBalancers(input *elb.DescribeLoadBalancersInput) (*elb.DescribeLoadBalancersOutput, error) { panic("Not implemented") } +// AddTags is not implemented but is required for interface conformance func (elb *FakeELB) AddTags(input *elb.AddTagsInput) (*elb.AddTagsOutput, error) { panic("Not implemented") } +// RegisterInstancesWithLoadBalancer is not implemented but is required for +// interface conformance func (elb *FakeELB) RegisterInstancesWithLoadBalancer(*elb.RegisterInstancesWithLoadBalancerInput) (*elb.RegisterInstancesWithLoadBalancerOutput, error) { panic("Not implemented") } +// DeregisterInstancesFromLoadBalancer is not implemented but is required for +// interface conformance func (elb *FakeELB) DeregisterInstancesFromLoadBalancer(*elb.DeregisterInstancesFromLoadBalancerInput) (*elb.DeregisterInstancesFromLoadBalancerOutput, error) { panic("Not implemented") } +// DetachLoadBalancerFromSubnets is not implemented but is required for +// interface conformance func (elb *FakeELB) DetachLoadBalancerFromSubnets(*elb.DetachLoadBalancerFromSubnetsInput) (*elb.DetachLoadBalancerFromSubnetsOutput, error) { panic("Not implemented") } +// AttachLoadBalancerToSubnets is not implemented but is required for interface +// conformance func (elb *FakeELB) AttachLoadBalancerToSubnets(*elb.AttachLoadBalancerToSubnetsInput) (*elb.AttachLoadBalancerToSubnetsOutput, error) { panic("Not implemented") } +// CreateLoadBalancerListeners is not implemented but is required for interface +// conformance func (elb *FakeELB) CreateLoadBalancerListeners(*elb.CreateLoadBalancerListenersInput) (*elb.CreateLoadBalancerListenersOutput, error) { panic("Not implemented") } +// DeleteLoadBalancerListeners is not implemented but is required for interface +// conformance func (elb *FakeELB) DeleteLoadBalancerListeners(*elb.DeleteLoadBalancerListenersInput) (*elb.DeleteLoadBalancerListenersOutput, error) { panic("Not implemented") } +// ApplySecurityGroupsToLoadBalancer is not implemented but is required for +// interface conformance func (elb *FakeELB) ApplySecurityGroupsToLoadBalancer(*elb.ApplySecurityGroupsToLoadBalancerInput) (*elb.ApplySecurityGroupsToLoadBalancerOutput, error) { panic("Not implemented") } +// ConfigureHealthCheck is not implemented but is required for interface +// conformance func (elb *FakeELB) ConfigureHealthCheck(*elb.ConfigureHealthCheckInput) (*elb.ConfigureHealthCheckOutput, error) { panic("Not implemented") } +// CreateLoadBalancerPolicy is not implemented but is required for interface +// conformance func (elb *FakeELB) CreateLoadBalancerPolicy(*elb.CreateLoadBalancerPolicyInput) (*elb.CreateLoadBalancerPolicyOutput, error) { panic("Not implemented") } +// SetLoadBalancerPoliciesForBackendServer is not implemented but is required +// for interface conformance func (elb *FakeELB) SetLoadBalancerPoliciesForBackendServer(*elb.SetLoadBalancerPoliciesForBackendServerInput) (*elb.SetLoadBalancerPoliciesForBackendServerOutput, error) { panic("Not implemented") } +// SetLoadBalancerPoliciesOfListener is not implemented but is required for +// interface conformance func (elb *FakeELB) SetLoadBalancerPoliciesOfListener(input *elb.SetLoadBalancerPoliciesOfListenerInput) (*elb.SetLoadBalancerPoliciesOfListenerOutput, error) { panic("Not implemented") } +// DescribeLoadBalancerPolicies is not implemented but is required for +// interface conformance func (elb *FakeELB) DescribeLoadBalancerPolicies(input *elb.DescribeLoadBalancerPoliciesInput) (*elb.DescribeLoadBalancerPoliciesOutput, error) { panic("Not implemented") } +// DescribeLoadBalancerAttributes is not implemented but is required for +// interface conformance func (elb *FakeELB) DescribeLoadBalancerAttributes(*elb.DescribeLoadBalancerAttributesInput) (*elb.DescribeLoadBalancerAttributesOutput, error) { panic("Not implemented") } +// ModifyLoadBalancerAttributes is not implemented but is required for +// interface conformance func (elb *FakeELB) ModifyLoadBalancerAttributes(*elb.ModifyLoadBalancerAttributesInput) (*elb.ModifyLoadBalancerAttributesOutput, error) { panic("Not implemented") } -func (self *FakeELB) expectDescribeLoadBalancers(loadBalancerName string) { +// expectDescribeLoadBalancers is not implemented but is required for interface +// conformance +func (elb *FakeELB) expectDescribeLoadBalancers(loadBalancerName string) { panic("Not implemented") } +// FakeELBV2 is a fake ELBV2 client used for testing type FakeELBV2 struct { aws *FakeAWSServices } -func (self *FakeELBV2) AddTags(input *elbv2.AddTagsInput) (*elbv2.AddTagsOutput, error) { +// AddTags is not implemented but is required for interface conformance +func (elb *FakeELBV2) AddTags(input *elbv2.AddTagsInput) (*elbv2.AddTagsOutput, error) { panic("Not implemented") } -func (self *FakeELBV2) CreateLoadBalancer(*elbv2.CreateLoadBalancerInput) (*elbv2.CreateLoadBalancerOutput, error) { - panic("Not implemented") -} -func (self *FakeELBV2) DescribeLoadBalancers(*elbv2.DescribeLoadBalancersInput) (*elbv2.DescribeLoadBalancersOutput, error) { - panic("Not implemented") -} -func (self *FakeELBV2) DeleteLoadBalancer(*elbv2.DeleteLoadBalancerInput) (*elbv2.DeleteLoadBalancerOutput, error) { +// CreateLoadBalancer is not implemented but is required for interface +// conformance +func (elb *FakeELBV2) CreateLoadBalancer(*elbv2.CreateLoadBalancerInput) (*elbv2.CreateLoadBalancerOutput, error) { panic("Not implemented") } -func (self *FakeELBV2) ModifyLoadBalancerAttributes(*elbv2.ModifyLoadBalancerAttributesInput) (*elbv2.ModifyLoadBalancerAttributesOutput, error) { - panic("Not implemented") -} -func (self *FakeELBV2) DescribeLoadBalancerAttributes(*elbv2.DescribeLoadBalancerAttributesInput) (*elbv2.DescribeLoadBalancerAttributesOutput, error) { +// DescribeLoadBalancers is not implemented but is required for interface +// conformance +func (elb *FakeELBV2) DescribeLoadBalancers(*elbv2.DescribeLoadBalancersInput) (*elbv2.DescribeLoadBalancersOutput, error) { panic("Not implemented") } -func (self *FakeELBV2) CreateTargetGroup(*elbv2.CreateTargetGroupInput) (*elbv2.CreateTargetGroupOutput, error) { - panic("Not implemented") -} -func (self *FakeELBV2) DescribeTargetGroups(*elbv2.DescribeTargetGroupsInput) (*elbv2.DescribeTargetGroupsOutput, error) { - panic("Not implemented") -} -func (self *FakeELBV2) ModifyTargetGroup(*elbv2.ModifyTargetGroupInput) (*elbv2.ModifyTargetGroupOutput, error) { - panic("Not implemented") -} -func (self *FakeELBV2) DeleteTargetGroup(*elbv2.DeleteTargetGroupInput) (*elbv2.DeleteTargetGroupOutput, error) { +// DeleteLoadBalancer is not implemented but is required for interface +// conformance +func (elb *FakeELBV2) DeleteLoadBalancer(*elbv2.DeleteLoadBalancerInput) (*elbv2.DeleteLoadBalancerOutput, error) { panic("Not implemented") } -func (self *FakeELBV2) DescribeTargetHealth(input *elbv2.DescribeTargetHealthInput) (*elbv2.DescribeTargetHealthOutput, error) { +// ModifyLoadBalancerAttributes is not implemented but is required for +// interface conformance +func (elb *FakeELBV2) ModifyLoadBalancerAttributes(*elbv2.ModifyLoadBalancerAttributesInput) (*elbv2.ModifyLoadBalancerAttributesOutput, error) { panic("Not implemented") } -func (self *FakeELBV2) DescribeTargetGroupAttributes(*elbv2.DescribeTargetGroupAttributesInput) (*elbv2.DescribeTargetGroupAttributesOutput, error) { - panic("Not implemented") -} -func (self *FakeELBV2) ModifyTargetGroupAttributes(*elbv2.ModifyTargetGroupAttributesInput) (*elbv2.ModifyTargetGroupAttributesOutput, error) { +// DescribeLoadBalancerAttributes is not implemented but is required for +// interface conformance +func (elb *FakeELBV2) DescribeLoadBalancerAttributes(*elbv2.DescribeLoadBalancerAttributesInput) (*elbv2.DescribeLoadBalancerAttributesOutput, error) { panic("Not implemented") } -func (self *FakeELBV2) RegisterTargets(*elbv2.RegisterTargetsInput) (*elbv2.RegisterTargetsOutput, error) { - panic("Not implemented") -} -func (self *FakeELBV2) DeregisterTargets(*elbv2.DeregisterTargetsInput) (*elbv2.DeregisterTargetsOutput, error) { +// CreateTargetGroup is not implemented but is required for interface +// conformance +func (elb *FakeELBV2) CreateTargetGroup(*elbv2.CreateTargetGroupInput) (*elbv2.CreateTargetGroupOutput, error) { panic("Not implemented") } -func (self *FakeELBV2) CreateListener(*elbv2.CreateListenerInput) (*elbv2.CreateListenerOutput, error) { - panic("Not implemented") -} -func (self *FakeELBV2) DescribeListeners(*elbv2.DescribeListenersInput) (*elbv2.DescribeListenersOutput, error) { - panic("Not implemented") -} -func (self *FakeELBV2) DeleteListener(*elbv2.DeleteListenerInput) (*elbv2.DeleteListenerOutput, error) { - panic("Not implemented") -} -func (self *FakeELBV2) ModifyListener(*elbv2.ModifyListenerInput) (*elbv2.ModifyListenerOutput, error) { +// DescribeTargetGroups is not implemented but is required for interface +// conformance +func (elb *FakeELBV2) DescribeTargetGroups(*elbv2.DescribeTargetGroupsInput) (*elbv2.DescribeTargetGroupsOutput, error) { panic("Not implemented") } -func (self *FakeELBV2) WaitUntilLoadBalancersDeleted(*elbv2.DescribeLoadBalancersInput) error { +// ModifyTargetGroup is not implemented but is required for interface +// conformance +func (elb *FakeELBV2) ModifyTargetGroup(*elbv2.ModifyTargetGroupInput) (*elbv2.ModifyTargetGroupOutput, error) { panic("Not implemented") } +// DeleteTargetGroup is not implemented but is required for interface +// conformance +func (elb *FakeELBV2) DeleteTargetGroup(*elbv2.DeleteTargetGroupInput) (*elbv2.DeleteTargetGroupOutput, error) { + panic("Not implemented") +} + +// DescribeTargetHealth is not implemented but is required for interface +// conformance +func (elb *FakeELBV2) DescribeTargetHealth(input *elbv2.DescribeTargetHealthInput) (*elbv2.DescribeTargetHealthOutput, error) { + panic("Not implemented") +} + +// DescribeTargetGroupAttributes is not implemented but is required for +// interface conformance +func (elb *FakeELBV2) DescribeTargetGroupAttributes(*elbv2.DescribeTargetGroupAttributesInput) (*elbv2.DescribeTargetGroupAttributesOutput, error) { + panic("Not implemented") +} + +// ModifyTargetGroupAttributes is not implemented but is required for interface +// conformance +func (elb *FakeELBV2) ModifyTargetGroupAttributes(*elbv2.ModifyTargetGroupAttributesInput) (*elbv2.ModifyTargetGroupAttributesOutput, error) { + panic("Not implemented") +} + +// RegisterTargets is not implemented but is required for interface conformance +func (elb *FakeELBV2) RegisterTargets(*elbv2.RegisterTargetsInput) (*elbv2.RegisterTargetsOutput, error) { + panic("Not implemented") +} + +// DeregisterTargets is not implemented but is required for interface +// conformance +func (elb *FakeELBV2) DeregisterTargets(*elbv2.DeregisterTargetsInput) (*elbv2.DeregisterTargetsOutput, error) { + panic("Not implemented") +} + +// CreateListener is not implemented but is required for interface conformance +func (elb *FakeELBV2) CreateListener(*elbv2.CreateListenerInput) (*elbv2.CreateListenerOutput, error) { + panic("Not implemented") +} + +// DescribeListeners is not implemented but is required for interface +// conformance +func (elb *FakeELBV2) DescribeListeners(*elbv2.DescribeListenersInput) (*elbv2.DescribeListenersOutput, error) { + panic("Not implemented") +} + +// DeleteListener is not implemented but is required for interface conformance +func (elb *FakeELBV2) DeleteListener(*elbv2.DeleteListenerInput) (*elbv2.DeleteListenerOutput, error) { + panic("Not implemented") +} + +// ModifyListener is not implemented but is required for interface conformance +func (elb *FakeELBV2) ModifyListener(*elbv2.ModifyListenerInput) (*elbv2.ModifyListenerOutput, error) { + panic("Not implemented") +} + +// WaitUntilLoadBalancersDeleted is not implemented but is required for +// interface conformance +func (elb *FakeELBV2) WaitUntilLoadBalancersDeleted(*elbv2.DescribeLoadBalancersInput) error { + panic("Not implemented") +} + +// FakeASG is a fake Autoscaling client used for testing type FakeASG struct { aws *FakeAWSServices } +// UpdateAutoScalingGroup is not implemented but is required for interface +// conformance func (a *FakeASG) UpdateAutoScalingGroup(*autoscaling.UpdateAutoScalingGroupInput) (*autoscaling.UpdateAutoScalingGroupOutput, error) { panic("Not implemented") } +// DescribeAutoScalingGroups is not implemented but is required for interface +// conformance func (a *FakeASG) DescribeAutoScalingGroups(*autoscaling.DescribeAutoScalingGroupsInput) (*autoscaling.DescribeAutoScalingGroupsOutput, error) { panic("Not implemented") } +// FakeKMS is a fake KMS client used for testing type FakeKMS struct { aws *FakeAWSServices } +// DescribeKey is not implemented but is required for interface conformance func (kms *FakeKMS) DescribeKey(*kms.DescribeKeyInput) (*kms.DescribeKeyOutput, error) { panic("Not implemented") } diff --git a/pkg/cloudprovider/providers/aws/aws_instancegroups.go b/pkg/cloudprovider/providers/aws/aws_instancegroups.go index 6bf63ae6fe..df6f3084dd 100644 --- a/pkg/cloudprovider/providers/aws/aws_instancegroups.go +++ b/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) diff --git a/pkg/cloudprovider/providers/aws/aws_loadbalancer.go b/pkg/cloudprovider/providers/aws/aws_loadbalancer.go index 25583ca569..4cc69a439e 100644 --- a/pkg/cloudprovider/providers/aws/aws_loadbalancer.go +++ b/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) } diff --git a/pkg/cloudprovider/providers/aws/aws_test.go b/pkg/cloudprovider/providers/aws/aws_test.go index 6e86f406c5..72a799357e 100644 --- a/pkg/cloudprovider/providers/aws/aws_test.go +++ b/pkg/cloudprovider/providers/aws/aws_test.go @@ -38,7 +38,7 @@ import ( kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis" ) -const TestClusterId = "clusterid.test" +const TestClusterID = "clusterid.test" const TestClusterName = "testCluster" type MockedFakeEC2 struct { @@ -46,10 +46,10 @@ type MockedFakeEC2 struct { mock.Mock } -func (m *MockedFakeEC2) expectDescribeSecurityGroups(clusterId, groupName, clusterID string) { +func (m *MockedFakeEC2) expectDescribeSecurityGroups(clusterID, groupName string) { tags := []*ec2.Tag{ - {Key: aws.String(TagNameKubernetesClusterLegacy), Value: aws.String(clusterId)}, - {Key: aws.String(fmt.Sprintf("%s%s", TagNameKubernetesClusterPrefix, clusterId)), Value: aws.String(ResourceLifecycleOwned)}, + {Key: aws.String(TagNameKubernetesClusterLegacy), Value: aws.String(clusterID)}, + {Key: aws.String(fmt.Sprintf("%s%s", TagNameKubernetesClusterPrefix, clusterID)), Value: aws.String(ResourceLifecycleOwned)}, } m.On("DescribeSecurityGroups", &ec2.DescribeSecurityGroupsInput{Filters: []*ec2.Filter{ @@ -139,17 +139,17 @@ func TestReadAWSCloudConfig(t *testing.T) { }, { "No zone in config, metadata does not have zone", - strings.NewReader("[global]\n"), newMockedFakeAWSServices(TestClusterId).WithAz(""), + strings.NewReader("[global]\n"), newMockedFakeAWSServices(TestClusterID).WithAz(""), true, "", }, { "No zone in config, metadata has zone", - strings.NewReader("[global]\n"), newMockedFakeAWSServices(TestClusterId), + strings.NewReader("[global]\n"), newMockedFakeAWSServices(TestClusterID), false, "us-east-1a", }, { "Zone in config should take precedence over metadata", - strings.NewReader("[global]\nzone = eu-west-1a"), newMockedFakeAWSServices(TestClusterId), + strings.NewReader("[global]\nzone = eu-west-1a"), newMockedFakeAWSServices(TestClusterID), false, "eu-west-1a", }, } @@ -192,24 +192,24 @@ func TestNewAWSCloud(t *testing.T) { }{ { "No config reader", - nil, newMockedFakeAWSServices(TestClusterId).WithAz(""), + nil, newMockedFakeAWSServices(TestClusterID).WithAz(""), true, "", }, { "Config specifies valid zone", - strings.NewReader("[global]\nzone = eu-west-1a"), newMockedFakeAWSServices(TestClusterId), + strings.NewReader("[global]\nzone = eu-west-1a"), newMockedFakeAWSServices(TestClusterID), false, "eu-west-1", }, { "Gets zone from metadata when not in config", strings.NewReader("[global]\n"), - newMockedFakeAWSServices(TestClusterId), + newMockedFakeAWSServices(TestClusterID), false, "us-east-1", }, { "No zone in config or metadata", strings.NewReader("[global]\n"), - newMockedFakeAWSServices(TestClusterId).WithAz(""), + newMockedFakeAWSServices(TestClusterID).WithAz(""), true, "", }, } @@ -237,7 +237,7 @@ func TestNewAWSCloud(t *testing.T) { } func mockInstancesResp(selfInstance *ec2.Instance, instances []*ec2.Instance) (*Cloud, *FakeAWSServices) { - awsServices := newMockedFakeAWSServices(TestClusterId) + awsServices := newMockedFakeAWSServices(TestClusterID) awsServices.instances = instances awsServices.selfInstance = selfInstance awsCloud, err := newAWSCloud(CloudConfig{}, awsServices) @@ -248,7 +248,7 @@ func mockInstancesResp(selfInstance *ec2.Instance, instances []*ec2.Instance) (* } func mockAvailabilityZone(availabilityZone string) *Cloud { - awsServices := newMockedFakeAWSServices(TestClusterId).WithAz(availabilityZone) + awsServices := newMockedFakeAWSServices(TestClusterID).WithAz(availabilityZone) awsCloud, err := newAWSCloud(CloudConfig{}, awsServices) if err != nil { panic(err) @@ -275,7 +275,7 @@ func TestNodeAddresses(t *testing.T) { // ClusterID needs to be set var tag ec2.Tag tag.Key = aws.String(TagNameKubernetesClusterLegacy) - tag.Value = aws.String(TestClusterId) + tag.Value = aws.String(TestClusterID) tags := []*ec2.Tag{&tag} //0 @@ -364,7 +364,7 @@ func TestNodeAddressesWithMetadata(t *testing.T) { // ClusterID needs to be set var tag ec2.Tag tag.Key = aws.String(TagNameKubernetesClusterLegacy) - tag.Value = aws.String(TestClusterId) + tag.Value = aws.String(TestClusterID) tags := []*ec2.Tag{&tag} instanceName := "instance.ec2.internal" @@ -412,7 +412,7 @@ func TestGetRegion(t *testing.T) { } func TestFindVPCID(t *testing.T) { - awsServices := newMockedFakeAWSServices(TestClusterId) + awsServices := newMockedFakeAWSServices(TestClusterID) c, err := newAWSCloud(CloudConfig{}, awsServices) if err != nil { t.Errorf("Error building aws cloud: %v", err) @@ -486,7 +486,7 @@ func constructRouteTable(subnetID string, public bool) *ec2.RouteTable { } func TestSubnetIDsinVPC(t *testing.T) { - awsServices := newMockedFakeAWSServices(TestClusterId) + awsServices := newMockedFakeAWSServices(TestClusterID) c, err := newAWSCloud(CloudConfig{}, awsServices) if err != nil { t.Errorf("Error building aws cloud: %v", err) @@ -532,13 +532,13 @@ func TestSubnetIDsinVPC(t *testing.T) { return } - result_set := make(map[string]bool) + resultSet := make(map[string]bool) for _, v := range result { - result_set[v] = true + resultSet[v] = true } for i := range subnets { - if !result_set[subnets[i]["id"]] { + if !resultSet[subnets[i]["id"]] { t.Errorf("Expected subnet%d '%s' in result: %v", i, subnets[i]["id"], result) return } @@ -562,13 +562,13 @@ func TestSubnetIDsinVPC(t *testing.T) { return } - result_set = make(map[string]bool) + resultSet = make(map[string]bool) for _, v := range result { - result_set[v] = true + resultSet[v] = true } for i := range subnets { - if !result_set[subnets[i]["id"]] { + if !resultSet[subnets[i]["id"]] { t.Errorf("Expected subnet%d '%s' in result: %v", i, subnets[i]["id"], result) return } @@ -665,7 +665,7 @@ func TestSubnetIDsinVPC(t *testing.T) { } func TestIpPermissionExistsHandlesMultipleGroupIds(t *testing.T) { - oldIpPermission := ec2.IpPermission{ + oldIPPermission := ec2.IpPermission{ UserIdGroupPairs: []*ec2.UserIdGroupPair{ {GroupId: aws.String("firstGroupId")}, {GroupId: aws.String("secondGroupId")}, @@ -673,36 +673,36 @@ func TestIpPermissionExistsHandlesMultipleGroupIds(t *testing.T) { }, } - existingIpPermission := ec2.IpPermission{ + existingIPPermission := ec2.IpPermission{ UserIdGroupPairs: []*ec2.UserIdGroupPair{ {GroupId: aws.String("secondGroupId")}, }, } - newIpPermission := ec2.IpPermission{ + newIPPermission := ec2.IpPermission{ UserIdGroupPairs: []*ec2.UserIdGroupPair{ {GroupId: aws.String("fourthGroupId")}, }, } - equals := ipPermissionExists(&existingIpPermission, &oldIpPermission, false) + equals := ipPermissionExists(&existingIPPermission, &oldIPPermission, false) if !equals { t.Errorf("Should have been considered equal since first is in the second array of groups") } - equals = ipPermissionExists(&newIpPermission, &oldIpPermission, false) + equals = ipPermissionExists(&newIPPermission, &oldIPPermission, false) if equals { t.Errorf("Should have not been considered equal since first is not in the second array of groups") } // The first pair matches, but the second does not - newIpPermission2 := ec2.IpPermission{ + newIPPermission2 := ec2.IpPermission{ UserIdGroupPairs: []*ec2.UserIdGroupPair{ {GroupId: aws.String("firstGroupId")}, {GroupId: aws.String("fourthGroupId")}, }, } - equals = ipPermissionExists(&newIpPermission2, &oldIpPermission, false) + equals = ipPermissionExists(&newIPPermission2, &oldIPPermission, false) if equals { t.Errorf("Should have not been considered equal since first is not in the second array of groups") } @@ -710,9 +710,9 @@ func TestIpPermissionExistsHandlesMultipleGroupIds(t *testing.T) { func TestIpPermissionExistsHandlesRangeSubsets(t *testing.T) { // Two existing scenarios we'll test against - emptyIpPermission := ec2.IpPermission{} + emptyIPPermission := ec2.IpPermission{} - oldIpPermission := ec2.IpPermission{ + oldIPPermission := ec2.IpPermission{ IpRanges: []*ec2.IpRange{ {CidrIp: aws.String("10.0.0.0/8")}, {CidrIp: aws.String("192.168.1.0/24")}, @@ -720,53 +720,53 @@ func TestIpPermissionExistsHandlesRangeSubsets(t *testing.T) { } // Two already existing ranges and a new one - existingIpPermission := ec2.IpPermission{ + existingIPPermission := ec2.IpPermission{ IpRanges: []*ec2.IpRange{ {CidrIp: aws.String("10.0.0.0/8")}, }, } - existingIpPermission2 := ec2.IpPermission{ + existingIPPermission2 := ec2.IpPermission{ IpRanges: []*ec2.IpRange{ {CidrIp: aws.String("192.168.1.0/24")}, }, } - newIpPermission := ec2.IpPermission{ + newIPPermission := ec2.IpPermission{ IpRanges: []*ec2.IpRange{ {CidrIp: aws.String("172.16.0.0/16")}, }, } - exists := ipPermissionExists(&emptyIpPermission, &emptyIpPermission, false) + exists := ipPermissionExists(&emptyIPPermission, &emptyIPPermission, false) if !exists { t.Errorf("Should have been considered existing since we're comparing a range array against itself") } - exists = ipPermissionExists(&oldIpPermission, &oldIpPermission, false) + exists = ipPermissionExists(&oldIPPermission, &oldIPPermission, false) if !exists { t.Errorf("Should have been considered existing since we're comparing a range array against itself") } - exists = ipPermissionExists(&existingIpPermission, &oldIpPermission, false) + exists = ipPermissionExists(&existingIPPermission, &oldIPPermission, false) if !exists { - t.Errorf("Should have been considered existing since 10.* is in oldIpPermission's array of ranges") + t.Errorf("Should have been considered existing since 10.* is in oldIPPermission's array of ranges") } - exists = ipPermissionExists(&existingIpPermission2, &oldIpPermission, false) + exists = ipPermissionExists(&existingIPPermission2, &oldIPPermission, false) if !exists { t.Errorf("Should have been considered existing since 192.* is in oldIpPermission2's array of ranges") } - exists = ipPermissionExists(&newIpPermission, &emptyIpPermission, false) + exists = ipPermissionExists(&newIPPermission, &emptyIPPermission, false) if exists { t.Errorf("Should have not been considered existing since we compared against a missing array of ranges") } - exists = ipPermissionExists(&newIpPermission, &oldIpPermission, false) + exists = ipPermissionExists(&newIPPermission, &oldIPPermission, false) if exists { - t.Errorf("Should have not been considered existing since 172.* is not in oldIpPermission's array of ranges") + t.Errorf("Should have not been considered existing since 172.* is not in oldIPPermission's array of ranges") } } func TestIpPermissionExistsHandlesMultipleGroupIdsWithUserIds(t *testing.T) { - oldIpPermission := ec2.IpPermission{ + oldIPPermission := ec2.IpPermission{ UserIdGroupPairs: []*ec2.UserIdGroupPair{ {GroupId: aws.String("firstGroupId"), UserId: aws.String("firstUserId")}, {GroupId: aws.String("secondGroupId"), UserId: aws.String("secondUserId")}, @@ -774,24 +774,24 @@ func TestIpPermissionExistsHandlesMultipleGroupIdsWithUserIds(t *testing.T) { }, } - existingIpPermission := ec2.IpPermission{ + existingIPPermission := ec2.IpPermission{ UserIdGroupPairs: []*ec2.UserIdGroupPair{ {GroupId: aws.String("secondGroupId"), UserId: aws.String("secondUserId")}, }, } - newIpPermission := ec2.IpPermission{ + newIPPermission := ec2.IpPermission{ UserIdGroupPairs: []*ec2.UserIdGroupPair{ {GroupId: aws.String("secondGroupId"), UserId: aws.String("anotherUserId")}, }, } - equals := ipPermissionExists(&existingIpPermission, &oldIpPermission, true) + equals := ipPermissionExists(&existingIPPermission, &oldIPPermission, true) if !equals { t.Errorf("Should have been considered equal since first is in the second array of groups") } - equals = ipPermissionExists(&newIpPermission, &oldIpPermission, true) + equals = ipPermissionExists(&newIPPermission, &oldIPPermission, true) if equals { t.Errorf("Should have not been considered equal since first is not in the second array of groups") } @@ -810,13 +810,13 @@ func TestFindInstanceByNodeNameExcludesTerminatedInstances(t *testing.T) { {64, ec2.InstanceStateNameStopping, true}, {80, ec2.InstanceStateNameStopped, true}, } - awsServices := newMockedFakeAWSServices(TestClusterId) + awsServices := newMockedFakeAWSServices(TestClusterID) nodeName := types.NodeName("my-dns.internal") var tag ec2.Tag tag.Key = aws.String(TagNameKubernetesClusterLegacy) - tag.Value = aws.String(TestClusterId) + tag.Value = aws.String(TestClusterID) tags := []*ec2.Tag{&tag} var testInstance ec2.Instance @@ -858,11 +858,11 @@ func TestFindInstanceByNodeNameExcludesTerminatedInstances(t *testing.T) { } func TestGetInstanceByNodeNameBatching(t *testing.T) { - awsServices := newMockedFakeAWSServices(TestClusterId) + awsServices := newMockedFakeAWSServices(TestClusterID) c, err := newAWSCloud(CloudConfig{}, awsServices) assert.Nil(t, err, "Error building aws cloud: %v", err) var tag ec2.Tag - tag.Key = aws.String(TagNameKubernetesClusterPrefix + TestClusterId) + tag.Key = aws.String(TagNameKubernetesClusterPrefix + TestClusterID) tag.Value = aws.String("") tags := []*ec2.Tag{&tag} nodeNames := []string{} @@ -870,8 +870,8 @@ func TestGetInstanceByNodeNameBatching(t *testing.T) { nodeName := fmt.Sprintf("ip-171-20-42-%d.ec2.internal", i) nodeNames = append(nodeNames, nodeName) ec2Instance := &ec2.Instance{} - instanceId := fmt.Sprintf("i-abcedf%d", i) - ec2Instance.InstanceId = aws.String(instanceId) + instanceID := fmt.Sprintf("i-abcedf%d", i) + ec2Instance.InstanceId = aws.String(instanceID) ec2Instance.PrivateDnsName = aws.String(nodeName) ec2Instance.State = &ec2.InstanceState{Code: aws.Int64(48), Name: aws.String("running")} ec2Instance.Tags = tags @@ -886,19 +886,19 @@ func TestGetInstanceByNodeNameBatching(t *testing.T) { } func TestGetVolumeLabels(t *testing.T) { - awsServices := newMockedFakeAWSServices(TestClusterId) + awsServices := newMockedFakeAWSServices(TestClusterID) c, err := newAWSCloud(CloudConfig{}, awsServices) assert.Nil(t, err, "Error building aws cloud: %v", err) - volumeId := awsVolumeID("vol-VolumeId") - expectedVolumeRequest := &ec2.DescribeVolumesInput{VolumeIds: []*string{volumeId.awsString()}} + volumeID := EBSVolumeID("vol-VolumeId") + expectedVolumeRequest := &ec2.DescribeVolumesInput{VolumeIds: []*string{volumeID.awsString()}} awsServices.ec2.(*MockedFakeEC2).On("DescribeVolumes", expectedVolumeRequest).Return([]*ec2.Volume{ { - VolumeId: volumeId.awsString(), + VolumeId: volumeID.awsString(), AvailabilityZone: aws.String("us-east-1a"), }, }) - labels, err := c.GetVolumeLabels(KubernetesVolumeID("aws:///" + string(volumeId))) + labels, err := c.GetVolumeLabels(KubernetesVolumeID("aws:///" + string(volumeID))) assert.Nil(t, err, "Error creating Volume %v", err) assert.Equal(t, map[string]string{ @@ -908,7 +908,7 @@ func TestGetVolumeLabels(t *testing.T) { } func TestDescribeLoadBalancerOnDelete(t *testing.T) { - awsServices := newMockedFakeAWSServices(TestClusterId) + awsServices := newMockedFakeAWSServices(TestClusterID) c, _ := newAWSCloud(CloudConfig{}, awsServices) awsServices.elb.(*MockedFakeELB).expectDescribeLoadBalancers("aid") @@ -916,7 +916,7 @@ func TestDescribeLoadBalancerOnDelete(t *testing.T) { } func TestDescribeLoadBalancerOnUpdate(t *testing.T) { - awsServices := newMockedFakeAWSServices(TestClusterId) + awsServices := newMockedFakeAWSServices(TestClusterID) c, _ := newAWSCloud(CloudConfig{}, awsServices) awsServices.elb.(*MockedFakeELB).expectDescribeLoadBalancers("aid") @@ -924,7 +924,7 @@ func TestDescribeLoadBalancerOnUpdate(t *testing.T) { } func TestDescribeLoadBalancerOnGet(t *testing.T) { - awsServices := newMockedFakeAWSServices(TestClusterId) + awsServices := newMockedFakeAWSServices(TestClusterID) c, _ := newAWSCloud(CloudConfig{}, awsServices) awsServices.elb.(*MockedFakeELB).expectDescribeLoadBalancers("aid") @@ -932,7 +932,7 @@ func TestDescribeLoadBalancerOnGet(t *testing.T) { } func TestDescribeLoadBalancerOnEnsure(t *testing.T) { - awsServices := newMockedFakeAWSServices(TestClusterId) + awsServices := newMockedFakeAWSServices(TestClusterID) c, _ := newAWSCloud(CloudConfig{}, awsServices) awsServices.elb.(*MockedFakeELB).expectDescribeLoadBalancers("aid") @@ -1164,7 +1164,7 @@ func TestGetLoadBalancerAdditionalTags(t *testing.T) { } func TestLBExtraSecurityGroupsAnnotation(t *testing.T) { - awsServices := newMockedFakeAWSServices(TestClusterId) + awsServices := newMockedFakeAWSServices(TestClusterID) c, _ := newAWSCloud(CloudConfig{}, awsServices) sg1 := map[string]string{ServiceAnnotationLoadBalancerExtraSecurityGroups: "sg-000001"} @@ -1183,7 +1183,7 @@ func TestLBExtraSecurityGroupsAnnotation(t *testing.T) { {"Multiple SGs specified", sg3, []string{sg1[ServiceAnnotationLoadBalancerExtraSecurityGroups], sg2[ServiceAnnotationLoadBalancerExtraSecurityGroups]}}, } - awsServices.ec2.(*MockedFakeEC2).expectDescribeSecurityGroups(TestClusterId, "k8s-elb-aid", "cluster.test") + awsServices.ec2.(*MockedFakeEC2).expectDescribeSecurityGroups(TestClusterID, "k8s-elb-aid") for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -1199,7 +1199,7 @@ func TestLBExtraSecurityGroupsAnnotation(t *testing.T) { } func TestLBSecurityGroupsAnnotation(t *testing.T) { - awsServices := newMockedFakeAWSServices(TestClusterId) + awsServices := newMockedFakeAWSServices(TestClusterID) c, _ := newAWSCloud(CloudConfig{}, awsServices) sg1 := map[string]string{ServiceAnnotationLoadBalancerSecurityGroups: "sg-000001"} @@ -1216,7 +1216,7 @@ func TestLBSecurityGroupsAnnotation(t *testing.T) { {"Multiple SGs specified", sg3, []string{sg1[ServiceAnnotationLoadBalancerSecurityGroups], sg2[ServiceAnnotationLoadBalancerSecurityGroups]}}, } - awsServices.ec2.(*MockedFakeEC2).expectDescribeSecurityGroups(TestClusterId, "k8s-elb-aid", "cluster.test") + awsServices.ec2.(*MockedFakeEC2).expectDescribeSecurityGroups(TestClusterID, "k8s-elb-aid") for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -1233,7 +1233,7 @@ func TestLBSecurityGroupsAnnotation(t *testing.T) { // Test that we can add a load balancer tag func TestAddLoadBalancerTags(t *testing.T) { loadBalancerName := "test-elb" - awsServices := newMockedFakeAWSServices(TestClusterId) + awsServices := newMockedFakeAWSServices(TestClusterID) c, _ := newAWSCloud(CloudConfig{}, awsServices) want := make(map[string]string) @@ -1289,7 +1289,7 @@ func TestEnsureLoadBalancerHealthCheck(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - awsServices := newMockedFakeAWSServices(TestClusterId) + awsServices := newMockedFakeAWSServices(TestClusterID) c, err := newAWSCloud(CloudConfig{}, awsServices) assert.Nil(t, err, "Error building aws cloud: %v", err) expectedHC := *defaultHC @@ -1307,7 +1307,7 @@ func TestEnsureLoadBalancerHealthCheck(t *testing.T) { } t.Run("does not make an API call if the current health check is the same", func(t *testing.T) { - awsServices := newMockedFakeAWSServices(TestClusterId) + awsServices := newMockedFakeAWSServices(TestClusterID) c, err := newAWSCloud(CloudConfig{}, awsServices) assert.Nil(t, err, "Error building aws cloud: %v", err) expectedHC := *defaultHC @@ -1329,7 +1329,7 @@ func TestEnsureLoadBalancerHealthCheck(t *testing.T) { }) t.Run("validates resulting expected health check before making an API call", func(t *testing.T) { - awsServices := newMockedFakeAWSServices(TestClusterId) + awsServices := newMockedFakeAWSServices(TestClusterID) c, err := newAWSCloud(CloudConfig{}, awsServices) assert.Nil(t, err, "Error building aws cloud: %v", err) expectedHC := *defaultHC @@ -1345,7 +1345,7 @@ func TestEnsureLoadBalancerHealthCheck(t *testing.T) { }) t.Run("handles invalid override values", func(t *testing.T) { - awsServices := newMockedFakeAWSServices(TestClusterId) + awsServices := newMockedFakeAWSServices(TestClusterID) c, err := newAWSCloud(CloudConfig{}, awsServices) assert.Nil(t, err, "Error building aws cloud: %v", err) annotations := map[string]string{ServiceAnnotationLoadBalancerHCTimeout: "3.3"} @@ -1357,7 +1357,7 @@ func TestEnsureLoadBalancerHealthCheck(t *testing.T) { }) t.Run("returns error when updating the health check fails", func(t *testing.T) { - awsServices := newMockedFakeAWSServices(TestClusterId) + awsServices := newMockedFakeAWSServices(TestClusterID) c, err := newAWSCloud(CloudConfig{}, awsServices) assert.Nil(t, err, "Error building aws cloud: %v", err) returnErr := fmt.Errorf("throttling error") diff --git a/pkg/cloudprovider/providers/aws/device_allocator.go b/pkg/cloudprovider/providers/aws/device_allocator.go index 490b673bb1..ae5b027577 100644 --- a/pkg/cloudprovider/providers/aws/device_allocator.go +++ b/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'} { diff --git a/pkg/cloudprovider/providers/aws/regions_test.go b/pkg/cloudprovider/providers/aws/regions_test.go index 03fb8ff16a..3210dcd85b 100644 --- a/pkg/cloudprovider/providers/aws/regions_test.go +++ b/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) diff --git a/pkg/cloudprovider/providers/aws/retry_handler.go b/pkg/cloudprovider/providers/aws/retry_handler.go index f403168e62..d6b382ccc3 100644 --- a/pkg/cloudprovider/providers/aws/retry_handler.go +++ b/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() diff --git a/pkg/cloudprovider/providers/aws/sets_ippermissions.go b/pkg/cloudprovider/providers/aws/sets_ippermissions.go index 8a268c4b0e..d71948f8d2 100644 --- a/pkg/cloudprovider/providers/aws/sets_ippermissions.go +++ b/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 diff --git a/pkg/cloudprovider/providers/aws/tags.go b/pkg/cloudprovider/providers/aws/tags.go index 43130c3601..8e44cbb5bd 100644 --- a/pkg/cloudprovider/providers/aws/tags.go +++ b/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) } diff --git a/pkg/cloudprovider/providers/aws/tags_test.go b/pkg/cloudprovider/providers/aws/tags_test.go index a8b55b8029..1de98a24a4 100644 --- a/pkg/cloudprovider/providers/aws/tags_test.go +++ b/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, diff --git a/pkg/cloudprovider/providers/aws/volumes.go b/pkg/cloudprovider/providers/aws/volumes.go index cb3a2aa275..4a082528f9 100644 --- a/pkg/cloudprovider/providers/aws/volumes.go +++ b/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 +// 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 awsVolumeID) awsString() *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() diff --git a/pkg/volume/awsebs/aws_util.go b/pkg/volume/awsebs/aws_util.go index 5a10d4d549..50fc701efe 100644 --- a/pkg/volume/awsebs/aws_util.go +++ b/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: