Skip to content

Commit

Permalink
code comments and warm prefix 0
Browse files Browse the repository at this point in the history
  • Loading branch information
@jayanthvn
jayanthvn committed Jun 17, 2021
1 parent 5660664 commit bb5c74a
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 41 deletions.
27 changes: 24 additions & 3 deletions pkg/ipamd/datastore/data_store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,12 @@ func (ds *DataStore) isRequiredForWarmIPTarget(warmIPTarget int, eni *ENI) bool
}
}
}

if ds.isPDEnabled {
_, numIPsPerPrefix, _ := GetPrefixDelegationDefaults()
numPrefixNeeded := DivCeil(warmIPTarget, numIPsPerPrefix)
warmIPTarget = numPrefixNeeded * numIPsPerPrefix
}
return otherWarmIPs < warmIPTarget
}

Expand All @@ -718,6 +724,12 @@ func (ds *DataStore) isRequiredForMinimumIPTarget(minimumIPTarget int, eni *ENI)
}
}
}

if ds.isPDEnabled {
_, numIPsPerPrefix, _ := GetPrefixDelegationDefaults()
numPrefixNeeded := DivCeil(minimumIPTarget, numIPsPerPrefix)
minimumIPTarget = numPrefixNeeded * numIPsPerPrefix
}
return otherIPs < minimumIPTarget
}

Expand All @@ -737,7 +749,7 @@ func (ds *DataStore) isRequiredForWarmPrefixTarget(warmPrefixTarget int, eni *EN
return freePrefixes < warmPrefixTarget
}

func (ds *DataStore) getDeletableENI(warmIPTarget, minimumIPTarget int) *ENI {
func (ds *DataStore) getDeletableENI(warmIPTarget, minimumIPTarget, warmPrefixTarget int) *ENI {
for _, eni := range ds.eniPool {
if eni.IsPrimary {
ds.log.Debugf("ENI %s cannot be deleted because it is primary", eni.ID)
Expand Down Expand Up @@ -769,6 +781,11 @@ func (ds *DataStore) getDeletableENI(warmIPTarget, minimumIPTarget int) *ENI {
continue
}

if ds.isPDEnabled && warmPrefixTarget != 0 && ds.isRequiredForWarmPrefixTarget(warmPrefixTarget, eni) {
ds.log.Debugf("ENI %s cannot be deleted because it is required for WARM_PREFIX_TARGET: %d", eni.ID, warmPrefixTarget)
continue
}

if eni.IsTrunk {
ds.log.Debugf("ENI %s cannot be deleted because it is a trunk ENI", eni.ID)
continue
Expand Down Expand Up @@ -828,11 +845,11 @@ func (ds *DataStore) GetENINeedsIP(maxIPperENI int, skipPrimary bool) *ENI {
// RemoveUnusedENIFromStore removes a deletable ENI from the data store.
// It returns the name of the ENI which has been removed from the data store and needs to be deleted,
// or empty string if no ENI could be removed.
func (ds *DataStore) RemoveUnusedENIFromStore(warmIPTarget, minimumIPTarget int) string {
func (ds *DataStore) RemoveUnusedENIFromStore(warmIPTarget, minimumIPTarget, warmPrefixTarget int) string {
ds.lock.Lock()
defer ds.lock.Unlock()

deletableENI := ds.getDeletableENI(warmIPTarget, minimumIPTarget)
deletableENI := ds.getDeletableENI(warmIPTarget, minimumIPTarget, warmPrefixTarget)
if deletableENI == nil {
return ""
}
Expand Down Expand Up @@ -1217,3 +1234,7 @@ func (ds *DataStore) FindFreeableCidrs(eniID string) []CidrInfo {
return freeable

}

func DivCeil(x, y int) int {
return (x + y - 1) / y
}
80 changes: 42 additions & 38 deletions pkg/ipamd/ipamd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ func (c *IPAMContext) tryFreeENI() {
return
}

eni := c.dataStore.RemoveUnusedENIFromStore(c.warmIPTarget, c.minimumIPTarget)
eni := c.dataStore.RemoveUnusedENIFromStore(c.warmIPTarget, c.minimumIPTarget, c.warmPrefixTarget)
if eni == "" {
return
}
Expand Down Expand Up @@ -647,16 +647,18 @@ func (c *IPAMContext) increaseDatastorePool() {
ipamdActionsInprogress.WithLabelValues("increaseDatastorePool").Add(float64(1))
defer ipamdActionsInprogress.WithLabelValues("increaseDatastorePool").Sub(float64(1))

short, _, warmTargetDefined := c.datastoreTargetState()
if warmTargetDefined && short == 0 {
short, _, warmIPTargetDefined := c.datastoreTargetState()
if warmIPTargetDefined && short == 0 {
log.Debugf("Skipping increase Datastore pool, warm target reached")
return
}

short, warmTargetDefined = c.datastorePrefixTargetState()
if warmTargetDefined && short == 0 {
log.Debugf("Skipping increase Datastore pool, warm prefix target reached")
return
if !warmIPTargetDefined {
shortPrefix, warmTargetDefined := c.datastorePrefixTargetState()
if warmTargetDefined && shortPrefix == 0 {
log.Debugf("Skipping increase Datastore pool, warm prefix target reached")
return
}
}

if c.isTerminating() {
Expand Down Expand Up @@ -759,17 +761,20 @@ func (c *IPAMContext) tryAllocateENI() error {
// For an ENI, try to fill in missing IPs on an existing ENI with PD disabled
// try to fill in missing Prefixes on an existing ENI with PD enabled
func (c *IPAMContext) tryAssignCidrs() (increasedPool bool, err error) {
short, _, warmTargetDefined := c.datastoreTargetState()
if warmTargetDefined && short == 0 {
short, _, warmIPTargetDefined := c.datastoreTargetState()
if warmIPTargetDefined && short == 0 {
log.Infof("Warm IP target set and short is 0 so not assigning Cidrs (IPs or Prefixes)")
return false, nil
}

short, warmTargetDefined = c.datastorePrefixTargetState()
if warmTargetDefined && short == 0 {
log.Infof("Warm prefix target set and short is 0 so not assigning Cidrs (Prefixes)")
return false, nil
if !warmIPTargetDefined {
shortPrefix, warmTargetDefined := c.datastorePrefixTargetState()
if warmTargetDefined && shortPrefix == 0 {
log.Infof("Warm prefix target set and short is 0 so not assigning Cidrs (Prefixes)")
return false, nil
}
}

if !c.enableIpv4PrefixDelegation {
return c.tryAssignIPs()
} else {
Expand Down Expand Up @@ -995,6 +1000,9 @@ func (c *IPAMContext) askForTrunkENIIfNeeded() {

// shouldRemoveExtraENIs returns true if we should attempt to find an ENI to free. When WARM_IP_TARGET is set, we
// always check and do verification in getDeletableENI()
// PD enabled : If the WARM_PREFIX_TARGET is spread across ENIs and we have more than needed then this function will return true.
// but if the number of prefixes are on just one ENI and is more than available even then it returns true so getDeletableENI will
// recheck if we need the ENI for prefix target.
func (c *IPAMContext) shouldRemoveExtraENIs() bool {
_, _, warmTargetDefined := c.datastoreTargetState()
if warmTargetDefined {
Expand Down Expand Up @@ -1029,20 +1037,25 @@ func (c *IPAMContext) shouldRemoveExtraPrefixes() int {

total, used, _ := c.dataStore.GetStats()
available := total - used
var shouldRemoveExtra bool

warmTarget := (c.warmPrefixTarget + 1)
freePrefixes := c.dataStore.GetFreePrefixes()
_, numIPsPerPrefix, _ := datastore.GetPrefixDelegationDefaults()

shouldRemoveExtra = available >= (warmTarget)*c.maxPrefixesPerENI
if shouldRemoveExtra {
freePrefixes := c.dataStore.GetFreePrefixes()
over = max(freePrefixes-c.warmPrefixTarget, 0)

over = max(freePrefixes-c.warmPrefixTarget, 0)
logPoolStats(total, used, c.maxIPsPerENI, c.enableIpv4PrefixDelegation)
log.Debugf("It might be possible to remove extra prefixes because available (%d) >= (Prefix target + 1 (%d) + 1) * prefixesPerENI (%d)", available, warmTarget, c.maxPrefixesPerENI)
//if warm prefix target is 0, we should not make available to reach 0 since reconciler will enter into a churn
// of allocating and freeing prefixes similar to issue. This will add continous EC2 calls on nodes which have
// no prefixes used. Hence better to handle it.
// [ENI allocates/frees in loop with custom networking and WARM_ENI_TARGET=0 #1451]
// But Will sync up internally with the team and see if need to even support WARM_ENI_TARGET = 0 and WARM_PREFIX_TARGET = 0
if over > 0 && ((available - (over * numIPsPerPrefix)) <= 0) && c.warmPrefixTarget == 0 {
log.Debugf("Warm prefix target is 0 and we will go below available hence capping to have atleast 1 free prefix")
over -= 1
}
logPoolStats(total, used, c.maxIPsPerENI, c.enableIpv4PrefixDelegation)
log.Debugf("shouldRemoveExtraPrefixes available %d over %d warm_prefix_target %d", available, over, c.warmPrefixTarget)

return over

}

func ipamdErrInc(fn string) {
Expand Down Expand Up @@ -1515,13 +1528,13 @@ func (c *IPAMContext) datastoreTargetState() (short int, over int, enabled bool)

_, numIPsPerPrefix, _ := datastore.GetPrefixDelegationDefaults()
// Number of prefixes IPAMD is short of to achieve warm targets
shortPrefix := divCeil(short, numIPsPerPrefix)
shortPrefix := datastore.DivCeil(short, numIPsPerPrefix)

// Over will have number of IPs more than needed but with PD we would have allocated in chunks of /28
// Say assigned = 1, warm ip target = 16, this will need 2 prefixes. But over will return 15.
// Hence we need to check if 'over' number of IPs are needed to maintain the warm targets
prefixNeededForWarmIP := divCeil(assigned+c.warmIPTarget, numIPsPerPrefix)
prefixNeededForMinIP := divCeil(c.minimumIPTarget, numIPsPerPrefix)
prefixNeededForWarmIP := datastore.DivCeil(assigned+c.warmIPTarget, numIPsPerPrefix)
prefixNeededForMinIP := datastore.DivCeil(c.minimumIPTarget, numIPsPerPrefix)

// over will be number of prefixes over than needed but could be spread across used prefixes,
// say, after couple of pod churns, 3 prefixes are allocated with 1 IP each assigned and warm ip target is 15
Expand All @@ -1548,8 +1561,7 @@ func (c *IPAMContext) datastorePrefixTargetState() (short int, enabled bool) {
toAllocate := max(c.warmPrefixTarget-freePrefixesInStore, 0)
log.Debugf("Prefix target is %d, short of %d prefixes, free %d prefixes", c.warmPrefixTarget, toAllocate, freePrefixesInStore)

toAllocate -= freePrefixesInStore
return toAllocate, true
return toAllocate, true
}

// setTerminating atomically sets the terminating flag.
Expand Down Expand Up @@ -1783,13 +1795,10 @@ func (c *IPAMContext) isDatastorePoolTooHigh() bool {

//For the existing ENIs check if we can cleanup prefixes
if c.warmPrefixTargetDefined() {
total, used, _ := c.dataStore.GetStats()
available := total - used
_, maxIpsPerPrefix, _ := datastore.GetPrefixDelegationDefaults()
poolTooHigh := available > (maxIpsPerPrefix * (c.warmPrefixTarget + 1))
freePrefixes := c.dataStore.GetFreePrefixes()
poolTooHigh := freePrefixes > c.warmPrefixTarget
if poolTooHigh {
logPoolStats(total, used, c.maxIPsPerENI, c.enableIpv4PrefixDelegation)
log.Debugf("Prefix pool is high: available (%d) > Warm prefix target (%d)+1 * maxIpsPerPrefix (%d)", available, c.warmPrefixTarget, maxIpsPerPrefix)
log.Debugf("Prefix pool is high so might be able to deallocate : free prefixes %d and warm prefix target %d", freePrefixes, c.warmPrefixTarget)
}
return poolTooHigh
}
Expand All @@ -1801,11 +1810,6 @@ func (c *IPAMContext) warmPrefixTargetDefined() bool {
return c.warmPrefixTarget >= noWarmPrefixTarget && c.enableIpv4PrefixDelegation
}


func divCeil(x, y int) int {
return (x + y - 1) / y
}

//DeallocCidrs frees IPs and Prefixes from EC2
func (c *IPAMContext) DeallocCidrs(eniID string, deletableCidrs []datastore.CidrInfo) {
var deletableIPs []string
Expand Down

0 comments on commit bb5c74a

Please sign in to comment.