vmware-archive / terraforming-aws

Templates to deploy PCF and PKS
Apache License 2.0
64 stars 91 forks source link

fix cidr calculations #108

Closed JaredGordon closed 5 years ago

JaredGordon commented 5 years ago

Hello!

We're having issues with CIDR errors when we try to create a /20 VPC. If we set the CIDR prefix to /20 we get errors such as this:

* module.infra.data.template_file.infrastructure_subnet_gateways[2]: cidrhost: prefix of 32 does not accommodate a host numbered 1 in:

${cidrhost(element(aws_subnet.infrastructure_subnets.*.cidr_block, count.index), 1)}

This PR fixes this, plus it consolidates the cidr subnet calculations into a module so they are easier to understand and maintain.

nwmahoney commented 5 years ago

Hey @JaredGordon. I'm reviewing this right now. I have a quick question about your latest commit ("pull down latest provider versions").

What is your thinking behind this change? The ~> syntax that you replaced means at least that value (1.60) but less than the next major (2.0.0). So your commit message says to pull latest version, but the effect of the change is just to allow versions less than 1.60. If that wasn't your intention, could you remove that commit?

JaredGordon commented 5 years ago

hey folks, am closing this and am going to re-submit an almost identical one that does not have merge conflicts...

JaredGordon commented 5 years ago

Thanks Nick, yes, that was not intentional, I removed and resubmitted a fixed up PR. Sorry about that...

On Thu, Mar 14, 2019 at 2:08 PM Nick Mahoney notifications@github.com wrote:

Hey @JaredGordon https://github.com/JaredGordon. I'm reviewing this right now. I have a quick question about your latest commit ("pull down latest provider versions" https://github.com/pivotal-cf/terraforming-aws/pull/108/commits/80c1b015a2a0aeef6a1e4f6f3de758628a81c76d ).

What is your thinking behind this change? The ~> syntax https://urldefense.proofpoint.com/v2/url?u=https-3A__www.terraform.io_docs_configuration_providers.html-23gt-2D1-2D2&d=DwMFaQ&c=lnl9vOaLMzsy2niBC8-h_K-7QJuNJEsFrzdndhuJ3Sw&r=l0G0aaaMytNrKTtiakfvKkxyb0pshgb4S9R_z5Iov38&m=TA54cxCyuTqPIXeIj9yZvFdtjOrga2wIxMwZay7Jo4Y&s=g9t6UBF42QIdBaRpu6-7aBNTI2VFBumhY5rT43OnOB0&e= that you replaced means at least that value (1.60) but less than the next major (2.0.0). So your commit message says to pull latest version, but the effect of the change is just to allow versions less than 1.60. If that wasn't your intention, could you remove that commit?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pivotal-cf/terraforming-aws/pull/108#issuecomment-472994570, or mute the thread https://github.com/notifications/unsubscribe-auth/AKJUaJpMc3CjRFJhO_YSFWGfFNp1Y4blks5vWpA1gaJpZM4bhx0I .

nwmahoney commented 5 years ago

No worries!