You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrei Budnik <ab...@mesosphere.com> on 2018/08/06 13:39:22 UTC
Re: Review Request 68021: Added `linux/seccomp` isolator.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68021/
-----------------------------------------------------------
(Updated Aug. 6, 2018, 1:39 p.m.)
Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
Bugs: MESOS-9035
https://issues.apache.org/jira/browse/MESOS-9035
Repository: mesos
Description
-------
This patch introduces `linux/seccomp` isolator which is used for
preparing `ContainerSeccompProfile` for the Mesos containerizer
launcher. If the `ContainerConfig` message has an info about Seccomp
profile name, then this info will be used to locate a Seccomp profile.
The given Seccomp profile is parsed and the resulting
`ContainerSeccompProfile` is stored in the `ContainerLaunchInfo`
message.
Diffs
-----
src/CMakeLists.txt 10b0946d6f49c7e9c201bad6f9f1b41cc8460fe5
src/Makefile.am ecb95ef152fd37e5e83c4f6415489f4cf3959578
src/slave/containerizer/mesos/containerizer.cpp 98129d006cda9b65804b518619b6addc8990410a
src/slave/containerizer/mesos/isolators/linux/seccomp.hpp PRE-CREATION
src/slave/containerizer/mesos/isolators/linux/seccomp.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/68021/diff/2/
Testing
-------
Thanks,
Andrei Budnik
Re: Review Request 68021: Added `linux/seccomp` isolator.
Posted by Andrei Budnik <ab...@mesosphere.com>.
> On Dec. 29, 2018, 1:40 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
> > Lines 89-92 (patched)
> > <https://reviews.apache.org/r/68021/diff/10/?file=2110838#file2110838line89>
> >
> > This is kind of strange to me, I think we do not have this kind of semantics in Mesos before. Can we have a bool field in `LinuxInfo.Seccomp` to explicitly enable/disable Seccomp for a container?
I've added `bool unconfined` flag into `LinuxInfo.Seccomp`.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68021/#review211573
-----------------------------------------------------------
On Nov. 8, 2018, 3:24 p.m., Andrei Budnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68021/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 3:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9035
> https://issues.apache.org/jira/browse/MESOS-9035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch introduces `linux/seccomp` isolator which is used for
> preparing `ContainerSeccompProfile` for the Mesos containerizer
> launcher. If the `ContainerConfig` message has an info about Seccomp
> profile name, then this info will be used to locate a Seccomp profile.
> The given Seccomp profile is parsed and the resulting
> `ContainerSeccompProfile` is stored in the `ContainerLaunchInfo`
> message.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8
> src/Makefile.am 7a4904a3d67479267087fd2313a263d8218843fa
> src/slave/containerizer/mesos/containerizer.cpp a5cf2da55c046c5c45e0c2ca3400f64de12de62b
> src/slave/containerizer/mesos/isolators/linux/seccomp.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/seccomp.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/68021/diff/11/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68021: Added `linux/seccomp` isolator.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68021/#review211573
-----------------------------------------------------------
src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
Lines 89-92 (patched)
<https://reviews.apache.org/r/68021/#comment296921>
This is kind of strange to me, I think we do not have this kind of semantics in Mesos before. Can we have a bool field in `LinuxInfo.Seccomp` to explicitly enable/disable Seccomp for a container?
- Qian Zhang
On Nov. 8, 2018, 11:24 p.m., Andrei Budnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68021/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 11:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9035
> https://issues.apache.org/jira/browse/MESOS-9035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch introduces `linux/seccomp` isolator which is used for
> preparing `ContainerSeccompProfile` for the Mesos containerizer
> launcher. If the `ContainerConfig` message has an info about Seccomp
> profile name, then this info will be used to locate a Seccomp profile.
> The given Seccomp profile is parsed and the resulting
> `ContainerSeccompProfile` is stored in the `ContainerLaunchInfo`
> message.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8
> src/Makefile.am 5da75d3ef263b86af8d914d82cae889830097042
> src/slave/containerizer/mesos/containerizer.cpp a5cf2da55c046c5c45e0c2ca3400f64de12de62b
> src/slave/containerizer/mesos/isolators/linux/seccomp.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/seccomp.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/68021/diff/10/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68021: Added `linux/seccomp` isolator.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68021/#review211541
-----------------------------------------------------------
Fix it, then Ship it!
src/slave/containerizer/mesos/containerizer.cpp
Lines 125-127 (patched)
<https://reviews.apache.org/r/68021/#comment296867>
I think this should be moved to line 112.
src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
Lines 24 (patched)
<https://reviews.apache.org/r/68021/#comment296868>
This should be moved to line 21.
- Qian Zhang
On Nov. 8, 2018, 11:24 p.m., Andrei Budnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68021/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 11:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9035
> https://issues.apache.org/jira/browse/MESOS-9035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch introduces `linux/seccomp` isolator which is used for
> preparing `ContainerSeccompProfile` for the Mesos containerizer
> launcher. If the `ContainerConfig` message has an info about Seccomp
> profile name, then this info will be used to locate a Seccomp profile.
> The given Seccomp profile is parsed and the resulting
> `ContainerSeccompProfile` is stored in the `ContainerLaunchInfo`
> message.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8
> src/Makefile.am 5da75d3ef263b86af8d914d82cae889830097042
> src/slave/containerizer/mesos/containerizer.cpp a5cf2da55c046c5c45e0c2ca3400f64de12de62b
> src/slave/containerizer/mesos/isolators/linux/seccomp.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/seccomp.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/68021/diff/10/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68021: Added `linux/seccomp` isolator.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68021/#review212370
-----------------------------------------------------------
Ship it!
Ship It!
- Gilbert Song
On Nov. 8, 2018, 7:24 a.m., Andrei Budnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68021/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 7:24 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9035
> https://issues.apache.org/jira/browse/MESOS-9035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch introduces `linux/seccomp` isolator which is used for
> preparing `ContainerSeccompProfile` for the Mesos containerizer
> launcher. If the `ContainerConfig` message has an info about Seccomp
> profile name, then this info will be used to locate a Seccomp profile.
> The given Seccomp profile is parsed and the resulting
> `ContainerSeccompProfile` is stored in the `ContainerLaunchInfo`
> message.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am cd785255fcdf1302a8f9fa358039e5d1f200e132
> src/slave/containerizer/mesos/containerizer.cpp 5016f2e9f0651abcb0a5f364e8eace458f2edeae
> src/slave/containerizer/mesos/isolators/linux/seccomp.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/seccomp.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/68021/diff/16/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68021: Added `linux/seccomp` isolator.
Posted by Qian Zhang <zh...@gmail.com>.
> On Jan. 15, 2019, 11:02 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
> > Lines 17-18 (patched)
> > <https://reviews.apache.org/r/68021/diff/12/?file=2117411#file2117411line17>
> >
> > A newline between.
>
> Andrei Budnik wrote:
> https://github.com/apache/mesos/blob/2aaf96ecbab316708afb401e43cad2f2f692f687/src/slave/containerizer/mesos/isolators/xfs/utils.cpp#L35-L38
Hmm, I think that is not correct. Our guideline should be headers are partitioned by their subfolders (https://github.com/apache/mesos/blob/master/docs/c++-style-guide.md#order-of-includes ), here are a few examples:
https://github.com/apache/mesos/blob/1.7.0/src/linux/routing/link/link.cpp#L21:L29
https://github.com/apache/mesos/blob/1.7.0/src/linux/capabilities.cpp#L19:L21
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68021/#review211985
-----------------------------------------------------------
On Nov. 8, 2018, 11:24 p.m., Andrei Budnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68021/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 11:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9035
> https://issues.apache.org/jira/browse/MESOS-9035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch introduces `linux/seccomp` isolator which is used for
> preparing `ContainerSeccompProfile` for the Mesos containerizer
> launcher. If the `ContainerConfig` message has an info about Seccomp
> profile name, then this info will be used to locate a Seccomp profile.
> The given Seccomp profile is parsed and the resulting
> `ContainerSeccompProfile` is stored in the `ContainerLaunchInfo`
> message.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am cd785255fcdf1302a8f9fa358039e5d1f200e132
> src/slave/containerizer/mesos/containerizer.cpp 5016f2e9f0651abcb0a5f364e8eace458f2edeae
> src/slave/containerizer/mesos/isolators/linux/seccomp.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/seccomp.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/68021/diff/13/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68021: Added `linux/seccomp` isolator.
Posted by Andrei Budnik <ab...@mesosphere.com>.
> On Jan. 15, 2019, 3:02 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
> > Lines 17-18 (patched)
> > <https://reviews.apache.org/r/68021/diff/12/?file=2117411#file2117411line17>
> >
> > A newline between.
https://github.com/apache/mesos/blob/2aaf96ecbab316708afb401e43cad2f2f692f687/src/slave/containerizer/mesos/isolators/xfs/utils.cpp#L35-L38
> On Jan. 15, 2019, 3:02 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
> > Lines 61 (patched)
> > <https://reviews.apache.org/r/68021/diff/12/?file=2117411#file2117411line61>
> >
> > Can we have a more explicit error message here? Like `The default seccomp profile is invalid: ...`.
Currenly, the error message is very long and pretty descriptive:
```
E0115 10:23:51.804989 2491 main.cpp:484] EXIT with status 1: Failed to create a containerizer: Could not create MesosContainerizer: Failed to create isolator 'linux/seccomp': Failed to read Seccomp profile file '/home/abudnik/default.json': No such file or directory
```
> On Jan. 15, 2019, 3:02 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
> > Lines 97 (patched)
> > <https://reviews.apache.org/r/68021/diff/12/?file=2117411#file2117411line97>
> >
> > Missing Seccomp profile name for container xxx.
I don't think we need to mention `containerId`. Other isolators don't specify `containerId` in failure messages.
We print `containerId` in `Http::_launchContainer` (and in `Slave::executorLaunched`) on failures.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68021/#review211985
-----------------------------------------------------------
On Nov. 8, 2018, 3:24 p.m., Andrei Budnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68021/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 3:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9035
> https://issues.apache.org/jira/browse/MESOS-9035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch introduces `linux/seccomp` isolator which is used for
> preparing `ContainerSeccompProfile` for the Mesos containerizer
> launcher. If the `ContainerConfig` message has an info about Seccomp
> profile name, then this info will be used to locate a Seccomp profile.
> The given Seccomp profile is parsed and the resulting
> `ContainerSeccompProfile` is stored in the `ContainerLaunchInfo`
> message.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am cd785255fcdf1302a8f9fa358039e5d1f200e132
> src/slave/containerizer/mesos/containerizer.cpp 5016f2e9f0651abcb0a5f364e8eace458f2edeae
> src/slave/containerizer/mesos/isolators/linux/seccomp.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/seccomp.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/68021/diff/13/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68021: Added `linux/seccomp` isolator.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68021/#review211985
-----------------------------------------------------------
Fix it, then Ship it!
src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
Lines 17-18 (patched)
<https://reviews.apache.org/r/68021/#comment297564>
A newline between.
src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
Lines 61 (patched)
<https://reviews.apache.org/r/68021/#comment297567>
Can we have a more explicit error message here? Like `The default seccomp profile is invalid: ...`.
src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
Lines 97 (patched)
<https://reviews.apache.org/r/68021/#comment297568>
Missing Seccomp profile name for container xxx.
src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
Lines 100 (patched)
<https://reviews.apache.org/r/68021/#comment297569>
I do not think we need `Option<std::string>` here.
- Qian Zhang
On Nov. 8, 2018, 11:24 p.m., Andrei Budnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68021/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 11:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9035
> https://issues.apache.org/jira/browse/MESOS-9035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch introduces `linux/seccomp` isolator which is used for
> preparing `ContainerSeccompProfile` for the Mesos containerizer
> launcher. If the `ContainerConfig` message has an info about Seccomp
> profile name, then this info will be used to locate a Seccomp profile.
> The given Seccomp profile is parsed and the resulting
> `ContainerSeccompProfile` is stored in the `ContainerLaunchInfo`
> message.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8
> src/Makefile.am 7a4904a3d67479267087fd2313a263d8218843fa
> src/slave/containerizer/mesos/containerizer.cpp a5cf2da55c046c5c45e0c2ca3400f64de12de62b
> src/slave/containerizer/mesos/isolators/linux/seccomp.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/seccomp.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/68021/diff/12/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68021: Added `linux/seccomp` isolator.
Posted by Andrei Budnik <ab...@mesosphere.com>.
> On Jan. 19, 2019, 1:05 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
> > Lines 49 (patched)
> > <https://reviews.apache.org/r/68021/diff/15/?file=2120370#file2120370line49>
> >
> > equivalent to `if(ret == -1 || EFAULT == errno)`?
https://www.wolframalpha.com/input/?i=!(a+%26+b)
No, it's equivalent to `(ret != -1 || EFAULT != errno)`.
I think Chromium devs disagree with your proposal:
https://chromium.googlesource.com/chromium/src/sandbox/+/refs/heads/master/linux/seccomp-bpf/sandbox_bpf.cc#44
> On Jan. 19, 2019, 1:05 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
> > Lines 62-63 (patched)
> > <https://reviews.apache.org/r/68021/diff/15/?file=2120370#file2120370line62>
> >
> > I would cache it and just parse the default profile once. Let's introduce a private variable:
> >
> > Option<ContainerSeccompProfile> defaultProfile;
Parsing step is very quick. Caching it just makes the code more complicated. Since parsing a profile is not an issue, while caching it adds more complexity, I don't think we need to do that.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68021/#review212160
-----------------------------------------------------------
On Nov. 8, 2018, 3:24 p.m., Andrei Budnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68021/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 3:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9035
> https://issues.apache.org/jira/browse/MESOS-9035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch introduces `linux/seccomp` isolator which is used for
> preparing `ContainerSeccompProfile` for the Mesos containerizer
> launcher. If the `ContainerConfig` message has an info about Seccomp
> profile name, then this info will be used to locate a Seccomp profile.
> The given Seccomp profile is parsed and the resulting
> `ContainerSeccompProfile` is stored in the `ContainerLaunchInfo`
> message.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am cd785255fcdf1302a8f9fa358039e5d1f200e132
> src/slave/containerizer/mesos/containerizer.cpp 5016f2e9f0651abcb0a5f364e8eace458f2edeae
> src/slave/containerizer/mesos/isolators/linux/seccomp.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/seccomp.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/68021/diff/15/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68021: Added `linux/seccomp` isolator.
Posted by Andrei Budnik <ab...@mesosphere.com>.
> On Jan. 19, 2019, 1:05 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
> > Lines 62-63 (patched)
> > <https://reviews.apache.org/r/68021/diff/15/?file=2120370#file2120370line62>
> >
> > I would cache it and just parse the default profile once. Let's introduce a private variable:
> >
> > Option<ContainerSeccompProfile> defaultProfile;
>
> Andrei Budnik wrote:
> Parsing step is very quick. Caching it just makes the code more complicated. Since parsing a profile is not an issue, while caching it adds more complexity, I don't think we need to do that.
> Parsing step is very quick.
My assumption was wrong. Parsing default Seccom profile takes ~7ms without compiler optimization.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68021/#review212160
-----------------------------------------------------------
On Nov. 8, 2018, 3:24 p.m., Andrei Budnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68021/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 3:24 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9035
> https://issues.apache.org/jira/browse/MESOS-9035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch introduces `linux/seccomp` isolator which is used for
> preparing `ContainerSeccompProfile` for the Mesos containerizer
> launcher. If the `ContainerConfig` message has an info about Seccomp
> profile name, then this info will be used to locate a Seccomp profile.
> The given Seccomp profile is parsed and the resulting
> `ContainerSeccompProfile` is stored in the `ContainerLaunchInfo`
> message.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am cd785255fcdf1302a8f9fa358039e5d1f200e132
> src/slave/containerizer/mesos/containerizer.cpp 5016f2e9f0651abcb0a5f364e8eace458f2edeae
> src/slave/containerizer/mesos/isolators/linux/seccomp.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/seccomp.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/68021/diff/16/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68021: Added `linux/seccomp` isolator.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68021/#review212160
-----------------------------------------------------------
Fix it, then Ship it!
src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
Lines 49 (patched)
<https://reviews.apache.org/r/68021/#comment297820>
equivalent to `if(ret == -1 || EFAULT == errno)`?
src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
Lines 62-63 (patched)
<https://reviews.apache.org/r/68021/#comment297824>
I would cache it and just parse the default profile once. Let's introduce a private variable:
Option<ContainerSeccompProfile> defaultProfile;
src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
Lines 95 (patched)
<https://reviews.apache.org/r/68021/#comment297826>
"Missing Seccomp profile name"
src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
Lines 107 (patched)
<https://reviews.apache.org/r/68021/#comment297825>
only parse it if it is the profile from the framework API.
src/slave/containerizer/mesos/isolators/linux/seccomp.cpp
Lines 114 (patched)
<https://reviews.apache.org/r/68021/#comment297828>
kill newline
- Gilbert Song
On Nov. 8, 2018, 7:24 a.m., Andrei Budnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68021/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2018, 7:24 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
>
>
> Bugs: MESOS-9035
> https://issues.apache.org/jira/browse/MESOS-9035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch introduces `linux/seccomp` isolator which is used for
> preparing `ContainerSeccompProfile` for the Mesos containerizer
> launcher. If the `ContainerConfig` message has an info about Seccomp
> profile name, then this info will be used to locate a Seccomp profile.
> The given Seccomp profile is parsed and the resulting
> `ContainerSeccompProfile` is stored in the `ContainerLaunchInfo`
> message.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf
> src/Makefile.am cd785255fcdf1302a8f9fa358039e5d1f200e132
> src/slave/containerizer/mesos/containerizer.cpp 5016f2e9f0651abcb0a5f364e8eace458f2edeae
> src/slave/containerizer/mesos/isolators/linux/seccomp.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/linux/seccomp.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/68021/diff/15/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 68021: Added `linux/seccomp` isolator.
Posted by Andrei Budnik <ab...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68021/
-----------------------------------------------------------
(Updated Nov. 8, 2018, 3:24 p.m.)
Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
Bugs: MESOS-9035
https://issues.apache.org/jira/browse/MESOS-9035
Repository: mesos
Description
-------
This patch introduces `linux/seccomp` isolator which is used for
preparing `ContainerSeccompProfile` for the Mesos containerizer
launcher. If the `ContainerConfig` message has an info about Seccomp
profile name, then this info will be used to locate a Seccomp profile.
The given Seccomp profile is parsed and the resulting
`ContainerSeccompProfile` is stored in the `ContainerLaunchInfo`
message.
Diffs (updated)
-----
src/CMakeLists.txt 732368293049b7d9d6f62057344d433637ad44e8
src/Makefile.am c17eae4ff1d019d515f67d81821e933ecb5dc190
src/slave/containerizer/mesos/containerizer.cpp 03a4e0f1567b27b2efd8a443caea3a2a087d858c
src/slave/containerizer/mesos/isolators/linux/seccomp.hpp PRE-CREATION
src/slave/containerizer/mesos/isolators/linux/seccomp.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/68021/diff/5/
Changes: https://reviews.apache.org/r/68021/diff/4-5/
Testing
-------
Thanks,
Andrei Budnik