You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2015/08/11 02:24:56 UTC
Review Request 37330: Added persistent volume support for linux
filesystem isolator.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37330/
-----------------------------------------------------------
Review request for mesos, Lily Chen, Ian Downes, Jojy Varghese, Timothy Chen, Vinod Kone, and Jiang Yan Xu.
Bugs: MESOS-2794
https://issues.apache.org/jira/browse/MESOS-2794
Repository: mesos
Description
-------
Added persistent volume support for linux filesystem isolator.
Diffs
-----
src/slave/containerizer/isolators/filesystem/linux.hpp PRE-CREATION
src/slave/containerizer/isolators/filesystem/linux.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/37330/diff/
Testing
-------
sudo make check
Will follow up with more tests specificially for persistent volumes.
Thanks,
Jie Yu
Re: Review Request 37330: Added persistent volume support for linux
filesystem isolator.
Posted by Jie Yu <yu...@gmail.com>.
> On Aug. 12, 2015, 10:47 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/isolators/filesystem/linux.cpp, line 485
> > <https://reviews.apache.org/r/37330/diff/2/?file=1037046#file1037046line485>
> >
> > s/other/another/
'other' sounds fine to me here.
> On Aug. 12, 2015, 10:47 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/isolators/filesystem/linux.cpp, line 579
> > <https://reviews.apache.org/r/37330/diff/2/?file=1037046#file1037046line579>
> >
> > What happens when persistent volumes are used without a new rootfs?
Nice catch! That's a bug. I haven't tested the cases where a container does not have a new rootfs. I'll follow up with a test.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37330/#review95154
-----------------------------------------------------------
On Aug. 11, 2015, 1:57 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37330/
> -----------------------------------------------------------
>
> (Updated Aug. 11, 2015, 1:57 a.m.)
>
>
> Review request for mesos, Lily Chen, Ian Downes, Jojy Varghese, Timothy Chen, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-2794
> https://issues.apache.org/jira/browse/MESOS-2794
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added persistent volume support for linux filesystem isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolators/filesystem/linux.hpp PRE-CREATION
> src/slave/containerizer/isolators/filesystem/linux.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/37330/diff/
>
>
> Testing
> -------
>
> sudo make check
>
> Will follow up with more tests specificially for persistent volumes.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 37330: Added persistent volume support for linux
filesystem isolator.
Posted by Jie Yu <yu...@gmail.com>.
> On Aug. 12, 2015, 10:47 p.m., Jiang Yan Xu wrote:
> > This thing is rather complex and it deals with Mesos managed persisetent volumes and other system volumes in different but related ways; rootfs may or may not be used; and we'll provision image volumes later. I hope we iterate on this later and look for ways (more documentation, refactorings) to simplify and clarify the code.
Yeah, we'll continue to work that later for 'volumes in image'. The involves some refactor. I'll try to think about a way to make it more readable.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37330/#review95154
-----------------------------------------------------------
On Aug. 12, 2015, 11:21 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37330/
> -----------------------------------------------------------
>
> (Updated Aug. 12, 2015, 11:21 p.m.)
>
>
> Review request for mesos, Lily Chen, Ian Downes, Jojy Varghese, Timothy Chen, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-2794
> https://issues.apache.org/jira/browse/MESOS-2794
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added persistent volume support for linux filesystem isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolators/filesystem/linux.hpp PRE-CREATION
> src/slave/containerizer/isolators/filesystem/linux.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/37330/diff/
>
>
> Testing
> -------
>
> sudo make check
>
> Will follow up with more tests specificially for persistent volumes.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 37330: Added persistent volume support for linux
filesystem isolator.
Posted by Jie Yu <yu...@gmail.com>.
> On Aug. 12, 2015, 10:47 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/isolators/filesystem/linux.cpp, line 579
> > <https://reviews.apache.org/r/37330/diff/2/?file=1037046#file1037046line579>
> >
> > What happens when persistent volumes are used without a new rootfs?
>
> Jie Yu wrote:
> Nice catch! That's a bug. I haven't tested the cases where a container does not have a new rootfs. I'll follow up with a test.
Added a test here
https://reviews.apache.org/r/37422/
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37330/#review95154
-----------------------------------------------------------
On Aug. 12, 2015, 11:21 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37330/
> -----------------------------------------------------------
>
> (Updated Aug. 12, 2015, 11:21 p.m.)
>
>
> Review request for mesos, Lily Chen, Ian Downes, Jojy Varghese, Timothy Chen, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-2794
> https://issues.apache.org/jira/browse/MESOS-2794
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added persistent volume support for linux filesystem isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolators/filesystem/linux.hpp PRE-CREATION
> src/slave/containerizer/isolators/filesystem/linux.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/37330/diff/
>
>
> Testing
> -------
>
> sudo make check
>
> Will follow up with more tests specificially for persistent volumes.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 37330: Added persistent volume support for linux
filesystem isolator.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37330/#review95154
-----------------------------------------------------------
This thing is rather complex and it deals with Mesos managed persisetent volumes and other system volumes in different but related ways; rootfs may or may not be used; and we'll provision image volumes later. I hope we iterate on this later and look for ways (more documentation, refactorings) to simplify and clarify the code.
src/slave/containerizer/isolators/filesystem/linux.hpp (line 105)
<https://reviews.apache.org/r/37330/#comment149999>
Mention it's an absoluate path and always under the rootfs?
src/slave/containerizer/isolators/filesystem/linux.cpp (lines 292 - 294)
<https://reviews.apache.org/r/37330/#comment150043>
Add a TODO here to change this after we start to provsision images in volumes?
src/slave/containerizer/isolators/filesystem/linux.cpp (line 485)
<https://reviews.apache.org/r/37330/#comment149989>
s/other/another/
src/slave/containerizer/isolators/filesystem/linux.cpp (line 526)
<https://reviews.apache.org/r/37330/#comment150038>
```
Check that the source of the mount matches the entry with the same target in the mount table if one can be found.
```
src/slave/containerizer/isolators/filesystem/linux.cpp (line 553)
<https://reviews.apache.org/r/37330/#comment149990>
Aha, OK I see that the assignment happens here.
src/slave/containerizer/isolators/filesystem/linux.cpp (line 579)
<https://reviews.apache.org/r/37330/#comment150042>
What happens when persistent volumes are used without a new rootfs?
src/slave/containerizer/isolators/filesystem/linux.cpp (line 589)
<https://reviews.apache.org/r/37330/#comment150016>
s/all/All/
- Jiang Yan Xu
On Aug. 10, 2015, 6:57 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37330/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2015, 6:57 p.m.)
>
>
> Review request for mesos, Lily Chen, Ian Downes, Jojy Varghese, Timothy Chen, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-2794
> https://issues.apache.org/jira/browse/MESOS-2794
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added persistent volume support for linux filesystem isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolators/filesystem/linux.hpp PRE-CREATION
> src/slave/containerizer/isolators/filesystem/linux.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/37330/diff/
>
>
> Testing
> -------
>
> sudo make check
>
> Will follow up with more tests specificially for persistent volumes.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 37330: Added persistent volume support for linux
filesystem isolator.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37330/#review95208
-----------------------------------------------------------
Ship it!
Ship It!
- Jiang Yan Xu
On Aug. 12, 2015, 4:21 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37330/
> -----------------------------------------------------------
>
> (Updated Aug. 12, 2015, 4:21 p.m.)
>
>
> Review request for mesos, Lily Chen, Ian Downes, Jojy Varghese, Timothy Chen, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-2794
> https://issues.apache.org/jira/browse/MESOS-2794
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added persistent volume support for linux filesystem isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolators/filesystem/linux.hpp PRE-CREATION
> src/slave/containerizer/isolators/filesystem/linux.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/37330/diff/
>
>
> Testing
> -------
>
> sudo make check
>
> Will follow up with more tests specificially for persistent volumes.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 37330: Added persistent volume support for linux
filesystem isolator.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37330/
-----------------------------------------------------------
(Updated Aug. 12, 2015, 11:21 p.m.)
Review request for mesos, Lily Chen, Ian Downes, Jojy Varghese, Timothy Chen, Vinod Kone, and Jiang Yan Xu.
Changes
-------
Addressed review comments.
Bugs: MESOS-2794
https://issues.apache.org/jira/browse/MESOS-2794
Repository: mesos
Description
-------
Added persistent volume support for linux filesystem isolator.
Diffs (updated)
-----
src/slave/containerizer/isolators/filesystem/linux.hpp PRE-CREATION
src/slave/containerizer/isolators/filesystem/linux.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/37330/diff/
Testing
-------
sudo make check
Will follow up with more tests specificially for persistent volumes.
Thanks,
Jie Yu
Re: Review Request 37330: Added persistent volume support for linux
filesystem isolator.
Posted by Jie Yu <yu...@gmail.com>.
> On Aug. 12, 2015, 1:43 a.m., Timothy Chen wrote:
> > src/slave/containerizer/isolators/filesystem/linux.cpp, line 399
> > <https://reviews.apache.org/r/37330/diff/2/?file=1037046#file1037046line399>
> >
> > Can we also log the container id and it's the linux filesystem isolator? It's much easier to debug instead of just seeing a message says "Unknown container".
The caller will print the container id in the error message.
> On Aug. 12, 2015, 1:43 a.m., Timothy Chen wrote:
> > src/slave/containerizer/isolators/filesystem/linux.cpp, line 412
> > <https://reviews.apache.org/r/37330/diff/2/?file=1037046#file1037046line412>
> >
> > I don't think we hold reference to tempoaries?
It returns a reference (not a temporary).
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37330/#review95046
-----------------------------------------------------------
On Aug. 11, 2015, 1:57 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37330/
> -----------------------------------------------------------
>
> (Updated Aug. 11, 2015, 1:57 a.m.)
>
>
> Review request for mesos, Lily Chen, Ian Downes, Jojy Varghese, Timothy Chen, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-2794
> https://issues.apache.org/jira/browse/MESOS-2794
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added persistent volume support for linux filesystem isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolators/filesystem/linux.hpp PRE-CREATION
> src/slave/containerizer/isolators/filesystem/linux.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/37330/diff/
>
>
> Testing
> -------
>
> sudo make check
>
> Will follow up with more tests specificially for persistent volumes.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 37330: Added persistent volume support for linux
filesystem isolator.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37330/#review95046
-----------------------------------------------------------
src/slave/containerizer/isolators/filesystem/linux.cpp (line 399)
<https://reviews.apache.org/r/37330/#comment149834>
Can we also log the container id and it's the linux filesystem isolator? It's much easier to debug instead of just seeing a message says "Unknown container".
src/slave/containerizer/isolators/filesystem/linux.cpp (line 412)
<https://reviews.apache.org/r/37330/#comment149835>
I don't think we hold reference to tempoaries?
- Timothy Chen
On Aug. 11, 2015, 1:57 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37330/
> -----------------------------------------------------------
>
> (Updated Aug. 11, 2015, 1:57 a.m.)
>
>
> Review request for mesos, Lily Chen, Ian Downes, Jojy Varghese, Timothy Chen, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-2794
> https://issues.apache.org/jira/browse/MESOS-2794
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added persistent volume support for linux filesystem isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolators/filesystem/linux.hpp PRE-CREATION
> src/slave/containerizer/isolators/filesystem/linux.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/37330/diff/
>
>
> Testing
> -------
>
> sudo make check
>
> Will follow up with more tests specificially for persistent volumes.
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 37330: Added persistent volume support for linux
filesystem isolator.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37330/
-----------------------------------------------------------
(Updated Aug. 11, 2015, 1:57 a.m.)
Review request for mesos, Lily Chen, Ian Downes, Jojy Varghese, Timothy Chen, Vinod Kone, and Jiang Yan Xu.
Changes
-------
Create the mount points if needed.
Bugs: MESOS-2794
https://issues.apache.org/jira/browse/MESOS-2794
Repository: mesos
Description
-------
Added persistent volume support for linux filesystem isolator.
Diffs (updated)
-----
src/slave/containerizer/isolators/filesystem/linux.hpp PRE-CREATION
src/slave/containerizer/isolators/filesystem/linux.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/37330/diff/
Testing
-------
sudo make check
Will follow up with more tests specificially for persistent volumes.
Thanks,
Jie Yu