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