Re: Review Request 65987: Allow nested containers in pods to have separate namespaces(Ref: MESOS-8534).

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['65987']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65987

Relevant logs:

- 
[libprocess-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65987/logs/libprocess-tests-stdout.log):

```
[ RUN  ] TimeTest.Output
[   OK ] TimeTest.Output (0 ms)
[--] 4 tests from TimeTest (7 ms total)

[--] 3 tests from TimeSeriesTest
[ RUN  ] TimeSeriesTest.Set
[   OK ] TimeSeriesTest.Set (0 ms)
[ RUN  ] TimeSeriesTest.Sparsify
[   OK ] TimeSeriesTest.Sparsify (1 ms)
[ RUN  ] TimeSeriesTest.Truncate
[   OK ] TimeSeriesTest.Truncate (1 ms)
[--] 3 tests from TimeSeriesTest (8 ms total)

[--] 3 tests from JWTTest
[ RUN  ] JWTTest.Parse
[   OK ] JWTTest.Parse (8 ms)
[ RUN  ] JWTTest.Create
[   OK ] JWTTest.Create (1 ms)
[ RUN  ] JWTTest.Stringify
[   OK ] JWTTest.Stringify (1 ms)
[--] 3 tests from JWTTest (14 ms total)

[--] 1 test from SSL
[ RUN  ] SSL.Disabled
[   OK ] SSL.Disabled (10 ms)
[--] 1 test from SSL (11 ms total)

[--] 17 tests from SSLTest
[ RUN  ] SSLTest.SSLSocket
```

- 
[libprocess-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65987/logs/libprocess-tests-stderr.log):

```
ABORT: 
(D:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/ssl/gtest.hpp:171): 
Could not generate certificate: Failed to set common name: 
X509_NAME_add_entry_by_txt
```

- 
[mesos-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65987/logs/mesos-tests-cmake-stdout.log):

```
 D:\DCOS\mesos\mesos\src\java\jni\convert.cpp(415): warning C4267: 
'argument': conversion from 'size_t' to 'jsize', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\java\jni\convert.cpp(437): warning C4267: 
'argument': conversion from 'size_t' to 'jsize', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\java\jni\convert.cpp(438): warning C4267: 
'argument': conversion from 'size_t' to 'jsize', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\java\jni\convert.cpp(460): warning C4267: 
'argument': conversion from 'size_t' to 'jsize', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\java\jni\convert.cpp(461): warning C4267: 
'argument': conversion from 'size_t' to 'jsize', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\java\jni\convert.cpp(500): warning C4267: 
'argument': conversion from 'size_t' to 'jsize', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\java\jni\convert.cpp(501): warning C4267: 
'argument': conversion from 'size_t' to 'jsize', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]


   "D:\DCOS\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
   "D:\DCOS\mesos\src\mesos.vcxproj" (default target) (25) ->
   (ClCompile target) -> 
 D:\DCOS\mesos\mesos\src\master\validation.cpp(1508): error C2001: 
newline in constant [D:\DCOS\mesos\src\mesos.vcxproj]
 D:\DCOS\mesos\mesos\src\master\validation.cpp(1509): error C2146: 
syntax error: missing ')' before identifier 'containers' 
[D:\DCOS\mesos\src\mesos.vcxproj]
 D:\DCOS\mesos\mesos\src\master\validation.cpp(1509): error C2146: 
syntax error: missing ';' before identifier 'containers' 
[D:\DCOS\mesos\src\mesos.vcxproj]
 D:\DCOS\mesos\mesos\src\master\validation.cpp(1509): error C2065: 
'containers': undeclared identifier [D:\DCOS\mesos\src\mesos.vcxproj]
 D:\DCOS\mesos\mesos\src\master\validation.cpp(1509): error C2143: 
syntax error: missing ';' before '!' [D:\DCOS\mesos\src\mesos.vcxproj]
 D:\DCOS\mesos\mesos\src\master\validation.cpp(1509): error C2065: 
'joining': undeclared identifier [D:\DCOS\mesos\src\mesos.vcxproj]
 D:\DCOS\mesos\mesos\src\master\validation.cpp(1509): error C2001: 
newline in constant [D:\DCOS\mesos\src\mesos.vcxproj]
 D:\DCOS\mesos\mesos\src\master\validation.cpp(1509): error C2015: too 
many characters in constant [D:\DCOS\mesos\src\mesos.vcxproj]
 D:\DCOS\mesos\mesos\src\master\validation.cpp(1509): error C2146: 
syntax error: missing ';' before identifier 'parent' 
[D:\DCOS\mesos\src\mesos.vcxproj]
 D:\DCOS\mesos\mesos\src\master\validation.cpp(1509): error C2065: 
'parent': undeclared identifier 

Re: Review Request 65987: Allow nested containers in pods to have separate namespaces(Ref: MESOS-8534).

[Sagar Patwardhan]

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

(Updated March 15, 2018, 4:30 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Added validation to disallow HTTP(s) and TCP health checks.


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


Repository: mesos


Description
---

Continued from https://github.com/apache/mesos/pull/263


Diffs (updated)
-

  src/master/validation.cpp f0b86775b 
  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 1d01915c2 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 896656987 


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

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


Testing
---

Manually tested.

Working on unit tests.


Thanks,

Sagar Patwardhan

Re: Review Request 66076: Used raw string literal in protobuf tests to avoid escaping.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66076']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66076

Relevant logs:

- 
[libprocess-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66076/logs/libprocess-tests-stdout.log):

```
[ RUN  ] TimeTest.Output
[   OK ] TimeTest.Output (0 ms)
[--] 4 tests from TimeTest (5 ms total)

[--] 3 tests from TimeSeriesTest
[ RUN  ] TimeSeriesTest.Set
[   OK ] TimeSeriesTest.Set (0 ms)
[ RUN  ] TimeSeriesTest.Sparsify
[   OK ] TimeSeriesTest.Sparsify (1 ms)
[ RUN  ] TimeSeriesTest.Truncate
[   OK ] TimeSeriesTest.Truncate (2 ms)
[--] 3 tests from TimeSeriesTest (8 ms total)

[--] 3 tests from JWTTest
[ RUN  ] JWTTest.Parse
[   OK ] JWTTest.Parse (11 ms)
[ RUN  ] JWTTest.Create
[   OK ] JWTTest.Create (1 ms)
[ RUN  ] JWTTest.Stringify
[   OK ] JWTTest.Stringify (1 ms)
[--] 3 tests from JWTTest (28 ms total)

[--] 1 test from SSL
[ RUN  ] SSL.Disabled
[   OK ] SSL.Disabled (9 ms)
[--] 1 test from SSL (10 ms total)

[--] 17 tests from SSLTest
[ RUN  ] SSLTest.SSLSocket
```

- 
[libprocess-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66076/logs/libprocess-tests-stderr.log):

```
ABORT: 
(D:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/ssl/gtest.hpp:171): 
Could not generate certificate: Failed to set common name: 
X509_NAME_add_entry_by_txt
```

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66076/logs/mesos-tests-stdout.log):

```
[   OK ] ContentType/SchedulerTest.Revive/1 (259 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/0
[   OK ] ContentType/SchedulerTest.Suppress/0 (250 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/1
[   OK ] ContentType/SchedulerTest.Suppress/1 (286 ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0 (263 ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1 (289 ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0
[   OK ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0 (310 
ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/1
[   OK ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/1 (336 
ms)
[ RUN  ] ContentType/SchedulerTest.Message/0
[   OK ] ContentType/SchedulerTest.Message/0 (327 ms)
[ RUN  ] ContentType/SchedulerTest.Message/1
[   OK ] ContentType/SchedulerTest.Message/1 (341 ms)
[ RUN  ] ContentType/SchedulerTest.Request/0
[   OK ] ContentType/SchedulerTest.Request/0 (103 ms)
[ RUN  ] ContentType/SchedulerTest.Request/1
[   OK ] ContentType/SchedulerTest.Request/1 (107 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/0
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (85 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/1
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (86 ms)
[--] 32 tests from ContentType/SchedulerTest (16842 ms total)

[--] 4 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66076/logs/mesos-tests-stderr.log):

```
I0315 03:36:25.695457 13804 master.cpp:1678] Recovering from registrar
I0315 03:36:25.697437  7632 registrar.cpp:391] Successfully fetched the 
registry (0B) in 963840ns
I0315 03:36:25.697437  7632 registrar.cpp:495] Applied 1 operations in 0ns; 
attempting to update the registry
I0315 03:36:25.698453  7632 registrar.cpp:552] Successfully updated the 
registry in 1.017088ms
I0315 03:36:25.699432  7632 registrar.cpp:424] Successfully recovered registrar
I0315 03:36:25.700461 12664 master.cpp:1792] Recovered 0 agents from the 
registry (239B); allowing 10mins for agents to reregister
I0315 03:36:25.707435  4240 scheduler.cpp:188] Version: 1.6.0
I0315 03:36:25.708436 10772 scheduler.cpp:311] Using default 'basic' HTTP 
authenticatee
I0315 03:36:25.708436 10804 scheduler.cpp:494] New master detected at 
master@10.3.1.8:62900
I0315 03:36:25.716439 13804 scheduler.cpp:468] Re-detecting master
I0315 03:36:25.718420 13804 scheduler.cpp:494] New master detected at 
master@10.3.1.8:62900
I0315 

Re: Review Request 59989: Added a test `ProtobufTest.JsonifyMap`.

[Qian Zhang]

> On March 2, 2018, 9:33 a.m., Benjamin Mahler wrote:
> > 3rdparty/stout/tests/protobuf_tests.cpp
> > Lines 675-676 (patched)
> > 
> >
> > Do you want to use a raw string literal here to avoid all the escaping? 
> > (6) from here: http://en.cppreference.com/w/cpp/language/string_literal
> 
> Qian Zhang wrote:
> Yeah, I thought that before. However I see all other protobuf tests in 
> this file are using `strings::remove()`, I think it may not appropriate to 
> change them to use raw string in this patch. How about I post a follow-up 
> patch to change all of them?
> 
> Qian Zhang wrote:
> Second thought, it seems we cannot do it in these tests, because raw 
> string literal will keep all the whitespaces and newlines, but 
> `stringify(JSON::Object)` will not, so the comparison between them in the 
> test will always fail.

We can combine the use of raw string literal and `strings::remove()`, i.e., use 
`strings:remove()` to remove the spaces and newlines in the raw string literal.

Here is the patch:
https://reviews.apache.org/r/66076/


- Qian


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


On March 6, 2018, 5:29 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59989/
> ---
> 
> (Updated March 6, 2018, 5:29 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Mahler, Chun-Hung Hsiao, 
> and Zhitao Li.
> 
> 
> Bugs: MESOS-7656
> https://issues.apache.org/jira/browse/MESOS-7656
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `ProtobufTest.JsonifyMap`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/protobuf_tests.cpp 
> be35ad0d1e16501df67752a1818f79751419a43d 
> 
> 
> Diff: https://reviews.apache.org/r/59989/diff/5/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 66076: Used raw string literal in protobuf tests to avoid escaping.

[Qian Zhang]

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

Review request for mesos, Benjamin Mahler and Chun-Hung Hsiao.


Repository: mesos


Description
---

Used raw string literal in protobuf tests to avoid escaping.


Diffs
-

  3rdparty/stout/tests/protobuf_tests.cpp 
a0ef1d110204fe2868ac9b686da090d9e7b3d2a3 


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


Testing
---

sudo make check


Thanks,

Qian Zhang

Re: Review Request 65833: Displayed resource provider resources in GET_AGENTS response.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [65587, 65588, 65589, 65590, 65591, 65673, 65674, 65832, 65833]

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 Feb. 28, 2018, 12:55 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65833/
> ---
> 
> (Updated Feb. 28, 2018, 12:55 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8388
> https://issues.apache.org/jira/browse/MESOS-8388
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Displayed resource provider resources in GET_AGENTS response.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 6622e1b82861f380346f505eca326f7174dd8bd6 
>   include/mesos/v1/master/master.proto 
> 6034bd5af8ad764f625f9fe80be08b48707bbadb 
>   src/common/protobuf_utils.cpp 9c5fb97afb58f98013b79f3cbaea7dacc3603271 
>   src/tests/api_tests.cpp e81d6ed25984c14e4143a048848c96e44e4f16e4 
> 
> 
> Diff: https://reviews.apache.org/r/65833/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65997: Made `PROTOC_GENERATE` compile proto files from 3rd-party libraries.

[Andrew Schwartzmeyer]

> On March 14, 2018, 6:45 p.m., Andrew Schwartzmeyer wrote:
> > Ship It!

Tested on Windows and Linux (at least the build and tests all pass).


- Andrew


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


On March 14, 2018, 4:05 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65997/
> ---
> 
> (Updated March 14, 2018, 4:05 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8657
> https://issues.apache.org/jira/browse/MESOS-8657
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> PROTOC_GENERATE now has the following new features:
>   (1) With the `LIB` option, compile .proto files found in a third-party
>   specification library.
>   (2) Provides the `GRPC` option that, once we support gRPC in CMake,
>   will generate the `.grpc.pb.h` and `.grpc.pb.cc` files.
>   (3) With the `LIB` option, append to list variable `PUBLIC_PROTO_PATH`
>   or `INTERNAL_PROTO_PATH` the fully qualified path to the library's
>   include directory, and append to list variable
>   `PUBLIC_PROTOBUF_INCLUDE_DIR` or `INTERNAL_PROTOBUF_INCLUDE_DIR`
>   the fully qualified path to the directory where the generated
>   `.pb.h` files are placed.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
>   src/cmake/MesosProtobuf.cmake ef90c15a6f2afaedaf7559c4a411098924528505 
> 
> 
> Diff: https://reviews.apache.org/r/65997/diff/4/
> 
> 
> Testing
> ---
> 
> `make check` with cmake.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65997: Made `PROTOC_GENERATE` compile proto files from 3rd-party libraries.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On March 14, 2018, 4:05 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65997/
> ---
> 
> (Updated March 14, 2018, 4:05 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8657
> https://issues.apache.org/jira/browse/MESOS-8657
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> PROTOC_GENERATE now has the following new features:
>   (1) With the `LIB` option, compile .proto files found in a third-party
>   specification library.
>   (2) Provides the `GRPC` option that, once we support gRPC in CMake,
>   will generate the `.grpc.pb.h` and `.grpc.pb.cc` files.
>   (3) With the `LIB` option, append to list variable `PUBLIC_PROTO_PATH`
>   or `INTERNAL_PROTO_PATH` the fully qualified path to the library's
>   include directory, and append to list variable
>   `PUBLIC_PROTOBUF_INCLUDE_DIR` or `INTERNAL_PROTOBUF_INCLUDE_DIR`
>   the fully qualified path to the directory where the generated
>   `.pb.h` files are placed.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
>   src/cmake/MesosProtobuf.cmake ef90c15a6f2afaedaf7559c4a411098924528505 
> 
> 
> Diff: https://reviews.apache.org/r/65997/diff/4/
> 
> 
> Testing
> ---
> 
> `make check` with cmake.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66015: Built CSI spec proto files with CMake.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On March 14, 2018, 5:29 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66015/
> ---
> 
> (Updated March 14, 2018, 5:29 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8657
> https://issues.apache.org/jira/browse/MESOS-8657
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Built CSI spec proto files with CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
>   3rdparty/cmake/Versions.cmake 94b0d8048412e00e2480f45e7ce4e6494da4bd5d 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
> 
> 
> Diff: https://reviews.apache.org/r/66015/diff/3/
> 
> 
> Testing
> ---
> 
> `make check` with CMake.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65997: Made `PROTOC_GENERATE` compile proto files from 3rd-party libraries.

[Chun-Hung Hsiao]

> On March 14, 2018, 11:51 p.m., Andrew Schwartzmeyer wrote:
> > src/cmake/MesosProtobuf.cmake
> > Lines 105 (patched)
> > 
> >
> > There needs to be an equivalent to these two `set(PROTO` lines in the 
> > `PROTOC_INTERNAL` logic to resolve the following dependency error:
> > 
> > ```
> > > ninja tests
> > ninja: error: '3rdparty/csi-0.1.0/src/csi-0.1.0/csi.proto', needed by 
> > 'include/csi/csi.pb.cc', missing and no known rule to make it
> > ```

Missing `csi.proto` as a byproduct. Fixed in r66015.


- Chun-Hung


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


On March 14, 2018, 11:05 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65997/
> ---
> 
> (Updated March 14, 2018, 11:05 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8657
> https://issues.apache.org/jira/browse/MESOS-8657
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> PROTOC_GENERATE now has the following new features:
>   (1) With the `LIB` option, compile .proto files found in a third-party
>   specification library.
>   (2) Provides the `GRPC` option that, once we support gRPC in CMake,
>   will generate the `.grpc.pb.h` and `.grpc.pb.cc` files.
>   (3) With the `LIB` option, append to list variable `PUBLIC_PROTO_PATH`
>   or `INTERNAL_PROTO_PATH` the fully qualified path to the library's
>   include directory, and append to list variable
>   `PUBLIC_PROTOBUF_INCLUDE_DIR` or `INTERNAL_PROTOBUF_INCLUDE_DIR`
>   the fully qualified path to the directory where the generated
>   `.pb.h` files are placed.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
>   src/cmake/MesosProtobuf.cmake ef90c15a6f2afaedaf7559c4a411098924528505 
> 
> 
> Diff: https://reviews.apache.org/r/65997/diff/4/
> 
> 
> Testing
> ---
> 
> `make check` with cmake.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66015: Built CSI spec proto files with CMake.

[Chun-Hung Hsiao]

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

(Updated March 15, 2018, 12:29 a.m.)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
Joseph Wu.


Changes
---

Fixed the commit message and added `csi.proto` as a byproduct.


Summary (updated)
-

Built CSI spec proto files with CMake.


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


Repository: mesos


Description (updated)
---

Built CSI spec proto files with CMake.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
  3rdparty/cmake/Versions.cmake 94b0d8048412e00e2480f45e7ce4e6494da4bd5d 
  src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 


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

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


Testing (updated)
---

`make check` with CMake.


Thanks,

Chun-Hung Hsiao

Re: Review Request 66046: CMake: Patched Boost to remove spurious warnings.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66007', '66008', '66009', '66010', '66011', '66012', 
'66013', '66014', '66046']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66046

Relevant logs:

- 
[libprocess-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66046/logs/libprocess-tests-stdout.log):

```
[ RUN  ] TimeTest.Output
[   OK ] TimeTest.Output (0 ms)
[--] 4 tests from TimeTest (5 ms total)

[--] 3 tests from TimeSeriesTest
[ RUN  ] TimeSeriesTest.Set
[   OK ] TimeSeriesTest.Set (0 ms)
[ RUN  ] TimeSeriesTest.Sparsify
[   OK ] TimeSeriesTest.Sparsify (1 ms)
[ RUN  ] TimeSeriesTest.Truncate
[   OK ] TimeSeriesTest.Truncate (2 ms)
[--] 3 tests from TimeSeriesTest (5 ms total)

[--] 3 tests from JWTTest
[ RUN  ] JWTTest.Parse
[   OK ] JWTTest.Parse (8 ms)
[ RUN  ] JWTTest.Create
[   OK ] JWTTest.Create (0 ms)
[ RUN  ] JWTTest.Stringify
[   OK ] JWTTest.Stringify (1 ms)
[--] 3 tests from JWTTest (13 ms total)

[--] 1 test from SSL
[ RUN  ] SSL.Disabled
[   OK ] SSL.Disabled (10 ms)
[--] 1 test from SSL (10 ms total)

[--] 17 tests from SSLTest
[ RUN  ] SSLTest.SSLSocket
```

- 
[libprocess-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66046/logs/libprocess-tests-stderr.log):

```
ABORT: 
(D:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/ssl/gtest.hpp:171): 
Could not generate certificate: Failed to set common name: 
X509_NAME_add_entry_by_txt
```

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66046/logs/mesos-tests-stdout.log):

```
[   OK ] ContentType/SchedulerTest.Revive/1 (281 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/0
[   OK ] ContentType/SchedulerTest.Suppress/0 (250 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/1
[   OK ] ContentType/SchedulerTest.Suppress/1 (275 ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0 (267 ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1 (284 ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0
[   OK ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0 (303 
ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/1
[   OK ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/1 (322 
ms)
[ RUN  ] ContentType/SchedulerTest.Message/0
[   OK ] ContentType/SchedulerTest.Message/0 (342 ms)
[ RUN  ] ContentType/SchedulerTest.Message/1
[   OK ] ContentType/SchedulerTest.Message/1 (342 ms)
[ RUN  ] ContentType/SchedulerTest.Request/0
[   OK ] ContentType/SchedulerTest.Request/0 (100 ms)
[ RUN  ] ContentType/SchedulerTest.Request/1
[   OK ] ContentType/SchedulerTest.Request/1 (112 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/0
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (84 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/1
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (81 ms)
[--] 32 tests from ContentType/SchedulerTest (13384 ms total)

[--] 4 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66046/logs/mesos-tests-stderr.log):

```
I0315 00:14:54.710726  2764 scheduler.cpp:472] Lost leading master
I0315 00:14:54.715724 11344 master.cpp:1136] Master terminating
I0315 00:14:54.742727 11344 cluster.cpp:172] Creating default 'local' authorizer
I0315 00:14:54.749727  8668 master.cpp:463] Master 
b5cc416d-b682-44f4-a089-17c3402f819e 
(win-bld-srv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) started on 
10.3.1.8:61003
I0315 00:14:54.750725  8668 master.cpp:465] Flags at startup: --acls="" 
--agent_ping_timeout="15secs" --agent_reregister_timeout="10mins" 
--allocation_interval="1secs" --allocator="HierarchicalDRF" 
--authenticate_agents="true" --authenticate_frameworks="true" 
--authenticate_http_frameworks="true" --authenticate_http_readonly="true" 
--authenticate_http_readwrite="true" --authenticators="crammd5" 
--authorizers="local" 
--credentials="C:\Users\mesos\AppData\Local\Temp\XGWMTS\credentials" 
--filter_gpu_resources="true" 

Re: Review Request 65679: Removed direct unmock calls and added missing mock call expectations.

[Chun-Hung Hsiao]

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




src/tests/slave_tests.cpp
Lines 4158-4160 (patched)


Can we do the following:
```
EXPECT_CALL(sched, executorLost(...))
  .Times(AtMost(1));
```
And remove `executorLost`, since it's not in the original test?



src/tests/slave_tests.cpp
Lines 4671 (patched)


Since we dispatch `unmocked__run` into the slave context, we can just leave 
the `FutureSatisfy` as is, to be consistent with other tests.



src/tests/slave_tests.cpp
Lines 4863-4865 (patched)


Ditto.



src/tests/slave_tests.cpp
Lines 4986-4988 (patched)


Ditto.



src/tests/slave_tests.cpp
Line 4982 (original), 5022 (patched)


Is it guaranteed that this lambda will finish before 
`exitedExecutorMessage` is satisfied, so we can make sure that is function 
won't be called after the variables are destructed?

If not, we probably should do the following instead:
```
Future unmocked___run = process::dispatch(slave.get()->pid, [&] {
  slave.get()->mock()->unmocked___run(
...);

  return Nothing();
});
```
And do `AWAIT_READY(unmocked___run)` at the end of the test.



src/tests/slave_tests.cpp
Lines 5179 (patched)


Is it guaranteed that this lambda will finish before `executorShutdown` is 
satisfied, so we can make sure that is function won't be called after the 
variables are destructed?

If not, we need to do the same thing I suggested above.



src/tests/slave_tests.cpp
Lines 5382-5389 (original), 5431-5443 (patched)


The high-level guideline is try to avoid using/exposing `Promise` unless 
necessary. Let's return a `Nothing` in the lambda instead, as I suggested above.


- Chun-Hung Hsiao


On Feb. 16, 2018, 1:22 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65679/
> ---
> 
> (Updated Feb. 16, 2018, 1:22 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8611
> https://issues.apache.org/jira/browse/MESOS-8611
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Directly invoking unmock calls in the test process can potentially
> cause races with the real mock slave process. It is more robust to
> dispatch the unmock calls to the real mock slave process.
> 
> Also added several mock expectations to avoid "uninteresting mock
> call" test warnings.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp e253317a67019302f18afe11e2a314e716cec226 
> 
> 
> Diff: https://reviews.apache.org/r/65679/diff/4/
> 
> 
> Testing
> ---
> 
> `./bin/mesos-tests.sh --gtest_filter=*SlaveTest* --gtest_repeat=-1 
> --gtest_break_on_failure` runs forever :)
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 65997: Made `PROTOC_GENERATE` compile proto files from 3rd-party libraries.

[Andrew Schwartzmeyer]

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




src/cmake/MesosProtobuf.cmake
Lines 105 (patched)


There needs to be an equivalent to these two `set(PROTO` lines in the 
`PROTOC_INTERNAL` logic to resolve the following dependency error:

```
> ninja tests
ninja: error: '3rdparty/csi-0.1.0/src/csi-0.1.0/csi.proto', needed by 
'include/csi/csi.pb.cc', missing and no known rule to make it
```



src/cmake/MesosProtobuf.cmake
Line 111 (original), 215 (patched)


This line sets up the build dependency between `${CC}` and `${PROTO}`. The 
build error noted above is due to `${PROTO}` being empty when `${PROTOC_LIB}` 
is true.


- Andrew Schwartzmeyer


On March 14, 2018, 4:05 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65997/
> ---
> 
> (Updated March 14, 2018, 4:05 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8657
> https://issues.apache.org/jira/browse/MESOS-8657
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> PROTOC_GENERATE now has the following new features:
>   (1) With the `LIB` option, compile .proto files found in a third-party
>   specification library.
>   (2) Provides the `GRPC` option that, once we support gRPC in cmake,
>   will generate the `.grpc.pb.h` and `.grpc.pb.cc` files.
>   (3) With the `LIB` option, append to list variable `PUBLIC_PROTO_PATH`
>   or `INTERNAL_PROTO_PATH` the fully qualified path to the library's
>   include directory, and append to list variable
>   `PUBLIC_PROTOBUF_INCLUDE_DIR` or `INTERNAL_PROTOBUF_INCLUDE_DIR`
>   the fully qualified path to the directory where the generated
>   `.pb.h` files are placed.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
>   src/cmake/MesosProtobuf.cmake ef90c15a6f2afaedaf7559c4a411098924528505 
> 
> 
> Diff: https://reviews.apache.org/r/65997/diff/4/
> 
> 
> Testing
> ---
> 
> `make check` with cmake.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 64618: Implemented the master's `ACKNOWLEDGE_OPERATION_STATUS` handler.

[Gaston Kleiman]

> On March 13, 2018, 4:19 p.m., Greg Mann wrote:
> > src/master/master.cpp
> > Lines 5960 (patched)
> > 
> >
> > Can we move this to?
> > 
> > `std::move(statusUuid.toBytes())`
> > 
> > Looks like `toBytes()` returns a `std::string` and `set_value` has an 
> > overload for rvalue reference, so I think we can avoid a copy here if we 
> > move?
> 
> Gaston Kleiman wrote:
> Since `statusUuid.toBytes()` here is an rvalue, I think that 
> `set_value(std::string&&)` should be automatically used. I updated the patch 
> anyway for the sake of explicitness/readability.

Well.. I just noticed that adding `std::move` breaks the build, so I reverted 
that part. Here's the error that it triggers:

```
master/master.cpp:6006:44: error: moving a temporary object prevents copy 
elision [-Werror,-Wpessimizing-move]
  message.mutable_status_uuid()->set_value(std::move(statusUuid.toBytes()));
   ^
../../src/master/master.cpp:6006:44: note: remove std::move call here
  message.mutable_status_uuid()->set_value(std::move(statusUuid.toBytes()));
   ^~~
```


- Gaston


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


On March 14, 2018, 4:50 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64618/
> ---
> 
> (Updated March 14, 2018, 4:50 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8184
> https://issues.apache.org/jira/browse/MESOS-8184
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the master's `ACKNOWLEDGE_OPERATION_STATUS` handler.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp cf03d8bf7df583c849a3b2a326d3772820181b0f 
>   src/master/master.hpp 8bf2c763dafdb7df55c46a56f2ff66f2a951d947 
>   src/master/master.cpp 223ebf29ac4dd1dea9080e4bef4b2d4d064d847f 
> 
> 
> Diff: https://reviews.apache.org/r/64618/diff/8/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 64618: Implemented the master's `ACKNOWLEDGE_OPERATION_STATUS` handler.

[Gaston Kleiman]

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

(Updated March 14, 2018, 4:50 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Reverted last revision to fix the build.


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


Repository: mesos


Description
---

Implemented the master's `ACKNOWLEDGE_OPERATION_STATUS` handler.


Diffs (updated)
-

  src/master/http.cpp cf03d8bf7df583c849a3b2a326d3772820181b0f 
  src/master/master.hpp 8bf2c763dafdb7df55c46a56f2ff66f2a951d947 
  src/master/master.cpp 223ebf29ac4dd1dea9080e4bef4b2d4d064d847f 


Diff: https://reviews.apache.org/r/64618/diff/8/

Changes: https://reviews.apache.org/r/64618/diff/7-8/


Testing
---

`make check` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 65997: Made `PROTOC_GENERATE` compile proto files from 3rd-party libraries.

[Andrew Schwartzmeyer]

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



Let me apply these patches on Windows and make sure everything is G2G before a 
ship it. Just a few minutes...

- Andrew Schwartzmeyer


On March 14, 2018, 4:05 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65997/
> ---
> 
> (Updated March 14, 2018, 4:05 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8657
> https://issues.apache.org/jira/browse/MESOS-8657
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> PROTOC_GENERATE now has the following new features:
>   (1) With the `LIB` option, compile .proto files found in a third-party
>   specification library.
>   (2) Provides the `GRPC` option that, once we support gRPC in cmake,
>   will generate the `.grpc.pb.h` and `.grpc.pb.cc` files.
>   (3) With the `LIB` option, append to list variable `PUBLIC_PROTO_PATH`
>   or `INTERNAL_PROTO_PATH` the fully qualified path to the library's
>   include directory, and append to list variable
>   `PUBLIC_PROTOBUF_INCLUDE_DIR` or `INTERNAL_PROTOBUF_INCLUDE_DIR`
>   the fully qualified path to the directory where the generated
>   `.pb.h` files are placed.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
>   src/cmake/MesosProtobuf.cmake ef90c15a6f2afaedaf7559c4a411098924528505 
> 
> 
> Diff: https://reviews.apache.org/r/65997/diff/4/
> 
> 
> Testing
> ---
> 
> `make check` with cmake.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66015: Built CSI spec proto files with cmake.

[Andrew Schwartzmeyer]

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


Ship it!




Nit on the commit message: `s/cmake/CMake/g`

- Andrew Schwartzmeyer


On March 13, 2018, 7:57 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66015/
> ---
> 
> (Updated March 13, 2018, 7:57 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8657
> https://issues.apache.org/jira/browse/MESOS-8657
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Built CSI spec proto files with cmake.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
>   3rdparty/cmake/Versions.cmake 94b0d8048412e00e2480f45e7ce4e6494da4bd5d 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
> 
> 
> Diff: https://reviews.apache.org/r/66015/diff/2/
> 
> 
> Testing
> ---
> 
> `make check` with cmake.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66060: Added end-to-end tests for operation feedback.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [63991, 63992, 63994, 65300, 64618, 65993, 65994, 66056, 
66057, 66058, 66059, 66060]

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 March 14, 2018, 9:29 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66060/
> ---
> 
> (Updated March 14, 2018, 9:29 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8201
> https://issues.apache.org/jira/browse/MESOS-8201
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added end-to-end tests for operation feedback.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 264d42b57fe1a8ea04c1de0a10112878c7b45d39 
> 
> 
> Diff: https://reviews.apache.org/r/66060/diff/1/
> 
> 
> Testing
> ---
> 
> The tests passed over 1000 iterations on GNU/Linux.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66046: CMake: Patched Boost to remove spurious warnings.

[Andrew Schwartzmeyer]

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

(Updated March 14, 2018, 4:18 p.m.)


Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
Kordich, Joseph Wu, and Michael Park.


Changes
---

Rebased.


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


Repository: mesos


Description
---

On Windows, Boost explicitly checks if it's being compiled with a
compiler version they've tested. As we tend to update Visual Studio
whenever a stable update is available, this leads to a Boost warning,
"Unknown compiler version..." being emitted repeatedly (for every file
which includes a Boost header).

If it were emitted once, we would leave it be, but because it's
repeated hundreds of times, and is mostly harmless, we patch the build
to remove the warning itself. Unfortunately, there is no compile-time
option to disable the warning instead of a patch.

Note that it is not straight-forward to generate a patch file for
Boost. The steps were: clone the Boost repo recursively, go the
`libs/config` submodule, edit the `visualc.hpp` file, go to the
`include` subdirectory, and use `git diff --relative` to generate the
patch with the corrects paths expected for the extracted tarball.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
  3rdparty/boost-1.65.0.patch PRE-CREATION 


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

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


Testing
---

Built on Windows; no more warning.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66013: Windows: Made ZooKeeper use default CRT linking.

[Andrew Schwartzmeyer]

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

(Updated March 14, 2018, 4:16 p.m.)


Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
Kordich, Joseph Wu, and Michael Park.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Also deleted extraneous patch files.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
  3rdparty/zookeeper-06d3f3f.patch 7f66875c7ef8bea3cc077d064b90e056c706487f 
  3rdparty/zookeeper-3.4.8.patch c532a696f8bf3373b7341ddf32dcf9c528ee4412 
  3rdparty/zookeeper-3.5.2-alpha.patch 6d5db536437b22a2903c1fa3348613e12fd35df0 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66012: CMake: Split `CMAKE_FORWARD_ARGS` into `C` and `CXX` versions.

[Andrew Schwartzmeyer]

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

(Updated March 14, 2018, 4:15 p.m.)


Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
Kordich, Joseph Wu, and Michael Park.


Changes
---

Rebased.


Repository: mesos


Description
---

By passing `CMAKE_C_FORWARD_ARGS` to C libraries, e.g. curl, libapr,
libevent, and zlib, we avoid CMake warning us about set but unused
variables (since they do not use the `CXX` variables).


Diffs (updated)
-

  3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66009: CMake: Added `-Wno-unused-local-typedefs` to Boost interface.

[Andrew Schwartzmeyer]

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

(Updated March 14, 2018, 4:11 p.m.)


Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
Kordich, Joseph Wu, and Michael Park.


Changes
---

Updated versions.


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


Repository: mesos


Description (updated)
---

As found in the Autotools builds, GCC and Clang will emit an used
local typedef warning for headers including Boost. Since it is
3rdparty code, we disable this warning in its interface.

The versions were updated with the latest known scenario.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
  configure.ac 30fbadc32d1d96f719d45fa8067f975283c25507 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66008: CMake: Enabled compiler warnings.

[Andrew Schwartzmeyer]

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

(Updated March 14, 2018, 4:10 p.m.)


Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
Kordich, Joseph Wu, and Michael Park.


Changes
---

Address feedback.


Bugs: MESOS-8033 and MESOS-8658
https://issues.apache.org/jira/browse/MESOS-8033
https://issues.apache.org/jira/browse/MESOS-8658


Repository: mesos


Description (updated)
---

We had previously been using the default sets of warnings, but now we
use the same warnings as on Autotools. This also means that we can
safely turn on the treat-warnings-as-errors (at least for the Mesos
code). However, doing so requires that we disable two common
possible-loss-of-data warnings on Windows that are not part of the
GNU/Clang default warnings.

Because `-Werror` semantics can be burdensome, it can be turned off at
configuration time with `-DENABLE_WERROR=FALSE`. It is on by default.

This also replaces the use of `string(APPEND CMAKE_CXX_FLAGS)` with
the canonical command `add_compile_options`. Although generally the
use of `target_compile_options` is preferred, it would currently
result in a lot more churn, and the build already supports setting
these flags globally.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake efee36c1ffda096a97af23d481fc0d0903124e54 
  docs/configuration/cmake.md 1e34657ea55fa324f65f865f7d0a67084c6719d9 
  src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 65997: Made `PROTOC_GENERATE` compile proto files from 3rd-party libraries.

[Chun-Hung Hsiao]

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

(Updated March 14, 2018, 11:05 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
Joseph Wu.


Changes
---

Addressed Andy's comments.


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


Repository: mesos


Description
---

PROTOC_GENERATE now has the following new features:
  (1) With the `LIB` option, compile .proto files found in a third-party
  specification library.
  (2) Provides the `GRPC` option that, once we support gRPC in cmake,
  will generate the `.grpc.pb.h` and `.grpc.pb.cc` files.
  (3) With the `LIB` option, append to list variable `PUBLIC_PROTO_PATH`
  or `INTERNAL_PROTO_PATH` the fully qualified path to the library's
  include directory, and append to list variable
  `PUBLIC_PROTOBUF_INCLUDE_DIR` or `INTERNAL_PROTOBUF_INCLUDE_DIR`
  the fully qualified path to the directory where the generated
  `.pb.h` files are placed.


Diffs (updated)
-

  src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
  src/cmake/MesosProtobuf.cmake ef90c15a6f2afaedaf7559c4a411098924528505 


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

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


Testing
---

`make check` with cmake.


Thanks,

Chun-Hung Hsiao

Re: Review Request 65997: Made `PROTOC_GENERATE` compile proto files from 3rd-party libraries.

[Chun-Hung Hsiao]

> On March 14, 2018, 7:50 p.m., Andrew Schwartzmeyer wrote:
> > src/cmake/MesosProtobuf.cmake
> > Lines 94 (patched)
> > 
> >
> > We put CPP_OUT files under an build/include/lib?
> 
> Chun-Hung Hsiao wrote:
> Yeah. The reason is that, for CSI support we introduce the 
> `DiskProfileAdaptor` Mesos module that uses `csi::VolumeCapability`: 
> https://github.com/apache/mesos/blob/1.5.x/include/mesos/resource_provider/storage/disk_profile.hpp#L66
> For this reason, we put `spec.hpp` under `include/csi`, which includes 
> `csi/csi.pb.h` from the `build` dir.
> 
> Now I rethink about this, actually we don't need the 
> `{PUBLIC,INTERNAL}_PROTOBUF_INCLUDE_DIR` at all since we have `csi/` 
> prepended in `spec.hpp`. I'll remove this var.

Hmm due to the way the `protoc`-generated files include headers, we still need 
to put "include/csi" in the include path.


- Chun-Hung


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


On March 14, 2018, 2:56 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65997/
> ---
> 
> (Updated March 14, 2018, 2:56 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8657
> https://issues.apache.org/jira/browse/MESOS-8657
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> PROTOC_GENERATE now has the following new features:
>   (1) With the `LIB` option, compile .proto files found in a third-party
>   specification library.
>   (2) Provides the `GRPC` option that, once we support gRPC in cmake,
>   will generate the `.grpc.pb.h` and `.grpc.pb.cc` files.
>   (3) With the `LIB` option, append to list variable `PUBLIC_PROTO_PATH`
>   or `INTERNAL_PROTO_PATH` the fully qualified path to the library's
>   include directory, and append to list variable
>   `PUBLIC_PROTOBUF_INCLUDE_DIR` or `INTERNAL_PROTOBUF_INCLUDE_DIR`
>   the fully qualified path to the directory where the generated
>   `.pb.h` files are placed.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
>   src/cmake/MesosProtobuf.cmake ef90c15a6f2afaedaf7559c4a411098924528505 
> 
> 
> Diff: https://reviews.apache.org/r/65997/diff/3/
> 
> 
> Testing
> ---
> 
> `make check` with cmake.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65899: Use launch actions in `MesosContainerizerLaunchHelper` instead.

[Jason Lai]

> On March 7, 2018, 2:28 a.m., Eric Chung wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Lines 583 (patched)
> > 
> >
> > why is the explicit delete needed here? was it not being cleaned up 
> > previously?

A `MesosContainerizerLauncherHelper` instance was initiated as a raw pointer to 
the heap. So the intention was that the receiver takes care of managing the 
instance's lifecycle.

However, I think that we can have something smarter like wrapping the raw 
pointer with libprocess' `Owned` as a uniquely owned pointer. Will do that with 
a revision.


- Jason


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


On March 5, 2018, 7:31 a.m., Jason Lai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65899/
> ---
> 
> (Updated March 5, 2018, 7:31 a.m.)
> 
> 
> Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, 
> James Peach, and Zhitao Li.
> 
> 
> Bugs: MESOS-8257
> https://issues.apache.org/jira/browse/MESOS-8257
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed launch actions in `src/slave/containerizer/mesos/launch.cpp`
> and replaced with those in a `MesosContainerLauncherHelper` subclass
> instead.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 75b7eaf9cd62d6b5f02896175168b651f4517e12 
> 
> 
> Diff: https://reviews.apache.org/r/65899/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Lai
> 
>

Re: Review Request 66046: CMake: Patched Boost to remove spurious warnings.

[Jeff Coffler]

> On March 14, 2018, 6:44 p.m., Andrew Schwartzmeyer wrote:
> > So, this patch is fairly temporary. The tip of `master` of Boost (of that 
> > submodule) has this commit:
> > 
> > ```
> > commit 5ad073063 (origin/develop, origin/HEAD, develop)
> > Author: jzmaddock 
> > Date:   Wed Mar 7 18:02:01 2018 +
> > 
> > visualc.hpp: Disable warning about outdated config.
> > 
> > diff --git a/include/boost/config/compiler/visualc.hpp 
> > b/include/boost/config/compiler/visualc.hpp
> > index 748d14076..c533c50df 100644
> > --- a/include/boost/config/compiler/visualc.hpp
> > +++ b/include/boost/config/compiler/visualc.hpp
> > @@ -346,6 +346,9 @@
> >  #  if defined(BOOST_ASSERT_CONFIG)
> >  # error "Boost.Config is older than your current compiler version."
> >  #  elif !defined(BOOST_CONFIG_SUPPRESS_OUTDATED_MESSAGE)
> > -  BOOST_PRAGMA_MESSAGE("Info: Boost.Config is older than your compiler 
> > version - probably nothing bad will happen - but you may wish to look for 
> > an updated Boost vers
> > ion. Define BOOST_CONFIG_SUPPRESS_OUTDATED_MESSAGE to suppress this 
> > message.")
> > +  //
> > +  // Disabled as of March 2018 - the pace of VS releases is hard to 
> > keep up with
> > +  // and in any case, we have relatively few defect macros defined now.
> > +  // BOOST_PRAGMA_MESSAGE("Info: Boost.Config is older than your 
> > compiler version - probably nothing bad will happen - but you may wish to 
> > look for an updated Boost v
> > ersion. Define BOOST_CONFIG_SUPPRESS_OUTDATED_MESSAGE to suppress this 
> > message.")
> >  #  endif
> >  #endif
> > ```
> > 
> > So when we get an updated version of Boost with this change, we can just 
> > add `BOOST_CONFIG_SUPPRESS_OUTDATED_MESSAGE` to the pre-processor 
> > definitions and delete this patch. But it is necessary for now (and even 
> > upstream they realized this code was awful).

With this in mind, I'm fine with the patch.


- Jeff


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


On March 13, 2018, 10:30 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66046/
> ---
> 
> (Updated March 13, 2018, 10:30 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8556
> https://issues.apache.org/jira/browse/MESOS-8556
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On Windows, Boost explicitly checks if it's being compiled with a
> compiler version they've tested. As we tend to update Visual Studio
> whenever a stable update is available, this leads to a Boost warning,
> "Unknown compiler version..." being emitted repeatedly (for every file
> which includes a Boost header).
> 
> If it were emitted once, we would leave it be, but because it's
> repeated hundreds of times, and is mostly harmless, we patch the build
> to remove the warning itself. Unfortunately, there is no compile-time
> option to disable the warning instead of a patch.
> 
> Note that it is not straight-forward to generate a patch file for
> Boost. The steps were: clone the Boost repo recursively, go the
> `libs/config` submodule, edit the `visualc.hpp` file, go to the
> `include` subdirectory, and use `git diff --relative` to generate the
> patch with the corrects paths expected for the extracted tarball.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
>   3rdparty/boost-1.65.0.patch PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66046/diff/1/
> 
> 
> Testing
> ---
> 
> Built on Windows; no more warning.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66046: CMake: Patched Boost to remove spurious warnings.

[Jeff Coffler]

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


Ship it!




Ship It!

- Jeff Coffler


On March 13, 2018, 10:30 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66046/
> ---
> 
> (Updated March 13, 2018, 10:30 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8556
> https://issues.apache.org/jira/browse/MESOS-8556
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On Windows, Boost explicitly checks if it's being compiled with a
> compiler version they've tested. As we tend to update Visual Studio
> whenever a stable update is available, this leads to a Boost warning,
> "Unknown compiler version..." being emitted repeatedly (for every file
> which includes a Boost header).
> 
> If it were emitted once, we would leave it be, but because it's
> repeated hundreds of times, and is mostly harmless, we patch the build
> to remove the warning itself. Unfortunately, there is no compile-time
> option to disable the warning instead of a patch.
> 
> Note that it is not straight-forward to generate a patch file for
> Boost. The steps were: clone the Boost repo recursively, go the
> `libs/config` submodule, edit the `visualc.hpp` file, go to the
> `include` subdirectory, and use `git diff --relative` to generate the
> patch with the corrects paths expected for the extracted tarball.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
>   3rdparty/boost-1.65.0.patch PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66046/diff/1/
> 
> 
> Testing
> ---
> 
> Built on Windows; no more warning.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66002: Fixed the HTTP API path variables on Windows.

[Jeff Coffler]

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


Ship it!




Ship It!

- Jeff Coffler


On March 9, 2018, 8:17 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66002/
> ---
> 
> (Updated March 9, 2018, 8:17 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, 
> and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Many mesos frameworks assume that path separators are forward slashes,
> like dcos-ui and marathon-ui. dcos-cli also assumes this. Previous to
> this patch, if a forward slash was given in the path variable to an HTTP
> API function call to the mesos agent on a Windows system, the Windows
> API would fail at recognizing the path, because the Windows API accepts
> only backslashes as path separators. To remedy this issue, we now
> convert all forward slashes passed as a path to the HTTP API to an agent
> to back slashes on Windows agents by using the path::from_uri function.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp 5f92d2af722c3201200fa1d6a75dc0b87fdc6078 
>   src/tests/files_tests.cpp c703cae03345112715aeab83cb0a74abe3e12469 
> 
> 
> Diff: https://reviews.apache.org/r/66002/diff/2/
> 
> 
> Testing
> ---
> 
> Tested on both Windows and Linux, all tests pass, including the two newly 
> enabled tests on Windows.
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 65811: Add `path::clean` to stout for normalizing path (for POSIX only now)

[Jason Lai]

> On March 2, 2018, 8:06 p.m., Chun-Hung Hsiao wrote:
> > 3rdparty/stout/include/stout/path.hpp
> > Lines 65 (patched)
> > 
> >
> > How about s/clean/normalize/?

Indeed I considered this option. And I also considered `normpath` as in 
`os.path.normpath` in Python but ended up picking `path::clean` from the Go 
counterpart (`filepath.Clean`) over `path::normalize`.
That said, I'm open to `path::normalize` if we have more folks in favor of it.


- Jason


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


On March 2, 2018, 7:40 p.m., Jason Lai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65811/
> ---
> 
> (Updated March 2, 2018, 7:40 p.m.)
> 
> 
> Review request for mesos, Anish Gupta, Eric Chung, Gilbert Song, Jie Yu, 
> James Peach, and Zhitao Li.
> 
> 
> Bugs: MESOS-8257
> https://issues.apache.org/jira/browse/MESOS-8257
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add `path::clean` to stout for normalizing path (for POSIX only now).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/path.hpp 
> 27438d31617b3b78bf3d4deffd25c93340610e8d 
> 
> 
> Diff: https://reviews.apache.org/r/65811/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Lai
> 
>

Re: Review Request 66045: Renamed `resources` in allocator to `toAllocate`.

[Benjamin Mahler]

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


Ship it!




- Benjamin Mahler


On March 13, 2018, 10:18 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66045/
> ---
> 
> (Updated March 13, 2018, 10:18 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `resources` is currently used in the allocator to denote
> resources that are going to be allocated. This name is too
> general and ambiguous. Renamed it to `toAllocate` for
> better readability.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 0e8c2c4a52969448f99bd5f42252a84cc52b9271 
> 
> 
> Diff: https://reviews.apache.org/r/66045/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66043: Renamed a variable in allocator to be consistent with others.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On March 13, 2018, 6:07 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66043/
> ---
> 
> (Updated March 13, 2018, 6:07 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed `getQuotaRoleAllocatedResources` to
> `getQuotaRoleAllocatedScalarQuantities` to be consistent with
> other naming.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 0e8c2c4a52969448f99bd5f42252a84cc52b9271 
> 
> 
> Diff: https://reviews.apache.org/r/66043/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 65997: Made `PROTOC_GENERATE` compile proto files from 3rd-party libraries.

[Chun-Hung Hsiao]

> On March 14, 2018, 7:50 p.m., Andrew Schwartzmeyer wrote:
> > src/cmake/MesosProtobuf.cmake
> > Lines 94 (patched)
> > 
> >
> > We put CPP_OUT files under an build/include/lib?

Yeah. The reason is that, for CSI support we introduce the `DiskProfileAdaptor` 
Mesos module that uses `csi::VolumeCapability`: 
https://github.com/apache/mesos/blob/1.5.x/include/mesos/resource_provider/storage/disk_profile.hpp#L66
For this reason, we put `spec.hpp` under `include/csi`, which includes 
`csi/csi.pb.h` from the `build` dir.

Now I rethink about this, actually we don't need the 
`{PUBLIC,INTERNAL}_PROTOBUF_INCLUDE_DIR` at all since we have `csi/` prepended 
in `spec.hpp`. I'll remove this var.


- Chun-Hung


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


On March 14, 2018, 2:56 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65997/
> ---
> 
> (Updated March 14, 2018, 2:56 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8657
> https://issues.apache.org/jira/browse/MESOS-8657
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> PROTOC_GENERATE now has the following new features:
>   (1) With the `LIB` option, compile .proto files found in a third-party
>   specification library.
>   (2) Provides the `GRPC` option that, once we support gRPC in cmake,
>   will generate the `.grpc.pb.h` and `.grpc.pb.cc` files.
>   (3) With the `LIB` option, append to list variable `PUBLIC_PROTO_PATH`
>   or `INTERNAL_PROTO_PATH` the fully qualified path to the library's
>   include directory, and append to list variable
>   `PUBLIC_PROTOBUF_INCLUDE_DIR` or `INTERNAL_PROTOBUF_INCLUDE_DIR`
>   the fully qualified path to the directory where the generated
>   `.pb.h` files are placed.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
>   src/cmake/MesosProtobuf.cmake ef90c15a6f2afaedaf7559c4a411098924528505 
> 
> 
> Diff: https://reviews.apache.org/r/65997/diff/3/
> 
> 
> Testing
> ---
> 
> `make check` with cmake.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66015: Built CSI spec proto files with cmake.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On March 13, 2018, 7:57 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66015/
> ---
> 
> (Updated March 13, 2018, 7:57 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8657
> https://issues.apache.org/jira/browse/MESOS-8657
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Built CSI spec proto files with cmake.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
>   3rdparty/cmake/Versions.cmake 94b0d8048412e00e2480f45e7ce4e6494da4bd5d 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
> 
> 
> Diff: https://reviews.apache.org/r/66015/diff/2/
> 
> 
> Testing
> ---
> 
> `make check` with cmake.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66002: Fixed the HTTP API path variables on Windows.

[Andrew Schwartzmeyer]

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


Ship it!




I think this approach is reasonable. `files` is the "public" API where we're 
getting (essentially) user input (i.e. things from frameworks and other 
external code beyond our control). This API had previously always expected `/` 
as the path separator, I think this is the correct place to "sanitize user 
input" and convert to the OS-specific path separator. This way, our OS layer 
(the file functions in stout) are left OS-specific, but the public API will 
work as expected.

- Andrew Schwartzmeyer


On March 9, 2018, 12:17 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66002/
> ---
> 
> (Updated March 9, 2018, 12:17 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, 
> and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Many mesos frameworks assume that path separators are forward slashes,
> like dcos-ui and marathon-ui. dcos-cli also assumes this. Previous to
> this patch, if a forward slash was given in the path variable to an HTTP
> API function call to the mesos agent on a Windows system, the Windows
> API would fail at recognizing the path, because the Windows API accepts
> only backslashes as path separators. To remedy this issue, we now
> convert all forward slashes passed as a path to the HTTP API to an agent
> to back slashes on Windows agents by using the path::from_uri function.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp 5f92d2af722c3201200fa1d6a75dc0b87fdc6078 
>   src/tests/files_tests.cpp c703cae03345112715aeab83cb0a74abe3e12469 
> 
> 
> Diff: https://reviews.apache.org/r/66002/diff/2/
> 
> 
> Testing
> ---
> 
> Tested on both Windows and Linux, all tests pass, including the two newly 
> enabled tests on Windows.
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 66002: Fixed the HTTP API path variables on Windows.

[Andrew Schwartzmeyer]

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




src/files/files.cpp
Lines 314-328 (original), 314-329 (patched)


const



src/files/files.cpp
Line 350 (original), 352 (patched)


const



src/files/files.cpp
Line 384 (original), 387 (patched)


Ha, this ought to be const too even though you didn't add the variable.

I sure wish C++ default to const like Rust :(



src/files/files.cpp
Line 453 (original), 458 (patched)


const



src/files/files.cpp
Line 628 (original), 634 (patched)


const



src/files/files.cpp
Line 769 (original), 776 (patched)


const



src/tests/files_tests.cpp
Lines 256-257 (original), 256-257 (patched)


With the above fixes, `files.attach` now accepts URI like paths, right? Or 
did we skip `attach`? Should we? I don't know.



src/tests/files_tests.cpp
Lines 336-345 (original), 336-349 (patched)


Ew. Can we test these without `stat`? I don't want to add to 
[MESOS-8275](https://issues.apache.org/jira/browse/MESOS-8275)


- Andrew Schwartzmeyer


On March 9, 2018, 12:17 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66002/
> ---
> 
> (Updated March 9, 2018, 12:17 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, 
> and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Many mesos frameworks assume that path separators are forward slashes,
> like dcos-ui and marathon-ui. dcos-cli also assumes this. Previous to
> this patch, if a forward slash was given in the path variable to an HTTP
> API function call to the mesos agent on a Windows system, the Windows
> API would fail at recognizing the path, because the Windows API accepts
> only backslashes as path separators. To remedy this issue, we now
> convert all forward slashes passed as a path to the HTTP API to an agent
> to back slashes on Windows agents by using the path::from_uri function.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp 5f92d2af722c3201200fa1d6a75dc0b87fdc6078 
>   src/tests/files_tests.cpp c703cae03345112715aeab83cb0a74abe3e12469 
> 
> 
> Diff: https://reviews.apache.org/r/66002/diff/2/
> 
> 
> Testing
> ---
> 
> Tested on both Windows and Linux, all tests pass, including the two newly 
> enabled tests on Windows.
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 65997: Made `PROTOC_GENERATE` compile proto files from 3rd-party libraries.

[Andrew Schwartzmeyer]

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




src/cmake/MesosProtobuf.cmake
Line 24 (original), 24 (patched)


Nit: s/support/supported



src/cmake/MesosProtobuf.cmake
Line 29 (original), 37 (patched)


Nit: s/if s/S



src/cmake/MesosProtobuf.cmake
Lines 51 (patched)


Same nit



src/cmake/MesosProtobuf.cmake
Lines 94 (patched)


We put CPP_OUT files under an build/include/lib?



src/cmake/MesosProtobuf.cmake
Lines 72-74 (original), 125-132 (patched)


Yeah! This looks reasonable.



src/cmake/MesosProtobuf.cmake
Lines 178-187 (patched)


I had previously added the custom mkdir targets to another file, and just 
added a dependency here. We should at least add a TODO to bring those here too 
(it seems to be a more appropriate location), and maybe a TODO to find a better 
way to ensure the directories are created. This always seemed hacky to me.


- Andrew Schwartzmeyer


On March 13, 2018, 7:56 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65997/
> ---
> 
> (Updated March 13, 2018, 7:56 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8657
> https://issues.apache.org/jira/browse/MESOS-8657
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> PROTOC_GENERATE now has the following new features:
>   (1) With the `LIB` option, compile .proto files found in a third-party
>   specification library.
>   (2) Provides the `GRPC` option that, once we support gRPC in cmake,
>   will generate the `.grpc.pb.h` and `.grpc.pb.cc` files.
>   (3) With the `LIB` option, append to list variable `PUBLIC_PROTO_PATH`
>   or `INTERNAL_PROTO_PATH` the fully qualified path to the library's
>   include directory, and append to list variable
>   `PUBLIC_PROTOBUF_INCLUDE_DIR` or `INTERNAL_PROTOBUF_INCLUDE_DIR`
>   the fully qualified path to the directory where the generated
>   `.pb.h` files are placed.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
>   src/cmake/MesosProtobuf.cmake ef90c15a6f2afaedaf7559c4a411098924528505 
> 
> 
> Diff: https://reviews.apache.org/r/65997/diff/3/
> 
> 
> Testing
> ---
> 
> `make check` with cmake.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66070: Document new `slave/recovery_time_secs` gauge.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On March 14, 2018, 4:26 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66070/
> ---
> 
> (Updated March 14, 2018, 4:26 p.m.)
> 
> 
> Review request for mesos, Jason Lai and James Peach.
> 
> 
> Bugs: MESOS-8609
> https://issues.apache.org/jira/browse/MESOS-8609
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Document new `slave/recovery_time_secs` gauge.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 0fba5cede553e2030260dcf8401d48e66e740270 
> 
> 
> Diff: https://reviews.apache.org/r/66070/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 65959: Added a test to make sure `slave/recovery_time_secs` is reported.

[James Peach]

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




src/tests/slave_tests.cpp
Lines 1377 (patched)


Nit:

```
// Make sure the agent finishes recovery.
```


- James Peach


On March 14, 2018, 6:03 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65959/
> ---
> 
> (Updated March 14, 2018, 6:03 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Jason Lai, and James Peach.
> 
> 
> Bugs: MESOS-8609
> https://issues.apache.org/jira/browse/MESOS-8609
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test to make sure `slave/recovery_time_secs` is reported.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp c2afaa573039e25fb59496765e6717e7643a692f 
> 
> 
> Diff: https://reviews.apache.org/r/65959/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66046: CMake: Patched Boost to remove spurious warnings.

[Andrew Schwartzmeyer]

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



So, this patch is fairly temporary. The tip of `master` of Boost (of that 
submodule) has this commit:

```
commit 5ad073063 (origin/develop, origin/HEAD, develop)
Author: jzmaddock 
Date:   Wed Mar 7 18:02:01 2018 +

visualc.hpp: Disable warning about outdated config.

diff --git a/include/boost/config/compiler/visualc.hpp 
b/include/boost/config/compiler/visualc.hpp
index 748d14076..c533c50df 100644
--- a/include/boost/config/compiler/visualc.hpp
+++ b/include/boost/config/compiler/visualc.hpp
@@ -346,6 +346,9 @@
 #  if defined(BOOST_ASSERT_CONFIG)
 # error "Boost.Config is older than your current compiler version."
 #  elif !defined(BOOST_CONFIG_SUPPRESS_OUTDATED_MESSAGE)
-  BOOST_PRAGMA_MESSAGE("Info: Boost.Config is older than your compiler 
version - probably nothing bad will happen - but you may wish to look for an 
updated Boost vers
ion. Define BOOST_CONFIG_SUPPRESS_OUTDATED_MESSAGE to suppress this message.")
+  //
+  // Disabled as of March 2018 - the pace of VS releases is hard to keep 
up with
+  // and in any case, we have relatively few defect macros defined now.
+  // BOOST_PRAGMA_MESSAGE("Info: Boost.Config is older than your compiler 
version - probably nothing bad will happen - but you may wish to look for an 
updated Boost v
ersion. Define BOOST_CONFIG_SUPPRESS_OUTDATED_MESSAGE to suppress this 
message.")
 #  endif
 #endif
```

So when we get an updated version of Boost with this change, we can just add 
`BOOST_CONFIG_SUPPRESS_OUTDATED_MESSAGE` to the pre-processor definitions and 
delete this patch. But it is necessary for now (and even upstream they realized 
this code was awful).

- Andrew Schwartzmeyer


On March 13, 2018, 3:30 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66046/
> ---
> 
> (Updated March 13, 2018, 3:30 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8556
> https://issues.apache.org/jira/browse/MESOS-8556
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On Windows, Boost explicitly checks if it's being compiled with a
> compiler version they've tested. As we tend to update Visual Studio
> whenever a stable update is available, this leads to a Boost warning,
> "Unknown compiler version..." being emitted repeatedly (for every file
> which includes a Boost header).
> 
> If it were emitted once, we would leave it be, but because it's
> repeated hundreds of times, and is mostly harmless, we patch the build
> to remove the warning itself. Unfortunately, there is no compile-time
> option to disable the warning instead of a patch.
> 
> Note that it is not straight-forward to generate a patch file for
> Boost. The steps were: clone the Boost repo recursively, go the
> `libs/config` submodule, edit the `visualc.hpp` file, go to the
> `include` subdirectory, and use `git diff --relative` to generate the
> patch with the corrects paths expected for the extracted tarball.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
>   3rdparty/boost-1.65.0.patch PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66046/diff/1/
> 
> 
> Testing
> ---
> 
> Built on Windows; no more warning.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65954: Add a gauge for how long agent recovery takes.

[James Peach]

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


Fix it, then Ship it!




Nit:

When you commit, can you s/Add/Added/ in the commit subject?


src/slave/metrics.cpp
Lines 254 (patched)


Nit:
```
CHECK_NONE(recovery_time_secs);
```


- James Peach


On March 14, 2018, 4:06 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65954/
> ---
> 
> (Updated March 14, 2018, 4:06 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Jason Lai, and James Peach.
> 
> 
> Bugs: MESOS-8609
> https://issues.apache.org/jira/browse/MESOS-8609
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new metric `slave/recover_time_secs` can be used to tell us how long
> Mesos agent needed to finish its recovery cycle. This is an important
> metric on agent machines which have a lot of completed executor
> sandboxes.
> 
> Note that the metric 1) will only be available after recovery succeeded
> and 2) never change its value across agent process lifecycle afterwards.
> 
> 
> Diffs
> -
> 
>   src/slave/metrics.hpp 3fc933ca65690d6fad63156398ad9c2c53789296 
>   src/slave/metrics.cpp 0eb2b59ed67e14e73b29d7592c239441df0008d5 
>   src/slave/slave.cpp 2f4ab157448eafc0f41372ee50255a76129e90db 
> 
> 
> Diff: https://reviews.apache.org/r/65954/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66046: CMake: Patched Boost to remove spurious warnings.

[Andrew Schwartzmeyer]

> On March 14, 2018, 9:47 a.m., Benjamin Bannier wrote:
> > Hmm, I am not sure I am super thrilled about this patch. How about not 
> > using unsupported compilers instead? And like you said, the warning is just 
> > annoying but harmless.

We're not using an unsupported compiler. We're using the latest stable MSVC 
compiler, which they (the MSVC team) test against Boost (and Mesos) before they 
update.

It's fine that Boost wants to say, "hey, you're compiler is a bit new, we're 
not 100% certain it'll work." But it's not fine to say it in an undisableable 
warning that's thrown from included headers and so repeated hundreds of times 
in a build of an upstream project.

It is true that maybe they'll push a patch version in which we hit a bug, but 
_this_ warning is not what's going to help us find it.

The problem here is that Boost hard-coded a "maximum" compiler version, down to 
the patch number, into their headers. It'd be like if they supported GCC 7.0, 
but threw a warning if you started using 7.0.1 instead. That's what this bit of 
code does. Throwing a warning for say 8.0 would be reasonable; but still not in 
the way they do it here, where it can't be turned off and gets repeated by 
every single source file in your code base.


- Andrew


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


On March 13, 2018, 3:30 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66046/
> ---
> 
> (Updated March 13, 2018, 3:30 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8556
> https://issues.apache.org/jira/browse/MESOS-8556
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On Windows, Boost explicitly checks if it's being compiled with a
> compiler version they've tested. As we tend to update Visual Studio
> whenever a stable update is available, this leads to a Boost warning,
> "Unknown compiler version..." being emitted repeatedly (for every file
> which includes a Boost header).
> 
> If it were emitted once, we would leave it be, but because it's
> repeated hundreds of times, and is mostly harmless, we patch the build
> to remove the warning itself. Unfortunately, there is no compile-time
> option to disable the warning instead of a patch.
> 
> Note that it is not straight-forward to generate a patch file for
> Boost. The steps were: clone the Boost repo recursively, go the
> `libs/config` submodule, edit the `visualc.hpp` file, go to the
> `include` subdirectory, and use `git diff --relative` to generate the
> patch with the corrects paths expected for the extracted tarball.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
>   3rdparty/boost-1.65.0.patch PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66046/diff/1/
> 
> 
> Testing
> ---
> 
> Built on Windows; no more warning.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66046: CMake: Patched Boost to remove spurious warnings.

[Andrew Schwartzmeyer]

> On March 13, 2018, 5:21 p.m., Jeff Coffler wrote:
> > 3rdparty/boost-1.65.0.patch
> > Lines 12 (patched)
> > 
> >
> > I'm thinking the other way around. That is, if the compiler version < 
> > 1910, issue the warning, otherwise be quiet.
> > 
> > That way:
> > 
> > 1. We only update this when we update minimum compiler version,
> > 2. Greater versions are legal, as long as it's >= the minimum compiler 
> > version.
> > 
> > The way you have it now, we'd need to update this every single time a 
> > new compiler came out. Yuck. I think we only care if new compiler is 
> > REQUIRED for some reason.
> 
> Andrew Schwartzmeyer wrote:
> That's not quite what's going on here. The upstream code is checking if 
> `_MSC_VER > 1910`, that is, "is the compiler newer than what we've tested 
> (1910)? If so, emit this warning." But we compile with, for instance, MSVC 
> 1912 (latest dot-update to VS).
> 
> But look closer, this is a new patch file _removing_ this code because 
> the check is causing _way_ too many warnings. No, Boost, you did not break 
> between 1910 and 1912; stop warning us.
> 
> Jeff Coffler wrote:
> I understand that. I agree the current code is broken. My question: If 
> we're using a "less than supported" version of the compiler, isn't this a 
> GOOD error to get? Or is this something that cmake checks for?
> 
> I guess, what I'm asking here: What happens if my version of Visual 
> Studio is too low? Do we support ALL versions of Visual Studio 2017? Or do we 
> need patch levels? If we do need a patch level, I think we should make it 
> obvious by checking for that here (or by aborting during the cmake step).

We're not using a less than supported version. We're using a "more" than 
supported version. This bit of code in Boost does not test less than, it tests 
more than.

If you're VS studio is too low, you'll fail a lot earlier than this. Moreover, 
the rest of this file does minimum version checks. This hunk does "max" version 
checks. It's confusing and dumb.


- Andrew


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


On March 13, 2018, 3:30 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66046/
> ---
> 
> (Updated March 13, 2018, 3:30 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8556
> https://issues.apache.org/jira/browse/MESOS-8556
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On Windows, Boost explicitly checks if it's being compiled with a
> compiler version they've tested. As we tend to update Visual Studio
> whenever a stable update is available, this leads to a Boost warning,
> "Unknown compiler version..." being emitted repeatedly (for every file
> which includes a Boost header).
> 
> If it were emitted once, we would leave it be, but because it's
> repeated hundreds of times, and is mostly harmless, we patch the build
> to remove the warning itself. Unfortunately, there is no compile-time
> option to disable the warning instead of a patch.
> 
> Note that it is not straight-forward to generate a patch file for
> Boost. The steps were: clone the Boost repo recursively, go the
> `libs/config` submodule, edit the `visualc.hpp` file, go to the
> `include` subdirectory, and use `git diff --relative` to generate the
> patch with the corrects paths expected for the extracted tarball.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
>   3rdparty/boost-1.65.0.patch PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66046/diff/1/
> 
> 
> Testing
> ---
> 
> Built on Windows; no more warning.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65959: Added a test to make sure `slave/recovery_time_secs` is reported.

[Zhitao Li]

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

(Updated March 14, 2018, 6:03 p.m.)


Review request for mesos, Gilbert Song, Greg Mann, Jason Lai, and James Peach.


Changes
---

Updated metric name.


Summary (updated)
-

Added a test to make sure `slave/recovery_time_secs` is reported.


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


Repository: mesos


Description (updated)
---

Added a test to make sure `slave/recovery_time_secs` is reported.


Diffs (updated)
-

  src/tests/slave_tests.cpp c2afaa573039e25fb59496765e6717e7643a692f 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66012: CMake: Split `CMAKE_FORWARD_ARGS` into `C` and `CXX` versions.

[Andrew Schwartzmeyer]

> On March 14, 2018, 9:44 a.m., Benjamin Bannier wrote:
> > Are we sure that this does not break other flags we forwarded to external 
> > projects? We seem to go from forwarding an open set of flags to only 
> > forwarding flags related to `C` and `CXX`. I am thinking e.g., about 
> > `CMAKE_GENERATOR`, see 
> > https://github.com/apache/mesos/blob/master/3rdparty/CMakeLists.txt#L101-L102
> > 
> >   # TODO(andschwa): Set the CMAKE_GENERATOR explicitly as an argmuent to
> >   # `ExternalProject_Add`.
> >   
> > but I could imagine this change to affect other areas as well. Or do I 
> > completely misunderstand this change?

You misunderstand, but I don't blame you :)

Both `CMAKE_CXX_FORWARD_ARGS` and `CMAKE_C_FORWARD_ARGS` start by including 
`CMAKE_FORWARD_ARGS`, which has all the "shared" flags like `CMAKE_GENERATOR`. 
The only difference now is instead of adding _both_ `CXX` and `C` flags to 
`CMAKE_FORWARD_ARGS`, I split it correctly into the first two mentioned 
variables, and they forward the respective `CXX/C` flags variables.

Otherwise during configuration, `C` projects complain about being given (but 
not using) `CXX` flags (and vice versa, if we had `C` flags...).


- Andrew


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


On March 9, 2018, 2:39 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66012/
> ---
> 
> (Updated March 9, 2018, 2:39 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By passing `CMAKE_C_FORWARD_ARGS` to C libraries, e.g. curl, libapr,
> libevent, and zlib, we avoid CMake warning us about set but unused
> variables (since they do not use the `CXX` variables).
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
> 
> 
> Diff: https://reviews.apache.org/r/66012/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66009: CMake: Added `-Wno-unused-local-typedefs` to Boost interface.

[Andrew Schwartzmeyer]

> On March 14, 2018, 9:38 a.m., Benjamin Bannier wrote:
> > 3rdparty/CMakeLists.txt
> > Lines 179-182 (patched)
> > 
> >
> > Since `469363d4322c7acda7fd10acbe8822f610af5a43` we package 
> > boost-1.65.0 amd does not appear to be needed anymore. Could you confirm 
> > that it is still needed and update the Boost versions both here and in 
> > `configure.ac`, or alternatively drop this patch and also remove the 
> > respective code in `configure.ac`?

Whoops, I thought I'd updated the version here. Yes, I confirmed we still 
needed it. Boost's variant type is unusable without this disabled. I'll update 
the comments.


- Andrew


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


On March 9, 2018, 2:38 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66009/
> ---
> 
> (Updated March 9, 2018, 2:38 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8658
> https://issues.apache.org/jira/browse/MESOS-8658
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As found in the Autotools builds, GCC and Clang will emit an used
> local typedef warning for headers including Boost. Since it is
> 3rdparty code, we disable this warning in its interface.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
> 
> 
> Diff: https://reviews.apache.org/r/66009/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66008: CMake: Enabled compiler warnings.

[Andrew Schwartzmeyer]

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

(Updated March 14, 2018, 10:57 a.m.)


Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
Kordich, Joseph Wu, and Michael Park.


Changes
---

Add other bug.


Bugs: MESOS-8033 and MESOS-8658
https://issues.apache.org/jira/browse/MESOS-8033
https://issues.apache.org/jira/browse/MESOS-8658


Repository: mesos


Description
---

We had previously been using the default sets of warnings, but now we
use the same warnings as on Autotools. This also means that we can
safely turn on the treat-warnings-as-errors (at least for the Mesos
code). However, doing so requires that we disable two common
possible-loss-of-data warnings on Windows that are not part of the
GNU/Clang default warnings.

This also replaces the use of `string(APPEND CMAKE_CXX_FLAGS)` with
the canonical command `add_compile_options` (though setting these on a
per-target basis would be even better, it's a separate issue).


Diffs
-

  cmake/CompilationConfigure.cmake efee36c1ffda096a97af23d481fc0d0903124e54 
  src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66008: CMake: Enabled compiler warnings.

[Andrew Schwartzmeyer]

> On March 14, 2018, 9:29 a.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 220 (patched)
> > 
> >
> > Can we at least add a `TODO` here for what was advertised in the commit 
> > message,
> > 
> > > This also replaces the use of string(APPEND CMAKE_CXX_FLAGS) with
> > the canonical command add_compile_options (though setting these on a
> > per-target basis would be even better, it's a separate issue).
> > 
> > Ideally we would reference a ticket.
> > 
> > No need to add this everywhere below, but this one sticks out 
> > especially since it says `GLOBAL` :/

I'm sorry, I thought I had. It's 
[MESOS-8033](https://issues.apache.org/jira/browse/MESOS-8033). Adding to the 
review.

And this says `GLOBAL` because `add_compile_options` is a global-ish 
side-effect. It affects all targets from "this" directory downward (and while I 
don't like this design, and think we should fix it at some point, this 
configuration file is included in the top-level `CMakeLists.txt` file, making 
this essentially global).

Oh wait, I get what you were saying now. So, adding compile options per target 
instead of here (and below) is "a separate issue" because it's a design change. 
It's not something we necessarily have to or even should do; as these flags are 
set similarly to the old build system. It is generally in CMake more 
appropriate to set flags on a per-target basis, so my commit message was just 
calling out why we didn't do that. I'll amend it and make it more clear.


> On March 14, 2018, 9:29 a.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 224 (patched)
> > 
> >
> > Nit: let's put these each flag on a separate line already now, it'll be 
> > much nicer to evolve over time.

;) I went back and forth on this three times. I'm happy to go either way.


> On March 14, 2018, 9:29 a.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 226 (patched)
> > 
> >
> > We definitely do not want to include `-Weverything` ever. It is 
> > intended for testing diagnostics by upstream developers, but would warn 
> > about issues we would not want to address (e.g., struct packing). You 
> > probably meant `-Wextra` like in the `GNU` case.

Good to know, thanks.


> On March 14, 2018, 9:29 a.m., Benjamin Bannier wrote:
> > src/CMakeLists.txt
> > Lines 485 (patched)
> > 
> >
> > We should add a cmake configuration flag to disable `-Werror`. We have 
> > an equivalent configure flag `--disable-werror` since it is extremely 
> > unlikely that we'll support every possible setup (e.g., bleeding edge 
> > compilers, packaging setups). Let's give users a way to opt out.

Hm... okay.


- Andrew


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


On March 9, 2018, 2:38 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66008/
> ---
> 
> (Updated March 9, 2018, 2:38 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8658
> https://issues.apache.org/jira/browse/MESOS-8658
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We had previously been using the default sets of warnings, but now we
> use the same warnings as on Autotools. This also means that we can
> safely turn on the treat-warnings-as-errors (at least for the Mesos
> code). However, doing so requires that we disable two common
> possible-loss-of-data warnings on Windows that are not part of the
> GNU/Clang default warnings.
> 
> This also replaces the use of `string(APPEND CMAKE_CXX_FLAGS)` with
> the canonical command `add_compile_options` (though setting these on a
> per-target basis would be even better, it's a separate issue).
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake efee36c1ffda096a97af23d481fc0d0903124e54 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
> 
> 
> Diff: https://reviews.apache.org/r/66008/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66007: CMake: Set C++11 as standard automatically.

[Andrew Schwartzmeyer]

> On March 14, 2018, 9:32 a.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 172 (patched)
> > 
> >
> > I am surprised that this does not disable permissive mode for msvc. Is 
> > that the case? Sounds like it might warrant a bug report against cmake.

I checked the CMake sources on this one. This controls e.g. `-std=gnu++11` vs 
`-std=c++11`, not `/permissive-`. Moreover, that was an option very recently 
added to MSVC; and it's not equivalent to disabling extensions, as it also 
disables stuff like allowing `_cast(ptr)` (address-of a 
temporary).


- Andrew


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


On March 9, 2018, 2:37 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66007/
> ---
> 
> (Updated March 9, 2018, 2:37 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8033
> https://issues.apache.org/jira/browse/MESOS-8033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of setting the compiler option manually, we use the
> `CMAKE_CXX_STANDARD` variable to set the default for all targets. This
> automatically appends the correct flag for each compiler.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake efee36c1ffda096a97af23d481fc0d0903124e54 
> 
> 
> Diff: https://reviews.apache.org/r/66007/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66070: Document new `slave/recovery_time_secs` gauge.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['65954', '65959', '66070']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66070

Relevant logs:

- 
[libprocess-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66070/logs/libprocess-tests-stdout.log):

```
[ RUN  ] TimeTest.Output
[   OK ] TimeTest.Output (0 ms)
[--] 4 tests from TimeTest (5 ms total)

[--] 3 tests from TimeSeriesTest
[ RUN  ] TimeSeriesTest.Set
[   OK ] TimeSeriesTest.Set (0 ms)
[ RUN  ] TimeSeriesTest.Sparsify
[   OK ] TimeSeriesTest.Sparsify (1 ms)
[ RUN  ] TimeSeriesTest.Truncate
[   OK ] TimeSeriesTest.Truncate (2 ms)
[--] 3 tests from TimeSeriesTest (8 ms total)

[--] 3 tests from JWTTest
[ RUN  ] JWTTest.Parse
[   OK ] JWTTest.Parse (10 ms)
[ RUN  ] JWTTest.Create
[   OK ] JWTTest.Create (2 ms)
[ RUN  ] JWTTest.Stringify
[   OK ] JWTTest.Stringify (1 ms)
[--] 3 tests from JWTTest (24 ms total)

[--] 1 test from SSL
[ RUN  ] SSL.Disabled
[   OK ] SSL.Disabled (12 ms)
[--] 1 test from SSL (12 ms total)

[--] 17 tests from SSLTest
[ RUN  ] SSLTest.SSLSocket
```

- 
[libprocess-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66070/logs/libprocess-tests-stderr.log):

```
ABORT: 
(D:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/ssl/gtest.hpp:171): 
Could not generate certificate: Failed to set common name: 
X509_NAME_add_entry_by_txt
```

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66070/logs/mesos-tests-stdout.log):

```
[   OK ] ContentType/SchedulerTest.Revive/1 (269 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/0
[   OK ] ContentType/SchedulerTest.Suppress/0 (271 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/1
[   OK ] ContentType/SchedulerTest.Suppress/1 (268 ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0 (287 ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1 (306 ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0
[   OK ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0 (327 
ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/1
[   OK ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/1 (334 
ms)
[ RUN  ] ContentType/SchedulerTest.Message/0
[   OK ] ContentType/SchedulerTest.Message/0 (339 ms)
[ RUN  ] ContentType/SchedulerTest.Message/1
[   OK ] ContentType/SchedulerTest.Message/1 (374 ms)
[ RUN  ] ContentType/SchedulerTest.Request/0
[   OK ] ContentType/SchedulerTest.Request/0 (111 ms)
[ RUN  ] ContentType/SchedulerTest.Request/1
[   OK ] ContentType/SchedulerTest.Request/1 (135 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/0
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (87 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/1
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (93 ms)
[--] 32 tests from ContentType/SchedulerTest (18424 ms total)

[--] 4 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66070/logs/mesos-tests-stderr.log):

```
I0314 17:29:42.920806  7300 scheduler.cpp:188] Version: 1.6.0
I0314 17:29:42.920806 12220 scheduler.cpp:311] Using default 'basic' HTTP 
authenticatee
I0314 17:29:42.921811  8536 scheduler.cpp:494] New master detected at 
master@10.3.1.8:55463
I0314 17:29:42.929823  7732 scheduler.cpp:468] Re-detecting master
I0314 17:29:42.931814  7732 scheduler.cpp:494] New master detected at 
master@10.3.1.8:55463
I0314 17:29:42.939290  7308 scheduler.cpp:472] Lost leading master
I0314 17:29:42.943809  7300 master.cpp:1131] Master terminating
I0314 17:29:42.971812  7300 cluster.cpp:172] Creating default 'local' authorizer
I0314 17:29:42.980823 12928 master.cpp:458] Master 
b8836eb4-f020-4c99-996e-cdb438e27094 
(win-bld-srv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) started on 
10.3.1.8:55463
I0314 17:29:42.980823 12928 master.cpp:460] Flags at startup: --acls="" 
--agent_ping_timeout="15secs" --agent_reregister_timeout="10mins" 
--allocation_interval="1secs" 

Re: Review Request 63992: Handled incorrect offer operation IDs in master's ACCEPT handler.

[Greg Mann]

> On March 14, 2018, 3:55 p.m., Benjamin Bannier wrote:
> > I am confused why this patch does not add a status update manager for 
> > master-generated operation status updates. Right now there is no way for 
> > frameworks to e.g., reconcile failed operations after framework failover 
> > since the master does not remember any operation status updates in 
> > generated (in addition to not retrying their delivery).
> > 
> > Apart from that the changes seem to do what they advertise, but I wonder 
> > whether now would be the right time to address the numerous `TOOD`s, e.g., 
> > the one about adding a helper to encapsulate the code around `drop`ped 
> > operations. I am pretty sure this will be useful in the future.

We're following the pattern we have already for task status updates, and not 
requiring acknowledgement for master-generated operation status updates. If the 
framework attempts to reconcile such an operation, it will indeed be unknown. I 
think that adding a status update manager to the master for this purpose alone 
introduces more complexity than it's worth. It seems to have worked fine thus 
far for tasks?

I think that adding the `drop` overload could be done in a separate patch.


- Greg


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


On March 13, 2018, 9:49 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63992/
> ---
> 
> (Updated March 13, 2018, 9:49 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
> 
> 
> Bugs: MESOS-8190
> https://issues.apache.org/jira/browse/MESOS-8190
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds code to send operation status updates in the master's
> ACCEPT call handler. In cases where operations are dropped and in
> cases where offer operation IDs are set when they should not be, the
> master will send operation status updates for the dropped operations.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 223ebf29ac4dd1dea9080e4bef4b2d4d064d847f 
> 
> 
> Diff: https://reviews.apache.org/r/63992/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 66046: CMake: Patched Boost to remove spurious warnings.

[Benjamin Bannier]

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



Hmm, I am not sure I am super thrilled about this patch. How about not using 
unsupported compilers instead? And like you said, the warning is just annoying 
but harmless.

- Benjamin Bannier


On March 13, 2018, 11:30 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66046/
> ---
> 
> (Updated March 13, 2018, 11:30 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8556
> https://issues.apache.org/jira/browse/MESOS-8556
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On Windows, Boost explicitly checks if it's being compiled with a
> compiler version they've tested. As we tend to update Visual Studio
> whenever a stable update is available, this leads to a Boost warning,
> "Unknown compiler version..." being emitted repeatedly (for every file
> which includes a Boost header).
> 
> If it were emitted once, we would leave it be, but because it's
> repeated hundreds of times, and is mostly harmless, we patch the build
> to remove the warning itself. Unfortunately, there is no compile-time
> option to disable the warning instead of a patch.
> 
> Note that it is not straight-forward to generate a patch file for
> Boost. The steps were: clone the Boost repo recursively, go the
> `libs/config` submodule, edit the `visualc.hpp` file, go to the
> `include` subdirectory, and use `git diff --relative` to generate the
> patch with the corrects paths expected for the extracted tarball.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
>   3rdparty/boost-1.65.0.patch PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66046/diff/1/
> 
> 
> Testing
> ---
> 
> Built on Windows; no more warning.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66012: CMake: Split `CMAKE_FORWARD_ARGS` into `C` and `CXX` versions.

[Benjamin Bannier]

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



Are we sure that this does not break other flags we forwarded to external 
projects? We seem to go from forwarding an open set of flags to only forwarding 
flags related to `C` and `CXX`. I am thinking e.g., about `CMAKE_GENERATOR`, 
see 
https://github.com/apache/mesos/blob/master/3rdparty/CMakeLists.txt#L101-L102

  # TODO(andschwa): Set the CMAKE_GENERATOR explicitly as an argmuent to
  # `ExternalProject_Add`.
  
but I could imagine this change to affect other areas as well. Or do I 
completely misunderstand this change?

- Benjamin Bannier


On March 9, 2018, 11:39 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66012/
> ---
> 
> (Updated March 9, 2018, 11:39 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By passing `CMAKE_C_FORWARD_ARGS` to C libraries, e.g. curl, libapr,
> libevent, and zlib, we avoid CMake warning us about set but unused
> variables (since they do not use the `CXX` variables).
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
> 
> 
> Diff: https://reviews.apache.org/r/66012/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66009: CMake: Added `-Wno-unused-local-typedefs` to Boost interface.

[Benjamin Bannier]

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




3rdparty/CMakeLists.txt
Lines 179-182 (patched)


Since `469363d4322c7acda7fd10acbe8822f610af5a43` we package boost-1.65.0 
amd does not appear to be needed anymore. Could you confirm that it is still 
needed and update the Boost versions both here and in `configure.ac`, or 
alternatively drop this patch and also remove the respective code in 
`configure.ac`?


- Benjamin Bannier


On March 9, 2018, 11:38 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66009/
> ---
> 
> (Updated March 9, 2018, 11:38 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8658
> https://issues.apache.org/jira/browse/MESOS-8658
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As found in the Autotools builds, GCC and Clang will emit an used
> local typedef warning for headers including Boost. Since it is
> 3rdparty code, we disable this warning in its interface.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
> 
> 
> Diff: https://reviews.apache.org/r/66009/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66007: CMake: Set C++11 as standard automatically.

[Benjamin Bannier]

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


Fix it, then Ship it!





cmake/CompilationConfigure.cmake
Lines 172 (patched)


I am surprised that this does not disable permissive mode for msvc. Is that 
the case? Sounds like it might warrant a bug report against cmake.


- Benjamin Bannier


On March 9, 2018, 11:37 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66007/
> ---
> 
> (Updated March 9, 2018, 11:37 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8033
> https://issues.apache.org/jira/browse/MESOS-8033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of setting the compiler option manually, we use the
> `CMAKE_CXX_STANDARD` variable to set the default for all targets. This
> automatically appends the correct flag for each compiler.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake efee36c1ffda096a97af23d481fc0d0903124e54 
> 
> 
> Diff: https://reviews.apache.org/r/66007/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66008: CMake: Enabled compiler warnings.

[Benjamin Bannier]

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




cmake/CompilationConfigure.cmake
Lines 220 (patched)


Can we at least add a `TODO` here for what was advertised in the commit 
message,

> This also replaces the use of string(APPEND CMAKE_CXX_FLAGS) with
the canonical command add_compile_options (though setting these on a
per-target basis would be even better, it's a separate issue).

Ideally we would reference a ticket.

No need to add this everywhere below, but this one sticks out especially 
since it says `GLOBAL` :/



cmake/CompilationConfigure.cmake
Lines 224 (patched)


Nit: let's put these each flag on a separate line already now, it'll be 
much nicer to evolve over time.



cmake/CompilationConfigure.cmake
Lines 226 (patched)


We definitely do not want to include `-Weverything` ever. It is intended 
for testing diagnostics by upstream developers, but would warn about issues we 
would not want to address (e.g., struct packing). You probably meant `-Wextra` 
like in the `GNU` case.



src/CMakeLists.txt
Lines 485 (patched)


We should add a cmake configuration flag to disable `-Werror`. We have an 
equivalent configure flag `--disable-werror` since it is extremely unlikely 
that we'll support every possible setup (e.g., bleeding edge compilers, 
packaging setups). Let's give users a way to opt out.


- Benjamin Bannier


On March 9, 2018, 11:38 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66008/
> ---
> 
> (Updated March 9, 2018, 11:38 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8658
> https://issues.apache.org/jira/browse/MESOS-8658
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We had previously been using the default sets of warnings, but now we
> use the same warnings as on Autotools. This also means that we can
> safely turn on the treat-warnings-as-errors (at least for the Mesos
> code). However, doing so requires that we disable two common
> possible-loss-of-data warnings on Windows that are not part of the
> GNU/Clang default warnings.
> 
> This also replaces the use of `string(APPEND CMAKE_CXX_FLAGS)` with
> the canonical command `add_compile_options` (though setting these on a
> per-target basis would be even better, it's a separate issue).
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake efee36c1ffda096a97af23d481fc0d0903124e54 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
> 
> 
> Diff: https://reviews.apache.org/r/66008/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 66070: Document new `slave/recovery_time_secs` gauge.

[Zhitao Li]

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

Review request for mesos, Jason Lai and James Peach.


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


Repository: mesos


Description
---

Document new `slave/recovery_time_secs` gauge.


Diffs
-

  docs/monitoring.md 0fba5cede553e2030260dcf8401d48e66e740270 


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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 65954: Add a gauge for how long agent recovery takes.

[Zhitao Li]

> On March 9, 2018, 6:53 p.m., James Peach wrote:
> > src/slave/metrics.hpp
> > Lines 45 (patched)
> > 
> >
> > This doesn't need to be atomic. The reader will just read either the 
> > old or new values and it doesn't matter which it gets.

Actually I don't even need a class variable. Directly capture it in lambda 
should be sufficient.


- Zhitao


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


On March 14, 2018, 4:06 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65954/
> ---
> 
> (Updated March 14, 2018, 4:06 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Jason Lai, and James Peach.
> 
> 
> Bugs: MESOS-8609
> https://issues.apache.org/jira/browse/MESOS-8609
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new metric `slave/recover_time_secs` can be used to tell us how long
> Mesos agent needed to finish its recovery cycle. This is an important
> metric on agent machines which have a lot of completed executor
> sandboxes.
> 
> Note that the metric 1) will only be available after recovery succeeded
> and 2) never change its value across agent process lifecycle afterwards.
> 
> 
> Diffs
> -
> 
>   src/slave/metrics.hpp 3fc933ca65690d6fad63156398ad9c2c53789296 
>   src/slave/metrics.cpp 0eb2b59ed67e14e73b29d7592c239441df0008d5 
>   src/slave/slave.cpp 2f4ab157448eafc0f41372ee50255a76129e90db 
> 
> 
> Diff: https://reviews.apache.org/r/65954/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 65954: Add a gauge for how long agent recovery takes.

[Zhitao Li]

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

(Updated March 14, 2018, 4:06 p.m.)


Review request for mesos, Gilbert Song, Greg Mann, Jason Lai, and James Peach.


Changes
---

Remove usage of atomic and rename metric.


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


Repository: mesos


Description (updated)
---

The new metric `slave/recover_time_secs` can be used to tell us how long
Mesos agent needed to finish its recovery cycle. This is an important
metric on agent machines which have a lot of completed executor
sandboxes.

Note that the metric 1) will only be available after recovery succeeded
and 2) never change its value across agent process lifecycle afterwards.


Diffs (updated)
-

  src/slave/metrics.hpp 3fc933ca65690d6fad63156398ad9c2c53789296 
  src/slave/metrics.cpp 0eb2b59ed67e14e73b29d7592c239441df0008d5 
  src/slave/slave.cpp 2f4ab157448eafc0f41372ee50255a76129e90db 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 64970: Use tox for linting and testing code living uder src/python.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['64970']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64970

Relevant logs:

- 
[libprocess-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64970/logs/libprocess-tests-stdout.log):

```
[ RUN  ] TimeTest.Output
[   OK ] TimeTest.Output (0 ms)
[--] 4 tests from TimeTest (4 ms total)

[--] 3 tests from TimeSeriesTest
[ RUN  ] TimeSeriesTest.Set
[   OK ] TimeSeriesTest.Set (0 ms)
[ RUN  ] TimeSeriesTest.Sparsify
[   OK ] TimeSeriesTest.Sparsify (1 ms)
[ RUN  ] TimeSeriesTest.Truncate
[   OK ] TimeSeriesTest.Truncate (1 ms)
[--] 3 tests from TimeSeriesTest (5 ms total)

[--] 3 tests from JWTTest
[ RUN  ] JWTTest.Parse
[   OK ] JWTTest.Parse (7 ms)
[ RUN  ] JWTTest.Create
[   OK ] JWTTest.Create (1 ms)
[ RUN  ] JWTTest.Stringify
[   OK ] JWTTest.Stringify (0 ms)
[--] 3 tests from JWTTest (11 ms total)

[--] 1 test from SSL
[ RUN  ] SSL.Disabled
[   OK ] SSL.Disabled (9 ms)
[--] 1 test from SSL (9 ms total)

[--] 17 tests from SSLTest
[ RUN  ] SSLTest.SSLSocket
```

- 
[libprocess-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64970/logs/libprocess-tests-stderr.log):

```
ABORT: 
(D:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/ssl/gtest.hpp:171): 
Could not generate certificate: Failed to set common name: 
X509_NAME_add_entry_by_txt
```

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64970/logs/mesos-tests-stdout.log):

```
[   OK ] ContentType/SchedulerTest.Revive/1 (259 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/0
[   OK ] ContentType/SchedulerTest.Suppress/0 (242 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/1
[   OK ] ContentType/SchedulerTest.Suppress/1 (246 ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0 (239 ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1 (254 ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0
[   OK ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0 (285 
ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/1
[   OK ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/1 (320 
ms)
[ RUN  ] ContentType/SchedulerTest.Message/0
[   OK ] ContentType/SchedulerTest.Message/0 (315 ms)
[ RUN  ] ContentType/SchedulerTest.Message/1
[   OK ] ContentType/SchedulerTest.Message/1 (319 ms)
[ RUN  ] ContentType/SchedulerTest.Request/0
[   OK ] ContentType/SchedulerTest.Request/0 (100 ms)
[ RUN  ] ContentType/SchedulerTest.Request/1
[   OK ] ContentType/SchedulerTest.Request/1 (101 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/0
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (90 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/1
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (85 ms)
[--] 32 tests from ContentType/SchedulerTest (15049 ms total)

[--] 4 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64970/logs/mesos-tests-stderr.log):

```
I0314 15:57:02.337544  9132 master.cpp:1673] Recovering from registrar
I0314 15:57:02.338690 10996 registrar.cpp:391] Successfully fetched the 
registry (0B) in 1.145088ms
I0314 15:57:02.338690 10996 registrar.cpp:495] Applied 1 operations in 0ns; 
attempting to update the registry
I0314 15:57:02.339540  1140 registrar.cpp:552] Successfully updated the 
registry in 850944ns
I0314 15:57:02.339540  1140 registrar.cpp:424] Successfully recovered registrar
I0314 15:57:02.340536  9100 master.cpp:1787] Recovered 0 agents from the 
registry (239B); allowing 10mins for agents to reregister
I0314 15:57:02.356540 11656 scheduler.cpp:188] Version: 1.6.0
I0314 15:57:02.357491  7076 scheduler.cpp:311] Using default 'basic' HTTP 
authenticatee
I0314 15:57:02.357491  6628 scheduler.cpp:494] New master detected at 
master@10.3.1.9:51848
I0314 15:57:02.366562 12228 scheduler.cpp:468] Re-detecting master
I0314 15:57:02.367517 12228 scheduler.cpp:494] New master detected at 
master@10.3.1.9:51848
I0314 

Re: Review Request 63991: Added helpers to create and forward offer operation updates.

[Benjamin Bannier]

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




src/common/protobuf_utils.hpp
Lines 165-170 (patched)


This looks unusual when compared to other helpers in this file which 
usually have arguments modelled pretty closely after the fields of message they 
produce. The function names usually also pretty directly reflect what kind of 
message they produce. This helper OTOH only can create a single flavor of 
`Scheduler::Event`.

Given that this function does not abstract away any logic and will be used 
just a single helper added in the next patch, I'd suggest to just inline the 
creation of `Scheduler::Event` there for now and not add this helper.



src/master/master.hpp
Lines 913-920 (patched)


Do we plan to use this same helper to forward operation updates from agents 
and rps as well in the future? Wouldn't we want to pass in the updates instead 
of exploding fields into separate args?



src/master/master.cpp
Lines 7979-7985 (patched)


Shouldn't we add this operation into some operation status update manager?

One possible way for that would be to just accept an operation update as 
arg here and deal with the distinction between master-generated and other 
updates elsewhere, e.g., in https://reviews.apache.org/r/63992/.


- Benjamin Bannier


On March 13, 2018, 5:33 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63991/
> ---
> 
> (Updated March 13, 2018, 5:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
> 
> 
> Bugs: MESOS-8190
> https://issues.apache.org/jira/browse/MESOS-8190
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helpers to create and forward offer operation updates.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 2ef1c9a4c2986e6f254e28387635bc01bb0b3259 
>   src/common/protobuf_utils.cpp 9c5fb97afb58f98013b79f3cbaea7dacc3603271 
>   src/master/master.hpp 8bf2c763dafdb7df55c46a56f2ff66f2a951d947 
>   src/master/master.cpp 223ebf29ac4dd1dea9080e4bef4b2d4d064d847f 
> 
> 
> Diff: https://reviews.apache.org/r/63991/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 63992: Handled incorrect offer operation IDs in master's ACCEPT handler.

[Benjamin Bannier]

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



I am confused why this patch does not add a status update manager for 
master-generated operation status updates. Right now there is no way for 
frameworks to e.g., reconcile failed operations after framework failover since 
the master does not remember any operation status updates in generated (in 
addition to not retrying their delivery).

Apart from that the changes seem to do what they advertise, but I wonder 
whether now would be the right time to address the numerous `TOOD`s, e.g., the 
one about adding a helper to encapsulate the code around `drop`ped operations. 
I am pretty sure this will be useful in the future.

- Benjamin Bannier


On March 13, 2018, 10:49 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63992/
> ---
> 
> (Updated March 13, 2018, 10:49 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
> 
> 
> Bugs: MESOS-8190
> https://issues.apache.org/jira/browse/MESOS-8190
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds code to send operation status updates in the master's
> ACCEPT call handler. In cases where operations are dropped and in
> cases where offer operation IDs are set when they should not be, the
> master will send operation status updates for the dropped operations.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 223ebf29ac4dd1dea9080e4bef4b2d4d064d847f 
> 
> 
> Diff: https://reviews.apache.org/r/63992/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 64970: Use tox for linting and testing code living uder src/python.

[Eric Chung]

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

(Updated March 14, 2018, 3:05 p.m.)


Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.


Repository: mesos


Description
---

Use tox for linting and testing code living uder src/python.

At the moment, all linting is done through the same `pylint`
installation under support/.virtualenv, which requires ALL dependencies
(i.e. pip-requirements.txt, requirements.in scattered in various
directories) to be installed in the same virtualenv, making things
really messy -- e.g. when I've changed some code under `src/python/lib`,
but don't have the dev virtualenv activated, linting will fail since
none of the dependencies under `src/python/lib` have been installed.

Using tox, we can solve this problem by distributing a "test spec"
(tox.ini) in each of the python source directories which are aware of
its local dependencies only. To test or lint the code there would be as
simple as running `tox -e py27-lint `, and the corresponding
virtualenv and test dependencies would automatically be setup.

This patch modifies `support/mesos-style.py` to install `tox` in
`support/.virtualenv` and delegates linting to a `tox` call when it sees
python directories that have tox setup for it. Linting for all other
languages will not be effected.

Testing Done:
1. intentionally create a lint error, such as extra spaces before a
parens in a python file
2. run the pre-commit hook and see tox in action

Reviewed at https://reviews.apache.org/r/64970/


Diffs (updated)
-

  src/python/cli_new/tests/__init__.py PRE-CREATION 
  src/python/cli_new/tox.ini PRE-CREATION 
  src/python/lib/requirements-test.in b2b73aab65377d9310797203ea84c5150ae60805 
  src/python/lib/tox.ini fd5e89c77c8608fea21e9caad814c6e111ad57db 
  support/mesos-style.py 47ec36949010fa511d1b3974739c5ad5c03f6f7f 


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

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


Testing
---

1. intentionally create a lint error, such as extra spaces before a parens in a 
python file
2. run the pre-commit hook and see tox in action


Thanks,

Eric Chung

Re: Review Request 64970: Use tox for linting and testing code living uder src/python.

[Eric Chung]

> On March 1, 2018, 11:20 p.m., Kevin Klues wrote:
> > support/mesos-style.py
> > Lines 413-414 (patched)
> > 
> >
> > This could probably be one line.

```
* Module mesos-style
C:409, 0: Line too long (82/80) (line-too-long)
C:442, 0: Line too long (91/80) (line-too-long)
```


- Eric


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


On March 14, 2018, 3:02 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64970/
> ---
> 
> (Updated March 14, 2018, 3:02 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use tox for linting and testing code living uder src/python.
> 
> At the moment, all linting is done through the same `pylint`
> installation under support/.virtualenv, which requires ALL dependencies
> (i.e. pip-requirements.txt, requirements.in scattered in various
> directories) to be installed in the same virtualenv, making things
> really messy -- e.g. when I've changed some code under `src/python/lib`,
> but don't have the dev virtualenv activated, linting will fail since
> none of the dependencies under `src/python/lib` have been installed.
> 
> Using tox, we can solve this problem by distributing a "test spec"
> (tox.ini) in each of the python source directories which are aware of
> its local dependencies only. To test or lint the code there would be as
> simple as running `tox -e py27-lint `, and the corresponding
> virtualenv and test dependencies would automatically be setup.
> 
> This patch modifies `support/mesos-style.py` to install `tox` in
> `support/.virtualenv` and delegates linting to a `tox` call when it sees
> python directories that have tox setup for it. Linting for all other
> languages will not be effected.
> 
> Testing Done:
> 1. intentionally create a lint error, such as extra spaces before a
> parens in a python file
> 2. run the pre-commit hook and see tox in action
> 
> Reviewed at https://reviews.apache.org/r/64970/
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/tests/__init__.py PRE-CREATION 
>   src/python/cli_new/tox.ini PRE-CREATION 
>   src/python/lib/requirements-test.in 
> b2b73aab65377d9310797203ea84c5150ae60805 
>   src/python/lib/tox.ini fd5e89c77c8608fea21e9caad814c6e111ad57db 
>   support/mesos-style.py 47ec36949010fa511d1b3974739c5ad5c03f6f7f 
> 
> 
> Diff: https://reviews.apache.org/r/64970/diff/5/
> 
> 
> Testing
> ---
> 
> 1. intentionally create a lint error, such as extra spaces before a parens in 
> a python file
> 2. run the pre-commit hook and see tox in action
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 64970: Use tox for linting and testing code living uder src/python.

[Eric Chung]

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

(Updated March 14, 2018, 3:02 p.m.)


Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.


Repository: mesos


Description
---

Use tox for linting and testing code living uder src/python.

At the moment, all linting is done through the same `pylint`
installation under support/.virtualenv, which requires ALL dependencies
(i.e. pip-requirements.txt, requirements.in scattered in various
directories) to be installed in the same virtualenv, making things
really messy -- e.g. when I've changed some code under `src/python/lib`,
but don't have the dev virtualenv activated, linting will fail since
none of the dependencies under `src/python/lib` have been installed.

Using tox, we can solve this problem by distributing a "test spec"
(tox.ini) in each of the python source directories which are aware of
its local dependencies only. To test or lint the code there would be as
simple as running `tox -e py27-lint `, and the corresponding
virtualenv and test dependencies would automatically be setup.

This patch modifies `support/mesos-style.py` to install `tox` in
`support/.virtualenv` and delegates linting to a `tox` call when it sees
python directories that have tox setup for it. Linting for all other
languages will not be effected.

Testing Done:
1. intentionally create a lint error, such as extra spaces before a
parens in a python file
2. run the pre-commit hook and see tox in action

Reviewed at https://reviews.apache.org/r/64970/


Diffs (updated)
-

  src/python/cli_new/tests/__init__.py PRE-CREATION 
  src/python/cli_new/tox.ini PRE-CREATION 
  src/python/lib/requirements-test.in b2b73aab65377d9310797203ea84c5150ae60805 
  src/python/lib/tox.ini fd5e89c77c8608fea21e9caad814c6e111ad57db 
  support/mesos-style.py 47ec36949010fa511d1b3974739c5ad5c03f6f7f 


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

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


Testing
---

1. intentionally create a lint error, such as extra spaces before a parens in a 
python file
2. run the pre-commit hook and see tox in action


Thanks,

Eric Chung

Re: Review Request 66037: Enabled agent resource provider capability by default.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66037']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66037

Relevant logs:

- 
[libprocess-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66037/logs/libprocess-tests-stdout.log):

```
[ RUN  ] TimeTest.Output
[   OK ] TimeTest.Output (0 ms)
[--] 4 tests from TimeTest (5 ms total)

[--] 3 tests from TimeSeriesTest
[ RUN  ] TimeSeriesTest.Set
[   OK ] TimeSeriesTest.Set (1 ms)
[ RUN  ] TimeSeriesTest.Sparsify
[   OK ] TimeSeriesTest.Sparsify (2 ms)
[ RUN  ] TimeSeriesTest.Truncate
[   OK ] TimeSeriesTest.Truncate (2 ms)
[--] 3 tests from TimeSeriesTest (6 ms total)

[--] 3 tests from JWTTest
[ RUN  ] JWTTest.Parse
[   OK ] JWTTest.Parse (9 ms)
[ RUN  ] JWTTest.Create
[   OK ] JWTTest.Create (1 ms)
[ RUN  ] JWTTest.Stringify
[   OK ] JWTTest.Stringify (0 ms)
[--] 3 tests from JWTTest (14 ms total)

[--] 1 test from SSL
[ RUN  ] SSL.Disabled
[   OK ] SSL.Disabled (10 ms)
[--] 1 test from SSL (11 ms total)

[--] 17 tests from SSLTest
[ RUN  ] SSLTest.SSLSocket
```

- 
[libprocess-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66037/logs/libprocess-tests-stderr.log):

```
ABORT: 
(D:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/ssl/gtest.hpp:171): 
Could not generate certificate: Failed to set common name: 
X509_NAME_add_entry_by_txt
```

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66037/logs/mesos-tests-stdout.log):

```
[   OK ] ContentType/SchedulerTest.Revive/1 (343 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/0
[   OK ] ContentType/SchedulerTest.Suppress/0 (338 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/1
[   OK ] ContentType/SchedulerTest.Suppress/1 (354 ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0 (328 ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1 (380 ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0
[   OK ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0 (414 
ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/1
[   OK ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/1 (420 
ms)
[ RUN  ] ContentType/SchedulerTest.Message/0
[   OK ] ContentType/SchedulerTest.Message/0 (440 ms)
[ RUN  ] ContentType/SchedulerTest.Message/1
[   OK ] ContentType/SchedulerTest.Message/1 (417 ms)
[ RUN  ] ContentType/SchedulerTest.Request/0
[   OK ] ContentType/SchedulerTest.Request/0 (148 ms)
[ RUN  ] ContentType/SchedulerTest.Request/1
[   OK ] ContentType/SchedulerTest.Request/1 (158 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/0
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (112 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/1
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (108 ms)
[--] 32 tests from ContentType/SchedulerTest (18823 ms total)

[--] 4 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66037/logs/mesos-tests-stderr.log):

```
I0314 14:07:07.654297  3784 scheduler.cpp:311] Using default 'basic' HTTP 
authenticatee
I0314 14:07:07.656338 12320 scheduler.cpp:494] New master detected at 
master@10.3.1.8:51772
I0314 14:07:07.664337  7720 scheduler.cpp:468] Re-detecting master
I0314 14:07:07.667351  7720 scheduler.cpp:494] New master detected at 
master@10.3.1.8:51772
I0314 14:07:07.676403 12488 scheduler.cpp:472] Lost leading master
I0314 14:07:07.683341  5936 master.cpp:1131] Master terminating
I0314 14:07:07.712404  5936 cluster.cpp:172] Creating default 'local' authorizer
I0314 14:07:07.721438 13376 master.cpp:458] Master 
9ea0ed67-5c4f-451b-b74c-9ac609fb2c07 
(win-bld-srv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) started on 
10.3.1.8:51772
I0314 14:07:07.721438 13376 master.cpp:460] Flags at startup: --acls="" 
--agent_ping_timeout="15secs" --agent_reregister_timeout="10mins" 
--allocation_interval="1secs" --allocator="HierarchicalDRF" 
--authenticate_agents="true" 

Re: Review Request 65833: Displayed resource provider resources in GET_AGENTS response.

[Benjamin Bannier]

> On March 13, 2018, 7:07 p.m., Greg Mann wrote:
> > I'm also unable to view the protobuf utils diff in RB, strange. 
> > Unfortunately, it looks like there is a conflict right now so I can't apply 
> > in order to view the diff either.

I have rebased the changes on a current recent master. They overlap with and 
depend on changes from https://reviews.apache.org/r/65591/.


- Benjamin


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


On Feb. 28, 2018, 1:55 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65833/
> ---
> 
> (Updated Feb. 28, 2018, 1:55 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8388
> https://issues.apache.org/jira/browse/MESOS-8388
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Displayed resource provider resources in GET_AGENTS response.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 6622e1b82861f380346f505eca326f7174dd8bd6 
>   include/mesos/v1/master/master.proto 
> 6034bd5af8ad764f625f9fe80be08b48707bbadb 
>   src/common/protobuf_utils.cpp 9c5fb97afb58f98013b79f3cbaea7dacc3603271 
>   src/tests/api_tests.cpp e81d6ed25984c14e4143a048848c96e44e4f16e4 
> 
> 
> Diff: https://reviews.apache.org/r/65833/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66037: Enabled agent resource provider capability by default.

[Benjamin Bannier]

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

(Updated March 14, 2018, 2:09 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Removed more now dead code.


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


Repository: mesos


Description (updated)
---

This patch add `RESOURCE_PROVIDER` to the list of default-enabled
agent capabilities. In addition we also adjust tests to accommodate
the change in the agent registration protocol this triggers. We rely
on masters ignoring redundant `UpdateSlaveMessage`s.


Diffs (updated)
-

  src/slave/constants.cpp 9b60bd0e808aab272039239db95bee71a3d910ab 
  src/tests/agent_resource_provider_config_api_tests.cpp 
bd5312b71f500a3572362a39f22d742c621762cf 
  src/tests/api_tests.cpp e81d6ed25984c14e4143a048848c96e44e4f16e4 
  src/tests/master_tests.cpp 909565188afbe1398dcaa7d69d3a6872dcdcdb78 
  src/tests/oversubscription_tests.cpp 564405743cbf5805e79c04c7b37df505e988d984 
  src/tests/persistent_volume_endpoints_tests.cpp 
ebdc409842ce36396ce7d7bc8d45f23db1eb1973 
  src/tests/persistent_volume_tests.cpp 
924d8458e54e34a49c99593482b5908c5f7c7a48 
  src/tests/reservation_endpoints_tests.cpp 
51146ea04fcd04efd489386be861415e2a066606 
  src/tests/reservation_tests.cpp 8d8e9c8390e65187269bd194bb322bbdff88e0bd 
  src/tests/resource_provider_manager_tests.cpp 
c8997ec41fe0c3e02b0f6ab205c9009205c992da 
  src/tests/slave_recovery_tests.cpp c856752fe1dc3f5b45adb21b65a736116184e10a 
  src/tests/slave_tests.cpp c2afaa573039e25fb59496765e6717e7643a692f 
  src/tests/storage_local_resource_provider_tests.cpp 
264d42b57fe1a8ea04c1de0a10112878c7b45d39 
  support/mesos-mini/Dockerfile f9bcdb59d233ddfa0a4523c84057c630d1d636a0 
  support/mesos-mini/mesos/agent_features.json 
a66852aff91e4b0c9ea996123cb6466a5ee0a050 


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

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


Testing
---

`sudo make check`


Thanks,

Benjamin Bannier

Re: Review Request 65833: Displayed resource provider resources in GET_AGENTS response.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['65589', '65590', '65591', '65673', '65674', '65832', 
'65833']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65833

Relevant logs:

- 
[libprocess-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65833/logs/libprocess-tests-stdout.log):

```
[ RUN  ] TimeTest.Output
[   OK ] TimeTest.Output (0 ms)
[--] 4 tests from TimeTest (5 ms total)

[--] 3 tests from TimeSeriesTest
[ RUN  ] TimeSeriesTest.Set
[   OK ] TimeSeriesTest.Set (0 ms)
[ RUN  ] TimeSeriesTest.Sparsify
[   OK ] TimeSeriesTest.Sparsify (1 ms)
[ RUN  ] TimeSeriesTest.Truncate
[   OK ] TimeSeriesTest.Truncate (2 ms)
[--] 3 tests from TimeSeriesTest (7 ms total)

[--] 3 tests from JWTTest
[ RUN  ] JWTTest.Parse
[   OK ] JWTTest.Parse (8 ms)
[ RUN  ] JWTTest.Create
[   OK ] JWTTest.Create (1 ms)
[ RUN  ] JWTTest.Stringify
[   OK ] JWTTest.Stringify (1 ms)
[--] 3 tests from JWTTest (14 ms total)

[--] 1 test from SSL
[ RUN  ] SSL.Disabled
[   OK ] SSL.Disabled (10 ms)
[--] 1 test from SSL (11 ms total)

[--] 17 tests from SSLTest
[ RUN  ] SSLTest.SSLSocket
```

- 
[libprocess-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65833/logs/libprocess-tests-stderr.log):

```
ABORT: 
(D:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/ssl/gtest.hpp:171): 
Could not generate certificate: Failed to set common name: 
X509_NAME_add_entry_by_txt
```

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65833/logs/mesos-tests-stdout.log):

```
[   OK ] ContentType/SchedulerTest.Revive/1 (245 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/0
[   OK ] ContentType/SchedulerTest.Suppress/0 (251 ms)
[ RUN  ] ContentType/SchedulerTest.Suppress/1
[   OK ] ContentType/SchedulerTest.Suppress/1 (261 ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/0 (259 ms)
[ RUN  ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1
[   OK ] ContentType/SchedulerTest.NoOffersWithAllRolesSuppressed/1 (265 ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0
[   OK ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/0 (327 
ms)
[ RUN  ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/1
[   OK ] 
ContentType/SchedulerTest.NoOffersOnReregistrationWithAllRolesSuppressed/1 (320 
ms)
[ RUN  ] ContentType/SchedulerTest.Message/0
[   OK ] ContentType/SchedulerTest.Message/0 (319 ms)
[ RUN  ] ContentType/SchedulerTest.Message/1
[   OK ] ContentType/SchedulerTest.Message/1 (341 ms)
[ RUN  ] ContentType/SchedulerTest.Request/0
[   OK ] ContentType/SchedulerTest.Request/0 (105 ms)
[ RUN  ] ContentType/SchedulerTest.Request/1
[   OK ] ContentType/SchedulerTest.Request/1 (111 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/0
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (82 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/1
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (83 ms)
[--] 32 tests from ContentType/SchedulerTest (17718 ms total)

[--] 4 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65833/logs/mesos-tests-stderr.log):

```
I0314 13:07:45.303478  8836 registrar.cpp:495] Applied 1 operations in 0ns; 
attempting to update the registry
I0314 13:07:45.305477  8836 registrar.cpp:552] Successfully updated the 
registry in 36ns
I0314 13:07:45.305477  8836 registrar.cpp:424] Successfully recovered registrar
I0314 13:07:45.306475  7348 master.cpp:1787] Recovered 0 agents from the 
registry (239B); allowing 10mins for agents to reregister
I0314 13:07:45.314481  9040 scheduler.cpp:188] Version: 1.6.0
I0314 13:07:45.314481  9620 scheduler.cpp:311] Using default 'basic' HTTP 
authenticatee
I0314 13:07:45.315476  8284 scheduler.cpp:494] New master detected at 
master@10.3.1.8:49968
I0314 13:07:45.322475  1968 scheduler.cpp:468] Re-detecting master
I0314 13:07:45.324477  1968 scheduler.cpp:494] New master detected at 
master@10.3.1.8:49968
I0314 13:07:45.331480  7516 scheduler.cpp:472] Lost leading master
I0314 13:07:45.336482  9040 master.cpp:1131] Master 

Re: Review Request 65591: Explicitly tracked resource providers in master.

[Benjamin Bannier]

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

(Updated March 14, 2018, 1:03 p.m.)


Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This patch adds explicit tracking of resource providers to the master
process. While we already had explicitly send resource provider
information in e.g., `UpdateSlaveMessage`, we only stored that
information aggregated over the full agent in the master up to now.


Diffs (updated)
-

  src/common/protobuf_utils.cpp 9c5fb97afb58f98013b79f3cbaea7dacc3603271 
  src/master/http.cpp cf03d8bf7df583c849a3b2a326d3772820181b0f 
  src/master/master.hpp 8bf2c763dafdb7df55c46a56f2ff66f2a951d947 
  src/master/master.cpp 223ebf29ac4dd1dea9080e4bef4b2d4d064d847f 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 66060: Added end-to-end tests for operation feedback.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66060 was successfully built and tested.

Reviews applied: `['63991', '63992', '63994', '65300', '64618', '65993', 
'65994', '66056', '66057', '66058', '66059', '66060']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66060

- Mesos Reviewbot Windows


On March 14, 2018, 10:29 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66060/
> ---
> 
> (Updated March 14, 2018, 10:29 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8201
> https://issues.apache.org/jira/browse/MESOS-8201
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added end-to-end tests for operation feedback.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 264d42b57fe1a8ea04c1de0a10112878c7b45d39 
> 
> 
> Diff: https://reviews.apache.org/r/66060/diff/1/
> 
> 
> Testing
> ---
> 
> The tests passed over 1000 iterations on GNU/Linux.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66057: Added a `createCallAcknowledgeOperationStatus()` test helper.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['63991', '63992', '63994', '65300', '64618', '65993', 
'65994', '66056', '66057']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66057

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66057/logs/mesos-tests-stdout.log):

```
[   OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (134 ms)
[--] 9 tests from Endpoint/SlaveEndpointTest (1181 ms total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (42 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (47 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (91 ms 
total)

[--] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN  ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[   OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (2585 ms)
[--] 1 test from IsolationFlag/CpuIsolatorTest (2610 ms total)

[--] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN  ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (2470 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (2496 ms total)

[--] Global test environment tear-down
[==] 920 tests from 91 test cases ran. (465971 ms total)
[  PASSED  ] 919 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] SlaveTest.RestartSlaveRequireExecutorAuthentication

 1 FAILED TEST
  YOU HAVE 210 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66057/logs/mesos-tests-stderr.log):

```
I0314 10:13:40.181313 10980 slave.cpp:3878] Shutting down framework 
771ab81f-6499-41be-abd8-06bf1e687987-
I0I0314 10:13:39.473310  8144 exec.cpp:162] Version: 1.6.0
I0314 10:13:39.502307  4408 exec.cpp:236] Executor registered on agent 
771ab81f-6499-41be-abd8-06bf1e687987-S0
I0314 10:13:39.507314  6808 executor.cpp:176] Received SUBSCRIBED event
I0314 10:13:39.512334  6808 executor.cpp:180] Subscribed executor on 
build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0314 10:13:39.512334  6808 executor.cpp:176] Received LAUNCH event
I0314 10:13:39.517334  6808 executor.cpp:648] Starting task 
f0c599cf-7811-4d7e-bba4-1b90ce834218
I0314 10:13:39.599334  6808 executor.cpp:483] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0314 10:13:40.145311  6808 executor.cpp:661] Forked command at 6672
I0314 10:13:40.185312  9212 exec.cpp:445] Executor asked to shutdown
I0314 10:13:40.186312  6808 executor.cpp:176] Received SHUTDOWN event
I0314 10:13:40.186312  6808 executor.cpp:758] Shutting down
I0314 10:13:40.186312  6808 executor.cpp:868] Sending SIGTERM to process tree 
at pid 6314 10:13:40.181313 10816 master.cpp:10414] Updating the state of task 
f0c599cf-7811-4d7e-bba4-1b90ce834218 of framework 
771ab81f-6499-41be-abd8-06bf1e687987- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0314 10:13:40.181313 10980 slave.cpp:6571] Shutting down executor 
'f0c599cf-7811-4d7e-bba4-1b90ce834218' of framework 
771ab81f-6499-41be-abd8-06bf1e687987- at executor(1)@10.3.1.5:58388
I0314 10:13:40.183312 10980 slave.cpp:924] Agent terminating
W0314 10:13:40.184314 10980 slave.cpp:3874] Ignoring shutdown framework 
771ab81f-6499-41be-abd8-06bf1e687987- because it is terminating
I0314 10:13:40.186312 10816 master.cpp:10513] Removing task 
f0c599cf-7811-4d7e-bba4-1b90ce834218 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework 771ab81f-6499-41be-abd8-06bf1e687987- on 
agent 771ab81f-6499-41be-abd8-06bf1e687987-S0 at slave(402)@10.3.1.5:58367 
(build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0314 10:13:40.187317 10980 containerizer.cpp:2338] Destroying container 
c4d2ec9a-e4bd-4452-a5ec-51266c3f1f63 in RUNNING state
I0314 10:13:40.188315 10980 containerizer.cpp:2952] Transitioning the state of 
container c4d2ec9a-e4bd-4452-a5ec-51266c3f1f63 from RUNNING to DESTROYING
I0314 10:13:40.189314 10980 launcher.cpp:156] Asked to destroy container 
c4d2ec9a-e4bd-4452-a5ec-51266c3f1f63
I0314 10:13:40.190312 10816 master.cpp:1288] Agent 
771ab81f-6499-41be-abd8-06bf1e687987-S0 at slave(402)@10.3.1.5:58367 
(build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) 

Review Request 66060: Added end-to-end tests for operation feedback.

[Gaston Kleiman]

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

Review request for mesos, Chun-Hung Hsiao and Greg Mann.


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


Repository: mesos


Description
---

Added end-to-end tests for operation feedback.


Diffs
-

  src/tests/storage_local_resource_provider_tests.cpp 
264d42b57fe1a8ea04c1de0a10112878c7b45d39 


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


Testing
---

The tests passed over 1000 iterations on GNU/Linux.


Thanks,

Gaston Kleiman

Review Request 66059: Added a `SendAcknowledgeOperationStatus` test action.

[Gaston Kleiman]

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

Review request for mesos, Chun-Hung Hsiao and Greg Mann.


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


Repository: mesos


Description
---

This patch adds a `SendAcknowledgeOperationStatus` action that can be
used in tests to automatically acknowledge operation status updates.


Diffs
-

  src/tests/mesos.hpp 2c3d0c9350cd2223ff20c0797d1849d38c19 


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


Testing
---

Used by the tests added in https://reviews.apache.org/r/66060/.


Thanks,

Gaston Kleiman

Re: Review Request 66007: CMake: Set C++11 as standard automatically.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On March 9, 2018, 2:37 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66007/
> ---
> 
> (Updated March 9, 2018, 2:37 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8033
> https://issues.apache.org/jira/browse/MESOS-8033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of setting the compiler option manually, we use the
> `CMAKE_CXX_STANDARD` variable to set the default for all targets. This
> automatically appends the correct flag for each compiler.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake efee36c1ffda096a97af23d481fc0d0903124e54 
> 
> 
> Diff: https://reviews.apache.org/r/66007/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 66058: Added v1 versions of some test helpers/matchers.

[Gaston Kleiman]

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

Review request for mesos, Chun-Hung Hsiao and Greg Mann.


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


Repository: mesos


Description
---

Added v1 versions of some test helpers/matchers.


Diffs
-

  src/tests/mesos.hpp 2c3d0c9350cd2223ff20c0797d1849d38c19 


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


Testing
---

Used by the tests added in https://reviews.apache.org/r/66060/.


Thanks,

Gaston Kleiman

Review Request 66057: Added a `createCallAcknowledgeOperationStatus()` test helper.

[Gaston Kleiman]

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

Review request for mesos and Greg Mann.


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


Repository: mesos


Description
---

Added a `createCallAcknowledgeOperationStatus()` test helper.


Diffs
-

  src/tests/mesos.hpp 2c3d0c9350cd2223ff20c0797d1849d38c19 


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


Testing
---

Used by the tests added in https://reviews.apache.org/r/66060/.


Thanks,

Gaston Kleiman

Review Request 66056: Updated `CREATE_VOLUME()` helper to allow specifying an operation ID.

[Gaston Kleiman]

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

Review request for mesos and Greg Mann.


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


Repository: mesos


Description
---

Updated `CREATE_VOLUME()` helper to allow specifying an operation ID.


Diffs
-

  src/tests/mesos.hpp 2c3d0c9350cd2223ff20c0797d1849d38c19 


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


Testing
---

Used by the tests added in https://reviews.apache.org/r/66060/.


Thanks,

Gaston Kleiman

Re: Review Request 64618: Implemented the master's `ACKNOWLEDGE_OPERATION_STATUS` handler.

[Gaston Kleiman]

> On March 13, 2018, 4:19 p.m., Greg Mann wrote:
> > src/master/master.cpp
> > Lines 5960 (patched)
> > 
> >
> > Can we move this to?
> > 
> > `std::move(statusUuid.toBytes())`
> > 
> > Looks like `toBytes()` returns a `std::string` and `set_value` has an 
> > overload for rvalue reference, so I think we can avoid a copy here if we 
> > move?

Since `statusUuid.toBytes()` here is an rvalue, I think that 
`set_value(std::string&&)` should be automatically used. I updated the patch 
anyway for the sake of explicitness/readability.


- Gaston


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


On March 14, 2018, 2:01 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64618/
> ---
> 
> (Updated March 14, 2018, 2:01 a.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8184
> https://issues.apache.org/jira/browse/MESOS-8184
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the master's `ACKNOWLEDGE_OPERATION_STATUS` handler.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp cf03d8bf7df583c849a3b2a326d3772820181b0f 
>   src/master/master.hpp 8bf2c763dafdb7df55c46a56f2ff66f2a951d947 
>   src/master/master.cpp 223ebf29ac4dd1dea9080e4bef4b2d4d064d847f 
> 
> 
> Diff: https://reviews.apache.org/r/64618/diff/7/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 64618: Implemented the master's `ACKNOWLEDGE_OPERATION_STATUS` handler.

[Gaston Kleiman]

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

(Updated March 14, 2018, 2:01 a.m.)


Review request for mesos and Greg Mann.


Changes
---

Addressed Greg's comment.


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


Repository: mesos


Description
---

Implemented the master's `ACKNOWLEDGE_OPERATION_STATUS` handler.


Diffs (updated)
-

  src/master/http.cpp cf03d8bf7df583c849a3b2a326d3772820181b0f 
  src/master/master.hpp 8bf2c763dafdb7df55c46a56f2ff66f2a951d947 
  src/master/master.cpp 223ebf29ac4dd1dea9080e4bef4b2d4d064d847f 


Diff: https://reviews.apache.org/r/64618/diff/7/

Changes: https://reviews.apache.org/r/64618/diff/6-7/


Testing
---

`make check` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 65300: Added master metrics for operation status acknowledgments handling.

[Gaston Kleiman]

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

(Updated March 14, 2018, 2 a.m.)


Review request for mesos and Greg Mann.


Changes
---

Rebased on top of https://reviews.apache.org/r/63994/.


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


Repository: mesos


Description
---

Added master metrics for operation status acknowledgments handling.


Diffs (updated)
-

  docs/monitoring.md 0fba5cede553e2030260dcf8401d48e66e740270 
  src/master/metrics.hpp b343904bd99defffe098077855d686b2d33c1fc1 
  src/master/metrics.cpp b627372fd09b84aeda0b8e1b209d3bc0547faa39 


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

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


Testing
---

`make check` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 65313: Refactored authorization logic in the agent.

[Alexander Rojas]

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

(Updated March 14, 2018, 9:58 a.m.)


Review request for mesos, Benjamin Hindman and Greg Mann.


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


Repository: mesos


Description
---

This patch makes uses of the new `ObjectApprovers` class which greatly
simplifies the logic for constructing and using authorization.


Diffs (updated)
-

  src/slave/http.hpp c33adeb2ddd36e8be1324b3ddb1401bdf7e4e80b 
  src/slave/http.cpp 7d7fa2b4ec2e1f8f65c5264ce72590d0d8195b9b 
  src/slave/slave.cpp 2f4ab157448eafc0f41372ee50255a76129e90db 


Diff: https://reviews.apache.org/r/65313/diff/8/

Changes: https://reviews.apache.org/r/65313/diff/7-8/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 65987: Allow nested containers in pods to have separate namespaces(Ref: MESOS-8534).

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65987 was successfully built and tested.

Reviews applied: `['65987']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65987

- Mesos Reviewbot Windows


On March 14, 2018, 7:01 a.m., Sagar Patwardhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65987/
> ---
> 
> (Updated March 14, 2018, 7:01 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-8534
> https://issues.apache.org/jira/browse/MESOS-8534
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Continued from https://github.com/apache/mesos/pull/263
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp f0b86775b 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 1d01915c2 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 896656987 
> 
> 
> Diff: https://reviews.apache.org/r/65987/diff/4/
> 
> 
> Testing
> ---
> 
> Manually tested.
> 
> Working on unit tests.
> 
> 
> Thanks,
> 
> Sagar Patwardhan
> 
>

Re: Review Request 65987: Allow nested containers in pods to have separate namespaces(Ref: MESOS-8534).

[Sagar Patwardhan]

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

(Updated March 14, 2018, 7:01 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Fix it for real.


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


Repository: mesos


Description
---

Continued from https://github.com/apache/mesos/pull/263


Diffs (updated)
-

  src/master/validation.cpp f0b86775b 
  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 1d01915c2 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 896656987 


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

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


Testing
---

Manually tested.

Working on unit tests.


Thanks,

Sagar Patwardhan

Re: Review Request 65987: Allow nested containers in pods to have separate namespaces(Ref: MESOS-8534).

[Sagar Patwardhan]

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

(Updated March 14, 2018, 6:39 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Removed an unused function and fixed a minor error.


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


Repository: mesos


Description
---

Continued from https://github.com/apache/mesos/pull/263


Diffs (updated)
-

  src/master/validation.cpp f0b86775b 
  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 1d01915c2 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 896656987 


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

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


Testing
---

Manually tested.

Working on unit tests.


Thanks,

Sagar Patwardhan