-
Notifications
You must be signed in to change notification settings - Fork 744
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
Prefix Delegation: Merge from prefix delegation preview branch to master #1516
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* ENABLE_PREFIX_DELEGATION knob WARM_PREFIX_TARGET knob cr https://code.amazon.com/reviews/CR-40610031 * PD changes - dev only * Cooldown prefix IP * minor fixes to support prefix count * Code cleanup * Handle few corner cases * Nitro based check * With custom networking, do not get prefix for primary ENI * Code refactor * Handle graceful upgrade/enable PD from disable PD * code refactoring * Code refactoring * fix computing too low IPs * UT for prefix store * Fix UTs and handle CR comments * Clean up SDK code and fix model code generation * fix format and merge induced error * Merge broke the code * Fix Dockerfile.test * Added IPAMD UTs and fixed removeENI total count * Couple more IPAMD UTs for PD * UTs for awsutils/imds * Handle graceful PD enable to disable knob * get prefix list for non-pd case * Prevent reconcile of prefix IPs in IP datastore * Handle disable scenario * fix formatting * clean up comment * Remove unnecessary debugs * Handle PR comments * formatting fix * Remodelled PD datastore * Fix up UTs and fix Prefix nil * formatting * PR comments - minor cosmetic changes * removed the sdk override from makefile * Internal repo merge added these lines * Update config file * Handle wrapper of DescribeNetworkInterfacesWithContext to take one eni * RemoveUnusedENIFromStore was not accounting for prefixes deleted * Removed hardcoding of 16
…ore of CIDRs (aws#1471) * Code refactor - merge to single DB * remove few debugs * remove prefix store files * PR comments * Fix up CR comments * formatting * Updated UT cases * UT and formatting * Minor fixes * Minor comments * Updated /32 store term * remove unused code
* Multi-pd and WARM targets support * cleanup * Updated variable names * Default prefix count to -1 * Get stats should be computed on the fly since CIDR pool can have /32 or /28 * Support for warm prefix 0 * code review comments
* Traffic test case and readme update * Added testcases for warm ip/min ip with PD * Testcases for prefix count * Testcase for warm prefix along with warm ip/min ip * Updated traffic test case while PD mode is flipped * Fix minor comments * pr comments * added pods per eni * fix up count
* Support mixed instances with PD * fix up the log
* optimization for prefixes Prefix store optimization * pr comment
* Fixup eni allocation with warm targets * fixup cidr count * code comments and warm prefix 0
* Handle prefix target 0 * pr commets
locally UTs are running fine and even build is good -
|
achevuru
reviewed
Jun 22, 2021
achevuru
reviewed
Jun 22, 2021
achevuru
reviewed
Jun 22, 2021
achevuru
reviewed
Jun 22, 2021
M00nF1sh
reviewed
Jun 22, 2021
M00nF1sh
reviewed
Jun 22, 2021
Error with merge
M00nF1sh
reviewed
Jun 23, 2021
M00nF1sh
reviewed
Jun 23, 2021
M00nF1sh
reviewed
Jun 23, 2021
M00nF1sh
reviewed
Jun 23, 2021
M00nF1sh
reviewed
Jun 23, 2021
M00nF1sh
reviewed
Jun 23, 2021
M00nF1sh
reviewed
Jun 23, 2021
M00nF1sh
reviewed
Jun 23, 2021
M00nF1sh
reviewed
Jun 23, 2021
M00nF1sh
reviewed
Jun 23, 2021
M00nF1sh
reviewed
Jun 23, 2021
M00nF1sh
reviewed
Jun 23, 2021
M00nF1sh
reviewed
Jun 23, 2021
M00nF1sh
reviewed
Jun 23, 2021
M00nF1sh
reviewed
Jun 23, 2021
M00nF1sh
reviewed
Jun 23, 2021
achevuru
approved these changes
Jun 24, 2021
M00nF1sh
reviewed
Jun 24, 2021
M00nF1sh
reviewed
Jun 24, 2021
M00nF1sh
reviewed
Jun 24, 2021
M00nF1sh
reviewed
Jun 24, 2021
M00nF1sh
reviewed
Jun 24, 2021
M00nF1sh
reviewed
Jun 24, 2021
M00nF1sh
approved these changes
Jun 24, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
feature
Which issue does this PR fix:
This is an enhancement to achieve higher pod density per node.
What does this PR do / Why do we need it:
Current implementation allocates secondary IPs per ENI which will be used for pod IPs. With the new VPC feature each secondary IP will be replaced with /28 prefix in the ENI's subnet. IPs will be allocated from this subnet.
If an issue # is not available please add repro steps and logs from IPAMD/CNI showing the issue:
N/A
Testing done on this change:
Yes
Automation added to e2e:
Yes
Will this break upgrades or downgrades. Has updating a running cluster been tested?:
New feature so downgrades won't be supported.
Does this change require updates to the CNI daemonset config files to work?:
Yes
ENABLE_PREFIX_DELEGATION to enable the feature
WARM_PREFIX_TARGET to allocate /28 prefixes and store in warm pool.
Does this PR introduce any user-facing change?:
Yes, scale per node will increase and max pods per node needs to be updated.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.