Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wait for SecondaryIPs in waitENIAttached() #1148

Closed
mogren opened this issue Aug 17, 2020 · 3 comments
Closed

Wait for SecondaryIPs in waitENIAttached() #1148

mogren opened this issue Aug 17, 2020 · 3 comments

Comments

@mogren
Copy link
Contributor

mogren commented Aug 17, 2020

We have seen an increase of PrivateIpAddressLimitExceeded since we started logging these errors back in May. #989

The issue seems to be that waitENIAttached() only check for the newly created ENI to be attached, not that the Secondary IPs have actually been assigned to that ENI. Because of that, the ENI gets added to the ipamd data store with no IPs, and on the next reconcile we think is empty, try to call AllocIPAddresses() on the same ENI again, which fails with PrivateIpAddressLimitExceeded that we ignore, and then go on to add those existing IPs to the data store.

We should instead wait for the secondary IPs to actually show up before adding the ENI to the data store.

@mogren mogren changed the title Wait for SecondaryIPs in waitENIAttached() Wait for SecondaryIPs in waitENIAttached() Aug 17, 2020
@naveenb29
Copy link

what CNI version does this affect ?

@mogren
Copy link
Contributor Author

mogren commented Aug 26, 2020

@naveenb29 All versions seem to have had this issue since before 1.0.0. We did not log this until May, just quietly ignoring. The issue is in this check:

// Verify that the ENI we are waiting for is in the returned list
for _, returnedENI := range enis {
if eni == returnedENI.ENIID {
return returnedENI, nil
}
}

Just getting the ENI attached is not enough, we also need to get the secondary IPv4 addresses. Added in #1174

One reason it was not found earlier is that in the first call to create a new ENI and attach it, we fail to add any new IPs to the datastore. When we 5s later try again, we find the ENI that the CNI think is empty, the AssignPrivateIPAddresses call fails since they are already attached. We ignore that error since we now have the IPs, and just add them to the datastore and make them available for pods.

@mogren
Copy link
Contributor Author

mogren commented Sep 1, 2020

Fix merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants