Revamp Dockerfiles #17403

blozano-tt · 2025-01-30T22:38:28Z

Problem(s) description

We have a dockerfile for every version of ubuntu, when we could have one
The way that customer sets up a baremetal machine is completely decoupled from CI, no verification of their flow.
Our Release Docker image is 6.5GB
Our CI Docker container has things it would never need, like gdb and vim, and we are pulling this many times.
We are not making use of multi-stage docker facilities
Changing one apt dependency incurs the cost of rebuilding gdb

What's changed

Introduce a single Dockerfile with a base, ci-build, ci-test, dev, release image definitions.
- In a subsequent PR we can make use of ci-build for build machines, and ci-test for test machines.
Remove unnecessary files used to augment the Dockerfile definition (requirements)
I added ClangBuildAnalyzer
I added several CPM dependencies, so we can prototype building with system deps.

Possible in future work

It will be possible to make a much smaller release image in future work
However there is work to be done to make sure that the smoke test works with the release image.

$ docker images | grep release 
ghcr.io/tenstorrent/tt-metal/tt-metalium-ubuntu-20.04-amd64-release   dev-blozano-docker   adfcfcb6d262   18 minutes ago   2.5GB
ghcr.io/tenstorrent/tt-metal/tt-metalium-ubuntu-20.04-amd64-release   latest-rc            a41a7472f8c6   23 hours ago     7.65GB

Checklist

dockerfile/Dockerfile

into blozano-docker

graphviz is needed by some test for “dot”

dockerfile/Dockerfile

.github/workflows/publish-release-image.yaml

dimitri-tenstorrent · 2025-02-05T17:21:02Z

dockerfile/Dockerfile

+
+#############################################################
+
+FROM ci-test AS dev


Are we going to trim down the runtime dependencies later after the basis is merged?

yes, I'll file a ticket for that.

dimitri-tenstorrent · 2025-02-05T17:23:26Z

dockerfile/Dockerfile

+#############################################################
+
+FROM dev AS release
+


There might be an issue with the release images as you merge @afuller-TT 's latest PR. See this line: https://github.com/tenstorrent/tt-metal/pull/17516/files#diff-6e6cd4043a79554fa4a311541ed0418aebae47985936fecc38c5a15ec0adb32bR1

I do not have a good idea of how to support that workflow with the stages outlined.

FROM $BASE_IMAGE as release?

I don't think we need the build argument anymore.

The release image is now based on an earlier layer, instead of a pushed tag.

We'll be losing out on the efficiency of re-using a previously built stage, unless we have dumb luck on the build machine.

I'm okay with this as a stepping stone. But certainly there's room for improvement. I don't think it's detrimental; just inefficient (compute and storage).

.github/workflows/all-static-checks.yaml

afuller-TT · 2025-02-05T17:42:44Z

scripts/docker/run_docker_cmd.sh

Any idea where this was used? I only see it referenced in egg-info.

dockerfile/Dockerfile

afuller-TT · 2025-02-05T18:25:58Z

dockerfile/Dockerfile

+    jq \
+    pandoc \
+    pkg-config \
+    python3-dev \


Aren't some of these hard requirements for building even outside of CI?

I put all build dependencies in ci-build layer.

runtime layer is just for running with precompiled binaries.

I thought the build dependencies went in install_dependencies.sh? Since we're calling it on line 20 asking for the build dependencies.

So if one adds a new build dependency, does it go here in the Dockerfile? Or in the install_dependencies.sh script?

You raise a really good point.
My thinking was install_dependencies.sh should manage all build dependencies, and this section should be anything we need in CI for our workflows ... for instance, we do some sudo rm in workflows, so that should go here. We need jq for some parsing? add it here.... clang-tidy ... add it here ...

Cargo ... is only needed for tt-train I think ... so I have mixed emotions about it

graphviz is only needed to run some pytests ...

I am not ready to put these in install dependencies as they aren't used as of yet.

libfmt-dev
libyaml-cpp-dev
pybind11-dev
nlohmann-json3-dev
libgtest-dev
libboost-all-dev \

I moved git, pkg-config, and python3-dev to install_dependencies.sh

afuller-TT

I like it ❤️
I'll review again after latest main is merged in.

dockerfile/Dockerfile

blozano-tt · 2025-02-05T19:45:50Z

dockerfile/Dockerfile

+    libtbb-dev \
+    libcapstone-dev \


these two are TRACY "requirements" ... a customer would never build with tracy? I'm thinking keep them here.
@mo-tenstorrent

Customers definitely build with tracy. This is a tool for anyone developing on our cards. I would keep them in any docker release we have to support profiler builds.

.github/workflows/build-docker-artifact.yaml

afuller-TT

Let's roll!

I know this doesn't USE these slimmed-down Docker images, but we can iterate after this lands.

blozano-tt added 17 commits January 30, 2025 21:13

Update install_dependencies.sh for usage in docker

7354d84

Add base image

55866c9

Add libnuma

78a3c32

Progress

427d091

Get ci image building successfully

95b3526

WIP save

960f312

Progress

9444983

Add unified Dockerfile

89353b7

Remove files

8d3ca2b

Remove the thing needed to build gdb

cdd61af

Add common requirements

70d3d23

cleanup requirements

8b06e79

Fix something

ed77b73

Remove duplicate

90c4c25

Use new docker image in CI

dee7606

Add some deps

a207d9d

Fix issue

7d26a88

blozano-tt commented Feb 1, 2025

View reviewed changes

dockerfile/Dockerfile Outdated Show resolved Hide resolved

blozano-tt added 12 commits January 31, 2025 22:54

Update dockerfile/Dockerfile

3083aeb

We now use sudo in CI

8c456fb

Merge branch 'blozano-docker' of https://github.com/tenstorrent/tt-metal

152b3d4

into blozano-docker

Update Dockerfile

43f1bbe

Update requirements_ci.txt

6e26fad

graphviz is needed by some test for “dot”

Merge branch 'main' into blozano-docker

135220f

Fix clang-tidy

0dc36e2

Dunno

ea677e6

Dunno

e8af405

Use dev packages in runtime

8340ebf

Update install_dependencies.sh

d8062ba

Try eliminating cache

bc619af

blozano-tt added this to the Metal CI to Green milestone Feb 4, 2025

blozano-tt added 6 commits February 4, 2025 13:50

Merge branch 'main' into blozano-docker

2380ae9

Try to fix release image issue

7959a1e

Oops

c4d7e1a

Update publish-release-image.yaml

1a82050

Update publish-release-image.yaml

a9a6212

Update Dockerfile

9672568

dimitri-tenstorrent reviewed Feb 5, 2025

View reviewed changes

dockerfile/Dockerfile Show resolved Hide resolved

dimitri-tenstorrent reviewed Feb 5, 2025

View reviewed changes

.github/workflows/publish-release-image.yaml Show resolved Hide resolved

dimitri-tenstorrent reviewed Feb 5, 2025

View reviewed changes

.github/workflows/all-static-checks.yaml Show resolved Hide resolved

afuller-TT reviewed Feb 5, 2025

View reviewed changes

scripts/docker/run_docker_cmd.sh Outdated

Copy link

Contributor

afuller-TT Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any idea where this was used? I only see it referenced in egg-info.

afuller-TT reviewed Feb 5, 2025

View reviewed changes

dockerfile/Dockerfile Outdated Show resolved Hide resolved

afuller-TT reviewed Feb 5, 2025

View reviewed changes

Merge remote-tracking branch 'origin/main' into blozano-docker

7c41206

blozano-tt commented Feb 5, 2025

View reviewed changes

dockerfile/Dockerfile Outdated Show resolved Hide resolved

blozano-tt added 2 commits February 5, 2025 11:29

Update dockerfile/Dockerfile

2c611a8

Move some build dependencies to install_dependencies.sh

d54d78a

blozano-tt commented Feb 5, 2025

View reviewed changes

afuller-TT reviewed Feb 5, 2025

View reviewed changes

.github/workflows/build-docker-artifact.yaml Show resolved Hide resolved

Update build-docker-artifact.yaml

c741683

afuller-TT reviewed Feb 5, 2025

View reviewed changes

.github/workflows/build-docker-artifact.yaml Outdated Show resolved Hide resolved

afuller-TT and others added 2 commits February 5, 2025 15:21

Fix build.

c938672

Fix issue

5bf8720

afuller-TT approved these changes Feb 5, 2025

View reviewed changes

blozano-tt merged commit 7f0f16f into main Feb 5, 2025
11 checks passed

blozano-tt deleted the blozano-docker branch February 5, 2025 20:41

blozano-tt mentioned this pull request Feb 5, 2025

Close loop between install_dependencies.sh and requirements.txt / Dockerfile #15034

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revamp Dockerfiles #17403

Revamp Dockerfiles #17403

blozano-tt commented Jan 30, 2025 •

edited

Loading

dimitri-tenstorrent Feb 5, 2025

blozano-tt Feb 5, 2025

dimitri-tenstorrent Feb 5, 2025 •

edited

Loading

blozano-tt Feb 5, 2025

afuller-TT Feb 5, 2025

afuller-TT Feb 5, 2025

afuller-TT Feb 5, 2025

blozano-tt Feb 5, 2025

afuller-TT Feb 5, 2025

blozano-tt Feb 5, 2025 •

edited

Loading

blozano-tt Feb 5, 2025

blozano-tt Feb 5, 2025

blozano-tt Feb 5, 2025

blozano-tt Feb 5, 2025

afuller-TT left a comment

blozano-tt Feb 5, 2025

mo-tenstorrent Feb 5, 2025

afuller-TT left a comment


		#############################################################

		FROM ci-test AS dev

		#############################################################

		FROM dev AS release

Revamp Dockerfiles #17403

Revamp Dockerfiles #17403

Conversation

blozano-tt commented Jan 30, 2025 • edited Loading

Problem(s) description

What's changed

Possible in future work

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dimitri-tenstorrent Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blozano-tt Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afuller-TT left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afuller-TT left a comment

Choose a reason for hiding this comment

blozano-tt commented Jan 30, 2025 •

edited

Loading

dimitri-tenstorrent Feb 5, 2025 •

edited

Loading

blozano-tt Feb 5, 2025 •

edited

Loading