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

Update instance ENI and IP mapping table #275

Merged
merged 1 commit into from
Dec 21, 2018

Conversation

hmizuma
Copy link
Contributor

@hmizuma hmizuma commented Dec 21, 2018

Issue #, if available:

Description of changes:
Update instance IP and ENI limits table.

  • Fix max IP per ENI of instance types f1.16xlarge, g3.16xlarge, h1.16xlarge, i3.16xlarge, and r4.16xlarge
  • General update with new instance types

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@mogren
Copy link
Contributor

mogren commented Dec 21, 2018

I think some mocks are missing:

=== RUN   TestAllocAllIPAddressOnErr
1545360897551287999 [Info] Trying to allocate all available ip addresses on eni: eni-id
1545360897551311828 [Error] Failed to get eni IP limit due to unknown instance type 
--- PASS: TestAllocAllIPAddressOnErr (0.00s)
1545360897551520883 [Error] Failed to allocate a private IP address Error on AssignPrivateIpAddresses
FAIL
coverage: 74.1% of statements
FAIL	github.com/aws/amazon-vpc-cni-k8s/pkg/awsutils	105.084s
?   	github.com/aws/amazon-vpc-cni-k8s/pkg/awsutils/mocks	[no test files]
make: *** [unit-test] Error 1
The command "make unit-test" exited with 2.

* New instnace types
* Adjustments to existing instances
* r4.16xlarge was used as an example for IP allocation unit test. r4.16xlarge requires unique configuration, so switch to using c5n.18xlarge.
@hmizuma
Copy link
Contributor Author

hmizuma commented Dec 21, 2018

IP allocation test was using r4.16xlarge as an example instance type which currently requires special IP configuration and may update again in the future.
I've switched to using c5n.18xlarge for this test with same max IP count.

Copy link
Contributor

@mogren mogren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lower number of InstanceIPsAvailable for some instances might cause issues for some users, but it's the correct limit.

@mogren mogren merged commit 5cce382 into aws:master Dec 21, 2018
@mogren mogren added this to the v1.4 milestone Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants