----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32911/#review79113 -----------------------------------------------------------
Ship it! src/slave/paths.cpp <https://reviews.apache.org/r/32911/#comment128225> Let's move the TODO into the body of the 'if' (closer to where the error checking is) and then add a comment above this 'if' that explains that we need to alays 'chown' the directory so that the permissions are correct. Feel free to go as far as noting the JIRA issue and that this had worked in the past because the fetcher was doing it but sometimes we won't be fetching anything hence the need to always do it here. Also, I'm guessing all containerizers call 'createExecutorPath' except the ExternalContainerizer? src/slave/slave.cpp <https://reviews.apache.org/r/32911/#comment128223> Great comment! Can we also add something to the end of the comment that says that the user is validated when the task goes through the master? Thanks! src/slave/slave.cpp <https://reviews.apache.org/r/32911/#comment128224> Why don't you need to check the taskInfo? Is it because we should have set the executorInfo's 'user' appropriately? If so, let's comment as much, even going as far as introducing a CHECK! - Benjamin Hindman On April 7, 2015, 12:40 a.m., Niklas Nielsen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32911/ > ----------------------------------------------------------- > > (Updated April 7, 2015, 12:40 a.m.) > > > Review request for mesos, Benjamin Hindman and Ian Downes. > > > Bugs: MESOS-2592 > https://issues.apache.org/jira/browse/MESOS-2592 > > > Repository: mesos > > > Description > ------- > > During recent refactorings, executor directory ownership was delegated to the > fetcher. However, the fetcher is not invoked if no URIs are present in the > executor or task command. This left some of these tasks broken as the > directory ownership defaulted to the mesos-slave's (root). > > > Diffs > ----- > > src/slave/containerizer/external_containerizer.cpp > 1bbd61cb096771b7e4a1350079f79a20102e78f9 > src/slave/paths.hpp 1618439d728ded347ec75317ce8dd998acd7ee94 > src/slave/paths.cpp 01ea856aa2e628d4aee5fd31f7e49d147f740e8f > src/slave/slave.cpp 521624c335b9110e12ee1ff21c3918e5af6a2bde > > Diff: https://reviews.apache.org/r/32911/diff/ > > > Testing > ------- > > Functional tests with mesos-execute and make check. Have created JIRA's for > introduction of more permission/user tests. > > > Thanks, > > Niklas Nielsen > >
