feat: allow defining custom subnet ip ranges for static agent pools #1903
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @pat-s, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces the capability to define custom subnet IP ranges for static agent pools. This enhancement addresses scenarios where default IP ranges might conflict with existing network configurations, such as occupied subnets or vswitch connections, providing greater flexibility and preventing IP collisions in cloud environments.
Highlights
- Custom Subnet IP Ranges: Introduced a new "subnet_ip_range" option for agent nodepools, allowing users to specify custom IP ranges instead of relying on default, incrementing ones.
- Configuration Flexibility: Enables users to avoid IP conflicts with existing infrastructure and scale beyond default limitations by explicitly defining subnet ranges.
- Example Configuration Update: The example Terraform configuration (kube.tf.example) has been updated to demonstrate the usage of the new "subnet_ip_range" parameter.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces the subnet_ip_range attribute for agent node pools, allowing users to define custom IP ranges for subnets. This is a valuable addition for flexibility in network configurations, especially in environments with pre-existing subnet allocations. The implementation is sound, and the example file is updated accordingly. I have a couple of suggestions to enhance code readability and fix a minor formatting issue in the example.
kube.tf.example
Outdated
|
|
||
| # Using the default configuration you can only create a maximum of 42 agent-nodepools. | ||
| # This is due to the creation of a subnet for each nodepool with CIDRs being in the shape of 10.[nodepool-index].0.0/16 which collides with k3s' cluster and service IP ranges (defaults below). | ||
| To create additional ones (or if you want to use different ranges for other reasons), set the `subnet_ip_range` explicitly for a node pool. |
There was a problem hiding this comment.
This line appears to be intended as a comment but is missing the # prefix. Without it, this line is not valid Terraform syntax and will cause a parsing error if a user tries to adapt this example file. Please add a # to comment it out correctly.
# To create additional ones (or if you want to use different ranges for other reasons), set the `subnet_ip_range` explicitly for a node pool.
main.tf
Outdated
| type = "cloud" | ||
| network_zone = var.network_region | ||
| ip_range = local.network_ipv4_subnets[count.index] | ||
| ip_range = var.agent_nodepools[count.index].subnet_ip_range != null ? var.agent_nodepools[count.index].subnet_ip_range : local.network_ipv4_subnets[count.index] |
There was a problem hiding this comment.
The conditional logic for ip_range is correct, but it can be simplified and made more idiomatic by using the coalesce function. This function returns the first non-null value from its arguments, which perfectly matches the intended logic and improves readability.
ip_range = coalesce(var.agent_nodepools[count.index].subnet_ip_range, local.network_ipv4_subnets[count.index])
|
@pat-s This is a breaking change. What I'm exploring now is for v3 to actually have one network per nodepool. That will allow us to have up to 100 nodes per nodepool, and not the current limited 100 nodes per cluster. It resonates a little bit with what you are trying to achieve here. |
|
Why is it breaking? It is optional and only used when specified and otherwise uses the old default incrementing from 10.x per node? |
|
@pat-s Man, subnet ranges are computed very carefully, it's a whole equilibrium. I will need to think about that one more. |
|
@codex review please, and let me know if backward compatible or not. |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
Instead of relying on the default ones incrementing from
10.0.x.This can be useful (and even needed) if certain subnets in this range are already occupied (e.g. by a vswitch connection).
The current approach uses a hardcoded logic to increment from 0, e.g. 10.0, 10.1, 10.2, etc.
subnet_ip_rangeallows overriding a node pool subnet on an individual basis.