-
Notifications
You must be signed in to change notification settings - Fork 234
konflux: push images to the quay.io/ramalama org after integration testing #1743
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's GuideThis PR extends the bats-integration Tekton pipeline by adding parameters for target registry and component skipping, refactors task references, enriches init-snapshot to detect sync status, and introduces a new push-snapshot task that conditionally pushes component images to quay.io/ramalama tagged by commit SHA. Sequence diagram for image push after successful integration testsequenceDiagram
participant Tekton as Tekton Pipeline
participant Init as init-snapshot Task
participant Test as test-vm-cmd Task
participant Push as push-snapshot Task
participant Quay as quay.io/ramalama
Tekton->>Init: Start pipeline (on push to main)
Init->>Test: Pass snapshot info, event-type, sync status
Test->>Push: On success, trigger push-snapshot
Push->>Quay: Push images (if event-type==push and sync==true)
Push-->>Tekton: Output TEST_OUTPUT result
Class diagram for new and updated Tekton tasksclassDiagram
class init-snapshot {
+event-type: result
+snapshot-type: result
+sync: result
+bats-image: result
+ramalama-image: result
+other component images: result
}
class push-snapshot {
+SNAPSHOT: param
+event-type: param
+sync: param
+repo-prefix: param
+skip-components: param
+TEST_OUTPUT: result
+push-to-registry: step
}
init-snapshot <|-- push-snapshot : uses results
class test-vm-cmd {
+Runs integration tests
}
init-snapshot --> test-vm-cmd : passes results
test-vm-cmd --> push-snapshot : triggers on success
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @mikebonnet, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances the bats-integration
pipeline by adding an automated image publishing step. Upon successful integration tests for main branch pushes, and provided all components are in sync, the built images are now automatically pushed to a designated quay.io
repository, ensuring a streamlined release process for validated artifacts.
Highlights
- Automated Image Pushing: Introduced a new
push-snapshot
Tekton Task and integrated it into thebats-integration
pipeline. This task is responsible for pushing successfully built images to a specifiedquay.io
organization. - Conditional Execution: The image pushing process is now conditional. Images will only be pushed if the pipeline run is triggered by a 'push' event (e.g., to the main branch) and if all components within the snapshot are built from the same Git commit (i.e., they are 'in sync').
- Pipeline Parameterization: The
bats-integration
pipeline now accepts new parameters:repo-prefix
(defaulting toquay.io/ramalama
) to define the destination registry, andskip-components
(defaulting tobats
) to specify components that should not be pushed. - Snapshot Metadata Enrichment: The
init-snapshot
task has been enhanced to extract and expose additional metadata about the snapshot, specifically thesnapshot-type
and async
flag indicating whether all components originate from the same Git revision.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @mikebonnet - I've reviewed your changes - here's some feedback:
- Both init-snapshot and push-snapshot tasks install jq at runtime via dnf—consider using a base image with jq pre-installed to reduce overhead and external dependencies.
- The push-snapshot task references skopeo:latest—pin a specific skopeo version to improve reproducibility and avoid unexpected breaking changes.
- The jq filter logic for skip-components is quite complex; consider simplifying it or adding inline comments/tests to ensure correct behavior when skip-components is empty or contains multiple entries.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Both init-snapshot and push-snapshot tasks install jq at runtime via dnf—consider using a base image with jq pre-installed to reduce overhead and external dependencies.
- The push-snapshot task references skopeo:latest—pin a specific skopeo version to improve reproducibility and avoid unexpected breaking changes.
- The jq filter logic for skip-components is quite complex; consider simplifying it or adding inline comments/tests to ensure correct behavior when skip-components is empty or contains multiple entries.
## Individual Comments
### Comment 1
<location> `.tekton/integration/pipelines/bats-integration.yaml:63` </location>
<code_context>
- - name: pathInRepo
- value: .tekton/integration/tasks/test-vm-cmd.yaml
+ name: test-vm-cmd
+ when:
+ - input: $(tasks.init.results.sync)
+ operator: in
+ values: ["true"]
+ - name: push
+ params:
</code_context>
<issue_to_address>
The 'when' condition on the test task may prevent execution in some scenarios.
If 'sync' is not exactly the string 'true', the test task will be skipped. Consider normalizing the value to ensure consistent behavior.
</issue_to_address>
### Comment 2
<location> `.tekton/integration/tasks/init-snapshot.yaml:59` </location>
<code_context>
echo
+ echo -n "$SNAPSHOT_TYPE" | tee "$RESULTS_SNAPSHOT_TYPE_PATH"
+ echo
+ jq -j '[.components[] | .source.git.revision] | unique | length == 1' <<< "$SNAPSHOT" | tee "$RESULTS_SYNC_PATH"
+ echo
component_image() {
</code_context>
<issue_to_address>
The jq command for sync detection may fail if components or source.git.revision are missing.
Add validation to ensure the required fields exist before running the jq command to prevent errors or incorrect results.
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
The code changes introduce the ability to load multiple signed images to a registry using a bulk load feature. The new tasks and pipeline modifications are well-thought-out. My review includes a few suggestions to enhance the robustness, maintainability, and efficiency of the new Tekton resources. Specifically, I'm recommending more declarative pipeline logic, adherence to image tagging best practices, and more resilient error handling in the image push script.
COPIES=0 | ||
while read SRC DEST; do | ||
log Copying "$SRC" to "$DEST" | ||
skopeo copy --all --multi-arch all --retry-times 5 "docker://$SRC" "docker://$DEST" | ||
(( COPIES+=1 )) | ||
done < <( | ||
jq -r --arg skip "$SKIP_COMPONENTS" --arg prefix "$REPO_PREFIX" ' | ||
.components[] | | ||
select(.name as $name | [$skip | split(" ").[] != $name] | all) | | ||
"\(.containerImage) \($prefix)/\(.name):\(.source.git.revision)" | ||
' <<< "$SNAPSHOT" | ||
) | ||
NOTE="$COPIES components successfully pushed, ${#SKIP[*]} components skipped" | ||
test_output SUCCESS "$COPIES" 0 0 "$NOTE" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The script will exit on the first skopeo copy
failure because of set -e
in the shebang (line 47). This prevents the script from attempting to copy subsequent images and from reporting the failure via the TEST_OUTPUT
result. The task will fail, but structured information about which images succeeded and which failed will be lost.
To make this task more robust, I recommend:
- Removing the
-e
flag from the shebang on line 47 (e.g.,#!/bin/bash -x
). - Modifying the loop to handle
skopeo copy
failures for each image individually. - Tracking successes and failures, and reporting a consolidated status at the end.
COPIES=0
FAILURES=0
FAILED_COMPONENTS=""
while read -r SRC DEST COMPONENT_NAME; do
log "Copying $SRC to $DEST"
if skopeo copy --all --multi-arch all --retry-times 5 "docker://$SRC" "docker://$DEST"; then
(( COPIES+=1 ))
else
log "ERROR: Failed to copy $SRC to $DEST"
(( FAILURES+=1 ))
FAILED_COMPONENTS="${FAILED_COMPONENTS} ${COMPONENT_NAME}"
fi
done < <(
jq -r --arg skip "$SKIP_COMPONENTS" --arg prefix "$REPO_PREFIX" '
.components[] |
select(.name as $name | [$skip | split(" ").[] != $name] | all) |
"\(.containerImage) \($prefix)/\(.name):\(.source.git.revision) \(.name)"
' <<< "$SNAPSHOT"
)
if [ "$FAILURES" -gt 0 ]; then
NOTE="$COPIES components successfully pushed. $FAILURES components failed to push:${FAILED_COMPONENTS}. ${#SKIP[*]} components skipped."
test_output FAILURE "$COPIES" "$FAILURES" 0 "$NOTE"
exit 1
else
NOTE="$COPIES components successfully pushed, ${#SKIP[*]} components skipped"
test_output SUCCESS "$COPIES" 0 0 "$NOTE"
fi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mikebonnet thoughts on this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, it's a good suggestion and I was working on a revised version that incorporates these suggestions. Just pushed it, take a look!
0659d60
to
9f76c46
Compare
…sting The "bats-integration" pipeline is now run on every pull request and push to the main branch. In the case of pushes, if the integration tests run successfully, the images are pushed to a repo matching their component name in the quay.io/ramalama org. Images will be tagged with the sha of the commit from which they were built. Signed-off-by: Mike Bonnet <[email protected]>
9f76c46
to
1a14b5c
Compare
LGTM |
The
bats-integration
pipeline is now run on every pull request and push to the main branch. In the case of pushes, if the integration tests run successfully, the images are pushed to a repo matching their component name in thequay.io/ramalama
org. Images will be tagged with the sha of the commit from which they were built.Summary by Sourcery
Extend the Tekton integration pipeline to push built component images to quay.io/ramalama after successful integration tests on push events
New Features:
Enhancements: