> On Sept. 12, 2016, 3:46 p.m., Benjamin Bannier wrote: > > src/slave/containerizer/mesos/containerizer.cpp, lines 1105-1107 > > <https://reviews.apache.org/r/51784/diff/1/?file=1495422#file1495422line1105> > > > > 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. > > Jie Yu wrote: > Do you have a good suggestion? I cannot come up with a better solution. > See my above response. This is a short term workaround and will be solved > soon. > > Keep in mind that we also need to support the cases where docker runtime > isolator is not used, but capabilities is turned on.
See my comment under Gilbert's response below. > On Sept. 12, 2016, 3:46 p.m., Benjamin Bannier wrote: > > src/slave/containerizer/mesos/containerizer.cpp, lines 1149-1154 > > <https://reviews.apache.org/r/51784/diff/1/?file=1495422#file1495422line1149> > > > > While these fields are `optional` in the interface explicitly dropping > > them here appears could break assumptions user made. > > Jie Yu wrote: > We never use those fields in 'launchCommand'. This just make it explicit. > See my above response, CommandInfo is a tech debt. We'll likely to clean it > up in v2. Fair enough. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51784/#review148482 ----------------------------------------------------------- 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 > >
