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 container images in parallel #59

Merged
merged 6 commits into from
Jun 28, 2018

Conversation

dirgim
Copy link
Collaborator

@dirgim dirgim commented Jun 22, 2018

This change introduces parallel builds in the playbooks/roles/os_temps/setup_containers.yml task in the following manner:

  1. All the information needed to initiate a build (params, buildconfig and imagestream checks etc.) gets proccessed and stored in corresponding hashes (dictionaries) for every template before the parallel builds get initiated (setup_os_templates.yml)
  2. The builds get started in (relatively) rapid succesion, only waiting to check if the build is started - this helps to stagger the builds if the number of concurrent builds gets too large to be run at the same time (handle_os_templates.yml which calls build_new_app.yml)
  3. While the builds are running in parallel, the next step waits for the first build to be finished, and then for the second one and so on - the result of every build is stored in a separate hash (check_new_app.yml)
  4. After all the builds are finished, the results of all the builds are checked and if any of them failed, all the failed build resources get cleaned up and the builds that failed get retried in another run which starts from the step 2. this gets repeated for a maximum of 4 times (handle_os_templates.yml)

In testing, building container images in parallel tended to be 40% faster on average than building them sequentially.

@dirgim dirgim requested review from scoheb and arilivigni June 22, 2018 13:12
@psiroky
Copy link

psiroky commented Jun 28, 2018

LGTM - this is a great improvement! I can confirm the ~40% improvement when running CVP setups.

Copy link
Member

@arilivigni arilivigni left a comment

Choose a reason for hiding this comment

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

Does this also take care of cleanup issues?

@dirgim
Copy link
Collaborator Author

dirgim commented Jun 28, 2018

Only partially, I'm testing some additional cleanup fixes right now

Copy link
Member

@arilivigni arilivigni left a comment

Choose a reason for hiding this comment

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

Do you think we should wait and add it to this PR?

@dirgim
Copy link
Collaborator Author

dirgim commented Jun 28, 2018

I was thinking of making any major changes to cleanup a separate PR. This last commit is just a small fix.

Copy link
Member

@arilivigni arilivigni left a comment

Choose a reason for hiding this comment

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

LGTM

@arilivigni arilivigni merged commit c3444a0 into CentOS-PaaS-SIG:master Jun 28, 2018
@dirgim dirgim deleted the parallel-builds branch August 31, 2018 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants