Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BREAKING CHANGE: SqlServerNetwork: Does not enable the IPs for the protocol #339

Closed
wsmelton opened this issue Jan 24, 2017 · 7 comments · Fixed by #1518
Closed

BREAKING CHANGE: SqlServerNetwork: Does not enable the IPs for the protocol #339

wsmelton opened this issue Jan 24, 2017 · 7 comments · Fixed by #1518
Labels
breaking change When used on an issue, the issue has been determined to be a breaking change.

Comments

@wsmelton
Copy link
Contributor

Details of the scenario you try and problem that is occurring:

The ability to actually enable the IPs are not available, or not being done in the current resource. TCP/IP protocol is enabled on the server, but the actual IP
image

The DSC configuration that is using the resource (as detailed as possible):

xSqlServerNetwork "SqlNetwork" {
	DependsOn = "[xSqlServerSetup]SqlInstallation"
	InstanceName = $AllNodes.InstanceName
	ProtocolName = 'tcp'
	TCPPort = $AllNodes.SqlPort
	IsEnabled = $true
	RestartService = $true
}

$AllNodesSqlPort is set to '1433'

Version of the Operating System, SQL Server and PowerShell the DSC Target Node is running:
OS: Window Server 2012 R2
SQL Server: SQL Server 2014 w/ SP1 x64
PowerShell:

PS C:\Windows\system32> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      5.0.10586.117
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.10586.117
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

Version of the DSC module you're using, or 'dev' if you're using current dev branch:

PS C:\Windows\system32> Get-Module xsqlserver -ListAvailable
 Directory: C:\Program Files\WindowsPowerShell\Modules

ModuleType Version    Name                                ExportedCommands
---------- -------    ----                                ----------------
Manifest   4.0.0.0    xSQLServer
@johlju johlju added bug The issue is a bug. help wanted The issue is up for grabs for anyone in the community. labels Jan 25, 2017
@johlju
Copy link
Member

johlju commented May 1, 2017

Done some work in this resource now to get tests done for it so it easier to make changes to the resource. I saw that today the resource only supports the IP address group 'IPAll'.

I think we need to add a new parameter IpAddressGroup (anyone got a better name?) which need to be a Key and would be set to a single value ('IPAll', 'IP1','IP2' etc). depending on which IP address group that should be changed.
I also think we need to change ProtocolName from required to Key to support more protocols in the future.
But since the other protocols does not have groups, and IpAdressGroup must be a key, then maybe IpAdressGroup must have a value 'None' when configuring other protocols.
A worse option would be to change the parameter ProtocolName to be assigned 'IPAll','IP1' instead of 'Tcp'.
Either way, this will be a breaking change.

Description on each property in this article (for when this is being worked on):
https://docs.microsoft.com/en-us/sql/tools/configuration-manager/tcp-ip-properties-ip-addresses-tab

Dumping some code here to get some test data out to help when this is being worked on later.

$version  = '13'
$applicationDomainObject = [System.AppDomain]::CreateDomain('xSQLServerNetwork')
$applicationDomainObject.Load("Microsoft.SqlServer.Smo, Version=$version.0.0.0, Culture=neutral, PublicKeyToken=89845dcd8080cc91") | Out-Null
$applicationDomainObject.Load("Microsoft.SqlServer.SqlWmiManagement, Version=$version.0.0.0, Culture=neutral, PublicKeyToken=89845dcd8080cc91") | Out-Null

$mangedComputerObject = new-object Microsoft.SqlServer.Management.Smo.Wmi.ManagedComputer
$mangedComputerObject.ServerInstances['SQL2016'].ServerProtocols['Tcp'].IPAddresses | Select name
$mangedComputerObject.ServerInstances['SQL2016'].ServerProtocols['Tcp'].IPAddresses['IP1'].IPAddressProperties | Select name

[System.AppDomain]::Unload($$applicationDomainObject)

@johlju johlju changed the title xSqlServerNetwork: Does not enable the IPs for the protocol BREAKING CHANGE: xSQLServerNetwork: Does not enable the IPs for the protocol May 1, 2017
@johlju johlju added the breaking change When used on an issue, the issue has been determined to be a breaking change. label May 1, 2017
@johlju johlju changed the title BREAKING CHANGE: xSQLServerNetwork: Does not enable the IPs for the protocol BREAKING CHANGE: SqlServerNetwork: Does not enable the IPs for the protocol Dec 24, 2017
@claudiospizzi
Copy link
Contributor

I'm not sure, but I think I've covered this in my PR: #1045

@johlju
Copy link
Member

johlju commented Feb 9, 2018

This is related to issue #14, see the #339 (comment) and in particular the mention of changing ProtocolName to a Key.

@wsmelton
Copy link
Contributor Author

I'm not sure this will work 100%:

which need to be a Key and would be set to a single value ('IPAll', 'IP1','IP2' etc). depending on which IP address group that should be changed.

The reason being that we won't know what IP is assigned to which key. SQL Server will pick up the IPs for all NICs on the server as it seems them...which you don't know what that order is until the installation is completed. It would likely be best to have it be a string value and then try and match the IP itself as that would be known.

@johlju
Copy link
Member

johlju commented Aug 17, 2018

That suggestion sounds promising. We don't want to write over any group, or getting duplicate IP addresses in two groups.
That IPAddress string parameter would accepts an IP address. and the string 'IPAll' to be able to configure that group?

@johlju
Copy link
Member

johlju commented Aug 17, 2018

@claudiospizzi is right, this is being worked on in PR #1045, and it looks like it is what @wsmelton suggest too?

@johlju
Copy link
Member

johlju commented Apr 17, 2020

Issue #1377 and issue #1378 will make this issue obsolete as they will deprecate the the SqlServerNetwork resource.

@johlju johlju added on hold The issue or pull request has been put on hold by a maintainer. and removed bug The issue is a bug. help wanted The issue is up for grabs for anyone in the community. labels Apr 17, 2020
@johlju johlju removed the on hold The issue or pull request has been put on hold by a maintainer. label May 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change When used on an issue, the issue has been determined to be a breaking change.
Projects
None yet
3 participants