> On March 11, 2015, 7:06 p.m., Adam B wrote: > > src/slave/containerizer/containerizer.cpp, lines 294-297 > > <https://reviews.apache.org/r/31889/diff/9-11/?file=891672#file891672line294> > > > > Not sure this TODO is accurate/necessary anymore. This patch assumes > > that the decorator hook returns the entire env from executorInfo as well as > > its own modifications, so it's up to the hook to decide how to resolve > > conflicts. > > On the other hand, if the hook incorrectly returns only its own > > environment, then the executorInfo.environment variables will be silently > > dropped. > > Kapil Arya wrote: > The hook still returns only it's own environment variables. The hook > manager merges the returned values with the executorInfo's environment and > finally returns an aggregate. However, the caller can still overwrite > anything in the aggregated result by overwriting with the values from the > original executorInfo's environment.
Updated the TODO comment to clarify the situation. - Kapil ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/#review76158 ----------------------------------------------------------- On March 11, 2015, 7:34 p.m., Kapil Arya wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31889/ > ----------------------------------------------------------- > > (Updated March 11, 2015, 7:34 p.m.) > > > Review request for mesos, Adam B, Ben Mahler, Niklas Nielsen, and Till > Toenshoff. > > > Bugs: MESOS-2463 > https://issues.apache.org/jira/browse/MESOS-2463 > > > Repository: mesos > > > Description > ------- > > The ExecutorInfo is immutable, and so the environment decoration is deferred > until containerizer launch. The new slaveExecutorHook simply notifies the hook > module of a new executor being launched. The environment decorator hook for > containerizer launch update it's environment variables as gathered from the > hook > modules. > > > Diffs > ----- > > include/mesos/hook.hpp d83ace576a2c78eb7b1e910d89d912f6df5c46ef > src/examples/test_hook_module.cpp 22212261053f2c10f7c4e5ae15aa3d4a2aa1c051 > src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 > src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 > src/slave/containerizer/containerizer.cpp > 33f8738fa229b42ebacfd75b762c977b33191b65 > src/slave/slave.cpp 71ae84bbfcef208cc2ee603f3c8a79225e48a7d5 > src/tests/hook_tests.cpp b5d776a55b6b205394469c176ac6be6702c218f3 > > Diff: https://reviews.apache.org/r/31889/diff/ > > > Testing > ------- > > make check. Also tried a negative test by replacing `test $FOO = 'bar'` with > `test $FOO = 'baz'` and surely, the test failed. > > > Thanks, > > Kapil Arya > >
