AWS: Use the instance id as the node name

The EC2 instance id is the canonical node name on EC2.

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 {
+	for _, id := range ids {
-		instance, err := a.getInstanceByDnsName(name)
+		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 {

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"}},
+		{"quux", []string{"instance3"}},
-		{"a", []string{"instance2", "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"),