-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
add resource "azurerm_data_protection_backup_instance_blob_storage" #12683
add resource "azurerm_data_protection_backup_instance_blob_storage" #12683
Conversation
a33ff7e
to
a7d7228
Compare
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 @ms-henglu - looks good but we have some tests failing:
------- Stdout: -------
=== RUN TestAccDataProtectionBackupInstanceBlobStorage_basic
=== PAUSE TestAccDataProtectionBackupInstanceBlobStorage_basic
=== CONT TestAccDataProtectionBackupInstanceBlobStorage_basic
testing_new.go:70: Error running post-test destroy, there may be dangling resources: exit status 1
Error: authorization.RoleAssignmentsClient#Delete: Failure responding to request: StatusCode=409 -- Original Error: autorest/azure: Service returned an error. Status=409 Code="ScopeLocked" Message="The scope '/subscriptions/*******/resourceGroups/acctest-dataprotection-210729063221901155/providers/Microsoft.Storage/storageAccounts/acctestsa21072955/providers/Microsoft.Authorization/roleAssignments/9270a77d-c21e-cc6f-65a9-90ae67476f4f' cannot perform delete operation because following scope(s) are locked: '/subscriptions/*******/resourcegroups/acctest-dataprotection-210729063221901155/providers/Microsoft.Storage/storageAccounts/acctestsa21072955'. Please remove the lock and try again."
--- FAIL: TestAccDataProtectionBackupInstanceBlobStorage_basic (236.38s)
FAIL
b41e430
to
9095ecc
Compare
tests are still failing:
|
I think the cause is
But this role is required to provision backup instance for blob storage, otherwise it will fail. Hi @katbyte , is it possible to remove the lock on storage account?
|
I didn't realize this PR was open when I opened #13071. One key element that is different is in the implementation of the acceptance tests. And it relates to the race condition captured in this thread. Since Azure backup creates a To account for this, I intentionally added a The rest of our implementation got to almost the exact same place with only some minor differences (i.e., my acceptance tests use indexed verbs, my error messages use the Hopefully that sheds some light on why you're seeing different results. Race conditions - yay! |
cfdd281
to
6e90f73
Compare
6e90f73
to
bce072f
Compare
@owenfarrell , Hi! Thank you so much for the help!! It did fix the tests. |
@katbyte , Hi, I've fixed the tests, please take another look, thanks! |
@ms-henglu Glad I could help! On a related note, I think the trend of this provider is to use indexed verbs in the acceptance test format strings to avoid unnecessary repetition in the parameters (i.e., multiple The other emerging trend is to use the I can't take credit for the above guidance. I'm just relaying feedback that I previously received and have incorporated in to other changes. And I totally understand how you got to where you did - I used the PostgreSQL backup instance as a starting point, too. |
@katbyte , could you please take another look? All tests passed. |
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 @ms-henglu - LGTM 🌿
This functionality has been released in v2.76.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
The added tests are