Re: Review Request 70678: Add containerizer support for masking paths.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70678/#review215491 --- Ship it! Ship It! - Jason Lai On May 22, 2019, 8:25 p.m., James Peach wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70678/ > --- > > (Updated May 22, 2019, 8:25 p.m.) > > > Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu. > > > Bugs: MESOS-9771 > https://issues.apache.org/jira/browse/MESOS-9771 > > > Repository: mesos > > > Description > --- > > Add support to the `filesystem/linux` isolator for masking container > paths. Add a set of standard default paths to be masked, as derived > from commonly used container runtimes. These paths either expose > information about other system processes, or capabilities that > should not be exposed to untrusted containers. > > We don't mask if the container is privileged, which is defined > as sharing the host's PID namespace. For nested containers, we > verify that the PID namespace is shared from the host all the way > up the tree. > > > Diffs > - > > include/mesos/slave/containerizer.proto > 48ffa2e6bd1a03f3dc68a3a78d883855f14bf10c > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp > 725754f26855ea54ccf8cbcb288ee3b29e8ed4e7 > src/slave/containerizer/mesos/launch.cpp > 88b97a572916defbe65692036be77395053eb8e8 > src/tests/containerizer/linux_filesystem_isolator_tests.cpp > 60e9ae5970a0a45314d0b3569556bef36d350d2b > src/tests/containerizer/rootfs.cpp 48eb0108cf26729a0528528a1102247410cf80fe > > > Diff: https://reviews.apache.org/r/70678/diff/4/ > > > Testing > --- > > sudo make check (Fedora 30) > > > Thanks, > > James Peach > >
Re: Review Request 70678: Add containerizer support for masking paths.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70678/#review215433 --- Fix it, then Ship it! Good stuff! LGTM overall with some nits. src/slave/containerizer/mesos/isolators/filesystem/linux.cpp Lines 500 (patched) <https://reviews.apache.org/r/70678/#comment302131> Nit: `return false` first, so we won't need to nest the logic inside the previous `if` statement. src/slave/containerizer/mesos/isolators/filesystem/linux.cpp Lines 816 (patched) <https://reviews.apache.org/r/70678/#comment302130> Nit: I feel we should consider making the masked paths an instance variable of the isolator class and initializing it with `ROOTFS_MASKED_PATHS` instead, in the purpose of avoid hard coding. - Jason Lai On May 20, 2019, 1:41 a.m., James Peach wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70678/ > --- > > (Updated May 20, 2019, 1:41 a.m.) > > > Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu. > > > Bugs: MESOS-9771 > https://issues.apache.org/jira/browse/MESOS-9771 > > > Repository: mesos > > > Description > --- > > Add support to the `filesystem/linux` isolator for masking container > paths. Add a set of standard default paths to be masked, as derived > from commonly used container runtimes. These paths either expose > information about other system processes, or capabilities that > should not be exposed to untrusted containers. > > We don't mask if the container is privileged, which is defined > as sharing the host's PID namespace. For nested containers, we > verify that the PID namespace is shared from the host all the way > up the tree. > > > Diffs > - > > include/mesos/slave/containerizer.proto > 48ffa2e6bd1a03f3dc68a3a78d883855f14bf10c > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp > 7b50258ef5480c1ea3f0016aace3b838395becfd > src/slave/containerizer/mesos/launch.cpp > 88b97a572916defbe65692036be77395053eb8e8 > src/tests/containerizer/linux_filesystem_isolator_tests.cpp > 60e9ae5970a0a45314d0b3569556bef36d350d2b > src/tests/containerizer/rootfs.cpp 48eb0108cf26729a0528528a1102247410cf80fe > > > Diff: https://reviews.apache.org/r/70678/diff/3/ > > > Testing > --- > > sudo make check (Fedora 30) > > > Thanks, > > James Peach > >
Review Request 70048: Documented the new `--host_path_volume_force_creation` agent option.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70048/ --- Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, and James Peach. Bugs: MESOS-9009 https://issues.apache.org/jira/browse/MESOS-9009 Repository: mesos Description --- Documented the new `--host_path_volume_force_creation` agent option. Diffs - docs/configuration/agent.md bb0f5d0c5d2f0fd1ecf14de7259ecb1688c2272c Diff: https://reviews.apache.org/r/70048/diff/1/ Testing --- N/A. Thanks, Jason Lai
Re: Review Request 69287: Added test cases for the `volume/host_path` isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69287/ --- (Updated Feb. 22, 2019, 1:44 a.m.) Review request for mesos, Chun-Hung Hsiao, Eric Chung, Gilbert Song, Jie Yu, and James Peach. Bugs: MESOS-9009 https://issues.apache.org/jira/browse/MESOS-9009 Repository: mesos Description --- Added test cases for the `volume/host_path` isolator for whitelisted non-existing host paths. Diffs (updated) - src/tests/containerizer/volume_host_path_isolator_tests.cpp 81bf72e869d36edb162b121f9e84a53d2096dae3 Diff: https://reviews.apache.org/r/69287/diff/3/ Changes: https://reviews.apache.org/r/69287/diff/2-3/ Testing --- `make check` Thanks, Jason Lai
Re: Review Request 69286: Allowed creating non-existing host paths for Mesos Containerizer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69286/ --- (Updated Feb. 22, 2019, 1:43 a.m.) Review request for mesos, Chun-Hung Hsiao, Eric Chung, Gilbert Song, Jie Yu, and James Peach. Changes --- Address review comment from @gilbert. Bugs: MESOS-9009 https://issues.apache.org/jira/browse/MESOS-9009 Repository: mesos Description --- Added a new agent flag `--host_path_volume_force_creation` for the `volume/host_path` isolator. The flag takes a colon-separated whitelist of paths, under which non-existing host paths are allowed to be created. If the flag is not specified, the isolator behaves in the original way of prohibiting all non-existing host paths from being created. Diffs (updated) - src/CMakeLists.txt 732368293049b7d9d6f62057344d433637ad44e8 src/Makefile.am c17eae4ff1d019d515f67d81821e933ecb5dc190 src/slave/containerizer/mesos/isolators/volume/host_path.hpp 4b509e91a056381ca90293d16a400ea4368234a3 src/slave/containerizer/mesos/isolators/volume/host_path.cpp 88ecf91d91e2bebd484a4ac94510a14b3500dbfb src/slave/containerizer/mesos/isolators/volume/utils.hpp PRE-CREATION src/slave/containerizer/mesos/isolators/volume/utils.cpp PRE-CREATION src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 Diff: https://reviews.apache.org/r/69286/diff/4/ Changes: https://reviews.apache.org/r/69286/diff/3-4/ Testing --- Thanks, Jason Lai
Re: Review Request 69286: Allowed creating non-existing host paths for Mesos Containerizer.
> On Feb. 21, 2019, 11:58 p.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/isolators/volume/host_path.cpp > > Lines 165-167 (original), 181-183 (patched) > > <https://reviews.apache.org/r/69286/diff/3/?file=2126236#file2126236line181> > > > > Sorry, I was wrong. Would you mind removing `createHostPathIfNotExists` > > again? > > > > We probably want to distinguish the case of `pathValidator` exist or > > not explicitly: > > > > Could you do your original logic in > > https://reviews.apache.org/r/69286/diff/2/ ? > > > > Just change the return type of validate(). > > > > Sorry for the overhead Sure thing. No worries. - Jason --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69286/#review213061 --- On Feb. 21, 2019, 11:34 p.m., Jason Lai wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69286/ > --- > > (Updated Feb. 21, 2019, 11:34 p.m.) > > > Review request for mesos, Chun-Hung Hsiao, Eric Chung, Gilbert Song, Jie Yu, > and James Peach. > > > Bugs: MESOS-9009 > https://issues.apache.org/jira/browse/MESOS-9009 > > > Repository: mesos > > > Description > --- > > Added a new agent flag `--host_path_volume_force_creation` for > the `volume/host_path` isolator. The flag takes a colon-separated > whitelist of paths, under which non-existing host paths are allowed to > be created. > > If the flag is not specified, the isolator behaves in the original way > of prohibiting all non-existing host paths from being created. > > > Diffs > - > > src/CMakeLists.txt 732368293049b7d9d6f62057344d433637ad44e8 > src/Makefile.am c17eae4ff1d019d515f67d81821e933ecb5dc190 > src/slave/containerizer/mesos/isolators/volume/host_path.hpp > 4b509e91a056381ca90293d16a400ea4368234a3 > src/slave/containerizer/mesos/isolators/volume/host_path.cpp > 88ecf91d91e2bebd484a4ac94510a14b3500dbfb > src/slave/containerizer/mesos/isolators/volume/utils.hpp PRE-CREATION > src/slave/containerizer/mesos/isolators/volume/utils.cpp PRE-CREATION > src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 > src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 > > > Diff: https://reviews.apache.org/r/69286/diff/3/ > > > Testing > --- > > > Thanks, > > Jason Lai > >
Re: Review Request 69286: Allowed creating non-existing host paths for Mesos Containerizer.
> On Feb. 21, 2019, 8:06 p.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/volume/host_path.hpp
> > Lines 23 (patched)
> > <https://reviews.apache.org/r/69286/diff/2/?file=2124791#file2124791line23>
> >
> > nits:
> >
> > newline above (usually we do that if the hierarchy of the file is
> > different)
> >
> > thanks!
Makes sense. Done.
> On Feb. 21, 2019, 8:06 p.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/volume/host_path.hpp
> > Lines 47 (patched)
> > <https://reviews.apache.org/r/69286/diff/2/?file=2124791#file2124791line47>
> >
> > Nits:
> >
> > could you do?
> > ```
> > const volume::PathValidator& pathValidator
> > ```
Done.
> On Feb. 21, 2019, 8:06 p.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/volume/host_path.hpp
> > Lines 50 (patched)
> > <https://reviews.apache.org/r/69286/diff/2/?file=2124791#file2124791line50>
> >
> > ditto
Done.
> On Feb. 21, 2019, 8:06 p.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/volume/host_path.cpp
> > Lines 70-71 (original), 73-78 (patched)
> > <https://reviews.apache.org/r/69286/diff/2/?file=2124792#file2124792line73>
> >
> > nits:
> >
> > probably I would prefer more explicit:
> > ```
> > Owned process;
> > if (flags.host_path_volume_force_creation.isSome()) {
> > process = new VolumeHostPathIsolatorProcess(
> > flags,
> >
> > PathValidator::parse(flags.host_path_volume_force_creation.get()));
> > } else {
> > process = new VolumeHostPathIsolatorProcess(flags);
> > }
> > ```
`Owned` doesn't support assignment after initialization, so the previous
ternary-operator way would make more sense.
However, I adjusted my code to the styling you suggested in our private
discussion.
> On Feb. 21, 2019, 8:06 p.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/volume/host_path.cpp
> > Lines 189 (patched)
> > <https://reviews.apache.org/r/69286/diff/2/?file=2124792#file2124792line189>
> >
> > nits:
> >
> > I would suggest to be more explicit on error msg.
> >
> > ```
> > return Failure("Failed to normalize the host path '" + hostPath.get()
> > + "': " + normalizedPath.error());
> > ```
Sure thing.
> On Feb. 21, 2019, 8:06 p.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/volume/utils.cpp
> > Lines 41 (patched)
> > <https://reviews.apache.org/r/69286/diff/2/?file=2124794#file2124794line41>
> >
> > instead of returning a boolean, do you think it is better to return a
> > `Try` (if we always regard `false` as a error case)?
Makes sense. Done.
- Jason
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69286/#review213043
---
On Feb. 11, 2019, 11:12 p.m., Jason Lai wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69286/
> ---
>
> (Updated Feb. 11, 2019, 11:12 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao, Eric Chung, Gilbert Song, Jie Yu,
> and James Peach.
>
>
> Bugs: MESOS-9009
> https://issues.apache.org/jira/browse/MESOS-9009
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Added a new agent flag `--host_path_volume_force_creation` for
> the `volume/host_path` isolator. The flag takes a colon-separated
> whitelist of paths, under which non-existing host paths are allowed to
> be created.
>
> If the flag is not specified, the isolator behaves in the original way
> of prohibiting all non-existing host paths from being created.
>
>
> Diffs
> -
>
> src/CMakeLists.txt 732368293049b7d9d6f62057344d433637ad44e8
> src/Makefile.am c17eae4ff1d019d515f67d81821e933ecb5dc190
> src/slave/containerizer/mesos/isolators/volume/host_path.hpp
> 4b509e91a056381ca90293d16a400ea4368234a3
> src/slave/containerizer/mesos/isolators/volume/host_path.cpp
> 88ecf91d91e2bebd484a4ac94510a14b3500dbfb
> src/slave/containerizer/mesos/isolators/volume/utils.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/volume/utils.cpp PRE-CREATION
> src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8
> src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686
>
>
> Diff: https://reviews.apache.org/r/69286/diff/2/
>
>
> Testing
> ---
>
>
> Thanks,
>
> Jason Lai
>
>
Re: Review Request 69287: Added test cases for the `volume/host_path` isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69287/ --- (Updated Feb. 21, 2019, 11:35 p.m.) Review request for mesos, Chun-Hung Hsiao, Eric Chung, Gilbert Song, Jie Yu, and James Peach. Changes --- Address review comments from @gilbert. Bugs: MESOS-9009 https://issues.apache.org/jira/browse/MESOS-9009 Repository: mesos Description --- Added test cases for the `volume/host_path` isolator for whitelisted non-existing host paths. Diffs (updated) - src/tests/containerizer/volume_host_path_isolator_tests.cpp 81bf72e869d36edb162b121f9e84a53d2096dae3 Diff: https://reviews.apache.org/r/69287/diff/2/ Changes: https://reviews.apache.org/r/69287/diff/1-2/ Testing --- `make check` Thanks, Jason Lai
Re: Review Request 69287: Added test cases for the `volume/host_path` isolator.
> On Feb. 21, 2019, 8:08 p.m., Gilbert Song wrote: > > src/tests/containerizer/volume_host_path_isolator_tests.cpp > > Lines 30 (patched) > > <https://reviews.apache.org/r/69287/diff/1/?file=2106294#file2106294line30> > > > > newline above Done. > On Feb. 21, 2019, 8:08 p.m., Gilbert Song wrote: > > src/tests/containerizer/volume_host_path_isolator_tests.cpp > > Lines 51 (patched) > > <https://reviews.apache.org/r/69287/diff/1/?file=2106294#file2106294line51> > > > > ditto Done. - Jason --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69287/#review213047 ------- On Nov. 7, 2018, 10:06 p.m., Jason Lai wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69287/ > --- > > (Updated Nov. 7, 2018, 10:06 p.m.) > > > Review request for mesos, Chun-Hung Hsiao, Eric Chung, Gilbert Song, Jie Yu, > and James Peach. > > > Bugs: MESOS-9009 > https://issues.apache.org/jira/browse/MESOS-9009 > > > Repository: mesos > > > Description > --- > > Added test cases for the `volume/host_path` isolator for whitelisted > non-existing host paths. > > > Diffs > - > > src/tests/containerizer/volume_host_path_isolator_tests.cpp > 81bf72e869d36edb162b121f9e84a53d2096dae3 > > > Diff: https://reviews.apache.org/r/69287/diff/1/ > > > Testing > --- > > `make check` > > > Thanks, > > Jason Lai > >
Re: Review Request 69286: Allowed creating non-existing host paths for Mesos Containerizer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69286/ --- (Updated Feb. 21, 2019, 11:34 p.m.) Review request for mesos, Chun-Hung Hsiao, Eric Chung, Gilbert Song, Jie Yu, and James Peach. Changes --- Address review comments from @gilbert. Bugs: MESOS-9009 https://issues.apache.org/jira/browse/MESOS-9009 Repository: mesos Description --- Added a new agent flag `--host_path_volume_force_creation` for the `volume/host_path` isolator. The flag takes a colon-separated whitelist of paths, under which non-existing host paths are allowed to be created. If the flag is not specified, the isolator behaves in the original way of prohibiting all non-existing host paths from being created. Diffs (updated) - src/CMakeLists.txt 732368293049b7d9d6f62057344d433637ad44e8 src/Makefile.am c17eae4ff1d019d515f67d81821e933ecb5dc190 src/slave/containerizer/mesos/isolators/volume/host_path.hpp 4b509e91a056381ca90293d16a400ea4368234a3 src/slave/containerizer/mesos/isolators/volume/host_path.cpp 88ecf91d91e2bebd484a4ac94510a14b3500dbfb src/slave/containerizer/mesos/isolators/volume/utils.hpp PRE-CREATION src/slave/containerizer/mesos/isolators/volume/utils.cpp PRE-CREATION src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 Diff: https://reviews.apache.org/r/69286/diff/3/ Changes: https://reviews.apache.org/r/69286/diff/2-3/ Testing --- Thanks, Jason Lai
Re: Review Request 69286: Allowed creating non-existing host paths for Mesos Containerizer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69286/ --- (Updated Feb. 11, 2019, 11:12 p.m.) Review request for mesos, Chun-Hung Hsiao, Eric Chung, Gilbert Song, Jie Yu, and James Peach. Changes --- Created `mesos::internal::slave::volume::PathValidator` that encapsulates the validation logic for whitelisted paths. Bugs: MESOS-9009 https://issues.apache.org/jira/browse/MESOS-9009 Repository: mesos Description --- Added a new agent flag `--host_path_volume_force_creation` for the `volume/host_path` isolator. The flag takes a colon-separated whitelist of paths, under which non-existing host paths are allowed to be created. If the flag is not specified, the isolator behaves in the original way of prohibiting all non-existing host paths from being created. Diffs (updated) - src/CMakeLists.txt 732368293049b7d9d6f62057344d433637ad44e8 src/Makefile.am c17eae4ff1d019d515f67d81821e933ecb5dc190 src/slave/containerizer/mesos/isolators/volume/host_path.hpp 4b509e91a056381ca90293d16a400ea4368234a3 src/slave/containerizer/mesos/isolators/volume/host_path.cpp 88ecf91d91e2bebd484a4ac94510a14b3500dbfb src/slave/containerizer/mesos/isolators/volume/utils.hpp PRE-CREATION src/slave/containerizer/mesos/isolators/volume/utils.cpp PRE-CREATION src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 Diff: https://reviews.apache.org/r/69286/diff/2/ Changes: https://reviews.apache.org/r/69286/diff/1-2/ Testing --- Thanks, Jason Lai
Re: Review Request 69286: Allowed creating non-existing host paths for Mesos Containerizer.
> On Nov. 8, 2018, 6:33 a.m., Gilbert Song wrote: > > Hi, @gilbert. I updated the patch a bit differently than you originally requested by creating a `PathValidator` that encapsulate the validation logic instead. Please check if this revision rings a bell to you. > On Nov. 8, 2018, 6:33 a.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/isolators/volume/host_path.hpp > > Lines 46 (patched) > > <https://reviews.apache.org/r/69286/diff/1/?file=2106288#file2106288line46> > > > > s/hostPathWhitelist/forceCreatedHostPaths/g? Created a validator class for this purpose instead. > On Nov. 8, 2018, 6:33 a.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/isolators/volume/host_path.cpp > > Lines 56-57 (patched) > > <https://reviews.apache.org/r/69286/diff/1/?file=2106289#file2106289line56> > > > > do we still need them if we remove the volume namespace? I created a `mesos::internal::slave::volume::PathValidator` helper for this purpose instead. > On Nov. 8, 2018, 6:33 a.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/isolators/volume/host_path.cpp > > Lines 86 (patched) > > <https://reviews.apache.org/r/69286/diff/1/?file=2106289#file2106289line86> > > > > I would move the parse() to create() Done. I created an overloaded constructor that takes a parsed `PathValidator` instance in the case where `_flags.host_path_volume_force_creation` is not `None`. > On Nov. 8, 2018, 6:33 a.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/isolators/volume/utils.cpp > > Lines 31 (patched) > > <https://reviews.apache.org/r/69286/diff/1/?file=2106291#file2106291line31> > > > > probably we do not need this method? True. I dropped it in favor of a factory method as part of the `PathValidator` class. > On Nov. 8, 2018, 6:33 a.m., Gilbert Song wrote: > > src/slave/flags.cpp > > Lines 657 (patched) > > <https://reviews.apache.org/r/69286/diff/1/?file=2106293#file2106293line657> > > > > if directories do not exist Done. > On Nov. 8, 2018, 6:33 a.m., Gilbert Song wrote: > > src/slave/flags.cpp > > Lines 658-660 (patched) > > <https://reviews.apache.org/r/69286/diff/1/?file=2106293#file2106293line658> > > > > Remove these lines? Done. - Jason ------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69286/#review210388 --- On Nov. 7, 2018, 10:03 p.m., Jason Lai wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69286/ > --- > > (Updated Nov. 7, 2018, 10:03 p.m.) > > > Review request for mesos, Chun-Hung Hsiao, Eric Chung, Gilbert Song, Jie Yu, > and James Peach. > > > Bugs: MESOS-9009 > https://issues.apache.org/jira/browse/MESOS-9009 > > > Repository: mesos > > > Description > --- > > Added a new agent flag `--host_path_volume_force_creation` for > the `volume/host_path` isolator. The flag takes a colon-separated > whitelist of paths, under which non-existing host paths are allowed to > be created. > > If the flag is not specified, the isolator behaves in the original way > of prohibiting all non-existing host paths from being created. > > > Diffs > - > > src/CMakeLists.txt 732368293049b7d9d6f62057344d433637ad44e8 > src/Makefile.am c17eae4ff1d019d515f67d81821e933ecb5dc190 > src/slave/containerizer/mesos/isolators/volume/host_path.hpp > 4b509e91a056381ca90293d16a400ea4368234a3 > src/slave/containerizer/mesos/isolators/volume/host_path.cpp > 88ecf91d91e2bebd484a4ac94510a14b3500dbfb > src/slave/containerizer/mesos/isolators/volume/utils.hpp PRE-CREATION > src/slave/containerizer/mesos/isolators/volume/utils.cpp PRE-CREATION > src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 > src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 > > > Diff: https://reviews.apache.org/r/69286/diff/1/ > > > Testing > --- > > > Thanks, > > Jason Lai > >
Re: Review Request 69377: Added blog post for Mesos Mini.
> On Nov. 18, 2018, 5:19 a.m., Jason Lai wrote: > > Ship It! With a few nits. - Jason --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69377/#review210627 --- On Nov. 18, 2018, 5:18 a.m., Jie Yu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69377/ > --- > > (Updated Nov. 18, 2018, 5:18 a.m.) > > > Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, Greg Mann, Jason > Lai, James DeFelice, James Peach, and Vinod Kone. > > > Repository: mesos > > > Description > --- > > Added blog post for Mesos Mini. > > > Diffs > - > > site/source/blog/2018-11-19-mesos-mini.md PRE-CREATION > > > Diff: https://reviews.apache.org/r/69377/diff/2/ > > > Testing > --- > > Markdown rendering can be viewed here: > https://github.com/jieyu/mesos/blob/mesos_mini_blog/site/source/blog/2018-11-19-mesos-mini.md > > > Thanks, > > Jie Yu > >
Re: Review Request 69377: Added blog post for Mesos Mini.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69377/#review210627 --- Fix it, then Ship it! Ship It! site/source/blog/2018-11-19-mesos-mini.md Lines 110 (patched) <https://reviews.apache.org/r/69377/#comment295360> Nit: 1. s/embed/embedded/ 2. s/underneath/within/ site/source/blog/2018-11-19-mesos-mini.md Lines 127 (patched) <https://reviews.apache.org/r/69377/#comment295362> Nit: 1. s/the followings/the following steps/ 2. s/as if it was/as if it were/ - Jason Lai On Nov. 18, 2018, 5:18 a.m., Jie Yu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69377/ > --- > > (Updated Nov. 18, 2018, 5:18 a.m.) > > > Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, Greg Mann, Jason > Lai, James DeFelice, James Peach, and Vinod Kone. > > > Repository: mesos > > > Description > --- > > Added blog post for Mesos Mini. > > > Diffs > - > > site/source/blog/2018-11-19-mesos-mini.md PRE-CREATION > > > Diff: https://reviews.apache.org/r/69377/diff/2/ > > > Testing > --- > > Markdown rendering can be viewed here: > https://github.com/jieyu/mesos/blob/mesos_mini_blog/site/source/blog/2018-11-19-mesos-mini.md > > > Thanks, > > Jie Yu > >
Re: Review Request 68832: Add unit tests for Stout `path::normalize` function in POSIX.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68832/ --- (Updated Nov. 7, 2018, 10:29 p.m.) Review request for mesos, Chun-Hung Hsiao, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Rebranch the commit parent to #65811 Bugs: MESOS-8257 and MESOS-9009 https://issues.apache.org/jira/browse/MESOS-8257 https://issues.apache.org/jira/browse/MESOS-9009 Repository: mesos Description --- Add unit tests for Stout `path::normalize` function in POSIX. Diffs - 3rdparty/stout/tests/path_tests.cpp 452865b919c0d3644eb0ece0e17e402318aaff41 Diff: https://reviews.apache.org/r/68832/diff/2/ Testing --- `make check` Thanks, Jason Lai
Re: Review Request 68832: Add unit tests for Stout `path::normalize` function in POSIX.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68832/ --- (Updated Nov. 7, 2018, 10:10 p.m.) Review request for mesos, Chun-Hung Hsiao, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Required for [MESOS-9009](https://issues.apache.org/jira/browse/MESOS-9009) Bugs: MESOS-8257 and MESOS-9009 https://issues.apache.org/jira/browse/MESOS-8257 https://issues.apache.org/jira/browse/MESOS-9009 Repository: mesos Description --- Add unit tests for Stout `path::normalize` function in POSIX. Diffs - 3rdparty/stout/tests/path_tests.cpp 452865b919c0d3644eb0ece0e17e402318aaff41 Diff: https://reviews.apache.org/r/68832/diff/2/ Testing --- `make check` Thanks, Jason Lai
Re: Review Request 65811: Added Stout `path::normalize` function for POSIX paths.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/ --- (Updated Nov. 7, 2018, 10:10 p.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Required for [MESOS-9009](https://issues.apache.org/jira/browse/MESOS-9009) Bugs: MESOS-8257 and MESOS-9009 https://issues.apache.org/jira/browse/MESOS-8257 https://issues.apache.org/jira/browse/MESOS-9009 Repository: mesos Description --- Added `path::normalize` to normalize a given pathname and remove redundant separators and up-level references. This function follows the rules described in `path_resolution(7)` for Linux. However, it only performs pure lexical processing without touching the actual filesystem. Diffs - 3rdparty/stout/include/stout/path.hpp ef5a2f3d14cc8cf7416ded5cad00d30a49fd5cf7 Diff: https://reviews.apache.org/r/65811/diff/9/ Testing --- `make tests and make check` with https://reviews.apache.org/r/68832/ Thanks, Jason Lai
Review Request 69286: Allowed creating non-existing host paths for Mesos Containerizer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69286/ --- Review request for mesos, Chun-Hung Hsiao, Eric Chung, Gilbert Song, Jie Yu, and James Peach. Bugs: MESOS-9009 https://issues.apache.org/jira/browse/MESOS-9009 Repository: mesos Description --- Added a new agent flag `--host_path_volume_force_creation` for the `volume/host_path` isolator. The flag takes a colon-separated whitelist of paths, under which non-existing host paths are allowed to be created. If the flag is not specified, the isolator behaves in the original way of prohibiting all non-existing host paths from being created. Diffs - src/CMakeLists.txt 732368293049b7d9d6f62057344d433637ad44e8 src/Makefile.am c17eae4ff1d019d515f67d81821e933ecb5dc190 src/slave/containerizer/mesos/isolators/volume/host_path.hpp 4b509e91a056381ca90293d16a400ea4368234a3 src/slave/containerizer/mesos/isolators/volume/host_path.cpp 88ecf91d91e2bebd484a4ac94510a14b3500dbfb src/slave/containerizer/mesos/isolators/volume/utils.hpp PRE-CREATION src/slave/containerizer/mesos/isolators/volume/utils.cpp PRE-CREATION src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 Diff: https://reviews.apache.org/r/69286/diff/1/ Testing --- Thanks, Jason Lai
Review Request 69287: Added test cases for the `volume/host_path` isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69287/ --- Review request for mesos, Chun-Hung Hsiao, Eric Chung, Gilbert Song, Jie Yu, and James Peach. Bugs: MESOS-9009 https://issues.apache.org/jira/browse/MESOS-9009 Repository: mesos Description --- Added test cases for the `volume/host_path` isolator for whitelisted non-existing host paths. Diffs - src/tests/containerizer/volume_host_path_isolator_tests.cpp 81bf72e869d36edb162b121f9e84a53d2096dae3 Diff: https://reviews.apache.org/r/69287/diff/1/ Testing --- `make check` Thanks, Jason Lai
Re: Review Request 68804: Added Stout `os::readlink` function for POSIX paths.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68804/ --- (Updated Nov. 7, 2018, 9:22 p.m.) Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Rebased to master Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Added `os::readlink` for reading value of a POSIX symbolic link. Diffs (updated) - 3rdparty/stout/include/stout/os/posix/readlink.hpp PRE-CREATION Diff: https://reviews.apache.org/r/68804/diff/2/ Changes: https://reviews.apache.org/r/68804/diff/1-2/ Testing --- Thanks, Jason Lai
Re: Review Request 65812: Added an overloaded version of `os::realpath` to stout
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65812/ --- (Updated Nov. 7, 2018, 9:23 p.m.) Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Rebased to master Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Added an overloaded version of `os::realpath` to stout. The new `os::realpath` function is used for evaluating real path within a scoped root directory. Diffs (updated) - 3rdparty/stout/include/stout/os/posix/realpath.hpp 31352cefc5b8d0ccd9af8f6dabdec4a959fded32 Diff: https://reviews.apache.org/r/65812/diff/8/ Changes: https://reviews.apache.org/r/65812/diff/7-8/ Testing --- Thanks, Jason Lai
Re: Review Request 68832: Add unit tests for Stout `path::normalize` function in POSIX.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68832/ --- (Updated Nov. 7, 2018, 9:26 p.m.) Review request for mesos, Chun-Hung Hsiao, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Rebased to master Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Add unit tests for Stout `path::normalize` function in POSIX. Diffs (updated) - 3rdparty/stout/tests/path_tests.cpp 452865b919c0d3644eb0ece0e17e402318aaff41 Diff: https://reviews.apache.org/r/68832/diff/2/ Changes: https://reviews.apache.org/r/68832/diff/1-2/ Testing (updated) --- `make check` Thanks, Jason Lai
Re: Review Request 67175: Added support for marking slave mounts.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67175/ --- (Updated Nov. 7, 2018, 9:24 p.m.) Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Rebased to master Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Added utility function for marking slave mounts in the mount table. Diffs (updated) - src/linux/fs.hpp 31969f6ba82bf5ee549bfdf9698a21adaa486a90 src/linux/fs.cpp 5cdffe1f4c7f00aee5b8f640e7cfa4a0018cfa0a Diff: https://reviews.apache.org/r/67175/diff/3/ Changes: https://reviews.apache.org/r/67175/diff/2-3/ Testing --- Thanks, Jason Lai
Re: Review Request 65811: Added Stout `path::normalize` function for POSIX paths.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/ --- (Updated Nov. 7, 2018, 9:22 p.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Rebased to master Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Added `path::normalize` to normalize a given pathname and remove redundant separators and up-level references. This function follows the rules described in `path_resolution(7)` for Linux. However, it only performs pure lexical processing without touching the actual filesystem. Diffs (updated) - 3rdparty/stout/include/stout/path.hpp ef5a2f3d14cc8cf7416ded5cad00d30a49fd5cf7 Diff: https://reviews.apache.org/r/65811/diff/9/ Changes: https://reviews.apache.org/r/65811/diff/8-9/ Testing --- `make tests and make check` with https://reviews.apache.org/r/68832/ Thanks, Jason Lai
Re: Review Request 65811: Added Stout `path::normalize` function for POSIX paths.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/ --- (Updated Sept. 26, 2018, 11:59 p.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Added `path::normalize` to normalize a given pathname and remove redundant separators and up-level references. This function follows the rules described in `path_resolution(7)` for Linux. However, it only performs pure lexical processing without touching the actual filesystem. Diffs - 3rdparty/stout/include/stout/path.hpp ef5a2f3d14cc8cf7416ded5cad00d30a49fd5cf7 Diff: https://reviews.apache.org/r/65811/diff/8/ Testing (updated) --- `make tests and make check` with https://reviews.apache.org/r/68832/ Thanks, Jason Lai
Re: Review Request 65811: Added Stout `path::normalize` function for POSIX paths.
> On March 16, 2018, 11:17 p.m., Jie Yu wrote: > > 3rdparty/stout/include/stout/path.hpp > > Lines 65 (patched) > > <https://reviews.apache.org/r/65811/diff/3/?file=1969142#file1969142line65> > > > > Can you add some tests for this? This warrents some tests. Also, please > > reach out to Andrew because this will also compile (maybe used) on windows. > > Jie Yu wrote: > if it's posix only for now, please move to posix only headers, or add an > assertion failure if this method is used on windows. Unit tests are added in https://reviews.apache.org/r/68832/. - Jason --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/#review199363 ------- On Sept. 25, 2018, 12:08 a.m., Jason Lai wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65811/ > --- > > (Updated Sept. 25, 2018, 12:08 a.m.) > > > Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, > James Peach, and Zhitao Li. > > > Bugs: MESOS-8257 > https://issues.apache.org/jira/browse/MESOS-8257 > > > Repository: mesos > > > Description > --- > > Added `path::normalize` to normalize a given pathname and remove > redundant separators and up-level references. > > This function follows the rules described in `path_resolution(7)` > for Linux. However, it only performs pure lexical processing without > touching the actual filesystem. > > > Diffs > - > > 3rdparty/stout/include/stout/path.hpp > ef5a2f3d14cc8cf7416ded5cad00d30a49fd5cf7 > > > Diff: https://reviews.apache.org/r/65811/diff/8/ > > > Testing > --- > > `make tests` with https://reviews.apache.org/r/68832/ > > > Thanks, > > Jason Lai > >
Re: Review Request 65811: Added Stout `path::normalize` function for POSIX paths.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/ --- (Updated Sept. 25, 2018, 12:08 a.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Updated comment for failure on attempting to escape root and minor change. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Added `path::normalize` to normalize a given pathname and remove redundant separators and up-level references. This function follows the rules described in `path_resolution(7)` for Linux. However, it only performs pure lexical processing without touching the actual filesystem. Diffs (updated) - 3rdparty/stout/include/stout/path.hpp ef5a2f3d14cc8cf7416ded5cad00d30a49fd5cf7 Diff: https://reviews.apache.org/r/65811/diff/8/ Changes: https://reviews.apache.org/r/65811/diff/7-8/ Testing (updated) --- `make tests` with https://reviews.apache.org/r/68832/ Thanks, Jason Lai
Review Request 68832: Add unit tests for Stout `path::normalize` function in POSIX.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68832/ --- Review request for mesos, Chun-Hung Hsiao, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Add unit tests for Stout `path::normalize` function in POSIX. Diffs - 3rdparty/stout/tests/path_tests.cpp 452865b919c0d3644eb0ece0e17e402318aaff41 Diff: https://reviews.apache.org/r/68832/diff/1/ Testing --- `make tests` Thanks, Jason Lai
Re: Review Request 65811: Added Stout `path::normalize` function for POSIX paths.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/ --- (Updated Sept. 24, 2018, 8:40 p.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Updated summary and description. Summary (updated) - Added Stout `path::normalize` function for POSIX paths. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description (updated) --- Added `path::normalize` to normalize a given pathname and remove redundant separators and up-level references. This function follows the rules described in `path_resolution(7)` for Linux. However, it only performs pure lexical processing without touching the actual filesystem. Diffs - 3rdparty/stout/include/stout/path.hpp ef5a2f3d14cc8cf7416ded5cad00d30a49fd5cf7 Diff: https://reviews.apache.org/r/65811/diff/7/ Testing --- Thanks, Jason Lai
Re: Review Request 65811: Add `path::normalize` to stout for normalizing path (for POSIX only now)
> On Sept. 24, 2018, 7:53 p.m., Eric Chung wrote: > > 3rdparty/stout/include/stout/path.hpp > > Lines 63 (patched) > > <https://reviews.apache.org/r/65811/diff/6/?file=2091015#file2091015line63> > > > > perhaps rephrase as: `...without touching the actual filesystem.` Fixed. > On Sept. 24, 2018, 7:53 p.m., Eric Chung wrote: > > 3rdparty/stout/include/stout/path.hpp > > Lines 77 (patched) > > <https://reviews.apache.org/r/65811/diff/6/?file=2091015#file2091015line77> > > > > `in Windows as well as absolute paths` sounds a little weird. are these > > two separate todo items or one? Fixed. - Jason --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/#review208903 ------- On Sept. 24, 2018, 8:38 p.m., Jason Lai wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65811/ > --- > > (Updated Sept. 24, 2018, 8:38 p.m.) > > > Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, > James Peach, and Zhitao Li. > > > Bugs: MESOS-8257 > https://issues.apache.org/jira/browse/MESOS-8257 > > > Repository: mesos > > > Description > --- > > Add `path::normalize` to stout for normalizing path (for POSIX only now). > > > Diffs > - > > 3rdparty/stout/include/stout/path.hpp > ef5a2f3d14cc8cf7416ded5cad00d30a49fd5cf7 > > > Diff: https://reviews.apache.org/r/65811/diff/7/ > > > Testing > --- > > > Thanks, > > Jason Lai > >
Re: Review Request 65811: Add `path::normalize` to stout for normalizing path (for POSIX only now)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/ --- (Updated Sept. 24, 2018, 8:38 p.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Address review comments from @cinchurge. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Add `path::normalize` to stout for normalizing path (for POSIX only now). Diffs (updated) - 3rdparty/stout/include/stout/path.hpp ef5a2f3d14cc8cf7416ded5cad00d30a49fd5cf7 Diff: https://reviews.apache.org/r/65811/diff/7/ Changes: https://reviews.apache.org/r/65811/diff/6-7/ Testing --- Thanks, Jason Lai
Re: Review Request 68804: Added Stout `os::readlink` function for POSIX paths.
> On Sept. 21, 2018, 10:53 p.m., Eric Chung wrote: > > 3rdparty/stout/include/stout/os/posix/readlink.hpp > > Lines 34 (patched) > > <https://reviews.apache.org/r/68804/diff/1/?file=2091016#file2091016line34> > > > > should the commented lines be removed? No. If you look at the rest of the repo, it's just the convention here. - Jason --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68804/#review208899 --- On Sept. 21, 2018, 10:34 p.m., Jason Lai wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68804/ > --- > > (Updated Sept. 21, 2018, 10:34 p.m.) > > > Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, James Peach, and > Zhitao Li. > > > Bugs: MESOS-8257 > https://issues.apache.org/jira/browse/MESOS-8257 > > > Repository: mesos > > > Description > --- > > Added `os::readlink` for reading value of a POSIX symbolic link. > > > Diffs > - > > 3rdparty/stout/include/stout/os/posix/readlink.hpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/68804/diff/1/ > > > Testing > --- > > > Thanks, > > Jason Lai > >
Re: Review Request 65812: Added an overloaded version of `os::realpath` to stout
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65812/ --- (Updated Sept. 21, 2018, 10:45 p.m.) Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Address review comments from @jieyu. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Added an overloaded version of `os::realpath` to stout. The new `os::realpath` function is used for evaluating real path within a scoped root directory. Diffs (updated) - 3rdparty/stout/include/stout/os/posix/realpath.hpp 31352cefc5b8d0ccd9af8f6dabdec4a959fded32 Diff: https://reviews.apache.org/r/65812/diff/7/ Changes: https://reviews.apache.org/r/65812/diff/6-7/ Testing --- Thanks, Jason Lai
Re: Review Request 65812: Added an overloaded version of `os::realpath` to stout
> On May 18, 2018, 10:12 p.m., Jie Yu wrote: > > 3rdparty/stout/include/stout/os/posix/realpath.hpp > > Lines 68-81 (patched) > > <https://reviews.apache.org/r/65812/diff/6/?file=2024593#file2024593line68> > > > > IN fact, I would consider making `unprocessed` a `vector` > > (which is a result of `tokenize`) > > > > I understand you want to reset `unprocessed` below, you can still do > > one more `tokenize` there to avoid the complex substr logic here. I tried with a local rewritten version per your suggestion, but it actually turned out to be a bit more complex and added perf overhead to this version. We simply need to split `unprocessed` into 0 up to 2 elements and the logic seems pretty intuitive here. - Jason --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65812/#review203455 ----------- On May 17, 2018, 1:07 a.m., Jason Lai wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65812/ > --- > > (Updated May 17, 2018, 1:07 a.m.) > > > Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, > James Peach, and Zhitao Li. > > > Bugs: MESOS-8257 > https://issues.apache.org/jira/browse/MESOS-8257 > > > Repository: mesos > > > Description > --- > > Added an overloaded version of `os::realpath` to stout. > > The new `os::realpath` function is used for evaluating real path > within a scoped root directory. > > > Diffs > - > > 3rdparty/stout/include/stout/os/posix/realpath.hpp > 31352cefc5b8d0ccd9af8f6dabdec4a959fded32 > > > Diff: https://reviews.apache.org/r/65812/diff/6/ > > > Testing > --- > > > Thanks, > > Jason Lai > >
Re: Review Request 65812: Added an overloaded version of `os::realpath` to stout
> On May 18, 2018, 10:08 p.m., Jie Yu wrote: > > 3rdparty/stout/include/stout/os/posix/realpath.hpp > > Lines 51 (patched) > > <https://reviews.apache.org/r/65812/diff/6/?file=2024593#file2024593line51> > > > > Any reason we need this parameter here? Can we just remove this > > parameter, and use `os::PATH_SEPARATOR` below? Done. > On May 18, 2018, 10:08 p.m., Jie Yu wrote: > > 3rdparty/stout/include/stout/os/posix/realpath.hpp > > Lines 65 (patched) > > <https://reviews.apache.org/r/65812/diff/6/?file=2024593#file2024593line65> > > > > No need for period for error messages. Done > On May 18, 2018, 10:08 p.m., Jie Yu wrote: > > 3rdparty/stout/include/stout/os/posix/realpath.hpp > > Lines 88 (patched) > > <https://reviews.apache.org/r/65812/diff/6/?file=2024593#file2024593line88> > > > > No need to specify `_separator`? Dropped. > On May 18, 2018, 10:08 p.m., Jie Yu wrote: > > 3rdparty/stout/include/stout/os/posix/realpath.hpp > > Lines 99 (patched) > > <https://reviews.apache.org/r/65812/diff/6/?file=2024593#file2024593line99> > > > > no need to specify `_separator` for both functions. They're the default > > value Dropped. > On May 18, 2018, 10:08 p.m., Jie Yu wrote: > > 3rdparty/stout/include/stout/os/posix/realpath.hpp > > Lines 108-112 (patched) > > <https://reviews.apache.org/r/65812/diff/6/?file=2024593#file2024593line108> > > > > Let's add a stout helper `os::readlink`: > > > > ``` > > Try readlink(const string& path); > > ``` I added https://reviews.apache.org/r/68804/ as the wrapper for this. > On May 18, 2018, 10:08 p.m., Jie Yu wrote: > > 3rdparty/stout/include/stout/os/posix/realpath.hpp > > Lines 130 (patched) > > <https://reviews.apache.org/r/65812/diff/6/?file=2024593#file2024593line130> > > > > Ditto. No need for `_separator` Dropped. - Jason --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65812/#review203430 --- On May 17, 2018, 1:07 a.m., Jason Lai wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65812/ > --- > > (Updated May 17, 2018, 1:07 a.m.) > > > Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, > James Peach, and Zhitao Li. > > > Bugs: MESOS-8257 > https://issues.apache.org/jira/browse/MESOS-8257 > > > Repository: mesos > > > Description > --- > > Added an overloaded version of `os::realpath` to stout. > > The new `os::realpath` function is used for evaluating real path > within a scoped root directory. > > > Diffs > - > > 3rdparty/stout/include/stout/os/posix/realpath.hpp > 31352cefc5b8d0ccd9af8f6dabdec4a959fded32 > > > Diff: https://reviews.apache.org/r/65812/diff/6/ > > > Testing > --- > > > Thanks, > > Jason Lai > >
Review Request 68804: Added Stout `os::readlink` function for POSIX paths.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68804/ --- Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Added `os::readlink` for reading value of a POSIX symbolic link. Diffs - 3rdparty/stout/include/stout/os/posix/readlink.hpp PRE-CREATION Diff: https://reviews.apache.org/r/68804/diff/1/ Testing --- Thanks, Jason Lai
Re: Review Request 65811: Add `path::normalize` to stout for normalizing path (for POSIX only now)
> On May 18, 2018, 10:08 p.m., Jie Yu wrote: > > 3rdparty/stout/include/stout/path.hpp > > Lines 60 (patched) > > <https://reviews.apache.org/r/65811/diff/5/?file=2024592#file2024592line60> > > > > In fact, I'd return `Try` if it escapes the root path, this will allow > > us to avoid some silent errors in the subsequent patch (silently change the > > intended path, instead of a failure). Done. - Jason --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/#review203453 ------- On Sept. 21, 2018, 9:53 p.m., Jason Lai wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65811/ > --- > > (Updated Sept. 21, 2018, 9:53 p.m.) > > > Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, > James Peach, and Zhitao Li. > > > Bugs: MESOS-8257 > https://issues.apache.org/jira/browse/MESOS-8257 > > > Repository: mesos > > > Description > --- > > Add `path::normalize` to stout for normalizing path (for POSIX only now). > > > Diffs > - > > 3rdparty/stout/include/stout/path.hpp > ef5a2f3d14cc8cf7416ded5cad00d30a49fd5cf7 > > > Diff: https://reviews.apache.org/r/65811/diff/6/ > > > Testing > --- > > > Thanks, > > Jason Lai > >
Re: Review Request 65811: Add `path::normalize` to stout for normalizing path (for POSIX only now)
> On May 18, 2018, 4:57 p.m., Jie Yu wrote:
> > 3rdparty/stout/include/stout/path.hpp
> > Lines 87 (patched)
> > <https://reviews.apache.org/r/65811/diff/5/?file=2024592#file2024592line87>
> >
> > I small nits, i'd suggest the following to make the logic a bit easier
> > to understand:
> > ```
> > if (component != "..") {
> > components.push_back(component);
> > } else {
> > if (isEmpty && !isAbs) {
> > components.push_back(component);
> > } else if (!isEmpty && components.back() == "..") {
> > components.push_back(component);
> > } else if (!isEmpty) {
> > components.pop_back();
> > }
> > }
> > ```
>
> Jason Lai wrote:
> I was originally using something similar to this. I think the current
> version should be fine, as the comment should explain it. I also just
> reference-checked [Python's implementation of
> `os.path.normpath`](https://github.com/python/cpython/blob/2.7/Lib/posixpath.py#L346-L347),
> it does things exactly the same way.
>
> Jie Yu wrote:
> The part that I found not intuitive to understand is the `else if` part.
> That was my suggestion. I didn't put an issue on this so it's up to you.
>
> Jie Yu wrote:
> BTW, using comments to make the code readable is the worst way.
I changed to the style you suggested for the sake of readability after adding
logic for bailing on escaping root.
- Jason
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65811/#review203392
---
On Sept. 21, 2018, 9:53 p.m., Jason Lai wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65811/
> ---
>
> (Updated Sept. 21, 2018, 9:53 p.m.)
>
>
> Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu,
> James Peach, and Zhitao Li.
>
>
> Bugs: MESOS-8257
> https://issues.apache.org/jira/browse/MESOS-8257
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Add `path::normalize` to stout for normalizing path (for POSIX only now).
>
>
> Diffs
> -
>
> 3rdparty/stout/include/stout/path.hpp
> ef5a2f3d14cc8cf7416ded5cad00d30a49fd5cf7
>
>
> Diff: https://reviews.apache.org/r/65811/diff/6/
>
>
> Testing
> ---
>
>
> Thanks,
>
> Jason Lai
>
>
Re: Review Request 65811: Add `path::normalize` to stout for normalizing path (for POSIX only now)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/ --- (Updated Sept. 21, 2018, 9:53 p.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Address @jieyu's review comments. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Add `path::normalize` to stout for normalizing path (for POSIX only now). Diffs (updated) - 3rdparty/stout/include/stout/path.hpp ef5a2f3d14cc8cf7416ded5cad00d30a49fd5cf7 Diff: https://reviews.apache.org/r/65811/diff/6/ Changes: https://reviews.apache.org/r/65811/diff/5-6/ Testing --- Thanks, Jason Lai
Re: Review Request 67175: Added support for marking slave mounts.
> On May 23, 2018, 12:18 a.m., James Peach wrote: > > src/linux/fs.cpp > > Lines 662 (patched) > > <https://reviews.apache.org/r/67175/diff/1/?file=2024600#file2024600line662> > > > > This seems like a logical extension to `os::touch`? > > > > If we can emulate the `os::stat` API and use enum constants to make > > this more obvious at the call site: > > ``` > > os::touch(path, os::Touch::RECURSIVE, os::Touch::FILE); > > ``` > > > > cc @jieyu > > Jason Lai wrote: > Makes sense. I've dropped the function in this patch and will create > another patch in favor of your suggestion. For `os::Touch::DIRECTORY`, I actually feel it doesn't bring much value to have `os::touch` to support both recursive and non-recursive cases (a direct call to `os::touch` would simply do the work). The semantics of touching with an extra call to `os::utime` upon existing paths also feels unnecessary and/or could introduce breaking risks whn the path is readonly and the action is supposed to be no-op. - Jason --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67175/#review203620 --- On Sept. 15, 2018, 12:36 a.m., Jason Lai wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67175/ > --- > > (Updated Sept. 15, 2018, 12:36 a.m.) > > > Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, James Peach, and > Zhitao Li. > > > Bugs: MESOS-8257 > https://issues.apache.org/jira/browse/MESOS-8257 > > > Repository: mesos > > > Description > --- > > Added utility function for marking slave mounts in the mount table. > > > Diffs > - > > src/linux/fs.hpp 502f85c4a32d8658bdd701975dd5ac3d802d308e > src/linux/fs.cpp 9055ef42edd1fb90e1026d1d603a9ba902cfc1fd > > > Diff: https://reviews.apache.org/r/67175/diff/2/ > > > Testing > --- > > > Thanks, > > Jason Lai > >
Re: Review Request 67022: Refreshed checkpointed SlaveInfo if `--reconfiguration_policy==any`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67022/#review208684 --- Ship it! Ship It! - Jason Lai On Sept. 13, 2018, 5:17 p.m., Zhitao Li wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67022/ > --- > > (Updated Sept. 13, 2018, 5:17 p.m.) > > > Review request for mesos, Eric Chung and Jason Lai. > > > Repository: mesos > > > Description > --- > > Refreshed checkpointed SlaveInfo if `--reconfiguration_policy==any`. > > > Diffs > - > > src/slave/flags.cpp fd53d90776967ae97575140778129d6fddd726d2 > src/slave/slave.cpp e6c7e686f287fb4448a0074d4e99298665fc866d > > > Diff: https://reviews.apache.org/r/67022/diff/3/ > > > Testing > --- > > > Thanks, > > Zhitao Li > >
Re: Review Request 67175: Added support for marking slave mounts and creating non-existing paths.
> On May 23, 2018, 12:18 a.m., James Peach wrote: > > src/linux/fs.hpp > > Lines 294 (patched) > > <https://reviews.apache.org/r/67175/diff/1/?file=2024599#file2024599line294> > > > > This only ever gets used by the `hashset` version, so don't make it a > > static function, just call `mount` directly. Sounds good. > On May 23, 2018, 12:18 a.m., James Peach wrote: > > src/linux/fs.hpp > > Lines 303 (patched) > > <https://reviews.apache.org/r/67175/diff/1/?file=2024599#file2024599line303> > > > > This also doesn't seem that related to the `MountInfoTable` object. > > Make it a standalone function that just does the same thing. It is actually an operation supported through `MountInfoTable` entries (but not applicable through `MountTable`). So I think it makes sense to stay within the scope of `MountInfoTable`. > On May 23, 2018, 12:18 a.m., James Peach wrote: > > src/linux/fs.cpp > > Lines 318 (patched) > > <https://reviews.apache.org/r/67175/diff/1/?file=2024600#file2024600line318> > > > > Can we write tests for this? Maybe something similar to > > `FsTest.ROOT_SlaveMount`? Will do. > On May 23, 2018, 12:18 a.m., James Peach wrote: > > src/linux/fs.cpp > > Lines 662 (patched) > > <https://reviews.apache.org/r/67175/diff/1/?file=2024600#file2024600line662> > > > > This seems like a logical extension to `os::touch`? > > > > If we can emulate the `os::stat` API and use enum constants to make > > this more obvious at the call site: > > ``` > > os::touch(path, os::Touch::RECURSIVE, os::Touch::FILE); > > ``` > > > > cc @jieyu Makes sense. I've dropped the function in this patch and will create another patch in favor of your suggestion. - Jason ------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67175/#review203620 --- On May 17, 2018, 1:23 a.m., Jason Lai wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67175/ > --- > > (Updated May 17, 2018, 1:23 a.m.) > > > Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, James Peach, and > Zhitao Li. > > > Bugs: MESOS-8257 > https://issues.apache.org/jira/browse/MESOS-8257 > > > Repository: mesos > > > Description > --- > > Added utility functions for marking slave mounts in the mount table and > creating file or directory as a mount point if not already existing. > > > Diffs > - > > src/linux/fs.hpp 76dc09c38996eefd8487ba6ef4977ef2f7c9df4c > src/linux/fs.cpp fbd03b19abb9b56dbf3fb8a84d09a39171bbc1b0 > > > Diff: https://reviews.apache.org/r/67175/diff/1/ > > > Testing > --- > > > Thanks, > > Jason Lai > >
Re: Review Request 67175: Added support for marking slave mounts.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67175/ --- (Updated Sept. 15, 2018, 12:36 a.m.) Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Address review comments. Summary (updated) - Added support for marking slave mounts. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description (updated) --- Added utility function for marking slave mounts in the mount table. Diffs (updated) - src/linux/fs.hpp 502f85c4a32d8658bdd701975dd5ac3d802d308e src/linux/fs.cpp 9055ef42edd1fb90e1026d1d603a9ba902cfc1fd Diff: https://reviews.apache.org/r/67175/diff/2/ Changes: https://reviews.apache.org/r/67175/diff/1-2/ Testing --- Thanks, Jason Lai
Re: Review Request 68088: Replaced `cgroups::DESTROY_TIMEOUT` with new agent flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68088/#review206604 --- Ship it! Ship It! - Jason Lai On July 30, 2018, 5:50 p.m., Zhitao Li wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68088/ > --- > > (Updated July 30, 2018, 5:50 p.m.) > > > Review request for mesos, Gilbert Song and Jason Lai. > > > Bugs: MESOS-8038 > https://issues.apache.org/jira/browse/MESOS-8038 > > > Repository: mesos > > > Description > --- > > The new agent flag can be used to reconfigure how long a container > destroy is allowed to take on Mesos containerizer. > > The default is also increased to 5 min based on suggestion from Gilbert > because certain containers could have deep system calls which may not > finish within previous 1 min timeout. > > > Diffs > - > > src/linux/cgroups.hpp 6a6f0e6df208bc0b0a888d132b3befd062755851 > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp > 8fba6f9d335cc43a7cd0fcb51ed047ba0e7027ed > src/slave/containerizer/mesos/linux_launcher.cpp > 3bddcece7028745cec6623ac33dbfcaced629629 > src/slave/flags.hpp eeb9708f9ec76d83b6719541f4a012544c7c0cbe > src/slave/flags.cpp 58cdc0f1100fe244e5bf1036e1ccf39478d5d478 > > > Diff: https://reviews.apache.org/r/68088/diff/2/ > > > Testing > --- > > `make` and `./bin/mesos-slave.sh --help` > > > Thanks, > > Zhitao Li > >
Re: Review Request 66875: Added an hourly timer for docker store pull latency.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66875/#review204958 --- Ship it! Ship It! - Jason Lai On May 21, 2018, 5:16 p.m., Zhitao Li wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66875/ > --- > > (Updated May 21, 2018, 5:16 p.m.) > > > Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Qian Zhang. > > > Bugs: MESOS-6451 > https://issues.apache.org/jira/browse/MESOS-6451 > > > Repository: mesos > > > Description > --- > > This patch added pull latency tracking for docker store, which can tell > us both latency distribution of pull as well as number of pulls. > > > Diffs > - > > src/slave/containerizer/mesos/provisioner/docker/store.cpp > 8b3f07f5027cb90d4b4ed401960494709d3eda5f > > > Diff: https://reviews.apache.org/r/66875/diff/2/ > > > Testing > --- > > Ran agent in command line and trigger several launches through > `mesos-execute`, observed following metrics from agent endpoint: > > ``` > "containerizer/mesos/docker_store/pull_ms": 4084.254208, > "containerizer/mesos/docker_store/pull_ms/count": 2, > "containerizer/mesos/docker_store/pull_ms/max": 4084.254208, > "containerizer/mesos/docker_store/pull_ms/min": 3525.044736, > "containerizer/mesos/docker_store/pull_ms/p50": 3804.649472, > "containerizer/mesos/docker_store/pull_ms/p90": 4028.3332608, > "containerizer/mesos/docker_store/pull_ms/p95": 4056.2937344, > "containerizer/mesos/docker_store/pull_ms/p99": 4078.66211328, > "containerizer/mesos/docker_store/pull_ms/p999": 4083.694998528, > "containerizer/mesos/docker_store/pull_ms/p": 4084.1982870528, > ``` > > > Thanks, > > Zhitao Li > >
Re: Review Request 53105: Added an hourly timer for `slave/docker_containerizer/pull`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53105/#review204957 --- Ship it! Ship It! - Jason Lai On May 21, 2018, 5:16 p.m., Zhitao Li wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53105/ > --- > > (Updated May 21, 2018, 5:16 p.m.) > > > Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Qian Zhang. > > > Bugs: MESOS-6451 > https://issues.apache.org/jira/browse/MESOS-6451 > > > Repository: mesos > > > Description > --- > > Added an hourly timer for `slave/docker_containerizer/pull`. > > > Diffs > - > > src/slave/containerizer/docker.hpp 6bdf4c7f60384473927de437a061fafe94170557 > src/slave/containerizer/docker.cpp a4c9c10e91e75f406329cbb2086f39b549cbeed0 > > > Diff: https://reviews.apache.org/r/53105/diff/5/ > > > Testing > --- > > Ran `mesos-execute` with a docker image and docker containerizer, observed > the following metrics: > ``` > curl -s localhost:5051/metrics/snapshot | jq . | grep pull > "containerizer/docker/pull_ms/p999": 26209.36173824, > "containerizer/docker/pull_ms/p90": 21036.405248, > "containerizer/docker/pull_ms/p": 26256.388615424, > "containerizer/docker/pull_ms/p50": 135.570944, > "containerizer/docker/pull_ms/max": 26261.613824, > "containerizer/docker/pull_ms/p95": 23649.009536, > "containerizer/docker/pull_ms/min": 103.215872, > "containerizer/docker/pull_ms/p99": 25739.0929664, > "containerizer/docker/pull_ms": 26261.613824, > "containerizer/docker/pull_ms/count": 3 > ``` > > > Thanks, > > Zhitao Li > >
Re: Review Request 67264: Unmounted any mount points in gc paths.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67264/#review204566
---
Ship it!
Ship It!
src/slave/gc.cpp
Lines 225-227 (patched)
<https://reviews.apache.org/r/67264/#comment287165>
This works, but if you wanna be consistent with the rest of how reverse
iteration of the mount entries, the following could be considered:
```
foreach (const MountTable::Entry& entry,
adaptor::reverse(mountTable->entries)) {
```
This is used in `src/linux/fs.cpp` and a couple of other places.
- Jason Lai
On June 11, 2018, 8:58 p.m., Zhitao Li wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67264/
> ---
>
> (Updated June 11, 2018, 8:58 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-8830
> https://issues.apache.org/jira/browse/MESOS-8830
>
>
> Repository: mesos
>
>
> Description
> ---
>
> In various corner cases, agent may not get chance to properly unmount
> persistent volumes mounted inside an executor's sandbox. When GC later
> gets to these sandbox directories, permanent data loss can happen (see
> MESOS-8830).
>
> Currently, the only mounts in the host mount namespace under the sandbox
> directories are persistent volumes, so this diff added protection to
> unmount any dangling mount points before calling `rmdir` on the
> directory.
>
> NOTE: this means agent will not garbage collect any path if it cannot
> read its own `mountinfo` table.
>
>
> Diffs
> -
>
> src/local/local.cpp afff54653e8e659d947ddbee6dc38ba2715f2a78
> src/slave/gc.hpp df40165bb8a23f065156bf6c5f354b143d88c088
> src/slave/gc.cpp 390b35e6d17d6614a73c9548decbf10739560106
> src/slave/gc_process.hpp 20374ad91820341282fdf18ecade60a020e26cea
> src/slave/main.cpp 646125344d590b28256d8ee684d7e51a90e82f23
> src/slave/paths.hpp 015896453410a33923eed07b3e676be19af62a48
> src/slave/paths.cpp ed0b1276908f4990ce7a24c96aea20e8c79d3126
> src/tests/cluster.cpp 01eb0950e687227dac81b1cdb9eaba3379cf5dbb
> src/tests/gc_tests.cpp 619ed22edd9b3909ea24cdcbf62c354420a8d031
> src/tests/mesos.hpp 733344a2f07ebd9d841a55fb9bbfda2e3c1a1eb2
> src/tests/mesos.cpp d3c87c295429481c59d5a49398e289a4b84e4496
> src/tests/slave_tests.cpp 3d67511de5abd3466eeb5ad1daf318209bd69eed
>
>
> Diff: https://reviews.apache.org/r/67264/diff/6/
>
>
> Testing
> ---
>
> Added a unit test in following patch.
>
> Tested with following procedures:
> 1. Start a test master and agent;
> 2. Created a persistent volume on agent through operator API;
> 3. Use `mesos-execute` to run a task;
> 4. Stop the agent;
> 5. Manually bind mount persistent volume path into a `volume` directory
> inside the executor sandbox (to simulate a dangling mount in MESOS-8830);
> 6. Restart agent with `--gc_disk_headroom=1.0 --gc_delay=1secs` to force it
> gc the path immediately.
>
> With this fix, we observed that the dangling mount is automatically cleaned
> up, and agent produces log line:
> ```
> W0523 06:00:04.001075 82745 gc.cpp:229] Unmounting dangling mount point
> '/home/zhitao/mesos-workdir/slaves/b3eb3aff-d19d-45ff-8113-f0316462d3fa-S0/frameworks/b3eb3aff-d19d-45ff-8113-f0316462d3fa-/executors/test_id/runs/1cd3bd06-2632-4541-a708-80c7cd51c74b/volume'
> of persistent volume '/home/zhitao/mesos-workdir/volumes/roles/role/id1'
> inside garbage collected path
> '/home/zhitao/mesos-workdir/slaves/b3eb3aff-d19d-45ff-8113-f0316462d3fa-S0'
> ```
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 67264: Unmounted any dangling persistent volume in gc paths.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67264/#review203912
---
src/slave/gc.cpp
Lines 221 (patched)
<https://reviews.apache.org/r/67264/#comment286292>
You may want to iterate the mount entries in reversed order. Otherwise, you
would likely run into the cases where you try to unmount a parent directory
before its descendant mount points get umounted.
src/slave/gc.cpp
Lines 228 (patched)
<https://reviews.apache.org/r/67264/#comment286290>
For checking whether a path is a descendant of a directory, it's not enough
to just use `strings::startsWith`, as you run into the case where
`strings::startsWith("/mnt/something-else", "/mnt/something")` returns `true`.
It would be safer to check the following:
1) Check if `entry.target` == `info->path`;
2) Check if `strings::startsWith(entry.target, path::join(info->path, ""))`
(`info->path` suffied with a `"/"`);
- Jason Lai
On May 24, 2018, 7:48 p.m., Zhitao Li wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67264/
> ---
>
> (Updated May 24, 2018, 7:48 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-8830
> https://issues.apache.org/jira/browse/MESOS-8830
>
>
> Repository: mesos
>
>
> Description
> ---
>
> In various corner cases, agent may not get chance to properly unmount
> persistent volumes mounted inside an executor's sandbox. When GC later
> gets to these sandbox directories, permanent data loss can happen (see
> MESOS-8830).
>
> This patch added some protection to unmount possible persistent
> volumes inside a path to gc, and skipped the path if unmount failed.
>
> NOTE: this means agent will not garbage collect any path if it cannot
> read its own `mountinfo` table.
>
>
> Diffs
> -
>
> src/local/local.cpp afff54653e8e659d947ddbee6dc38ba2715f2a78
> src/slave/gc.hpp df40165bb8a23f065156bf6c5f354b143d88c088
> src/slave/gc.cpp 390b35e6d17d6614a73c9548decbf10739560106
> src/slave/gc_process.hpp 20374ad91820341282fdf18ecade60a020e26cea
> src/slave/main.cpp 646125344d590b28256d8ee684d7e51a90e82f23
> src/slave/paths.hpp 015896453410a33923eed07b3e676be19af62a48
> src/slave/paths.cpp ed0b1276908f4990ce7a24c96aea20e8c79d3126
> src/tests/cluster.cpp b56212f6529a4d307e65797ad9bb34f2104fc832
> src/tests/gc_tests.cpp 619ed22edd9b3909ea24cdcbf62c354420a8d031
> src/tests/mesos.hpp 733344a2f07ebd9d841a55fb9bbfda2e3c1a1eb2
> src/tests/mesos.cpp d3c87c295429481c59d5a49398e289a4b84e4496
> src/tests/slave_tests.cpp 65d860594572b58a50a89358e31e97fd2a10bf08
>
>
> Diff: https://reviews.apache.org/r/67264/diff/2/
>
>
> Testing
> ---
>
> Tested with following procedures:
> 1. Start a test master and agent;
> 2. Created a persistent volume on agent through operator API;
> 3. Use `mesos-execute` to run a task;
> 4. Stop the agent;
> 5. Manually bind mount persistent volume path into a `volume` directory
> inside the executor sandbox (to simulate a dangling mount in MESOS-8830);
> 6. Restart agent with `--gc_disk_headroom=1.0 --gc_delay=1secs` to force it
> gc the path immediately.
>
> With this fix, we observed that the dangling mount is automatically cleaned
> up, and agent produces log line:
> ```
> W0523 06:00:04.001075 82745 gc.cpp:229] Unmounting dangling mount point
> '/home/zhitao/mesos-workdir/slaves/b3eb3aff-d19d-45ff-8113-f0316462d3fa-S0/frameworks/b3eb3aff-d19d-45ff-8113-f0316462d3fa-/executors/test_id/runs/1cd3bd06-2632-4541-a708-80c7cd51c74b/volume'
> of persistent volume '/home/zhitao/mesos-workdir/volumes/roles/role/id1'
> inside garbage collected path
> '/home/zhitao/mesos-workdir/slaves/b3eb3aff-d19d-45ff-8113-f0316462d3fa-S0'
> ```
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 65811: Add `path::normalize` to stout for normalizing path (for POSIX only now)
> On May 18, 2018, 4:57 p.m., Jie Yu wrote:
> > 3rdparty/stout/include/stout/path.hpp
> > Lines 87 (patched)
> > <https://reviews.apache.org/r/65811/diff/5/?file=2024592#file2024592line87>
> >
> > I small nits, i'd suggest the following to make the logic a bit easier
> > to understand:
> > ```
> > if (component != "..") {
> > components.push_back(component);
> > } else {
> > if (isEmpty && !isAbs) {
> > components.push_back(component);
> > } else if (!isEmpty && components.back() == "..") {
> > components.push_back(component);
> > } else if (!isEmpty) {
> > components.pop_back();
> > }
> > }
> > ```
I was originally using something similar to this. I think the current version
should be fine, as the comment should explain it. I also just reference-checked
[Python's implementation of
`os.path.normpath`](https://github.com/python/cpython/blob/2.7/Lib/posixpath.py#L346-L347),
it does things exactly the same way.
- Jason
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65811/#review203392
---
On May 17, 2018, 1:06 a.m., Jason Lai wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65811/
> ---
>
> (Updated May 17, 2018, 1:06 a.m.)
>
>
> Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu,
> James Peach, and Zhitao Li.
>
>
> Bugs: MESOS-8257
> https://issues.apache.org/jira/browse/MESOS-8257
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Add `path::normalize` to stout for normalizing path (for POSIX only now).
>
>
> Diffs
> -
>
> 3rdparty/stout/include/stout/path.hpp
> 27438d31617b3b78bf3d4deffd25c93340610e8d
>
>
> Diff: https://reviews.apache.org/r/65811/diff/5/
>
>
> Testing
> ---
>
>
> Thanks,
>
> Jason Lai
>
>
Re: Review Request 65811: Add `path::normalize` to stout for normalizing path (for POSIX only now)
> On May 14, 2018, 9:34 p.m., Andrew Schwartzmeyer wrote: > > 3rdparty/stout/include/stout/path.hpp > > Lines 74 (patched) > > <https://reviews.apache.org/r/65811/diff/4/?file=2018188#file2018188line74> > > > > This should just be `is Abs = path::absolute(path)`. The function is > > already written, also only performs pure lexical processing, and is > > cross-platform. Will address that in another update. > On May 14, 2018, 9:34 p.m., Andrew Schwartzmeyer wrote: > > 3rdparty/stout/include/stout/path.hpp > > Lines 77 (patched) > > <https://reviews.apache.org/r/65811/diff/4/?file=2018188#file2018188line77> > > > > ;) > > > > Given some unit to tests to prove it, I don't see a reason why this > > wouldn't handle Windows paths pretty much as-is. Maybe one extra step to > > convert `/` to `\` if the given separator is `\`. Since often "normalizing" > > on Windows also means converting slashes to something consistent (and we > > opt for `\` because of long paths). Will do in a separate patch. > On May 14, 2018, 9:34 p.m., Andrew Schwartzmeyer wrote: > > 3rdparty/stout/include/stout/path.hpp > > Lines 103-104 (patched) > > <https://reviews.apache.org/r/65811/diff/4/?file=2018188#file2018188line103> > > > > I guess, to make it cross platform, you'd want to save the original > > prefix (which could be a tad annoying), and then re-add it here. Indeed. As said in a previous comment, will do that in a separate patch for cross platform support. - Jason --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/#review203072 --- On May 17, 2018, 1:06 a.m., Jason Lai wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65811/ > --- > > (Updated May 17, 2018, 1:06 a.m.) > > > Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, > James Peach, and Zhitao Li. > > > Bugs: MESOS-8257 > https://issues.apache.org/jira/browse/MESOS-8257 > > > Repository: mesos > > > Description > --- > > Add `path::normalize` to stout for normalizing path (for POSIX only now). > > > Diffs > - > > 3rdparty/stout/include/stout/path.hpp > 27438d31617b3b78bf3d4deffd25c93340610e8d > > > Diff: https://reviews.apache.org/r/65811/diff/5/ > > > Testing > --- > > > Thanks, > > Jason Lai > >
Review Request 67177: Sorted container mounts by their target paths.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67177/ --- Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-6798 https://issues.apache.org/jira/browse/MESOS-6798 Repository: mesos Description --- Container mounts are sorted by: 1) their target paths, and then 2) their source paths, before mounting them upon container launch. Diffs - src/slave/containerizer/mesos/launch.cpp f25d90651ef32495c9161c3eaed8a327d1b2b926 Diff: https://reviews.apache.org/r/67177/diff/1/ Testing --- Manual Thanks, Jason Lai
Review Request 67176: Fixed target path resolution for mounts within container rootfs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67176/ --- Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- The patch ensures that container mount targets that point to (or are under) symlinks to absolute paths are resolved within the container root filesystem, rather than to paths on host filesystem, when mounting them prior to `chroot` (or `pivot_root`). Creation of non-existing mount target paths is also done upon container launch, rather than by their isolators. Diffs - src/slave/containerizer/mesos/launch.cpp f25d90651ef32495c9161c3eaed8a327d1b2b926 Diff: https://reviews.apache.org/r/67176/diff/1/ Testing --- Thanks, Jason Lai
Review Request 67175: Added support for marking slave mounts and creating non-existing paths.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67175/ --- Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Added utility functions for marking slave mounts in the mount table and creating file or directory as a mount point if not already existing. Diffs - src/linux/fs.hpp 76dc09c38996eefd8487ba6ef4977ef2f7c9df4c src/linux/fs.cpp fbd03b19abb9b56dbf3fb8a84d09a39171bbc1b0 Diff: https://reviews.apache.org/r/67175/diff/1/ Testing --- Thanks, Jason Lai
Re: Review Request 65900: Defer creation of volume target paths to container launch.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65900/ --- (Updated May 17, 2018, 1:16 a.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- * Rebased to the latest `master` branch; * Renamed `getAbsoluteMountPoint` to `getAbsolutePathForMountPoint`; * Removed references to `ContainerConfig` in `getAbsolutePathForMountPoint`; * Minor refactors on `isolators/volume/sandbox_path.cpp`, some code extracted to `isolators/volume/utils.cpp`; Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Defer creation of volume target paths to container launch. Diffs (updated) - src/Makefile.am c08ac6e2f5deec4d05f59f71ff6c51382f216708 src/slave/containerizer/mesos/isolators/volume/host_path.cpp 9127cf4a9d8aa2b16bb5b9903103f1f76a1e2b1a src/slave/containerizer/mesos/isolators/volume/image.cpp 631553e2f61a1b95dd93d333b547ff237f65f59e src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp e0cae1036e2e49b4f61705c77f31ae166d1b1380 src/slave/containerizer/mesos/isolators/volume/utils.hpp PRE-CREATION Diff: https://reviews.apache.org/r/65900/diff/3/ Changes: https://reviews.apache.org/r/65900/diff/2-3/ Testing --- Thanks, Jason Lai
Re: Review Request 65812: Added an overloaded version of `os::realpath` to stout
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65812/ --- (Updated May 17, 2018, 1:07 a.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Rebased to the latest `master` branch. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Added an overloaded version of `os::realpath` to stout. The new `os::realpath` function is used for evaluating real path within a scoped root directory. Diffs (updated) - 3rdparty/stout/include/stout/os/posix/realpath.hpp 31352cefc5b8d0ccd9af8f6dabdec4a959fded32 Diff: https://reviews.apache.org/r/65812/diff/6/ Changes: https://reviews.apache.org/r/65812/diff/5-6/ Testing --- Thanks, Jason Lai
Re: Review Request 65811: Add `path::normalize` to stout for normalizing path (for POSIX only now)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/ --- (Updated May 17, 2018, 1:06 a.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Rebased to the latest `master` branch. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Add `path::normalize` to stout for normalizing path (for POSIX only now). Diffs (updated) - 3rdparty/stout/include/stout/path.hpp 27438d31617b3b78bf3d4deffd25c93340610e8d Diff: https://reviews.apache.org/r/65811/diff/5/ Changes: https://reviews.apache.org/r/65811/diff/4-5/ Testing --- Thanks, Jason Lai
Re: Review Request 65900: Defer creation of volume target paths to container launch.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65900/#review203156
---
src/slave/containerizer/mesos/isolators/volume/utils.hpp
Lines 39-69 (patched)
<https://reviews.apache.org/r/65900/#comment285233>
@jieyu: If you really think we should get rid of `containerConfig` from
`getAbsoluteMountPoint`, we can do things this way:
```
inline Try getAbsolutePathForMountPoint(
const std::string& mountPoint,
const Option& rootfs,
const std::string& containerSandboxPath,
const std::string& hostSandboxPath)
{
if (path::absolute(mountPoint)) {
if (rootfs.isSome()) {
return path::join(rootfs.get(), mountPoint);
}
if (!os::exists(mountPoint)) {
return Error(
"Mount point '" + mountPoint + "' is an absolute path. "
"It must exist if the container shares the host filesystem");
}
return mountPoint;
}
if (rootfs.isSome()) {
return path::join(rootfs.get(), containerSandboxPath, mountPoint);
}
return path::join(hostSandboxPath, mountPoint);
}
```
- Jason Lai
On May 8, 2018, 7:07 p.m., Jason Lai wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65900/
> ---
>
> (Updated May 8, 2018, 7:07 p.m.)
>
>
> Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu,
> James Peach, and Zhitao Li.
>
>
> Bugs: MESOS-8257
> https://issues.apache.org/jira/browse/MESOS-8257
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Defer creation of volume target paths to container launch.
>
>
> Diffs
> -
>
> src/Makefile.am a2ba1e6de0868c7a4fe58304ec479dce039bc288
> src/slave/containerizer/mesos/isolators/volume/host_path.cpp
> 9127cf4a9d8aa2b16bb5b9903103f1f76a1e2b1a
> src/slave/containerizer/mesos/isolators/volume/image.cpp
> 8ca3d55d5792ed1cfc8e49df40587ac2abc83fec
> src/slave/containerizer/mesos/isolators/volume/utils.hpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/65900/diff/2/
>
>
> Testing
> ---
>
>
> Thanks,
>
> Jason Lai
>
>
Re: Review Request 67095: Added a containerizer devices path helper.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67095/#review202990 --- Ship it! Ship It! - Jason Lai On May 11, 2018, 6:32 p.m., James Peach wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67095/ > --- > > (Updated May 11, 2018, 6:32 p.m.) > > > Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li. > > > Bugs: MESOS-8792 > https://issues.apache.org/jira/browse/MESOS-8792 > > > Repository: mesos > > > Description > --- > > Added a helper to define a per-container directory for storing > container device nodes. > > > Diffs > - > > src/slave/containerizer/mesos/paths.hpp > b9f0f454ada3ee4e6648916a2c582bcfeebe1732 > src/slave/containerizer/mesos/paths.cpp > cf7d47bf501f6401183c0026cf1aca98395351a0 > > > Diff: https://reviews.apache.org/r/67095/diff/1/ > > > Testing > --- > > manual > > > Thanks, > > James Peach > >
Re: Review Request 67094: Split linux chroot into prepare and enter phases.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67094/#review202976 --- src/linux/fs.cpp Lines 968-1034 (original), 973-1041 (patched) <https://reviews.apache.org/r/67094/#comment285038> We can also consider changing `fs::chroot::enter` with the [`pivotRoot` implementation in runc's `rootfs_linux.go`](https://github.com/opencontainers/runc/blob/0cbfd8392fff2462701507296081e835b3b0b99a/libcontainer/rootfs_linux.go#L645-L702) - Jason Lai On May 11, 2018, 6:31 p.m., James Peach wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67094/ > --- > > (Updated May 11, 2018, 6:31 p.m.) > > > Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li. > > > Bugs: MESOS-8792 > https://issues.apache.org/jira/browse/MESOS-8792 > > > Repository: mesos > > > Description > --- > > Since we will need to perform additional work to configure > the chroot before entering it, split the Linux chroot API > into `fs::chroot::prepare()` and `fs::chroot::enter()`. > > > Diffs > - > > src/linux/fs.hpp 76dc09c38996eefd8487ba6ef4977ef2f7c9df4c > src/linux/fs.cpp fbd03b19abb9b56dbf3fb8a84d09a39171bbc1b0 > src/slave/containerizer/mesos/launch.cpp > f25d90651ef32495c9161c3eaed8a327d1b2b926 > > > Diff: https://reviews.apache.org/r/67094/diff/1/ > > > Testing > --- > > manual > > > Thanks, > > James Peach > >
Re: Review Request 67094: Split linux chroot into prepare and enter phases.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67094/#review202969
---
Great to see the split of `fs::enter::prepare` from `fs::enter::chroot`. Let's
make sure the parts dealing with `$rootfs/tmp` are symmetric within
`fs::enter::chroot` first and I'm fine with the rest.
We could even consider moving some parts of `fs::enter::prepare` into
`launch.cpp` after my chain gets merged into master.
src/linux/fs.cpp
Lines 940-966 (original), 940-966 (patched)
<https://reviews.apache.org/r/67094/#comment285032>
I would suggest that `fs::chroot::enter` starts here, as later in
`fs::chroot::enter` we have `fs::umount("/tmp")`. We should consider making
`fs::chroot::enter` symmetric when dealing with `$rootfs/tmp`, as well as
making it an extended `pivot_root(2)` with no unnecessary preparations and
cleanups.
- Jason Lai
On May 11, 2018, 6:31 p.m., James Peach wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67094/
> ---
>
> (Updated May 11, 2018, 6:31 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.
>
>
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Since we will need to perform additional work to configure
> the chroot before entering it, split the Linux chroot API
> into `fs::chroot::prepare()` and `fs::chroot::enter()`.
>
>
> Diffs
> -
>
> src/linux/fs.hpp 76dc09c38996eefd8487ba6ef4977ef2f7c9df4c
> src/linux/fs.cpp fbd03b19abb9b56dbf3fb8a84d09a39171bbc1b0
> src/slave/containerizer/mesos/launch.cpp
> f25d90651ef32495c9161c3eaed8a327d1b2b926
>
>
> Diff: https://reviews.apache.org/r/67094/diff/1/
>
>
> Testing
> ---
>
> manual
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 67013: Sort container mounts by their target paths.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67013/ --- (Updated May 10, 2018, 11:32 p.m.) Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-6798 https://issues.apache.org/jira/browse/MESOS-6798 Repository: mesos Description --- Sort container mounts by their target paths. Diffs (updated) - src/slave/containerizer/mesos/launch/linux_helper.cpp PRE-CREATION Diff: https://reviews.apache.org/r/67013/diff/2/ Changes: https://reviews.apache.org/r/67013/diff/1-2/ Testing --- Thanks, Jason Lai
Re: Review Request 67012: Resolve container target paths within their rootfs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67012/ --- (Updated May 10, 2018, 11:31 p.m.) Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Add an extra argument of `rootfs` to `createMount`. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Resolve container target paths within their rootfs. Diffs (updated) - src/slave/containerizer/mesos/launch/linux_helper.cpp PRE-CREATION Diff: https://reviews.apache.org/r/67012/diff/2/ Changes: https://reviews.apache.org/r/67012/diff/1-2/ Testing --- Thanks, Jason Lai
Re: Review Request 67022: Refreshed checkpointed SlaveInfo if `--reconfiguration_policy==any`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67022/#review202723 --- Ship it! Ship It! - Jason Lai On May 9, 2018, 12:40 a.m., Zhitao Li wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67022/ > --- > > (Updated May 9, 2018, 12:40 a.m.) > > > Review request for mesos, Eric Chung and Jason Lai. > > > Repository: mesos > > > Description > --- > > Refreshed checkpointed SlaveInfo if `--reconfiguration_policy==any`. > > > Diffs > - > > src/slave/flags.cpp a319b5ea633c41fd8a252c5e1617ac52d1480ba5 > src/slave/slave.cpp c6d9152d9de4184f9107bb8242b41d468d76e018 > > > Diff: https://reviews.apache.org/r/67022/diff/2/ > > > Testing > --- > > > Thanks, > > Zhitao Li > >
Re: Review Request 65900: Defer creation of volume target paths to container launch.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65900/ --- (Updated May 8, 2018, 7:07 p.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Refactored and extracted bidirectional mount propagation code to `utils.hpp`. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Defer creation of volume target paths to container launch. Diffs (updated) - src/Makefile.am a2ba1e6de0868c7a4fe58304ec479dce039bc288 src/slave/containerizer/mesos/isolators/volume/host_path.cpp 9127cf4a9d8aa2b16bb5b9903103f1f76a1e2b1a src/slave/containerizer/mesos/isolators/volume/image.cpp 8ca3d55d5792ed1cfc8e49df40587ac2abc83fec src/slave/containerizer/mesos/isolators/volume/utils.hpp PRE-CREATION Diff: https://reviews.apache.org/r/65900/diff/2/ Changes: https://reviews.apache.org/r/65900/diff/1-2/ Testing --- Thanks, Jason Lai
Review Request 67013: Sort container mounts by their target paths.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67013/ --- Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-6798 https://issues.apache.org/jira/browse/MESOS-6798 Repository: mesos Description --- Sort container mounts by their target paths. Diffs - src/slave/containerizer/mesos/launch/linux_helper.cpp PRE-CREATION Diff: https://reviews.apache.org/r/67013/diff/1/ Testing --- Thanks, Jason Lai
Review Request 67012: Resolve container target paths within their rootfs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67012/ --- Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Resolve container target paths within their rootfs. Diffs - src/slave/containerizer/mesos/launch/linux_helper.cpp PRE-CREATION Diff: https://reviews.apache.org/r/67012/diff/1/ Testing --- Thanks, Jason Lai
Re: Review Request 65812: Added an overloaded version of `os::realpath` to stout
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65812/ --- (Updated May 8, 2018, 6:43 p.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Rename `path::clean` to `path::normalize`. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Added an overloaded version of `os::realpath` to stout. The new `os::realpath` function is used for evaluating real path within a scoped root directory. Diffs (updated) - 3rdparty/stout/include/stout/os/posix/realpath.hpp 31352cefc5b8d0ccd9af8f6dabdec4a959fded32 Diff: https://reviews.apache.org/r/65812/diff/5/ Changes: https://reviews.apache.org/r/65812/diff/4-5/ Testing --- Thanks, Jason Lai
Re: Review Request 65811: Add `path::normalize` to stout for normalizing path (for POSIX only now)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/ --- (Updated May 8, 2018, 6:41 p.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Renamed `path::clean` to `path::normalize` per review comment. Summary (updated) - Add `path::normalize` to stout for normalizing path (for POSIX only now) Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description (updated) --- Add `path::normalize` to stout for normalizing path (for POSIX only now). Diffs (updated) - 3rdparty/stout/include/stout/path.hpp 27438d31617b3b78bf3d4deffd25c93340610e8d Diff: https://reviews.apache.org/r/65811/diff/4/ Changes: https://reviews.apache.org/r/65811/diff/3-4/ Testing --- Thanks, Jason Lai
Re: Review Request 66342: Added difference operator overload for hashset.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66342/#review200167 --- Ship it! Ship It! - Jason Lai On March 28, 2018, 6:23 p.m., Zhitao Li wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66342/ > --- > > (Updated March 28, 2018, 6:23 p.m.) > > > Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Jason Lai. > > > Bugs: MESOS-8746 > https://issues.apache.org/jira/browse/MESOS-8746 > > > Repository: mesos > > > Description > --- > > Added difference operator overload for hashset. > > > Diffs > - > > 3rdparty/stout/include/stout/hashset.hpp > 6af209c53185207b53396e7687e3bd7357e57bf1 > > > Diff: https://reviews.apache.org/r/66342/diff/1/ > > > Testing > --- > > > Thanks, > > Zhitao Li > >
Re: Review Request 65812: Added an overloaded version of `os::realpath` to stout
> On March 16, 2018, 11:20 p.m., Jie Yu wrote: > > 3rdparty/stout/include/stout/os/posix/realpath.hpp > > Lines 48 (patched) > > <https://reviews.apache.org/r/65812/diff/4/?file=1970034#file1970034line48> > > > > Please add unit test for this. Will do in a separate patch. - Jason --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65812/#review199365 --- On March 5, 2018, 7:29 a.m., Jason Lai wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65812/ > --- > > (Updated March 5, 2018, 7:29 a.m.) > > > Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, > James Peach, and Zhitao Li. > > > Bugs: MESOS-8257 > https://issues.apache.org/jira/browse/MESOS-8257 > > > Repository: mesos > > > Description > --- > > Added an overloaded version of `os::realpath` to stout. > > The new `os::realpath` function is used for evaluating real path > within a scoped root directory. > > > Diffs > - > > 3rdparty/stout/include/stout/os/posix/realpath.hpp > 31352cefc5b8d0ccd9af8f6dabdec4a959fded32 > > > Diff: https://reviews.apache.org/r/65812/diff/4/ > > > Testing > --- > > > Thanks, > > Jason Lai > >
Re: Review Request 66034: Remount several proc filesystem entries as read-only.
> On March 15, 2018, 6:30 p.m., Zhitao Li wrote:
> > I feel that the complexity of this code justifies better user doc, possibly
> > when we create a new isolator for this?
> >
> > Also, how much of each mount should be allow to reconfigure? Should this
> > behavior be dictated for every user of Mesos containerizer?
Totally agreed that we should move the above mount points to different
isolators and it's part of my plan about the patches for
[MESOS-6798](https://issues.apache.org/jira/browse/MESOS-6798).
It would make more sense if the mount points under `/proc` and `/sys` (as well
as some of `/dev`) are moved to `filesystem/linux`.
As for whether these extra mount points should be applied to each and every
Mesos containers, my answer is no. But they should definitely be applied to
most of Mesos containers for security purpose, as they are usually application
containers.
That said, for more privileged containers, they should not be mandated. We
could consider adding a few knobs to different levels to allow users to tweak
the behavior. For example, an extra agent flag can be added, so we can have the
agent level default of container security settings. And further more we could
also consider adding an extra field like `privileged` or something else
(similar to Docker's `--privileged` flag), or have something finer-grained like
negated versions of `Protect*` directives in [Systemd's sandboxing
configurations](https://www.freedesktop.org/software/systemd/man/systemd.exec.html#Sandboxing),
to `LinuxInfo`, if people need control security settings of Mesos containers.
I'll put the comments on the tasks themselves, so we can track this better.
> On March 15, 2018, 6:30 p.m., Zhitao Li wrote:
> > src/linux/fs.cpp
> > Lines 686-692 (original), 686-692 (patched)
> > <https://reviews.apache.org/r/66034/diff/1/?file=1974223#file1974223line686>
> >
> > Can we move the `TODO` to the sentence about follow-up? The sentence
> > `These special filesystem mount points need to be bind-mounted prior to all
> > other ...` is a comment on requirement which your follow up work would not
> > change.
Makes sense. It's worth nothing, though, as I said in the other comment in this
thread, the list will eventually be moved away from this file, as I polish up
the mounts with other isolators.
- Jason
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66034/#review199281
-------
On March 15, 2018, 6:24 p.m., Jason Lai wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66034/
> ---
>
> (Updated March 15, 2018, 6:24 p.m.)
>
>
> Review request for mesos, Eric Chung, Gilbert Song, Ian Downes, Jie Yu, James
> Peach, and Zhitao Li.
>
>
> Bugs: MESOS-8654
> https://issues.apache.org/jira/browse/MESOS-8654
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Several entries under the proc FS within Mesos containers need to be
> remounted as readonly for improved security reasons.
>
> The list should include the important ones introduced by Systemd's
> `ProtectKernelTunables` option:
>
> * `/proc/bus`
> * `/proc/fs`
> * `/proc/irq`
> * `/proc/sys`
> * `/proc/sysrq-trigger`
>
> It is particularly necessary to remount `/proc/sysrq-trigger` as
> read-only. Otherwise, it would be possible for processes running in
> containers as `root` to perform privileged operations, such as host
> reboot.
>
> Extra mount options should include `nosuid,noexec,nodev` (see also
> `mount(2)` for detailed explanations of the options).
>
>
> Diffs
> -
>
> src/linux/fs.cpp ed26f80ef7315809a1df9f2c50b4fe3445810f8a
>
>
> Diff: https://reviews.apache.org/r/66034/diff/1/
>
>
> Testing
> ---
>
> The mount table of the container launched by the patched version of
> `mesos-containerizer launch` include the entries listed below, with
> `nosuid,noexec,nodev` mount options:
> ```
> $ sudo unshare -m -p -f /usr/local/libexec/mesos/mesos-containerizer launch
> --launch_info="$(jq -c . launch_info.json)" --runtime_directory="$(pwd)"
> Marked '/' as rslave
> Prepared mount
> '{"flags":20480,"source":"\/etc\/hostname","target":"\/home\/jlai\/containers\/rootfs\/etc\/hostname"}'
> Prepared mount
> '{"flags":20480,"source":"\/etc\/hosts","target":"\/home\/jlai\/containers\/rootfs\/e
Review Request 66104: Fixed potential memory leak in the `volume/sandbox_path` isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66104/ --- Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, and Zhitao Li. Bugs: MESOS-8651 https://issues.apache.org/jira/browse/MESOS-8651 Repository: mesos Description --- The `volume/sandbox_path` isolator inserts a string of the sandbox path to its `sandboxes` hashmap instance variable upon the launch of each container. However, it never cleans it up properly and can cause unbounded growth of the hashmap object, as isolators are global singleton objects. The patch ensures the sandbox path associated with a given container ID gets removed from the `sandboxes` hashmap upon container cleanup. Diffs - src/slave/containerizer/mesos/isolators/volume/sandbox_path.hpp 20d5b32fb7ada1a17a40bf1a2438db4d85cf1063 src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp 5801977e93bcb8f463a2635f73e763098f2aa97d Diff: https://reviews.apache.org/r/66104/diff/1/ Testing --- Thanks, Jason Lai
Re: Review Request 66034: Remount several proc filesystem entries as read-only.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66034/
---
(Updated March 15, 2018, 6:24 p.m.)
Review request for mesos, Eric Chung, Gilbert Song, Ian Downes, Jie Yu, James
Peach, and Zhitao Li.
Changes
---
Fix typos in test plan.
Bugs: MESOS-8654
https://issues.apache.org/jira/browse/MESOS-8654
Repository: mesos
Description
---
Several entries under the proc FS within Mesos containers need to be
remounted as readonly for improved security reasons.
The list should include the important ones introduced by Systemd's
`ProtectKernelTunables` option:
* `/proc/bus`
* `/proc/fs`
* `/proc/irq`
* `/proc/sys`
* `/proc/sysrq-trigger`
It is particularly necessary to remount `/proc/sysrq-trigger` as
read-only. Otherwise, it would be possible for processes running in
containers as `root` to perform privileged operations, such as host
reboot.
Extra mount options should include `nosuid,noexec,nodev` (see also
`mount(2)` for detailed explanations of the options).
Diffs
-
src/linux/fs.cpp ed26f80ef7315809a1df9f2c50b4fe3445810f8a
Diff: https://reviews.apache.org/r/66034/diff/1/
Testing (updated)
---
The mount table of the container launched by the patched version of
`mesos-containerizer launch` include the entries listed below, with
`nosuid,noexec,nodev` mount options:
```
$ sudo unshare -m -p -f /usr/local/libexec/mesos/mesos-containerizer launch
--launch_info="$(jq -c . launch_info.json)" --runtime_directory="$(pwd)"
Marked '/' as rslave
Prepared mount
'{"flags":20480,"source":"\/etc\/hostname","target":"\/home\/jlai\/containers\/rootfs\/etc\/hostname"}'
Prepared mount
'{"flags":20480,"source":"\/etc\/hosts","target":"\/home\/jlai\/containers\/rootfs\/etc\/hosts"}'
Prepared mount
'{"flags":20480,"source":"\/etc\/resolv.conf","target":"\/home\/jlai\/containers\/rootfs\/etc\/resolv.conf"}'
Changing root to /home/jlai/containers/rootfs
bash-4.4# findmnt -a
TARGET SOURCE FSTYPE OPTIONS
/ alpine overlay
rw,relatime,lowerdir=overlay/lower,upperdir=overlay/upper,workdir=overlay/work
|-/etc/hostname /dev/dm-0[/etc/hostname]ext4
rw,noatime,errors=panic,data=ordered
|-/etc/hosts/dev/dm-0[/etc/hosts] ext4
rw,noatime,errors=panic,data=ordered
|-/etc/resolv.conf /dev/dm-0[/etc/resolv.conf] ext4
rw,noatime,errors=panic,data=ordered
|-/proc procproc
rw,nosuid,nodev,noexec,relatime
| |-/proc/bus proc[/bus] proc
ro,nosuid,nodev,noexec,relatime
| |-/proc/fsproc[/fs] proc
ro,nosuid,nodev,noexec,relatime
| |-/proc/irq proc[/irq] proc
ro,nosuid,nodev,noexec,relatime
| |-/proc/sys proc[/sys] proc
ro,nosuid,nodev,noexec,relatime
| `-/proc/sysrq-trigger proc[/sysrq-trigger]proc
ro,nosuid,nodev,noexec,relatime
|-/sys sysfs sysfs
ro,nosuid,nodev,noexec,relatime
`-/dev tmpfs tmpfs
rw,nosuid,noexec,mode=755
|-/dev/ptsdevpts devpts
rw,nosuid,noexec,relatime,mode=600,ptmxmode=666
`-/dev/shmtmpfs tmpfs rw,nosuid,nodev
```
Thanks,
Jason Lai
Re: Review Request 65899: Use launch actions in `MesosContainerizerLaunchHelper` instead.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65899/ --- (Updated March 15, 2018, 8:18 a.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Use unique pointer over raw pointer of `MesosContainerizerLaunchHelper` instance. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Removed launch actions in `src/slave/containerizer/mesos/launch.cpp` and replaced with those in a `MesosContainerLauncherHelper` subclass instead. Diffs (updated) - src/slave/containerizer/mesos/launch.cpp 75b7eaf9cd62d6b5f02896175168b651f4517e12 Diff: https://reviews.apache.org/r/65899/diff/2/ Changes: https://reviews.apache.org/r/65899/diff/1-2/ Testing --- Thanks, Jason Lai
Re: Review Request 65898: Add `MesosContainerizerLaunchHelper` and subclasses for different OSes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65898/ --- (Updated March 15, 2018, 8:17 a.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Returns a unique pointer of `MesosContainerizerLaunchHelper` subclass instance instead of a raw pointer. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- This patch is a refactor from `launch.cpp`, intended for porting and splitting OS-specific launch logic (initially `installResourceLimits`, `prepareMounts` and `chroot`) into different OS-based subclasses. There is no change in business logic from the counterparts in `launch.cpp`. Diffs (updated) - src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef src/Makefile.am a2ba1e6de0868c7a4fe58304ec479dce039bc288 src/slave/containerizer/mesos/launch/helper.hpp PRE-CREATION src/slave/containerizer/mesos/launch/helper.cpp PRE-CREATION src/slave/containerizer/mesos/launch/linux_helper.hpp PRE-CREATION src/slave/containerizer/mesos/launch/linux_helper.cpp PRE-CREATION src/slave/containerizer/mesos/launch/posix_helper.hpp PRE-CREATION src/slave/containerizer/mesos/launch/posix_helper.cpp PRE-CREATION src/slave/containerizer/mesos/launch/windows_helper.hpp PRE-CREATION src/slave/containerizer/mesos/launch/windows_helper.cpp PRE-CREATION Diff: https://reviews.apache.org/r/65898/diff/3/ Changes: https://reviews.apache.org/r/65898/diff/2-3/ Testing --- Thanks, Jason Lai
Re: Review Request 65899: Use launch actions in `MesosContainerizerLaunchHelper` instead.
> On March 7, 2018, 2:28 a.m., Eric Chung wrote: > > src/slave/containerizer/mesos/launch.cpp > > Lines 583 (patched) > > <https://reviews.apache.org/r/65899/diff/1/?file=1970044#file1970044line824> > > > > why is the explicit delete needed here? was it not being cleaned up > > previously? A `MesosContainerizerLauncherHelper` instance was initiated as a raw pointer to the heap. So the intention was that the receiver takes care of managing the instance's lifecycle. However, I think that we can have something smarter like wrapping the raw pointer with libprocess' `Owned` as a uniquely owned pointer. Will do that with a revision. - Jason --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65899/#review198759 ------- On March 5, 2018, 7:31 a.m., Jason Lai wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65899/ > --- > > (Updated March 5, 2018, 7:31 a.m.) > > > Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, > James Peach, and Zhitao Li. > > > Bugs: MESOS-8257 > https://issues.apache.org/jira/browse/MESOS-8257 > > > Repository: mesos > > > Description > --- > > Removed launch actions in `src/slave/containerizer/mesos/launch.cpp` > and replaced with those in a `MesosContainerLauncherHelper` subclass > instead. > > > Diffs > - > > src/slave/containerizer/mesos/launch.cpp > 75b7eaf9cd62d6b5f02896175168b651f4517e12 > > > Diff: https://reviews.apache.org/r/65899/diff/1/ > > > Testing > --- > > > Thanks, > > Jason Lai > >
Re: Review Request 65811: Add `path::clean` to stout for normalizing path (for POSIX only now)
> On March 2, 2018, 8:06 p.m., Chun-Hung Hsiao wrote: > > 3rdparty/stout/include/stout/path.hpp > > Lines 65 (patched) > > <https://reviews.apache.org/r/65811/diff/3/?file=1969142#file1969142line65> > > > > How about s/clean/normalize/? Indeed I considered this option. And I also considered `normpath` as in `os.path.normpath` in Python but ended up picking `path::clean` from the Go counterpart (`filepath.Clean`) over `path::normalize`. That said, I'm open to `path::normalize` if we have more folks in favor of it. - Jason --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/#review198548 --- On March 2, 2018, 7:40 p.m., Jason Lai wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65811/ > --- > > (Updated March 2, 2018, 7:40 p.m.) > > > Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, > James Peach, and Zhitao Li. > > > Bugs: MESOS-8257 > https://issues.apache.org/jira/browse/MESOS-8257 > > > Repository: mesos > > > Description > --- > > Add `path::clean` to stout for normalizing path (for POSIX only now). > > > Diffs > - > > 3rdparty/stout/include/stout/path.hpp > 27438d31617b3b78bf3d4deffd25c93340610e8d > > > Diff: https://reviews.apache.org/r/65811/diff/3/ > > > Testing > --- > > > Thanks, > > Jason Lai > >
Re: Review Request 66034: Remount several proc filesystem entries as read-only.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66034/
---
(Updated March 13, 2018, 1:29 a.m.)
Review request for mesos, Eric Chung, Gilbert Song, Ian Downes, Jie Yu, James
Peach, and Zhitao Li.
Changes
---
Updated description.
Bugs: MESOS-8654
https://issues.apache.org/jira/browse/MESOS-8654
Repository: mesos
Description (updated)
---
Several entries under the proc FS within Mesos containers need to be
remounted as readonly for improved security reasons.
The list should include the important ones introduced by Systemd's
`ProtectKernelTunables` option:
* `/proc/bus`
* `/proc/fs`
* `/proc/irq`
* `/proc/sys`
* `/proc/sysrq-trigger`
It is particularly necessary to remount `/proc/sysrq-trigger` as
read-only. Otherwise, it would be possible for processes running in
containers as `root` to perform privileged operations, such as host
reboot.
Extra mount options should include `nosuid,noexec,nodev` (see also
`mount(2)` for detailed explanations of the options).
Diffs
-
src/linux/fs.cpp ed26f80ef7315809a1df9f2c50b4fe3445810f8a
Diff: https://reviews.apache.org/r/66034/diff/1/
Testing
---
The mount table of the container launched by the patched version of
`mesos-containerizer launch` include the entries listed above, with
`nosuid,noexec,nodev` mount points.
```
$ sudo unshare -m -p -f /usr/local/libexec/mesos/mesos-containerizer launch
--launch_info="$(jq -c . launch_info.json)" --runtime_directory="$(pwd)"
Marked '/' as rslave
Prepared mount
'{"flags":20480,"source":"\/etc\/hostname","target":"\/home\/jlai\/containers\/rootfs\/etc\/hostname"}'
Prepared mount
'{"flags":20480,"source":"\/etc\/hosts","target":"\/home\/jlai\/containers\/rootfs\/etc\/hosts"}'
Prepared mount
'{"flags":20480,"source":"\/etc\/resolv.conf","target":"\/home\/jlai\/containers\/rootfs\/etc\/resolv.conf"}'
Changing root to /home/jlai/containers/rootfs
bash-4.4# findmnt -a
TARGET SOURCE FSTYPE OPTIONS
/ alpine overlay
rw,relatime,lowerdir=overlay/lower,upperdir=overlay/upper,workdir=overlay/work
|-/etc/hostname /dev/dm-0[/etc/hostname]ext4
rw,noatime,errors=panic,data=ordered
|-/etc/hosts/dev/dm-0[/etc/hosts] ext4
rw,noatime,errors=panic,data=ordered
|-/etc/resolv.conf /dev/dm-0[/etc/resolv.conf] ext4
rw,noatime,errors=panic,data=ordered
|-/proc procproc
rw,nosuid,nodev,noexec,relatime
| |-/proc/bus proc[/bus] proc
ro,nosuid,nodev,noexec,relatime
| |-/proc/fsproc[/fs] proc
ro,nosuid,nodev,noexec,relatime
| |-/proc/irq proc[/irq] proc
ro,nosuid,nodev,noexec,relatime
| |-/proc/sys proc[/sys] proc
ro,nosuid,nodev,noexec,relatime
| `-/proc/sysrq-trigger proc[/sysrq-trigger]proc
ro,nosuid,nodev,noexec,relatime
|-/sys sysfs sysfs
ro,nosuid,nodev,noexec,relatime
`-/dev tmpfs tmpfs
rw,nosuid,noexec,mode=755
|-/dev/ptsdevpts devpts
rw,nosuid,noexec,relatime,mode=600,ptmxmode=666
`-/dev/shmtmpfs tmpfs rw,nosuid,nodev
```
Thanks,
Jason Lai
Review Request 66034: Remount several proc filesystem entries as read-only.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66034/
---
Review request for mesos, Eric Chung, Gilbert Song, Ian Downes, Jie Yu, James
Peach, and Zhitao Li.
Bugs: MESOS-8654
https://issues.apache.org/jira/browse/MESOS-8654
Repository: mesos
Description
---
Several entries under the proc FS within Mesos containers need to be
remounted as readonly for improved security reasons.
The list should include the important ones introduced by Systemd's
`ProtectKernelTunables` option:
* `/proc/bus`
* `/proc/fs`
* `/proc/irq`
* `/proc/sys`
* `/proc/sysrq-trigger`
It is particularly necessary to remount `/proc/sysrq-trigger` as
read-only. Otherwise, it would be possible for users running in
containers as `root` to perform privileged operations, such as host
reboot.
Extra mount options should include `nosuid,noexec,nodev` (see also
`mount(2)` for detailed explanations of the options).
Diffs
-
src/linux/fs.cpp ed26f80ef7315809a1df9f2c50b4fe3445810f8a
Diff: https://reviews.apache.org/r/66034/diff/1/
Testing
---
The mount table of the container launched by the patched version of
`mesos-containerizer launch` include the entries listed above, with
`nosuid,noexec,nodev` mount points.
```
$ sudo unshare -m -p -f /usr/local/libexec/mesos/mesos-containerizer launch
--launch_info="$(jq -c . launch_info.json)" --runtime_directory="$(pwd)"
Marked '/' as rslave
Prepared mount
'{"flags":20480,"source":"\/etc\/hostname","target":"\/home\/jlai\/containers\/rootfs\/etc\/hostname"}'
Prepared mount
'{"flags":20480,"source":"\/etc\/hosts","target":"\/home\/jlai\/containers\/rootfs\/etc\/hosts"}'
Prepared mount
'{"flags":20480,"source":"\/etc\/resolv.conf","target":"\/home\/jlai\/containers\/rootfs\/etc\/resolv.conf"}'
Changing root to /home/jlai/containers/rootfs
bash-4.4# findmnt -a
TARGET SOURCE FSTYPE OPTIONS
/ alpine overlay
rw,relatime,lowerdir=overlay/lower,upperdir=overlay/upper,workdir=overlay/work
|-/etc/hostname /dev/dm-0[/etc/hostname]ext4
rw,noatime,errors=panic,data=ordered
|-/etc/hosts/dev/dm-0[/etc/hosts] ext4
rw,noatime,errors=panic,data=ordered
|-/etc/resolv.conf /dev/dm-0[/etc/resolv.conf] ext4
rw,noatime,errors=panic,data=ordered
|-/proc procproc
rw,nosuid,nodev,noexec,relatime
| |-/proc/bus proc[/bus] proc
ro,nosuid,nodev,noexec,relatime
| |-/proc/fsproc[/fs] proc
ro,nosuid,nodev,noexec,relatime
| |-/proc/irq proc[/irq] proc
ro,nosuid,nodev,noexec,relatime
| |-/proc/sys proc[/sys] proc
ro,nosuid,nodev,noexec,relatime
| `-/proc/sysrq-trigger proc[/sysrq-trigger]proc
ro,nosuid,nodev,noexec,relatime
|-/sys sysfs sysfs
ro,nosuid,nodev,noexec,relatime
`-/dev tmpfs tmpfs
rw,nosuid,noexec,mode=755
|-/dev/ptsdevpts devpts
rw,nosuid,noexec,relatime,mode=600,ptmxmode=666
`-/dev/shmtmpfs tmpfs rw,nosuid,nodev
```
Thanks,
Jason Lai
Re: Review Request 65930: Start heartbeater after SUBSCRIBED event.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65930/#review198714 --- Ship it! Ship It! - Jason Lai On March 6, 2018, 4:57 p.m., Zhitao Li wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65930/ > --- > > (Updated March 6, 2018, 4:57 p.m.) > > > Review request for mesos, Greg Mann, Jason Lai, Quinn Leng, and Vinod Kone. > > > Bugs: MESOS-8641 > https://issues.apache.org/jira/browse/MESOS-8641 > > > Repository: mesos > > > Description > --- > > https://reviews.apache.org/r/61262 added heartbeat support to > event stream, but the implementation could send a `HEARTBEAT` > event before `SUBSCRIBED` event, which changed previous behavior. > > This patch fixed the issue by starting heartbeater only after > `SUBSCRIBED` event is sent. > > > Diffs > - > > src/master/http.cpp acf10a0f396234ec9f444c9b41515d657379d6ad > > > Diff: https://reviews.apache.org/r/65930/diff/1/ > > > Testing > --- > > > Thanks, > > Zhitao Li > >
Re: Review Request 65899: Use launch actions in `MesosContainerizerLaunchHelper` instead.
e@internal@mesos@@VErrorAEBVContainerLaunchInfo@24@@Z) > > referenced in function "protected: virtual int __cdecl > > mesos::internal::slave::MesosContainerizerLaunch::execute(void)" > > (?execute@MesosContainerizerLaunch@slave@internal@mesos@@MEAAHXZ) > > [D:\DCOS\mesos\src\launcher\mesos-executor.vcxproj] > > D:\DCOS\mesos\src\mesos-executor.exe : fatal error LNK1120: 1 > > unresolved externals [D:\DCOS\mesos\src\launcher\mesos-executor.vcxproj] > > > > 214 Warning(s) > > 4 Error(s) > > > > Time Elapsed 00:23:35.74 > > ``` [65900](https://reviews.apache.org/r/65900/) has the latest test results for Windows and the 4 errors have been fixed. - Jason --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65899/#review198616 --- On March 5, 2018, 7:31 a.m., Jason Lai wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65899/ > ------- > > (Updated March 5, 2018, 7:31 a.m.) > > > Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, > James Peach, and Zhitao Li. > > > Bugs: MESOS-8257 > https://issues.apache.org/jira/browse/MESOS-8257 > > > Repository: mesos > > > Description > --- > > Removed launch actions in `src/slave/containerizer/mesos/launch.cpp` > and replaced with those in a `MesosContainerLauncherHelper` subclass > instead. > > > Diffs > - > > src/slave/containerizer/mesos/launch.cpp > 75b7eaf9cd62d6b5f02896175168b651f4517e12 > > > Diff: https://reviews.apache.org/r/65899/diff/1/ > > > Testing > --- > > > Thanks, > > Jason Lai > >
Review Request 65900: Defer creation of volume target paths to container launch.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65900/ --- Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Defer creation of volume target paths to container launch. Diffs - src/Makefile.am a2ba1e6de0868c7a4fe58304ec479dce039bc288 src/slave/containerizer/mesos/isolators/volume/host_path.cpp 9127cf4a9d8aa2b16bb5b9903103f1f76a1e2b1a src/slave/containerizer/mesos/isolators/volume/image.cpp 8ca3d55d5792ed1cfc8e49df40587ac2abc83fec src/slave/containerizer/mesos/isolators/volume/utils.hpp PRE-CREATION Diff: https://reviews.apache.org/r/65900/diff/1/ Testing --- Thanks, Jason Lai
Re: Review Request 65898: Add `MesosContainerizerLaunchHelper` and subclasses for different OSes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65898/ --- (Updated March 5, 2018, 9:48 p.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Updated `CMakeLists.txt` to reflect the new source files. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- This patch is a refactor from `launch.cpp`, intended for porting and splitting OS-specific launch logic (initially `installResourceLimits`, `prepareMounts` and `chroot`) into different OS-based subclasses. There is no change in business logic from the counterparts in `launch.cpp`. Diffs (updated) - src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef src/Makefile.am a2ba1e6de0868c7a4fe58304ec479dce039bc288 src/slave/containerizer/mesos/launch/helper.hpp PRE-CREATION src/slave/containerizer/mesos/launch/helper.cpp PRE-CREATION src/slave/containerizer/mesos/launch/linux_helper.hpp PRE-CREATION src/slave/containerizer/mesos/launch/linux_helper.cpp PRE-CREATION src/slave/containerizer/mesos/launch/posix_helper.hpp PRE-CREATION src/slave/containerizer/mesos/launch/posix_helper.cpp PRE-CREATION src/slave/containerizer/mesos/launch/windows_helper.hpp PRE-CREATION src/slave/containerizer/mesos/launch/windows_helper.cpp PRE-CREATION Diff: https://reviews.apache.org/r/65898/diff/2/ Changes: https://reviews.apache.org/r/65898/diff/1-2/ Testing --- Thanks, Jason Lai
Review Request 65899: Use launch actions in `MesosContainerizerLaunchHelper` instead.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65899/ --- Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Removed launch actions in `src/slave/containerizer/mesos/launch.cpp` and replaced with those in a `MesosContainerLauncherHelper` subclass instead. Diffs - src/slave/containerizer/mesos/launch.cpp 75b7eaf9cd62d6b5f02896175168b651f4517e12 Diff: https://reviews.apache.org/r/65899/diff/1/ Testing --- Thanks, Jason Lai
Review Request 65898: Add `MesosContainerizerLaunchHelper` and subclasses for different OSes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65898/ --- Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- This patch is a refactor from `launch.cpp`, intended for porting and splitting OS-specific launch logic (initially `installResourceLimits`, `prepareMounts` and `chroot`) into different OS-based subclasses. There is no change in business logic from the counterparts in `launch.cpp`. Diffs - src/Makefile.am a2ba1e6de0868c7a4fe58304ec479dce039bc288 src/slave/containerizer/mesos/launch/helper.hpp PRE-CREATION src/slave/containerizer/mesos/launch/helper.cpp PRE-CREATION src/slave/containerizer/mesos/launch/linux_helper.hpp PRE-CREATION src/slave/containerizer/mesos/launch/linux_helper.cpp PRE-CREATION src/slave/containerizer/mesos/launch/posix_helper.hpp PRE-CREATION src/slave/containerizer/mesos/launch/posix_helper.cpp PRE-CREATION src/slave/containerizer/mesos/launch/windows_helper.hpp PRE-CREATION src/slave/containerizer/mesos/launch/windows_helper.cpp PRE-CREATION Diff: https://reviews.apache.org/r/65898/diff/1/ Testing --- Thanks, Jason Lai
Re: Review Request 65812: Added an overloaded version of `os::realpath` to stout
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65812/ --- (Updated March 5, 2018, 7:29 a.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Add `static` to the constant of `MAX_SYMLINKS`. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Added an overloaded version of `os::realpath` to stout. The new `os::realpath` function is used for evaluating real path within a scoped root directory. Diffs (updated) - 3rdparty/stout/include/stout/os/posix/realpath.hpp 31352cefc5b8d0ccd9af8f6dabdec4a959fded32 Diff: https://reviews.apache.org/r/65812/diff/4/ Changes: https://reviews.apache.org/r/65812/diff/3-4/ Testing --- Thanks, Jason Lai
Re: Review Request 65812: Added an overloaded version of `os::realpath` to stout
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65812/ --- (Updated March 2, 2018, 7:41 p.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Use `inline` for the overloaded `os::realpath`. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Added an overloaded version of `os::realpath` to stout. The new `os::realpath` function is used for evaluating real path within a scoped root directory. Diffs (updated) - 3rdparty/stout/include/stout/os/posix/realpath.hpp 31352cefc5b8d0ccd9af8f6dabdec4a959fded32 Diff: https://reviews.apache.org/r/65812/diff/3/ Changes: https://reviews.apache.org/r/65812/diff/2-3/ Testing --- Thanks, Jason Lai
Re: Review Request 65811: Add `path::clean` to stout for normalizing path (for POSIX only now)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/ --- (Updated March 2, 2018, 7:40 p.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Use `inline` for `patch::clean` Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Add `path::clean` to stout for normalizing path (for POSIX only now). Diffs (updated) - 3rdparty/stout/include/stout/path.hpp 27438d31617b3b78bf3d4deffd25c93340610e8d Diff: https://reviews.apache.org/r/65811/diff/3/ Changes: https://reviews.apache.org/r/65811/diff/2-3/ Testing --- Thanks, Jason Lai
Re: Review Request 65812: Added an overloaded version of `os::realpath` to stout
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65812/ --- (Updated Feb. 27, 2018, 7:05 p.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Summary (updated) - Added an overloaded version of `os::realpath` to stout Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description (updated) --- Added an overloaded version of `os::realpath` to stout. The new `os::realpath` function is used for evaluating real path within a scoped root directory. Diffs - 3rdparty/stout/include/stout/os/posix/realpath.hpp 31352cefc5b8d0ccd9af8f6dabdec4a959fded32 Diff: https://reviews.apache.org/r/65812/diff/2/ Testing --- Thanks, Jason Lai
Re: Review Request 65812: Added an overloaded version of `os::realpath` to stout for evaluating real path within a scoped root directory
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65812/ --- (Updated Feb. 27, 2018, 7:04 p.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description (updated) --- Added an overloaded version of `os::realpath` to stout for evaluating real path within a scoped root directory. Diffs (updated) - 3rdparty/stout/include/stout/os/posix/realpath.hpp 31352cefc5b8d0ccd9af8f6dabdec4a959fded32 Diff: https://reviews.apache.org/r/65812/diff/2/ Changes: https://reviews.apache.org/r/65812/diff/1-2/ Testing --- Thanks, Jason Lai
Re: Review Request 65811: Add `path::clean` to stout for normalizing path (for POSIX only now)
> On Feb. 27, 2018, 5:42 p.m., Zhitao Li wrote: > > 3rdparty/stout/include/stout/path.hpp > > Lines 61 (patched) > > <https://reviews.apache.org/r/65811/diff/1/?file=1965810#file1965810line61> > > > > Can you clarify whether the rules also apply to windows properly with > > correct separator? Mostly it's about how to deal with the drive part of Windows pathnames. There is code within the same file that I can share, but I intend to do it later (marked with a TODO). > On Feb. 27, 2018, 5:42 p.m., Zhitao Li wrote: > > 3rdparty/stout/include/stout/path.hpp > > Lines 85 (patched) > > <https://reviews.apache.org/r/65811/diff/1/?file=1965810#file1965810line85> > > > > I don't think c++ vector supports negative index (this is not python). > > > > You can use `components.back()` as long as `components` is guaranteed > > not empty. Doh! This was dumb. Nice catch! - Jason --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/#review198334 --- On Feb. 27, 2018, 6:52 p.m., Jason Lai wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65811/ > --- > > (Updated Feb. 27, 2018, 6:52 p.m.) > > > Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, > James Peach, and Zhitao Li. > > > Bugs: MESOS-8257 > https://issues.apache.org/jira/browse/MESOS-8257 > > > Repository: mesos > > > Description > --- > > Add `path::clean` to stout for normalizing path (for POSIX only now). > > > Diffs > - > > 3rdparty/stout/include/stout/path.hpp > 27438d31617b3b78bf3d4deffd25c93340610e8d > > > Diff: https://reviews.apache.org/r/65811/diff/2/ > > > Testing > --- > > > Thanks, > > Jason Lai > >
Re: Review Request 65811: Add `path::clean` to stout for normalizing path (for POSIX only now)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/ --- (Updated Feb. 27, 2018, 6:52 p.m.) Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Changes --- Address review comments. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description (updated) --- Add `path::clean` to stout for normalizing path (for POSIX only now). Diffs (updated) - 3rdparty/stout/include/stout/path.hpp 27438d31617b3b78bf3d4deffd25c93340610e8d Diff: https://reviews.apache.org/r/65811/diff/2/ Changes: https://reviews.apache.org/r/65811/diff/1-2/ Testing --- Thanks, Jason Lai
Review Request 65812: Added an overloaded version of `os::realpath` to stout for evaluating real path within a scoped root directory
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65812/ --- Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Added an overloaded version of `os::realpath` to stout for evaluating real path within a scoped root directory Diffs - 3rdparty/stout/include/stout/os/posix/realpath.hpp 31352cefc5b8d0ccd9af8f6dabdec4a959fded32 Diff: https://reviews.apache.org/r/65812/diff/1/ Testing --- Thanks, Jason Lai
Review Request 65811: Add `path::clean` to stout for normalizing path (for POSIX only now)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/ --- Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, James Peach, and Zhitao Li. Bugs: MESOS-8257 https://issues.apache.org/jira/browse/MESOS-8257 Repository: mesos Description --- Add `path::clean` to stout for normalizing path (for POSIX only now) Diffs - 3rdparty/stout/include/stout/path.hpp 27438d31617b3b78bf3d4deffd25c93340610e8d Diff: https://reviews.apache.org/r/65811/diff/1/ Testing --- Thanks, Jason Lai
Re: Review Request 65294: Support `revocable_resources` capability in `mesos-execute`.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65294/#review196550
---
Ship it!
Ship It!
- Jason Lai
On Jan. 30, 2018, 12:03 a.m., Zhitao Li wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65294/
> ---
>
> (Updated Jan. 30, 2018, 12:03 a.m.)
>
>
> Review request for mesos, Anish Gupta, Jason Lai, and James Peach.
>
>
> Bugs: MESOS-8471
> https://issues.apache.org/jira/browse/MESOS-8471
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Add revocable resources support to mesos-execute.
>
>
> Diffs
> -
>
> src/cli/execute.cpp 6e626893a64dc07ac9a5f9a5c31e680cb80938fe
>
>
> Diff: https://reviews.apache.org/r/65294/diff/2/
>
>
> Testing
> ---
>
> Submitted a test task with following configuration:
> ```
> {
>"name": "Name",
>"task_id": {"value" : "1"},
>"agent_id": {"value" : ""},
>"resources": [
> {
>"name": "cpus",
>"type": "SCALAR",
>"revocable": {},
>"scalar": {
> "value": 0.1
>}
> }
>],
>"command": {
> "value": "sleep 1000"
>}
> }
> ```
>
> With command `mesos-execute --master=localhost:5050 --revocable_resources
> --task=file:///home/user/test_rev_task.json`
>
> If the master/agent has revocable cpu, this allows the task to execute.
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and sweep in docker store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56721/#review190783 --- Fix it, then Ship it! Great stuff! Just a few nits and a question src/slave/containerizer/mesos/containerizer.cpp Lines 2802 (patched) <https://reviews.apache.org/r/56721/#comment268388> Typo: **Legacy** instead of **Legacy** src/slave/containerizer/mesos/provisioner/docker/paths.hpp Lines 98 (patched) <https://reviews.apache.org/r/56721/#comment268386> Nit: I'd suggest using `GC` over `Gc` src/slave/containerizer/mesos/provisioner/docker/paths.hpp Lines 101 (patched) <https://reviews.apache.org/r/56721/#comment268387> Ditto `GC` over `Gc` src/slave/containerizer/mesos/provisioner/docker/store.cpp Lines 594 (patched) <https://reviews.apache.org/r/56721/#comment268389> Will there be a race here? - Jason Lai On Nov. 10, 2017, 7:34 p.m., Zhitao Li wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56721/ > --- > > (Updated Nov. 10, 2017, 7:34 p.m.) > > > Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu. > > > Bugs: MESOS-4945 > https://issues.apache.org/jira/browse/MESOS-4945 > > > Repository: mesos > > > Description > --- > > This includes the following changes: > - add a `pruneImages()` function on the chain of relevant classes; > - implement prune in docker store; > - fix mock interface to keep existing tests pass. > > > Diffs > - > > src/slave/containerizer/composing.hpp > 06d68eef5de7745e32f0e808f11016bcc285dd8f > src/slave/containerizer/composing.cpp > 587f009384f0c7ef87482686578dc822d3d5b208 > src/slave/containerizer/containerizer.hpp > 449bb5d0902936faae7bf9bae9c703b219aed842 > src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270 > src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67 > src/slave/containerizer/mesos/containerizer.hpp > 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b > src/slave/containerizer/mesos/containerizer.cpp > 100e3bbda543d87808da9ff6bea42da5099ea8c5 > src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp > 954da1681778878c8aff6150002e52ecb648d1bb > src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp > d86afd2a6ff0bf87e624db1c99255c85068bf6ab > src/slave/containerizer/mesos/provisioner/docker/paths.hpp > 232c027f8f96da0cb30b957bce4607d3695050d2 > src/slave/containerizer/mesos/provisioner/docker/paths.cpp > cd684b33eb308ce1eeb4539a5b2d51985d835db7 > src/slave/containerizer/mesos/provisioner/docker/store.hpp > 1cf68665d33bd40a7605d26c96fb7b618407fdd0 > src/slave/containerizer/mesos/provisioner/docker/store.cpp > f357710cb19aec3654b0604f7909d068eaf20095 > src/slave/containerizer/mesos/provisioner/provisioner.hpp > 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e > src/slave/containerizer/mesos/provisioner/provisioner.cpp > 450a3b32d69d2882973a6ed4e94e169a0256056b > src/slave/containerizer/mesos/provisioner/store.hpp > 01ab83dca79e51b8c96d18ee65705912b0ac8324 > src/slave/containerizer/mesos/provisioner/store.cpp > cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f > src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac > src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a > src/tests/containerizer/mock_containerizer.hpp > 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569 > > > Diff: https://reviews.apache.org/r/56721/diff/15/ > > > Testing > --- > > > Thanks, > > Zhitao Li > >
Re: Review Request 54693: Add ProtoBuf schema for Blkio cgroup subsystem
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54693/ --- (Updated June 26, 2017, 10:50 p.m.) Review request for mesos, Xiaojian Huang, Gilbert Song, haosdent huang, Jie Yu, Kunal Thakar, and Zhitao Li. Bugs: MESOS-6162 https://issues.apache.org/jira/browse/MESOS-6162 Repository: mesos Description --- Add ProtoBuf schema for Blkio cgroups per the kernel Blkio controller doc. Diffs - include/mesos/mesos.proto 0017d3d9d46433b391703025f611ce437bbc7ebe Diff: https://reviews.apache.org/r/54693/diff/3/ Testing --- Thanks, Jason Lai
Re: Review Request 60170: Supported 'cgroups_auto' flag in unified cgroup isolator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60170/#review178153 --- Ship it! Ship It! - Jason Lai On June 17, 2017, 4:58 p.m., Gilbert Song wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60170/ > --- > > (Updated June 17, 2017, 4:58 p.m.) > > > Review request for mesos, haosdent huang, Hao Yixin, Jason Lai, Jie Yu, and > Vinod Kone. > > > Bugs: MESOS-7691 > https://issues.apache.org/jira/browse/MESOS-7691 > > > Repository: mesos > > > Description > --- > > Supported 'cgroups_auto' flag in unified cgroup isolator. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp > d7a95b2d49fd2ae743e2bffc25f15c4ce4b96f39 > > > Diff: https://reviews.apache.org/r/60170/diff/1/ > > > Testing > --- > > make check > > > Thanks, > > Gilbert Song > >
Re: Review Request 60170: Supported 'cgroups_auto' flag in unified cgroup isolator.
> On June 18, 2017, 4:08 a.m., Jason Lai wrote: > > Ship It! Ditto Qian's comment. - Jason --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60170/#review178153 --- On June 17, 2017, 4:58 p.m., Gilbert Song wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60170/ > --- > > (Updated June 17, 2017, 4:58 p.m.) > > > Review request for mesos, haosdent huang, Hao Yixin, Jason Lai, Jie Yu, and > Vinod Kone. > > > Bugs: MESOS-7691 > https://issues.apache.org/jira/browse/MESOS-7691 > > > Repository: mesos > > > Description > --- > > Supported 'cgroups_auto' flag in unified cgroup isolator. > > > Diffs > - > > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp > d7a95b2d49fd2ae743e2bffc25f15c4ce4b96f39 > > > Diff: https://reviews.apache.org/r/60170/diff/1/ > > > Testing > --- > > make check > > > Thanks, > > Gilbert Song > >
Review Request 59960: Add data structures and stats/control helpers for the Blkio cgroup subsystem
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59960/ --- Review request for mesos, Xiaojian Huang, Gilbert Song, haosdent huang, Jie Yu, and Zhitao Li. Bugs: MESOS-6162 https://issues.apache.org/jira/browse/MESOS-6162 Repository: mesos Description --- * Data structure for Blkio entities * Stats helpers for `blkio.throttle.io*` (generic blkio stats) * Stats helpers for `blkio.io*` (CFQ related stats) * Comments from the kernel blkio doc for helper functions Diffs - src/linux/cgroups.hpp eaf0dcad0ed38c507564624f1647e0c731b8b433 src/linux/cgroups.cpp 334005abfc4ec9b20b7dc0212d852ba1f505dbb5 Diff: https://reviews.apache.org/r/59960/diff/1/ Testing --- Thanks, Jason Lai
