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

Prototype using docker buildx bake #1604

Closed
wants to merge 2 commits into from

Conversation

romainx
Copy link
Collaborator

@romainx romainx commented Feb 5, 2022

Hello,

Before going further #1585 on I wanted to give a try to docker buildx bake.

Bake is a high-level build command. Each specified target will run in parallel as part of the build.

My inspiration comes from the Official Jenkins Docker image repository.

Conclusion

Pros

  • Permit to build everything in parallel and so improve performances by parallelization.
    • avoiding to start / stop build process and to load / unload resources multiple times.
    • Hard to say but could improve by a little bit more of ~ 15 % the build time and therefore resource consumption.
  • Permits to have a standard way to manage build definitions and the various platforms.
    • Definition can be displayed by running make print/<image short name> .

Cons

  • In our use case with something like 11 different and dynamic tags applied on 8 images, it appears difficult to manage tags in the build definition file docker-backe.hcl
    • The only solution I have seen is to use environment variables to transmit tags information.
    • If we consider that most of the tags can be different between two images, the number of variable become unmanageable
    • It is possible to define function to factorize tagging (see example below), but variables still need to be defined in a hcl file (if not the build bake complains).
    • It could be possible to generate this variable definition in another file (it is possible to use variables across files) but it does not seem to be a good solution.
    • So according to those constrains, I have not modified the existing to push mechanism.

Example of tagging function and its usage.

function "tags" {
  params = [image]
  result = [
    "${OWNER}/${image}:latest",
    notequal("",T_SHA) ? "${OWNER}/${image}:${T_SHA}": "",
    notequal("",T_DATE) ? "${OWNER}/${image}:${T_DATE}": "",
    notequal("",T_UBUNTU) ? "${OWNER}/${image}:${T_UBUNTU}": "",
  ]
}

target "base-notebook" {
  inherits = ["_default"]
  context = "base-notebook"
  tags = tags("minimal-notebook")
}

I'm unsure if we should merge or not this PR just to save time and resources without having tags managed in this way 😕.
In any case it was interesting at least for me, and I hope it will open new possibilities or axis of improvements.

@consideRatio and @mathbunnyru do not hesitate to share your feedback.

Best.

@romainx romainx closed this Feb 5, 2022
@romainx romainx reopened this Feb 5, 2022
@romainx romainx marked this pull request as draft February 5, 2022 04:25
@romainx romainx changed the title Prototype using docker bake Prototype using docker builds bake Feb 5, 2022
@romainx romainx changed the title Prototype using docker builds bake Prototype using docker buildx bake Feb 5, 2022
@romainx
Copy link
Collaborator Author

romainx commented Feb 5, 2022

It seems to run a bit faster and I think the introduction of the docker-bake.hcl brings clarity in the way we support platforms. I will go further by having a look to a trickiest part: how to deal with our dynamic tags.

@mathbunnyru
Copy link
Member

@romainx before you put too much effort, please, take a look here:
#1407 (comment)

I think the solution of many of our problems requires different approach and I described it there.

@romainx
Copy link
Collaborator Author

romainx commented Feb 5, 2022

@mathbunnyru thanks for the heads-up. However, I don't think this change is incompatible with what you intend to do. I see this as a step in this direction by standardizing some code. So I would like to finish it if you don't mind, I think the effort is not too important. Then we will choose if we want to merge it or not.
@consideRatio what is your opinion?

@mathbunnyru
Copy link
Member

Sure, proceed as you wish :)

@romainx romainx marked this pull request as ready for review February 6, 2022 09:18
@romainx
Copy link
Collaborator Author

romainx commented Feb 6, 2022

@mathbunnyru I've put my conclusions in the PR description. It appears that managing tags this way in our very dynamic tagging policy is difficult.

@consideRatio
Copy link
Collaborator

Bake is a high-level build command. Each specified target will run in parallel as part of the build.

So this is an optimization to let our build command, when using multi platform builds, run the multi-platform build more effectively - but still do it with one single process, running in a single github workflow's job?

I'm understanding it as this is an optimization that we can't benefit from if we try to do standalone jobs to build amd64 and arm64 in parallell in different github CI jobs. Is that correct? If so, I'm quite hesitant to increase the complexity further with this strategy.

@romainx
Copy link
Collaborator Author

romainx commented Feb 6, 2022

I'm understanding it as this is an optimization that we can't benefit from if we try to do standalone jobs to build amd64 and arm64 in parallell in different github CI jobs.

@consideRatio if we decide to split the image workflow by platform we will still benefit from this optimization because it permits to build all the images of the stack in a single build instead of launching 8 builds on after the other in a loop. So the build engine can optimize some steps, do stuff in // and save initial start and load times.

If so, I'm quite hesitant to increase the complexity further with this strategy.

I'm not sure it increases the complexity, it's a standard way to define how we build the images.
However I understand your arguments. Optimizing some jobs among a tons that run on GitHub Let's keep it as it is.
Best.

@romainx
Copy link
Collaborator Author

romainx commented Feb 6, 2022

According to your feedbacks I'm closing this PR. Thank you for your time @mathbunnyru and @consideRatio.
Best

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.

3 participants