Skip to content

Commit 0f699d4

Browse files
author
Joseph Chen
committed
Subnet Discovery - Unfilled ENI fix
1 parent 8f9253e commit 0f699d4

2 files changed

Lines changed: 91 additions & 65 deletions

File tree

‎pkg/ipamd/datastore/data_store.go‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -974,21 +974,28 @@ func (e *ENI) hasPods() bool {
974974
}
975975

976976
// GetENINeedsIP finds an ENI in the datastore that needs more IP addresses allocated
977-
func (ds *DataStore) GetENINeedsIP(maxIPperENI int, skipPrimary bool) *ENI {
977+
func (ds *DataStore) GetENINeedsIP(maxIPperENI int, skipPrimary bool, useSubnetDiscovery bool, insufficientENI map[*ENI]bool) (*ENI, int) {
978978
ds.lock.Lock()
979979
defer ds.lock.Unlock()
980980
for _, eni := range ds.eniPool {
981981
if (skipPrimary && eni.IsPrimary) || eni.IsTrunk {
982982
ds.log.Debugf("Skip needs IP check for trunk ENI of primary ENI when Custom Networking is enabled")
983983
continue
984984
}
985+
if _, ok := insufficientENI[eni]; ok {
986+
continue
987+
}
985988
if len(eni.AvailableIPv4Cidrs) < maxIPperENI {
986989
ds.log.Debugf("Found ENI %s that has less than the maximum number of IP/Prefixes addresses allocated: cur=%d, max=%d",
987990
eni.ID, len(eni.AvailableIPv4Cidrs), maxIPperENI)
988-
return eni
991+
if useSubnetDiscovery {
992+
return eni, len(ds.eniPool)
993+
}
994+
return eni, 1
989995
}
990996
}
991-
return nil
997+
// Returning 1 since we want one iteration of IP allcocation if subnet discovery is off
998+
return nil, 1
992999
}
9931000

9941001
// RemoveUnusedENIFromStore removes a deletable ENI from the data store.

‎pkg/ipamd/ipamd.go‎

Lines changed: 81 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -907,6 +907,8 @@ func (c *IPAMContext) tryAssignCidrs() (increasedPool bool, err error) {
907907
// For an ENI, try to fill in missing IPs on an existing ENI.
908908
// PRECONDITION: isDatastorePoolTooLow returned true
909909
func (c *IPAMContext) tryAssignIPs() (increasedPool bool, err error) {
910+
insufficientENI := make(map[*datastore.ENI]bool)
911+
910912
// If WARM_IP_TARGET is set, only proceed if we are short of target
911913
short, _, warmIPTargetsDefined := c.datastoreTargetState(nil)
912914
if warmIPTargetsDefined && short == 0 {
@@ -920,45 +922,52 @@ func (c *IPAMContext) tryAssignIPs() (increasedPool bool, err error) {
920922
}
921923

922924
// Find an ENI where we can add more IPs
923-
eni := c.dataStore.GetENINeedsIP(c.maxIPsPerENI, c.useCustomNetworking)
924-
if eni != nil && len(eni.AvailableIPv4Cidrs) < c.maxIPsPerENI {
925-
currentNumberOfAllocatedIPs := len(eni.AvailableIPv4Cidrs)
926-
// Try to allocate all available IPs for this ENI
927-
resourcesToAllocate := min((c.maxIPsPerENI - currentNumberOfAllocatedIPs), toAllocate)
928-
output, err := c.awsClient.AllocIPAddresses(eni.ID, resourcesToAllocate)
929-
if err != nil && !containsPrivateIPAddressLimitExceededError(err) {
930-
log.Warnf("failed to allocate all available IP addresses on ENI %s, err: %v", eni.ID, err)
931-
// Try to just get one more IP
932-
output, err = c.awsClient.AllocIPAddresses(eni.ID, 1)
925+
eni, numOfENIs := c.dataStore.GetENINeedsIP(c.maxIPsPerENI, c.useCustomNetworking, c.useSubnetDiscovery, insufficientENI)
926+
for i := 0; i < numOfENIs; i++ {
927+
if eni != nil && len(eni.AvailableIPv4Cidrs) < c.maxIPsPerENI {
928+
currentNumberOfAllocatedIPs := len(eni.AvailableIPv4Cidrs)
929+
// Try to allocate all available IPs for this ENI
930+
resourcesToAllocate := min((c.maxIPsPerENI - currentNumberOfAllocatedIPs), toAllocate)
931+
output, err := c.awsClient.AllocIPAddresses(eni.ID, resourcesToAllocate)
933932
if err != nil && !containsPrivateIPAddressLimitExceededError(err) {
934-
ipamdErrInc("increaseIPPoolAllocIPAddressesFailed")
935-
return false, errors.Wrap(err, fmt.Sprintf("failed to allocate one IP addresses on ENI %s, err ", eni.ID))
933+
log.Warnf("failed to allocate all available IP addresses on ENI %s, err: %v", eni.ID, err)
934+
// Try to just get one more IP
935+
output, err = c.awsClient.AllocIPAddresses(eni.ID, 1)
936+
if err != nil && !containsPrivateIPAddressLimitExceededError(err) {
937+
ipamdErrInc("increaseIPPoolAllocIPAddressesFailed")
938+
if c.useSubnetDiscovery && containsInsufficientCIDRsOrSubnetIPs(err) {
939+
insufficientENI[eni] = true
940+
eni, _ = c.dataStore.GetENINeedsIP(c.maxIPsPerENI, c.useCustomNetworking, c.useSubnetDiscovery, insufficientENI)
941+
continue
942+
}
943+
return false, errors.Wrap(err, fmt.Sprintf("failed to allocate one IP addresses on ENI %s, err ", eni.ID))
944+
}
936945
}
937-
}
938946

939-
var ec2ip4s []*ec2.NetworkInterfacePrivateIpAddress
940-
if containsPrivateIPAddressLimitExceededError(err) {
941-
log.Debug("AssignPrivateIpAddresses returned PrivateIpAddressLimitExceeded. This can happen if the data store is out of sync." +
942-
"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.")
943-
// This call to EC2 is needed to verify which IPs got attached to this ENI.
944-
ec2ip4s, err = c.awsClient.GetIPv4sFromEC2(eni.ID)
945-
if err != nil {
946-
ipamdErrInc("increaseIPPoolGetENIaddressesFailed")
947-
return true, errors.Wrap(err, "failed to get ENI IP addresses during IP allocation")
948-
}
949-
} else {
950-
if output == nil {
951-
ipamdErrInc("increaseIPPoolGetENIaddressesFailed")
952-
return true, errors.Wrap(err, "failed to get ENI IP addresses during IP allocation")
953-
}
947+
var ec2ip4s []*ec2.NetworkInterfacePrivateIpAddress
948+
if containsPrivateIPAddressLimitExceededError(err) {
949+
log.Debug("AssignPrivateIpAddresses returned PrivateIpAddressLimitExceeded. This can happen if the data store is out of sync." +
950+
"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.")
951+
// This call to EC2 is needed to verify which IPs got attached to this ENI.
952+
ec2ip4s, err = c.awsClient.GetIPv4sFromEC2(eni.ID)
953+
if err != nil {
954+
ipamdErrInc("increaseIPPoolGetENIaddressesFailed")
955+
return true, errors.Wrap(err, "failed to get ENI IP addresses during IP allocation")
956+
}
957+
} else {
958+
if output == nil {
959+
ipamdErrInc("increaseIPPoolGetENIaddressesFailed")
960+
return true, errors.Wrap(err, "failed to get ENI IP addresses during IP allocation")
961+
}
954962

955-
ec2Addrs := output.AssignedPrivateIpAddresses
956-
for _, ec2Addr := range ec2Addrs {
957-
ec2ip4s = append(ec2ip4s, &ec2.NetworkInterfacePrivateIpAddress{PrivateIpAddress: aws.String(aws.StringValue(ec2Addr.PrivateIpAddress))})
963+
ec2Addrs := output.AssignedPrivateIpAddresses
964+
for _, ec2Addr := range ec2Addrs {
965+
ec2ip4s = append(ec2ip4s, &ec2.NetworkInterfacePrivateIpAddress{PrivateIpAddress: aws.String(aws.StringValue(ec2Addr.PrivateIpAddress))})
966+
}
958967
}
968+
c.addENIsecondaryIPsToDataStore(ec2ip4s, eni.ID)
969+
return true, nil
959970
}
960-
c.addENIsecondaryIPsToDataStore(ec2ip4s, eni.ID)
961-
return true, nil
962971
}
963972
return false, nil
964973
}
@@ -1004,42 +1013,52 @@ func (c *IPAMContext) assignIPv6Prefix(eniID string) (err error) {
10041013

10051014
// PRECONDITION: isDatastorePoolTooLow returned true
10061015
func (c *IPAMContext) tryAssignPrefixes() (increasedPool bool, err error) {
1016+
insufficientENI := make(map[*datastore.ENI]bool)
1017+
10071018
toAllocate := c.getPrefixesNeeded()
10081019
// Returns an ENI which has space for more prefixes to be attached, but this
10091020
// ENI might not suffice the WARM_IP_TARGET/WARM_PREFIX_TARGET
1010-
eni := c.dataStore.GetENINeedsIP(c.maxPrefixesPerENI, c.useCustomNetworking)
1011-
if eni != nil {
1012-
currentNumberOfAllocatedPrefixes := len(eni.AvailableIPv4Cidrs)
1013-
resourcesToAllocate := min((c.maxPrefixesPerENI - currentNumberOfAllocatedPrefixes), toAllocate)
1014-
output, err := c.awsClient.AllocIPAddresses(eni.ID, resourcesToAllocate)
1015-
if err != nil && !containsPrivateIPAddressLimitExceededError(err) {
1016-
log.Warnf("failed to allocate all available IPv4 Prefixes on ENI %s, err: %v", eni.ID, err)
1017-
// Try to just get one more prefix
1018-
output, err = c.awsClient.AllocIPAddresses(eni.ID, 1)
1021+
eni, numOfENIs := c.dataStore.GetENINeedsIP(c.maxPrefixesPerENI, c.useCustomNetworking, c.useSubnetDiscovery, nil)
1022+
for i := 0; i < numOfENIs; i++ {
1023+
if eni != nil {
1024+
currentNumberOfAllocatedPrefixes := len(eni.AvailableIPv4Cidrs)
1025+
resourcesToAllocate := min((c.maxPrefixesPerENI - currentNumberOfAllocatedPrefixes), toAllocate)
1026+
output, err := c.awsClient.AllocIPAddresses(eni.ID, resourcesToAllocate)
10191027
if err != nil && !containsPrivateIPAddressLimitExceededError(err) {
1020-
ipamdErrInc("increaseIPPoolAllocIPAddressesFailed")
1021-
return false, errors.Wrap(err, fmt.Sprintf("failed to allocate one IPv4 prefix on ENI %s, err: %v", eni.ID, err))
1022-
}
1023-
}
1024-
var ec2Prefixes []*ec2.Ipv4PrefixSpecification
1025-
if containsPrivateIPAddressLimitExceededError(err) {
1026-
log.Debug("AssignPrivateIpAddresses returned PrivateIpAddressLimitExceeded. This can happen if the data store is out of sync." +
1027-
"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.")
1028-
// This call to EC2 is needed to verify which IPs got attached to this ENI.
1029-
ec2Prefixes, err = c.awsClient.GetIPv4PrefixesFromEC2(eni.ID)
1030-
if err != nil {
1031-
ipamdErrInc("increaseIPPoolGetENIaddressesFailed")
1032-
return true, errors.Wrap(err, "failed to get ENI IP addresses during IP allocation")
1028+
log.Warnf("failed to allocate all available IPv4 Prefixes on ENI %s, err: %v", eni.ID, err)
1029+
// Try to just get one more prefix
1030+
output, err = c.awsClient.AllocIPAddresses(eni.ID, 1)
1031+
if err != nil && !containsPrivateIPAddressLimitExceededError(err) {
1032+
ipamdErrInc("increaseIPPoolAllocIPAddressesFailed")
1033+
if c.useSubnetDiscovery && containsInsufficientCIDRsOrSubnetIPs(err) {
1034+
insufficientENI[eni] = true
1035+
eni, _ = c.dataStore.GetENINeedsIP(c.maxIPsPerENI, c.useCustomNetworking, c.useSubnetDiscovery, insufficientENI)
1036+
continue
1037+
}
1038+
return false, errors.Wrap(err, fmt.Sprintf("failed to allocate one IPv4 prefix on ENI %s, err: %v", eni.ID, err))
1039+
}
10331040
}
1034-
} else {
1035-
if output == nil {
1036-
ipamdErrInc("increaseIPPoolGetENIprefixedFailed")
1037-
return true, errors.Wrap(err, "failed to get ENI Prefix addresses during IPv4 Prefix allocation")
1041+
1042+
var ec2Prefixes []*ec2.Ipv4PrefixSpecification
1043+
if containsPrivateIPAddressLimitExceededError(err) {
1044+
log.Debug("AssignPrivateIpAddresses returned PrivateIpAddressLimitExceeded. This can happen if the data store is out of sync." +
1045+
"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.")
1046+
// This call to EC2 is needed to verify which IPs got attached to this ENI.
1047+
ec2Prefixes, err = c.awsClient.GetIPv4PrefixesFromEC2(eni.ID)
1048+
if err != nil {
1049+
ipamdErrInc("increaseIPPoolGetENIaddressesFailed")
1050+
return true, errors.Wrap(err, "failed to get ENI IP addresses during IP allocation")
1051+
}
1052+
} else {
1053+
if output == nil {
1054+
ipamdErrInc("increaseIPPoolGetENIprefixedFailed")
1055+
return true, errors.Wrap(err, "failed to get ENI Prefix addresses during IPv4 Prefix allocation")
1056+
}
1057+
ec2Prefixes = output.AssignedIpv4Prefixes
10381058
}
1039-
ec2Prefixes = output.AssignedIpv4Prefixes
1059+
c.addENIv4prefixesToDataStore(ec2Prefixes, eni.ID)
1060+
return true, nil
10401061
}
1041-
c.addENIv4prefixesToDataStore(ec2Prefixes, eni.ID)
1042-
return true, nil
10431062
}
10441063
return false, nil
10451064
}

0 commit comments

Comments
 (0)