My first Advanced Function

PowerShell Help
1

Hello dudes

I’ve created my advanced function and I want to know your remarks and notes. This function should read computer names from AD an invoke scriptblocks on them:

Function Invoke-CommandOnADComputers
{
    [CmdletBinding(SupportsShouldProcess=$True,ConfirmImpact='Low')]
    Param
    (
        # This is Active Directory Search Base to limit selection for computer accounts in domain.
        # It can be for example "OU=Computers,OU=Company Name,DC=domain,DC=local"
        [parameter(Mandatory=$true)]
        [string]
        $SearchBase,

        # Active Directory filter to merge your computer selection in to the detail.
        # It can be for example 'Name -like "Desktop*"'
        [string]
        $Filter = "*",

        # This is scriptblock which should be run on every computer.
        # For example { Restart-Service W32Time; }
        [parameter(Mandatory=$true)]
        [scriptblock]
        $ScriptBlock
    )
    Begin
    {
        #
        # Get list of computer accounts
        #
        Write-Debug "Getting list of computer from $ADSear"
        try
        {
            $ADComputersList = Get-ADComputer -SearchBase $SearchBase -Filter $Filter -ErrorAction Stop
        }
        catch
        {
            Write-Error -Message "Couldn't search in $SearchBase" -ErrorAction Stop
        }
        #
        # Write number of found computers
        #
        Write-Output "Found $($ADComputersList.Count) computers"
        #
        # If in debug, write list of computers
        #
        Write-Debug "List of machines:"
        If (!$PSDebugContext)
        {
            foreach ($item in $ADComputersList)
            {
                Write-Debug " $($item.Name)"
            }
        }
        Write-Debug "Done with domain computer list"
    }
    Process
    {
        #
        # Let's invoke command on remote computer
        #
        foreach ($ADComputer in $ADComputersList)
        {
            Write-Output $ADComputer.Name
                try
                {
                    Write-Debug "Invoking scriptblock on computer"
                    Invoke-Command -ComputerName $ADComputer.Name -ScriptBlock { $ScriptBlock } -ErrorAction Stop
                    Write-Output " Scriptblock invoked successful."
                }
                catch 
                {
                    Write-Output " Scriptblock invoked UNSUCCESSFUL."
                }
        }
    }
}
2

Hi Ondrej,

Thanks for sharing your function with us.

My thoughts:

  1. I would not execute the AD search in your function. Get-ADComputer has some other parameters like -Server or -Credential which I’m unable to use now because you’re function does not allow them. My approach would be to have the function take any array of computer names and work with it. It does not matter if the computer names came from AD or another database.
  2. It looks like you’re using Write-Output instead of Write-Host to output to the host but don’t output proper objects into the pipeline. My recommendation is to use Write-Verbose instead and to think about a [pscustomobject] which you can output to the pipeline with the results of the Invoke-Command to digest later.
  3. I think your usage of Write-Debug is not correct because with debug enabled your Write-Debug statement act like breakpoints as far as I know. I would change to Write-Verbose and ask users to specify -Verbose if they want to see what is happening.
  4. You have 2 foreach loops going over ADComputerList and you output the computer name anyway in your main foreach without debug enabled. I would remove the 1st foreach and use Write-Verbose instead of Write-Output in the main foreach loop.
  5. You’re not grabbing the results of Invoke-Command. I would grab the results and return them in a [pscustomobject] into the pipeline to make your functions more useful.

Best,
Daniel

3

Thank you for your remarks. I will think of them and maybe post changed function :slight_smile: