Powershell script to stop and start services on demand

Question

The purpose of below code is to stop/start a couple of windows services if they are running/not running, this will be called from a windows command line. The services could be either on a windows stand alone server or could be a bundled into a Windows Failover cluster Role on a clustered server.

I believe I handled all the cases and validating the inputs and using functions where ever possible. However I'm sure there is room for improvement and to make it more robust and reliable, please critique or comment on it.

this will be run or evoked by humans thus the need to make it robust ..

$mode = 'run'
$Servername = $env:computername
$Environment = $Servername.substring(9)
$DateTime = Get-Date -Uformat "%Y%m%d";
$LogfilePath = 'c:\Logs\MySoftware_' + $action + '_' + $Environment + '_' + $DateTime + '.txt'
Start-Transcript -path $LogfilePath -append

function Getvariables {
    Write-Host   'action is'                            $action 
    Write-Host   'action.Length is'                     $action.Length 
    Write-Host   'IsComputerACluster is'                $IsComputerACluster
    Write-Host   'IsComputerACluster.Length is'         $IsComputerACluster.Length
    Write-Host   'Servername is'                        $Servername 
    Write-Host   'ServiceName1 is'                      $ServiceName1 
    Write-Host   'ServiceName2 is'                      $ServiceName2 
    Write-Host   'ClusterGroup is'                      $ClusterGroup 
}
function IsCluster { 
    param([string]$serverName) 

    $ErrorActionPreference = "SilentlyContinue" 

    trap [Exception] {  
        return $false 
    } 

    if ($null -ne (Get-WMIObject -Class MSCluster_ResourceGroup -ComputerName $Servername -Namespace root\mscluster) ) { 
        return $true
    }
    else {
        return $false
    } 
}

function Get-ServiceStatus {
    Param ($ServiceName)
    $arrService = (Get-Service -Name $ServiceName).Status
    #Write-Host $ServiceName is $arrService.Status
    return $arrService
}

function Get-ClusterGroupStatus {
    Param ($ClusterGroup)
    [string ]$arrService = (Get-ClusterGroup -Name $ClusterGroup).State 
    #Write-Host $ServiceName is $arrService.Status
    return $arrService
}

$IsComputerACluster = (IsCluster $Servername)
if ($mode -eq 'debug') { Getvariables }
if (($action -in ('start' , 'stop')) -and ($IsComputerACluster -eq $false )) {
    #Start MySoftware Services


    $ServiceName1 = 'MySoftwareServer'
    $ServiceName2 = 'MySoftwareUIConsole'
    [string]$ServiceStatus1 = ''
    [string]$ServiceStatus2 = ''

    $ServiceStatus1 = (Get-ServiceStatus $ServiceName1)
    $ServiceStatus2 = (Get-ServiceStatus $ServiceName2)
    if ($mode -eq 'debug') { Getvariables }
    if ($action -eq 'stop') {

        if ($ServiceStatus1 -ne 'Stopped' -Or $ServiceStatus2 -ne 'Stopped') {   
            Write-Host $ServiceName1 is now (Get-ServiceStatus $ServiceName1)
            Write-Host $ServiceName2 is now (Get-ServiceStatus $ServiceName2)
            Write-Host 'Now Attempting to Stop the services '$ServiceName1' & '$ServiceName2
            stop-Service $ServiceName1
            if (Get-ServiceStatus $ServiceName1 -eq 'Stopped') {
                stop-Service $ServiceName2
            }
            else { Write-Host 'Waiting for '+$ServiceName1+' to stop..' }

            Write-Host $ServiceName1 is now (Get-ServiceStatus $ServiceName1)
            Write-Host $ServiceName2 is now (Get-ServiceStatus $ServiceName2)

        }
        else
        { Write-Host 'Services are already stopped' }
    }
    else {
        if ($ServiceStatus1 -ne 'Running' -Or $ServiceStatus2 -ne 'Running') {   
            Write-Host $ServiceName1 is now (Get-ServiceStatus $ServiceName1)
            Write-Host $ServiceName2 is now (Get-ServiceStatus $ServiceName2)
            Write-Host 'Now Attempting to Start the services '$ServiceName1' & '$ServiceName2
            start-Service $ServiceName1
            if (Get-ServiceStatus $ServiceName1 -eq 'Running') {
                start-Service $ServiceName2
            }
            else { Write-Host 'Waiting for '+$ServiceName1+' to start..' }

            Write-Host $ServiceName1 is now (Get-ServiceStatus $ServiceName1)
            Write-Host $ServiceName2 is now (Get-ServiceStatus $ServiceName2)

        }
        else
        { Write-Host 'Services are already running' }
    }

}

elseif (($action -in ('start' , 'stop')) -and ($IsComputerACluster -eq 'True' )) {
    $ClusterGroup = 'MySoftware'


    $ClusterGroupStatus = Get-ClusterGroupStatus $ClusterGroup
    if ($mode -eq 'debug') { Getvariables }

    if ($action -eq 'stop') {

        if ($ClusterGroupStatus -eq 'Online' ) {   
            Write-Host $ClusterGroup is now $ClusterGroupStatus

            Write-Host 'Now Attempting to Stop the cluster group ' $ClusterGroup
            Stop-ClusterGroup $ClusterGroup
            if (Get-ClusterGroupStatus $ClusterGroup -eq 'Offline') {
                Write-Host $ClusterGroup is now Offline
                break
            }
            else { Write-Host 'Waiting for '+$ClusterGroup+' to go offline..' }
            $ClusterGroupStatus = Get-ClusterGroupStatus $ClusterGroup
            Write-Host $ClusterGroup is now $ClusterGroupStatus

        }
        else {
            Write-Host $ClusterGroup 'is already offline' 
        }
    }
    else {
        if ($ClusterGroupStatus -eq 'Offline' ) {   
            Write-Host $ClusterGroup is now $ClusterGroupStatus

            Write-Host 'Now Attempting to Start the cluster group ' $ClusterGroup
            Start-ClusterGroup $ClusterGroup
            if (Get-ClusterGroupStatus $ClusterGroup -eq 'Online') {
                Write-Host $ClusterGroup is now Online
                break
            }
            else { Write-Host 'Waiting for '+$ClusterGroup+' to become Online..' }

            $ClusterGroupStatus = Get-ClusterGroupStatus $ClusterGroup
            Write-Host $ClusterGroup is now $ClusterGroupStatus

        }
        else
        { Write-Host $ClusterGroup 'is already Online' }
    }



}
else { 
    if ($mode -eq 'debug') { Getvariables }


    Write-Host 'Invalid paramater value for variable: action' 
    Write-Host 'Valid ways to call the script are: 
                powershell.exe -file FullyQualifiedPowershellFilePath.ps1 -action "stop" 
                powershell.exe -file FullyQualifiedPowershellFilePath.ps1 -action "start"'
}


Write-Host 'Scritp Execution Completed'




Stop-Transcript```

Answer 1

Here are my thoughts, in no particular order of importance:

1. Use parameter validation and script parameters, they are stupidly convenient in powershell, for example, put this at the very top of the script:

   param(
       [Parameter]
       [ValidateSet('Start','Stop')} # Can have more than two possible values of course
       [string] $mode
   )
   

   The error message from ValidateSet can be a bit cryptic, but you can also use ValidateScript to print your own exceptions and error messages. Look up ValidateRegex as well, you'll use it someday.

2. You could make shorter function declarations with function myfunc([type]$arg) { ... instead of using two more verbose lines for that. Don't overdo it if you have lots of parameters, but it's perfectly acceptable for short functions with few arguments.

3. Line 103, $IsComputerACluster -eq 'True': you can trim off everything and just keep $IsComputerACluster. It is already $true or $false from your IsCluster function. Generally, avoid checking if something is equal to the string 'true' or 'false'*. In if blocks, just use if($MyVar) or if(-not $MyVar).

4. Lines 55-58, I don't really get what you are going for, you're assigning a value to a variable and then overwriting it with an empty string ?

5. That's a personal choice, but I'd replace every occurence of Get-ServiceStatus $name by Get-Service $name | select -exp Status. But your function name is pretty explicit and that's just my love of one-liners speaking. Same for Get-ClusterGroupStatus which has a less explicit name.

6. Line 51, $var -eq $false can be replaced by -not $var or even !$var.

7. Line 135, if you don't include -Wait in your call to Start-ClusterGroup, the cmdlet will wait until the operation is over. Two cases where this could go wrong:

   • Your cluster gets stuck in the startup process and the cmdlet never returns
   • Your cluster fails to start and the else block executes, telling your user the cluster is waiting to become online, despite it having failed.

   So I'd either include a -Wait with a reasonably long time (after which you assume that the cluster is stuck), or include a -wait 0 so that the cmdlet returns immediately, then check the cluster status with Get-ClusterGroupStatus until you reach a timeout or until the cluster comes online. Here's a template:

   Start-LongOperation -NoWait -Wait 0 -WhateverYourCmdletUsesToRunAsynchronously
   
   While(-not Check-LongOperationCompleted -and $CheckRetries -lt 20) { 
       Start-Sleep -Seconds 5 
       $CheckRetries += 1
   }
   
   $success = ($CheckRetries -lt 20) # If this is true, it's because the operation completed before the timeout was reached
   

   You can make countless variations of this, with exceptions, with actual timers using Get-Date, etc., which are left as an exercise to the reader. Also look into powershell jobs if you want to make asynchronous stuff.

8. Change the log directory to something like $env:temp to avoid scaring the user for whom c:\Logs will not exist ? You could make a parameter with a default value from this.

9. In IsCluster, there is a if(condition){return $true}else{return $false} block. You can shorten it into return (condition) 95% of the time. This is true for other languages as well.

10. Make this a module for easier deployment ?

That's all I can come up with. I'd say your script is pretty good, it's verbose but I found that most user-facing powershell scripts are super verbose when you just try to make a wrapper. If you don't have too many potential users, consider just explaining them the basics of Get/Start/Stop-Service and Get/Start/Stop-ClusterGroup ? I'd definitely make IsCluster a module or put it in my powershell profile though.

*For example, 'false' -eq $false is True (pretty reasonable), but $false -eq 'false' returns False (counter-intuitive).