diff --git a/pkg/cloudprovider/aws/aws.go b/pkg/cloudprovider/aws/aws.go index c6a9d099ebf..67dad251cd4 100644 --- a/pkg/cloudprovider/aws/aws.go +++ b/pkg/cloudprovider/aws/aws.go @@ -235,7 +235,11 @@ func (self *AWSCloud) AddSSHKeyToAllInstances(user string, keyData []byte) error } func (a *AWSCloud) CurrentNodeName(hostname string) (string, error) { - return hostname, nil + selfInstance, err := a.getSelfAWSInstance() + if err != nil { + return "", err + } + return selfInstance.awsID, nil } // Implementation of EC2.Instances @@ -551,7 +555,7 @@ func (aws *AWSCloud) Routes() (cloudprovider.Routes, bool) { // NodeAddresses is an implementation of Instances.NodeAddresses. func (aws *AWSCloud) NodeAddresses(name string) ([]api.NodeAddress, error) { - instance, err := aws.getInstanceByDnsName(name) + instance, err := aws.getInstanceById(name) if err != nil { return nil, err } @@ -585,7 +589,8 @@ func (aws *AWSCloud) NodeAddresses(name string) ([]api.NodeAddress, error) { // ExternalID returns the cloud provider ID of the specified instance (deprecated). func (aws *AWSCloud) ExternalID(name string) (string, error) { - inst, err := aws.getInstanceByDnsName(name) + // TODO: Do we need to verify it exists, or can we just return name + inst, err := aws.getInstanceById(name) if err != nil { return "", err } @@ -594,7 +599,8 @@ func (aws *AWSCloud) ExternalID(name string) (string, error) { // InstanceID returns the cloud provider ID of the specified instance. func (aws *AWSCloud) InstanceID(name string) (string, error) { - inst, err := aws.getInstanceByDnsName(name) + // TODO: Do we need to verify it exists, or can we just construct it knowing our AZ (or via caching?) + inst, err := aws.getInstanceById(name) if err != nil { return "", err } @@ -603,46 +609,6 @@ func (aws *AWSCloud) InstanceID(name string) (string, error) { return "/" + orEmpty(inst.Placement.AvailabilityZone) + "/" + orEmpty(inst.InstanceID), nil } -// Return the instances matching the relevant private dns name. -func (s *AWSCloud) getInstanceByDnsName(name string) (*ec2.Instance, error) { - filters := []*ec2.Filter{ - newEc2Filter("private-dns-name", name), - } - filters = s.addFilters(filters) - request := &ec2.DescribeInstancesInput{ - Filters: filters, - } - - instances, err := s.ec2.DescribeInstances(request) - if err != nil { - return nil, err - } - - matchingInstances := []*ec2.Instance{} - for _, instance := range instances { - // TODO: Push running logic down into filter? - if !isAlive(instance) { - continue - } - - if orEmpty(instance.PrivateDNSName) != name { - // TODO: Should we warn here? - the filter should have caught this - // (this will happen in the tests if they don't fully mock the EC2 API) - continue - } - - matchingInstances = append(matchingInstances, instance) - } - - if len(matchingInstances) == 0 { - return nil, fmt.Errorf("no instances found for host: %s", name) - } - if len(matchingInstances) > 1 { - return nil, fmt.Errorf("multiple instances found for host: %s", name) - } - return matchingInstances[0], nil -} - // Check if the instance is alive (running or pending) // We typically ignore instances that are not alive func isAlive(instance *ec2.Instance) bool { @@ -702,16 +668,9 @@ func (s *AWSCloud) getInstancesByRegex(regex string) ([]string, error) { continue } - privateDNSName := orEmpty(instance.PrivateDNSName) - if privateDNSName == "" { - glog.V(2).Infof("skipping EC2 instance (no PrivateDNSName): %s", - orEmpty(instance.InstanceID)) - continue - } - for _, tag := range instance.Tags { if orEmpty(tag.Key) == "Name" && re.MatchString(orEmpty(tag.Value)) { - matchingInstances = append(matchingInstances, privateDNSName) + matchingInstances = append(matchingInstances, orEmpty(instance.InstanceID)) break } } @@ -728,7 +687,7 @@ func (aws *AWSCloud) List(filter string) ([]string, error) { // GetNodeResources implements Instances.GetNodeResources func (aws *AWSCloud) GetNodeResources(name string) (*api.NodeResources, error) { - instance, err := aws.getInstanceByDnsName(name) + instance, err := aws.getInstanceById(name) if err != nil { return nil, err } @@ -1189,7 +1148,7 @@ func (aws *AWSCloud) getAwsInstance(instanceName string) (*awsInstance, error) { return nil, fmt.Errorf("error getting self-instance: %v", err) } } else { - instance, err := aws.getInstanceByDnsName(instanceName) + instance, err := aws.getInstanceById(instanceName) if err != nil { return nil, fmt.Errorf("error finding instance: %v", err) } @@ -1656,7 +1615,7 @@ func (s *AWSCloud) CreateTCPLoadBalancer(name, region string, publicIP net.IP, p return nil, fmt.Errorf("publicIP cannot be specified for AWS ELB") } - instances, err := s.getInstancesByDnsNames(hosts) + instances, err := s.getInstancesByIds(hosts) if err != nil { return nil, err } @@ -2068,7 +2027,7 @@ func (s *AWSCloud) EnsureTCPLoadBalancerDeleted(name, region string) error { // UpdateTCPLoadBalancer implements TCPLoadBalancer.UpdateTCPLoadBalancer func (s *AWSCloud) UpdateTCPLoadBalancer(name, region string, hosts []string) error { - instances, err := s.getInstancesByDnsNames(hosts) + instances, err := s.getInstancesByIds(hosts) if err != nil { return err } @@ -2143,21 +2102,40 @@ func (s *AWSCloud) UpdateTCPLoadBalancer(name, region string, hosts []string) er } // TODO: Make efficient -func (a *AWSCloud) getInstancesByDnsNames(names []string) ([]*ec2.Instance, error) { +func (a *AWSCloud) getInstancesByIds(ids []string) ([]*ec2.Instance, error) { instances := []*ec2.Instance{} - for _, name := range names { - instance, err := a.getInstanceByDnsName(name) + 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 " + name) + 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{ + InstanceIDs: []*string{&instanceID}, + } + + instances, err := a.ec2.DescribeInstances(request) + 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 +} + // Add additional filters, to match on our tags // This lets us run multiple k8s clusters in a single EC2 AZ func (s *AWSCloud) addFilters(filters []*ec2.Filter) []*ec2.Filter { diff --git a/pkg/cloudprovider/aws/aws_test.go b/pkg/cloudprovider/aws/aws_test.go index 469b8e352da..60462a74155 100644 --- a/pkg/cloudprovider/aws/aws_test.go +++ b/pkg/cloudprovider/aws/aws_test.go @@ -412,7 +412,7 @@ func TestList(t *testing.T) { Value: aws.String("foo"), } instance0.Tags = []*ec2.Tag{&tag0} - instance0.PrivateDNSName = aws.String("instance1") + instance0.InstanceID = aws.String("instance0") state0 := ec2.InstanceState{ Name: aws.String("running"), } @@ -424,7 +424,7 @@ func TestList(t *testing.T) { Value: aws.String("bar"), } instance1.Tags = []*ec2.Tag{&tag1} - instance1.PrivateDNSName = aws.String("instance2") + instance1.InstanceID = aws.String("instance1") state1 := ec2.InstanceState{ Name: aws.String("running"), } @@ -436,7 +436,7 @@ func TestList(t *testing.T) { Value: aws.String("baz"), } instance2.Tags = []*ec2.Tag{&tag2} - instance2.PrivateDNSName = aws.String("instance3") + instance2.InstanceID = aws.String("instance2") state2 := ec2.InstanceState{ Name: aws.String("running"), } @@ -448,7 +448,7 @@ func TestList(t *testing.T) { Value: aws.String("quux"), } instance3.Tags = []*ec2.Tag{&tag3} - instance3.PrivateDNSName = aws.String("instance4") + instance3.InstanceID = aws.String("instance3") state3 := ec2.InstanceState{ Name: aws.String("running"), } @@ -462,8 +462,8 @@ func TestList(t *testing.T) { expect []string }{ {"blahonga", []string{}}, - {"quux", []string{"instance4"}}, - {"a", []string{"instance2", "instance3"}}, + {"quux", []string{"instance3"}}, + {"a", []string{"instance1", "instance2"}}, } for _, item := range table { @@ -493,7 +493,7 @@ func TestNodeAddresses(t *testing.T) { var instance1 ec2.Instance //0 - instance0.PrivateDNSName = aws.String("instance1") + instance0.InstanceID = aws.String("instance-same") instance0.PrivateIPAddress = aws.String("192.168.0.1") instance0.PublicIPAddress = aws.String("1.2.3.4") instance0.InstanceType = aws.String("c3.large") @@ -503,7 +503,7 @@ func TestNodeAddresses(t *testing.T) { instance0.State = &state0 //1 - instance1.PrivateDNSName = aws.String("instance1") + instance1.InstanceID = aws.String("instance-same") instance1.PrivateIPAddress = aws.String("192.168.0.2") instance1.InstanceType = aws.String("c3.large") state1 := ec2.InstanceState{ @@ -514,19 +514,19 @@ func TestNodeAddresses(t *testing.T) { instances := []*ec2.Instance{&instance0, &instance1} aws1 := mockInstancesResp([]*ec2.Instance{}) - _, err1 := aws1.NodeAddresses("instance") + _, err1 := aws1.NodeAddresses("instance-mismatch") if err1 == nil { t.Errorf("Should error when no instance found") } aws2 := mockInstancesResp(instances) - _, err2 := aws2.NodeAddresses("instance1") + _, err2 := aws2.NodeAddresses("instance-same") if err2 == nil { t.Errorf("Should error when multiple instances found") } aws3 := mockInstancesResp(instances[0:1]) - addrs3, err3 := aws3.NodeAddresses("instance1") + addrs3, err3 := aws3.NodeAddresses("instance-same") if err3 != nil { t.Errorf("Should not error when instance found") } @@ -562,7 +562,7 @@ func TestGetResources(t *testing.T) { var instance2 ec2.Instance //0 - instance0.PrivateDNSName = aws.String("m3.medium") + instance0.InstanceID = aws.String("m3.medium") instance0.InstanceType = aws.String("m3.medium") state0 := ec2.InstanceState{ Name: aws.String("running"), @@ -570,7 +570,7 @@ func TestGetResources(t *testing.T) { instance0.State = &state0 //1 - instance1.PrivateDNSName = aws.String("r3.8xlarge") + instance1.InstanceID = aws.String("r3.8xlarge") instance1.InstanceType = aws.String("r3.8xlarge") state1 := ec2.InstanceState{ Name: aws.String("running"), @@ -578,7 +578,7 @@ func TestGetResources(t *testing.T) { instance1.State = &state1 //2 - instance2.PrivateDNSName = aws.String("unknown.type") + instance2.InstanceID = aws.String("unknown.type") instance2.InstanceType = aws.String("unknown.type") state2 := ec2.InstanceState{ Name: aws.String("running"),