> On April 6, 2017, 12:22 a.m., Jie Yu wrote:
> > src/tests/containerizer/linux_filesystem_isolator_tests.cpp
> > Lines 1523 (patched)
> > <https://reviews.apache.org/r/57884/diff/2/?file=1685497#file1685497line1523>
> >
> >     Instead of putting this test under LinuxFilesystemIsolatorTest, I would 
> > put it under CniIsolatorTest. The code change is in CNI isolator, so 
> > putting it in CniIsolatorTest is more appropriate?
> >     
> >     I would write a similar test like this test:
> >     TEST_F(CniIsolatorTest, ROOT_OverrideHostname)
> 
> Silas Snider wrote:
>     I disagree (though not super strongly) -- I think that the functionality 
> being guarded/tested is that the linux fs isolator correctly mounts the /etc/ 
> networking files readonly. That it does so *today* by stealth-including the 
> CNI isolator is an implementation detail, and a surprising one at that. To 
> protect against the general case, I put it here.
>     
>     If you still think it should move, I'm totally willing to move it though.

the mount is done by the CNI isolator, not linux filesystem isolator. Even in 
the future, we allow isolator dependency, and let linux filesystem isolator 
handles all the bind mount, this is still a logic (read-only bind mount for 
/etc) introduced by CNI isolator. Put that in other words, if we disable cni 
isolator and enable linux filesystem isolator, your test will fail.


- Jie


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


On April 5, 2017, 7:53 p.m., Silas Snider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57884/
> -----------------------------------------------------------
> 
> (Updated April 5, 2017, 7:53 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7268
>     https://issues.apache.org/jira/browse/MESOS-7268
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 6e95315b70a5d9d3b4b21c4cf235b0a483760190 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 5e489ef6a522000c55b0fb9a27bce2567f82bb73 
> 
> 
> Diff: https://reviews.apache.org/r/57884/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Silas Snider
> 
>

Reply via email to