Skip to content

Commit

Permalink
Merge pull request #223 from SaranBalaji90/eni-support
Browse files Browse the repository at this point in the history
Add ENI support for nodes(for Fargate nodes)
  • Loading branch information
k8s-ci-robot authored Jun 23, 2021
2 parents 54f2b5c + 87c90e1 commit 7d6f102
Show file tree
Hide file tree
Showing 5 changed files with 222 additions and 7 deletions.
100 changes: 100 additions & 0 deletions pkg/providers/v1/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,12 @@ const (
// Number of node names that can be added to a filter. The AWS limit is 200
// but we are using a lower limit on purpose
filterNodeLimit = 150

// fargateNodeNamePrefix string is added to awsInstance nodeName and providerID of Fargate nodes.
fargateNodeNamePrefix = "fargate-"

// privateDNSNamePrefix is the prefix added to ENI Private DNS Name.
privateDNSNamePrefix = "ip-"
)

const (
Expand Down Expand Up @@ -365,6 +371,8 @@ type EC2 interface {
ModifyInstanceAttribute(request *ec2.ModifyInstanceAttributeInput) (*ec2.ModifyInstanceAttributeOutput, error)

DescribeVpcs(input *ec2.DescribeVpcsInput) (*ec2.DescribeVpcsOutput, error)

DescribeNetworkInterfaces(input *ec2.DescribeNetworkInterfacesInput) (*ec2.DescribeNetworkInterfacesOutput, error)
}

// ELB is a simple pass-through of AWS' ELB client interface, which allows for testing
Expand Down Expand Up @@ -971,6 +979,15 @@ func (s *awsSdkEC2) DescribeInstances(request *ec2.DescribeInstancesInput) ([]*e
return results, nil
}

// DescribeNetworkInterfaces describes network interface provided in the input.
func (s *awsSdkEC2) DescribeNetworkInterfaces(input *ec2.DescribeNetworkInterfacesInput) (*ec2.DescribeNetworkInterfacesOutput, error) {
requestTime := time.Now()
resp, err := s.ec2.DescribeNetworkInterfaces(input)
timeTaken := time.Since(requestTime).Seconds()
recordAWSMetric("describe_network_interfaces", timeTaken, err)
return resp, err
}

// Implements EC2.DescribeSecurityGroups
func (s *awsSdkEC2) DescribeSecurityGroups(request *ec2.DescribeSecurityGroupsInput) ([]*ec2.SecurityGroup, error) {
// Security groups are paged
Expand Down Expand Up @@ -1616,6 +1633,16 @@ func extractNodeAddresses(instance *ec2.Instance) ([]v1.NodeAddress, error) {
return addresses, nil
}

// getNodeAddressesForFargateNode generates list of Node addresses for Fargate node.
func getNodeAddressesForFargateNode(privateDNSName, privateIP string) []v1.NodeAddress {
addresses := []v1.NodeAddress{}
addresses = append(addresses, v1.NodeAddress{Type: v1.NodeInternalIP, Address: privateIP})
if privateDNSName != "" {
addresses = append(addresses, v1.NodeAddress{Type: v1.NodeInternalDNS, Address: privateDNSName})
}
return addresses
}

// NodeAddressesByProviderID returns the node addresses of an instances with the specified unique providerID
// 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
Expand All @@ -1625,6 +1652,14 @@ func (c *Cloud) NodeAddressesByProviderID(ctx context.Context, providerID string
return nil, err
}

if isFargateNode(string(instanceID)) {
eni, err := c.describeNetworkInterfaces(string(instanceID))
if eni == nil || err != nil {
return nil, err
}
return getNodeAddressesForFargateNode(aws.StringValue(eni.PrivateDnsName), aws.StringValue(eni.PrivateIpAddress)), nil
}

instance, err := describeInstance(c.ec2, instanceID)
if err != nil {
return nil, err
Expand All @@ -1641,6 +1676,11 @@ func (c *Cloud) InstanceExistsByProviderID(ctx context.Context, providerID strin
return false, err
}

if isFargateNode(string(instanceID)) {
eni, err := c.describeNetworkInterfaces(string(instanceID))
return eni != nil, err
}

request := &ec2.DescribeInstancesInput{
InstanceIds: []*string{instanceID.awsString()},
}
Expand Down Expand Up @@ -1676,6 +1716,11 @@ func (c *Cloud) InstanceShutdownByProviderID(ctx context.Context, providerID str
return false, err
}

if isFargateNode(string(instanceID)) {
eni, err := c.describeNetworkInterfaces(string(instanceID))
return eni != nil, err
}

request := &ec2.DescribeInstancesInput{
InstanceIds: []*string{instanceID.awsString()},
}
Expand Down Expand Up @@ -1732,6 +1777,10 @@ func (c *Cloud) InstanceTypeByProviderID(ctx context.Context, providerID string)
return "", err
}

if isFargateNode(string(instanceID)) {
return "", nil
}

instance, err := describeInstance(c.ec2, instanceID)
if err != nil {
return "", err
Expand Down Expand Up @@ -1836,6 +1885,18 @@ func (c *Cloud) GetZoneByProviderID(ctx context.Context, providerID string) (clo
if err != nil {
return cloudprovider.Zone{}, err
}

if isFargateNode(string(instanceID)) {
eni, err := c.describeNetworkInterfaces(string(instanceID))
if eni == nil || err != nil {
return cloudprovider.Zone{}, err
}
return cloudprovider.Zone{
FailureDomain: *eni.AvailabilityZone,
Region: c.region,
}, nil
}

instance, err := c.getInstanceByID(string(instanceID))
if err != nil {
return cloudprovider.Zone{}, err
Expand Down Expand Up @@ -4981,6 +5042,11 @@ func (c *Cloud) getFullInstance(nodeName types.NodeName) (*awsInstance, *ec2.Ins
return awsInstance, instance, err
}

// isFargateNode returns true if given node runs on Fargate compute
func isFargateNode(nodeName string) bool {
return strings.HasPrefix(nodeName, fargateNodeNamePrefix)
}

func (c *Cloud) nodeNameToProviderID(nodeName types.NodeName) (InstanceID, error) {
if len(nodeName) == 0 {
return "", fmt.Errorf("no nodeName provided")
Expand Down Expand Up @@ -5034,3 +5100,37 @@ func getInitialAttachDetachDelay(status string) time.Duration {
}
return volumeAttachmentStatusInitialDelay
}

// describeNetworkInterfaces returns network interface information for the given DNS name.
func (c *Cloud) describeNetworkInterfaces(nodeName string) (*ec2.NetworkInterface, error) {
eniEndpoint := strings.TrimPrefix(nodeName, fargateNodeNamePrefix)

filters := []*ec2.Filter{
newEc2Filter("attachment.status", "attached"),
newEc2Filter("vpc-id", c.vpcID),
}

// when enableDnsSupport is set to false in a VPC, interface will not have private DNS names.
if strings.HasPrefix(eniEndpoint, privateDNSNamePrefix) {
filters = append(filters, newEc2Filter("private-dns-name", eniEndpoint))
} else {
filters = append(filters, newEc2Filter("private-ip-address", eniEndpoint))
}

request := &ec2.DescribeNetworkInterfacesInput{
Filters: filters,
}

eni, err := c.ec2.DescribeNetworkInterfaces(request)
if err != nil {
return nil, err
}
if len(eni.NetworkInterfaces) == 0 {
return nil, nil
}
if len(eni.NetworkInterfaces) != 1 {
// This should not be possible - ids should be unique
return nil, fmt.Errorf("multiple interfaces found with same id %q", eni.NetworkInterfaces)
}
return eni.NetworkInterfaces[0], nil
}
27 changes: 27 additions & 0 deletions pkg/providers/v1/aws_fakes.go
Original file line number Diff line number Diff line change
Expand Up @@ -684,3 +684,30 @@ func contains(haystack []*string, needle string) bool {
}
return false
}

// DescribeNetworkInterfaces returns list of ENIs for testing
func (ec2i *FakeEC2Impl) DescribeNetworkInterfaces(input *ec2.DescribeNetworkInterfacesInput) (*ec2.DescribeNetworkInterfacesOutput, error) {
networkInterface := []*ec2.NetworkInterface{
{
PrivateIpAddress: aws.String("1.2.3.4"),
AvailabilityZone: aws.String("us-west-2c"),
},
}
for _, filter := range input.Filters {
if strings.HasPrefix(*filter.Values[0], fargateNodeNamePrefix) {
// verify filter doesn't have fargate prefix
panic(fmt.Sprintf("invalid endpoint specified for DescribeNetworkInterface call %s", *filter.Values[0]))
} else if strings.HasPrefix(*filter.Values[0], "not-found") {
// for negative testing
return &ec2.DescribeNetworkInterfacesOutput{}, nil
}

if *filter.Name == "private-dns-name" {
networkInterface[0].PrivateDnsName = aws.String("ip-1-2-3-4.compute.amazon.com")
}
}

return &ec2.DescribeNetworkInterfacesOutput{
NetworkInterfaces: networkInterface,
}, nil
}
82 changes: 82 additions & 0 deletions pkg/providers/v1/aws_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3516,3 +3516,85 @@ func Test_parseStringSliceAnnotation(t *testing.T) {
})
}
}

func TestNodeAddressesForFargate(t *testing.T) {
awsServices := newMockedFakeAWSServices(TestClusterID)
c, _ := newAWSCloud(CloudConfig{}, awsServices)

nodeAddresses, _ := c.NodeAddressesByProviderID(context.TODO(), "aws:///us-west-2c/1abc-2def/fargate-ip-192.168.164.88")
verifyNodeAddressesForFargate(t, true, nodeAddresses)
}

func TestNodeAddressesForFargatePrivateIP(t *testing.T) {
awsServices := newMockedFakeAWSServices(TestClusterID)
c, _ := newAWSCloud(CloudConfig{}, awsServices)

nodeAddresses, _ := c.NodeAddressesByProviderID(context.TODO(), "aws:///us-west-2c/1abc-2def/fargate-192.168.164.88")
verifyNodeAddressesForFargate(t, false, nodeAddresses)
}

func verifyNodeAddressesForFargate(t *testing.T, verifyPublicIP bool, nodeAddresses []v1.NodeAddress) {
if verifyPublicIP {
assert.Equal(t, 2, len(nodeAddresses))
assert.Equal(t, "ip-1-2-3-4.compute.amazon.com", nodeAddresses[1].Address)
assert.Equal(t, v1.NodeInternalDNS, nodeAddresses[1].Type)
} else {
assert.Equal(t, 1, len(nodeAddresses))
}
assert.Equal(t, "1.2.3.4", nodeAddresses[0].Address)
assert.Equal(t, v1.NodeInternalIP, nodeAddresses[0].Type)
}

func TestInstanceExistsByProviderIDForFargate(t *testing.T) {
awsServices := newMockedFakeAWSServices(TestClusterID)
c, _ := newAWSCloud(CloudConfig{}, awsServices)

instanceExist, err := c.InstanceExistsByProviderID(context.TODO(), "aws:///us-west-2c/1abc-2def/fargate-192.168.164.88")
assert.Nil(t, err)
assert.True(t, instanceExist)
}

func TestInstanceNotExistsByProviderIDForFargate(t *testing.T) {
awsServices := newMockedFakeAWSServices(TestClusterID)
c, _ := newAWSCloud(CloudConfig{}, awsServices)

instanceExist, err := c.InstanceExistsByProviderID(context.TODO(), "aws:///us-west-2c/1abc-2def/fargate-not-found")
assert.Nil(t, err)
assert.False(t, instanceExist)
}

func TestInstanceShutdownByProviderIDForFargate(t *testing.T) {
awsServices := newMockedFakeAWSServices(TestClusterID)
c, _ := newAWSCloud(CloudConfig{}, awsServices)

instanceExist, err := c.InstanceShutdownByProviderID(context.TODO(), "aws:///us-west-2c/1abc-2def/fargate-192.168.164.88")
assert.Nil(t, err)
assert.True(t, instanceExist)
}

func TestInstanceShutdownNotExistsByProviderIDForFargate(t *testing.T) {
awsServices := newMockedFakeAWSServices(TestClusterID)
c, _ := newAWSCloud(CloudConfig{}, awsServices)

instanceExist, err := c.InstanceShutdownByProviderID(context.TODO(), "aws:///us-west-2c/1abc-2def/fargate-not-found")
assert.Nil(t, err)
assert.False(t, instanceExist)
}

func TestInstanceTypeByProviderIDForFargate(t *testing.T) {
awsServices := newMockedFakeAWSServices(TestClusterID)
c, _ := newAWSCloud(CloudConfig{}, awsServices)

instanceType, err := c.InstanceTypeByProviderID(context.TODO(), "aws:///us-west-2c/1abc-2def/fargate-not-found")
assert.Nil(t, err)
assert.Equal(t, "", instanceType)
}

func TestGetZoneByProviderIDForFargate(t *testing.T) {
awsServices := newMockedFakeAWSServices(TestClusterID)
c, _ := newAWSCloud(CloudConfig{}, awsServices)

zoneDetails, err := c.GetZoneByProviderID(context.TODO(), "aws:///us-west-2c/1abc-2def/fargate-192.168.164.88")
assert.Nil(t, err)
assert.Equal(t, "us-west-2c", zoneDetails.FailureDomain)
}
12 changes: 5 additions & 7 deletions pkg/providers/v1/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ func (i InstanceID) awsString() *string {
// the following form
// * aws:///<zone>/<awsInstanceId>
// * aws:////<awsInstanceId>
// * aws:///<zone>/fargate-<eni-ip-address>
// * <awsInstanceId>
type KubernetesInstanceID string

Expand All @@ -73,17 +74,14 @@ func (name KubernetesInstanceID) MapToAWSInstanceID() (InstanceID, error) {

awsID := ""
tokens := strings.Split(strings.Trim(url.Path, "/"), "/")
if len(tokens) == 1 {
// instanceId
awsID = tokens[0]
} else if len(tokens) == 2 {
// az/instanceId
awsID = tokens[1]
// last token in the providerID is the aws resource ID for both EC2 and Fargate nodes
if len(tokens) > 0 {
awsID = tokens[len(tokens)-1]
}

// We sanity check the resulting volume; the two known formats are
// i-12345678 and i-12345678abcdef01
if awsID == "" || !awsInstanceRegMatch.MatchString(awsID) {
if awsID == "" || !(awsInstanceRegMatch.MatchString(awsID) || isFargateNode(awsID)) {
return "", fmt.Errorf("Invalid format for AWS instance (%s)", name)
}

Expand Down
8 changes: 8 additions & 0 deletions pkg/providers/v1/instances_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,14 @@ func TestMapToAWSInstanceIDs(t *testing.T) {
Kubernetes: "",
ExpectError: true,
},
{
Kubernetes: "aws:///us-west-2c/1abc-2def/fargate-ip-192-168-164-88.internal",
Aws: "fargate-ip-192-168-164-88.internal",
},
{
Kubernetes: "aws:///us-west-2c/1abc-2def/fargate-192.168.164.88",
Aws: "fargate-192.168.164.88",
},
}

for _, test := range tests {
Expand Down

0 comments on commit 7d6f102

Please sign in to comment.