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

Update platform spec to improve performance when restoring launch sboms from daemon #276

Closed
natalieparellano opened this issue Dec 10, 2021 · 3 comments

Comments

@natalieparellano
Copy link
Member

In implementing RFC 0095, we inadvertently made rebuilds much slower on a daemon when the previous image has a launch sbom. For launch layers, the sbom files must be restored to the buildpack layers directory (just like layer.toml files) on the next build; however the sbom files are stored in the previous image's filesystem (instead of labels in the config).

As a result, we need to download from the daemon a single layer from the previous image. However we didn't realize that in order to download a single layer from the daemon, you need to first download ALL layers from the image in order to find the one that you want (see imgutil implementation).

To fix this, we need to pass the -launch-cache to the phase that downloads the sbom. This is currently the analyzer (because the analyzer already has access to the previous image) but we think it would make more sense for this to move to the restorer, so that detect can fail faster if it is going to fail. Therefore we'll also need to add a -daemon flag to the restorer (we only allow cache images if they are on a registry, so this flag doesn't exist yet for restorer). This will require platforms like pack that use the daemon to mount in the daemon socket for restore.

@natalieparellano
Copy link
Member Author

Option 1:
Add to analyzer:

  • -launch-cache flag
  • -skip-layers flag
    Philosophy: analyzer does "previous image things", restorer does "cache things"

Option 2:
Add to restorer:

  • -daemon flag
  • -launch-cache flag
    Philosophy: consolidate as much logic as possible in the restorer; analyzer does as little as possible

I'm a bit partial to option 1 as it's slightly less work for platforms and the lifecycle to implement. But in theory either would work.

Note that the performance issue can be fixed for the creator (trusted builder) workflow without platform api changes, see buildpacks/lifecycle#784.

@hone
Copy link
Member

hone commented Dec 15, 2021

My vote is for Option #1 as well.

@jromero
Copy link
Member

jromero commented Apr 13, 2022

It looks like this should be closed as per #278. Please re-open if I'm mistaken.

@jromero jromero closed this as completed Apr 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants