diff --git a/CHANGELOG.md b/CHANGELOG.md index c4abc4802..4578c75cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Now imports the preferred module into the global scope so that MOF-based resources (that is in another module scope) can use the imported module. - Some code cleanup ([issue #1881](https://github.com/dsccommunity/SqlServerDsc/issues/1881)). +- SqlServerDsc.Common + - `Restart-SqlService` no longer silently ignores errors that prevents + the instance to go online. If the instance has not gone online during + the timeout period the error thrown will no contain the last error + reported by `Connect-SQL` ([issue #1891](https://github.com/dsccommunity/SqlServerDsc/issues/1891)). ### Fixed diff --git a/source/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psm1 b/source/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psm1 index d4cf3ddd4..33f42dbe5 100644 --- a/source/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psm1 +++ b/source/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psm1 @@ -1014,10 +1014,12 @@ function Restart-SqlService { $connectTimer = [System.Diagnostics.StopWatch]::StartNew() + $connectSqlError = $null + do { # This call, if it fails, will take between ~9-10 seconds to return. - $testConnectionServerObject = Connect-SQL -ServerName $ServerName -InstanceName $InstanceName -ErrorAction 'SilentlyContinue' + $testConnectionServerObject = Connect-SQL -ServerName $ServerName -InstanceName $InstanceName -ErrorAction 'SilentlyContinue' -ErrorVariable 'connectSqlError' # Make sure we have an SMO object to test Status if ($testConnectionServerObject) @@ -1037,8 +1039,22 @@ function Restart-SqlService # Was the timeout period reach before able to connect to the SQL Server instance? if (-not $testConnectionServerObject -or $testConnectionServerObject.Status -ne 'Online') { - $errorMessage = $script:localizedData.FailedToConnectToInstanceTimeout -f $ServerName, $InstanceName, $Timeout - New-InvalidOperationException -Message $errorMessage + $errorMessage = $script:localizedData.FailedToConnectToInstanceTimeout -f @( + $ServerName, + $InstanceName, + $Timeout + ) + + $newInvalidOperationExceptionParameters = @{ + Message = $errorMessage + } + + if ($connectSqlError) + { + $newInvalidOperationExceptionParameters.ErrorRecord = $connectSqlError[$connectSqlError.Count - 1] + } + + New-InvalidOperationException @newInvalidOperationExceptionParameters } } } diff --git a/tests/Unit/SqlServerDsc.Common.Tests.ps1 b/tests/Unit/SqlServerDsc.Common.Tests.ps1 index 215e7ac59..7eb4d85c7 100644 --- a/tests/Unit/SqlServerDsc.Common.Tests.ps1 +++ b/tests/Unit/SqlServerDsc.Common.Tests.ps1 @@ -801,6 +801,35 @@ Describe 'SqlServerDsc.Common\Start-SqlSetupProcess' -Tag 'StartSqlSetupProcess' } Describe 'SqlServerDsc.Common\Restart-SqlService' -Tag 'RestartSqlService' { + BeforeAll { + InModuleScope -ScriptBlock { + # Stubs for cross-platform testing. + function script:Get-Service + { + throw '{0}: StubNotImplemented' -f $MyInvocation.MyCommand + } + + function script:Restart-Service + { + throw '{0}: StubNotImplemented' -f $MyInvocation.MyCommand + } + + function script:Start-Service + { + throw '{0}: StubNotImplemented' -f $MyInvocation.MyCommand + } + } + } + + AfterAll { + InModuleScope -ScriptBlock { + # Remove stubs that was used for cross-platform testing. + Remove-Item -Path function:Get-Service + Remove-Item -Path function:Restart-Service + Remove-Item -Path function:Start-Service + } + } + Context 'Restart-SqlService standalone instance' { Context 'When the Windows services should be restarted' { BeforeAll { @@ -834,7 +863,7 @@ Describe 'SqlServerDsc.Common\Restart-SqlService' -Tag 'RestartSqlService' { } It 'Should restart SQL Service and running SQL Agent service' { - { Restart-SqlService -ServerName $env:COMPUTERNAME -InstanceName 'MSSQLSERVER' } | Should -Not -Throw + { Restart-SqlService -ServerName (Get-ComputerName) -InstanceName 'MSSQLSERVER' } | Should -Not -Throw Should -Invoke -CommandName Connect-SQL -ParameterFilter { <# @@ -854,7 +883,7 @@ Describe 'SqlServerDsc.Common\Restart-SqlService' -Tag 'RestartSqlService' { Context 'When skipping the cluster check' { It 'Should restart SQL Service and running SQL Agent service' { - { Restart-SqlService -ServerName $env:COMPUTERNAME -InstanceName 'MSSQLSERVER' -SkipClusterCheck } | Should -Not -Throw + { Restart-SqlService -ServerName (Get-ComputerName) -InstanceName 'MSSQLSERVER' -SkipClusterCheck } | Should -Not -Throw Should -Invoke -CommandName Connect-SQL -ParameterFilter { <# @@ -875,7 +904,7 @@ Describe 'SqlServerDsc.Common\Restart-SqlService' -Tag 'RestartSqlService' { Context 'When skipping the online check' { It 'Should restart SQL Service and running SQL Agent service and not wait for the SQL Server instance to come back online' { - { Restart-SqlService -ServerName $env:COMPUTERNAME -InstanceName 'MSSQLSERVER' -SkipWaitForOnline } | Should -Not -Throw + { Restart-SqlService -ServerName (Get-ComputerName) -InstanceName 'MSSQLSERVER' -SkipWaitForOnline } | Should -Not -Throw Should -Invoke -CommandName Connect-SQL -ParameterFilter { <# @@ -923,7 +952,7 @@ Describe 'SqlServerDsc.Common\Restart-SqlService' -Tag 'RestartSqlService' { } It 'Should just call Restart-SqlClusterService to restart the SQL Server cluster instance' { - { Restart-SqlService -ServerName $env:COMPUTERNAME -InstanceName 'MSSQLSERVER' } | Should -Not -Throw + { Restart-SqlService -ServerName (Get-ComputerName) -InstanceName 'MSSQLSERVER' } | Should -Not -Throw Should -Invoke -CommandName Connect-SQL -ParameterFilter { <# @@ -943,7 +972,7 @@ Describe 'SqlServerDsc.Common\Restart-SqlService' -Tag 'RestartSqlService' { Context 'When passing the Timeout value' { It 'Should just call Restart-SqlClusterService with the correct parameter' { - { Restart-SqlService -ServerName $env:COMPUTERNAME -InstanceName 'MSSQLSERVER' -Timeout 120 } | Should -Not -Throw + { Restart-SqlService -ServerName (Get-ComputerName) -InstanceName 'MSSQLSERVER' -Timeout 120 } | Should -Not -Throw Should -Invoke -CommandName Restart-SqlClusterService -ParameterFilter { <# @@ -957,7 +986,7 @@ Describe 'SqlServerDsc.Common\Restart-SqlService' -Tag 'RestartSqlService' { Context 'When passing the OwnerNode value' { It 'Should just call Restart-SqlClusterService with the correct parameter' { - { Restart-SqlService -ServerName $env:COMPUTERNAME -InstanceName 'MSSQLSERVER' -OwnerNode @('TestNode') } | Should -Not -Throw + { Restart-SqlService -ServerName (Get-ComputerName) -InstanceName 'MSSQLSERVER' -OwnerNode @('TestNode') } | Should -Not -Throw Should -Invoke -CommandName Restart-SqlClusterService -ParameterFilter { <# @@ -995,7 +1024,7 @@ Describe 'SqlServerDsc.Common\Restart-SqlService' -Tag 'RestartSqlService' { } It 'Should restart SQL Service and not try to restart missing SQL Agent service' { - { Restart-SqlService -ServerName $env:COMPUTERNAME -InstanceName 'NOAGENT' -SkipClusterCheck } | Should -Not -Throw + { Restart-SqlService -ServerName (Get-ComputerName) -InstanceName 'NOAGENT' -SkipClusterCheck } | Should -Not -Throw Should -Invoke -CommandName Get-Service -Scope It -Exactly -Times 1 Should -Invoke -CommandName Restart-Service -Scope It -Exactly -Times 1 @@ -1035,7 +1064,7 @@ Describe 'SqlServerDsc.Common\Restart-SqlService' -Tag 'RestartSqlService' { } It 'Should restart SQL Service and not try to restart stopped SQL Agent service' { - { Restart-SqlService -ServerName $env:COMPUTERNAME -InstanceName 'STOPPEDAGENT' -SkipClusterCheck } | Should -Not -Throw + { Restart-SqlService -ServerName (Get-ComputerName) -InstanceName 'STOPPEDAGENT' -SkipClusterCheck } | Should -Not -Throw Should -Invoke -CommandName Get-Service -Scope It -Exactly -Times 1 Should -Invoke -CommandName Restart-Service -Scope It -Exactly -Times 1 @@ -1044,63 +1073,122 @@ Describe 'SqlServerDsc.Common\Restart-SqlService' -Tag 'RestartSqlService' { } Context 'When it fails to connect to the instance within the timeout period' { - BeforeAll { - Mock -CommandName Connect-SQL -MockWith { - return @{ - Name = 'MSSQLSERVER' - InstanceName = '' - ServiceName = 'MSSQLSERVER' - Status = 'Offline' + Context 'When the connection throws an exception' { + BeforeAll { + Mock -CommandName Connect-SQL -MockWith { + # Using SilentlyContinue to not show the errors in the Pester output. + Write-Error -Message 'Mock connection error' -ErrorAction 'SilentlyContinue' } - } - Mock -CommandName Get-Service -MockWith { - return @{ - Name = 'MSSQLSERVER' - DisplayName = 'Microsoft SQL Server (MSSQLSERVER)' - DependentServices = @( - @{ - Name = 'SQLSERVERAGENT' - DisplayName = 'SQL Server Agent (MSSQLSERVER)' - Status = 'Running' - DependentServices = @() - } - ) + Mock -CommandName Get-Service -MockWith { + return @{ + Name = 'MSSQLSERVER' + DisplayName = 'Microsoft SQL Server (MSSQLSERVER)' + DependentServices = @( + @{ + Name = 'SQLSERVERAGENT' + DisplayName = 'SQL Server Agent (MSSQLSERVER)' + Status = 'Running' + DependentServices = @() + } + ) + } } + + Mock -CommandName Restart-Service + Mock -CommandName Start-Service } - Mock -CommandName Restart-Service - Mock -CommandName Start-Service + It 'Should wait for timeout before throwing error message' { + $mockLocalizedString = InModuleScope -ScriptBlock { + $localizedData.FailedToConnectToInstanceTimeout + } + + $mockErrorMessage = Get-InvalidOperationRecord -Message ( + ($mockLocalizedString -f (Get-ComputerName), 'MSSQLSERVER', 4) + '*Mock connection error*' + ) + + $mockErrorMessage.Exception.Message | Should -Not -BeNullOrEmpty + + { + Restart-SqlService -ServerName (Get-ComputerName) -InstanceName 'MSSQLSERVER' -Timeout 4 -SkipClusterCheck + } | Should -Throw -ExpectedMessage $mockErrorMessage + + <# + Not using -Exactly to handle when CI is slower, result is + that there are 3 calls to Connect-SQL. + #> + Should -Invoke -CommandName Connect-SQL -ParameterFilter { + <# + Make sure we assert the second call to Connect-SQL + + Due to issue https://github.com/pester/Pester/issues/1542 + we cannot use `$PSBoundParameters.ContainsKey('ErrorAction') -eq $true`. + #> + $ErrorAction -eq 'SilentlyContinue' + } -Scope It -Times 2 + } } - It 'Should wait for timeout before throwing error message' { - $mockLocalizedString = InModuleScope -ScriptBlock { - $localizedData.FailedToConnectToInstanceTimeout + Context 'When the Status returns offline' { + BeforeAll { + Mock -CommandName Connect-SQL -MockWith { + return @{ + Name = 'MSSQLSERVER' + InstanceName = '' + ServiceName = 'MSSQLSERVER' + Status = 'Offline' + } + } + + Mock -CommandName Get-Service -MockWith { + return @{ + Name = 'MSSQLSERVER' + DisplayName = 'Microsoft SQL Server (MSSQLSERVER)' + DependentServices = @( + @{ + Name = 'SQLSERVERAGENT' + DisplayName = 'SQL Server Agent (MSSQLSERVER)' + Status = 'Running' + DependentServices = @() + } + ) + } + } + + Mock -CommandName Restart-Service + Mock -CommandName Start-Service } - $mockErrorMessage = Get-InvalidOperationRecord -Message ( - $mockLocalizedString -f $env:ComputerName, 'MSSQLSERVER', 4 - ) + It 'Should wait for timeout before throwing error message' { + $mockLocalizedString = InModuleScope -ScriptBlock { + $localizedData.FailedToConnectToInstanceTimeout + } - $mockErrorMessage.Exception.Message | Should -Not -BeNullOrEmpty + $mockErrorMessage = Get-InvalidOperationRecord -Message ( + $mockLocalizedString -f (Get-ComputerName), 'MSSQLSERVER', 4 + ) - { - Restart-SqlService -ServerName $env:ComputerName -InstanceName 'MSSQLSERVER' -Timeout 4 -SkipClusterCheck - } | Should -Throw -ExpectedMessage $mockErrorMessage + $mockErrorMessage.Exception.Message | Should -Not -BeNullOrEmpty - <# - Not using -Exactly to handle when CI is slower, result is - that there are 3 calls to Connect-SQL. - #> - Should -Invoke -CommandName Connect-SQL -ParameterFilter { - <# - Make sure we assert the second call to Connect-SQL + { + Restart-SqlService -ServerName (Get-ComputerName) -InstanceName 'MSSQLSERVER' -Timeout 4 -SkipClusterCheck + } | Should -Throw -ExpectedMessage $mockErrorMessage - Due to issue https://github.com/pester/Pester/issues/1542 - we cannot use `$PSBoundParameters.ContainsKey('ErrorAction') -eq $true`. + <# + Not using -Exactly to handle when CI is slower, result is + that there are 3 calls to Connect-SQL. #> - $ErrorAction -eq 'SilentlyContinue' - } -Scope It -Times 2 + Should -Invoke -CommandName Connect-SQL -ParameterFilter { + <# + Make sure we assert the second call to Connect-SQL + + Due to issue https://github.com/pester/Pester/issues/1542 + we cannot use `$PSBoundParameters.ContainsKey('ErrorAction') -eq $true`. + #> + $ErrorAction -eq 'SilentlyContinue' + } -Scope It -Times 2 + } } } }