Bump up network sdk version from 17.0.0.preview to 18.0.0.preview #5993

Nilambari · 2018-04-19T18:59:32Z

Description

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
- the cmdlet markdown files were generated using the platyPS module

…nto preview

cormacpayne · 2018-04-19T20:50:45Z

@Nilambari tests are failing due to the change in the API version -- please let us know if you have any issues using the Network recording job on our Jenkins server to mitigate this issue

…nto preview2

Nilambari · 2018-04-20T22:04:33Z

@cormacpayne I have included all the tests recorded with new API version. Please take a look now and help merge the PR. Thanks!

Nilambari · 2018-04-20T23:24:37Z

@cormacpayne Looks like build is failed due to some tests, which are not recorded/included in this PR / owned by us. Can you tell us the next action. Thanks!

…nto preview2

…ervices.Backup, Compute, DNS, and SQL

Nilambari · 2018-04-24T01:07:14Z

@maddieclayton Thanks. Updated the latest Network version in all test csproj files: RecoveryServices.Backup, Compute, DNS, and SQL. Can you help accept this PR?

Thanks,
Nilambari

…nto preview2

Nilambari · 2018-04-26T17:28:43Z

Can you Please help accept this PR? Thanks.

…nto preview2

into preview2

Nilambari · 2018-04-28T00:20:49Z

@maddieclayton Re-recorded failing tests and unskipped them. Only one test: TestConnectivityCheck is skipped in this PR:- This test is failing due to new parameters added to the api and test needs to be updated accordingly. Related swagger changes are done but service team([email protected]) is still working on related PS changes. They said they will own fixing and rerecording this failing test.

Can you Please help accept the PR? Thanks!

markcowl · 2018-04-28T18:24:19Z

src/ResourceManager/Network/Commands.Network/packages.config

@@ -2,5 +2,8 @@
 <packages>
  <package id="AutoMapper" version="6.0.2" targetFramework="net452" />
  <package id="Microsoft.Azure.KeyVault.Core" version="1.0.0" targetFramework="net45" />
-  <package id="Microsoft.Azure.Management.Network" version="17.0.0-preview" targetFramework="net452" />
+  <package id="Microsoft.Azure.Management.Network" version="18.0.0-preview" targetFramework="net452" />


All of the changes to this file other than the network client management update must be removed.

markcowl · 2018-04-28T18:43:00Z

src/ResourceManager/Network/Commands.Network.Test/ScenarioTests/NetworkWatcherAPITests.cs

@@ -75,7 +75,7 @@ public void TestFlowLog()
            NetworkResourcesController.NewInstance.RunPsTest("Test-FlowLog");
        }

-        [Fact]
+        [Fact(Skip = "Test is failing due to design changes. Service team is going to re-record and submit it.")]


Is this failing due to design changes that impact cmdlets, or design changes that impact test setup?

Service team said they need to fix the test and re-record it.

markcowl · 2018-04-28T18:44:01Z

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

@@ -21,8 +21,9 @@ function Test-ExpressRouteBGPServiceCommunities
 	$communities = Get-AzureRmBgpServiceCommunity

 	Assert-NotNull $communities
-	Assert-NotNull $communities[0].BgpCommunities
-	Assert-AreEqual true $communities[0].BgpCommunities[0].IsAuthorizedToUse
+	$crmOnlineCommunity = $communities | Where-Object {$_.ServiceName -match "CRMOnline"}


This seems to be testing a different property in the output. Is the old property no longer available? Why is it OK not to test for the old property?

No, they are testing same property. Just previously, it was returning only one entry, now it returns many entries and we need to specify which entry param we need to validate in the test.

markcowl · 2018-04-28T18:44:24Z

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

@@ -51,7 +51,7 @@ function Test-VirtualNetworkExpressRouteGatewayCRUD
      $expected = Get-AzureRmVirtualNetworkGateway -ResourceGroupName $rgname -name $rname
      Assert-AreEqual $expected.ResourceGroupName $actual.ResourceGroupName	
      Assert-AreEqual $expected.Name $actual.Name	
-      #Assert-AreEqual "ExpressRoute" $expected.GatewayType
+      Assert-AreEqual "ExpressRoute" $expected.GatewayType


Thanks for putting the asserts back

markcowl · 2018-04-28T18:46:16Z

src/ResourceManager/Network/Commands.Network/Commands.Network.csproj

    </Reference>
+    <Reference Include="Microsoft.Rest.ClientRuntime, Version=2.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL" />


Do not add these bacj - these come from the common targets you have imported. The only dependency change in this file should be the network management client version

markcowl · 2018-04-28T18:53:01Z

...rk/Commands.Network/ExpressRouteCircuit/Peering/AzureExpressRouteCircuitPeeringConfigBase.cs

-           MNM.ExpressRouteCircuitPeeringType.AzurePrivatePeering,
-           MNM.ExpressRouteCircuitPeeringType.AzurePublicPeering,
-           MNM.ExpressRouteCircuitPeeringType.MicrosoftPeering,
+           MNM.ExpressRoutePeeringType.AzurePrivatePeering,


Are we sure these values are the same?

Yes, they are same.

markcowl · 2018-04-28T18:54:25Z

src/ResourceManager/Network/Commands.Network/Microsoft.Azure.Commands.Network.format.ps1xml

@@ -2604,5 +2608,39 @@
        </CustomEntries>
      </CustomControl>
    </View>
+    <View>


Note: in the review, we asked that you add a table control, as table view should be the default. We can let this go for this release, but we will need to change this soon for all of these cmdlets.

I will check with you offline and make this change in my next PS PR.

markcowl · 2018-04-28T18:55:59Z

src/ResourceManager/Network/Commands.Network/Models/PSVirtualNetwork.cs

@@ -31,10 +31,12 @@ public class PSVirtualNetwork : PSTopLevelResource, IResourceReference

        public string ProvisioningState { get; set; }

-        public bool? EnableDDoSProtection { get; set; }
+        public bool? EnableDdosProtection { get; set; }


This is marked as a breaking change, but you have used this spelling throughout. Is this a product level decision to change the way DDOS is spelled? If so, this is good, and you can simply suppress the breakign change warning, as, in PowerShell script, either 'DDos' or 'Ddos' in a script will find either value.

Hi Mark, We have used the same casingin our resource on the networking provider and during reviews. We will suppress the warning.

markcowl · 2018-04-28T18:57:40Z

src/ResourceManager/Network/Commands.Network/Models/PSVirtualNetwork.cs


        public bool? EnableVmProtection { get; set; }

+        public PSResourceId DdosProtectionPlan { get; set; }


We have used Ddos everywhere.

markcowl · 2018-04-28T19:05:56Z

src/ResourceManager/Network/Commands.Network/VirtualNetwork/NewAzureVirtualNetworkCommand.cs

+
+        [Parameter(
+            Mandatory = false,
+            HelpMessage = "A switch parameter which represents whether DDoS protection is enabled or not. It can only be turned on if a DDoS Protection Plan is associated with the virtual network.")]


Is this a breaking change? If so, you need to ensure that you document this in the 'upcoming braking changes' document in network so that customers know the change is breaking.

-Also, since the protection plan is required, You should create a new parameter set that makes both of these parameters mandatory

This functionality was only in private preview. We have never GAd this ability. However, we will add it in upcoming braking change. We will also create a new parameter set.

We will add the parameter set in the next release. We added the breaking change documentation

markcowl · 2018-04-28T19:50:07Z

on demand: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/784/

signing: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/ps-sign/504/

into preview2

markcowl · 2018-04-29T22:49:16Z

signing: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/ps-sign/508/

on demand: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/785/

murilogr and others added 21 commits April 3, 2018 19:23

Update SDK version

9788293

Bump up network sdk version from 17.0.0.preview to 18.0.0.preview

95ca31f

Add implementations

882c770

Update list of operations

2ba5085

Add tests

6e8b869

Merge branch 'preview' of https://github.com/Azure/azure-powershell i…

f292f3b

…nto preview

Add new help

364ddfc

Add tests for Set-VirtualNetwork

23bb0f8

Add test recordings

5365497

Rerun help with latest platyps

9dc1221

Remove duplicated files

c50dd42

Add change to ChangeLog.md

8146fe1

Add examples

a3470b1

Remove unnecessary ValueFromPipelineByPropertyName

2629662

Remove Force flags

61eaa87

Update help and examples

36183fc

Add boilerplate description for BGP

ed42572

Add test recordings to csproj

ae07722

fix Build merge errors

080fd2c

fix Build merge errors

321cb11

Merge branch 'preview' into ddosProtectionPlan

dac2b82

azuresdkci added the needs-review label Apr 19, 2018

cormacpayne assigned Nilambari Apr 19, 2018

cormacpayne added needs-revision and removed needs-review labels Apr 19, 2018

Nilambari added 2 commits April 20, 2018 15:02

Record all network tests

ee4b062

Merge branch 'preview' of https://github.com/Azure/azure-powershell i…

4fd1a8a

…nto preview2

Nilambari added 2 commits April 23, 2018 17:41

Merge branch 'preview' of https://github.com/Azure/azure-powershell i…

1ecfdb4

…nto preview2

Update the latest Network version in all test csproj files: RecoveryS…

766e148

…ervices.Backup, Compute, DNS, and SQL

Nilambari and others added 4 commits April 24, 2018 15:17

Merge branch 'preview' of https://github.com/Azure/azure-powershell i…

a5eb1c5

…nto preview2

Record failing tests

fefe249

Suppress cred in failing test

aaed159

Re-recording Private DNS tests.

d11bd93

markcowl added the Post Build label Apr 25, 2018

Nilambari added 3 commits April 27, 2018 13:03

Merge branch 'preview' of https://github.com/Azure/azure-powershell i…

82aafbe

…nto preview2

Merge branch 'preview2' of https://github.com/Nilambari/azure-powershell

ac22762

into preview2

Merge PR:Azure#5968

f1969cd

Nilambari mentioned this pull request Apr 27, 2018

[Microsoft.Network] DDoS protection plans #5968

Closed

8 tasks

Re-record failing tests and unskip them

e76e76b

markcowl requested changes Apr 28, 2018

View reviewed changes

markcowl changed the base branch from preview to release-6.0.0 April 28, 2018 19:09

Merge branch 'release-6.0.0' into preview2

6492d60

Nilambari and others added 3 commits April 28, 2018 14:10

Incorporate code review comments

d4bd842

Merge branch 'preview2' of https://github.com/Nilambari/azure-powershell

9ad2bc7

into preview2

Handle breaking changes

3869e4c

markcowl approved these changes Apr 29, 2018

View reviewed changes

markcowl added Waiting for On Demand and removed Post Build needs-revision labels Apr 29, 2018

markcowl merged commit 5919291 into Azure:release-6.0.0 Apr 30, 2018

maddieclayton mentioned this pull request May 7, 2018

Bump Network SDK version to 19.0.0-preview #6132

Merged

8 tasks

maddieclayton mentioned this pull request May 23, 2018

Expose new SKUs for VMSS Vpn gateways #6277

Closed

8 tasks

		</Reference>
		<Reference Include="Microsoft.Rest.ClientRuntime, Version=2.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL" />


		public bool? EnableVmProtection { get; set; }

		public PSResourceId DdosProtectionPlan { get; set; }

Bump up network sdk version from 17.0.0.preview to 18.0.0.preview #5993

Bump up network sdk version from 17.0.0.preview to 18.0.0.preview #5993

Uh oh!

Conversation

Nilambari commented Apr 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

cormacpayne commented Apr 19, 2018

Uh oh!

Nilambari commented Apr 20, 2018

Uh oh!

Nilambari commented Apr 20, 2018

Uh oh!

Nilambari commented Apr 24, 2018

Uh oh!

Nilambari commented Apr 26, 2018

Uh oh!

Nilambari commented Apr 28, 2018

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!

Choose a reason for hiding this comment

Uh oh!

Nilambari Apr 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avijitgupta Apr 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

markcowl commented Apr 28, 2018

Uh oh!

markcowl commented Apr 29, 2018

Uh oh!

Uh oh!

Nilambari commented Apr 19, 2018 •

edited

Loading

Nilambari Apr 28, 2018 •

edited

Loading

avijitgupta Apr 28, 2018 •

edited

Loading