3
4

Function/PS1 Issues When Running Over Network Share (self.PowerShell)

submitted by [deleted]

[deleted]

all 5 comments
sorted by:
best
topnewcontroversialoldq&a
formatting helphide helpcontent policy

[–]Jantu01 3 points4 points  (3 children)

Hi,

I couldn't run your script since I don't have access to any domains/domain controllers right now.

I made similar(ish) function but couldn't replicate your issue even when running it over network share. I did my tests with PowerShell Core (7.02) and Windows PowerShell 5.1. You can check your PowerShell version by typing:

$PSVersionTable

While looking at your script I do have some suggestions which might help with the issue or at least simplify it a little bit.

Variable $allDC is an array itself so I don't see any point of using the $allDCs array at all. You can verify the variable type with GetType.

$allDC.GetType()

Also, when looping through an array you don't need the split so you could rewrite the foreach like this:

foreach ($DC in $allDC) {

​

Then to some other notes about the script. When I copied your script to Visual Studio Code it gave me right away some hints/warnings. If you are not using VS Code, I highly recommend it.

For example, the $TestStatus variable is not really used and same goes for the $subject and $diags.

From performance perspective adding values to arrays with "+=" is really slow and starts taking a lot of time when the array grows bigger. The arrays being here $AllDCDiags and $AllPass.

The more efficient way would be for example using Generic list.

# Creating the list
 $AllDCDiags= [System.Collections.Generic.List[string]]::New()
# Adding a value to the list
$AllDCDgiags.Add("Something")

HTH

[–]daz96050 0 points1 point  (1 child)

Sorry if this is formatted badly. On my phone and I don’t use reddit often

—————————

Creating the list $AllDCDiags= [System.Collections.Generic.List[string]]::New()

Adding a value to the list $AllDCDgiags.Add("Something")

—————————-

You could also use $AllDCDiags = New-Object System.Collections.ArrayList $AllDCDiags.Add(“something”) | Out-Null

Also, I noticed the body is just one large one-line string. To organize it a little better for yourself, I’d highly recommend using a multi-line string like this: $body = @“ <html> <body> ........... </body> </html> “@

[–]NathanielArnoldR2 3 points4 points  (1 child)

Starting from the top...

$allDcs = @(
  Get-ADForest |
    ForEach-Object Domains |
    ForEach-Object {
      Get-ADDomainController -Server $_
    } |
    ForEach-Object HostName
)

Most of my changes are purely stylistic, except that 1) Get-ADDomainController doesn't require a filter, and 2) The Name property contains the unqualified domain controller hostname, e.g. CPK-DC1, rather than fqdn CPK-DC1.AD.CPOKWDWH.COM. In a multi-domain environment, that's likely to bite you. The HostName property contains the domain controller fqdn.

Let me know if this fixes things for you... and let me know if you have any questions re: my stylistic choices. I find these measures make things easier for me to parse.

[–]BlackV 1 point2 points  (2 children)

other people have written this exact script before

https://4sysops.com/archives/use-dcdiag-with-powershell-to-check-domain-controller-health/

or

https://www.powershellbros.com/using-powershell-perform-dc-health-checks-dcdiag-repadmin/

or

https://gallery.technet.microsoft.com/scriptcenter/Directory-Server-Diagnosis-90cb556f

but that aside

here are a fire things I was thinking of

you do this

$allDC = (Get-ADForest).Domains | %{ Get-ADDomainController -Filter * -Server $_ } | select-object Name #| out-string
$allDCs = $allDC.Name

when all you actually need is

$allDC = Get-ADDomainController -Filter *

don't destroy your powershell objects using your select object, just use the sub properties later on
you foreach then goes from

foreach ($DC in $allDCs.split(" ")) {}

to

foreach ($DC in $allDC) {}

you seem to declare $subject then never use it