mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 51784: Supported merging the launch command from isolators.
Date Mon, 12 Sep 2016 13:46:07 GMT

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



I am not really sure it is a good idea to follow this approach. It might make things simpler
for anybody working on the Mesos containerizer, but I fear it might make the interfaces we
expose hard to use for everybody else since it i) doesn't try hard enough to shield isolator
developers from problems in other isolators, and ii) introduces special semantics for `CommandInfos`
when contained in certain messages.


src/slave/containerizer/mesos/containerizer.cpp (lines 1105 - 1107)
<https://reviews.apache.org/r/51784/#comment215927>

    IMHO this is overly simplistic. We should always be prepared for modules being loaded
from modules, and potentially executed in any order. We should really put in a safety net
at a place where we have all information, e.g., run some sanity check in a good spot (here
seems to be one candidate). I am not sure how much we can sanitize though.



src/slave/containerizer/mesos/containerizer.cpp (lines 1149 - 1154)
<https://reviews.apache.org/r/51784/#comment215928>

    While these fields are `optional` in the interface explicitly dropping them here appears
could break assumptions user made.


- Benjamin Bannier


On Sept. 11, 2016, 2:17 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51784/
> -----------------------------------------------------------
> 
> (Updated Sept. 11, 2016, 2:17 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Gilbert Song.
> 
> 
> Bugs: MESOS-5275
>     https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously, we only allow one isolator to specify the launch command
> for the container. This is not ideal because multiple isolators might
> want to add some flags to the command executor. For instance, the
> 'docker/runtime' isolator wants to specify '--task_command' and
> '--working_directory', and 'linux/capabilities' isolator wants to
> specify '--capabilities'.
> 
> This patch changes the semantics so that launch command from isolators
> are merged. However, it is isolator's responsibility to make sure the
> merged command is a valid command.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.cpp 89b7e8db38916d69d9b2d4fe305d4397b0859a10

> 
> Diff: https://reviews.apache.org/r/51784/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


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