diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index 31f9a28caf4..994d671ecfb 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -204,7 +204,6 @@ type Services interface { type EC2 interface { // Query EC2 for instances matching the filter DescribeInstances(request *ec2.DescribeInstancesInput) ([]*ec2.Instance, error) - DescribeAddresses(request *ec2.DescribeAddressesInput) ([]*ec2.Address, error) // Attach a volume to an instance AttachVolume(*ec2.AttachVolumeInput) (*ec2.VolumeAttachment, error) @@ -609,20 +608,6 @@ func (s *awsSdkEC2) DescribeInstances(request *ec2.DescribeInstancesInput) ([]*e return results, nil } -// Implementation of EC2.DescribeAddresses -func (s *awsSdkEC2) DescribeAddresses(request *ec2.DescribeAddressesInput) ([]*ec2.Address, error) { - requestTime := time.Now() - response, err := s.ec2.DescribeAddresses(request) - if err != nil { - recordAwsMetric("describe_address", 0, err) - return nil, fmt.Errorf("error listing AWS addresses: %v", err) - } - - timeTaken := time.Since(requestTime).Seconds() - recordAwsMetric("describe_address", timeTaken, nil) - return response.Addresses, nil -} - // Implements EC2.DescribeSecurityGroups func (s *awsSdkEC2) DescribeSecurityGroups(request *ec2.DescribeSecurityGroupsInput) ([]*ec2.SecurityGroup, error) { // Security groups are not paged @@ -996,38 +981,51 @@ func (c *Cloud) NodeAddresses(name types.NodeName) ([]v1.NodeAddress, error) { return addresses, nil } + instance, err := c.getInstanceByNodeName(name) if err != nil { return nil, fmt.Errorf("getInstanceByNodeName failed for %q with %v", name, err) } + return extractNodeAddresses(instance) +} + +// extractNodeAddresses maps the instance information from EC2 to an array of NodeAddresses +func extractNodeAddresses(instance *ec2.Instance) ([]v1.NodeAddress, error) { + // Not clear if the order matters here, but we might as well indicate a sensible preference order + + if instance == nil { + return nil, fmt.Errorf("nil instance passed to extractNodeAddresses") + } addresses := []v1.NodeAddress{} - if !isNilOrEmpty(instance.PrivateIpAddress) { - ipAddress := *instance.PrivateIpAddress - ip := net.ParseIP(ipAddress) + privateIPAddress := aws.StringValue(instance.PrivateIpAddress) + if privateIPAddress != "" { + ip := net.ParseIP(privateIPAddress) if ip == nil { - return nil, fmt.Errorf("EC2 instance had invalid private address: %s (%s)", orEmpty(instance.InstanceId), ipAddress) + return nil, fmt.Errorf("EC2 instance had invalid private address: %s (%s)", aws.StringValue(instance.InstanceId), privateIPAddress) } addresses = append(addresses, v1.NodeAddress{Type: v1.NodeInternalIP, Address: ip.String()}) } // TODO: Other IP addresses (multiple ips)? - if !isNilOrEmpty(instance.PublicIpAddress) { - ipAddress := *instance.PublicIpAddress - ip := net.ParseIP(ipAddress) + publicIPAddress := aws.StringValue(instance.PublicIpAddress) + if publicIPAddress != "" { + ip := net.ParseIP(publicIPAddress) if ip == nil { - return nil, fmt.Errorf("EC2 instance had invalid public address: %s (%s)", orEmpty(instance.InstanceId), ipAddress) + return nil, fmt.Errorf("EC2 instance had invalid public address: %s (%s)", aws.StringValue(instance.InstanceId), publicIPAddress) } addresses = append(addresses, v1.NodeAddress{Type: v1.NodeExternalIP, Address: ip.String()}) } - if !isNilOrEmpty(instance.PrivateDnsName) { - addresses = append(addresses, v1.NodeAddress{Type: v1.NodeInternalDNS, Address: *instance.PrivateDnsName}) + privateDNSName := aws.StringValue(instance.PrivateDnsName) + if privateDNSName != "" { + addresses = append(addresses, v1.NodeAddress{Type: v1.NodeInternalDNS, Address: privateDNSName}) } - if !isNilOrEmpty(instance.PublicDnsName) { - addresses = append(addresses, v1.NodeAddress{Type: v1.NodeExternalDNS, Address: *instance.PublicDnsName}) + publicDNSName := aws.StringValue(instance.PublicDnsName) + if publicDNSName != "" { + addresses = append(addresses, v1.NodeAddress{Type: v1.NodeExternalDNS, Address: publicDNSName}) } return addresses, nil @@ -1037,42 +1035,17 @@ func (c *Cloud) NodeAddresses(name types.NodeName) ([]v1.NodeAddress, error) { // This method will not be called from the node that is requesting this ID. i.e. metadata service // and other local methods cannot be used here func (c *Cloud) NodeAddressesByProviderID(providerID string) ([]v1.NodeAddress, error) { - instanceID, err := instanceIDFromProviderID(providerID) + instanceID, err := kubernetesInstanceID(providerID).mapToAWSInstanceID() if err != nil { return nil, err } - addresses, err := c.describeAddressesByInstanceID(instanceID) + instance, err := describeInstance(c.ec2, instanceID) if err != nil { return nil, err } - instances, err := c.describeInstancesByInstanceID(instanceID) - if err != nil { - return nil, err - } - - nodeAddresses := []v1.NodeAddress{} - - for _, address := range addresses { - convertedAddress, err := convertAwsAddress(address) - if err != nil { - return nil, err - } - - nodeAddresses = append(nodeAddresses, convertedAddress...) - } - - for _, instance := range instances { - addresses, err := instanceAddresses(instance) - if err != nil { - return nil, err - } - - nodeAddresses = append(nodeAddresses, addresses...) - } - - return nodeAddresses, nil + return extractNodeAddresses(instance) } // ExternalID returns the cloud provider ID of the node with the specified nodeName (deprecated). @@ -1111,12 +1084,12 @@ func (c *Cloud) InstanceID(nodeName types.NodeName) (string, error) { // This method will not be called from the node that is requesting this ID. i.e. metadata service // and other local methods cannot be used here func (c *Cloud) InstanceTypeByProviderID(providerID string) (string, error) { - instanceID, err := instanceIDFromProviderID(providerID) + instanceID, err := kubernetesInstanceID(providerID).mapToAWSInstanceID() if err != nil { return "", err } - instance, err := c.describeInstanceByInstanceID(instanceID) + instance, err := describeInstance(c.ec2, instanceID) if err != nil { return "", err } @@ -1133,7 +1106,7 @@ func (c *Cloud) InstanceType(nodeName types.NodeName) (string, error) { if err != nil { return "", fmt.Errorf("getInstanceByNodeName failed for %q with %v", nodeName, err) } - return orEmpty(inst.InstanceType), nil + return aws.StringValue(inst.InstanceType), nil } // Return a list of instances matching regex string. @@ -1302,22 +1275,7 @@ func (i *awsInstance) getInstanceType() *awsInstanceType { // Gets the full information about this instance from the EC2 API func (i *awsInstance) describeInstance() (*ec2.Instance, error) { - instanceID := i.awsID - request := &ec2.DescribeInstancesInput{ - InstanceIds: []*string{&instanceID}, - } - - instances, err := i.ec2.DescribeInstances(request) - if err != nil { - return nil, err - } - if len(instances) == 0 { - return nil, fmt.Errorf("no instances found for instance: %s", i.awsID) - } - if len(instances) > 1 { - return nil, fmt.Errorf("multiple instances found for instance: %s", i.awsID) - } - return instances[0], nil + return describeInstance(i.ec2, awsInstanceID(i.awsID)) } // Gets the mountDevice already assigned to the volume, or assigns an unused mountDevice. @@ -3402,25 +3360,6 @@ func (c *Cloud) getInstancesByNodeNames(nodeNames []string, states ...string) ([ return instances, nil } -func (c *Cloud) describeInstancesByInstanceID(instanceID string) ([]*ec2.Instance, error) { - filters := []*ec2.Filter{newEc2Filter("instance-id", instanceID)} - return c.describeInstances(filters) -} - -func (c *Cloud) describeInstanceByInstanceID(instanceID string) (*ec2.Instance, error) { - filters := []*ec2.Filter{newEc2Filter("instance-id", instanceID)} - instances, err := c.describeInstances(filters) - if err != nil { - return nil, err - } - - if len(instances) != 1 { - return nil, fmt.Errorf("expected 1 instance, found %d for instanceID %s", len(instances), instanceID) - } - - return instances[0], nil -} - func (c *Cloud) describeInstances(filters []*ec2.Filter) ([]*ec2.Instance, error) { filters = c.tagging.addFilters(filters) request := &ec2.DescribeInstancesInput{ @@ -3441,20 +3380,6 @@ func (c *Cloud) describeInstances(filters []*ec2.Filter) ([]*ec2.Instance, error return matches, nil } -func (c *Cloud) describeAddressesByInstanceID(instanceID string) ([]*ec2.Address, error) { - filters := []*ec2.Filter{newEc2Filter("instance-id", instanceID)} - params := &ec2.DescribeAddressesInput{ - Filters: filters, - } - - addresses, err := c.ec2.DescribeAddresses(params) - if err != nil { - return nil, err - } - - return addresses, nil -} - // mapNodeNameToPrivateDNSName maps a k8s NodeName to an AWS Instance PrivateDNSName // This is a simple string cast func mapNodeNameToPrivateDNSName(nodeName types.NodeName) string { @@ -3512,78 +3437,6 @@ func (c *Cloud) getFullInstance(nodeName types.NodeName) (*awsInstance, *ec2.Ins return awsInstance, instance, err } -func instanceAddresses(instance *ec2.Instance) ([]v1.NodeAddress, error) { - addresses := []v1.NodeAddress{} - privateDNSName := aws.StringValue(instance.PrivateDnsName) - unsafePrivateIP := aws.StringValue(instance.PrivateIpAddress) - publicDNSName := aws.StringValue(instance.PublicDnsName) - unsafePublicIP := aws.StringValue(instance.PublicIpAddress) - - if privateDNSName != "" { - addresses = append(addresses, v1.NodeAddress{Type: v1.NodeInternalDNS, Address: privateDNSName}) - } - - if unsafePrivateIP != "" { - ip := net.ParseIP(unsafePrivateIP) - if ip != nil { - addresses = append(addresses, v1.NodeAddress{Type: v1.NodeInternalIP, Address: ip.String()}) - } else { - return nil, fmt.Errorf("EC2 address had invalid private IP: %s", unsafePrivateIP) - } - } - - if publicDNSName != "" { - addresses = append(addresses, v1.NodeAddress{Type: v1.NodeExternalDNS, Address: publicDNSName}) - } - - if unsafePublicIP != "" { - ip := net.ParseIP(unsafePublicIP) - if ip != nil { - addresses = append(addresses, v1.NodeAddress{Type: v1.NodeExternalIP, Address: ip.String()}) - } else { - return nil, fmt.Errorf("EC2 address had invalid public IP: %s", unsafePublicIP) - } - } - - return addresses, nil -} - -func convertAwsAddress(address *ec2.Address) ([]v1.NodeAddress, error) { - nodeAddresses := []v1.NodeAddress{} - if aws.StringValue(address.PrivateIpAddress) != "" { - unsafeIP := *address.PrivateIpAddress - ip := net.ParseIP(unsafeIP) - if ip != nil { - nodeAddresses = append(nodeAddresses, v1.NodeAddress{Type: v1.NodeInternalIP, Address: ip.String()}) - } else { - return nil, fmt.Errorf("EC2 address had invalid private IP: %s", unsafeIP) - } - } - - if aws.StringValue(address.PublicIp) != "" { - unsafeIP := *address.PublicIp - ip := net.ParseIP(unsafeIP) - if ip != nil { - nodeAddresses = append(nodeAddresses, v1.NodeAddress{Type: v1.NodeExternalIP, Address: ip.String()}) - } else { - return nil, fmt.Errorf("EC2 address had invalid public IP: %s", unsafeIP) - } - } - - return nodeAddresses, nil -} - -var providerIDRegexp = regexp.MustCompile(`^aws://([^/]+)$`) - -func instanceIDFromProviderID(providerID string) (instanceID string, err error) { - matches := providerIDRegexp.FindStringSubmatch(providerID) - if len(matches) != 2 { - return "", fmt.Errorf("ProviderID \"%s\" didn't match expected format \"aws://InstanceID\"", providerID) - } - - return matches[1], nil -} - func setNodeDisk( nodeDiskMap map[types.NodeName]map[KubernetesVolumeID]bool, volumeID KubernetesVolumeID, diff --git a/pkg/cloudprovider/providers/aws/instances.go b/pkg/cloudprovider/providers/aws/instances.go index 01830b65e39..2d8c1ea9655 100644 --- a/pkg/cloudprovider/providers/aws/instances.go +++ b/pkg/cloudprovider/providers/aws/instances.go @@ -22,6 +22,7 @@ import ( "strings" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" ) // awsInstanceID represents the ID of the instance in the AWS API, e.g. i-12345678 @@ -78,3 +79,22 @@ func (name kubernetesInstanceID) mapToAWSInstanceID() (awsInstanceID, error) { return awsInstanceID(awsID), nil } + +// Gets the full information about this instance from the EC2 API +func describeInstance(ec2Client EC2, instanceID awsInstanceID) (*ec2.Instance, error) { + request := &ec2.DescribeInstancesInput{ + InstanceIds: []*string{instanceID.awsString()}, + } + + instances, err := ec2Client.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 +}