-
Notifications
You must be signed in to change notification settings - Fork 107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SPSearchServiceApp: Added logic and parameter to provision default search topology #1401
base: master
Are you sure you want to change the base?
Conversation
ProvisionDefaultTopology when no server able to host Search Service found
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Just a few comments.
Also, I do not see an updated unit test for this resource, which means the updated code is not tested. Please also update the unit test (tests/Unit/SharePointDsc/SharePointDsc.SPSearchServiceApp.Tests.ps1), so your code is tested.
Reviewed 2 of 4 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: 3 of 4 files reviewed, 3 unresolved discussions (waiting on @petepuu and @ykuijs)
SharePointDsc/DSCResources/MSFT_SPSearchServiceApp/MSFT_SPSearchServiceApp.psm1, line 483 at r2 (raw file):
if ($params.ContainsKey("ProvisionDefaultTopology") -eq $true) { if ($params.ProvisionDefaultTopology -eq $true)
You can combine these two lines into a single statement:
if (($params.ContainsKey("ProvisionDefaultTopology") -eq $true) -and ($params.ProvisionDefaultTopology -eq $true))
{
.....
}
If the first condition fails, it does not evaluate the second part. That way it will never throw an error.
Code quote:
if ($params.ContainsKey("ProvisionDefaultTopology") -eq $true)
{
if ($params.ProvisionDefaultTopology -eq $true)
SharePointDsc/DSCResources/MSFT_SPSearchServiceApp/MSFT_SPSearchServiceApp.psm1, line 509 at r2 (raw file):
if ($searchServers.Count -eq 0) { Write-Verbose -Message "Provisioning default search topology"
Why is this Verbose logging specified here? The message doesn't make much sense. I would update to something like "No servers found with Search or ApplicationWithSearch roles, checking Custom role"
SharePointDsc/DSCResources/MSFT_SPSearchServiceApp/MSFT_SPSearchServiceApp.psm1, line 515 at r2 (raw file):
if ($searchServers.Count -eq 0) { $searchServers = $possibleSearchServers | Where-Object { $_.Role -in ("SingleServerFarm", "SingleServer") }
For troubleshooting purposes: Add Verbose message like "No servers found with Custom role. Server role must be SingleServer or SingleServerFarm"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 3 of 4 files reviewed, 5 unresolved discussions (waiting on @petepuu and @ykuijs)
SharePointDsc/DSCResources/MSFT_SPSearchServiceApp/MSFT_SPSearchServiceApp.psm1, line 362 at r2 (raw file):
[Parameter()] [System.Boolean] $ProvisionDefaultTopology
We should set the default value of this parameter to False, so:
$ProvisionDefaultTopology = $false
Do this for the Get/Test/Set methods
SharePointDsc/DSCResources/MSFT_SPSearchServiceApp/MSFT_SPSearchServiceApp.psm1, line 366 at r2 (raw file):
Write-Verbose -Message "Setting Search service application '$Name'"
To make sure the Default value is also added to the PSBounParameters collection, add this line here:
$PSBoundParameters.ProvisionDefaultTopology = $ProvisionDefaultTopology
Do this for the Get/Test/Set methods
Pull Request (PR) description
Added optional ProvisionDefaultTopology (boolean) parameter to control should the SPSearchServiceApp provision deafult search topology. Use 'ProvisionDefaultTopology = $true' parameter to provision default search topology. When this parameter is defined and value is TRUE then topology is created as below:
First we check are there any servers having Search or ApplicationWithSearch role. If there are then all search components are
provisioned to all these servers
If no Search or ApplicationWithSearch role servers exist then we check are there servers having Custom role. If yes then all
search server components are provisioned to one (1) server having Custom role
If no servers exist having roles defined in 1 and 2 then we check is this SingleServer or SingleServerFarm deployment and if yes the all search server components are provisioned to that single server
If you do not want to provision default topology then you need to define search topology using the SPSearchTopology resource!
This Pull Request (PR) fixes the following issues
None
Task list
Entry should say what was changed and how that affects users (if applicable), and
reference the issue being resolved (if applicable).
help.
This change is