patternshellMinor
Powershell Script for setting DHCP reservation description & host value
Viewed 0 times
scriptdescriptionpowershellreservationvaluesettingdhcphostfor
Problem
I'm new to Powershell scripting and trying to write a script that will go through printer queues on a print server, get the print queue's port address, then compare the print queues port address to the DHCP reservation associated with the print queue port address on the server. If the port address of the print queue compares, it should then set the description of the DHCP reservation to reflect the print queue name and location. It should then also set the hostname option to be the print queue's name.
I'm a bit unsure of the following code within the IF statement, is this the right way to have the loop process the two commands?
```
{Set-DhcpServerv4Reservation -ComputerName $DHCPSvr -IPAddress $PrinterObjects.PortName -Description "$PrinterObjects.Name & $PrinterObjects.Location"}
{Set-DhcpServerv4OptionV
#Set DHCP reservation description and host value on server
#DHCP Server for site
$DHCPSvr = "Server1"
#DHCP Server Scope ID
$DHCPSvrScope = "10.224.148.0"
#Get Printer Management list of shared available printers
$PrinterObjects = Get-Printer -ComputerName Server1 * | Select-Object Name, PortName, Shared, Location |
Where-Object { ($_.Name -match "^.{6}$") -and ($_.PortName -match "IP*" -or $_.PortName -match '^10*') -and ($_.Shared -EQ $True)} |
Sort-Object -Property PortName -Descending
#Get DHCP Reservation list from server
$DHCPSvrReservations = Get-DhcpServerv4Reservation -ComputerName $DHCPSvr -ScopeId $DHCPSvrScope
foreach ($DHCPSvrReservation in $DHCPSvrReservations){
foreach ($PrinterObject in $PrinterObjects)
{
if ($DHCPSvrReservation.IPAddress -eq $PrinterObjects.PortName)
{
{Set-DhcpServerv4Reservation -ComputerName $DHCPSvr -IPAddress $PrinterObjects.PortName -Description "$PrinterObjects.Name & $PrinterObjects.Location"}
{Set-DhcpServerv4OptionValue -ScopeId $DHCPSvrScope -OptionId 12 $PrinterObjects.Name}
}
}
}I'm a bit unsure of the following code within the IF statement, is this the right way to have the loop process the two commands?
```
{Set-DhcpServerv4Reservation -ComputerName $DHCPSvr -IPAddress $PrinterObjects.PortName -Description "$PrinterObjects.Name & $PrinterObjects.Location"}
{Set-DhcpServerv4OptionV
Solution
Very impressed with your formatting and spacing. Top notch to see that in practice. I see a couple areas that could be improved and I see at least 2 problems.
Variable Expansion in Strings
Variables will expand in double quotes easy. However you need to be using subexpressions in order to get the property portion to work correctly. You can also use the format operator
If this code is in use already check existing descriptions for dependencies.
Efficient and Reliable conditions
The first clause looks like you are just trying to find
In the second clause you are using regex as well and while I doubt it will cause conflicts you need to be aware you would match IP as well as IPPPPPPPPPPP.
Similarily with
Possible trade off of readabilty but for the last clause here (
Curly Braces
You are slightly inconsistent in your
Same goes for your
Your
Using if when Where would do
You act on all
That should work but you should test first.
Variable Expansion in Strings
Variables will expand in double quotes easy. However you need to be using subexpressions in order to get the property portion to work correctly. You can also use the format operator
-f but that is better used for more complicated string.... -Description "$($PrinterObjects.Name) & $($PrinterObjects.Location)"If this code is in use already check existing descriptions for dependencies.
Efficient and Reliable conditions
Where-Object { ($_.Name -match "^.{6}$") -and ($_.PortName -match "IP*" -or $_.PortName -match '^10*') -and ($_.Shared -EQ $True)}The first clause looks like you are just trying to find
Name's that are exactly 6 characters long? Just use the length property all strings have instead. No need for regex: $_.Name.Length -eq 6).In the second clause you are using regex as well and while I doubt it will cause conflicts you need to be aware you would match IP as well as IPPPPPPPPPPP.
-like "IP*" or the string method .StartsWith("IP") would work just as well. Note that StartsWith() is case sensitive.Similarily with
$_.PortName -match '^10' if you are just trying to find ones where the first octet is 10 you will also match ones that are 100 or 1000000 since the greedy quantifier is in use here. Solution is the same as the last one. -like "10." or the string method .StartsWith("10."). Notice I added periods. That may or not be an issue for you in production.Possible trade off of readabilty but for the last clause here (
($_.Shared -EQ $True)) you don't need to use -eq. PowerShell will evaluate $_.Shared as its own expression. If it is a non-zero length string or non-zero integer it will evaluate to true.Curly Braces
You are slightly inconsistent in your
foreach blocks. Some have the opening brace after the block and some have it on the following line. I would pick on standard and stick with it. I personally favour the former.foreach ($DHCPSvrReservation in $DHCPSvrReservations){
# Do stuff
}Same goes for your
if and other similar blocks and constructs.Your
Set-DhcpServerv4Reservation and Set-DhcpServerv4OptionValue dont need to be surrounded by curly braces. They should function the same without them.Using if when Where would do
You act on all
$printerobject's that have a matching reservation. You loop though is still running through the $printerobject's that don't match. True, you don't do anything with them but you could move that condition into your loop then you can drop the ifforeach ($DHCPSvrReservation in $DHCPSvrReservations){
foreach ($PrinterObject in ($PrinterObjects | Where-Object{$DHCPSvrReservation.IPAddress -eq $_.PortName})){
Set-DhcpServerv4Reservation -ComputerName $DHCPSvr -IPAddress $PrinterObjects.PortName -Description "$($PrinterObjects.Name) & $($PrinterObjects.Location)"
Set-DhcpServerv4OptionValue -ScopeId $DHCPSvrScope -OptionId 12 $PrinterObjects.Name
}
}That should work but you should test first.
Code Snippets
... -Description "$($PrinterObjects.Name) & $($PrinterObjects.Location)"Where-Object { ($_.Name -match "^.{6}$") -and ($_.PortName -match "IP*" -or $_.PortName -match '^10*') -and ($_.Shared -EQ $True)}foreach ($DHCPSvrReservation in $DHCPSvrReservations){
# Do stuff
}foreach ($DHCPSvrReservation in $DHCPSvrReservations){
foreach ($PrinterObject in ($PrinterObjects | Where-Object{$DHCPSvrReservation.IPAddress -eq $_.PortName})){
Set-DhcpServerv4Reservation -ComputerName $DHCPSvr -IPAddress $PrinterObjects.PortName -Description "$($PrinterObjects.Name) & $($PrinterObjects.Location)"
Set-DhcpServerv4OptionValue -ScopeId $DHCPSvrScope -OptionId 12 $PrinterObjects.Name
}
}Context
StackExchange Code Review Q#122869, answer score: 4
Revisions (0)
No revisions yet.