From eb7a9bd6c4b785c8b145d926be1da798de23d0ad Mon Sep 17 00:00:00 2001 From: pavanipt Date: Wed, 25 Sep 2024 10:05:50 -0700 Subject: [PATCH] Fix fetching enimetadata (#3035) * 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> --- pkg/awsutils/awsutils.go | 145 +++++++++++++++++++--------------- pkg/awsutils/awsutils_test.go | 74 ++++++++++++----- pkg/awsutils/imds.go | 6 ++ 3 files changed, 139 insertions(+), 86 deletions(-) diff --git a/pkg/awsutils/awsutils.go b/pkg/awsutils/awsutils.go index f9ba346915..22a115ea19 100644 --- a/pkg/awsutils/awsutils.go +++ b/pkg/awsutils/awsutils.go @@ -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) @@ -602,82 +608,91 @@ 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()), + }) + } } } @@ -685,7 +700,7 @@ func (cache *EC2InstanceMetadataCache) getENIMetadata(eniMAC string) (ENIMetadat ENIID: eniID, MAC: eniMAC, DeviceNumber: deviceNum, - SubnetIPv4CIDR: cidr.String(), + SubnetIPv4CIDR: subnetV4Cidr, IPv4Addresses: ec2ip4s, IPv4Prefixes: ec2ipv4Prefixes, SubnetIPv6CIDR: subnetV6Cidr, @@ -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 diff --git a/pkg/awsutils/awsutils_test.go b/pkg/awsutils/awsutils_test.go index cf93040526..72ebda0dd4 100644 --- a/pkg/awsutils/awsutils_test.go +++ b/pkg/awsutils/awsutils_test.go @@ -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" @@ -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 @@ -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" ) @@ -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 { @@ -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}} @@ -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, @@ -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) @@ -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} diff --git a/pkg/awsutils/imds.go b/pkg/awsutils/imds.go index 69c9343501..e3174ba5e9 100644 --- a/pkg/awsutils/imds.go +++ b/pkg/awsutils/imds.go @@ -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)