From 24c44e7a8eec0806c25967ba712c8c80828fa853 Mon Sep 17 00:00:00 2001
From: Daniel Schonfeld <daniel@schonfeld.org>
Date: Tue, 5 Jan 2016 12:33:14 -0500
Subject: [PATCH] optimize ListRoutes to fetch instances only once per call

Issue #12121 - fixes courtesy of @justinsb - thank you
---
 pkg/cloudprovider/providers/aws/aws.go        | 40 +++++++++++++++----
 pkg/cloudprovider/providers/aws/aws_routes.go | 24 +++++++++--
 2 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go
index 4023b776759..4fba5177c19 100644
--- a/pkg/cloudprovider/providers/aws/aws.go
+++ b/pkg/cloudprovider/providers/aws/aws.go
@@ -2104,22 +2104,48 @@ func (s *AWSCloud) UpdateTCPLoadBalancer(name, region string, hosts []string) er
 }
 
 // Returns the instance with the specified ID
-func (a *AWSCloud) getInstanceById(instanceID string) (*ec2.Instance, error) {
-	request := &ec2.DescribeInstancesInput{
-		InstanceIds: []*string{&instanceID},
-	}
-
-	instances, err := a.ec2.DescribeInstances(request)
+// This function is currently unused, but seems very likely to be needed again
+func (a *AWSCloud) getInstanceByID(instanceID string) (*ec2.Instance, error) {
+	instances, err := a.getInstancesByIDs([]*string{&instanceID})
 	if err != nil {
 		return nil, err
 	}
+
 	if len(instances) == 0 {
 		return nil, fmt.Errorf("no instances found for instance: %s", instanceID)
 	}
 	if len(instances) > 1 {
 		return nil, fmt.Errorf("multiple instances found for instance: %s", instanceID)
 	}
-	return instances[0], nil
+
+	return instances[instanceID], nil
+}
+
+func (a *AWSCloud) getInstancesByIDs(instanceIDs []*string) (map[string]*ec2.Instance, error) {
+	instancesByID := make(map[string]*ec2.Instance)
+	if len(instanceIDs) == 0 {
+		return instancesByID, nil
+	}
+
+	request := &ec2.DescribeInstancesInput{
+		InstanceIds: instanceIDs,
+	}
+
+	instances, err := a.ec2.DescribeInstances(request)
+	if err != nil {
+		return nil, err
+	}
+
+	for _, instance := range instances {
+		instanceID := orEmpty(instance.InstanceId)
+		if instanceID == "" {
+			continue
+		}
+
+		instancesByID[instanceID] = instance
+	}
+
+	return instancesByID, nil
 }
 
 // TODO: Make efficient
diff --git a/pkg/cloudprovider/providers/aws/aws_routes.go b/pkg/cloudprovider/providers/aws/aws_routes.go
index 7564831af51..c1ffddc3a25 100644
--- a/pkg/cloudprovider/providers/aws/aws_routes.go
+++ b/pkg/cloudprovider/providers/aws/aws_routes.go
@@ -56,6 +56,23 @@ func (s *AWSCloud) ListRoutes(clusterName string) ([]*cloudprovider.Route, error
 	}
 
 	var routes []*cloudprovider.Route
+	var instanceIDs []*string
+
+	for _, r := range table.Routes {
+		instanceID := orEmpty(r.InstanceId)
+
+		if instanceID == "" {
+			continue
+		}
+
+		instanceIDs = append(instanceIDs, &instanceID)
+	}
+
+	instances, err := s.getInstancesByIDs(instanceIDs)
+	if err != nil {
+		return nil, err
+	}
+
 	for _, r := range table.Routes {
 		instanceID := orEmpty(r.InstanceId)
 		destinationCIDR := orEmpty(r.DestinationCidrBlock)
@@ -64,9 +81,10 @@ func (s *AWSCloud) ListRoutes(clusterName string) ([]*cloudprovider.Route, error
 			continue
 		}
 
-		instance, err := s.getInstanceById(instanceID)
-		if err != nil {
-			return nil, err
+		instance, found := instances[instanceID]
+		if !found {
+			glog.Warningf("unable to find instance ID %s in the list of instances being routed to", instanceID)
+			continue
 		}
 		instanceName := orEmpty(instance.PrivateDnsName)
 		routeName := clusterName + "-" + destinationCIDR