mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jie Yu <yujie....@gmail.com>
Subject Re: Review Request 51857: Modified the `network/cni` isolator to be nesting aware.
Date Wed, 21 Sep 2016 00:08:55 GMT


> On Sept. 20, 2016, 9:10 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 725-733
> > <https://reviews.apache.org/r/51857/diff/5/?file=1505752#file1505752line725>
> >
> >     This is problematic.
> >     
> >     If both rootDir and pluginDir is not set, meaning that containers will only
join host network. Say the parent container joins the host network. The child container will
join host network as well, but it has a rootfs so we need to setup network files for it.
> >     
> >     First of all, the CHECK_SOME above will fail. Then the `CHECK(!infos.contains(...))`
will fail as well.
> >     
> >     That's the reason I highly suggested previously that you combine the logic here
with the if block in line 701.
> >     
> >     For child containers, do not add 'containerNetworks' to its info because itself
does not create/join a new CNI network. You'll need to get the Info for its parent container
anyway, so that's redundant information.
> >     
> >     In that way, the 'if' condition at line 701 still applies to child containers:
> >     ```
> >     if (infos[containerId]->containerNetworks.empty() &&
> >         infos[containerId]->rootfs.isSome()) {
> >     ```
> >     
> >     However, instead of using the host file directly, you may want to check if the
parent container has network files or not. It's likely that the parent container joins host
network without rootfs. In that case, it does not have network files.
> 
> Avinash sridharan wrote:
>     Why is this is an issue for a child container with a rootfs ? 
>     The child container with a rootfs is no different than the parent container. For
these containers, the code would never reach line `727`. The execution would be that of code
block `707` to `725`. This shouldn't any different than the way it is working today.
> 
> Avinash sridharan wrote:
>     By the way my above comment is valid only for child containers with rootfs joining
the host network.

oops. my bad. I forgot that the if block starting from line 701 will short circuit.

So info.containerNetworks for nested container is the networks of its parent. Let's document
this in hpp.


- Jie


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


On Sept. 21, 2016, 12:06 a.m., Avinash sridharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51857/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2016, 12:06 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Joseph Wu, and Qian Zhang.
> 
> 
> Bugs: MESOS-6156
>     https://issues.apache.org/jira/browse/MESOS-6156
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The network file setup in the `network/cni` isolator is now nesting
> aware. Since the children share the network and UTS namespace with the
> parent, the network files need to be created only for the parent
> container. For the child containers, the network files will be simply
> a bind mount of the parents network files.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 949da8f70fb1cd13d6359780b032cb170693ea3e

>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 359479083894e887647a694a1a133dce44817073

> 
> Diff: https://reviews.apache.org/r/51857/diff/
> 
> 
> Testing
> -------
> 
> make 
> make check
> and
> sudo ./bin/mesos-tests.sh
> 
> The only tests that failed were the SUDO make check tests:
> [  FAILED  ] 3 tests, listed below:
> [  FAILED  ] CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_Listen
> [  FAILED  ] CgroupsAnyHierarchyMemoryPressureTest.ROOT_IncreaseRSS
> [  FAILED  ] LinuxFilesystemIsolatorTest.ROOT_RecoverOrphanedPersistentVolume
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message