Add Hyper-V host lifecycle audit script#48
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new administrator-run PowerShell script that audits a Hyper-V host by starting a transcript log, collecting host OS/hardware, disks, storage controllers, VM inventory, VHD details, checkpoints, virtual switches, network adapters, and VMQ status, then stops the transcript and reports the log path. (50 words) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as "Administrator (User)"
participant Script as "Invoke-HyperVHostAudit.ps1"
participant Host as "Hyper-V Host (CIM/Hyper-V cmdlets)"
participant Files as "VHD files / Log storage"
User->>Script: Launch with -LogPath
Script->>Files: Start-Transcript (create log)
Script->>Host: Query OS, ComputerSystem, Processor
Script->>Host: Query LogicalDisks, StorageControllers
Script->>Host: Get-VM (cache VM list)
loop For each VM
Script->>Host: Get-VM/VHD info (Get-VHD)
Host-->>Script: VHD size / used (or error)
Script->>Host: Get-VMSnapshot (checkpoints)
end
Script->>Host: Get-VMSwitch, Get-NetAdapter, Get-NetAdapterVmq
Script->>Files: Write summary to transcript
Script->>Files: Stop-Transcript
Script->>User: Print "HOST AUDIT COMPLETE" and transcript path
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
msft-windows/msft-windows-host-lifecycle-audit (2)
22-22: Cache VM inventory once for consistency.
Get-VMis called repeatedly at Line [22], Line [25], and Line [28]. Capturing it once reduces overhead and avoids cross-section drift during long runs.Suggested refactor
Write-Host "===== VM INVENTORY =====" -ForegroundColor Cyan -Get-VM | Select-Object Name, State, Generation, @{N='MemoryAssignedGB';E={[math]::Round($_.MemoryAssigned/1GB,1)}}, @{N='MemoryStartupGB';E={[math]::Round($_.MemoryStartup/1GB,1)}}, ProcessorCount, DynamicMemoryEnabled | Format-Table -AutoSize +$vms = Get-VM +$vms | Select-Object Name, State, Generation, @{N='MemoryAssignedGB';E={[math]::Round($_.MemoryAssigned/1GB,1)}}, @{N='MemoryStartupGB';E={[math]::Round($_.MemoryStartup/1GB,1)}}, ProcessorCount, DynamicMemoryEnabled | Format-Table -AutoSize Write-Host "===== VHDX PATHS & SIZES =====" -ForegroundColor Cyan -Get-VM | Get-VMHardDiskDrive | ForEach-Object { $vhd = Get-VHD -Path $_.Path -ErrorAction SilentlyContinue; [PSCustomObject]@{VMName=$_.VMName;Path=$_.Path;MaxSizeGB=if($vhd){[math]::Round($vhd.Size/1GB,1)}else{'ERROR'};UsedGB=if($vhd){[math]::Round($vhd.FileSize/1GB,1)}else{'ERROR'};Type=if($vhd){$vhd.VhdType}else{'N/A'}} } | Format-Table -AutoSize +$vms | Get-VMHardDiskDrive | ForEach-Object { $vhd = Get-VHD -Path $_.Path -ErrorAction SilentlyContinue; [PSCustomObject]@{VMName=$_.VMName;Path=$_.Path;MaxSizeGB=if($vhd){[math]::Round($vhd.Size/1GB,1)}else{'ERROR'};UsedGB=if($vhd){[math]::Round($vhd.FileSize/1GB,1)}else{'ERROR'};Type=if($vhd){$vhd.VhdType}else{'N/A'}} } | Format-Table -AutoSize Write-Host "===== VM CHECKPOINTS =====" -ForegroundColor Cyan -$snaps = Get-VM | Get-VMSnapshot -ErrorAction SilentlyContinue +$snaps = $vms | Get-VMSnapshot -ErrorAction SilentlyContinueAlso applies to: 25-25, 28-28
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@msft-windows/msft-windows-host-lifecycle-audit` at line 22, Capture the output of Get-VM once into a local variable (e.g., $vms) and reuse that variable in the subsequent pipelines instead of calling Get-VM multiple times; update the commands that currently use Get-VM followed by Select-Object / Format-Table (the pipeline that selects Name, State, Generation, MemoryAssignedGB, MemoryStartupGB, ProcessorCount, DynamicMemoryEnabled) and any other occurrences to read from the cached variable to reduce overhead and ensure a consistent VM snapshot across the report.
6-40: Persist audit output to%WINDIR%\Logsfor traceability.Current output is console-only. Adding transcript logging improves QA evidence retention and post-run validation.
Suggested transcript wrapper
+$logPath = Join-Path $env:WINDIR ("Logs\\HyperV-Host-Audit-{0}.log" -f (Get-Date -Format "yyyyMMdd-HHmmss")) +Start-Transcript -Path $logPath -Force | Out-Null + Write-Host "===== HOST OS & HARDWARE =====" -ForegroundColor Cyan ... Write-Host "===== HOST AUDIT COMPLETE =====" -ForegroundColor Green + +Stop-Transcript | Out-Null +Write-Host ("Audit transcript: {0}" -f $logPath) -ForegroundColor GreenBased on learnings:
Applies to **/*.ps1 : Verify transcripts/logs in %WINDIR%\logs after execution; support RMM simulation by setting $RMM=1 and predefining required variables during testing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@msft-windows/msft-windows-host-lifecycle-audit` around lines 6 - 40, The script currently only writes to the console (Write-Host) and should persist output to %WINDIR%\Logs; update the top of the script to start a transcript (Start-Transcript) into a uniquely named file under $env:WINDIR\Logs after creating that folder if missing, and ensure Stop-Transcript runs at the end (or in a finally block) so all output from the host and VM enumeration sections (calls such as Get-CimInstance, Get-VM, Get-VHD, Get-VMSnapshot, Get-VMSwitch, Get-NetAdapter, Get-NetAdapterVmq) is captured; also add support for RMM simulation by honoring a $RMM variable (default 0) and allowing predefinition of required variables during testing, and log any Start/Stop-Transcript errors to the console (Write-Host) so failures to write the log are visible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@msft-windows/msft-windows-host-lifecycle-audit`:
- Around line 1-9: Add explicit preflight checks before any Hyper-V cmdlet
usage: verify the script is running elevated (check membership in Administrators
or use Test-ProcessAdminRights) and confirm the Hyper-V module is available and
importable (use Get-Command Get-VM/Get-VHD/Get-VMSnapshot/Get-VMSwitch or try
Import-Module Hyper-V -ErrorAction Stop); if checks fail, emit a clear error and
exit early so later calls to Get-VM, Get-VMHardDiskDrive, Get-VHD,
Get-VMSnapshot and Get-VMSwitch are never attempted without privileges or the
module, or alternatively gate the Hyper-V sections behind those checks to skip
them safely.
- Around line 28-29: Replace the silent failures around the Get-VMSnapshot and
Get-NetAdapterVmq calls with explicit error handling: call Get-VMSnapshot and
Get-NetAdapterVmq with ErrorAction Stop inside a try/catch, assign results to
$snaps (and the net adapter var) and in the catch use Write-Warning (or output
an "ERROR" marker) to indicate the query failed; only if the call succeeds check
for empty result and emit "No checkpoints found" (or the existing success
output). Use the same pattern as the inline check at line 25
(if($vhd){...}else{'ERROR'}) and reference the Get-VMSnapshot/Get-NetAdapterVmq
call sites and $snaps variable to locate and update the code.
---
Nitpick comments:
In `@msft-windows/msft-windows-host-lifecycle-audit`:
- Line 22: Capture the output of Get-VM once into a local variable (e.g., $vms)
and reuse that variable in the subsequent pipelines instead of calling Get-VM
multiple times; update the commands that currently use Get-VM followed by
Select-Object / Format-Table (the pipeline that selects Name, State, Generation,
MemoryAssignedGB, MemoryStartupGB, ProcessorCount, DynamicMemoryEnabled) and any
other occurrences to read from the cached variable to reduce overhead and ensure
a consistent VM snapshot across the report.
- Around line 6-40: The script currently only writes to the console (Write-Host)
and should persist output to %WINDIR%\Logs; update the top of the script to
start a transcript (Start-Transcript) into a uniquely named file under
$env:WINDIR\Logs after creating that folder if missing, and ensure
Stop-Transcript runs at the end (or in a finally block) so all output from the
host and VM enumeration sections (calls such as Get-CimInstance, Get-VM,
Get-VHD, Get-VMSnapshot, Get-VMSwitch, Get-NetAdapter, Get-NetAdapterVmq) is
captured; also add support for RMM simulation by honoring a $RMM variable
(default 0) and allowing predefinition of required variables during testing, and
log any Start/Stop-Transcript errors to the console (Write-Host) so failures to
write the log are visible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0f92c44d-a0d1-44bc-b3dc-10ccf428616c
📒 Files selected for processing (1)
msft-windows/msft-windows-host-lifecycle-audit
Addresses all CodeRabbit review feedback from DTC-Inc/msp-script-library#48: - Add #Requires -RunAsAdministrator and #Requires -Modules Hyper-V for preflight validation (Issue 1: missing preflight checks) - Cache Get-VM result once ($allVMs) to avoid repeated calls and cross-section drift during long runs (Issue 2: repeated Get-VM) - Add Start-Transcript/Stop-Transcript logging to %SystemRoot%\Logs for QA evidence retention (nitpick: console-only output) - Replace -ErrorAction SilentlyContinue with try/catch blocks on Get-VHD, Get-VMSnapshot, and Get-NetAdapterVmq to surface real errors instead of silently masking them (nitpick: audit masking) - Add proper comment-based help, CmdletBinding, and -LogPath param to match repo script conventions https://claude.ai/code/session_016vxJGXe9VEcq5g2rqDPK4J
Audits Hyper-V host for server lifecycle quote validation, gathering comprehensive information including OS, hardware, CPU, memory, disk layout, VM inventory, and network configuration.
Gumbees
left a comment
There was a problem hiding this comment.
Summary
Adds a Hyper-V host audit script that inventories OS, hardware, disks, controllers, VMs, VHDX paths/sizes, checkpoints, virtual switches, and network adapters. Designed to run on the existing host during VM migration scenarios.
What works
- Genuinely useful coverage for migration QA: VM inventory with memory and CPU allocation, VHDX max-vs-used sizing, checkpoint state, VMQ status. Operators get the full picture in one log.
- Caches
Get-VMonce into$allVMsand reuses it across sections ... avoids drift between sections that would otherwise hit the API multiple times. - VHD enumeration wraps
Get-VHDin try/catch with an explicit ERROR row in the output, so a single unreadable VHD doesn't kill the whole audit. - Checkpoint query catches the specific
Microsoft.HyperV.PowerShell.VirtualizationExceptionso "no checkpoints" prints cleanly rather than crashing.
Concerns
BLOCKER: Filename has no .ps1 extension. The diff adds msft-windows/msft-windows-host-lifecycle-audit ... no extension. PowerShell will not execute this as a script. Same issue as PRs #43, #44, #47. Rename to msft-windows/msft-windows-host-lifecycle-audit.ps1.
Does not follow the template. No ## $VariableName RMM comment block, no $RMM dual-mode handling, no $Description audit trail. The script uses a [CmdletBinding()] param block with $LogPath instead. That's a reasonable cmdlet pattern but it's not what the repo template asks for. Either follow the template or update the template if [CmdletBinding()] is the new direction.
Em dashes in section banners (lines like # ── Logging ───). The repo style explicitly avoids these per CLAUDE.md. Not a hard blocker but worth a sweep.
Suggestions
- The Unicode box drawing characters in section headers render poorly in NinjaOne script logs depending on encoding. Plain ASCII (
===== HOST OS & HARDWARE =====) is safer ... matches the pattern PR #47 uses. - Consider exporting structured output (JSON or CSV) in addition to the transcript, so the audit can be consumed programmatically by a quote tool.
- The reference to "BookStack Page 1787" should be a clickable link to the actual page on
kb.dtctoday.com, not a bare page ID ... per CLAUDE.md surfacing rules.
Verdict
REQUEST CHANGES ... missing .ps1 extension is the only hard blocker; rename and this is approveable. Template-alignment is worth doing but won't gate merge if you decide the [CmdletBinding()] shape is acceptable for audit-style scripts.
used during server quote QA checks to gather all HV host information
Summary by CodeRabbit