> On June 5, 2019, 7:16 a.m., Jiang Yan Xu wrote: > > src/slave/containerizer/mesos/isolators/volume/secret.cpp > > Line 138 (original), 147 (patched) > > <https://reviews.apache.org/r/70741/diff/2/?file=2147411#file2147411line147> > > > > Do you know why `launchInfo.mounts` was not used (and for the same > > reason it couldn't be used here)?
You need to sequence the mount with other operations, so you can't use the container mounts. We can revisit coalescing these in the future, but for now I opted for compatibility and simplicity. > On June 5, 2019, 7:16 a.m., Jiang Yan Xu wrote: > > src/slave/containerizer/mesos/launch.cpp > > Lines 516 (patched) > > <https://reviews.apache.org/r/70741/diff/2/?file=2147412#file2147412line524> > > > > Is it more explicit if you just name the operation > > `ContainerFileOperation::MV` or `ContainerFileOperation::MOVE` and > > implement it as such (rename + fallback) either here or in stout since you > > are essentially implementing how we use a `mv` command? `rename` vs. `move` seems pretty similar. To me, `rename` seemed clear so I prefer it. I didn't add a stout implementation because I didn't have the time to handle all the corner cases and write proper tests for a general utility. - James ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70741/#review215696 ----------------------------------------------------------- On May 29, 2019, 4:29 a.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70741/ > ----------------------------------------------------------- > > (Updated May 29, 2019, 4:29 a.m.) > > > Review request for mesos, Xudong Ni, Gilbert Song, Jie Yu, Jacob Janco, and > Jiang Yan Xu. > > > Bugs: MESOS-9799 > https://issues.apache.org/jira/browse/MESOS-9799 > > > Repository: mesos > > > Description > ------- > > Switched the `volumes/secrets` isolator from using container > pre-exec commands to using file operations. This allows us to > reduce the number of forked child processes, consolidate code > and leaves the `network/port_mapping` isolator as the last > consumer of pre-exec commands. > > > Diffs > ----- > > include/mesos/slave/containerizer.proto > b2e35cbf01caea6c1e4f45c7b7d833bc7f065099 > src/common/protobuf_utils.hpp 8aa9aff71cbbd8e98573d0095cfc007cdea163bc > src/common/protobuf_utils.cpp 870859de93b44a1debc6786a562b4bc28955ddab > src/slave/containerizer/mesos/isolators/volume/secret.cpp > 7a9bb82b23b35728408fb37bac53d79883c0a19f > src/slave/containerizer/mesos/launch.cpp > 0c482f46a97063133edfe29ae3c6a2721d29f6c6 > > > Diff: https://reviews.apache.org/r/70741/diff/2/ > > > Testing > ------- > > sudo make check (Fedora 30) > > > Thanks, > > James Peach > >
