-
Notifications
You must be signed in to change notification settings - Fork 297
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
Implementation of multi-arch support for extension packages #2320
base: main
Are you sure you want to change the base?
Conversation
dcf618a
to
7622d59
Compare
Signed-off-by: Costas Papastathis <papastathiscr@gmail.com>
…ension .toml file Signed-off-by: Costas Papastathis <papastathiscr@gmail.com>
Signed-off-by: Costas Papastathis <papastathiscr@gmail.com>
Signed-off-by: Costas Papastathis <papastathiscr@gmail.com>
Signed-off-by: Costas Papastathis <papastathiscr@gmail.com>
Signed-off-by: Costas Papastathis <papastathiscr@gmail.com>
98cc239
to
3137310
Compare
@pacostas Thank you so much for this work!!! I will review it tomorrow!!! |
You are welcome, feel free to make any suggestions! :) |
@jjbustamante Hello! With respect to your time, did you have the chance to take a look on the implementation? |
@pacostas I am so sorry I haven't reviewed your PR, I will be pulling it and testing it locally, also, we are evaluating the current state of Kaniko because we are not sure how it will affect us. We use Kaniko for our extension implementation. |
Thank you very much :) Thank you for the information about kaniko, do you know if kaniko is used somewhere else except extension and if there is a discussion/thread about that issue (to be in the loop)? |
We only use Kaniko for extensions, we discussed it during our Working Group meeting on December 12th, but the recording is not on Youtube 😞 |
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.
Hi @pacostas
Thanks for working on this! I apologize for the late review, the PR is fine, I added a few nits.
One doubt, do you also want to add support for extensions added to a builder?
…rdcoded values Signed-off-by: Costas Papastathis <papastathiscr@gmail.com>
Signed-off-by: Costas Papastathis <papastathiscr@gmail.com>
c002ca3
to
912e3a8
Compare
Thank you for your review :)
good point :) I havent tested it with a builder, as there are a lot of moving parts on the multi-arch side. Do you think we could leave it for now? And I can follow with another PR once things have settled. |
One test that is failing, I think is not related to this PR. By re-running, I think it will pass. |
Yeah, I need to prune some networks in our windows runner to fix it |
I thought I fixed this! So sad |
@pacostas Thanks for doing the change, the only reason stopping me from merging the change is just because I want to give it a try before merging it! The PR is ready to be merged. @natalieparellano I am not sure if you have the time, but I thought that maybe we could update the samples repo with the multi-arch version for the extension and test them with this branch. Is it something you can help me? updating the samples repo with multi-arch extensions? |
@jjbustamante no rush, thank you very much :) BTW I have tested it with this extension.toml, https://github.com/paketo-buildpacks/ubi-nodejs-extension/pull/273/files#diff-21e42dd2276e594017780607a13f07b058d020447a73c11f032c77823a8ed493 I hope the code is useful for testing it with the samples too. |
Sure! |
Summary
This implementation is based on the RFC https://github.com/buildpacks/rfcs/blob/main/text/0128-multiarch-builders-and-package.md
This PR allows the
pack extension package
to generate multi-platform OCI images and create an image index to combine them.It also adds the
--path
flag to point to a directory on the extension that needs to be packaged.This PR has not been tested in packaging a builder with multi-platform extension.
Output
Before
After
--target
flag--path
flag.Documentation
Related
Resolves #2183
Resolves #2160