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

Proposal: Better distinguish Site Recovery and Azure Backup resource names #5089

Closed
sean-nixon opened this issue Dec 6, 2019 · 5 comments · Fixed by #5170
Closed

Proposal: Better distinguish Site Recovery and Azure Backup resource names #5089

sean-nixon opened this issue Dec 6, 2019 · 5 comments · Fixed by #5170

Comments

@sean-nixon
Copy link
Contributor

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

Azure Recovery Services is used for both backups (Azure Backup) and VM replication (Azure Site Recovery or ASR). They share similar concepts and terminology (e.g. policies, protected items, protection containers, fabrics) and both use the same recovery services vaults; however, they do not share resources other than the underlying vault. They have two relatively distinct APIs, have different packages in the azure-go-sdk, and have distinct sections in the Azure API docs.

Currently, the resources created so far in this provider are not clear in distinguishing between the two which causes issues with naming resources which causes potential issues as well as confusion.

For example, azurerm_recovery_services_protection_container creates a Site Recovery replication protection container (https://docs.microsoft.com/en-us/rest/api/site-recovery/replicationprotectioncontainers). Azure Backups also has a protection container that is used for registering storage accounts with the recovery vault for Azure File backup (https://docs.microsoft.com/en-us/rest/api/backup/protectioncontainers) among other purposes. A new resource for this would be required to implement #5026. We could use azurerm_recovery_services_backup_protection_container for this hypothetical new resource, but that's not consistent with any of the other backup resources, is extremely long, and is IMHO not very intuitive.

I'm proposing that for clarity, consistency, and easier extensibility, recovery services resource names be updated with clear name spacing. I'm open to other suggestions, but my opinion is to make all Azure Site Recovery resources use the site_recovery prefix while Azure Backup resources would use the backup prefix. This is of course a breaking change, so I'd like it to be considered for the 2.0 release.

I'm of course open for feedback and alternative approaches.

New or Affected Resource(s)

I'm proposing the following resource name changes:

Old Resource Name New Resource Name
azurerm_recovery_services_vault azurerm_recovery_services_vault (no change)
azurerm_recovery_services_fabric azurerm_site_recovery_fabric
azurerm_recovery_network_mapping azurerm_site_recovery_network_mapping
azurerm_recovery_services_protection_container azurerm_site_recovery_protection_container
azurerm_recovery_services_protection_container_mapping azurerm_site_recovery_protection_container_mapping
azurerm_recovery_services_replication_policy azurerm_site_recovery_protection_policy
azurerm_recovery_replicated_vm azurerm_site_recovery_replicated_vm
azurerm_recovery_services_protection_policy_vm azurerm_backup_policy_vm
azurerm_recovery_services_protected_vm azurerm_backup_protected_vm

References

Azure Backup API docs: https://docs.microsoft.com/en-us/rest/api/backup/
Azure Site Recovery API docs: https://docs.microsoft.com/en-us/rest/api/site-recovery/

@katbyte
Copy link
Collaborator

katbyte commented Dec 8, 2019

Hi @sean-nixon,

This sounds like a great idea to me! And we would be happy to accept a PR that deprecates the old resources and adds them with the new names. We probably won't have time to do this ourselves for 2.0.

I do wonder if it would make sense to continue to include recovery in the names as backup and site recovery are a subset of recovery services:

Old Resource Name New Resource Name
azurerm_recovery_services_vault azurerm_recovery_services_vault (no change)
azurerm_recovery_services_fabric azurerm_recovery_site_fabric
azurerm_recovery_network_mapping azurerm_recovery_site_network_mapping
azurerm_recovery_services_protection_policy_vm azurerm_recovery_backup_policy_vm
azurerm_recovery_services_protected_vm azurerm_recovery_backup_protected_vm

?

@sean-nixon
Copy link
Contributor Author

@katbyte Very glad you're onboard with this idea. I will try to get a PR prepared for this but it may take a while due to other obligations on my end.

You do make a valid point about them both being a subset of recovery services. My main concern about the naming you suggested is that it sounds like we're creating a "recovery site" fabric, etc. which is not quick accurate. I'd argue that while both services are technically under the "Recovery Services" umbrella, both services are independent and usually referred to on their own. For example, we almost always refer to "Azure Backup" not "Recovery Services Azure Backup".

One alternative would be to use the acronym "ASR" (e.g. azurerm_recovery_asr_fabric) which used to be commonly used to refer to Azure Site Recovery. I can't find any references to it in the official Microsoft documentation any more so I think it may have fallen out of favor. I wouldn't be opposed to using it, but personally I think I still prefer the names in my initial proposal.

In any case, I'll work on putting together a PR where there can be further discussion on the final naming convention.

@sean-nixon
Copy link
Contributor Author

@katbyte I submitted a PR for review that implements the new resource names as specified in the initial table with the exception of changing azurerm_backup_policy_vm to azurerm_backup_protection_policy_vm. It also deprecates the old resources and updates (hopefully) all the necessary documentation.

katbyte pushed a commit that referenced this issue Dec 19, 2019
Fixes #5089

This PR implements the proposed changes in the above issue to better distinguish Azure Site Recovery and Azure Backup resources.
@katbyte katbyte added this to the v1.40.0 milestone Dec 19, 2019
katbyte pushed a commit that referenced this issue Dec 19, 2019
Addresses #5026

An additional resource beyond the suggested resources in the associated issue was required in order to first register a storage account with a recovery services vault. I've also used an updated naming scheme based on my proposal in #5089 and associated PR #5170 in order to better distinguish Azure Backup resources from Site Recovery (DR) resources. That PR does not technically block this one, but it would make the naming more consistent with the existing Azure Backup resources.
@ghost
Copy link

ghost commented Jan 8, 2020

This has been released in version 1.40.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.40.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Mar 28, 2020

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 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants