Added cmdlets instant file recovery and enhanced tests #4834

dragonfly91 · 2017-10-23T13:55:12Z

Description

Added cmdlets for instant file recovery from a recovery point.
Updated version of Recovery Services Backup .NET SDK to latest.
Test enhancements: all the IaaS VM tests would be doing their setup on their own; thus not needing external resources / setups to record the tests.

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.

dragonfly91 · 2017-10-23T13:57:29Z

This PR already includes changes proposed in #4821. Once this PR gets merged, will pull from upstream and update the current PR.

cormacpayne

@dragonfly91 a few initial comments

Also, would you mind updating the RecoveryServices.Backup change log to reflect the changes you are making in this PR?

cormacpayne · 2017-10-31T00:23:55Z

...ces.Backup/Commands.RecoveryServices.Backup/help/Backup-AzureRmRecoveryServicesBackupItem.md

@@ -42,6 +44,19 @@ The last command triggers the backup job for the Backup item in $Item.

 ## PARAMETERS

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


@dragonfly91 for all of the files where you find this applies, please make sure the ```yaml string at the end of this parameter description is moved to a line by itself. See other parameters in this file for how this should look.

cormacpayne · 2017-10-31T00:25:04Z

...veryServices.Backup/Commands.RecoveryServices.Backup/help/Disable-AzureRmRSBRPMountScript.md

+### Example 1: Select the recovery point and get the script
+```
+PS C:\> $namedContainer = Get-AzureRmRecoveryServicesBackupContainer  -ContainerType "AzureVM" �Status "Registered" -FriendlyName "V2VM"
+PS C:\> $backupitem = Get-AzureRmRecoveryServicesBackupItem �Container $namedContainer  �WorkloadType "AzureVM"


@dragonfly91 it looks like there might have been some bad encoding on the first two lines of this example

cormacpayne · 2017-10-31T00:26:14Z

...RecoveryServices.Backup/Commands.RecoveryServices.Backup/help/Get-AzureRmRSBRPMountScript.md

+### Example 1: Select the recovery point and get the script
+```
+PS C:\> $namedContainer = Get-AzureRmRecoveryServicesBackupContainer  -ContainerType "AzureVM" �Status "Registered" -FriendlyName "V2VM"
+PS C:\> $backupitem = Get-AzureRmRecoveryServicesBackupItem �Container $namedContainer  �WorkloadType "AzureVM"


@dragonfly91 same comment about bad encoding in the example

cormacpayne · 2017-10-31T00:29:36Z

...eryServices.Backup/Cmdlets/RecoveryPoint/AzureRmRecoveryServicesBackupRecoveryPointAccess.cs

+        /// </summary>
+        [Parameter(
+            Mandatory = false,
+            ValueFromPipeline = false,


@dragonfly91 this parameter should not have the ValueFromPipeline attribute

cormacpayne · 2017-10-31T00:34:36Z

...eryServices.Backup/Cmdlets/RecoveryPoint/AzureRmRecoveryServicesBackupRecoveryPointAccess.cs

+                var response = psBackupProvider.ProvisionItemLevelRecoveryAccess();
+
+                WriteDebug(string.Format("Mount Script download completed"));
+                WriteInformation(string.Format(Resources.MountRecoveryPointInfoMessage, response.Password),


@dragonfly91 I would recommend writing this as a part of the debug or verbose stream

This text needs to be displayed as it contains a password. When the script is run, it asks for this password to be able to mount the files of the recovery point. Do you have any other option to display the text?

@dragonfly91 WriteInformation is only available in PowerShell 5. Please use one of the other streams. If this needs to be displayed, then it should be part of the output object for the cmdlet

cormacpayne · 2017-10-31T00:35:11Z

...eryServices.Backup/Cmdlets/RecoveryPoint/AzureRmRecoveryServicesBackupRecoveryPointAccess.cs

+                WriteInformation(string.Format(Resources.MountRecoveryPointInfoMessage, response.Password),
+                    null);
+                WriteObject(response);
+            }, ShouldProcess(RecoveryPoint.ItemName, VerbsCommon.Get));


@dragonfly91 instead of using "Get" as the action, I would say something like "Downloading script..."

dragonfly91 · 2017-10-31T17:18:25Z

Hi Cormac, I've addressed all of your comments. Can you please take a look?

dragonfly91 · 2017-11-01T21:30:23Z

Hi @markcowl and @cormacpayne I've addressed @markcowl 's comments. Can you please take a look?

cormacpayne

@dragonfly91 a few more comments

cormacpayne · 2017-11-01T22:05:37Z

...ceManager/RecoveryServices.Backup/Commands.RecoveryServices.Backup.Providers/packages.config

@@ -1,8 +1,9 @@
 <?xml version="1.0" encoding="utf-8"?>
 <packages>
  <package id="Hyak.Common" version="1.0.3" targetFramework="net45" />
+  <package id="Microsoft.AspNet.WebApi.Core" version="5.2.3" targetFramework="net462" />


@dragonfly91 what is this package being used for?

cormacpayne · 2017-11-01T22:10:39Z

...rvices.Backup/Commands.RecoveryServices.Backup.Providers/Providers/IaasVmPsBackupProvider.cs

+            string content = string.Empty;
+            AzureVmRecoveryPoint rp = ProviderData[RestoreBackupItemParams.RecoveryPoint]
+                as AzureVmRecoveryPoint;
+            if (rp.EncryptionEnabled == true)


@dragonfly91 do we need to check if rp is null before accessing a property of it?

This case would never happen as we have [ValidateNotNullOrEmpty] declared on top of the RecoveryPoint parameter in the cmdlet definition. So, I don't think a null check is needed.

cormacpayne · 2017-11-01T22:14:29Z

...rvices.Backup/Commands.RecoveryServices.Backup.Providers/Providers/IaasVmPsBackupProvider.cs

+                scriptDownloadLocation = Directory.GetCurrentDirectory();
+            }
+            absoluteFilePath = Path.Combine(scriptDownloadLocation, result.Filename);
+            File.WriteAllBytes(absoluteFilePath, Convert.FromBase64String(content));


@dragonfly91 we should check for the existence of absoluteFilePath before writing to it

The designed behavior is that the cmdlet overwrites any existing files. However, the file name already contains a randomized string (based on timestamp) and thus every time a new file is generated. So, there's no point in checking for the existence of the file.

cormacpayne · 2017-11-01T22:16:07Z

...rvices.Backup/Commands.RecoveryServices.Backup.Providers/Providers/IaasVmPsBackupProvider.cs

+                scriptDownloadLocation = Directory.GetCurrentDirectory();
+            }
+            absoluteFilePath = Path.Combine(scriptDownloadLocation, result.Filename);
+            File.WriteAllBytes(absoluteFilePath, Convert.FromBase64String(content));


@dragonfly91 also, please use AzureSession.Instance.DataStore to read from/write to a file.

cormacpayne · 2017-11-01T22:27:13Z

...ryServices.Backup/Cmdlets/RecoveryPoint/DisableAzureRmRecoveryServicesBackupRPMountScript.cs

+    /// Disable the mount script of recovery point of an item.
+    /// Files won't be mounted after running this cmdlet.
+    /// </summary>
+    [Cmdlet(VerbsLifecycle.Disable, "AzureRmRecoveryServicesBackupRPMountScript",


@dragonfly91 make sure this cmdlet has an OutputType, even if it's void or RecoveryPointBase

Taken this.

cormacpayne · 2017-11-01T22:29:09Z

...ckup/Commands.RecoveryServices.Backup/help/Get-AzureRmRecoveryServicesBackupRPMountScript.md

+```
+
+### -Path
+Location where the file should be downloaded in the case of file recovery.


@dragonfly91 it should be made clear in the documentation that if no Path is provided, then you use the current directory as the save location of the script

dragonfly91 · 2017-11-02T17:13:52Z

Hi @cormacpayne and @markcowl , I've addressed all the given comments. Can you please take a look?

cormacpayne · 2017-11-02T18:21:27Z

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

…very Services Backup. Updating Microsoft.Azure.Management.RecoveryServices.Backup dll version to 2.1.0-preview Updating Microsoft.Azure.Management.RecoveryServices.Backup dll version to 2.1.0-preview in Test projects Updating hint path for Microsoft.Azure.Management.RecoveryServices.Backup dll version to 2.1.0-preview

Renaming model for RecoveryPointAccessInfo Fixing few bugs Fex bug fixes Adding timezone info changes Bug fixes Bug fixes

disabling create vault vm rg backup test test infra betterments, job tests wait job test recording job and get rp tests ilr test get item test recording add protection and backup test recordings get rps test recording Fixes to item tests all tests except restore tests working in both record and playback modes Updated help files Edits to make restore test work upstream pull + full restore test pass ILR test test fixes

Test fixes Renaming instant file restore cmdlets based on design review comments.

Fix 12577430: In Get-Container, should use exact equal check instead of contains check when checking for container name.

…d to the user in Get-AzureRmRSBRPMountScript cmdlet as per PR review comments.

cormacpayne · 2017-11-06T06:10:15Z

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

…hell into ilr

cormacpayne · 2017-11-06T22:06:26Z

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

azuresdkci added the needs-review label Oct 23, 2017

azuresdkci assigned shahabhijeet Oct 23, 2017

markcowl assigned cormacpayne and unassigned shahabhijeet Oct 23, 2017

markcowl added Cmdlet Review Required ⚠️ and removed needs-review labels Oct 23, 2017

dragonfly91 force-pushed the ilr branch from 66c594a to ed1d965 Compare October 24, 2017 18:54

cormacpayne suggested changes Oct 31, 2017

View reviewed changes

cormacpayne added the needs-revision label Oct 31, 2017

cormacpayne assigned dragonfly91 and unassigned cormacpayne Oct 31, 2017

dragonfly91 force-pushed the ilr branch from 0caa3b1 to bbab20b Compare October 31, 2017 13:29

dragonfly91 assigned cormacpayne and unassigned dragonfly91 Oct 31, 2017

dragonfly91 force-pushed the ilr branch 2 times, most recently from c88008d to ed66bdf Compare November 1, 2017 16:57

cormacpayne added needs-review and removed needs-revision labels Nov 1, 2017

cormacpayne suggested changes Nov 1, 2017

View reviewed changes

cormacpayne added needs-revision and removed needs-review labels Nov 1, 2017

cormacpayne assigned dragonfly91 and unassigned cormacpayne Nov 1, 2017

cormacpayne added Cmdlet Review Complete and removed Cmdlet Review Required ⚠️ labels Nov 1, 2017

dragonfly91 force-pushed the ilr branch from db6fa1e to 3cc603f Compare November 2, 2017 12:24

markcowl changed the base branch from preview to release-5.0.0 November 2, 2017 23:24

markcowl added the Merge Conflicts label Nov 2, 2017

pragrawa and others added 10 commits November 3, 2017 12:36

Updating version of RecoveryServices.Backup SDK to latest.

64cb1fd

Renaming model for RecoveryPointAccessInfo Fixing few bugs Fex bug fixes Adding timezone info changes Bug fixes Bug fixes

Wxi file update + date time zone fixes in tests

9a95f7b

Test fixes Renaming instant file restore cmdlets based on design review comments.

Addressing PR comments

808e581

Fix 12577430: In Get-Container, should use exact equal check instead of contains check when checking for container name.

Using WriteVerbose instead of WriteInformation to display the passwor…

461bd53

…d to the user in Get-AzureRmRSBRPMountScript cmdlet as per PR review comments.

Changes to RP Mount Script cmdlets based on PR review.

30047e4

Wxi file update

45612a2

Addressing review comments.

2940e02

Help update

ad2334c

dragonfly91 force-pushed the ilr branch from 3cc603f to ad2334c Compare November 3, 2017 08:27

dragonfly91 added needs-review and removed Merge Conflicts needs-revision labels Nov 3, 2017

dragonfly91 assigned cormacpayne and unassigned dragonfly91 Nov 3, 2017

cormacpayne approved these changes Nov 6, 2017

View reviewed changes

cormacpayne added Waiting for On Demand and removed needs-review labels Nov 6, 2017

cormacpayne removed their assignment Nov 6, 2017

Merge branch 'release-5.0.0' of https://github.com/Azure/azure-powers…

fe7fec4

…hell into ilr

cormacpayne merged commit 6451aa7 into Azure:release-5.0.0 Nov 7, 2017

Added cmdlets instant file recovery and enhanced tests #4834

Added cmdlets instant file recovery and enhanced tests #4834

Uh oh!

Conversation

dragonfly91 commented Oct 23, 2017 • edited by azuresdkci Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

dragonfly91 commented Oct 23, 2017

Uh oh!

cormacpayne left a comment

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!

dragonfly91 commented Oct 31, 2017

Uh oh!

dragonfly91 commented Nov 1, 2017

Uh oh!

cormacpayne left a comment

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!

dragonfly91 commented Nov 2, 2017

Uh oh!

cormacpayne commented Nov 2, 2017

Uh oh!

cormacpayne commented Nov 6, 2017

Uh oh!

cormacpayne commented Nov 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

dragonfly91 commented Oct 23, 2017 •

edited by azuresdkci

Loading

cormacpayne commented Nov 6, 2017 •

edited

Loading