-
Notifications
You must be signed in to change notification settings - Fork 53
Image validation workflow #297
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: ruokun-niu <[email protected]>
Signed-off-by: ruokun-niu <[email protected]>
Signed-off-by: ruokun-niu <[email protected]>
Signed-off-by: ruokun-niu <[email protected]>
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.
Approving with suggestions.
echo "" | ||
echo "✅ SUCCESS: All images meet their architecture requirements for version $IMAGE_TAG" | ||
|
||
- name: Validate azure-linux variant architecture support |
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.
Would a matrix strategy be better to reduce the workflow file size?
Something like:
multi-arch-validation:
...
strategy:
matrix:
variant: ["", "-azure-linux"]
steps:
- name: Validate ${{ matrix.variant || 'default' }} variant architecture support
run: |
IMAGE_TAG="${{ needs.get-version.outputs.image_tag }}${{ matrix.variant }}"
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.
Good point. Will update this
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.
Can it make use of the existing matrix?
|
||
for image in $DRASI_IMAGES; do | ||
# Skip reaction-dataverse unless we're on x86-64 runner | ||
if echo "$AMD64_ONLY_IMAGES" | grep -q "$image" && [[ "${{ matrix.runner }}" != "oracle-vm-8cpu-32gb-x86-64" ]]; then |
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.
Should we check like [[ "${{ matrix.arch }}" == "arm64" ]]
instead of the runner name?
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.
Good suggestion
a0af0e5
to
28e49a5
Compare
echo "📋 Using image tag: ${{ needs.get-version.outputs.image_tag }}-azure-linux" | ||
|
||
# Images that only need amd64 support | ||
AMD64_ONLY_IMAGES="ghcr.io/drasi-project/reaction-dataverse" |
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.
This could come from the matrix too
Signed-off-by: ruokun-niu <[email protected]>
28e49a5
to
8892617
Compare
- arch: amd64 | ||
runner: oracle-vm-8cpu-32gb-x86-64 | ||
variant: "" | ||
amd64_only_images: "ghcr.io/drasi-project/reaction-dataverse" |
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.
Why are we only doing an amd64 build for dataverse?
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 Dataverse reaction is only built with amd64 architecture in the release workflow. IIRC, we had a dependency that would only work when we build the image in amd architecture.
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.
Looking at the dependencies, there is nothing that stands out as being an issue. Can we confirm this?
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 let me check again
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.
I've added the arm64 variant of the dataverse reaction to both the release workflow and the image validation workflow
Signed-off-by: ruokun-niu <[email protected]>
Signed-off-by: ruokun-niu <[email protected]>
This pull request introduces a new image validation workflow into the release process to ensure that all Drasi container images meet architecture requirements before a release is finalized. The workflow checks for the presence of required architectures (amd64 and arm64) for each image, tests pulling images on real hardware runners, and integrates these checks into the existing release pipeline.
Release process improvements:
validate-images
job to the.github/workflows/draft-release.yml
workflow, which runs after manifest creation and before the final release, ensuring image validation is a required step for releases. [1] [2]Image validation workflow:
.github/workflows/image-validation.yml
, which:reaction-dataverse
only requires amd64).Fixes: #issue_number