HiveBrain v1.2.0
Get Started
← Back to all entries
patternshellMinor

Babysitting a server reboot

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
babysittingrebootserver

Problem

I'm looking to improve a loop within a script that performs scheduled server restarts.

Currently, I'm using a DO While loop with an exit condition within an IF statement once a counter reaches a limit.

I'm looking to change this into a more simpler loop using a Labeled While loop containing a For loop that breaks when the nested If loop catches the counter variable reaching a limit. I've read it's best to avoid using Break but I just cannot come up with anything.

#current loop
$max = 60;$i = 0
 DO
 {
  IF($i -gt $max){
  [PsCustomObject]@{
  "Server" =  $server
  "Status" = "FailedToReboot!"
  "LastRebootTime" = "$LastReboot"
  "CurrentRebootTime" = "FailedToReboot!"
   }#end custom object

Send-MailMessage 

exit}#exit script and log failed to reboot.

$i++

Start-Sleep -Seconds 10

}#end DO

While (Test-path "\\$server\c$")#After this point, server went offline, continue script.

#new test loop    
:WaitingForReboot while (test-path \\$server\C$){
 for($i = 0; $i -le 7){    
 $i++
 "Waiting for $server to reboot, waited " + $(10 * $i) + " of 50 seconds"

  IF($i -eq 5){"Exit wait loop";break WaitingForReboot}Else{
    #do nothing in Else, until $i hits 5.
    }#else block
Start-Sleep -Seconds 10        
 }#for loop
}#while loop

Solution

As you have read it is usually preferred to exit a loop via its condition(s) as supposed to force exiting via commands like break, exit and continue. Those commands do have their place but your loop and logic could do without them. You are only executing the code inside the if once. Why bother keeping it inside the loop? Lets update the while condition and move the if clause outside.

$maximumAttempts = 60
$attemptIndex = 0

do {
    # Wait between checks. 
    $attemptIndex++
    Start-Sleep -Seconds 10
} While (Test-path "\\$server\c$" -and $attemptIndex -le $maximumAttempts)

# Check if the loop ran for its maximumAttempts
if($attemptIndex -ge $maximumAttempts){

  [PsCustomObject]@{
      "Server" =  $server
      "Status" = "FailedToReboot!"
      "LastRebootTime" = "$LastReboot"
      "CurrentRebootTime" = "FailedToReboot!"
   }

   Send-MailMessage 
}


You will also see that I have used more meaningful variable names. I have also used standardized indentation. Doing that removes the need for comments like }#while loop. The indentation makes it clear where the brackets start and stop and make reading the code easier.

Code Snippets

$maximumAttempts = 60
$attemptIndex = 0

do {
    # Wait between checks. 
    $attemptIndex++
    Start-Sleep -Seconds 10
} While (Test-path "\\$server\c$" -and $attemptIndex -le $maximumAttempts)

# Check if the loop ran for its maximumAttempts
if($attemptIndex -ge $maximumAttempts){

  [PsCustomObject]@{
      "Server" =  $server
      "Status" = "FailedToReboot!"
      "LastRebootTime" = "$LastReboot"
      "CurrentRebootTime" = "FailedToReboot!"
   }

   Send-MailMessage <Details>
}

Context

StackExchange Code Review Q#133711, answer score: 2

Revisions (0)

No revisions yet.