There is no pooling concept for this OCI resolver. Should directly implement object that implements the Resolver with the Fetcher method. No other methods or inputs are needed.

Even better. That will make the whole GetOCILayoutResolver concept much simpler. I will update.

Done. Completely in parallel here with the source. We can move it to a util, if you want.

Why is this duplicated? Looks very similar.

It is, but some of the parameters are different. I planned to try to get it more concise, but didn't care too much for first review of general direction.

I managed to get it a bit more DRY.

This input isn't necessarily a reference. It should be a name that the client exposes and that can be pulled in.

Yeah, that is a good point. It should be a path to somewhere, like oci-layout://var/layouts/setup

Removed.

oci-layout might be better. docker-image can also be OCI mediatype image.

We already have --output type=oci, so oci might be fine.

Or we might want to consider renaming --output type=oci to --output type=oci-layout too.

Or oci-archive maybe

Atm. this string isn't even visible to the user so some verbosity should be fine in here.

--output type=oci gives an OCI tarball. It is the same format, except in a tarball. Which leads to the question of whether we should support tarball input as well as directory (and maybe directory layout in addition to tarball).

It is much easier to do tarball, because it is a stream, no need to worry about access to local directories.

I don't think they all need to be solved now, but we should get the nomenclature correct so we have options in the future without breaking things.

We can keep just a single scheme/type oci (or oci-layout, if we prefer) for all of it, with the below:

• source directory: oci://path/to/layout/dir
• source tar file: oci://path/to/tarball.tar (future, would depend on if it is a file of type tar or not)
• source tar stream: oci:// or oci://- or even oci://stdin
• output directory: --output type=oci,dest=path/to/layout/dir
• output tar file: --output type=oci,dest=path/to/output.tar
• output stdout: --output type=oci

The only really weak point there is output directory, as it might not exist, so how do we know if they want a tar file or a layout directory? We could say, "if dest ends in .tar, we will write a tar file, else a directory, and if you really want to write to a tar file that does not end in .tar, then write to stdout and save the output as you wish.

Might need to change it to remotes.Resolver. Or if image requires resolver.Resolver (not sure atm where) then can add remotes.Resolver as a second option and make oci-layout use that.

As an interim step, I created an interface in the same package that both util/resolver/pool.Resolver struct and source/containerimage/OCILayoutResolver struct satisfy. It should not be final, but lets us focus it in one place (and out of the pool resolver package), whence we can clean it up.

The only difference between what our implementations of Resolver do and remotes.Resolver are:

type Resolver interface {
	remotes.Resolver
	HostsFunc(host string) ([]docker.RegistryHost, error)
	// WithSession returns a new resolver that works with new session group
	WithSession(s session.Group) Resolver

	// WithImageStore returns new resolver that can also resolve from local images store
	WithImageStore(is images.Store, mode source.ResolveMode) Resolver
}

I have not figured out yet where/how that gets used. If we can get around it, this whole thing does simplify to remotes.Resolver.

And, it doesn't quite work, because those funcs return *util/resolver.Resolver (pointer to a struct) and not util/pull.Resolver (interface). It satisfies the interface requirements, but that doesn't make the signature identical.

I have not figured out yet where/how that gets used. If we can get around it, this whole thing does simplify to remotes.Resolver.

So:

• HostsFunc is not used externally, so that is easy
• WithImageStore only ever is used by something that knows it is calling resolver.Resolver or resolver.OCILayoutResolver, so that is easy
• WithSession is the one that gets called from within util/pull.

I still had to change it a bit, including util/resolver, but minimally to comply.

WithSession is used for attaching authentication. Do not need it as you are not providing credentials for accessing OCI layout.

That should make it easier. But it is called here, from within util/pull/pull.go, as part of PullManifests().

Actually, it is used inside a func as a property of PulledManifests:

	return &PulledManifests{
		Ref:              p.ref,
		MainManifestDesc: p.desc,
		ConfigDesc:       p.configDesc,
		Nonlayers:        p.nonlayers,
		Descriptors:      p.layers,
		Provider: func(g session.Group) content.Provider {
			return &provider{puller: p, resolver: p.Resolver.WithSession(g)}
		},
	}, nil

I cannot quite figure out where that is consumed. Either way, is it not easier to just have a WithSession() where the remote registry puller cares, and the local registry ignores it?

You should be able to get the provider directly from content. You might need to pass in "registry-mode" bool or smth like this so you know which type to use.

get the provider directly from content

I don't understand.

When you have an implementation that implements Resolver (or Fetcher or Provider) based on client side contents that is all you need to get content.Provider out of it.

There is an additional question in here about do we want the lazy pull behavior for these layers. For registry images we avoid pulling the layers until the content is really needed(this is why there is this callback here). For OCI case I think the lazy behavior doesn't make quite sense and may cause more harm than benefits. Still I think this pullmanifest may remain as is, with just a different content.Provider callback implementation but we might want to just call Unlazy() for this reference directly before returning from Snapshot() @sipsma

@tonistiigi I think I get what you are after. Take a look at the latest. I made Puller reference just a remotes.Resolver:

type Puller struct {
	ContentStore content.Store
	Resolver     remotes.Resolver
	Src          reference.Spec
...
}

and then PullManifests() takes an arg to get a resolver passing it a session. This allows us to keep it all within containerimage/pull.go, where it knows if it is OCI or registry, and therefore can call WithSession() on registry, or ignore it on OCI.

Still a bit convoluted, but much cleaner than before.

Is this what you were suggesting?

Do not make changes to this file. This package is for registry connection pooling. It doesn't have anything to do with OCI layout parsing.

Sure, once you made it clear above that pooling is only for that, no big deal.

No idea what this is supposed to be. I can't understand what is passed in here and the PR does not seem to even build. You need to pull in the blobs from the client side through a session API that is exposed by the client. Not allowed to read any files from host disk based on user input. Also local.NewStore is not OCI layout but containerd contentstore boltdb layout.

the PR does not seem to even build

I didn't expect it to. This is just for design review, to see that we are moving in the right direction and take course corrections.

No idea what this is supposed to be. I can't understand what is passed in here

I largely copied what the existing resolver from the registry does. Instead of a ref, I passed where the expected oci-layout directory is.

Also local.NewStore is not OCI layout but containerd contentstore boltdb layout.

I know. But it happens to be oci-layout compliant except for the stuff that goes in boltdb (image to hash mapping, etc.). This way, we inherit the reading of hashes from containerd's library. I could write it from scratch, but why bother? It does all of the reading, validation, etc. And buildkit already imports containerd packages.

You need to pull in the blobs from the client side through a session API that is exposed by the client. Not allowed to read any files from host disk based on user input.

This is precisely what I punted on (see the PR opening comment). I wasn't sure how we do that, as buildkit runs potentially on a different host than the client.

How does local source do that? Does the client recognize the option, mount the directory, and stream it to buildkitd? Can you share a link to consuming that session API?

Client exposes service on session with s.Allow https://github.com/moby/buildkit/blob/master/client/solve.go#L127 . Daemon side gets the caller for it https://github.com/moby/buildkit/blob/master/source/local/local.go#L98-L101 .

For the contentstore this already exists for local cache import https://github.com/moby/buildkit/blob/master/client/solve.go#L161 so basically you need to attach your contentstore to this same map. Then pass the sessionID and digest with the LLB definition so the daemon side can provide it as input when accessing the session service.

Not sure I get this, let's try.

• when the client sets up grpc to the daemon, the internal structure that represents that communication is a session
• the client "allows" various client-side services to be accessed by the daemon via s.Allow(service), where the filesystem sync service (aka "get things from client filesystem") is "allowed" here
• anything daemon side that wants to consume one of those services, e.g. local or our new OCI needing access to client-side filesystems, needs to get a caller for that session and service, e.g. local gets the specific session based on ID here, and then calls ls.snapshot() using that session here, which appears to do all of the syncing up.

A lot still isn't clear to me:

• why do we have both this s.Allow() and this one? One uses a specific map, another syncedDirs, which seems to come from opt.LocalDirs. Is one a general list of dirs mapped in (like docker run -v ...) and the other ones that come from options (like --build-context= or --cache-export)?
• somewhere, it must parse the options (e.g. --build-context), find which are local (right now just filesystem path, but also oci) and add them to the cacheOpt.contentStores; where does it do that, so I can add the build-context oci://?
• once this is in place, then the OCIResolver is just going to need to handle the session and resolve content from there? Does the filesync just allow daemon-side processes use session to list files, walk directories, and read content via the session almost as if it was local?

I think I got some of the daemon-side. I haven't figured out how to read the data, but the setting up of the session I just took from local and adapted.

I still need to figure out how to get an io.ReadCloser for the file I want from the session. It looks like it has to do with filesync.FSSync() like here for local? Not sure.

If so, that looks like it syncs everything from the remote to the local in some tmp dir. Is there any way to ask it, "here is a path, stream me the file contents"? Is there an example of that?

Can you help explain the above @tonistiigi ? I hit another wall.

Do I just need to create a content.Store that accesses the local filesystem path and pass it?

yes

I don't see documented anywhere how the docker buildx concept of build-context translates into something that buildkit understands.

It's in Dockerfile frontend. Read #2827 (comment)

what does the --build-context foo=oci-layout://... get translated to in terms of buildkit options? What does it look like?

Client converts the local directory to contentstore(and finds the target digest). Add OCIArchives map[string]content.Store to https://pkg.go.dev/github.com/moby/buildkit@v0.10.2/client#SolveOpt . In FrontendOpts/FrontendAttr buildx would set set context:foo=oci-layout://contentstoreID/image-digest. Handler for the FrontendOpts in Dockerfile should be in a later PR(and --build-context is in buildx). Only LLB support for now.

where does that get parsed, such that I can extract the path and pass the local content store into contentStores?

Only buildx implements this flag. If we want buildctl support then the flag for that would be something like --oci-archive digest=./path/to/store similar how buildctl exposes --local. But for me just Go LLB library support for this is ok for now.

where does the image name side of it

No idea what you mean in here. Maybe answered above.

how does it translate the image name into the root manifest? An OCI layout can have many manifests.

This is for the client side to decide when it parses --build-context value. For the daemon, the only inputs are contentStoreID and digest. Multiple digests could come from same contentstore.

Going to give this a shot @tonistiigi ; tell me what I got right and wrong.

• the daemon is responsible for the build itself. It doesn't understand dockerfile or other frontends; it gets an LLB via a SolveRequest and processes that.
• the client is responsible for parsing the frontend (e.g. Dockerfile) into an LLB, creating that SolveRequest and sending it to the daemon (as well as awaiting status updates and opening the channel to send requested info by the daemon).

The process looks like:

1. client starts a build with Dockerfile (or other frontend) and context(s) and options
2. client (built-in or via some other frontend processor) processes the frontend, getting to an LLB, which includes some sources
3. sends the SolveRequest to the daemon, which includes each source's, well, "source"

In practical terms, this means that if I have the following:

FROM golang:latest as builder
RUN something
FROM alpine:3.15
COPY --from=builder /else /here

The determining of builder as based on the image golang:latest happens on the client-end; buildkitd doesn't know or understand any of that. The client using a frontend processor determines "I need to resolve golang:latest and alpine:3.15", and the client figures out how to resolve each of those. It just passes each of those as a source. In the case of an image, that source will be a registry reference; in the case of local, it will be give it some reference that says, "go request this reference thing from the channel I opened with you."

Translating all of that to OCI, the client:

1. calls the frontend processor (unchanged)
2. sees that one of the sources is golang:latest (unchanged)
3. knows (based on its CLI flags or whatever) that golang:latest should be converted to a hash in a local content.Store (new)
4. sends the LLB to the daemon, along with the source for that particular element as "go request this reference thing from the channel I opened with you."

For that to work:

• the daemon needs to know that it has a new kind of source and how to get it (similar to local: it just reaches back on the channel to the client; similar to registry: everything is hashes and manifests)
• the client needs to know, when its frontend processor has an image that matches some indication (i.e. new context given), that it should find the local directory, open a local content.Store and make it available via the channel

Everything we did in this PR to date was about the first half of that: getting the daemon to recognize a new source reference that the client will pass it in its LLB; everything in the follow-on here is about getting the client to know how to find those source and create the correct reference to pass to the daemon.

If that is correct, let me know and I will try to interpret your above comments in that light.

Clients sends a Solve or Build request to build using a Dockerfile frontend. Dockerfile frontend runs inside a container, parses the Dockerfile, generates LLB and issues LLB Solve requests.

Buildkitd runs in one container; frontend can be in another. So client issues solve requests to frontend container, gets response as a whole bunch of LLB Solve requests, and then passes those to buildkitd?

This signature is wrong #2827 (comment)

I'll check.

IS it? I looked at your comment above, you suggested:

func OCILayout(contentStoreID, digest string, opts ...OCILayoutOptions)

But in the end, all it does is return a call to NewSource, whose signature is:

func NewSource(id string, attrs map[string]string, c Constraints) *SourceOp {

So it is just going to combine the contentStoreID and hash anyways, isn't it? What do I gain by:

func OCILayout(contentStoreID, digest string, opts ...OCILayoutOptions)
...
	source := NewSource("oci-layout://"+contentStoreID+"@"+digest, attrs, gi.Constraints)

Over just:

func OCILayout(ref string, opts ...OCILayoutOptions)
...
	source := NewSource("oci-layout://"+ref, attrs, gi.Constraints)

?

This needs to be connected with the store map used for local cache. Othewise you are registering the same service twice.

Ah does s.Allow() only have one service of each type?

Think I got that. Have a look.

Might be better to skip this for now and concentrate on LLB tests. This value can't be just a dir name anyway. You need to find a specific image and its digest from that dir.

Done and removed. Now I wonder, we added opt.OCILayouts as you suggested, but how do we then populate it?

OK, completely ignored the --oci-layout option, focused on the --opt context:foo= path for now.

No, this is not ref. ContentStoreID and digest.

I don't get that. What do you mean by contentstoreID?

From the end user perspective, all I do is pass "alias image foo to an oci layout at path x with hash abc123", using opt context.

Internally, the client passes this to the frontend, which translates "foo" to a reference to an OCI layout path and hash (the root manifest for that image).

Where does this become an ID? How is it referenced, both in passing to remote components (frontend and buildkitd), and when buildkitd passes it back up the channel to the client?

contentstoreID is the key in the map[string]content.Store that is exposed on the client side. Unique key that is used in the session request https://pkg.go.dev/github.com/moby/buildkit@v0.10.3/session/content#NewCallerStore. Client side session handler receives the request and by ContentStoreID finds the correct content.Store and by digest the correct root object from that content.Store.

So when client side sees, eg, 3 opts with context, each pointing to a different source, it gives each one some unique ID, which is passed on to the builder.

Is there an example of that being done? I figure that local must have it?

https://github.com/moby/buildkit/blob/master/client/solve.go#L466

Oh ok, now I see it. So the contentStoreID in the local case is just local:/path/to/store. This is what gets passed to the frontend to use in place of whatever image is aliases (e.g. foo), and what is sent to buildkitd, and in turn is then sent back to the client via the channel.

In the local case, a directory is enough, as it is just one thing, so local: (this is a local path) and <dir> (where to get it).
In the oci-layout case, we would need to indicate oci-layout: (this is an OCI layout) and <dir> where to get it and <hash> (what to extract from that layout. Roughly similar in concept to what we do with docker docker-image: followed by registry (where to get it) and ref (which to pull).

Not sure I got it entirely. See above with the signature.

@sipsma PTAL #2827 (comment)

@deitch Not a blocker for this PR. We can do this in follow-up.