Update dockerfiles to do staged builds #19952

saiarcot895 · 2024-08-19T19:05:12Z

Why I did it

On newer versions of Docker, only the buildkit builder is supported, and
cannot be disabled by setting DOCKER_BUILDKIT to 0. The side effect of
this is that the behavior of --squash is different (see
moby/moby#38903). This will result in the container sizes being
significantly higher.

Work item tracking

Microsoft ADO (number only): 29277538

How I did it

To work around this, make all of our builds two-stage builds, with the
--squash flag entirely removed. The way this works is in the first
stage, whatever new files/packages need to be added are added (along
with files/packages that need to be removed). Then, in the second stage,
use rsync to copy over the changed files as a single command/layer. In the
case of the base layer for each Debian version, the final result of the first
stage will be copied into an empty base layer.

As part of this, also consolidate the container cleanup code into
post_run_cleanup, and remove it from the individual containers (for
consistency). Also experiment a bit with not needing to explicitly
install library dependencies, and let apt install it as necessary. This
will help during upgrades in the case of ABI changes for packages.

Also, remove the SONIC_USE_DOCKER_BUILDKIT option, and don't
set DOCKER_BUILDKIT option. This option will eventually have no impact.
This also means that builds will now use buildkit, as that is the default now.

How to verify it

Which release branch to backport (provide reason below if selected)

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

k-v1 · 2024-10-17T19:28:23Z

dockers/docker-base-bookworm/Dockerfile.j2

 {% else %}
-FROM {{ prefix }}{{DOCKER_BASE_ARCH}}/debian:bookworm
+ARG BASE={{ prefix }}{{DOCKER_BASE_ARCH}}/debian:bookworm


@saiarcot895
You can use debian slim images to reduce final image size as it was suggested here: #19008.

Agreed. I want to keep the focus of this PR on unblocking Docker upgrades, but I had to bring in some space optimization stuff (see the COPY at the end of this file) to get things to work.

On newer versions of Docker, only the buildkit builder is supported, and cannot be disabled by setting DOCKER_BUILDKIT to 0. The side effect of this is that the behavior of `--squash` is different (see moby/moby#38903). This will result in the container sizes being significantly higher. To work around this, make all of our builds two-stage builds, with the `--squash` flag entirely removed. The way this works is in the first stage, whatever new files/packages need to be added are added (along with files/packages that need to be removed). Then, in the second stage, all of the files from the final state of the first stage are copied to the second stage. As part of this, also consolidate the container cleanup code into `post_run_cleanup`, and remove it from the individual containers (for consistency). Also experiment a bit with not needing to explcitly install library dependencies, and let apt install it as necessary. This will help during upgrades in the case of ABI changes for packages. Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

This shouldn't be committed Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

The docker root cleanup is removing the contents of the docker root directory we create from within a container. However, this container isn't using the container registry variable, which menas it may fail depending on the network environment. Fix this by prefixing the container registry variable The docker root directory creation is missing the `shell` at the beginning, which means the directory doesn't actually get created. While the docker command later will still create the directory automatically, fix this and make sure it gets created here. Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

It seems that on the Bullseye slave container (not sure about Buster), the nofile ulimit is set to 1048576:1048576 (as in, 1048576 for both the soft and hard limit). However, the Docker startup script in version 25 and newer sets the hard limit to 524288 (because of moby/moby#c8930105b), which fails because then the soft limit will be higher than the hard limit, which doesn't make sense. However, on a Bookworm slave container, the nofile ulimit is set to 1024:1048576, and the startup script's ulimit command goes through. A simple workaround would be to explicitly set the nofile ulimit to be 1024:1048576 for all slave containers. However, sonic-swss's tests needs more than 1024 file descriptors open, because the test code doesn't clean up file descriptors at the end of each test case/test suite. This results in FD leaks. Therefore, set the ulimit to 524288:1048576, so that Docker's startup script can lower it to 524288 and swss can open file descriptors. Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

With the new approach of building the images (where the entire final rootfs is copied into the second stage), if the system building the containers is using the overlay2 storage driver (which is the default) and is able to use native diffs (which could be true if CONFIG_OVERLAY_FS_REDIRECT_DIR isn't enabled in the kernel), then the final result of the image will be different than if naive diffs (where Docker compares the metadata of each file and, if needed, the contents to find out if something has changed) were used. Specifically, with native diffs, each container would be much larger, since technically speaking, the whole rootfs is being written to, even if the content ends up the same. This appears to be a known issue (in some form), and workarounds are being thought of in moby/moby#35280. As a workaround, install rsync into the base container, copy the entirely of that into an empty base image, and use rsync to copy only the changed files into the layer in one shot. This does mean that rsync will remain installed in the final built containers, but hopefully this is fine. Signe-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-01-10T00:31:40Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-01-10T00:31:52Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-01-10T07:02:00Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-01-10T07:02:16Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-01-10T18:20:46Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-01-10T18:21:00Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-01-15T18:12:37Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-01-15T18:12:50Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-01-24T02:44:30Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-01-24T02:44:42Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-02-23T17:39:28Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-02-23T17:39:41Z

Azure Pipelines successfully started running 1 pipeline(s).

liushilongbuaa · 2025-03-12T07:32:57Z

Makefile.work

@@ -310,6 +312,7 @@ DOCKER_RUN := docker run --rm=true --privileged --init \
    -e "https_proxy=$(https_proxy)" \
    -e "no_proxy=$(no_proxy)" \
    -i$(shell { if [ -t 0 ]; then echo t; fi }) \
+    --ulimit nofile=524288:524288 \


Why bullseye needs this option but bookworm doesn't?

This was needed with an older version of changes that included a Docker daemon version upgrade, but isn't needed anymore. It may be needed in the future when the upgrade is done.

There was previously a commit to upgrade Docker to version 25, for the purposes of using containerd image store. With that upgrade, docker-in-docker in the Bullseye slave container did not start up because of a ulimit error. In the Bullseye slave container, the ulimit for the max open files was set to 1048576:1048576 (as in, 1048576 for both the soft and hard limit). However, Docker 25 and newer lowered just the hard limit to 524288 in moby/moby@c8930105b. This caused the soft limit to be higher than the hard limit, which is an issue, and so starting up docker failed. Based on local testing, the Bookworm slave container there appeared to be not affected because this ulimit was 1024:1048576, and so even lowering just the hard limit to 524288 would be fine.

Rechecking my testing now, when starting up a slave container locally, I see that both Bullseye and Bookworm have 1048576:1048576; I'm not sure what changed to also cause Bookworm to be potentially affected.

This issue affects us only after we upgrade to Docker 25 or newer; we're still on Docker 24 for now. I can either keep this change or take it out of the PR.

liushilongbuaa · 2025-03-12T09:27:13Z

dockers/docker-config-engine-bookworm/Dockerfile.j2

+
+FROM $BASE
+
+RUN --mount=type=bind,from=base,target=/changes-to-image rsync -axAX --no-D --exclude=/sys --exclude=resolv.conf /changes-to-image/ /


Do you have a test result that shows the how much the disk saves after this PR?

Taking a sonic-broadcom.bin image built by the pipeline, from around January 15th, the sonic-broadcom.bin file with this PR is about 80MB smaller than the sonic-broadcom.bin from January 15th official build (921MB vs 1002MB). The docker directory, after extraction, is about 170MB smaller (1592MB vs 1766MB).

I need to resolve new merge conflicts, so I can get updated numbers after doing that.

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

mssonicbld · 2025-03-13T22:12:18Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-03-13T22:12:31Z

Azure Pipelines successfully started running 1 pipeline(s).

liushilongbuaa · 2025-03-14T17:19:27Z

dockers/docker-base-bookworm/Dockerfile.j2

+
+FROM scratch
+
+COPY --from=base / /


Why docker-base use COPY but other dockers use RUN rsync?

rsync is run from the base/source layer, and the new contents of the new image is mounted in a directory. In the case of docker-base-bookworm, the base layer is debian:bookworm, which doesn't have rsync installed, so rsync can't be run from there. Since rsync is installed in docker-base-bookworm, and everything else is on top of this image, rsync can be used elsewhere.

While it might be technically possible to run rsync from that mounted directory, it's far easier and more reliable to just copy the final results of docker-base-bookworm into an empty layer. This results in two things:

When this container gets built, docker-base-bookworm will be the top-level image for any container that gets built from this container, instead of being debian:bookworm.

For files that get removed in this container, those files will not be present in the final container build at all. Currently, when any file/directory is removed, there is a "whiteout" file that gets added into the container indicating that the file/directory referenced in the base layer no longer exists, but the space by that file/directory is still taken up. Now, that file/directory will not exist at all, and thus not take up disk space.

krismarvell mentioned this pull request Sep 16, 2024

Marvell: Update Dockerfiles for staged builds saiarcot895/sonic-buildimage#21

Merged

5 tasks

saiarcot895 force-pushed the update-dockerfiles branch from 5287e97 to 0115b4d Compare September 29, 2024 23:31

k-v1 reviewed Oct 17, 2024

View reviewed changes

saiarcot895 and others added 14 commits January 9, 2025 08:55

Remove the Dockerfile.cleanup for docker-dash-engine

d910457

This shouldn't be committed Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Fix typos/duplicate lines in dockerfiles

5e61a87

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Fix version control for dockers not working

1cb7f15

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Don't simulate removal, actually do it

1e2a34b

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Re-add purge command that was accidentally removed for orchagent

edf701d

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Update gbsyncd Dockerfiles, and use macros for docker-syncd-brcm

e153fa4

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Update docker-dash-engine

939af50

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Fix macro includes

21c9591

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Make sure environment variables are present in the second (final) stage

74cc236

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Marvell: Update Dockerfiles for staged builds (#21)

7ea1005

saiarcot895 force-pushed the update-dockerfiles branch from 7968803 to 0b85785 Compare January 10, 2025 00:30

saiarcot895 added 2 commits January 9, 2025 23:00

Fix docker builds

0f81931

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Fix armhf and arm64 build

6443892

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

saiarcot895 added 3 commits January 10, 2025 09:12

Remove armhf and arm64 development packages as well

1678fbd

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Fix base container for Broadcom syncd

2bdcd69

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Fix exclude argument to not exclude /usr/include/<triple>/sys

da5c8a8

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Merge remote-tracking branch 'origin/master' into update-dockerfiles

07f8c02

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

Remove documentation about SONIC_USE_DOCKER_BUILDKIT

803314c

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

saiarcot895 marked this pull request as ready for review January 29, 2025 03:58

saiarcot895 requested review from qiluo-msft, xumia and lguohan as code owners January 29, 2025 03:58

saiarcot895 requested a review from liushilongbuaa February 5, 2025 16:06

Merge remote-tracking branch 'origin/master' into update-dockerfiles

87e2373

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

liushilongbuaa reviewed Mar 12, 2025

View reviewed changes

Merge remote-tracking branch 'origin/master' into update-dockerfiles

7127dc2

Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>

liushilongbuaa reviewed Mar 14, 2025

View reviewed changes

liushilongbuaa approved these changes Mar 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update dockerfiles to do staged builds #19952

Update dockerfiles to do staged builds #19952

saiarcot895 commented Aug 19, 2024 •

edited

Loading

k-v1 Oct 17, 2024

saiarcot895 Jan 24, 2025

mssonicbld commented Jan 10, 2025

azure-pipelines bot commented Jan 10, 2025

mssonicbld commented Jan 10, 2025

azure-pipelines bot commented Jan 10, 2025

mssonicbld commented Jan 10, 2025

azure-pipelines bot commented Jan 10, 2025

mssonicbld commented Jan 15, 2025

azure-pipelines bot commented Jan 15, 2025

mssonicbld commented Jan 24, 2025

azure-pipelines bot commented Jan 24, 2025

mssonicbld commented Feb 23, 2025

azure-pipelines bot commented Feb 23, 2025

liushilongbuaa Mar 12, 2025

saiarcot895 Mar 13, 2025

liushilongbuaa Mar 12, 2025

saiarcot895 Mar 13, 2025 •

edited

Loading

mssonicbld commented Mar 13, 2025

azure-pipelines bot commented Mar 13, 2025

liushilongbuaa Mar 14, 2025

saiarcot895 Mar 16, 2025


		FROM $BASE

		RUN --mount=type=bind,from=base,target=/changes-to-image rsync -axAX --no-D --exclude=/sys --exclude=resolv.conf /changes-to-image/ /

Update dockerfiles to do staged builds #19952

Are you sure you want to change the base?

Update dockerfiles to do staged builds #19952

Conversation

saiarcot895 commented Aug 19, 2024 • edited Loading

Why I did it

Work item tracking

How I did it

How to verify it

Which release branch to backport (provide reason below if selected)

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

k-v1 Oct 17, 2024

Choose a reason for hiding this comment

saiarcot895 Jan 24, 2025

Choose a reason for hiding this comment

mssonicbld commented Jan 10, 2025

azure-pipelines bot commented Jan 10, 2025

mssonicbld commented Jan 10, 2025

azure-pipelines bot commented Jan 10, 2025

mssonicbld commented Jan 10, 2025

azure-pipelines bot commented Jan 10, 2025

mssonicbld commented Jan 15, 2025

azure-pipelines bot commented Jan 15, 2025

mssonicbld commented Jan 24, 2025

azure-pipelines bot commented Jan 24, 2025

mssonicbld commented Feb 23, 2025

azure-pipelines bot commented Feb 23, 2025

liushilongbuaa Mar 12, 2025

Choose a reason for hiding this comment

saiarcot895 Mar 13, 2025

Choose a reason for hiding this comment

liushilongbuaa Mar 12, 2025

Choose a reason for hiding this comment

saiarcot895 Mar 13, 2025 • edited Loading

Choose a reason for hiding this comment

mssonicbld commented Mar 13, 2025

azure-pipelines bot commented Mar 13, 2025

liushilongbuaa Mar 14, 2025

Choose a reason for hiding this comment

saiarcot895 Mar 16, 2025

Choose a reason for hiding this comment

saiarcot895 commented Aug 19, 2024 •

edited

Loading

saiarcot895 Mar 13, 2025 •

edited

Loading