Feat: support for internal cross-regional load balancer #490
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
pawan1210
force-pushed
the
feat/internal-lb-cloud-run
branch
from
fd1ab74 to
c2f23ee
Compare
pawan1210
changed the title
pawan1210
changed the title
pawan1210
force-pushed
the
feat/internal-lb-cloud-run
branch
7 times, most recently
from
defff92 to
c26b8fd
Compare
modules/frontend/main.tf
Outdated
| resource "google_compute_global_address" "default_ipv6" { | ||
| provider = google-beta | ||
| count = local.is_internal ? 0 : (var.enable_ipv6 && var.create_ipv6_address) ? 1 : 0 | ||
| count = local.is_internal_self_managed ? 0 : (var.enable_ipv6 && var.create_ipv6_address) ? 1 : 0 |
There was a problem hiding this comment.
Is this needed for INTERNAL_MANAGED?
There was a problem hiding this comment.
Yes, only for INTERNAL_SELF_MANAGED it's not needed. For INTERNAL_MANAGED, this can be used.
There was a problem hiding this comment.
Is this address being used by module for INTERNAL_MANAGED?
There was a problem hiding this comment.
Yes, for initializing the 2 frontend cloudruns. Which forwards the request to http port of the forwarding rules.
|
/gcbrun |
1 similar comment
|
/gcbrun |
|
With the examples, we need to add a integration test and update gcb file for integration test to test this example. Please add this as done in this PR GoogleCloudPlatform/terraform-google-regional-lb-http#28 |
|
/gcbrun |
4 similar comments
|
/gcbrun |
|
/gcbrun |
|
/gcbrun |
|
/gcbrun |
|
/gcbrun |
q2w
force-pushed
the
feat/internal-lb-cloud-run
branch
from
e89632c to
66cfc36
Compare
bharathkkb
changed the title
bharathkkb
left a comment
bharathkkb
left a comment
There was a problem hiding this comment.
Overall lgtm, cc @imrannayer if you have bandwidth for another pass
modules/frontend/main.tf
Outdated
| } | ||
|
|
||
| resource "google_compute_global_forwarding_rule" "internal_managed_http" { | ||
| count = local.create_http_forward && local.is_internal_managed ? length(var.internal_forwarding_rule_subnetworks) : 0 |
There was a problem hiding this comment.
We should use for_each instead of count so we are not dependent on ordering of the list elements (here and below). This may need an update to the variable too since if we use set(list(..)), TF might complain if we dynamically create subnet. So maybe var could be map(string) where k=region, value =subnet.
There was a problem hiding this comment.
@pawan1210 with index wouldn't we still have the problem of ordering. Fo example internal_forwarding_rules_config=[a,b]. For_each resource addressing becomes [0=>a, 1 =>b]. Now if value is updated to [b,a], then TF would want to delete and recreate?
There was a problem hiding this comment.
As discussed added region and using it for creating resource names. Please check.
modules/frontend/variables.tf
Outdated
|
|
||
| variable "internal_forwarding_rule_configs" { | ||
| description = "Map of internal managed forwarding rule configs. One of 'address' or 'subnetwork' is required for each." | ||
| type = map(object({ |
There was a problem hiding this comment.
Can we use list(object) here?
| type = map(object({ | |
| type = list(object({ | |
| name = optional(string) | |
| address = optional(string) | |
| subnetwork = optional(string) | |
| }) |
There was a problem hiding this comment.
Done. Please check the latest changes
3 participants
No description provided.