[build] Add option to avoid Docker base image :latest tag

[gpaussabrcm]

• Define slave_base_tag_ref variable in Makefile.work containing
  specific base image tag to use, rather than always defaulting
  to :latest.

• Add an ARG command before FROM statement in Dockerfile.user
  for sonic-slave and sonic-slave-stretch. ARG variable defaults
  to latest if slave_base_tag_ref not specified in Makefile.work.

The presumption to always refer to the :latest tagged Docker base
image when creating the user image causes problems in a shared
build server environment, where the most recently created base
image (i.e. the current :latest tag) may not be compatible with
the current build. For example, different users working in
different branches may all be sharing the same build server.

Signed-off-by: Greg Paussa [email protected]

- What I did
Added a DOCKER_AVOID_BASE_TAG_LATEST build option to rules/config that forces the Docker user image creation to refer to its base image by a specific tag rather than rely on the :latest tag. This is needed in a shared build server environment where builds from different developers and/or different SONiC branches all converge on the same Docker daemon instance running on the build server. The :latest tag is always assigned to the most recent base image built, which might not correspond to the base image needed for a particular build, thus causing various build errors that mostly manifest as missing Debian packages or package version mismatches.

NOTE TO REVIEWERS: This PR relies on Docker support of "ARG before FROM," which was first introduced in Docker version 17.05.1-ce. Although there is no mention of a minimum required Docker version for the build server in the SONiC Building Guide pages, please consider whether it is reasonable to assume that Docker 17.05.1-ce or later must be used for SONiC build hosts before approving this PR.

- How I did it
Added an ARG before the FROM statement at the top of the sonic-slave/Dockerfile.user and sonic-slave-stretch/Dockerfile.user files. The ARG variable defaults to latest, but can be overridden in Makefile.work to reference the SLAVE_BASE_TAG so that it refers to the specific, matching base image for the build. This override is activated by un-commenting the DOCKER_AVOID_BASE_TAG_LATEST = y line in rules/config.

- How to verify it
A mismatch in the base image tag causes an error during the user image creation, which would be obvious to the developer. To verify a successful build of the user image, check its Docker history to see that the right base image ID is referenced, as illustrated by the following example:

Hint: Focus on bold values in this output.

# make showtag
+++ Making showtag +++
BLDENV=stretch make -f Makefile.work showtag
make[1]: Entering directory '/builds/sonic/someuser/repos/sonic-buildimage'
sonic-slave-stretch-someuser:ed01ec3b145
sonic-slave-stretch-base:93e3a1c4b07
make[1]: Leaving directory '/builds/sonic/someuser/repos/sonic-buildimage'

# docker images | egrep "sonic-slave-stretch-someuser|REPOSITORY"
REPOSITORY                                     TAG                 IMAGE ID            CREATED             SIZE
sonic-slave-stretch-someuser                   ed01ec3b145         692723e02f00        About an hour ago   4.16GB

# docker images | egrep "sonic-slave-stretch-base|REPOSITORY"
REPOSITORY                                     TAG                 IMAGE ID            CREATED             SIZE
sonic-slave-stretch-base                       b501a9391d2         384ac9293e94        7 days ago          4.15GB
sonic-slave-stretch-base                       cd636ae59de         0433427b4850        8 days ago          4.3GB
sonic-slave-stretch-base                       3e6941322e5         e06c913f4263        11 days ago         4.17GB
sonic-slave-stretch-base                       93e3a1c4b07         19a7e622da3d        11 days ago         4.16GB
sonic-slave-stretch-base                       9324698b31e         1faf09d99eee        12 days ago         4.16GB
sonic-slave-stretch-base                       b12f3e165ce         1d5f3588a83e        12 days ago         4.3GB
sonic-slave-stretch-base                       latest              1d5f3588a83e        12 days ago         4.3GB
sonic-slave-stretch-base                       8f70fa10dae         7d220a420831        2 weeks ago         4.16GB
sonic-slave-stretch-base                       efee9dd0e26         00663d432f28        2 weeks ago         4.15GB

# docker history 692723e02f00
IMAGE               CREATED             CREATED BY                                      SIZE                COMMENT
692723e02f00        About an hour ago   /bin/sh -c #(nop)  USER someuser                0B                         
1711748a2f3d        About an hour ago   |4 guid=841 hostname=sonic-bldsrvr-2 uid=7⦠    701B                      
d87d8799c2b3        About an hour ago   |4 guid=841 hostname=sonic-bldsrvr-2 uid=7⦠    398B                      
d0ade02c8a82        About an hour ago   |4 guid=841 hostname=sonic-bldsrvr-2 uid=7⦠    398B                      
57c29a0d6bd7        About an hour ago   /bin/sh -c #(nop) COPY file:551f41e8b32096c1⦠  398B                      
c8b473835fe8        About an hour ago   |4 guid=841 hostname=sonic-bldsrvr-2 uid=7⦠    54B                       
186483e78c95        About an hour ago   |4 guid=841 hostname=sonic-bldsrvr-2 uid=7⦠    24B                       
cf35220b7742        About an hour ago   |4 guid=841 hostname=sonic-bldsrvr-2 uid=7⦠    2.52kB                    
7cd978c6cc08        About an hour ago   |4 guid=841 hostname=sonic-bldsrvr-2 uid=7⦠    10kB                      
3a2584a6f546        About an hour ago   |4 guid=841 hostname=sonic-bldsrvr-2 uid=7⦠    2.47kB                    
c4ea8ffe9efe        About an hour ago   /bin/sh -c #(nop)  ENV USER=someuser            0B                         
829ee3b3af2c        About an hour ago   /bin/sh -c #(nop)  ENV BUILD_HOSTNAME=sonic-⦠  0B                        
a191fb031dda        About an hour ago   /bin/sh -c #(nop)  ARG hostname                 0B                         
b1949b721d79        About an hour ago   /bin/sh -c #(nop)  ARG guid                     0B                         
e4ec477ad1b7        About an hour ago   /bin/sh -c #(nop)  ARG uid                      0B                         
ac52e018e0e2        About an hour ago   /bin/sh -c #(nop)  ARG user                     0B                         
19a7e622da3d        11 days ago         |2 debian_archive_mirror= debian_org_mirror=⦠  704B                      
           11 days ago         |2 debian_archive_mirror= debian_org_mirror=⦠  117MB                     
           11 days ago         |2 debian_archive_mirror= debian_org_mirror=⦠  670kB                     
---snip---

- Description for the changelog

Add optional build configuration variable to avoid relying on :latest tag when identifying Docker base build image.

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

[lguohan]

retest this please

[lguohan]

for developer perspective, how does he know which ref tag to use? can the build system automatically calcuate it without user to specify that?

[gpaussabrcm]

Yes, in fact we don't want the user to specify it. Makefile.work already calculates the base image tag in the SLAVE_BASE_TAG variable, which is currently used to assign a tag to the sonic-slave-base and sonic-slave-stretch-base images in addition to the :latest tag being applied to each. (The SLAVE_BASE_TAG is the same one displayed by the 'make showtag' command.) What this PR does is eliminate creating the :latest tag for these base images, and pass the same SLAVE_BASE_TAG value into the corresponding user image Dockerfile through the new slave_base_tag_ref build-arg variable in the 'docker build' command. This modified behavior is enabled by setting DOCKER_AVOID_BASE_TAG_LATEST = y in rules/config.

[lguohan]

why not make this option as default? what is the reason not to make it as default?

I think it is fine to require a docker version. my request is that can you explicit add a check in the Makefile so that it can explicitly check the docker version and report error to user if it does not meet?

[gpaussabrcm]

No particular reason, other than taking a cautious approach to not potentially disrupt existing build environments by having to opt-in to the modified build behavior. Actually, I added support for the rules/config variable in preparation for upstreaming this change, but can take it back out.

I did some experimenting with checking the Docker version in Makefile.work and ended up with the following:

# Check for minimum Docker version on build host
docker_min := 17.05.0
docker_min_ver := $(shell echo "$(docker_min)" | awk -F. '{printf("%d%03d%03d\n",$$1,$$2,$$3);}' 2>/dev/null)
docker_ver := $(shell docker info 2>/dev/null | grep -i "server version" | cut -d' ' -f3 | awk -F. '{printf("%d%03d%03d\n",$$1,$$2,$$3);}' 2>/dev/null)
docker_is_valid := $(shell if [ $(docker_ver) -lt $(docker_min_ver) ] ; then echo "0"; else echo "1"; fi)
ifeq (0,$(docker_is_valid))
$(error SONiC requires Docker version $(docker_min) or later)
endif

The error message shown when the above check fails looks like this:

# make showtag
+++ Making showtag +++
BLDENV=stretch make -f Makefile.work showtag
make[1]: Entering directory '/builds/sonic/someuser/repos/sonic-buildimage'
Makefile.work:52: *** SONiC requires Docker version 17.05.0 or later.  Stop.
make[1]: Leaving directory '/builds/sonic/someuser/repos/sonic-buildimage'
Makefile:19: recipe for target 'showtag' failed
make: *** [showtag] Error 2

Does the above look OK?

Note: I changed the minimum Docker version to 17.05.0 instead of 17.05.1 based on the official Docker CE release notes. However, this support wasn't added to Docker EE until version 17.06.1, and as far as I can tell, there's no easy way to discern the CE from EE version number. For purposes of this version check in SONiC, I think it would be best to bump the minimum Docker version to 17.06.1 to cover both EE and CE. Do you agree?

I plan on trying this change in-house for a couple of days before updating the PR with a new commit, just to make sure it works as intended.

[lguohan]

i feel make it default is the right way to go instead introducing as an optional build option.

[gpaussabrcm]

OK, I'll rework the PR to remove the build option. I'll also add a build time check in Makefile.work for a minimum Docker version of 17.06.1 on the build host, which will ensure both CE and EE are capable of supporting "ARG before FROM" usage.

Procedural question: Given the comment history, is it best to amend the existing commit, or submit a second commit for this PR?

[gpaussabrcm]

I went ahead and pushed a second commit with the requested changes.

[lguohan]

retest vs please