mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Peach <jpe...@apache.org>
Subject Re: Review Request 70741: Adopted container file operations for secrets volumes.
Date Wed, 05 Jun 2019 08:44:22 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message