fix: Avm post deployment - #673
Open
Prachig-Microsoft wants to merge 5 commits into
Open
Prachig-Microsoft wants to merge 5 commits into
Prachig-Microsoft wants to merge 5 commits into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
- Add -MaxApiRetries (default 20) and -ApiRetryIntervalSeconds (default 15) parameters, giving a 5-minute default wait budget instead of 2.5 minutes. This accounts for the container app pulling a freshly-built image right after acr_build_push.ps1 and passing its startup probe before schema registration is attempted. - On readiness failure, print the last HTTP/connection error plus diagnostics (az containerapp revision list status/replicas and recent az containerapp logs show console output) instead of silently skipping, making failures actionable. - Build the schema-upload multipart body as raw bytes (MemoryStream) instead of round-tripping file bytes through UTF8.GetString, avoiding potential corruption of non-ASCII schema content. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds pre-flight checks that detect and auto-correct known AVM deployment issues before running schema registration, so the script works reliably against AVM-deployed resource groups (deployed via the bicep-registry ContentProcessingAVM fork) without requiring a .env file: - Storage account publicNetworkAccess: detects Disabled with 0 private endpoints on Non-WAF deployments and re-enables it. - Cosmos DB publicNetworkAccess: same self-healing check for Cosmos DB (real bug in the published avm/res/document-db/database-account module). - Web container app ingressTargetPort: detects when it defaulted to 80 instead of 3000 and corrects it. - API Easy Auth: detects when anonymous schema registration calls are blocked and handles it. - AI Services multi-account auto-selection when more than one aicu-* account exists in the resource group. Also updated the script to accept -ResourceGroupName as a parameter instead of requiring a .env file, since that workflow is needed for AVM registry deployments. Verified end-to-end against live resource groups (pgcp4, pgcp11) deployed via the AVM fork: all pre-flight checks correctly no-op when resources are already healthy, and correctly self-heal when the known AVM bugs are present. Schema and schema set registration confirmed working after each fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
docs/AVMPostDeploymentGuide.md was out of date - it described registering schemas via a standalone Python script (register_schema.py) with no ACR build/push step at all. Updated it to reflect the actual required sequence after deploying via the AVM bicep registry module: 1. Build and push container images (infra/scripts/acr_build_push.ps1 "<resource-group>") - the AVM module provisions ACR/Container Apps but does not build or push application images itself. 2. Run infra/scripts/post_deployment.ps1 -ResourceGroupName "<resource-group>" [-ApiBaseUrl "<url>"] to register schemas and create the schema set (replaces the old Python script; ApiBaseUrl is optional thanks to auto-discovery). 3. Configure authentication (unchanged). Also documented the AVM-specific self-healing pre-flight checks the script now runs (storage/Cosmos DB network access, web ingress port, API auth) so users understand what they do. Updated prerequisites to drop the Python/pip requirement (no longer used) and added PowerShell as a requirement instead. Added a short pointer in README.md's Getting Started section so users who deploy via the AVM registry module (rather than azd up) know to follow the AVM Post Deployment Guide instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… flow" This reverts commit 321c4bd.
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the post_deployment.ps1 script to support AVM-style deployments by allowing the resource group (and related values) to be provided as parameters, while still supporting existing azd env-based deployments.
Changes:
- Adds parameterized AVM deployment mode (resource group, optional subscription, optional API base URL, optional AI Services account).
- Improves post-deployment robustness with pre-flight checks (network access, ingress port, auth), API readiness polling, and richer diagnostics.
- Refactors schema registration multipart upload to send raw bytes (avoids string re-encoding/corruption).
infra/scripts/post_deployment.ps1:92
- az containerapp list ... | ConvertFrom-Json will throw if the Azure CLI command fails or returns an empty string (stderr is suppressed via 2>$null). That prevents the later $ContainerApps.Count -eq 0 check from running and hides the underlying az error.
# Discover container apps in the resource group
Write-Host "[Info] Discovering container apps in resource group..."
$ContainerApps = @(az containerapp list -g $RESOURCE_GROUP --query "[].{name:name, fqdn:properties.configuration.ingress.fqdn}" -o json 2>$null | ConvertFrom-Json)
infra/scripts/post_deployment.ps1:145
- In AZD mode $ApiBaseUrl is constructed even if CONTAINER_API_APP_FQDN is missing/empty in the azd environment, which will result in requests to https:///schemavault/ and a confusing readiness loop. It’s better to fail fast with a clear error when the env value is not set.
$SUBSCRIPTION_ID = azd env get-value AZURE_SUBSCRIPTION_ID
$RESOURCE_GROUP = azd env get-value AZURE_RESOURCE_GROUP
$ApiBaseUrl = "https://$CONTAINER_API_APP_FQDN"
}
infra/scripts/post_deployment.ps1:320
- The script temporarily relaxes the API container app auth setting, but restoration only happens in the two explicit call sites. If the script terminates early due to an unhandled exception (e.g., schema manifest parse error, missing schema_info.json, Ctrl+C), the original auth setting may not be restored. Consider guaranteeing cleanup via a top-level try { ... } finally { Restore-ApiAuthSetting } or a trap/exit handler once $ApiAuthOriginalAction is set.
if ($AuthAction -and $AuthAction -ne 'AllowAnonymous') {
Write-Host " [Warn] API container app has authentication enabled (unauthenticatedClientAction=$AuthAction)."
Write-Host " Temporarily allowing anonymous access for schema registration; original setting will be restored afterwards..."
az containerapp auth update -g $RESOURCE_GROUP -n $CONTAINER_API_APP_NAME --unauthenticated-client-action AllowAnonymous -o none 2>$null
$ApiAuthOriginalAction = $AuthAction
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
Comment thread
infra/scripts/post_deployment.ps1
Comment on lines
+83
to
+88
| } else { | ||
| $SUBSCRIPTION_ID = $SubscriptionId | ||
| } | ||
|
|
||
| $RESOURCE_GROUP = $ResourceGroupName | ||
|
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reviewers
At least 1 approving review is required to merge this pull request.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This PR fixed post deployment script to pass RG as paramter for AVM deployment
Does this introduce a breaking change?
Golden Path Validation
Deployment Validation
What to Check
Verify that the following are valid
Other Information