From 95b84b60385231442a3600453e7d7067b345ba76 Mon Sep 17 00:00:00 2001 From: bpopovschi Date: Fri, 3 May 2019 15:13:41 +0300 Subject: [PATCH] Moved all GetEnv's calls to init step --- ipamd/ipamd.go | 47 +++++++++++++++---------------- ipamd/ipamd_test.go | 67 +-------------------------------------------- 2 files changed, 23 insertions(+), 91 deletions(-) diff --git a/ipamd/ipamd.go b/ipamd/ipamd.go index e7344b1f76..ec9e76e2f7 100644 --- a/ipamd/ipamd.go +++ b/ipamd/ipamd.go @@ -157,6 +157,8 @@ type IPAMContext struct { // 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 @@ -193,6 +195,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 { @@ -208,15 +214,13 @@ func (c *IPAMContext) nodeInit() error { log.Debugf("Start node init") - instanceMaxENIs, _ := c.awsClient.GetENILimit() - 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) @@ -494,14 +498,12 @@ func (c *IPAMContext) increaseIPPool() { return } - instanceMaxENIs, err := c.awsClient.GetENILimit() - maxENIs := getMaxENI(instanceMaxENIs) - if maxENIs >= 1 { - enisMax.Set(float64(maxENIs)) + if c.maxENI >= 1 { + enisMax.Set(float64(c.maxENI)) } - if err == nil && maxENIs == c.dataStore.GetENIs() { - log.Debugf("Skipping increase IP pool due to max ENI already attached to the instance: %d", 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 } if (c.maxENI > 0) && (c.maxENI == c.dataStore.GetENIs()) { @@ -782,29 +784,25 @@ 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 - return available < c.maxAddrsPerENI*warmENITarget + return available < c.maxAddrsPerENI*c.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) available := total - used - target := getWarmIPTarget() - if target != noWarmIPTarget { - return target > available + if c.warmIPTarget != noWarmIPTarget { + return c.warmIPTarget > available } - return available >= (warmENITarget + 1) * c.maxAddrsPerENI + return available >= (c.warmENITarget+1)*c.maxAddrsPerENI } func ipamdErrInc(fn string, err error) { @@ -942,8 +940,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 } @@ -952,12 +949,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 } @@ -982,4 +979,4 @@ func min(x, y int) int { return y } return x -} \ No newline at end of file +} diff --git a/ipamd/ipamd_test.go b/ipamd/ipamd_test.go index 757146264b..2d8f0898cb 100644 --- a/ipamd/ipamd_test.go +++ b/ipamd/ipamd_test.go @@ -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) @@ -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", @@ -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: ¬Primary }, - }, - &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: ¬Primary }, - { PrivateIpAddress: &testAddr12, Primary: ¬Primary }, - }, - &attachmentID, nil) - mockContext.increaseIPPool() - } func TestNodeIPPoolReconcile(t *testing.T) { @@ -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)