-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Refactor image builds to use buildx for multi arch image building #2754
Conversation
Thanks for this @RobReus! I'll look at it in more detail later today. For now, can you double check to make sure that all changed files have their copyright year set to 2020? |
Signed-off-by: Rob Reus <rob@devrobs.nl>
@nrb I have updated the copyright of the files I touched with this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RobReus Thank you for picking this up!
The changes makes sense to me. But I've not been able to build a container from this PR.
Can you please clarify the buildx installation and also the platforms you've been able to build this in?
Signed-off-by: Rob Reus <rob@devrobs.nl>
Signed-off-by: Rob Reus <rob@devrobs.nl>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RobReus Can you also please add a changelog file?
https://github.com/vmware-tanzu/velero/pull/2754/checks?check_run_id=907666453
Signed-off-by: Rob Reus <rob@devrobs.nl>
Signed-off-by: Rob Reus <rob@devrobs.nl>
@ashish-amarnath I have updated the contribution docs on building the container images. I have also implemented the feedback given. Let me know if there's anything else you want to discuss or see changed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks good, thanks a ton @RobReus!
I have one more big thing to address, and I think this would be ready for merge.
Signed-off-by: Rob Reus <rob@devrobs.nl>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Thank you so much for this change and your patience!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RobReus Thank you for the PR! 🎉
…ware-tanzu#2754) * Refactor image builds to use buildx for multi arch image building Signed-off-by: Rob Reus <rob@devrobs.nl> * Adding image build sanity checks to Makefile Signed-off-by: Rob Reus <rob@devrobs.nl> * Making locally building of docker images possible Signed-off-by: Rob Reus <rob@devrobs.nl> * Adding docs on building container images using buildx Signed-off-by: Rob Reus <rob@devrobs.nl> * Adding changelog and implementing feedback from PR Signed-off-by: Rob Reus <rob@devrobs.nl> * Making GOPROXY used in the build containers configurable Signed-off-by: Rob Reus <rob@devrobs.nl>
…ware-tanzu#2754) * Refactor image builds to use buildx for multi arch image building Signed-off-by: Rob Reus <rob@devrobs.nl> * Adding image build sanity checks to Makefile Signed-off-by: Rob Reus <rob@devrobs.nl> * Making locally building of docker images possible Signed-off-by: Rob Reus <rob@devrobs.nl> * Adding docs on building container images using buildx Signed-off-by: Rob Reus <rob@devrobs.nl> * Adding changelog and implementing feedback from PR Signed-off-by: Rob Reus <rob@devrobs.nl> * Making GOPROXY used in the build containers configurable Signed-off-by: Rob Reus <rob@devrobs.nl>
…ware-tanzu#2754) * Refactor image builds to use buildx for multi arch image building Signed-off-by: Rob Reus <rob@devrobs.nl> * Adding image build sanity checks to Makefile Signed-off-by: Rob Reus <rob@devrobs.nl> * Making locally building of docker images possible Signed-off-by: Rob Reus <rob@devrobs.nl> * Adding docs on building container images using buildx Signed-off-by: Rob Reus <rob@devrobs.nl> * Adding changelog and implementing feedback from PR Signed-off-by: Rob Reus <rob@devrobs.nl> * Making GOPROXY used in the build containers configurable Signed-off-by: Rob Reus <rob@devrobs.nl>
…ware-tanzu#2754) * Refactor image builds to use buildx for multi arch image building Signed-off-by: Rob Reus <rob@devrobs.nl> * Adding image build sanity checks to Makefile Signed-off-by: Rob Reus <rob@devrobs.nl> * Making locally building of docker images possible Signed-off-by: Rob Reus <rob@devrobs.nl> * Adding docs on building container images using buildx Signed-off-by: Rob Reus <rob@devrobs.nl> * Adding changelog and implementing feedback from PR Signed-off-by: Rob Reus <rob@devrobs.nl> * Making GOPROXY used in the build containers configurable Signed-off-by: Rob Reus <rob@devrobs.nl>
…ware-tanzu#2754) * Refactor image builds to use buildx for multi arch image building Signed-off-by: Rob Reus <rob@devrobs.nl> * Adding image build sanity checks to Makefile Signed-off-by: Rob Reus <rob@devrobs.nl> * Making locally building of docker images possible Signed-off-by: Rob Reus <rob@devrobs.nl> * Adding docs on building container images using buildx Signed-off-by: Rob Reus <rob@devrobs.nl> * Adding changelog and implementing feedback from PR Signed-off-by: Rob Reus <rob@devrobs.nl> * Making GOPROXY used in the build containers configurable Signed-off-by: Rob Reus <rob@devrobs.nl>
…ware-tanzu#2754) * Refactor image builds to use buildx for multi arch image building Signed-off-by: Rob Reus <rob@devrobs.nl> * Adding image build sanity checks to Makefile Signed-off-by: Rob Reus <rob@devrobs.nl> * Making locally building of docker images possible Signed-off-by: Rob Reus <rob@devrobs.nl> * Adding docs on building container images using buildx Signed-off-by: Rob Reus <rob@devrobs.nl> * Adding changelog and implementing feedback from PR Signed-off-by: Rob Reus <rob@devrobs.nl> * Making GOPROXY used in the build containers configurable Signed-off-by: Rob Reus <rob@devrobs.nl>
Fixes #2466 , Fixes #2486 , Closes #2566
This change introduces buildx to build multi arch container images.
Only the container image building is affected by this change, all other functionality (including locally building) remains the same.
Would love to get some feedback on this. We would probably improve it further to reduce build times. Right now the GitHub workflow takes about the same amount of time as before, even though images are now built in parallel. I noticed that cross compiling within the new way of doing it went from 90 seconds to around 300 seconds. I feel like we can win plenty of time by making the cross compile of the velero binary more efficient.
Succesful workflow on my fork: https://github.com/RobReus/velero/actions/runs/180022436 (most recent ones failed due to my credentials now having access to the velero registry).