Skip to content

vendor: libcontainer: update libcontainer#19250

Closed
cyphar wants to merge 3 commits intomoby:masterfrom
cyphar:update-libcontainer-vendor
Closed

vendor: libcontainer: update libcontainer#19250
cyphar wants to merge 3 commits intomoby:masterfrom
cyphar:update-libcontainer-vendor

Conversation

@cyphar
Copy link
Copy Markdown
Contributor

@cyphar cyphar commented Jan 12, 2016

Update the vendor'd version of libcontainer to include path sanitisation
fixes and some other cgroup fixes.

Signed-off-by: Aleksa Sarai asarai@suse.com

This is the same as #19024, except GitHub won't let me push to it.

/cc @icecrime @runcom

@thaJeztah
Copy link
Copy Markdown
Member

Moving to "merge" as the original PR already got LGTM'd

@cyphar
Copy link
Copy Markdown
Contributor Author

cyphar commented Jan 12, 2016

I'm not sure why the tests are failing, but the fact that I haven't changed any code in Docker other than the code vendored in tells me something is broken upstream in runC.

/cc @crosbymichael @mrunalp

@estesp
Copy link
Copy Markdown
Contributor

estesp commented Jan 12, 2016

Seems there really is an issue with the new vendored runc (libcontainer specifically) with container stop/destroy? Don't have enough data yet, but clearly the tests that are failing are failing because there are containers still running affecting expected ps output, for example. One container that is all over the output (because it can be matched up to a specific test) is from here: https://github.com/docker/docker/blob/master/integration-cli/docker_cli_commit_test.go#L154-L159

@LK4D4
Copy link
Copy Markdown
Contributor

LK4D4 commented Jan 12, 2016

Yup, and I have some of those failures locally :(
However, I can't reproduce it with just command-line, I think it's probably related to nested cgroups in docker-in-docker case.

@cyphar
Copy link
Copy Markdown
Contributor Author

cyphar commented Jan 13, 2016

@estesp Yeah, that's what I was thinking. The problem is that I can't effectively bisect on my machine (I don't get the errors). Since #19182 has the same error, it look like the bad commit occurs before opencontainers/runc@c0ad40c. I'll try pushing to this PR to do the bisecting, but I suspect it's related to the state machine stuff.

@cyphar cyphar force-pushed the update-libcontainer-vendor branch 6 times, most recently from 0d48acb to 0d169cc Compare January 13, 2016 04:56
@cyphar
Copy link
Copy Markdown
Contributor Author

cyphar commented Jan 13, 2016

Okay, I've confirmed that @crosbymichael's commit opencontainers/runc@4415446 causes this build issue (after bisecting, using this PR as a testbed). I'll open up an issue about this later I've opened opencontainers/runc#470 as a stop-gap so we can fix the failing tests while @crosbymichael can figure out why the tests fail.

@cyphar cyphar force-pushed the update-libcontainer-vendor branch from 0d169cc to 53fe088 Compare January 13, 2016 06:07
@jessfraz jessfraz added this to the 1.10.0 milestone Jan 13, 2016
Update the vendor'd version of libcontainer to include path sanitisation
fixes and some other cgroup fixes.

Signed-off-by: Aleksa Sarai <asarai@suse.com>
To ensure we don't regress on bad --cgroup-parent paths, add some
integration tests that check that the host hasn't toppled (or suddently
started to create files in the host).

Signed-off-by: Aleksa Sarai <asarai@suse.com>
@cyphar cyphar force-pushed the update-libcontainer-vendor branch from 53fe088 to 7b3b788 Compare January 13, 2016 10:41
@GordonTheTurtle GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Jan 13, 2016
@cyphar cyphar force-pushed the update-libcontainer-vendor branch from 1967468 to 0286e48 Compare January 13, 2016 11:09
@GordonTheTurtle GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Jan 13, 2016
@cyphar
Copy link
Copy Markdown
Contributor Author

cyphar commented Jan 13, 2016

DO NOT MERGE YET, I'M TESTING WITH A BRANCH ON MY PERSONAL FORK OF RUNC

Okay, so it looks it was all fixed by opencontainers/runc#470 (which seems to fix all of the failing tests). Can you all take a look at that PR and get it merged ASAP?

/cc @crosbymichael @mrunalp @estesp @LK4D4

DO NOT MERGE THIS, IT IS BASED ON MY PERSONAL RUNC BRANCH!

Signed-off-by: Aleksa Sarai <asarai@suse.com>
@cyphar cyphar force-pushed the update-libcontainer-vendor branch from 04a44c5 to d23c2e3 Compare January 13, 2016 14:29
@tiborvass
Copy link
Copy Markdown
Contributor

@cyphar whats up with this?

@LK4D4
Copy link
Copy Markdown
Contributor

LK4D4 commented Jan 13, 2016

@tiborvass we need opencontainers/runc#470 or fix from @crosbymichael

@crosbymichael
Copy link
Copy Markdown
Contributor

I'm looking into the state changes and any bugs associated. we can close this for now until the bugs are fixed and we determine if we even need to bump libcontainer at this time.

@cyphar
Copy link
Copy Markdown
Contributor Author

cyphar commented Jan 15, 2016

@crosbymichael There are security fixes to the Docker daemon (specifically, a DoS attack) in this libcontainer bump.

@cyphar cyphar deleted the update-libcontainer-vendor branch January 27, 2016 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants