buildpacks / lifecycle

Reference implementation of the Cloud Native Buildpacks lifecycle
https://buildpacks.io
Apache License 2.0
186 stars 106 forks source link

Run image extension + export to OCI layout should work together #1102

Open natalieparellano opened 1 year ago

natalieparellano commented 1 year ago

Summary

We don't allow this today because there were some issues using the imgutil sparse.Image with run image extension. This shouldn't be an insurmountable problem, but requires investigation.

natalieparellano commented 8 months ago

The issue with the imgutil/sparse package should be fixed in https://github.com/buildpacks/lifecycle/pull/1281. I believe all that needs to be done to enable run image extension with OCI layout in the lifecycle is remove the guards here:

func (r *restoreCmd) supportsRunImageExtension() bool {
    return r.PlatformAPI.AtLeast("0.12") && !r.UseLayout 
}

and here:

func (e *exportCmd) supportsRunImageExtension() bool {
    return e.PlatformAPI.AtLeast("0.12") && !e.UseLayout 
}

However, I think the bulk of the work actually needs to be done on the platform side, such as in pack. Currently, when we expect the app image to be written in OCI layout format on disk, we prepare the build inputs (in particular, the run image) as OCI layout (sparse or not sparse, depending on end user preference). But in order to do run image extension, we need to run the extender in the context of the run image, so we need the run image to exist in the daemon (not sparse). I think there are a couple flows we need to consider:

In either case, we'll need to provide an image with a manifest to (at least) the restorer. See here where pack is pulling the run image (always from a registry) when run image extension is required. Probably the simplest approach would be to always provide a registry reference for the run image to the restorer, even when we are using OCI layout (a similar requirement for the builder already exists when build image extension is required). This places some limitations around the feature but would allow us to ship something that will probably work for most use cases. Anyway, we need to decide what we are doing in pack and then the lifecycle change will be relatively simple.

@jjbustamante do you have thoughts here?

jjbustamante commented 8 months ago

@natalieparellano thinking about this from pack perspective, I think the weird case is:

I think today when a user wants to export to OCI layout, we assumed all the required images exists in a registry or the daemon, and pack is taking care of putting them on disk for the lifecycle. I don't think there is a way for users to run pack build and export to OCI layout using a run-image that only exists on disk.

I think that if you want to extend a run image, should be obvious for the user that we need to apply some changes to the original image and it must be present into the daemon. My vote will be to avoid trying to support this case, and probably provide good error messages to end users so they can actually save their image in the daemon or registry by themself, if they do it, then we will be in the first case:

  • If the run image exists in a registry, we need to pull it into the daemon AND save it as OCI layout (sparse or not sparse)
    • In this case, we'll get the manifest from the registry

right?