> On Sept. 19, 2016, 11:55 p.m., Jie Yu wrote:
> > Let's merge the patches into one single patch. These patches are highly 
> > related. It's nice to review them in a single diff.

Will do.


> On Sept. 19, 2016, 11:55 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 736-764
> > <https://reviews.apache.org/r/51871/diff/7/?file=1501010#file1501010line736>
> >
> >     I'd combine this with the host network namespace with rootfs case. See 
> > my comments in 'prepare' patch.

The reason I wanted to treat them separate from the host network namespace with 
rootfs case, is that child containers joining host network with rootfs are the 
same as any other container, they don't get a new network namespace and they 
basically get the hosts network files bind mounted to the container's rootfs. 
However, a child container joining a non-host network namespace is special in 
the sense that it basically gets the networks files from its parent (root 
container). Given the dissimilarity of the cases, didn't make sense to treat 
them the same?


> On Sept. 19, 2016, 11:55 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 728
> > <https://reviews.apache.org/r/51871/diff/7/?file=1501010#file1501010line728>
> >
> >     Instead of a CHECK, i'd prefer just return Failure here.

Doesn't this look like a bug? We should never hit this condition. Hence thought 
a CHECK made more sense than a FAILURE?


- Avinash


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


On Sept. 18, 2016, 5:40 a.m., Avinash sridharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51871/
> -----------------------------------------------------------
> 
> (Updated Sept. 18, 2016, 5:40 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 symlink to a parents network files.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 359479083894e887647a694a1a133dce44817073 
> 
> Diff: https://reviews.apache.org/r/51871/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Reply via email to