diff --git a/CHANGELOG.md b/CHANGELOG.md index 95bea7b9f..1b9db5590 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,11 @@ - Updated the security token for AppVeyor status badge in README.md. When we renamed the repository the security token was changed ([issue #1012](https://github.com/PowerShell/SqlServerDsc/issues/1012)). + - Now the helper function Restart-SqlService, after restarting the SQL Server + service, does not return until it can connect to the SQL Server instance, and + the instance returns status 'Online' ([issue #1008](https://github.com/PowerShell/SqlServerDsc/issues/1008)). + If it fails to connect within the timeout period (defaults to 120 seconds) it + throws an error. - Fixed typo in comment-base help for helper function Test-AvailabilityReplicaSeedingModeAutomatic. - Style cleanup in helper function tests. - Changes to SqlAG @@ -123,6 +128,11 @@ name is correctly returned as a string ([issue #982](https://github.com/PowerShell/SqlServerDsc/issues/982)). - Added integration tests ([issue #980](https://github.com/PowerShell/SqlServerDsc/issues/980)). - Minor code style cleanup. + - The timing issue that the resource returned before SQL Server service was + actually restarted has been solved by a change in the helper function + Restart-SqlService ([issue #1008](https://github.com/PowerShell/SqlServerDsc/issues/1008)). + Now Restart-SqlService waits for the instance to return status 'Online' or + throws an error saying it failed to connect within the timeout period. - Changes to SqlSetup - Added parameter `ASServerMode` to support installing Analysis Services in Multidimensional mode, Tabular mode and PowerPivot mode diff --git a/SqlServerDscHelper.psm1 b/SqlServerDscHelper.psm1 index 3f64b7879..48d264a6b 100644 --- a/SqlServerDscHelper.psm1 +++ b/SqlServerDscHelper.psm1 @@ -842,6 +842,61 @@ function Restart-SqlService $_ | Start-Service } } + + Write-Verbose -Message ($script:localizedData.WaitingInstanceTimeout -f $SQLServer, $SQLInstanceName, $Timeout) -Verbose + + $startJobScriptBlock = { + param + ( + [Parameter()] + [System.String] + $ServerName, + + [Parameter()] + [System.String] + $InstanceName, + + [Parameter()] + [System.String] + $ScriptRoot + ) + + Import-Module -Name (Join-Path -Path $ScriptRoot -ChildPath 'SqlServerDscHelper.psm1') + + do + { + # This call, if it fails, will take between ~9-10 seconds to return. + $serverObject = Connect-SQL -SQLServer $ServerName -SQLInstanceName $InstanceName -ErrorAction SilentlyContinue + if ($serverObject.Status -ne 'Online') + { + # Waiting 2 seconds to not hammer the SQL Server instance. + Start-Sleep -Seconds 2 + } + } until ($serverObject.Status -eq 'Online') + } + + $startJobResult = Start-Job -ScriptBlock $startJobScriptBlock -ArgumentList @( + $SQLServer + $SQLInstanceName + $PSScriptRoot + ) + + Wait-Job -Job $startJobResult -Timeout $Timeout + + if ($startJobResult.JobStateInfo.State -ne 'Completed') + { + # Output any verbose messages and error messages. + Receive-Job -Job $startJobResult + + # Make sure the job is stopped. + Stop-Job -Job $startJobResult + + $errorMessage = $script:localizedData.FailedToConnectToInstanceTimeout -f $SQLServer, $SQLInstanceName, $Timeout + New-InvalidOperationException -Message $errorMessage + } + + # Make sure the job is removed. + Remove-Job -Job $startJobResult -Force } <# diff --git a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 index e28588418..d42e09880 100644 --- a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 +++ b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 @@ -123,76 +123,122 @@ InModuleScope $script:moduleName { $mockSetupCredentialSecurePassword = ConvertTo-SecureString -String $mockSetupCredentialPassword -AsPlainText -Force $mockSetupCredential = New-Object -TypeName PSCredential -ArgumentList ($mockSetupCredentialUserName, $mockSetupCredentialSecurePassword) + $mockStartJobName = 'RestartSqlServiceUnitTest' + Describe 'Testing Restart-SqlService' { - Context 'Restart-SqlService standalone instance' { - Mock -CommandName Connect-SQL -MockWith { - return @{ - Name = 'MSSQLSERVER' - InstanceName = '' - ServiceName = 'MSSQLSERVER' - } - } -Verifiable -ParameterFilter { $SQLInstanceName -eq 'MSSQLSERVER' } + BeforeAll { + # Setting up static variables. + $mockJobStateInfoStateCompleted = 'Completed' + $mockJobStateInfoStateFailed = 'Failed' + } - Mock -CommandName Connect-SQL -MockWith { - return @{ - Name = 'NOAGENT' - InstanceName = 'NOAGENT' - ServiceName = 'NOAGENT' + BeforeEach { + <# + Unable to find a way to mock the class System.Management.Automation.Job + without actually starting a job. + Making sure the jobs are stopped and removed in an AfterEch-block. + + NOTE! We cannot mock Wait-Job, Receive-Job, Stop-Job and Remove-job. + We actually run those on this job object we "mock". + #> + Mock -CommandName Start-Job -MockWith { + if ($mockDynamicJobStateInfoState -eq $mockJobStateInfoStateCompleted) + { + $startJobScriptBlock = { + Write-Verbose -Message 'Dummy scriptblock for Start-Job mock' -Verbose + } } - } -Verifiable -ParameterFilter { $SQLInstanceName -eq 'NOAGENT' } - - Mock -CommandName Connect-SQL -MockWith { - return @{ - Name = 'STOPPEDAGENT' - InstanceName = 'STOPPEDAGENT' - ServiceName = 'STOPPEDAGENT' + else + { + $startJobScriptBlock = { + Start-Sleep -Seconds 10 + } } - } -Verifiable -ParameterFilter { $SQLInstanceName -eq 'STOPPEDAGENT' } - ## SQL instance with running SQL Agent Service - Mock -CommandName Get-Service { - return @{ - Name = 'MSSQLSERVER' - DisplayName = 'Microsoft SQL Server (MSSQLSERVER)' - DependentServices = @( - @{ - Name = 'SQLSERVERAGENT' - DisplayName = 'SQL Server Agent (MSSQLSERVER)' - Status = 'Running' - DependentServices = @() - } - ) - } - } -Verifiable -ParameterFilter { $DisplayName -eq 'SQL Server (MSSQLSERVER)' } + $jobObject = Start-Job -Name $mockStartJobName -ScriptBlock $startJobScriptBlock - ## SQL instance with no installed SQL Agent Service - Mock -CommandName Get-Service { - return @{ - Name = 'MSSQL$NOAGENT' - DisplayName = 'Microsoft SQL Server (NOAGENT)' - DependentServices = @() - } - } -Verifiable -ParameterFilter { $DisplayName -eq 'SQL Server (NOAGENT)' } + return $jobObject + } -Verifiable -ParameterFilter { + <# + NOTE! The parameter filter must be here so we are not + mocking ourself (in an endless loop). + #> + $Name -ne $mockStartJobName + } + } - ## SQL instance with stopped SQL Agent Service - Mock -CommandName Get-Service { - return @{ - Name = 'MSSQL$STOPPEDAGENT' - DisplayName = 'Microsoft SQL Server (STOPPEDAGENT)' - DependentServices = @( - @{ - Name = 'SQLAGENT$STOPPEDAGENT' - DisplayName = 'SQL Server Agent (STOPPEDAGENT)' - Status = 'Stopped' - DependentServices = @() - } - ) - } - } -Verifiable -ParameterFilter { $DisplayName -eq 'SQL Server (STOPPEDAGENT)' } + Context 'Restart-SqlService standalone instance' { + BeforeAll { + Mock -CommandName Connect-SQL -MockWith { + return @{ + Name = 'MSSQLSERVER' + InstanceName = '' + ServiceName = 'MSSQLSERVER' + } + } -Verifiable -ParameterFilter { $SQLInstanceName -eq 'MSSQLSERVER' } - Mock -CommandName Restart-Service -Verifiable + Mock -CommandName Connect-SQL -MockWith { + return @{ + Name = 'NOAGENT' + InstanceName = 'NOAGENT' + ServiceName = 'NOAGENT' + } + } -Verifiable -ParameterFilter { $SQLInstanceName -eq 'NOAGENT' } - Mock -CommandName Start-Service -Verifiable + Mock -CommandName Connect-SQL -MockWith { + return @{ + Name = 'STOPPEDAGENT' + InstanceName = 'STOPPEDAGENT' + ServiceName = 'STOPPEDAGENT' + } + } -Verifiable -ParameterFilter { $SQLInstanceName -eq 'STOPPEDAGENT' } + + ## SQL instance with running SQL Agent Service + Mock -CommandName Get-Service -MockWith { + return @{ + Name = 'MSSQLSERVER' + DisplayName = 'Microsoft SQL Server (MSSQLSERVER)' + DependentServices = @( + @{ + Name = 'SQLSERVERAGENT' + DisplayName = 'SQL Server Agent (MSSQLSERVER)' + Status = 'Running' + DependentServices = @() + } + ) + } + } -Verifiable -ParameterFilter { $DisplayName -eq 'SQL Server (MSSQLSERVER)' } + + ## SQL instance with no installed SQL Agent Service + Mock -CommandName Get-Service -MockWith { + return @{ + Name = 'MSSQL$NOAGENT' + DisplayName = 'Microsoft SQL Server (NOAGENT)' + DependentServices = @() + } + } -Verifiable -ParameterFilter { $DisplayName -eq 'SQL Server (NOAGENT)' } + + ## SQL instance with stopped SQL Agent Service + Mock -CommandName Get-Service -MockWith { + return @{ + Name = 'MSSQL$STOPPEDAGENT' + DisplayName = 'Microsoft SQL Server (STOPPEDAGENT)' + DependentServices = @( + @{ + Name = 'SQLAGENT$STOPPEDAGENT' + DisplayName = 'SQL Server Agent (STOPPEDAGENT)' + Status = 'Stopped' + DependentServices = @() + } + ) + } + } -Verifiable -ParameterFilter { $DisplayName -eq 'SQL Server (STOPPEDAGENT)' } + + Mock -CommandName Restart-Service -Verifiable + Mock -CommandName Start-Service -Verifiable + + $mockDynamicJobStateInfoState = $mockJobStateInfoStateCompleted + } It 'Should restart SQL Service and running SQL Agent service' { { Restart-SqlService -SQLServer $env:ComputerName -SQLInstanceName 'MSSQLSERVER' } | Should -Not -Throw @@ -201,6 +247,7 @@ InModuleScope $script:moduleName { Assert-MockCalled -CommandName Get-Service -Scope It -Exactly -Times 1 Assert-MockCalled -CommandName Restart-Service -Scope It -Exactly -Times 1 Assert-MockCalled -CommandName Start-Service -Scope It -Exactly -Times 1 + Assert-MockCalled -CommandName Start-Job -Scope It -Exactly -Times 1 } It 'Should restart SQL Service and not try to restart missing SQL Agent service' { @@ -210,6 +257,7 @@ InModuleScope $script:moduleName { Assert-MockCalled -CommandName Get-Service -Scope It -Exactly -Times 1 Assert-MockCalled -CommandName Restart-Service -Scope It -Exactly -Times 1 Assert-MockCalled -CommandName Start-Service -Scope It -Exactly -Times 0 + Assert-MockCalled -CommandName Start-Job -Scope It -Exactly -Times 1 } It 'Should restart SQL Service and not try to restart stopped SQL Agent service' { @@ -219,62 +267,81 @@ InModuleScope $script:moduleName { Assert-MockCalled -CommandName Get-Service -Scope It -Exactly -Times 1 Assert-MockCalled -CommandName Restart-Service -Scope It -Exactly -Times 1 Assert-MockCalled -CommandName Start-Service -Scope It -Exactly -Times 0 + Assert-MockCalled -CommandName Start-Job -Scope It -Exactly -Times 1 } - } - Context 'Restart-SqlService clustered instance' { - Mock -CommandName Connect-SQL -MockWith { - return @{ - Name = 'MSSQLSERVER' - InstanceName = '' - ServiceName = 'MSSQLSERVER' - IsClustered = $true + Context 'When it fails to connect to the instance within the timeout period' { + BeforeEach { + $mockDynamicJobStateInfoState = $mockJobStateInfoStateFailed } - } -Verifiable -ParameterFilter { ($SQLServer -eq 'CLU01') -and ($SQLInstanceName -eq 'MSSQLSERVER') } - Mock -CommandName Connect-SQL -MockWith { - return @{ - Name = 'NAMEDINSTANCE' - InstanceName = 'NAMEDINSTANCE' - ServiceName = 'NAMEDINSTANCE' - IsClustered = $true - } - } -Verifiable -ParameterFilter { ($SQLServer -eq 'CLU01') -and ($SQLInstanceName -eq 'NAMEDINSTANCE') } + It 'Should throw the correct error message' { + $errorMessage = $localizedData.FailedToConnectToInstanceTimeout -f $env:ComputerName, 'MSSQLSERVER', 1 - Mock -CommandName Connect-SQL -MockWith { - return @{ - Name = 'STOPPEDAGENT' - InstanceName = 'STOPPEDAGENT' - ServiceName = 'STOPPEDAGENT' - IsClustered = $true + { + Restart-SqlService -SQLServer $env:ComputerName -SQLInstanceName 'MSSQLSERVER' -Timeout 1 + } | Should -Throw $errorMessage } - } -Verifiable -ParameterFilter { ($SQLServer -eq 'CLU01') -and ($SQLInstanceName -eq 'STOPPEDAGENT') } + } + } - Mock -CommandName Get-CimInstance -MockWith { - @('MSSQLSERVER','NAMEDINSTANCE','STOPPEDAGENT') | ForEach-Object { - $mock = New-Object Microsoft.Management.Infrastructure.CimInstance 'MSCluster_Resource','root/MSCluster' - $mock | Add-Member -MemberType NoteProperty -Name 'Name' -Value "SQL Server ($($_))" -TypeName 'String' - $mock | Add-Member -MemberType NoteProperty -Name 'Type' -Value 'SQL Server' -TypeName 'String' - $mock | Add-Member -MemberType NoteProperty -Name 'PrivateProperties' -Value @{ InstanceName = $_ } + Context 'Restart-SqlService clustered instance' { + BeforeAll { + $mockDynamicJobStateInfoState = $mockJobStateInfoStateCompleted - return $mock - } - } -Verifiable -ParameterFilter { ($ClassName -eq 'MSCluster_Resource') -and ($Filter -eq "Type = 'SQL Server'") } + Mock -CommandName Connect-SQL -MockWith { + return @{ + Name = 'MSSQLSERVER' + InstanceName = '' + ServiceName = 'MSSQLSERVER' + IsClustered = $true + } + } -Verifiable -ParameterFilter { ($SQLServer -eq 'CLU01') -and ($SQLInstanceName -eq 'MSSQLSERVER') } - Mock -CommandName Get-CimAssociatedInstance -MockWith { - $mock = New-Object Microsoft.Management.Infrastructure.CimInstance 'MSCluster_Resource','root/MSCluster' + Mock -CommandName Connect-SQL -MockWith { + return @{ + Name = 'NAMEDINSTANCE' + InstanceName = 'NAMEDINSTANCE' + ServiceName = 'NAMEDINSTANCE' + IsClustered = $true + } + } -Verifiable -ParameterFilter { ($SQLServer -eq 'CLU01') -and ($SQLInstanceName -eq 'NAMEDINSTANCE') } - $mock | Add-Member -MemberType NoteProperty -Name 'Name' -Value "SQL Server Agent ($($InputObject.PrivateProperties.InstanceName))" -TypeName 'String' - $mock | Add-Member -MemberType NoteProperty -Name 'Type' -Value 'SQL Server Agent' -TypeName 'String' - $mock | Add-Member -MemberType NoteProperty -Name 'State' -Value (@{ $true = 3; $false = 2 }[($InputObject.PrivateProperties.InstanceName -eq 'STOPPEDAGENT')]) -TypeName 'Int32' + Mock -CommandName Connect-SQL -MockWith { + return @{ + Name = 'STOPPEDAGENT' + InstanceName = 'STOPPEDAGENT' + ServiceName = 'STOPPEDAGENT' + IsClustered = $true + } + } -Verifiable -ParameterFilter { ($SQLServer -eq 'CLU01') -and ($SQLInstanceName -eq 'STOPPEDAGENT') } - return $mock - } -Verifiable -ParameterFilter { $ResultClassName -eq 'MSCluster_Resource' } + Mock -CommandName Get-CimInstance -MockWith { + @('MSSQLSERVER','NAMEDINSTANCE','STOPPEDAGENT') | ForEach-Object { + $mock = New-Object Microsoft.Management.Infrastructure.CimInstance 'MSCluster_Resource','root/MSCluster' - Mock -CommandName Invoke-CimMethod -Verifiable -ParameterFilter { $MethodName -eq 'TakeOffline' } + $mock | Add-Member -MemberType NoteProperty -Name 'Name' -Value "SQL Server ($($_))" -TypeName 'String' + $mock | Add-Member -MemberType NoteProperty -Name 'Type' -Value 'SQL Server' -TypeName 'String' + $mock | Add-Member -MemberType NoteProperty -Name 'PrivateProperties' -Value @{ InstanceName = $_ } - Mock -CommandName Invoke-CimMethod -Verifiable -ParameterFilter { $MethodName -eq 'BringOnline' } + return $mock + } + } -Verifiable -ParameterFilter { ($ClassName -eq 'MSCluster_Resource') -and ($Filter -eq "Type = 'SQL Server'") } + + Mock -CommandName Get-CimAssociatedInstance -MockWith { + $mock = New-Object Microsoft.Management.Infrastructure.CimInstance 'MSCluster_Resource','root/MSCluster' + + $mock | Add-Member -MemberType NoteProperty -Name 'Name' -Value "SQL Server Agent ($($InputObject.PrivateProperties.InstanceName))" -TypeName 'String' + $mock | Add-Member -MemberType NoteProperty -Name 'Type' -Value 'SQL Server Agent' -TypeName 'String' + $mock | Add-Member -MemberType NoteProperty -Name 'State' -Value (@{ $true = 3; $false = 2 }[($InputObject.PrivateProperties.InstanceName -eq 'STOPPEDAGENT')]) -TypeName 'Int32' + + return $mock + } -Verifiable -ParameterFilter { $ResultClassName -eq 'MSCluster_Resource' } + + Mock -CommandName Invoke-CimMethod -ParameterFilter { $MethodName -eq 'TakeOffline' } -Verifiable + Mock -CommandName Invoke-CimMethod -ParameterFilter { $MethodName -eq 'BringOnline' } -Verifiable + } It 'Should restart SQL Server and SQL Agent resources for a clustered default instance' { { Restart-SqlService -SQLServer 'CLU01' } | Should -Not -Throw @@ -284,6 +351,7 @@ InModuleScope $script:moduleName { Assert-MockCalled -CommandName Get-CimAssociatedInstance -Scope It -Exactly -Times 1 Assert-MockCalled -CommandName Invoke-CimMethod -ParameterFilter { $MethodName -eq 'TakeOffline' } -Scope It -Exactly -Times 1 Assert-MockCalled -CommandName Invoke-CimMethod -ParameterFilter { $MethodName -eq 'BringOnline' } -Scope It -Exactly -Times 2 + Assert-MockCalled -CommandName Start-Job -Scope It -Exactly -Times 1 } It 'Should restart SQL Server and SQL Agent resources for a clustered named instance' { @@ -294,6 +362,7 @@ InModuleScope $script:moduleName { Assert-MockCalled -CommandName Get-CimAssociatedInstance -Scope It -Exactly -Times 1 Assert-MockCalled -CommandName Invoke-CimMethod -ParameterFilter { $MethodName -eq 'TakeOffline' } -Scope It -Exactly -Times 1 Assert-MockCalled -CommandName Invoke-CimMethod -ParameterFilter { $MethodName -eq 'BringOnline' } -Scope It -Exactly -Times 2 + Assert-MockCalled -CommandName Start-Job -Scope It -Exactly -Times 1 } It 'Should not try to restart a SQL Agent resource that is not online' { @@ -304,6 +373,7 @@ InModuleScope $script:moduleName { Assert-MockCalled -CommandName Get-CimAssociatedInstance -Scope It -Exactly -Times 1 Assert-MockCalled -CommandName Invoke-CimMethod -ParameterFilter { $MethodName -eq 'TakeOffline' } -Scope It -Exactly -Times 1 Assert-MockCalled -CommandName Invoke-CimMethod -ParameterFilter { $MethodName -eq 'BringOnline' } -Scope It -Exactly -Times 1 + Assert-MockCalled -CommandName Start-Job -Scope It -Exactly -Times 1 } } } diff --git a/en-US/SqlServerDscHelper.strings.psd1 b/en-US/SqlServerDscHelper.strings.psd1 index c24df571e..d10e85953 100644 --- a/en-US/SqlServerDscHelper.strings.psd1 +++ b/en-US/SqlServerDscHelper.strings.psd1 @@ -34,6 +34,8 @@ ConvertFrom-StringData @' GetServiceInformation = Getting {0} service information. RestartService = {0} service restarting. StartingDependentService = Starting service {0} + WaitingInstanceTimeout = Waiting for instance {0}\\{1} to report status online, with a timeout value of {2} seconds. + FailedToConnectToInstanceTimeout = Failed to connect to the instance {0}\\{1} within the timeout period of {2} seconds. ExecuteQueryWithResultsFailed = Executing query with results failed on database '{0}'. ExecuteNonQueryFailed = Executing non-query failed on database '{0}'. AlterAvailabilityGroupReplicaFailed = Failed to alter the availability group replica '{0}'.