From 319c608fddeb3a2b25438f6ee8bc1eeb2c01b0fe Mon Sep 17 00:00:00 2001 From: Matthew Wong Date: Thu, 25 May 2017 16:49:09 -0400 Subject: [PATCH] Get instances of all states in DisksAreAttached, not just "running" --- pkg/cloudprovider/providers/aws/aws.go | 19 ++++++++++--------- pkg/cloudprovider/providers/aws/aws_test.go | 2 +- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index 7c5cac8d352..6e85d9b974d 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -2599,7 +2599,7 @@ func (c *Cloud) EnsureLoadBalancer(clusterName string, apiService *v1.Service, n return nil, fmt.Errorf("LoadBalancerIP cannot be specified for AWS ELB") } - instances, err := c.getInstancesByNodeNamesCached(nodeNames(nodes)) + instances, err := c.getInstancesByNodeNamesCached(nodeNames(nodes), "running") if err != nil { return nil, err } @@ -3157,7 +3157,7 @@ func (c *Cloud) EnsureLoadBalancerDeleted(clusterName string, service *v1.Servic // UpdateLoadBalancer implements LoadBalancer.UpdateLoadBalancer func (c *Cloud) UpdateLoadBalancer(clusterName string, service *v1.Service, nodes []*v1.Node) error { - instances, err := c.getInstancesByNodeNamesCached(nodeNames(nodes)) + instances, err := c.getInstancesByNodeNamesCached(nodeNames(nodes), "running") if err != nil { return err } @@ -3229,10 +3229,11 @@ func (c *Cloud) getInstancesByIDs(instanceIDs []*string) (map[string]*ec2.Instan return instancesByID, nil } -// Fetches and caches instances by node names; returns an error if any cannot be found. +// Fetches and caches instances in the given state, by node names; returns an error if any cannot be found. If no states +// are given, no state filter is used and instances of all states are fetched. // This is implemented with a multi value filter on the node names, fetching the desired instances with a single query. // TODO(therc): make all the caching more rational during the 1.4 timeframe -func (c *Cloud) getInstancesByNodeNamesCached(nodeNames sets.String) ([]*ec2.Instance, error) { +func (c *Cloud) getInstancesByNodeNamesCached(nodeNames sets.String, states ...string) ([]*ec2.Instance, error) { c.mutex.Lock() defer c.mutex.Unlock() if nodeNames.Equal(c.lastNodeNames) { @@ -3243,7 +3244,7 @@ func (c *Cloud) getInstancesByNodeNamesCached(nodeNames sets.String) ([]*ec2.Ins return c.lastInstancesByNodeNames, nil } } - instances, err := c.getInstancesByNodeNames(nodeNames.List()) + instances, err := c.getInstancesByNodeNames(nodeNames.List(), states...) if err != nil { return nil, err @@ -3259,7 +3260,7 @@ func (c *Cloud) getInstancesByNodeNamesCached(nodeNames sets.String) ([]*ec2.Ins return instances, nil } -func (c *Cloud) getInstancesByNodeNames(nodeNames []string) ([]*ec2.Instance, error) { +func (c *Cloud) getInstancesByNodeNames(nodeNames []string, states ...string) ([]*ec2.Instance, error) { names := aws.StringSlice(nodeNames) nodeNameFilter := &ec2.Filter{ @@ -3267,9 +3268,9 @@ func (c *Cloud) getInstancesByNodeNames(nodeNames []string) ([]*ec2.Instance, er Values: names, } - filters := []*ec2.Filter{ - nodeNameFilter, - newEc2Filter("instance-state-name", "running"), + filters := []*ec2.Filter{nodeNameFilter} + if len(states) > 0 { + filters = append(filters, newEc2Filter("instance-state-name", states...)) } instances, err := c.describeInstances(filters) diff --git a/pkg/cloudprovider/providers/aws/aws_test.go b/pkg/cloudprovider/providers/aws/aws_test.go index ff45d403710..9d059dbdb16 100644 --- a/pkg/cloudprovider/providers/aws/aws_test.go +++ b/pkg/cloudprovider/providers/aws/aws_test.go @@ -1050,7 +1050,7 @@ func TestFindInstancesByNodeNameCached(t *testing.T) { } nodeNames := sets.NewString(nodeNameOne) - returnedInstances, errr := c.getInstancesByNodeNamesCached(nodeNames) + returnedInstances, errr := c.getInstancesByNodeNamesCached(nodeNames, "running") if errr != nil { t.Errorf("Failed to find instance: %v", err)