Skip to content
This repository has been archived by the owner on Feb 24, 2020. It is now read-only.

add support to import image from docker-daemon in rkt #3939

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

surajssd
Copy link

@surajssd surajssd commented Jun 15, 2018

Add support to import image from docker-daemon in rkt

Now you can import an image that is available in docker's image store on
a local machine into rkt's imagestore.

To do so run following command:

rkt fetch --insecure-options=image docker-daemon://docker.io/library/alpine

or directly run it the usual way

rkt run --insecure-options=image docker-daemon://docker.io/library/alpine

This will import the image docker.io/library/alpine into rkt's store.

@surajssd
Copy link
Author

This PR first fetches the image from machine's running docker daemon into a tar format. This is similar to running a docker save.

Then it is converted into aci format and then imported into the rkt's image local store.

@surajssd surajssd changed the title add support to import image from docker-daemon in rkt [WIP] add support to import image from docker-daemon in rkt Jun 15, 2018
Now you can import an image that is available in docker's image store on
a local machine into rkt's imagestore.

To do so run following command:

```
rkt fetch --insecure-options=image docker-daemon://docker.io/library/alpine
```

or directly run it the usual way

```
rkt run --insecure-options=image docker-daemon://docker.io/library/alpine
```

This will import the image `docker.io/library/alpine` into rkt's store.
@surajssd surajssd changed the title [WIP] add support to import image from docker-daemon in rkt add support to import image from docker-daemon in rkt Jun 21, 2018
if err != nil {
return "", err
}
defer aciCleaner()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aciCleaner() called twice? (line 45 and 52)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, I can't read. ignore me

@onlyjob
Copy link

onlyjob commented Sep 24, 2018

That's a nice change to automatically invoke equivalent of two commands (docker image save, docker2aci) in order to convert docker container image from local store. It works as expected and makes rkt slightly more intuitive when it comes to first import of a foreign container image from Docker.

However I recommend not to merge it as it is, for numerous reasons:

  1. This change belongs to docker2aci which should support similar syntax.
    However there is appc->oci: get rid of docker2aci  #3378 hence the proper place to implement it would be in github.com/containers/image.

  2. In order to work this change requires Docker daemon to be running. But Docker is selfish and hostile: currently it unconditionally modifies iptables rules in a way that breaks networking in rkt. At least at the moment (and already for a while because Docker couldn't care less) Docker and rkt can not be used together.

  3. My biggest concern is about introducing competitor as a direct dependency.
    This is not just a strategic concern but also a practical problem because it bloats rkt's dependency graph with 100+ libraries, dramatically increasing surface for bugs.
    Apparently that's not only my concern: Replace github.com/docker/docker/client dependency? containers/image#476
    Downstream in Debian where we build software from fully tested libraries, bugs in Docker dependency tree are regularly exposed - that makes maintenance of Docker very tedious. rkt is much better in that regards due to much smaller dependency tree.

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

Successfully merging this pull request may close these issues.

None yet

3 participants