Skip to content

Commit

Permalink
Ignore non-zero cards for free eni calculation and free device (#2784) (
Browse files Browse the repository at this point in the history
#2802)

Co-authored-by: Joseph Chen <[email protected]>
  • Loading branch information
jchen6585 and Joseph Chen authored Feb 20, 2024
1 parent 69b5945 commit 71ac4da
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 64 deletions.
54 changes: 35 additions & 19 deletions pkg/awsutils/awsutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,9 @@ type APIs interface {
// GetENILimit returns the number of ENIs that can be attached to an instance
GetENILimit() int

// GetNetworkCards returns the network cards the instance has
GetNetworkCards() []vpc.NetworkCard

// GetPrimaryENImac returns the mac address of the primary ENI
GetPrimaryENImac() string

Expand All @@ -153,11 +156,11 @@ type APIs interface {
// WaitForENIAndIPsAttached waits until the ENI has been attached and the secondary IPs have been added
WaitForENIAndIPsAttached(eni string, wantedSecondaryIPs int) (ENIMetadata, error)

//SetCNIunmanaged ENI
SetCNIUnmanagedENIs(eniID []string) error
//SetMultiCardENIs ENI
SetMultiCardENIs(eniID []string) error

//IsCNIUnmanagedENI
IsCNIUnmanagedENI(eniID string) bool
//IsMultiCardENI
IsMultiCardENI(eniID string) bool

//IsPrimaryENI
IsPrimaryENI(eniID string) bool
Expand Down Expand Up @@ -200,7 +203,7 @@ type EC2InstanceMetadataCache struct {

unmanagedENIs StringSet
useCustomNetworking bool
cniunmanagedENIs StringSet
multiCardENIs StringSet
enablePrefixDelegation bool

clusterName string
Expand Down Expand Up @@ -497,7 +500,7 @@ func (cache *EC2InstanceMetadataCache) RefreshSGIDs(mac string) error {
newENIs := StringSet{}
newENIs.Set(eniIDs)

tempfilteredENIs := newENIs.Difference(&cache.cniunmanagedENIs)
tempfilteredENIs := newENIs.Difference(&cache.multiCardENIs)
filteredENIs := tempfilteredENIs.Difference(&cache.unmanagedENIs)

sgIDsPtrs := aws.StringSlice(sgIDs)
Expand Down Expand Up @@ -703,13 +706,16 @@ func (cache *EC2InstanceMetadataCache) awsGetFreeDeviceNumber() (int, error) {
inst := result.Reservations[0].Instances[0]
var device [maxENIs]bool
for _, eni := range inst.NetworkInterfaces {
if aws.Int64Value(eni.Attachment.DeviceIndex) > maxENIs {
log.Warnf("The Device Index %d of the attached ENI %s > instance max slot %d",
aws.Int64Value(eni.Attachment.DeviceIndex), aws.StringValue(eni.NetworkInterfaceId),
maxENIs)
} else {
log.Debugf("Discovered device number is used: %d", aws.Int64Value(eni.Attachment.DeviceIndex))
device[aws.Int64Value(eni.Attachment.DeviceIndex)] = true
// We don't support multi-card yet, so only account for network card zero
if aws.Int64Value(eni.Attachment.NetworkCardIndex) == 0 {
if aws.Int64Value(eni.Attachment.DeviceIndex) > maxENIs {
log.Warnf("The Device Index %d of the attached ENI %s > instance max slot %d",
aws.Int64Value(eni.Attachment.DeviceIndex), aws.StringValue(eni.NetworkInterfaceId),
maxENIs)
} else {
log.Debugf("Discovered device number is used: %d", aws.Int64Value(eni.Attachment.DeviceIndex))
device[aws.Int64Value(eni.Attachment.DeviceIndex)] = true
}
}
}

Expand Down Expand Up @@ -780,6 +786,7 @@ func (cache *EC2InstanceMetadataCache) attachENI(eniID string) (string, error) {
DeviceIndex: aws.Int64(int64(freeDevice)),
InstanceId: aws.String(cache.instanceID),
NetworkInterfaceId: aws.String(eniID),
NetworkCardIndex: aws.Int64(0),
}
start := time.Now()
attachOutput, err := cache.ec2SVC.AttachNetworkInterfaceWithContext(context.Background(), attachInput)
Expand Down Expand Up @@ -1447,6 +1454,15 @@ func (cache *EC2InstanceMetadataCache) GetENILimit() int {
return eniLimit
}

// GetNetworkCards returns the network cards the instance has
func (cache *EC2InstanceMetadataCache) GetNetworkCards() []vpc.NetworkCard {
networkCards, err := vpc.GetNetworkCards(cache.instanceType)
if err != nil {
return nil
}
return networkCards
}

// GetInstanceHypervisorFamily returns hypervisor of EC2 instance type
func (cache *EC2InstanceMetadataCache) GetInstanceHypervisorFamily() string {
hypervisor, err := vpc.GetHypervisorType(cache.instanceType)
Expand Down Expand Up @@ -1922,18 +1938,18 @@ func (cache *EC2InstanceMetadataCache) getENIsFromPaginatedDescribeNetworkInterf
return innerErr
}

// SetCNIUnmanagedENIs Set unmanaged ENI set
func (cache *EC2InstanceMetadataCache) SetCNIUnmanagedENIs(eniID []string) error {
// SetMultiCardENIs creates a StringSet tracking ENIs not behind the default network card index
func (cache *EC2InstanceMetadataCache) SetMultiCardENIs(eniID []string) error {
if len(eniID) != 0 {
cache.cniunmanagedENIs.Set(eniID)
cache.multiCardENIs.Set(eniID)
}
return nil
}

// IsCNIUnmanagedENI returns if the eni is unmanaged
func (cache *EC2InstanceMetadataCache) IsCNIUnmanagedENI(eniID string) bool {
// IsMultiCardENI returns if the ENI is not behind the default network card index (multi-card ENI)
func (cache *EC2InstanceMetadataCache) IsMultiCardENI(eniID string) bool {
if len(eniID) != 0 {
return cache.cniunmanagedENIs.Has(eniID)
return cache.multiCardENIs.Has(eniID)
}
return false
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/awsutils/awsutils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ func TestAWSGetFreeDeviceNumberNoDevice(t *testing.T) {
for i := 0; i < maxENIs; i++ {
var deviceNums [maxENIs]int64
deviceNums[i] = int64(i)
ec2ENI := &ec2.InstanceNetworkInterface{Attachment: &ec2.InstanceNetworkInterfaceAttachment{DeviceIndex: &deviceNums[i]}}
ec2ENI := &ec2.InstanceNetworkInterface{Attachment: &ec2.InstanceNetworkInterfaceAttachment{DeviceIndex: &deviceNums[i], NetworkCardIndex: aws.Int64(0)}}
ec2ENIs = append(ec2ENIs, ec2ENI)
}
result := &ec2.DescribeInstancesOutput{
Expand Down Expand Up @@ -426,7 +426,7 @@ func TestAllocENINoFreeDevice(t *testing.T) {
for i := 0; i < maxENIs; i++ {
var deviceNums [maxENIs]int64
deviceNums[i] = int64(i)
ec2ENI := &ec2.InstanceNetworkInterface{Attachment: &ec2.InstanceNetworkInterfaceAttachment{DeviceIndex: &deviceNums[i]}}
ec2ENI := &ec2.InstanceNetworkInterface{Attachment: &ec2.InstanceNetworkInterfaceAttachment{DeviceIndex: &deviceNums[i], NetworkCardIndex: aws.Int64(0)}}
ec2ENIs = append(ec2ENIs, ec2ENI)
}
result := &ec2.DescribeInstancesOutput{
Expand Down
39 changes: 27 additions & 12 deletions pkg/awsutils/mocks/awsutils_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 13 additions & 11 deletions pkg/ipamd/ipamd.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,13 +198,15 @@ type IPAMContext struct {
maxENI int
maxPrefixesPerENI int
unmanagedENI int
warmENITarget int
warmIPTarget int
minimumIPTarget int
warmPrefixTarget int
primaryIP map[string]string // primaryIP is a map from ENI ID to primary IP of that ENI
lastNodeIPPoolAction time.Time
lastDecreaseIPPool time.Time
numNetworkCards int

warmENITarget int
warmIPTarget int
minimumIPTarget int
warmPrefixTarget int
primaryIP map[string]string // primaryIP is a map from ENI ID to primary IP of that ENI
lastNodeIPPoolAction time.Time
lastDecreaseIPPool time.Time
// reconcileCooldownCache keeps timestamps of the last time an IP address was unassigned from an ENI,
// so that we don't reconcile and add it back too quickly if IMDS lags behind reality.
reconcileCooldownCache ReconcileCooldownCache
Expand Down Expand Up @@ -346,6 +348,7 @@ func New(k8sClient client.Client) (*IPAMContext, error) {
c.enablePodENI = enablePodENI()
c.enableManageUntaggedMode = enableManageUntaggedMode()
c.enablePodIPAnnotation = enablePodIPAnnotation()
c.numNetworkCards = len(c.awsClient.GetNetworkCards())

err = c.awsClient.FetchInstanceTypeLimits()
if err != nil {
Expand Down Expand Up @@ -408,7 +411,7 @@ func (c *IPAMContext) nodeInit() error {
}

log.Debugf("DescribeAllENIs success: ENIs: %d, tagged: %d", len(metadataResult.ENIMetadata), len(metadataResult.TagMap))
c.awsClient.SetCNIUnmanagedENIs(metadataResult.MultiCardENIIDs)
c.awsClient.SetMultiCardENIs(metadataResult.MultiCardENIIDs)
c.setUnmanagedENIs(metadataResult.TagMap)
enis := c.filterUnmanagedENIs(metadataResult.ENIMetadata)

Expand Down Expand Up @@ -1359,7 +1362,7 @@ func (c *IPAMContext) nodeIPPoolReconcile(ctx context.Context, interval time.Dur
efaENIs = metadataResult.EFAENIs
eniTagMap = metadataResult.TagMap
c.setUnmanagedENIs(metadataResult.TagMap)
c.awsClient.SetCNIUnmanagedENIs(metadataResult.MultiCardENIIDs)
c.awsClient.SetMultiCardENIs(metadataResult.MultiCardENIIDs)
attachedENIs = c.filterUnmanagedENIs(metadataResult.ENIMetadata)
}

Expand Down Expand Up @@ -1768,9 +1771,8 @@ func (c *IPAMContext) filterUnmanagedENIs(enis []awsutils.ENIMetadata) []awsutil
log.Debugf("Skipping ENI %s: since it is unmanaged", eni.ENIID)
numFiltered++
continue
} else if c.awsClient.IsCNIUnmanagedENI(eni.ENIID) {
} else if c.awsClient.IsMultiCardENI(eni.ENIID) {
log.Debugf("Skipping ENI %s: since on non-zero network card", eni.ENIID)
numFiltered++
continue
}
ret = append(ret, eni)
Expand Down
Loading

0 comments on commit 71ac4da

Please sign in to comment.