Merge pull request #14879 from tpounds/aws-cloud-cleanup

pkg/cloudprovider/providers/aws/aws.go

@@ -91,8 +91,6 @@ type EC2 interface {
 	AuthorizeSecurityGroupIngress(*ec2.AuthorizeSecurityGroupIngressInput) (*ec2.AuthorizeSecurityGroupIngressOutput, error)
 	RevokeSecurityGroupIngress(*ec2.RevokeSecurityGroupIngressInput) (*ec2.RevokeSecurityGroupIngressOutput, error)
 
-	DescribeVPCs(*ec2.DescribeVpcsInput) ([]*ec2.Vpc, error)
-
 	DescribeSubnets(*ec2.DescribeSubnetsInput) ([]*ec2.Subnet, error)
 
 	CreateTags(*ec2.CreateTagsInput) (*ec2.CreateTagsOutput, error)
@@ -172,7 +170,6 @@ type InstanceGroupInfo interface {
 
 // AWSCloud is an implementation of Interface, TCPLoadBalancer and Instances for Amazon Web Services.
 type AWSCloud struct {
-	awsServices      AWSServices
 	ec2              EC2
 	elb              ELB
 	asg              ASG
@@ -378,15 +375,6 @@ func (s *awsSdkEC2) DeleteVolume(volumeID string) (resp *ec2.DeleteVolumeOutput,
 	return s.ec2.DeleteVolume(&request)
 }
 
-func (s *awsSdkEC2) DescribeVPCs(request *ec2.DescribeVpcsInput) ([]*ec2.Vpc, error) {
-	// VPCs are not paged
-	response, err := s.ec2.DescribeVpcs(request)
-	if err != nil {
-		return nil, fmt.Errorf("error listing AWS VPCs: %v", err)
-	}
-	return response.Vpcs, nil
-}
-
 func (s *awsSdkEC2) DescribeSubnets(request *ec2.DescribeSubnetsInput) ([]*ec2.Subnet, error) {
 	// Subnets are not paged
 	response, err := s.ec2.DescribeSubnets(request)
@@ -544,7 +532,6 @@ func newAWSCloud(config io.Reader, awsServices AWSServices) (*AWSCloud, error) {
 	}
 
 	awsCloud := &AWSCloud{
-		awsServices:      awsServices,
 		ec2:              ec2,
 		elb:              elb,
 		asg:              asg,
@@ -757,10 +744,6 @@ func (aws *AWSCloud) List(filter string) ([]string, error) {
 
 // GetZone implements Zones.GetZone
 func (self *AWSCloud) GetZone() (cloudprovider.Zone, error) {
-	if self.availabilityZone == "" {
-		// Should be unreachable
-		panic("availabilityZone not set")
-	}
 	return cloudprovider.Zone{
 		FailureDomain: self.availabilityZone,
 		Region:        self.region,
@@ -1258,35 +1241,6 @@ func (self *AWSCloud) findVPCID() (string, error) {
 	return "", fmt.Errorf("Could not find VPC ID in instance metadata")
 }
 
-// Find the VPC which self is attached to.
-func (self *AWSCloud) findVPC() (*ec2.Vpc, error) {
-	request := &ec2.DescribeVpcsInput{}
-
-	// find by vpcID from metadata
-	vpcID, err := self.findVPCID()
-	if err != nil {
-		return nil, err
-	}
-	filters := []*ec2.Filter{newEc2Filter("vpc-id", vpcID)}
-	// Don't bother adding the filterTags as we know this VPC is valid for this instance from findVPCID above.
-	// This is important as sharing a single regional VPC with multiple per-AZ clusters is a common deployment.
-	request.Filters = filters
-
-	vpcs, err := self.ec2.DescribeVPCs(request)
-	if err != nil {
-		glog.Error("error listing VPCs", err)
-		return nil, err
-	}
-
-	if len(vpcs) == 0 {
-		return nil, nil
-	}
-	if len(vpcs) == 1 {
-		return vpcs[0], nil
-	}
-	return nil, fmt.Errorf("Found multiple matching VPCs for vpcID = %s", vpcID)
-}
-
 // Retrieves the specified security group from the AWS API, or returns nil if not found
 func (s *AWSCloud) findSecurityGroup(securityGroupId string) (*ec2.SecurityGroup, error) {
 	describeSecurityGroupsRequest := &ec2.DescribeSecurityGroupsInput{
@@ -1574,13 +1528,13 @@ func (s *AWSCloud) createTags(request *ec2.CreateTagsInput) (*ec2.CreateTagsOutp
 	}
 }
 
-func (s *AWSCloud) listSubnetIDsinVPC(vpc *ec2.Vpc) ([]string, error) {
+func (s *AWSCloud) listSubnetIDsinVPC(vpcId string) ([]string, error) {
 
 	subnetIds := []string{}
 
 	request := &ec2.DescribeSubnetsInput{}
 	filters := []*ec2.Filter{}
-	filters = append(filters, newEc2Filter("vpc-id", orEmpty(vpc.VpcId)))
+	filters = append(filters, newEc2Filter("vpc-id", vpcId))
 	// Note, this will only return subnets tagged with the cluster identifier for this Kubernetes cluster.
 	// In the case where an AZ has public & private subnets per AWS best practices, the deployment should ensure
 	// only the public subnet (where the ELB will go) is so tagged.
@@ -1631,17 +1585,13 @@ func (s *AWSCloud) EnsureTCPLoadBalancer(name, region string, publicIP net.IP, p
 		return nil, err
 	}
 
-	vpc, err := s.findVPC()
+	vpcId, err := s.findVPCID()
 	if err != nil {
-		glog.Error("error finding VPC", err)
 		return nil, err
 	}
-	if vpc == nil {
-		return nil, fmt.Errorf("Unable to find VPC")
-	}
 
 	// Construct list of configured subnets
-	subnetIDs, err := s.listSubnetIDsinVPC(vpc)
+	subnetIDs, err := s.listSubnetIDsinVPC(vpcId)
 	if err != nil {
 		glog.Error("error listing subnets in VPC", err)
 		return nil, err
@@ -1652,7 +1602,7 @@ func (s *AWSCloud) EnsureTCPLoadBalancer(name, region string, publicIP net.IP, p
 	{
 		sgName := "k8s-elb-" + name
 		sgDescription := "Security group for Kubernetes ELB " + name
-		securityGroupID, err = s.ensureSecurityGroup(sgName, sgDescription, orEmpty(vpc.VpcId))
+		securityGroupID, err = s.ensureSecurityGroup(sgName, sgDescription, vpcId)
 		if err != nil {
 			glog.Error("Error creating load balancer security group: ", err)
 			return nil, err
@@ -2021,22 +1971,6 @@ func (s *AWSCloud) UpdateTCPLoadBalancer(name, region string, hosts []string) er
 	return nil
 }
 
-// TODO: Make efficient
-func (a *AWSCloud) getInstancesByIds(ids []string) ([]*ec2.Instance, error) {
-	instances := []*ec2.Instance{}
-	for _, id := range ids {
-		instance, err := a.getInstanceById(id)
-		if err != nil {
-			return nil, err
-		}
-		if instance == nil {
-			return nil, fmt.Errorf("unable to find instance " + id)
-		}
-		instances = append(instances, instance)
-	}
-	return instances, nil
-}
-
 // Returns the instance with the specified ID
 func (a *AWSCloud) getInstanceById(instanceID string) (*ec2.Instance, error) {
 	request := &ec2.DescribeInstancesInput{

pkg/cloudprovider/providers/aws/aws_test.go

@@ -383,10 +383,6 @@ func (ec2 *FakeEC2) RevokeSecurityGroupIngress(*ec2.RevokeSecurityGroupIngressIn
 	panic("Not implemented")
 }
 
-func (ec2 *FakeEC2) DescribeVPCs(*ec2.DescribeVpcsInput) ([]*ec2.Vpc, error) {
-	panic("Not implemented")
-}
-
 func (ec2 *FakeEC2) DescribeSubnets(request *ec2.DescribeSubnetsInput) ([]*ec2.Subnet, error) {
 	ec2.DescribeSubnetsInput = request
 	return ec2.Subnets, nil
@@ -474,7 +470,6 @@ func (a *FakeASG) DescribeAutoScalingGroups(*autoscaling.DescribeAutoScalingGrou
 func mockInstancesResp(instances []*ec2.Instance) *AWSCloud {
 	awsServices := NewFakeAWSServices().withInstances(instances)
 	return &AWSCloud{
-		awsServices:      awsServices,
 		ec2:              awsServices.ec2,
 		availabilityZone: awsServices.availabilityZone,
 	}
@@ -483,7 +478,6 @@ func mockInstancesResp(instances []*ec2.Instance) *AWSCloud {
 func mockAvailabilityZone(region string, availabilityZone string) *AWSCloud {
 	awsServices := NewFakeAWSServices().withAz(availabilityZone)
 	return &AWSCloud{
-		awsServices:      awsServices,
 		ec2:              awsServices.ec2,
 		availabilityZone: awsServices.availabilityZone,
 		region:           region,
@@ -730,9 +724,6 @@ func TestSubnetIDsinVPC(t *testing.T) {
 	}
 
 	vpcID := "vpc-deadbeef"
-	vpc := &ec2.Vpc{
-		VpcId: &vpcID,
-	}
 
 	// test with 3 subnets from 3 different AZs
 	subnets := make(map[int]map[string]string)
@@ -747,7 +738,7 @@ func TestSubnetIDsinVPC(t *testing.T) {
 	subnets[2]["az"] = "af-south-1c"
 	awsServices.ec2.Subnets = constructSubnets(subnets)
 
-	result, err := c.listSubnetIDsinVPC(vpc)
+	result, err := c.listSubnetIDsinVPC(vpcID)
 	if err != nil {
 		t.Errorf("Error listing subnets: %v", err)
 		return
@@ -777,7 +768,7 @@ func TestSubnetIDsinVPC(t *testing.T) {
 	subnets[3]["az"] = "af-south-1c"
 	awsServices.ec2.Subnets = constructSubnets(subnets)
 
-	result, err = c.listSubnetIDsinVPC(vpc)
+	result, err = c.listSubnetIDsinVPC(vpcID)
 	if err != nil {
 		t.Errorf("Error listing subnets: %v", err)
 		return