From 00f95f01cdf55a53f3877364115903864933c864 Mon Sep 17 00:00:00 2001 From: Arcturus Date: Mon, 25 May 2020 05:29:12 +0800 Subject: [PATCH] Fix `azurerm_orchestrated_virtual_machine_scale_set` - `azurerm_linux|windows_virtual_machine` should allow to assign `virtual_machine_scale_set_id` in non-zonal deployment (#7057) The zone in azurerm_linux|windows_virtual_machine should be allowed to be empty when the virtual_machine_scale_set_id is assigned, since the VMSS referenced by the VM can be deployed non-zonal. --- .../compute/linux_virtual_machine_resource.go | 9 +- ...tual_machine_resource_orchestrated_test.go | 222 +++++++++++++++++- ...tual_machine_resource_orchestrated_test.go | 219 ++++++++++++++++- .../windows_virtual_machine_resource.go | 8 +- 4 files changed, 424 insertions(+), 34 deletions(-) diff --git a/azurerm/internal/services/compute/linux_virtual_machine_resource.go b/azurerm/internal/services/compute/linux_virtual_machine_resource.go index 473d7a7fc96d..d7d6e45513d1 100644 --- a/azurerm/internal/services/compute/linux_virtual_machine_resource.go +++ b/azurerm/internal/services/compute/linux_virtual_machine_resource.go @@ -232,8 +232,9 @@ func resourceLinuxVirtualMachine() *schema.Resource { Type: schema.TypeString, Optional: true, ForceNew: true, - // this has to be computed because when you are trying to assign this VM to a VMSS in VMO mode, - // the VMO mode VMSS will assign a zone for each of its instance + // this has to be computed because when you are trying to assign this VM to a VMSS in VMO mode with zones, + // the VMO mode VMSS will assign a zone for each of its instance. + // and if the VMSS in not zonal, this value should be left empty Computed: true, ConflictsWith: []string{ "availability_set_id", @@ -434,10 +435,6 @@ func resourceLinuxVirtualMachineCreate(d *schema.ResourceData, meta interface{}) } if v, ok := d.GetOk("virtual_machine_scale_set_id"); ok { - // you must also specify a zone in order to assign this vm to a orchestrated vmss - if _, ok := d.GetOk("zone"); !ok { - return fmt.Errorf("`zone` must be specified when `virtual_machine_scale_set_id` is set") - } params.VirtualMachineScaleSet = &compute.SubResource{ ID: utils.String(v.(string)), } diff --git a/azurerm/internal/services/compute/tests/linux_virtual_machine_resource_orchestrated_test.go b/azurerm/internal/services/compute/tests/linux_virtual_machine_resource_orchestrated_test.go index 01933332cbe1..4f36914e5cfd 100644 --- a/azurerm/internal/services/compute/tests/linux_virtual_machine_resource_orchestrated_test.go +++ b/azurerm/internal/services/compute/tests/linux_virtual_machine_resource_orchestrated_test.go @@ -8,7 +8,7 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/acceptance" ) -func TestAccAzureRMLinuxVirtualMachine_orchestrated(t *testing.T) { +func TestAccAzureRMLinuxVirtualMachine_orchestratedZonal(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_linux_virtual_machine", "test") resource.ParallelTest(t, resource.TestCase{ @@ -17,7 +17,7 @@ func TestAccAzureRMLinuxVirtualMachine_orchestrated(t *testing.T) { CheckDestroy: checkLinuxVirtualMachineIsDestroyed, Steps: []resource.TestStep{ { - Config: testAccAzureRMLinuxVirtualMachine_orchestrated(data), + Config: testAccAzureRMLinuxVirtualMachine_orchestratedZonal(data), Check: resource.ComposeTestCheckFunc( checkLinuxVirtualMachineExists(data.ResourceName), ), @@ -26,7 +26,7 @@ func TestAccAzureRMLinuxVirtualMachine_orchestrated(t *testing.T) { }) } -func TestAccAzureRMLinuxVirtualMachine_orchestratedMultiple(t *testing.T) { +func TestAccAzureRMLinuxVirtualMachine_orchestratedNonZonal(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_linux_virtual_machine", "test") resource.ParallelTest(t, resource.TestCase{ @@ -35,7 +35,7 @@ func TestAccAzureRMLinuxVirtualMachine_orchestratedMultiple(t *testing.T) { CheckDestroy: checkLinuxVirtualMachineIsDestroyed, Steps: []resource.TestStep{ { - Config: testAccAzureRMLinuxVirtualMachine_orchestratedMultiple(data), + Config: testAccAzureRMLinuxVirtualMachine_orchestratedNonZonal(data), Check: resource.ComposeTestCheckFunc( checkLinuxVirtualMachineExists(data.ResourceName), ), @@ -44,8 +44,47 @@ func TestAccAzureRMLinuxVirtualMachine_orchestratedMultiple(t *testing.T) { }) } -func testAccAzureRMLinuxVirtualMachine_orchestrated(data acceptance.TestData) string { - template := testLinuxVirtualMachine_templateBaseForOchestratedVMSS(data) +func TestAccAzureRMLinuxVirtualMachine_orchestratedMultipleZonal(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_linux_virtual_machine", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: checkLinuxVirtualMachineIsDestroyed, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMLinuxVirtualMachine_orchestratedMultipleZonal(data), + Check: resource.ComposeTestCheckFunc( + checkLinuxVirtualMachineExists(data.ResourceName), + ), + }, + }, + }) +} + +func TestAccAzureRMLinuxVirtualMachine_orchestratedMultipleNonZonal(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_linux_virtual_machine", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: checkLinuxVirtualMachineIsDestroyed, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMLinuxVirtualMachine_orchestratedMultipleNonZonal(data), + Check: resource.ComposeTestCheckFunc( + checkLinuxVirtualMachineExists(data.ResourceName), + ), + }, + }, + }) +} + +func testAccAzureRMLinuxVirtualMachine_orchestratedZonal(data acceptance.TestData) string { + // in VMSS VMO mode, the `platform_fault_domain_count` has different acceptable values for different locations, + // therefore this location is fixed to EastUS2 to make sure the acceptance test has no issues about this value + location := "EastUS2" + template := testLinuxVirtualMachine_templateBaseForOchestratedVMSS(data, location) return fmt.Sprintf(` %s @@ -106,8 +145,70 @@ resource "azurerm_linux_virtual_machine" "test" { `, template, data.RandomInteger, data.RandomInteger, data.RandomInteger) } -func testAccAzureRMLinuxVirtualMachine_orchestratedMultiple(data acceptance.TestData) string { - template := testLinuxVirtualMachine_templateBaseForOchestratedVMSS(data) +func testAccAzureRMLinuxVirtualMachine_orchestratedNonZonal(data acceptance.TestData) string { + template := testLinuxVirtualMachine_templateBaseForOchestratedVMSS(data, data.Locations.Primary) + return fmt.Sprintf(` +%s + +resource "azurerm_network_interface" "test" { + name = "acctestnic-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + + ip_configuration { + name = "internal" + subnet_id = azurerm_subnet.test.id + private_ip_address_allocation = "Dynamic" + } +} + +resource "azurerm_orchestrated_virtual_machine_scale_set" "test" { + name = "acctestVMO-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + + platform_fault_domain_count = 2 + single_placement_group = true + + tags = { + ENV = "Test" + } +} + +resource "azurerm_linux_virtual_machine" "test" { + name = "acctestVM-%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + size = "Standard_F2" + admin_username = "adminuser" + admin_password = "P@ssw0rd1234!" + disable_password_authentication = false + network_interface_ids = [ + azurerm_network_interface.test.id, + ] + + source_image_reference { + publisher = "Canonical" + offer = "UbuntuServer" + sku = "16.04-LTS" + version = "latest" + } + + os_disk { + storage_account_type = "Standard_LRS" + caching = "ReadWrite" + } + + virtual_machine_scale_set_id = azurerm_orchestrated_virtual_machine_scale_set.test.id +} +`, template, data.RandomInteger, data.RandomInteger, data.RandomInteger) +} + +func testAccAzureRMLinuxVirtualMachine_orchestratedMultipleZonal(data acceptance.TestData) string { + // in VMSS VMO mode, the `platform_fault_domain_count` has different acceptable values for different locations, + // therefore this location is fixed to EastUS2 to make sure the acceptance test has no issues about this value + location := "EastUS2" + template := testLinuxVirtualMachine_templateBaseForOchestratedVMSS(data, location) return fmt.Sprintf(` %s @@ -208,10 +309,107 @@ resource "azurerm_linux_virtual_machine" "another" { `, template, data.RandomInteger, data.RandomInteger, data.RandomInteger, data.RandomInteger, data.RandomInteger) } -func testLinuxVirtualMachine_templateBaseForOchestratedVMSS(data acceptance.TestData) string { - // in VMSS VMO mode, the `platform_fault_domain_count` has different acceptable values for different locations, - // therefore this location is fixed to EastUS2 to make sure the acceptance test has no issues about this value - location := "EastUS2" +func testAccAzureRMLinuxVirtualMachine_orchestratedMultipleNonZonal(data acceptance.TestData) string { + template := testLinuxVirtualMachine_templateBaseForOchestratedVMSS(data, data.Locations.Primary) + return fmt.Sprintf(` +%s + +resource "azurerm_orchestrated_virtual_machine_scale_set" "test" { + name = "acctestVMO-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + + platform_fault_domain_count = 5 + single_placement_group = true + + zones = ["1"] + + tags = { + ENV = "Test" + } +} + +resource "azurerm_network_interface" "first" { + name = "acctestnic1-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + + ip_configuration { + name = "internal" + subnet_id = azurerm_subnet.test.id + private_ip_address_allocation = "Dynamic" + } +} + +resource "azurerm_linux_virtual_machine" "test" { + name = "acctestVM1-%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + size = "Standard_F2" + admin_username = "adminuser" + admin_password = "P@ssw0rd1234!" + disable_password_authentication = false + network_interface_ids = [ + azurerm_network_interface.first.id, + ] + + source_image_reference { + publisher = "Canonical" + offer = "UbuntuServer" + sku = "16.04-LTS" + version = "latest" + } + + os_disk { + storage_account_type = "Standard_LRS" + caching = "ReadWrite" + } + + virtual_machine_scale_set_id = azurerm_orchestrated_virtual_machine_scale_set.test.id +} + +resource "azurerm_network_interface" "second" { + name = "acctestnic2-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + + ip_configuration { + name = "internal" + subnet_id = azurerm_subnet.test.id + private_ip_address_allocation = "Dynamic" + } +} + +resource "azurerm_linux_virtual_machine" "another" { + name = "acctestVM2-%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + size = "Standard_F2" + admin_username = "adminuser" + admin_password = "P@ssw0rd1234!" + disable_password_authentication = false + network_interface_ids = [ + azurerm_network_interface.second.id, + ] + + source_image_reference { + publisher = "Canonical" + offer = "UbuntuServer" + sku = "18.04-LTS" + version = "latest" + } + + os_disk { + storage_account_type = "Standard_LRS" + caching = "ReadWrite" + } + + virtual_machine_scale_set_id = azurerm_orchestrated_virtual_machine_scale_set.test.id +} +`, template, data.RandomInteger, data.RandomInteger, data.RandomInteger, data.RandomInteger, data.RandomInteger) +} + +func testLinuxVirtualMachine_templateBaseForOchestratedVMSS(data acceptance.TestData, location string) string { return fmt.Sprintf(` locals { vm_name = "acctestvm%s" diff --git a/azurerm/internal/services/compute/tests/windows_virtual_machine_resource_orchestrated_test.go b/azurerm/internal/services/compute/tests/windows_virtual_machine_resource_orchestrated_test.go index 9d76a48fa68c..2698e8b7cdc9 100644 --- a/azurerm/internal/services/compute/tests/windows_virtual_machine_resource_orchestrated_test.go +++ b/azurerm/internal/services/compute/tests/windows_virtual_machine_resource_orchestrated_test.go @@ -8,7 +8,7 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/acceptance" ) -func TestAccAzureRMWindowsVirtualMachine_orchestrated(t *testing.T) { +func TestAccAzureRMWindowsVirtualMachine_orchestratedZonal(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_windows_virtual_machine", "test") resource.ParallelTest(t, resource.TestCase{ @@ -17,7 +17,7 @@ func TestAccAzureRMWindowsVirtualMachine_orchestrated(t *testing.T) { CheckDestroy: checkWindowsVirtualMachineIsDestroyed, Steps: []resource.TestStep{ { - Config: testAccAzureRMWindowsVirtualMachine_orchestrated(data), + Config: testAccAzureRMWindowsVirtualMachine_orchestratedZonal(data), Check: resource.ComposeTestCheckFunc( checkWindowsVirtualMachineExists(data.ResourceName), ), @@ -27,7 +27,7 @@ func TestAccAzureRMWindowsVirtualMachine_orchestrated(t *testing.T) { }) } -func TestAccAzureRMWindowsVirtualMachine_orchestratedMultiple(t *testing.T) { +func TestAccAzureRMWindowsVirtualMachine_orchestratedNonZonal(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_windows_virtual_machine", "test") resource.ParallelTest(t, resource.TestCase{ @@ -36,7 +36,7 @@ func TestAccAzureRMWindowsVirtualMachine_orchestratedMultiple(t *testing.T) { CheckDestroy: checkWindowsVirtualMachineIsDestroyed, Steps: []resource.TestStep{ { - Config: testAccAzureRMWindowsVirtualMachine_orchestratedMultiple(data), + Config: testAccAzureRMWindowsVirtualMachine_orchestratedNonZonal(data), Check: resource.ComposeTestCheckFunc( checkWindowsVirtualMachineExists(data.ResourceName), ), @@ -46,8 +46,49 @@ func TestAccAzureRMWindowsVirtualMachine_orchestratedMultiple(t *testing.T) { }) } -func testAccAzureRMWindowsVirtualMachine_orchestrated(data acceptance.TestData) string { - template := testWindowsVirtualMachine_templateBaseForOchestratedVMSS(data) +func TestAccAzureRMWindowsVirtualMachine_orchestratedMultipleZonal(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_windows_virtual_machine", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: checkWindowsVirtualMachineIsDestroyed, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMWindowsVirtualMachine_orchestratedMultipleZonal(data), + Check: resource.ComposeTestCheckFunc( + checkWindowsVirtualMachineExists(data.ResourceName), + ), + }, + data.ImportStep("admin_password"), + }, + }) +} + +func TestAccAzureRMWindowsVirtualMachine_orchestratedMultipleNoneZonal(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_windows_virtual_machine", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: checkWindowsVirtualMachineIsDestroyed, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMWindowsVirtualMachine_orchestratedMultipleNonZonal(data), + Check: resource.ComposeTestCheckFunc( + checkWindowsVirtualMachineExists(data.ResourceName), + ), + }, + data.ImportStep("admin_password"), + }, + }) +} + +func testAccAzureRMWindowsVirtualMachine_orchestratedZonal(data acceptance.TestData) string { + // in VMSS VMO mode, the `platform_fault_domain_count` has different acceptable values for different locations, + // therefore this location is fixed to EastUS2 to make sure the acceptance test has no issues about this value + location := "EastUS2" + template := testWindowsVirtualMachine_templateBaseForOchestratedVMSS(data, location) return fmt.Sprintf(` %s @@ -107,8 +148,69 @@ resource "azurerm_windows_virtual_machine" "test" { `, template, data.RandomInteger, data.RandomInteger) } -func testAccAzureRMWindowsVirtualMachine_orchestratedMultiple(data acceptance.TestData) string { - template := testWindowsVirtualMachine_templateBaseForOchestratedVMSS(data) +func testAccAzureRMWindowsVirtualMachine_orchestratedNonZonal(data acceptance.TestData) string { + template := testWindowsVirtualMachine_templateBaseForOchestratedVMSS(data, data.Locations.Primary) + return fmt.Sprintf(` +%s + +resource "azurerm_network_interface" "test" { + name = "acctestnic-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + + ip_configuration { + name = "internal" + subnet_id = azurerm_subnet.test.id + private_ip_address_allocation = "Dynamic" + } +} + +resource "azurerm_orchestrated_virtual_machine_scale_set" "test" { + name = "acctestVMO-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + + platform_fault_domain_count = 2 + single_placement_group = true + + tags = { + ENV = "Test" + } +} + +resource "azurerm_windows_virtual_machine" "test" { + name = local.vm_name + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + size = "Standard_F2" + admin_username = "adminuser" + admin_password = "P@ssw0rd1234!" + network_interface_ids = [ + azurerm_network_interface.test.id, + ] + + source_image_reference { + publisher = "MicrosoftWindowsServer" + offer = "WindowsServer" + sku = "2016-Datacenter" + version = "latest" + } + + os_disk { + storage_account_type = "Standard_LRS" + caching = "ReadWrite" + } + + virtual_machine_scale_set_id = azurerm_orchestrated_virtual_machine_scale_set.test.id +} +`, template, data.RandomInteger, data.RandomInteger) +} + +func testAccAzureRMWindowsVirtualMachine_orchestratedMultipleZonal(data acceptance.TestData) string { + // in VMSS VMO mode, the `platform_fault_domain_count` has different acceptable values for different locations, + // therefore this location is fixed to EastUS2 to make sure the acceptance test has no issues about this value + location := "EastUS2" + template := testWindowsVirtualMachine_templateBaseForOchestratedVMSS(data, location) return fmt.Sprintf(` %s @@ -207,10 +309,103 @@ resource "azurerm_windows_virtual_machine" "another" { `, template, data.RandomInteger, data.RandomInteger, data.RandomInteger, data.RandomIntOfLength(9), data.RandomIntOfLength(9)) } -func testWindowsVirtualMachine_templateBaseForOchestratedVMSS(data acceptance.TestData) string { - // in VMSS VMO mode, the `platform_fault_domain_count` has different acceptable values for different locations, - // therefore this location is fixed to EastUS2 to make sure the acceptance test has no issues about this value - location := "EastUS2" +func testAccAzureRMWindowsVirtualMachine_orchestratedMultipleNonZonal(data acceptance.TestData) string { + template := testWindowsVirtualMachine_templateBaseForOchestratedVMSS(data, data.Locations.Primary) + return fmt.Sprintf(` +%s + +resource "azurerm_orchestrated_virtual_machine_scale_set" "test" { + name = "acctestVMO-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + + platform_fault_domain_count = 2 + single_placement_group = true + + tags = { + ENV = "Test" + } +} + +resource "azurerm_network_interface" "first" { + name = "acctestnic1-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + + ip_configuration { + name = "internal" + subnet_id = azurerm_subnet.test.id + private_ip_address_allocation = "Dynamic" + } +} + +resource "azurerm_network_interface" "second" { + name = "acctestnic2-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + + ip_configuration { + name = "internal" + subnet_id = azurerm_subnet.test.id + private_ip_address_allocation = "Dynamic" + } +} + +resource "azurerm_windows_virtual_machine" "test" { + name = "accVM1%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + size = "Standard_F2" + admin_username = "adminuser" + admin_password = "P@ssw0rd1234!" + network_interface_ids = [ + azurerm_network_interface.first.id, + ] + + source_image_reference { + publisher = "MicrosoftWindowsServer" + offer = "WindowsServer" + sku = "2016-Datacenter" + version = "latest" + } + + os_disk { + storage_account_type = "Standard_LRS" + caching = "ReadWrite" + } + + virtual_machine_scale_set_id = azurerm_orchestrated_virtual_machine_scale_set.test.id +} + +resource "azurerm_windows_virtual_machine" "another" { + name = "accVM2%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + size = "Standard_F2" + admin_username = "adminuser" + admin_password = "P@ssw0rd1234!" + network_interface_ids = [ + azurerm_network_interface.second.id, + ] + + source_image_reference { + publisher = "MicrosoftWindowsServer" + offer = "WindowsServer" + sku = "2019-Datacenter" + version = "latest" + } + + os_disk { + storage_account_type = "Standard_LRS" + caching = "ReadWrite" + } + + virtual_machine_scale_set_id = azurerm_orchestrated_virtual_machine_scale_set.test.id +} +`, template, data.RandomInteger, data.RandomInteger, data.RandomInteger, data.RandomIntOfLength(9), data.RandomIntOfLength(9)) +} + +func testWindowsVirtualMachine_templateBaseForOchestratedVMSS(data acceptance.TestData, location string) string { return fmt.Sprintf(` locals { vm_name = "acctestvm%s" diff --git a/azurerm/internal/services/compute/windows_virtual_machine_resource.go b/azurerm/internal/services/compute/windows_virtual_machine_resource.go index 3ebc57e3246b..04ee00d365d1 100644 --- a/azurerm/internal/services/compute/windows_virtual_machine_resource.go +++ b/azurerm/internal/services/compute/windows_virtual_machine_resource.go @@ -253,6 +253,10 @@ func resourceWindowsVirtualMachine() *schema.Resource { Type: schema.TypeString, Optional: true, ForceNew: true, + // this has to be computed because when you are trying to assign this VM to a VMSS in VMO mode with zones, + // the VMO mode VMSS will assign a zone for each of its instance. + // and if the VMSS in not zonal, this value should be left empty + Computed: true, ConflictsWith: []string{ "availability_set_id", }, @@ -463,10 +467,6 @@ func resourceWindowsVirtualMachineCreate(d *schema.ResourceData, meta interface{ } if v, ok := d.GetOk("virtual_machine_scale_set_id"); ok { - // you must also specify a zone in order to assign this vm to a orchestrated vmss - if _, ok := d.GetOk("zone"); !ok { - return fmt.Errorf("`zone` must be specified when `virtual_machine_scale_set_id` is set") - } params.VirtualMachineScaleSet = &compute.SubResource{ ID: utils.String(v.(string)), }