Handle private IP exceeded error (#2210)

* Handle private IP exceeded error

* Check for err when adding extra 1 IP

pkg/awsutils/awsutils.go

@@ -935,14 +935,6 @@ func (cache *EC2InstanceMetadataCache) TagENI(eniID string, currentTags map[stri
 	})
 }
 
-// containsPrivateIPAddressLimitExceededError returns whether exceeds ENI's IP address limit
-func containsPrivateIPAddressLimitExceededError(err error) bool {
-	if aerr, ok := err.(awserr.Error); ok {
-		return aerr.Code() == "PrivateIpAddressLimitExceeded"
-	}
-	return false
-}
-
 func awsAPIErrInc(api string, err error) {
 	if aerr, ok := err.(awserr.Error); ok {
 		awsAPIErr.With(prometheus.Labels{"api": api, "error": aerr.Code()}).Inc()
@@ -1524,11 +1516,6 @@ func (cache *EC2InstanceMetadataCache) AllocIPAddresses(eniID string, numIPs int
 	awsAPILatency.WithLabelValues("AssignPrivateIpAddresses", fmt.Sprint(err != nil), awsReqStatus(err)).Observe(msSince(start))
 	if err != nil {
 		CheckAPIErrorAndBroadcastEvent(err, "ec2:AssignPrivateIpAddresses")
-		if containsPrivateIPAddressLimitExceededError(err) {
-			log.Debug("AssignPrivateIpAddresses returned PrivateIpAddressLimitExceeded. This can happen if the data store is out of sync." +
-				"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.")
-			return nil, nil
-		}
 		log.Errorf("Failed to allocate a private IP/Prefix addresses on ENI %v: %v", eniID, err)
 		awsAPIErrInc("AssignPrivateIpAddresses", err)
 		ec2ApiErr.WithLabelValues("AssignPrivateIpAddresses").Inc()

pkg/awsutils/awsutils_test.go

@@ -669,9 +669,9 @@ func TestAllocIPAddressesAlreadyFull(t *testing.T) {
 
 	retErr := awserr.New("PrivateIpAddressLimitExceeded", "Too many IPs already allocated", nil)
 	mockEC2.EXPECT().AssignPrivateIpAddressesWithContext(gomock.Any(), input, gomock.Any()).Return(nil, retErr)
-	// If EC2 says that all IPs are already attached, we do nothing
+	// If EC2 says that all IPs are already attached, then DS is out of sync so alloc will fail
 	_, err := cache.AllocIPAddresses(eniID, 14)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 }
 
 func TestAllocPrefixAddresses(t *testing.T) {
@@ -706,9 +706,9 @@ func TestAllocPrefixesAlreadyFull(t *testing.T) {
 
 	retErr := awserr.New("PrivateIpAddressLimitExceeded", "Too many IPs already allocated", nil)
 	mockEC2.EXPECT().AssignPrivateIpAddressesWithContext(gomock.Any(), input, gomock.Any()).Return(nil, retErr)
-	// If EC2 says that all IPs are already attached, we do nothing
+	// If EC2 says that all IPs are already attached, then DS is out of sync so alloc will fail
 	_, err := cache.AllocIPAddresses(eniID, 1)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 }
 
 func Test_badENIID(t *testing.T) {

pkg/ipamd/ipamd.go

@@ -363,6 +363,14 @@ func containsInsufficientCIDRsOrSubnetIPs(err error) bool {
 	return false
 }
 
+// containsPrivateIPAddressLimitExceededError returns whether exceeds ENI's IP address limit
+func containsPrivateIPAddressLimitExceededError(err error) bool {
+	if aerr, ok := err.(awserr.Error); ok {
+		return aerr.Code() == "PrivateIpAddressLimitExceeded"
+	}
+	return false
+}
+
 // inInsufficientCidrCoolingPeriod checks whether IPAMD is in insufficientCidrErrorCooldown
 func (c *IPAMContext) inInsufficientCidrCoolingPeriod() bool {
 	return time.Since(c.lastInsufficientCidrError) <= insufficientCidrErrorCooldown
@@ -972,25 +980,36 @@ func (c *IPAMContext) tryAssignIPs() (increasedPool bool, err error) {
 		// Try to allocate all available IPs for this ENI
 		resourcesToAllocate := min((c.maxIPsPerENI - currentNumberOfAllocatedIPs), toAllocate)
 		output, err := c.awsClient.AllocIPAddresses(eni.ID, resourcesToAllocate)
-		if err != nil {
+		if err != nil && !containsPrivateIPAddressLimitExceededError(err) {
 			log.Warnf("failed to allocate all available IP addresses on ENI %s, err: %v", eni.ID, err)
 			// Try to just get one more IP
 			output, err = c.awsClient.AllocIPAddresses(eni.ID, 1)
-			if err != nil {
+			if err != nil && !containsPrivateIPAddressLimitExceededError(err) {
 				ipamdErrInc("increaseIPPoolAllocIPAddressesFailed")
 				return false, errors.Wrap(err, fmt.Sprintf("failed to allocate one IP addresses on ENI %s, err ", eni.ID))
 			}
 		}
 
-		if output == nil {
-			ipamdErrInc("increaseIPPoolGetENIaddressesFailed")
-			return true, errors.Wrap(err, "failed to get ENI IP addresses during IP allocation")
-		}
-
 		var ec2ip4s []*ec2.NetworkInterfacePrivateIpAddress
-		ec2Addrs := output.AssignedPrivateIpAddresses
-		for _, ec2Addr := range ec2Addrs {
-			ec2ip4s = append(ec2ip4s, &ec2.NetworkInterfacePrivateIpAddress{PrivateIpAddress: aws.String(aws.StringValue(ec2Addr.PrivateIpAddress))})
+		if containsPrivateIPAddressLimitExceededError(err) {
+			log.Debug("AssignPrivateIpAddresses returned PrivateIpAddressLimitExceeded. This can happen if the data store is out of sync." +
+				"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.")
+			// This call to EC2 is needed to verify which IPs got attached to this ENI.
+			ec2ip4s, err = c.awsClient.GetIPv4sFromEC2(eni.ID)
+			if err != nil {
+				ipamdErrInc("increaseIPPoolGetENIaddressesFailed")
+				return true, errors.Wrap(err, "failed to get ENI IP addresses during IP allocation")
+			}
+		} else {
+			if output == nil {
+				ipamdErrInc("increaseIPPoolGetENIaddressesFailed")
+				return true, errors.Wrap(err, "failed to get ENI IP addresses during IP allocation")
+			}
+
+			ec2Addrs := output.AssignedPrivateIpAddresses
+			for _, ec2Addr := range ec2Addrs {
+				ec2ip4s = append(ec2ip4s, &ec2.NetworkInterfacePrivateIpAddress{PrivateIpAddress: aws.String(aws.StringValue(ec2Addr.PrivateIpAddress))})
+			}
 		}
 		c.addENIsecondaryIPsToDataStore(ec2ip4s, eni.ID)
 		return true, nil
@@ -1046,20 +1065,32 @@ func (c *IPAMContext) tryAssignPrefixes() (increasedPool bool, err error) {
 		currentNumberOfAllocatedPrefixes := len(eni.AvailableIPv4Cidrs)
 		resourcesToAllocate := min((c.maxPrefixesPerENI - currentNumberOfAllocatedPrefixes), toAllocate)
 		output, err := c.awsClient.AllocIPAddresses(eni.ID, resourcesToAllocate)
-		if err != nil {
+		if err != nil && !containsPrivateIPAddressLimitExceededError(err) {
 			log.Warnf("failed to allocate all available IPv4 Prefixes on ENI %s, err: %v", eni.ID, err)
 			// Try to just get one more prefix
 			output, err = c.awsClient.AllocIPAddresses(eni.ID, 1)
-			if err != nil {
+			if err != nil && !containsPrivateIPAddressLimitExceededError(err) {
 				ipamdErrInc("increaseIPPoolAllocIPAddressesFailed")
 				return false, errors.Wrap(err, fmt.Sprintf("failed to allocate one IPv4 prefix on ENI %s, err: %v", eni.ID, err))
 			}
 		}
-		if output == nil {
-			ipamdErrInc("increaseIPPoolGetENIprefixedFailed")
-			return true, errors.Wrap(err, "failed to get ENI Prefix addresses during IPv4 Prefix allocation")
+		var ec2Prefixes []*ec2.Ipv4PrefixSpecification
+		if containsPrivateIPAddressLimitExceededError(err) {
+			log.Debug("AssignPrivateIpAddresses returned PrivateIpAddressLimitExceeded. This can happen if the data store is out of sync." +
+				"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.")
+			// This call to EC2 is needed to verify which IPs got attached to this ENI.
+			ec2Prefixes, err = c.awsClient.GetIPv4PrefixesFromEC2(eni.ID)
+			if err != nil {
+				ipamdErrInc("increaseIPPoolGetENIaddressesFailed")
+				return true, errors.Wrap(err, "failed to get ENI IP addresses during IP allocation")
+			}
+		} else {
+			if output == nil {
+				ipamdErrInc("increaseIPPoolGetENIprefixedFailed")
+				return true, errors.Wrap(err, "failed to get ENI Prefix addresses during IPv4 Prefix allocation")
+			}
+			ec2Prefixes = output.AssignedIpv4Prefixes
 		}
-		ec2Prefixes := output.AssignedIpv4Prefixes
 		c.addENIv4prefixesToDataStore(ec2Prefixes, eni.ID)
 		return true, nil
 	}