-
Notifications
You must be signed in to change notification settings - Fork 262
modules,steps: enable existing vpc #3231
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,7 +31,7 @@ output "worker_sg_id" { | |
|
|
||
| # TNC | ||
| output "private_zone_id" { | ||
| value = "${aws_route53_zone.tectonic_int.id}" | ||
| value = "${join("", aws_route53_zone.tectonic_int.*.zone_id)}" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm curious if you considered to use a ternary here to return either
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes I did, but I decided against it for consistency and clarity. Otherwise, I foresee a near future where someone forgets that this variable is already the result of a ternary and adds another ternary on top. It wouldn't hurt the output but it is muddy. |
||
| } | ||
|
|
||
| output "tnc_elb_dns_name" { | ||
|
|
||
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.
this resource was never used :/
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.
Does this mean we're not doing split horizon anymore?
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 still have split horizon, it was just moved somewhere else and this was never cleaned up: https://github.com/coreos/tectonic-installer/blob/master/steps/topology/aws/main.tf#L22
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.
Got it. Makes sense then. All cool!