Update Batch cmdlets to use latest management plane and data plane sdk

[matthchr]

Description

• Update Batch cmdlets to use latest version of the management plane and data plane SDKs.
• This PR does included breaking changes

Checklist

• I have read the Submitting Changes section of CONTRIBUTING.md
• The title of the PR is clear and informative
• The appropriate ChangeLog.md file(s) 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:
  • a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
  • the markdown help files have been regenerated using the commands listed here

[matthchr]

Cmdlet review for the new cmdlets added in this PR was completed here: https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/547

[matthchr]

I still need to update the management plane SDK reference so don't merge this yet.

[matthchr]

This is good to merge now -- the management plane SDK has been updated

[bgklein]

One question if these changes were intentional

[matthchr]

@bgklein - Yeah, that happened when I regenerated the help text, so I assume that it's a change pushed by the powershell core devs.

[isra-fel]

@bgklein - Yeah, that happened when I regenerated the help text, so I assume that it's a change pushed by the powershell core devs.

Matthew, you probably need to use -UseFullTypeName when updating the documents.
See https://github.com/Azure/azure-powershell/blob/master/documentation/development-docs/help-generation.md#updating-all-markdown-files-in-a-module

[isra-fel]

Hi Matthew, your design review stated that this was for a preview release, right? If that's still the case, I'll create a branch for your PR, it contains breaking change so it cannot be merged into master.

[matthchr]

@isra-fel - we changed our plans to merge as a master release when we realized that there is a breaking change planned for 5/7 (code complete) as part of the S169 milestone - since there is a breaking change planned for 5/7 we figured it made sense to go in there rather than wait another 6 months.

The only reason we were originally planning private preview is because this included breaking changes and we hadn't yet realized that a breaking change release was imminent.

[matthchr]

Will fix the issue with help documents and -UseFullTypeName

[isra-fel]

@matthchr That's totally fine. Thought I highly recommend adding breaking change attributes ahead of breaking change release to surprise our customers less next time :)

[isra-fel]

There's no need to use nullable types for optional parameters (a [Parameter] is by default not mandatory)

[matthchr]

The issue here is that I really need 3 states.

1. Kind == null which means "use the old defaulting behavior, CER if no password, PFX if password"
2. Kind == CER - the data is a certificate.
3. Kind == PFX - The data is a PFX.

If I make this non-nullable I need some other way to determine if this parameter was specified or not - it seemed easier to just let the parameter itself support 3 states, "not specified" (null), and the other two.

If there's a better way to do this let me know (see below in the ExecuteCmdletImpl where this logic is applied and you see the defaulting behavior).

[isra-fel]

I think the problem with a nullable + optional parameter is the user may think that passing null or not specifying this param may have different results.

Besides, making it not-nullable, i.e. changing the first state to "user not specify Kind" won't make the code much more complex:

- this.Kind ?? (password == null ? PSCertificateKind.Cer : PSCertificateKind.Pfx),
+ this.IsParameterBound(c => c.Kind) ? (password == null ? PSCertificateKind.Cer : PSCertificateKind.Pfx) : null

[matthchr]

Thanks @isra-fel - I was not familiar with IsparameterBound - will use that isntead.

[matthchr]

I pushed an update which does as you suggested @isra-fel

[matthchr]

Thought I highly recommend adding breaking change attributes ahead of breaking change release to surprise our customers less next time :)

@isra-fel - Yep, totally agree. Part of the issue this time is that these changes were for JEDI and so we didn't have a lot of lead time, so the changes were implemented quickly and we didn't get a chance to do much ahead of time notification. But will keep your suggestion in mind for future changes (cc @bgklein).

[matthchr]

I also removed all of the new cmdlets which were duplicate from what the networking team already had (as per email thread we had going on). I think that this should be good to merge now!

[matthchr]

All of our tests are failing in CI due to:

EXEC : PowerShell error Record: Could not load file or assembly 'Microsoft.Azure.Management.Network, Version=19.21.1.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35'. [D:\a\1\s\build.proj] 
Exception:System.IO.FileLoadException: Could not load file or assembly 'Microsoft.Azure.Management.Network, Version=19.21.1.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35'. 
at System.Management.Automation.Runspaces.InitialSessionState.Bind_LoadAssemblies(ExecutionContext context) 
at System.Management.Automation.Runspaces.InitialSessionState.Bind(ExecutionContext context, Boolean updateOnly, PSModuleInfo module, Boolean noClobber, Boolean local) 
at System.Management.Automation.Runspaces.InitialSessionState.Bind(ExecutionContext context, Boolean updateOnly) 
at Microsoft.PowerShell.Commands.ModuleCmdletBase.LoadModuleManifest(String moduleManifestPath, ExternalScriptInfo manifestScriptInfo, Hashtable data, Hashtable localizedData, ManifestProcessingFlags manifestProcessingFlags, Version minimumVersion, Version maximumVersion, Version requiredVersion, Nullable`1 requiredModuleGuid, ImportModuleOptions& options, Boolean& containedErrors) 
Details: 
Script Stack Trace: at <ScriptBlock>, <No file>: line 1 
: Target:

@isra-fel, can you help on figuring out why our tests are failing to import this assembly? From what I can tell, the nuget package I am referencing (Microsoft.Azure.Management.Network 19.20.0-preview) has AssemblyVersion=19.5.0.0, so I am not sure where 19.21.1.0 is coming from. Additionally, the tests all pass locally for me. I compared with what Azure KeyVault is doing for their PEC tests and I don’t see anything different. My guess is that I am not setting up something correctly in our test controller here: C:\work\github\azure-powershell\src\Batch\Batch.Test\ScenarioTests\BatchController.cs and we’re not importing the right dependencies for some reason.

Given the breaking change contents of this PR and the upcoming breaking change release (whose code complete date has already passed), I am worried these last minute changes are going to prevent us from making it in in time – can you help? If we can’t figure it out ASAP I can revert the new test (and changes) which are causing the issue for now and we can investigate further after the release happens (I have already tested the cmdlets manually and they work, and as I mentioned our automated test also works on my machine).

[matthchr]

I think I got it figured out -- master had updated their reference to Microsoft.Azure.Management.networking but my branch hadn't.

[isra-fel]

LGTM