onprem - Setup publishing job after push to stage #4616
Conversation
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
Reviewer's GuideThis PR introduces a new GitHub Actions workflow to generate dynamic image tags based on git refs and automate building and publishing the Koku UI OnPrem container images to Quay.io when changes are pushed to the main branch or onprem-* tags. Flow diagram for dynamic image tag generation and publishingflowchart TD
A["Push to main branch or onprem-* tag"] --> B["generate-tags job"]
B -->|"If ref is tag"| C["Set image tag to release tag (without 'v' prefix)"]
B -->|"If ref is not tag"| D["Set image tags to latest-SHA, latest, and version (without 'v' prefix)"]
C & D --> E["publish-onprem-ui job"]
E --> F["Build container image"]
F --> G["Push image to Quay.io"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The shell conditional for github.ref_type inside the run block won’t work as written—move that check to a step-level “if: github.ref_type == 'tag'” or use a proper shell test on GITHUB_REF.
- You’re filtering tags with onprem-* but then only stripping a leading “v”; make sure your version extraction logic matches your actual tag naming convention or adjust the prefix removal accordingly.
- Consider adding a concurrency group to the workflow or job to prevent overlapping builds when multiple commits or tags land on main in quick succession.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The shell conditional for github.ref_type inside the run block won’t work as written—move that check to a step-level “if: github.ref_type == 'tag'” or use a proper shell test on GITHUB_REF.
- You’re filtering tags with onprem-* but then only stripping a leading “v”; make sure your version extraction logic matches your actual tag naming convention or adjust the prefix removal accordingly.
- Consider adding a concurrency group to the workflow or job to prevent overlapping builds when multiple commits or tags land on main in quick succession.
## Individual Comments
### Comment 1
<location> `.github/workflows/koku-ui-onprem-push-main.yml:36-41` </location>
<code_context>
+ echo "image_tags=${image_tags[@]}"
+
+ else
+ version=$(git describe --long --tags --exclude latest)
+ version=${version#v} # remove the leading v prefix for version
+ # The images tags are taken from git
+ image_tags=( latest-${GITHUB_SHA} latest ${version} )
+ echo "image_tags=${image_tags[@]}" >> $GITHUB_OUTPUT
+ echo "image_tags=${image_tags[@]}"
</code_context>
<issue_to_address>
**suggestion:** Potential for 'git describe' to fail if no tags are present.
Add error handling or a fallback value for cases where 'git describe' fails due to missing tags to prevent workflow errors.
```suggestion
# Try to get version from git describe, fallback to untagged-SHA if no tags exist
if version=$(git describe --long --tags --exclude latest 2>/dev/null); then
version=${version#v} # remove the leading v prefix for version
else
version="untagged-${GITHUB_SHA}"
fi
# The images tags are taken from git
image_tags=( latest-${GITHUB_SHA} latest ${version} )
echo "image_tags=${image_tags[@]}" >> $GITHUB_OUTPUT
echo "image_tags=${image_tags[@]}"
```
</issue_to_address>
### Comment 2
<location> `.github/workflows/koku-ui-onprem-push-main.yml:73` </location>
<code_context>
+ with:
+ image: ${{ steps.build.outputs.image }}
+ tags: ${{ needs.generate-tags.outputs.image_tags }}
+ registry: ${{ env.QUAY_ORG }}
+ username: ${{ secrets.QUAY_INSIGHTS_ONPREM_ROBOT_USERNAME }}
+ password: ${{ secrets.QUAY_INSIGHTS_ONPREM_ROBOT_PASSWORD }}
</code_context>
<issue_to_address>
**issue (bug_risk):** Registry variable includes trailing slash, which may cause issues with some tools.
Ensure that QUAY_ORG does not include a trailing slash to avoid compatibility issues with registry tools during the push-to-registry action.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
rawagner
force-pushed
the
onprem_quay
branch
from
032f3cd to
eed5bd5
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4616 +/- ##
=======================================
Coverage 87.07% 87.07%
=======================================
Files 480 480
Lines 9078 9078
Branches 2198 2195 -3
=======================================
Hits 7905 7905
Misses 1107 1107
Partials 66 66 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
rawagner
force-pushed
the
onprem_quay
branch
from
eed5bd5 to
37d8aba
Compare
| on: | ||
| push: | ||
| branches: | ||
| - main |
There was a problem hiding this comment.
Would it be possible to run this for a branch other than main (e.g., stage-onprem)?
I modified our Konflux builds to run only when pushing to stage-hccm and prod-hccm, for example. The CI workflow still builds and tests all apps, but quay images are not created for unrelated changes. I imagine build times would be shorter, too.
There was a problem hiding this comment.
onprem generally doesnt have any stage environment. You can either run the latest (IOW main), or from a release branch.
Having a stage branch is a big hassle -> you need to closely follow all changes to common libs/other places that may affect you and do the backports. Unless we have a very good reason for stage - and in onprem we don't - i'd like to avoid it.
Im not sure how build times would get shorter. This build & push job runs after push to main branch, it should not block/slow down anything .. ?
There was a problem hiding this comment.
I probably misunderstood how this works. Does this workflow run for every push to main or is it triggered with a tag? If it's by tag only, then no worries. Seems like it's both, tho?
Our stage and prod branches are simply used to generate quay images. (We also need the unique SHA as a ref when deploying to app-interface.) There's no back porting here, no merge conflicts, etc. -- we're just merging the latest copy of main. However, by pushing to these branches, it triggers Konflux to generate quay images.
A branch is an extra step; however, changes to koku-ui-hccm probably shouldn't automatically deploy koku-ui-onprem and vice versa.
There was a problem hiding this comment.
You understood it correctly, we would build & push on every push to main (which would result to quay.io/insights-onprem/koku-ui-onprem:latest), and on every tag (quay.io/insights-onprem/koku-ui-onprem:<tag>).
I'd honestly prefer to have more quay images, than doing a manual merge to stage branch - to get latest builds to QE faster & not require merge to stage branch.
We do not have any issue with things slowing down right now - it seems like we are doing a premature optimization here and just pushing more work on us.
If we notice there is an issue this approach, i'd be happy to change it.
However, if you still feel we need to use the stage branch here from the beginning, i'll change it. Just wanted to lay my case here :)
There was a problem hiding this comment.
I've updated the job definition - it does not run on push to main if /apps/<saas_app> changed, for other changes it does. I didnt try this approach yet, so unsure how well it will work, but I guess this could be worth trying.
There was a problem hiding this comment.
Hi. I spoke with the team and the consensus was "no crossover". Cost/ROS updates (libs, dependencies, etc.) shouldn't automatically publish onprem. Although, we can revisit the onprem releases later.
There was a problem hiding this comment.
Okay. Thanks for following up on this.
I've updated the branch trigger to stage-onprem (and also tag)
rawagner
force-pushed
the
onprem_quay
branch
from
cefe8b6 to
15b6941
Compare
dlabrecq
force-pushed
the
main
branch
6 times, most recently
from
04368d9 to
4e35c2c
Compare
rawagner
force-pushed
the
onprem_quay
branch
from
15b6941 to
24cd310
Compare
rawagner
changed the title
rawagner
force-pushed
the
onprem_quay
branch
from
24cd310 to
34fc15c
Compare
rawagner
force-pushed
the
onprem_quay
branch
from
34fc15c to
0b4c7d2
Compare
2 participants
Summary by Sourcery
Add a GitHub Actions workflow that generates versioned tags and builds plus pushes the Koku UI OnPrem container image to Quay.io when changes are pushed to the main branch or onprem-* tags.
CI: