locked
How can I send an email when a function returns false? RRS feed

  • Question

  • Im trying to make this code work, but i still receive an email when the function is equal to false. Can anyone assist me in finding the issue? What the program does is look for servers that has a set limit (in GB) on specific drives.

    #This Program goes through all the servers and informs any level below the limit
    $ErrorActionPreference = "SilentlyContinue";
    $scriptpath = $MyInvocation.MyCommand.Definition 
    $dir = Split-Path $scriptpath 
    
    
    #=================Function============
    
    Function isLow($server, $drive, $limit)
    {	
       
    	$disk = Get-WmiObject -ComputerName $server -Class Win32_LogicalDisk -Filter "DriveType = 3 and DeviceID = '$drive'";
    	[float]$size = $disk.Capacity;
    	[float]$freespace = $disk.FreeSpace; 
    	$sizeGB = [Math]::Round($size / 1073741824, 2); 
    	$freeSpaceGB = [Math]::Round($freespace / 1073741824, 2);
    	
    		if($freeSpaceGB -lt $limit){
    			return $true;
    		}
    		else{
    			return $false;
    			}
    }
    
    #================Servers==============
    #------------------###server1--------------
    $server = "###server1";
    $drive = "C:";
    $lim = 25;
    
    if(isLow $server $drive $lim)
    {
    	$Alert += $server + "  ||   " + $drive + "   is low<br>"
    }
    
    $server = "###server1";
    $drive = "D:";
    $lim = 35;
    if(isLow $server $drive $lim)
    {
    	$Alert += $server + "  ||   " + $drive + "   is low<br>"
    }
    
    
    #-----------------###(more servers ect.)--------------
    
    #================EMAIL===============
    
    $smtpServer = "192.168.x.x" 
    $ReportSender = "sender@email.com"  
    $users = "user@email.com"
    $MailSubject = "ALERT!!! Low DiskSpace"
    
    
    foreach($user in $users){
    if($true){
    	Write-Host "Sending Email notification to $user"
    	$smtp = New-Object Net.Mail.SmtpClient($smtpServer)
    		$msg = New-Object Net.Mail.MailMessage
    		$msg.To.Add($user)
            $msg.From = $ReportSender
    		$msg.Subject = $MailSubject
            $msg.IsBodyHTML = $True
            $msg.Body = $Alert
    		$smtp.Send($msg)
    		}
    else($false){
    	Write-Host "No one is pass the limit"
    }
    }
    
    
    
    
    

    Thursday, December 19, 2013 3:25 PM

Answers

  • This is the code that gets the result:

    if(isLow $server $drive $lim)
    {
     $Alert += $server + "  ||   " + $drive + "   is low<br>"
    }

    Here is how to test that.

    if($alert){
         # send mail
    }else{
        # don't send mail
    }


    ¯\_(ツ)_/¯

    • Marked as answer by KillahHerb Thursday, December 19, 2013 4:37 PM
    Thursday, December 19, 2013 4:26 PM

All replies

  • Try writing a small script that contains only the minimum amount of code needed to reproduce the problem.

    Bill

    Thursday, December 19, 2013 3:32 PM
  • Hi,

    if($true) is always going to be true. Try setting a variable to be true/false and then test for that instead.

    If ($someVar) {send-mailmessage} Else {}


    Don't retire TechNet! - (Don't give up yet - 12,420+ strong and growing)

    Thursday, December 19, 2013 3:34 PM
  • Will try that, Bill
    Thursday, December 19, 2013 3:52 PM
  • Even if i set 

    If ($someVar) {send-mailmessage} Else {}

    I still get an email with 0 servers not passing the limit, which is correct but i dont want to receive an email if 0 server are passing the limit.

    I realize the problem is within the fuction 

    if($freeSpaceGB -lt $limit){
    $email = "True";
    }
    else{
    $email = "False";
    }

    Im gonna try out the strategy Bill suggested and see if i get that part working first.

    Thursday, December 19, 2013 3:57 PM
  • Even if i set 

    If ($someVar) {send-mailmessage} Else {}

    <snip>

    In that case, can't you just test on $email?

    If ($email) {Send-MailMessage} Else {}

    Now that I look at your code again, I don't see this $email variable anywhere.


    Don't retire TechNet! - (Don't give up yet - 12,420+ strong and growing)

    Thursday, December 19, 2013 4:06 PM
  • I've changed 

    if($freeSpaceGB -lt $limit){
    			return $true;
    		}
    		else{
    			return $false;
    			}

    to

    if($freeSpaceGB -lt $limit){
    	$email = "True";
          }
    else{
    	$email = "False";
    	}
    Still get the same results 


    • Edited by KillahHerb Thursday, December 19, 2013 4:21 PM
    Thursday, December 19, 2013 4:21 PM
  • This is the code that gets the result:

    if(isLow $server $drive $lim)
    {
     $Alert += $server + "  ||   " + $drive + "   is low<br>"
    }

    Here is how to test that.

    if($alert){
         # send mail
    }else{
        # don't send mail
    }


    ¯\_(ツ)_/¯

    • Marked as answer by KillahHerb Thursday, December 19, 2013 4:37 PM
    Thursday, December 19, 2013 4:26 PM
  • Eureka! wow over saw that completely -_-

    Someone give this man a cookie ^

    Thanks jrv!

    Thursday, December 19, 2013 4:36 PM
  • I'm still not sure whether or not the current version of isLow does what it is supposed to. If the script is still not working I suggest you re-post the whole thing.

    a few other, unrelated suggestions:

    1. replace all instances of 1073741824 in the code with 1GB. This will make the intent more clear and it will be easier to be sure that the value is correct.
    2. put all of the server, drive, and lim values into a .csv file and replace all that repetitious code with a loop, somewhat along the lines of the (untested) code below. This will allow you to add or remove servers and drives, and change the limit values without having to modify the code.
    import-csv ./info.csv |
    	foreach {
    		if ( isLow $_.server $_.drive $_.lim ) {
    			$Alert += $_.server + "  ||   " + $_.drive + "   is low<br>"
    		}
    
    


    Al Dunbar -- remember to 'mark or propose as answer' or 'vote as helpful' as appropriate.

    Thursday, December 19, 2013 4:45 PM
  • Using a CSV is good.  Just load the "$servers" hash array from a CSV and the rest will work the same. The overall structure would work better if it was more like this.

    Function isLow($server, $drive, $limit){	
    
    	$disk = Get-WmiObject -ComputerName $server -Class Win32_LogicalDisk -Filter "DriveType = 3 and DeviceID = '$drive'"	
    	if($disk.FreeSSpace -lt $limit){
            $true
    	}else{
    	    $false
    	}
    }
    
    $servers=@()
    
    $servers=@{
        Server="###server1"
        Drive='C:'
        Limit=25Gb
    }
    
    $servers=@{
        Server="###server2"
        Drive='D:'
        Limit=15Gb
    }
    
    $servers=@{
        Server="###server3"
        Drive='C:'
        Limit=50Gb
    }
    
    $results=foreach($servers in $servers){
        if(isLow $server.Server $server.Drive $server.Limit){
    		'{0} || {1}   is low<br>' -f  $servers.Server,$server.Drive
        }
    }
    
    if($results){
     	Write-Host 'Sending Email notification' -fore green
        $mailprops=@{
            SmtpServer='192.168.x.x' 
            From='sender@email.com' 
            To=$users 
            Subject='ALERT!!! Low DiskSpace'
            Body=$results
            BodyAsHtml=$true
        }
        Send-MailMessage @mailprops
    }else{
    	Write-Host 'No one is past the limit' -ForegroundColor green
    }
    

    The biggest issue is to realize that this is a computer.  If you type the same thing more than once then consider that the computer can do it for you.  Once you learn to think like a computer all of this becomes easier.


    ¯\_(ツ)_/¯


    • Edited by jrv Thursday, December 19, 2013 4:49 PM
    Thursday, December 19, 2013 4:49 PM
  • Too extend this, how would we make it show how much free GB it currently contains? 
    Thursday, January 16, 2014 6:34 PM
  • Too extend this, how would we make it show how much free GB it currently contains? 

    Start by redesigning the script to give you an answer in Gb and not just a True/False.  For that you need to open a new question.


    ¯\_(ツ)_/¯

    Thursday, January 16, 2014 7:41 PM