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

SEND CAPABILITY IDS TO LXC #10152

Merged
merged 1 commit into from
Jan 19, 2015
Merged

Conversation

ashahab-altiscale
Copy link
Contributor

Sending capability ids instead of capability names ot LXC for --cap-add and --cap-drop.
Also fixed tests.

Docker-DCO-1.1-Signed-off-by: Abin Shahab ashahab@altiscale.com (github: ashahab-altiscale)

Sending capability ids instead of capability names ot LXC for --cap-add and --cap-drop.
Also fixed tests.

Docker-DCO-1.1-Signed-off-by: Abin Shahab <ashahab@altiscale.com> (github: ashahab-altiscale)
@ashahab-altiscale
Copy link
Contributor Author

@jessfraz
Copy link
Contributor

@ashahab-altiscale Thanks so much for the continual effort of fixing the tests :)

Do you think there is a way to check the output differently for these so that it works for lxc as well, because it looks like they should be passing:

--- FAIL: TestRunCapDropCannotMknod (0.87s)
    docker_cli_run_test.go:962: <nil> ok

--- FAIL: TestRunCapDropCannotMknodLowerCase (1.11s)
    docker_cli_run_test.go:977: <nil> ok

--- FAIL: TestRunCapDropALLCannotMknod (1.14s)
    docker_cli_run_test.go:992: <nil> ok

--- FAIL: TestRunCapAddALLDropNetAdminCanDownInterface (1.13s)
    docker_cli_run_test.go:1064: <nil> ok

--- FAIL: TestRunUnPrivilegedCannotMount (1.10s)
    docker_cli_run_test.go:1096: <nil> ok

@ashahab-altiscale
Copy link
Contributor Author

Odd, they all passed for me. I will have to check how its failing in
Jenkins. Can you send me a link?
On Jan 17, 2015 2:26 PM, "Jessie Frazelle" notifications@github.com wrote:

@ashahab-altiscale https://github.com/ashahab-altiscale Thanks so much
for the continual effort of fixing the tests :)

Do you think there is a way to check the output differently for these so
that it works for lxc as well, because it looks like they should be passing:

--- FAIL: TestRunCapDropCannotMknod (0.87s)
docker_cli_run_test.go:962: ok

--- FAIL: TestRunCapDropCannotMknodLowerCase (1.11s)
docker_cli_run_test.go:977: ok

--- FAIL: TestRunCapDropALLCannotMknod (1.14s)
docker_cli_run_test.go:992: ok

--- FAIL: TestRunCapAddALLDropNetAdminCanDownInterface (1.13s)
docker_cli_run_test.go:1064: ok

--- FAIL: TestRunUnPrivilegedCannotMount (1.10s)
docker_cli_run_test.go:1096: ok


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

@jessfraz
Copy link
Contributor

Oh I tested locally not on Jenkins. I am on Debian with Btrfs, but idk if
that could have anything to do with it.

On Sat, Jan 17, 2015 at 2:47 PM, Abin Shahab notifications@github.com
wrote:

Odd, they all passed for me. I will have to check how its failing in
Jenkins. Can you send me a link?
On Jan 17, 2015 2:26 PM, "Jessie Frazelle" notifications@github.com
wrote:

@ashahab-altiscale https://github.com/ashahab-altiscale Thanks so
much
for the continual effort of fixing the tests :)

Do you think there is a way to check the output differently for these so
that it works for lxc as well, because it looks like they should be
passing:

--- FAIL: TestRunCapDropCannotMknod (0.87s)
docker_cli_run_test.go:962: ok

--- FAIL: TestRunCapDropCannotMknodLowerCase (1.11s)
docker_cli_run_test.go:977: ok

--- FAIL: TestRunCapDropALLCannotMknod (1.14s)
docker_cli_run_test.go:992: ok

--- FAIL: TestRunCapAddALLDropNetAdminCanDownInterface (1.13s)
docker_cli_run_test.go:1064: ok

--- FAIL: TestRunUnPrivilegedCannotMount (1.10s)
docker_cli_run_test.go:1096: ok


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


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

@ashahab-altiscale
Copy link
Contributor Author

Its not an issue with output. Its just not working. If you want to dig more
into this, you can tell lxc where to log and the log level(in
lxc/driver.go). I am thinking if I should create a patch with that feature,
which would be enabled only in debug mode.
On Jan 17, 2015 2:49 PM, "Jessie Frazelle" notifications@github.com wrote:

Oh I tested locally not on Jenkins. I am on Debian with Btrfs, but idk if
that could have anything to do with it.

On Sat, Jan 17, 2015 at 2:47 PM, Abin Shahab notifications@github.com
wrote:

Odd, they all passed for me. I will have to check how its failing in
Jenkins. Can you send me a link?
On Jan 17, 2015 2:26 PM, "Jessie Frazelle" notifications@github.com
wrote:

@ashahab-altiscale https://github.com/ashahab-altiscale Thanks so
much
for the continual effort of fixing the tests :)

Do you think there is a way to check the output differently for these
so
that it works for lxc as well, because it looks like they should be
passing:

--- FAIL: TestRunCapDropCannotMknod (0.87s)
docker_cli_run_test.go:962: ok

--- FAIL: TestRunCapDropCannotMknodLowerCase (1.11s)
docker_cli_run_test.go:977: ok

--- FAIL: TestRunCapDropALLCannotMknod (1.14s)
docker_cli_run_test.go:992: ok

--- FAIL: TestRunCapAddALLDropNetAdminCanDownInterface (1.13s)
docker_cli_run_test.go:1064: ok

--- FAIL: TestRunUnPrivilegedCannotMount (1.10s)
docker_cli_run_test.go:1096: ok


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


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


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

@jessfraz
Copy link
Contributor

I'll try again thanks

On Saturday, January 17, 2015, Abin Shahab notifications@github.com wrote:

Its not an issue with output. Its just not working. If you want to dig
more
into this, you can tell lxc where to log and the log level(in
lxc/driver.go). I am thinking if I should create a patch with that
feature,
which would be enabled only in debug mode.
On Jan 17, 2015 2:49 PM, "Jessie Frazelle" <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

Oh I tested locally not on Jenkins. I am on Debian with Btrfs, but idk
if
that could have anything to do with it.

On Sat, Jan 17, 2015 at 2:47 PM, Abin Shahab <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');>
wrote:

Odd, they all passed for me. I will have to check how its failing in
Jenkins. Can you send me a link?
On Jan 17, 2015 2:26 PM, "Jessie Frazelle" <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');>
wrote:

@ashahab-altiscale https://github.com/ashahab-altiscale Thanks so
much
for the continual effort of fixing the tests :)

Do you think there is a way to check the output differently for
these
so
that it works for lxc as well, because it looks like they should be
passing:

--- FAIL: TestRunCapDropCannotMknod (0.87s)
docker_cli_run_test.go:962: ok

--- FAIL: TestRunCapDropCannotMknodLowerCase (1.11s)
docker_cli_run_test.go:977: ok

--- FAIL: TestRunCapDropALLCannotMknod (1.14s)
docker_cli_run_test.go:992: ok

--- FAIL: TestRunCapAddALLDropNetAdminCanDownInterface (1.13s)
docker_cli_run_test.go:1064: ok

--- FAIL: TestRunUnPrivilegedCannotMount (1.10s)
docker_cli_run_test.go:1096: ok


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


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


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


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

@ashahab-altiscale
Copy link
Contributor Author

Jessie,
What do you see if you try this: docker run --cap-add=ALL
--cap-drop=NET_ADMIN busybox sh -c 'ip link set eth0 down && echo ok'
(replace the docker binary with one created from "make binary" in my
branch)?

Abin

On Sat, Jan 17, 2015 at 3:25 PM, Jessie Frazelle notifications@github.com
wrote:

I'll try again thanks

On Saturday, January 17, 2015, Abin Shahab notifications@github.com
wrote:

Its not an issue with output. Its just not working. If you want to dig
more
into this, you can tell lxc where to log and the log level(in
lxc/driver.go). I am thinking if I should create a patch with that
feature,
which would be enabled only in debug mode.
On Jan 17, 2015 2:49 PM, "Jessie Frazelle" <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

Oh I tested locally not on Jenkins. I am on Debian with Btrfs, but idk
if
that could have anything to do with it.

On Sat, Jan 17, 2015 at 2:47 PM, Abin Shahab <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');>
wrote:

Odd, they all passed for me. I will have to check how its failing in
Jenkins. Can you send me a link?
On Jan 17, 2015 2:26 PM, "Jessie Frazelle" <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');>
wrote:

@ashahab-altiscale https://github.com/ashahab-altiscale Thanks
so
much
for the continual effort of fixing the tests :)

Do you think there is a way to check the output differently for
these
so
that it works for lxc as well, because it looks like they should
be
passing:

--- FAIL: TestRunCapDropCannotMknod (0.87s)
docker_cli_run_test.go:962: ok

--- FAIL: TestRunCapDropCannotMknodLowerCase (1.11s)
docker_cli_run_test.go:977: ok

--- FAIL: TestRunCapDropALLCannotMknod (1.14s)
docker_cli_run_test.go:992: ok

--- FAIL: TestRunCapAddALLDropNetAdminCanDownInterface (1.13s)
docker_cli_run_test.go:1064: ok

--- FAIL: TestRunUnPrivilegedCannotMount (1.10s)
docker_cli_run_test.go:1096: ok


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


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


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


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


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

@ashahab-altiscale
Copy link
Contributor Author

Actually, please also make sure there are no old docker images lurking
around. I got the same errors, but once I cleared the old images,
everything was fine. Ideally this tests should run in a machine with only
busybox.
Abin

On Sat, Jan 17, 2015 at 8:39 PM, Abin Shahab ashahab@altiscale.com wrote:

Jessie,
What do you see if you try this: docker run --cap-add=ALL
--cap-drop=NET_ADMIN busybox sh -c 'ip link set eth0 down && echo ok'
(replace the docker binary with one created from "make binary" in my
branch)?

Abin

On Sat, Jan 17, 2015 at 3:25 PM, Jessie Frazelle <notifications@github.com

wrote:

I'll try again thanks

On Saturday, January 17, 2015, Abin Shahab notifications@github.com
wrote:

Its not an issue with output. Its just not working. If you want to dig
more
into this, you can tell lxc where to log and the log level(in
lxc/driver.go). I am thinking if I should create a patch with that
feature,
which would be enabled only in debug mode.
On Jan 17, 2015 2:49 PM, "Jessie Frazelle" <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

Oh I tested locally not on Jenkins. I am on Debian with Btrfs, but
idk
if
that could have anything to do with it.

On Sat, Jan 17, 2015 at 2:47 PM, Abin Shahab <
notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');>
wrote:

Odd, they all passed for me. I will have to check how its failing
in
Jenkins. Can you send me a link?
On Jan 17, 2015 2:26 PM, "Jessie Frazelle" <
notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');>
wrote:

@ashahab-altiscale https://github.com/ashahab-altiscale Thanks
so
much
for the continual effort of fixing the tests :)

Do you think there is a way to check the output differently for
these
so
that it works for lxc as well, because it looks like they should
be
passing:

--- FAIL: TestRunCapDropCannotMknod (0.87s)
docker_cli_run_test.go:962: ok

--- FAIL: TestRunCapDropCannotMknodLowerCase (1.11s)
docker_cli_run_test.go:977: ok

--- FAIL: TestRunCapDropALLCannotMknod (1.14s)
docker_cli_run_test.go:992: ok

--- FAIL: TestRunCapAddALLDropNetAdminCanDownInterface (1.13s)
docker_cli_run_test.go:1064: ok

--- FAIL: TestRunUnPrivilegedCannotMount (1.10s)
docker_cli_run_test.go:1096: ok


Reply to this email directly or view it on GitHub
<
https://github.com/docker/docker/pull/10152#issuecomment-70387204>.


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


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


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


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

@ashahab-altiscale
Copy link
Contributor Author

@jfrazelle Any feedback on this?

@jessfraz
Copy link
Contributor

so sorry, going back through it all now

@jessfraz
Copy link
Contributor

I am running them again here so it is easier to show you the output I am seeing https://jenkins.dockerproject.com/job/LXC%20PR%20Test/label=ubuntu-aufs-lxc/1/console

@jessfraz
Copy link
Contributor

welp the tests work there so great :)

@jessfraz
Copy link
Contributor

LGTM

@@ -986,7 +986,7 @@ func TestRunCapDropCannotMknodLowerCase(t *testing.T) {
}

func TestRunCapDropALLCannotMknod(t *testing.T) {
cmd := exec.Command(dockerBinary, "run", "--cap-drop=ALL", "busybox", "sh", "-c", "mknod /tmp/sda b 8 0 && echo ok")
cmd := exec.Command(dockerBinary, "run", "--cap-drop=ALL", "--cap-add=SETGID", "busybox", "sh", "-c", "mknod /tmp/sda b 8 0 && echo ok")
Copy link
Contributor

Choose a reason for hiding this comment

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

This may need a little explanation as to why we are special casing setgid in this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LXC cap-drop=ALL means it won't allow docker do any preliminary setup work, without SETGID.

@dineshs-altiscale
Copy link
Contributor

LGTM

@ashahab-altiscale
Copy link
Contributor Author

Thanks @jfrazelle. So is https://jenkins.dockerproject.com/job/LXC%20PR%20Test a permanent set of jobs that I can check for my pull requests? The drone.io console logs are always from a native driver build I think.

@jessfraz
Copy link
Contributor

Not permanent, but I will probably trigger it if you open more PRs :)

On Mon, Jan 19, 2015 at 10:05 AM, Abin Shahab notifications@github.com
wrote:

Thanks @jfrazelle https://github.com/jfrazelle. So is
https://jenkins.dockerproject.com/job/LXC%20PR%20Test a permanent set of
jobs that I can check for my pull requests? The drone.io console logs are
always from a native driver build I think.


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

@jessfraz
Copy link
Contributor

But yes, the tests it runs are a better representation than drone, which
uses the native graph driver, eventually we hope, once all tests pass
across lxc is to run these on every PR so we can be sure no additions to
the code base break the lxc execdriver as well.

On Mon, Jan 19, 2015 at 10:15 AM, Jessica Frazelle jess@docker.com wrote:

Not permanent, but I will probably trigger it if you open more PRs :)

On Mon, Jan 19, 2015 at 10:05 AM, Abin Shahab notifications@github.com
wrote:

Thanks @jfrazelle https://github.com/jfrazelle. So is
https://jenkins.dockerproject.com/job/LXC%20PR%20Test a permanent set of
jobs that I can check for my pull requests? The drone.io console logs
are always from a native driver build I think.


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

@crosbymichael
Copy link
Contributor

@ashahab-altiscale is this good to be merged?

@ashahab-altiscale
Copy link
Contributor Author

@crosbymichael yes it is ready to merge.

@crosbymichael
Copy link
Contributor

OK, thanks!

LGTM

crosbymichael added a commit that referenced this pull request Jan 19, 2015
@crosbymichael crosbymichael merged commit 979a4cd into moby:master Jan 19, 2015
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

4 participants