Skip to content
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

[build]: Parallel building of sonic dockers using native dockerd(dood) #6086

Closed

Conversation

Kalimuthu-Velappan
Copy link
Contributor

@Kalimuthu-Velappan Kalimuthu-Velappan commented Dec 2, 2020

Currently, the build dockers are created as a user dockers(docker-base-stretch-, etc) that are
specific to each user. But the sonic dockers (docker-database, docker-swss, etc) are
created with a fixed docker name and common to all the users.

docker-database:latest
docker-swss:latest

When multiple builds are triggered on the same build server that creates parallel building issue because
all the build jobs are trying to create the same docker with latest tag.
This happens only when sonic dockers are built using native host dockerd for sonic docker image creation.

This patch creates all sonic dockers as user sonic dockers and then, while
saving and loading the user sonic dockers, it rename the user sonic
dockers into correct sonic dockers with tag as latest.

The user sonic docker names are derived from '_LOAD_DOCKER' make
variable and using Jinja template, it replaces the FROM docker name with correct user sonic docker name for
loading and saving the docker image.

The template processing covers only for common dockers, Broadcom and VS platform dockers. For other vendor specific dockers, respective vendors need to add the support.

- Why I did it
To enable the parallel build using native dockerd for sonic dockers.

- How I did it
Created user dockers for each of the sonic dockers.
docker-swss:latest <=SAVE/LOAD=> docker-swss-<user>:latest

- How to verify it
Parallel build on the same build server

- 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)

xumia
xumia previously approved these changes Dec 9, 2020
@liushilongbuaa
Copy link
Contributor

retest this please

@Kalimuthu-Velappan
Copy link
Contributor Author

retest this please

@xumia
Copy link
Collaborator

xumia commented Dec 18, 2020

retest vs please

@ben-gale
Copy link
Collaborator

@xumia - what's the next step? Can we advance towards merge pls?

@ben-gale
Copy link
Collaborator

@lguohan, @xumia - how do we proceed to approve/merge? Is anything else needed from Broadcom?

Thx.

@ben-gale
Copy link
Collaborator

ben-gale commented Jan 4, 2021

@lguohan, @xumia

@lguohan, @xumia - please advise - let's get this merged pls.

Makefile.work Outdated Show resolved Hide resolved
slave.mk Show resolved Hide resolved
rules/config Outdated Show resolved Hide resolved
rules/config Outdated Show resolved Hide resolved
Makefile.work Show resolved Hide resolved
@lguohan
Copy link
Collaborator

lguohan commented Jan 6, 2021

@ben-gale , i have posted my comments, we have some questions.

@lguohan
Copy link
Collaborator

lguohan commented Jan 6, 2021

@xumia , does this have any interference with reproducible build as they rename the docker from name. not sure if you have some hardcoded rule to tack this.

@ben-gale
Copy link
Collaborator

ben-gale commented Jan 6, 2021

@ben-gale , i have posted my comments, we have some questions.

Thx

Makefile.work Outdated Show resolved Hide resolved
Makefile.work Show resolved Hide resolved
Makefile.work Show resolved Hide resolved
@lguohan lguohan added the Build label Jan 7, 2021
@lguohan lguohan changed the title Parallel building of sonic dockers using native dockerd(dood) [build]: Parallel building of sonic dockers using native dockerd(dood) Jan 7, 2021
Currently, the build dockers are created as a user dockers(docker-base-stretch-<user>, etc) that are
specific to each user. But the sonic dockers (docker-database, docker-swss, etc) are
created with a fixed docker name and common to all the users.

    docker-database:latest
    docker-swss:latest

When multiple builds are triggered on the same build server that creates parallel building issue because
all the build jobs are trying to create the same docker with latest tag.
This happens only when sonic dockers are built using native host dockerd for sonic docker image creation.

This patch creates all sonic dockers as user sonic dockers and then, while
saving and loading the user sonic dockers, it rename the user sonic
dockers into correct sonic dockers with tag as latest.

The user sonic docker names are derived from '_LOAD_DOCKER' make
variable and using Jinja template, it replaces the FROM docker name with correct user sonic docker name for
loading and saving the docker image.
@ben-gale
Copy link
Collaborator

retest vsimage please

@lguohan
Copy link
Collaborator

lguohan commented Jan 26, 2021

@lguohan
Copy link
Collaborator

lguohan commented Jan 26, 2021

if I am doing only the armhf build without this pr, then it works. https://dev.azure.com/sonicswitch/build/_build/results?buildId=1817&view=logs&j=83516c17-6666-5250-abde-63983ce72a49

i am not sure what went wrong here.

@@ -1,5 +1,5 @@
{% from "dockers/dockerfile-macros.j2" import install_debian_packages, install_python_wheels, copy_files %}
FROM docker-base-buster
FROM {{ docker_config_engine_buster_load_image }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

i am not able to figure out what went wrong for this pr for armhf build, but can you split this pr into following pr so that the problem can be isolated.

  1. use the load_image from the .mk file.
  2. everything else.

@@ -278,6 +288,69 @@ endif
export kernel_procure_method=$(KERNEL_PROCURE_METHOD)
export vs_build_prepare_mem=$(VS_PREPARE_MEM)

###############################################################################
## Canned sequences
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe we should move this to Makefile.docker? all instructions related to docker should go there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will make the change as you suggested and will get back to you.

@Kalimuthu-Velappan
Copy link
Contributor Author

It is conflicting with the reproducible build PR and requires some rework. We will fix and submit the changes in a separate PR and will close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants