> On Sept. 20, 2016, 2:38 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, lines 1242-1251
> > <https://reviews.apache.org/r/50271/diff/11/?file=1502610#file1502610line1242>
> >
> >     I understand why you want to do this check, but we usually do not do 
> > such checks in containerizer. We rely on operator to properly configure the 
> > agent.
> >     
> >     The correct solution I believe is to advertise agent isolation 
> > capabilities to the master and let master reject those tasks.
> >     
> >     I would simply remove this check here for now.
> 
> Benjamin Bannier wrote:
>     Like you probably know I added this to satisfy a requirement from the 
> design doc where we specified for the case when `--isolation=capabilities` is 
> absent:
>     
>     a) ContainerInfo::CapabilityInfo not provided:
>         All capabilities for the user. (no isolation)
>     b) ContainerInfo::CapabilityInfo provided, ContainerInfo::CapabilityInfo 
> is empty:
>         Not supported. Agent will fail the task with appropriate reasoning.
>     c) ContainerInfo::CapabilityInfo is NOT empty:
>         Not supported. Agent will fail the task with appropriate reasoning.   
>  
>     
>     Do you mean the design doc is wrong when it says this work should be 
> performed in the agent (instead the master should be fail the task)? In that 
> case I wonder if we wouldn't still need to check in the agent, e.g., to allow 
> restarting agents with changed capabilities.

yeah, I feel this is the wrong way to solve the issue. If we need to do this, 
we need to do a lot for other isolators as well (e.g., gpu).

For instance, maybe we should still invoke those some validation method even if 
the isolator is not enabled. In that way, the checks can be distributed to each 
isolator, instead of accumulating in containerizer. I just feel that adding 
isolator specific checks in the containerizer like that is not going to scale.

For now, i'll just punt on that and clearly document that in the doc. Sounds 
good?


- Jie


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


On Sept. 20, 2016, 11:40 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> -----------------------------------------------------------
> 
> (Updated Sept. 20, 2016, 11:40 a.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
>     https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -----
> 
>   include/mesos/slave/containerizer.proto 
> 20db010ea158a813034b411111ce9cddac7d8317 
>   src/CMakeLists.txt 42c52b60cc850901f2eff1545cf7900f4a65ca81 
>   src/Makefile.am 478fb5ae01dbdf1a735680edf7c5f30867726e07 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e54169ba00f6e0cdd7043075b4cdd12d714c99e3 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.hpp 
> 859990cb85e9e8c06400397256cfc512f0811800 
>   src/slave/containerizer/mesos/launch.cpp 
> fc51e04ec1572679e6a48ff4f0fa31ef2dfd6ec3 
>   src/slave/flags.hpp 3952d04f6a00ac1dca1adf2bea7cc6e415620ce5 
>   src/tests/containerizer/isolator_tests.cpp 
> 93ce75180520d382f63ce7323be844fe43c6594e 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> -------
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Reply via email to