Skip to content
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

New Resource: azurerm_site_recovery_replication_recovery_plan;New Data Source: azurerm_site_recovery_replication_recovery_plan #19940

Merged
merged 20 commits into from
Jan 24, 2023

Conversation

ziyeqf
Copy link
Contributor

@ziyeqf ziyeqf commented Jan 10, 2023

Test

TF_ACC=1 go test -parallel 10 -v ./internal/services/recoveryservices -run=RecoveryPlan -timeout '720m' -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccSiteRecoveryReplicationRecoveryPlan_basic
=== PAUSE TestAccSiteRecoveryReplicationRecoveryPlan_basic
=== RUN   TestAccSiteRecoveryReplicationRecoveryPlan_withPreActions
=== PAUSE TestAccSiteRecoveryReplicationRecoveryPlan_withPreActions
=== RUN   TestAccSiteRecoveryReplicationRecoveryPlan_withPostActions
=== PAUSE TestAccSiteRecoveryReplicationRecoveryPlan_withPostActions
=== CONT  TestAccSiteRecoveryReplicationRecoveryPlan_basic
=== CONT  TestAccSiteRecoveryReplicationRecoveryPlan_withPostActions
=== CONT  TestAccSiteRecoveryReplicationRecoveryPlan_withPreActions
--- PASS: TestAccSiteRecoveryReplicationRecoveryPlan_withPostActions (2870.90s)
--- PASS: TestAccSiteRecoveryReplicationRecoveryPlan_basic (3267.83s)
--- PASS: TestAccSiteRecoveryReplicationRecoveryPlan_withPreActions (4353.15s)
PASS
ok  	github.com/hashicorp/terraform-provider-azurerm/internal/services/recoveryservices

data source

 TF_ACC=1 go test -v ./internal/services/recoveryservices -run=TestAccDataSourceSiteRecoveryReplicationRecoveryPlan_basic -timeout=600m
=== RUN   TestAccDataSourceSiteRecoveryReplicationRecoveryPlan_basic
=== PAUSE TestAccDataSourceSiteRecoveryReplicationRecoveryPlan_basic
=== CONT  TestAccDataSourceSiteRecoveryReplicationRecoveryPlan_basic
--- PASS: TestAccDataSourceSiteRecoveryReplicationRecoveryPlan_basic (2746.93s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/recoveryservices      2748.085s

@ziyeqf ziyeqf changed the title Site Recovery - add new resource azurerm_site_recovery_replication_recovery_plan New Resource: azurerm_site_recovery_replication_recovery_plan Jan 10, 2023
@ziyeqf ziyeqf changed the title New Resource: azurerm_site_recovery_replication_recovery_plan New Resource: azurerm_site_recovery_replication_recovery_plan, New Data Source: azurerm_site_recovery_replication_recovery_plan Jan 10, 2023
@ziyeqf
Copy link
Contributor Author

ziyeqf commented Jan 11, 2023

close and will reopen when the test finish

@ziyeqf ziyeqf closed this Jan 11, 2023
@ziyeqf ziyeqf reopened this Jan 12, 2023
@ziyeqf ziyeqf closed this Jan 12, 2023
@ziyeqf ziyeqf reopened this Jan 12, 2023
@ziyeqf ziyeqf marked this pull request as ready for review January 12, 2023 08:42
@ziyeqf ziyeqf changed the title New Resource: azurerm_site_recovery_replication_recovery_plan, New Data Source: azurerm_site_recovery_replication_recovery_plan New Resource: azurerm_site_recovery_replication_recovery_plan;New Data Source: azurerm_site_recovery_replication_recovery_plan Jan 12, 2023

* `target_recovery_fabric_id` - (Required) ID of target fabric to recover. Changing this forces a new Replication Plan to be created.

* `recovery_group` - (Required) Three or more `recovery_group` block. At least one `recovery_group` for every `group_type` needed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite understand the second sentence. group_type is one element of recovery_group, so each recovery_group has a group_type ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, and there should be at least three recovery_group Maybe we can make recovery_group a computed one to avoid this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it be computed? Isn't recovery_group a configurable property?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember there is some trouble as it's typeSet.. if I set it to computed there might be a diff, let me give it a try.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default hash for Set will ignore the computed properties. The problem is: if this property is configurable by users, it should not be set as computed?

Copy link
Contributor Author

@ziyeqf ziyeqf Jan 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's configurable and if the users do not configure it, it will also be generated by service.
My problem is the block, I want to set it to computed then the users have no need to add blocks they do not use in their config file.
while in this situation, it get response with 3 recovery_group, but only one in the config file, then there will be a diff.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Souds like a backend problem. Normally, the backend should not change the configuration sent by the frontend.

remove `vault_name` & `resource_group_name` and use `recovery_vault_id`
@ziyeqf
Copy link
Contributor Author

ziyeqf commented Jan 13, 2023

convert to draft to re run tests

@ziyeqf ziyeqf marked this pull request as draft January 13, 2023 09:32
@ziyeqf
Copy link
Contributor Author

ziyeqf commented Jan 13, 2023

new test result

=== CONT  TestAccSiteRecoveryReplicationRecoveryPlan_withPreActions
=== CONT  TestAccSiteRecoveryReplicationRecoveryPlan_basic
=== CONT  TestAccSiteRecoveryReplicationRecoveryPlan_withPostActions
--- PASS: TestAccDataSourceSiteRecoveryReplicationRecoveryPlan_basic (2679.47s)
--- PASS: TestAccSiteRecoveryReplicationRecoveryPlan_withPostActions (2679.93s)
--- PASS: TestAccSiteRecoveryReplicationRecoveryPlan_basic (2680.92s)
--- PASS: TestAccSiteRecoveryReplicationRecoveryPlan_withPreActions (2864.01s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/recoveryservices      2865.193s

@ziyeqf ziyeqf marked this pull request as ready for review January 13, 2023 14:45
@khouloudbo
Copy link

Hello, the azurerm recovery plan still not supported in the new version (3.39.1) ?

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 💾

@katbyte katbyte merged commit 34b8046 into hashicorp:main Jan 24, 2023
@github-actions github-actions bot added this to the v3.41.0 milestone Jan 24, 2023
katbyte added a commit that referenced this pull request Jan 24, 2023
@ziyeqf ziyeqf deleted the tengzh/recovery/epic_output/plan branch January 24, 2023 04:04
@github-actions
Copy link

This functionality has been released in v3.41.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!

@github-actions
Copy link

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants