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

Remove lxc exec driver #5797

Closed
wants to merge 6 commits into from
Closed

Conversation

alexlarsson
Copy link
Contributor

This removes the lxc exec driver and the -lxc-conf option, as well as some now unused things.

This pull request is mostly of a suggestion and request for comments, but i think it makes a lot of sense.

So, Why remove the lxc driver?

At the core, the lxc driver and the native driver do the same thing. They rely on the same kernel features, and set them up in more or less the same way. The only difference is that in the lxc backend this happens by setting up an lxc template file that describes what we want and then asking lxc to do that. The problem with this that whenever we need to do something during container setup that the lxc templates don't support we run into problems (we'll never have the opposite problem, anything lxc adds we can also do in the native driver). Also, we have to maintain two completely different codebases that do the same thing.

Since its hard to keep up maintainership of the lxc backend its already starting to diverge slightly in behaviour. For instance, it does not use the new /dev tmpfs that the native driver does (it can't because it requires custom setup that the templates don't support). It also doesn't create bind-mount target that don't exist, which means that volumes don't work in places like /run or /dev (if the previous bug was fixed).

There is also ongoing work to have a separate supervisor process for each container, and the work that will be involved in this will require various changes in how containers are setup that will be very hard to implement using lxc.

Furthermore, if we remove lxc we remove the requirement that docker has to be statically linked, which makes building docker a lot easier.

Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson)
This is not needed/used anymore without the lxc driver

Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson)
Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson)
Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson)
This is not needed anymore, only the lxc driver used it.

Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson)
Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson)
@cyphar
Copy link
Contributor

cyphar commented May 14, 2014

+1. Since we finally added namespaces to libcontainer, there really is no need to use LXC.

@wking
Copy link

wking commented May 14, 2014

On Wed, May 14, 2014 at 06:57:38AM -0700, Alexander Larsson wrote:

At the core, the lxc driver and the native driver do the same
thing. They rely on the same kernel features, and set them up in
more or less the same way. The only difference is that in the lxc
backend this happens by setting up an lxc template file that
describes what we want and then asking lxc to do that. The problem
with this that whenever we need to do something during container
setup that the lxc templates don't support we run into problems
(we'll never have the opposite problem, anything lxc adds we can
also do in the native driver). Also, we have to maintain two
completely different codebases that do the same thing.

I know nothing about the code complexity of the native driver or the
LXC project, but my gut says it would be easier to just push your
extensions upstream to LXC. Is the problem with that just that having
folks run bleeding-edge Docker is easier without requiring
bleeding-edge LXC? Maybe the native driver is just a stop-gap until
Docker's driver requirements settle down, and can be dropped after LXC
catches back up and stabilizes? Long term maintenance of two
code-bases that do the same thing just seems wasteful. Of course, if
the native driver is a piece of cake, then it's not much of a waste
;).

@cyphar
Copy link
Contributor

cyphar commented May 14, 2014

@wking The reason that the native driver was written was so that we can interface directly with the cgroup and namespace features of the kernel, rather than through other scripts. LXC was the initial driver, since that was the technology available at the time, however LXC lacks certain features (since you can only really control it using configs, while the native driver can be modified to add more functionality). Besides, anything which LXC implements the native driver can implement too. The reverse is not necessarily true, since for some features (such as adding /dev as a tmpfs, etc. would require upstream patches [which makes the dependencies for docker features more complicated then necessary]). By containing all of the namespace and cgroup magic to a Docker package, you remove many of the complications concerning LXC. Besides, the choice to deprecate LXC has already been made, someone just had to do the dirty work of purging the codebase.

@alexlarsson
Copy link
Contributor Author

@wking Fundamentally, a lot of the things we do and want to do require more flexibility than a template setup can support. Especially since we have so very little control of what is inside the container. (I.e. the way you set up thing in the container with lxc is to use triggers that run some script, but with a docker image there might not even be a shell in the container). So, i don't think using the current lxc is possible.

@fermayo
Copy link
Member

fermayo commented May 14, 2014

The LXC driver has user namespaces implemented and libcontainer does not. In my opinion I would not remove the LXC driver until libcontainer has support for it.

@alexlarsson
Copy link
Contributor Author

@fermayo You can sort of tweak the lxc backend to support user namespaces with -lxc-conf, but its never gonna play nice with docker doing things like rewriting uids between containers for shared volumes, etc. For a fuller discussion how docker should support user namespaces, see #4572

@fermayo
Copy link
Member

fermayo commented May 14, 2014

@alexlarsson We have a very simple implementation of user namespaces using the LXC driver without support for shared volumes at https://github.com/tutumcloud/docker/tree/userns
I know it's a bigger topic, but LXC is the only backend that currently supports it, even if it's by tweaking stuff.

@jamtur01
Copy link
Contributor

Ping @shykes @crosbymichael

@crosbymichael
Copy link
Contributor

@fermayo On a side note, have you thought about contributing your lxc user namespace patch? We cannot support features in a fork. I also agree that we need this support in libcontainer, we have a few ppl experimenting with it right now.

@fermayo
Copy link
Member

fermayo commented May 14, 2014

@crosbymichael the work I've done in that fork has some big limitations (doesn't support volumes, has UIDs hardcoded and it's not compatible with pre-existing downloaded images as it does the UID/GID translation at pull time, for example), and having had a look at the referenced pull request it seems like a more sustainable approach than what I have done. However, if you guys have a look at my fork and decide that with a few changes we could merge it in docker, then absolutely, but I don't think it's going to be that easy ;-)

@crosbymichael
Copy link
Contributor

@alexlarsson do you know if @mrunalp has made any more progress with userns for libcontainer?

@mrunalp
Copy link
Contributor

mrunalp commented May 14, 2014

@crosbymichael I am on PTO for a bit. Hoping to get back to it in 2-3 days. Do you have any further comments on the prototype at mrunalp@78cff10? Do you think using SYS_CLONE is okay or we should be going the route of launching another executable that execs the user command?

@crosbymichael
Copy link
Contributor

@mrunalp i'll take another look and play around again if we fork then exec right after in the process we maybe fine, I'm sure @alexlarsson could help porting everything to raw syscalls ;)

@cpuguy83
Copy link
Member

One thing that the lxc driver has over libcontainer is the flexibility of --lxc-conf which does not currently have a counterpart in libcontainer.

@alexlarsson
Copy link
Contributor Author

@cpuguy83 We used to have lots of similar options as --lxc-conf with a generic -o option. However, that was removed (temporarily) to make sure the UI is right for this. So, this lack of flexibility is not inherent to non-lxc backend, but it is rather an accident, and its kind of "cheating" to assign it as an advantage to lxc.

In fact, --lxc-conf is part of the problem. We've decided that may not be a good ui, but we still have to keep it for lxc, and if lxc makes it into 1.0 then we have to essentially support it forever. I don't think supporting lxc forever is a great idea, especially with people using it in combination with various --lxc-conf hackery. Keeping such things working over time is going to become increasingly problematic.

@cpuguy83
Copy link
Member

@alexlarsson 100% agree, but we can't remove LXC support until libcontainer has this functionality.

@wking
Copy link

wking commented May 15, 2014

On Wed, May 14, 2014 at 07:45:32AM -0700, Aleksa Sarai wrote:

Besides, anything which LXC implements the native driver can
implement too. The reverse is not necessarily true, since for some
features (such as adding /dev as a tmpfs, etc. would require
upstream patches [which makes the dependencies for docker features
more complicated then necessary]).

And since there are already LXC devs not working on Docker's native
implementation, it seems easier to join forces with them and submit
the upstream patches. Then use the native driver as a temporary shim
to provide critical-but-not-in-a-stable-LXC support for people to
impatient to wait for LXC to stabilize and too lazy to install an
unstable LXC with the patches ;).

Wed, May 14, 2014 at 07:50:25AM -0700, Alexander Larsson:

Fundamentally, a lot of the things we do and want to do require more
flexibility than a template setup can support. Especially since we
have so very little control of what is inside the
container. (I.e. the way you set up thing in the container with lxc
is to use triggers that run some script, but with a docker image
there might not even be a shell in the container).

This sounds more convincing to me. So something like:

  • Fancy LXC hooks for setting up $some_feature require scripts.
  • Docker containers used for $some_task need $some_feature, but need
    not have a shell to execute the relevant setup scripts.
  • The LXC devs wouldn't accept patches that made $some_feature
    possible without scripting, because that's too complicated and they
    don't have a problem requiring folks to have a shell.

Do you have examples of $some_feature and $some_task to fill in that
hypothetical example? I've read through #4327, but didn't see
anything like that. The closest I found was asking for pro/con driver
comparisons. I wasn't able to turn up and such comparisons though.

@alexlarsson
Copy link
Contributor Author

@wking Well, for example, we're working on having each container have a long running supervisor process. The docker daemon would talk to this process via beam sockets, and the supervisor would be responsible for spawning and monitoring the main process in the container. Additionally, it will support the supervisor spawning plugin processes at various stages of container creation.

For such a setup we would need the supervisor to partially set up the container, then run any arbitrary plugins, and then finish the container setup switching to a non-privileged mode to spawn the container main process. Something like that just is not doable via some scripting hackery.

@unclejack
Copy link
Contributor

@cpuguy83 Just to make it clear, --lxc-conf is one of the void-your-warranty features which was never guaranteed to be around forever. Once these features are implemented and available to be used by other means, we can remove LXC.

@wking I'm not sure how closely you've followed the discussions around the native backend, LXC and the problems LXC is causing. Are you aware of the countless problems we've discovered lxc was causing? We've had to test various LXC versions with multiple kernels to find the best one (the one which was causing the least troubles) and spend a lot of time debugging problems which were caused by LXC itself.
Docker had to use all sorts of workarounds for various problems because of LXC and that was an endless game of whack-a-mole.

Different versions of LXC were being packaged across operating systems and this was yet another problem. Making sure your software is up to date across Linux distributions requires coordination, dedication and a lot of effort from all parties involved.
Can you imagine having to tell everyone to package and carry a specific version of a package in their repositories because that's what works best for your software, in addition to packaging your software? It's difficult even if everyone is willing to help you out.

Libcontainer and the native execution driver can be bundled into Docker, thus eliminating the dependency on LXC.

The native execution driver based on libcontainer doesn't represent an "extension" to LXC. The native execution driver is an interface to the exact same kernel features used by LXC. Libcontainer is written in Go. Asking us to "push your extensions upstream to LXC" doesn't make sense because libcontainer and the native execdriver which uses it aren't "plugins", "extensions" or "addons" for LXC.

Libcontainer and the native driver based on it are both using features found in the Linux kernel, just like LXC.

@crosbymichael
Copy link
Contributor

We discussed this and we are not ready to remove the lxc driver right now. However, we need help from the authors or community to maintain this driver if people want to continue to use it in docker. A few requirements for maintaining the driver is to port it from shelling out to lxc-start and using the Go bindings for better flexibility. Another requirement is that the maintainer should continue to work on bugs, port new features to the lxc driver, and make sure it maintains compatibility between the other drivers.

If no one steps up with a real commitment ( meaning actual code not just talk ) to support this then we will drop it from the project.

As for now we will add a warning saying that this driver is currently unmaintained.

@wking
Copy link

wking commented May 15, 2014

On Thu, May 15, 2014 at 09:12:42AM -0700, Alexander Larsson wrote:

For such a setup we would need the supervisor to partially set up
the container, then run any arbitrary plugins, and then finish the
container setup switching to a non-privileged mode to spawn the
container main process. Something like that just is not doable via
some scripting hackery.

That makes lots of sense, thanks :)

Thu, May 15, 2014 at 09:15:44AM -0700, unclejack:

@wking I'm not sure how closely you've followed the discussions
around the native backend, LXC and the problems LXC is causing.

Not at all ;). In fact, I haven't even had time to upgrade and follow
up on the one I reported myself (#5242). I just saw the PR removing a
component I've used successfully, and wanted to make sure it wasn't a
knee jerk “we'll just roll our own”. In fact, it's “moving forward,
we want to add features like an injectable, in-container supervisor
process which isn't supported by the LXC approach”, and I'm +1 on
that.

Docker had to use all sorts of workarounds for various problems
because of LXC and that was an endless game of whack-a-mole.

“This software is full of bugs, lets start over” works sometimes, but
I've written enough bugs to be skeptical that it's always a good idea
;).

Different versions of LXC were being packaged across operating
systems and this was yet another problem. Making sure your software
is up to date across Linux distributions requires coordination,
dedication and a lot of effort from all parties involved. Can you
imagine having to tell everyone to package and carry a specific
version of a package in their repositories because that's what works
best for your software, in addition to packaging your software? It's
difficult even if everyone is willing to help you out.

I get this, and I think telling folks that “Docker works best with LXC
x.y.z, here's a list of known issues with other versions: …” is fine
(especially before Docker 1.0). If the LXC model was compatible, I
don't see why a known-better version of LXC couldn't trickle down
through the various package repositories alongside a stable Docker
release. I'm not saying that such reasons don't exist, but I imagine
that someone here would know about them if they did, and be able to
explain them to me ;).

Libcontainer and the native driver can be bundled into Docker, thus
eliminating the dependency on LXC.

When you can factor your problem into layers, I like dependencies.
It makes it easier to share maintenance of the lower-level stuff, even
if folks aren't using the same high-level stuff. In fact, I'd be
happier if the stuff under pkg/ was all developed separately and
pushed to http://golang.org/pkg/ ;). Of course, I have easy access to
a whole range of LXC versions 1, and no packages besides Docker as
consumers, so maybe it's harder for folks using other package
managers.

Anyhow, I have a much clearer understanding of why the LXC driver is
losing Docker-dev maintenance now. Thanks for explaining to me, even
though I'm just standing here peering over shoulders instead of
rolling up my sleeves and helping out :). I'll try to give-back via
docker-registry, which has the Python code I understand :p.

@kingdonb
Copy link

I am real sorry to bring up i386 support, but lxc driver is the only way to use docker 0.11.1 with i386 in my so far brief, but successful testing. I came here from #5242 and I'm glad to see that lxc support is here to stay for now.

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 this pull request may close these issues.

None yet