From b09d0fb168ca4033e7faa4721478432da49ee749 Mon Sep 17 00:00:00 2001 From: Jay Deokar Date: Thu, 26 Sep 2024 11:31:52 -0700 Subject: [PATCH] Revert "Fix fetching enimetadata (#3035)" This reverts commit eb7a9bd6c4b785c8b145d926be1da798de23d0ad. --- pkg/awsutils/awsutils.go | 145 +++++++++++++++------------------- pkg/awsutils/awsutils_test.go | 74 +++++------------ pkg/awsutils/imds.go | 6 -- 3 files changed, 86 insertions(+), 139 deletions(-) diff --git a/pkg/awsutils/awsutils.go b/pkg/awsutils/awsutils.go index 22a115ea19..f9ba346915 100644 --- a/pkg/awsutils/awsutils.go +++ b/pkg/awsutils/awsutils.go @@ -585,12 +585,6 @@ 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) @@ -608,91 +602,82 @@ func (cache *EC2InstanceMetadataCache) getENIMetadata(eniMAC string) (ENIMetadat deviceNum = 0 } - log.Debugf("Found ENI: %s, MAC %s, device %d, network card %d", eniID, eniMAC, deviceNum, networkCard) + 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()), + } + } - var subnetV4Cidr string - var ec2ip4s []*ec2.NetworkInterfacePrivateIpAddress var ec2ip6s []*ec2.NetworkInterfaceIpv6Address var subnetV6Cidr string - var ec2ipv4Prefixes []*ec2.Ipv4PrefixSpecification - var ec2ipv6Prefixes []*ec2.Ipv6PrefixSpecification - - // 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 cache.v6Enabled { + // For IPv6 ENIs, do not error on missing IPv6 information + v6cidr, err := cache.imds.GetSubnetIPv6CIDRBlocks(ctx, eniMAC) if err != nil { - awsAPIErrInc("GetSubnetIPv4CIDRBlock", err) - return ENIMetadata{}, err + awsAPIErrInc("GetSubnetIPv6CIDRBlocks", err) } else { - subnetV4Cidr = cidr.String() + subnetV6Cidr = v6cidr.String() } - imdsIPv4s, err := cache.imds.GetLocalIPv4s(ctx, eniMAC) + imdsIPv6s, err := cache.imds.GetIPv6s(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()), + awsAPIErrInc("GetIPv6s", err) + } else { + ec2ip6s = make([]*ec2.NetworkInterfaceIpv6Address, len(imdsIPv6s)) + for i, ip6 := range imdsIPv6s { + ec2ip6s[i] = &ec2.NetworkInterfaceIpv6Address{ + Ipv6Address: aws.String(ip6.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) - 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()), - } - } - } + // 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 } - - // 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()), - }) - } + 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()), + }) } } @@ -700,7 +685,7 @@ func (cache *EC2InstanceMetadataCache) getENIMetadata(eniMAC string) (ENIMetadat ENIID: eniID, MAC: eniMAC, DeviceNumber: deviceNum, - SubnetIPv4CIDR: subnetV4Cidr, + SubnetIPv4CIDR: cidr.String(), IPv4Addresses: ec2ip4s, IPv4Prefixes: ec2ipv4Prefixes, SubnetIPv6CIDR: subnetV6Cidr, @@ -1371,7 +1356,7 @@ func (cache *EC2InstanceMetadataCache) DescribeAllENIs() (DescribeAllENIsResult, if interfaceType == "trunk" { trunkENI = eniID } - if interfaceType == "efa" || interfaceType == "efa-only" { + if interfaceType == "efa" { efaENIs[eniID] = true } // Check IPv4 addresses diff --git a/pkg/awsutils/awsutils_test.go b/pkg/awsutils/awsutils_test.go index 72ebda0dd4..cf93040526 100644 --- a/pkg/awsutils/awsutils_test.go +++ b/pkg/awsutils/awsutils_test.go @@ -48,7 +48,6 @@ 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" @@ -62,7 +61,6 @@ 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 @@ -72,19 +70,14 @@ 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" ) @@ -97,15 +90,14 @@ func testMetadata(overrides map[string]interface{}) FakeIMDS { metadataInstanceType: instanceType, metadataMAC: primaryMAC, metadataMACPath: primaryMAC, - 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, + 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, } for k, v := range overrides { @@ -212,31 +204,10 @@ func TestInitWithEC2metadataErr(t *testing.T) { func TestGetAttachedENIs(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, - }) - - 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, + metadataMACPath + eni2MAC + metadataDeviceNum: eni2Device, + metadataMACPath + eni2MAC + metadataInterface: eni2ID, + metadataMACPath + eni2MAC + metadataSubnetCIDR: subnetCIDR, + metadataMACPath + eni2MAC + metadataIPv4s: eni2PrivateIP, }) cache := &EC2InstanceMetadataCache{imds: TypedIMDS{mockMetadata}} @@ -249,7 +220,6 @@ func TestGetAttachedENIsWithEfa(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, @@ -1037,11 +1007,10 @@ func TestEC2InstanceMetadataCache_waitForENIAndIPsAttached(t *testing.T) { fmt.Println("eniips", eniIPs) 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: eniIPs, + 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) @@ -1133,12 +1102,11 @@ func TestEC2InstanceMetadataCache_waitForENIAndPrefixesAttached(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: eniIPs, - metadataMACPath + eni2MAC + metaDataPrefixPath: eniPrefixes, + 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 e3174ba5e9..69c9343501 100644 --- a/pkg/awsutils/imds.go +++ b/pkg/awsutils/imds.go @@ -166,12 +166,6 @@ 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)