Use the same canary rollout script for both appsvc and containerapp #975
Conversation
🦋 Changeset detectedLatest commit: 28de110 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (44.12%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #975 +/- ##
===========================================
- Coverage 67.85% 44.12% -23.74%
===========================================
Files 31 40 +9
Lines 840 1609 +769
Branches 146 184 +38
===========================================
+ Hits 570 710 +140
- Misses 258 885 +627
- Partials 12 14 +2 🚀 New features to boost your workflow:
|
lucacavallaro
force-pushed
the
feats/unify-incremental-rollout-apis
branch
2 times, most recently
from
05aaf0f to
2015895
Compare
|
|
||
| - name: Swap Staging and Production Slots | ||
| uses: pagopa/dx/actions/swap-appsvc-slot@main | ||
| uses: pagopa/dx/actions/incremental-rollout@feats/unify-incremental-rollout-apis |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
lucacavallaro
force-pushed
the
feats/unify-incremental-rollout-apis
branch
from
2015895 to
28de110
Compare
lucacavallaro
changed the title
| echo "ca_id=$container_app_id" >> $GITHUB_OUTPUT | ||
| - name: Check for New Revision Errors and Increment Traffic | ||
| uses: pagopa/dx/actions/incremental-rollout@feats/unify-incremental-rollout-apis |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
lucacavallaro
changed the title
lucacavallaro
changed the title
|
|
||
| - name: Swap Staging and Production Slots | ||
| uses: pagopa/dx/actions/swap-appsvc-slot@main | ||
| uses: pagopa/dx/actions/incremental-rollout@feats/unify-incremental-rollout-apis |
There was a problem hiding this comment.
| uses: pagopa/dx/actions/incremental-rollout@feats/unify-incremental-rollout-apis | |
| uses: pagopa/dx/actions/incremental-rollout@main |
reminder: return to main before merge
| echo "ca_id=$container_app_id" >> $GITHUB_OUTPUT | ||
| - name: Check for New Revision Errors and Increment Traffic | ||
| uses: pagopa/dx/actions/incremental-rollout@feats/unify-incremental-rollout-apis |
There was a problem hiding this comment.
| uses: pagopa/dx/actions/incremental-rollout@feats/unify-incremental-rollout-apis | |
| uses: pagopa/dx/actions/incremental-rollout@main |
reminder: return to main before merge
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the incremental rollout action to support both Azure App Service and Container App deployments by introducing a unified interface with resource-type-specific implementations.
Key changes:
- Generalized the rollout script to support multiple Azure resource types through a plugin-like architecture
- Added support for Container App deployments alongside existing App Service functionality
- Renamed the action from "swap-appsvc-slot" to "incremental-rollout" to reflect its broader scope
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| actions/incremental-rollout/rollout.sh | Core script refactored to use resource-type-specific implementations via sourced files |
| actions/incremental-rollout/azure-appsvc.sh | New file containing App Service-specific traffic management functions |
| actions/incremental-rollout/azure-containerapp.sh | New file containing Container App-specific traffic management functions |
| actions/incremental-rollout/action.yml | Updated action interface to accept resource type parameter |
| actions/incremental-rollout/package.json | Renamed package and updated lint script to check all shell files |
| actions/incremental-rollout/README.md | Updated documentation to reflect new resource type parameter and action name |
| actions/incremental-rollout/CHANGELOG.md | Updated title to match renamed action |
| .github/workflows/release-azure-containerapp-v1.yaml | Replaced inline rollout logic with the unified incremental-rollout action |
| .github/workflows/release-azure-appsvc-v1.yaml | Updated to use renamed action with resource type parameter |
| .changeset/chubby-emus-happen.md | Added changeset documenting the rename and Container App support |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| az containerapp ingress traffic set \ | ||
| --resource-group "$resource_group_name" \ | ||
| --name "$containerapp_name" \ | ||
| --revision-weight "latest=0" |
There was a problem hiding this comment.
The __revert_traffic function sets traffic to latest=0 but should set traffic to 100% for the production revision. This will result in no traffic being routed to any revision, breaking the revert functionality.
| az containerapp ingress traffic set \ | |
| --resource-group "$resource_group_name" \ | |
| --name "$containerapp_name" \ | |
| --revision-weight "latest=0" | |
| # Get the current production revision (the one with traffic weight > 0) | |
| local prod_revision=$(az containerapp revision list \ | |
| --resource-group "$resource_group_name" \ | |
| --name "$containerapp_name" \ | |
| --query "[?trafficWeight>0].name" \ | |
| --output tsv) | |
| # Set 100% traffic to the production revision | |
| az containerapp ingress traffic set \ | |
| --resource-group "$resource_group_name" \ | |
| --name "$containerapp_name" \ | |
| --revision-weight "${prod_revision}=100" |
Copilot uses AI. Check for mistakes.
| } | ||
|
|
||
| __swap_versions() { | ||
| __revert_traffic "$1" "$2" |
There was a problem hiding this comment.
The __swap_versions function calls __revert_traffic which sets traffic to 0% instead of swapping to the new revision. This should set traffic to 100% for the latest revision to complete the rollout.
| __revert_traffic "$1" "$2" | |
| __set_traffic "$1" "$2" "100" |
Copilot uses AI. Check for mistakes.
|
|
||
| - name: Swap Staging and Production Slots | ||
| uses: pagopa/dx/actions/swap-appsvc-slot@main | ||
| uses: pagopa/dx/actions/incremental-rollout@feats/unify-incremental-rollout-apis |
There was a problem hiding this comment.
Using a feature branch reference in the workflow. This should be updated to use a stable reference like @main or a specific version tag before merging to production.
| uses: pagopa/dx/actions/incremental-rollout@feats/unify-incremental-rollout-apis | |
| uses: pagopa/dx/actions/incremental-rollout@main |
Copilot uses AI. Check for mistakes.
| "incremental-rollout": minor | ||
| --- | ||
|
|
||
| Rename action, add support to containerapp |
There was a problem hiding this comment.
it looks like a major since you're changing the input web_app_name to resource_name
5 participants
Closes CES-1274