Remove LXC support. #17700

Merged
merged 2 commits into from Nov 5, 2015

Conversation

Projects
None yet
Contributor

calavera commented Nov 4, 2015

The LXC driver was deprecated in Docker 1.8.
Following the deprecation rules, we can remove a deprecated feature
after two major releases. LXC won't be supported anymore starting on Docker 1.10.

It also removes the global daemon option --exec-driver because it doesn't make sense anymore.

Signed-off-by: David Calavera david.calavera@gmail.com

Member

tianon commented Nov 4, 2015

LGTM! 👍

Member

thaJeztah commented Nov 4, 2015

Contributor

cpuguy83 commented Nov 4, 2015

runconfig/parse.go:65: flLxcOpts declared and not used

Contributor

calavera commented Nov 4, 2015

thanks @cpuguy83

@thaJeztah yeah, I left a few things behind. I wanted to check if it actually worked :)
There are a few things in contrib that I don't really know how to clean, apparmor, selinux and other related scripts.

Member

thaJeztah commented Nov 4, 2015

@calavera no problem, just mentioned it so that we don't skip docs review on this one. I can help checking once we're there

Contributor

calavera commented Nov 4, 2015

please, make sure you check this PR, because it's happening 😁

@@ -72,16 +72,6 @@ RUN cd /usr/local/lvm2 \
&& make install_device-mapper
# see https://git.fedorahosted.org/cgit/lvm2.git/tree/INSTALL
-# Install lxc
@tiborvass

tiborvass Nov 4, 2015

Collaborator

ugh i know it's silly but if we could merge this at the same time as #17701 to suffer only one cache bust :)

@calavera

calavera Nov 5, 2015

Contributor

we can put both changes in the same PR to merge them at the same time, although it's unusual.

Collaborator

vieux commented Nov 5, 2015

@calavera are we going to keep the execdrivers since there is only one now ?
I think it's a good idea if one another ever pops up.

Contributor

calavera commented Nov 5, 2015

@vieux that's a very good question. Maybe we can just remove that option all together. After all, windows and linux are already separated by build flags.

Contributor

calavera commented Nov 5, 2015

I've also removed the global daemon option --exec-driver in a separated commit. Because as @vieux mentions, it doesn't make sense to keep it anymore.

Contributor

rhatdan commented Nov 5, 2015

Does this mean dockerinit goes away? Can we switch to building docker as dynbinary as default?

Member

vdemeester commented Nov 5, 2015

@calavera the --exec-driver flag will go without deprecation notice though, right ? (I don't think this is a bad thing but prefer to mention/ask about it).

Contributor

cpuguy83 commented Nov 5, 2015

Yeah, thought abut this too. Ultimately I think it should fail and not just silently ignore the fact that the user was specifying LXC.

Contributor

calavera commented Nov 5, 2015

@rhatdan dynbinary has been the default since 1.8.

@vdemeester yes it goes away without deprecation notice. It doesn't make sense to keep it as deprecated when it doesn't do anything.

@cpuguy83 it fails if you set the flag, because the parser doesn't recognize that option 😄

Member

vdemeester commented Nov 5, 2015

@calavera ok,seems fair. LGTM then :-)

Contributor

ewindisch commented Nov 5, 2015

Conceptually, LGTM! Only glanced at the change itself, but nothing shocking.

Collaborator

tiborvass commented Nov 5, 2015

I'm not sure about removing --exec-driver even if there's just now one driver left. In fact there are two drivers left, one windows and libcontainer even if you can't really choose between them because they are for different platforms.

Still, removing --exec-driver is dependent on the runc integration, since runc integration is what will give people a way to write and choose exec drivers. But those decisions are not finalized. If today, we remove the flag only to add it back later, it doesn't make sense. I prefer to live with a nop flag then with back and forth decisions.

To complicate the matter further, I'd be interested in moving the --exec-driver flag to the run command so that i can have multiple exec drivers at the same time.

Contributor

calavera commented Nov 5, 2015

To complicate the matter further, I'd be interested in moving the --exec-driver flag to the run command so that i can have multiple exec drivers at the same time.

@tiborvass That's one more reason to remove this flag from the daemon. It's a completely different feature.

I'd rather not keep flags that we don't use only just in case. It will be easier to move forward with runc if we start with a clean slate and we don't try to keep the old idioms around only because we had them already.

@crosbymichael I'd really appreciate your input in this.

Collaborator

tiborvass commented Nov 5, 2015

It will be easier to move forward with runc if we start with a clean slate and we don't try to keep the old idioms around only because we had them already.

@calavera that's why it's the runc integration PR that should remove the flag.

Anyway I won't fight it.

Contributor

LK4D4 commented Nov 5, 2015

Btw, libnetwork depends on .dockerinit inside container.

Contributor

cpuguy83 commented Nov 5, 2015

noooo

Contributor

jessfraz commented Nov 5, 2015

hahaha that could be a phase 2?

Contributor

calavera commented Nov 5, 2015

@LK4D4 it looks like .dockerinit still exists in a container booted with this branch, I'm not sure if this changes anything:

➜  docker git:(remove_lxc) bundles/1.10.0-dev/binary/docker run --rm -it alpine ash
/ # ls -la
total 56
drwxr-xr-x   17 root     root          4096 Nov  5 21:15 .
drwxr-xr-x   17 root     root          4096 Nov  5 21:15 ..
-rwxr-xr-x    1 root     root             0 Nov  5 21:15 .dockerenv
-rwxr-xr-x    1 root     root             0 Nov  5 21:15 .dockerinit
drwxr-xr-x    2 root     root          4096 Sep 14 19:17 bin

It looks like libnetwork only uses that file to decide whether to raise an error or not. I'm not sure if we need to do anything, but if we do, maybe it's something that we can do in some other way that doesn't block this PR:

https://github.com/docker/libnetwork/blob/9fb7ba8fa01f0ddce99713799e760223c842fb4b/drivers/bridge/setup_bridgenetfiltering.go#L36-L41

Member

tianon commented Nov 5, 2015

Contributor

calavera commented Nov 5, 2015

I agree with @tianon

Contributor

jessfraz commented Nov 5, 2015

LGTM

Contributor

jessfraz commented Nov 5, 2015

lets move ahead!

calavera added some commits Nov 4, 2015

Remove LXC support.
The LXC driver was deprecated in Docker 1.8.
Following the deprecation rules, we can remove a deprecated feature
after two major releases. LXC won't be supported anymore starting on Docker 1.10.

Signed-off-by: David Calavera <david.calavera@gmail.com>
Remove exec-driver global daemon option.
Each platform has only a driver now.

Signed-off-by: David Calavera <david.calavera@gmail.com>

@calavera calavera added this to the 1.10 milestone Nov 5, 2015

Member

thaJeztah commented Nov 5, 2015

@calavera impact/deprecation too? It's marked deprecated already, but for easier finding.. Idk

Contributor

calavera commented Nov 5, 2015

@thaJeztah impact/💀 maybe? 😁

Member

vdemeester commented Nov 5, 2015

impact/👻

Member

thaJeztah commented Nov 5, 2015

impact/byebye-it-was-nice-knowing-you-but-we-moved-on

Contributor

jessfraz commented Nov 5, 2015

impact/RIP-jess-is-glad-for-after-over-2-years-never-having-to-run-lxc-start-again
😇

On Thu, Nov 5, 2015 at 2:53 PM, Sebastiaan van Stijn <
notifications@github.com> wrote:

impact/byebye-it-was-nice-knowing-you-but-we-moved-on


Reply to this email directly or view it on GitHub
docker#17700 (comment).

Member

thaJeztah commented Nov 5, 2015

impact deprecate !rebuild docker#17700/lxc

Collaborator

shykes commented Nov 5, 2015

LGTM

shykes added a commit that referenced this pull request Nov 5, 2015

@shykes shykes merged commit 2519f46 into moby:master Nov 5, 2015

2 of 5 checks passed

janky Jenkins build Docker-PRs 19273 has failed
Details
windows Jenkins build Windows-PRs 16597 has failed
Details
userns Jenkins build Docker-PRs-userns 1678 is running
Details
docker/dco-signed All commits signed
Details
experimental Jenkins build Docker-PRs-experimental 10508 has succeeded
Details

@calavera calavera deleted the calavera:remove_lxc branch Nov 5, 2015

Contributor

michaelneale commented Nov 5, 2015

Shed a single tear

Contributor

jhowardmsft commented Nov 6, 2015

Ah please leave --exec-driver for the moment. There will soon be two Windows drivers while I'm developing the libcontainer Windows one

tianon added a commit to tianon/docker-overlay that referenced this pull request Nov 6, 2015

Contributor

xnox commented Nov 12, 2015

How is OCI exec driver going to be added?

Note that native driver / libcontainer != OCI. There are three OCI complaint executors already: clr, runv, runc. With only one of them based on libcontainer bits.

This kills clr exec-driver that uses VMs on Linux, which Highland team was about to be piloting to use. And which was requested to be merged to master...

Given that imminently we will need oci exec driver to supplement and migrate from native driver on linux at least, removing this support adds no value, and makes current and future development match harder.

Please revert?

Contributor

stevvooe commented Nov 12, 2015

@xnox Might be better to open a separate issue rather than commenting on a closed pull request. I would also suggest you be more specific about why this affects OCI integration.

vincentbernat added a commit to vincentbernat/docker that referenced this pull request Nov 18, 2015

zsh: remove lxc-related completion
LXC support has been deprecated and the related completion has been
removed in #17700 but was added back in #17334.

Signed-off-by: Vincent Bernat <vincent@bernat.im>

kimh commented Dec 8, 2015

I just want to be clear, but does this PR mean that I cannot run Docker on LXC from docker 1.10? Or is there a way other than using --exec-driver lxc to do this?

In other words, what should I do if I want to keep using Docker on LXC?

Member

thaJeztah commented Dec 8, 2015

@kimh correct; are there specific reasons you cannot use the "native" driver?

kimh commented Dec 8, 2015

@thaJeztah

I work in CircleCI and we use LXC container to run builds. Therefore, we've used LXC exec-driver to run Docker.

are there specific reasons you cannot use the "native" driver?

Actually, I'm wondering if I can use libcontainer on LXC and made this SO question which led me to this PR.

So repeating the same question, is it technically possible to use libcontainer driver on LXC container?

Contributor

cpuguy83 commented Dec 10, 2015

@kimh There should be nothing stopping it.

netbrain added a commit to netbrain/docker that referenced this pull request Jan 4, 2016

zsh: remove lxc-related completion
LXC support has been deprecated and the related completion has been
removed in #17700 but was added back in #17334.

Signed-off-by: Vincent Bernat <vincent@bernat.im>

@mkuchin mkuchin referenced this pull request in saltstack/salt Feb 24, 2016

Closed

dockerng failed on inspection of lxc_conf parameter #31451

jheiss added a commit to jheiss/docker that referenced this pull request Mar 10, 2016

zsh: remove lxc-related completion
LXC support has been deprecated and the related completion has been
removed in #17700 but was added back in #17334.

Signed-off-by: Vincent Bernat <vincent@bernat.im>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment