Azure Site Recovery new cmdlet to support Azure to Azure (A2A) replication #5533

viverm · 2018-02-14T17:55:58Z

Description

https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/2

This checklist is used to make sure that common guidelines for a pull request are followed. You can find a more complete discussion of PowerShell cmdlet best practices here.

I have read the contribution guidelines.
If changes were made to any cmdlet, the XML help was regenerated using the platyPSHelp module.
If any large changes are made to a service, they are reflected in the respective change log.

General Guidelines

Title of the pull request is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.
The pull request does not introduce breaking changes (unless a major version change occurs in the assembly and module).

Testing Guidelines

Pull request includes test coverage for the included changes.
PowerShell scripts used in tests should do any necessary setup as part of the test or suite setup, and should not use hard-coded values for locations or existing resources.

Cmdlet Signature Guidelines

New cmdlets that make changes or have side effects should implement ShouldProcess and have SupportShouldProcess=true specified in the cmdlet attribute. You can find more information on ShouldProcess here.
Cmdlet specifies OutputType attribute if any output is produced - if the cmdlet produces no output, it should implement a PassThru parameter.

Cmdlet Parameter Guidelines

Parameter types should not expose types from the management library - complex parameter types should be defined in the module.
Complex parameter types are discouraged - a parameter type should be simple types as often as possible. If complex types are used, they should be shallow and easily creatable from a constructor or another cmdlet.
Cmdlet parameter sets should be mutually exclusive - each parameter set must have at least one mandatory parameter not in other parameter sets.

MiYanni · 2018-02-14T21:21:54Z

...y/Commands.RecoveryServices.SiteRecovery/help/Edit-AzureRmRecoveryServicesAsrRecoveryPlan.md

@@ -97,13 +97,26 @@ Accept pipeline input: False
 Accept wildcard characters: False
 ```

+### -DefaultProfile
+The credentials, account, tenant, and subscription used for communication with Azure.```yaml


@viverm There is a platyPS bug that adds these ```yaml endings. You'll need to move each down to a new line for the build to succeed.

viverm · 2018-02-15T14:11:47Z

Changes done . @MiYanni Please check.

MiYanni · 2018-02-15T21:43:41Z

@azuresdkci Retest this please

viverm · 2018-02-20T12:24:08Z

waiting for PR review

maddieclayton · 2018-02-20T19:40:54Z

src/ResourceManager/RecoveryServices.SiteRecovery/AzureRM.RecoveryServices.SiteRecovery.psd1

               'New-AzureRmRecoveryServicesAsrProtectionContainerMapping',
               'New-AzureRmRecoveryServicesAsrRecoveryPlan',
               'New-AzureRmRecoveryServicesAsrReplicationProtectedItem',
               'New-AzureRmRecoveryServicesAsrStorageClassificationMapping',
               'New-AzureRmRecoveryServicesAsrvCenter',
+               'New-AzureToAzureDiskReplicationConfiguration',


This needs at least an "AzureRm" prefix

maddieclayton · 2018-02-20T19:47:31Z

...covery/Commands.RecoveryServices.SiteRecovery.Test/ScenarioTests/A2A/AsrA2A.VaultCredentials

@@ -0,0 +1,14 @@
+<ASRVaultCreds xmlns="http://schemas.datacontract.org/2004/07/Microsoft.Azure.Portal.RecoveryServices.Models.Common" xmlns:i="http://www.w3.org/2001/XMLSchema-instance">


Is it a security risk to have this in our public repo?

No. ManagementCert is needed to access vault which is already removed from the file .

maddieclayton · 2018-02-20T19:52:24Z

...s.SiteRecovery/Commands.RecoveryServices.SiteRecovery.Test/ScenarioTests/A2A/AsrA2ATests.ps1

+########################## Site Recovery Tests #############################
+
+##Default Value ##
+$vaultName = "A2APowershellTest"


Please use getAssetName for generating resource names and Get-Location for getting valid locations.

maddieclayton · 2018-02-20T20:36:35Z

...s.SiteRecovery/Commands.RecoveryServices.SiteRecovery.Test/ScenarioTests/A2A/AsrA2ATests.ps1

+$primaryFabricName = "a2aPrimaryFabric3"
+$recoveryFabricName = "a2aRecoveryFabric3"
+
+$PrimaryAzureNetworkId = "/subscriptions/7c943c1b-5122-4097-90c8-861411bdd574/resourceGroups/powershellTest/providers/Microsoft.Network/virtualNetworks/powershellTest-vnet"


Anyone should be able to rerecord this test, which will not be possible if you are using hard coded subscription ids. You should build up and break down the test infrastructure each time the test is recorded rather than using existing infrastructure that people might not have access to.

Azure Site Recovery works on existing resources .And it needs N number of preexisting resources setup for recovery . For other ASR scenerio rerecording thing will not be easy as Enterprise infra will be non-existing at time of reRecording. ASR cmdlet runs in context of vault and subscription which already is imported from vaultCredentials file. If someone is rerecording he should have prior knowledge of creating infra to rerun and he can pass infra details at starting of the Test PS file.
If we put the logic of infra creation in test it will be pretty complex.

Can discuss if there is easy to make this independent.

All of our tests need to be rerecordable by people outside of your team, so having hardcoded resource ids with a subscription that most people cannot access will not work. Is there a reason you can't create the resources and infrastructure at test time? We can have a meeting about this if necessary.

maddieclayton · 2018-02-20T20:42:18Z

...s.SiteRecovery/Commands.RecoveryServices.SiteRecovery.Test/ScenarioTests/A2A/AsrA2ATests.ps1

+                [Microsoft.Azure.Test.TestUtilities]::Wait($JobQueryWaitTimeInSeconds * 1000)
+            }else
+            {
+                if( !(($job.State -eq "Succeeded") -or ($job.State -eq "CompletedWithInformation")))


Should it not be !($job.State -eq "CompletedWithInformation")?

There are two successful condition Succeeded and CompletedWithInformation . If the value is something else it should throw the error.

maddieclayton · 2018-02-20T22:21:38Z

.../Commands.RecoveryServices.SiteRecovery/help/New-AzureRmRecoveryServicesAsrNetworkMapping.md

@@ -19,6 +19,13 @@ New-AzureRmRecoveryServicesAsrNetworkMapping -Name <String> -PrimaryNetwork <ASR
 [<CommonParameters>]
 ```

+### AzureToAzure


Please add an example for this new parameter set

maddieclayton · 2018-02-20T22:21:49Z

...Recovery/Commands.RecoveryServices.SiteRecovery/help/New-AzureRmRecoveryServicesAsrPolicy.md

@@ -37,6 +37,13 @@ New-AzureRmRecoveryServicesAsrPolicy [-AzureToVMware] -Name <String> -RecoveryPo
 [<CommonParameters>]
 ```

+### AzureToAzure


Please add an example for this new parameter set

maddieclayton · 2018-02-20T22:22:44Z

...RecoveryServices.SiteRecovery/help/New-AzureRmRecoveryServicesAsrReplicationProtectedItem.md

@@ -47,6 +47,15 @@ New-AzureRmRecoveryServicesAsrReplicationProtectedItem [-HyperVToAzure] -Protect
 [-Confirm] [<CommonParameters>]
 ```

+### AzureToAzure


Please add an example for this new parameter set

maddieclayton · 2018-02-20T22:27:44Z

...overy/Commands.RecoveryServices.SiteRecovery/help/Update-AzureRmRecoveryServicesAsrPolicy.md

@@ -30,6 +30,14 @@ Update-AzureRmRecoveryServicesAsrPolicy [-VMwareToAzure] -InputObject <ASRPolicy
 [-DefaultProfile <IAzureContextContainer>] [-WhatIf] [-Confirm] [<CommonParameters>]
 ```

+### AzureToAzure


Please add an example for this new parameter set

maddieclayton · 2018-02-20T22:28:09Z

...s.RecoveryServices.SiteRecovery/help/Update-AzureRmRecoveryServicesAsrProtectionDirection.md

@@ -51,6 +51,25 @@ Update-AzureRmRecoveryServicesAsrProtectionDirection [-VmmToVmm]
 [-DefaultProfile <IAzureContextContainer>] [-WhatIf] [-Confirm] [<CommonParameters>]
 ```

+### AzureToAzure


Please add an example for these new parameter sets

maddieclayton · 2018-02-20T22:29:53Z

@viverm Sorry for the late review. Some comments for you to look at, I'll look again in more depth when the big issues are fixed. Please reassign to me when you have finished looking at these comments.

…nto a2apowershell_preview

viverm · 2018-02-24T08:31:31Z

@maddieclayton Still fix for GetAssetName and hardcoding in test needs to be disscussed.

…nto a2apowershell_preview

viverm · 2018-03-16T22:06:28Z

@cormacpayne done with changes

scheduled the build https://azuresdkci.westus2.cloudapp.azure.com/job/powershell-demand/626/console

viverm · 2018-03-19T17:46:27Z

@cormacpayne Few formatting changes in help files when generated with platyps 2.9.
Check the PR .

cormacpayne · 2018-03-19T18:30:46Z

...RecoveryServices.SiteRecovery/help/New-AzureRmRecoveryServicesAsrReplicationProtectedItem.md

+### Example 3
+```
+PS C:\>$job = New-AzureRmRecoveryServicesAsrReplicationProtectedItem -AzureToAzure -AzureToAzureDiskReplicationConfig disk1,disk2 -AzureVmId $vmId `
+ -Name <String> -RecoveryVmName "vmName" -ProtectionContainerMapping $pcmMapping `


@viverm nit: change <String>

PS C:>$job = New-AzureRmRecoveryServicesAsrReplicationProtectedItem -AzureToAzure -AzureToAzureDiskReplicationConfig disk1,disk2 -AzureVmId $vmId -Name "a2aprotectedItem" -RecoveryVmName "vmName" -ProtectionContainerMapping $pcmMapping
-RecoveryResourceGroupId $recoveryResourceGroup
corrected

cormacpayne · 2018-03-19T18:31:06Z

...RecoveryServices.SiteRecovery/help/New-AzureRmRecoveryServicesAsrReplicationProtectedItem.md

+```
+PS C:\> $disk1 = new-AzureToAzureDiskReplicationConfiguration -vhdUri  $diskUri1 -RecoveryAzureStorageAccountId $recoveryAzureStorageAccountId -LogStorageAccountId $logStorageAccountId  
+PS C:\> $disk2 = new-AzureToAzureDiskReplicationConfiguration -vhdUri  $diskUri2 -RecoveryAzureStorageAccountId $recoveryAzureStorageAccountId -LogStorageAccountId $logStorageAccountId  
+PS C:\> $enableDRjob = New-AzureRmRecoveryServicesAsrReplicationProtectedItem -AzureToAzure -AzureVmId $vmId -Name $rpiName -RecoveryCloudServiceId  $recoveryCloudServiceId -ProtectionContainerMapping $pcm -RecoveryResourceGroupId  $RecoveryResourceGroupId -AzureToAzureDiskReplicationConfiguration $disk1,$disk2


@viverm same comment here about trailing backtick

@cormacpayne these individual commands are in one line that why there is no backtick.

For the readability purpose breaking the in multiple line with backtick.

viverm · 2018-03-20T11:49:12Z

@cormacpayne I am not able to find generated msi file to run and verify the things manually.

https://azuresdkci.westus2.cloudapp.azure.com/job/powershell-demand/638/

How can I generate/find it?

viverm · 2018-03-20T13:04:25Z

@cormacpayne Build is failing after merge . Can you check.

cormacpayne · 2018-03-20T15:55:04Z

You can find the generated msi in the following sign job: https://azuresdkci.westus2.cloudapp.azure.com/job/ps-sign/267/

cormacpayne · 2018-03-20T15:58:19Z

...eManager/RecoveryServices/Commands.RecoveryServices/Vault/NewAzureRmRecoveryServicesVault.cs

+    /// </summary>
+    [Cmdlet(VerbsCommon.New, "AzureRmRecoveryServicesVault", SupportsShouldProcess = true),
+        OutputType(typeof(ARSVault))]
+    public class NewAzureRmRecoveryServicesVault : RecoveryServicesCmdletBase


@viverm ping on this comment. This file should be removed.

cormacpayne · 2018-03-20T16:07:24Z

...ry/Commands.RecoveryServices.SiteRecovery/Vault/SetAzureRmRecoveryServicesAsrVaultContext.cs

@@ -76,16 +72,24 @@ public override void ExecuteSiteRecoveryCmdlet()
            {
                using (var powerShell = System.Management.Automation.PowerShell.Create())


@viverm this powershell object isn't used anymore, so we should get rid of the using that instantiates it

File name changed from NewAzureRMRecoveryServicesVault.cs to NewAzureRmRecoveryServicesVault.cs reindex ed it.

….cs with older case

cormacpayne · 2018-03-21T15:51:39Z

On-demand: https://azuresdkci.westus2.cloudapp.azure.com/job/powershell-demand/645/

Approved offline

Azure to Azure Powershell

742e8c0

azuresdkci added the needs-review label Feb 14, 2018

azuresdkci assigned cormacpayne Feb 14, 2018

maddieclayton changed the title ~~Azure to Azure Powershell~~ Azure Site Recovery new cmdlet to support Azure to Azure (A2A) replication Feb 14, 2018

MiYanni reviewed Feb 14, 2018

View reviewed changes

MiYanni added needs-revision and removed needs-review labels Feb 14, 2018

MiYanni assigned viverm and unassigned cormacpayne Feb 14, 2018

maddieclayton added needs-review and removed needs-revision labels Feb 15, 2018

maddieclayton unassigned viverm Feb 15, 2018

MiYanni assigned maddieclayton and cormacpayne Feb 20, 2018

maddieclayton suggested changes Feb 20, 2018

View reviewed changes

maddieclayton assigned viverm and unassigned cormacpayne and maddieclayton Feb 20, 2018

maddieclayton added needs-revision and removed needs-review labels Feb 20, 2018

Merge branch 'preview' of https://github.com/Azure/azure-powershell i…

1fda474

…nto a2apowershell_preview

viverm added needs-review and removed needs-revision labels Feb 24, 2018

viverm assigned maddieclayton Feb 24, 2018

Merge branch 'preview' of https://github.com/Azure/azure-powershell i…

838309d

…nto a2apowershell_preview

viverm removed the needs-revision label Mar 16, 2018

viverm removed their assignment Mar 16, 2018

cormacpayne reviewed Mar 19, 2018

View reviewed changes

PR comments

cca0e0b

cormacpayne suggested changes Mar 20, 2018

View reviewed changes

cormacpayne added needs-revision and removed needs-review labels Mar 20, 2018

cormacpayne assigned viverm and unassigned cormacpayne Mar 20, 2018

removing using powershell and reindex NewAzureRMRecoveryServicesVault…

7cad909

….cs with older case

viverm added needs-review and removed needs-revision labels Mar 20, 2018

viverm assigned cormacpayne Mar 20, 2018

cormacpayne changed the base branch from preview to release-2018-03-23 March 21, 2018 15:44

cormacpayne unassigned cormacpayne and viverm Mar 21, 2018

cormacpayne added Waiting for On Demand and removed needs-review labels Mar 21, 2018

maddieclayton approved these changes Mar 21, 2018

View reviewed changes

cormacpayne approved these changes Mar 21, 2018

View reviewed changes

cormacpayne merged commit d841703 into Azure:release-2018-03-23 Mar 21, 2018

viverm deleted the a2apowershell_preview branch April 17, 2018 15:52

		@@ -0,0 +1,14 @@
		<ASRVaultCreds xmlns="http://schemas.datacontract.org/2004/07/Microsoft.Azure.Portal.RecoveryServices.Models.Common" xmlns:i="http://www.w3.org/2001/XMLSchema-instance">

		@@ -76,16 +72,24 @@ public override void ExecuteSiteRecoveryCmdlet()
		{
		using (var powerShell = System.Management.Automation.PowerShell.Create())

Azure Site Recovery new cmdlet to support Azure to Azure (A2A) replication #5533

Azure Site Recovery new cmdlet to support Azure to Azure (A2A) replication #5533

Uh oh!

Conversation

viverm commented Feb 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

MiYanni Feb 14, 2018 • edited by maddieclayton Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

viverm commented Feb 15, 2018

Uh oh!

MiYanni commented Feb 15, 2018

Uh oh!

viverm commented Feb 20, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

viverm Feb 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maddieclayton commented Feb 20, 2018

Uh oh!

viverm commented Feb 24, 2018

Uh oh!

viverm commented Mar 16, 2018

Uh oh!

viverm commented Mar 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

viverm commented Mar 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

viverm commented Mar 20, 2018

viverm commented Feb 14, 2018 •

edited

Loading

MiYanni Feb 14, 2018 •

edited by maddieclayton

Loading

viverm Feb 22, 2018 •

edited

Loading

viverm commented Mar 20, 2018 •

edited

Loading