Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AWS: findInstancesByNodeNames should not make O(N) API calls #19874

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 41 additions & 8 deletions pkg/cloudprovider/providers/aws/aws.go
Expand Up @@ -2146,22 +2146,55 @@ func (a *AWSCloud) getInstancesByIDs(instanceIDs []*string) (map[string]*ec2.Ins
return instancesByID, nil
}

// TODO: Make efficient
// Fetches instances by node names; returns an error if any cannot be found.
// This is currently implemented by fetching all the instances, because this is currently called for all nodes (i.e. the majority)
// In practice, the breakeven vs looping through and calling getInstanceByNodeName is probably around N=2.
func (a *AWSCloud) getInstancesByNodeNames(nodeNames []string) ([]*ec2.Instance, error) {
instances := []*ec2.Instance{}
for _, nodeName := range nodeNames {
instance, err := a.getInstanceByNodeName(nodeName)
if err != nil {
return nil, err
allInstances, err := a.getAllInstances()
if err != nil {
return nil, err
}

nodeNamesMap := make(map[string]int, len(nodeNames))
for i, nodeName := range nodeNames {
nodeNamesMap[nodeName] = i
}

instances := make([]*ec2.Instance, len(nodeNames))
for _, instance := range allInstances {
nodeName := aws.StringValue(instance.PrivateDnsName)
if nodeName == "" {
glog.V(2).Infof("ignoring ec2 instance with no PrivateDnsName: %q", aws.StringValue(instance.InstanceId))
continue
}
i, found := nodeNamesMap[nodeName]
if !found {
continue
}
instances[i] = instance
}

for i, instance := range instances {
if instance == nil {
return nil, fmt.Errorf("unable to find instance " + nodeName)
nodeName := nodeNames[i]
return nil, fmt.Errorf("unable to find instance %q", nodeName)
}
instances = append(instances, instance)
}

return instances, nil
}

// Returns all instances that are tagged as being in this cluster.
func (a *AWSCloud) getAllInstances() ([]*ec2.Instance, error) {
filters := []*ec2.Filter{}
filters = a.addFilters(filters)
request := &ec2.DescribeInstancesInput{
Filters: filters,
}

return a.ec2.DescribeInstances(request)
}

// Returns the instance with the specified node name
// Returns nil if it does not exist
func (a *AWSCloud) findInstanceByNodeName(nodeName string) (*ec2.Instance, error) {
Expand Down