xSQLServerAlwaysOnAvailabilityGroup: Resolved FQDN and Logic Bugs

CHANGELOG.md

@@ -7,6 +7,19 @@
     collation ([issue #767](https://github.com/PowerShell/xSQLServer/issues/767)).
   - Fixed unit tests for Get-TargetResource to ensure correctly testing return
     values ([issue #849](https://github.com/PowerShell/xSQLServer/issues/849))
+- Changes to xSQLServerAlwaysOnAvailabilityGroup
+  - Refactored the unit tests to allow them to be more user friendly and to test
+    additional SQLServer variations.
+    - Each test will utilize the Import-SQLModuleStub to ensure the correct
+      module is loaded ([issue #784](https://github.com/PowerShell/xSQLServer/issues/784)).
+  - Fixed an issue when setting the SQLServer parameter to a Fully Qualified
+    Domain Name (FQDN) ([issue #468](https://github.com/PowerShell/xSQLServer/issues/468)).
+  - Fixed the logic so that if a parameter is not supplied to the resource, the
+    resource will not attempt to apply the defaults on subsequent checks
+    ([issue #517](https://github.com/PowerShell/xSQLServer/issues/517)).
+- Added the CommonTestHelper.psm1 to store common testing functions.
+  - Added the Import-SQLModuleStub function to ensure the correct version of the
+    module stubs are loaded ([issue #784](https://github.com/PowerShell/xSQLServer/issues/784)).
 
 ## 8.2.0.0
 
@@ -68,12 +81,12 @@
     Group is created.
   - Use the new helper function "Test-ClusterPermissions".
   - Refactored the unit tests to allow them to be more user friendly.
-    Added the following read-only properties to the schema ([issue #476](https://github.com/PowerShell/xSQLServer/issues/476))
+  - Added the following read-only properties to the schema ([issue #476](https://github.com/PowerShell/xSQLServer/issues/476))
     - EndpointPort
     - EndpointURL
     - SQLServerNetName
     - Version
-  - Use the Get-PrimaryReplicaServerObject helper function
+  - Use the Get-PrimaryReplicaServerObject helper function.
 - Changes to xSQLServerAlwaysOnAvailabilityGroupReplica
   - Fixed the formatting for the AvailabilityGroupNotFound error.
   - Added the following read-only properties to the schema ([issue #477](https://github.com/PowerShell/xSQLServer/issues/477))

DSCResources/MSFT_xSQLServerAlwaysOnAvailabilityGroup/MSFT_xSQLServerAlwaysOnAvailabilityGroup.psm1

@@ -59,14 +59,14 @@ function Get-TargetResource
             SQLInstanceName               = $SQLInstanceName
             Ensure                        = 'Present'
             AutomatedBackupPreference     = $availabilityGroup.AutomatedBackupPreference
-            AvailabilityMode              = $availabilityGroup.AvailabilityReplicas[$serverObject.Name].AvailabilityMode
-            BackupPriority                = $availabilityGroup.AvailabilityReplicas[$serverObject.Name].BackupPriority
-            ConnectionModeInPrimaryRole   = $availabilityGroup.AvailabilityReplicas[$serverObject.Name].ConnectionModeInPrimaryRole
-            ConnectionModeInSecondaryRole = $availabilityGroup.AvailabilityReplicas[$serverObject.Name].ConnectionModeInSecondaryRole
+            AvailabilityMode              = $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].AvailabilityMode
+            BackupPriority                = $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].BackupPriority
+            ConnectionModeInPrimaryRole   = $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].ConnectionModeInPrimaryRole
+            ConnectionModeInSecondaryRole = $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].ConnectionModeInSecondaryRole
             FailureConditionLevel         = $availabilityGroup.FailureConditionLevel
-            FailoverMode                  = $availabilityGroup.AvailabilityReplicas[$serverObject.Name].FailoverMode
+            FailoverMode                  = $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].FailoverMode
             HealthCheckTimeout            = $availabilityGroup.HealthCheckTimeout
-            EndpointURL                   = $availabilityGroup.AvailabilityReplicas[$serverObject.Name].EndpointUrl
+            EndpointURL                   = $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].EndpointUrl
             EndpointPort                  = $endpointPort
             SQLServerNetName              = $serverObject.NetName
             Version                       = $sqlMajorVersion
@@ -252,7 +252,7 @@ function Set-TargetResource
             if ( $availabilityGroup )
             {
                 # If the primary replica is currently on this instance
-                if ( $availabilityGroup.PrimaryReplicaServerName -eq $serverObject.Name )
+                if ( $availabilityGroup.PrimaryReplicaServerName -eq $serverObject.DomainInstanceName )
                 {
                     try
                     {
@@ -292,7 +292,7 @@ function Set-TargetResource
             {
                 # Set up the parameters to create the AG Replica
                 $newReplicaParams = @{
-                    Name             = $serverObject.Name
+                    Name             = $serverObject.DomainInstanceName
                     Version          = $sqlMajorVersion
                     AsTemplate       = $true
                     AvailabilityMode = $AvailabilityMode
@@ -367,92 +367,95 @@ function Set-TargetResource
             # Otherwise let's check each of the parameters passed and update the Availability Group accordingly
             else
             {
+                # Get the parameters that were submitted to the function
+                [System.Array] $submittedParameters = $PSBoundParameters.Keys
+
                 # Make sure we're communicating with the primary replica
                 $primaryServerObject = Get-PrimaryReplicaServerObject -ServerObject $serverObject -AvailabilityGroup $availabilityGroup
                 $availabilityGroup = $primaryServerObject.AvailabilityGroups[$Name]
 
-                if ( $AutomatedBackupPreference -ne $availabilityGroup.AutomatedBackupPreference )
+                if ( ( $submittedParameters -contains 'AutomatedBackupPreference' ) -and ( $AutomatedBackupPreference -ne $availabilityGroup.AutomatedBackupPreference ) )
                 {
                     $availabilityGroup.AutomatedBackupPreference = $AutomatedBackupPreference
                     Update-AvailabilityGroup -AvailabilityGroup $availabilityGroup
                 }
 
-                if ( $AvailabilityMode -ne $availabilityGroup.AvailabilityReplicas[$serverObject.Name].AvailabilityMode )
+                if ( ( $submittedParameters -contains 'AvailabilityMode' ) -and ( $AvailabilityMode -ne $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].AvailabilityMode ) )
                 {
-                    $availabilityGroup.AvailabilityReplicas[$serverObject.Name].AvailabilityMode = $AvailabilityMode
-                    Update-AvailabilityGroupReplica -AvailabilityGroupReplica $availabilityGroup.AvailabilityReplicas[$serverObject.Name]
+                    $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].AvailabilityMode = $AvailabilityMode
+                    Update-AvailabilityGroupReplica -AvailabilityGroupReplica $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName]
                 }
 
-                if ( $BackupPriority -ne $availabilityGroup.AvailabilityReplicas[$serverObject.Name].BackupPriority )
+                if ( ( $submittedParameters -contains 'BackupPriority' ) -and ( $BackupPriority -ne $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].BackupPriority ) )
                 {
-                    $availabilityGroup.AvailabilityReplicas[$serverObject.Name].BackupPriority = $BackupPriority
-                    Update-AvailabilityGroupReplica -AvailabilityGroupReplica $availabilityGroup.AvailabilityReplicas[$serverObject.Name]
+                    $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].BackupPriority = $BackupPriority
+                    Update-AvailabilityGroupReplica -AvailabilityGroupReplica $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName]
                 }
 
-                if ( ( $sqlMajorVersion -ge 13 ) -and ( $BasicAvailabilityGroup -ne $availabilityGroup.BasicAvailabilityGroup ) )
+                if ( ( $submittedParameters -contains 'BasicAvailabilityGroup' ) -and ( $sqlMajorVersion -ge 13 ) -and ( $BasicAvailabilityGroup -ne $availabilityGroup.BasicAvailabilityGroup ) )
                 {
                     $availabilityGroup.BasicAvailabilityGroup = $BasicAvailabilityGroup
                     Update-AvailabilityGroup -AvailabilityGroup $availabilityGroup
                 }
 
-                if ( ( $sqlMajorVersion -ge 13 ) -and ( $DatabaseHealthTrigger -ne $availabilityGroup.DatabaseHealthTrigger ) )
+                if ( ( $submittedParameters -contains 'DatabaseHealthTrigger' ) -and ( $sqlMajorVersion -ge 13 ) -and ( $DatabaseHealthTrigger -ne $availabilityGroup.DatabaseHealthTrigger ) )
                 {
                     $availabilityGroup.DatabaseHealthTrigger = $DatabaseHealthTrigger
                     Update-AvailabilityGroup -AvailabilityGroup $availabilityGroup
                 }
 
                 # Make sure ConnectionModeInPrimaryRole has a value in order to avoid false positive matches when the parameter is not defined
-                if ( ( -not [string]::IsNullOrEmpty($ConnectionModeInPrimaryRole) ) -and ( $ConnectionModeInPrimaryRole -ne $availabilityGroup.AvailabilityReplicas[$serverObject.Name].ConnectionModeInPrimaryRole ) )
+                if ( ( $submittedParameters -contains 'ConnectionModeInPrimaryRole' ) -and ( -not [string]::IsNullOrEmpty($ConnectionModeInPrimaryRole) ) -and ( $ConnectionModeInPrimaryRole -ne $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].ConnectionModeInPrimaryRole ) )
                 {
-                    $availabilityGroup.AvailabilityReplicas[$serverObject.Name].ConnectionModeInPrimaryRole = $ConnectionModeInPrimaryRole
-                    Update-AvailabilityGroupReplica -AvailabilityGroupReplica $availabilityGroup.AvailabilityReplicas[$serverObject.Name]
+                    $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].ConnectionModeInPrimaryRole = $ConnectionModeInPrimaryRole
+                    Update-AvailabilityGroupReplica -AvailabilityGroupReplica $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName]
                 }
 
                 # Make sure ConnectionModeInSecondaryRole has a value in order to avoid false positive matches when the parameter is not defined
-                if ( ( -not [string]::IsNullOrEmpty($ConnectionModeInSecondaryRole) ) -and ( $ConnectionModeInSecondaryRole -ne $availabilityGroup.AvailabilityReplicas[$serverObject.Name].ConnectionModeInSecondaryRole ) )
+                if ( ( $submittedParameters -contains 'ConnectionModeInSecondaryRole' ) -and ( -not [string]::IsNullOrEmpty($ConnectionModeInSecondaryRole) ) -and ( $ConnectionModeInSecondaryRole -ne $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].ConnectionModeInSecondaryRole ) )
                 {
-                    $availabilityGroup.AvailabilityReplicas[$serverObject.Name].ConnectionModeInSecondaryRole = $ConnectionModeInSecondaryRole
-                    Update-AvailabilityGroupReplica -AvailabilityGroupReplica $availabilityGroup.AvailabilityReplicas[$serverObject.Name]
+                    $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].ConnectionModeInSecondaryRole = $ConnectionModeInSecondaryRole
+                    Update-AvailabilityGroupReplica -AvailabilityGroupReplica $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName]
                 }
 
                 # Break out the EndpointUrl properties
-                $currentEndpointProtocol, $currentEndpointHostName, $currentEndpointPort = $availabilityGroup.AvailabilityReplicas[$serverObject.Name].EndpointUrl.Replace('//', '').Split(':')
+                $currentEndpointProtocol, $currentEndpointHostName, $currentEndpointPort = $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].EndpointUrl.Replace('//', '').Split(':')
 
                 if ( $endpoint.Protocol.Tcp.ListenerPort -ne $currentEndpointPort )
                 {
-                    $newEndpointUrl = $availabilityGroup.AvailabilityReplicas[$serverObject.Name].EndpointUrl.Replace($currentEndpointPort, $endpoint.Protocol.Tcp.ListenerPort)
-                    $availabilityGroup.AvailabilityReplicas[$serverObject.Name].EndpointUrl = $newEndpointUrl
-                    Update-AvailabilityGroupReplica -AvailabilityGroupReplica $availabilityGroup.AvailabilityReplicas[$serverObject.Name]
+                    $newEndpointUrl = $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].EndpointUrl.Replace($currentEndpointPort, $endpoint.Protocol.Tcp.ListenerPort)
+                    $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].EndpointUrl = $newEndpointUrl
+                    Update-AvailabilityGroupReplica -AvailabilityGroupReplica $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName]
                 }
 
-                if ( $EndpointHostName -ne $currentEndpointHostName )
+                if ( ( $submittedParameters -contains 'EndpointHostName' ) -and ( $EndpointHostName -ne $currentEndpointHostName ) )
                 {
-                    $newEndpointUrl = $availabilityGroup.AvailabilityReplicas[$serverObject.Name].EndpointUrl.Replace($currentEndpointHostName, $EndpointHostName)
-                    $availabilityGroup.AvailabilityReplicas[$serverObject.Name].EndpointUrl = $newEndpointUrl
-                    Update-AvailabilityGroupReplica -AvailabilityGroupReplica $availabilityGroup.AvailabilityReplicas[$serverObject.Name]
+                    $newEndpointUrl = $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].EndpointUrl.Replace($currentEndpointHostName, $EndpointHostName)
+                    $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].EndpointUrl = $newEndpointUrl
+                    Update-AvailabilityGroupReplica -AvailabilityGroupReplica $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName]
                 }
 
                 if ( $currentEndpointProtocol -ne 'TCP' )
                 {
-                    $newEndpointUrl = $availabilityGroup.AvailabilityReplicas[$serverObject.Name].EndpointUrl.Replace($currentEndpointProtocol, 'TCP')
-                    $availabilityGroup.AvailabilityReplicas[$serverObject.Name].EndpointUrl = $newEndpointUrl
-                    Update-AvailabilityGroupReplica -AvailabilityGroupReplica $availabilityGroup.AvailabilityReplicas[$serverObject.Name]
+                    $newEndpointUrl = $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].EndpointUrl.Replace($currentEndpointProtocol, 'TCP')
+                    $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].EndpointUrl = $newEndpointUrl
+                    Update-AvailabilityGroupReplica -AvailabilityGroupReplica $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName]
                 }
 
                 # Make sure FailureConditionLevel has a value in order to avoid false positive matches when the parameter is not defined
-                if ( ( -not [string]::IsNullOrEmpty($FailureConditionLevel) ) -and ( $FailureConditionLevel -ne $availabilityGroup.FailureConditionLevel ) )
+                if ( ( $submittedParameters -contains 'FailureConditionLevel' ) -and ( -not [string]::IsNullOrEmpty($FailureConditionLevel) ) -and ( $FailureConditionLevel -ne $availabilityGroup.FailureConditionLevel ) )
                 {
                     $availabilityGroup.FailureConditionLevel = $FailureConditionLevel
                     Update-AvailabilityGroup -AvailabilityGroup $availabilityGroup
                 }
 
-                if ( $FailoverMode -ne $availabilityGroup.AvailabilityReplicas[$serverObject.Name].FailoverMode )
+                if ( ( $submittedParameters -contains 'FailoverMode' ) -and ( $FailoverMode -ne $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].FailoverMode ) )
                 {
-                    $availabilityGroup.AvailabilityReplicas[$serverObject.Name].FailoverMode = $FailoverMode
-                    Update-AvailabilityGroupReplica -AvailabilityGroupReplica $availabilityGroup.AvailabilityReplicas[$serverObject.Name]
+                    $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName].FailoverMode = $FailoverMode
+                    Update-AvailabilityGroupReplica -AvailabilityGroupReplica $availabilityGroup.AvailabilityReplicas[$serverObject.DomainInstanceName]
                 }
 
-                if ( $HealthCheckTimeout -ne $availabilityGroup.HealthCheckTimeout )
+                if ( ( $submittedParameters -contains 'HealthCheckTimeout' ) -and ( $HealthCheckTimeout -ne $availabilityGroup.HealthCheckTimeout ) )
                 {
                     $availabilityGroup.HealthCheckTimeout = $HealthCheckTimeout
                     Update-AvailabilityGroup -AvailabilityGroup $availabilityGroup
@@ -647,8 +650,11 @@ function Test-TargetResource
 
             if ( $getTargetResourceResult.Ensure -eq 'Present' )
             {
-                # PsBoundParameters won't work here because it doesn't account for default values
-                foreach ( $parameter in $MyInvocation.MyCommand.Parameters.GetEnumerator() )
+                # Use $PSBoundParameters rather than $MyInvocation.MyCommand.Parameters.GetEnumerator()
+                # This allows us to only validate the supplied parameters
+                # If the parameter is not defined by the configuration, we don't care what
+                # it gets set to.
+                foreach ( $parameter in $PSBoundParameters.GetEnumerator() )
                 {
                     $parameterName = $parameter.Key
                     $parameterValue = Get-Variable -Name $parameterName -ErrorAction SilentlyContinue | Select-Object -ExpandProperty Value

Tests/TestHelpers/CommonTestHelper.psm1

@@ -0,0 +1,64 @@
+<#
+    .SYNOPSIS
+        Ensure the correct module stubs are loaded.
+
+    .PARAMETER SQLVersion
+        The major version of the SQL instance.
+
+    .PARAMETER ModuleName
+        The name of the module to load the stubs for. Default is 'SQLServer'.
+#>
+function Import-SQLModuleStub
+{
+    [CmdletBinding(DefaultParameterSetName = 'Module')]
+    param
+    (
+        [Parameter(Mandatory = $true, ParameterSetName = 'Version')]
+        [System.UInt32]
+        $SQLVersion,
+
+        [Parameter(ParameterSetName = 'Module')]
+        [ValidateSet('SQLPS','SQLServer')]
+        [System.String]
+        $ModuleName = 'SQLServer'
+    )
+
+    # Translate the module names to their appropriate stub name
+    $modulesAndStubs = @{
+        SQLPS = 'SQLPSStub'
+        SQLServer = 'SQLServerStub'
+    }
+
+    # Determine which module to ensure is loaded based on the parameters passed
+    if ( $PsCmdlet.ParameterSetName -eq 'Version' )
+    {
+        if ( $SQLVersion -le 12 )
+        {
+            $ModuleName = 'SQLPS'
+        }
+        elseif ( $SQLVersion -ge 13 )
+        {
+            $ModuleName = 'SQLServer'
+        }
+    }
+
+    # Get the stub name
+    $stubModuleName = $modulesAndStubs.$ModuleName
+
+    # Ensure none of the other stub modules are loaded
+    [System.Array] $otherStubModules = $modulesAndStubs.Values | Where-Object -FilterScript { $_ -ne $stubModuleName }
+
+    if ( Get-Module -Name $otherStubModules )
+    {
+        Remove-Module -Name $otherStubModules
+    }
+
+    # If the desired module is not loaded, load it now
+    if ( -not ( Get-Module -Name $stubModuleName ) )
+    {
+        # Build the path to the module stub
+        $moduleStubPath = Join-Path -Path ( Join-Path -Path ( Join-Path -Path ( Split-Path -Path $PSScriptRoot -Parent ) -ChildPath Unit ) -ChildPath Stubs ) -ChildPath "$($stubModuleName).psm1"
+
+        Import-Module -Name $moduleStubPath -Force -Global
+    }
+}