-
Notifications
You must be signed in to change notification settings - Fork 86
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
Fix the docker image tag specified for smoke tests in Release Docker images workflow #15842
Fix the docker image tag specified for smoke tests in Release Docker images workflow #15842
Conversation
@@ -9,6 +9,9 @@ inputs: | |||
description: 'Docker image architecture' | |||
required: false | |||
default: tt-metalium/ubuntu-20.04-amd64 | |||
docker_image_tag: |
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.
@ttmchiou You will find this interesting :)
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.
We have similar logic in generate-docker-tag.
Can we either put this docker_image_tag into there or combine the logic so that it's all in one step with one arg?
Change seems to make this more wordy then it needs to be
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.
You can add an argument into generate-docker-tag to specify a tag i think as its set to either use 'latest' or branch name in an if statement. should be a matter of adding an extra if statement and feeding the argument in to docker-run
@@ -91,6 +91,7 @@ jobs: | |||
uses: ./.github/actions/docker-run | |||
with: | |||
docker_os_arch: tt-metalium-${{ matrix.os }}-amd64-release/${{ matrix.test_group.arch }} |
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.
do we still need docker_os_arch if we've added docker_verison?
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.
Yes, because it is a different image name. Not providing the parameter would select the image of tt-metalium/ubuntu-20.04-amd64
instead of tt-metalium-${{ matrix.os }}-amd64-release/${{ matrix.test_group.arch }}
that we need for release images.
In another issue, we should rename that parameter to be called something like docker_image_name
.
Ticket
Problem description
The smoke tests should mention not the latest tag but the tag of the version being built.
What's changed
Add {{inputs.version}} to the tag being used to run Smoke tests.
Also modified Docker Run Action to take a new parameter called
docker_image_tag
that allows to specify the image.Checklist
https://github.com/tenstorrent/tt-metal/actions/runs/12264928767
https://github.com/tenstorrent/tt-metal/actions/runs/12264932641