Skip to content
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

Merged
merged 9 commits into from
Dec 11, 2024

Conversation

dimitri-tenstorrent
Copy link
Contributor

@dimitri-tenstorrent dimitri-tenstorrent commented Dec 9, 2024

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

@dimitri-tenstorrent dimitri-tenstorrent marked this pull request as ready for review December 10, 2024 16:29
@dimitri-tenstorrent dimitri-tenstorrent requested a review from a team as a code owner December 10, 2024 16:29
@@ -9,6 +9,9 @@ inputs:
description: 'Docker image architecture'
required: false
default: tt-metalium/ubuntu-20.04-amd64
docker_image_tag:
Copy link
Contributor Author

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 :)

Copy link
Contributor

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

Copy link
Contributor

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 }}
Copy link
Contributor

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?

Copy link
Contributor Author

@dimitri-tenstorrent dimitri-tenstorrent Dec 10, 2024

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.

@dimitri-tenstorrent dimitri-tenstorrent merged commit 4d8eb70 into main Dec 11, 2024
9 checks passed
@dimitri-tenstorrent dimitri-tenstorrent deleted the dimitri/fix-package-workflow-tagging branch December 11, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants