optimize ListRoutes to fetch instances only once per call

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

pkg/cloudprovider/providers/aws/aws_routes.go

@@ -56,18 +56,36 @@ 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)
-		destinationCIDR := orEmpty(r.DestinationCidrBlock)
 
-		if instanceID == "" || destinationCIDR == "" {
+		if instanceID == "" {
 			continue
 		}
 
-		instance, err := s.getInstanceById(instanceID)
+		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)
+
+		if instanceID == "" || destinationCIDR == "" {
+			continue
+		}
+
+		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
 		routes = append(routes, &cloudprovider.Route{Name: routeName, TargetInstance: instanceName, DestinationCIDR: destinationCIDR})