You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by haosdent huang <ha...@gmail.com> on 2015/10/03 17:55:55 UTC
Re: Review Request 38338: Enhanced option for Docker cli volume
plugin.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38338/
-----------------------------------------------------------
(Updated Oct. 3, 2015, 3:55 p.m.)
Review request for mesos and Timothy Chen.
Changes
-------
update test description.
Bugs: MESOS-3392
https://issues.apache.org/jira/browse/MESOS-3392
Repository: mesos
Description
-------
Enhanced option for Docker cli volume plugin.
Diffs (updated)
-----
include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999
src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7
Diff: https://reviews.apache.org/r/38338/diff/
Testing (updated)
-------
# When use volume-driver
```
I1003 23:49:41.986047 11268 docker.cpp:571] Running docker -H unix:///var/run/docker.sock run -e MESOS_SANDBOX=/mnt/mesos/sandbox -e MESOS_CONTAINER_NAME=mesos-docker-test -v src:target:rw -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_D3Fxye:/mnt/mesos/sandbox --volume-driver=flocker --net host --entrypoint /bin/sh --name mesos-docker-test busybox -c sleep 120
```
Could see `-v src:target`
# When don't use volume-driver
```
1003 23:53:48.028715 12125 docker.cpp:571] Running docker -H unix:///var/run/docker.sock run -e MESOS_SANDBOX=/mnt/mesos/sandbox -e MESOS_CONTAINER_NAME=mesos-docker-test -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt/src:target:rw -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt:/mnt/mesos/sandbox --net host --entrypoint /bin/sh --name mesos-docker-test busybox -c sleep 120
```
Could see `-v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt/src:target:rw`
Thanks,
haosdent huang
Re: Review Request 38338: Enhanced option for Docker cli volume
plugin.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38338/#review101406
-----------------------------------------------------------
Patch looks great!
Reviews applied: [38338]
All tests passed.
- Mesos ReviewBot
On Oct. 3, 2015, 3:55 p.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38338/
> -----------------------------------------------------------
>
> (Updated Oct. 3, 2015, 3:55 p.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Bugs: MESOS-3392
> https://issues.apache.org/jira/browse/MESOS-3392
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Enhanced option for Docker cli volume plugin.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999
> src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7
>
> Diff: https://reviews.apache.org/r/38338/diff/
>
>
> Testing
> -------
>
> # When use volume-driver
> ```
> I1003 23:49:41.986047 11268 docker.cpp:571] Running docker -H unix:///var/run/docker.sock run -e MESOS_SANDBOX=/mnt/mesos/sandbox -e MESOS_CONTAINER_NAME=mesos-docker-test -v src:target:rw -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_D3Fxye:/mnt/mesos/sandbox --volume-driver=flocker --net host --entrypoint /bin/sh --name mesos-docker-test busybox -c sleep 120
> ```
> Could see `-v src:target`
>
> # When don't use volume-driver
> ```
> 1003 23:53:48.028715 12125 docker.cpp:571] Running docker -H unix:///var/run/docker.sock run -e MESOS_SANDBOX=/mnt/mesos/sandbox -e MESOS_CONTAINER_NAME=mesos-docker-test -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt/src:target:rw -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt:/mnt/mesos/sandbox --net host --entrypoint /bin/sh --name mesos-docker-test busybox -c sleep 120
> ```
> Could see `-v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt/src:target:rw`
>
>
> Thanks,
>
> haosdent huang
>
>
Re: Review Request 38338: Enhanced option for Docker cli volume
plugin.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38338/#review107695
-----------------------------------------------------------
Ship it!
Ship It!
- Timothy Chen
On Oct. 3, 2015, 3:55 p.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38338/
> -----------------------------------------------------------
>
> (Updated Oct. 3, 2015, 3:55 p.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Bugs: MESOS-3392
> https://issues.apache.org/jira/browse/MESOS-3392
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Enhanced option for Docker cli volume plugin.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999
> src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7
>
> Diff: https://reviews.apache.org/r/38338/diff/
>
>
> Testing
> -------
>
> # When use volume-driver
> ```
> I1003 23:49:41.986047 11268 docker.cpp:571] Running docker -H unix:///var/run/docker.sock run -e MESOS_SANDBOX=/mnt/mesos/sandbox -e MESOS_CONTAINER_NAME=mesos-docker-test -v src:target:rw -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_D3Fxye:/mnt/mesos/sandbox --volume-driver=flocker --net host --entrypoint /bin/sh --name mesos-docker-test busybox -c sleep 120
> ```
> Could see `-v src:target`
>
> # When don't use volume-driver
> ```
> 1003 23:53:48.028715 12125 docker.cpp:571] Running docker -H unix:///var/run/docker.sock run -e MESOS_SANDBOX=/mnt/mesos/sandbox -e MESOS_CONTAINER_NAME=mesos-docker-test -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt/src:target:rw -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt:/mnt/mesos/sandbox --net host --entrypoint /bin/sh --name mesos-docker-test busybox -c sleep 120
> ```
> Could see `-v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt/src:target:rw`
>
>
> Thanks,
>
> haosdent huang
>
>
Re: Review Request 38338: Enhanced option for Docker cli volume
plugin.
Posted by haosdent huang <ha...@gmail.com>.
> On Oct. 4, 2015, 2:09 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, line 463
> > <https://reviews.apache.org/r/38338/diff/3/?file=1089388#file1089388line463>
> >
> > Is it possible to add a unit test for this change?
Because it depends on install and configure flocker in host, I not idea how to add a unit test when host don't have flocker. Do you have some ideas how to test this?
- haosdent
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38338/#review101425
-----------------------------------------------------------
On Oct. 3, 2015, 3:55 p.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38338/
> -----------------------------------------------------------
>
> (Updated Oct. 3, 2015, 3:55 p.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Bugs: MESOS-3392
> https://issues.apache.org/jira/browse/MESOS-3392
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Enhanced option for Docker cli volume plugin.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999
> src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7
>
> Diff: https://reviews.apache.org/r/38338/diff/
>
>
> Testing
> -------
>
> # When use volume-driver
> ```
> I1003 23:49:41.986047 11268 docker.cpp:571] Running docker -H unix:///var/run/docker.sock run -e MESOS_SANDBOX=/mnt/mesos/sandbox -e MESOS_CONTAINER_NAME=mesos-docker-test -v src:target:rw -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_D3Fxye:/mnt/mesos/sandbox --volume-driver=flocker --net host --entrypoint /bin/sh --name mesos-docker-test busybox -c sleep 120
> ```
> Could see `-v src:target`
>
> # When don't use volume-driver
> ```
> 1003 23:53:48.028715 12125 docker.cpp:571] Running docker -H unix:///var/run/docker.sock run -e MESOS_SANDBOX=/mnt/mesos/sandbox -e MESOS_CONTAINER_NAME=mesos-docker-test -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt/src:target:rw -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt:/mnt/mesos/sandbox --net host --entrypoint /bin/sh --name mesos-docker-test busybox -c sleep 120
> ```
> Could see `-v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt/src:target:rw`
>
>
> Thanks,
>
> haosdent huang
>
>
Re: Review Request 38338: Enhanced option for Docker cli volume
plugin.
Posted by Guangya Liu <gy...@gmail.com>.
> On 十月 4, 2015, 2:09 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, line 463
> > <https://reviews.apache.org/r/38338/diff/3/?file=1089388#file1089388line463>
> >
> > Is it possible to add a unit test for this change?
>
> haosdent huang wrote:
> Because it depends on install and configure flocker in host, I not idea how to add a unit test when host don't have flocker. Do you have some ideas how to test this?
My original thinking is only code level testing by adding some check to see if "--volume-driver" is in dockerInfo, but this does not seems to help much. I think it is OK as long as you the feature test passed in your env. ;-)
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38338/#review101425
-----------------------------------------------------------
On 十月 3, 2015, 3:55 p.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38338/
> -----------------------------------------------------------
>
> (Updated 十月 3, 2015, 3:55 p.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Bugs: MESOS-3392
> https://issues.apache.org/jira/browse/MESOS-3392
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Enhanced option for Docker cli volume plugin.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999
> src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7
>
> Diff: https://reviews.apache.org/r/38338/diff/
>
>
> Testing
> -------
>
> # When use volume-driver
> ```
> I1003 23:49:41.986047 11268 docker.cpp:571] Running docker -H unix:///var/run/docker.sock run -e MESOS_SANDBOX=/mnt/mesos/sandbox -e MESOS_CONTAINER_NAME=mesos-docker-test -v src:target:rw -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_D3Fxye:/mnt/mesos/sandbox --volume-driver=flocker --net host --entrypoint /bin/sh --name mesos-docker-test busybox -c sleep 120
> ```
> Could see `-v src:target`
>
> # When don't use volume-driver
> ```
> 1003 23:53:48.028715 12125 docker.cpp:571] Running docker -H unix:///var/run/docker.sock run -e MESOS_SANDBOX=/mnt/mesos/sandbox -e MESOS_CONTAINER_NAME=mesos-docker-test -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt/src:target:rw -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt:/mnt/mesos/sandbox --net host --entrypoint /bin/sh --name mesos-docker-test busybox -c sleep 120
> ```
> Could see `-v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt/src:target:rw`
>
>
> Thanks,
>
> haosdent huang
>
>
Re: Review Request 38338: Enhanced option for Docker cli volume
plugin.
Posted by haosdent huang <ha...@gmail.com>.
> On Oct. 4, 2015, 2:09 a.m., Guangya Liu wrote:
> > include/mesos/mesos.proto, line 1443
> > <https://reviews.apache.org/r/38338/diff/3/?file=1089387#file1089387line1443>
> >
> > Does v1 also needs to be updated?
I notice we have some inconsistent between v1 and current proto, so I not sure should add or not.
- haosdent
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38338/#review101425
-----------------------------------------------------------
On Oct. 3, 2015, 3:55 p.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38338/
> -----------------------------------------------------------
>
> (Updated Oct. 3, 2015, 3:55 p.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Bugs: MESOS-3392
> https://issues.apache.org/jira/browse/MESOS-3392
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Enhanced option for Docker cli volume plugin.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999
> src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7
>
> Diff: https://reviews.apache.org/r/38338/diff/
>
>
> Testing
> -------
>
> # When use volume-driver
> ```
> I1003 23:49:41.986047 11268 docker.cpp:571] Running docker -H unix:///var/run/docker.sock run -e MESOS_SANDBOX=/mnt/mesos/sandbox -e MESOS_CONTAINER_NAME=mesos-docker-test -v src:target:rw -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_D3Fxye:/mnt/mesos/sandbox --volume-driver=flocker --net host --entrypoint /bin/sh --name mesos-docker-test busybox -c sleep 120
> ```
> Could see `-v src:target`
>
> # When don't use volume-driver
> ```
> 1003 23:53:48.028715 12125 docker.cpp:571] Running docker -H unix:///var/run/docker.sock run -e MESOS_SANDBOX=/mnt/mesos/sandbox -e MESOS_CONTAINER_NAME=mesos-docker-test -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt/src:target:rw -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt:/mnt/mesos/sandbox --net host --entrypoint /bin/sh --name mesos-docker-test busybox -c sleep 120
> ```
> Could see `-v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt/src:target:rw`
>
>
> Thanks,
>
> haosdent huang
>
>
Re: Review Request 38338: Enhanced option for Docker cli volume
plugin.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38338/#review101425
-----------------------------------------------------------
include/mesos/mesos.proto (line 1443)
<https://reviews.apache.org/r/38338/#comment158820>
Does v1 also needs to be updated?
src/docker/docker.cpp (line 463)
<https://reviews.apache.org/r/38338/#comment158821>
Is it possible to add a unit test for this change?
- Guangya Liu
On 十月 3, 2015, 3:55 p.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38338/
> -----------------------------------------------------------
>
> (Updated 十月 3, 2015, 3:55 p.m.)
>
>
> Review request for mesos and Timothy Chen.
>
>
> Bugs: MESOS-3392
> https://issues.apache.org/jira/browse/MESOS-3392
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Enhanced option for Docker cli volume plugin.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999
> src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7
>
> Diff: https://reviews.apache.org/r/38338/diff/
>
>
> Testing
> -------
>
> # When use volume-driver
> ```
> I1003 23:49:41.986047 11268 docker.cpp:571] Running docker -H unix:///var/run/docker.sock run -e MESOS_SANDBOX=/mnt/mesos/sandbox -e MESOS_CONTAINER_NAME=mesos-docker-test -v src:target:rw -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_D3Fxye:/mnt/mesos/sandbox --volume-driver=flocker --net host --entrypoint /bin/sh --name mesos-docker-test busybox -c sleep 120
> ```
> Could see `-v src:target`
>
> # When don't use volume-driver
> ```
> 1003 23:53:48.028715 12125 docker.cpp:571] Running docker -H unix:///var/run/docker.sock run -e MESOS_SANDBOX=/mnt/mesos/sandbox -e MESOS_CONTAINER_NAME=mesos-docker-test -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt/src:target:rw -v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt:/mnt/mesos/sandbox --net host --entrypoint /bin/sh --name mesos-docker-test busybox -c sleep 120
> ```
> Could see `-v /tmp/DockerTest_ROOT_DOCKER_volume_driver_jmtVzt/src:target:rw`
>
>
> Thanks,
>
> haosdent huang
>
>