adding remove disk api, introducing zone, network fields in enable protection #9

ayfathim · 2019-04-11T15:46:03Z

Following changes are made in this:

Added remove disk api
Updated replicated Protected Item with new field "AllowedDisklevelOperation"
Added Zone, recoveryNetwork, recoverySubnet in enable protection api.
Updated replicated protected Item with new field zone
updated to latest sdkForNet 2.0 Version.
Commented unprotected disk field for temporary basis.
(Note this is a temporary update, till the final sdkForNet with unProtectedDisk field is not published . The PR for this is already created AutoPr-RecoveryServices.SiteRecovery-ayfathim-REST Spec PrNumber 5576 Azure/azure-sdk-for-net#5754)

Also made fix for Bug#4227197: [A2A][Powershell]reprotect command fails when disk information is passed
Issue: When disk details are passed during reprotect, the log storage account is optional. And we were using the optional-logStorageAccount field.

Resolution: using the cache storage account passed as part of diskDetails.

@innosam and @mady4ever : please review the zone and network changes

Description

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.

…otection

...ryServices/Commands.RecoveryServices.SiteRecovery/AzureRM.RecoveryServices.SiteRecovery.psd1

...rvices/Commands.RecoveryServices.SiteRecovery/Common/PSAsrReplicationProtectedItemsClient.cs

sriramvu · 2019-04-12T09:56:53Z

...teRecovery/ReplicationProtectedItem/NewAzureRmRecoveryServicesAsrReplicationProtectedItem.cs

@@ -261,6 +265,14 @@ public class NewAzureRmRecoveryServicesAsrReplicationProtectedItem : SiteRecover
        [ValidateNotNullOrEmpty]
        public string RecoveryAvailabilitySetId { get; set; }

+        /// <summary>
+        /// Gets or sets ID of the AvailabilityZone to recover the machine to in the event of a failover.


Zone is just a number and not an ID.

sriramvu · 2019-04-12T10:03:26Z

...ery/ReplicationProtectedItem/RemoveAzureRmRecoveryServicesAsrReplicationProtectedItemDisk.cs

+        /// <summary>
+        ///     Gets or sets the disk Id.
+        /// </summary>
+        [Parameter(ParameterSetName = ASRParameterSets.AzureToAzureManagedDisk,


did not see any requirement of parameter sets - please remove.

looks fine to me there will be two parameter set one for mananged disk other for non Managed disk.

Or we can call field some common identifier and take diskId or VhdUri in that .In that the approach Sriram you are suggesting ?

...ery/ReplicationProtectedItem/RemoveAzureRmRecoveryServicesAsrReplicationProtectedItemDisk.cs

viverm · 2019-04-18T06:47:04Z

Please add Test cases and recording

viverm · 2019-04-17T11:32:11Z

...ryServices/Commands.RecoveryServices.SiteRecovery/AzureRM.RecoveryServices.SiteRecovery.psd1

-               'Remove-ASRRecoveryPlan', 'Remove-ASRReplicationProtectedItem', 
+               'Remove-ASRProtectionContainerMapping', 'Remove-ASRRP',
+               'Remove-ASRRecoveryPlan', 'Remove-ASRReplicationProtectedItem',
+	       'Remove-ASRReplicationProtectedItemDisk',


viverm · 2019-04-17T11:35:44Z

...teRecovery/ReplicationProtectedItem/NewAzureRmRecoveryServicesAsrReplicationProtectedItem.cs

            };

            if (!string.IsNullOrEmpty(this.RecoveryCloudServiceId))
            {
                providerSettings.RecoveryResourceGroupId = null;
            }

+            if (!string.IsNullOrEmpty(this.RecoveryAvailabilityZone) &&
+                !string.IsNullOrEmpty(this.RecoveryAvailabilitySetId))
+            {


use different parameter set instead of having this check.

viverm · 2019-04-17T11:39:46Z

...ery/ReplicationProtectedItem/RemoveAzureRmRecoveryServicesAsrReplicationProtectedItemDisk.cs

+        /// <summary>
+        ///     Gets or sets the disk Id.
+        /// </summary>
+        [Parameter(ParameterSetName = ASRParameterSets.AzureToAzureManagedDisk,


looks fine to me there will be two parameter set one for mananged disk other for non Managed disk.

Or we can call field some common identifier and take diskId or VhdUri in that .In that the approach Sriram you are suggesting ?

viverm · 2019-04-17T11:41:05Z

...ery/ReplicationProtectedItem/RemoveAzureRmRecoveryServicesAsrReplicationProtectedItemDisk.cs

+        public string[] DiskId { get; set; }
+
+        [Parameter]
+        public SwitchParameter WaitForCompletion { get; set; }


This is not required any more. Powershell team has AsJob new flag to handle that .
We have to move all longrunning cmdlet to that pattern.

viverm · 2019-04-17T11:45:48Z

...ery/ReplicationProtectedItem/RemoveAzureRmRecoveryServicesAsrReplicationProtectedItemDisk.cs

+    /// <summary>
+    /// Removes disks to replication protected item.
+    /// </summary>
+    [Cmdlet("Remove", ResourceManager.Common.AzureRMConstants.AzureRMPrefix + "RecoveryServicesAsrReplicationProtectedItemDisk", DefaultParameterSetName = ASRParameterSets.EnterpriseToEnterprise, SupportsShouldProcess = true)]


check the sematics around shouldProcess

https://docs.microsoft.com/en-us/powershell/developer/cmdlet/approved-verbs-for-windows-powershell-commands

viverm · 2019-04-17T11:46:08Z

...ery/ReplicationProtectedItem/RemoveAzureRmRecoveryServicesAsrReplicationProtectedItemDisk.cs

+namespace Microsoft.Azure.Commands.RecoveryServices.SiteRecovery
+{
+    /// <summary>
+    /// Removes disks to replication protected item.


viverm · 2019-04-17T11:48:24Z

...ery/ReplicationProtectedItem/RemoveAzureRmRecoveryServicesAsrReplicationProtectedItemDisk.cs

+
+        /// <summary>
+        /// Writes Job.
+        /// </summary>


no need for these method and private variables ,move the code to ExecuteSiteRecoveryCmdlet method

Resolve cormacs comments

* Generate Migrate & OffAzur * custom * using directives to customise properties * made hash code * complete branch * adding interal files * fixed internal cmdlet * hid expanded * fixed directive' * cleanup * cleanup * m * m * revision * fix * added a variant * fixed mistake * added getremi and restartjobs * added disable * fixed bug * fixed restart to resync * added new enable * mock * good night * m * as * s * added changes * designreview * last commit before AMH (#1) * merging 2 readmes in 1 (#2) * merging 2 readmes * merging 2 readmes * Srsamh (#4) * merging 2 readmes * merging 2 readmes * added changes after DR * fix * bug fix * Az migrate staging219 (#7) * added changes to fix bug * fix bug * Az migrate staging219 (#8) * added changes to fix bug * fix bug * Az migrate staging219 (#9) * added changes to fix bug * fix bug * fix bug * merged AMH and SDS swagger generated cmdlets with SRS cmdlets. * Delete Get-AzmigrateJob.md * Delete Get-AzmigrateJob.md * updated and added some tests. New/remove cmdlet tests remain * Delete Get-AzmigrateJob.Tests.ps1 * fixed some syntax issues, exposed some cmdlets * fixed some syntax issues, exposed some cmdlets * test,rec,ex for job * test,rec,ex for job * fixed bug * added fabric * policy * Update Get-AzMigrateReplicationPolicy.Tests.ps1 * container * mapping * provider,server,disk * fix * server * disk * disk * policy * maping * enable * added changes files * set * test * New and Remove cmdlet tests added. * fix syntax * added remove * added changes * added register tool commandlet and its tests. * leftovers * Added examples. * fixed machine get doc * removed get by identity and updated docs * adding env files * added get sds machines by migrate project * added recordings * added code changes * added set * get server * docs * adding back file * new * remove * restart * set * mig * test * vars * leftovers * removed whatif * output * removed asjob * added if else * docs Co-authored-by: msJinLei <[email protected]> Co-authored-by: Kunal Chaturvedi <[email protected]> Co-authored-by: kuchatur-ms <[email protected]>

* init add powershell cmdlet * add customization for filedetails * correct readme to include file workspace create * add custom cmdlet for creating and uploading the file * chunking logic for larger files * add error for custom cmdlet when file too big * combined cmdlets * edit custom file upload command to not use default subscriptionid * no default value for subid in custom file upload * separate subscription and no subscription commands * remove comments from combined cmd for file * fix cmd * custom no subscription file upload commands * autogen docs for no sub file upload * hide individual file commands * add back workspace commands * add name as alias for fileworkspacename fileworkspacesnosubscription * remove update files and add alias for nosubscription file commands * tests for get service * tests for problem classification * try adding erroractionpreference = stop * tests for new file workspace * tests for get file workspace * more tests * add tests for new file and upload, removefile name as a parameter * remove new files no subscription * tests for get file * erroraction stop * remove unnecessary comments and print statements * docs for get support service * problem classification docs * add titles * documentation for file workspace commands * documentation for file/file workspace cmdlets * documentation for checkNameAvailability * added tests and documentation for operations, support ticket, communication and chat transcript cmdlets * added examples for operations, support ticket, communication and chat transcript cmdlets * resolve merge conflicts * resolved PR comments * get conflict file changes from grhuang/azsupport-autorest * resolved merge conflict * removed update communication sub and no sub scenarios * Revert "removed update communication sub and no sub scenarios" This reverts commit ccbfdee. * removed update communication sub and no sub scenarios , updated readme * make communiation and support ticket properties required in readme, edit get operation file name, edit calling internal cmdlets for file upload * fix documentation * fix top query * add custom error handler * consolidate list and get communicationsnosubscription and chattranscriptsnosubscription * init changes to allow no subscription recording tests-need to use csp partner account in tenant 2e6a0c9f-986d-480e-ad4b-bdfddc047aba * changes to not create new resources in playback * remove custom error handler csharp * init add changes to split subscription and no subscription tests * update skip * Update recordings * update recordings * add more examples to new-azsupportticket documentation * update documentation and readme * add directive back in * make advanced diagnostic consent required * Add custom error handling for New- and Update- cmdlets to print full details of error (#9) * Add custom error handler * Add default filter to retrieve tickets from the past week for Get-AzSupportTickets and Get-AzSupportTicketsNoSubscription (#10) * add custom filtering for list support ticket if no filter is applied * make transformations in swagger in readme to make enrollment id not readonly and show isTempTicket (#11) * Add argument completer (#12) * add argument completers * add quotes to argument completers * remove repeated time zone * Regenerate powershell module with GA swagger (#13) * Rerecord tests using GA version (#14) * add tests for file size (will add recordings once Ga swagger available) * Regenerate powershell module with GA swagger * Rerecord tests with GA version * Fix documentation * fix url for file upload --------- Co-authored-by: Grace Huang (from Dev Box) <[email protected]> Co-authored-by: grhuangmsft <[email protected]> Co-authored-by: Shreya Kumar <[email protected]> Co-authored-by: Yunchi Wang <[email protected]>

adding remove disk api, introducing zone, network fields in enable pr…

529c7bb

…otection

ayfathim requested review from innosam, sriramvu, mady4ever, viverm and mukeshk78 April 11, 2019 15:46

sriramvu requested changes Apr 12, 2019

View reviewed changes

ayfathim added 3 commits April 17, 2019 16:42

Updated with comments,Fix for reprotect bug#227197

537ad52

correcting the indentaion

db854ed

correting the default parameter set name

7982833

ayfathim added 2 commits April 22, 2019 00:32

updating to new nuget

5c282cb

ResolveHealthError API and tests

814cb41

viverm reviewed Aug 28, 2019

View reviewed changes

viverm pushed a commit that referenced this pull request Oct 21, 2019

Merge pull request #9 from gaurangisaxena/gasaxena/powershell

b14b730

Resolve cormacs comments

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

adding remove disk api, introducing zone, network fields in enable protection #9

adding remove disk api, introducing zone, network fields in enable protection #9

Uh oh!

ayfathim commented Apr 11, 2019 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sriramvu Apr 12, 2019

Uh oh!

sriramvu Apr 12, 2019

Uh oh!

viverm Apr 17, 2019

Uh oh!

Uh oh!

Uh oh!

viverm commented Apr 18, 2019

Uh oh!

viverm Apr 17, 2019

Uh oh!

viverm Apr 17, 2019

Uh oh!

viverm Apr 17, 2019

Uh oh!

viverm Apr 17, 2019

Uh oh!

viverm Apr 17, 2019

Uh oh!

viverm Apr 17, 2019

Uh oh!

viverm Apr 17, 2019

Uh oh!

Uh oh!

adding remove disk api, introducing zone, network fields in enable protection #9

Are you sure you want to change the base?

adding remove disk api, introducing zone, network fields in enable protection #9

Uh oh!

Conversation

ayfathim commented Apr 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Resolution: using the cache storage account passed as part of diskDetails.

Description

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

viverm commented Apr 18, 2019

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!

Uh oh!

ayfathim commented Apr 11, 2019 •

edited

Loading