-
Notifications
You must be signed in to change notification settings - Fork 11
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
Improve subnet CIDR calculation #23
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@balazsgaspar - thanks for taking care of this.
Just a small change required to add the new variables to the top-level pre-req module so that they are exposed to the user.
Also can you run terraform fmt -recursive
to format all Terraform source files. The GitHub actions are failing on this.
…n added to CDP deployment module
…n added to CDP deployment module
Done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested CDP deployment with these changes to public and private subnets. All deployment patterns work well. Also confirmed that the change has no impact on the BYO-VPC/VNet configuration.
Approved.
@balazsgaspar - this PR is all tested and ready to be merged. However the merge is blocking as we need DCO signoff is needed on all commits. Details of how to do this are at: https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification |
Closing without merging as functionality was added as part of #29 |
Currently, the prereq modules configure the vpc/cnet submodules in a way that the total VPC/VNet CIDR range is split up equally between the number of subnets created.
While this is a sound approach, it results in a networking layout that does not align with our recommended network settings (and the current "Create new VPC" feature of CDP) and may be a source of confusion. See also https://docs.cloudera.com/cdp-public-cloud/cloud/requirements-aws/topics/mc-aws-req-vpc.html
We should implement a logic where the user can select the size of the public/private (internal/gateway) subnets and the module calculates each subnet's CIDR range accordingly.
Defaults subnet sizes will be set according to the linked documentation (/19 for internal/private and /24 for public/CDP subnets)