> On June 24, 2015, 8:04 a.m., Adam B wrote: > > This seems like a positive step forward, since all volumes were previously > > owned by root (or whatever user the slave was run as). However, if a > > persistent volume is passed from one framework to another within the same > > role, they might have different users, so the second framework would still > > be unable to access it. Moreover, different tasks within the same framework > > might run as different users, so if two subsequent tasks try to mount the > > same persistent volume as different users, they won't both be able to > > access it. > > So, in addition to initially chowning the volume to the frameworkInfo.user, > > we should also consider chowning the volume to the commandInfo.user within > > `_runTask` so that the volume ownership is changed to the user running each > > task before the task begins executing, so that each task mounting the > > volume is guaranteed access to the volume. > > haosdent huang wrote: > Thank you very much for your review. If we change the persistent volume > in _runTask, does it cause the scenario like this: > > task_A use user_A, task_B use user_B. And then they running at the same > time. And the persistent volume could not belong to two different users at > the same time. So one of these two tasks would be failed. Is it acceptable? > > Adam B wrote: > Currently we don't allow two concurrent tasks to use the same persistent > volume. I suppose this could change in the future, but for now we should be > safe chowning the volume before each task takes ownership of the volume. > In a future with shared persistent volumes, perhaps we could create a > linux group for the role, make the volume rw for the group, and add each task > user to the group. This is probably overkill for the moment. > > haosdent huang wrote: > Got it. Let me remove it and chown user in runTask
@adam-mesos, @jieyu, @macro Thank you very much for your review. And now I change the implementation to sync the ownership from sandbox directory to persistent volume in "MesosContainerizerProcess::updateVolumes". Could you help me review it again? Thank you very much. - haosdent ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35721/#review89138 ----------------------------------------------------------- On June 28, 2015, 5:46 a.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35721/ > ----------------------------------------------------------- > > (Updated June 28, 2015, 5:46 a.m.) > > > Review request for mesos, Adam B, Jie Yu, and Marco Massenzio. > > > Bugs: MESOS-2603 > https://issues.apache.org/jira/browse/MESOS-2603 > > > Repository: mesos > > > Description > ------- > > Set the owner of persistent volumes to frameworkInfo.user . > > > Diffs > ----- > > src/slave/containerizer/mesos/containerizer.cpp > 313e9b74d3a0157609226041246575d2c4a503f8 > > Diff: https://reviews.apache.org/r/35721/diff/ > > > Testing > ------- > > > Thanks, > > haosdent huang > >
