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/07/30 02:19:22 UTC
Review Request 36930: Forced the network isolator to use the mount
namespace.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36930/
-----------------------------------------------------------
Review request for mesos, Chi Zhang and Vinod Kone.
Repository: mesos
Description
-------
Forced the network isolator to use the mount namespace.
The code of the network isolator actually relies on the fact that the child is in a seprate mount namespace. For example:
https://github.com/apache/mesos/blob/master/src/slave/containerizer/isolators/network/port_mapping.cpp#L1527
https://github.com/apache/mesos/blob/master/src/slave/containerizer/isolators/network/port_mapping.cpp#L3533
It originally depends on mount namespace, but was removed in this patch:
https://reviews.apache.org/r/26274
That was a bug to me. It didn't cause any issue because we don't clone the mounts (since we are not using mount namespace) anymore after the above patch. So the kernel won't have an extra reference to the mount when we try to umount it in `_cleanup()`.
Diffs
-----
src/slave/containerizer/isolators/network/port_mapping.cpp 3f6e9df8711995d0dd3903c6170fdd5ad61aac5a
Diff: https://reviews.apache.org/r/36930/diff/
Testing
-------
sudo make check
Thanks,
Jie Yu
Re: Review Request 36930: Forced the network isolator to use the mount
namespace.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36930/
-----------------------------------------------------------
(Updated July 31, 2015, 10:23 p.m.)
Review request for mesos, Chi Zhang and Vinod Kone.
Changes
-------
Added comments.
Repository: mesos
Description
-------
Forced the network isolator to use the mount namespace.
The code of the network isolator actually relies on the fact that the child is in a seprate mount namespace. For example:
https://github.com/apache/mesos/blob/master/src/slave/containerizer/isolators/network/port_mapping.cpp#L1527
https://github.com/apache/mesos/blob/master/src/slave/containerizer/isolators/network/port_mapping.cpp#L3533
It originally depends on mount namespace, but was removed in this patch:
https://reviews.apache.org/r/26274
That was a bug to me. It didn't cause any issue because we don't clone the mounts (since we are not using mount namespace) anymore after the above patch. So the kernel won't have an extra reference to the mount when we try to umount it in `_cleanup()`.
Diffs (updated)
-----
src/slave/containerizer/isolators/network/port_mapping.cpp 3f6e9df8711995d0dd3903c6170fdd5ad61aac5a
Diff: https://reviews.apache.org/r/36930/diff/
Testing
-------
sudo make check
Thanks,
Jie Yu
Re: Review Request 36930: Forced the network isolator to use the mount
namespace.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36930/#review93548
-----------------------------------------------------------
Patch looks great!
Reviews applied: [36929, 36930]
All tests passed.
- Mesos ReviewBot
On July 30, 2015, 12:19 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36930/
> -----------------------------------------------------------
>
> (Updated July 30, 2015, 12:19 a.m.)
>
>
> Review request for mesos, Chi Zhang and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Forced the network isolator to use the mount namespace.
>
> The code of the network isolator actually relies on the fact that the child is in a seprate mount namespace. For example:
> https://github.com/apache/mesos/blob/master/src/slave/containerizer/isolators/network/port_mapping.cpp#L1527
> https://github.com/apache/mesos/blob/master/src/slave/containerizer/isolators/network/port_mapping.cpp#L3533
>
> It originally depends on mount namespace, but was removed in this patch:
> https://reviews.apache.org/r/26274
>
> That was a bug to me. It didn't cause any issue because we don't clone the mounts (since we are not using mount namespace) anymore after the above patch. So the kernel won't have an extra reference to the mount when we try to umount it in `_cleanup()`.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolators/network/port_mapping.cpp 3f6e9df8711995d0dd3903c6170fdd5ad61aac5a
>
> Diff: https://reviews.apache.org/r/36930/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 36930: Forced the network isolator to use the mount
namespace.
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36930/#review93680
-----------------------------------------------------------
Ship it!
Could you make a comment that port mapping doesn't need mount namespace itself, i.e., the make-share outside and the make-slave inside on /var/run/netns are noops when it's disabled, but doing so avoids the race in MESOS-1558 when it is enabled, in case when it is required by other components of mesos?
- Chi Zhang
On July 30, 2015, 12:19 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36930/
> -----------------------------------------------------------
>
> (Updated July 30, 2015, 12:19 a.m.)
>
>
> Review request for mesos, Chi Zhang and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Forced the network isolator to use the mount namespace.
>
> The code of the network isolator actually relies on the fact that the child is in a seprate mount namespace. For example:
> https://github.com/apache/mesos/blob/master/src/slave/containerizer/isolators/network/port_mapping.cpp#L1527
> https://github.com/apache/mesos/blob/master/src/slave/containerizer/isolators/network/port_mapping.cpp#L3533
>
> It originally depends on mount namespace, but was removed in this patch:
> https://reviews.apache.org/r/26274
>
> That was a bug to me. It didn't cause any issue because we don't clone the mounts (since we are not using mount namespace) anymore after the above patch. So the kernel won't have an extra reference to the mount when we try to umount it in `_cleanup()`.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolators/network/port_mapping.cpp 3f6e9df8711995d0dd3903c6170fdd5ad61aac5a
>
> Diff: https://reviews.apache.org/r/36930/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>