Remove hardcoded network from example #36
Conversation
averbuks
commented
Oct 17, 2019
averbuks
commented
- Fix readme files
- Clean up old files not needed after Event function: update testing to new approach #29
- Remove hard-coded network from examples Remove hard-coded network from examples #33
averbuks
requested review from
aaron-lane,
adrienthebo and
morgante
as code owners
| variable "network" { | ||
| type = string | ||
| description = "The name or self_link of the network to create compute instance in. Only one of network or subnetwork should be specified." | ||
| default = "" |
There was a problem hiding this comment.
Empty defaults should ~never be used. If a default value is provided, it must be a usable valuable for at least some customers.
In this case, I think default = "default" would work.
There was a problem hiding this comment.
The problem with default = "default” is that thegoogle_compute_instance resource expect only one of the network, subnetwork to be specified, and only subnetwork should be specified in case of custom network.
So, if I put var network { default = "default" } and the user has custom network, the user will have to provide subnetwork = "subSelfLink" and it will fail due to network = default.
Approach with “null” defaults has been gotten from https://github.com/terraform-google-modules/terraform-google-vm/tree/master/modules/compute_instance
.
But I found a solution, we can specify only subnetwork and default = default
So, if you have a default/auto net you can go with default value, if a custom one then you have to specify subnetwork anyway.
| variable "network" { | ||
| type = string | ||
| description = "The name or self_link of the network to create compute instance in. Only one of network or subnetwork should be specified." | ||
| default = "" |
| - [Terraform Provider for Google Cloud Platform][t7m-provider-gcp-site] | ||
| v2.1.Z | ||
| - [Terraform][terraform-site] v0.12 | ||
| - [Terraform Provider for Google Cloud Platform][terraformm-provider-gcp-site] v2.5 |
There was a problem hiding this comment.
| - [Terraform Provider for Google Cloud Platform][terraformm-provider-gcp-site] v2.5 | |
| - [Terraform Provider for Google Cloud Platform][terraform-provider-gcp-site] v2.5 |
| ## Usage | ||
|
|
||
| To provision this example, populate `terraform.tfvars` with the [required variables][#inputs] and run the following commands within | ||
| To provision this example, populate `terraform.tfvars` with the [required variables][variables] and run the following commands within |
There was a problem hiding this comment.
For what it's worth, I don't think we should link to the variables file directly. The inputs table works fine.
There was a problem hiding this comment.
oh, I did not realised that it works this way. Thanks, fixed
| variable "subnetwork" { | ||
| type = string | ||
| description = "The name or self_link of the subnetwork to create compute instance in." | ||
| default = "" |
There was a problem hiding this comment.
This needs to be a usable value.
morgante
left a comment
morgante
left a comment
There was a problem hiding this comment.
LGTM, but linters/tests are failing. Please fix.
2 participants
