Adding subnet delegation cmdlets #7220

rupalivohra · 2018-09-12T19:32:35Z

Description

Adding PS support for CRUD operations for delegations on subnets

Checklist

I have read the Submitting Changes section of CONTRIBUTING.md
The title of the PR is clear and informative
The appropriate change log has been updated
The PR does not introduce breaking changes
If applicable, the changes made in the PR have proper test coverage
For public API changes to cmdlets:
- the changes have gone through a cmdlet design review
  - Review Link: https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/157
- the cmdlet markdown files were generated using the platyPS module

src/ResourceManager/Network/Commands.Network/ChangeLog.md

praries880 · 2018-09-12T19:40:33Z

@rupalivohra

We need a design review link for the PR, information here on how to open one.
There are no tests in the PR, we would need all the cmdlets to be tested.
Help file generation documentation here.

rupalivohra · 2018-09-13T00:20:44Z

@praries880 Can you please review the code that is here in parallel? I'm in the process of getting through the cmdlet review and adding tests.

praries880 · 2018-09-13T01:22:00Z

@rupalivohra Follow instructions inthis document to add alias mappings for netcore.

Remove the file tools/AzureRM/AzureRM.psm1

...anager/Network/Commands.Network/VirtualNetwork/Subnet/AzureVirtualNetworkSubnetConfigBase.cs

...nager/Network/Commands.Network/VirtualNetwork/Subnet/Delegation/NewAzureDelegationCommand.cs

src/ResourceManager/Network/Commands.Network/help/New-AzureRmDelegation.md

src/ResourceManager/Network/Commands.Network/help/New-AzureRmVirtualNetworkSubnetConfig.md

src/ResourceManager/Network/Commands.Network/help/Set-AzureRmVirtualNetworkSubnetConfig.md

praries880

@rupalivohra yuou need to fix the following static analysis errors :

AssemblyFileName	ClassName	Target	Severity	ProblemId	Description	Remediation
c:\workspace\powershell\src\Package\Debug\ResourceManager\AzureResourceManager\AzureRM.Network\Microsoft.Azure.Commands.Network.dll	Microsoft.Azure.Commands.Network.AddAzureSubnetDelegation	Add-AzureRmDelegation	1	8100	Add-AzureRmDelegation Does not support ShouldProcess but the cmdlet verb Add indicates that it should.	Determine if the cmdlet should implement ShouldProcess and if so determine if it should implement Force / ShouldContinue
c:\workspace\powershell\src\Package\Debug\ResourceManager\AzureResourceManager\AzureRM.Network\Microsoft.Azure.Commands.Network.dll	Microsoft.Azure.Commands.Network.VirtualNetwork.Subnet.RemoveAzureSubnetDelegation	Remove-AzureRmDelegation	1	8100	Remove-AzureRmDelegation Does not support ShouldProcess but the cmdlet verb Remove indicates that it should.	Determine if the cmdlet should implement ShouldProcess and if so determine if it should implement Force / ShouldContinue
c:\workspace\powershell\src\Package\Debug\ResourceManager\AzureResourceManager\AzureRM.Network\Microsoft.Azure.Commands.Network.dll	Microsoft.Azure.Commands.Network.VirtualNetwork.Subnet.NewAzureDelegationCommand	New-AzureRmDelegation	1	8100	New-AzureRmDelegation Does not support ShouldProcess but the cmdlet verb New indicates that it should.	Determine if the cmdlet should implement ShouldProcess and if so determine if it should implement Force / ShouldContinue
c:\workspace\powershell\src\Package\Debug\ResourceManager\AzureResourceManager\AzureRM.Network\Microsoft.Azure.Commands.Network.dll	Microsoft.Azure.Commands.Network.AddAzureSubnetDelegation	Add-AzureRmDelegation	1	8100	Add-AzureRmDelegation Does not support ShouldProcess but the cmdlet verb Add indicates that it should.	Determine if the cmdlet should implement ShouldProcess and if so determine if it should implement Force / ShouldContinue
c:\workspace\powershell\src\Package\Debug\ResourceManager\AzureResourceManager\AzureRM.Network\Microsoft.Azure.Commands.Network.dll	Microsoft.Azure.Commands.Network.VirtualNetwork.Subnet.RemoveAzureSubnetDelegation	Remove-AzureRmDelegation	1	8100	Remove-AzureRmDelegation Does not support ShouldProcess but the cmdlet verb Remove indicates that it should.	Determine if the cmdlet should implement ShouldProcess and if so determine if it should implement Force / ShouldContinue
c:\workspace\powershell\src\Package\Debug\ResourceManager\AzureResourceManager\AzureRM.Network\Microsoft.Azure.Commands.Network.dll	Microsoft.Azure.Commands.Network.VirtualNetwork.Subnet.NewAzureDelegationCommand	New-AzureRmDelegation	1	8100	New-AzureRmDelegation Does not support ShouldProcess but the cmdlet verb New indicates that it should.	Determine if the cmdlet should implement ShouldProcess and if so determine if it should implement Force / ShouldContinue

praries880 · 2018-09-13T18:01:14Z

src/ResourceManager/Network/Commands.Network.Test/ScenarioTests/VirtualNetworkTests.ps1

+.DESCRIPTION
+SmokeTest
+#>
+function Test-subnetDelegationCRUD


you need to add the test recodings for the tests to both git and the project. Example here and here](https://github.com/Azure/azure-powershell/blob/preview/src/ResourceManager/Network/Commands.Network.Test/Commands.Network.Test.csproj#L321).

static analysis errors should be fixed in the latest build.
Running a local build to re-run tests before I add the recordings. Should be there soon.

I added these cmdlets to the exception, but I'm still experiencing build failures :(

...nager/Network/Commands.Network/VirtualNetwork/Subnet/Delegation/NewAzureDelegationCommand.cs

...anager/Network/Commands.Network/VirtualNetwork/Subnet/AzureVirtualNetworkSubnetConfigBase.cs

praries880 · 2018-09-13T18:52:23Z

@rupalivohra can you pull the latest changes in Azure:network-september-release

I am not sure why is this failing : https://travis-ci.org/Azure/azure-powershell/builds/428268315#L1652

tools/StaticAnalysis/Exceptions/AzureRM.Network/SignatureIssues.csv

maddieclayton · 2018-09-14T04:41:23Z

@rupalivohra retargeted the PR, please resolve the merge conflict when making other changes

maddieclayton · 2018-09-14T05:26:59Z

@rupalivohra Fixed the build failure and update two more of the delegation descriptions with the update description. Should be good to go as long as the build passes this time. Will check either late tonight or tomorrow morning.

rupalivohra · 2018-09-14T14:33:20Z

@maddieclayton thanks for the fix. It looks like unrelated tests are failing in the default build. Can we ignore those and check in regardless?
I also see these signature issues again, but I've already added them to exceptions. Is there anything else I need to do? If the ci build succeeded, does that mean these can be ignored?

Add-AzureRmDelegation   Does not support ShouldProcess but the cmdlet verb Add indicates that it   should.
Remove-AzureRmDelegation Does not support   ShouldProcess but the cmdlet verb Remove indicates that it should.
New-AzureRmDelegation Does not support   ShouldProcess but the cmdlet verb New indicates that it should.

maddieclayton · 2018-09-14T16:30:30Z

@rupalivohra Looks like the names of the paths changed since the last time the signature issues were ran - fixed it. Letting CI run now, hopefully all the tests will pass as they seem to be unrelated. We cannot merge this without CI passing, but I will gate creating the second release candidate on merging this PR so no need to worry about that.

maddieclayton · 2018-09-14T17:40:35Z

@rupalivohra Looking at the test failures now

rupalivohra · 2018-09-14T17:46:51Z

Failing tests in build:
From Microsoft.Azure.Commands.Profile.Test: Microsoft.Azure.Commands.ResourceManager.Common.Test.AzureRMProfileTests.LoadingProfileWorks<br clear="all" /><pre>System.IO.DirectoryNotFoundException : Could not find a part of the path 'C:\Users\fxsign\AppData\Roaming\.Azure\TokenCache.dat'

From Microsoft.WindowsAzure.Commands.Test:
Azure Authoring Tools are not installed and are required

Changing to published SDK Adding help documentation for CRUD ops on delegations

MikhailTryakhov · 2018-09-14T20:04:09Z

@maddieclayton @praries880 ready for merge

addressed

maddieclayton added the Cmdlet Review Required ⚠️ label Sep 12, 2018

praries880 suggested changes Sep 12, 2018

View reviewed changes

src/ResourceManager/Network/Commands.Network/ChangeLog.md Outdated Show resolved Hide resolved

maddieclayton assigned rupalivohra Sep 12, 2018

azuresdkci1x added the needs-review label Sep 12, 2018

rupalivohra force-pushed the delegation branch 2 times, most recently from b07f81a to f7b7b2d Compare September 12, 2018 22:28

rupalivohra force-pushed the delegation branch 2 times, most recently from fe79b8e to ea5e1ff Compare September 13, 2018 01:09

praries880 suggested changes Sep 13, 2018

View reviewed changes

praries880 added needs-revision and removed needs-review labels Sep 13, 2018

rupalivohra force-pushed the delegation branch 7 times, most recently from 8b0b261 to 9375cd9 Compare September 13, 2018 17:28

praries880 suggested changes Sep 13, 2018

View reviewed changes

praries880 previously requested changes Sep 13, 2018

View reviewed changes

...nager/Network/Commands.Network/VirtualNetwork/Subnet/Delegation/NewAzureDelegationCommand.cs Outdated Show resolved Hide resolved

...anager/Network/Commands.Network/VirtualNetwork/Subnet/AzureVirtualNetworkSubnetConfigBase.cs Show resolved Hide resolved

rupalivohra force-pushed the delegation branch from 9375cd9 to ecb41cb Compare September 13, 2018 19:09

praries880 added the Merge Conflicts label Sep 13, 2018

praries880 reviewed Sep 13, 2018

View reviewed changes

tools/StaticAnalysis/Exceptions/AzureRM.Network/SignatureIssues.csv Outdated Show resolved Hide resolved

rupalivohra force-pushed the delegation branch 3 times, most recently from 87c5aaf to 9850ea2 Compare September 13, 2018 21:20

maddieclayton added Ignite and removed needs-review 👀 labels Sep 14, 2018

maddieclayton changed the base branch from network-september-release to release-2018-09-17 September 14, 2018 04:37

rupalivohra force-pushed the delegation branch from 0af412f to faeff1b Compare September 14, 2018 04:38

rupalivohra force-pushed the delegation branch 2 times, most recently from 46426ea to 08843c5 Compare September 14, 2018 04:50

rupalivohra added this to the 2018-09-17 - Ignite milestone Sep 14, 2018

maddieclayton removed the needs-revision label Sep 14, 2018

Rupali Vohra and others added 11 commits September 14, 2018 11:06

Adding subnet delegation cmdlets

df32a96

Changing to published SDK Adding help documentation for CRUD ops on delegations

Modifications to existing subnet commands

f4affad

Updating AzureRm <-> Az mappings

0c0c99e

Adding tests

3caff88

Adding psm1 to try and fix build

a4fdbd8

Update Commands.Network.csproj

e04ef74

Update Set-AzureRmVirtualNetworkSubnetConfig.md

4ddeaa6

Update New-AzureRmVirtualNetworkSubnetConfig.md

912fded

Update Mappings.cs

c4ab084

Update SignatureIssues.csv

c70aad4

Update SignatureIssues.csv

284ef53

rupalivohra force-pushed the delegation branch from bb9688b to 284ef53 Compare September 14, 2018 18:13

maddieclayton approved these changes Sep 14, 2018

View reviewed changes

maddieclayton merged commit b797ada into Azure:release-2018-09-17 Sep 14, 2018

Adding subnet delegation cmdlets #7220

Adding subnet delegation cmdlets #7220

Uh oh!

Conversation

rupalivohra commented Sep 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

Uh oh!

praries880 commented Sep 12, 2018

Uh oh!

rupalivohra commented Sep 13, 2018

Uh oh!

praries880 commented Sep 13, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

praries880 left a comment

Choose a reason for hiding this comment

Uh oh!

praries880 Sep 13, 2018

Choose a reason for hiding this comment

Uh oh!

rupalivohra Sep 13, 2018

Choose a reason for hiding this comment

Uh oh!

rupalivohra Sep 13, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

praries880 commented Sep 13, 2018

Uh oh!

Uh oh!

maddieclayton commented Sep 14, 2018

Uh oh!

maddieclayton commented Sep 14, 2018

Uh oh!

rupalivohra commented Sep 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maddieclayton commented Sep 14, 2018

Uh oh!

maddieclayton commented Sep 14, 2018

Uh oh!

rupalivohra commented Sep 14, 2018

Uh oh!

MikhailTryakhov commented Sep 14, 2018

Uh oh!

Uh oh!

rupalivohra commented Sep 12, 2018 •

edited

Loading

rupalivohra commented Sep 14, 2018 •

edited

Loading