containerd integration: docker run
#44804
Conversation
daemon/image_service.go
Outdated
|
|
||
| // Containerd related methods | ||
|
|
||
| ResolveDescriptor(ctx context.Context, refOrID string) (ocispec.Descriptor, error) |
There was a problem hiding this comment.
In my fork we had an interface that we would use only in the case we use the containerd store but that sucked.
I realise this isn't ideal, having an interface that does too much and having one of the implementors panic...
I considered moving some of the things over to the libcontainerd but I'm not sure it would help and also I'm not really a fan of that wrapper.
Thoughts?
There was a problem hiding this comment.
The idea is also that this interface will be more and more c8d specific as we move along with the migration.
rumpl
added
containerd-integration
rumpl
force-pushed
the
containerd-run
branch
2 times, most recently
from
1a6ea5c to
6b05652
Compare
|
The failure is unrelated |
| if daemon.UsesSnapshotter() { | ||
| if err := daemon.imageService.PrepareSnapshot(ctx, ctr.ID, opts.params.Config.Image, opts.params.Platform); err != nil { | ||
| return nil, err | ||
| } | ||
| } else { | ||
| // Set RWLayer for container after mount labels have been set | ||
| rwLayer, err := daemon.imageService.CreateLayer(ctr, setupInitLayer(daemon.idMapping)) | ||
| if err != nil { | ||
| return nil, errdefs.System(err) | ||
| } | ||
| ctr.RWLayer = rwLayer | ||
| } |
There was a problem hiding this comment.
🙈
I think it's fine for now, just to get things working, but we definitely should look into refactoring the RWLayer into an implementation detail of the graphdriver's ImageService implementation, so that we could just drop the hacky if daemon.UsesSnapshotter to just rely on different interface implementation.
The RWLayer is all over the place which makes this a lot of work, so I think it's reasonable to just go with this one for now.
tianon
left a comment
tianon
left a comment
There was a problem hiding this comment.
Seems OK to me, although my knowledge of these codepaths is admittedly very limited. 🙈
rumpl
force-pushed
the
containerd-run
branch
2 times, most recently
from
627dabc to
312db91
Compare
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM, thanks!
As discussed on slack; last change went in the wrong commit, and probably ok to squash all commits
|
doh! Looks like something needs to be fixed up; |
Oop, will fix! |
rumpl
force-pushed
the
containerd-run
branch
2 times, most recently
from
1225a2b to
1c8f9ca
Compare
|
Hm... Some of the checks fail because of |
Signed-off-by: Djordje Lukic <[email protected]> c8d/daemon: Mount root and fill BaseFS This fixes things that were broken due to nil BaseFS like `docker cp` and running a container with workdir override. This is more of a temporary hack than a real solution. The correct fix would be to refactor the code to make BaseFS and LayerRW an implementation detail of the old image store implementation and use the temporary mounts for the c8d implementation instead. That requires more work though. Signed-off-by: Paweł Gronowski <[email protected]> daemon/images: Don't unset BaseFS Signed-off-by: Paweł Gronowski <[email protected]>
6 participants
- What I did
Implemented
docker runwith the containerd store.Upstreams:
I added the second and third commit because parts of the code changes are needed for run to work.
- How I did it
Mostly avoiding doing anything with layers if containerd is used as an image store and using snapshots instead. Some tweaks were also needed in the way we create the OCI spec for the container.
- How to verify it
Run:
All of these should work after #44756 is merged.
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)