Skip to content

Conversation

@mbrat2005
Copy link
Contributor

No description provided.

@mbrat2005 mbrat2005 requested a review from Copilot August 22, 2025 15:51
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR simplifies NAT rule migration by removing asynchronous job handling and improving log timestamp precision. The changes focus on streamlining the load balancer update process and enhancing logging capabilities.

  • Replaced asynchronous Set-AzLoadBalancer job with synchronous call
  • Enhanced log timestamp format to include milliseconds
  • Updated module version and release notes

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
NatRulesMigration.psm1 Simplified load balancer update from async job to synchronous call
Log.psm1 Enhanced timestamp format to include milliseconds for better precision
AzureBasicLoadBalancerUpgrade.psd1 Updated module version and release notes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

If ($UpdateLBNATRulesJob.Error -or $UpdateLBNATRulesJob.State -eq 'Failed') {
log -Severity Error -Message "Saving Standard Load Balancer $($StdLoadBalancer.Name) failed with the following errors: $($UpdateLBNATRulesJob.error; $UpdateLBNATRulesJob | Receive-Job). Migration will continue--to recover, manually add the NAT rules to the load balancer, then correct NAT Rule NIC membership after the script completes."
}
Set-AzLoadBalancer -LoadBalancer $StdLoadBalancer > $null
Copy link

Copilot AI Aug 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the asynchronous job handling eliminates progress feedback and detailed error information that was previously available. The original implementation provided status updates during long-running operations and captured specific job errors, which could be valuable for troubleshooting.

Suggested change
Set-AzLoadBalancer -LoadBalancer $StdLoadBalancer > $null
$job = Set-AzLoadBalancer -LoadBalancer $StdLoadBalancer -AsJob
log -Message "[NatRulesMigration] Set-AzLoadBalancer job started (Id: $($job.Id)). Monitoring progress..."
while ($job.State -eq 'Running' -or $job.State -eq 'NotStarted') {
log -Message "[NatRulesMigration] Set-AzLoadBalancer job status: $($job.State)..."
Start-Sleep -Seconds 5
$job = Get-Job -Id $job.Id
}
if ($job.State -eq 'Completed') {
log -Message "[NatRulesMigration] Set-AzLoadBalancer job completed successfully."
# Optionally, retrieve job output if needed
# $output = Receive-Job -Id $job.Id
}
elseif ($job.State -eq 'Failed') {
$errorMsg = "[NatRulesMigration] Set-AzLoadBalancer job failed. Error: $((Receive-Job -Id $job.Id | Out-String))"
log "Error" $errorMsg
}
else {
$errorMsg = "[NatRulesMigration] Set-AzLoadBalancer job ended with unexpected state: $($job.State)."
log "Error" $errorMsg
}
# Clean up job
Remove-Job -Id $job.Id

Copilot uses AI. Check for mistakes.
…rade/modules/NatRulesMigration/NatRulesMigration.psm1

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@mbrat2005 mbrat2005 merged commit 5477837 into main Aug 22, 2025
2 checks passed
mbrat2005 added a commit that referenced this pull request Dec 19, 2025
* add milliseconds to logging

* Remove use of AzJob for LB save to simplify flow

* v2.5.40

* Update AzureBasicLoadBalancerUpgrade/module/AzureBasicLoadBalancerUpgrade/modules/NatRulesMigration/NatRulesMigration.psm1

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants