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

Proposal for adding commands to handle manifest list in pack #283

Merged
merged 13 commits into from
Sep 28, 2023

Conversation

jjbustamante
Copy link
Member

@jjbustamante jjbustamante commented Apr 19, 2023

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>
@buildpack-bot
Copy link
Member

Maintainers,

As you review this RFC please queue up issues to be created using the following commands:

/queue-issue <repo> "<title>" [labels]...
/unqueue-issue <uid>

Issues

(none)

@jjbustamante jjbustamante marked this pull request as draft April 19, 2023 17:42
jjbustamante and others added 3 commits April 20, 2023 08:11
Co-authored-by: Aidan Delaney <adelaney21@bloomberg.net>
Signed-off-by: Juan Bustamante <jbustamante@vmware.com>
Signed-off-by: Juan Bustamante <jbustamante@vmware.com>
Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

Why should we *not* do this?

We should decide to do not add this feature and users could use tools like `docker` or `podman` to create and handle their manifest list, however this is a poor user experience forcing users to use different tools to fulfill scenarios that are part of the business domain of pack.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would agree with this if it required additional tools to be installed. If you're using pack, you have docker or podman already though, so I'm not sure it's really that big of a deal. Just my $0.02.

@dmikusa
Copy link
Contributor

dmikusa commented Apr 24, 2023

One thing that has been a little weird working with multi-arch support and image indexes is that the different images do not have to be the same for each arch. My amd64 image can be quite different from the arm64 image. I'm not entirely sure how I feel about that.

In terms of buildpacks, I feel like it seems logical that the two could be different. Maybe a particular buildpack doesn't function on arm64 (maybe there are just no binaries that it needs)? So you don't include that one.

The problem is that I think users will expect them to be the same, and I don't think there is presently a good way for users to interrogate and see what's in each architecture. If you pack buildpack inspect an image, it's not clear what it shows exactly (I'm guessing my native architecture but maybe just the first one in the image?). Anyway, I think we need to make it clear what's being show in the inspect related commands. I also think we'll probably need an arch flag to specify a non-native arch if you want to see what's included in a different arch.

I'm not 100% sure that's relevant here, but I wanted to mention this as it was a sticking point with multi-arch images.

@jjbustamante jjbustamante self-assigned this Apr 24, 2023
Signed-off-by: Juan Bustamante <jbustamante@vmware.com>
Signed-off-by: Juan Bustamante <jbustamante@vmware.com>
Signed-off-by: Juan Bustamante <jbustamante@vmware.com>
@jkutner
Copy link
Member

jkutner commented Jul 14, 2023

Is this ready for review?

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>
@jjbustamante
Copy link
Member Author

One thing that has been a little weird working with multi-arch support and image indexes is that the different images do not have to be the same for each arch. My amd64 image can be quite different from the arm64 image. I'm not entirely sure how I feel about that.

In terms of buildpacks, I feel like it seems logical that the two could be different. Maybe a particular buildpack doesn't function on arm64 (maybe there are just no binaries that it needs)? So you don't include that one.

The problem is that I think users will expect them to be the same, and I don't think there is presently a good way for users to interrogate and see what's in each architecture. If you pack buildpack inspect an image, it's not clear what it shows exactly (I'm guessing my native architecture but maybe just the first one in the image?). Anyway, I think we need to make it clear what's being show in the inspect related commands. I also think we'll probably need an arch flag to specify a non-native arch if you want to see what's included in a different arch.

I'm not 100% sure that's relevant here, but I wanted to mention this as it was a sticking point with multi-arch images.

@dmikusa

I just added this to try to address this requirement, thanks for your feedback an let me know how it looks!

@jjbustamante jjbustamante marked this pull request as ready for review July 17, 2023 16:28
@jjbustamante
Copy link
Member Author

Is this ready for review?

Hi @jkutner !

Yes, I think it is ready for a review, it is not perfect but I put most of the stuff I had in mind for these new set of commands. I will be taking some days off (3 weeks) but Jerico or Husni will follow up on this, the idea is to get all the feedback as possible.

cc @drac98 @jericop @AidanDelaney

@natalieparellano natalieparellano added team/platform scope/team RFC scoped to a sub-team as opposed to the entire project. type/rfc status/steward-assigned labels Aug 2, 2023
@chenbh
Copy link

chenbh commented Aug 18, 2023

I'm not a fan of introducing such low level concepts like a manifest to buildpack authors. If all I want to do is to get my image working on popular platforms (linux/amd64, linux/arm64, windows/amd64), I shouldn't have to learn what a manifest is, how to add images to manifests, and how to annotate each image on the manifests with the os/arch. I don't see a point of doing this through pack when there's plenty of other more dedicated options for it (as you said, docker, podman, and crane).

What I would like to see is for pack to abstract away a lot of the nitty gritty and provide a flow tailored to multi-platform images:

Create an (empty) image index

$ pack buildpack multi-platform create <registry.com/multiarch-buildpack[:tag]>
-> Image index created at registry.com/multiarch-buildpack@sha256:1234

Images are added to it while specifying the os and arch

$ pack buildpack multi-platform append --image <registry.com/linux-buildpack> --os linux --arch amd64 <registry.com/multiarch-buildpack[:tag]>
-> linux/amd64 image added to registry.com/multiarch-buildpack@sha256:5678
$ pack buildpack multi-platform append --image <registry.com/windows-buildpack> --os windows --arch amd64 <registry.com/multiarch-buildpack[:tag]>
-> windows/amd64 image added to registry.com/multiarch-buildpack@sha256:abcd

One downside(?) with this approach is that these instructions actually generate an image index for each step, which means we end up with 3 indexes at the end. I don't think this actually matters because I think most registry garbage collection is based on tags and not digests/manifests.

EDIT:
Note that these instruction are operating on the remote registry directly, there's no local manifest file that then has to be pushed to the registry. I prefer this approach because

a. manifests and indexes are cheap, there's very little downside to creating these
b. the end goal of this sequence of operations is to push the manifest anyways, there's nothing to gain by batching the operations together
c. because we do the blob/layer relocation at each pack multi-platform append step, it's easier to retry if something goes wrong

@chenbh
Copy link

chenbh commented Aug 18, 2023

I would also like to align on how we should call it, Docker calls it a manifest list, while OCI calls it an image index. They're pretty much identical, but I wonder if we should pander to the vendor-neutral OCI or the more popular Docker.

@jjbustamante
Copy link
Member Author

@chenbh Thank you very much for this awesome feedback!!!

Definitly it is a very interesting user experience the one you are proposing! I think it worth to be discuss.

@husni-faiz , @jericop , @AidanDelaney any thoughts about it? I think we will end up with the same outcome for users but with a very different flow.

@jjbustamante
Copy link
Member Author

jjbustamante commented Aug 31, 2023

A quick note from our conversation during the WG today

  • We like the idea of the abstraction suggested by Daniel. It is something we see possible to develop in the future.
  • We want to keep moving forward with the RFC proposal, for us, they are like primitives commands to start and because they are experimental, we are free change them in the future.
  • We can think of the following: we are proposing pack manifest create commands, Daniel suggest pack buildpack multi-platform (or something else, Daniel just made up that one), but we can find a good name for the command that could include the current subcommands proposed in the RFC an a future subcommand for Daniel suggested behavior.

Open to feedback on what could be that name!!

cc @husni-faiz @jericop @AidanDelaney @chenbh

@hone
Copy link
Member

hone commented Sep 6, 2023

@jjbustamante once we have said future abstractions, do you see us removing this lower level primitive commands since many other tool support manipulating the image index/manifests like @chenbh / @dmikusa has suggested.

@jjbustamante
Copy link
Member Author

@jjbustamante once we have said future abstractions, do you see us removing this lower level primitive commands since many other tool support manipulating the image index/manifests like @chenbh / @dmikusa has suggested.

If we can hide the complexity of the primitives command behind the scene with a better user experience as Daniel suggest. I think in the future we can remove those commands, but probably the code will still be necessary to implement the abstraction. That's something the community can guide us, right? if the abstraction is so good that it fills most the scenarios, maybe the primitives will not be required anymore.

@jjbustamante
Copy link
Member Author

@hone, @chenbh

Thinking a little bit about this, right now pack creates and push OCI artifacts with 3 commands. pack build, pack builder create and pack buildpack package. We know the primitives suggested in this RFC are just a first step in the multi-arch journey.

Based on the scenario proposed by Daniel, what about this idea:

Propose adding flags like --platform and --append to our current 3 commands mentioned above.

For example:

Let's take the Paketo Java Buildpack gcr.io/paketo-buildpacks/java, it is distributed without an Image Index and with os=linux and architecture=""

Let's suppose we already defined on how a multi-arch buildpack must be structured and the buildpacks authors execute:

pack buildpack package  --append --platform linux/amd64,linux/arm64 --publish gcr.io/paketo-buildpacks/java

We should expect:

  • Pack will create two OCI artifacts, one for each platform specified.
  • Pack will create an Image Index and adds the two OCI artifacts to that index
  • Pack should append the current manifest and replace it with the Image Index?

Similar to the buildpack package, we can add similar behavior to the other two commands

pack build --platform linux/amd64,linux/arm64 --publish
pack builder create --platform linux/amd64,linux/arm64 --publish

In the reference doesn't exists in the registry, we can create the Image Index by default or maybe also add a flag? I think an idea in this direction could be next steps after these primitives

@hone
Copy link
Member

hone commented Sep 13, 2023

@jjbustamante are you still planning on moving away from manifest to another name?

@jjbustamante
Copy link
Member Author

@jjbustamante are you still planning on moving away from manifest to another name?

I haven't thought of a different name honestly. I am okay with keeping it. Again, the idea is to keep working on this and open a new RFC to start discussing the idea of:

pack build --platform linux/amd64,linux/arm64 --publish
pack builder create --platform linux/amd64,linux/arm64 --publish

@hone
Copy link
Member

hone commented Sep 13, 2023

@hone, @chenbh

Thinking a little bit about this, right now pack creates and push OCI artifacts with 3 commands. pack build, pack builder create and pack buildpack package. We know the primitives suggested in this RFC are just a first step in the multi-arch journey.

Based on the scenario proposed by Daniel, what about this idea:

Propose adding flags like --platform and --append to our current 3 commands mentioned above.

For example:

Let's take the Paketo Java Buildpack gcr.io/paketo-buildpacks/java, it is distributed without an Image Index and with os=linux and architecture=""

Let's suppose we already defined on how a multi-arch buildpack must be structured and the buildpacks authors execute:

pack buildpack package  --append --platform linux/amd64,linux/arm64 --publish gcr.io/paketo-buildpacks/java

We should expect:

* Pack will create two OCI artifacts, one for each platform specified.

* Pack will create an Image Index and adds the two OCI artifacts to that index

* Pack should **append** the current manifest and replace it with the Image Index?

Similar to the buildpack package, we can add similar behavior to the other two commands

pack build --platform linux/amd64,linux/arm64 --publish
pack builder create --platform linux/amd64,linux/arm64 --publish

In the reference doesn't exists in the registry, we can create the Image Index by default or maybe also add a flag? I think an idea in this direction could be next steps after these primitives

This looks great and fits more inline with what I would expect most people to use. Definitely a lot of things to work through. Just some of the questions that are coming to my head:

pack build --platform linux/amd64,linux/arm64 --publish

does this execute two different builds (runs through lifecycle)?

pack buildpack package  --append --platform linux/amd64,linux/arm64 --publish gcr.io/paketo-buildpacks/java

What are the intermediate images for each platform named/called? What happens if you don't put --append here?

@jjbustamante
Copy link
Member Author

This looks great and fits more inline with what I would expect most people to use. Definitely a lot of things to work through. Just some of the questions that are coming to my head:

Hi @hone.

I just created this tracking issue with my idea for keep working on this, we going into phase 2 and I will try to answer your question:

pack buildpack package  --append --platform linux/amd64,linux/arm64 --publish gcr.io/paketo-buildpacks/java

What are the intermediate images for each platform named/called? What happens if you don't put --append here?

In the next RFC I will open.

Regarding this other one:

pack build --platform linux/amd64,linux/arm64 --publish

does this execute two different builds (runs through lifecycle)?

Will be something to think on phase 3

@jkutner jkutner merged commit 4be3186 into main Sep 28, 2023
7 checks passed
@jkutner jkutner deleted the jjbustamante/feature/pack-manifest-list-commands branch September 28, 2023 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope/team RFC scoped to a sub-team as opposed to the entire project. status/voting team/platform type/rfc
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

10 participants