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

Support more than 4 azs #2804

Merged
merged 5 commits into from
Nov 10, 2020
Merged

Support more than 4 azs #2804

merged 5 commits into from
Nov 10, 2020

Conversation

aclevername
Copy link
Contributor

@aclevername aclevername commented Nov 9, 2020

Description

Currently EKSCTL limits you to specify 4 AZs, if you attempt to specify more it fails as we only split up the VPC CIDR block into 8 chunks (4x 1 private & 1 public subnet). This PR ups the capacity to 8 AZs (currently the biggest region has only 6 AZs) by adding the option to split the VPC CIDR block into 16.

related issue: #1359

Checklist

  • Added tests that cover your change (if possible)
  • Added/modified documentation as required (such as the README.md, or the userdocs directory)
  • Manually tested
  • Added labels for change area (e.g. area/nodegroup), target version (e.g. version/0.12.0) and kind (e.g. kind/improvement)
  • Make sure the title of the PR is a good description that can go into the release notes

@aclevername aclevername added kind/feature New feature or request area/aws-vpc labels Nov 9, 2020
Copy link
Contributor

@michaelbeaumont michaelbeaumont left a comment

Choose a reason for hiding this comment

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

One potential improvement but either way, LGTM!

return subnets, nil
}

func SplitInto8(parent *net.IPNet) ([]*net.IPNet, error) {
Copy link
Contributor

@michaelbeaumont michaelbeaumont Nov 10, 2020

Choose a reason for hiding this comment

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

Sorry forgot to include comment, these 2 functions could be reduced into SplitIntoN

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@michaelbeaumont I'm a bit hesitant to do that, while I managed to manipulate the first function to accurately produce splits of 16 instead of 8, I don't know how to ensure the same change for any given input, e.g. splitting into 10.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it should work for any power of two (and only for powers of two).

@aclevername aclevername merged commit 0a710a4 into master Nov 10, 2020
@aclevername aclevername deleted the support-more-than-3-azs branch November 10, 2020 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/aws-vpc kind/feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants