Author Posts

June 8, 2016 at 2:25 pm

Hi All,

I was hoping any of you could take a look at my advanced function, I was hoping for some good criticism.

Check-NTUpStatus

the function checks in the system in pingable, is listening for RDP connections and and gets the up time.

While these things are easy to gather I took a while to get this to happen as background jobs. I figured it out but was wondering if you had any thoughts on on how it could be done better.

June 8, 2016 at 2:46 pm

get rid of the redundant code. Put your custom object in a function and supply computername as an argument.

Your datetime conversion doesn't have to be that complicated.

$Boot = Get-WmiObject Win32_OperatingSystem
$uptime = $Boot.ConvertToDateTime($Boot.LastBootupTime)

June 8, 2016 at 3:40 pm

Good shout on the code cleanup, i'll do that.
And the date time would definitely be easier, I was over thinking it and making it easier to read for the end user in our scenario (Patching)

June 8, 2016 at 4:26 pm

use $env:COMPUTERNAME rather than localhost for default computer name. I've seen issues with local host in the past

Your parameter block would be easier to read with a blank line between parameters

Help messages on parameters only work if the parameter is mandatory

Need an -ErrorAction Stop on the Import-Module for the try – catch to work

$computername parameter isn't defined as an array so will only have a count of 1. If you want it to be an array need to define the parameter as
[String[]]$ComputerName

Use full parameter names -ErrorAction not -ea

If you use Get-CimInstance rather than Get-WmiObject the dates will be in a usable format without any need to convert

Create properties for your object in a hashtable and then use New-object – lots less typing than multiple Add-Members

If you create the object before doing the WMI query you can reduce code – in fact create it near the top of the code than fill in the values – much less coding

If you make the computername parameter an array you can cut your code in half as the singleton case is dealt with as part of the array processing

You can reduce the code to a single foreach loop in the PROCESS block if you pass the values in computername or from the file to a $computers variable

Overall you can cut this code down by a large percentage if you consolidate the code blocks – it'll then be much easier to update and expand

June 8, 2016 at 5:07 pm

ComputerName param:
Why not use "Name" instead, and add ComputerName and __SERVER as aliases? This will allow it to more easily pipe with wmio and Adcomp gets.

June 9, 2016 at 12:43 pm

Thanks everyone.

I've made the changes as suggested.
Both old and new functions are still on the page.
I'm having issues with the order of the output object.
I made the hashtable [Ordered] but I think as soon as I changed the object it seems to loose the order.

Any ideas?

June 9, 2016 at 2:10 pm

I think you could get this down to about 10 lines..You are only doing two things. I have a full gui that does this in more in less than 200 lines.

 photo Untitled.png

June 9, 2016 at 2:23 pm

If you #requires -version 3 why you work with hashtables and not use just

$ObjectProps =[PSCustomObject] [Ordered]@{
					"ComputerName" = "";
					"LastBootUpTime" = "Could Not Query";
					"Ping" = "No Responce";
					"RDP" = "Not Listening";
					"ObjectCount" = ""
				}

?
and now you create job even for one computer...
it's a great waste of time/resources
may be it's time to try
https://github.com/proxb/PoshRSJob ?

btw, I don't know why you need ObjectCount, but if you use it, yiu can generate it outside of a job in one place or while creating your obj and not copypasting it in every code branch

  • This reply was modified 2 years, 3 months ago by  Max Kozlov.

June 9, 2016 at 9:51 pm

1) This may be picky, but your parameter set naming does not really represent what is happening there. Singular represents commandline input, and Multiple represents file input. In both cases, however, multiple servers to process against are accepted.

Singular accepts an array of strings which can be many server names.
Multiple accepts a text file input that can be many server names.

2) In your Importing needed modules section. It may be worth while to add the actual error message that generated the stop action. It will be helpful in determining which module had the issue.

		Catch
		{
			$ErrorMessage = $_.Exception.Message
			Write-Warning "Module NetTCPIP or CimCmdlets not installed: $ErrorMessage"
			Break
		}

3) Again this one is a little picky, but this loop runs logically opposite from the verbiage used. When $AddJobs is true, it loops and does not proceed to add any jobs, When $AddJobs is false, it breaks the loop and proceeds with adding a job.

			do
			{
				$LimitJobCount = @(Get-Job | Where-Object State -eq 'Running')
				if ($LimitJobCount.Count -gt $ConcurrentJobs)
				{
					$AddJobs = $true
				}
				else
				{
					$AddJobs = $false
				}
			}
			while ($AddJobs)

Using until and less than makes the verbiage match the logic

			do
			{
				$LimitJobCount = @(Get-Job | Where-Object State -eq 'Running')
				if ($LimitJobCount.Count -lt $ConcurrentJobs)
				{
					$AddJobs = $true
				}
				else
				{
					$AddJobs = $false
				}
			}
			until ($AddJobs)

4) I may also be wise to a some down time to the above Do loop using start-sleep so that the processor is not constantly churning while waiting to start another job.

June 10, 2016 at 8:56 am

May be I'm repeating, but all job related problems Curtis mentioned already solved with PoshRSJob 🙂
look at usage example and note -Trottle parameter

1..25 | Start-RSJob -Batch pingtest -ScriptBlock { "{0}: {1}" -f $_, [DateTime]::Now; Start-Sleep -Seconds 5 } -Throttle 5

Get-RSJob -Batch pingtest | Wait-RSJob | Receive-RSJob

Get-RSJob -Batch pingtest | Remove-RSJob

Inside of scriptblock there can be your code and instead of 1..25 can be your prepared input objects

code schema:

#requires -Version 3 -Module modulename1, modulename2 #your modules here, not need module checking
# but can't remember is v3.0 support it or not
begin {
# some checking, test function/scriptblock declaration can be here
}
process{
  if (parameterset -eq 'file') {
    $computername = gc $file # there you can do txt/csv conversions/parsing
  }
  foreach ($comp in $computername) {
    $null = # not need to save Start-RSJob output if use -batch name
      [PSCustomObject]@{
         ComputerName = $comp
         Object = 'Here'
         Another = '...'
     } | Start-RSJob -batch #  complete code here ....
  }
}
end {
  Get-RSJob -batch | Wait-RSJob | Receive-RSJob
  Get-RSJob -batch | Remove-RSJob
}

I think its much shorter and cleaner

June 10, 2016 at 9:23 am

Should say:
My code schema above have one flaw: -Throttle parameter for Start-RSJob work only for current pipeline input
i.e 1..25 | Start-RSJob -throttle 5 can work as expected
and
1..25 | foreach { $_ | Start-RSJob -throttle 5 } is not
thus for full throttling support code must make its own queue checking like yours or collect all input objects and send it to start-rsjob later #more memory pressure!
something like that

begin {
   $all = {}.Invoke() # use collection instead of array
}
process{
  if ( parameterset -eq 'file') {
    gc $file | foreach { $all.Add($_)  }
  }
  else {
    $computername | foreach { $all.Add($_) }
  }
}
end{
  $null =  $all | foreach { [pscustomobject]@{ }  } | start-rsjob
  get-rsjob ...
  ...
}
  • This reply was modified 2 years, 3 months ago by  Max Kozlov.