mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Avinash sridharan <avin...@mesosphere.io>
Subject Re: Review Request 51871: Modified the `isolate` method to be nesting aware.
Date Tue, 20 Sep 2016 00:04:15 GMT


> 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
> 
>


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