----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31889/#review76029 -----------------------------------------------------------
Looks good to me, barring my question about giving the hook access to the TaskInfo/labels. I think it's ready to share with the rest of the community. include/mesos/hook.hpp <https://reviews.apache.org/r/31889/#comment123413> What happens to conflicting changes? Overwritten, prepended, appended? Seems like it's up to the hook; maybe worth mentioning? include/mesos/hook.hpp <https://reviews.apache.org/r/31889/#comment123417> Will the hook still be able to read labels or the rest of the TaskInfo to make decisions about what to place in the environment? src/examples/test_hook_module.cpp <https://reviews.apache.org/r/31889/#comment123416> Does this TODO need to be addressed in this review, i.e. is it a blocker for 0.22? src/slave/containerizer/containerizer.cpp <https://reviews.apache.org/r/31889/#comment123418> Hook overrides any conflicting values? I guess you're passing in the input environment, so the hook can choose to prepend/append when creating a variable's new value. - Adam B On March 10, 2015, 6:11 p.m., Kapil Arya wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31889/ > ----------------------------------------------------------- > > (Updated March 10, 2015, 6:11 p.m.) > > > Review request for mesos, 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 8faf6850aafcebda7e9f0d1b735d61f7effa842d > src/hook/manager.hpp d3729eaca1bf2893cbc94cf62835dafb2d9a22a1 > src/hook/manager.cpp 3fd9d5e7f81e3d0ca2aca4091beba4ae555ae7e7 > src/slave/containerizer/containerizer.cpp > 33f8738fa229b42ebacfd75b762c977b33191b65 > src/slave/slave.cpp 364d911b086dfe1f15f76aa3888f99146aa8d876 > > Diff: https://reviews.apache.org/r/31889/diff/ > > > Testing > ------- > > make check. The master failover tests have not been performed yet. > > > Thanks, > > Kapil Arya > >
