Get-/Set-DbaNetworkConfiguration - New commands for managing network configuration of instance

[andreasjordan]

Type of Change

• Bug fix (non-breaking change, fixes # )
• New feature (non-breaking change, adds functionality, fixes Set-dbatcpport static port incorrect #3170, fixes Enable/Disable Client/Server Protocol #5327 )
• Breaking change (effects multiple commands or functionality, fixes # )
• Ran manual Pester test and has passed (`.\tests\manual.pester.ps1)
• Adding code coverage to existing functionality
• Pester test is included
• If new file reference added for test, has is been added to github.com/sqlcollaborative/appveyor-lab ?
• Nunit test is included
• Documentation
• Build system

Purpose

General commands that get or set the network configuration like SQL Server Configuration Manager does.

At the moment, this is just a first concept and I would like to have feedback about it.

[andreasjordan]

How to use the commands:

$netconf = Get-DbaNetworkConfiguration -SqlInstance SRV1\SQL2017 
$netconf.SharedMemoryEnabled = $false
$netconf.TCPIPProtokoll.KeepAlive = 20000
$netconf.TCPIPIPAddresses.Where({$_.Name -eq 'IPAll'})[0].TCPDynamicPorts = ''
$netconf.TCPIPIPAddresses.Where({$_.Name -eq 'IPAll'})[0].TCPPort = '12345' 
$netconf | Set-DbaNetworkConfiguration

[potatoqualitee]

I like it! Can we cover 1-3 common scenarios in the params for Set-DbaNetworkConfiguration? Like -TCPPort or something? Open to the answer being no.

[potatoqualitee]

Oh, I just realized this has the .Where( method. That is only PS4+ compat and we support 3+.

[andreasjordan]

I can rewrite this - just wanted to play a bit with that syntax style.
Will think about some common scenarios, already some in mind.

[andreasjordan]

Hooray, all tests are green!
@potatoqualitee Please have a look if this is the functionality you have been looking for. And then please have a look at the documentation and all the messages - you know, I'm not a nativ english speaking person.

[wsmelton]

@jpomfret the test for these commands should be included in the service_restart list so they do not adversely affect the other test groups since they change the network configuration of the instance in Appveyor.

[andreasjordan]

Hi @wsmelton : Thanks for the review. Next time you can just say: "Grab a dictionary and have a look at protocol" - would save you a lot of time. I accepted all of you changes, thank you very much.

[andreasjordan]

Let me copy that here:

These commands use Invoke-ManagedComputerCommand with -ArgumentList, so we have to make sure that the change from [string[]] to [object[]] has no negative effect.
Marked commands have been tested and alle commands have been code reviewed for only using strings in -ArgumentList:
[x] Enable-DbaAgHadr
[x] Disable-DbaAgHadr
[x] Get-DbaStartupParameter
[ ] Set-DbaStartupParameter
[x] Get-DbaTcpPort
[x] Test-DbaSpn
[ ] Update-DbaServiceAccount
[ ] Get-WmiHadr

This command needed to be changed, see next commit:
Set-DbaTcpPort

[andreasjordan]

Thanks again Shawn for the hint, good we found this before the rollout.
Shall we rewrite Set-DbaTcpPort to use Set-DbaNetworkConfiguration within this pull request?
And maybe even rewrite Get-DbaTcpPort? The first approch is a bit strange, I would prefer to use Get-DbaNetworkConfiguration.
But it would be ok for me to first release DbaNetworkConfiguration and let some people use it and then later rewrite DbaTcpPort.

[wsmelton]

No problems from me for reusing code for network config in other commands. I'd say rewrite them all to use the get network command if it fits and simplifies the other commands; gives us one place to manage it instead of across all of them.

This PR can be released first though, and then PR(s) the changes for the other commands.

// @potatoqualitee @jpomfret

[potatoqualitee]

agreed, will be able to merge then can create another PR 🙇🏼 Thank you both!

[andreasjordan]

So yes, this PR can be merged, I have stopped working on this.