> On Sept. 12, 2016, 10:16 p.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/containerizer.cpp, line 1109 > > <https://reviews.apache.org/r/51784/diff/1/?file=1495422#file1495422line1109> > > > > Could we just have `arguments` MergeFrom? Because singular field (e.g., > > `value`) may be overwritten by some user modules accidentally. > > Benjamin Bannier wrote: > +1 > > Taking this a step further, given own function to merge a list of > `CommandInfo` into a single `CommandInfo`, e.g., `[CommandInfo] -> > Try(CommandInfo)`, would give us a chance to sanitize (e.g., are all > non-ignored scalar fields compatible?), and explicitly drop ignored elements > in a single, exposed spot.
What is the status on this? I still believe it is important to verify that isolators play nice with each other as writers of isolators have very little control over this. I have sketched this on top of this change over here, https://github.com/bbannier/mesos/tree/r/51784. Currently that code emits `VLOG` messages whenever `CommandInfo`s from `ContainerLaunchInfo`s not as compatible as we wish, but I think ultimately we'd want to fail hard for such scenarios. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51784/#review148571 ----------------------------------------------------------- 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 > >
