----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56341/#review164541 -----------------------------------------------------------
src/launcher/executor.cpp (line 860) <https://reviews.apache.org/r/56341/#comment236297> Similar to capabilities, can we add a parse function for Environment and use `Option<Environment>` here. src/launcher/executor.cpp (line 928) <https://reviews.apache.org/r/56341/#comment236298> Let's avoid the `mesos::` prefix here by using a `using` clause at the begining of the file. src/launcher/executor.cpp (lines 937 - 940) <https://reviews.apache.org/r/56341/#comment236299> Let's just pass `environment` to `CommandExecutor` below. src/launcher/executor.cpp (line 939) <https://reviews.apache.org/r/56341/#comment236300> I'd do that in `launch`. I'll also add a TODO. Ideally, we should use execvpe, rather than relying on inheritence. Are you sure it won't affect command health check if you do that in this way? src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 140) <https://reviews.apache.org/r/56341/#comment236303> Let's update AppcRuntimeIsolator as well. - Jie Yu On Feb. 7, 2017, 1:19 a.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56341/ > ----------------------------------------------------------- > > (Updated Feb. 7, 2017, 1:19 a.m.) > > > Review request for mesos, Jie Yu and Kevin Klues. > > > Bugs: MESOS-7027 > https://issues.apache.org/jira/browse/MESOS-7027 > > > Repository: mesos > > > Description > ------- > > This commit adds more special-casing in the `docker/runtime` isolator > for the command executor. The command executor will generally break > when the `docker/runtime` isolator provides environment variables > directly to the executor. This is because the environment variables > are provided in the context of the container image, rather than the > host. > > For example, a container image may provide an environment variable > like `LD_LIBRARY_PATH=/image/specific/location`, whereas the default > executor expects to find libraries in the host's environment. If the > image's environment end up in the command executor at launch time, > the command executor may simply fail to launch. > > > Diffs > ----- > > src/launcher/executor.cpp 0c770bb18ae8bd8df85589b5262f457ab50574a9 > src/slave/containerizer/mesos/isolators/docker/runtime.cpp > 2d816e512c95ed2922c9578ba796908c5ce23da4 > > Diff: https://reviews.apache.org/r/56341/diff/ > > > Testing > ------- > > make check > > sudo src/mesos-tests --gtest_filter="*ROOT*" > > All the tests broken by https://reviews.apache.org/r/56339/ (earlier in the > chain) are now passing. > > > Thanks, > > Joseph Wu > >
