Skip to content

Commit

Permalink
Fix fetching enimetadata (aws#3035)
Browse files Browse the repository at this point in the history
* Fix fetching enimetadata for efa-only enis

* Fix format

* Fix and add tests

* fix format

* Add comments

---------

Co-authored-by: Jayanth Varavani <1111446+jayanthvn@users.noreply.github.com>
  • Loading branch information
Pavani-Panakanti and jayanthvn authored Sep 25, 2024
1 parent abaf575 commit eb7a9bd
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 86 deletions.
145 changes: 80 additions & 65 deletions pkg/awsutils/awsutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,12 @@ func (cache *EC2InstanceMetadataCache) getENIMetadata(eniMAC string) (ENIMetadat
return ENIMetadata{}, err
}

networkCard, err := cache.imds.GetNetworkCard(ctx, eniMAC)
if err != nil {
awsAPIErrInc("GetNetworkCard", err)
return ENIMetadata{}, err
}

deviceNum, err = cache.imds.GetDeviceNumber(ctx, eniMAC)
if err != nil {
awsAPIErrInc("GetDeviceNumber", err)
Expand All @@ -602,90 +608,99 @@ func (cache *EC2InstanceMetadataCache) getENIMetadata(eniMAC string) (ENIMetadat
deviceNum = 0
}

log.Debugf("Found ENI: %s, MAC %s, device %d", eniID, eniMAC, deviceNum)

// Get IPv4 and IPv6 addresses assigned to interface
cidr, err := cache.imds.GetSubnetIPv4CIDRBlock(ctx, eniMAC)
if err != nil {
awsAPIErrInc("GetSubnetIPv4CIDRBlock", err)
return ENIMetadata{}, err
}

imdsIPv4s, err := cache.imds.GetLocalIPv4s(ctx, eniMAC)
if err != nil {
awsAPIErrInc("GetLocalIPv4s", err)
return ENIMetadata{}, err
}

ec2ip4s := make([]*ec2.NetworkInterfacePrivateIpAddress, len(imdsIPv4s))
for i, ip4 := range imdsIPv4s {
ec2ip4s[i] = &ec2.NetworkInterfacePrivateIpAddress{
Primary: aws.Bool(i == 0),
PrivateIpAddress: aws.String(ip4.String()),
}
}
log.Debugf("Found ENI: %s, MAC %s, device %d, network card %d", eniID, eniMAC, deviceNum, networkCard)

var subnetV4Cidr string
var ec2ip4s []*ec2.NetworkInterfacePrivateIpAddress
var ec2ip6s []*ec2.NetworkInterfaceIpv6Address
var subnetV6Cidr string
if cache.v6Enabled {
// For IPv6 ENIs, do not error on missing IPv6 information
v6cidr, err := cache.imds.GetSubnetIPv6CIDRBlocks(ctx, eniMAC)
if err != nil {
awsAPIErrInc("GetSubnetIPv6CIDRBlocks", err)
} else {
subnetV6Cidr = v6cidr.String()
}
var ec2ipv4Prefixes []*ec2.Ipv4PrefixSpecification
var ec2ipv6Prefixes []*ec2.Ipv6PrefixSpecification

imdsIPv6s, err := cache.imds.GetIPv6s(ctx, eniMAC)
// CNI only manages ENI's on network card 0. We need to get complete metadata info only for ENI's on network card 0.
// For ENI's on other network cards, there might not be IP related info present at all like 'efa-only' interfaces
// So we are skipping fetching IP related info for all ENI's other than card 0
if networkCard == 0 {
// Get IPv4 and IPv6 addresses assigned to interface
cidr, err := cache.imds.GetSubnetIPv4CIDRBlock(ctx, eniMAC)
if err != nil {
awsAPIErrInc("GetIPv6s", err)
awsAPIErrInc("GetSubnetIPv4CIDRBlock", err)
return ENIMetadata{}, err
} else {
ec2ip6s = make([]*ec2.NetworkInterfaceIpv6Address, len(imdsIPv6s))
for i, ip6 := range imdsIPv6s {
ec2ip6s[i] = &ec2.NetworkInterfaceIpv6Address{
Ipv6Address: aws.String(ip6.String()),
}
}
subnetV4Cidr = cidr.String()
}
}

var ec2ipv4Prefixes []*ec2.Ipv4PrefixSpecification
var ec2ipv6Prefixes []*ec2.Ipv6PrefixSpecification

// If IPv6 is enabled, get attached v6 prefixes.
if cache.v6Enabled {
imdsIPv6Prefixes, err := cache.imds.GetIPv6Prefixes(ctx, eniMAC)
imdsIPv4s, err := cache.imds.GetLocalIPv4s(ctx, eniMAC)
if err != nil {
awsAPIErrInc("GetIPv6Prefixes", err)
awsAPIErrInc("GetLocalIPv4s", err)
return ENIMetadata{}, err
}
for _, ipv6prefix := range imdsIPv6Prefixes {
ec2ipv6Prefixes = append(ec2ipv6Prefixes, &ec2.Ipv6PrefixSpecification{
Ipv6Prefix: aws.String(ipv6prefix.String()),
})

ec2ip4s = make([]*ec2.NetworkInterfacePrivateIpAddress, len(imdsIPv4s))
for i, ip4 := range imdsIPv4s {
ec2ip4s[i] = &ec2.NetworkInterfacePrivateIpAddress{
Primary: aws.Bool(i == 0),
PrivateIpAddress: aws.String(ip4.String()),
}
}
} else if cache.v4Enabled && ((eniMAC == primaryMAC && !cache.useCustomNetworking) || (eniMAC != primaryMAC)) {
// Get prefix on primary ENI when custom networking is enabled is not needed.
// If primary ENI has prefixes attached and then we move to custom networking, we don't need to fetch
// the prefix since recommendation is to terminate the nodes and that would have deleted the prefix on the
// primary ENI.
imdsIPv4Prefixes, err := cache.imds.GetIPv4Prefixes(ctx, eniMAC)
if err != nil {
awsAPIErrInc("GetIPv4Prefixes", err)
return ENIMetadata{}, err

if cache.v6Enabled {
// For IPv6 ENIs, do not error on missing IPv6 information
v6cidr, err := cache.imds.GetSubnetIPv6CIDRBlocks(ctx, eniMAC)
if err != nil {
awsAPIErrInc("GetSubnetIPv6CIDRBlocks", err)
} else {
subnetV6Cidr = v6cidr.String()
}

imdsIPv6s, err := cache.imds.GetIPv6s(ctx, eniMAC)
if err != nil {
awsAPIErrInc("GetIPv6s", err)
} else {
ec2ip6s = make([]*ec2.NetworkInterfaceIpv6Address, len(imdsIPv6s))
for i, ip6 := range imdsIPv6s {
ec2ip6s[i] = &ec2.NetworkInterfaceIpv6Address{
Ipv6Address: aws.String(ip6.String()),
}
}
}
}
for _, ipv4prefix := range imdsIPv4Prefixes {
ec2ipv4Prefixes = append(ec2ipv4Prefixes, &ec2.Ipv4PrefixSpecification{
Ipv4Prefix: aws.String(ipv4prefix.String()),
})

// If IPv6 is enabled, get attached v6 prefixes.
if cache.v6Enabled {
imdsIPv6Prefixes, err := cache.imds.GetIPv6Prefixes(ctx, eniMAC)
if err != nil {
awsAPIErrInc("GetIPv6Prefixes", err)
return ENIMetadata{}, err
}
for _, ipv6prefix := range imdsIPv6Prefixes {
ec2ipv6Prefixes = append(ec2ipv6Prefixes, &ec2.Ipv6PrefixSpecification{
Ipv6Prefix: aws.String(ipv6prefix.String()),
})
}
} else if cache.v4Enabled && ((eniMAC == primaryMAC && !cache.useCustomNetworking) || (eniMAC != primaryMAC)) {
// Get prefix on primary ENI when custom networking is enabled is not needed.
// If primary ENI has prefixes attached and then we move to custom networking, we don't need to fetch
// the prefix since recommendation is to terminate the nodes and that would have deleted the prefix on the
// primary ENI.
imdsIPv4Prefixes, err := cache.imds.GetIPv4Prefixes(ctx, eniMAC)
if err != nil {
awsAPIErrInc("GetIPv4Prefixes", err)
return ENIMetadata{}, err
}
for _, ipv4prefix := range imdsIPv4Prefixes {
ec2ipv4Prefixes = append(ec2ipv4Prefixes, &ec2.Ipv4PrefixSpecification{
Ipv4Prefix: aws.String(ipv4prefix.String()),
})
}
}
}

return ENIMetadata{
ENIID: eniID,
MAC: eniMAC,
DeviceNumber: deviceNum,
SubnetIPv4CIDR: cidr.String(),
SubnetIPv4CIDR: subnetV4Cidr,
IPv4Addresses: ec2ip4s,
IPv4Prefixes: ec2ipv4Prefixes,
SubnetIPv6CIDR: subnetV6Cidr,
Expand Down Expand Up @@ -1356,7 +1371,7 @@ func (cache *EC2InstanceMetadataCache) DescribeAllENIs() (DescribeAllENIsResult,
if interfaceType == "trunk" {
trunkENI = eniID
}
if interfaceType == "efa" {
if interfaceType == "efa" || interfaceType == "efa-only" {
efaENIs[eniID] = true
}
// Check IPv4 addresses
Expand Down
74 changes: 53 additions & 21 deletions pkg/awsutils/awsutils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const (
metadataSubnetID = "/subnet-id"
metadataVpcID = "/vpc-id"
metadataVPCcidrs = "/vpc-ipv4-cidr-blocks"
metadataNetworkCard = "/network-card"
metadataDeviceNum = "/device-number"
metadataInterface = "/interface-id"
metadataSubnetCIDR = "/subnet-ipv4-cidr-block"
Expand All @@ -61,6 +62,7 @@ const (
instanceType = "c1.medium"
primaryMAC = "12:ef:2a:98:e5:5a"
eni2MAC = "12:ef:2a:98:e5:5b"
eni3MAC = "12:ef:2a:98:e5:5c"
sg1 = "sg-2e080f50"
sg2 = "sg-2e080f51"
sgs = sg1 + " " + sg2
Expand All @@ -70,14 +72,19 @@ const (
primaryeniID = "eni-00000000"
eniID = primaryeniID
eniAttachID = "eni-attach-beb21856"
eni1NetworkCard = "0"
eni1Device = "0"
eni1PrivateIP = "10.0.0.1"
eni1Prefix = "10.0.1.0/28"
eni2NetworkCard = "0"
eni2Device = "1"
eni2PrivateIP = "10.0.0.2"
eni2Prefix = "10.0.2.0/28"
eni2v6Prefix = "2001:db8::/64"
eni2ID = "eni-12341234"
eni3NetworkCard = "1"
eni3Device = "1"
eni3ID = "eni-67896789"
metadataVPCIPv4CIDRs = "192.168.0.0/16 100.66.0.0/1"
myNodeName = "testNodeName"
)
Expand All @@ -90,14 +97,15 @@ func testMetadata(overrides map[string]interface{}) FakeIMDS {
metadataInstanceType: instanceType,
metadataMAC: primaryMAC,
metadataMACPath: primaryMAC,
metadataMACPath + primaryMAC + metadataDeviceNum: eni1Device,
metadataMACPath + primaryMAC + metadataInterface: primaryeniID,
metadataMACPath + primaryMAC + metadataSGs: sgs,
metadataMACPath + primaryMAC + metadataIPv4s: eni1PrivateIP,
metadataMACPath + primaryMAC + metadataSubnetID: subnetID,
metadataMACPath + primaryMAC + metadataVpcID: vpcID,
metadataMACPath + primaryMAC + metadataSubnetCIDR: subnetCIDR,
metadataMACPath + primaryMAC + metadataVPCcidrs: metadataVPCIPv4CIDRs,
metadataMACPath + primaryMAC + metadataDeviceNum: eni1Device,
metadataMACPath + primaryMAC + metadataInterface: primaryeniID,
metadataMACPath + primaryMAC + metadataNetworkCard: eni1NetworkCard,
metadataMACPath + primaryMAC + metadataSGs: sgs,
metadataMACPath + primaryMAC + metadataIPv4s: eni1PrivateIP,
metadataMACPath + primaryMAC + metadataSubnetID: subnetID,
metadataMACPath + primaryMAC + metadataVpcID: vpcID,
metadataMACPath + primaryMAC + metadataSubnetCIDR: subnetCIDR,
metadataMACPath + primaryMAC + metadataVPCcidrs: metadataVPCIPv4CIDRs,
}

for k, v := range overrides {
Expand Down Expand Up @@ -204,10 +212,31 @@ func TestInitWithEC2metadataErr(t *testing.T) {
func TestGetAttachedENIs(t *testing.T) {
mockMetadata := testMetadata(map[string]interface{}{
metadataMACPath: primaryMAC + " " + eni2MAC,
metadataMACPath + eni2MAC + metadataDeviceNum: eni2Device,
metadataMACPath + eni2MAC + metadataInterface: eni2ID,
metadataMACPath + eni2MAC + metadataSubnetCIDR: subnetCIDR,
metadataMACPath + eni2MAC + metadataIPv4s: eni2PrivateIP,
metadataMACPath + eni2MAC + metadataNetworkCard: eni2NetworkCard,
metadataMACPath + eni2MAC + metadataDeviceNum: eni2Device,
metadataMACPath + eni2MAC + metadataInterface: eni2ID,
metadataMACPath + eni2MAC + metadataSubnetCIDR: subnetCIDR,
metadataMACPath + eni2MAC + metadataIPv4s: eni2PrivateIP,
})

cache := &EC2InstanceMetadataCache{imds: TypedIMDS{mockMetadata}}
ens, err := cache.GetAttachedENIs()
if assert.NoError(t, err) {
assert.Equal(t, len(ens), 2)
}
}

func TestGetAttachedENIsWithEfa(t *testing.T) {
mockMetadata := testMetadata(map[string]interface{}{
metadataMACPath: primaryMAC + " " + eni2MAC,
metadataMACPath + eni2MAC + metadataNetworkCard: eni2NetworkCard,
metadataMACPath + eni2MAC + metadataDeviceNum: eni2Device,
metadataMACPath + eni2MAC + metadataInterface: eni2ID,
metadataMACPath + eni2MAC + metadataSubnetCIDR: subnetCIDR,
metadataMACPath + eni2MAC + metadataIPv4s: eni2PrivateIP,
metadataMACPath + eni3MAC + metadataNetworkCard: eni3NetworkCard,
metadataMACPath + eni3MAC + metadataDeviceNum: eni3Device,
metadataMACPath + eni3MAC + metadataInterface: eni3ID,
})

cache := &EC2InstanceMetadataCache{imds: TypedIMDS{mockMetadata}}
Expand All @@ -220,6 +249,7 @@ func TestGetAttachedENIs(t *testing.T) {
func TestGetAttachedENIsWithPrefixes(t *testing.T) {
mockMetadata := testMetadata(map[string]interface{}{
metadataMACPath: primaryMAC + " " + eni2MAC,
metadataMACPath + eni2MAC + metadataNetworkCard: eni2NetworkCard,
metadataMACPath + eni2MAC + metadataDeviceNum: eni2Device,
metadataMACPath + eni2MAC + metadataInterface: eni2ID,
metadataMACPath + eni2MAC + metadataSubnetCIDR: subnetCIDR,
Expand Down Expand Up @@ -1007,10 +1037,11 @@ func TestEC2InstanceMetadataCache_waitForENIAndIPsAttached(t *testing.T) {
fmt.Println("eniips", eniIPs)
mockMetadata := testMetadata(map[string]interface{}{
metadataMACPath: primaryMAC + " " + eni2MAC,
metadataMACPath + eni2MAC + metadataDeviceNum: eni2Device,
metadataMACPath + eni2MAC + metadataInterface: eni2ID,
metadataMACPath + eni2MAC + metadataSubnetCIDR: subnetCIDR,
metadataMACPath + eni2MAC + metadataIPv4s: eniIPs,
metadataMACPath + eni2MAC + metadataNetworkCard: eni2NetworkCard,
metadataMACPath + eni2MAC + metadataDeviceNum: eni2Device,
metadataMACPath + eni2MAC + metadataInterface: eni2ID,
metadataMACPath + eni2MAC + metadataSubnetCIDR: subnetCIDR,
metadataMACPath + eni2MAC + metadataIPv4s: eniIPs,
})
cache := &EC2InstanceMetadataCache{imds: TypedIMDS{mockMetadata}, ec2SVC: mockEC2}
gotEniMetadata, err := cache.waitForENIAndIPsAttached(tt.args.eni, tt.args.wantedSecondaryIPs, tt.args.maxBackoffDelay)
Expand Down Expand Up @@ -1102,11 +1133,12 @@ func TestEC2InstanceMetadataCache_waitForENIAndPrefixesAttached(t *testing.T) {
}
mockMetadata := testMetadata(map[string]interface{}{
metadataMACPath: primaryMAC + " " + eni2MAC,
metadataMACPath + eni2MAC + metadataDeviceNum: eni2Device,
metadataMACPath + eni2MAC + metadataInterface: eni2ID,
metadataMACPath + eni2MAC + metadataSubnetCIDR: subnetCIDR,
metadataMACPath + eni2MAC + metadataIPv4s: eniIPs,
metadataMACPath + eni2MAC + metaDataPrefixPath: eniPrefixes,
metadataMACPath + eni2MAC + metadataNetworkCard: eni2NetworkCard,
metadataMACPath + eni2MAC + metadataDeviceNum: eni2Device,
metadataMACPath + eni2MAC + metadataInterface: eni2ID,
metadataMACPath + eni2MAC + metadataSubnetCIDR: subnetCIDR,
metadataMACPath + eni2MAC + metadataIPv4s: eniIPs,
metadataMACPath + eni2MAC + metaDataPrefixPath: eniPrefixes,
})
cache := &EC2InstanceMetadataCache{imds: TypedIMDS{mockMetadata}, ec2SVC: mockEC2,
enablePrefixDelegation: true, v4Enabled: tt.args.v4Enabled, v6Enabled: tt.args.v6Enabled}
Expand Down
6 changes: 6 additions & 0 deletions pkg/awsutils/imds.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,12 @@ func (imds TypedIMDS) getInt(ctx context.Context, key string) (int, error) {
return dataInt, err
}

// GetNetworkCard returns the unique network card number associated with an interface.
func (imds TypedIMDS) GetNetworkCard(ctx context.Context, mac string) (int, error) {
key := fmt.Sprintf("network/interfaces/macs/%s/network-card", mac)
return imds.getInt(ctx, key)
}

// GetDeviceNumber returns the unique device number associated with an interface. The primary interface is 0.
func (imds TypedIMDS) GetDeviceNumber(ctx context.Context, mac string) (int, error) {
key := fmt.Sprintf("network/interfaces/macs/%s/device-number", mac)
Expand Down

0 comments on commit eb7a9bd

Please sign in to comment.