Bin optimize #10718

Kalimuthu-Velappan · 2022-04-29T18:01:51Z

Sonic image build optimization

The current SONIC build infrastructure always prepares the rootfs for final image generation.
Because of rootfs preparation, final image generation takes long regardless of SONiC changes.

The rootfs preparation consists of two parts.
Debian packages installation
Bootstrap preparation
General packages installation, such as curl, vim, sudo, python3, etc
Sonic packages installation
Packages that are built and installed from the sonic repo.
Docker images that are built and installed from the sonic repo

The build time can be optimized by generating the Debian packages as a base image and
it can be run in parallel with the other targets, before the final image.
Benefits:
- High hit rate, for fewer dependencies.
- Reduce the cache size.
- Improve the concurrency when the cache is not hit, the step has small dependencies and can be run with any other steps.
The docker load is also optimized by running in parallel.
Added support for gipz compression
Added tmpfs support for rootfs and final image generation.

Why I did it

How I did it

How to verify it

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

Description for the changelog

Link to config_db schema for YANG module changes

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

lgtm-com · 2022-04-29T19:07:08Z

This pull request introduces 1 alert when merging 8c1df69b4b2748909ee6734a8491182b95e5ebf9 into 53e5fe6 - view on LGTM.com

new alerts:

1 for Modification of parameter with default

xumia · 2022-05-05T01:49:20Z

Makefile.cache

+							$(SONIC_VERSION_CONTROL_COMPONENTS) \
+							$(SONIC_VERSION_CACHE) \


Please fix the TAB/SPACE alignment issue.

xumia · 2022-05-05T01:53:21Z

Makefile.work

    scripts/generate_buildinfo_config.sh)

 # Generate the slave Dockerfile, and prepare build info for it
 $(shell CONFIGURED_ARCH=$(CONFIGURED_ARCH) MULTIARCH_QEMU_ENVIRON=$(MULTIARCH_QEMU_ENVIRON) DOCKER_EXTRA_OPTS=$(DOCKER_EXTRA_OPTS) DEFAULT_CONTAINER_REGISTRY=$(DEFAULT_CONTAINER_REGISTRY) j2 $(SLAVE_DIR)/Dockerfile.j2 > $(SLAVE_DIR)/Dockerfile)
 $(shell CONFIGURED_ARCH=$(CONFIGURED_ARCH) MULTIARCH_QEMU_ENVIRON=$(MULTIARCH_QEMU_ENVIRON) j2 $(SLAVE_DIR)/Dockerfile.user.j2 > $(SLAVE_DIR)/Dockerfile.user)
-$(shell BUILD_SLAVE=y DEFAULT_CONTAINER_REGISTRY=$(DEFAULT_CONTAINER_REGISTRY) scripts/prepare_docker_buildinfo.sh $(SLAVE_BASE_IMAGE) $(SLAVE_DIR)/Dockerfile $(CONFIGURED_ARCH) "" $(BLDENV))


It has impact on the value of SLAVE_BASE_TAG, we may need to run it before line 160.

xumia · 2022-05-05T01:58:23Z

files/build/versions/build/build-sonic-slave-bullseye/versions-deb-bullseye-amd64

@@ -0,0 +1,25 @@
+libhiredis-dev==0.14.0-3~bpo9+1


Are the files in files/build/versions checked in by mistake? We will not enable reproducible build on master branch.

I mean to remove all the files in files/build/versions.

xumia · 2022-05-05T02:04:49Z

files/build_templates/sonic_debian_extension.j2

@@ -668,8 +673,10 @@ sudo LANG=C DOCKER_HOST="$DOCKER_HOST" chroot $FILESYSTEM_ROOT docker info
 {% set imagefilepath = image.split(':')|first -%}
 {% set imagefilename = imagefilepath.split('/')|last -%}
 {% set imagename = imagefilename.split('.')|first -%}
+function {{imagename}}()


Why dynamic function name? Can we use static function name and pass parameter instead?

xumia · 2022-05-05T02:11:17Z

files/build_templates/sonic_debian_extension.j2

+if [ ! -z "${USE_TMPFS}" ]; then
+	NPROC=$(nproc)
+	if [ ${NPROC} -gt 16 ]; then NPROC=16; fi
+	printf '%s\n' "${DLIST[@]}" | xargs -I{} -P ${NPROC}  bash -c " SONIC_IMAGE_VERSION=${SONIC_IMAGE_VERSION} {}"
+else
+	printf '%s\n' "${DLIST[@]}" | xargs -I{} -P 1  bash -c "SONIC_IMAGE_VERSION=${SONIC_IMAGE_VERSION} {}"
+fi
+


Could you please improve the logic? The line 703 and 705 can be called only once.
Why there is a space chareacter in the command line before SONIC_IMAGE_VERSION in line 703, comparing to line 705?

NPROC=$(nproc) [ $NPROC > 16 ] && NPROC=16 [ In some conditions ] && NPROC=1 printf '%s\n' "${DLIST[@]}" | xargs -I{} -P ${NPROC} bash -c " SONIC_IMAGE_VERSION=${SONIC_IMAGE_VERSION} {}"

lgtm-com · 2022-05-05T16:53:30Z

This pull request introduces 1 alert when merging d33da70678d70e9e095792d0c6ed289d91fd13f0 into 7c4ee43 - view on LGTM.com

new alerts:

1 for Modification of parameter with default

lgtm-com · 2022-05-05T18:48:01Z

This pull request introduces 1 alert when merging 5a9bb2222fc70ca54f6fcd29092cc54a025b8c36 into db94886 - view on LGTM.com

new alerts:

1 for Modification of parameter with default

xumia · 2022-05-06T00:46:29Z

Makefile.work


+PREPARE_DOCKER=BUILD_SLAVE=y DBGOPT='$(DBGOPT)' SONIC_VERSION_CACHE=$(SONIC_VERSION_CACHE) \


Missing one of the options of command scripts/prepare_docker_buildinfo.sh as below:

DEFAULT_CONTAINER_REGISTRY=$(DEFAULT_CONTAINER_REGISTRY)

Done as part of #10613

xumia · 2022-05-06T01:10:06Z

@Kalimuthu-Velappan , can we split the PR into several small ones?

Kalimuthu-Velappan · 2022-05-10T13:19:40Z

@Kalimuthu-Velappan , can we split the PR into several small ones?

It is already split into separate PR for version cache.
#10613

Kalimuthu-Velappan · 2022-05-17T13:31:48Z

/azpw run Azure.sonic-buildimage

mssonicbld · 2022-05-17T13:31:50Z

/AzurePipelines run Azure.sonic-buildimage

azure-pipelines · 2022-05-17T13:32:03Z

Azure Pipelines successfully started running 1 pipeline(s).

The current SONIC build infrastructure always prepares the rootfs for final image generation. Because of rootfs preparation, final image generation takes long regardless of SONiC changes. The rootfs preparation consists of two parts. Debian packages installation Bootstrap preparation General packages installation, such as curl, vim, sudo, python3, etc Sonic packages installation Packages that are built and installed from the sonic repo. Docker images that are built and installed from the sonic repo The build time can be optimized by generating the Debian packages as a base image and it can be run in parallel with the other targets, before the final image. Benefits: - High hit rate, for fewer dependencies. - Reduce the cache size. - Improve the concurrency when the cache is not hit, the step has small dependencies and can be run with any other steps. Other enhancements: - The docker load is also optimized by running in parallel. - Added support for gipz compression - Added tmpfs support for rootfs and final image generation.

xumia · 2023-04-16T01:56:52Z

installer/x86_64/install.sh

+    if $( unzip -l $ONIE_INSTALLER_PAYLOAD 2>/dev/null 1>/dev/null ); then
+		unzip -o $ONIE_INSTALLER_PAYLOAD -d $demo_mnt/$image_dir
+	else
+        tar -C $demo_mnt/$image_dir -xvf $ONIE_INSTALLER_PAYLOAD


Do you add the new ONIE_INSTALLER_PAYLOAD format? Why need to handle the tar case? It is relative to new feature SONIC_FS_IMAGE_COMPRESSION_TYPE? If yes, suggest to send a new PR for the different feature.

xumia · 2023-04-16T02:27:03Z

scripts/fsutil.sh

+	umount_allfs
+	delete_rootfs_image
+    if [[  -e ${FSROOT_IMAGE_FILE} ]]; then
+        return 
+    fi
+
+    if [[ "${SONIC_USE_TMPFS_BUILD}" == "y" ]] ; then
+        sudo mount -t tmpfs -o size=${BUILD_TMP_ROOTFS_SIZE} tmpfs ${FSROOT_IMAGE_DIR}
+    fi


If SONIC_USE_TMPFS_BUILD != y, all the options can be skipped, right? Do we need to check if SONIC_USE_TMPFS_BUILD == y before running the lines from 47~51?

xumia · 2023-04-16T02:31:04Z

scripts/fsutil.sh

+	umount_allfs
+	delete_rootfs_image


Looks likes mount_imagefs does some additional cleanup operations? Can we remove them? Better to have another command to do the cleanup explicitly.

xumia · 2023-04-16T02:32:15Z

scripts/fsutil.sh

+
+
+
+
+


Please remove the empty lines at the end of the script file, thanks.

xumia · 2023-04-16T02:33:39Z

scripts/fsutil.sh

+    rootfs)
+        rootfs_fun $1 $2
+        ;;
+    overlayfs)


Do not find any reference to overlayfs, can we remove it?

xumia · 2023-04-16T02:34:36Z

scripts/fsutil.sh

+    #sudo mount -o rw,loop ${FSROOT_IMAGE_FILENAME} ${FILESYSTEM_BASE}/${FSROOT_IMAGE_FILENAME}  || true
+	#sudo mkdir -p ${FILESYSTEM_BASE}/fsroot ${FILESYSTEM_BASE}/upper ${FILESYSTEM_BASE}/work
+    #sudo chmod 777 ${FILESYSTEM_BASE}
+	#sudo mount -t overlay overlay -o lowerdir=${FILESYSTEM_BASE}/${FSROOT_IMAGE_FILENAME},upperdir=${FILESYSTEM_BASE}/upper,workdir=${FILESYSTEM_BASE}/work ${FILESYSTEM_BASE}/fsroot


Remove the no use lines?

xumia · 2023-04-16T02:37:42Z

scripts/fsutil.sh

+## the ONIE installer image generation to speed up the processing.
+##
+## USAGE:
+##   ./tmpfs <storage> <operation>


The description looks like out of date, right? Change ./tmpfs to fsutil.sh? And the script does not only handle the tmpfs for different mount point, right?

qiluo-msft · 2023-05-30T04:59:44Z

@Kalimuthu-Velappan Could you resolve the conflicts?

Kalimuthu-Velappan requested review from qiluo-msft, xumia and lguohan as code owners April 29, 2022 18:01

Kalimuthu-Velappan force-pushed the BIN_OPTIMIZE branch from 00a0ea4 to 8c1df69 Compare April 29, 2022 18:45

xumia reviewed May 5, 2022

View reviewed changes

Kalimuthu-Velappan force-pushed the BIN_OPTIMIZE branch from 8c1df69 to d33da70 Compare May 5, 2022 16:01

Kalimuthu-Velappan force-pushed the BIN_OPTIMIZE branch from d33da70 to 5a9bb22 Compare May 5, 2022 17:53

xumia reviewed May 6, 2022

View reviewed changes

Kalimuthu-Velappan force-pushed the BIN_OPTIMIZE branch from 5a9bb22 to 6182b36 Compare May 10, 2022 13:16

Kalimuthu-Velappan mentioned this pull request May 10, 2022

SONiC Build Improvements sonic-net/SONiC#942

Merged

Kalimuthu-Velappan force-pushed the BIN_OPTIMIZE branch 5 times, most recently from fb0038b to 513313d Compare May 16, 2022 10:41

Kalimuthu-Velappan force-pushed the BIN_OPTIMIZE branch from 513313d to 0e5c319 Compare May 23, 2022 09:11

Kalimuthu-Velappan mentioned this pull request Dec 12, 2022

02.Version cache - docker cache build framework #12001

Merged

7 tasks

Kalimuthu-Velappan mentioned this pull request Dec 12, 2022

[Build] use pigz to speed up a build #12825

Merged

xumia reviewed Apr 16, 2023

View reviewed changes

scripts/fsutil.sh

Comment on lines +316 to +320

Copy link

Collaborator

xumia Apr 16, 2023

Choose a reason for hiding this comment

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

Please remove the empty lines at the end of the script file, thanks.

xumia reviewed Apr 16, 2023

View reviewed changes

scripts/fsutil.sh

rootfs)

rootfs_fun $1 $2

;;

overlayfs)

Copy link

Collaborator

xumia Apr 16, 2023

Choose a reason for hiding this comment

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

Do not find any reference to overlayfs, can we remove it?

xumia reviewed Apr 16, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bin optimize #10718

Bin optimize #10718

Kalimuthu-Velappan commented Apr 29, 2022

lgtm-com bot commented Apr 29, 2022

xumia May 5, 2022

Kalimuthu-Velappan May 5, 2022

xumia May 5, 2022

Kalimuthu-Velappan May 5, 2022

xumia May 5, 2022

Kalimuthu-Velappan May 5, 2022

xumia May 6, 2022

xumia May 5, 2022

Kalimuthu-Velappan May 5, 2022

xumia May 5, 2022 •

edited

Loading

Kalimuthu-Velappan May 5, 2022

lgtm-com bot commented May 5, 2022

lgtm-com bot commented May 5, 2022

xumia May 6, 2022

Kalimuthu-Velappan May 10, 2022

xumia commented May 6, 2022

Kalimuthu-Velappan commented May 10, 2022

Kalimuthu-Velappan commented May 17, 2022

mssonicbld commented May 17, 2022

azure-pipelines bot commented May 17, 2022

xumia Apr 16, 2023 •

edited

Loading

xumia Apr 16, 2023

xumia Apr 16, 2023 •

edited

Loading

xumia Apr 16, 2023

xumia Apr 16, 2023

xumia Apr 16, 2023

xumia Apr 16, 2023

qiluo-msft commented May 30, 2023

		$(SONIC_VERSION_CONTROL_COMPONENTS) \
		$(SONIC_VERSION_CACHE) \


		PREPARE_DOCKER=BUILD_SLAVE=y DBGOPT='$(DBGOPT)' SONIC_VERSION_CACHE=$(SONIC_VERSION_CACHE) \

Bin optimize #10718

Are you sure you want to change the base?

Bin optimize #10718

Conversation

Kalimuthu-Velappan commented Apr 29, 2022

Why I did it

How I did it

How to verify it

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

Description for the changelog

Link to config_db schema for YANG module changes

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

lgtm-com bot commented Apr 29, 2022

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xumia May 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgtm-com bot commented May 5, 2022

lgtm-com bot commented May 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xumia commented May 6, 2022

Kalimuthu-Velappan commented May 10, 2022

Kalimuthu-Velappan commented May 17, 2022

mssonicbld commented May 17, 2022

azure-pipelines bot commented May 17, 2022

xumia Apr 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xumia Apr 16, 2023 • 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

qiluo-msft commented May 30, 2023

xumia May 5, 2022 •

edited

Loading

xumia Apr 16, 2023 •

edited

Loading

xumia Apr 16, 2023 •

edited

Loading