From a67f9821f564c89880bc9a1b96f1807eff801714 Mon Sep 17 00:00:00 2001 From: hashitop <50546939+hashitop@users.noreply.github.com> Date: Fri, 19 Jul 2019 02:40:11 +1000 Subject: [PATCH] =?UTF-8?q?[azurerm=5Frecovery=5Fservices=5Fprotected=5Fvm?= =?UTF-8?q?]=20-=20changing=20`backup=5Fpol=E2=80=A6=20(#3822)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refer to #3441 now changing the behaviour of backup_policy_id so it should not trigger recreation of resource when value changed (fixes #3441) --- ...urce_arm_recovery_services_protected_vm.go | 25 +- ...arm_recovery_services_protected_vm_test.go | 252 ++++++++++++++++++ .../r/recovery_services_protected_vm.markdown | 2 +- 3 files changed, 273 insertions(+), 6 deletions(-) diff --git a/azurerm/resource_arm_recovery_services_protected_vm.go b/azurerm/resource_arm_recovery_services_protected_vm.go index 2f22ea11d9b6..7f76069a9934 100644 --- a/azurerm/resource_arm_recovery_services_protected_vm.go +++ b/azurerm/resource_arm_recovery_services_protected_vm.go @@ -54,7 +54,6 @@ func resourceArmRecoveryServicesProtectedVm() *schema.Resource { "backup_policy_id": { Type: schema.TypeString, Required: true, - ForceNew: true, ValidateFunc: azure.ValidateResourceID, }, @@ -118,10 +117,11 @@ func resourceArmRecoveryServicesProtectedVmCreateUpdate(d *schema.ResourceData, return fmt.Errorf("Error creating/updating Recovery Service Protected VM %q (Resource Group %q): %+v", protectedItemName, resourceGroup, err) } - resp, err := resourceArmRecoveryServicesProtectedVmWaitForState(client, ctx, true, vaultName, resourceGroup, containerName, protectedItemName) + resp, err := resourceArmRecoveryServicesProtectedVmWaitForState(client, ctx, true, vaultName, resourceGroup, containerName, protectedItemName, policyId, d.IsNewResource()) if err != nil { return err } + id := strings.Replace(*resp.ID, "Subscriptions", "subscriptions", 1) d.SetId(id) @@ -195,14 +195,14 @@ func resourceArmRecoveryServicesProtectedVmDelete(d *schema.ResourceData, meta i } } - if _, err := resourceArmRecoveryServicesProtectedVmWaitForState(client, ctx, false, vaultName, resourceGroup, containerName, protectedItemName); err != nil { + if _, err := resourceArmRecoveryServicesProtectedVmWaitForState(client, ctx, false, vaultName, resourceGroup, containerName, protectedItemName, "", false); err != nil { return err } return nil } -func resourceArmRecoveryServicesProtectedVmWaitForState(client backup.ProtectedItemsGroupClient, ctx context.Context, found bool, vaultName, resourceGroup, containerName, protectedItemName string) (backup.ProtectedItemResource, error) { +func resourceArmRecoveryServicesProtectedVmWaitForState(client backup.ProtectedItemsGroupClient, ctx context.Context, found bool, vaultName, resourceGroup, containerName, protectedItemName string, policyId string, newResource bool) (backup.ProtectedItemResource, error) { state := &resource.StateChangeConf{ Timeout: 30 * time.Minute, MinTimeout: 30 * time.Second, @@ -216,8 +216,23 @@ func resourceArmRecoveryServicesProtectedVmWaitForState(client backup.ProtectedI } return resp, "Error", fmt.Errorf("Error making Read request on Recovery Service Protected VM %q (Resource Group %q): %+v", protectedItemName, resourceGroup, err) + } else if !newResource && policyId != "" { + if properties := resp.Properties; properties != nil { + if vm, ok := properties.AsAzureIaaSComputeVMProtectedItem(); ok { + if v := vm.PolicyID; v != nil { + if strings.Replace(*v, "Subscriptions", "subscriptions", 1) != policyId { + return resp, "NotFound", nil + } + } else { + return resp, "Error", fmt.Errorf("Error reading policy ID attribute nil on Recovery Service Protected VM %q (Resource Group %q)", protectedItemName, resourceGroup) + } + } else { + return resp, "Error", fmt.Errorf("Error reading properties on Recovery Service Protected VM %q (Resource Group %q)", protectedItemName, resourceGroup) + } + } else { + return resp, "Error", fmt.Errorf("Error reading properties on empty Recovery Service Protected VM %q (Resource Group %q)", protectedItemName, resourceGroup) + } } - return resp, "Found", nil }, } diff --git a/azurerm/resource_arm_recovery_services_protected_vm_test.go b/azurerm/resource_arm_recovery_services_protected_vm_test.go index 4feee23e6d00..8e1c9b61bc11 100644 --- a/azurerm/resource_arm_recovery_services_protected_vm_test.go +++ b/azurerm/resource_arm_recovery_services_protected_vm_test.go @@ -103,6 +103,55 @@ func TestAccAzureRMRecoveryServicesProtectedVm_separateResourceGroups(t *testing }) } +func TestAccAzureRMRecoveryServicesProtectedVm_updateBackupPolicyId(t *testing.T) { + virtualMachine := "azurerm_virtual_machine.test" + protectedVmResourceName := "azurerm_recovery_services_protected_vm.test" + fBackupPolicyResourceName := "azurerm_recovery_services_protection_policy_vm.test" + sBackupPolicyResourceName := "azurerm_recovery_services_protection_policy_vm.test_change_backup" + + ri := tf.AccRandTimeInt() + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureRMRecoveryServicesProtectedVmDestroy, + Steps: []resource.TestStep{ + { // Create resources and link first backup policy id + ResourceName: fBackupPolicyResourceName, + Config: testAccAzureRMRecoveryServicesProtectedVm_linkFirstBackupPolicy(ri, testLocation()), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrPair(protectedVmResourceName, "backup_policy_id", fBackupPolicyResourceName, "id"), + ), + }, + { // Modify backup policy id to the second one + // Set Destroy false to prevent error from cleaning up dangling resource + ResourceName: sBackupPolicyResourceName, + Config: testAccAzureRMRecoveryServicesProtectedVm_linkSecondBackupPolicy(ri, testLocation()), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrPair(protectedVmResourceName, "backup_policy_id", sBackupPolicyResourceName, "id"), + ), + }, + { // Remove backup policy link + // Backup policy link will need to be removed first so the VM's backup policy subsequently reverts to Default + // Azure API is quite sensitive, adding the step to control resource cleanup order + ResourceName: fBackupPolicyResourceName, + Config: testAccAzureRMRecoveryServicesProtectedVm_withVM(ri, testLocation()), + Check: resource.ComposeTestCheckFunc(), + }, + { // Then VM can be removed + ResourceName: virtualMachine, + Config: testAccAzureRMRecoveryServicesProtectedVm_withSecondPolicy(ri, testLocation()), + Check: resource.ComposeTestCheckFunc(), + }, + { // Remove backup policies and vault + ResourceName: protectedVmResourceName, + Config: testAccAzureRMRecoveryServicesProtectedVm_basePolicyTest(ri, testLocation()), + Check: resource.ComposeTestCheckFunc(), + }, + }, + }) +} + func testCheckAzureRMRecoveryServicesProtectedVmDestroy(s *terraform.State) error { for _, rs := range s.RootModule().Resources { if rs.Type != "azurerm_recovery_services_protected_vm" { @@ -331,6 +380,209 @@ resource "azurerm_recovery_services_protected_vm" "test" { `, testAccAzureRMRecoveryServicesProtectedVm_base(rInt, location)) } +// For update backup policy id test +func testAccAzureRMRecoveryServicesProtectedVm_basePolicyTest(rInt int, location string) string { + rstr := strconv.Itoa(rInt) + return fmt.Sprintf(` +resource "azurerm_resource_group" "test" { + name = "acctestRG-%[1]d" + location = "%[2]s" +} + +resource "azurerm_virtual_network" "test" { + name = "vnet" + location = "${azurerm_resource_group.test.location}" + address_space = ["10.0.0.0/16"] + resource_group_name = "${azurerm_resource_group.test.name}" +} + +resource "azurerm_subnet" "test" { + name = "acctest_subnet" + virtual_network_name = "${azurerm_virtual_network.test.name}" + resource_group_name = "${azurerm_resource_group.test.name}" + address_prefix = "10.0.10.0/24" +} + +resource "azurerm_network_interface" "test" { + name = "acctest_nic" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + + ip_configuration { + name = "acctestipconfig" + subnet_id = "${azurerm_subnet.test.id}" + private_ip_address_allocation = "Dynamic" + public_ip_address_id = "${azurerm_public_ip.test.id}" + } +} + +resource "azurerm_public_ip" "test" { + name = "acctest-ip" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + allocation_method = "Dynamic" + domain_name_label = "acctestip%[1]d" +} + +resource "azurerm_storage_account" "test" { + name = "acctest%[3]s" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + account_tier = "Standard" + account_replication_type = "LRS" +} + +resource "azurerm_managed_disk" "test" { + name = "acctest-datadisk" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + storage_account_type = "Standard_LRS" + create_option = "Empty" + disk_size_gb = "1023" +} +`, rInt, location, rstr[len(rstr)-5:]) +} + +// For update backup policy id test +func testAccAzureRMRecoveryServicesProtectedVm_withVault(rInt int, location string) string { + return fmt.Sprintf(` +%[1]s + +resource "azurerm_recovery_services_vault" "test" { + name = "acctest-%[2]d" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + sku = "Standard" +} +`, testAccAzureRMRecoveryServicesProtectedVm_basePolicyTest(rInt, location), rInt) +} + +// For update backup policy id test +func testAccAzureRMRecoveryServicesProtectedVm_withFirstPolicy(rInt int, location string) string { + return fmt.Sprintf(` +%[1]s + +resource "azurerm_recovery_services_protection_policy_vm" "test" { + name = "acctest-%[2]d" + resource_group_name = "${azurerm_resource_group.test.name}" + recovery_vault_name = "${azurerm_recovery_services_vault.test.name}" + + backup { + frequency = "Daily" + time = "23:00" + } + + retention_daily { + count = 10 + } +} +`, testAccAzureRMRecoveryServicesProtectedVm_withVault(rInt, location), rInt) +} + +// For update backup policy id test +func testAccAzureRMRecoveryServicesProtectedVm_withSecondPolicy(rInt int, location string) string { + return fmt.Sprintf(` +%[1]s + +resource "azurerm_recovery_services_protection_policy_vm" "test_change_backup" { + name = "acctest2-%[2]d" + resource_group_name = "${azurerm_resource_group.test.name}" + recovery_vault_name = "${azurerm_recovery_services_vault.test.name}" + + backup { + frequency = "Daily" + time = "23:00" + } + + retention_daily { + count = 15 + } +} +`, testAccAzureRMRecoveryServicesProtectedVm_withFirstPolicy(rInt, location), rInt) +} + +// For update backup policy id test +func testAccAzureRMRecoveryServicesProtectedVm_withVM(rInt int, location string) string { + return fmt.Sprintf(` +%[1]s + +resource "azurerm_virtual_machine" "test" { + name = "acctestvm-%[2]d" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + vm_size = "Standard_A0" + network_interface_ids = ["${azurerm_network_interface.test.id}"] + delete_os_disk_on_termination = true + + storage_image_reference { + publisher = "Canonical" + offer = "UbuntuServer" + sku = "16.04-LTS" + version = "latest" + } + + storage_os_disk { + name = "acctest-osdisk" + managed_disk_type = "Standard_LRS" + caching = "ReadWrite" + create_option = "FromImage" + } + + storage_data_disk { + name = "acctest-datadisk" + managed_disk_id = "${azurerm_managed_disk.test.id}" + managed_disk_type = "Standard_LRS" + disk_size_gb = "${azurerm_managed_disk.test.disk_size_gb}" + create_option = "Attach" + lun = 0 + } + + os_profile { + computer_name = "acctest" + admin_username = "vmadmin" + admin_password = "Password123!@#" + } + + os_profile_linux_config { + disable_password_authentication = false + } + + boot_diagnostics { + enabled = true + storage_uri = "${azurerm_storage_account.test.primary_blob_endpoint}" + } +} +`, testAccAzureRMRecoveryServicesProtectedVm_withSecondPolicy(rInt, location), rInt) +} + +// For update backup policy id test +func testAccAzureRMRecoveryServicesProtectedVm_linkFirstBackupPolicy(rInt int, location string) string { + return fmt.Sprintf(` +%s + +resource "azurerm_recovery_services_protected_vm" "test" { + resource_group_name = "${azurerm_resource_group.test.name}" + recovery_vault_name = "${azurerm_recovery_services_vault.test.name}" + source_vm_id = "${azurerm_virtual_machine.test.id}" + backup_policy_id = "${azurerm_recovery_services_protection_policy_vm.test.id}" +} +`, testAccAzureRMRecoveryServicesProtectedVm_withVM(rInt, location)) +} + +// For update backup policy id test +func testAccAzureRMRecoveryServicesProtectedVm_linkSecondBackupPolicy(rInt int, location string) string { + return fmt.Sprintf(` +%s + +resource "azurerm_recovery_services_protected_vm" "test" { + resource_group_name = "${azurerm_resource_group.test.name}" + recovery_vault_name = "${azurerm_recovery_services_vault.test.name}" + source_vm_id = "${azurerm_virtual_machine.test.id}" + backup_policy_id = "${azurerm_recovery_services_protection_policy_vm.test_change_backup.id}" +} +`, testAccAzureRMRecoveryServicesProtectedVm_withVM(rInt, location)) +} + func testAccAzureRMRecoveryServicesProtectedVm_requiresImport(rInt int, location string) string { return fmt.Sprintf(` %s diff --git a/website/docs/r/recovery_services_protected_vm.markdown b/website/docs/r/recovery_services_protected_vm.markdown index 737c314004cf..46622b5a7378 100644 --- a/website/docs/r/recovery_services_protected_vm.markdown +++ b/website/docs/r/recovery_services_protected_vm.markdown @@ -54,7 +54,7 @@ The following arguments are supported: * `source_vm_id` - (Required) Specifies the ID of the VM to backup. Changing this forces a new resource to be created. -* `backup_policy_id` - (Required) Specifies the id of the backup policy to use. Changing this forces a new resource to be created. +* `backup_policy_id` - (Required) Specifies the id of the backup policy to use. * `tags` - (Optional) A mapping of tags to assign to the resource.