-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
chore: Support secondary eips #1109
Closed
Closed
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
80cd541
add configuraiton for secondary ip on natgateways
AlexisColes 79778fb
default array
AlexisColes 2823a66
fix
AlexisColes 9b409ae
hard code default vaules
AlexisColes bff8601
try emtpy arrays
AlexisColes 3cc7409
test
AlexisColes 75b2896
tidy
AlexisColes 04d8ce6
fix
AlexisColes b92bce8
Merge pull request #1 from AlexisColes/nat-gateway-secondary-ips
AlexisColes 2d5750c
inprove description
AlexisColes 1361fbe
update docs
AlexisColes 81ad757
Merge pull request #2 from AlexisColes/nat-gateway-secondary-ips
AlexisColes 1cc8ee5
Merge branch 'master' into master
AlexisColes 7a83c05
add number_of_secondary_eips_per_gateway to complete example
AlexisColes File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
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.
we cannot have computed values as keys in maps
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.
Hey, could you explain why please and if you could recommend an alternative approach?
I did consider doing this with count like the other arrays were doing, however if you changed the number of AZs or number of ips per az it would start trying to move the ips from 1 natgateway to the other which then wants to destroy and re-create the natgateway which isn't ideal. Using keys seem to make the solution much more solid.
Or is it the use of the
nat.tags.Name
that is the issue. This does seem like it could be flaky 🤔 I guess we could use a range based onlocal.create_vpc && var.enable_nat_gateway && !var.reuse_nat_ips ? local.nat_gateway_count : 0
As the first part of the composite key, this would be much more solid thinking about it 🚀
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.
hashicorp/terraform#30937
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.
I see, but a fresh plan with the current code does not result in unknown values being passed to the for_each argument as the Name tag can be determined.
I added 2 ips per gateway to the complete example and a fresh plan is good.
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.
@AlexisColes I tested your solution and got this errors:
module.vpc.aws_eip.secondary["nebula-paastocaas-eks-vpc-nprd-eu-central-1c-s2"]: Creation complete after 0s [id=eipalloc-0b8add66c71b6ab16]
module.vpc.aws_eip.secondary["nebula-paastocaas-eks-vpc-nprd-eu-central-1b-s2"]: Creation complete after 0s [id=eipalloc-08c1ad57da35acdf2]
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[1] to include
╷
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" changed the planned action from
│ Update to DeleteThenCreate.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[1] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .id: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[1] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .network_interface_id: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[1] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .public_ip: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[1] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .association_id: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[1] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .private_ip: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[1] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .secondary_private_ip_address_count: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[1] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .secondary_private_ip_addresses: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[0] to include
│ Warning: Argument is deprecated
│
│ with module.eks.aws_eks_addon.this["kube-proxy"],
│ on .terraform/modules/eks/main.tf line 400, in resource "aws_eks_addon" "this":
│ 400: resolve_conflicts = try(each.value.resolve_conflicts, "OVERWRITE")
│
│ The "resolve_conflicts" attribute can't be set to "PRESERVE" on initial
│ resource creation. Use "resolve_conflicts_on_create" and/or
│ "resolve_conflicts_on_update" instead
│
│ (and 3 more similar warnings elsewhere)
╵
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" changed the planned action from
│ Update to DeleteThenCreate.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[0] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .id: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[0] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .network_interface_id: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[0] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .public_ip: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[0] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .association_id: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[0] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .private_ip: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[0] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .secondary_private_ip_address_count: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[0] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .secondary_private_ip_addresses: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[2] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" changed the planned action from
│ Update to DeleteThenCreate.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[2] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .id: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[2] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .network_interface_id: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[2] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .public_ip: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[2] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .secondary_private_ip_addresses: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[2] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .association_id: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[2] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .private_ip: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for module.vpc.aws_nat_gateway.this[2] to include
│ new values learned so far during apply, provider
│ "registry.terraform.io/hashicorp/aws" produced an invalid new value for
│ .secondary_private_ip_address_count: was known, but now unknown.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
Error: Process completed with exit code 1.
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.
The above errors happened running the code in a previously created VPC (with the 5.13.0 version of the module) with the default of 3 NATGW (1 per az), and now running against your version changing the secondary IP's from 0 to 2.
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.
I'm testing the following:
Adding to locals:
seips_names = flatten([for nat_index in range(0, local.nat_gateway_count) : [for suffix in local.seips_suffixs : "${aws_eip.nat[nat_index].tags.Name}-${suffix}"]])
and replacing the for_each with:
for_each = toset(local.seips_names)
Would that help?
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.
guess not... same errors "Error: Provider produced inconsistent final plan" when going from 0 secondary IP's to 2 :-(
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.
@flaviomoringa I have done some testing and it seems a provider issue indeed. The only way to add secondary IPs with terraform is on NAT Gateway creation. Any further change to that results in a re-creation of the NAT Gateway. However, if you have created it without secondary IPs you get the inconsistent plan error. I believe that has to do with how it associates the private IPs to the NAT Gateway interfaces but it doesn't allow manual association.
I tested with this very basic example:
I think this should be an issue on the aws provider repo if there is not one already.
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.
hashicorp/terraform-provider-aws#33964
hashicorp/terraform-provider-aws#34268