You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Guangya Liu <gy...@gmail.com> on 2016/04/02 15:46:53 UTC
Re: Review Request 45377: Updated prepare() logic for unified
container.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45377/
-----------------------------------------------------------
(Updated 四月 2, 2016, 1:46 p.m.)
Review request for mesos, Gilbert Song and Jie Yu.
Summary (updated)
-----------------
Updated prepare() logic for unified container.
Bugs: MESOS-5013
https://issues.apache.org/jira/browse/MESOS-5013
Repository: mesos
Description (updated)
-------
Updated prepare() logic for unified container.
Diffs (updated)
-----
src/cli/execute.cpp af62f41622e1c43acd8f257c54f8754162c433b8
src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/45377/diff/
Testing (updated)
-------
Updating the execute.cpp for test, will remove it once test finished.
Thanks,
Guangya Liu
Re: Review Request 45377: Updated prepare() logic for unified
container.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45377/#review128981
-----------------------------------------------------------
Patch looks great!
Reviews applied: [45214, 45217, 45270, 45360, 45373, 45326, 46180, 45370, 44454, 45377]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On April 14, 2016, 4:30 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45377/
> -----------------------------------------------------------
>
> (Updated April 14, 2016, 4:30 p.m.)
>
>
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and Jie Yu.
>
>
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated prepare() logic for unified container.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp PRE-CREATION
> src/slave/slave.cpp de99e9eb5cc812b2e07deb749b98b4f4db363728
>
> Diff: https://reviews.apache.org/r/45377/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 45377: Ignored docker volume when updating
container volume path.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45377/#review130282
-----------------------------------------------------------
Fix it, then Ship it!
src/slave/slave.cpp (line 3825)
<https://reviews.apache.org/r/45377/#comment194002>
Add one more line above.
- Gilbert Song
On April 22, 2016, 7:03 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45377/
> -----------------------------------------------------------
>
> (Updated April 22, 2016, 7:03 p.m.)
>
>
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and Jie Yu.
>
>
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Ignored docker volume when updating container volume path.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp ebf26065326be67db435f1b097964c8ff2e1dbe0
>
> Diff: https://reviews.apache.org/r/45377/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 45377: Ignored docker volume when updating
container volume path.
Posted by Guangya Liu <gy...@gmail.com>.
> On 四月 24, 2016, 5:29 a.m., Jie Yu wrote:
> > src/slave/slave.cpp, lines 3826-3831
> > <https://reviews.apache.org/r/45377/diff/7/?file=1358660#file1358660line3826>
> >
> > Can you adjust the comments here? It's not just for docker volumes. For other volume source types we'll add in the future, it also applies, right?
>
> Guangya Liu wrote:
> Yes, may apply to other volume source types, what about adding a TODO here but keep the current comments unchanged, as currently, only docker volume is supported. what about adding a TODO here as `TODO(gyliu513): revist this part when introducing new volume source types.`
We may have another two sources as `HOST_PATH` and `IMAGE` but they may not fit into the case here as the `IMAGE` source will be provisioned and then set the rootfs as the container host path. https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/containerizer.cpp#L811-L835
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45377/#review130284
-----------------------------------------------------------
On 四月 24, 2016, 3:44 a.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45377/
> -----------------------------------------------------------
>
> (Updated 四月 24, 2016, 3:44 a.m.)
>
>
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and Jie Yu.
>
>
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Ignored docker volume when updating container volume path.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp ebf26065326be67db435f1b097964c8ff2e1dbe0
>
> Diff: https://reviews.apache.org/r/45377/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 45377: Ignored docker volume when updating
container volume path.
Posted by Guangya Liu <gy...@gmail.com>.
> On 四月 24, 2016, 5:29 a.m., Jie Yu wrote:
> > src/slave/slave.cpp, lines 3826-3831
> > <https://reviews.apache.org/r/45377/diff/7/?file=1358660#file1358660line3826>
> >
> > Can you adjust the comments here? It's not just for docker volumes. For other volume source types we'll add in the future, it also applies, right?
Yes, may apply to other volume source types, what about adding a TODO here but keep the current comments unchanged, as currently, only docker volume is supported. what about adding a TODO here as `TODO(gyliu513): revist this part when introducing new volume source types.`
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45377/#review130284
-----------------------------------------------------------
On 四月 24, 2016, 3:44 a.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45377/
> -----------------------------------------------------------
>
> (Updated 四月 24, 2016, 3:44 a.m.)
>
>
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and Jie Yu.
>
>
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Ignored docker volume when updating container volume path.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp ebf26065326be67db435f1b097964c8ff2e1dbe0
>
> Diff: https://reviews.apache.org/r/45377/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 45377: Ignored docker volume when updating
container volume path.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45377/#review130284
-----------------------------------------------------------
src/slave/slave.cpp (lines 3826 - 3831)
<https://reviews.apache.org/r/45377/#comment194004>
Can you adjust the comments here? It's not just for docker volumes. For other volume source types we'll add in the future, it also applies, right?
- Jie Yu
On April 24, 2016, 3:44 a.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45377/
> -----------------------------------------------------------
>
> (Updated April 24, 2016, 3:44 a.m.)
>
>
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and Jie Yu.
>
>
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Ignored docker volume when updating container volume path.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp ebf26065326be67db435f1b097964c8ff2e1dbe0
>
> Diff: https://reviews.apache.org/r/45377/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 45377: Ignored docker volume when updating
container volume path.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45377/
-----------------------------------------------------------
(Updated 四月 24, 2016, 8:08 a.m.)
Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and Jie Yu.
Bugs: MESOS-5013
https://issues.apache.org/jira/browse/MESOS-5013
Repository: mesos
Description
-------
Ignored docker volume when updating container volume path.
Diffs (updated)
-----
src/slave/slave.cpp ebf26065326be67db435f1b097964c8ff2e1dbe0
Diff: https://reviews.apache.org/r/45377/diff/
Testing
-------
make
make check
Thanks,
Guangya Liu
Re: Review Request 45377: Ignored docker volume when updating
container volume path.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45377/
-----------------------------------------------------------
(Updated 四月 24, 2016, 3:44 a.m.)
Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and Jie Yu.
Bugs: MESOS-5013
https://issues.apache.org/jira/browse/MESOS-5013
Repository: mesos
Description
-------
Ignored docker volume when updating container volume path.
Diffs (updated)
-----
src/slave/slave.cpp ebf26065326be67db435f1b097964c8ff2e1dbe0
Diff: https://reviews.apache.org/r/45377/diff/
Testing
-------
make
make check
Thanks,
Guangya Liu
Re: Review Request 45377: Ignored docker volume when updating
container volume path.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45377/#review130245
-----------------------------------------------------------
Patch looks great!
Reviews applied: [45377]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On April 23, 2016, 2:03 a.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45377/
> -----------------------------------------------------------
>
> (Updated April 23, 2016, 2:03 a.m.)
>
>
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and Jie Yu.
>
>
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Ignored docker volume when updating container volume path.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp ebf26065326be67db435f1b097964c8ff2e1dbe0
>
> Diff: https://reviews.apache.org/r/45377/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 45377: Ignored docker volume when updating
container volume path.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45377/
-----------------------------------------------------------
(Updated 四月 23, 2016, 2:03 a.m.)
Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and Jie Yu.
Summary (updated)
-----------------
Ignored docker volume when updating container volume path.
Bugs: MESOS-5013
https://issues.apache.org/jira/browse/MESOS-5013
Repository: mesos
Description (updated)
-------
Ignored docker volume when updating container volume path.
Diffs (updated)
-----
src/slave/slave.cpp ebf26065326be67db435f1b097964c8ff2e1dbe0
Diff: https://reviews.apache.org/r/45377/diff/
Testing
-------
make
make check
Thanks,
Guangya Liu
Re: Review Request 45377: Updated prepare() logic for unified
container.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45377/
-----------------------------------------------------------
(Updated 四月 14, 2016, 4:30 p.m.)
Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and Jie Yu.
Bugs: MESOS-5013
https://issues.apache.org/jira/browse/MESOS-5013
Repository: mesos
Description
-------
Updated prepare() logic for unified container.
Diffs (updated)
-----
src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp PRE-CREATION
src/slave/slave.cpp de99e9eb5cc812b2e07deb749b98b4f4db363728
Diff: https://reviews.apache.org/r/45377/diff/
Testing (updated)
-------
make
make check
Thanks,
Guangya Liu
Re: Review Request 45377: Updated prepare() logic for unified
container.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45377/#review126981
-----------------------------------------------------------
Could we keep the cli/execute.cpp changes private for now?
and rebase this with the prepare() method implementation.
- Gilbert Song
On April 4, 2016, 2:28 a.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45377/
> -----------------------------------------------------------
>
> (Updated April 4, 2016, 2:28 a.m.)
>
>
> Review request for mesos, Gilbert Song and Jie Yu.
>
>
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated prepare() logic for unified container.
>
>
> Diffs
> -----
>
> src/cli/execute.cpp af62f41622e1c43acd8f257c54f8754162c433b8
> src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45377/diff/
>
>
> Testing
> -------
>
> Updating the execute.cpp for test, will remove it once test finished.
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 45377: Updated prepare() logic for unified
container.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45377/
-----------------------------------------------------------
(Updated 四月 4, 2016, 9:28 a.m.)
Review request for mesos, Gilbert Song and Jie Yu.
Bugs: MESOS-5013
https://issues.apache.org/jira/browse/MESOS-5013
Repository: mesos
Description
-------
Updated prepare() logic for unified container.
Diffs (updated)
-----
src/cli/execute.cpp af62f41622e1c43acd8f257c54f8754162c433b8
src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/45377/diff/
Testing
-------
Updating the execute.cpp for test, will remove it once test finished.
Thanks,
Guangya Liu