You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jiang Yan Xu <ya...@jxu.me> on 2015/08/12 07:49:55 UTC

Review Request 37382: Introduced provisioner Backend interface.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37382/
-----------------------------------------------------------

Review request for mesos, Lily Chen, Ian Downes, Jie Yu, and Timothy Chen.


Bugs: MESOS-2968
    https://issues.apache.org/jira/browse/MESOS-2968


Repository: mesos


Description
-------

The Backend interface is made generic so it can be used by different provisioners. See [MESOS-2968](https://issues.apache.org/jira/browse/MESOS-2968?focusedCommentId=14652859&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14652859) for details.


Diffs
-----

  src/Makefile.am 07502f0f5523b972888ceab9cf3af309c8441d7f 
  src/slave/containerizer/provisioners/backend.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/backend.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/37382/diff/


Testing
-------

make check.


Thanks,

Jiang Yan Xu


Re: Review Request 37382: Introduced provisioner Backend interface.

Posted by Lily Chen <li...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37382/#review95113
-----------------------------------------------------------



src/slave/containerizer/provisioners/backend.hpp (line 52)
<https://reviews.apache.org/r/37382/#comment149928>

    Should this return a Future<bool> since the provisioner interface returns a Future<bool> for destroy?


- Lily Chen


On Aug. 12, 2015, 5:49 a.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37382/
> -----------------------------------------------------------
> 
> (Updated Aug. 12, 2015, 5:49 a.m.)
> 
> 
> Review request for mesos, Lily Chen, Ian Downes, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2968
>     https://issues.apache.org/jira/browse/MESOS-2968
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The Backend interface is made generic so it can be used by different provisioners. See [MESOS-2968](https://issues.apache.org/jira/browse/MESOS-2968?focusedCommentId=14652859&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14652859) for details.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 07502f0f5523b972888ceab9cf3af309c8441d7f 
>   src/slave/containerizer/provisioners/backend.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/backend.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37382/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>


Re: Review Request 37382: Introduced provisioner Backend interface.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37382/#review95065
-----------------------------------------------------------


Patch looks great!

Reviews applied: [37382]

All tests passed.

- Mesos ReviewBot


On Aug. 12, 2015, 5:49 a.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37382/
> -----------------------------------------------------------
> 
> (Updated Aug. 12, 2015, 5:49 a.m.)
> 
> 
> Review request for mesos, Lily Chen, Ian Downes, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2968
>     https://issues.apache.org/jira/browse/MESOS-2968
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The Backend interface is made generic so it can be used by different provisioners. See [MESOS-2968](https://issues.apache.org/jira/browse/MESOS-2968?focusedCommentId=14652859&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14652859) for details.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 07502f0f5523b972888ceab9cf3af309c8441d7f 
>   src/slave/containerizer/provisioners/backend.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/backend.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37382/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>


Re: Review Request 37382: Introduced provisioner Backend interface.

Posted by Jie Yu <yu...@gmail.com>.

> On Aug. 14, 2015, 6:15 p.m., Jie Yu wrote:
> > src/slave/containerizer/provisioners/backend.hpp, lines 55-57
> > <https://reviews.apache.org/r/37382/diff/2/?file=1040433#file1040433line55>
> >
> >     Hum, what do yo mean here?
> >     
> >     I think the backend should be container aware. I.e., we should pass in containerid in 'provision'  and destroy will take a containerid as well.
> >     
> >     There could be multiple rootfs for a container (because image in volumes). When the container finishes, the provisioner will try to destroy all rootfs provisioned for that container.
> >     
> >     Thoughts?
> 
> Jiang Yan Xu wrote:
>     This is intended for the image in volumes case but I was trying to not be too explicit because it's not implemented yet (we should track it with a separate ticket).
>     
>     My comment about 'nested' was incorrect, let me clarify here.
>     
>     The 'caller' is the provisioner. What I had in mind was: each Backend call handles one image, if `Backend::provision()`s one image and then it should `destroy()` the rootfs provisioned by one image.
>     
>     Suppose we have such a rootfs dir and each container has multiple rootfses, some from ContainerInfo::volumnes and one from ContainerInfo::mesos::image but they are stored side-by-side under <container_id>/<image_type>.
>     
>     ```
>     <--rootfs_dir>
>     |--<container_id>
>        |--<appc>
>           |--<image_id>
>           |--<image_id>
>        |--<docker>
>     ```
>     
>     IMO since LinuxFilesystemIsolatorProcess::cleanup() calls each `Provisioner::destory(containerId)` (appc|docker), the provisioner can look at the subdir it manages and scan it and call `Backend::destroy(rootfsPath)` for each image under it.
>     
>     So each `Backend::destroy(rootfsPath)` call destroys one rootfs and there is no `nesting` under it.
>     Passing `containerId` into Backend isn't as clean as it should be the provisioner who converts the id into a path and Backends are this dumb thing which only knows paths.
>     
>     More clear?

So the directory layout listed above is managed by the provisioner or backend? It cannot be backend because otherwise that means provisioner needs to know about the layout used by the backend.

So if it's managed by provisioner, the layout should probably be:
```
/var/lib/mesos/provisioners/
    |-- appc/
          |-- <container_id>
                 |-- <image1>
                 |-- <image2>
    |-- docker/
          |-- <container_id>
                 |-- <image3>
                 |-- <image4>
```

So we only allow one backend, or we allow multiple backends (e.g., what if we want to use bind mount backend for appc but overlayfs backend for docker, or we want to gradually roll out overlayfs backend so that two backends co-exists for a while)? If the later, how can we figure out which backend to use when destroying root filesystems?

I just want to get the full picture sorted out. We can chat next week.


- Jie


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37382/#review95455
-----------------------------------------------------------


On Aug. 14, 2015, 5:51 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37382/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2015, 5:51 p.m.)
> 
> 
> Review request for mesos, Lily Chen, Ian Downes, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2968
>     https://issues.apache.org/jira/browse/MESOS-2968
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The Backend interface is made generic so it can be used by different provisioners. See [MESOS-2968](https://issues.apache.org/jira/browse/MESOS-2968?focusedCommentId=14652859&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14652859) for details.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am b5dbdc316cad179cd265bd81900999fab35940b9 
>   src/slave/containerizer/provisioners/backend.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/backend.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37382/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>


Re: Review Request 37382: Introduced provisioner Backend interface.

Posted by Jiang Yan Xu <ya...@jxu.me>.

> On Aug. 14, 2015, 11:15 a.m., Jie Yu wrote:
> > src/slave/containerizer/provisioners/backend.hpp, lines 55-57
> > <https://reviews.apache.org/r/37382/diff/2/?file=1040433#file1040433line55>
> >
> >     Hum, what do yo mean here?
> >     
> >     I think the backend should be container aware. I.e., we should pass in containerid in 'provision'  and destroy will take a containerid as well.
> >     
> >     There could be multiple rootfs for a container (because image in volumes). When the container finishes, the provisioner will try to destroy all rootfs provisioned for that container.
> >     
> >     Thoughts?

This is intended for the image in volumes case but I was trying to not be too explicit because it's not implemented yet (we should track it with a separate ticket).

My comment about 'nested' was incorrect, let me clarify here.

The 'caller' is the provisioner. What I had in mind was: each Backend call handles one image, if `Backend::provision()`s one image and then it should `destroy()` the rootfs provisioned by one image.

Suppose we have such a rootfs dir and each container has multiple rootfses, some from ContainerInfo::volumnes and one from ContainerInfo::mesos::image but they are stored side-by-side under <container_id>/<image_type>.

```
<--rootfs_dir>
|--<container_id>
   |--<appc>
      |--<image_id>
      |--<image_id>
   |--<docker>
```

IMO since LinuxFilesystemIsolatorProcess::cleanup() calls each `Provisioner::destory(containerId)` (appc|docker), the provisioner can look at the subdir it manages and scan it and call `Backend::destroy(rootfsPath)` for each image under it.

So each `Backend::destroy(rootfsPath)` call destroys one rootfs and there is no `nesting` under it.
Passing `containerId` into Backend isn't as clean as it should be the provisioner who converts the id into a path and Backends are this dumb thing which only knows paths.

More clear?


- Jiang Yan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37382/#review95455
-----------------------------------------------------------


On Aug. 14, 2015, 10:51 a.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37382/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2015, 10:51 a.m.)
> 
> 
> Review request for mesos, Lily Chen, Ian Downes, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2968
>     https://issues.apache.org/jira/browse/MESOS-2968
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The Backend interface is made generic so it can be used by different provisioners. See [MESOS-2968](https://issues.apache.org/jira/browse/MESOS-2968?focusedCommentId=14652859&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14652859) for details.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am b5dbdc316cad179cd265bd81900999fab35940b9 
>   src/slave/containerizer/provisioners/backend.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/backend.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37382/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>


Re: Review Request 37382: Introduced provisioner Backend interface.

Posted by Jiang Yan Xu <ya...@jxu.me>.

> On Aug. 14, 2015, 11:15 a.m., Jie Yu wrote:
> > src/slave/containerizer/provisioners/backend.hpp, lines 55-57
> > <https://reviews.apache.org/r/37382/diff/2/?file=1040433#file1040433line55>
> >
> >     Hum, what do yo mean here?
> >     
> >     I think the backend should be container aware. I.e., we should pass in containerid in 'provision'  and destroy will take a containerid as well.
> >     
> >     There could be multiple rootfs for a container (because image in volumes). When the container finishes, the provisioner will try to destroy all rootfs provisioned for that container.
> >     
> >     Thoughts?
> 
> Jiang Yan Xu wrote:
>     This is intended for the image in volumes case but I was trying to not be too explicit because it's not implemented yet (we should track it with a separate ticket).
>     
>     My comment about 'nested' was incorrect, let me clarify here.
>     
>     The 'caller' is the provisioner. What I had in mind was: each Backend call handles one image, if `Backend::provision()`s one image and then it should `destroy()` the rootfs provisioned by one image.
>     
>     Suppose we have such a rootfs dir and each container has multiple rootfses, some from ContainerInfo::volumnes and one from ContainerInfo::mesos::image but they are stored side-by-side under <container_id>/<image_type>.
>     
>     ```
>     <--rootfs_dir>
>     |--<container_id>
>        |--<appc>
>           |--<image_id>
>           |--<image_id>
>        |--<docker>
>     ```
>     
>     IMO since LinuxFilesystemIsolatorProcess::cleanup() calls each `Provisioner::destory(containerId)` (appc|docker), the provisioner can look at the subdir it manages and scan it and call `Backend::destroy(rootfsPath)` for each image under it.
>     
>     So each `Backend::destroy(rootfsPath)` call destroys one rootfs and there is no `nesting` under it.
>     Passing `containerId` into Backend isn't as clean as it should be the provisioner who converts the id into a path and Backends are this dumb thing which only knows paths.
>     
>     More clear?
> 
> Jie Yu wrote:
>     So the directory layout listed above is managed by the provisioner or backend? It cannot be backend because otherwise that means provisioner needs to know about the layout used by the backend.
>     
>     So if it's managed by provisioner, the layout should probably be:
>     ```
>     /var/lib/mesos/provisioners/
>         |-- appc/
>               |-- <container_id>
>                      |-- <image1>
>                      |-- <image2>
>         |-- docker/
>               |-- <container_id>
>                      |-- <image3>
>                      |-- <image4>
>     ```
>     
>     So we only allow one backend, or we allow multiple backends (e.g., what if we want to use bind mount backend for appc but overlayfs backend for docker, or we want to gradually roll out overlayfs backend so that two backends co-exists for a while)? If the later, how can we figure out which backend to use when destroying root filesystems?
>     
>     I just want to get the full picture sorted out. We can chat next week.

Note that the above rootfs I mentioned is the result of the Backend either collapsing its layers or overlay them and in the provisioner's view it's just one entity.

So the backend know about layout within a rootfs (e.g., its knows that a rootfs is "ImageA(a script) -> ImageB(essential software) ->ImageC(base OS)"); the provisioner knows about / manages the direcotry layout of 'rootfs'es but not inside 'rootfs'es (e.g. This container wants three rootfses, two of which from volumes).

Yeah I agree the interface design requires a full picture of multi-image-per-contain design. I will jot down my thoughts in a doc and let's chat next week.


- Jiang Yan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37382/#review95455
-----------------------------------------------------------


On Aug. 14, 2015, 10:51 a.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37382/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2015, 10:51 a.m.)
> 
> 
> Review request for mesos, Lily Chen, Ian Downes, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2968
>     https://issues.apache.org/jira/browse/MESOS-2968
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The Backend interface is made generic so it can be used by different provisioners. See [MESOS-2968](https://issues.apache.org/jira/browse/MESOS-2968?focusedCommentId=14652859&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14652859) for details.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am b5dbdc316cad179cd265bd81900999fab35940b9 
>   src/slave/containerizer/provisioners/backend.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/backend.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37382/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>


Re: Review Request 37382: Introduced provisioner Backend interface.

Posted by Timothy Chen <tn...@apache.org>.

> On Aug. 14, 2015, 6:15 p.m., Jie Yu wrote:
> > src/slave/containerizer/provisioners/backend.hpp, lines 55-57
> > <https://reviews.apache.org/r/37382/diff/2/?file=1040433#file1040433line55>
> >
> >     Hum, what do yo mean here?
> >     
> >     I think the backend should be container aware. I.e., we should pass in containerid in 'provision'  and destroy will take a containerid as well.
> >     
> >     There could be multiple rootfs for a container (because image in volumes). When the container finishes, the provisioner will try to destroy all rootfs provisioned for that container.
> >     
> >     Thoughts?
> 
> Jiang Yan Xu wrote:
>     This is intended for the image in volumes case but I was trying to not be too explicit because it's not implemented yet (we should track it with a separate ticket).
>     
>     My comment about 'nested' was incorrect, let me clarify here.
>     
>     The 'caller' is the provisioner. What I had in mind was: each Backend call handles one image, if `Backend::provision()`s one image and then it should `destroy()` the rootfs provisioned by one image.
>     
>     Suppose we have such a rootfs dir and each container has multiple rootfses, some from ContainerInfo::volumnes and one from ContainerInfo::mesos::image but they are stored side-by-side under <container_id>/<image_type>.
>     
>     ```
>     <--rootfs_dir>
>     |--<container_id>
>        |--<appc>
>           |--<image_id>
>           |--<image_id>
>        |--<docker>
>     ```
>     
>     IMO since LinuxFilesystemIsolatorProcess::cleanup() calls each `Provisioner::destory(containerId)` (appc|docker), the provisioner can look at the subdir it manages and scan it and call `Backend::destroy(rootfsPath)` for each image under it.
>     
>     So each `Backend::destroy(rootfsPath)` call destroys one rootfs and there is no `nesting` under it.
>     Passing `containerId` into Backend isn't as clean as it should be the provisioner who converts the id into a path and Backends are this dumb thing which only knows paths.
>     
>     More clear?
> 
> Jie Yu wrote:
>     So the directory layout listed above is managed by the provisioner or backend? It cannot be backend because otherwise that means provisioner needs to know about the layout used by the backend.
>     
>     So if it's managed by provisioner, the layout should probably be:
>     ```
>     /var/lib/mesos/provisioners/
>         |-- appc/
>               |-- <container_id>
>                      |-- <image1>
>                      |-- <image2>
>         |-- docker/
>               |-- <container_id>
>                      |-- <image3>
>                      |-- <image4>
>     ```
>     
>     So we only allow one backend, or we allow multiple backends (e.g., what if we want to use bind mount backend for appc but overlayfs backend for docker, or we want to gradually roll out overlayfs backend so that two backends co-exists for a while)? If the later, how can we figure out which backend to use when destroying root filesystems?
>     
>     I just want to get the full picture sorted out. We can chat next week.
> 
> Jiang Yan Xu wrote:
>     Note that the above rootfs I mentioned is the result of the Backend either collapsing its layers or overlay them and in the provisioner's view it's just one entity.
>     
>     So the backend know about layout within a rootfs (e.g., its knows that a rootfs is "ImageA(a script) -> ImageB(essential software) ->ImageC(base OS)"); the provisioner knows about / manages the direcotry layout of 'rootfs'es but not inside 'rootfs'es (e.g. This container wants three rootfses, two of which from volumes).
>     
>     Yeah I agree the interface design requires a full picture of multi-image-per-contain design. I will jot down my thoughts in a doc and let's chat next week.

Hey we should talk about this as it affects all of our work, I think I didn't realize we're ending up doing multiple provisioner images for a single executor.
I think my assumption was that the backend when given a provision call is for a image as Yan mentioned, and returning the rootfs directory path which the provisioner can hold on to know where the backend has provisioned it to.

We don't really allow multiple backends with our current design, I don't think there is anything stopping us but it then become a very interesting API choice to see where we allow the user to specify this. My intuition is to just use one single backend and we record the backend that is used to provisioner a rootfs, so at least on recovery we can say we can't recover due to a backend change.


- Timothy


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37382/#review95455
-----------------------------------------------------------


On Aug. 14, 2015, 5:51 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37382/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2015, 5:51 p.m.)
> 
> 
> Review request for mesos, Lily Chen, Ian Downes, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2968
>     https://issues.apache.org/jira/browse/MESOS-2968
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The Backend interface is made generic so it can be used by different provisioners. See [MESOS-2968](https://issues.apache.org/jira/browse/MESOS-2968?focusedCommentId=14652859&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14652859) for details.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am b5dbdc316cad179cd265bd81900999fab35940b9 
>   src/slave/containerizer/provisioners/backend.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/backend.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37382/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>


Re: Review Request 37382: Introduced provisioner Backend interface.

Posted by Jiang Yan Xu <ya...@jxu.me>.

> On Aug. 14, 2015, 11:15 a.m., Jie Yu wrote:
> > src/slave/containerizer/provisioners/backend.hpp, lines 55-57
> > <https://reviews.apache.org/r/37382/diff/2/?file=1040433#file1040433line55>
> >
> >     Hum, what do yo mean here?
> >     
> >     I think the backend should be container aware. I.e., we should pass in containerid in 'provision'  and destroy will take a containerid as well.
> >     
> >     There could be multiple rootfs for a container (because image in volumes). When the container finishes, the provisioner will try to destroy all rootfs provisioned for that container.
> >     
> >     Thoughts?
> 
> Jiang Yan Xu wrote:
>     This is intended for the image in volumes case but I was trying to not be too explicit because it's not implemented yet (we should track it with a separate ticket).
>     
>     My comment about 'nested' was incorrect, let me clarify here.
>     
>     The 'caller' is the provisioner. What I had in mind was: each Backend call handles one image, if `Backend::provision()`s one image and then it should `destroy()` the rootfs provisioned by one image.
>     
>     Suppose we have such a rootfs dir and each container has multiple rootfses, some from ContainerInfo::volumnes and one from ContainerInfo::mesos::image but they are stored side-by-side under <container_id>/<image_type>.
>     
>     ```
>     <--rootfs_dir>
>     |--<container_id>
>        |--<appc>
>           |--<image_id>
>           |--<image_id>
>        |--<docker>
>     ```
>     
>     IMO since LinuxFilesystemIsolatorProcess::cleanup() calls each `Provisioner::destory(containerId)` (appc|docker), the provisioner can look at the subdir it manages and scan it and call `Backend::destroy(rootfsPath)` for each image under it.
>     
>     So each `Backend::destroy(rootfsPath)` call destroys one rootfs and there is no `nesting` under it.
>     Passing `containerId` into Backend isn't as clean as it should be the provisioner who converts the id into a path and Backends are this dumb thing which only knows paths.
>     
>     More clear?
> 
> Jie Yu wrote:
>     So the directory layout listed above is managed by the provisioner or backend? It cannot be backend because otherwise that means provisioner needs to know about the layout used by the backend.
>     
>     So if it's managed by provisioner, the layout should probably be:
>     ```
>     /var/lib/mesos/provisioners/
>         |-- appc/
>               |-- <container_id>
>                      |-- <image1>
>                      |-- <image2>
>         |-- docker/
>               |-- <container_id>
>                      |-- <image3>
>                      |-- <image4>
>     ```
>     
>     So we only allow one backend, or we allow multiple backends (e.g., what if we want to use bind mount backend for appc but overlayfs backend for docker, or we want to gradually roll out overlayfs backend so that two backends co-exists for a while)? If the later, how can we figure out which backend to use when destroying root filesystems?
>     
>     I just want to get the full picture sorted out. We can chat next week.
> 
> Jiang Yan Xu wrote:
>     Note that the above rootfs I mentioned is the result of the Backend either collapsing its layers or overlay them and in the provisioner's view it's just one entity.
>     
>     So the backend know about layout within a rootfs (e.g., its knows that a rootfs is "ImageA(a script) -> ImageB(essential software) ->ImageC(base OS)"); the provisioner knows about / manages the direcotry layout of 'rootfs'es but not inside 'rootfs'es (e.g. This container wants three rootfses, two of which from volumes).
>     
>     Yeah I agree the interface design requires a full picture of multi-image-per-contain design. I will jot down my thoughts in a doc and let's chat next week.
> 
> Timothy Chen wrote:
>     Hey we should talk about this as it affects all of our work, I think I didn't realize we're ending up doing multiple provisioner images for a single executor.
>     I think my assumption was that the backend when given a provision call is for a image as Yan mentioned, and returning the rootfs directory path which the provisioner can hold on to know where the backend has provisioned it to.
>     
>     We don't really allow multiple backends with our current design, I don't think there is anything stopping us but it then become a very interesting API choice to see where we allow the user to specify this. My intuition is to just use one single backend and we record the backend that is used to provisioner a rootfs, so at least on recovery we can say we can't recover due to a backend change.

Jie and I chatted offline and I am going to submit another review regarding the file system layout that addresses multiple provisioners / mutliple backends. The current Backend interface design is compatible with it and not itself controversial so I am just going to make some documentation adjustments here.

Regardign a few issues which I'll describe in details in another review:

Multiple provisioners: To support [MESOS-3004](https://issues.apache.org/jira/browse/MESOS-3004) there are already multiple images within a container and there is no real obstacle to prevent them from being different image types.

Multiple backends: I agree that we don't have a use case for allowing the users to choose the backend (so we don't need to design the API for it) but the case for updating the Backend for a running cluster is real. We don't have to turn away a rootfs known to one of the backends during recovery / cleanup just because it's not the currently selected backend for new containers. I'll explain in a new review.


- Jiang Yan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37382/#review95455
-----------------------------------------------------------


On Aug. 14, 2015, 10:51 a.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37382/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2015, 10:51 a.m.)
> 
> 
> Review request for mesos, Lily Chen, Ian Downes, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2968
>     https://issues.apache.org/jira/browse/MESOS-2968
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The Backend interface is made generic so it can be used by different provisioners. See [MESOS-2968](https://issues.apache.org/jira/browse/MESOS-2968?focusedCommentId=14652859&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14652859) for details.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am b5dbdc316cad179cd265bd81900999fab35940b9 
>   src/slave/containerizer/provisioners/backend.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/backend.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37382/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>


Re: Review Request 37382: Introduced provisioner Backend interface.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37382/#review95455
-----------------------------------------------------------



src/slave/containerizer/provisioners/backend.hpp (line 49)
<https://reviews.apache.org/r/37382/#comment150400>

    s/roots/layers/ ?



src/slave/containerizer/provisioners/backend.hpp (line 50)
<https://reviews.apache.org/r/37382/#comment150401>

    How about s/directory/rootfs/ to make it more clear that 'directory' is the path to the rootfs?



src/slave/containerizer/provisioners/backend.hpp (lines 55 - 57)
<https://reviews.apache.org/r/37382/#comment150402>

    Hum, what do yo mean here?
    
    I think the backend should be container aware. I.e., we should pass in containerid in 'provision'  and destroy will take a containerid as well.
    
    There could be multiple rootfs for a container (because image in volumes). When the container finishes, the provisioner will try to destroy all rootfs provisioned for that container.
    
    Thoughts?


- Jie Yu


On Aug. 14, 2015, 5:51 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37382/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2015, 5:51 p.m.)
> 
> 
> Review request for mesos, Lily Chen, Ian Downes, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2968
>     https://issues.apache.org/jira/browse/MESOS-2968
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The Backend interface is made generic so it can be used by different provisioners. See [MESOS-2968](https://issues.apache.org/jira/browse/MESOS-2968?focusedCommentId=14652859&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14652859) for details.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am b5dbdc316cad179cd265bd81900999fab35940b9 
>   src/slave/containerizer/provisioners/backend.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/backend.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37382/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>


Re: Review Request 37382: Introduced provisioner Backend interface.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37382/#review95471
-----------------------------------------------------------


Patch looks great!

Reviews applied: [37382]

All tests passed.

- Mesos ReviewBot


On Aug. 14, 2015, 5:51 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37382/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2015, 5:51 p.m.)
> 
> 
> Review request for mesos, Lily Chen, Ian Downes, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2968
>     https://issues.apache.org/jira/browse/MESOS-2968
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The Backend interface is made generic so it can be used by different provisioners. See [MESOS-2968](https://issues.apache.org/jira/browse/MESOS-2968?focusedCommentId=14652859&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14652859) for details.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am b5dbdc316cad179cd265bd81900999fab35940b9 
>   src/slave/containerizer/provisioners/backend.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/backend.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37382/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>


Re: Review Request 37382: Introduced provisioner Backend interface.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37382/#review95886
-----------------------------------------------------------

Ship it!


Ship It!

- Jie Yu


On Aug. 19, 2015, 10:32 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37382/
> -----------------------------------------------------------
> 
> (Updated Aug. 19, 2015, 10:32 p.m.)
> 
> 
> Review request for mesos, Lily Chen, Ian Downes, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2968
>     https://issues.apache.org/jira/browse/MESOS-2968
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The Backend interface is made generic so it can be used by different provisioners. See [MESOS-2968](https://issues.apache.org/jira/browse/MESOS-2968?focusedCommentId=14652859&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14652859) for details.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 457ad26ee55bd7a2aedf27f45db58a9a4a6a5dc5 
>   src/slave/containerizer/provisioners/backend.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/backend.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37382/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>


Re: Review Request 37382: Introduced provisioner Backend interface.

Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37382/
-----------------------------------------------------------

(Updated Aug. 19, 2015, 3:32 p.m.)


Review request for mesos, Lily Chen, Ian Downes, Jie Yu, and Timothy Chen.


Changes
-------

Comments. NNFR.


Bugs: MESOS-2968
    https://issues.apache.org/jira/browse/MESOS-2968


Repository: mesos


Description
-------

The Backend interface is made generic so it can be used by different provisioners. See [MESOS-2968](https://issues.apache.org/jira/browse/MESOS-2968?focusedCommentId=14652859&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14652859) for details.


Diffs (updated)
-----

  src/Makefile.am 457ad26ee55bd7a2aedf27f45db58a9a4a6a5dc5 
  src/slave/containerizer/provisioners/backend.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/backend.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/37382/diff/


Testing
-------

make check.


Thanks,

Jiang Yan Xu


Re: Review Request 37382: Introduced provisioner Backend interface.

Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37382/
-----------------------------------------------------------

(Updated Aug. 14, 2015, 10:51 a.m.)


Review request for mesos, Lily Chen, Ian Downes, Jie Yu, and Timothy Chen.


Changes
-------

Comments.


Bugs: MESOS-2968
    https://issues.apache.org/jira/browse/MESOS-2968


Repository: mesos


Description
-------

The Backend interface is made generic so it can be used by different provisioners. See [MESOS-2968](https://issues.apache.org/jira/browse/MESOS-2968?focusedCommentId=14652859&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14652859) for details.


Diffs (updated)
-----

  src/Makefile.am b5dbdc316cad179cd265bd81900999fab35940b9 
  src/slave/containerizer/provisioners/backend.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/backend.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/37382/diff/


Testing
-------

make check.


Thanks,

Jiang Yan Xu


Re: Review Request 37382: Introduced provisioner Backend interface.

Posted by Jiang Yan Xu <ya...@jxu.me>.

> On Aug. 12, 2015, 1:45 a.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioners/backend.hpp, line 48
> > <https://reviews.apache.org/r/37382/diff/1/?file=1038090#file1038090line48>
> >
> >     Should we just use Path here?

Path is still hard to use in an interface particularly when it comes to joins: Path is implicitly convertable to string but not vice versa, join results are strings; already checked in code in the provisioning / FS isolation components don't use Path in the interface. We can revisit this in the future but for now I think we should still use string.


- Jiang Yan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37382/#review95069
-----------------------------------------------------------


On Aug. 14, 2015, 10:51 a.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37382/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2015, 10:51 a.m.)
> 
> 
> Review request for mesos, Lily Chen, Ian Downes, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2968
>     https://issues.apache.org/jira/browse/MESOS-2968
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The Backend interface is made generic so it can be used by different provisioners. See [MESOS-2968](https://issues.apache.org/jira/browse/MESOS-2968?focusedCommentId=14652859&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14652859) for details.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am b5dbdc316cad179cd265bd81900999fab35940b9 
>   src/slave/containerizer/provisioners/backend.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/backend.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37382/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>


Re: Review Request 37382: Introduced provisioner Backend interface.

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37382/#review95069
-----------------------------------------------------------

Ship it!


Ship It!


src/slave/containerizer/provisioners/backend.hpp (line 48)
<https://reviews.apache.org/r/37382/#comment149879>

    Should we just use Path here?



src/slave/containerizer/provisioners/backend.hpp (line 49)
<https://reviews.apache.org/r/37382/#comment149880>

    Probably good to mention what a "directory" here means.


- Timothy Chen


On Aug. 12, 2015, 5:49 a.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37382/
> -----------------------------------------------------------
> 
> (Updated Aug. 12, 2015, 5:49 a.m.)
> 
> 
> Review request for mesos, Lily Chen, Ian Downes, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2968
>     https://issues.apache.org/jira/browse/MESOS-2968
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The Backend interface is made generic so it can be used by different provisioners. See [MESOS-2968](https://issues.apache.org/jira/browse/MESOS-2968?focusedCommentId=14652859&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14652859) for details.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 07502f0f5523b972888ceab9cf3af309c8441d7f 
>   src/slave/containerizer/provisioners/backend.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/backend.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37382/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>