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

[Feature] import images from docker daemon into k3d #91

Merged
merged 10 commits into from
Jul 23, 2019

Conversation

iwilltry42
Copy link
Member

@iwilltry42 iwilltry42 commented Jul 10, 2019

Context

k3d import-images to import docker images from the used docker daemon into containerd inside the k3d nodes.
This is a new version of PR #83 which makes use of a named volume and an additional k3d-tools container.

How it works

  1. upon k3d create, a named volume for storing images will be created and mounted on /images in all nodes (NOTE: this requires at least --image rancher/k3s:v0.7.0-rc2 because of ctr)
  2. user has nginx:local and redis:v2 cached locally after issuing docker build and wants to have them available to the k3d nodes (without having to push to a registry)
  3. user issues $ k3d import-images --cluster test-cluster nginx:local redis:v2
  4. a helper container (currently iwilltry42/k3d-tools:v0.0.1) will be spawned, mounting the same volume as the nodes and /var/run/docker.sock to have access to the docker daemon/API
  5. the helper container will internally run a docker save nginx:local redis:v2 -o /images/images.tar (API variant)
  6. the /iamges/images.tar is then available to the k3d nodes
  7. k3d will exec a ctr image import /images/images.tar in every k3d node to import the images into each node's containerd image cache
  8. the helper container will be removed again (and the tarball if wanted)

Explanation

I chose this approach, since I had much trouble with the other approach (saving the stream to a tarball on the fly) and because I think that we can save bandwidth and avoid possible network latency by working directly on the docker host (which might be remote) instead of saving from docker host to local host and copy the tarball back.
The drawback is, that we'll have to maintain the additional docker image as well, though it's really small.

Possible TODOs

  • add either --rm or --no-rm flag to enable/disable automatic removal of the tarball
    • UPDATE 11.07.2019: added --no-remove, --no-rm, --keep, -k flag to disable automatic removal
  • add --all-cached (or similar) flag to directly import all tarballs that are already in the /images directory
  • add --from-tar flag to import from a tarball that you have saved locally (branching before we create the helper container)
  • rollback (i.e. delete helper container) in any case of error

FYI @andyz-dev

@iwilltry42 iwilltry42 added the enhancement New feature or request label Jul 10, 2019
@iwilltry42 iwilltry42 added this to the v1.3.0 milestone Jul 10, 2019
@iwilltry42 iwilltry42 self-assigned this Jul 10, 2019
@andyz-dev
Copy link
Contributor

Found a small issue that may be worth fixing:

Suppose local docker has the image alpine:3.9, the following command sequence can be improved with better error handling:

bin/k3d import-images alpine:4.0
2019/07/15 16:28:16 INFO: Saving images [alpine:4.0] from local docker daemon...
2019/07/15 16:28:17 ERROR: helper container failed to save images -> Logs from [k3d-k3s-default-tools]:

=2019/07/15 23:28:16 ERROR: couldn't save images [alpine:4.0]
5Error response from daemon: reference does not exist

This error is expected, no problem. Then I won't be able import the right image any more.

bin/k3d import-images alpine:3.9
2019/07/15 16:28:29 INFO: Saving images [alpine:3.9] from local docker daemon...
2019/07/15 16:28:29 ERROR: couldn't create container k3d-k3s-default-tools
Error response from daemon: Conflict. The container name "/k3d-k3s-default-tools" is already in use by container "6f70a6e635bb0164492129e0cc7fc623c8944bb8ef736a3aa9946d24f2a26764". You have to remove (or rename) that container to be able to reuse that name.

Until the 'iwlltry42/k3d-tools:v0.0.1' container is manually deleted from docker.

@andyz-dev
Copy link
Contributor

What's plan for maintaining the helper container? May be we should plan to build and release the helper containers for each k3d release?

@iwilltry42
Copy link
Member Author

@andyz-dev true, I totally forgot about deleting the container again in case of error, because I moved the code back and forth quite a lot. I will fix this soon 👍

Right now, the helper container has a single functionality, so I'm unsure if it's worth building it everytime, though I don't have a problem with it, as that way we can ensure compatibility with each release 👍
I put the code in a separate repository, because I didn't want to bother with adjusting all the little knobs to treat it as a sub-project.
Would you prefer having it embedded in this repo or maintain it separately?
Or some setup with a git submodule/subrepo/subtree?

@andyz-dev
Copy link
Contributor

I agree with you that a sub project may be an overkill. This is one of the downside of this approach. On the other hand, when we merge the PR in (hopefully soon), we will have to have a plan for publish the "official" image for it.

Would it be possible to add a makefile rule (and a corresponding dockerfile) for building the image? The rule can be tied into the build-cross target, so that pushing an official image can be part of the release.

If this is too much work, and keeping an separate repo is easier, that will be fine too. We can model it after how k3s images are supported. Personally, I'd prefer the makefile approach if it is possible.

@iwilltry42
Copy link
Member Author

@andyz-dev ready for another review 👍
I decided to cleanup the tools container whenever we return.
Pro: we catch any case
Con: bad for debugging, as all traces of it will be gone if it fails
So either we go this way or we put a catch for every case or we enable re-using an already existing tools container, so we don't have conflicting names.

Copy link
Contributor

@andyz-dev andyz-dev left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for adding the fix.

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

Successfully merging this pull request may close these issues.

None yet

2 participants