Add datastorage disk #9
Conversation
|
Looks good. I just don't want to force people to use a datastorage disk, so can you make it a conditional? |
|
To make datadisk conditional I had to create two new resources named |
main.tf
Outdated
| } | ||
|
|
||
| storage_os_disk { | ||
| name = "osdisk${count.index}" |
There was a problem hiding this comment.
Please change to "osdisk-${var.vm_hostname}-${count.index}" to make things consistent and prevent same named disk conflicts.
Please fix on line 195 as well.
main.tf
Outdated
| @@ -48,11 +48,19 @@ resource "azurerm_virtual_machine" "vm-linux" { | |||
| } | |||
|
|
|||
| storage_os_disk { | |||
| name = "osdisk-${var.vm_hostname}-${count.index}" | |||
| name = "osdisk${count.index}" | |||
There was a problem hiding this comment.
Please change to "osdisk-${var.vm_hostname}-${count.index}" to make things consistent and prevent same named disk conflicts.
variables.tf
Outdated
| default = "" | ||
| } | ||
|
|
||
| variable "datadisk" { |
There was a problem hiding this comment.
nit: keep names consistent with _ so data_disk
main.tf
Outdated
| } | ||
|
|
||
| storage_data_disk { | ||
| name = "datadisk-${var.vm_hostname}-${count.index}" |
There was a problem hiding this comment.
nit: keep spacing with = aligned throughout (i.e. here, lines 201-208, etc)
main.tf
Outdated
| @@ -30,7 +30,7 @@ resource "azurerm_storage_account" "vm-sa" { | |||
| } | |||
|
|
|||
| resource "azurerm_virtual_machine" "vm-linux" { | |||
| count = "${contains(list("${var.vm_os_simple}","${var.vm_os_offer}"), "WindowsServer") ? 0 : var.nb_instances}" | |||
| count = "${!contains(list("${var.vm_os_simple}","${var.vm_os_offer}"), "WindowsServer") && var.datadisk == "false" ? var.nb_instances : 0}" | |||
There was a problem hiding this comment.
This logic does not work for me. I am unable to provision a VM without a datadisk.
There was a problem hiding this comment.
Fixed. Regression from a bad merge :(
|
I updated the readme to be current with the code up there in this PR #17. If you want to add the data disk option for the Windows VM in the "advanced" example, that'd be cool. |
- Fix spacing with = - Fix consistent variable name on datadisk
|
LGTM. Thanks for your contribution! |
Add datadisk to instances