You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jacob Janco <jj...@gmail.com> on 2019/07/17 19:19:26 UTC
Re: Review Request 70757: Added docs for the NNP isolator.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------
(Updated July 17, 2019, 7:19 p.m.)
Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.
Changes
-------
Added docs and fixed patch series I think.
Summary (updated)
-----------------
Added docs for the NNP isolator.
Bugs: MESOS-9770
https://issues.apache.org/jira/browse/MESOS-9770
Repository: mesos
Description (updated)
-------
Added docs for the NNP isolator.
Diffs (updated)
-----
CHANGELOG 164465a71c660ab9f01fb18d43076afc4b892ad5
docs/isolators/linux-nnp.md PRE-CREATION
docs/mesos-containerizer.md e79976111ec8e9cc8e8d44b5f1b8d6e2c7e072d6
Diff: https://reviews.apache.org/r/70757/diff/9/
Changes: https://reviews.apache.org/r/70757/diff/8-9/
Testing
-------
Thanks,
Jacob Janco
Re: Review Request 70757: Added a NNP isolator.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/#review216755
-----------------------------------------------------------
Fix it, then Ship it!
Just some minor formatting style nits.
src/slave/containerizer/mesos/isolators/linux/nnp.hpp
Lines 21 (patched)
<https://reviews.apache.org/r/70757/#comment303994>
These should be the other way round and newline separated:
```
#include "slave/flags.hpp"
#include "slave/containerizer/mesos/isolator.hpp"
```
src/slave/containerizer/mesos/isolators/linux/nnp.cpp
Lines 18 (patched)
<https://reviews.apache.org/r/70757/#comment303995>
You don't need `flags.hpp` again because it is in the isolator header.
src/slave/containerizer/mesos/isolators/linux/nnp.cpp
Lines 22 (patched)
<https://reviews.apache.org/r/70757/#comment303996>
This should come before "common/kernel_version.hpp"
https://github.com/apache/mesos/blob/master/docs/c%2B%2B-style-guide.md#order-of-includes
src/slave/containerizer/mesos/isolators/linux/nnp.cpp
Lines 48 (patched)
<https://reviews.apache.org/r/70757/#comment303992>
Nit: Newline here (I can add it when I commit).
src/tests/containerizer/linux_nnp_isolator_tests.cpp
Lines 19 (patched)
<https://reviews.apache.org/r/70757/#comment303997>
Need to fix order of includes (see c++ style guide).
src/tests/containerizer/linux_nnp_isolator_tests.cpp
Lines 57 (patched)
<https://reviews.apache.org/r/70757/#comment303993>
We can clean up the includes and using statements:
```
$ git diff
diff --git a/src/tests/containerizer/linux_nnp_isolator_tests.cpp b/src/tests/containerizer/linux_nnp_isolator_tests.cpp
index 3afd6c08b..8045acfec 100644
--- a/src/tests/containerizer/linux_nnp_isolator_tests.cpp
+++ b/src/tests/containerizer/linux_nnp_isolator_tests.cpp
@@ -14,31 +14,26 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-#include <mesos/mesos.hpp>
+#include <map>
+#include <string>
-#include "common/kernel_version.hpp"
+#include <mesos/mesos.hpp>
-#include <process/owned.hpp>
#include <process/gtest.hpp>
+#include <process/owned.hpp>
-#include <stout/error.hpp>
-#include <stout/gtest.hpp>
-#include <stout/os.hpp>
-#include <stout/path.hpp>
#include <stout/try.hpp>
+#include "common/kernel_version.hpp"
+
#include "slave/containerizer/mesos/containerizer.hpp"
-#include "slave/containerizer/mesos/isolators/linux/nnp.hpp"
-#include "tests/cluster.hpp"
-#include "tests/environment.hpp"
#include "tests/mesos.hpp"
#include "tests/containerizer/docker_archive.hpp"
using process::Future;
using process::Owned;
-using process::PID;
using std::map;
using std::string;
@@ -47,14 +42,9 @@ using mesos::internal::master::Master;
using mesos::internal::slave::Containerizer;
using mesos::internal::slave::Fetcher;
-using mesos::internal::slave::LinuxNNPIsolatorProcess;
using mesos::internal::slave::MesosContainerizer;
-using mesos::internal::slave::Slave;
-
-using mesos::master::detector::MasterDetector;
using mesos::slave::ContainerTermination;
-using mesos::slave::Isolator;
namespace mesos {
namespace internal {
```
- James Peach
On July 18, 2019, 8:36 p.m., Jacob Janco wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70757/
> -----------------------------------------------------------
>
> (Updated July 18, 2019, 8:36 p.m.)
>
>
> Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.
>
>
> Bugs: MESOS-9770
> https://issues.apache.org/jira/browse/MESOS-9770
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds the isolation capability of flipping the
> NO_NEW_PRIVILEGES bit for process control.
>
>
> Diffs
> -----
>
> include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12
> src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8
> src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153
> src/slave/containerizer/mesos/containerizer.cpp c9a369bcdbfc1676912430c3e84fa567bbd7f766
> src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION
> src/slave/containerizer/mesos/launch.cpp 0419e8e0fbf154396ab5fb5026d77b94cc9fca5b
> src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db
> src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/70757/diff/15/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jacob Janco
>
>
Re: Review Request 70757: Added a NNP isolator.
Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------
(Updated July 19, 2019, 7:53 p.m.)
Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.
Bugs: MESOS-9770
https://issues.apache.org/jira/browse/MESOS-9770
Repository: mesos
Description
-------
This patch adds the isolation capability of flipping the
NO_NEW_PRIVILEGES bit for process control.
Diffs (updated)
-----
include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12
src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8
src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153
src/slave/containerizer/mesos/containerizer.cpp c9a369bcdbfc1676912430c3e84fa567bbd7f766
src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION
src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION
src/slave/containerizer/mesos/launch.cpp 0419e8e0fbf154396ab5fb5026d77b94cc9fca5b
src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db
src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/70757/diff/16/
Changes: https://reviews.apache.org/r/70757/diff/15-16/
Testing
-------
Thanks,
Jacob Janco
Re: Review Request 70757: Added a NNP isolator.
Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------
(Updated July 18, 2019, 8:36 p.m.)
Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.
Bugs: MESOS-9770
https://issues.apache.org/jira/browse/MESOS-9770
Repository: mesos
Description
-------
This patch adds the isolation capability of flipping the
NO_NEW_PRIVILEGES bit for process control.
Diffs (updated)
-----
include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12
src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8
src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153
src/slave/containerizer/mesos/containerizer.cpp c9a369bcdbfc1676912430c3e84fa567bbd7f766
src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION
src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION
src/slave/containerizer/mesos/launch.cpp 0419e8e0fbf154396ab5fb5026d77b94cc9fca5b
src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db
src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/70757/diff/15/
Changes: https://reviews.apache.org/r/70757/diff/14-15/
Testing
-------
Thanks,
Jacob Janco
Re: Review Request 70757: Added a NNP isolator.
Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------
(Updated July 17, 2019, 8:38 p.m.)
Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.
Changes
-------
Broke this off into 3 patches, nnp docs -> this core patch -> kernel version change
Bugs: MESOS-9770
https://issues.apache.org/jira/browse/MESOS-9770
Repository: mesos
Description
-------
This patch adds the isolation capability of flipping the
NO_NEW_PRIVILEGES bit for process control.
Diffs (updated)
-----
include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12
src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8
src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153
src/slave/containerizer/mesos/containerizer.cpp c9a369bcdbfc1676912430c3e84fa567bbd7f766
src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION
src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION
src/slave/containerizer/mesos/launch.cpp 0419e8e0fbf154396ab5fb5026d77b94cc9fca5b
src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db
src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/70757/diff/14/
Changes: https://reviews.apache.org/r/70757/diff/13-14/
Testing
-------
Thanks,
Jacob Janco
Re: Review Request 70757: Added a NNP isolator.
Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------
(Updated July 17, 2019, 7:49 p.m.)
Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.
Changes
-------
Opening docs review in a separate patch, for some reason that is dropping the previous commits when posting.
Summary (updated)
-----------------
Added a NNP isolator.
Bugs: MESOS-9770
https://issues.apache.org/jira/browse/MESOS-9770
Repository: mesos
Description (updated)
-------
This patch adds the isolation capability of flipping the
NO_NEW_PRIVILEGES bit for process control.
Diffs (updated)
-----
include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12
src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8
src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153
src/slave/containerizer/mesos/containerizer.cpp c9a369bcdbfc1676912430c3e84fa567bbd7f766
src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION
src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION
src/slave/containerizer/mesos/launch.cpp 0419e8e0fbf154396ab5fb5026d77b94cc9fca5b
src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db
src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/70757/diff/13/
Changes: https://reviews.apache.org/r/70757/diff/12-13/
Testing
-------
Thanks,
Jacob Janco
Re: Review Request 70757: Added docs for the NNP isolator.
Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------
(Updated July 17, 2019, 7:34 p.m.)
Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.
Changes
-------
Fixed branch.
Bugs: MESOS-9770
https://issues.apache.org/jira/browse/MESOS-9770
Repository: mesos
Description
-------
Added docs for the NNP isolator.
Diffs (updated)
-----
CHANGELOG 164465a71c660ab9f01fb18d43076afc4b892ad5
docs/isolators/linux-nnp.md PRE-CREATION
docs/mesos-containerizer.md e79976111ec8e9cc8e8d44b5f1b8d6e2c7e072d6
Diff: https://reviews.apache.org/r/70757/diff/12/
Changes: https://reviews.apache.org/r/70757/diff/11-12/
Testing
-------
Thanks,
Jacob Janco
Re: Review Request 70757: Added docs for the NNP isolator.
Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------
(Updated July 17, 2019, 7:31 p.m.)
Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.
Bugs: MESOS-9770
https://issues.apache.org/jira/browse/MESOS-9770
Repository: mesos
Description
-------
Added docs for the NNP isolator.
Diffs (updated)
-----
CHANGELOG 164465a71c660ab9f01fb18d43076afc4b892ad5
docs/isolators/linux-nnp.md PRE-CREATION
docs/mesos-containerizer.md e79976111ec8e9cc8e8d44b5f1b8d6e2c7e072d6
Diff: https://reviews.apache.org/r/70757/diff/11/
Changes: https://reviews.apache.org/r/70757/diff/10-11/
Testing
-------
Thanks,
Jacob Janco
Re: Review Request 70757: Added docs for the NNP isolator.
Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------
(Updated July 17, 2019, 7:27 p.m.)
Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.
Bugs: MESOS-9770
https://issues.apache.org/jira/browse/MESOS-9770
Repository: mesos
Description
-------
Added docs for the NNP isolator.
Diffs (updated)
-----
CHANGELOG 164465a71c660ab9f01fb18d43076afc4b892ad5
docs/isolators/linux-nnp.md PRE-CREATION
docs/mesos-containerizer.md e79976111ec8e9cc8e8d44b5f1b8d6e2c7e072d6
Diff: https://reviews.apache.org/r/70757/diff/10/
Changes: https://reviews.apache.org/r/70757/diff/9-10/
Testing
-------
Thanks,
Jacob Janco