Calling function multiple times

This topic contains 4 replies, has 2 voices, and was last updated by  Mike Edwards 1 month, 1 week ago.

  • Author
    Posts
  • #88705

    Mike Edwards
    Participant

    Hi,

    I'm building a report about Computer information for various OUs in my AD environment. To do this, I wrote a script that collects various attribute information. I've put all of these informational queries into a function, then run them against several OUs that are in an array. Here's my code:

    Function Get-OUStats{
    Foreach($ou in $ous){
    $DaysInactive = 180
    $time = (Get-Date).Adddays(-($DaysInactive))
    $ouname = $ou.name
    $dn = $ou.DistinguishedName
    $windows2008 = (Get-ADComputer -Filter {OperatingSystem -Like '*Windows Server 2008*'} -SearchBase $dn -SearchScope Subtree | Measure-Object).count
    $active2008 = (Get-ADComputer -filter { OperatingSystem -Like '*Windows Server 2008*' -and LastLogontimestamp -gt $time} -Properties lastlogontimestamp -SearchBase $dn -searchscope Subtree | Measure-Object).count
    $inactive2008 = $windows2008 - $active2008
    $Windows2012 = (Get-ADComputer -Filter {OperatingSystem -Like '*Windows Server 2012*'} -SearchBase $dn -Properties lastlogontimestamp -SearchScope Subtree | Measure-Object).count
    $active2012 = (Get-ADComputer -filter { OperatingSystem -Like '*Windows Server 2012*' -and LastLogontimestamp -gt $time} -Properties lastlogontimestamp -SearchBase $dn -searchscope Subtree | Measure-Object).count
    $inactive2012 = $windows2012 - $active2012
    ....script continues, but was removed for brevity.
    }
    }
    $ounames = "OU1", "OU2", "OU3"
    foreach($ouname in $ounames){
    Get-OUStats
    $ous = Get-ADOrganizationalUnit -Filter {name -eq $ouname} -SearchBase "" -SearchScope onelevel -Properties distinguishedname, name
    }
    

    The problem is, the function runs for OU1 & OU2, but not OU3. If I remove OU3 from $ounames, it will run against OU1, but not OU2. This only appears to be a problem when the script runs for the first time. If I run it again in the ISE, it seems to work fine.

    Any advice/assistance would be greatly appreciated.

    Thanks.

  • #88706

    Don Jones
    Keymaster

    So, to start with, I'm not seeing where $ous gets populated. I'd expect that to be defined in a Param() block of the function, but I suspect you're relying on an out-of-scope variable. Bad. And this is in fact your problem.

    Indenting is your friend 😉

    Function Get-OUStats{
     Param($WorkWithThis)
     Foreach($ou in $WorkWithThis){
      $DaysInactive = 180
      $time = (Get-Date).Adddays(-($DaysInactive))
      $ouname = $ou.name
      $dn = $ou.DistinguishedName
      $windows2008 = (Get-ADComputer -Filter {OperatingSystem -Like '*Windows Server 2008*'} -SearchBase $dn -SearchScope Subtree | Measure-Object).count
      $active2008 = (Get-ADComputer -filter { OperatingSystem -Like '*Windows Server 2008*' -and LastLogontimestamp -gt $time} -Properties lastlogontimestamp -SearchBase $dn -searchscope Subtree | Measure-Object).count
      $inactive2008 = $windows2008 - $active2008
      $Windows2012 = (Get-ADComputer -Filter {OperatingSystem -Like '*Windows Server 2012*'} -SearchBase $dn -Properties lastlogontimestamp -SearchScope Subtree | Measure-Object).count
      $active2012 = (Get-ADComputer -filter { OperatingSystem -Like '*Windows Server 2012*' -and LastLogontimestamp -gt $time} -Properties lastlogontimestamp -SearchBase $dn -searchscope Subtree | Measure-Object).count
      $inactive2012 = $windows2012 - $active2012
    ....script continues, but was removed for brevity.
     }
    }
    
    $ounames = "OU1", "OU2", "OU3"
    foreach($ouname in $ounames){
    Get-OUStats $ouname
    $ous = Get-ADOrganizationalUnit -Filter {name -eq $ouname} -SearchBase "" -SearchScope onelevel -Properties distinguishedname, name
    }
    

    Now, I suspect I'm not following your logic, because the logic is why you're getting the results you are. But you can see how I've defined an input parameter for the function. You need to feed that whatever it is you want the function to work with. Here's what I think you were doing:

    1. You ran Get-OUStats with no input. So it didn't actually do anything.
    2. $ous was then populated by Get-ADOrganizationalUnit
    3. Get-OUStats ran again, this time relying on the higher-scope $ous, which contained OU1 at that point.
    4. $ous was re-populated by Get-ADOrganizationalUnit, this time with OU2.
    5. Get-OUStats ran again, using OU2.
    6. $ous was re-populated to contain OU3...
    7. ... but Get-OUStats didn't run again this time.

    I think you meant to run Get-OUStats AFTER Get-ADOrganizationalUnit. But trust me, you want to explicitly pass in, via parameters, whatever Get-OUStats is supposed to be working with. I THINK you meant:

    Function Get-OUStats{
     Param($WorkWithThis)
     Foreach($ou in $WorkWithThis){
      $DaysInactive = 180
      $time = (Get-Date).Adddays(-($DaysInactive))
      $ouname = $ou.name
      $dn = $ou.DistinguishedName
      $windows2008 = (Get-ADComputer -Filter {OperatingSystem -Like '*Windows Server 2008*'} -SearchBase $dn -SearchScope Subtree | Measure-Object).count
      $active2008 = (Get-ADComputer -filter { OperatingSystem -Like '*Windows Server 2008*' -and LastLogontimestamp -gt $time} -Properties lastlogontimestamp -SearchBase $dn -searchscope Subtree | Measure-Object).count
      $inactive2008 = $windows2008 - $active2008
      $Windows2012 = (Get-ADComputer -Filter {OperatingSystem -Like '*Windows Server 2012*'} -SearchBase $dn -Properties lastlogontimestamp -SearchScope Subtree | Measure-Object).count
      $active2012 = (Get-ADComputer -filter { OperatingSystem -Like '*Windows Server 2012*' -and LastLogontimestamp -gt $time} -Properties lastlogontimestamp -SearchBase $dn -searchscope Subtree | Measure-Object).count
      $inactive2012 = $windows2012 - $active2012
    ....script continues, but was removed for brevity.
     }
    }
    
    $ounames = "OU1", "OU2", "OU3"
    foreach($ouname in $ounames){
     $ous = Get-ADOrganizationalUnit -Filter {name -eq $ouname} -SearchBase "" -SearchScope onelevel -Properties distinguishedname, name
     Get-OUStats -WorkWithThis $ous
    }
    
  • #88919

    Mike Edwards
    Participant

    Thanks, Don. I understand that I need to use parameters. I'd like the function to work with OU1, OU2 & OU3. I replaced the "$workwiththis" variable in your example with $ous, which should represent the output of the Get-ADOrganizationalUnit cmdlet, but it didn't work. If I output the contents of $ous variable to the command window, it shows the appropriate information. Ideally, the Get-OUStats function would gather all of the information from the child OUs of OU1, then the child OUs of OU2, etc.

    Am I taking the right approach?

  • #88921

    Don Jones
    Keymaster

    Nope. Your problem is that you're using the same variable names both inside and outside of the function; because you're not really passing the data correctly, that's causing the confusion. Try re-doing the function to use unique variable names; it'll make the behavior and debugging more obvious.

    Also, you need to re-locate the call to the function, as I did in my second "fix."

  • #89324

    Mike Edwards
    Participant

    Hi, Don – I'm still struggling. I created separate names for each variable, but it's still not working. My main roadblock appears to be my lack of understanding of how the information in the param() statement interacts with the foreach loop (or does it?).

    Function Get-OUStats{
     param($workstationsou,$serversOU,$childous)
     Foreach($ou in $workstationsou,$serversOU,$childous){
        $DaysInactive = 180
        $time = (Get-Date).Adddays(-($DaysInactive))
        $ouname = $ou.name
        $dn = $ou.DistinguishedName
        [string]$ouowner = $ou.postofficebox
        $windows2008 = (Get-AdComputer -Filter {OperatingSystem -like "*Windows Server 2008*"} -SearchBase $dn -     SearchScope Subtree | Measure-Object).count
        $active2008 = (Get-ADComputer -filter { OperatingSystem -Like '*Windows Server 2008*' -and LastLogontimestamp     -gt $time} -Properties lastlogontimestamp -SearchBase $dn -searchscope Subtree | Measure-Object).count
        $inactive2008 = $windows2008 - $active2008
        $windows2012 = (Get-ADComputer -Filter {OperatingSystem -Like '*Windows Server 2012*'} -SearchBase $dn -    SearchScope Subtree | Measure-Object).count
        $active2012 = (Get-ADComputer -filter { OperatingSystem -Like '*Windows Server 2012*' -and LastLogontimestamp     -gt $time} -Properties lastlogontimestamp -SearchBase $dn -searchscope Subtree | Measure-Object).count
        $inactive2012 = $windows2012 - $active2012
    
    ....code continues, but was removed for brevity
    
    
    $workstationsOU = Get-ADOrganizationalUnit -Filter {name -eq "OU1 Name"} -SearchBase "dn of OU" -SearchScope OneLevel -Properties distinguishedname, name
    Get-OUStats $workstationsOU
    $serversOU = Get-ADOrganizationalUnit -Filter {name -eq "OU2 Name"} -SearchBase "dn of OU" -SearchScope OneLevel -Properties distinguishedname, name
    Get-OUStats $serversOU
    $childous = Get-ADOrganizationalUnit -Filter {name -like "*"} -SearchBase "dn of ou" -SearchScope OneLevel -Properties distinguishedname, name, postofficebox
    Get-OUStats $childOUs
    

    Is it even necessary to put all of the variable names in the foreach loop? If not, how does Powershell process the variable names that are in the param() statement, but not in the foreach loop? I tried combining the values of all 3 variables into 1, i.e. "$variable = $workstationsOU + $serversOU + $childous", but I got a method error indicating the Get-ADorganizationalUnit cmdlet doesn't have an "addition" method.

You must be logged in to reply to this topic.