-
Notifications
You must be signed in to change notification settings - Fork 72
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
Reduce the size of the clp container images. #288
Reduce the size of the clp container images. #288
Conversation
RUN update-alternatives --install /usr/bin/g++ g++ /usr/bin/g++-10 10 | ||
RUN update-alternatives --set gcc /usr/bin/gcc-10 | ||
RUN update-alternatives --set g++ /usr/bin/g++-10 | ||
# Set gcc/g++ 10 as the default compiler |
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 actually feel the original way we wrote it is more readable. is there a reason why you turn it into a one line command?
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 was trying to reduce the number of generated layers since each RUN
command generates a separate layer; but I guess we can keep them separate since we're flattening the image in the end.
@@ -7,7 +7,7 @@ set -e | |||
set -u | |||
|
|||
apt-get update | |||
DEBIAN_FRONTEND=noninteractive apt-get install -y \ | |||
DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \ | |||
checkinstall \ | |||
curl \ |
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 should add libssl-dev here unless we plan to remove the dependency in the short term?
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 catch.
@@ -7,7 +7,7 @@ set -e | |||
set -u | |||
|
|||
apt-get update | |||
DEBIAN_FRONTEND=noninteractive apt-get install -y \ | |||
DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \ |
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.
same comment for libssl-dev
Reduce the size of the clp container images.? Can't think of a very different title. |
Description
This PR modifies the generation of the docker images to reduce their final size. This has two benefits:
For (2), our current solution is to build the image and use it all within a single job (e.g., when core's dependencies change in a PR, we need to rebuild the container image before rebuilding core). This requires using a complex set of GitHub actions; besides the complexity, actions are more opaque than jobs and don't allow creation of a DAG of dependencies. Ideally, we should switch to building the container in one job and using it in (multiple) dependent jobs.
Sharing a container image between jobs requires uploading the image in the producer and downloading it in the consumers. GitHub only allows 2GB per upload, so we need to optimize the size of our images to stay under this limit.
This PR reduces the size of images in a few ways:
To understand (3), recall that each step in a Dockerfile is cached (into what's called a layer) by a Docker registry so that modifications to the later steps in the Dockerfile don't require a full rebuild. However, this means pulling a Docker image requires downloading all of these layers. Docker has a feature to instead reset the state of an image and then copy the filesystem state, essentially flattening the layers.
The effect of this PR is modest (~100 MiB) for most images but for clp-core-x86-ubuntu-focal, the difference is ~1 GiB since we switched to using
ubuntu:focal
as the base image rather than clp-core-dependencies-x86-ubuntu-focal (essentially removing the build dependencies).This PR also configures cc to map to gcc-10 in clp-core-dependencies-x86-ubuntu-focal since we no longer install the default gcc in the image.
Validation performed
Validated all images still build
Validated the binaries in
clp-core-x86-ubuntu-focal
still workValidated
clp
,clp-s
, andclo
still work in the execution images (clp-execution-x86-ubuntu-focal
&clp-execution-x86-ubuntu-jammy
).Validated that the image size was reduced for every image using the following steps: