You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Timothy Chen <tn...@apache.org> on 2015/11/12 01:29:37 UTC
Review Request 40211: Add mesos provisioner doc.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40211/
-----------------------------------------------------------
Review request for mesos, Jie Yu and Jojy Varghese.
Repository: mesos
Description
-------
Add mesos provisioner doc.
Diffs
-----
docs/mesos-provisioner.md PRE-CREATION
Diff: https://reviews.apache.org/r/40211/diff/
Testing
-------
Thanks,
Timothy Chen
Re: Review Request 40211: Add mesos provisioner doc.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40211/#review106184
-----------------------------------------------------------
Patch looks great!
Reviews applied: [40211]
All tests passed.
- Mesos ReviewBot
On Nov. 12, 2015, 12:29 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40211/
> -----------------------------------------------------------
>
> (Updated Nov. 12, 2015, 12:29 a.m.)
>
>
> Review request for mesos, Jie Yu and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add mesos provisioner doc.
>
>
> Diffs
> -----
>
> docs/mesos-provisioner.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40211/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 40211: Add mesos provisioner doc.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40211/#review106186
-----------------------------------------------------------
Patch looks great!
Reviews applied: [40211]
All tests passed.
- Mesos ReviewBot
On Nov. 12, 2015, 12:29 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40211/
> -----------------------------------------------------------
>
> (Updated Nov. 12, 2015, 12:29 a.m.)
>
>
> Review request for mesos, Jie Yu and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add mesos provisioner doc.
>
>
> Diffs
> -----
>
> docs/mesos-provisioner.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40211/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 40211: Add mesos provisioner doc.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40211/#review106703
-----------------------------------------------------------
I would suggest we have a single doc explaining the whole "unified containerizer" stuff (from framework API to agent configurations/components), rather than having multiple docs each explaining one piece of the big pie. Here is what I would suggest for the layout:
```
1. Motivation. Explain why we need 'unified containerizer' and what problem it's solving.
2. Framework API. Explain protobuf like Image, ContainerInfo::MesosInfo::image, Volume::image, etc.
3. Agent configuration. Explain the command line flags.
4. Internals. Firt, have a figure explaining the interaction between components (e.g., MesosContainerizer, filesystem isolator, provisioner, image provider, etc). Then, dive into each component in details and explain its corresponding agent flags.
```
docs/mesos-provisioner.md (line 2)
<https://reviews.apache.org/r/40211/#comment165446>
After reading the doc, I guess one thing missing here is that we need to have a brief summary about *why* we need image provisioner and what's its relationship with Mesos containerizer.
I guess we need to include the motivation why we need a unified containerizer here.
docs/mesos-provisioner.md (line 4)
<https://reviews.apache.org/r/40211/#comment165436>
Can you provide links to App Container Image (ACI) spec and Docker Image spec.
https://github.com/appc/spec/blob/master/spec/aci.md
https://github.com/docker/docker/blob/master/image/spec/v1.md
docs/mesos-provisioner.md (line 7)
<https://reviews.apache.org/r/40211/#comment165448>
Please briefly explain what is a layer.
Also, please explain how the backend stacks layers into one single root filesystem (e.g., copy, overlayfs, bind, etc.)
- Jie Yu
On Nov. 15, 2015, 7:26 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40211/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2015, 7:26 p.m.)
>
>
> Review request for mesos, Jie Yu and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add mesos provisioner doc.
>
>
> Diffs
> -----
>
> docs/mesos-provisioner.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40211/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 40211: Add mesos provisioner doc.
Posted by Timothy Chen <tn...@apache.org>.
> On Dec. 9, 2015, 1:42 a.m., Jie Yu wrote:
> > docs/mesos-provisioner.md, line 51
> > <https://reviews.apache.org/r/40211/diff/3/?file=1153688#file1153688line51>
> >
> > Why this limitation. I think we can still use image-in-volume even if command line executor is used, right?
I think it's just current code doesn't support it, but not a technical limitation. I'll remove this.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40211/#review109432
-----------------------------------------------------------
On Dec. 4, 2015, 9:26 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40211/
> -----------------------------------------------------------
>
> (Updated Dec. 4, 2015, 9:26 a.m.)
>
>
> Review request for mesos, Jie Yu and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add mesos provisioner doc.
>
>
> Diffs
> -----
>
> docs/mesos-provisioner.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40211/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 40211: Add mesos provisioner doc.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40211/#review109432
-----------------------------------------------------------
Ship it!
Looks good overall. Thanks Tim!
docs/mesos-provisioner.md (line 5)
<https://reviews.apache.org/r/40211/#comment168957>
It's not just container filesystem spec, right? It also includes runtime specification. Can you just say:
```
There are multiple container specifications, ...
```
For runC, it's only for runtime right? So your next sentense does not sound so right. How about this:
```
Most of the container specifications, to varying degrees, conflate image format specification with other components of a container, including...
```
docs/mesos-provisioner.md (line 7)
<https://reviews.apache.org/r/40211/#comment168955>
s/R/r/
s/different formats/different image formats/
docs/mesos-provisioner.md (line 13)
<https://reviews.apache.org/r/40211/#comment168961>
I wouldn't mention the runtime configuration here since it's pretty confusing. I would rather add a section later to explain the runtime configuration part (but not in this review probably).
docs/mesos-provisioner.md (line 51)
<https://reviews.apache.org/r/40211/#comment168962>
Why this limitation. I think we can still use image-in-volume even if command line executor is used, right?
docs/mesos-provisioner.md (line 59)
<https://reviews.apache.org/r/40211/#comment168963>
you probably want to explain the backend flag (or add a TODO) since it's not quite intuitive what that is.
docs/mesos-provisioner.md (line 63)
<https://reviews.apache.org/r/40211/#comment168965>
Kill extra blank line
docs/mesos-provisioner.md (line 67)
<https://reviews.apache.org/r/40211/#comment168964>
kill extra blank line
- Jie Yu
On Dec. 4, 2015, 9:26 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40211/
> -----------------------------------------------------------
>
> (Updated Dec. 4, 2015, 9:26 a.m.)
>
>
> Review request for mesos, Jie Yu and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add mesos provisioner doc.
>
>
> Diffs
> -----
>
> docs/mesos-provisioner.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40211/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 40211: Add mesos provisioner doc.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40211/#review108961
-----------------------------------------------------------
Patch looks great!
Reviews applied: [40211]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Dec. 4, 2015, 9:26 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40211/
> -----------------------------------------------------------
>
> (Updated Dec. 4, 2015, 9:26 a.m.)
>
>
> Review request for mesos, Jie Yu and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add mesos provisioner doc.
>
>
> Diffs
> -----
>
> docs/mesos-provisioner.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40211/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 40211: Add mesos provisioner doc.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40211/
-----------------------------------------------------------
(Updated Dec. 4, 2015, 9:26 a.m.)
Review request for mesos, Jie Yu and Jojy Varghese.
Repository: mesos
Description
-------
Add mesos provisioner doc.
Diffs (updated)
-----
docs/mesos-provisioner.md PRE-CREATION
Diff: https://reviews.apache.org/r/40211/diff/
Testing
-------
Thanks,
Timothy Chen
Re: Review Request 40211: Add mesos provisioner doc.
Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40211/#review106570
-----------------------------------------------------------
docs/mesos-provisioner.md (line 4)
<https://reviews.apache.org/r/40211/#comment165290>
to be created and managed...
docs/mesos-provisioner.md (line 21)
<https://reviews.apache.org/r/40211/#comment165289>
s/and download/download/
s/unpact/unpack/
- Cong Wang
On Nov. 15, 2015, 7:26 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40211/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2015, 7:26 p.m.)
>
>
> Review request for mesos, Jie Yu and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add mesos provisioner doc.
>
>
> Diffs
> -----
>
> docs/mesos-provisioner.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40211/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 40211: Add mesos provisioner doc.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40211/#review106569
-----------------------------------------------------------
Patch looks great!
Reviews applied: [40211]
All tests passed.
- Mesos ReviewBot
On Nov. 15, 2015, 7:26 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40211/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2015, 7:26 p.m.)
>
>
> Review request for mesos, Jie Yu and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add mesos provisioner doc.
>
>
> Diffs
> -----
>
> docs/mesos-provisioner.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40211/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 40211: Add mesos provisioner doc.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40211/
-----------------------------------------------------------
(Updated Nov. 15, 2015, 7:26 p.m.)
Review request for mesos, Jie Yu and Jojy Varghese.
Repository: mesos
Description
-------
Add mesos provisioner doc.
Diffs (updated)
-----
docs/mesos-provisioner.md PRE-CREATION
Diff: https://reviews.apache.org/r/40211/diff/
Testing
-------
Thanks,
Timothy Chen
Re: Review Request 40211: Add mesos provisioner doc.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40211/#review106182
-----------------------------------------------------------
docs/mesos-provisioner.md (line 13)
<https://reviews.apache.org/r/40211/#comment164876>
/s/linx/linux/g
docs/mesos-provisioner.md (line 20)
<https://reviews.apache.org/r/40211/#comment164877>
non-existed link
/s/https://github.com/apache/mesos/blob/master/docs/mesos-containerizer.md /https://github.com/apache/mesos/blob/master/docs/containerizer.md /g
- Gilbert Song
On Nov. 11, 2015, 4:29 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40211/
> -----------------------------------------------------------
>
> (Updated Nov. 11, 2015, 4:29 p.m.)
>
>
> Review request for mesos, Jie Yu and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add mesos provisioner doc.
>
>
> Diffs
> -----
>
> docs/mesos-provisioner.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40211/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Timothy Chen
>
>