Re: Review Request 69211: Improved the code comments for `getContainerDevicesPath`.

[Mesos Reviewbot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69211/#review210432
---



Patch looks great!

Reviews applied: [69086, 69210, 69211]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Oct. 30, 2018, 9:04 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69211/
> ---
> 
> (Updated Oct. 30, 2018, 9:04 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-9319
> https://issues.apache.org/jira/browse/MESOS-9319
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved the code comments for `getContainerDevicesPath`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/paths.hpp 
> de3981db7eb08e53901547037c947f594c8d46ab 
> 
> 
> Diff: https://reviews.apache.org/r/69211/diff/7/
> 
> 
> Testing
> ---
> 
> make check (Fedora 28)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 69303: Implemented an optional time-to-live on subscriber connections.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69303/#review210431
---



PASS: Mesos patch 69303 was successfully built and tested.

Reviews applied: `['69302', '69303']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2588/mesos-review-69303

- Mesos Reviewbot Windows


On Nov. 9, 2018, 1:57 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69303/
> ---
> 
> (Updated Nov. 9, 2018, 1:57 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Greg Mann.
> 
> 
> Bugs: MESOS-9258
> https://issues.apache.org/jira/browse/MESOS-9258
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a subscriber to the master's event stream passes in an optional
> argument, the master will set a timer to close the streaming connection
> after the specified time.  This is one workaround for managing potential
> connection leakage from having subscribers that do not close connections
> (or proxies not closing connections).
> 
> TODO: Due to how the disconnection is implemented, the logs will always show:
> ```
> master.cpp:12110] Removed subscriber  from the list of active 
> subscribers
> master.cpp:12106] Unknown subscriber  disconnected
> ```
> But otherwise works as expected.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 75ab6ea422a77e25049cd6afe007507d11650a06 
>   src/master/master.hpp e77babf22126838c63cd05e483875c9beb3ac5ff 
>   src/master/master.cpp 1e326ec42a7f79a0835529a4655e7ec272f1cf40 
>   src/master/validation.cpp 5768ac8fe802f28855fbd7be135c75532771 
>   src/tests/api_tests.cpp fdd9f871f75617fc26a28679e2a1e41f506c6133 
> 
> 
> Diff: https://reviews.apache.org/r/69303/diff/1/
> 
> 
> Testing
> ---
> 
> ```
> make check
> 
> src/mesos-tests --gtest_filter="*SubscribeMaxLifetime*" --gtest_repeat=-1 
> --gtest_break_on_failure
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 69301: Fixed the ulimit validation in the parallel test runner.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69301/#review210430
---



PASS: Mesos patch 69301 was successfully built and tested.

Reviews applied: `['69301']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2586/mesos-review-69301

- Mesos Reviewbot Windows


On Nov. 8, 2018, 5 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69301/
> ---
> 
> (Updated Nov. 8, 2018, 5 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the ulimit validation in the parallel test runner.
> 
> 
> Diffs
> -
> 
>   support/mesos-gtest-runner.py d2a57eef62421ba93d48959839ea2ac84adb9f8c 
> 
> 
> Diff: https://reviews.apache.org/r/69301/diff/1/
> 
> 
> Testing
> ---
> 
> support/mesos-gtest-runner.py -j5 build/bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69211: Improved the code comments for `getContainerDevicesPath`.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69211/#review210429
---



PASS: Mesos patch 69211 was successfully built and tested.

Reviews applied: `['69086', '69210', '69211']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2587/mesos-review-69211

- Mesos Reviewbot Windows


On Oct. 30, 2018, 9:04 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69211/
> ---
> 
> (Updated Oct. 30, 2018, 9:04 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-9319
> https://issues.apache.org/jira/browse/MESOS-9319
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved the code comments for `getContainerDevicesPath`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/paths.hpp 
> de3981db7eb08e53901547037c947f594c8d46ab 
> 
> 
> Diff: https://reviews.apache.org/r/69211/diff/7/
> 
> 
> Testing
> ---
> 
> make check (Fedora 28)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 69302: Added optional argument to Master operator SUBSCRIBE call.

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69302/
---

Review request for mesos, Gastón Kleiman and Greg Mann.


Bugs: MESOS-9258
https://issues.apache.org/jira/browse/MESOS-9258


Repository: mesos


Description
---

This introduces a workaround for cases where clients connect to the
master API via a proxy or load balancer.  When the proxy/load-balancer
does not close connections, the master will continue to send data
through the connection; potentially causing performance degredation
on the master.

This new argument will tell the master to close the stream after some
time has elapsed.  For the client, this disruption should be minimal
because clients should generally account for random disconnections.


Diffs
-

  include/mesos/master/master.proto 80bd7c43cbb6d7df0dbd1c162cf4dda48f0e2e30 
  include/mesos/v1/master/master.proto cd41a7e8dc98753b5d2b8f93d92114fe97463636 


Diff: https://reviews.apache.org/r/69302/diff/1/


Testing
---

See last review in chain.


Thanks,

Joseph Wu

Review Request 69303: Implemented an optional time-to-live on subscriber connections.

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69303/
---

Review request for mesos, Gastón Kleiman and Greg Mann.


Bugs: MESOS-9258
https://issues.apache.org/jira/browse/MESOS-9258


Repository: mesos


Description
---

When a subscriber to the master's event stream passes in an optional
argument, the master will set a timer to close the streaming connection
after the specified time.  This is one workaround for managing potential
connection leakage from having subscribers that do not close connections
(or proxies not closing connections).

TODO: Due to how the disconnection is implemented, the logs will always show:
```
master.cpp:12110] Removed subscriber  from the list of active subscribers
master.cpp:12106] Unknown subscriber  disconnected
```
But otherwise works as expected.


Diffs
-

  src/master/http.cpp 75ab6ea422a77e25049cd6afe007507d11650a06 
  src/master/master.hpp e77babf22126838c63cd05e483875c9beb3ac5ff 
  src/master/master.cpp 1e326ec42a7f79a0835529a4655e7ec272f1cf40 
  src/master/validation.cpp 5768ac8fe802f28855fbd7be135c75532771 
  src/tests/api_tests.cpp fdd9f871f75617fc26a28679e2a1e41f506c6133 


Diff: https://reviews.apache.org/r/69303/diff/1/


Testing
---

```
make check

src/mesos-tests --gtest_filter="*SubscribeMaxLifetime*" --gtest_repeat=-1 
--gtest_break_on_failure
```


Thanks,

Joseph Wu

Review Request 69301: Fixed the ulimit validation in the parallel test runner.

[Chun-Hung Hsiao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69301/
---

Review request for mesos, Benjamin Bannier and Benjamin Mahler.


Repository: mesos


Description
---

Fixed the ulimit validation in the parallel test runner.


Diffs
-

  support/mesos-gtest-runner.py d2a57eef62421ba93d48959839ea2ac84adb9f8c 


Diff: https://reviews.apache.org/r/69301/diff/1/


Testing
---

support/mesos-gtest-runner.py -j5 build/bin/mesos-tests.sh


Thanks,

Chun-Hung Hsiao

Re: Review Request 69086: Moved container root construction to the isolators.

[James Peach]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69086/
---

(Updated Nov. 9, 2018, 12:53 a.m.)


Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Jiang Yan Xu.


Bugs: MESOS-9319
https://issues.apache.org/jira/browse/MESOS-9319


Repository: mesos


Description
---

Previously, if the container was configured with a root filesytem,
the root was populated by a combination of the `fs::chroot:prepare`
API and the various isolators. The implementation details of some
isolators had leaked into the chroot code, which had a special case
for adding GPU devices.

This change moves all the responsibility for defining the
root filesystem from the `fs::chroot::prepare()` API to the
`filesystem/linux` isolator. The `filesystem/linux` isolator is
now the single place that captures how to mount the container
pseudo-filesystems as well as how to construct a proper `/dev`
directory.

Since the `linux/filesystem` isolator is now entirely responsible
for creating and mounting the container `/dev`, any other isolators
that enable access to devices should populate device nodes in the
container devices directory and add a corresponding bind mount.


Diffs (updated)
-

  src/linux/fs.hpp 31969f6ba82bf5ee549bfdf9698a21adaa486a90 
  src/linux/fs.cpp 5cdffe1f4c7f00aee5b8f640e7cfa4a0018cfa0a 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
c7d753ac2e5575a8d687600bfb9e0617fa72c990 
  src/slave/containerizer/mesos/isolators/gpu/isolator.hpp 
4645c625877d9451516133b24bd3959e0f49c0a9 
  src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
56d835779618fd965d928c6926664583e9141f79 
  src/slave/containerizer/mesos/isolators/linux/devices.cpp 
8f8ff95ec3856ba06647637a80315365d0e66e23 
  src/slave/containerizer/mesos/launch.cpp 
882bcdf89e2b0cca3d3f62e6d017849a51ceaead 


Diff: https://reviews.apache.org/r/69086/diff/9/

Changes: https://reviews.apache.org/r/69086/diff/8-9/


Testing
---

sudo make check (Fedora 28)


Thanks,

James Peach

Re: Review Request 69086: Moved container root construction to the isolators.

[James Peach]

> On Nov. 3, 2018, 6:49 p.m., James Peach wrote:
> > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
> > Lines 146 (patched)
> > 
> >
> > This mount fails with user namespaces:
> > ```
> > Failed to prepare mounts: Failed to mount 
> > '{"flags":15,"source":"sysfs","target":"/srv/mesos/work/provisioner/containers/ac807664-4109-409e-8cfb-a4285009598b/backends/overlay/rootfses/2b851cdf-08e2-456f-99bd-f5ff5011c678/sys","type":"sysfs"}':
> >  Operation not permitted
> > ```
> > 
> > I was carrying an out of tree hack for that.

I'll continue to carry an out of tree hack for this.


- James


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69086/#review210313
---


On Oct. 30, 2018, 9:03 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69086/
> ---
> 
> (Updated Oct. 30, 2018, 9:03 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-9319
> https://issues.apache.org/jira/browse/MESOS-9319
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, if the container was configured with a root filesytem,
> the root was populated by a combination of the `fs::chroot:prepare`
> API and the various isolators. The implementation details of some
> isolators had leaked into the chroot code, which had a special case
> for adding GPU devices.
> 
> This change moves all the responsibility for defining the
> root filesystem from the `fs::chroot::prepare()` API to the
> `filesystem/linux` isolator. The `filesystem/linux` isolator is
> now the single place that captures how to mount the container
> pseudo-filesystems as well as how to construct a proper `/dev`
> directory.
> 
> Since the `linux/filesystem` isolator is now entirely responsible
> for creating and mounting the container `/dev`, any other isolators
> that enable access to devices should populate device nodes in the
> container devices directory and add a corresponding bind mount.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp 31969f6ba82bf5ee549bfdf9698a21adaa486a90 
>   src/linux/fs.cpp 5cdffe1f4c7f00aee5b8f640e7cfa4a0018cfa0a 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> c7d753ac2e5575a8d687600bfb9e0617fa72c990 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.hpp 
> 4645c625877d9451516133b24bd3959e0f49c0a9 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
> 56d835779618fd965d928c6926664583e9141f79 
>   src/slave/containerizer/mesos/isolators/linux/devices.cpp 
> 8f8ff95ec3856ba06647637a80315365d0e66e23 
>   src/slave/containerizer/mesos/launch.cpp 
> 7193da0a094df3e441e185c62b3a0379a0bdc4a2 
> 
> 
> Diff: https://reviews.apache.org/r/69086/diff/8/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 28)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 68804: Added Stout `os::readlink` function for POSIX paths.

[James Peach]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68804/#review210424
---



Can you please add a simple test case? Just update `FsTest.SYMLINK_Symlink` to 
read back the symlink is fine.

3rdparty/stout/tests/os/filesystem_tests.cpp


3rdparty/stout/include/stout/os/posix/readlink.hpp
Lines 24 (patched)


For consistency, we should have a Windows version that does:
```
Try readlink(const std::string& path) = delete;
```



3rdparty/stout/include/stout/os/posix/readlink.hpp
Lines 27 (patched)


To detect truncation we should do something like this:
```
char temp[PATH_MAX + 1];
ssize_t len = ::readlink(path.c_str(), temp, sizeof(temp));
if (len == -1) {
  return ErrnoError();
}

// Name is truncated and not NUL-terminated.
if (len >= sizeof(temp)) {
  return ErrnoError(ENAMETOOLONG);
}
  
return std::string(temp);
```


- James Peach


On Nov. 7, 2018, 9:22 p.m., Jason Lai wrote:
> 
> ---
> 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.
> 
> 
> 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/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Lai
> 
>

Re: Review Request 69287: Added test cases for the `volume/host_path` isolator.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69287/#review210398
---



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['65811', '68832', '69286', '69287']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2581/mesos-review-69287

Relevant logs:

- 
[mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2581/mesos-review-69287/logs/mesos-tests.log):

```
W1108 03:00:42.907641 16308 slave.cpp:3923] Ignoring shutdown framework 
6239b51c-626f-4507-91f7-3501385616c0- because it is terminating
I1108 03:00:42.910627 13708 master.cpp:1273] Agent 
6239b51c-626f-4507-91f7-3501385616c0-S0 at slave(461)@192.10.1.5:50763 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net) disconnected
I1108 03:00:42.910627 13708 master.cpp:3289] Disconnecting agent 
6239b51c-626f-4507-91f7-3501385616c0-S0 at slave(461)@192.10.1.5:50763 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I1108 03:00:42.910627 13708 master.cpp:3308] Deactivating agent 
6239b51c-626f-4507-91f7-3501385616c0-S0 at slave(461)@192.10.1.5:50763 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I1108 03:00:42.911661 10460 hierarchical.cpp:357] Removed framework 
6239b51c-626f-4507-91f7-3501385616c0-
I1108 03:00:42.911661 10460 hierarchical.cpp:801] Agent 
6239b51c-626f-4507-91f7-3501385616c0-S0 deactivated
I1108 03:00:42.912629 24040 containerizer.cpp:2455] Destroying container 
9d9b21d8-927a-4da9-ac87-47535632c8eb in RUNNING state
I1108 03:00:42.912629 24040 containerizer.cpp:3122] Transitioning the state of 
container 9d9b21d8-927a-4da9-ac87-47535632c8eb from RUNNING to DESTROYING
I1108 03:00:42.912629 24040 launcher.cpp:166] Asked to destroy container 
9d9b21d8-927a-4da9-ac87-47535632c8eb
W[   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (683 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (704 ms total)

[--] Global test environment tear-down
[==] 1053 tests from 103 test cases ran. (506347 ms total)
[  PASSED  ] 1052 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage

 1 FAILED TEST
  YOU HAVE 231 DISABLED TESTS

1108 03:00:42.913674 21184 process.cpp:1423] Failed to recv on socket 
WindowsFD::Type::SOCKET=1936 to peer '192.10.1.5:52761': IO failed with error 
code: The specified network name is no longer available.

W1108 03:00:42.914630 21184 process.cpp:838] Failed to recv on socket 
WindowsFD::Type::SOCKET=2572 to peer '192.10.1.5:52762': IO failed with error 
code: The specified network name is no longer available.

I1108 03:00:43.002650 22272 containerizer.cpp:2961] Container 
9d9b21d8-927a-4da9-ac87-47535632c8eb has exited
I1108 03:00:43.033638 24056 master.cpp:1115] Master terminating
I1108 03:00:43.035632 10460 hierarchical.cpp:643] Removed agent 
6239b51c-626f-4507-91f7-3501385616c0-S0
I1108 03:00:43.422701 21184 process.cpp:927] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Nov. 7, 2018, 11: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, 11: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 68018: Added `SeccompFilter` class.

[Andrei Budnik]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68018/
---

(Updated Nov. 8, 2018, 3:24 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.


Bugs: MESOS-9034
https://issues.apache.org/jira/browse/MESOS-9034


Repository: mesos


Description
---

`SeccompFilter` class is a wrapper for `libseccomp` API. Its main
purpose is to provide a translation of the `ContainerSeccompProfile`
message into calls of `libseccomp` API.


Diffs (updated)
-

  src/CMakeLists.txt 732368293049b7d9d6f62057344d433637ad44e8 
  src/Makefile.am c17eae4ff1d019d515f67d81821e933ecb5dc190 
  src/linux/seccomp/seccomp.hpp PRE-CREATION 
  src/linux/seccomp/seccomp.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/68018/diff/4/

Changes: https://reviews.apache.org/r/68018/diff/3-4/


Testing
---


Thanks,

Andrei Budnik

Re: Review Request 68017: Added Seccomp-related protobuf messages.

[Andrei Budnik]

> On Oct. 22, 2018, 5:18 p.m., James Peach wrote:
> > include/mesos/mesos.proto
> > Lines 3085 (patched)
> > 
> >
> > I'm not sure we should have a default here. The default seccomp profile 
> > should probably be defined by te operator, which implies an agent flag 
> > (which could have a default value). Specifying a default in the protobuf 
> > seems likely to cause interoperatbility problems.

I've removed default value here and added `--seccomp_profile_name` agent flag.


- Andrei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/#review209886
---


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/68017/
> ---
> 
> (Updated Nov. 8, 2018, 3:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 06a901d26693757edc653cd833d55aa42e4ff2c6 
>   include/mesos/slave/containerizer.proto 
> 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9 
> 
> 
> Diff: https://reviews.apache.org/r/68017/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 69286: Allowed creating non-existing host paths for Mesos Containerizer.

[Gilbert Song]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69286/#review210388
---




src/slave/containerizer/mesos/isolators/volume/host_path.hpp
Lines 46 (patched)


s/hostPathWhitelist/forceCreatedHostPaths/g?



src/slave/containerizer/mesos/isolators/volume/host_path.cpp
Lines 56-57 (patched)


do we still need them if we remove the volume namespace?



src/slave/containerizer/mesos/isolators/volume/host_path.cpp
Lines 86 (patched)


I would move the parse() to create()



src/slave/containerizer/mesos/isolators/volume/utils.cpp
Lines 31 (patched)


probably we do not need this method?



src/slave/flags.cpp
Lines 657 (patched)


if directories do not exist



src/slave/flags.cpp
Lines 658-660 (patched)


Remove these lines?


- Gilbert Song


On Nov. 7, 2018, 2: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, 2: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 69293: Disabled parallel test execution for reviewbot.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69293/#review210400
---



PASS: Mesos patch 69293 was successfully built and tested.

Reviews applied: `['69293']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2582/mesos-review-69293

- Mesos Reviewbot Windows


On Nov. 7, 2018, 2:57 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69293/
> ---
> 
> (Updated Nov. 7, 2018, 2:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently no dedicated handling of XML reporting from multiple gtest
> shards is implemented in our gtest runner.
> 
> This patch disables the now default-enabled parallel test runner for
> any build & tests cycles under the ASF CI reviewbot job.
> 
> 
> Diffs
> -
> 
>   support/jenkins/reviewbot.sh 0dd81fd1c067dfe84c4e638ebcee87bc6d4d73a7 
> 
> 
> Diff: https://reviews.apache.org/r/69293/diff/1/
> 
> 
> Testing
> ---
> 
> NOTE: The flags for `Mesos-Buildbot` are configured directly in Jenkins and 
> already adjusted.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 68020: Added Seccomp-related flags to the agent.

[Andrei Budnik]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68020/
---

(Updated Nov. 8, 2018, 3:24 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.


Summary (updated)
-

Added Seccomp-related flags to the agent.


Repository: mesos


Description (updated)
---

`--seccomp_config_dir` and `--seccomp_profile_name` flags have been
added to the agent. These flags are used by the `linux/seccomp`
isolator to specify the path of the directory containing Seccomp
profiles and the path of the default Seccomp profile.


Diffs (updated)
-

  src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
  src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 


Diff: https://reviews.apache.org/r/68020/diff/4/

Changes: https://reviews.apache.org/r/68020/diff/3-4/


Testing
---


Thanks,

Andrei Budnik

Re: Review Request 68016: Added libseccomp to the build.

[Andrei Budnik]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68016/
---

(Updated Nov. 8, 2018, 3:23 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Gilbert Song, Jie Yu, James 
Peach, and Qian Zhang.


Bugs: MESOS-9032
https://issues.apache.org/jira/browse/MESOS-9032


Repository: mesos


Description
---

This library is needed to implement Seccomp syscall filtering in the
Mesos containerizer. This patch introduces `seccomp-isolator` build
flag, which is used to include or exclude sources related to Seccomp
from the build. Since Seccomp is a Linux-specific feature, the flag
is disabled by default. Enabling `seccomp-isolator` means either:

1. Compiling and linking against the bundled version of libseccomp from
   sources (default).

2. Linking against the libseccomp installed in the OS,
   if `--with-libseccomp` build flag is provided.


Diffs (updated)
-

  3rdparty/CMakeLists.txt fb80b39d1717948e87256272bc5989f73d4f5995 
  3rdparty/Makefile.am e625e7be1743348d02c6dbb8e0a92d1a395b0ef4 
  3rdparty/cmake/Versions.cmake 69fc594ec5ba2887b20b88ec0767a5d801411411 
  3rdparty/versions.am 99ef92087f6958d83ba415e84db5cbbb0c597573 
  cmake/CompilationConfigure.cmake 2130c9b70eae62948d469b37692ff4eea3a4baf2 
  configure.ac 1caab4d0630821a937d92f3c788a01ae3b5dc228 
  src/CMakeLists.txt 732368293049b7d9d6f62057344d433637ad44e8 
  src/Makefile.am c17eae4ff1d019d515f67d81821e933ecb5dc190 


Diff: https://reviews.apache.org/r/68016/diff/6/

Changes: https://reviews.apache.org/r/68016/diff/5-6/


Testing
---


Thanks,

Andrei Budnik

Re: Review Request 69267: Fixed flaky SchedulerTest.MasterFailover.

[Alexander Rukletsov]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69267/#review210396
---


Ship it!




Ship It!

- Alexander Rukletsov


On Nov. 7, 2018, 1:26 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69267/
> ---
> 
> (Updated Nov. 7, 2018, 1:26 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Greg Mann.
> 
> 
> Bugs: MESOS-6949
> https://issues.apache.org/jira/browse/MESOS-6949
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test was flaky because there is a double-master-detection race
> after the master fails over.  This test uses the Standalone master
> detector, which keeps a single Master PID in memory and always returns
> that one PID as the leader.  This means there is almost no delay
> between failing over the master and detecting a new leader.
> 
> The scheduler in this test tries to send a SUBSCRIBE call to the master
> as soon as the master is detected.  Normally, there will only be two
> total SUBSCRIBE calls during the test, before and after the master
> failover.  However, the test also manually appoints the leader after
> failing over the master.  This step races against the scheduler's own
> retry logic, and can potentially cause a third SUBSCRIBE if the second
> SUBSCRIBE has already started.
> 
> Because the scheduler in this test does not enable checkpointing, the
> third SUBSCRIBE will actively disconnect the framework, causing the
> master to remove the framework.  This removal also prevents the
> framework from ever registering again, and thereby times out the test.
> 
> This fixes the test to prevent excess master detection events.
> 
> We could also change the HTTP scheduler driver to ignore these extra
> master detection events when the master in question has not changed.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 0ee5b77e5a667e37ac13553e15f634b2cb19ea65 
> 
> 
> Diff: https://reviews.apache.org/r/69267/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> GLOG_v=1 src/mesos-tests --gtest_filter="*SchedulerTest.MasterFailover*" 
> --gtest_repeat=-1 --gtest_break_on_failure --verbose
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 68017: Added Seccomp-related protobuf messages.

[Andrei Budnik]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68017/
---

(Updated Nov. 8, 2018, 3:24 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.


Bugs: MESOS-9033
https://issues.apache.org/jira/browse/MESOS-9033


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/mesos.proto 06a901d26693757edc653cd833d55aa42e4ff2c6 
  include/mesos/slave/containerizer.proto 
5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9 


Diff: https://reviews.apache.org/r/68017/diff/6/

Changes: https://reviews.apache.org/r/68017/diff/5-6/


Testing
---


Thanks,

Andrei Budnik

Re: Review Request 69267: Fixed flaky SchedulerTest.MasterFailover.

[Mesos Reviewbot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69267/#review210414
---



Patch looks great!

Reviews applied: [69267]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Nov. 7, 2018, 1:26 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69267/
> ---
> 
> (Updated Nov. 7, 2018, 1:26 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Greg Mann.
> 
> 
> Bugs: MESOS-6949
> https://issues.apache.org/jira/browse/MESOS-6949
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test was flaky because there is a double-master-detection race
> after the master fails over.  This test uses the Standalone master
> detector, which keeps a single Master PID in memory and always returns
> that one PID as the leader.  This means there is almost no delay
> between failing over the master and detecting a new leader.
> 
> The scheduler in this test tries to send a SUBSCRIBE call to the master
> as soon as the master is detected.  Normally, there will only be two
> total SUBSCRIBE calls during the test, before and after the master
> failover.  However, the test also manually appoints the leader after
> failing over the master.  This step races against the scheduler's own
> retry logic, and can potentially cause a third SUBSCRIBE if the second
> SUBSCRIBE has already started.
> 
> Because the scheduler in this test does not enable checkpointing, the
> third SUBSCRIBE will actively disconnect the framework, causing the
> master to remove the framework.  This removal also prevents the
> framework from ever registering again, and thereby times out the test.
> 
> This fixes the test to prevent excess master detection events.
> 
> We could also change the HTTP scheduler driver to ignore these extra
> master detection events when the master in question has not changed.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 0ee5b77e5a667e37ac13553e15f634b2cb19ea65 
> 
> 
> Diff: https://reviews.apache.org/r/69267/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> GLOG_v=1 src/mesos-tests --gtest_filter="*SchedulerTest.MasterFailover*" 
> --gtest_repeat=-1 --gtest_break_on_failure --verbose
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 68832: Add unit tests for Stout `path::normalize` function in POSIX.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68832/#review210397
---



PASS: Mesos patch 68832 was successfully built and tested.

Reviews applied: `['65811', '68832']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2580/mesos-review-68832

- Mesos Reviewbot Windows


On Nov. 7, 2018, 10:29 p.m., Jason Lai wrote:
> 
> ---
> 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.
> 
> 
> 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 69235: Added a test `ROOT_UNPRIVILEGED_USER_DefaultExecutorCommandHealthCheck`.

[Gilbert Song]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69235/#review210404
---


Ship it!




Ship It!

- Gilbert Song


On Nov. 1, 2018, 2:28 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69235/
> ---
> 
> (Updated Nov. 1, 2018, 2:28 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-9332
> https://issues.apache.org/jira/browse/MESOS-9332
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `ROOT_UNPRIVILEGED_USER_DefaultExecutorCommandHealthCheck`.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp da64dc2b4de6e1526628522af34ee83b8ef9f469 
> 
> 
> Diff: https://reviews.apache.org/r/69235/diff/1/
> 
> 
> Testing
> ---
> 
> Ran this test repeatedly, all iterations succeeded.
> 
> This test will fail without its previous patch 
> https://reviews.apache.org/r/69234/.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 69274: Reduced default oversubscription in parallel test runner.

[Mesos Reviewbot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69274/#review210390
---



Patch looks great!

Reviews applied: [69274]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Nov. 7, 2018, 8:30 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69274/
> ---
> 
> (Updated Nov. 7, 2018, 8:30 p.m.)
> 
> 
> Review request for mesos, Armand Grillet and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reduced default oversubscription in parallel test runner.
> 
> 
> Diffs
> -
> 
>   support/mesos-gtest-runner.py 9cf72af246a2fabf2cb57ed42136eeaaa1495f20 
> 
> 
> Diff: https://reviews.apache.org/r/69274/diff/1/
> 
> 
> Testing
> ---
> 
> Executed `mesos-tests` in parallel with default settings.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69290: Added a new scheduler v1 call `unsuppress`.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69290/#review210402
---



PASS: Mesos patch 69290 was successfully built and tested.

Reviews applied: `['69288', '69289', '69290']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2583/mesos-review-69290

- Mesos Reviewbot Windows


On Nov. 8, 2018, 6:29 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69290/
> ---
> 
> (Updated Nov. 8, 2018, 6:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-9028
> https://issues.apache.org/jira/browse/MESOS-9028
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `unsuppress` informs master to resume sending offers to
> the framework without altering any existing filters.
> Master will return 200 OK for the request after processing.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler/scheduler.proto 
> f6a780a7b75878b9e74402a28a25bb868f7ac36f 
>   include/mesos/v1/scheduler/scheduler.proto 
> fcfec5e417463103e98dd6917722b4fde41cac7c 
>   src/master/http.cpp 75ab6ea422a77e25049cd6afe007507d11650a06 
>   src/master/master.hpp e77babf22126838c63cd05e483875c9beb3ac5ff 
>   src/master/master.cpp f88c7c1f03f0de7236aad9e3bf4bfac82e91bc65 
>   src/master/validation.cpp 5768ac8fe802f28855fbd7be135c75532771 
> 
> 
> Diff: https://reviews.apache.org/r/69290/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 68019: Added a parser for the Docker Seccomp config format.

[Andrei Budnik]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68019/
---

(Updated Nov. 8, 2018, 3:24 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.


Bugs: MESOS-9105
https://issues.apache.org/jira/browse/MESOS-9105


Repository: mesos


Description
---

Docker Seccomp config is a JSON file containing Seccomp filtering
rules. This patch introduces a parser for Docker Seccomp config format.
This parser accepts a JSON-string, parses and validates it, then
returns a prepared `ContainerSeccompProfile` message.


Diffs (updated)
-

  src/CMakeLists.txt 732368293049b7d9d6f62057344d433637ad44e8 
  src/Makefile.am c17eae4ff1d019d515f67d81821e933ecb5dc190 
  src/linux/seccomp/seccomp_parser.hpp PRE-CREATION 
  src/linux/seccomp/seccomp_parser.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/68019/diff/4/

Changes: https://reviews.apache.org/r/68019/diff/3-4/


Testing
---


Thanks,

Andrei Budnik

Re: Review Request 68021: Added `linux/seccomp` isolator.

[Andrei Budnik]

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

Re: Review Request 68642: Added `lsof()` into stout.

[Gilbert Song]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68642/#review210405
---


Ship it!




Ship It!

- Gilbert Song


On Oct. 23, 2018, 7:30 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68642/
> ---
> 
> (Updated Oct. 23, 2018, 7:30 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-9152
> https://issues.apache.org/jira/browse/MESOS-9152
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `lsof()` into stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am a9c61fcba253a811494cdcdb0afb3d3a018f4585 
>   3rdparty/stout/include/stout/os.hpp 
> aee041891b7e7ff93a0b1ac31019a7a3d4eae962 
>   3rdparty/stout/include/stout/os/lsof.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/posix/lsof.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/lsof.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68642/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 68645: Updated IO switchboard to use subprocess's `whitelist_fds` parameter.

[Gilbert Song]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68645/#review210410
---


Ship it!




Ship It!

- Gilbert Song


On Oct. 14, 2018, 7:10 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68645/
> ---
> 
> (Updated Oct. 14, 2018, 7:10 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-9164
> https://issues.apache.org/jira/browse/MESOS-9164
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated IO switchboard to use subprocess's `whitelist_fds` parameter.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> e96504dcc843b78600985182156569817e566318 
> 
> 
> Diff: https://reviews.apache.org/r/68645/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 69287: Added test cases for the `volume/host_path` isolator.

[Mesos Reviewbot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69287/#review210415
---



Patch looks great!

Reviews applied: [65811, 68832, 69286, 69287]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


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 69211: Improved the code comments for `getContainerDevicesPath`.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69211/#review210421
---



PASS: Mesos patch 69211 was successfully built and tested.

Reviews applied: `['69086', '69210', '69211']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2585/mesos-review-69211

- Mesos Reviewbot Windows


On Oct. 30, 2018, 9:04 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69211/
> ---
> 
> (Updated Oct. 30, 2018, 9:04 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-9319
> https://issues.apache.org/jira/browse/MESOS-9319
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved the code comments for `getContainerDevicesPath`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/paths.hpp 
> de3981db7eb08e53901547037c947f594c8d46ab 
> 
> 
> Diff: https://reviews.apache.org/r/69211/diff/6/
> 
> 
> Testing
> ---
> 
> make check (Fedora 28)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 69211: Improved the code comments for `getContainerDevicesPath`.

[Mesos Reviewbot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69211/#review210422
---



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos Reviewbot


On Oct. 30, 2018, 2:04 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69211/
> ---
> 
> (Updated Oct. 30, 2018, 2:04 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-9319
> https://issues.apache.org/jira/browse/MESOS-9319
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved the code comments for `getContainerDevicesPath`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/paths.hpp 
> de3981db7eb08e53901547037c947f594c8d46ab 
> 
> 
> Diff: https://reviews.apache.org/r/69211/diff/6/
> 
> 
> Testing
> ---
> 
> make check (Fedora 28)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 69293: Disabled parallel test execution for reviewbot.

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69293/#review210401
---


Ship it!




Ship It!

- Vinod Kone


On Nov. 7, 2018, 10:57 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69293/
> ---
> 
> (Updated Nov. 7, 2018, 10:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently no dedicated handling of XML reporting from multiple gtest
> shards is implemented in our gtest runner.
> 
> This patch disables the now default-enabled parallel test runner for
> any build & tests cycles under the ASF CI reviewbot job.
> 
> 
> Diffs
> -
> 
>   support/jenkins/reviewbot.sh 0dd81fd1c067dfe84c4e638ebcee87bc6d4d73a7 
> 
> 
> Diff: https://reviews.apache.org/r/69293/diff/1/
> 
> 
> Testing
> ---
> 
> NOTE: The flags for `Mesos-Buildbot` are configured directly in Jenkins and 
> already adjusted.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 68022: Enabled Seccomp filter in the containerizer launcher.

[Mesos Reviewbot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68022/#review210419
---



Bad patch!

Reviews applied: [68022, 68021, 68020, 68019, 68018, 68017, 68016, 67844]

Failed command: /usr/bin/python3 support/apply-reviews.py -n -r 67844

Error:
2018-11-08 18:16:55 URL:https://reviews.apache.org/r/67844/diff/raw/ [260/260] 
-> "67844.patch" [1]
error: missing binary patch data for '3rdparty/libseccomp-2.3.3.tar.gz'
error: binary patch does not apply to '3rdparty/libseccomp-2.3.3.tar.gz'
error: 3rdparty/libseccomp-2.3.3.tar.gz: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/23543/console

- Mesos Reviewbot


On Aug. 6, 2018, 1:39 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68022/
> ---
> 
> (Updated Aug. 6, 2018, 1:39 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9106
> https://issues.apache.org/jira/browse/MESOS-9106
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Containerizer launcher creates an instance of `SeccompFilter`, which is
> used to setup Seccomp profile using `ContainerSeccompProfile` message
> prepared by the `linux/seccomp` isolator. The Seccomp filter is loaded
> right before calling `execve()`, so that a container will be running
> with a syscall filtering enabled.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 7193da0a094df3e441e185c62b3a0379a0bdc4a2 
> 
> 
> Diff: https://reviews.apache.org/r/68022/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 68022: Enabled Seccomp filter in the containerizer launcher.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68022/#review210417
---



FAIL: Failed to apply the dependent review: 67844.

Failed command: `python.exe .\support\apply-reviews.py -n -r 67844`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2584/mesos-review-68022

Relevant logs:

- 
[apply-review-67844.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2584/mesos-review-68022/logs/apply-review-67844.log):

```
error: missing binary patch data for '3rdparty/libseccomp-2.3.3.tar.gz'
error: binary patch does not apply to '3rdparty/libseccomp-2.3.3.tar.gz'
error: 3rdparty/libseccomp-2.3.3.tar.gz: patch does not apply
```

- Mesos Reviewbot Windows


On Aug. 6, 2018, 1:39 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68022/
> ---
> 
> (Updated Aug. 6, 2018, 1:39 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9106
> https://issues.apache.org/jira/browse/MESOS-9106
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Containerizer launcher creates an instance of `SeccompFilter`, which is
> used to setup Seccomp profile using `ContainerSeccompProfile` message
> prepared by the `linux/seccomp` isolator. The Seccomp filter is loaded
> right before calling `execve()`, so that a container will be running
> with a syscall filtering enabled.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 7193da0a094df3e441e185c62b3a0379a0bdc4a2 
> 
> 
> Diff: https://reviews.apache.org/r/68022/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>