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

Replace tarball.LayerFromOpener with stream.NewLayer #413

Open
jonjohnsonjr opened this issue Mar 19, 2019 · 1 comment
Open

Replace tarball.LayerFromOpener with stream.NewLayer #413

jonjohnsonjr opened this issue Mar 19, 2019 · 1 comment

Comments

@jonjohnsonjr
Copy link
Collaborator

The tarball package has methods for reading/writing images from/to the docker save tarball format. This aligns with how the other packages are organized, for example:

  • remote is for reading/writing from/to registries.
  • daemon is for reading/writing from/to container daemons.
  • layout is for reading/writing from/to OCI image layouts.

However, the tarball package also has methods for constructing v1.Layers from a file or from an Opener. It kind of makes sense to be in the tarball package because the layers happen to be compressed tarballs, but really that's an implementation detail. These functions aren't for tarball-specific layers, they're just for layers in general.

We should try to replace these with stream.Layer wherever we can. The tarball.Layer currently reads the whole tarball twice because it eagerly computes the digest/size/diffid. For uncompressed layers, stream.Layer is just better.

For compressed layers, we usually have some metadata that makes eagerly computing those things unnecessary (e.g. remoteLayer). For reading compressed layer from a tarball.Image, we should know the diffids from the config file.

tl;dr LayerFromOpener is bad, let's make stream.Layer work. I elaborated on this a bit here.

@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

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

1 participant