Merge pull request #169 from liwenwu-amazon/cronjob

Maintain the right number of  ENIs and its IP addresses in WARM-IP pool

ipamd/ipamd.go

@@ -108,8 +108,8 @@ type IPAMContext struct {
 	dockerClient  docker.APIs
 	networkClient networkutils.NetworkAPIs
 
-	currentMaxAddrsPerENI int
-	maxAddrsPerENI        int
+	currentMaxAddrsPerENI int64
+	maxAddrsPerENI        int64
 	// 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
@@ -359,7 +359,7 @@ func (c *IPAMContext) decreaseIPPool() {
 	c.lastNodeIPPoolAction = time.Now()
 	total, used := c.dataStore.GetStats()
 	log.Debugf("Successfully decreased IP Pool")
-	logPoolStats(total, used, c.currentMaxAddrsPerENI, c.maxAddrsPerENI)
+	logPoolStats(int64(total), int64(used), c.currentMaxAddrsPerENI, c.maxAddrsPerENI)
 }
 
 func isAttachmentLimitExceededError(err error) bool {
@@ -438,7 +438,7 @@ func (c *IPAMContext) increaseIPPool() {
 	c.lastNodeIPPoolAction = time.Now()
 	total, used := c.dataStore.GetStats()
 	log.Debugf("Successfully increased IP Pool")
-	logPoolStats(total, used, c.currentMaxAddrsPerENI, c.maxAddrsPerENI)
+	logPoolStats(int64(total), int64(used), c.currentMaxAddrsPerENI, c.maxAddrsPerENI)
 }
 
 // setupENI does following:
@@ -458,7 +458,15 @@ func (c *IPAMContext) setupENI(eni string, eniMetadata awsutils.ENIMetadata) err
 		return errors.Wrapf(err, "failed to retrieve eni %s ip addresses", eni)
 	}
 
-	c.currentMaxAddrsPerENI = len(ec2Addrs)
+	c.currentMaxAddrsPerENI, err = c.awsClient.GetENIipLimit()
+
+	if err != nil {
+		// if the instance type is not supported in ipamD and the GetENIipLimit() call returns an error
+		// the code here fallbacks to use the number of IPs discovered on the ENI.
+		// note: the number of IP discovered on the ENI at a time can be less than the number of supported IPs on
+		// an ENI, for example:  ipamD has NOT allocated all IPs on the ENI yet.
+		c.currentMaxAddrsPerENI = int64(len(ec2Addrs))
+	}
 	if c.currentMaxAddrsPerENI > c.maxAddrsPerENI {
 		c.maxAddrsPerENI = c.currentMaxAddrsPerENI
 	}
@@ -562,7 +570,7 @@ func getWarmENITarget() int {
 	return defaultWarmENITarget
 }
 
-func logPoolStats(total, used, currentMaxAddrsPerENI, maxAddrsPerENI int) {
+func logPoolStats(total, used, currentMaxAddrsPerENI, maxAddrsPerENI int64) {
 	log.Debugf("IP pool stats: total = %d, used = %d, c.currentMaxAddrsPerENI = %d, c.maxAddrsPerENI = %d",
 		total, used, currentMaxAddrsPerENI, maxAddrsPerENI)
 }
@@ -581,26 +589,26 @@ func (c *IPAMContext) nodeIPPoolTooLow() bool {
 	// 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)
+	logPoolStats(int64(total), int64(used), c.currentMaxAddrsPerENI, c.maxAddrsPerENI)
 
 	available := total - used
-	return (available <= c.maxAddrsPerENI*warmENITarget)
+	return (int64(available) < c.maxAddrsPerENI*int64(warmENITarget))
 }
 
 // NodeIPPoolTooHigh returns true if IP pool is above high threshold
 func (c *IPAMContext) nodeIPPoolTooHigh() bool {
 	warmENITarget := getWarmENITarget()
 	total, used := c.dataStore.GetStats()
-	logPoolStats(total, used, c.currentMaxAddrsPerENI, c.maxAddrsPerENI)
+	logPoolStats(int64(total), int64(used), c.currentMaxAddrsPerENI, c.maxAddrsPerENI)
 
 	available := total - used
 
 	target := getWarmIPTarget()
-	if target != noWarmIPTarget && target >= available {
+	if int64(target) != noWarmIPTarget && int64(target) >= int64(available) {
 		return false
 	}
 
-	return (available > (warmENITarget+1)*c.maxAddrsPerENI)
+	return (int64(available) >= (int64(warmENITarget)+1)*c.maxAddrsPerENI)
 }
 
 func ipamdErrInc(fn string, err error) {
@@ -739,17 +747,17 @@ func getWarmIPTarget() int {
 	return noWarmIPTarget
 }
 
-func (c *IPAMContext) getCurWarmIPTarget() (int, bool) {
+func (c *IPAMContext) getCurWarmIPTarget() (int64, bool) {
 	target := getWarmIPTarget()
 	if target == noWarmIPTarget {
 		// there is no WARM_IP_TARGET defined, fallback to use all IP addresses on ENI
-		return target, false
+		return int64(target), false
 	}
 
 	total, used := c.dataStore.GetStats()
 	log.Debugf("Current warm IP stats: target: %d, total: %d, used: %d",
 		target, total, used)
-	curTarget := target - (total - used)
+	curTarget := int64(target) - int64(total-used)
 
 	return curTarget, true
 }

ipamd/ipamd_test.go

@@ -111,6 +111,7 @@ func TestNodeInit(t *testing.T) {
 			PrivateIpAddress: &testAddr1, Primary: &primary},
 		{
 			PrivateIpAddress: &testAddr2, Primary: &primary}}
+	mockAWS.EXPECT().GetENIipLimit().Return(int64(56), nil)
 	mockAWS.EXPECT().GetPrimaryENI().Return(primaryENIid)
 	mockAWS.EXPECT().DescribeENI(primaryENIid).Return(eniResp, &attachmentID, nil)
 
@@ -125,6 +126,7 @@ func TestNodeInit(t *testing.T) {
 			PrivateIpAddress: &testAddr11, Primary: &primary},
 		{
 			PrivateIpAddress: &testAddr12, Primary: &primary}}
+	mockAWS.EXPECT().GetENIipLimit().Return(int64(56), nil)
 	mockAWS.EXPECT().GetPrimaryENI().Return(primaryENIid)
 	mockAWS.EXPECT().DescribeENI(secENIid).Return(eniResp, &attachmentID, nil)
 	mockNetwork.EXPECT().SetupENINetwork(gomock.Any(), secMAC, secDevice, secSubnet)
@@ -181,6 +183,7 @@ func TestIncreaseIPPool(t *testing.T) {
 			LocalIPv4s:     []string{ipaddr11, ipaddr12}},
 	}, nil)
 
+	mockAWS.EXPECT().GetENIipLimit().Return(int64(5), nil)
 	mockAWS.EXPECT().GetPrimaryENI().Return(primaryENIid)
 
 	primary := false
@@ -239,6 +242,7 @@ func TestNodeIPPoolReconcile(t *testing.T) {
 				PrivateIpAddress: &testAddr1, Primary: &primary},
 			{
 				PrivateIpAddress: &testAddr2, Primary: &notPrimary}}, &attachmentID, nil)
+	mockAWS.EXPECT().GetENIipLimit().Return(int64(5), nil)
 	mockAWS.EXPECT().GetPrimaryENI().Return(primaryENIid)
 
 	mockContext.nodeIPPoolReconcile(0)
@@ -309,7 +313,7 @@ func TestGetCurWarmIPTarget(t *testing.T) {
 	os.Setenv("WARM_IP_TARGET", "5")
 	curWarmIPTarget, warmIPTargetDefined := mockContext.getCurWarmIPTarget()
 	assert.True(t, warmIPTargetDefined)
-	assert.Equal(t, curWarmIPTarget, 5)
+	assert.Equal(t, curWarmIPTarget, int64(5))
 
 	// add 2 addresses to datastore
 	mockContext.dataStore.AddENI("eni-1", 1, true)
@@ -318,7 +322,7 @@ func TestGetCurWarmIPTarget(t *testing.T) {
 
 	curWarmIPTarget, warmIPTargetDefined = mockContext.getCurWarmIPTarget()
 	assert.True(t, warmIPTargetDefined)
-	assert.Equal(t, curWarmIPTarget, 3)
+	assert.Equal(t, curWarmIPTarget, int64(3))
 
 	// add 3 more addresses to datastore
 	mockContext.dataStore.AddENIIPv4Address("eni-1", "1.1.1.3")
@@ -327,5 +331,5 @@ func TestGetCurWarmIPTarget(t *testing.T) {
 
 	curWarmIPTarget, warmIPTargetDefined = mockContext.getCurWarmIPTarget()
 	assert.True(t, warmIPTargetDefined)
-	assert.Equal(t, curWarmIPTarget, 0)
+	assert.Equal(t, curWarmIPTarget, int64(0))
 }