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 50271: Created an isolator for Linux capabilities.
Date Thu, 22 Sep 2016 18:49:42 GMT


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


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