-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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_backup_policy_file_share add weekly, monthly, and yearly retention #10733
Conversation
@WodansSon , could this be a candidate for v2.49.0 ? |
@tombuildsstuff , would you be able to review this PR as this functionality is needed for a project I'm working on at the moment? |
@jackofallops or @katbyte , would you be able to review and assign a milestone? |
Can someone review? |
@manicminer would you be able to review this PR? I'm desperately trying to find someone who can review it.. |
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 PR @IanMoroney - overall it looks good but i see a couple test failures:
------- Stdout: -------
=== RUN TestAccBackupProtectionPolicyFileShare_updateDaily
=== PAUSE TestAccBackupProtectionPolicyFileShare_updateDaily
=== CONT TestAccBackupProtectionPolicyFileShare_updateDaily
testing.go:620: Step 2/3 error: Check failed: 1 error occurred:
* Check 6/13 error: azurerm_backup_policy_file_share.test: Attribute 'retention_weekly.0.weekdays.#' expected "2", got "3"
--- FAIL: TestAccBackupProtectionPolicyFileShare_updateDaily (188.60s)
FAIL
------- Stderr: -------
2021/03/30 23:03:26 [DEBUG] not using binary driver name, it's no longer needed
2021/03/30 23:03:26 [DEBUG] not using binary driver name, it's no longer needed
------- Stdout: -------
=== RUN TestAccBackupProtectionPolicyFileShare_updateDailyToPartial
=== PAUSE TestAccBackupProtectionPolicyFileShare_updateDailyToPartial
=== CONT TestAccBackupProtectionPolicyFileShare_updateDailyToPartial
testing.go:620: Step 3/4 error: Error running pre-apply refresh:
Error: "retention_daily": required field is not set
on config428031155/terraform_plugin_test.tf line 22, in resource "azurerm_backup_policy_file_share" "test":
22: resource "azurerm_backup_policy_file_share" "test" {
testing_new.go:21: WARNING: destroy failed, so remote objects may still exist and be subject to billing
testing_new.go:21: failed to destroy:
Error: "retention_daily": required field is not set
on config428031155/terraform_plugin_test.tf line 22, in resource "azurerm_backup_policy_file_share" "test":
22: resource "azurerm_backup_policy_file_share" "test" {
--- FAIL: TestAccBackupProtectionPolicyFileShare_updateDailyToPartial (88.01s)
FAIL
------- Stderr: -------
2021/03/30 23:03:26 [DEBUG] not using binary driver name, it's no longer needed
could we ensure each timescale is represented in tests? daily, monthly, yearly, then going from daily->monthly->yearly->daily,
thanks
hi @katbyte , test have been corrected and should pass 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.
@IanMoroney Thanks for the updates, this is looking good and the tests are passing. Could you add a test with 4 steps plus imports as per @katbyte's review, with configuration starting at daily, then changing to monthly, yearly and back to daily? Once that's in and passing this should be good to merge, thanks!
As discussed, I added more tests.
|
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 @IanMoroney - LGTM 👍
…ntion (hashicorp#10733) Co-authored-by: ian.moroney <ian.moroney@demica.com> Fixes hashicorp#10732 Features Adds weekly, monthly, and yearly retention Fixes validation and enforces Azure maximums for daily, weekly, monthly, and annual retention values Aligns code to match format of azurerm_backup_policy_vm Added tests for new scenarios Updated documentation
This has been released in version 2.57.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 = "~> 2.57.0"
}
# ... other configuration ... |
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. |
Fixes #10732
Features
azurerm_backup_policy_vm
Potential Terraform Configuration
Test Results