Re: Review Request 69543: Implemented recovery for volume gid manager.

[Qian Zhang]

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

(Updated Dec. 18, 2018, 4:09 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, 
and Jie Yu.


Changes
---

Do not checkpoint gid ranges.


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


Repository: mesos


Description
---

Implemented recovery for volume gid manager.


Diffs (updated)
-

  src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8 
  src/Makefile.am 35500516e18ff251357b9e8d6bba894c44a9253b 
  src/slave/containerizer/mesos/containerizer.cpp 
a5cf2da55c046c5c45e0c2ca3400f64de12de62b 
  src/slave/containerizer/mesos/volume_gid_manager/state.hpp PRE-CREATION 
  src/slave/containerizer/mesos/volume_gid_manager/state.proto PRE-CREATION 
  src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp 
PRE-CREATION 


Diff: https://reviews.apache.org/r/69543/diff/2/

Changes: https://reviews.apache.org/r/69543/diff/1-2/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 69541: Added volume gid manager.

[Qian Zhang]

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

(Updated Dec. 18, 2018, 4:11 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, 
and Jie Yu.


Changes
---

Added a new field `totalGids`.


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


Repository: mesos


Description
---

Added volume gid manager.


Diffs (updated)
-

  src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8 
  src/Makefile.am 35500516e18ff251357b9e8d6bba894c44a9253b 
  src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp 
PRE-CREATION 


Diff: https://reviews.apache.org/r/69541/diff/3/

Changes: https://reviews.apache.org/r/69541/diff/2-3/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 69543: Implemented recovery for volume gid manager.

[Qian Zhang]

> On Dec. 12, 2018, 2:38 a.m., Andrei Budnik wrote:
> > src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp
> > Lines 168 (patched)
> > 
> >
> > Consider adding `gidRanges` to the `message VolumeGidInfos` (rename 
> > it?), so that the whole state will be stored in a single file. This 
> > requires storing original gid ranges as a const class variable and 
> > modifying `persist()` method.

I updated the patches by not checkpointing the gid ranges, we should allow 
operators to specify any gid ranges as they want.


- Qian


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


On Dec. 18, 2018, 4:09 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69543/
> ---
> 
> (Updated Dec. 18, 2018, 4:09 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya 
> Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-8810
> https://issues.apache.org/jira/browse/MESOS-8810
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recovery for volume gid manager.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8 
>   src/Makefile.am 35500516e18ff251357b9e8d6bba894c44a9253b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a5cf2da55c046c5c45e0c2ca3400f64de12de62b 
>   src/slave/containerizer/mesos/volume_gid_manager/state.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/volume_gid_manager/state.proto PRE-CREATION 
>   src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69543/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 69544: Made non-root containers can access shared persistent volume.

[Qian Zhang]

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

(Updated Dec. 18, 2018, 4:08 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, 
and Jie Yu.


Changes
---

Added more logs.


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


Repository: mesos


Description
---

Made non-root containers can access shared persistent volume.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
a5cf2da55c046c5c45e0c2ca3400f64de12de62b 
  src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
8f76944cd9058c0ba809443e32a3d8c8a26ac4a6 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
2a9ea448d7f963f86e8b2909d83e82b498e4104c 
  src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
deacc909a2d323671667cb646c019664bdb660e7 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
f91a2eeb835bb65a855eeb314d4c69e3b58fecae 
  src/slave/containerizer/mesos/isolators/filesystem/windows.hpp 
2bf011d3e7b014a17f759851d755b161c897b131 
  src/slave/containerizer/mesos/isolators/filesystem/windows.cpp 
f169c380f803a2111b1612cee60250ee9a30ef2e 
  src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp 
PRE-CREATION 


Diff: https://reviews.apache.org/r/69544/diff/3/

Changes: https://reviews.apache.org/r/69544/diff/2-3/


Testing
---


Thanks,

Qian Zhang

Review Request 69579: Added a test `ROOT_UNPRIVILEGED_USER_TaskSandboxLocalPersistentVolume`.

[Qian Zhang]

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

Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, 
and Jie Yu.


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


Repository: mesos


Description
---

Added a test `ROOT_UNPRIVILEGED_USER_TaskSandboxLocalPersistentVolume`.


Diffs
-

  src/tests/default_executor_tests.cpp 86c3a98ec9e3c5d9d8f2a88218dec1e56d0ebc4c 


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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 69064: Added unit tests for Master HTTP endpoints.

[Greg Mann]

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




src/tests/master_load_tests.cpp
Lines 96-102 (patched)


Let's be consistent with the underscore convention for data members in this 
file. This class uses a trailing underscore for data member names, while the 
blocking authorizer does not.



src/tests/master_load_tests.cpp
Lines 152 (patched)


We could do

```
promises.front().associate(futures.front());
```



src/tests/master_load_tests.cpp
Lines 224 (patched)


If you end up keeping the trailing underscores on the data members of this 
class, then: s/_authorizer/authorizer/



src/tests/master_load_tests.cpp
Lines 293-297 (patched)


Should we add something like

ASSERT_TRUE(Clock::now() - whileLoopStartTime < Seconds(15));

to make sure that we don't end up getting stuck spinning here due to some 
bug in the future?



src/tests/master_load_tests.cpp
Lines 310 (patched)


s/deferals/deferrals/



src/tests/master_load_tests.cpp
Lines 357 (patched)


Why not use `LOG()` here?



src/tests/master_load_tests.cpp
Lines 400-401 (patched)


Could you make this patch a dependant of the metric patch and assert that 
the metric value has been incremented here?

Ditto for the other tests in this file.



src/tests/master_load_tests.cpp
Lines 414 (patched)


s/roles/frameworks/ ??

Here and below.



src/tests/master_load_tests.cpp
Lines 461 (patched)


Remove trailing space after `EXPECT_TRUE`.



src/tests/master_load_tests.cpp
Lines 480 (patched)


I think you could remove this comment.



src/tests/master_load_tests.cpp
Lines 526 (patched)


Remove space after `EXPECT_TRUE`.



src/tests/master_load_tests.cpp
Lines 555 (patched)


Can you use some other header to test this case, so that we don't need to 
disable the test? You could even set an arbitrary header which is not 
interpreted by libprocess.


- Greg Mann


On Dec. 12, 2018, 8:53 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69064/
> ---
> 
> (Updated Dec. 12, 2018, 8:53 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adds a set of unit test to verify that
> basic Master HTTP endpoints still work correctly
> under the presence of request caching.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 35500516e18ff251357b9e8d6bba894c44a9253b 
>   src/tests/master_load_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69064/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 69064: Added unit tests for Master HTTP endpoints.

[Greg Mann]

> On Nov. 26, 2018, 12:25 p.m., Benno Evers wrote:
> > src/tests/master_load_tests.cpp
> > Lines 216 (patched)
> > 
> >
> > I'm not 100% happy about this part, but I also couldn't think of a 
> > better way to generate the references. Any ideas/comments?

No brilliant ideas here - as long as the structure of these responses is 
completed encoded within the HTTP handlers themselves, I'm not sure that we 
have any choice. Feel free to drop this issue.


- Greg


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


On Dec. 12, 2018, 8:53 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69064/
> ---
> 
> (Updated Dec. 12, 2018, 8:53 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adds a set of unit test to verify that
> basic Master HTTP endpoints still work correctly
> under the presence of request caching.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 35500516e18ff251357b9e8d6bba894c44a9253b 
>   src/tests/master_load_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69064/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 69422: WIP: Add new metric for cache hits.

[Greg Mann]

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


Ship it!




I would advocate integrating this with the tests now. I think we can just 
assert that cache hits have occurred; if the test fails for this reason, then 
it's flaky and should be fixed or disabled.

- Greg Mann


On Dec. 12, 2018, 8:54 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69422/
> ---
> 
> (Updated Dec. 12, 2018, 8:54 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The idea was to use this in unit tests as well as for
> benchmarks to verify that caching actually happens.
> 
> However, it's currently not integrated into the unit test
> because it's unclear what should happen in case the test
> detects that no caching occurred.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 68ee2a6dcffbc772afec6e797b1af8da48f61937 
>   src/master/metrics.hpp f6bb89da9394bfe469bc690ff2de66824e994098 
>   src/master/metrics.cpp f69ed52c1c89912e7a5d26cb9409f5959b09111a 
> 
> 
> Diff: https://reviews.apache.org/r/69422/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Review Request 69581: Fixed an interleaving bug on the master actor.

[Greg Mann]

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

Review request for mesos, Benno Evers, Chun-Hung Hsiao, Gastón Kleiman, and 
Meng Zhu.


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


Repository: mesos


Description
---

Upon receipt of an operation via the operator API, the
master would previously update the allocator and then
update its `Slave` struct in two different continuations.
This created the possibility that the allocator could be
updated in between those two continuations, leading to
inconsistency between the allocator state and the
master's state.

This patch invokes these two blocks of code
synchronously to avoid this issue.


Diffs
-

  src/master/master.cpp ae5b24093156a6ba8ffa3e94a46e600eb4def5fa 


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


Testing
---

Ran the test in the subsequent patch both before and after this fix. Before the 
fix, the test fails reliably. After the fix, the test passes reliably.


Thanks,

Greg Mann

Review Request 69582: Added a test to verify a bug fix for the master.

[Greg Mann]

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

Review request for mesos, Benno Evers, Chun-Hung Hsiao, Gastón Kleiman, and 
Meng Zhu.


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


Repository: mesos


Description
---

Added a test to verify a bug fix for the master.


Diffs
-

  src/tests/master_tests.cpp 3bff7a13bb8013ef91b72c14ace23ee94bc190e9 


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


Testing
---

Ran this test before and after the preceding fix. Before the fix, the test 
fails reliably. After the fix, the test passes reliably.


Thanks,

Greg Mann

Re: Review Request 69579: Added a test `ROOT_UNPRIVILEGED_USER_TaskSandboxLocalPersistentVolume`.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['69342', '69541', '69542', '69543', '69478', '69553', 
'69479', '69544', '67997', '68162', '68163', '69547', '69579']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
W1218 09:35:08.019917 32608 slave.cpp:3933] Ignoring shutdown framework 
7346568c-14a3-4931-bade-bfb0f130663a- because it is terminating
I1218 09:35:08.021931 28320 master.cpp:1277] Agent 
7346568c-14a3-4931-bade-bfb0f130663a-S0 at slave(464)@192.10.1.6:49931 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net) disconnected
I1218 09:35:08.021931 28320 master.cpp:3280] Disconnecting agent 
7346568c-14a3-4931-bade-bfb0f130663a-S0 at slave(464)@192.10.1.6:49931 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I1218 09:35:08.021931 28320 master.cpp:3299] Deactivating agent 
7346568c-14a3-4931-bade-bfb0f130663a-S0 at slave(464)@192.10.1.6:49931 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I1218 09:35:08.022918 33592 hierarchical.cpp:357] Removed framework 
7346568c-14a3-4931-bade-bfb0f130663a-
I1218 09:35:08.022918 33592 hierarchical.cpp:801] Agent 
7346568c-14a3-4931-bade-bfb0f130663a-S0 deactivated
I1218 09:35:08.023924 31460 containerizer.cpp:2537] Destroying container 
a2b13aad-0f24-4406-a2fe-12bd18bbcaeb in RUNNING state
I1218 09:35:08.023924 31460 containerizer.cpp:3234] Transitioning the state of 
container a2b13aad-0f24-4406-a2fe-12bd18bbcaeb from RUNNING to DESTROYING
I1218 09:35:08.024922 31460 launcher.cpp:161] Asked to destroy container 
a2b13aad-0f24-4406-a2fe-12bd18bbcaeb
W1218 09:35:08.025914 33344 process.cpp:1423] Failed to recv on socket 
WindowsFD::Type::SOCKET=1952 to peer '192.10.1.6:51750': IO failed with error 
code: The specified network name is no longer available.

W1218 09:35:08.025914 33344 process.cpp:838] Failed to recv on socket 
WindowsFD::Type::SOCKET=2496 to peer '192.10.1.6:51751': IO failed with error 
code: The specified network name is no longer available.

I1218 09:35:08.066912 31352 containerizer.cpp:3073] Container 
a2b13aad-0f24-4406-a2fe-12bd18bbcaeb has exited
I1218 09:35:08.09[   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 
(676 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (697 ms total)

[--] Global test environment tear-down
[==] 1079 tests from 104 test cases ran. (537871 ms total)
[  PASSED  ] 1078 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName

 1 FAILED TEST
  YOU HAVE 231 DISABLED TESTS

5933 30108 master.cpp:1117] Master terminating
I1218 09:35:08.097931 33592 hierarchical.cpp:643] Removed agent 
7346568c-14a3-4931-bade-bfb0f130663a-S0
I1218 09:35:08.581962 33344 process.cpp:927] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Dec. 18, 2018, 8:25 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69579/
> ---
> 
> (Updated Dec. 18, 2018, 8:25 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya 
> Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-8810
> https://issues.apache.org/jira/browse/MESOS-8810
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `ROOT_UNPRIVILEGED_USER_TaskSandboxLocalPersistentVolume`.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 86c3a98ec9e3c5d9d8f2a88218dec1e56d0ebc4c 
> 
> 
> Diff: https://reviews.apache.org/r/69579/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 69582: Added a test to verify a bug fix for the master.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69582 was successfully built and tested.

Reviews applied: `['69581', '69582']`

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

- Mesos Reviewbot Windows


On Dec. 18, 2018, 9:08 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69582/
> ---
> 
> (Updated Dec. 18, 2018, 9:08 a.m.)
> 
> 
> Review request for mesos, Benno Evers, Chun-Hung Hsiao, Gastón Kleiman, and 
> Meng Zhu.
> 
> 
> Bugs: MESOS-9460
> https://issues.apache.org/jira/browse/MESOS-9460
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test to verify a bug fix for the master.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 3bff7a13bb8013ef91b72c14ace23ee94bc190e9 
> 
> 
> Diff: https://reviews.apache.org/r/69582/diff/1/
> 
> 
> Testing
> ---
> 
> Ran this test before and after the preceding fix. Before the fix, the test 
> fails reliably. After the fix, the test passes reliably.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

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

[Andrei Budnik]

> On Dec. 12, 2018, 11:51 p.m., Gilbert Song wrote:
> > configure.ac
> > Lines 352 (patched)
> > 
> >
> > Do we have a plan to deprecate this configuraton flag in the future? 
> > E.g., always only compile the seccomp library on linux.
> 
> Andrei Budnik wrote:
> Currently, we support old kernel version that does not support `seccomp`. 
> E.g., `centos 6` is bases on pre-seccomp kernel.
> I think we will deprecate this flag in the future.
> 
> Gilbert Song wrote:
> ok, could we add a TODO here? so that people would capture our 
> deprecation plan:)

Updated flag description.


- Andrei


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


On Nov. 8, 2018, 3:23 p.m., Andrei Budnik wrote:
> 
> ---
> 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
> -
> 
>   3rdparty/CMakeLists.txt 703808d063e4bba58f647b5d48b78724003bcc4e 
>   3rdparty/Makefile.am e625e7be1743348d02c6dbb8e0a92d1a395b0ef4 
>   3rdparty/cmake/Versions.cmake 69fc594ec5ba2887b20b88ec0767a5d801411411 
>   3rdparty/versions.am 99ef92087f6958d83ba415e84db5cbbb0c597573 
>   cmake/CompilationConfigure.cmake 2485a8a580dcc2ad9b026e389b6525ef3a19f98e 
>   configure.ac 6778f119570def1838e26cddf7b0192bfe6e37d4 
>   src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8 
>   src/Makefile.am 7a4904a3d67479267087fd2313a263d8218843fa 
>   src/python/native_common/ext_modules.py.in 
> 1f2e6c131d18e3e2fbc2e865c4698c83e73b87ba 
> 
> 
> Diff: https://reviews.apache.org/r/68016/diff/10/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 69493: Documented the `linux/seccomp` isolator.

[Mesos Reviewbot Windows]

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



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/2720/mesos-review-69493

Relevant logs:

- 
[apply-review-67844.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2720/mesos-review-69493/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 Nov. 30, 2018, 4:33 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69493/
> ---
> 
> (Updated Nov. 30, 2018, 4:33 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9036
> https://issues.apache.org/jira/browse/MESOS-9036
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/isolators/linux-seccomp.md PRE-CREATION 
>   docs/mesos-containerizer.md d15e82583fa207ba78e9fc1e83da0cf1f469ec4e 
> 
> 
> Diff: https://reviews.apache.org/r/69493/diff/3/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 69572: Validated that resource providers use correct ID in operation states.

[Benjamin Bannier]

> On Dec. 17, 2018, 11:33 p.m., Chun-Hung Hsiao wrote:
> > src/tests/master_slave_reconciliation_tests.cpp
> > Lines 501 (patched)
> > 
> >
> > Not sure if this is gramatically correct.
> > Should it be (The resource provider is/has been) "Asked to reconcile 
> > before subscription finishes"?
> > 
> > I'm not a native speaker so please excuse me if I'm wrong ;)

I wanted to emphasize the temporal ordering (subscription -> reconciliation). 
I'll leave this as is, dropping.


- Benjamin


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


On Dec. 17, 2018, 10:56 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69572/
> ---
> 
> (Updated Dec. 17, 2018, 10:56 a.m.)
> 
> 
> Review request for mesos and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-9479
> https://issues.apache.org/jira/browse/MESOS-9479
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We expect resource providers to set their IDs in operation status messages.
> This patch adds some assertion that the IDs match our expectations, and
> adjusts some test code to honor these assumptions.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp 103f5458ec882a37357265bd680a59da0bbad6b3 
>   src/tests/master_slave_reconciliation_tests.cpp 
> de6e382ed16d8c02d90dd6157ded1aaae9b593aa 
>   src/tests/resource_provider_manager_tests.cpp 
> b61c50f839b7a0f49a781a6f44aa4f10ad7ebafe 
> 
> 
> Diff: https://reviews.apache.org/r/69572/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67844: Bundled libseccomp v2.3.3 into 3rdparty libraries.

[Qian Zhang]

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


Ship it!




Ship It!

- Qian Zhang


On Aug. 6, 2018, 9:37 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67844/
> ---
> 
> (Updated Aug. 6, 2018, 9:37 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9032
> https://issues.apache.org/jira/browse/MESOS-9032
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The bundled library is downloaded from the `libseccomp` github page:
> https://github.com/seccomp/libseccomp/releases/tag/v2.3.3
> 
> This library is bundled for Linux only, because Seccomp is a
> Linux-specific feature.
> 
> 
> Diffs
> -
> 
>   3rdparty/libseccomp-2.3.3.tar.gz PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67844/diff/8/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Review Request 69584: Relaxed matching criteria for benchmark filter.

[Benno Evers]

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

Review request for mesos and Meng Zhu.


Repository: mesos


Description
---

Changed the benchmark filter to match all test names
containing the string `BENCHMARK`, as opposed to `BENCHMARK_`.

The latter filter would fail to match test names like
`HierarchicalAllocations_BENCHMARK.MultiFrameworkAllocations`,
contrary to the test authors assumptions.


Diffs
-

  src/tests/environment.cpp 03f8f5971b1e91e62ecfbf965028e6f2388d2a7b 


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


Testing
---


Thanks,

Benno Evers

Re: Review Request 69571: Made master process all authorization results for `LAUNCH_GROUP`.

[Jan Schlicht]

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


Ship it!




Ship It!

- Jan Schlicht


On Dec. 17, 2018, 11:58 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69571/
> ---
> 
> (Updated Dec. 17, 2018, 11:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, Jan Schlicht, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-9480
> https://issues.apache.org/jira/browse/MESOS-9480
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixes a bug where the master does not complete the processing
> of authorization results for `LAUNCH_GROUP`, causing a subsequent
> operation to drop if one of the remaining authorization is denied.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 3de0fd35cc815f4b5787ee2cb5e81f5059d7a47c 
> 
> 
> Diff: https://reviews.apache.org/r/69571/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69563: Made master to authorize `CREATE_DISK` and `DESTROY_DISK` properly.

[Jan Schlicht]

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


Ship it!




Ship It!

- Jan Schlicht


On Dec. 18, 2018, midnight, Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69563/
> ---
> 
> (Updated Dec. 18, 2018, midnight)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, Jan Schlicht, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-9474
> https://issues.apache.org/jira/browse/MESOS-9474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixes a bug where the master does not respect the
> authorization result for `CREATE_DISK` or`DESTROY_DISK`, causing
> subsequent operations to drop if the authorization is denied.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 3de0fd35cc815f4b5787ee2cb5e81f5059d7a47c 
> 
> 
> Diff: https://reviews.apache.org/r/69563/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69559: Simplified verify-reviews.py to be more similar to the Python 2 script.

[Till Toenshoff via Review Board]

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


Ship it!




Ship It!

- Till Toenshoff


On Dec. 17, 2018, 3:55 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69559/
> ---
> 
> (Updated Dec. 17, 2018, 3:55 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Dragos Schebesch, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-9459
> https://issues.apache.org/jira/browse/MESOS-9459
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We have switched the Python 2 support scripts to Python 3 a few months
> ago. During this transition, 'verify-reviews.py' has been modified,
> mainly to work better on Windows: https://reviews.apache.org/r/67505/.
> 
> The Python 3 script has then replaced the Python 2 one and we now face
> an issue with the script, MESOS-9459. Due to all the changes made
> between the Python 2 and Python 3 scripts, it is hard for us to
> know when the script started failing and why.
> 
> To solve this issue, this commit takes the state of 'verify-reviews.py'
> as it was in 74121798f24fca372180b8c4bc00b4df07d46240, applies 2to3,
> adds the '--out-file' option, then adds the content of:
> * ac766c8bd0fba348399d8eac52f75f62e64db64e
> * 43eba285cb5c8622a6be8a5bdad637ed53431e85
> * 5b348b6070f0d0403cb69b6a7fa638fc46b7ff49
> * 568fcdfd29788d9df89a51ffae7969c2bf0ea173
> 
> This gives a clean diff between 74121798f24fca372180b8c4bc00b4df07d46240
> and this commit, fixes the issue raised in MESOS-9459, and does not
> remove existing features. However, this uses less our common library
> developed in the chain that landed while migrating from Python 2 to 3.
> 
> This should be followed by a refactoring of 'verify-review.py' and
> 'common.py' to use our library more while not breaking the script
> when using Reviewbot.
> 
> 
> Diffs
> -
> 
>   support/verify-reviews.py 552dc366a588e6c5ad0efdcdca03d66f1d7044c3 
> 
> 
> Diff: https://reviews.apache.org/r/69559/diff/4/
> 
> 
> Testing
> ---
> 
> Checked https://www.diffchecker.com/R5uYlefc a diff of 'verify-reviews.py' 
> between 74121798f24fca372180b8c4bc00b4df07d46240 and this commit.
> 
> 
> ```
> ?  apache-mesos (MESOS-9459) ? BUILD_URL=yolo python3 
> support/verify-reviews.py -u user -p password
> git rev-parse HEAD
> Checking if review: 69544 needs verification
> Skipping blocking review 69544
> Checking if review: 69342 needs verification
> Skipping blocking review 69342
> Checking if review: 69478 needs verification
> Skipping blocking review 69478
> Checking if review: 69479 needs verification
> Skipping blocking review 69479
> Checking if review: 67997 needs verification
> Skipping blocking review 67997
> Checking if review: 68162 needs verification
> Skipping blocking review 68162
> Checking if review: 68163 needs verification
> Skipping blocking review 68163
> Checking if review: 69071 needs verification
> Skipping blocking review 69071
> Checking if review: 68795 needs verification
> Skipping blocking review 68795
> Checking if review: 69421 needs verification
> Skipping blocking review 69421
> Checking if review: 69422 needs verification
> Skipping blocking review 69422
> Checking if review: 69064 needs verification
> Skipping blocking review 69064
> Checking if review: 69541 needs verification
> Skipping blocking review 69541
> Checking if review: 68018 needs verification
> Skipping blocking review 68018
> Checking if review: 68017 needs verification
> Skipping blocking review 68017
> Checking if review: 68016 needs verification
> Skipping blocking review 68016
> Checking if review: 69551 needs verification
> Latest diff timestamp: 2018-12-13 14:20:34
> Verifying review 69551
> Applying review 69551
> /usr/local/opt/python/bin/python3.7 support/apply-reviews.py -n -r 69551
> Posting review: Bad patch!
> 
> Reviews applied: [69551]
> 
> Failed command: /usr/local/opt/python/bin/python3.7 support/apply-reviews.py 
> -n -r 69551
> 
> Error:
> 2018-12-14 13:30:42 URL:https://reviews.apache.org/r/69551/diff/raw/ 
> [52606/52606] -> "69551.patch" [1]
> Done processing src/tests/oversubscription_tests.cpp
> Done processing src/tests/gc_tests.cpp
> Done processing src/tests/containerizer/xfs_quota_tests.cpp
> Done processing src/tests/log_tests.cpp
> Done processing src/tests/persistent_volume_tests.cpp
> Done processing src/tests/master_quota_tests.cpp
> Done processing src/tests/partition_tests.cpp
> Done processing src/tests/master_slave_reconciliation_tests.cpp
> Done processing src/tests/containerizer/mesos_containerizer_tests.cpp
> Done processing src/tests/containerizer/port_mapping_tests.cpp
> Done processing src/tests/fetcher_tests.cpp

Re: Review Request 69584: Relaxed matching criteria for test filters.

[Benno Evers]

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

(Updated Dec. 18, 2018, 2:59 p.m.)


Review request for mesos and Meng Zhu.


Changes
---

Generalized changes to other test filters.


Summary (updated)
-

Relaxed matching criteria for test filters.


Repository: mesos


Description (updated)
---

Changed the benchmark filter to remove the required trailing
underscore, e.g. the `BenchmarkFilter` would now match all test names
containing the string `BENCHMARK`, as opposed to `BENCHMARK_`.

The latter filter would fail to match test names like
`HierarchicalAllocations_BENCHMARK.MultiFrameworkAllocations`,
contrary to the test authors assumptions.

This change is also safe regarding false positives, since our
naming conventions forbid the matched strings from appearing
naturally in any test name.


Diffs (updated)
-

  src/tests/environment.cpp 03f8f5971b1e91e62ecfbf965028e6f2388d2a7b 


Diff: https://reviews.apache.org/r/69584/diff/2/

Changes: https://reviews.apache.org/r/69584/diff/1-2/


Testing
---


Thanks,

Benno Evers

Re: Review Request 69572: Validated that resource providers use correct ID in operation states.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69572 was successfully built and tested.

Reviews applied: `['69569', '69572']`

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

- Mesos Reviewbot Windows


On Dec. 17, 2018, 9:56 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69572/
> ---
> 
> (Updated Dec. 17, 2018, 9:56 a.m.)
> 
> 
> Review request for mesos and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-9479
> https://issues.apache.org/jira/browse/MESOS-9479
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We expect resource providers to set their IDs in operation status
> messages. This patch adds some assertion that the IDs match our
> expectations, and adjusts some test code to honor these assumptions.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp 103f5458ec882a37357265bd680a59da0bbad6b3 
>   src/tests/master_slave_reconciliation_tests.cpp 
> de6e382ed16d8c02d90dd6157ded1aaae9b593aa 
>   src/tests/resource_provider_manager_tests.cpp 
> b61c50f839b7a0f49a781a6f44aa4f10ad7ebafe 
> 
> 
> Diff: https://reviews.apache.org/r/69572/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67844: Bundled libseccomp v2.3.3 into 3rdparty libraries.

[Andrei Budnik]

> On Dec. 6, 2018, 7:01 p.m., Gilbert Song wrote:
> > Could you post this patch as a github PR? so that I could apply and commit.

https://github.com/apache/mesos/pull/320


- Andrei


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


On Aug. 6, 2018, 1:37 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67844/
> ---
> 
> (Updated Aug. 6, 2018, 1:37 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9032
> https://issues.apache.org/jira/browse/MESOS-9032
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The bundled library is downloaded from the `libseccomp` github page:
> https://github.com/seccomp/libseccomp/releases/tag/v2.3.3
> 
> This library is bundled for Linux only, because Seccomp is a
> Linux-specific feature.
> 
> 
> Diffs
> -
> 
>   3rdparty/libseccomp-2.3.3.tar.gz PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67844/diff/8/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 69577: Documented `Object.value` authorization field for certain operations.

[Benjamin Bannier]

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


Fix it, then Ship it!




Ship It!


include/mesos/authorizer/authorizer.proto
Lines 268 (patched)


`s/, The/, the/g`

Here and below.


- Benjamin Bannier


On Dec. 18, 2018, 7:13 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69577/
> ---
> 
> (Updated Dec. 18, 2018, 7:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, Jan Schlicht, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-9486
> https://issues.apache.org/jira/browse/MESOS-9486
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented `Object.value` authorization field for certain operations.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> a51d2f2ab38a0ce6c54db1d9f529b94e27e45928 
> 
> 
> Diff: https://reviews.apache.org/r/69577/diff/1/
> 
> 
> Testing
> ---
> 
> N/A. The behavior is implemented in the next patch.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69578: Set up `Object.value` for `CREATE_DISK` and `DESTROY_DISK` authorization.

[Benjamin Bannier]

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


Fix it, then Ship it!





src/master/master.cpp
Lines 3869-3872 (patched)


How about:
```
// The master ensures that resources are in post-reservation refinement
// format and we set the `object.value` field to the most refined role.
// If there is no reservation, the value is by default set to `*` for
// consistency.




src/master/master.cpp
Lines 3874 (patched)


Instead of making this a very specific `TODO` you could instead make this a 
more general comment, e.g.,

```
We set `object.value` in addition to `object.resource` to support legacy 
authorizers making only use of `value`.
```



src/master/master.cpp
Lines 3877 (patched)


`resource.reservations.empty()`



src/master/master.cpp
Lines 3931-3935 (patched)


Ditto.



src/master/master.cpp
Lines 3936 (patched)


Ditto.



src/master/master.cpp
Lines 3939 (patched)


Ditto.


- Benjamin Bannier


On Dec. 18, 2018, 7:14 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69578/
> ---
> 
> (Updated Dec. 18, 2018, 7:14 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, Jan Schlicht, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-9486
> https://issues.apache.org/jira/browse/MESOS-9486
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch sets up `Object.value` to the role of the resource for
> authorization actions `CREATE_BLOCK_DISK`, `DESTROY_BLOCK_DISK`,
> `CREATE_MOUNT_DISK` and `DESTROY_MOUNT_DISK` so an old-school authorizer
> can rely on the field to perform authorization.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 3de0fd35cc815f4b5787ee2cb5e81f5059d7a47c 
> 
> 
> Diff: https://reviews.apache.org/r/69578/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69541: Added volume gid manager.

[Andrei Budnik]

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


Fix it, then Ship it!




Ship It!


src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp
Lines 35 (patched)


`vector` is not used.


- Andrei Budnik


On Dec. 18, 2018, 8:11 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69541/
> ---
> 
> (Updated Dec. 18, 2018, 8:11 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya 
> Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-8810
> https://issues.apache.org/jira/browse/MESOS-8810
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added volume gid manager.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8 
>   src/Makefile.am 35500516e18ff251357b9e8d6bba894c44a9253b 
>   src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69541/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 69543: Implemented recovery for volume gid manager.

[Andrei Budnik]

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


Fix it, then Ship it!




Ship It!


src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp
Lines 52 (patched)


We should either get rid of `VOLUME_GIDS_FILE` constant or introduce a new 
constant for `volume_gid_manager`.



src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp
Lines 338 (patched)


Can we add `const` qualifier to `persist()`?
`Try persist() const`


- Andrei Budnik


On Dec. 18, 2018, 8:09 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69543/
> ---
> 
> (Updated Dec. 18, 2018, 8:09 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya 
> Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-8810
> https://issues.apache.org/jira/browse/MESOS-8810
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recovery for volume gid manager.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8 
>   src/Makefile.am 35500516e18ff251357b9e8d6bba894c44a9253b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a5cf2da55c046c5c45e0c2ca3400f64de12de62b 
>   src/slave/containerizer/mesos/volume_gid_manager/state.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/volume_gid_manager/state.proto PRE-CREATION 
>   src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69543/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 68795: Added deduplication for read-only master requests.

[Benno Evers]

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

(Updated Dec. 18, 2018, 6:33 p.m.)


Review request for mesos, Alexander Rukletsov and Joseph Wu.


Changes
---

Added comment about authorization caching.


Repository: mesos


Description
---

This change will skip the computation of an HTTP response
for requests with the same principal, endpoint and request
headers where both requests are authenticated within the
same batching windows.


Diffs (updated)
-

  src/master/http.cpp e38513bf8feda7ffc55d9f571e41ff970df28299 
  src/master/master.hpp 99549ab857b16d722f0dd991f98dbe54e9ed19a1 


Diff: https://reviews.apache.org/r/68795/diff/5/

Changes: https://reviews.apache.org/r/68795/diff/4-5/


Testing
---


Thanks,

Benno Evers

Re: Review Request 69064: Added unit tests for Master HTTP endpoints.

[Benno Evers]

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

(Updated Dec. 18, 2018, 6:41 p.m.)


Review request for mesos, Alexander Rukletsov and Joseph Wu.


Changes
---

Addressed review comments.


Repository: mesos


Description
---

This commit adds a set of unit test to verify that
basic Master HTTP endpoints still work correctly
under the presence of request caching.


Diffs (updated)
-

  src/Makefile.am 7a4904a3d67479267087fd2313a263d8218843fa 
  src/tests/master_load_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/69064/diff/5/

Changes: https://reviews.apache.org/r/69064/diff/4-5/


Testing
---


Thanks,

Benno Evers

Re: Review Request 69584: Relaxed matching criteria for test filters.

[Meng Zhu]

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



Not quite sure about the false positive issue:

"This change is also safe regarding false positives, since our
naming conventions forbid the matched strings from appearing
naturally in any test name."

Ah, I wasn't aware of the convention. But I feel it is easy to slip?
For example, after this patch, would we match 
"ROOT_DOCKER_CGROUPS_CFS_CgroupsEnableCFS" if I remove the "CFS_" tag?

If that is the case, I would prefer to keep the original matching criteria and 
fix the test name. It is easy to
spot a test that ran (but not supposed to) than finding out a silent missing 
test.

- Meng Zhu


On Dec. 18, 2018, 6:59 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69584/
> ---
> 
> (Updated Dec. 18, 2018, 6:59 a.m.)
> 
> 
> Review request for mesos and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed the test filters to remove the required trailing
> underscore, e.g. the `BenchmarkFilter` would now match all test names
> containing the string `BENCHMARK`, as opposed to `BENCHMARK_`.
> 
> The latter filter would fail to match test names like
> `HierarchicalAllocations_BENCHMARK.MultiFrameworkAllocations`,
> contrary to the test authors assumptions.
> 
> This change is also safe regarding false positives, since our
> naming conventions forbid the matched strings from appearing
> naturally in any test name.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp 03f8f5971b1e91e62ecfbf965028e6f2388d2a7b 
> 
> 
> Diff: https://reviews.apache.org/r/69584/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 69422: Added new metric for cache hits.

[Benno Evers]

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

(Updated Dec. 18, 2018, 6:41 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Changed description, some fixups.


Summary (updated)
-

Added new metric for cache hits.


Repository: mesos


Description (updated)
---

This new metric counts the total number of cache
hits in the newly added request batching mechanism
of the mesos master.


Diffs (updated)
-

  src/master/http.cpp e38513bf8feda7ffc55d9f571e41ff970df28299 
  src/master/metrics.hpp eca48e6d0ff0270f1e57d68126e240abd231eb94 
  src/master/metrics.cpp bb029d3fe79c04a1d4891a696cd5d9e50ed86230 
  src/tests/master_tests.cpp a67e3db731212e245c8fadc9e14913f01dcf646d 


Diff: https://reviews.apache.org/r/69422/diff/3/

Changes: https://reviews.apache.org/r/69422/diff/2-3/


Testing
---


Thanks,

Benno Evers

Re: Review Request 69064: Added unit tests for Master HTTP endpoints.

[Benno Evers]

> On Dec. 18, 2018, 8:32 a.m., Greg Mann wrote:
> > src/tests/master_load_tests.cpp
> > Lines 293-297 (patched)
> > 
> >
> > Should we add something like
> > 
> > ASSERT_TRUE(Clock::now() - whileLoopStartTime < Seconds(15));
> > 
> > to make sure that we don't end up getting stuck spinning here due to 
> > some bug in the future?

Good point!


> On Dec. 18, 2018, 8:32 a.m., Greg Mann wrote:
> > src/tests/master_load_tests.cpp
> > Lines 461 (patched)
> > 
> >
> > Remove trailing space after `EXPECT_TRUE`.

I did, but not without shedding a silent tear for the loss of alignment with 
the `EXPECT_FALSE()` below ;)


> On Dec. 18, 2018, 8:32 a.m., Greg Mann wrote:
> > src/tests/master_load_tests.cpp
> > Lines 555 (patched)
> > 
> >
> > Can you use some other header to test this case, so that we don't need 
> > to disable the test? You could even set an arbitrary header which is not 
> > interpreted by libprocess.

The endpoints themselves will ignore all headers, so the idea of this test was 
to verify that libprocess can still correctly handle the headers that 
libprocess cares about. So using another header which is ignored by libprocess 
will not really work for that purpose.

I suppose we could drop the test completely, but I think the chances are pretty 
low that we will remember (and allocate time) to write this test from scratch 
in the future, when it becomes possible to enable it.


- Benno


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


On Dec. 18, 2018, 6:41 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69064/
> ---
> 
> (Updated Dec. 18, 2018, 6:41 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adds a set of unit test to verify that
> basic Master HTTP endpoints still work correctly
> under the presence of request caching.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 7a4904a3d67479267087fd2313a263d8218843fa 
>   src/tests/master_load_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69064/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 69584: Relaxed matching criteria for test filters.

[Joseph Wu]

> On Dec. 18, 2018, 10:44 a.m., Meng Zhu wrote:
> > Not quite sure about the false positive issue:
> > 
> > "This change is also safe regarding false positives, since our
> > naming conventions forbid the matched strings from appearing
> > naturally in any test name."
> > 
> > Ah, I wasn't aware of the convention. But I feel it is easy to slip?
> > For example, after this patch, would we match 
> > "ROOT_DOCKER_CGROUPS_CFS_CgroupsEnableCFS" if I remove the "CFS_" tag?
> > 
> > If that is the case, I would prefer to keep the original matching criteria 
> > and fix the test name. It is easy to
> > spot a test that ran (but not supposed to) than finding out a silent 
> > missing test.

I agree about the false positive case.  We should just update the specific 
BENCHMARK test to properly filter it.

I don't know if we codify our test naming style anywhere at the moment.  
However, our naming convention is to put any of the test filters at the 
beginning of a test name, rather than the end.


- Joseph


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


On Dec. 18, 2018, 6:59 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69584/
> ---
> 
> (Updated Dec. 18, 2018, 6:59 a.m.)
> 
> 
> Review request for mesos and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed the test filters to remove the required trailing
> underscore, e.g. the `BenchmarkFilter` would now match all test names
> containing the string `BENCHMARK`, as opposed to `BENCHMARK_`.
> 
> The latter filter would fail to match test names like
> `HierarchicalAllocations_BENCHMARK.MultiFrameworkAllocations`,
> contrary to the test authors assumptions.
> 
> This change is also safe regarding false positives, since our
> naming conventions forbid the matched strings from appearing
> naturally in any test name.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp 03f8f5971b1e91e62ecfbf965028e6f2388d2a7b 
> 
> 
> Diff: https://reviews.apache.org/r/69584/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 69577: Documented `Object.value` authorization field for certain operations.

[Chun-Hung Hsiao]

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

(Updated Dec. 18, 2018, 8:33 p.m.)


Review request for mesos, Benjamin Bannier, Greg Mann, Jan Schlicht, and Till 
Toenshoff.


Changes
---

Addressed Benjamin's comment.


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


Repository: mesos


Description
---

Documented `Object.value` authorization field for certain operations.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.proto 
a51d2f2ab38a0ce6c54db1d9f529b94e27e45928 


Diff: https://reviews.apache.org/r/69577/diff/2/

Changes: https://reviews.apache.org/r/69577/diff/1-2/


Testing
---

N/A. The behavior is implemented in the next patch.


Thanks,

Chun-Hung Hsiao

Re: Review Request 69578: Set up `Object.value` for `CREATE_DISK` and `DESTROY_DISK` authorization.

[Chun-Hung Hsiao]

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

(Updated Dec. 18, 2018, 9:24 p.m.)


Review request for mesos, Benjamin Bannier, Greg Mann, Jan Schlicht, and Till 
Toenshoff.


Changes
---

Addressed Benjamin's comments.


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


Repository: mesos


Description
---

This patch sets up `Object.value` to the role of the resource for
authorization actions `CREATE_BLOCK_DISK`, `DESTROY_BLOCK_DISK`,
`CREATE_MOUNT_DISK` and `DESTROY_MOUNT_DISK` so an old-school authorizer
can rely on the field to perform authorization.


Diffs (updated)
-

  src/master/master.cpp 3de0fd35cc815f4b5787ee2cb5e81f5059d7a47c 


Diff: https://reviews.apache.org/r/69578/diff/2/

Changes: https://reviews.apache.org/r/69578/diff/1-2/


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 69581: Fixed an interleaving bug on the master actor.

[Chun-Hung Hsiao]

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


Ship it!




Ship It!

- Chun-Hung Hsiao


On Dec. 18, 2018, 9:06 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69581/
> ---
> 
> (Updated Dec. 18, 2018, 9:06 a.m.)
> 
> 
> Review request for mesos, Benno Evers, Chun-Hung Hsiao, Gastón Kleiman, and 
> Meng Zhu.
> 
> 
> Bugs: MESOS-9460
> https://issues.apache.org/jira/browse/MESOS-9460
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upon receipt of an operation via the operator API, the
> master would previously update the allocator and then
> update its `Slave` struct in two different continuations.
> This created the possibility that the allocator could be
> updated in between those two continuations, leading to
> inconsistency between the allocator state and the
> master's state.
> 
> This patch invokes these two blocks of code
> synchronously to avoid this issue.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp ae5b24093156a6ba8ffa3e94a46e600eb4def5fa 
> 
> 
> Diff: https://reviews.apache.org/r/69581/diff/1/
> 
> 
> Testing
> ---
> 
> Ran the test in the subsequent patch both before and after this fix. Before 
> the fix, the test fails reliably. After the fix, the test passes reliably.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 69581: Fixed an interleaving bug on the master actor.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Dec. 18, 2018, 1:06 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69581/
> ---
> 
> (Updated Dec. 18, 2018, 1:06 a.m.)
> 
> 
> Review request for mesos, Benno Evers, Chun-Hung Hsiao, Gastón Kleiman, and 
> Meng Zhu.
> 
> 
> Bugs: MESOS-9460
> https://issues.apache.org/jira/browse/MESOS-9460
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upon receipt of an operation via the operator API, the
> master would previously update the allocator and then
> update its `Slave` struct in two different continuations.
> This created the possibility that the allocator could be
> updated in between those two continuations, leading to
> inconsistency between the allocator state and the
> master's state.
> 
> This patch invokes these two blocks of code
> synchronously to avoid this issue.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp ae5b24093156a6ba8ffa3e94a46e600eb4def5fa 
> 
> 
> Diff: https://reviews.apache.org/r/69581/diff/1/
> 
> 
> Testing
> ---
> 
> Ran the test in the subsequent patch both before and after this fix. Before 
> the fix, the test fails reliably. After the fix, the test passes reliably.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 69590: Moves CNI root directory to a persistent location.

[Deepak Goel]

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

Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Sergey 
Urbanovich.


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


Repository: mesos


Description
---

This patch moves the CNI root directory to mesos working directory
so that the network related information persist across reboot which
will allow cni isolator to do proper cleanup post reboot


Diffs
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
f19edce0671686bc2dccf5dc6d3abc40b925852e 
  src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
3b06fb101d03b32a3a148e2b79502413a7268d1e 
  src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
c1cb4f7d667159abbf87219cde30528a4a7b5283 
  src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
  src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 
  src/tests/containerizer/cni_isolator_tests.cpp 
5473737cdc71a3179c22be0d4db2060a6ef53d24 


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


Testing
---


Thanks,

Deepak Goel

Re: Review Request 69590: Moves CNI root directory to a persistent location.

[Till Toenshoff via Review Board]

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



Thanks for this. Just some nitpicking below :)


src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
Lines 1307-1308 (original)


Are we assuming that this location was never meant for users?



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
Lines 1312 (patched)


s/"network/"Network/g



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
Line 1779 (original)


See above.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
Lines 1785 (patched)


See above.



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


```
"This setting controls whether the CNI root directory\n"
" persists beyond reboots or not."
```
Maybe?



src/tests/containerizer/cni_isolator_tests.cpp
Lines 2542 (patched)


Add a trailing period please. And maybe 

```
// This test verifies the CNI root directory path.
```



src/tests/containerizer/cni_isolator_tests.cpp
Lines 2564 (patched)


We can do without this, I feel.



src/tests/containerizer/cni_isolator_tests.cpp
Lines 2567 (patched)


This seems to be the most relevant part of this test - checking with and 
without this flag enabled. Maybe we should add a comment here?



src/tests/containerizer/cni_isolator_tests.cpp
Lines 2569 (patched)


Either remove these two please or start with capital letter and end with 
punctuation. Thanks.


- Till Toenshoff


On Dec. 19, 2018, 12:12 a.m., Deepak Goel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69590/
> ---
> 
> (Updated Dec. 19, 2018, 12:12 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Sergey 
> Urbanovich.
> 
> 
> Bugs: MESOS-9492
> https://issues.apache.org/jira/browse/MESOS-9492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch moves the CNI root directory to mesos working directory
> so that the network related information persist across reboot which
> will allow cni isolator to do proper cleanup post reboot
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f19edce0671686bc2dccf5dc6d3abc40b925852e 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
> 3b06fb101d03b32a3a148e2b79502413a7268d1e 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
> c1cb4f7d667159abbf87219cde30528a4a7b5283 
>   src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
>   src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 5473737cdc71a3179c22be0d4db2060a6ef53d24 
> 
> 
> Diff: https://reviews.apache.org/r/69590/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Deepak Goel
> 
>

Re: Review Request 69590: Moves CNI root directory to a persistent location.

[Deepak Goel]

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

(Updated Dec. 19, 2018, 2:03 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Sergey 
Urbanovich.


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


Repository: mesos


Description
---

This patch moves the CNI root directory to mesos working directory
so that the network related information persist across reboot which
will allow cni isolator to do proper cleanup post reboot


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
f19edce0671686bc2dccf5dc6d3abc40b925852e 
  src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
3b06fb101d03b32a3a148e2b79502413a7268d1e 
  src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
c1cb4f7d667159abbf87219cde30528a4a7b5283 
  src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
  src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 
  src/tests/containerizer/cni_isolator_tests.cpp 
5473737cdc71a3179c22be0d4db2060a6ef53d24 


Diff: https://reviews.apache.org/r/69590/diff/2/

Changes: https://reviews.apache.org/r/69590/diff/1-2/


Testing
---


Thanks,

Deepak Goel

Re: Review Request 69590: Moves CNI root directory to a persistent location.

[Deepak Goel]

> On Dec. 19, 2018, 12:48 a.m., Till Toenshoff wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
> > Lines 1307-1308 (original)
> > 
> >
> > Are we assuming that this location was never meant for users?

Didn't understand your comment? Which location you are talking about?


- Deepak


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


On Dec. 19, 2018, 2:03 a.m., Deepak Goel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69590/
> ---
> 
> (Updated Dec. 19, 2018, 2:03 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Sergey 
> Urbanovich.
> 
> 
> Bugs: MESOS-9492
> https://issues.apache.org/jira/browse/MESOS-9492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch moves the CNI root directory to mesos working directory
> so that the network related information persist across reboot which
> will allow cni isolator to do proper cleanup post reboot
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f19edce0671686bc2dccf5dc6d3abc40b925852e 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
> 3b06fb101d03b32a3a148e2b79502413a7268d1e 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
> c1cb4f7d667159abbf87219cde30528a4a7b5283 
>   src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
>   src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 5473737cdc71a3179c22be0d4db2060a6ef53d24 
> 
> 
> Diff: https://reviews.apache.org/r/69590/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Deepak Goel
> 
>

Re: Review Request 69590: Moves CNI root directory to a persistent location.

[Qian Zhang]

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




src/slave/containerizer/mesos/isolators/network/cni/paths.hpp
Line 49 (original), 52 (patched)


One more newline.



src/slave/containerizer/mesos/isolators/network/cni/paths.hpp
Line 50 (original), 54 (patched)


Ditto.



src/slave/containerizer/mesos/isolators/network/cni/paths.cpp
Lines 37-38 (patched)


A newline between.



src/slave/containerizer/mesos/isolators/network/cni/paths.cpp
Lines 40 (patched)


One more newline.



src/slave/flags.hpp
Lines 165 (patched)


We need to update `agent.md` by adding this new flag into it, and also need 
to update `cni.md` to describe this new flag.



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


Should the default value be `false` for backward compatibility?


- Qian Zhang


On Dec. 19, 2018, 10:03 a.m., Deepak Goel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69590/
> ---
> 
> (Updated Dec. 19, 2018, 10:03 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Sergey 
> Urbanovich.
> 
> 
> Bugs: MESOS-9492
> https://issues.apache.org/jira/browse/MESOS-9492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch moves the CNI root directory to mesos working directory
> so that the network related information persist across reboot which
> will allow cni isolator to do proper cleanup post reboot
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f19edce0671686bc2dccf5dc6d3abc40b925852e 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
> 3b06fb101d03b32a3a148e2b79502413a7268d1e 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
> c1cb4f7d667159abbf87219cde30528a4a7b5283 
>   src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
>   src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 5473737cdc71a3179c22be0d4db2060a6ef53d24 
> 
> 
> Diff: https://reviews.apache.org/r/69590/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Deepak Goel
> 
>

Re: Review Request 69590: Moves CNI root directory to a persistent location.

[Deepak Goel]

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

(Updated Dec. 19, 2018, 3:25 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Sergey 
Urbanovich.


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


Repository: mesos


Description
---

This patch moves the CNI root directory to mesos working directory
so that the network related information persist across reboot which
will allow cni isolator to do proper cleanup post reboot


Diffs (updated)
-

  docs/cni.md 4a0e0bf57c6ff8d2bcc9fdf30d20672324da224c 
  docs/configuration/agent.md 7a8df6852dc2af174a6c5a552dca88fa1b1c29f3 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
f19edce0671686bc2dccf5dc6d3abc40b925852e 
  src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
3b06fb101d03b32a3a148e2b79502413a7268d1e 
  src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
c1cb4f7d667159abbf87219cde30528a4a7b5283 
  src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
  src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 
  src/tests/containerizer/cni_isolator_tests.cpp 
5473737cdc71a3179c22be0d4db2060a6ef53d24 


Diff: https://reviews.apache.org/r/69590/diff/3/

Changes: https://reviews.apache.org/r/69590/diff/2-3/


Testing
---


Thanks,

Deepak Goel

Re: Review Request 69590: Moves CNI root directory to a persistent location.

[Chun-Hung Hsiao]

> On Dec. 19, 2018, 12:48 a.m., Till Toenshoff wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
> > Lines 1307-1308 (original)
> > 
> >
> > Are we assuming that this location was never meant for users?
> 
> Deepak Goel wrote:
> Didn't understand your comment? Which location you are talking about?

I guess Till was asking why the log of `networkConfigJSON` is hidden from 
user-facing logs (`LOG(INFO)`), unlike the other log lines.


- Chun-Hung


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


On Dec. 19, 2018, 3:25 a.m., Deepak Goel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69590/
> ---
> 
> (Updated Dec. 19, 2018, 3:25 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Sergey 
> Urbanovich.
> 
> 
> Bugs: MESOS-9492
> https://issues.apache.org/jira/browse/MESOS-9492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch moves the CNI root directory to mesos working directory
> so that the network related information persist across reboot which
> will allow cni isolator to do proper cleanup post reboot
> 
> 
> Diffs
> -
> 
>   docs/cni.md 4a0e0bf57c6ff8d2bcc9fdf30d20672324da224c 
>   docs/configuration/agent.md 7a8df6852dc2af174a6c5a552dca88fa1b1c29f3 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f19edce0671686bc2dccf5dc6d3abc40b925852e 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
> 3b06fb101d03b32a3a148e2b79502413a7268d1e 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
> c1cb4f7d667159abbf87219cde30528a4a7b5283 
>   src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
>   src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 5473737cdc71a3179c22be0d4db2060a6ef53d24 
> 
> 
> Diff: https://reviews.apache.org/r/69590/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Deepak Goel
> 
>

Re: Review Request 69590: Moves CNI root directory to a persistent location.

[Chun-Hung Hsiao]

> On Dec. 19, 2018, 2:51 a.m., Qian Zhang wrote:
> > src/slave/flags.cpp
> > Lines 1191 (patched)
> > 
> >
> > Should the default value be `false` for backward compatibility?

Hmm yeah maybe it's better if we make it default to `false` for now, and ask 
around the community to make the discussion transparent for a plan to enable it 
by default if needed.

Also it is worth mentioning this new flag in `docs/configuration/agent.md` and 
`docs/upgrades.md`.


- Chun-Hung


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


On Dec. 19, 2018, 3:25 a.m., Deepak Goel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69590/
> ---
> 
> (Updated Dec. 19, 2018, 3:25 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Sergey 
> Urbanovich.
> 
> 
> Bugs: MESOS-9492
> https://issues.apache.org/jira/browse/MESOS-9492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch moves the CNI root directory to mesos working directory
> so that the network related information persist across reboot which
> will allow cni isolator to do proper cleanup post reboot
> 
> 
> Diffs
> -
> 
>   docs/cni.md 4a0e0bf57c6ff8d2bcc9fdf30d20672324da224c 
>   docs/configuration/agent.md 7a8df6852dc2af174a6c5a552dca88fa1b1c29f3 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f19edce0671686bc2dccf5dc6d3abc40b925852e 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
> 3b06fb101d03b32a3a148e2b79502413a7268d1e 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
> c1cb4f7d667159abbf87219cde30528a4a7b5283 
>   src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
>   src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 5473737cdc71a3179c22be0d4db2060a6ef53d24 
> 
> 
> Diff: https://reviews.apache.org/r/69590/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Deepak Goel
> 
>

Re: Review Request 69590: Moves CNI root directory to a persistent location.

[Chun-Hung Hsiao]

> On Dec. 19, 2018, 2:51 a.m., Qian Zhang wrote:
> > src/slave/flags.cpp
> > Lines 1191 (patched)
> > 
> >
> > Should the default value be `false` for backward compatibility?
> 
> Chun-Hung Hsiao wrote:
> Hmm yeah maybe it's better if we make it default to `false` for now, and 
> ask around the community to make the discussion transparent for a plan to 
> enable it by default if needed.
> 
> Also it is worth mentioning this new flag in 
> `docs/configuration/agent.md` and `docs/upgrades.md`.

Oops it seems that you already updated `docs/configuration/agent.md`. My bad on 
missing that.


- Chun-Hung


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


On Dec. 19, 2018, 3:25 a.m., Deepak Goel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69590/
> ---
> 
> (Updated Dec. 19, 2018, 3:25 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Sergey 
> Urbanovich.
> 
> 
> Bugs: MESOS-9492
> https://issues.apache.org/jira/browse/MESOS-9492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch moves the CNI root directory to mesos working directory
> so that the network related information persist across reboot which
> will allow cni isolator to do proper cleanup post reboot
> 
> 
> Diffs
> -
> 
>   docs/cni.md 4a0e0bf57c6ff8d2bcc9fdf30d20672324da224c 
>   docs/configuration/agent.md 7a8df6852dc2af174a6c5a552dca88fa1b1c29f3 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f19edce0671686bc2dccf5dc6d3abc40b925852e 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
> 3b06fb101d03b32a3a148e2b79502413a7268d1e 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
> c1cb4f7d667159abbf87219cde30528a4a7b5283 
>   src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
>   src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 5473737cdc71a3179c22be0d4db2060a6ef53d24 
> 
> 
> Diff: https://reviews.apache.org/r/69590/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Deepak Goel
> 
>

Re: Review Request 69590: Moves CNI root directory to a persistent location.

[Deepak Goel]

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

(Updated Dec. 19, 2018, 4:52 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Sergey 
Urbanovich.


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


Repository: mesos


Description
---

This patch moves the CNI root directory to mesos working directory
so that the network related information persist across reboot which
will allow cni isolator to do proper cleanup post reboot


Diffs (updated)
-

  docs/cni.md 4a0e0bf57c6ff8d2bcc9fdf30d20672324da224c 
  docs/configuration/agent.md 7a8df6852dc2af174a6c5a552dca88fa1b1c29f3 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
f19edce0671686bc2dccf5dc6d3abc40b925852e 
  src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
3b06fb101d03b32a3a148e2b79502413a7268d1e 
  src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
c1cb4f7d667159abbf87219cde30528a4a7b5283 
  src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
  src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 
  src/tests/containerizer/cni_isolator_tests.cpp 
5473737cdc71a3179c22be0d4db2060a6ef53d24 


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

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


Testing
---


Thanks,

Deepak Goel

Re: Review Request 69590: Moves CNI root directory to a persistent location.

[Deepak Goel]

> On Dec. 19, 2018, 12:48 a.m., Till Toenshoff wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
> > Lines 1307-1308 (original)
> > 
> >
> > Are we assuming that this location was never meant for users?
> 
> Deepak Goel wrote:
> Didn't understand your comment? Which location you are talking about?
> 
> Chun-Hung Hsiao wrote:
> I guess Till was asking why the log of `networkConfigJSON` is hidden from 
> user-facing logs (`LOG(INFO)`), unlike the other log lines.

Got it. networkConfigJSON might get big and could quickly fill up the log 
space. Hence, kept it at VLOG level just for debugging purpose.


- Deepak


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


On Dec. 19, 2018, 4:52 a.m., Deepak Goel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69590/
> ---
> 
> (Updated Dec. 19, 2018, 4:52 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Sergey 
> Urbanovich.
> 
> 
> Bugs: MESOS-9492
> https://issues.apache.org/jira/browse/MESOS-9492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch moves the CNI root directory to mesos working directory
> so that the network related information persist across reboot which
> will allow cni isolator to do proper cleanup post reboot
> 
> 
> Diffs
> -
> 
>   docs/cni.md 4a0e0bf57c6ff8d2bcc9fdf30d20672324da224c 
>   docs/configuration/agent.md 7a8df6852dc2af174a6c5a552dca88fa1b1c29f3 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f19edce0671686bc2dccf5dc6d3abc40b925852e 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
> 3b06fb101d03b32a3a148e2b79502413a7268d1e 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
> c1cb4f7d667159abbf87219cde30528a4a7b5283 
>   src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
>   src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 5473737cdc71a3179c22be0d4db2060a6ef53d24 
> 
> 
> Diff: https://reviews.apache.org/r/69590/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Deepak Goel
> 
>

Re: Review Request 69590: Moves CNI root directory to a persistent location.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
Lines 1312-1313 (patched)


This log line along is not enough information. You need to at containerId 
and network name so that folks can correlate.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
Lines 1785 (patched)


Ditto



src/slave/containerizer/mesos/isolators/network/cni/paths.hpp
Line 35 (original), 37 (patched)


Please update the comments here.



src/slave/containerizer/mesos/isolators/network/cni/paths.hpp
Line 48 (original), 50 (patched)


No need for this variable anymore. See my comments below.



src/slave/containerizer/mesos/isolators/network/cni/paths.cpp
Lines 37 (patched)


instead of use `paths::ROOT_DIR`, you should use `flags.runtime_dir`


- Jie Yu


On Dec. 19, 2018, 4:52 a.m., Deepak Goel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69590/
> ---
> 
> (Updated Dec. 19, 2018, 4:52 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Sergey 
> Urbanovich.
> 
> 
> Bugs: MESOS-9492
> https://issues.apache.org/jira/browse/MESOS-9492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch moves the CNI root directory to mesos working directory
> so that the network related information persist across reboot which
> will allow cni isolator to do proper cleanup post reboot
> 
> 
> Diffs
> -
> 
>   docs/cni.md 4a0e0bf57c6ff8d2bcc9fdf30d20672324da224c 
>   docs/configuration/agent.md 7a8df6852dc2af174a6c5a552dca88fa1b1c29f3 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f19edce0671686bc2dccf5dc6d3abc40b925852e 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
> 3b06fb101d03b32a3a148e2b79502413a7268d1e 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
> c1cb4f7d667159abbf87219cde30528a4a7b5283 
>   src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
>   src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 5473737cdc71a3179c22be0d4db2060a6ef53d24 
> 
> 
> Diff: https://reviews.apache.org/r/69590/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Deepak Goel
> 
>

Re: Review Request 69590: Moves CNI root directory to a persistent location.

[Qian Zhang]

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


Fix it, then Ship it!




The commit message of this patch seems not accurate to me, we are not moving 
CNI root directory to a persistent location, we actually added a new agent flag 
for operator to specify whether persist the network related information or not.


docs/cni.md
Lines 106 (patched)


As Chun mentioned, please also mention this flag in `docs/upgrades.md`.



docs/configuration/agent.md
Lines 1184 (patched)


s/persist/persists/



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


s/persist/persists/

And no need the leading space.


- Qian Zhang


On Dec. 19, 2018, 12:52 p.m., Deepak Goel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69590/
> ---
> 
> (Updated Dec. 19, 2018, 12:52 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Sergey 
> Urbanovich.
> 
> 
> Bugs: MESOS-9492
> https://issues.apache.org/jira/browse/MESOS-9492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch moves the CNI root directory to mesos working directory
> so that the network related information persist across reboot which
> will allow cni isolator to do proper cleanup post reboot
> 
> 
> Diffs
> -
> 
>   docs/cni.md 4a0e0bf57c6ff8d2bcc9fdf30d20672324da224c 
>   docs/configuration/agent.md 7a8df6852dc2af174a6c5a552dca88fa1b1c29f3 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f19edce0671686bc2dccf5dc6d3abc40b925852e 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
> 3b06fb101d03b32a3a148e2b79502413a7268d1e 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
> c1cb4f7d667159abbf87219cde30528a4a7b5283 
>   src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
>   src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 5473737cdc71a3179c22be0d4db2060a6ef53d24 
> 
> 
> Diff: https://reviews.apache.org/r/69590/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Deepak Goel
> 
>

Review Request 69591: Fixed allocator benchmark names to ensure proper filtering.

[Meng Zhu]

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

Review request for mesos, Benno Evers and Joseph Wu.


Repository: mesos


Description
---

Updated names of allocator benchmarks in
`tests/hierarchical_allocator_benchmarks.cpp` to ensure
proper filtering.

The test filter `BENCHMARK_` is put at the beginning of the
test name for consistency.


Diffs
-

  src/tests/hierarchical_allocator_benchmarks.cpp 
822c2ef5f4fcdb1f4839ad5c2d6884c87cca3be2 


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


Testing
---

Before the patch the test `MultiFrameworkAllocations` in the default test 
invocation setting; after applying the patch, it does not run.


Thanks,

Meng Zhu

Review Request 69592: Updated upgrades.md.

[Deepak Goel]

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

Review request for mesos, Jie Yu and Qian Zhang.


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


Repository: mesos


Description
---

Updated upgrades.md with the information about the new flag
`--network_cni_root_dir_persist`. (MESOS-9492)


Diffs
-

  docs/upgrades.md f8166be75924f4f72ac291dfe7df92bd32f79f06 


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


Testing
---


Thanks,

Deepak Goel

Re: Review Request 69592: Updated upgrades.md.

[Qian Zhang]

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


Ship it!




Ship It!

- Qian Zhang


On Dec. 19, 2018, 3:22 p.m., Deepak Goel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69592/
> ---
> 
> (Updated Dec. 19, 2018, 3:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-9492
> https://issues.apache.org/jira/browse/MESOS-9492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated upgrades.md with the information about the new flag
> `--network_cni_root_dir_persist`. (MESOS-9492)
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md f8166be75924f4f72ac291dfe7df92bd32f79f06 
> 
> 
> Diff: https://reviews.apache.org/r/69592/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Deepak Goel
> 
>