8000 Client API for importing/exporting OCI images · Issue #831 · containerd/containerd · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Client API for importing/exporting OCI images #831

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

Closed
AkihiroSuda opened this issue May 10, 2017 · 18 comments
Closed

Client API for importing/exporting OCI images #831

AkihiroSuda opened this issue May 10, 2017 · 18 comments

Comments

@AkihiroSuda
Copy link
Member
AkihiroSuda commented May 10, 2017

I'd like to propose a new remote resolver named "filesystem", which allows pulling (and pushing) OCI images from a filesystem.

Filesystem is expected to be a local filesystem, but can be arbitrary network filesystems (e.g. NFS, S3 on FUSE, IPFS on FUSE...)

PR

PR is almost ready, but didn't open yet because this seems to require technical discussion.
master...AkihiroSuda:resolver-filesystem.20170510

Currently, OCI 1.0.0-rc5 is supported via my generic OCI image manipulation library (https://github.com/AkihiroSuda/filegrain/tree/master/image)

Example

$ ls /foo/bar
oci-layout
index.json
...
$ dist pull --resolver filesystem localhost/foo/bar:latest

Concern

  • Since current resolver interface requires "hostname" part, the filesystem resolver always requires localhost/ prefix. This might be weird.
@stevvooe
Copy link
Member

Could you propose the reference syntax in the context of what is described in reference? For example, how is localhost/foo/bar broken down into remote and object? What if the "remote" is not localhost? What happens?

@AkihiroSuda
Copy link
Member Author

reference spec is not changed at all.

If object is not specified reference.Parse returns ErrObjectRequired

If hostname is not localhost filesystemResolver.Resolve returns "host must be localhost" error

@AkihiroSuda
Copy link
Member Author

It might be good to modify ref spec to make hostname optional.

It might be useful for other resolvers as well.
e.g. Native IPFS support dist pull --resolver ipfs /ipfs/QmFooBar

@stevvooe
Copy link
Member

It might be good to modify ref spec to make hostname optional.

Again, the hostname is not a hostname, it is a namespace. We need to think very hard before dropping that requirement.

@AkihiroSuda
Copy link
Member Author

ok, and on second thought, vfs/foo/bar might be better than localhost/foo/bar.
WDYT?
Can I open PR?

@stevvooe
Copy link
Member

Can I open PR?

A WIP PR might be good. I am not sure if we should merge anything yet.

@stevvooe
Copy link
Member

ok, and on second thought, vfs/foo/bar might be better than localhost/foo/bar.

vfs isn't really a namespace. Let's work on finding something more tasteful.

@AkihiroSuda
Copy link
Member Author

How about this (cc @jhowardmsft )

containerd ref spec Mapped path on Unix on Windows
fs/foo/bar /foo/bar
fs/c/foo/bar /c/foo/bar C:\foo\bar
unc/foo/bar \\foo\bar

@lowenna
Copy link
lowenna commented May 12, 2017

This is very confusing to me, I don't know the back-story. Why not just have natural platform semantic syntax? fs/c/foo/bar is meaningless to any Windows user....

@stevvooe
Copy link
Member

@jhowardmsft I agree that this is confusing. He is proposing this because these are image references, not paths.

Frankly, this is why this idea is so insidious: we limit ourselves a layer of indirection, in the same way that docker does today.

This should really require a namespace configuration:

namespace fs/** {
  path = "C:\myimages\
}

The mapping and data are up to the resolver. If the configuration chooses to use fs/c/foo/bar, then we can do allow it but it should be explicit.

Let's reduce the desire to overload. It just creates confusion and trade offs.

@AkihiroSuda
Copy link
Member Author
AkihiroSuda commented May 13, 2017

I think just setting the root dir via resolver config would be enough.

$ ls /localstore/foo/bar
oci-layout
index.json
blobs
$ dist pull --resolver filesystem --filesystem-root /localstore foo/bar:latest
C:\> dir C:\localstore\foo\bar
oci-layout
index.json
blobs
C:\> dist pull --resolver filesystem --filesystem-root C:\localstore foo/bar:latest

@AkihiroSuda
Copy link
Member Author

opened #920

@crosbymichael crosbymichael reopened this Jun 1, 2017
@AkihiroSuda AkihiroSuda changed the title Proposal: new resolver: filesystem API for importing/exporting OCI images Jun 2, 2017
@AkihiroSuda
Copy link
Member Author

Copy-pasting conversation at Slack today

crosbymichael [9:11 AM] 
same thing with your resolver work for a filesystem resolver.  I think you should be able to implement that outside of the codebase and just `client.Pull(ref, suda.WithFilesystemResolver)`

[9:11] 
and if you cannot implement that outside , we need to see if its a flaw in our interface

akihirosuda [9:12 AM] 
ok

[9:14] 
Instead of filesystem resolver, is it ok to implement `dist image import --type oci /path/to/oci-image-dir latest` and `dist image export ...` (as a dev tool)?

crosbymichael [9:15 AM] 
i think that makes sense to use it as an import/export format

[9:15] 
maybe add that to the client in the image type?  `archive, err := image.Export(OCIFormat)`

[9:16] 
or can can pass different formatters for various things.  it would be better in the client to return readers instead of taking a path and writing to disk

[9:16] 
that way consumers to export containers/images over an api using a reader

akihirosuda [9:18 AM] 
makes sense

@crosbymichael
Copy link
Member

We can probably handle all of this client side.

@AkihiroSuda AkihiroSuda changed the title API for importing/exporting OCI images Client API for importing/exporting OCI images Jun 5, 2017
@AkihiroSuda
Copy link
Member Author

opened #1013

@AkihiroSuda
Copy link
Member Author

updated #1013 to support Import() as well

@crosbymichael
Copy link
Member

@AkihiroSuda can this be closed now?

@AkihiroSuda
Copy link
Member Author

yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
0