[Feat] Add full terraform.tfvars variable list #10
Conversation
|
Missing newlines at the ends of the files, otherwise LGTM |
ikatz-nvidia
left a comment
ikatz-nvidia
left a comment
There was a problem hiding this comment.
This looks good. Am I wrong in observing that it appears to cause a breaking change and would require a major version bump when released?
| # Sample tfvars file. Uncomment out values to use | ||
| # Do not commit this file to Git with sensitive values | ||
|
|
||
| # cluster_name = "" |
There was a problem hiding this comment.
To maintain consistency with other tfvars, can you add the cluster_name to be holoscan-cluster
There was a problem hiding this comment.
These are auto generated. The values which appear here are the defaults from variables.tf. The tfvars make it simple for people to select the values themselves
| # cluster_name = "holoscan-cluster" | ||
| # region = "us-west-2" | ||
|
|
||
| # Optional: If deploying into an existing VPC, use the following variable |
There was a problem hiding this comment.
Probably this comment should be moved just above existing_vpc_details variable.
| # additional_user_data = "" | ||
| # aws_profile = "development" | ||
| # cidr_block = "10.0.0.0/16" | ||
| # cluster_name = "" |
There was a problem hiding this comment.
Already have cluster_name above
There was a problem hiding this comment.
Caught this after the merge. Fixed in #11
| |------|-------------|------|---------|:--------:| | ||
| | <a name="input_admin_group_object_ids"></a> [admin\_group\_object\_ids](#input\_admin\_group\_object\_ids) | (Required) A list of Object IDs (GUIDs) of Azure Active Directory Groups which should have Owner Role on the Cluster. <br> This is not the email address of the group, the GUID can be found in the Azure panel by searching for the AD Group<br> NOTE: You will need Azure "Owner" role (not "Contributor") to attach an AD role to the Kubernetes cluster. | `list(any)` | n/a | yes | | ||
| | <a name="input_az_monitor-user-managed-id"></a> [az\_monitor-user-managed-id](#input\_az\_monitor-user-managed-id) | The user managed identity to *create* for use with the Azure monitor-- at this time this does not accept existing user or system managed identity | `string` | `"tf-holoscan-identity"` | no | | ||
| | <a name="input_az_monitor_user_managed_id"></a> [az\_monitor-user-managed-id](#input\_az\_monitor-user-managed-id) | The user managed identity to *create* for use with the Azure monitor-- at this time this does not accept existing user or system managed identity | `string` | `"tf-holoscan-identity"` | no | |
There was a problem hiding this comment.
Needs text update az\_monitor-user-managed-id
There is no functional change here. The tfvars file is just a helper file. See Hashicorp's docs:
This file just has a commented out version of all possible variables |
4 participants
Based on earlier discussion, users would find it helpful to have the full list of values prepopulated (and commented out) in the terraform.tfvars file
These values were autogenerated by running the terraform-docs tool