ci: ensure image is built and published once #1538
Conversation
📝 WalkthroughWalkthroughThe pull request introduces several modifications to the CI/CD workflow configurations across multiple YAML files. Key changes include the removal of the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
.github/workflows/ci-cd-yt01.yml (1)
Line range hint
1-143: Consider implementing robust image coordination mechanismWhile centralizing image building in the main workflow reduces redundancy, the distributed nature of deployments across different environments (staging, yt01) requires careful coordination. Consider these architectural improvements:
- Implement a shared status mechanism (e.g., using GitHub deployment status API) to track image availability
- Add explicit wait/retry logic in deployment workflows
- Consider using image tags or SHA digests instead of relying on version labels for more deterministic deployments
This would help prevent race conditions and ensure reliable deployments across environments.
.github/workflows/ci-cd-staging.yml (1)
Line range hint
1-152: Consider addressing the potential race conditionWhile the removal of redundant image builds is a good improvement, the PR objectives mention a potential race condition when staging/yt01 workflows are triggered by GitHub releases. This could be problematic if the build and publish process in the main workflow takes longer than expected.
Consider implementing one of these solutions:
- Add a wait step in staging/yt01 workflows that checks for image availability before proceeding with deployment
- Implement a semaphore or lock mechanism using GitHub environment protection rules
- Use artifact dependencies between workflows to ensure proper sequencing
Would you like me to provide a detailed implementation for any of these approaches?
.github/workflows/ci-cd-main.yml (2)
48-58: Well-structured separation of release and main publishingThe split into
publish-releaseandpublish-mainjobs effectively implements the single-build objective while maintaining clear separation of concerns. The mutually exclusive conditions ensure images are built only once.Consider adding a comment in the workflow file explaining the purpose of each publish job and their triggering conditions. This would help future maintainers understand the workflow's design.
+ # Triggered when a new release is created by release-please name: Build and publish release docker images + # Triggered for non-release changes to main branch name: Build and publish main docker imagesAlso applies to: 59-69
168-169: Address TODO for handling multiple publish statusesThe current implementation only reports the release publish status. This should be enhanced to handle both publish job statuses.
Would you like me to help implement a solution that properly handles both
publish-releaseandpublish-mainstatuses in the Slack notification? This could include:
- Adding separate status outputs for each publish job
- Implementing conditional logic to show the relevant status
Also applies to: 181-182
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
.github/workflows/ci-cd-main.yml(9 hunks).github/workflows/ci-cd-staging.yml(2 hunks).github/workflows/ci-cd-yt01.yml(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/ci-cd-main.yml
63-63: property "release-please" is not defined in object type {build-and-test: {outputs: {}; result: string}; check-for-changes: {outputs: {hasbackendchanges: string; hasinfrachanges: string; hasmigrationchanges: string; hasschemachanges: string; hasslacknotifierchanges: string; hastestchanges: string}; result: string}; generate-git-short-sha: {outputs: {gitshortsha: string}; result: string}; get-current-version: {outputs: {version: string}; result: string}}
(expression)
73-73: property "release-please" is not defined in object type {check-for-changes: {outputs: {hasbackendchanges: string; hasinfrachanges: string; hasmigrationchanges: string; hasschemachanges: string; hasslacknotifierchanges: string; hastestchanges: string}; result: string}; generate-git-short-sha: {outputs: {gitshortsha: string}; result: string}; get-current-version: {outputs: {version: string}; result: string}}
(expression)
99-99: property "release-please" is not defined in object type {check-for-changes: {outputs: {hasbackendchanges: string; hasinfrachanges: string; hasmigrationchanges: string; hasschemachanges: string; hasslacknotifierchanges: string; hastestchanges: string}; result: string}; deploy-infra: {outputs: {}; result: string}; generate-git-short-sha: {outputs: {gitshortsha: string}; result: string}; get-current-version: {outputs: {version: string}; result: string}; publish-main: {outputs: {}; result: string}}
(expression)
120-120: property "release-please" is not defined in object type {check-for-changes: {outputs: {hasbackendchanges: string; hasinfrachanges: string; hasmigrationchanges: string; hasschemachanges: string; hasslacknotifierchanges: string; hastestchanges: string}; result: string}}
(expression)
136-136: property "release-please" is not defined in object type {check-for-changes: {outputs: {hasbackendchanges: string; hasinfrachanges: string; hasmigrationchanges: string; hasschemachanges: string; hasslacknotifierchanges: string; hastestchanges: string}; result: string}; deploy-apps: {outputs: {deployment_executed: string}; result: string}; generate-git-short-sha: {outputs: {gitshortsha: string}; result: string}; get-current-version: {outputs: {version: string}; result: string}}
(expression)
146-146: property "release-please" is not defined in object type {check-for-changes: {outputs: {hasbackendchanges: string; hasinfrachanges: string; hasmigrationchanges: string; hasschemachanges: string; hasslacknotifierchanges: string; hastestchanges: string}; result: string}; deploy-apps: {outputs: {deployment_executed: string}; result: string}}
(expression)
🔇 Additional comments (6)
.github/workflows/ci-cd-yt01.yml (2)
133-133: LGTM: Failure notification dependencies updated correctly
The updated job dependencies correctly reflect the workflow changes, ensuring that failure notifications will trigger for all critical deployment and test steps.
66-66: Verify image availability before deployment
The removal of the publish job dependency aligns with the goal of building images once. However, this creates a potential race condition where the deployment might start before the image is available from the main workflow.
Consider these mitigations:
- Add a step to verify image existence before deployment
- Implement a retry mechanism in case the image isn't immediately available
✅ Verification successful
Image availability is handled correctly - no changes needed
The removal of the publish job dependency is safe because:
- The main workflow publishes images before any environment deployments start
- The deployment uses the same version tag that was published in the main workflow
- Azure Container Apps handles image pull retries automatically if the image isn't immediately available
The race condition concern is mitigated by the natural workflow sequence - main workflow publishes before environment-specific workflows run.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for related changes in other workflow files to ensure consistent image handling
# Check for image verification steps in the deploy-apps workflow
rg -A 10 "workflow-deploy-apps.yml" .github/workflows/
Length of output: 9526
Script:
#!/bin/bash
# Let's check the workflow-deploy-apps.yml file to see if it has any image verification steps
rg -l "workflow-deploy-apps.yml" .github/workflows/
# If found, let's examine its content for image verification
cat .github/workflows/workflow-deploy-apps.yml
Length of output: 12824
Script:
#!/bin/bash
# Let's check the main workflow to understand how the image is published and how it relates to the yt01 workflow
rg -A 10 "publish" .github/workflows/ci-cd-main.yml
# Also check if there's any image verification in the bicep templates
rg -l "image:" .azure/applications/
Length of output: 4338
.github/workflows/ci-cd-staging.yml (2)
66-66: LGTM! Dependencies correctly updated for deploy-apps job
The removal of the publish job dependency aligns with the PR objective of eliminating redundant image builds. This change maintains the essential dependencies while streamlining the workflow.
142-142: LGTM! Failure notification dependencies properly updated
The send-slack-message-on-failure job dependencies have been correctly updated to reflect the current workflow structure, ensuring proper failure notifications while removing the redundant publish job dependency.
.github/workflows/ci-cd-main.yml (2)
73-73: Consistent conditional execution based on release status
The conditions across deployment jobs are consistently implemented to run only when there's no release. This aligns with the workflow's objective.
Note: You can safely ignore the static analysis warnings about undefined "release-please" property. These are false positives as the release-please job and its outputs are properly defined in the workflow.
Also applies to: 99-99, 120-120, 136-136, 146-146
🧰 Tools
🪛 actionlint (1.7.4)
73-73: property "release-please" is not defined in object type {check-for-changes: {outputs: {hasbackendchanges: string; hasinfrachanges: string; hasmigrationchanges: string; hasschemachanges: string; hasslacknotifierchanges: string; hastestchanges: string}; result: string}; generate-git-short-sha: {outputs: {gitshortsha: string}; result: string}; get-current-version: {outputs: {version: string}; result: string}}
(expression)
21-22: LGTM: Release output addition enables conditional workflows
The addition of release_created output is well-implemented and crucial for controlling downstream jobs.
Let's verify the release-please action's output usage:
✅ Verification successful
✅ Release output is correctly implemented and consistently used
The release_created output from the release-please job is properly implemented and consistently used throughout the workflow to control job execution:
- Used to trigger release-specific jobs when
true - Controls main branch deployments when
false - Properly referenced in all conditional statements using the correct syntax
needs.release-please.outputs.release_created
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify release-please action output usage
# Check if the release_created output is consistently used across the workflow
rg -l "release-please.outputs.release_created" .github/workflows/
Length of output: 98
Script:
#!/bin/bash
# Let's check how the release_created output is used in the workflow
rg -A 2 "release-please.outputs.release_created" .github/workflows/ci-cd-main.yml
Length of output: 1931
Description
Ensure that the image is build and published once per version. So when the release PR is merged into
main, and release please notices that it should create a release, it builds and publishes the image and that's it. Removed the build and publish step in the staging/yt01 workflows.There might be a race-condition if the build/publish step takes a long time, and the trigger for staging and yt01 workflows are releases that have been created in github. So an improvement would be looking into a solution for that. Using some other trigger..
Related Issue(s)
Verification
Documentation
docs-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)Summary by CodeRabbit
New Features
publish-mainto enhance deployment logic.release_createdto improve release tracking.Bug Fixes
publishjob, simplifying job dependencies.Documentation