Network Persistence For Windows

[meghansh-pf9]

What this PR does / why we need it

This PR enables automatic network configuration persistence for Windows VMs during migration. When migrating a Windows VM, the network interfaces are captured from the source and applied on the first boot of the migrated VM using a PowerShell script and a batch launcher, ensuring the VM retains its network interface names post migration

fixes #1348

Testing done

Pre Migration

Post Migration

[windsurf-bot]

Other comments (10)

• scripts/firstboot/windows/NIC-Recovery/Recover-HiddenNICMapping.ps1 (102-104) The script doesn't include error handling when writing the output file. Consider adding try/catch blocks to handle potential file writing errors:

  if (-not $result) { $result = @() }
  try {
      $result | ConvertTo-Json -Depth 4 | Set-Content -Encoding UTF8 $OutFile -ErrorAction Stop
      exit 0
  } catch {
      Write-Error "Failed to write network configuration: $_"
      exit 1
  }
  

• scripts/firstboot/windows/NIC-Recovery/Orchestrate-NICRecovery.ps1 (43-44) The redirection approach using `*>> $LogFile` might miss some PowerShell errors that don't set $LASTEXITCODE. Consider using a try/catch with transcript or capturing output with a more robust pattern:

          Write-Log "Running Recover-HiddenNICMapping.ps1..."
          $output = & "$ScriptRoot\Recover-HiddenNICMapping.ps1" 2>&1
          $output | Out-File -FilePath $LogFile -Append -Encoding utf8
          if ($LASTEXITCODE -ne 0 -or $?) { throw "Script failed with exit code $LASTEXITCODE or error $output" }
  

• scripts/firstboot/windows/NIC-Recovery/Orchestrate-NICRecovery.ps1 (42-44) The script attempts to execute dependent scripts without first checking if they exist. Consider adding existence checks before execution to provide clearer error messages:

          Write-Log "Running Recover-HiddenNICMapping.ps1..."
          $scriptPath = "$ScriptRoot\Recover-HiddenNICMapping.ps1"
          if (-not (Test-Path $scriptPath)) {
              throw "Required script not found: $scriptPath"
          }
          & $scriptPath *>> $LogFile
          if ($LASTEXITCODE -ne 0) { throw "Script failed with exit code $LASTEXITCODE" }
  

• scripts/firstboot/windows/NIC-Recovery/Orchestrate-NICRecovery.ps1 (1-14) The script only checks and creates the log directory when writing to the log file. This should be done once at script initialization to avoid potential race conditions or errors with multiple log writes:

  # Orchestrate-NICRecovery.ps1
  $ScriptRoot = "C:\NIC-Recovery"
  $LogFile = Join-Path $ScriptRoot "NIC-Recovery.log"
  
  # Ensure log directory exists at script start
  if (-not (Test-Path $ScriptRoot)) {
      New-Item -ItemType Directory -Path $ScriptRoot -Force | Out-Null
  }
  
  function Write-Log {
      param([string]$Message, [string]$Level = "INFO")
      $timestamp = Get-Date -Format "yyyy-MM-dd HH:mm:ss"
      $logEntry = "$timestamp - $Level - $Message"
      
      try {
          $logEntry | Out-File -FilePath $LogFile -Append -Encoding utf8
  

• v2v-helper/migrate/migrate.go (1062-1062) There's an inconsistency in variable naming between functions. In `configureWindowsNetwork()` you use `persistNetwork`, but in `configureLinuxNetwork()` you use `persisNetwork`. Consider standardizing the variable name across both functions for consistency.
• v2v-helper/pkg/constants/constants.go (223-224) The comment on line 223 refers to 'NICRecoveryFirstBootScript' but the actual constant is named 'WindowsPersistFirstBootScript'. Consider updating the comment to match the constant name for consistency.
• scripts/firstboot/windows/NIC-Recovery/Cleanup-GhostNICs.ps1 (123-123) The file is missing a newline at the end. Adding one would follow best practices for text files.
• scripts/firstboot/windows/NIC-Recovery/Cleanup-GhostNICs.ps1 (1-1) The comment at the top references 'Enhanced-Cleanup-v2.ps1' but the file is named 'Cleanup-GhostNICs.ps1'. Consider updating the comment to match the actual filename.
• scripts/firstboot/windows/NIC-Recovery/Cleanup-GhostNICs.ps1 (6-6) The default log file path references 'Enhanced-Cleanup-v2.log' but should probably match the script name 'Cleanup-GhostNICs.log' for consistency.
• scripts/firstboot/windows/NIC-Recovery/Restore-Network.ps1 (127-127) Add a newline at the end of the file to follow text file best practices.

💡 To request another review, post a new comment with "/windsurf-review".

[windsurf-bot]

The batch script contains multiple pause commands (lines 245, 271, 321) which will halt execution waiting for user input. For an automated migration process, these could cause the script to hang indefinitely. Consider removing these pauses or adding a timeout mechanism for automated scenarios.

[windsurf-bot]

The error message doesn't include the specific command that failed. For consistency with other error handling in this file, consider updating the error return to include the command name:

Suggested change

	if err != nil {
	fmt.Printf("failed to run command (%s): %v: %s\n", "copy-in", err, strings.TrimSpace(ans))
	return err
	}
	if err != nil {
	fmt.Printf("failed to run command (%s): %v: %s\n", "copy-in", err, strings.TrimSpace(ans))
	return fmt.Errorf("failed to run command (%s): %w: %s", "copy-in", err, strings.TrimSpace(ans))
	}

[windsurf-bot]

The script attempts to write to C:\NIC-Recovery\netconfig.json but doesn't verify if the directory exists first. Consider adding directory creation logic to prevent failures when the directory doesn't exist:

Suggested change

	# Recover-HiddenNICMapping.ps1
	$OutFile = "C:\NIC-Recovery\netconfig.json"
	# Recover-HiddenNICMapping.ps1
	$OutputDir = "C:\NIC-Recovery"
	$OutFile = "$OutputDir\netconfig.json"
	# Ensure output directory exists
	if (-not (Test-Path $OutputDir)) {
	New-Item -ItemType Directory -Path $OutputDir -Force | Out-Null
	}