TargetVmSize, AvSet, Sql RP and ResourceTagging changes for V2A and H2A#21

Merged

vidyadharijami merged 20 commits intonewApiVersion/asr2021-02-10from

dev/vijami/V2AH2AChangesLatest

Mar 26, 2021

vidyadharijami commented Mar 25, 2021 •

edited

Loading

This PR includes changes for following for both V2A and H2A

TargetVmSize in enable protection,
AvSet in enable protection
Sql License type in enable, update and get APIs
ResourceTagging [VmTags, NicTags and DiskTags] in enable, update and get APIs
Design PRs:
Azure/azure-powershell-cmdlet-review-pr#879
Azure/azure-powershell-cmdlet-review-pr#888

Testing:
H2A is done.
V2A is done.

Added test cases and recordings for V2A and H2A

nilgupta6 and others added 14 commits

March 16, 2021 17:59


          Merge pull request #16 from Azure/master

7a0ee9c

bringing Asr one sdk azure- master in sync with master branch for azure/azure-powershell


          Fixing PFO/TFO/UFO build errors

21a948e


          mulltiple ip per nic - cmdlet changes

f39ee62


          review changes

5451de4


          code review changes

b3d874e


          moved validaton to nicConfig cmdlet and review changes

c03880a


          minor fix

5f80211


          handled review comments

767e166


          parameter validation changes, logs

b3da526


          design review changes

a157e37


          Merge pull request #18 from AsrOneSdk/multipleIPPerNicCmdletChanges

1c3f963

Multiple IP per nic - cmdlet changes


          Merge branch 'newApiVersion/asr2021-02-10' of https://github.com/AsrO…

9e7af56

…neSdk/azure-powershell into newApiVersion/asr2021-02-10


          [ASR][V2A/H2A] TargetVmSize, AvSet, Sql RP and ResourceTagging change…

bf9627a

…s for V2A and H2A


          Minor fix

a776238

vidyadharijami requested review from Arpitaji and anmolbhatia289

March 25, 2021 05:55

Arpitaji reviewed

View reviewed changes

src/RecoveryServices/RecoveryServices/help/New-AzRecoveryServicesAsrReplicationProtectedItem.md Outdated

               New-AzRecoveryServicesAsrReplicationProtectedItem [-VmmToVmm] -ProtectableItem <ASRProtectableItem>
-               -Name <String> -ProtectionContainerMapping <ASRProtectionContainerMapping> [-WaitForCompletion]
-               [-DefaultProfile <IAzureContextContainer>] [-WhatIf] [-Confirm] [<CommonParameters>]
+               -Name <String> -ProtectionContainerMapping <ASRProtectionContainerMapping> [-UseManagedDisk <String>]

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

UseManagedDisk [](start = 78, length = 14)

This is E2E right??? #Resolved

Author

vidyadharijami Mar 25, 2021 •

edited

Loading

Will remove from here. It's by overlook. #Resolved

Arpitaji reviewed

View reviewed changes

src/RecoveryServices/RecoveryServices/help/New-AzRecoveryServicesAsrReplicationProtectedItem.md Outdated

+               [-RecoveryVmTag <System.Collections.Generic.IDictionary`2[System.String,System.String]>]
+               [-DiskTag <System.Collections.Generic.IDictionary`2[System.String,System.String]>]
+               [-RecoveryNicTag <System.Collections.Generic.IDictionary`2[System.String,System.String]>]
+               [-UseManagedDisk <String>] [-WaitForCompletion] -DiskType <String> [-DiskEncryptionSetId <String>]

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

[-UseManagedDisk ] [](start = 1, length = 26)

Do we want to expose in V2A??? The SRS decides on its own right based on Geo
Also in this we have DiskType so this shd be MD only #Resolved

Author

vidyadharijami Mar 25, 2021 •

edited

Loading

In powershell also, we don't expose this for V2A. Will remove it. It has come from auto generation of help file. #Resolved

Arpitaji reviewed

View reviewed changes

src/RecoveryServices/RecoveryServices/help/New-AzRecoveryServicesAsrReplicationProtectedItem.md Outdated

+               [-RecoveryVmTag <System.Collections.Generic.IDictionary`2[System.String,System.String]>]
+               [-DiskTag <System.Collections.Generic.IDictionary`2[System.String,System.String]>]
+               [-RecoveryNicTag <System.Collections.Generic.IDictionary`2[System.String,System.String]>]
+               [-UseManagedDisk <String>] [-WaitForCompletion] [-DiskEncryptionSetId <String>]

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

UseManagedDisk [](start = 3, length = 14)

Please check @anmolbhatia289 #Resolved

Author

vidyadharijami Mar 25, 2021

We don't expose this in V2A. Hence removing this.

In reply to: 601410343 [](ancestors = 601410343)


          Addressed review comments and added new tests and recordings for B2A

55f907e

Arpitaji reviewed

View reviewed changes

src/RecoveryServices/RecoveryServices/help/New-AzRecoveryServicesAsrReplicationProtectedItem.md

               ### EnterpriseToAzure
               ```
               New-AzRecoveryServicesAsrReplicationProtectedItem [-HyperVToAzure] -ProtectableItem <ASRProtectableItem>

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

HyperVToAzure [](start = 52, length = 13)

Actually E2A also we have all right? then why this split #Resolved

Author

vidyadharijami Mar 25, 2021

Closing this as discussed.

In reply to: 601427963 [](ancestors = 601427963)

Arpitaji reviewed

View reviewed changes

src/RecoveryServices/RecoveryServices/help/New-AzRecoveryServicesAsrReplicationProtectedItem.md

+              Type: System.String
+              Parameter Sets: VMwareToAzureWithDiskType, VMwareToAzure, HyperVSiteToAzure
+              Aliases:
+              Accepted values: NoLicenseType, PAYG, AHUB

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

NoLicenseType [](start = 17, length = 13)

I remember 4 values in SRS #Resolved

Author

vidyadharijami Mar 25, 2021 •

edited

Loading

I haven't considered the NotSpecified case because this parameter itself won't be given if the user doesn't want to specify.
Similar to LicenseType which is already present. #Resolved

Author

vidyadharijami Mar 25, 2021

Need to double check with Prachetos on how is NoLicenseType different from NotSpecified.

In reply to: 601548721 [](ancestors = 601548721)

Arpitaji reviewed

View reviewed changes

src/RecoveryServices/RecoveryServices/help/Set-AzRecoveryServicesAsrReplicationProtectedItem.md

+              Type: System.String
+              Parameter Sets: (All)
+              Aliases:
+              Accepted values: NoLicenseType, PAYG, AHUB

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

NoLicenseType [](start = 17, length = 13)

Same as above #Resolved

Author

vidyadharijami Mar 25, 2021

Will cross check on this and will add in next PR if any change required.

In reply to: 601541382 [](ancestors = 601541382)

Arpitaji reviewed

View reviewed changes

src/RecoveryServices/RecoveryServices.SiteRecovery/Models/PSFabricDetails.cs Outdated

                       public string RecoveryProximityPlacementGroupId { get; set; }
+                      /// <summary>
+                      /// Gets or sets the SQL Server license type to the machine to in the event of a failover.

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

machine to [](start = 60, length = 10)

Please correct the description, something like
Gets or sets the SQL Server license type of the machine in the event of a failover. #Resolved

Author

vidyadharijami Mar 25, 2021 •

edited

Loading

Done #Resolved

Author

vidyadharijami Mar 25, 2021

Done

In reply to: 601542526 [](ancestors = 601542526)

Arpitaji reviewed

View reviewed changes

src/RecoveryServices/RecoveryServices.SiteRecovery/Models/PSFabricDetails.cs Outdated

                       public string RecoveryProximityPlacementGroupId { get; set; }
+                      /// <summary>
+                      /// Gets or sets the SQL Server license type to the machine to in the event of a failover.

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

ts the SQL Server license type to the [](start = 22, length = 37)

Same as above #Resolved

Author

vidyadharijami Mar 25, 2021 •

edited

Loading

Done. #Resolved

Arpitaji reviewed

View reviewed changes

...teRecovery/ReplicationProtectedItem/NewAzureRmRecoveryServicesAsrReplicationProtectedItem.cs Outdated

+                      [Parameter(ParameterSetName = ASRParameterSets.HyperVSiteToAzure, HelpMessage = "Specify the SQL Server license type of the VM.")]
+                      [ValidateNotNullOrEmpty]
+                      [ValidateSet(
+                          Constants.NoLicenseType,

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

NoLicenseType [](start = 22, length = 13)

Same as above #Resolved

Author

vidyadharijami Mar 25, 2021

Resolving as discussed.

In reply to: 601544871 [](ancestors = 601544871)

Arpitaji reviewed

View reviewed changes

...teRecovery/ReplicationProtectedItem/NewAzureRmRecoveryServicesAsrReplicationProtectedItem.cs Outdated

+                              SqlServerLicenseType = this.SqlServerLicenseType,
+                              TargetVmTags = this.RecoveryVmTag,
+                              TargetNicTags = this.RecoveryNicTag,
+                              SeedManagedDiskTags = this.DiskTag,

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

SeedManagedDiskTags = this.DiskTag [](start = 16, length = 34)

If RecoveryAzureStorageAccountId is not null or string.empty, then also we shd set this. Otherwise service will throw exception.
@anmolbhatia289 #Resolved

anmolbhatia289 Mar 25, 2021

Yes, that is correct

In reply to: 601546593 [](ancestors = 601546593)

Author

vidyadharijami Mar 25, 2021

Handled in powershell itself.

In reply to: 601546593 [](ancestors = 601546593)

Arpitaji reviewed

View reviewed changes

...teRecovery/ReplicationProtectedItem/NewAzureRmRecoveryServicesAsrReplicationProtectedItem.cs Outdated

+                          providerSettings.TargetVmSize = this.Size;
+                          providerSettings.SqlServerLicenseType = this.SqlServerLicenseType;
+                          providerSettings.TargetVmTags = this.RecoveryVmTag;
+                          providerSettings.TargetManagedDiskTags = this.DiskTag;

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

providerSettings.TargetManagedDiskTags = this.DiskTag; [](start = 12, length = 54)

Only if useManagedDisks is true else service will throw error.
Should we throw error in powershell or we want to surface the srs exception?? #Resolved

Author

vidyadharijami Mar 25, 2021

Handled in powershell

In reply to: 601548064 [](ancestors = 601548064)

Arpitaji reviewed

View reviewed changes

...teRecovery/ReplicationProtectedItem/SetAzureRmRecoveryServicesAsrReplicationProtectedItem.cs Outdated

+                      [Parameter]
+                      [ValidateNotNullOrEmpty]
+                      [ValidateSet(
+                          Constants.NoLicenseType,

Arpitaji Mar 25, 2021

NoLicenseType [](start = 22, length = 13)

Same as above

Author

vidyadharijami Mar 25, 2021

Need to check on this once if we need to consider NotSpecified.

In reply to: 601549109 [](ancestors = 601549109)

Arpitaji reviewed

View reviewed changes

...teRecovery/ReplicationProtectedItem/SetAzureRmRecoveryServicesAsrReplicationProtectedItem.cs

+                              var sqlServerLicenseType = this.SqlServerLicenseType;
+                              var recoveryVmTag = this.RecoveryVmTag;
+                              var recoveryNicTag = this.RecoveryNicTag;
+                              var diskTag = this.DiskTag;

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

DiskTag [](start = 35, length = 7)

What if for H2A useManagedDisk is false? SRS will throw exception so are we fine with that? or we want to throw from powershell #Resolved

Author

vidyadharijami Mar 25, 2021

Handled in powershell.

In reply to: 601551907 [](ancestors = 601551907)

Arpitaji reviewed

View reviewed changes

...teRecovery/ReplicationProtectedItem/SetAzureRmRecoveryServicesAsrReplicationProtectedItem.cs

+                                              TargetProximityPlacementGroupId = proximityPlacementGroupId,
+                                              SqlServerLicenseType = sqlServerLicenseType,
+                                              TargetVmTags = recoveryVmTag,
+                                              TargetManagedDiskTags = diskTag,

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

argetManagedDiskTags = diskTa [](start = 33, length = 29)

question as above #Resolved

Arpitaji reviewed

View reviewed changes

src/RecoveryServices/RecoveryServices.SiteRecovery.Test/ScenarioTests/B2A/AsrB2ATests.ps1

+                  $diskTag.Add("DiskTag1","powershellDisk")
+                  $nicTag = New-Object "System.Collections.Generic.Dictionary``2[System.String,System.String]"
+                  $nicTag.Add("NicTag1","powershellNic")
+                  $EnableDRjob = New-AsrReplicationProtectedItem -ProtectableItem $VM -Name $VM.Name -ProtectionContainerMapping $pcm -RecoveryAzureStorageAccountId $StorageAccountID -OSDiskName $VMName -OS Windows -RecoveryResourceGroupId $RecoveryResourceGroupId -RecoveryProximityPlacementGroupId $ppg -UseManagedDisk true -RecoveryAvailabilitySetId $avset -Size $size -SqlServerLicenseType $sqlLicenseType -RecoveryVmTag $vmTag -RecoveryNicTag $nicTag -DiskTag $diskTag -RecoveryAzureNetworkId $AzureVmNetworkId

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

AsrReplicationProtectedItem [](start = 23, length = 27)

In future we should wait for enable and IR to finish and check all these are coming fine, for now u can check in armclient #Resolved

Author

vidyadharijami Mar 25, 2021

For now checked from portal. Will do it next release.

In reply to: 601630510 [](ancestors = 601630510)

Arpitaji reviewed

View reviewed changes

src/RecoveryServices/RecoveryServices.SiteRecovery.Test/ScenarioTests/B2A/AsrB2ATests.ps1

                   $rpi = Get-AsrReplicationProtectedItem -ProtectionContainer $pc -FriendlyName $VMName
-                  $ppgSet="/subscriptions/b364ed8d-4279-4bf8-8fd1-56f8fa0ae05c/resourceGroups/Arpita-air/providers/Microsoft.Compute/proximityPlacementGroups/h2atestppgupdate"
-                  $currentJob = Set-AsrReplicationProtectedItem -InputObject $rpi -RecoveryProximityPlacementGroupId $ppgSet -UpdateNic $rpi.NicDetailsList[0].NicId -RecoveryNetworkId $AzureNetworkID -RecoveryNicSubnetName $subnet

Arpitaji Mar 25, 2021

$currentJob [](start = 4, length = 11)

Please set network else in future error will come

Author

vidyadharijami Mar 25, 2021

I have set the network during enable protection. So no need to do from here.

In reply to: 601631470 [](ancestors = 601631470)

Arpitaji reviewed

View reviewed changes

src/RecoveryServices/RecoveryServices.SiteRecovery.Test/ScenarioTests/B2A/AsrB2ATests.ps1



		$rpi = Get-AsrReplicationProtectedItem -ProtectionContainer $pc -FriendlyName $VMName
		Assert-NotNull($rpi.ProviderSpecificDetails.RecoveryProximityPlacementGroupId)

Arpitaji Mar 25, 2021

why did we remove, we should add check for value as well.

Author

vidyadharijami Mar 25, 2021

Will add this check. Will take care of it when I do add tests for V2A.

In reply to: 601631955 [](ancestors = 601631955)

Arpitaji reviewed

View reviewed changes

src/RecoveryServices/RecoveryServices.SiteRecovery.Test/ScenarioTests/B2A/AsrB2ATests.ps1

                   $rpi = Get-AsrReplicationProtectedItem -ProtectionContainer $pc -FriendlyName $VMName
-                  Assert-NotNull($rpi.ProviderSpecificDetails.RecoveryProximityPlacementGroupId)
+                  Assert-NotNull($rpi.ProviderSpecificDetails.RecoveryVmTag)

Arpitaji Mar 25, 2021

RecoveryVmTag [](start = 48, length = 13)

All assert, we should add check for value as well.

Author

vidyadharijami Mar 25, 2021

same as above

In reply to: 601632193 [](ancestors = 601632193)

Arpitaji reviewed

View reviewed changes

src/RecoveryServices/RecoveryServices/help/Set-AzRecoveryServicesAsrReplicationProtectedItem.md Outdated

-               [-RecoveryAvailabilitySet <String>] [-RecoveryAvailabilityZone <String>]
-               [-RecoveryProximityPlacementGroupId <String>] [-EnableAcceleratedNetworkingOnRecovery]
-               [-RecoveryBootDiagStorageAccountId <String>]
+               [-RecoveryAvailabilitySet <String>] [-SqlServerLicenseType <String>]

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

We should update the description based on provider #Resolved

anmolbhatia289 reviewed

View reviewed changes

src/RecoveryServices/RecoveryServices/help/Set-AzRecoveryServicesAsrReplicationProtectedItem.md


		### -DiskTag
		Specify the tags for the disks of the VM.

anmolbhatia289 Mar 25, 2021 •

edited by vidyadharijami

Loading

please update the description so that user knows for what scenarios, this is applicable. Same for all tags #Resolved


          Addressed review comments

838a284

Arpitaji reviewed

View reviewed changes

src/RecoveryServices/RecoveryServices/help/Set-AzRecoveryServicesAsrReplicationProtectedItem.md Outdated

               ```
+              ### -SqlServerLicenseType
+              Specify the SQL Server license type of the VM.

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

VM [](start = 43, length = 2)

this is applicable to which providers #Resolved

Arpitaji commented Mar 25, 2021 •

edited by vidyadharijami

Loading

    public const string NoLicenseType = "NoLicenseType";

We are reusing this but the link is incorrect for us #Resolved

Refers to: src/RecoveryServices/RecoveryServices.SiteRecovery/Models/PSConstants.cs:288 in 838a284. [](commit_id = 838a284, deletion_comment = False)

Arpitaji reviewed

View reviewed changes

src/RecoveryServices/RecoveryServices.SiteRecovery/Models/PSFabricDetails.cs Outdated

                       public string RecoveryProximityPlacementGroupId { get; set; }
+                      /// <summary>
+                      /// Gets or sets the SQL Server license type of the machine to in the event of a failover.

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

to [](start = 68, length = 2)

"to" we need to remove #Resolved

Arpitaji reviewed

View reviewed changes

src/RecoveryServices/RecoveryServices.SiteRecovery/Models/PSFabricDetails.cs Outdated

                       public string RecoveryProximityPlacementGroupId { get; set; }
+                      /// <summary>
+                      /// Gets or sets the SQL Server license type of the machine to in the event of a failover.

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

to [](start = 68, length = 2)

"to" we need to remove #Resolved

Arpitaji reviewed

View reviewed changes

...teRecovery/ReplicationProtectedItem/NewAzureRmRecoveryServicesAsrReplicationProtectedItem.cs Outdated

+                              }
+                              else
+                              {
+                                  providerSettings.TargetManagedDiskTags = this.DiskTag;

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

providerSettings.TargetManagedDiskTags = this.DiskTag [](start = 20, length = 53)

we can assign irrespective of the check like

providerSettings.TargetManagedDiskTags = this.DiskTag;
if(this.UseManagedDisk == Constants.False && this.DiskTag != null && this.DiskTag.Count > 0)
{
throw new PSArgumentException(
string.Format(
Resources.DiskTagCannotBeSet,
this.DiskTag,
this.UseManagedDisk));
}
or u can move the error above #Resolved

Arpitaji reviewed

View reviewed changes

...teRecovery/ReplicationProtectedItem/SetAzureRmRecoveryServicesAsrReplicationProtectedItem.cs Outdated

+                                  {
+                                      if(diskTag != null ||
+                                          diskTag.Count > 0)
+                                      {

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

we are checking effectively (this.DiskTag == null ||
this.DiskTag.Count == 0)

twice
something like
if (this.DiskTag == null ||
this.DiskTag.Count == 0)
{
diskTag = providerSpecificDetails.TargetManagedDiskTags;
}
else if(useManagedDisk == Constants.False)
{throw new PSArgumentException(
string.Format(
Resources.DiskTagCannotBeSet,
diskTag,
useManagedDisk));
} #Resolved

Arpitaji reviewed

View reviewed changes

...teRecovery/ReplicationProtectedItem/NewAzureRmRecoveryServicesAsrReplicationProtectedItem.cs Outdated

+                          providerSettings.TargetVmTags = this.RecoveryVmTag;
+                          providerSettings.TargetNicTags = this.RecoveryNicTag;
+                          if(this.DiskTag != null || this.DiskTag.Count > 0)

Arpitaji Mar 25, 2021 •

edited by vidyadharijami

Loading

|| [](start = 36, length = 2)

&& #Resolved

Author

vidyadharijami commented Mar 25, 2021

    public const string NoLicenseType = "NoLicenseType";

Shall we have another constant for Sql License type ?

In reply to: 807333660 [](ancestors = 807333660)

Refers to: src/RecoveryServices/RecoveryServices.SiteRecovery/Models/PSConstants.cs:288 in 838a284. [](commit_id = 838a284, deletion_comment = False)


          Minor fixes

8a9138d

Arpitaji reviewed

View reviewed changes

...teRecovery/ReplicationProtectedItem/SetAzureRmRecoveryServicesAsrReplicationProtectedItem.cs

+                                    this.DiskTag.Count == 0)
+                                  {
+                                      diskTag = providerSpecificDetails.TargetManagedDiskTags;
+                                  }

Arpitaji Mar 26, 2021 •

edited by vidyadharijami

Loading

} [](start = 20, length = 1)

V2A edit also we had to throw error right #Resolved

Author

vidyadharijami Mar 26, 2021

Fixed it

In reply to: 602052721 [](ancestors = 602052721)


          Added V2A tests and recordings

2d1a123

anmolbhatia289 reviewed

View reviewed changes

src/RecoveryServices/RecoveryServices.SiteRecovery/Properties/Resources.resx Outdated

                   <value>IP Config "{0}" not found in VM NIC "{1}".</value>
                 </data>
+                <data name="DiskTagCannotBeSet" xml:space="preserve">
+                  <value>Disk Tag "{0}" cannot be set if UseManagedDisk is "{1}".</value>

anmolbhatia289 Mar 26, 2021 •

edited by vidyadharijami

Loading

Tag [](start = 16, length = 3)

Tag(s) #Resolved

Arpitaji approved these changes

View reviewed changes

Arpitaji left a comment

anmolbhatia289 approved these changes

View reviewed changes

anmolbhatia289 left a comment

vidyadharijami added 2 commits

March 26, 2021 16:07


          Minor Fix

3d45cad


          Merge changes of newApiVersion/asr2021-02-10 branch

d7920f9

vidyadharijami merged commit 2d0bec1 into newApiVersion/asr2021-02-10

shesamian pushed a commit that referenced this pull request


          [Storage] Support dfs sas encryptionscope (Azure#19814)

43bd025

* Upgrade to new storage dataplane SDK

* [Storage] Support dfs sas encryptionscope

* Update DependencyAnalyzer.cs (#21)

Co-authored-by: Dingmeng Xue <dixue@microsoft.com>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet