Support SONiC Reproduceable Build-debian/pip/web packages by xumia · Pull Request #5718 · sonic-net/sonic-buildimage

xumia · 2020-10-26T13:43:35Z

- Why I did it
Support SONiC Reproduceable Build, see design doc: sonic-net/SONiC#684
Features:

Collect the version information to the folder target/versions when building any targets
Sample commands:
make configure PLATFORM=broadcom
make target/sonic-aboot-broadcom.swi
Support to freeze the versions after build by command: make freeze
Sample 1: Initialize the versions or rebuild the versions
make freeze OPTIONS="-r"
Sample 2: Freeze and merge the versions to current distribution and the architecture
make freeze
Sample 3: Merge the current target versions to all distributions and all architectures
make freeze OPTIONS="-d -a"

You can add the version change by: git add files/build/versions
In most cases, simply run "make freeze" to freeze the versions.
Control the version in build, change the configuration file rules/config
SONIC_VERSION_CONTROL_COMPONENTS=all
To upgrade the version configuration, just build any targets, then freeze your versions.
Sample commands:
make configure SONIC_VERSION_CONTROL_COMPONENTS=none PLATFORM=broadcom
make SONIC_VERSION_CONTROL_COMPONENTS=none target/sonic-aboot-broadcom.swi
make freeze OPTIONS="-d -a"

- How I did it

- How to verify it

Make the targets again
Sample commands:
make configure PLATFORM=broadcom
make target/sonic-aboot-broadcom.swi
Freeze the versions again
make freeze
You can see no version change in files/build/versions, git status files/build/versions.
The version files are in target/versions, you can find and verify all the version changes for all the build targets.

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

201811
201911
202006

- Description for the changelog

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

lgtm-com · 2020-10-26T13:59:32Z

This pull request introduces 4 alerts when merging 32fa8dc into 7d4ab42 - view on LGTM.com

new alerts:

3 for Unused local variable
1 for Comparison of identical values #Closed

lgtm-com · 2020-10-26T14:53:30Z

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

new alerts:

1 for Unused local variable #Closed

dockers/docker-base-buster/Dockerfile.j2

qiluo-msft · 2020-11-17T03:21:41Z

files/build/scripts/apt-get

@@ -0,0 +1,35 @@
+#!/bin/bash


Could you add comments to explain the purpose and usage of this script?
My understanding is that they are hooks based on the vanilla executables. So:

What is the new feature?

Any new command line arguments?

How to call vanilla ones? #Closed

It should be the old draft script, it will print error message if version control enabled and the version is not set. #Closed

files/build/scripts/pip

files/build/templates/buildinfo.config.j2

xumia · 2020-11-18T06:16:13Z

See #5786 #Resolved

lgtm-com · 2020-11-18T06:28:49Z

This pull request introduces 1 alert when merging c912a8f into 1ba583c - view on LGTM.com

new alerts:

1 for Unused local variable #Resolved

lgtm-com · 2020-11-18T11:41:58Z

This pull request introduces 1 alert when merging 5c150bb into 2fe79c2 - view on LGTM.com

new alerts:

1 for Unused local variable #Resolved

build_debian.sh

dockers/docker-base-buster/sources.list

files/build/scripts/buildinfo_base.sh

qiluo-msft · 2020-11-19T02:46:49Z

files/build/scripts/buildinfo_base.sh

+POST_VERSION_PATH=$BUILDINFO_PATH/post-versions
+VERSION_DEB_PREFERENCE=$BUILDINFO_PATH/versions/01-versions-deb
+
+. $BUILDINFO_PATH/config/buildinfo.config


buildinfo.config [](start = 25, length = 16)

I cannot find this file in this PR, or in the design doc. #Closed

It is created in scripts/generate_buildinfo_config.sh as the config settings for wget/curl/pip. #Resolved

qiluo-msft · 2020-11-19T02:54:54Z

files/build/scripts/apt-get

+fi
+
+
+/usr/bin/apt-get $@


/usr/bin/apt-get [](start = 0, length = 16)

Could you not hard code this path for vanilla executables? #Closed

files/build/scripts/curl

files/build/scripts/pip3

qiluo-msft · 2020-12-10T02:00:59Z

scripts/prepare_debian_image_buildinfo.sh

+fi
+
+sudo LANG=C chroot $FILESYSTEM_ROOT /bin/bash -c "dpkg -i /usr/local/share/buildinfo/sonic-build-hooks_1.0_all.deb"
+sudo LANG=C chroot $FILESYSTEM_ROOT /bin/bash -c "apt-mark hold sonic-build-hooks"


apt-mark hold sonic-build-hooks [](start = 50, length = 31)

I plan to fix the issue in #6159
So you don't need this line after my PR merged. #Closed

qiluo-msft · 2020-12-10T03:04:43Z

build_debian.sh

 sudo LANG=C chroot $FILESYSTEM_ROOT mount

+## Install the trusted gpg public keys
+[ -d $TRUSTED_GPG_DIR ] && [ ! -z "$(ls $TRUSTED_GPG_DIR)" ] && sudo cp $TRUSTED_GPG_DIR/* ${FILESYSTEM_ROOT}/etc/apt/trusted.gpg.d/


Is it better to install gpg file by curl | sudo apt-key add -.
There is an example in sonic-slave-buster/Dockerfile.j2 #Closed

See another comment in scrpts/prepare_docker_buildinfo.sh

In reply to: 539809352 [](ancestors = 539809352)

qiluo-msft · 2020-12-10T03:15:46Z

build_debian.sh

 sudo mksquashfs $FILESYSTEM_ROOT $FILESYSTEM_SQUASHFS -e boot -e var/lib/docker -e $PLATFORM_DIR

+scripts/collect_host_image_version_files.sh $TARGET_PATH $FILESYSTEM_ROOT
+sudo LANG=C chroot $FILESYSTEM_ROOT set_build_hooks -d


This line is already covered by above line. #Closed

Removed.

In reply to: 539813194 [](ancestors = 539813194)

qiluo-msft · 2020-12-10T03:18:02Z

scripts/collect_docker_version_files.sh

+fi
+docker create --name $DOCKER_CONTAINER --entrypoint /bin/bash $DOCKER_IMAGE
+docker cp -L $DOCKER_CONTAINER:/etc/os-release $TARGET_VERSIONS_PATH/  > /dev/null 2>&1
+docker cp -L $DOCKER_CONTAINER:/usr/local/share/buildinfo/pre-versions $TARGET_VERSIONS_PATH/  > /dev/null 2>&1


/dev/null 2>&1 [](start = 95, length = 16)

Why redirect stdout and stderr? They are useful #Closed

qiluo-msft · 2020-12-10T03:18:39Z

scripts/collect_docker_version_files.sh

+fi
+docker create --name $DOCKER_CONTAINER --entrypoint /bin/bash $DOCKER_IMAGE
+docker cp -L $DOCKER_CONTAINER:/etc/os-release $TARGET_VERSIONS_PATH/  > /dev/null 2>&1
+docker cp -L $DOCKER_CONTAINER:/usr/local/share/buildinfo/pre-versions $TARGET_VERSIONS_PATH/  > /dev/null 2>&1


pre [](start = 58, length = 3)

{pre,post}- #Closed

It is not supported by docker cp.

In reply to: 539814093 [](ancestors = 539814093)

qiluo-msft · 2020-12-10T03:21:23Z

scripts/prepare_docker_buildinfo.sh

+DOCKERFILE_PRE_SCRIPT='# Auto-Generated for buildinfo 
+COPY ["buildinfo", "/usr/local/share/buildinfo"]
+RUN dpkg -i /usr/local/share/buildinfo/sonic-build-hooks_1.0_all.deb
+RUN cp -rf /usr/local/share/buildinfo/trusted.gpg.d/* /etc/apt/trusted.gpg.d/


cp -rf /usr/local/share/buildinfo/trusted.gpg.d/* /etc/apt/trusted.gpg.d/ [](start = 4, length = 73)

Is it better to install gpg file by curl | sudo apt-key add -.
There is an example in sonic-slave-buster/Dockerfile.j2 #Closed

It is not good enough to copy several time when build each docker image. But it should has less impact. I do not use it for several reasons as below:

The curl is not a required package, as a web package it has lot of dependent packages. We want to control all packages including the curl itself.

We may have several gpg files, any one can add more flexibly.

In reply to: 539815009 [](ancestors = 539815009)

When you enable this feature in future, I think you need a place to download the gpg files sometime during the build process.
I agree we should not install many packages inside a docker image just for a one time curl.
How about download outside docker build, and COPY into the image?

In reply to: 540038466 [](ancestors = 540038466,539815009)

qiluo-msft · 2020-12-10T03:26:14Z

scripts/prepare_docker_buildinfo.sh

+    [ -z "$DISTRO" ] && DISTRO=jessie
+fi
+
+DOCKERFILE_PRE_SCRIPT='# Auto-Generated for buildinfo 


DOCKERFILE_PRE_SCRIPT [](start = 0, length = 21)

If gpg comment is right, then DOCKERFILE_PRE_SCRIPT has only 2 command lines. Suggest move them into docker-base, docker-slave's Dockerfile as plaintext. No need to manipulate Dockerfile here. #Closed

See the comments below in pending.

In reply to: 539816717 [](ancestors = 539816717)

qiluo-msft · 2020-12-10T03:28:29Z

scripts/prepare_slave_container_buildinfo.sh

+VERSION_DEB_PREFERENCE=$BUILD_VERSIONS_PATH/01-versions-deb
+
+# Enable the build hooks
+set_build_hooks


Better name: symlink_hooks #Closed

Changed to symlink_build_hooks.

In reply to: 539817491 [](ancestors = 539817491)

qiluo-msft · 2020-12-14T05:35:36Z

scripts/prepare_docker_buildinfo.sh

+DOCKERFILE_PRE_SCRIPT='# Auto-Generated for buildinfo 
+COPY ["buildinfo", "/usr/local/share/buildinfo"]
+RUN dpkg -i /usr/local/share/buildinfo/sonic-build-hooks_1.0_all.deb
+COPY ["buildinfo/trusted.gpg.d/*", "/etc/apt/trusted.gpg.d/"]


COPY [](start = 0, length = 4)

Move the download process into above deb package #Resolved

qiluo-msft · 2020-12-14T05:36:04Z

scripts/prepare_docker_buildinfo.sh

+    awk -v text="${DOCKERFILE_PRE_SCRIPT}" -v linenumber=$LINE_NUMBER 'NR==linenumber{print text}1' $DOCKERFILE > $TEMP_FILE
+
+    # Append the docker build script at the end of the docker file
+    echo "RUN post_run_buildinfo" >> $TEMP_FILE


echo [](start = 4, length = 4)

Corner case: $TEMP_FILE may be not end with \n #Closed

xumia · 2020-12-15T03:30:18Z

retest vsimage please

xumia · 2020-12-15T12:41:56Z

retest vsimage please

xumia · 2020-12-16T04:29:06Z

retest vsimage please

xumia · 2020-12-16T04:29:54Z

retest vsimage please

xumia · 2020-12-16T13:23:53Z

retest vsimage please

)" This reverts commit 55a7075.

…ages (sonic-net#5718)"" This reverts commit 17497a6.

…6255) * Revert "Revert "Support SONiC Reproduceable Build-debian/pip/web packages (#5718)"" This reverts commit 17497a6. * Revert "Revert "Remove unnecessary sudo authority in build Makefile (#6237)"" This reverts commit 163b711.

xumia marked this pull request as draft October 26, 2020 13:48

xumia changed the title ~~Support SONiC Reproduceable Build~~ Support SONiC Reproduceable Build-debian/pip/web packages Oct 27, 2020

lguohan requested a review from qiluo-msft October 30, 2020 14:21

qiluo-msft reviewed Nov 17, 2020

View reviewed changes

dockers/docker-base-buster/Dockerfile.j2 Outdated Show resolved Hide resolved

qiluo-msft reviewed Nov 17, 2020

View reviewed changes

files/build/scripts/pip Outdated Show resolved Hide resolved

qiluo-msft reviewed Nov 17, 2020

View reviewed changes

files/build/templates/buildinfo.config.j2 Outdated Show resolved Hide resolved

qiluo-msft reviewed Nov 17, 2020

View reviewed changes

files/build/templates/buildinfo.config.j2 Outdated Show resolved Hide resolved

qiluo-msft reviewed Nov 17, 2020

View reviewed changes

files/build/templates/buildinfo.config.j2 Outdated Show resolved Hide resolved

xumia closed this Nov 18, 2020

xumia reopened this Nov 18, 2020

xumia force-pushed the repd7 branch from d6432dd to c912a8f Compare November 18, 2020 06:21

xumia marked this pull request as ready for review November 18, 2020 06:28

xumia mentioned this pull request Nov 18, 2020

Support SONiC reproduceable build for deb/py2/py3/web #5786

Closed

3 tasks

qiluo-msft reviewed Nov 18, 2020

View reviewed changes

build_debian.sh Outdated Show resolved Hide resolved

qiluo-msft reviewed Nov 18, 2020

View reviewed changes

build_debian.sh Show resolved Hide resolved

qiluo-msft reviewed Nov 18, 2020

View reviewed changes

dockers/docker-base-buster/sources.list Show resolved Hide resolved

qiluo-msft reviewed Nov 18, 2020

View reviewed changes

dockers/docker-base-buster/sources.list Show resolved Hide resolved