-
Notifications
You must be signed in to change notification settings - Fork 229
Dump out correlation id without verbose logging for resource deployment #4532
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -723,29 +723,27 @@ try { | |
| Log $msg | ||
|
|
||
| $deployment = Retry { | ||
| $lastDebugPreference = $DebugPreference | ||
| try { | ||
| if ($CI) { | ||
| $DebugPreference = 'Continue' | ||
| } | ||
| New-AzResourceGroupDeployment -Name $BaseName -ResourceGroupName $resourceGroup.ResourceGroupName -TemplateFile $templateFile.jsonFilePath -TemplateParameterObject $templateFileParameters -Force:$Force | ||
| } catch { | ||
| Write-Output @' | ||
| New-AzResourceGroupDeployment ` | ||
| -Name $BaseName ` | ||
| -ResourceGroupName $resourceGroup.ResourceGroupName ` | ||
| -TemplateFile $templateFile.jsonFilePath ` | ||
| -TemplateParameterObject $templateFileParameters ` | ||
| -Force:$Force | ||
| } | ||
|
|
||
| if ($deployment.ProvisioningState -ne 'Succeeded') { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hopefully explicitly checking for succeeded will help handle some error cases that were slipping through.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah we weren't handling non-succeeded states any differently before. IIRC we saw some issues with the state being |
||
| Write-Host "Deployment '$($deployment.DeploymentName)' has state '$($deployment.ProvisioningState)' with CorrelationId '$($deployment.CorrelationId)'. Exiting..." | ||
| Write-Host @' | ||
| ##################################################### | ||
| # For help debugging live test provisioning issues, # | ||
| # see http://aka.ms/azsdk/engsys/live-test-help, # | ||
| # see http://aka.ms/azsdk/engsys/live-test-help # | ||
| ##################################################### | ||
| '@ | ||
| throw | ||
| } finally { | ||
| $DebugPreference = $lastDebugPreference | ||
| } | ||
| exit 1 | ||
| } | ||
|
|
||
| if ($deployment.ProvisioningState -eq 'Succeeded') { | ||
| # New-AzResourceGroupDeployment would've written an error and stopped the pipeline by default anyway. | ||
| Write-Verbose "Successfully deployed template '$($templateFile.jsonFilePath)' to resource group '$($resourceGroup.ResourceGroupName)'" | ||
| } | ||
| Write-Host "Deployment '$($deployment.DeploymentName)' has CorrelationId '$($deployment.CorrelationId)'" | ||
| Write-Host "Successfully deployed template '$($templateFile.jsonFilePath)' to resource group '$($resourceGroup.ResourceGroupName)'" | ||
|
|
||
| $deploymentOutputs = SetDeploymentOutputs $serviceName $context $deployment $templateFile | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the old code it would seem as though this might throw exceptions if it failed. Do you know if that is true or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it could throw or return an error-
try...catch...finallyin powershell works for both. The idea was to reset the$DebugPreference, which we don't need anymore. Why it kept going I don't know - might've been an oversight after all the changes in this area over the years.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does throw still, which gets caught by the
Retryfunction handler instead. I was confused at first as well (though it was my old code so...)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess Retry only exits with a Write-Error, so perhaps we should actually re-throw in case the ErrorActionPreference is different for local users?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That could work.
There are some ill-behaved cmdlets like this that actual throw instead of write error, including the first-party
Invoke-WebRequest(which just bit me today; I had no idea it wasn't...proper).