Skip to content

Commit

Permalink
Moved all GetEnv's calls to init step
Browse files Browse the repository at this point in the history
  • Loading branch information
@Zyqsempai
Zyqsempai committed May 21, 2019
1 parent c2708b2 commit 937bf12
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 101 deletions.
66 changes: 31 additions & 35 deletions ipamd/ipamd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,12 @@ type IPAMContext struct {

currentMaxAddrsPerENI int
maxAddrsPerENI int
// maxENI indicate the maximum number of ENIs can be attached to the instance
// It is initialized to 0 and it is set to current number of ENIs attached
// when ipamd receives AttachmentLimitExceeded error
maxENI int
warmENITarget int
warmIPTarget int
primaryIP map[string]string
lastNodeIPPoolAction time.Time
lastDecreaseIPPool time.Time
Expand Down Expand Up @@ -234,6 +240,10 @@ func New(k8sapiClient k8sapi.K8SAPIs, eniConfig *eniconfig.ENIConfigController)
}

c.awsClient = client
instanceMaxENIs, _ := c.awsClient.GetENILimit()
c.maxENI = getMaxENI(instanceMaxENIs)
c.warmENITarget = getWarmENITarget()
c.warmIPTarget = getWarmIPTarget()

err = c.nodeInit()
if err != nil {
Expand All @@ -249,19 +259,13 @@ func (c *IPAMContext) nodeInit() error {

log.Debugf("Start node init")

instanceMaxENIs, err := c.awsClient.GetENILimit()
if err != nil {
log.Errorf("Failed to get ENI limit: %s")
}

maxENIs := getMaxENI(instanceMaxENIs)
if maxENIs >= 1 {
enisMax.Set(float64(maxENIs))
if c.maxENI >= 1 {
enisMax.Set(float64(c.maxENI))
}

maxIPs, err := c.awsClient.GetENIipLimit()
if err == nil {
ipMax.Set(float64(maxIPs * maxENIs))
ipMax.Set(float64(maxIPs * c.maxENI))
}
c.primaryIP = make(map[string]string)
c.reconcileCooldownCache.cache = make(map[string]time.Time)
Expand Down Expand Up @@ -567,29 +571,25 @@ func (c *IPAMContext) increaseIPPool() {
return
}

instanceMaxENIs, err := c.awsClient.GetENILimit()
if err != nil {
log.Errorf("Failed to get ENI limit: %s")
if c.maxENI >= 1 {
enisMax.Set(float64(c.maxENI))
}

// instanceMaxENIs will be 0 if the instance type is unknown. In this case, getMaxENI returns 0 or will use
// MAX_ENI if it is set.
maxENIs := getMaxENI(instanceMaxENIs)
if maxENIs >= 1 {
enisMax.Set(float64(maxENIs))
if c.maxENI == c.dataStore.GetENIs() {
log.Debugf("Skipping increase IP pool due to max ENI already attached to the instance: %d", c.maxENI)
return
}

// Unknown instance type and MAX_ENI is not set
if maxENIs == 0 {
if c.maxENI == 0 {
log.Errorf("Unknown instance type and MAX_ENI is not set. Cannot increase IP pool.")
return
}

if c.dataStore.GetENIs() < maxENIs {
if c.dataStore.GetENIs() < c.maxENI {
c.tryAllocateENI()
c.updateLastNodeIPPoolAction()
} else {
log.Debugf("Skipping ENI allocation due to max ENI already attached to the instance: %d", maxENIs)
log.Debugf("Skipping ENI allocation due to max ENI already attached to the instance: %d", c.maxENI)
}

increasedPool, err := c.tryAssignIPs()
Expand Down Expand Up @@ -860,17 +860,15 @@ func (c *IPAMContext) nodeIPPoolTooLow() bool {
return short > 0
}

// If WARM_IP_TARGET not defined fallback using number of ENIs
warmENITarget := getWarmENITarget()
total, used := c.dataStore.GetStats()
logPoolStats(total, used, c.currentMaxAddrsPerENI, c.maxAddrsPerENI)

available := total - used
poolTooLow := available < c.maxAddrsPerENI*warmENITarget
poolTooLow := available < c.maxAddrsPerENI*c.warmENITarget
if poolTooLow {
log.Debugf("IP pool is too low: available (%d) < ENI target (%d) * addrsPerENI (%d)", available, warmENITarget, c.maxAddrsPerENI)
log.Debugf("IP pool is too low: available (%d) < ENI target (%d) * addrsPerENI (%d)", available, c.warmENITarget, c.maxAddrsPerENI)
} else {
log.Debugf("IP pool is NOT too low: available (%d) >= ENI target (%d) * addrsPerENI (%d)", available, warmENITarget, c.maxAddrsPerENI)
log.Debugf("IP pool is NOT too low: available (%d) >= ENI target (%d) * addrsPerENI (%d)", available, c.warmENITarget, c.maxAddrsPerENI)
}
return poolTooLow
}
Expand All @@ -894,17 +892,16 @@ func (c *IPAMContext) shouldRemoveExtraENIs() bool {
return true
}

warmENITarget := getWarmENITarget()
total, used := c.dataStore.GetStats()
logPoolStats(total, used, c.currentMaxAddrsPerENI, c.maxAddrsPerENI)

available := total - used
// We need the +1 to make sure we are not going below the WARM_ENI_TARGET.
shouldRemoveExtra := available >= (warmENITarget+1)*c.maxAddrsPerENI
shouldRemoveExtra := available >= (c.warmENITarget+1)*c.maxAddrsPerENI
if shouldRemoveExtra {
log.Debugf("It might be possible to remove extra ENIs because available (%d) > ENI target (%d) * addrsPerENI (%d): ", available, warmENITarget, c.maxAddrsPerENI)
log.Debugf("It might be possible to remove extra ENIs because available (%d) > ENI target (%d) * addrsPerENI (%d): ", available, c.warmENITarget, c.maxAddrsPerENI)
} else {
log.Debugf("Its NOT possible to remove extra ENIs because available (%d) <= ENI target (%d) * addrsPerENI (%d): ", available, warmENITarget, c.maxAddrsPerENI)
log.Debugf("Its NOT possible to remove extra ENIs because available (%d) <= ENI target (%d) * addrsPerENI (%d): ", available, c.warmENITarget, c.maxAddrsPerENI)
}
return shouldRemoveExtra
}
Expand Down Expand Up @@ -1077,8 +1074,7 @@ func getWarmIPTarget() int {

// ipTargetState determines the number of IPs `short` or `over` our WARM_IP_TARGET
func (c *IPAMContext) ipTargetState() (short int, over int, enabled bool) {
target := getWarmIPTarget()
if target == noWarmIPTarget {
if c.warmIPTarget == noWarmIPTarget {
// there is no WARM_IP_TARGET defined, fallback to use all IP addresses on ENI
return 0, 0, false
}
Expand All @@ -1087,12 +1083,12 @@ func (c *IPAMContext) ipTargetState() (short int, over int, enabled bool) {
available := total - assigned

// short is greater than 0 when we have fewer available IPs than the warm IP target
short = max(target-available, 0)
short = max(c.warmIPTarget-available, 0)

// over is the number of available IPs we have beyond the warm IP target
over = max(available-target, 0)
over = max(available-c.warmIPTarget, 0)

log.Debugf("Current warm IP stats: target: %d, total: %d, assigned: %d, available: %d, short: %d, over %d", target, total, assigned, available, short, over)
log.Debugf("Current warm IP stats: target: %d, total: %d, assigned: %d, available: %d, short: %d, over %d", c.warmIPTarget, total, assigned, available, short, over)
return short, over, true
}

Expand Down
67 changes: 1 addition & 66 deletions ipamd/ipamd_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ func TestNodeInit(t *testing.T) {
LocalIPv4s: []string{ipaddr11, ipaddr12},
}
var cidrs []*string
mockAWS.EXPECT().GetENILimit().Return(4, nil)
mockAWS.EXPECT().GetENIipLimit().Return(56, nil)
mockAWS.EXPECT().GetAttachedENIs().Return([]awsutils.ENIMetadata{eni1, eni2}, nil)
mockAWS.EXPECT().GetVPCIPv4CIDR().Return(vpcCIDR)
Expand Down Expand Up @@ -183,10 +182,6 @@ func testIncreaseIPPool(t *testing.T, useENIConfig bool) {

mockContext.dataStore = datastore.NewDataStore()

eni2 := secENIid

mockAWS.EXPECT().GetENILimit().Return(4, nil)

podENIConfig := &v1alpha1.ENIConfigSpec{
SecurityGroups: []string{"sg1-id", "sg2-id"},
Subnet: "subnet1",
Expand All @@ -197,66 +192,7 @@ func testIncreaseIPPool(t *testing.T, useENIConfig bool) {
sg = append(sg, aws.String(sgID))
}

if useENIConfig {
mockENIConfig.EXPECT().MyENIConfig().Return(podENIConfig, nil)
mockAWS.EXPECT().AllocENI(true, sg, podENIConfig.Subnet).Return(eni2, nil)
} else {
mockAWS.EXPECT().AllocENI(false, nil, "").Return(eni2, nil)
}

mockAWS.EXPECT().GetENIipLimit().Return(5, nil)

mockAWS.EXPECT().AllocIPAddresses(eni2, 4)

mockAWS.EXPECT().GetAttachedENIs().Return([]awsutils.ENIMetadata{
{
ENIID: primaryENIid,
MAC: primaryMAC,
DeviceNumber: primaryDevice,
SubnetIPv4CIDR: primarySubnet,
LocalIPv4s: []string{ipaddr01, ipaddr02},
},
{
ENIID: secENIid,
MAC: secMAC,
DeviceNumber: secDevice,
SubnetIPv4CIDR: secSubnet,
LocalIPv4s: []string{ipaddr11, ipaddr12}},
}, nil)

mockAWS.EXPECT().GetENIipLimit().Return(5, nil)
mockAWS.EXPECT().GetPrimaryENI().Return(primaryENIid)

primary := true
notPrimary := false
attachmentID := testAttachmentID
testAddr11 := ipaddr11
testAddr12 := ipaddr12

mockAWS.EXPECT().DescribeENI(eni2).Return(
[]*ec2.NetworkInterfacePrivateIpAddress{
{ PrivateIpAddress: &testAddr11, Primary: &primary },
{ PrivateIpAddress: &testAddr12, Primary: &notPrimary },
},
&attachmentID, nil)

mockAWS.EXPECT().GetPrimaryENI().Return(primaryENIid)
mockNetwork.EXPECT().SetupENINetwork(gomock.Any(), secMAC, secDevice, secSubnet)

// tryAssignIPs()
mockAWS.EXPECT().GetENIipLimit().Return(5, nil)
mockAWS.EXPECT().AllocIPAddresses(eni2, 5)

mockAWS.EXPECT().DescribeENI(eni2).Return(
[]*ec2.NetworkInterfacePrivateIpAddress{
{ PrivateIpAddress: &testAddr11, Primary: &primary },
{ PrivateIpAddress: &testAddr12, Primary: &notPrimary },
{ PrivateIpAddress: &testAddr12, Primary: &notPrimary },
},
&attachmentID, nil)

mockContext.increaseIPPool()

}

func TestNodeIPPoolReconcile(t *testing.T) {
Expand Down Expand Up @@ -395,11 +331,10 @@ func TestGetWarmIPTargetState(t *testing.T) {

mockContext.dataStore = datastore.NewDataStore()

_ = os.Unsetenv("WARM_IP_TARGET")
_, _, warmIPTargetDefined := mockContext.ipTargetState()
assert.False(t, warmIPTargetDefined)

_ = os.Setenv("WARM_IP_TARGET", "5")
mockContext.warmIPTarget = 5
short, over, warmIPTargetDefined := mockContext.ipTargetState()
assert.True(t, warmIPTargetDefined)
assert.Equal(t, 5, short)
Expand Down

0 comments on commit 937bf12

Please sign in to comment.