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

Enable Prefix Delegation on Bare metal instances #1937

Merged
merged 7 commits into from
Mar 23, 2022

Conversation

achevuru
Copy link
Contributor

@achevuru achevuru commented Mar 21, 2022

What type of PR is this?
bug

Which issue does this PR fix:
VPC CNI currently relies on hyperVisor field to be nitro to decide whether an instance is capable of supporting Prefix Delegation/Assignment feature. hyperVisor filed is empty for Bare metal instances and therefore it incorrectly assumes that these instances are not capable of supporting Prefix Delegation/Assignment feature. PR addresses this issue.

What does this PR do / Why do we need it:
Enables PD for Bare Metal instances.

If an issue # is not available please add repro steps and logs from IPAMD/CNI showing the issue:
Identified via #1929.

Testing done on this change:
Validated that PD enabled flag is respected on Metal instances and scheduling pods is working as intended.

Automation added to e2e:
NA

Will this PR introduce any new dependencies?:
No

Will this break upgrades or downgrades. Has updating a running cluster been tested?:
Downgrading VPC CNI to an older version on Bare metal instances with ENABLE_PREFIX_DELEGATION set to true will disable Prefix delegation and will fall back to Secondary IP mode.

Does this change require updates to the CNI daemonset config files to work?:
No

Does this PR introduce any user-facing change?:
No


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

Copy link
Contributor

@srini-ram srini-ram left a comment

Choose a reason for hiding this comment

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

Please update CNI documentation to point to new CNI release (1.10.x or 1.11.x) for PD support on bare metal instances. Also as part of the release note documentation. it should be clarified that CNI downgrade from version with fix (1.10.x or 1.11.x) along with PD + Baremetal config to older version of CNI without PD + baremetal support is not allowed.

README.md Outdated
@@ -457,7 +457,7 @@ Setting ENABLE_PREFIX_DELEGATION to true will not increase the density of branch

Please refer to [VPC CNI Feature Matrix](https://github.com/aws/amazon-vpc-cni-k8s#vpc-cni-feature-matrix) section below for additional information around using Prefix delegation with Custom Networking and Security Groups Per Pod features.

**Note:** `ENABLE_PREFIX_DELEGATION` needs to be set to `true` when VPC CNI is configured to operate in IPv6 mode (supported in v1.10.0+).
**Note:** `ENABLE_PREFIX_DELEGATION` needs to be set to `true` when VPC CNI is configured to operate in IPv6 mode (supported in v1.10.0+). Prefix Delegation in IPv4 and IPv6 modes is supported on Bare Metal instances only from v1.11+. If you're using Prefix Delegation feature on Bare Metal instances, downgrading to an earlier version of VPC CNI from v1.11+ will be disruptive and not supported.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we reword this a little bit something like - Prefix Delegation in IPv4 and IPv6 modes were supported on Nitro instances and 1.11+ onwards it is supported on Bare Metal instances as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But Bare Metal instances are also Nitro instances though. So, essentially we were missing out on Nitro based Bare metal instances.

https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-types.html#ec2-nitro-instances

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm..my only concern was "Prefix Delegation in IPv4 and IPv6 modes is supported on Bare Metal instances only" and someone who isn't very familiar about this might think nitro isn't supported any more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can rephrase it as - Prefix Delegation in IPv4 and IPv6 modes is supported on Nitro based Bare Metal instances as well from v1.11+

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good.

Copy link
Contributor

@jayanthvn jayanthvn left a comment

Choose a reason for hiding this comment

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

Lgtm

@achevuru achevuru merged commit 0dd549f into aws:master Mar 23, 2022
Copy link
Contributor

@srini-ram srini-ram left a comment

Choose a reason for hiding this comment

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

LGTM

@jayanthvn jayanthvn added this to the v1.10.3 milestone Mar 29, 2022
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.

3 participants