[AzureRT] Disk RP cmdlets #3471

hyonholee · 2017-02-07T08:17:43Z

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.

…shell code with DiskRP support

…mode

…upport Managed disk

…iskStorageAccountType to StorageAccountType

Powershell tests for VMSS with managed disks

disk and snapshot tests

…rshell into release-3.5.0

Add TestVirtualMachine_Managed test.

…wershell into release-3.5.0

…eter.

…dlets

…-AzureRmContainerService

cormacpayne

@hyonholee a lot of the comments I left have been addressed in the email containing the csv files

I just wanted to go through and finish the review in case there was anything that wasn't caught by those files

cormacpayne · 2017-02-07T17:35:12Z

...sourceManager/Compute/Commands.Compute/AvailabilitySets/RemoveAzureAvailabilitySetCommand.cs

@@ -1,68 +0,0 @@
-// ----------------------------------------------------------------------------------


@hyonholee why is this cmdlet being removed? This is a breaking change

cormacpayne · 2017-02-07T17:35:57Z

...esourceManager/Compute/Commands.Compute/AvailabilitySets/UpdatAzureAvailabilitySetCommand.cs

@@ -0,0 +1,97 @@
+// ----------------------------------------------------------------------------------


@hyonholee there is a typo in the name of this file - Updat instead of Update

cormacpayne · 2017-02-07T17:38:01Z

...esourceManager/Compute/Commands.Compute/AvailabilitySets/UpdatAzureAvailabilitySetCommand.cs

+{
+    [Cmdlet(VerbsData.Update, ProfileNouns.AvailabilitySet)]
+    [OutputType(typeof(PSAvailabilitySet))]
+    public class UpdateAzureAvailabilitySetCommand : AvailabilitySetBaseCmdlet


@hyonholee should this cmdlet implement SupportShouldProcess since it is making changes?

cormacpayne · 2017-02-07T17:40:26Z

...mpute/Generated/ContainerService/Config/AddAzureRmContainerServiceAgentPoolProfileCommand.cs

@@ -49,7 +49,7 @@ public class AddAzureRmContainerServiceAgentPoolProfileCommand : Microsoft.Azure
            Mandatory = false,
            Position = 2,
            ValueFromPipelineByPropertyName = true)]
-        public int? Count { get; set; }
+        public int Count { get; set; }


@hyonholee why was this change made?

It is changed because the base client library is changed. The base Swagger spec is the same, but AutoRest generates a different type after we updated AutoRest version. Now the client library has int type instead of int?

cormacpayne · 2017-02-07T17:43:13Z

...ommands.Compute/Generated/ContainerService/Config/NewAzureRmContainerServiceConfigCommand.cs

@@ -54,55 +54,79 @@ public class NewAzureRmContainerServiceConfigCommand : Microsoft.Azure.Commands.
            Mandatory = false,
            Position = 3,
            ValueFromPipelineByPropertyName = true)]
-        public int? MasterCount { get; set; }
+        public string CustomProfileOrchestrator { get; set; }


@hyonholee changing the positions of existing parameters is a breaking change - these changes need to be reverted in this file. The new parameters should be at the end of the position list.

cormacpayne · 2017-02-08T01:14:17Z

...e/Commands.Compute/Generated/VirtualMachineScaleSet/Config/AddAzureRmVmssExtensionCommand.cs

-            Mandatory = false,
-            Position = 8,
-            ValueFromPipelineByPropertyName = true)]
-        public string Id { get; set; }


@hyonholee removing this parameter is a breaking change

The reason of removal of this parameter is this parameter (ID) is read-only in VMSS. This was a bug in Swagger spec and this was fixed.

cormacpayne · 2017-02-08T01:14:55Z

...pute/Commands.Compute/Generated/VirtualMachineScaleSet/Config/NewAzureRmVmssConfigCommand.cs

@@ -42,67 +42,100 @@ public class NewAzureRmVmssConfigCommand : Microsoft.Azure.Commands.ResourceMana
            Mandatory = false,
            Position = 1,
            ValueFromPipelineByPropertyName = true)]
-        public string Location { get; set; }


@hyonholee changing the position of existing parameters is a breaking change

cormacpayne · 2017-02-08T01:15:24Z

...Commands.Compute/Generated/VirtualMachineScaleSet/Config/RemoveAzureRmVmssDataDiskCommand.cs

+{
+    [Cmdlet("Remove", "AzureRmVmssDataDisk")]
+    [OutputType(typeof(VirtualMachineScaleSet))]
+    public class RemoveAzureRmVmssDataDiskCommand : Microsoft.Azure.Commands.ResourceManager.Common.AzureRMCmdlet


@hyonholee same comment about output type and SupportShouldProcess

cormacpayne · 2017-02-08T01:15:43Z

...ommands.Compute/Generated/VirtualMachineScaleSet/Config/RemoveAzureRmVmssExtensionCommand.cs

-            Position = 1,
-            ParameterSetName = "IdParameterSet",
-            ValueFromPipelineByPropertyName = true)]
-        public string Id { get; set; }


@hyonholee removing a parameter is a breaking change

cormacpayne · 2017-02-08T01:16:12Z

...mands.Compute/Generated/VirtualMachineScaleSet/Config/SetAzureRmVmssStorageProfileCommand.cs

@@ -67,38 +67,56 @@ public class SetAzureRmVmssStorageProfileCommand : Microsoft.Azure.Commands.Reso
            Mandatory = false,
            Position = 5,
            ValueFromPipelineByPropertyName = true)]
-        public string Name { get; set; }
+        public string ImageReferenceId { get; set; }


@hyonholee changing positions of parameters is a breaking change

cormacpayne · 2017-02-08T03:33:35Z

On-demand: http://azuresdkci.cloudapp.net/view/1-AzurePowerShell/job/powershell-demand/1389/

hyonholee and others added 30 commits February 2, 2017 22:50

Update Compute client library version to 14.0.0 and re-generate Power…

0795b48

…shell code with DiskRP support

Fix VM output issue when AvailabilitySet is set with Compact display …

a5a6d83

…mode

Update VM helper cmdlets to support Managed disk

0a93ca8

Update New-AzureRmAvailabilitySet and Get-AzureRmAvailabilitySet to s…

cb21520

…upport Managed disk

Update change log for Compute

c2ef923

WORK: PS test for Images

a852301

WORK: add images PS tests

50ee244

Fix tab space

00895c3

Adding test for ConvertTo-AzureRmVMManagedDisk

79a26ef

Fix tab spacing

edc0e2f

Change CreationDataCreateOption to CreateOption

c7c12aa

Add Update-AzureRmAvailabilitySet cmdlet

8eba017

Add SourceVaultId to Key encryption key commands

1e62a7a

Add Access and DurationIdSecond to Grant Access cmdlets

486b7c1

Add StorageAccountType to Add-AzureRmVmssDataDisk and rename ManagedD…

7c5526f

…iskStorageAccountType to StorageAccountType

Update image tests.

78da58f

Update the markdown and help files.

d820aa1

disk tests

5fe80b6

Powershell tests for VMSS with managed disks

cbe43c0

update compute test proj file to include any recorded files.

38ee3fd

Merge pull request #9 from soshekar/release-3.5.0

dd0e024

Powershell tests for VMSS with managed disks

adding disk tests

cd62060

adding disk and snapshot test

b109163

records

55264d1

session records

253639e

Merge branch 'release-3.5.0' into release-3.5.0

4840e8e

Merge pull request #10 from carissasun/release-3.5.0

75e8090

disk and snapshot tests

Add TestVirtualMachine_Managed test.

8de1e29

Merge branch 'release-3.5.0' of https://github.com/prashmo/azure-powe…

a40cc8f

…rshell into release-3.5.0

Add session record files.

3f41043

hyonholee and others added 9 commits February 6, 2017 22:01

Merge branch 'release-3.5.0' of https://github.com/prashmo/azure-powe…

a79c998

…rshell into release-3.5.0

Merge pull request #11 from prashmo/release-3.5.0

2a8c7b0

Add TestVirtualMachine_Managed test.

Add more validation for disk rp tests.

223c199

Merge branch 'release-3.5.0' of https://github.com/hyonholee/azure-po…

7a16669

…wershell into release-3.5.0

Remove tabs in VirtualMachineTests.ps1 file

43c7a6e

Add Remove-AzureRmDataDisk test

76fbd4e

Add Set-AzureRmDiskImage and Set-AzureRmSnapshotImage to the tests

2b75a10

Add Update-AzureRmImage cmdlet and set alias Name for ImageName param…

d65ccfd

…eter.

Update help and markdown files.

dd452df

azurecla added the cla-not-required label Feb 7, 2017

hyonholee added 8 commits February 7, 2017 13:32

Revert auto-generated cmdlets to manually written cmdlets.

d9693bc

Add ShouldProcess support to Update, Grant, Revoke, Start, Restart cm…

de6f08e

…dlets

Add ShouldProcess support to config cmdlets

da985a2

Updating Remove-AzureRmVmssExtension cmdlet back to previous.

81fc47d

Remove Get-AzureRmContainerServiceListNext because it is included Get…

47fe1f9

…-AzureRmContainerService

Adding Add-AzureRmVmssDataDisk and Remove-AzureRmVmssDataDisk

2316a7a

Update help and markdown files.

e819645

Update the order of the paramters not to break existing ones.

40d2e44

cormacpayne suggested changes Feb 8, 2017

View reviewed changes

hyonholee added 2 commits February 7, 2017 17:55

Update the markdown file.

3b87009

Limit the max number of position value to 3 for new cmdlets.

2e5388f

hyonholee force-pushed the release-3.5.0 branch from f8eee67 to 2e5388f Compare February 8, 2017 02:24

Suppress breaking change exceptions.

e03f9b1

cormacpayne approved these changes Feb 8, 2017

View reviewed changes

cormacpayne merged commit 838fb6e into Azure:release-3.5.0 Feb 8, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[AzureRT] Disk RP cmdlets #3471

[AzureRT] Disk RP cmdlets #3471

Uh oh!

hyonholee commented Feb 7, 2017 •

edited

Loading

Uh oh!

cormacpayne left a comment

Uh oh!

cormacpayne Feb 7, 2017

Uh oh!

cormacpayne Feb 7, 2017

Uh oh!

cormacpayne Feb 7, 2017

Uh oh!

cormacpayne Feb 7, 2017

Uh oh!

hyonholee Feb 8, 2017

Uh oh!

cormacpayne Feb 7, 2017

Uh oh!

cormacpayne Feb 8, 2017

Uh oh!

hyonholee Feb 8, 2017

Uh oh!

cormacpayne Feb 8, 2017

Uh oh!

cormacpayne Feb 8, 2017

Uh oh!

cormacpayne Feb 8, 2017

Uh oh!

cormacpayne Feb 8, 2017

Uh oh!

cormacpayne commented Feb 8, 2017

Uh oh!

Uh oh!

		@@ -1,68 +0,0 @@
		// ----------------------------------------------------------------------------------

		@@ -0,0 +1,97 @@
		// ----------------------------------------------------------------------------------

[AzureRT] Disk RP cmdlets #3471

[AzureRT] Disk RP cmdlets #3471

Uh oh!

Conversation

hyonholee commented Feb 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

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!

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!

cormacpayne commented Feb 8, 2017

Uh oh!

Uh oh!

hyonholee commented Feb 7, 2017 •

edited

Loading