-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[azurerm_recovery_services_protected_vm] - changing backup_policy_id
no longer forces a new resource
#3822
[azurerm_recovery_services_protected_vm] - changing backup_policy_id
no longer forces a new resource
#3822
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @hashitop,
Thank you for this bugfix. Could we add a test that updates the policy? Aside from that LGTM
Hi @katbyte As you reminded me about that I went for further testing and found the destroy operation is erratic and always failed on first attempt once updated the backup_policy_id, a rerun seems to fix it. I may need some more time to debug the destroy scenario. Thanks! |
Cool, thanks for the update @hashitop 🙂 |
Hi @katbyte This would be a piece of information in relation to the commit for adding test case and a minor fix on top of previous commit so please have another review The issue with destroy appeared to be related to the state of azurerm_recovery_services_protection_policy_vm resource went out-of-sync. The cause of the glitch in the state was due to a bug or may be a strange symptom in Azure API, checked the go sdk part and it looks ok. The trace logs below captured from when the change made to the resource and then read back the status from the same resource where it showed the discrepancy. It's ended up adding a slight delay prior to sending out next subsequent GET API request solved this strange symptom.
2019-07-13T02:38:29.288+1000 [DEBUG] plugin.terraform-provider-azurerm: 2019/07/13 02:38:29 =============================================
2019-07-13T02:38:41.363+1000 [DEBUG] plugin.terraform-provider-azurerm: GET /Subscriptions/xxxxx-xxx-xxx-xxxx-xxxxx/resourceGroups/rg-demo-backup-test14/providers/Microsoft.RecoveryServices/vaults/rsv-demo-test14/backupFabrics/Azure/protectionContainers/IaasVMContainer%3Biaasvmcontainerv2%3Brg-demo-backup-test14%3Bdemovm-test14/protectedItems/VM%3Biaasvmcontainerv2%3Brg-demo-backup-test14%3Bdemovm-test14?api-version=2016-12-01 HTTP/1.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the revisions @hashitop,
Aside from a few minor comments i've left inline this LGTM. Once those are addressed this should be good to merge 🙂
backup_policy_id
no longer forces a new resource
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes @hashitop! LGTM now 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes @hashitop! LGTM now 👍
This has been released in version 1.32.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example: provider "azurerm" {
version = "~> 1.32.0"
}
# ... other configuration ... |
* r/storage_blob: refactoring the existing tests * Updating to use hashicorp#3857 * Add metadata to example usage * r/storage_blob: rewriting the acceptance tests * r/storage_blob: test fixes * Adding a todo * r/private_dns_a_record: fixing comments from code review * Updating to include hashicorp#3847 * Updating to include hashicorp#3849 * Add schedule backup functionality. * Add doc in app_service.html.markdown file * Add deafult value to backup_schedule_enabled * Add default value to backup_schedule_enabled parameter * delete backup-schedule_enabled parameter * Delete backupScheduleEnabled param * Add condition err in resourceArmDeleteScheduleBackup * In deleteConfiguration function delete check * Add HasChange in update function * Fix errcheck in DeleteBackupConfiguration * Frequency interval validation range limit * Delete backupSchedule in create function * Update azurerm/resource_arm_app_service.go Co-Authored-By: Matthew Frahry <mbfrahry@gmail.com> * Validate retention period range limit * Fix tabulation and add validate to backup_name * Add frequency_unit validation * azurerm_app_service: fixing the code comments from hashicorp#3330 ``` $ acctests azurerm TestAccAzureRMAppService_backup === RUN TestAccAzureRMAppService_backup === PAUSE TestAccAzureRMAppService_backup === CONT TestAccAzureRMAppService_backup --- PASS: TestAccAzureRMAppService_backup (660.26s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm 660.313s ``` ``` $ acctests azurerm TestAccAzureRMAppService_basic === RUN TestAccAzureRMAppService_basic === PAUSE TestAccAzureRMAppService_basic === RUN TestAccAzureRMAppService_basicWindowsContainer === PAUSE TestAccAzureRMAppService_basicWindowsContainer === CONT TestAccAzureRMAppService_basic --- PASS: TestAccAzureRMAppService_basic (189.59s) === CONT TestAccAzureRMAppService_basicWindowsContainer --- PASS: TestAccAzureRMAppService_basicWindowsContainer (369.53s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm 559.155s ``` * fwixing comments from code review * Updating to include hashicorp#3804 * [azurerm_recovery_services_protected_vm] - changing `backup_pol… (hashicorp#3822) Refer to hashicorp#3441 now changing the behaviour of backup_policy_id so it should not trigger recreation of resource when value changed (fixes hashicorp#3441) * Update CHANGELOG.md to include hashicorp#3822 * storage: using the environment as available * Fix documentation. Real resource name is azurerm_application_in… (hashicorp#3877) * Fix azurerm_redis_cache non-SSL port in docs * fix merging problem * change firewall test to explicit config fix bug of returning nil * fix some errors; enable adminUsers test but not working currently * fix linting error * fix linting error * fix admin users test * r/storage_share: switching the state migration over to 0.12 format ``` $ go test -v ./azurerm/ -run=TestAzureRMStorageShareMigrateState === RUN TestAzureRMStorageShareMigrateStateV0ToV1 2019/07/19 13:33:46 [DEBUG] Updating ID from "share1" to "share1/group1/account1" 2019/07/19 13:33:46 [DEBUG] Updating ID from "share1" to "share1/group1/account1" 2019/07/19 13:33:46 [DEBUG] Updating ID from "share1" to "share1/group1/account1" 2019/07/19 13:33:46 [DEBUG] Updating ID from "share1" to "share1/group1/account1" --- PASS: TestAzureRMStorageShareMigrateStateV0ToV1 (0.00s) resource_arm_storage_share_migration_test.go:20: [DEBUG] Testing with Cloud "AzureChinaCloud" resource_arm_storage_share_migration_test.go:49: [DEBUG] Ok! resource_arm_storage_share_migration_test.go:20: [DEBUG] Testing with Cloud "AzureGermanCloud" resource_arm_storage_share_migration_test.go:49: [DEBUG] Ok! resource_arm_storage_share_migration_test.go:20: [DEBUG] Testing with Cloud "AzurePublicCloud" resource_arm_storage_share_migration_test.go:49: [DEBUG] Ok! resource_arm_storage_share_migration_test.go:20: [DEBUG] Testing with Cloud "AzureUSGovernmentCloud" resource_arm_storage_share_migration_test.go:49: [DEBUG] Ok! === RUN TestAzureRMStorageShareMigrateStateV1ToV2 2019/07/19 13:33:46 [DEBUG] Updating Resource ID from "share1/group1/account1" to "https://account1.file.core.chinacloudapi.cn/share1" 2019/07/19 13:33:46 [DEBUG] Updating Resource ID from "share1/group1/account1" to "https://account1.file.core.cloudapi.de/share1" 2019/07/19 13:33:46 [DEBUG] Updating Resource ID from "share1/group1/account1" to "https://account1.file.core.windows.net/share1" 2019/07/19 13:33:46 [DEBUG] Updating Resource ID from "share1/group1/account1" to "https://account1.file.core.usgovcloudapi.net/share1" --- PASS: TestAzureRMStorageShareMigrateStateV1ToV2 (0.00s) resource_arm_storage_share_migration_test.go:62: [DEBUG] Testing with Cloud "AzureChinaCloud" resource_arm_storage_share_migration_test.go:91: [DEBUG] Ok! resource_arm_storage_share_migration_test.go:62: [DEBUG] Testing with Cloud "AzureGermanCloud" resource_arm_storage_share_migration_test.go:91: [DEBUG] Ok! resource_arm_storage_share_migration_test.go:62: [DEBUG] Testing with Cloud "AzurePublicCloud" resource_arm_storage_share_migration_test.go:91: [DEBUG] Ok! resource_arm_storage_share_migration_test.go:62: [DEBUG] Testing with Cloud "AzureUSGovernmentCloud" resource_arm_storage_share_migration_test.go:91: [DEBUG] Ok! PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm 0.038s ``` * r/kubernetes_cluster: fixing comments from code review * r/kubernetes_cluster: ordering the sections alphabetically * adjust pr comments * r/kubernetes_cluster: switching to an import test * fixing comments from code review * sidebar: fixing the ordering * Updating to include hashicorp#3785 * add function app auth settings * docs * bug: missing link to azurerm_api_management_subscription (hashicorp#3888) * r/analysis_services_server: ensuring the ID is set in the create * updating to include hashicorp#3721 * updating to include hashicorp#3893 * Add auth_settings support to app service slot * Add auth_settings tests to app service slot * auth_settings block in app service slot docs * Missed import modules for test * return value for UpdateAuthSettingsSlot * Wrong resource name in tests * r/app_service_slot: wrapping the errors * r/app_service_slot: renaming the CreateUpdate method to Create since it's been split * r/storage_container: metadata should be computed ``` $ acctests azurerm TestAccAzureRMStorageContainer_metaData === RUN TestAccAzureRMStorageContainer_metaData === PAUSE TestAccAzureRMStorageContainer_metaData === CONT TestAccAzureRMStorageContainer_metaData --- PASS: TestAccAzureRMStorageContainer_metaData (165.53s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm 165.571s ``` * [Feature] Azure Maps Account Support Adding Azure Map Accounts support to Terraform. Adds azurerm_maps_account data source. Adds azurerm_maps_account resource type. Adds website documentation for data source and resource. Adds data source and resource acceptance tests. * vendoring the maps sdk * r/maps_account: changes from code review * docs: adding to the sidebar * updating to include hashicorp#3897 * r/maps: removing dead code * d/cosmosdb_account: handling the document endpoint being nil (hashicorp#3899) * maps: making the sku lower-case to match the api * r/storage_container: support for containers named `$web` (hashicorp#3896) Fixes hashicorp#3894 * Update CHANGELOG.md to include hashicorp#3896 * maps: fixing the tests ``` $ acctests azurerm TestAccAzureRMMapsAccount_ === RUN TestAccAzureRMMapsAccount_basic === PAUSE TestAccAzureRMMapsAccount_basic === RUN TestAccAzureRMMapsAccount_sku === PAUSE TestAccAzureRMMapsAccount_sku === RUN TestAccAzureRMMapsAccount_tags === PAUSE TestAccAzureRMMapsAccount_tags === CONT TestAccAzureRMMapsAccount_basic === CONT TestAccAzureRMMapsAccount_tags --- PASS: TestAccAzureRMMapsAccount_basic (80.43s) --- PASS: TestAccAzureRMMapsAccount_tags (111.23s) === CONT TestAccAzureRMMapsAccount_sku --- PASS: TestAccAzureRMMapsAccount_sku (82.26s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm 273.956s ``` * [azurerm_kubernetes_cluster ] - support for the `windows_profil… (hashicorp#3519) fixes hashicorp#3508 * Update CHANGELOG.md to include hashicorp#3519 * updating to include hashicorp#3698 * sorting/reordering the changelog * decrease gogc to prevent linting OOM * website: Fix a sidebar name (hashicorp#3911) * `azurerm_storage_account`: set `queue_properties` (hashicorp#3859) * Update CHANGELOG.md * azurerm_cosmosdb_account: validate max_interval_in_seconds and… (hashicorp#3906) fixes hashicorp#3551 * Update CHANGELOG.md to include hashicorp#3906 * kubernetes_cluster: support specifying load balancer sku (hashicorp#3890) Fixes hashicorp#3726 * Update CHANGELOG.md to include hashicorp#3890 * Update mysql_server.html.markdown (hashicorp#3914) * azurerm_application_gateway - Support for Managed Identity (resolves hashicorp#3640) (hashicorp#3648) resolves hashicorp#3640 * Update CHANGELOG.md to include hashicorp#3648 * v1.32.0 * Cleanup after v1.32.0 release * Sending nil certificate when no certificate is specified (hashicorp#3931) * azurerm_kubernetes_cluster - Remove duplicated line in docs (hashicorp#3940) * Bug Fix: `azurerm_api_managment_product` - Additional validation for `approval_required` (hashicorp#3945) * Update CHANGELOG.md * Bug Fix: `azurerm_storage_table` - Migrate id to new format hashicorp#3932 * Bug Fix: `azurerm_kubernetes_cluster` - suppress case difference on `load_balancer_sku` (hashicorp#3958) * Fix crash (hashicorp#3966) * Update CHANGELOG.md * Fix the typo in the application gateway name (hashicorp#3953) The application gateway was referenced by name `test` while it's defined as `network` * azurerm_storage_account - fix `enable_advanced_threat_protectio… (hashicorp#3947) fixes hashicorp#3920 * Update CHANGELOG.md to include hashicorp#3947 * Update CHANGELOG.md * Bug Fix - `azurerm_iot_dps` fix deletion issue with service principal hashicorp#3973 * Update CHANGELOG.md * Update hash function for os_profile_linux_config (hashicorp#3837) * Add new acc test to verify ssh key issue This commit adds a new acceptance test to verify that the vmss is not calculating correctly the hashcode for the `os_profile_linux_config` Related: hashicorp#3836 * Update OsProfileLinuxConfigHash generator Update the `resourceArmVirtualMachineScaleSetOsProfileLinuxConfigHash` method to include in the hash function the fields from the `ssh_keys` subelements. Fixes: hashicorp#3836 * Update CHANGELOG.md to include hashicorp#3837 * Bugfix: Fixes the acceptance tests for resource_arm_dev_test_*_virtual_machine_test.go (hashicorp#3717) * Update CHANGELOG.md to include hashicorp#3717 * azurerm_network_ddos_protection_plan - correctly decodes the re… (hashicorp#3975) Fixes hashicorp#3974 * Update CHANGELOG.md to include hashicorp#3975 * azurerm_virtual_machine_scale_set - changes made to `network… (hashicorp#3821) The acceptance tests added here are describing the issue as reported by this ticket hashicorp#3812 Initially tried to remove the custom Set method which calculates the hashcode similar to what is suggested by hashicorp/terraform#10520 and hashicorp/terraform#15420 but then we got into a position that the test framework was generating a perpetual diff. So the current solution was to include the other fields in the hashcode calculation. Many thanks to @kwilczynski Fixes: hashicorp#3812 Co-authored-by: Krzysztof Wilczynski kw@linux.com * Update CHANGELOG.md to include hashicorp#3821 * azurerm_postgresql_server - support for version `11.0` (hashicorp#3970) Fixes hashicorp#3969, and relates to hashicorp#3327 even so it doesn't solve the problem for 10.X versions. The Azure SDK for go doesn't provide version 10.7 for example, see here. But fortunately it has a version 11. * Update CHANGELOG.md to include hashicorp#3970 * Update CHANGELOG.md to include hashicorp#3986 * storage: always nil check resource group before deref (hashicorp#3986) * v1.32.1 * Cleanup after v1.32.1 release * Added LogAnalyticsDestinationType * Updated the docs * Added the attribute to the doc * Added the value to the error string * New Resource azurerm_dev_test_lab_schedule - Fix for issue 2334 (hashicorp#3554) This pull request is to fix issue hashicorp#2334, It has changes to add new resource provider **azurerm_dev_test_lab_schedule**. **Additional Points** * Resource provider has been created based on [microsoft.devtestlab/2016-05-15/schedules](https://docs.microsoft.com/en-us/azure/templates/microsoft.devtestlab/2016-05-15/schedules) (fixes hashicorp#2334) * Update CHANGELOG.md to include hashicorp#3554 * Bug Fix: `azurerm_app_service_plan` - workaround for missing error on 404 hashicorp#3990 * Update CHANGELOG.md * Fixing typo (hashicorp#3992) * New Feature: `azrurerm_iot_dps` - add support for `linked_hub` (hashicorp#3922) * Update CHANGELOG.md * azurerm_postgresql_server - remove unsupported version `10.2` (hashicorp#3915) Closes hashicorp#3327 * Update CHANGELOG.md to include hashicorp#3915 * Switch more client registrations to new format (hashicorp#3895) * Added property read and simplified validation * Added check that the log analytics workspace id is provided * Added log_analytics_destination_type to the logAnalyticsWorkspace test * Data source for azurerm_dev_test_virtual_network (hashicorp#3746) * Update CHANGELOG.md to include hashicorp#3554 * azurerm_function_app - support for cors (hashicorp#3949) * Update CHANGELOG.md to include hashicorp#3949 * azurerm_role_definition - enture role_definition_id is correctl… (hashicorp#3913) We encountered the issue where role_definition_id remained nil after create, returning 409s on RoleDefinitionsClient.CreateOrUpdate with code RoleDefinitionWithSameNameExists. Updating role definition resource was failing because resourceArmRoleDefinitionCreateUpdate checks if role_definition_id is nil and, if so, creates a new uuid and calls client.CreateOrUpdate which attempts to create a new resource instead of updating. * Update CHANGELOG.md to include hashicorp#3913 * Updated api_management_api_policy for XML file explanation (hashicorp#3994) Added a note for those seeking to utilize -PolicyFilePath from docs.microsoft.com/en-us/powershell/module/azurerm.apimanagement/set-azurermapimanagementpolicy?view=azurermps-6.13.0#parameters Tested in Azure, this works and would prevent work on implementing PolicyFilePath as another parameter. * `azurerm_traffic_manager_profile` - support for the `interval_… (hashicorp#3473) * Update CHANGELOG.md to include hashicorp#3473 * Migrate more clients to new registration pattern (hashicorp#3999) * azurerm_batch_certificate - fix thumbprint casing in tests (hashicorp#3977) * update CHANGELOG.md to include hashicorp#3977 * Update azurerm/resource_arm_cosmosdb_account.go * Update azurerm/resource_arm_cosmosdb_account.go * quote echo * update echo * change GOGC to 15 * Support for registering Data Sources and Resources from packages * tags: refactoring out to a new package ``` $ go test -v ./azurerm/internal/tags/ === RUN TestExpand --- PASS: TestExpand (0.00s) === RUN TestFilter --- PASS: TestFilter (0.00s) === RUN TestFlatten --- PASS: TestFlatten (0.00s) flatten_test.go:46: [DEBUG] Test "Empty" flatten_test.go:46: [DEBUG] Test "One Item" flatten_test.go:46: [DEBUG] Test "Multiple Items" === RUN TestValidateMaximumNumberOfTags --- PASS: TestValidateMaximumNumberOfTags (0.00s) === RUN TestValidateTagMaxKeyLength --- PASS: TestValidateTagMaxKeyLength (0.00s) === RUN TestValidateTagMaxValueLength --- PASS: TestValidateTagMaxValueLength (0.00s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags 0.021s * tags: moving the existing methods to deprecated.go * refactor: moving the top level methods * refactor: moving the networking specific method out * azurerm_analysis_services_server - correct import documentation (server -> servers) (hashicorp#4005) * change GOGC to 20 * Moved test changes to new test TestAccAzureRMMonitorDiagnosticSetting_logAnalyticsWorkspaceDedicated. * Update storage_account.html.markdown * azurerm_container_group - allow empty log_type This is not documented anywhere, as far as I can tell, but if you leave out `logType` when creating the container, you get the behavior of both `ContainerInsights` and `ContainerInstanceLogs` at once. If `logType` is not set, metadata can't be set either. * refactor: moving the validation functions into the validate package * refactor: moving into the deprecated.go * f/service-registration: updating the checklist needed to refactor into packages * azurem_container_group - add a test for unset log_type * fixing the linting * fixing the linting * refactor: moving locks into the internal package * Fix typo in private_dns_a_record docs * provider: add the disable_correlation_request_id option and red… (hashicorp#4023) * resolve conflicts * Add resource_arm_private_dns_cname_record * updating to include hashicorp#4024 * Fix review comments Co-Authored-By: Tom Harvey <tombuildsstuff@users.noreply.github.com> * Fix lint issue * dns_zone: deprecatiog the `zone_type` field * updating to include hashicorp#4028 * Update postgresql_server.html.markdown Clarified what geo_redundant_backup means. Source: https://docs.microsoft.com/en-us/azure/postgresql/concepts-backup * updating to include hashicorp#4033 * r/container_group: polling to ensure the NIC is removed from the Network Profile * removing a todo * provider: ensure test logging has date/time * updating to include hashicorp#3716 * r/storage_account: sorting the docs alphabtically * r/storage_account: defaulting the default_action field to 'Allow' * r/storage_account: moving the 'not applicable' text under the top level block definition * provider: misc golang inspection cleanup (hashicorp#4039) * Updating to include hashicorp#4013 * clarification on which API to use (hashicorp#3946) * go get github.com/hashicorp/terraform@v0.12.6; go mod tidy; go mod vendor; * updating to include hashicorp#4041 * Updating to include hashicorp#4037 * r/monitor_diagnostic_setting: fixing the formatting of the note * updating to include hashicorp#3987 * change graphrabc client registrations to new pattern * change compute client registrations to new pattern * change datalake client registrations to new pattern * change monitor client registrations to new pattern * Update servicebus_subscription_rule.html.markdown (hashicorp#4051) The import example for the subscription filter is incorrect- the resource type is 'azurerm_servicebus_subscription', where it should be 'azurerm_servicebus_subscription_rule' * change network client registrations to new pattern * change resource client registrations to new pattern * change sql client registrations to new pattern * Upped Storage share limit to 100TB; updated docs (hashicorp#4054) * Update CHANGELOG.md to include hashicorp#4054 * I noticed at the top of the page, it still said azurerm_dns_a_record instead of azurerm_private_dns_a_record * Update function_app.html.markdown (hashicorp#4073) * Update function_app.html.markdown I have run into hours of headaches with Azure in particular -- unless some default app settings are specified / seeded within my terraform code. Please include note specified under app settings (and/or something similar). We deploy with Azure DevOps release pipeline, and without these defaults, we cannot successfully deploy to a terraformed function app. * Update function_app.html.markdown * provider: increase linter deadline to 10m (hashicorp#4077) * New Resource: azurerm_mariadb_configuration (hashicorp#4060) * Update to include hashicorp#4060 * [azurerm_role_assignment] Add support for Management Groups (hashicorp#4063) Solves hashicorp#4040 * update CHANGELOG.md to include hashicorp#4063 * [resource_arm_function_app] Add missing vnet_name property (hashicorp#4078) * Update CHANGELOG.md to include hashicorp#4078
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks! |
Refer to #3441 now changing the behaviour of backup_policy_id so it should not trigger recreation of resource when value changed
(fixes #3441)