Add boot diagnostics option #7
Conversation
…Azure/network/azurerm
Fixed issue with second example to call Azure/network/azurerm module
There was a problem hiding this comment.
I just did some testing to try and make this work. count does not work and if you go down the path to do a conditional on the storage_uri you'll run into this limitation of HCL: hashicorp/terraform#15605
| @@ -83,6 +96,10 @@ resource "azurerm_virtual_machine" "vm-windows" { | |||
| admin_username = "${var.admin_username}" | |||
| admin_password = "${var.admin_password}" | |||
| } | |||
| boot_diagnostics { | |||
| enabled = "${var.boot_diagnostics}" | |||
There was a problem hiding this comment.
Unfortunately, it seems this code is not working for me because when I use the default of setting boot_diagnostics to false, I still get these errors:
Error running plan: 1 error(s) occurred:
* module.mycompute.azurerm_virtual_machine.vm-windows: 2 error(s) occurred:
* module.mycompute.azurerm_virtual_machine.vm-windows[1]: Resource 'azurerm_storage_account.vm-sa' not found for variable 'azurerm_storage_account.vm-sa.primary_blob_endpoint'
* module.mycompute.azurerm_virtual_machine.vm-windows[0]: Resource 'azurerm_storage_account.vm-sa' not found for variable 'azurerm_storage_account.vm-sa.primary_blob_endpoint'
When I set it to true, it works as expected. I'm using the latest terraform v0.10.7 with Azure module 0.2.2.
There was a problem hiding this comment.
Thanks for your review. I fixed this issue in commit 4cc1a4a.
Add vm_hostname to nic interface name for more comprehension
Add vm_hostname to os_disk name Allow delete_os_disk_on_termination to be specified in variables
main.tf
Outdated
| @@ -13,6 +13,15 @@ resource "azurerm_resource_group" "vm" { | |||
| tags = "${var.tags}" | |||
| } | |||
|
|
|||
| resource "azurerm_storage_account" "vm-sa" { | |||
| count = "${var.boot_diagnostics == "true" ? 1 : 0}" | |||
| name = "${lower(replace(var.vm_hostname,"/[[:^alpha:]]/",""))}" | |||
There was a problem hiding this comment.
This storage account name needs to be unique per subscription, so I worry it will easily become a problem for people using the vm_hostname.
* azurerm_storage_account.vm-sa: Error creating Azure Storage Account 'myvm': storage.AccountsClient#Create: Failure responding to request: StatusCode=409 -- Original Error: autorest/azure: Service returned an error. Status=409 Code="StorageAccountAlreadyExists" Message="The storage account named myvm already exists under the subscription."
If we use the public_ip_dns value then we'd require people use a public IP address and we don't want that. If we add another variable with a default value then people would have to change it manually and that's not good. I'll just approve this as-is. Only thing which would be nice (not sure if its possible with hcl) is to append a random number to the storage account after the hostname. Thoughts?
There was a problem hiding this comment.
The random value seems to be a good idea. I'll try to do something with the random module : https://www.terraform.io/docs/providers/random/index.html
There was a problem hiding this comment.
I've done some tests, and we can add a random id on the storage account name.
But, the storage account name is still base on vm hostname and we may go through the character limitation (between 3 and 24). What do you think about a full random name with a prefix like bootdiagsa ?
There was a problem hiding this comment.
The random provider looks like it will work perfect! You can control the byte length so exceeding the characters won't be a problem. The safest route would be to use something like bootdiag plus the random string, which I'd be happy with. Only if you wanted to be fancy (not required ;)) then you could do something like format(var.vm_hostname, arg which uses only first # of characters) + diag + random id.
|
LGTM |
Allow to add boot diagnostics option.