Skip to content

Commit

Permalink
Subnet Discovery - Unfilled ENI fix
Browse files Browse the repository at this point in the history
  • Loading branch information
Joseph Chen committed Jun 17, 2024
1 parent 71263b9 commit e28f7ff
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 65 deletions.
13 changes: 10 additions & 3 deletions pkg/ipamd/datastore/data_store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -974,21 +974,28 @@ func (e *ENI) hasPods() bool {
}

// GetENINeedsIP finds an ENI in the datastore that needs more IP addresses allocated
func (ds *DataStore) GetENINeedsIP(maxIPperENI int, skipPrimary bool) *ENI {
func (ds *DataStore) GetENINeedsIP(maxIPperENI int, skipPrimary bool, useSubnetDiscovery bool, insufficientENI map[*ENI]bool) (*ENI, int) {
ds.lock.Lock()
defer ds.lock.Unlock()
for _, eni := range ds.eniPool {
if (skipPrimary && eni.IsPrimary) || eni.IsTrunk {
ds.log.Debugf("Skip needs IP check for trunk ENI of primary ENI when Custom Networking is enabled")
continue
}
if _, ok := insufficientENI[eni]; ok {
continue
}
if len(eni.AvailableIPv4Cidrs) < maxIPperENI {
ds.log.Debugf("Found ENI %s that has less than the maximum number of IP/Prefixes addresses allocated: cur=%d, max=%d",
eni.ID, len(eni.AvailableIPv4Cidrs), maxIPperENI)
return eni
if useSubnetDiscovery {
return eni, len(ds.eniPool)
}
return eni, 1
}
}
return nil
// Returning 1 since we want one iteration of IP allcocation if subnet discovery is off
return nil, 1
}

// RemoveUnusedENIFromStore removes a deletable ENI from the data store.
Expand Down
143 changes: 81 additions & 62 deletions pkg/ipamd/ipamd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,8 @@ func (c *IPAMContext) tryAssignCidrs() (increasedPool bool, err error) {
// For an ENI, try to fill in missing IPs on an existing ENI.
// PRECONDITION: isDatastorePoolTooLow returned true
func (c *IPAMContext) tryAssignIPs() (increasedPool bool, err error) {
insufficientENI := make(map[*datastore.ENI]bool)

// If WARM_IP_TARGET is set, only proceed if we are short of target
short, _, warmIPTargetsDefined := c.datastoreTargetState(nil)
if warmIPTargetsDefined && short == 0 {
Expand All @@ -928,45 +930,52 @@ func (c *IPAMContext) tryAssignIPs() (increasedPool bool, err error) {
}

// Find an ENI where we can add more IPs
eni := c.dataStore.GetENINeedsIP(c.maxIPsPerENI, c.useCustomNetworking)
if eni != nil && len(eni.AvailableIPv4Cidrs) < c.maxIPsPerENI {
currentNumberOfAllocatedIPs := len(eni.AvailableIPv4Cidrs)
// Try to allocate all available IPs for this ENI
resourcesToAllocate := min((c.maxIPsPerENI - currentNumberOfAllocatedIPs), toAllocate)
output, err := c.awsClient.AllocIPAddresses(eni.ID, resourcesToAllocate)
if err != nil && !containsPrivateIPAddressLimitExceededError(err) {
log.Warnf("failed to allocate all available IP addresses on ENI %s, err: %v", eni.ID, err)
// Try to just get one more IP
output, err = c.awsClient.AllocIPAddresses(eni.ID, 1)
eni, numOfENIs := c.dataStore.GetENINeedsIP(c.maxIPsPerENI, c.useCustomNetworking, c.useSubnetDiscovery, insufficientENI)
for i := 0; i < numOfENIs; i++ {
if eni != nil && len(eni.AvailableIPv4Cidrs) < c.maxIPsPerENI {
currentNumberOfAllocatedIPs := len(eni.AvailableIPv4Cidrs)
// Try to allocate all available IPs for this ENI
resourcesToAllocate := min((c.maxIPsPerENI - currentNumberOfAllocatedIPs), toAllocate)
output, err := c.awsClient.AllocIPAddresses(eni.ID, resourcesToAllocate)
if err != nil && !containsPrivateIPAddressLimitExceededError(err) {
ipamdErrInc("increaseIPPoolAllocIPAddressesFailed")
return false, errors.Wrap(err, fmt.Sprintf("failed to allocate one IP addresses on ENI %s, err ", eni.ID))
log.Warnf("failed to allocate all available IP addresses on ENI %s, err: %v", eni.ID, err)
// Try to just get one more IP
output, err = c.awsClient.AllocIPAddresses(eni.ID, 1)
if err != nil && !containsPrivateIPAddressLimitExceededError(err) {
ipamdErrInc("increaseIPPoolAllocIPAddressesFailed")
if c.useSubnetDiscovery && containsInsufficientCIDRsOrSubnetIPs(err) {
insufficientENI[eni] = true
eni, _ = c.dataStore.GetENINeedsIP(c.maxIPsPerENI, c.useCustomNetworking, c.useSubnetDiscovery, insufficientENI)
continue
}
return false, errors.Wrap(err, fmt.Sprintf("failed to allocate one IP addresses on ENI %s, err ", eni.ID))
}
}
}

var ec2ip4s []*ec2.NetworkInterfacePrivateIpAddress
if containsPrivateIPAddressLimitExceededError(err) {
log.Debug("AssignPrivateIpAddresses returned PrivateIpAddressLimitExceeded. This can happen if the data store is out of sync." +
"Returning without an error here since we will verify the actual state by calling EC2 to see what addresses have already assigned to this ENI.")
// This call to EC2 is needed to verify which IPs got attached to this ENI.
ec2ip4s, err = c.awsClient.GetIPv4sFromEC2(eni.ID)
if err != nil {
ipamdErrInc("increaseIPPoolGetENIaddressesFailed")
return true, errors.Wrap(err, "failed to get ENI IP addresses during IP allocation")
}
} else {
if output == nil {
ipamdErrInc("increaseIPPoolGetENIaddressesFailed")
return true, errors.Wrap(err, "failed to get ENI IP addresses during IP allocation")
}
var ec2ip4s []*ec2.NetworkInterfacePrivateIpAddress
if containsPrivateIPAddressLimitExceededError(err) {
log.Debug("AssignPrivateIpAddresses returned PrivateIpAddressLimitExceeded. This can happen if the data store is out of sync." +
"Returning without an error here since we will verify the actual state by calling EC2 to see what addresses have already assigned to this ENI.")
// This call to EC2 is needed to verify which IPs got attached to this ENI.
ec2ip4s, err = c.awsClient.GetIPv4sFromEC2(eni.ID)
if err != nil {
ipamdErrInc("increaseIPPoolGetENIaddressesFailed")
return true, errors.Wrap(err, "failed to get ENI IP addresses during IP allocation")
}
} else {
if output == nil {
ipamdErrInc("increaseIPPoolGetENIaddressesFailed")
return true, errors.Wrap(err, "failed to get ENI IP addresses during IP allocation")
}

ec2Addrs := output.AssignedPrivateIpAddresses
for _, ec2Addr := range ec2Addrs {
ec2ip4s = append(ec2ip4s, &ec2.NetworkInterfacePrivateIpAddress{PrivateIpAddress: aws.String(aws.StringValue(ec2Addr.PrivateIpAddress))})
ec2Addrs := output.AssignedPrivateIpAddresses
for _, ec2Addr := range ec2Addrs {
ec2ip4s = append(ec2ip4s, &ec2.NetworkInterfacePrivateIpAddress{PrivateIpAddress: aws.String(aws.StringValue(ec2Addr.PrivateIpAddress))})
}
}
c.addENIsecondaryIPsToDataStore(ec2ip4s, eni.ID)
return true, nil
}
c.addENIsecondaryIPsToDataStore(ec2ip4s, eni.ID)
return true, nil
}
return false, nil
}
Expand Down Expand Up @@ -1012,42 +1021,52 @@ func (c *IPAMContext) assignIPv6Prefix(eniID string) (err error) {

// PRECONDITION: isDatastorePoolTooLow returned true
func (c *IPAMContext) tryAssignPrefixes() (increasedPool bool, err error) {
insufficientENI := make(map[*datastore.ENI]bool)

toAllocate := c.getPrefixesNeeded()
// Returns an ENI which has space for more prefixes to be attached, but this
// ENI might not suffice the WARM_IP_TARGET/WARM_PREFIX_TARGET
eni := c.dataStore.GetENINeedsIP(c.maxPrefixesPerENI, c.useCustomNetworking)
if eni != nil {
currentNumberOfAllocatedPrefixes := len(eni.AvailableIPv4Cidrs)
resourcesToAllocate := min((c.maxPrefixesPerENI - currentNumberOfAllocatedPrefixes), toAllocate)
output, err := c.awsClient.AllocIPAddresses(eni.ID, resourcesToAllocate)
if err != nil && !containsPrivateIPAddressLimitExceededError(err) {
log.Warnf("failed to allocate all available IPv4 Prefixes on ENI %s, err: %v", eni.ID, err)
// Try to just get one more prefix
output, err = c.awsClient.AllocIPAddresses(eni.ID, 1)
eni, numOfENIs := c.dataStore.GetENINeedsIP(c.maxPrefixesPerENI, c.useCustomNetworking, c.useSubnetDiscovery, nil)
for i := 0; i < numOfENIs; i++ {
if eni != nil {
currentNumberOfAllocatedPrefixes := len(eni.AvailableIPv4Cidrs)
resourcesToAllocate := min((c.maxPrefixesPerENI - currentNumberOfAllocatedPrefixes), toAllocate)
output, err := c.awsClient.AllocIPAddresses(eni.ID, resourcesToAllocate)
if err != nil && !containsPrivateIPAddressLimitExceededError(err) {
ipamdErrInc("increaseIPPoolAllocIPAddressesFailed")
return false, errors.Wrap(err, fmt.Sprintf("failed to allocate one IPv4 prefix on ENI %s, err: %v", eni.ID, err))
}
}
var ec2Prefixes []*ec2.Ipv4PrefixSpecification
if containsPrivateIPAddressLimitExceededError(err) {
log.Debug("AssignPrivateIpAddresses returned PrivateIpAddressLimitExceeded. This can happen if the data store is out of sync." +
"Returning without an error here since we will verify the actual state by calling EC2 to see what addresses have already assigned to this ENI.")
// This call to EC2 is needed to verify which IPs got attached to this ENI.
ec2Prefixes, err = c.awsClient.GetIPv4PrefixesFromEC2(eni.ID)
if err != nil {
ipamdErrInc("increaseIPPoolGetENIaddressesFailed")
return true, errors.Wrap(err, "failed to get ENI IP addresses during IP allocation")
log.Warnf("failed to allocate all available IPv4 Prefixes on ENI %s, err: %v", eni.ID, err)
// Try to just get one more prefix
output, err = c.awsClient.AllocIPAddresses(eni.ID, 1)
if err != nil && !containsPrivateIPAddressLimitExceededError(err) {
ipamdErrInc("increaseIPPoolAllocIPAddressesFailed")
if c.useSubnetDiscovery && containsInsufficientCIDRsOrSubnetIPs(err) {
insufficientENI[eni] = true
eni, _ = c.dataStore.GetENINeedsIP(c.maxIPsPerENI, c.useCustomNetworking, c.useSubnetDiscovery, insufficientENI)
continue
}
return false, errors.Wrap(err, fmt.Sprintf("failed to allocate one IPv4 prefix on ENI %s, err: %v", eni.ID, err))