-
Notifications
You must be signed in to change notification settings - Fork 16.1k
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
Create README.md #2479
Create README.md #2479
Conversation
Hi @trinadhk, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!
TTYL, AZPRBOT; |
<img src="http://armviz.io/visualizebutton.png"/> | ||
</a> | ||
|
||
This template will use existing recovery services vault and policy, and enables protection of ARM based IaaSVMs. VM and vault - both must be in same GEO. |
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.
typo... sapce between IaaS and VMs
|
||
"protectedItems": "[concat('vm;iaasvmcontainerv2;', parameters('vmResourceGroupName'), ';', parameters('vmName'))]", | ||
|
||
"protectedItemType": "Microsoft.Compute/virtualMachines", |
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.
Please confirm this, I guess this should be: Microsoft.ClassicCompute/virtualMachines while passing this to "properties". However, for constructing v2 vm's resourceId this is correct.
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.
This is only for Resource Manager VMs
@@ -0,0 +1,16 @@ | |||
# Backup Resource Manager IaaS VMs to Recovery Services Vault |
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.
You may want to specify Backup Resource Manager IaaS VM (instead of IaaS VMs).
May also include another (older) template link to backup multiple iaas vms...
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.
I am planning to add PS script which lets to do at scale protection using this template. So plural .
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.
@trinadhk are you planning a PS script which will take N vms as inputs and then that will call this ARM template in N times? If that's the case, we'll not leverage the benefits of ARM which can create single parent request and all configure protections will happen part of same request. And single configure protection can be tracked as one of the N operations.
|
||
"parameters": { | ||
|
||
"vaultName": { |
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.
I am not sure whether it's general guidelines or not, but if we are expecting this to be already existing, should prefix with "existing", in that case variable names should be existingVaultName, existingVmResourceGroupName, existingVmName etc.
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.
I will check with Kay on this. Thanks for the feedback.
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.
This is good feedback by Nilay. Should be prefixed with "existing"
Can you remove all the double spacing in the files? |
Contributing guide
https://github.com/Azure/azure-quickstart-templates/blob/master/1-CONTRIBUTION-GUIDE/README.md
Changelog
Description of the change