feat: sort subnets to calculate by their netmask to efficiently use ip space #130
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.
The previous implementation calculates the ipv4 subnet cidr ranges in an arbitrary order (actually alphabetically based on the subnet type string). This means that if you have different netmasks for different subnets you end up trying to take your vpc cidr range, then cut out some small netmasks, then encounter a large netmask, at which point you have to skip a bunch of ips in order to get to the next start address for that larger netmask. In practice this causes really inefficient use of ip space. For example, with a vpc netmask of 22, 3 db subnets with a netmask of 27, 3 public subnets with a netmask of 28, 3 private subnets with a netmask of 24, and 3 transit gateway subnets with a netmask of 28, then the module is unable to calculate the cidr ranges, because first it creates the db subnets with their 27 netmask, then the public with their 28 netmask, then it gets to the private ones and has to skip a huge chunk to get to the start of a
/24
block. At this point it has skipped so many that its unable to create the remaining subnets. The fix for this is to first calculate the subnets where netmask is 24, then the ones where it is 27, then the ones where it is 28. If you do this, then the subnet calculator is then able to create all the required subnets, as its no longer skipping large chunks due to starting with small netmasks.Example:
On the existing version, this throws an error:
On this version with sorting, it outputs the following: