by themoblin at 2013-03-16 16:56:42
Made an edit as a realized that the while loop wasn’t doing as I expected, a for loop seems to suit this much better. This is running as a scheduled task on a couple of my servers and seems to work.by coderaven at 2013-03-20 12:13:32
Can anyone see anything that needs improving?
#Email Alert Parameters
$to = "user@mydomain.com,"
$from = "unreachable@mydomain.com"
$smtpserver = "my_Exchange"
#Array of computers to test
$Computers = ("comp1" , "comp2" , "comp3" , "comp4")
#Variable to hold INT value 0
$zero = 0
Foreach ($Computer in $Computers)
{
if
(
#Checks for a file with the host computers name in the Reports folder and if it doesn’t exist creates it with content 0
Test-Path $("C:\Reports" + $Computer + ".txt")
)
{
}
else
{
$zero > $("C:\Reports" + $Computer + ".txt")
}
#Reads the content of the file and saves to variable as text
$FailedPings = Get-Content $("C:\Reports" + $Computer + ".txt")
#Converts the value to INT
$INT_FailedPings = [INT]$FailedPings
#Actually runs the ping test
$PingTest = Test-Connection -ComputerName $Computer -count 1
if
(
#If ping is unsuccessful
$PingTest.StatusCode -ne "0"
)
{
if
(
#If previous failed pings value is less or equal to 3
$INT_FailedPings -le 3
)
{
#Increment the value by 1
$INT_FailedPings++
#Write the value out to the reports folder file for this host
$INT_FailedPings > $("C:\Reports" + $Computer + ".txt")
#Send an alert of failed ping
Send-MailMessage -to $to -subject "Warning, Host $Computer is down. You will only receive 4 of these messages!" -from $from -body "Ping to $Computer failed, you will only receive 3 of these messages!" -smtpserver $smtpserver
}
}
elseif
(
#If previous checks have failed the value will be non zero, as checks are now working sets the value back to zero and alerts that host is back up
$INT_FailedPings -ne 0
)
{
$zero > $("C:\Reports" + $Computer + ".txt")
Send-MailMessage -to $to -subject "Host $Computer is back up" -from $from -body "Panic over" -smtpserver $smtpserver
}
else
#If ping is successful and past pings were successful do nothing
{
}
}
If it is working and doing exactly what you want, it usually means you are doing ok. As you learn, you may find ways to improve it but is hard for someone from the outside to completely understand you processes.by MasterOfTheHat at 2013-03-20 12:51:13
My suggestions would be:
1. Break up the different steps, processes or checks into Functions so that you can change one part with out messing with the other ( and you learn functions)
2. Look at the PSObject as a way to store status information and then import/export a single set of results at the beginning and end.
3. Just have fun with it, as you modify test it well, and make backups.
You’ll definitely learn more as you go and will find ways to streamline your code! Just yesterday, I was trying to help someone get started on a script, so I grabbed one I wrote about a year ago that did pretty much what they wanted. When I actually looked at it, I had one of those "what were you THINKING?!" moments and decided to rewrite it. The original was a 25 line with multiple variables being passed around, some .NET constructs were being used, and some logic that was just completely unnecessary. When I got finished, I had a 10 line script where 5 lines are actually one pipeline, no variables at all, and very simple/basic logic. It’s amazing how much you’ll learn in a year! Which is why I never look at my old code… lol!by DexterPOSH at 2013-03-20 16:29:03
[quote="MasterOfTheHat"]It’s amazing how much you’ll learn in a year! Which is why I never look at my old code… lol![/quote]
Lol
