diff --git a/pkg/cloudprovider/aws/aws.go b/pkg/cloudprovider/aws/aws.go index 30d302f5050..623ce0bf94e 100644 --- a/pkg/cloudprovider/aws/aws.go +++ b/pkg/cloudprovider/aws/aws.go @@ -278,7 +278,7 @@ func (a *AWSCloud) CurrentNodeName(hostname string) (string, error) { if err != nil { return "", err } - return selfInstance.awsID, nil + return selfInstance.nodeName, nil } // Implementation of EC2.Instances @@ -624,7 +624,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.getInstanceById(name) + instance, err := aws.getInstanceByNodeName(name) if err != nil { return nil, err } @@ -660,7 +660,7 @@ func (aws *AWSCloud) NodeAddresses(name string) ([]api.NodeAddress, error) { // Note that if the instance does not exist or is no longer running, we must return ("", cloudprovider.InstanceNotFound) func (aws *AWSCloud) ExternalID(name string) (string, error) { // We must verify that the instance still exists - instance, err := aws.getInstanceById(name) + instance, err := aws.getInstanceByNodeName(name) if err != nil { return "", err } @@ -673,7 +673,7 @@ 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) { // 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) + inst, err := aws.getInstanceByNodeName(name) if err != nil { return "", err } @@ -741,9 +741,16 @@ 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, orEmpty(instance.InstanceID)) + matchingInstances = append(matchingInstances, privateDNSName) break } } @@ -760,7 +767,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.getInstanceById(name) + instance, err := aws.getInstanceByNodeName(name) if err != nil { return nil, err } @@ -968,6 +975,9 @@ type awsInstance struct { // id in AWS awsID string + // node name in k8s + nodeName string + mutex sync.Mutex // We must cache because otherwise there is a race condition, @@ -975,8 +985,8 @@ type awsInstance struct { deviceMappings map[string]string } -func newAWSInstance(ec2 EC2, awsID string) *awsInstance { - self := &awsInstance{ec2: ec2, awsID: awsID} +func newAWSInstance(ec2 EC2, awsID, nodeName string) *awsInstance { + self := &awsInstance{ec2: ec2, awsID: awsID, nodeName: nodeName} // We lazy-init deviceMappings self.deviceMappings = nil @@ -1216,29 +1226,34 @@ func (s *AWSCloud) getSelfAWSInstance() (*awsInstance, error) { if err != nil { return nil, fmt.Errorf("error fetching instance-id from ec2 metadata service: %v", err) } - i = newAWSInstance(s.ec2, string(instanceIdBytes)) + privateDnsNameBytes, err := metadata.GetMetaData("local-hostname") + if err != nil { + return nil, fmt.Errorf("error fetching local-hostname from ec2 metadata service: %v", err) + } + + i = newAWSInstance(s.ec2, string(instanceIdBytes), string(privateDnsNameBytes)) s.selfAWSInstance = i } return i, nil } -// Gets the awsInstance named instanceName, or the 'self' instance if instanceName == "" -func (aws *AWSCloud) getAwsInstance(instanceName string) (*awsInstance, error) { +// Gets the awsInstance with node-name nodeName, or the 'self' instance if nodeName == "" +func (aws *AWSCloud) getAwsInstance(nodeName string) (*awsInstance, error) { var awsInstance *awsInstance var err error - if instanceName == "" { + if nodeName == "" { awsInstance, err = aws.getSelfAWSInstance() if err != nil { return nil, fmt.Errorf("error getting self-instance: %v", err) } } else { - instance, err := aws.getInstanceById(instanceName) + instance, err := aws.getInstanceByNodeName(nodeName) if err != nil { - return nil, fmt.Errorf("error finding instance: %v", err) + return nil, fmt.Errorf("error finding instance %s: %v", nodeName, err) } - awsInstance = newAWSInstance(aws.ec2, orEmpty(instance.InstanceID)) + awsInstance = newAWSInstance(aws.ec2, orEmpty(instance.InstanceID), orEmpty(instance.PrivateDNSName)) } return awsInstance, nil @@ -1700,7 +1715,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.getInstancesByIds(hosts) + instances, err := s.getInstancesByNodeNames(hosts) if err != nil { return nil, err } @@ -2112,7 +2127,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.getInstancesByIds(hosts) + instances, err := s.getInstancesByNodeNames(hosts) if err != nil { return err } @@ -2221,6 +2236,45 @@ func (a *AWSCloud) getInstanceById(instanceID string) (*ec2.Instance, error) { return instances[0], nil } +// TODO: Make efficient +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 + } + if instance == nil { + return nil, fmt.Errorf("unable to find instance " + nodeName) + } + instances = append(instances, instance) + } + return instances, nil +} + +// Returns the instance with the specified node name +func (a *AWSCloud) getInstanceByNodeName(nodeName string) (*ec2.Instance, error) { + filters := []*ec2.Filter{ + newEc2Filter("private-dns-name", nodeName), + } + filters = a.addFilters(filters) + request := &ec2.DescribeInstancesInput{ + Filters: filters, + } + + instances, err := a.ec2.DescribeInstances(request) + if err != nil { + return nil, err + } + if len(instances) == 0 { + return nil, fmt.Errorf("no instances found for name: %s", nodeName) + } + if len(instances) > 1 { + return nil, fmt.Errorf("multiple instances found for name: %s", nodeName) + } + 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_routes.go b/pkg/cloudprovider/aws/aws_routes.go index c28cd8a471f..3666fcab94a 100644 --- a/pkg/cloudprovider/aws/aws_routes.go +++ b/pkg/cloudprovider/aws/aws_routes.go @@ -85,9 +85,14 @@ func (s *AWSCloud) configureInstanceSourceDestCheck(instanceID string, sourceDes // CreateRoute implements Routes.CreateRoute // Create the described route func (s *AWSCloud) CreateRoute(clusterName string, nameHint string, route *cloudprovider.Route) error { + instance, err := s.getInstanceByNodeName(route.TargetInstance) + if err != nil { + return err + } + // In addition to configuring the route itself, we also need to configure the instance to accept that traffic // On AWS, this requires turning source-dest checks off - err := s.configureInstanceSourceDestCheck(route.TargetInstance, false) + err = s.configureInstanceSourceDestCheck(orEmpty(instance.InstanceID), false) if err != nil { return err } @@ -100,7 +105,7 @@ func (s *AWSCloud) CreateRoute(clusterName string, nameHint string, route *cloud request := &ec2.CreateRouteInput{} // TODO: use ClientToken for idempotency? request.DestinationCIDRBlock = aws.String(route.DestinationCIDR) - request.InstanceID = aws.String(route.TargetInstance) + request.InstanceID = instance.InstanceID request.RouteTableID = table.RouteTableID _, err = s.ec2.CreateRoute(request) diff --git a/pkg/cloudprovider/aws/aws_test.go b/pkg/cloudprovider/aws/aws_test.go index 5fd13daa4e7..c765ac1c69f 100644 --- a/pkg/cloudprovider/aws/aws_test.go +++ b/pkg/cloudprovider/aws/aws_test.go @@ -108,6 +108,7 @@ type FakeAWSServices struct { availabilityZone string instances []*ec2.Instance instanceId string + privateDnsName string ec2 *FakeEC2 elb *FakeELB @@ -124,8 +125,10 @@ func NewFakeAWSServices() *FakeAWSServices { s.metadata = &FakeMetadata{aws: s} s.instanceId = "i-self" + s.privateDnsName = "ip-172-20-0-100.ec2.internal" var selfInstance ec2.Instance selfInstance.InstanceID = &s.instanceId + selfInstance.PrivateDNSName = &s.privateDnsName s.instances = []*ec2.Instance{&selfInstance} var tag ec2.Tag @@ -310,6 +313,8 @@ func (self *FakeMetadata) GetMetaData(key string) ([]byte, error) { return []byte(self.aws.availabilityZone), nil } else if key == "instance-id" { return []byte(self.aws.instanceId), nil + } else if key == "local-hostname" { + return []byte(self.aws.privateDnsName), nil } else { return nil, nil } @@ -448,6 +453,7 @@ func TestList(t *testing.T) { } instance0.Tags = []*ec2.Tag{&tag0} instance0.InstanceID = aws.String("instance0") + instance0.PrivateDNSName = aws.String("instance0.ec2.internal") state0 := ec2.InstanceState{ Name: aws.String("running"), } @@ -460,6 +466,7 @@ func TestList(t *testing.T) { } instance1.Tags = []*ec2.Tag{&tag1} instance1.InstanceID = aws.String("instance1") + instance1.PrivateDNSName = aws.String("instance1.ec2.internal") state1 := ec2.InstanceState{ Name: aws.String("running"), } @@ -472,6 +479,7 @@ func TestList(t *testing.T) { } instance2.Tags = []*ec2.Tag{&tag2} instance2.InstanceID = aws.String("instance2") + instance2.PrivateDNSName = aws.String("instance2.ec2.internal") state2 := ec2.InstanceState{ Name: aws.String("running"), } @@ -484,6 +492,7 @@ func TestList(t *testing.T) { } instance3.Tags = []*ec2.Tag{&tag3} instance3.InstanceID = aws.String("instance3") + instance3.PrivateDNSName = aws.String("instance3.ec2.internal") state3 := ec2.InstanceState{ Name: aws.String("running"), } @@ -497,8 +506,8 @@ func TestList(t *testing.T) { expect []string }{ {"blahonga", []string{}}, - {"quux", []string{"instance3"}}, - {"a", []string{"instance1", "instance2"}}, + {"quux", []string{"instance3.ec2.internal"}}, + {"a", []string{"instance1.ec2.internal", "instance2.ec2.internal"}}, } for _, item := range table { @@ -529,6 +538,7 @@ func TestNodeAddresses(t *testing.T) { //0 instance0.InstanceID = aws.String("instance-same") + instance0.PrivateDNSName = aws.String("instance-same.ec2.internal") instance0.PrivateIPAddress = aws.String("192.168.0.1") instance0.PublicIPAddress = aws.String("1.2.3.4") instance0.InstanceType = aws.String("c3.large") @@ -539,6 +549,7 @@ func TestNodeAddresses(t *testing.T) { //1 instance1.InstanceID = aws.String("instance-same") + instance1.PrivateDNSName = aws.String("instance-same.ec2.internal") instance1.PrivateIPAddress = aws.String("192.168.0.2") instance1.InstanceType = aws.String("c3.large") state1 := ec2.InstanceState{ @@ -549,19 +560,19 @@ func TestNodeAddresses(t *testing.T) { instances := []*ec2.Instance{&instance0, &instance1} aws1 := mockInstancesResp([]*ec2.Instance{}) - _, err1 := aws1.NodeAddresses("instance-mismatch") + _, err1 := aws1.NodeAddresses("instance-mismatch.ec2.internal") if err1 == nil { t.Errorf("Should error when no instance found") } aws2 := mockInstancesResp(instances) - _, err2 := aws2.NodeAddresses("instance-same") + _, err2 := aws2.NodeAddresses("instance-same.ec2.internal") if err2 == nil { t.Errorf("Should error when multiple instances found") } aws3 := mockInstancesResp(instances[0:1]) - addrs3, err3 := aws3.NodeAddresses("instance-same") + addrs3, err3 := aws3.NodeAddresses("instance-same.ec2.internal") if err3 != nil { t.Errorf("Should not error when instance found") } @@ -598,6 +609,7 @@ func TestGetResources(t *testing.T) { //0 instance0.InstanceID = aws.String("m3.medium") + instance0.PrivateDNSName = aws.String("m3-medium.ec2.internal") instance0.InstanceType = aws.String("m3.medium") state0 := ec2.InstanceState{ Name: aws.String("running"), @@ -606,6 +618,7 @@ func TestGetResources(t *testing.T) { //1 instance1.InstanceID = aws.String("r3.8xlarge") + instance1.PrivateDNSName = aws.String("r3-8xlarge.ec2.internal") instance1.InstanceType = aws.String("r3.8xlarge") state1 := ec2.InstanceState{ Name: aws.String("running"), @@ -614,6 +627,7 @@ func TestGetResources(t *testing.T) { //2 instance2.InstanceID = aws.String("unknown.type") + instance2.PrivateDNSName = aws.String("unknown-type.ec2.internal") instance2.InstanceType = aws.String("unknown.type") state2 := ec2.InstanceState{ Name: aws.String("running"), @@ -624,7 +638,7 @@ func TestGetResources(t *testing.T) { aws1 := mockInstancesResp(instances) - res1, err1 := aws1.GetNodeResources("m3.medium") + res1, err1 := aws1.GetNodeResources("m3-medium.ec2.internal") if err1 != nil { t.Errorf("Should not error when instance type found: %v", err1) } @@ -638,7 +652,7 @@ func TestGetResources(t *testing.T) { t.Errorf("Expected %v, got %v", e1, res1) } - res2, err2 := aws1.GetNodeResources("r3.8xlarge") + res2, err2 := aws1.GetNodeResources("r3-8xlarge.ec2.internal") if err2 != nil { t.Errorf("Should not error when instance type found: %v", err2) } @@ -652,7 +666,7 @@ func TestGetResources(t *testing.T) { t.Errorf("Expected %v, got %v", e2, res2) } - res3, err3 := aws1.GetNodeResources("unknown.type") + res3, err3 := aws1.GetNodeResources("unknown-type.ec2.internal") if err3 != nil { t.Errorf("Should not error when unknown instance type") }