From decc395b36eeff78935439a5877571923212cccf Mon Sep 17 00:00:00 2001 From: Johan Ljunggren Date: Fri, 6 Jul 2018 10:10:20 +0200 Subject: [PATCH 1/2] Changes to SqlServerDsc - Updated helper function `Import-SQLPSModule` - To only import module if the module does not exist in the session. - To always import the latest version of 'SqlServer' or 'SQLPS' module, if more than one version exist on the target node. It will still prefer to use 'SqlServer' module. - Changes to SqlAG - Removed excess `Import-SQLPSModule` call. - Changes to SqlSetup - Now after a successful install the "SQL PowerShell module" is reevaluated and forced to be reimported into the session. This is to support that a never version of SQL Server was installed side-by-side so that SQLPS module should be used instead. --- CHANGELOG.md | 13 ++ DSCResources/MSFT_SqlAG/MSFT_SqlAG.psm1 | 2 - DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1 | 17 ++- SqlServerDscHelper.psm1 | 144 ++++++++++++------ Tests/Unit/MSFT_SqlAG.Tests.ps1 | 9 -- Tests/Unit/MSFT_SqlSetup.Tests.ps1 | 13 +- Tests/Unit/SqlServerDSCHelper.Tests.ps1 | 112 +++++++++++--- en-US/SqlServerDscHelper.strings.psd1 | 5 +- sv-SE/SqlServerDscHelper.strings.psd1 | 5 +- 9 files changed, 228 insertions(+), 92 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa06a610b..bb54e6188 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ - Updated helper function Restart-SqlService to have to new optional parameters `SkipClusterCheck` and `SkipWaitForOnline`. This was to support more aspects of the resource SqlServerNetwork. + - Updated helper function `Import-SQLPSModule` + - To only import module if the + module does not exist in the session. + - To always import the latest version of 'SqlServer' or 'SQLPS' module, if + more than one version exist on the target node. It will still prefer to + use 'SqlServer' module. - Changes to SqlAlwaysOnService - Integration tests was updated to handle new IPv6 addresses on the AppVeyor build worker ([issue #1155](https://github.com/PowerShell/SqlServerDsc/issues/1155)). @@ -15,6 +21,13 @@ enabling and disabling the protocol. - Added integration tests for this resource ([issue #751](https://github.com/PowerShell/SqlServerDsc/issues/751)). +- Changes to SqlAG + - Removed excess `Import-SQLPSModule` call. +- Changes to SqlSetup + - Now after a successful install the "SQL PowerShell module" is reevaluated and + forced to be reimported into the session. This is to support that a never + version of SQL Server was installed side-by-side so that SQLPS module should + be used instead. ## 11.3.0.0 diff --git a/DSCResources/MSFT_SqlAG/MSFT_SqlAG.psm1 b/DSCResources/MSFT_SqlAG/MSFT_SqlAG.psm1 index 387c9e2b3..41ca2887d 100644 --- a/DSCResources/MSFT_SqlAG/MSFT_SqlAG.psm1 +++ b/DSCResources/MSFT_SqlAG/MSFT_SqlAG.psm1 @@ -232,8 +232,6 @@ function Set-TargetResource $ProcessOnlyOnActiveNode ) - Import-SQLPSModule - # Connect to the instance $serverObject = Connect-SQL -SQLServer $ServerName -SQLInstanceName $InstanceName diff --git a/DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1 b/DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1 index e50fa2f72..47a6b2d6f 100644 --- a/DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1 +++ b/DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1 @@ -1420,8 +1420,9 @@ function Set-TargetResource elseif ($processExitCode -ne 0) { $setupExitMessageError = ('{0} {1}' -f $setupExitMessage, ($script:localizedData.SetupFailed)) - Write-Warning $setupExitMessageError + + $setupEndedInError = $true } else { @@ -1436,13 +1437,27 @@ function Set-TargetResource { Write-Verbose -Message $script:localizedData.Reboot + # Rebooting, so no point in refreshing the session. + $forceReloadPowerShellModule = $false + $global:DSCMachineStatus = 1 } else { Write-Verbose -Message $script:localizedData.SuppressReboot + $forceReloadPowerShellModule = $true } } + else + { + $forceReloadPowerShellModule = $true + } + + if ((-not $setupEndedInError) -and $forceReloadPowerShellModule) + { + # Force reload of SQLPS module if newer version was installed. + Import-SQLPSModule -Force + } if (-not (Test-TargetResource @PSBoundParameters)) { diff --git a/SqlServerDscHelper.psm1 b/SqlServerDscHelper.psm1 index dab9543d8..651aa7063 100644 --- a/SqlServerDscHelper.psm1 +++ b/SqlServerDscHelper.psm1 @@ -1,7 +1,7 @@ # Load Localization Data Import-Module -Name (Join-Path -Path (Join-Path -Path $PSScriptRoot ` - -ChildPath 'DscResources') ` - -ChildPath 'CommonResourceHelper.psm1') + -ChildPath 'DscResources') ` + -ChildPath 'CommonResourceHelper.psm1') $script:localizedData = Get-LocalizedData -ResourceName 'SqlServerDscHelper' -ScriptRoot $PSScriptRoot @@ -271,11 +271,11 @@ function New-TerminatingError $errorMessage = $script:localizedData.$ErrorType - if(!$errorMessage) + if (!$errorMessage) { $errorMessage = ($script:localizedData.NoKeyFound -f $ErrorType) - if(!$errorMessage) + if (!$errorMessage) { $errorMessage = ("No Localization key found for key: {0}" -f $ErrorType) } @@ -283,7 +283,7 @@ function New-TerminatingError $errorMessage = ($errorMessage -f $FormatArgs) - if( $InnerException ) + if ( $InnerException ) { $errorMessage += " InnerException: $($InnerException.Message)" } @@ -291,7 +291,7 @@ function New-TerminatingError $callStack = Get-PSCallStack # Get Name of calling script - if($callStack[1] -and $callStack[1].ScriptName) + if ($callStack[1] -and $callStack[1].ScriptName) { $scriptPath = $callStack[1].ScriptName @@ -351,10 +351,10 @@ function New-WarningMessage if (!$warningMessage) { $errorParams = @{ - ErrorType = 'NoKeyFound' - FormatArgs = $WarningType + ErrorType = 'NoKeyFound' + FormatArgs = $WarningType ErrorCategory = 'InvalidArgument' - TargetObject = 'New-WarningMessage' + TargetObject = 'New-WarningMessage' } ## Raise an error indicating the localization data is not present @@ -431,8 +431,8 @@ function Test-SQLDscParameterState $returnValue = $true if (($DesiredValues.GetType().Name -ne 'HashTable') ` - -and ($DesiredValues.GetType().Name -ne 'CimInstance') ` - -and ($DesiredValues.GetType().Name -ne 'PSBoundParametersDictionary')) + -and ($DesiredValues.GetType().Name -ne 'CimInstance') ` + -and ($DesiredValues.GetType().Name -ne 'PSBoundParametersDictionary')) { $errorMessage = $script:localizedData.PropertyTypeInvalidForDesiredValues -f $($DesiredValues.GetType().Name) New-InvalidArgumentException -ArgumentName 'DesiredValues' -Message $errorMessage @@ -457,11 +457,11 @@ function Test-SQLDscParameterState if (($_ -ne 'Verbose')) { if (($CurrentValues.ContainsKey($_) -eq $false) ` - -or ($CurrentValues.$_ -ne $DesiredValues.$_) ` - -or (($DesiredValues.GetType().Name -ne 'CimInstance' -and $DesiredValues.ContainsKey($_) -eq $true) -and ($null -ne $DesiredValues.$_ -and $DesiredValues.$_.GetType().IsArray))) + -or ($CurrentValues.$_ -ne $DesiredValues.$_) ` + -or (($DesiredValues.GetType().Name -ne 'CimInstance' -and $DesiredValues.ContainsKey($_) -eq $true) -and ($null -ne $DesiredValues.$_ -and $DesiredValues.$_.GetType().IsArray))) { if ($DesiredValues.GetType().Name -eq 'HashTable' -or ` - $DesiredValues.GetType().Name -eq 'PSBoundParametersDictionary') + $DesiredValues.GetType().Name -eq 'PSBoundParametersDictionary') { $checkDesiredValue = $DesiredValues.ContainsKey($_) } @@ -485,7 +485,7 @@ function Test-SQLDscParameterState if ($desiredType.IsArray -eq $true) { if (($CurrentValues.ContainsKey($fieldName) -eq $false) ` - -or ($null -eq $CurrentValues.$fieldName)) + -or ($null -eq $CurrentValues.$fieldName)) { Write-Verbose -Message ($script:localizedData.PropertyValidationError -f $fieldName) -Verbose @@ -494,7 +494,7 @@ function Test-SQLDscParameterState else { $arrayCompare = Compare-Object -ReferenceObject $CurrentValues.$fieldName ` - -DifferenceObject $DesiredValues.$fieldName + -DifferenceObject $DesiredValues.$fieldName if ($null -ne $arrayCompare) { Write-Verbose -Message ($script:localizedData.PropertiesDoesNotMatch -f $fieldName) -Verbose @@ -514,10 +514,10 @@ function Test-SQLDscParameterState 'String' { if (-not [System.String]::IsNullOrEmpty($CurrentValues.$fieldName) -or ` - -not [System.String]::IsNullOrEmpty($DesiredValues.$fieldName)) + -not [System.String]::IsNullOrEmpty($DesiredValues.$fieldName)) { Write-Verbose -Message ($script:localizedData.ValueOfTypeDoesNotMatch ` - -f $desiredType.Name, $fieldName, $($CurrentValues.$fieldName), $($DesiredValues.$fieldName)) -Verbose + -f $desiredType.Name, $fieldName, $($CurrentValues.$fieldName), $($DesiredValues.$fieldName)) -Verbose $returnValue = $false } @@ -526,10 +526,10 @@ function Test-SQLDscParameterState 'Int32' { if (-not ($DesiredValues.$fieldName -eq 0) -or ` - -not ($null -eq $CurrentValues.$fieldName)) + -not ($null -eq $CurrentValues.$fieldName)) { Write-Verbose -Message ($script:localizedData.ValueOfTypeDoesNotMatch ` - -f $desiredType.Name, $fieldName, $($CurrentValues.$fieldName), $($DesiredValues.$fieldName)) -Verbose + -f $desiredType.Name, $fieldName, $($CurrentValues.$fieldName), $($DesiredValues.$fieldName)) -Verbose $returnValue = $false } @@ -538,10 +538,10 @@ function Test-SQLDscParameterState { $_ -eq 'Int16' -or $_ -eq 'UInt16'} { if (-not ($DesiredValues.$fieldName -eq 0) -or ` - -not ($null -eq $CurrentValues.$fieldName)) + -not ($null -eq $CurrentValues.$fieldName)) { Write-Verbose -Message ($script:localizedData.ValueOfTypeDoesNotMatch ` - -f $desiredType.Name, $fieldName, $($CurrentValues.$fieldName), $($DesiredValues.$fieldName)) -Verbose + -f $desiredType.Name, $fieldName, $($CurrentValues.$fieldName), $($DesiredValues.$fieldName)) -Verbose $returnValue = $false } @@ -550,7 +550,7 @@ function Test-SQLDscParameterState default { Write-Warning -Message ($script:localizedData.UnableToCompareProperty ` - -f $fieldName, $desiredType.Name) + -f $fieldName, $desiredType.Name) $returnValue = $false } @@ -567,16 +567,49 @@ function Test-SQLDscParameterState <# .SYNOPSIS Imports the module SQLPS in a standardized way. + + .PARAMETER Force + Forces the removal of the previous SQL module, to load the same or newer + version fresh. + This is meant to make sure the newest version is used, with the latest + assemblies. + #> function Import-SQLPSModule { [CmdletBinding()] param ( + [Parameter()] + [Switch] + $Force ) - $module = (Get-Module -FullyQualifiedName 'SqlServer' -ListAvailable).Name - if ($module) + if ($Force.IsPresent) + { + Write-Verbose -Message $script:localizedData.ModuleForceRemoval -Verbose + Remove-Module -Name @('SqlServer','SQLPS','SQLASCmdlets') -Force -ErrorAction SilentlyContinue + } + + <# + Check if either of the modules are already loaded into the session. + Prefer to use the first one (in order found). + NOTE: There should actually only be either SqlServer or SQLPS loaded, + otherwise there can be problems with wrong assemblies being loaded. + #> + $loadedModuleName = (Get-Module -Name @('SqlServer', 'SQLPS') | Select-Object -First 1).Name + if ($loadedModuleName) + { + Write-Verbose -Message ($script:localizedData.PowerShellModuleAlreadyImported -f $loadedModuleName) -Verbose + return + } + + # Get the newest SqlServer module if more than one exist + $availableModuleName = (Get-Module -FullyQualifiedName 'SqlServer' -ListAvailable | + Sort-Object -Property 'Version' -Descending | + Select-Object -First 1).Name + + if ($availableModuleName) { Write-Verbose -Message ($script:localizedData.PreferredModuleFound) -Verbose } @@ -592,29 +625,40 @@ function Import-SQLPSModule #> $env:PSModulePath = [System.Environment]::GetEnvironmentVariable('PSModulePath', 'Machine') - $module = (Get-Module -FullyQualifiedName 'SQLPS' -ListAvailable).Name + <# + Get the newest SQLPS module if more than one exist. + This sets $availableModuleName to the Path of the module to be loaded. + #> + $availableModuleName = (Get-Module -FullyQualifiedName 'SQLPS' -ListAvailable | + Select-Object -Property Name, Path, @{ + Name = 'Version' + Expression = { + # Parse the build version number '120', '130' from the Path. + (Select-String -InputObject $_.Path -Pattern '\\([0-9]{3})\\' -List).Matches.Groups[1].Value + } + } | + Sort-Object -Property 'Version' -Descending | + Select-Object -First 1).Path } - if ($module) + if ($availableModuleName) { try { Write-Debug -Message ($script:localizedData.DebugMessagePushingLocation) Push-Location - Write-Verbose -Message ($script:localizedData.ImportingPowerShellModule -f $module) -Verbose - <# SQLPS has unapproved verbs, disable checking to ignore Warnings. Suppressing verbose so all cmdlet is not listed. #> - Import-Module -Name $module -DisableNameChecking -Verbose:$False -ErrorAction Stop + $importedModule = Import-Module -Name $availableModuleName -DisableNameChecking -Verbose:$False -Force:$Force -PassThru -ErrorAction Stop - Write-Debug -Message ($script:localizedData.DebugMessageImportedPowerShellModule -f $module) + Write-Verbose -Message ($script:localizedData.ImportedPowerShellModule -f $importedModule.Name, $importedModule.Version, $importedModule.Path) -Verbose } catch { - $errorMessage = $script:localizedData.FailedToImportPowerShellSqlModule -f $module + $errorMessage = $script:localizedData.FailedToImportPowerShellSqlModule -f $availableModuleName New-InvalidOperationException -Message $errorMessage -ErrorRecord $_ } finally @@ -705,11 +749,11 @@ function Restart-SqlService # Get the cluster resources Write-Verbose -Message ($script:localizedData.GetSqlServerClusterResources) -Verbose $sqlService = Get-CimInstance -Namespace root/MSCluster -ClassName MSCluster_Resource -Filter "Type = 'SQL Server'" | - Where-Object -FilterScript { $_.PrivateProperties.InstanceName -eq $serverObject.ServiceName } + Where-Object -FilterScript { $_.PrivateProperties.InstanceName -eq $serverObject.ServiceName } Write-Verbose -Message ($script:localizedData.GetSqlAgentClusterResource) -Verbose $agentService = $sqlService | Get-CimAssociatedInstance -ResultClassName MSCluster_Resource | - Where-Object -FilterScript { ($_.Type -eq 'SQL Server Agent') -and ($_.State -eq 2) } + Where-Object -FilterScript { ($_.Type -eq 'SQL Server Agent') -and ($_.State -eq 2) } # Build a listing of resources being acted upon $resourceNames = @($sqlService.Name, ($agentService | Select-Object -ExpandProperty Name)) -join "," @@ -1012,10 +1056,10 @@ function Test-LoginEffectivePermissions $permissionsPresent = $false $invokeQueryParameters = @{ - SQLServer = $SQLServer + SQLServer = $SQLServer SQLInstanceName = $SQLInstanceName - Database = 'master' - WithResults = $true + Database = 'master' + WithResults = $true } $queryToGetEffectivePermissionsForLogin = " @@ -1094,10 +1138,10 @@ function Test-AvailabilityReplicaSeedingModeAutomatic if ( $serverObject.Version -ge 13 ) { $invokeQueryParams = @{ - SQLServer = $SQLServer + SQLServer = $SQLServer SQLInstanceName = $SQLInstanceName - Database = 'master' - WithResults = $true + Database = 'master' + WithResults = $true } $queryToGetSeedingMode = " @@ -1170,10 +1214,10 @@ function Test-ImpersonatePermissions ) $testLoginEffectivePermissionsParams = @{ - SQLServer = $ServerObject.ComputerNamePhysicalNetBIOS + SQLServer = $ServerObject.ComputerNamePhysicalNetBIOS SQLInstanceName = $ServerObject.ServiceName - LoginName = $ServerObject.ConnectionContext.TrueLogin - Permissions = @('IMPERSONATE ANY LOGIN') + LoginName = $ServerObject.ConnectionContext.TrueLogin + Permissions = @('IMPERSONATE ANY LOGIN') } $impersonatePermissionsPresent = Test-LoginEffectivePermissions @testLoginEffectivePermissionsParams @@ -1206,7 +1250,7 @@ function Split-FullSQLInstanceName $FullSQLInstanceName ) - $sqlServer,$sqlInstanceName = $FullSQLInstanceName.Split('\') + $sqlServer, $sqlInstanceName = $FullSQLInstanceName.Split('\') if ( [System.String]::IsNullOrEmpty($sqlInstanceName) ) { @@ -1214,7 +1258,7 @@ function Split-FullSQLInstanceName } return @{ - SQLServer = $sqlServer + SQLServer = $sqlServer SQLInstanceName = $sqlInstanceName } } @@ -1265,12 +1309,12 @@ function Test-ClusterPermissions { $clusterServiceName { - Write-Verbose -Message ( $script:localizedData.ClusterLoginMissingRecommendedPermissions -f $loginName,( $availabilityGroupManagementPerms -join ', ' ) ) -Verbose + Write-Verbose -Message ( $script:localizedData.ClusterLoginMissingRecommendedPermissions -f $loginName, ( $availabilityGroupManagementPerms -join ', ' ) ) -Verbose } $ntAuthoritySystemName { - Write-Verbose -Message ( $script:localizedData.ClusterLoginMissingPermissions -f $loginName,( $availabilityGroupManagementPerms -join ', ' ) ) -Verbose + Write-Verbose -Message ( $script:localizedData.ClusterLoginMissingPermissions -f $loginName, ( $availabilityGroupManagementPerms -join ', ' ) ) -Verbose } } } @@ -1285,12 +1329,12 @@ function Test-ClusterPermissions { $clusterServiceName { - Write-Verbose -Message ($script:localizedData.ClusterLoginMissingRecommendedPermissions -f $loginName,"Trying with '$ntAuthoritySystemName'.") -Verbose + Write-Verbose -Message ($script:localizedData.ClusterLoginMissingRecommendedPermissions -f $loginName, "Trying with '$ntAuthoritySystemName'.") -Verbose } $ntAuthoritySystemName { - Write-Verbose -Message ( $script:localizedData.ClusterLoginMissing -f $loginName,'' ) -Verbose + Write-Verbose -Message ( $script:localizedData.ClusterLoginMissing -f $loginName, '' ) -Verbose } } } @@ -1299,7 +1343,7 @@ function Test-ClusterPermissions # If neither 'NT SERVICE\ClusSvc' or 'NT AUTHORITY\SYSTEM' have the required permissions, throw an error. if ( -not $clusterPermissionsPresent ) { - throw ($script:localizedData.ClusterPermissionsMissing -f $sqlServer,$sqlInstanceName ) + throw ($script:localizedData.ClusterPermissionsMissing -f $sqlServer, $sqlInstanceName ) } return $clusterPermissionsPresent diff --git a/Tests/Unit/MSFT_SqlAG.Tests.ps1 b/Tests/Unit/MSFT_SqlAG.Tests.ps1 index 11e3b17aa..75b305637 100644 --- a/Tests/Unit/MSFT_SqlAG.Tests.ps1 +++ b/Tests/Unit/MSFT_SqlAG.Tests.ps1 @@ -774,7 +774,6 @@ try BeforeAll { Mock -CommandName Connect-SQL -MockWith $mockConnectSql -Verifiable Mock -CommandName Get-PrimaryReplicaServerObject -MockWith $mockConnectSql -Verifiable - Mock -CommandName Import-SQLPSModule -Verifiable Mock -CommandName New-SqlAvailabilityGroup $mockNewSqlAvailabilityGroup -Verifiable Mock -CommandName New-SqlAvailabilityReplica -MockWith $mockNewSqlAvailabilityGroupReplica -Verifiable Mock -CommandName New-TerminatingError -MockWith { @@ -822,7 +821,6 @@ try Assert-MockCalled -CommandName Connect-SQL -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Get-PrimaryReplicaServerObject -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Import-SQLPSModule -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName New-SqlAvailabilityGroup -Scope It -Times @{Absent=0;Present=1}.$Ensure -Exactly Assert-MockCalled -CommandName New-SqlAvailabilityReplica -Scope It -Times @{Absent=0;Present=1}.$Ensure -Exactly Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly @@ -877,7 +875,6 @@ try Assert-MockCalled -CommandName Connect-SQL -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Get-PrimaryReplicaServerObject -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Import-SQLPSModule -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName New-SqlAvailabilityGroup -Scope It -Times $assertCreateAvailabilityGroup -Exactly Assert-MockCalled -CommandName New-SqlAvailabilityReplica -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 1 -Exactly @@ -934,7 +931,6 @@ try Assert-MockCalled -CommandName Connect-SQL -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Get-PrimaryReplicaServerObject -Scope It -Times 1 -Exactly - Assert-MockCalled -CommandName Import-SQLPSModule -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName New-SqlAvailabilityGroup -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName New-SqlAvailabilityReplica -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly @@ -975,7 +971,6 @@ try Assert-MockCalled -CommandName Connect-SQL -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Get-PrimaryReplicaServerObject -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Import-SQLPSModule -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName New-SqlAvailabilityGroup -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName New-SqlAvailabilityReplica -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly @@ -1030,7 +1025,6 @@ try Assert-MockCalled -CommandName Connect-SQL -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Get-PrimaryReplicaServerObject -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Import-SQLPSModule -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName New-SqlAvailabilityGroup -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName New-SqlAvailabilityReplica -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 1 -Exactly @@ -1080,7 +1074,6 @@ try Assert-MockCalled -CommandName Connect-SQL -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Get-PrimaryReplicaServerObject -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Import-SQLPSModule -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName New-SqlAvailabilityGroup -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName New-SqlAvailabilityReplica -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 1 -Exactly @@ -1130,7 +1123,6 @@ try Assert-MockCalled -CommandName Connect-SQL -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Get-PrimaryReplicaServerObject -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Import-SQLPSModule -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName New-SqlAvailabilityGroup -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName New-SqlAvailabilityReplica -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 1 -Exactly @@ -1188,7 +1180,6 @@ try Assert-MockCalled -CommandName Connect-SQL -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Get-PrimaryReplicaServerObject -Scope It -Times 1 -Exactly - Assert-MockCalled -CommandName Import-SQLPSModule -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName New-SqlAvailabilityGroup -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName New-SqlAvailabilityReplica -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly diff --git a/Tests/Unit/MSFT_SqlSetup.Tests.ps1 b/Tests/Unit/MSFT_SqlSetup.Tests.ps1 index b3f152896..c13e57c57 100644 --- a/Tests/Unit/MSFT_SqlSetup.Tests.ps1 +++ b/Tests/Unit/MSFT_SqlSetup.Tests.ps1 @@ -1003,8 +1003,9 @@ try $testParameters = $mockDefaultParameters.Clone() } - $testProductVersion | ForEach-Object -Process { - $mockSqlMajorVersion = $_ + foreach ($version in $testProductVersion) + { + $mockSqlMajorVersion = $version $mockDefaultInstance_InstanceId = "$($mockSqlDatabaseEngineName)$($mockSqlMajorVersion).$($mockDefaultInstance_InstanceName)" @@ -2960,6 +2961,7 @@ try BeforeAll { # General mocks + Mock -CommandName Import-SQLPSModule Mock -CommandName Get-SqlMajorVersion -MockWith $mockGetSqlMajorVersion -Verifiable # Mocking SharedDirectory and SharedWowDirectory (when not previously installed) @@ -3041,7 +3043,7 @@ try Mock -CommandName Write-Warning - It 'Should warn that target nod need to restart' { + It 'Should warn that target node need to restart' { { Set-TargetResource @testParameters } | Should -Not -Throw Assert-MockCalled -CommandName Write-Warning -Exactly -Times 1 -Scope It @@ -3057,6 +3059,7 @@ try { Set-TargetResource @testParameters } | Should -Not -Throw Assert-MockCalled -CommandName Write-Warning -Exactly -Times 1 -Scope It + Assert-MockCalled -CommandName Import-SQLPSModule -Exactly -Times 0 -Scope It } } } @@ -3151,6 +3154,7 @@ try Assert-MockCalled -CommandName Start-SqlSetupProcess -Exactly -Times 1 -Scope It Assert-MockCalled -CommandName Test-TargetResource -Exactly -Times 1 -Scope It + Assert-MockCalled -CommandName Import-SQLPSModule -Exactly -Times 1 -Scope It } if( $mockSqlMajorVersion -in (13,14) ) @@ -3530,6 +3534,7 @@ try Assert-MockCalled -CommandName Start-SqlSetupProcess -Exactly -Times 1 -Scope It Assert-MockCalled -CommandName Test-TargetResource -Exactly -Times 1 -Scope It + Assert-MockCalled -CommandName Import-SQLPSModule -Exactly -Times 1 -Scope It } if( $mockSqlMajorVersion -in (13,14) ) @@ -4701,7 +4706,7 @@ try } { Copy-ItemWithRobocopy @copyItemWithRobocopyParameter } | Should -Not -Throw - + } It 'Should finish successfully with exit code 2' { diff --git a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 index 91aa4d079..b83a7286c 100644 --- a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 +++ b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 @@ -725,10 +725,36 @@ InModuleScope $script:moduleName { } } - $mockGetModule = { - return New-Object -TypeName PSObject -Property @{ - Name = $mockModuleNameToImport - } + $mockGetModuleSqlServer = { + # Return an array to test so that the latest version is only imported. + return @( + New-Object -TypeName PSObject -Property @{ + Name = 'SqlServer' + Version = [Version] '1.0' + } + + New-Object -TypeName PSObject -Property @{ + Name = 'SqlServer' + Version = [Version] '2.0' + } + ) + } + + $sqlPsLatestModulePath = 'C:\Program Files (x86)\Microsoft SQL Server\130\Tools\PowerShell\Modules\SQLPS\Sqlps.ps1' + + $mockGetModuleSqlPs = { + # Return an array to test so that the latest version is only imported. + return @( + New-Object -TypeName PSObject -Property @{ + Name = 'SQLPS' + Path = 'C:\Program Files (x86)\Microsoft SQL Server\120\Tools\PowerShell\Modules\SQLPS\Sqlps.ps1' + } + + New-Object -TypeName PSObject -Property @{ + Name = 'SQLPS' + Path = $sqlPsLatestModulePath + } + ) } $mockGetModule_SqlServer_ParameterFilter = { @@ -739,7 +765,7 @@ InModuleScope $script:moduleName { $FullyQualifiedName.Name -eq 'SQLPS' -and $ListAvailable -eq $true } - Describe 'Testing Import-SQLPSModule' -Tag ImportSQLPSModule { + Describe 'Testing Import-SQLPSModule' -Tag 'ImportSQLPSModule' { BeforeEach { Mock -CommandName Push-Location -Verifiable Mock -CommandName Pop-Location -Verifiable @@ -747,12 +773,49 @@ InModuleScope $script:moduleName { Mock -CommandName New-InvalidOperationException -MockWith $mockThrowLocalizedMessage -Verifiable } - Context 'When module SqlServer exists' { - $mockModuleNameToImport = 'SqlServer' - $mockExpectedModuleNameToImport = 'SqlServer' + Context 'When module SqlServer is already loaded into the session' { + BeforeAll { + Mock -CommandName Get-Module -MockWith { + return @{ + Name = 'SqlServer' + } + } + } + + It 'Should use the already loaded module and not call Import-Module' { + { Import-SQLPSModule } | Should -Not -Throw + + Assert-MockCalled -CommandName Import-Module -Exactly -Times 0 -Scope It + } + } + + Context 'When module SQLPS is already loaded into the session' { + BeforeAll { + Mock -CommandName Get-Module -MockWith { + return @{ + Name = 'SQLPS' + } + } + } + + It 'Should use the already loaded module and not call Import-Module' { + { Import-SQLPSModule } | Should -Not -Throw + + Assert-MockCalled -CommandName Import-Module -Exactly -Times 0 -Scope It + } + } + + Context 'When module SqlServer exists, but not loaded into the session' { + BeforeAll { + Mock -CommandName Get-Module -ParameterFilter { + $PSBoundParameters.ContainsKey('Name') -eq $true + } + + $mockExpectedModuleNameToImport = 'SqlServer' + } It 'Should import the SqlServer module without throwing' { - Mock -CommandName Get-Module -MockWith $mockGetModule -ParameterFilter $mockGetModule_SqlServer_ParameterFilter -Verifiable + Mock -CommandName Get-Module -MockWith $mockGetModuleSqlServer -ParameterFilter $mockGetModule_SqlServer_ParameterFilter -Verifiable { Import-SQLPSModule } | Should -Not -Throw @@ -763,36 +826,43 @@ InModuleScope $script:moduleName { } } - Context 'When only module SQLPS exists' { - $mockModuleNameToImport = 'SQLPS' - $mockExpectedModuleNameToImport = 'SQLPS' + Context 'When only module SQLPS exists, but not loaded into the session, and using -Force' { + BeforeAll { + Mock -CommandName Remove-Module + Mock -CommandName Get-Module -ParameterFilter { + $PSBoundParameters.ContainsKey('Name') -eq $true + } + + $mockExpectedModuleNameToImport = $sqlPsLatestModulePath + } It 'Should import the SqlServer module without throwing' { - Mock -CommandName Get-Module -MockWith $mockGetModule -ParameterFilter $mockGetModule_SQLPS_ParameterFilter -Verifiable + Mock -CommandName Get-Module -MockWith $mockGetModuleSqlPs -ParameterFilter $mockGetModule_SQLPS_ParameterFilter -Verifiable Mock -CommandName Get-Module -MockWith { return $null } -ParameterFilter $mockGetModule_SqlServer_ParameterFilter -Verifiable - { Import-SQLPSModule } | Should -Not -Throw + { Import-SQLPSModule -Force } | Should -Not -Throw Assert-MockCalled -CommandName Get-Module -ParameterFilter $mockGetModule_SqlServer_ParameterFilter -Exactly -Times 1 -Scope It Assert-MockCalled -CommandName Get-Module -ParameterFilter $mockGetModule_SQLPS_ParameterFilter -Exactly -Times 1 -Scope It Assert-MockCalled -CommandName Push-Location -Exactly -Times 1 -Scope It Assert-MockCalled -CommandName Pop-Location -Exactly -Times 1 -Scope It + Assert-MockCalled -CommandName Remove-Module -Exactly -Times 1 -Scope It Assert-MockCalled -CommandName Import-Module -Exactly -Times 1 -Scope It } } Context 'When neither SqlServer or SQLPS exists' { - $mockModuleNameToImport = 'UnknownModule' - $mockExpectedModuleNameToImport = 'SQLPS' + $mockExpectedModuleNameToImport = $sqlPsLatestModulePath It 'Should throw the correct error message' { Mock -CommandName Get-Module { Import-SQLPSModule } | Should -Throw $script:localizedData.PowerShellSqlModuleNotFound - Assert-MockCalled -CommandName Get-Module -Exactly -Times 2 -Scope It + Assert-MockCalled -CommandName Get-Module -ParameterFilter $mockGetModule_SqlServer_ParameterFilter -Exactly -Times 1 -Scope It + Assert-MockCalled -CommandName Get-Module -ParameterFilter $mockGetModule_SQLPS_ParameterFilter -Exactly -Times 1 -Scope It Assert-MockCalled -CommandName Push-Location -Exactly -Times 0 -Scope It Assert-MockCalled -CommandName Pop-Location -Exactly -Times 0 -Scope It Assert-MockCalled -CommandName Import-Module -Exactly -Times 0 -Scope It @@ -800,12 +870,11 @@ InModuleScope $script:moduleName { } Context 'When Import-Module fails to load the module' { - $mockModuleNameToImport = 'SqlServer' $mockExpectedModuleNameToImport = 'SqlServer' It 'Should throw the correct error message' { $errorMessage = 'Mock Import-Module throwing a mocked error.' - Mock -CommandName Get-Module -MockWith $mockGetModule -ParameterFilter $mockGetModule_SqlServer_ParameterFilter -Verifiable + Mock -CommandName Get-Module -MockWith $mockGetModuleSqlServer -ParameterFilter $mockGetModule_SqlServer_ParameterFilter -Verifiable Mock -CommandName Import-Module -MockWith { throw $errorMessage } @@ -821,13 +890,12 @@ InModuleScope $script:moduleName { # This is to test the tests (so the mock throws correctly) Context 'When mock Import-Module is called with wrong module name' { - $mockModuleNameToImport = 'SqlServer' $mockExpectedModuleNameToImport = 'UnknownModule' It 'Should throw the correct error message' { - Mock -CommandName Get-Module -MockWith $mockGetModule -ParameterFilter $mockGetModule_SqlServer_ParameterFilter -Verifiable + Mock -CommandName Get-Module -MockWith $mockGetModuleSqlServer -ParameterFilter $mockGetModule_SqlServer_ParameterFilter -Verifiable - { Import-SQLPSModule } | Should -Throw ($script:localizedData.FailedToImportPowerShellSqlModule -f $mockModuleNameToImport) + { Import-SQLPSModule } | Should -Throw ($script:localizedData.FailedToImportPowerShellSqlModule -f 'SqlServer') Assert-MockCalled -CommandName Get-Module -Exactly -Times 1 -Scope It Assert-MockCalled -CommandName Push-Location -Exactly -Times 1 -Scope It diff --git a/en-US/SqlServerDscHelper.strings.psd1 b/en-US/SqlServerDscHelper.strings.psd1 index 33fcb096e..71acf9033 100644 --- a/en-US/SqlServerDscHelper.strings.psd1 +++ b/en-US/SqlServerDscHelper.strings.psd1 @@ -16,9 +16,10 @@ ConvertFrom-StringData @' UnableToCompareProperty = Unable to compare property {0} as the type {1} is not handled by the Test-SQLDSCParameterState cmdlet. PreferredModuleFound = Preferred module SqlServer found. PreferredModuleNotFound = Information: PowerShell module SqlServer not found, trying to use older SQLPS module. - ImportingPowerShellModule = Importing PowerShell module {0}. + ImportedPowerShellModule = Importing PowerShell module '{0}' with version '{1}' from path '{2}'. + PowerShellModuleAlreadyImported = Found PowerShell module {0} already imported in the session. + ModuleForceRemoval = Forcibly removed the SQL PowerShell module from the session to import it fresh again. DebugMessagePushingLocation = SQLPS module changes CWD to SQLSERVER:\ when loading, pushing location to pop it when module is loaded. - DebugMessageImportedPowerShellModule = Module {0} imported. DebugMessagePoppingLocation = Popping location back to what it was before importing SQLPS module. PowerShellSqlModuleNotFound = Neither PowerShell module SqlServer or SQLPS was found. Unable to run SQL Server cmdlets. FailedToImportPowerShellSqlModule = Failed to import {0} module. diff --git a/sv-SE/SqlServerDscHelper.strings.psd1 b/sv-SE/SqlServerDscHelper.strings.psd1 index fcf20b6b9..905d49201 100644 --- a/sv-SE/SqlServerDscHelper.strings.psd1 +++ b/sv-SE/SqlServerDscHelper.strings.psd1 @@ -16,9 +16,10 @@ ConvertFrom-StringData @' UnableToCompareProperty = Inte möjligt att jämföra egenskapen {0} som typen {1}. {1} hanteras inte av Test-SQLDscParameterState cmdlet. PreferredModuleFound = Föredragen modul SqlServer funnen. PreferredModuleNotFound = Information: PowerShell modul SqlServer ej funnen, försöker att använda äldre SQLPS modul. - ImportingPowerShellModule = Importerar PowerShell modul {0}. + ImportedPowerShellModule = Importerade PowerShell modul '{0}' med version '{1}' från mapp '{2}'. + PowerShellModuleAlreadyImported = Fann att PowerShell modul {0} redan är importerad i sessionen. + ModuleForceRemoval = Tvingade bort den tidigare SQL PowerShell modulen från sessionen för att importera den fräsch igen. DebugMessagePushingLocation = SQLPS modul ändrar nuvarande katalog till SQLSERVER:\ när modulen laddas, sparar nuvarande katalog så den kan återställas efter modulen laddats. - DebugMessageImportedPowerShellModule = Modul {0} importerad. DebugMessagePoppingLocation = Återställer nuvarande katalog till vad den var innan modulen SQLPS importerades. PowerShellSqlModuleNotFound = Varken PowerShell modulen SqlServer eller SQLPS kunde hittas. Kommer inte kunna köra SQL Server cmdlets. FailedToImportPowerShellSqlModule = Misslyckades att importera {0} modulen. From 57670fb034e3dc3fe51bc5ffa690c4e588b3a606 Mon Sep 17 00:00:00 2001 From: Johan Ljunggren Date: Sat, 7 Jul 2018 11:48:10 +0200 Subject: [PATCH 2/2] Fix review comments at r1 --- DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1 | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1 b/DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1 index 47a6b2d6f..9cdbec2a3 100644 --- a/DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1 +++ b/DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1 @@ -1455,7 +1455,15 @@ function Set-TargetResource if ((-not $setupEndedInError) -and $forceReloadPowerShellModule) { - # Force reload of SQLPS module if newer version was installed. + <# + Force reload of SQLPS module in case a newer version of + SQL Server was installed that contains a newer version + of the SQLPS module, although if SqlServer module exist + on the target node, that will be used regardless. + This is to make sure we use the latest SQLPS module that + matches the latest assemblies in GAC, mitigating for example + issue #1151. + #> Import-SQLPSModule -Force }