SqlSetup: Added support for RSINSTALLMODE parameter

[bozho]

Pull Request (PR) description

Added RSInstallMode parameter, to support RSINSTALLMODE SQL Setup config option.

This Pull Request (PR) fixes the following issues

• Fixes a part of SqlSetup: Missing Configuration Feature/Options : Report Service #1163

Task list

• Added an entry under the Unreleased section of the change log in the CHANGELOG.md.
  Entry should say what was changed, and how that affects users (if applicable).
• Resource documentation added/updated in README.md.
• Resource parameter descriptions added/updated in README.md, schema.mof
  and comment-based help.
• Comment-based help added/updated.
• Localization strings added/updated in all localization files as appropriate.
• Examples appropriately added/updated.
• Unit tests added/updated. See DSC Resource Testing Guidelines.
• Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
• New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

---

This change is 

[johlju]

Reviewed 2 of 4 files at r1.
Reviewable status: 2 of 4 files reviewed, 2 unresolved discussions (waiting on @bozho)

---

DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1, line 528 at r1 (raw file):

    }

    return @{

We need to return the RSInstallMode property here. Can we detect the install mode from registry or by some other means?

---

DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1, line 1457 at r1 (raw file):

$setupArguments += @{ RSINSTALLMODE = $RSInstallMode}

We should test this line in a unit test.

[johlju]

Reviewed 2 of 4 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @bozho)

[bozho]

@johlju Apologies, I didn't get a notification about your reply... I'll get on this

[codecov-io]

Codecov Report

Merging #1361 into dev will increase coverage by <1%.
The diff coverage is 100%.

@@          Coverage Diff          @@
##            dev   #1361    +/-   ##
=====================================
+ Coverage    98%     98%   +<1%     
=====================================
  Files        36      36            
  Lines      5335    5338     +3     
=====================================
+ Hits       5229    5232     +3     
  Misses      106     106

[bozho]

Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @bozho and @johlju)

---

DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1, line 528 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

We need to return the RSInstallMode property here. Can we detect the install mode from registry or by some other means?

I haven't found a way. This parameter is just telling the setup whether to initialise SSRS with defaults on install. I don't think it leaves a trace anywhere - we may be able to parse the setup log, but that seems brittle.

---

DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1, line 1457 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

$setupArguments += @{ RSINSTALLMODE = $RSInstallMode}

We should test this line in a unit test.

Will do

[bozho]

Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @bozho and @johlju)

---

DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1, line 528 at r1 (raw file):

Previously, bozho (Marko Bozikovic) wrote…

I haven't found a way. This parameter is just telling the setup whether to initialise SSRS with defaults on install. I don't think it leaves a trace anywhere - we may be able to parse the setup log, but that seems brittle.

I've done a bit of digging and the only thing I can find is HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Microsoft SQL Server\MSRS13.MSSQLSERVER\Setup Registry key, with RSConfiguration value, but that one seems to store current SSRS configuration state.

---

DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1, line 1457 at r1 (raw file):

Previously, bozho (Marko Bozikovic) wrote…

Will do

Hm, since we can't seem to get this value from Get-TargetResource, what would we be unit testing here?

[johlju]

Reviewed 4 of 4 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @bozho)

---

DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1, line 528 at r1 (raw file):

Previously, bozho (Marko Bozikovic) wrote…

I've done a bit of digging and the only thing I can find is HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Microsoft SQL Server\MSRS13.MSSQLSERVER\Setup Registry key, with RSConfiguration value, but that one seems to store current SSRS configuration state.

In that case, let us just add the RSInstallMode parameter to the Get-TargetResource so you can return the $RSInstallMode parameter in this hashtable.

return @{
    ...
    RSInstallMode = $RSInstallMode
}

---

DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1, line 1457 at r1 (raw file):

Previously, bozho (Marko Bozikovic) wrote…

Hm, since we can't seem to get this value from Get-TargetResource, what would we be unit testing here?

We have a test that verifies that the setup.exe is called with this argument. I think we could add the parameter and argument to these hashtables
https://github.com/PowerShell/SqlServerDsc/blob/e8298d59934eeaa68ce373afc683d2aefa579136/Tests/Unit/MSFT_SqlSetup.Tests.ps1#L2863-L2920

The mock for Start-SqlSetupProcess check the actual arguments against the expected arguments (using what I call a "dynamic mock").
https://github.com/PowerShell/SqlServerDsc/blob/e8298d59934eeaa68ce373afc683d2aefa579136/Tests/Unit/MSFT_SqlSetup.Tests.ps1#L915-L919

[bozho]

Reviewable status: 3 of 5 files reviewed, 2 unresolved discussions (waiting on @johlju)

---

DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1, line 528 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

In that case, let us just add the RSInstallMode parameter to the Get-TargetResource so you can return the $RSInstallMode parameter in this hashtable.

return @{
    ...
    RSInstallMode = $RSInstallMode
}

Done.

---

DSCResources/MSFT_SqlSetup/MSFT_SqlSetup.psm1, line 1457 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

We have a test that verifies that the setup.exe is called with this argument. I think we could add the parameter and argument to these hashtables
https://github.com/PowerShell/SqlServerDsc/blob/e8298d59934eeaa68ce373afc683d2aefa579136/Tests/Unit/MSFT_SqlSetup.Tests.ps1#L2863-L2920

The mock for Start-SqlSetupProcess check the actual arguments against the expected arguments (using what I call a "dynamic mock").
https://github.com/PowerShell/SqlServerDsc/blob/e8298d59934eeaa68ce373afc683d2aefa579136/Tests/Unit/MSFT_SqlSetup.Tests.ps1#L915-L919

Done.

[johlju]

Looks good. Waiting for the tests to pass.

Reviewed 2 of 2 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved

[johlju]

Reviewable status: complete! all files reviewed, all discussions resolved

[johlju]

@bozho Thank you for this! Awesome to get this one over the finish line! Great work!