Skip to content

Commit a98fefe

Browse files
author
Joseph Chen
committed
Subnet Discovery - Unfilled ENI fix
1 parent 86f2c72 commit a98fefe

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
@@ -915,6 +915,8 @@ func (c *IPAMContext) tryAssignCidrs() (increasedPool bool, err error) {
915915
// For an ENI, try to fill in missing IPs on an existing ENI.
916916
// PRECONDITION: isDatastorePoolTooLow returned true
917917
func (c *IPAMContext) tryAssignIPs() (increasedPool bool, err error) {
918+
insufficientENI := make(map[*datastore.ENI]bool)
919+
918920
// If WARM_IP_TARGET is set, only proceed if we are short of target
919921
short, _, warmIPTargetsDefined := c.datastoreTargetState(nil)
920922
if warmIPTargetsDefined && short == 0 {
@@ -928,45 +930,52 @@ func (c *IPAMContext) tryAssignIPs() (increasedPool bool, err error) {
928930
}
929931

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

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-
}
955+
var ec2ip4s []*ec2.NetworkInterfacePrivateIpAddress
956+
if containsPrivateIPAddressLimitExceededError(err) {
957+
log.Debug("AssignPrivateIpAddresses returned PrivateIpAddressLimitExceeded. This can happen if the data store is out of sync." +
958+
"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.")
959+
// This call to EC2 is needed to verify which IPs got attached to this ENI.
960+
ec2ip4s, err = c.awsClient.GetIPv4sFromEC2(eni.ID)
961+
if err != nil {
962+
ipamdErrInc("increaseIPPoolGetENIaddressesFailed")
963+
return true, errors.Wrap(err, "failed to get ENI IP addresses during IP allocation")
964+
}
965+
} else {
966+
if output == nil {
967+
ipamdErrInc("increaseIPPoolGetENIaddressesFailed")
968+
return true, errors.Wrap(err, "failed to get ENI IP addresses during IP allocation")
969+
}
962970

963-
ec2Addrs := output.AssignedPrivateIpAddresses
964-
for _, ec2Addr := range ec2Addrs {
965-
ec2ip4s = append(ec2ip4s, &ec2.NetworkInterfacePrivateIpAddress{PrivateIpAddress: aws.String(aws.StringValue(ec2Addr.PrivateIpAddress))})
971+
ec2Addrs := output.AssignedPrivateIpAddresses
972+
for _, ec2Addr := range ec2Addrs {
973+
ec2ip4s = append(ec2ip4s, &ec2.NetworkInterfacePrivateIpAddress{PrivateIpAddress: aws.String(aws.StringValue(ec2Addr.PrivateIpAddress))})
974+
}
966975
}
976+
c.addENIsecondaryIPsToDataStore(ec2ip4s, eni.ID)
977+
return true, nil
967978
}
968-
c.addENIsecondaryIPsToDataStore(ec2ip4s, eni.ID)
969-
return true, nil
970979
}
971980
return false, nil
972981
}
@@ -1012,42 +1021,52 @@ func (c *IPAMContext) assignIPv6Prefix(eniID string) (err error) {
10121021

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

0 commit comments

Comments
 (0)