SqlRS: Fails when SQL Instance and RS Instance Are Different #1868
Labels
bug
The issue is a bug.
good first issue
The issue should be easier to fix and can be taken up by a beginner to learn to contribute on GitHub
Comments
|
This is strange because the integration tests are using different instances too. SqlServerDsc/tests/Integration/DSC_SqlRS.config.ps1 Lines 168 to 185 in 54b66c6 |
johlju
added
the
needs investigation
|
It must fail on this row in Test-methid because the value returned for the current state is Though, not sure why older version works because this hasn't change in the time the resource has been named SqlRS. Can you confirm it fails on that line? |
johlju
added
needs more information
|
@johlju how could I confirm that? |
|
The easiest I would do is to add a |
|
Thanks for clarification! I set this content https://gist.github.com/shurick81/b99631fef89e8da18a6e5de7a7f5c2b2 to Then I set the content of https://gist.github.com/shurick81/2dab56f7dcd3247f43ee5fba59b5f6e9 to |
|
So it returns Looking at your gist of 16.1 it sets it to $null here: https://gist.github.com/shurick81/b99631fef89e8da18a6e5de7a7f5c2b2#file-dsc_sqlrs-psm1-L60-L71 But the bug is that Test-TargetResource must handle This code: SqlServerDsc/source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1 Lines 956 to 976 in 54b66c6 Should be changed to something like this: if ($PSBoundParameters.ContainsKey('ReportServerReservedUrl'))
{
if ($null -eq $currentConfig.ReportServerReservedUrl)
{
Write-Verbose -Message "Report server reserved URLs on $DatabaseServerName\$DatabaseInstanceName are missing, should be $($ReportServerReservedUrl -join ', ')."
$result = $false
}
else
{
$compareParameters = @{
ReferenceObject = $currentConfig.ReportServerReservedUrl
DifferenceObject = $ReportServerReservedUrl
}
if ($null -ne (Compare-Object @compareParameters))
{
Write-Verbose -Message "Report server reserved URLs on $DatabaseServerName\$DatabaseInstanceName are $($currentConfig.ReportServerReservedUrl -join ', '), should be $($ReportServerReservedUrl -join ', ')."
$result = $false
}
}
}
if ($PSBoundParameters.ContainsKey('ReportsReservedUrl'))
{
if ($null -eq $currentConfig.ReportServerReservedUrl)
{
Write-Verbose -Message "Reports reserved URLs on $DatabaseServerName\$DatabaseInstanceName are missing, should be $($ReportsReservedUrl -join ', ')."
$result = $false
}
else
{
$compareParameters = @{
ReferenceObject = $currentConfig.ReportsReservedUrl
DifferenceObject = $ReportsReservedUrl
}
if ($null -ne (Compare-Object @compareParameters))
{
Write-Verbose -Message "Reports reserved URLs on $DatabaseServerName\$DatabaseInstanceName are $($currentConfig.ReportsReservedUrl -join ', ')), should be $($ReportsReservedUrl -join ', ')."
$result = $false
}
}
} |
johlju
added
bug
|
So what's the next step? I can try the code you suggest with 16.1.0 @johlju |
|
If you can confirm it works with the proposed code it would be great. Next step is to send in a PR to propose the change, and also add a unit test that fails with current implementation and passes with the suggest change. I won't have time to look at it in the near future, so if you (or someone else) have time, please send in a PR. |
|
When I set this contents https://gist.github.com/shurick81/e76065f2748288943bacd0f69d40f77c to |
|
I can come up with a PR and update the |
|
I think you could copy the block here, to a new block just after. SqlServerDsc/tests/Unit/DSC_SqlRS.Tests.ps1 Lines 1137 to 1165 in 54b66c6 Then change the new block to something like this: Context 'When current Report Server virtual directory is $null' {
BeforeAll {
Mock -CommandName Get-TargetResource -MockWith {
return @{
IsInitialized = $true
ReportServerVirtualDirectory = $null
}
}
}
It 'Should return state as not in desired state' {
InModuleScope -ScriptBlock {
Set-StrictMode -Version 1.0
$testParameters = @{
InstanceName = 'INSTANCE'
DatabaseServerName = 'DBSERVER'
DatabaseInstanceName = 'DBINSTANCE'
ReportServerVirtualDirectory = 'ReportServer_SQL2016'
}
$resultTestTargetResource = Test-TargetResource @testParameters
$resultTestTargetResource | Should -BeFalse
}
}
}Then another one that tests those test should fail with the current implementation, and work with the proposed change. Although, not certain as I just made it from the top of my head. |
|
@johlju then I guess you want me to run these tests somehow locally before committing to the PR? |
|
After you clone the repo you run (in the repo folder): ./build.ps1 -ResolveDependency -Tasks noopThe above is just needed once. Then you run the following to build the module: After that you can run the unit test by running: It is also possible running the individual test through the build script (the pipeline). But it is usually bulkier than just doing After you see the tests pass above you add the new test I suggested and run the test again to see that they fail. Then add the code changes, run the build task again, then run the tests a third time to see all tests pass. Hope it works... 🙂 |
|
I guess I need to have some prerequisites installed, like even Pester? Maybe there is some simple way of running tests in a Windows container where I could temporary install Pester and other prerequisites for running tests? |
9 tasks
|
When you run ResolveDependencies as shown above it will download all prerequisite into a ./output/RequiredModules. All that is needed will be inside the local repo folder, and it will not change your machine. The build script setups up the PowerShell session so the pipeline can be run, but closing the session and opening a new then everything is back to normal. So for each session opened, if you want to run tests for example, the minimum requirement is to run |
When we mock it, why do we want |
|
In this case I think it should be $false. It was just an example, so change it as required. |
|
I'm trying to get my head around how we write the tests in the way that they fail when we have issues with the code. Should they even fail with the 16.1.0 codebase? I guess the reason to write the tests is that they fail and indicate something for us in the future :) |
|
added these tests: I get the following output on them, even from main branch: |
Yes, it should fail the the code in the main branch. In this case we could leverage TDD (test-driven development). The test you write should fail with the same error as you saw when you run your configuration. When you then add the code change to handle the $null value the tests should pass. |
|
I will see if I have some time tomorrow to give some more suggestions. |
|
Also, when I run But it also throws errors:
maybe we should somehow make tests like
|
These integration tests, are they running in environment where reporting services are not configured? |
shurick81
added a commit
to shurick81/SqlServerDsc
that referenced
this issue
Mar 11, 2023
|
I see now that the tests I suggest was using wrong properties, so that is why they did not throw an error. Add these tests between the tests at line 1107. Context 'When current Report Server reserved URL is $null' {
BeforeAll {
Mock -CommandName Get-TargetResource -MockWith {
return @{
IsInitialized = $false
ReportServerReservedUrl = $null
}
}
}
It 'Should return state as not in desired state' {
InModuleScope -ScriptBlock {
Set-StrictMode -Version 1.0
$testParameters = @{
InstanceName = 'INSTANCE'
DatabaseServerName = 'DBSERVER'
DatabaseInstanceName = 'DBINSTANCE'
ReportServerReservedUrl = 'ReportServer_SQL2016'
}
$resultTestTargetResource = Test-TargetResource @testParameters
$resultTestTargetResource | Should -BeFalse
}
}
}
Context 'When current Reports reserved URL is $null' {
BeforeAll {
Mock -CommandName Get-TargetResource -MockWith {
return @{
IsInitialized = $false
ReportsReservedUrl = $null
}
}
}
It 'Should return state as not in desired state' {
InModuleScope -ScriptBlock {
Set-StrictMode -Version 1.0
$testParameters = @{
InstanceName = 'INSTANCE'
DatabaseServerName = 'DBSERVER'
DatabaseInstanceName = 'DBINSTANCE'
ReportsReservedUrl = 'Reports_SQL2016'
}
$resultTestTargetResource = Test-TargetResource @testParameters
$resultTestTargetResource | Should -BeFalse
}
}
}They return an exception when run on branch |
|
Also, after applying the suggested change (and running build task) the new tests pass. |
|
Added suggested changes to shurick81#1 since I already had the code written. 🙂 If you merge that (when you approve of the suggestions) your PR will be automatically updated. |
johlju
added
in progress
Yes. The integration tests installs a new RS instance. Either by SqlSetup (SQL2016 and lower) or SqlRSSetup (SQL2017 and higher). Integration tests for SqlRS is then configuring the RS instance installed by the prior integration tests. |
|
ok, so when I was writing tests I confused URL and directory, thanks for finding this out :) |
shurick81
pushed a commit
to shurick81/SqlServerDsc
that referenced
this issue
Mar 12, 2023
|
@johlju I'm using the updated version, but I still seem to be getting the same error. Am I missing something? Here's my config: |
|
@rachaelsingleton Did you use the latest preview version https://www.powershellgallery.com/packages/SqlServerDsc/16.2.0-preview0003? |
|
@johlju I did |
|
Reopening this until I have the integration tests in the PR #1877 working, having issues with them unrelated to this. @rachaelsingleton can you add the following: Write-Verbose -Message ($null -eq $currentConfig.ReportServerReservedUrl | Out-String) -Verbose
Write-Verbose -Message ([System.String]::IsNullOrEmpty($currentConfig.ReportServerReservedUrl) | Out-String) -VerboseBefore this line in the module on your node: Also can you add the following: Write-Verbose -Message ($null -eq $currentConfig.ReportsReservedUrl) | Out-String) -Verbose
Write-Verbose -Message ([System.String]::IsNullOrEmpty($currentConfig.ReportsReservedUrl)) | Out-String) -VerboseBefore this line in the module on your node: The run the configuration again and let me know the verbose output. |
|
@rachaelsingleton did you have an opportunity to debug using the above suggestion? I have the integration tests passing in the PR #1877, so the change should have fixed it for your configuration too. 🤔 Closing this until I get more information. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Problem description
When we try to run SqlRS to configure, we get failure.
The issue might be related to the fact that use separate DBENGINE and RS instances. For example, DBENGINE instance is called "SQLInstance01" and RS instance is called "RSInstance01".
Here are test results for different SqlServerDsc versions:
14.2.1 works
15.2.0 works
16.0.0 fails
16.1.0 fails
So it looks like it stopped working since 16.0.0 release.
Verbose logs
DSC configuration
Suggested solution
I don't have
SQL Server edition and version
SQL Server PowerShell modules
Operating system
PowerShell version
SqlServerDsc version
The text was updated successfully, but these errors were encountered: