MAINT: Decouple CoPyRIT code and infrastructure deployments - #2660
Adrian Gavrila (adrian-gavrila) wants to merge 4 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Behnam (behnam-o)
left a comment
There was a problem hiding this comment.
A few comments on keeping the application deployment path uniform.
| pool: | ||
| vmImage: 'ubuntu-latest' | ||
| jobs: | ||
| - deployment: DeployInfrastructure |
There was a problem hiding this comment.
Could we keep one deployment script that always deploys the application, with infrastructure reconciliation controlled by a parameter? That would give us only two workflows: app, or infra + app.
| immutable_image="$PYRIT_ACR_LOGIN_SERVER/$PYRIT_IMAGE_NAME@$digest" | ||
| echo "##vso[task.setvariable variable=immutableImage;isOutput=true]$immutable_image" | ||
|
|
||
| - ${{ if eq(parameters.deployInfra, true) }}: |
There was a problem hiding this comment.
Could deployInfra control a runtime condition instead of conditionally adding stages? Keeping the stages present but skipped gives every ADO run the same graph.
| print(f"Previous image (not automatically restored): {previous_image}", flush=True) | ||
| if previous_image != image: | ||
| _az( | ||
| "containerapp", |
There was a problem hiding this comment.
The Container App is currently managed by both main.bicep and deploy_code.py. Could we retain one deployment path so app deployment behaves identically whether infrastructure reconciliation is enabled or skipped?
| raise RuntimeError("Application health check failed; networking was not changed") | ||
|
|
||
|
|
||
| def deploy_code(*, slot: str, resource_group: str, app_name: str, acr_resource_id: str, image: str) -> None: |
There was a problem hiding this comment.
Could the app deployment remain in the shell workflow instead of introducing deploy_code.py? Much of its validation duplicates assumptions already enforced by the existing deployment script and Bicep.
| exit 1 | ||
| fi | ||
| immutable_image="$registry_server/$repository@$digest" | ||
| echo "Infrastructure-only deployment; retaining current image: $immutable_image" |
There was a problem hiding this comment.
If deploy_public_nat.sh becomes the unified deployment entry point, could we rename it? It now handles Private Link, Front Door, infrastructure reconciliation, application deployment, and health verification, so “public NAT” no longer describes its responsibility. Something like deploy_gui.sh or deploy_environment.sh would be clearer.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Default to code-only test deployment. Add optional infrastructure stages while preserving production opt-in and manual approval. Includes documentation and 27 passing targeted tests. Live code-only deployment validation is pending.
This adds a new checkbox to deploy infra alongside the prod deployment toggle; this defaults to off.