> On Sept. 19, 2016, 8:35 p.m., Joseph Wu wrote: > > src/slave/containerizer/fetcher.cpp, lines 773-775 > > <https://reviews.apache.org/r/52058/diff/1/?file=1502968#file1502968line773> > > > > (You'll want to update this comment.) > > > > There isn't always a stdout/stderr file in the sandbox, as this depends > > on the ContainerLogger module. Would non-recursively chown-ing the sandbox > > directory work as well? (This is already done when the agent first creates > > the sandbox.) > > Megha Sharma wrote: > The FetcherProcess::run(...) already creates the stdout and stderr and > then recursively changes the ownership of the sandbox to make sure these 2 > files have the right ownership which I modified to change the ownership of > just these 2 files exclusively and not the entire sandbox. So, at this point > the stdout and stderr are already created. > Also, like you mentioned the agent is already doing a chown while > creating the sandbox directory so the sandbox is already owned by the desired > user. The concern that I am addressing here is avoiding unnecessary chowning > of sandbox so files laid out by other entities don't change ownership but we > still anyway need to set the right owners for stdout and stderr here. > > Joseph Wu wrote: > Right, forgot about the fetcher's own stdout/stderr :) > > I'd argue it makes sense to not do any chown-ing at all. The > stdout/stderr files are technically created by other entities than the > executor (i.e. the fetcher, agent, and logger module). Not sure if this > breaks any expectations in the fetcher though...
Since now the fetcher is being run as task user as a result of jira https://issues.apache.org/jira/browse/MESOS-5845 so these 2 files need to be chowned to the same user for the fetcher to be able to write to them. - Megha ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52058/#review149541 ----------------------------------------------------------- On Sept. 19, 2016, 11:08 p.m., Megha Sharma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52058/ > ----------------------------------------------------------- > > (Updated Sept. 19, 2016, 11:08 p.m.) > > > Review request for mesos and Jiang Yan Xu. > > > Bugs: MESOS-5218 > https://issues.apache.org/jira/browse/MESOS-5218 > > > Repository: mesos > > > Description > ------- > > Fetcher code changes the ownership of entire sandbox directory > recursively to the taskuser and as a resut also changes the > ownership of files laid out by other entities in the sandbox. > > > Diffs > ----- > > src/slave/containerizer/fetcher.cpp > 4d3fc521bf8a7c438c48e31c549f108ac3602b3f > src/tests/containerizer/mesos_containerizer_tests.cpp > 96e24500a12825161553eb050da389088b122695 > > Diff: https://reviews.apache.org/r/52058/diff/ > > > Testing > ------- > > make check on linux > > > Thanks, > > Megha Sharma > >
