Chef extension ARM commands #2587

NimishaS · 2016-07-08T12:08:09Z

Using new classes like CreateOrUpdateWithHttpMessagesAsync for Set-AzureRmVMChefExtension
Passing settings file as Hashtable
Implements Get-AzureRmVMChefExtension
Implements Remove-AzureRmVMChefExtension

…s file as hash

…_client_name are passed separately

…ChefExtension

azurecla · 2016-07-08T12:08:15Z

Hi @NimishaS, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, AZPRBOT;

NimishaS · 2016-07-08T12:29:48Z

@markcowl , Travis is failing but I don't have the permission to see the details. Please help.

The commands are tested and functional.

hovsepm · 2016-07-08T17:58:34Z

Several issues:

Help missing for cmdlet Get-AzureRmVMChefExtension implemented by class Microsoft.Azure.Commands.Compute.Extension.Chef.GetAzureRmVMChefExtension
Help missing for cmdlet Remove-AzureRmVMChefExtension implemented by class Microsoft.Azure.Commands.Compute.Extension.Chef.RemoveAzureRmVMChefExtension

…hefExtension

hovsepm · 2016-07-11T17:16:17Z

src/ResourceManager/Compute/Commands.Compute/Extension/Chef/GetAzureRmVMChefExtension.cs

@@ -0,0 +1,125 @@
+using System;


Please add License header

hovsepm · 2016-07-11T17:29:09Z

@NimishaS please add test coverage.

NimishaS · 2016-07-12T07:16:32Z

@hovsepm we faced some issues in past while running the tests in recording mode. We had followed the documentation: https://github.com/Azure/azure-powershell/wiki/Microsoft-Azure-PowerShell-Developer-Guide#ad-scenario-tests, but couldn't get pass through.

Is it possible for you to schedule an interactive session to help us guide how the test recording can be done? We'll keep the specs ready.

This is the PR which couldn't be merged because of test recording issue: #1676

NimishaS · 2016-07-14T10:24:40Z

@hovsepm , any updates here?

hovsepm · 2016-07-15T20:17:46Z

@NimishaS what I understand from you previous PR is that your tenant ID does not have any subscription. To be able to help you I'll need access to your test environment and devbox. Do you have access to MS corp net?

markcowl · 2016-07-18T19:00:20Z

@azuresdkci retest this please

NimishaS · 2016-07-19T06:46:11Z

@hovsepm we don't have access to MS corp net. I can share the details of a VM which will have our setup. If this approach sounds fine then please share your email id on [email protected].

@markcowl , @hovsepm : Do you expect us to add test suite in this PR only or can we add that in a separate PR? Finishing test suite will take some time as we need to co-ordinate with you for that.

markcowl · 2016-07-19T20:13:59Z

@NimishaS As long as you have tested the cmdlets thoroughly, you commit to testing them in the candidate installer during the release, and you file an issue to add the test suite (and make sure to finish before the end of next sprint), then we can defer this. If you decide to go this route, please add a reference to the github issue for adding the tests here.

NimishaS · 2016-07-20T06:52:08Z

@markcowl , created #2638. Please assign this issue to me. I don't have the rights.
We have tested the cmdlets thoroughly and will also take up the task to test them during the release on high priority. Since we are ruby developers and new to C# and visual studio, it's not clear to me what it means to test in candidate installer. Can you please share some links?

markcowl · 2016-07-20T22:59:18Z

src/ResourceManager/Compute/Commands.Compute/Extension/Chef/RemoveAzureRmVMChefExtension.cs

+{
+    [Cmdlet(
+        VerbsCommon.Remove,
+        ProfileNouns.VirtualMachineChefExtension)]


SupportsShouldProcess=true

markcowl · 2016-07-20T23:00:14Z

src/ResourceManager/Compute/Commands.Compute/Extension/Chef/RemoveAzureRmVMChefExtension.cs

+                this.Name = ExtensionDefaultName;
+            }
+
+            ExecuteClientAction(() =>


Protect with PSCmdlet.ConfirmAction(target, processMessage, Action)

markcowl · 2016-07-20T23:03:29Z

@NimishaS, A couple of additoonal comments. Please see: https://gist.github.com/markcowl/338e16fe5c8bbf195aff9f8af0db585d For more information on implementing the ShouldProcess pattern using ConfirmAction. We are now requiring this on all new cmdlets (and wnever a cmdlet is changed)

markcowl · 2016-07-22T22:07:15Z

@NimishaS Please update per the review comments. Your invitation is here: https://github.com/Azure/azure-powershell/invitations this will enable you to assign yourself to the issue (and close when it is done)

NimishaS · 2016-07-23T03:01:12Z

Sorry @markcowl, I had got busy with knife-ec2 release stuff.I will update for the review comments.

NimishaS · 2016-07-26T06:53:01Z

@markcowl , I have got a little occupied with knife-ec2 release. I will be picking this up again in 2-3 days, once knife-ec2 is out.

NimishaS · 2016-07-27T10:31:54Z

@markcowl , added SupportsShouldProcess and ConfirmAction for Set-AzureRmVMChefExtension and Remove-AzureRmVMChefExtension commands. Also tested with -confirm option.

NimishaS · 2016-07-29T06:05:39Z

@markcowl I will be able to start with #2638 once this PR is merged as I need to create the PR for #2638 from dev branch. Please review this and let us know if anything needs to be fixed.

markcowl · 2016-07-29T18:05:35Z

on demand run here: http://azuresdkci.cloudapp.net/view/1-AzurePowerShell/job/powershell-demand/1032/

LGTM once the on-demand run passes

nimisha added 11 commits July 5, 2016 10:24

Updated Set-AzureRmVMChefExtension cmd implementation to pass setting…

4cbae31

…s file as hash

Fixed client.rb file parsing error

20778b2

Fixed parsing issues when options like chef_server_url and validation…

0e4ead2

…_client_name are passed separately

Added tests without recording

cac9a72

Added Remove-AzureRmVMChefExtension command in ARM

9ed63a6

Implemented Get-AzureRmVMChefExtension command

fea53ee

Updated specs to test Remove-AzureRmVMChefExtension and Get-AzureRmVM…

5a4b990

…ChefExtension

Added BootstrapVersion and UninstallChefClient options

d234d67

Fetching latest extension version

c4dac2f

Removed unwanted options and updated help file

503c9c4

Removed specs for now

a572ad2

azurecla added the cla-not-required label Jul 8, 2016

Updated helpfile for Get-AzureRmVMChefExtension and Remove-AzureRmVMC…

80a9010

…hefExtension

hovsepm reviewed Jul 11, 2016
View reviewed changes

Added license header

bde7d03

NimishaS mentioned this pull request Jul 20, 2016

Add test coverage for Chef Extension ARM commands #2638

Closed

markcowl reviewed Jul 20, 2016
View reviewed changes

Added SupportsShouldProcess and ConfirmAction

b868867

markcowl merged commit 6aad8f4 into Azure:dev Jul 29, 2016

NimishaS deleted the nim/Update_Set-AzureRmVMChefExtension branch August 1, 2016 10:21

NimishaS mentioned this pull request Aug 2, 2016

Tests for chef extension arm commands #2713

Merged

Chef extension ARM commands #2587

Chef extension ARM commands #2587

Uh oh!

Conversation

NimishaS commented Jul 8, 2016

Uh oh!

azurecla commented Jul 8, 2016

Uh oh!

NimishaS commented Jul 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hovsepm commented Jul 8, 2016

Uh oh!

hovsepm Jul 11, 2016

Choose a reason for hiding this comment

Uh oh!

hovsepm commented Jul 11, 2016

Uh oh!

NimishaS commented Jul 12, 2016

Uh oh!

NimishaS commented Jul 14, 2016

Uh oh!

hovsepm commented Jul 15, 2016

Uh oh!

markcowl commented Jul 18, 2016

Uh oh!

NimishaS commented Jul 19, 2016

Uh oh!

markcowl commented Jul 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NimishaS commented Jul 20, 2016

Uh oh!

markcowl Jul 20, 2016

Choose a reason for hiding this comment

Uh oh!

markcowl Jul 20, 2016

Choose a reason for hiding this comment

Uh oh!

markcowl commented Jul 20, 2016

Uh oh!

markcowl commented Jul 22, 2016

Uh oh!

NimishaS commented Jul 23, 2016

Uh oh!

NimishaS commented Jul 26, 2016

Uh oh!

NimishaS commented Jul 27, 2016

Uh oh!

NimishaS commented Jul 29, 2016

Uh oh!

markcowl commented Jul 29, 2016

Uh oh!

Uh oh!

NimishaS commented Jul 8, 2016 •

edited

Loading

markcowl commented Jul 19, 2016 •

edited

Loading