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/#review199152
---



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 13, 2018, 11:10 p.m., Sagar Patwardhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65987/
> ---
> 
> (Updated March 13, 2018, 11:10 p.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 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 7678a7c81 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp f9056c90f 
> 
> 
> Diff: https://reviews.apache.org/r/65987/diff/2/
> 
> 
> Testing
> ---
> 
> Manually tested.
> 
> Working on unit tests.
> 
> 
> Thanks,
> 
> Sagar Patwardhan
> 
>

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?

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.


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

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66049', '66050', '66052', '66051']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
[--] 9 tests from Endpoint/SlaveEndpointTest (1178 ms total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (39 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (44 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (85 ms 
total)

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

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

[--] Global test environment tear-down
[==] 916 tests from 91 test cases ran. (479165 ms total)
[  PASSED  ] 914 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] CommandExecutorCheckTest.CommandCheckTimeout
[  FAILED  ] SlaveTest.RestartSlaveRequireExecutorAuthentication

 2 FAILED TESTS
  YOU HAVE 210 DISABLED TESTS

```

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

```
I0314 03:26:29.838824  8100 master.cpp:10380] Updating the state of task 
b83253aa-a02f-4b84-9b7a-172cf8cef559 of framework 
f033d295-0a24-4df2-ba84-a6caf5e462b1- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0314 03:26:29.838824  9448 slave.cpp:3878] Shutting down framework 
f033d295-0a24-4df2-ba84-a6caf5e462b1I0314 03:26:29.110826  7716 exec.cpp:162] 
Version: 1.6.0
I0314 03:26:29.140822  7680 exec.cpp:236] Executor registered on agent 
f033d295-0a24-4df2-ba84-a6caf5e462b1-S0
I0314 03:26:29.144822 10668 executor.cpp:176] Received SUBSCRIBED event
I0314 03:26:29.150826 10668 executor.cpp:180] Subscribed executor on 
build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0314 03:26:29.150826 10668 executor.cpp:176] Received LAUNCH event
I0314 03:26:29.155833 10668 executor.cpp:648] Starting task 
b83253aa-a02f-4b84-9b7a-172cf8cef559
I0314 03:26:29.239827 10668 executor.cpp:483] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0314 03:26:29.801826 10668 executor.cpp:661] Forked command at 9432
I0314 03:26:29.842823  5456 exec.cpp:445] Executor asked to shutdown
I0314 03:26:29.843825 10668 executor.cpp:176] Received SHUTDOWN event
I0314 03:26:29.843825 10668 executor.cpp:758] Shutting down
I0314 03:26:29.844822 10668 executor.cpp:868] Sending SIGTERM to process tree 
at pid 9-
I0314 03:26:29.839823  9448 slave.cpp:6571] Shutting down executor 
'b83253aa-a02f-4b84-9b7a-172cf8cef559' of framework 
f033d295-0a24-4df2-ba84-a6caf5e462b1- at executor(1)@10.3.1.11:65161
I0314 03:26:29.841823  9448 slave.cpp:924] Agent terminating
I0314 03:26:29.841823  8100 master.cpp:10479] Removing task 
b83253aa-a02f-4b84-9b7a-172cf8cef559 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework f033d295-0a24-4df2-ba84-a6caf5e462b1- on 
agent f033d295-0a24-4df2-ba84-a6caf5e462b1-S0 at slave(398)@10.3.1.11:65140 
(build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
W0314 03:26:29.841823  9448 slave.cpp:3874] Ignoring shutdown framework 
f033d295-0a24-4df2-ba84-a6caf5e462b1- because it is terminating
I0314 03:26:29.844822  8100 master.cpp:1288] Agent 
f033d295-0a24-4df2-ba84-a6caf5e462b1-S0 at slave(398)@10.3.1.11:65140 
(build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0314 03:26:29.844822  8100 master.cpp:3258] Disconnecting agent 
f033d295-0a24-4df2-ba84-a6caf5e462b1-S0 at slave(398)@10.3.1.11:65140 
(build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0314 03:26:29.845823  3464 hierarchical.cpp:344] Removed framework 
f033d295-0a24-4df2-ba84-a6caf5e462b1-
I0314 03:26:29.845823  7644 containerizer.cpp:2338] Destroying container 
e7c5bd6e-37c7-46c1-a7e4-b19ccd0156fd 

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 14, 2018, 2:57 a.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
---

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

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


Testing
---

`make check` with cmake.


Thanks,

Chun-Hung Hsiao

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

[Jeff Coffler]

> On March 14, 2018, 12:21 a.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.

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


- Jeff


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


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.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66046 was successfully built and tested.

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

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

- Mesos Reviewbot Windows


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 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, 2:56 a.m.)


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


Changes
---

Addressed Andy's comments.


Summary (updated)
-

Made `PROTOC_GENERATE` compile proto files from 3rd-party libraries.


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


Repository: mesos


Description (updated)
---

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

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


Testing
---

`make check` with cmake.


Thanks,

Chun-Hung Hsiao

Re: Review Request 66048: Added missing comment on a test case.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66048]

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 13, 2018, 10:46 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66048/
> ---
> 
> (Updated March 13, 2018, 10:46 p.m.)
> 
> 
> Review request for mesos and Chun-Hung Hsiao.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added missing comment on a test case.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 924d8458e54e34a49c99593482b5908c5f7c7a48 
> 
> 
> Diff: https://reviews.apache.org/r/66048/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66048: Added missing comment on a test case.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66048']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

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

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (37 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (46 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (87 ms 
total)

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

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

[--] Global test environment tear-down
[==] 916 tests from 91 test cases ran. (48 ms total)
[  PASSED  ] 915 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] CommandExecutorCheckTest.CommandCheckTimeout

 1 FAILED TEST
  YOU HAVE 210 DISABLED TESTS

```

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

```
I0314 02:23:47.278494 11020 slave.cpp:3878] Shutting down framework 
b705ee89-009c-4e2c-be5d-176e80a3108f-
I0314 02:23:47.278494  2300 master.cpp:10245] Updating the state of task 
871495bf-7fec-4a5f-aa4e-949974eecf2c of framework 
b705ee89-009c-4e2c-be5d-176e80a3108f- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0314 02:23:47.278494 11020 slave.cpp:6571] Shutting down executor 
'871495bf-7fec-4a5f-aa4e-949974eecf2c' of framework 
b705ee89-009c-4e2c-be5d-176e80a3108f- at executor(1)@10.3.1.11:62973
I0314 02:23:47.280494 11020 slave.cpp:924] Agent terminating
W0314 02:23:47.280494 11020 slave.cpp:3874] Ignoring shutdown framework 
b705ee89-009c-4e2c-be5d-176e80a3108f- because it is terminating
I0314 02:23:47.281496  230I0314 02:23:46.550496  8168 exec.cpp:162] Version: 
1.6.0
I0314 02:23:46.579524  1228 exec.cpp:236] Executor registered on agent 
b705ee89-009c-4e2c-be5d-176e80a3108f-S0
I0314 02:23:46.584492  7740 executor.cpp:176] Received SUBSCRIBED event
I0314 02:23:46.589493  7740 executor.cpp:180] Subscribed executor on 
build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0314 02:23:46.590493  7740 executor.cpp:176] Received LAUNCH event
I0314 02:23:46.595494  7740 executor.cpp:648] Starting task 
871495bf-7fec-4a5f-aa4e-949974eecf2c
I0314 02:23:46.680492  7740 executor.cpp:483] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0314 02:23:47.240520  7740 executor.cpp:661] Forked command at 2516
I0314 02:23:47.282495  5872 exec.cpp:445] Executor asked to shutdown
I0314 02:23:47.282495  7740 executor.cpp:176] Received SHUTDOWN event
I0314 02:23:47.283494  7740 executor.cpp:758] Shutting down
I0314 02:23:47.283494  7740 executor.cpp:868] Sending SIGTERM to process tree 
at pid 20 master.cpp:10344] Removing task 871495bf-7fec-4a5f-aa4e-949974eecf2c 
with resources cpus(allocated: *):4; mem(allocated: *):2048; disk(allocated: 
*):1024; ports(allocated: *):[31000-32000] of framework 
b705ee89-009c-4e2c-be5d-176e80a3108f- on agent 
b705ee89-009c-4e2c-be5d-176e80a3108f-S0 at slave(398)@10.3.1.11:62952 
(build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0314 02:23:47.284492  8272 containerizer.cpp:2338] Destroying container 
dd72b1a3-477a-4d0d-a687-350c682baf4b in RUNNING state
I0314 02:23:47.284492  8272 containerizer.cpp:2952] Transitioning the state of 
container dd72b1a3-477a-4d0d-a687-350c682baf4b from RUNNING to DESTROYING
I0314 02:23:47.286497  8272 launcher.cpp:156] Asked to destroy container 
dd72b1a3-477a-4d0d-a687-350c682baf4b
I0314 02:23:47.286497  2300 master.cpp:1288] Agent 
b705ee89-009c-4e2c-be5d-176e80a3108f-S0 at slave(398)@10.3.1.11:62952 
(build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0314 02:23:47.286497  2300 master.cpp:3258] Disconnecting 

Re: Review Request 65997: Added `PROTOC_SPEC_GENERATE` helper for cmake.

[Chun-Hung Hsiao]

> On March 13, 2018, 6:54 p.m., Andrew Schwartzmeyer wrote:
> > src/cmake/MesosProtobuf.cmake
> > Lines 174-176 (patched)
> > 
> >
> > This is probably something for later, but these essentially hard-coded 
> > directories have never sat well with me.

Yeah this is kinda unfortunate. It would be better if the CSI package itself 
comes with its own makefile to build the proto files, and I might work on a PR 
to the CSI repo for this in the future. As a workaround for now, I think this 
hard-coded directory structure is good enough for us to handle similar cases we 
may have in the future.


- Chun-Hung


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


On March 9, 2018, 10:44 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65997/
> ---
> 
> (Updated March 9, 2018, 10:44 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_SPEC_GENERATE is a convenience function that will:
>   (1) Compile .proto files found in the third-party specification
>   library under its include directory and place the generated files
>   in the build folder, under the `include/` directory, or with the
>   option `INTERNAL` the `src/` directory.
>   (2) Append to list variables `PUBLIC_PROTO_PATH` or
>   `INTERNAL_PROTO_PATH` the fully qualified path to the library's
>   include directory depending on options passed in.
>   (3) Append to list variables `PUBLIC_PROTOBUF_INCLUDE_DIR` or
>   `INTERNAL_PROTOBUF_INCLUDE_DIR` (depending on options passed in)
>   the fully qualified path to the directory where the files are
>   generated.
>   (4) Append to list variables `PUBLIC_PROTOBUF_SRC` or
>   `INTERNAL_PROTOBUF_SRC` (depending on options passed in) the fully
>   qualified path to the generated .cc file.
> Where the exports in (2)(3)(4) are *side effects* and modifies the
> variables in the parent scope.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
>   src/cmake/MesosProtobuf.cmake ef90c15a6f2afaedaf7559c4a411098924528505 
> 
> 
> Diff: https://reviews.apache.org/r/65997/diff/2/
> 
> 
> Testing
> ---
> 
> `make check` with cmake.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

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.

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.


- 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 66025: Converted `JSON::String` to bool and numbers.

[Qian Zhang]

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

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


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


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Previsouly when converting a JSON to a protobuf message in stout, we
cannot handle the fields like below which are actually valid.
  "int32": "-2147483647"
  "int64": "-9223372036854775807"
  "float": "1.5"
  "bool": "true"
The conversion will fail with an error like "Not expecting a JSON string
for field 'int32'".

So in this patch, `Try operator()(const JSON::String& string)`
was enhanced to be able to convert `JSON::String` to bool and numbers.
This is to match Google's json -> protobuf behavior, see the doc below
for details:
https://developers.google.com/protocol-buffers/docs/proto3#json


Diffs (updated)
-

  3rdparty/stout/include/stout/protobuf.hpp 
4a1605e5130dbf7e8286dbb43d0d04ab4394e79a 


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

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


Testing
---

sudo make check


Thanks,

Qian Zhang

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

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66043', '66044', '66045']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

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

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (38 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (44 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (84 ms 
total)

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

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

[--] Global test environment tear-down
[==] 916 tests from 91 test cases ran. (478830 ms total)
[  PASSED  ] 915 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] CommandExecutorCheckTest.CommandCheckTimeout

 1 FAILED TEST
  YOU HAVE 210 DISABLED TESTS

```

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

```
I0314 01:21:22.078588  7796 master.cpp:10245] Updating the state of task 
f5a632e5-5f48-4797-8945-1d650bed106c of framework 
1ad1cc77-e2d2-4f8d-aef1-fee24dce5532- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0314 01:21:22.078588  8552 slave.cpp:3878] Shutting down framework 
1ad1cc77-e2d2-4f8d-aef1-fee24dce5532-
I0314 01:21:22.078588  8552 slave.cpp:6571] Shutting down executor 
'f5a632e5-5f48-4797-8945-1d650bed106c' of framework 
1ad1cc77-e2d2-4f8d-aef1-fee24dce5532- at executor(1)@10.3.1.11:60818
I0314 01:21:22.079625  8552 slave.cpp:924] Agent terminating
W0314 01:21:22.080740  8552 slave.cpp:3874] Ignoring shutdown framework 
1ad1cc77-e2d2-4f8d-aef1-fee24dce5532- because it iI0314 01:21:21.354588 
10480 exec.cpp:162] Version: 1.6.0
I0314 01:21:21.384590  7648 exec.cpp:236] Executor registered on agent 
1ad1cc77-e2d2-4f8d-aef1-fee24dce5532-S0
I0314 01:21:21.389588  7492 executor.cpp:176] Received SUBSCRIBED event
I0314 01:21:21.394593  7492 executor.cpp:180] Subscribed executor on 
build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0314 01:21:21.394593  7492 executor.cpp:176] Received LAUNCH event
I0314 01:21:21.399595  7492 executor.cpp:648] Starting task 
f5a632e5-5f48-4797-8945-1d650bed106c
I0314 01:21:21.483616  7492 executor.cpp:483] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0314 01:21:22.035612  7492 executor.cpp:661] Forked command at 9412
I0314 01:21:22.081590 10756 exec.cpp:445] Executor asked to shutdown
I0314 01:21:22.083591  7492 executor.cpp:176] Received SHUTDOWN event
I0314 01:21:22.083591  7492 executor.cpp:758] Shutting down
I0314 01:21:22.084589  7492 executor.cpp:868] Sending SIGTERM to process tree 
at pid 9s terminating
I0314 01:21:22.081590  7796 master.cpp:10344] Removing task 
f5a632e5-5f48-4797-8945-1d650bed106c with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework 1ad1cc77-e2d2-4f8d-aef1-fee24dce5532- on 
agent 1ad1cc77-e2d2-4f8d-aef1-fee24dce5532-S0 at slave(398)@10.3.1.11:60797 
(build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0314 01:21:22.084589  7796 master.cpp:1288] Agent 
1ad1cc77-e2d2-4f8d-aef1-fee24dce5532-S0 at slave(398)@10.3.1.11:60797 
(build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0314 01:21:22.084589  7796 master.cpp:3258] Disconnecting agent 
1ad1cc77-e2d2-4f8d-aef1-fee24dce5532-S0 at slave(398)@10.3.1.11:60797 
(build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0314 01:21:22.084589  7796 master.cpp:3277] Deactivating agent 
1ad1cc77-e2d2-4f8d-aef1-fee24dce5532-S0 at slave(398)@10.3.1.11:60797 
(build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0314 01:21:22.084589 11124 

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/#review199133
---




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.


- 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 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.cpp
> > Lines 259 (patched)
> > 
> >
> > I don't know that I like the idea of a metric that is absent and then 
> > present. I'd prefer that we just published a `0.0` until recovert is 
> > complete.
> > 
> > Suggest we keep the recovery timestamp in the `Slave` and just publish 
> > that.

I thought about that too, but I actually like the idea of the metric being 
absent when the value is not available yet. A zero value could confuse 
downstream aggregation.

For example, our team want to gather an average of recovery time across our 
cluster of thousands of agents, but a presence of zero value could mistake the 
calculation.

I think Mesos already have some precedence on absent then present metrics. For 
instance, metrics in `allocator/mesos/roles//...` could show up if 
framework under a new role registers after Master started.

Let me know what do you think.


> On March 9, 2018, 6:53 p.m., James Peach wrote:
> > src/slave/slave.cpp
> > Lines 7322 (patched)
> > 
> >
> > Since the gauge is being published in seconds, you need to use 
> > `Duration::secs` to convert.

I prefer the API call to work on `Duration` and perform the `secs()` as late as 
possible, as I've seen so many times when people pass a wrong time unit if the 
API task an integer/float.


- Zhitao


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


On March 7, 2018, 11:20 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65954/
> ---
> 
> (Updated March 7, 2018, 11:20 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_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 e2facb3c15a2f907f6497c58a36842ed707f2c70 
> 
> 
> Diff: https://reviews.apache.org/r/65954/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

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

[Zhitao Li]

> On March 9, 2018, 6:54 p.m., James Peach wrote:
> > This also needs to be documented in `docs/monitoring.md`.

Will do in a follow up patch after receiving shipit.


- Zhitao


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


On March 7, 2018, 11:20 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65954/
> ---
> 
> (Updated March 7, 2018, 11:20 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_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 e2facb3c15a2f907f6497c58a36842ed707f2c70 
> 
> 
> Diff: https://reviews.apache.org/r/65954/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

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

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66037]

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 13, 2018, 5:26 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66037/
> ---
> 
> (Updated March 13, 2018, 5:26 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-8647
> https://issues.apache.org/jira/browse/MESOS-8647
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 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.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.cpp 9b60bd0e808aab272039239db95bee71a3d910ab 
>   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/slave_recovery_tests.cpp c856752fe1dc3f5b45adb21b65a736116184e10a 
>   src/tests/slave_tests.cpp c2afaa573039e25fb59496765e6717e7643a692f 
> 
> 
> Diff: https://reviews.apache.org/r/66037/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

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

[Greg Mann]

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


Fix it, then Ship it!





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?


- Greg Mann


On Feb. 1, 2018, 11:52 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64618/
> ---
> 
> (Updated Feb. 1, 2018, 11:52 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/6/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66030: Fixed perf stat output parsing.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On March 13, 2018, 12:03 a.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66030/
> ---
> 
> (Updated March 13, 2018, 12:03 a.m.)
> 
> 
> Review request for mesos and James Peach.
> 
> 
> Bugs: MESOS-8664
> https://issues.apache.org/jira/browse/MESOS-8664
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a workaround for a bug in 'perf stat'
> (https://lkml.org/lkml/2018/3/6/22) and fixes handling of nameless
> counters.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp b301e25d5cac6488c57e4f983e4867c72368a040 
>   src/tests/containerizer/perf_tests.cpp 
> 7176240bd9caff1efc8b213fe7ae7faae59c0e3e 
> 
> 
> Diff: https://reviews.apache.org/r/66030/diff/1/
> 
> 
> Testing
> ---
> 
> Added new test cases. Ran `sudo make check`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

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 13, 2018, 11:10 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Addressed review comments.


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 
  src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 7678a7c81 
  src/slave/containerizer/mesos/isolators/network/cni/paths.cpp f9056c90f 


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

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


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]

> On March 9, 2018, 11:40 p.m., Jie Yu wrote:
> > Looking good! Is it possible to add some test?

Working on the unit tests.


- Sagar


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


On March 13, 2018, 11:10 p.m., Sagar Patwardhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65987/
> ---
> 
> (Updated March 13, 2018, 11:10 p.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 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 7678a7c81 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp f9056c90f 
> 
> 
> Diff: https://reviews.apache.org/r/65987/diff/2/
> 
> 
> Testing
> ---
> 
> Manually tested.
> 
> Working on unit tests.
> 
> 
> Thanks,
> 
> Sagar Patwardhan
> 
>

Re: Review Request 63994: Added a new test for validation of offer operation IDs.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63994 was successfully built and tested.

Reviews applied: `['63991', '63992', '63994']`

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

- Mesos Reviewbot Windows


On March 13, 2018, 9:48 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63994/
> ---
> 
> (Updated March 13, 2018, 9:48 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 two new scheduler tests,
> 'OperationFeedbackValidationWithResourceProviderCapability' and
> 'OperationFeedbackValidationNoResourceProviderCapability',
> to verify that task and offer operation status updates are correctly
> sent when the 'id' field is set incorrectly on an operation.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d 
> 
> 
> Diff: https://reviews.apache.org/r/63994/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 66051: Implemented operator API to grow and shrink persistent volume.

[Zhitao Li]

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

Review request for mesos, Chun-Hung Hsiao and Gaston Kleiman.


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


Repository: mesos


Description
---

Implemented operator API to grow and shrink persistent volume.


Diffs
-

  include/mesos/master/master.proto 6622e1b82861f380346f505eca326f7174dd8bd6 
  include/mesos/v1/master/master.proto 6034bd5af8ad764f625f9fe80be08b48707bbadb 
  src/master/http.cpp cf03d8bf7df583c849a3b2a326d3772820181b0f 
  src/master/master.hpp 8bf2c763dafdb7df55c46a56f2ff66f2a951d947 
  src/master/validation.cpp 9254cd22e67d9aca53135701575fb0857ff6f7d6 


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


Testing
---


Thanks,

Zhitao Li

Review Request 66052: Added new operator API to grow and shrink persistent volume.

[Zhitao Li]

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

Review request for mesos, Chun-Hung Hsiao and Gaston Kleiman.


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


Repository: mesos


Description
---

Added new operator API to grow and shrink persistent volume.


Diffs
-

  include/mesos/master/master.proto 6622e1b82861f380346f505eca326f7174dd8bd6 
  include/mesos/v1/master/master.proto 6034bd5af8ad764f625f9fe80be08b48707bbadb 


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


Testing
---


Thanks,

Zhitao Li

Review Request 66050: Implement grow and shrink of persistent volume.

[Zhitao Li]

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

Review request for mesos, Chun-Hung Hsiao and Gaston Kleiman.


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


Repository: mesos


Description
---

Implement grow and shrink of persistent volume.


Diffs
-

  src/common/protobuf_utils.cpp 9c5fb97afb58f98013b79f3cbaea7dacc3603271 
  src/common/resources_utils.cpp 99b16e0d17b224eefa1e28f5f66c4284069c0e57 
  src/master/master.cpp 223ebf29ac4dd1dea9080e4bef4b2d4d064d847f 
  src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
  src/master/validation.cpp 9254cd22e67d9aca53135701575fb0857ff6f7d6 
  src/tests/mesos.hpp 2c3d0c9350cd2223ff20c0797d1849d38c19 


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


Testing
---


Thanks,

Zhitao Li

Review Request 66049: Added offer operation to grow and shrink persistent volumes.

[Zhitao Li]

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

Review request for mesos, Chun-Hung Hsiao and Gaston Kleiman.


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


Repository: mesos


Description
---

Added offer operation to grow and shrink persistent volumes.


Diffs
-

  include/mesos/mesos.proto e6ba3746456c9241ceaefac39200f68562dd5cb9 
  include/mesos/v1/mesos.proto 30d4d35e865db2af1ba85b12e2b5b0e499ef8de8 


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


Testing
---


Thanks,

Zhitao Li

Review Request 66048: Added missing comment on a test case.

[Zhitao Li]

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

Review request for mesos and Chun-Hung Hsiao.


Repository: mesos


Description
---

Added missing comment on a test case.


Diffs
-

  src/tests/persistent_volume_tests.cpp 
924d8458e54e34a49c99593482b5908c5f7c7a48 


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


Testing
---


Thanks,

Zhitao Li

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

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

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

[Meng Zhu]

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

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

Review Request 66044: Refactored resources chopping logic in allocator.

[Meng Zhu]

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

Review request for mesos, Benjamin Mahler, Kapil Arya, Joseph Wu, Michael Park, 
and Till Toenshoff.


Repository: mesos


Description
---

Introduced a `shrinkResources()` lambda in allocator
so that the same resource chopping logic can be re-used
in the future, in particular, when introducing the quota
limit.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
0e8c2c4a52969448f99bd5f42252a84cc52b9271 


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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 65995: Declined unwanted offers in `RetryOperationStatusUpdate*` SLRP tests.

[Chun-Hung Hsiao]

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

(Updated March 13, 2018, 9:58 p.m.)


Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.


Changes
---

Addressed Gaston's comment.


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


Repository: mesos


Description
---

The two SLRP tests assume that SLRP will send out a RAW resource in its
first `UPDATE_STATE` message, and expect that the test framework would
receive an offer containing the RAW resource in its first offer. However
this behavior is not guaranteed and should not be relied on. This patch
makes the tests decline unwanted offers by default so they no longer
rely on SLRP's internal behavior.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
264d42b57fe1a8ea04c1de0a10112878c7b45d39 


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

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


Testing
---

sudo make check
Ran the two tests in repitition.


Thanks,

Chun-Hung Hsiao

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

[Gaston Kleiman]

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


Ship it!




Ship It!

- Gaston Kleiman


On March 13, 2018, 2: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, 2: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 63994: Added a new test for validation of offer operation IDs.

[Gaston Kleiman]

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


Ship it!




Ship It!

- Gaston Kleiman


On March 13, 2018, 2:48 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63994/
> ---
> 
> (Updated March 13, 2018, 2:48 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 two new scheduler tests,
> 'OperationFeedbackValidationWithResourceProviderCapability' and
> 'OperationFeedbackValidationNoResourceProviderCapability',
> to verify that task and offer operation status updates are correctly
> sent when the 'id' field is set incorrectly on an operation.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d 
> 
> 
> Diff: https://reviews.apache.org/r/63994/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 65995: Declined unwanted offers in `RetryOperationStatusUpdate*` SLRP tests.

[Gaston Kleiman]

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




src/tests/storage_local_resource_provider_tests.cpp
Lines 2892 (patched)


nit: s/decline/declined/


- Gaston Kleiman


On March 13, 2018, 2:45 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65995/
> ---
> 
> (Updated March 13, 2018, 2:45 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The two SLRP tests assume that SLRP will send out a RAW resource in its
> first `UPDATE_STATE` message, and expect that the test framework would
> receive an offer containing the RAW resource in its first offer. However
> this behavior is not guaranteed and should not be relied on. This patch
> makes the tests decline unwanted offers by default so they no longer
> rely on SLRP's internal behavior.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 264d42b57fe1a8ea04c1de0a10112878c7b45d39 
> 
> 
> Diff: https://reviews.apache.org/r/65995/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> Ran the two tests in repitition.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

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

[Greg Mann]

---
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 (updated)
-

  src/master/master.cpp 223ebf29ac4dd1dea9080e4bef4b2d4d064d847f 


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

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


Testing
---

make check


Thanks,

Greg Mann

Re: Review Request 63994: Added a new test for validation of offer operation IDs.

[Greg Mann]

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

(Updated March 13, 2018, 9:48 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 two new scheduler tests,
'OperationFeedbackValidationWithResourceProviderCapability' and
'OperationFeedbackValidationNoResourceProviderCapability',
to verify that task and offer operation status updates are correctly
sent when the 'id' field is set incorrectly on an operation.


Diffs (updated)
-

  src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d 


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

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


Testing
---

make check


Thanks,

Greg Mann

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

[Gaston Kleiman]

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




src/master/master.cpp
Lines 4075-4082 (original), 4111-4117 (patched)


Nitpick: the comment looks a bit redundant. I'd go with this:

```
// The `id` field is set, which means operation feedback is 
requested.
//
// Operation feedback is not supported for LAUNCH or LAUNCH_GROUP
// operations, so we drop them and send TASK_ERROR status updates.
//
// For other operatations verify that they are destined for an agent
// with the RESOURCE_PROVIDER capability.
```


- Gaston Kleiman


On March 13, 2018, 9:38 a.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:38 a.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/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 65995: Declined unwanted offers in `RetryOperationStatusUpdate*` SLRP tests.

[Chun-Hung Hsiao]

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

(Updated March 13, 2018, 9:45 p.m.)


Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.


Changes
---

Addressed the flakiness in the tests.


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


Repository: mesos


Description
---

The two SLRP tests assume that SLRP will send out a RAW resource in its
first `UPDATE_STATE` message, and expect that the test framework would
receive an offer containing the RAW resource in its first offer. However
this behavior is not guaranteed and should not be relied on. This patch
makes the tests decline unwanted offers by default so they no longer
rely on SLRP's internal behavior.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
264d42b57fe1a8ea04c1de0a10112878c7b45d39 


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

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


Testing
---

sudo make check
Ran the two tests in repitition.


Thanks,

Chun-Hung Hsiao

Re: Review Request 63994: Added a new test for validation of offer operation IDs.

[Gaston Kleiman]

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




src/tests/scheduler_tests.cpp
Lines 1080 (patched)


I don't think we need a block for this?

I'd remove this `{` and the `}` on line 1119.



src/tests/scheduler_tests.cpp
Lines 1081-1086 (patched)


This `call` is unused and this code should be removed.



src/tests/scheduler_tests.cpp
Lines 1185 (patched)


I'd remove these braces, unless there's a reason for this block?


- Gaston Kleiman


On March 13, 2018, 9:41 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63994/
> ---
> 
> (Updated March 13, 2018, 9:41 a.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 two new scheduler tests,
> 'OperationFeedbackValidationWithResourceProviderCapability' and
> 'OperationFeedbackValidationNoResourceProviderCapability',
> to verify that task and offer operation status updates are correctly
> sent when the 'id' field is set incorrectly on an operation.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d 
> 
> 
> Diff: https://reviews.apache.org/r/63994/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

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

[Gaston Kleiman]

> On March 7, 2018, 11:21 a.m., Gaston Kleiman wrote:
> > src/master/master.cpp
> > Line 4136 (original), 4184 (patched)
> > 
> >
> > If the operation has an operation ID, we'll always land on this 
> > `continue` even if the validations/checks pass.
> > 
> > This means that line #4187 will be skipped, and operations with an 
> > operatin ID won't be added to the object passed to the `_accept` 
> > continuation.
> 
> Greg Mann wrote:
> Ah thank you; this should be removed.
> 
> Greg Mann wrote:
> I elected to add another case to the conditional because I thought this 
> was more readable; let me know what you think.

LGTM!


- Gaston


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


On March 13, 2018, 9:38 a.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:38 a.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/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 63994: Added a new test for validation of offer operation IDs.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [63991, 63992, 63994]

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 13, 2018, 9:41 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63994/
> ---
> 
> (Updated March 13, 2018, 9:41 a.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 two new scheduler tests,
> 'OperationFeedbackValidationWithResourceProviderCapability' and
> 'OperationFeedbackValidationNoResourceProviderCapability',
> to verify that task and offer operation status updates are correctly
> sent when the 'id' field is set incorrectly on an operation.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d 
> 
> 
> Diff: https://reviews.apache.org/r/63994/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 66010: Windows: Switched to default CRT linkage.

[Andrew Schwartzmeyer]

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

(Updated March 13, 2018, 1:45 p.m.)


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


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


Repository: mesos


Description
---

We previously attempted to manually override the CRT to be static
everywhere. Not only did this emit warnings, it was also error-prone
and unnecessary. We can, and should, just use the defaults, which is
`/MDd` in debug mode (multi-threaded, dynamic, debug linkage). Linking
to the CRT dynamically results in smaller libraries and executables,
reduces linking time, and avoids bugs when sharing allocated memory
across modules.


Diffs
-

  cmake/CompilationConfigure.cmake efee36c1ffda096a97af23d481fc0d0903124e54 


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


Testing (updated)
---

NOTE: I checked with Alex Ionescu and since we are deploying on the same OS 
versions as we're targetting, we don't need to worry about redistributing the 
CRT.

If/when we start deploying on older OS versions, we can simply add instructions 
on installing the correct CRT redistributable (or make it part of the eventual 
installer).


Thanks,

Andrew Schwartzmeyer

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

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66043 was successfully built and tested.

Reviews applied: `['66043']`

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

- Mesos Reviewbot Windows


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: Added `PROTOC_SPEC_GENERATE` helper for cmake.

[Chun-Hung Hsiao]

> On March 13, 2018, 6:54 p.m., Andrew Schwartzmeyer wrote:
> > src/cmake/MesosProtobuf.cmake
> > Lines 125-163 (patched)
> > 
> >
> > Most of this function looks duplicated from `function(PROTOC_GENERATE)` 
> > but with additional GRPC logic. Can they be combined or refactored? Looking 
> > at it, it seems we could add the same options instead to 
> > `function(PROTOC_GENERATE)` and re-use it. Is there anything that 
> > `function(PROTOC_GENERATE)` does which `function(PROTOC_SPEC_GENERATE)` 
> > _shouldn't_ do, and can that be conditionalized?
> > 
> > I'm not saying this is a great idea; it may not make sense to share the 
> > code. But let's take a look.

Let me come up with another prototype that combines the two and we can see if 
it looks cleaner.


- Chun-Hung


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


On March 9, 2018, 10:44 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65997/
> ---
> 
> (Updated March 9, 2018, 10:44 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_SPEC_GENERATE is a convenience function that will:
>   (1) Compile .proto files found in the third-party specification
>   library under its include directory and place the generated files
>   in the build folder, under the `include/` directory, or with the
>   option `INTERNAL` the `src/` directory.
>   (2) Append to list variables `PUBLIC_PROTO_PATH` or
>   `INTERNAL_PROTO_PATH` the fully qualified path to the library's
>   include directory depending on options passed in.
>   (3) Append to list variables `PUBLIC_PROTOBUF_INCLUDE_DIR` or
>   `INTERNAL_PROTOBUF_INCLUDE_DIR` (depending on options passed in)
>   the fully qualified path to the directory where the files are
>   generated.
>   (4) Append to list variables `PUBLIC_PROTOBUF_SRC` or
>   `INTERNAL_PROTOBUF_SRC` (depending on options passed in) the fully
>   qualified path to the generated .cc file.
> Where the exports in (2)(3)(4) are *side effects* and modifies the
> variables in the parent scope.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
>   src/cmake/MesosProtobuf.cmake ef90c15a6f2afaedaf7559c4a411098924528505 
> 
> 
> Diff: https://reviews.apache.org/r/65997/diff/2/
> 
> 
> 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/#review199106
---




3rdparty/CMakeLists.txt
Lines 260 (patched)


Nit: a few more `###` :)



src/CMakeLists.txt
Lines 31-32 (patched)


But also the prior directories too; hence why I think these two functions 
may be combined with a `SPEC` option or something to `PROTOC_GENERATE`.


- Andrew Schwartzmeyer


On March 9, 2018, 2:45 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66015/
> ---
> 
> (Updated March 9, 2018, 2:45 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/1/
> 
> 
> Testing
> ---
> 
> `make check` with cmake.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

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/#review199105
---



PASS: Mesos patch 66037 was successfully built and tested.

Reviews applied: `['66037']`

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

- Mesos Reviewbot Windows


On March 13, 2018, 5:26 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66037/
> ---
> 
> (Updated March 13, 2018, 5:26 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-8647
> https://issues.apache.org/jira/browse/MESOS-8647
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 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.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.cpp 9b60bd0e808aab272039239db95bee71a3d910ab 
>   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/slave_recovery_tests.cpp c856752fe1dc3f5b45adb21b65a736116184e10a 
>   src/tests/slave_tests.cpp c2afaa573039e25fb59496765e6717e7643a692f 
> 
> 
> Diff: https://reviews.apache.org/r/66037/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65997: Added `PROTOC_SPEC_GENERATE` helper for cmake.

[Andrew Schwartzmeyer]

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




src/CMakeLists.txt
Lines 106 (patched)


Should these be `PRIVATE` since they're "internal"?



src/CMakeLists.txt
Line 487 (original), 496 (patched)


I guess this comment is out-of-date here; it should be deleted.



src/CMakeLists.txt
Lines 497-498 (patched)


This is not necessary because mesos links to mesos-protobufs, the protobufs 
library/interface.



src/cmake/MesosProtobuf.cmake
Lines 125-163 (patched)


Most of this function looks duplicated from `function(PROTOC_GENERATE)` but 
with additional GRPC logic. Can they be combined or refactored? Looking at it, 
it seems we could add the same options instead to `function(PROTOC_GENERATE)` 
and re-use it. Is there anything that `function(PROTOC_GENERATE)` does which 
`function(PROTOC_SPEC_GENERATE)` _shouldn't_ do, and can that be 
conditionalized?

I'm not saying this is a great idea; it may not make sense to share the 
code. But let's take a look.



src/cmake/MesosProtobuf.cmake
Lines 174-176 (patched)


This is probably something for later, but these essentially hard-coded 
directories have never sat well with me.


- Andrew Schwartzmeyer


On March 9, 2018, 2:44 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65997/
> ---
> 
> (Updated March 9, 2018, 2:44 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_SPEC_GENERATE is a convenience function that will:
>   (1) Compile .proto files found in the third-party specification
>   library under its include directory and place the generated files
>   in the build folder, under the `include/` directory, or with the
>   option `INTERNAL` the `src/` directory.
>   (2) Append to list variables `PUBLIC_PROTO_PATH` or
>   `INTERNAL_PROTO_PATH` the fully qualified path to the library's
>   include directory depending on options passed in.
>   (3) Append to list variables `PUBLIC_PROTOBUF_INCLUDE_DIR` or
>   `INTERNAL_PROTOBUF_INCLUDE_DIR` (depending on options passed in)
>   the fully qualified path to the directory where the files are
>   generated.
>   (4) Append to list variables `PUBLIC_PROTOBUF_SRC` or
>   `INTERNAL_PROTOBUF_SRC` (depending on options passed in) the fully
>   qualified path to the generated .cc file.
> Where the exports in (2)(3)(4) are *side effects* and modifies the
> variables in the parent scope.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
>   src/cmake/MesosProtobuf.cmake ef90c15a6f2afaedaf7559c4a411098924528505 
> 
> 
> Diff: https://reviews.apache.org/r/65997/diff/2/
> 
> 
> Testing
> ---
> 
> `make check` with cmake.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 59987: Added protobuf map support to stout JSON<->protobuf conversion.

[Chun-Hung Hsiao]

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


Ship it!




Ship It!

- Chun-Hung Hsiao


On March 13, 2018, 9:13 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59987/
> ---
> 
> (Updated March 13, 2018, 9:13 a.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
> ---
> 
> Map is a feature of proto2 syntax, but it can only be compiled
> with 3.0.0+ protobuf compiler, see the following discussion for
> details:
> 
> https://groups.google.com/forum/#!topic/protobuf/p4WxcplrlA4
> 
> We have already upgraded the compiler from 2.6.1 to 3.3.0 in
> MESOS-7228. This patch adds map support in the json <-> protobuf
> conversion in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/protobuf.hpp 
> 4a1605e5130dbf7e8286dbb43d0d04ab4394e79a 
> 
> 
> Diff: https://reviews.apache.org/r/59987/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 66025: Converted `JSON::String` to bool and numbers.

[Chun-Hung Hsiao]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/protobuf.hpp
Lines 469 (patched)


`"Failed to parse '" + ... + '" as a JSON number...` since we're using the 
same parse function no matter what the actual field type is.



3rdparty/stout/include/stout/protobuf.hpp
Lines 470 (patched)


`s/for the field/for field`. Ditto below.



3rdparty/stout/include/stout/protobuf.hpp
Lines 479 (patched)


`"Failed to parse '" ... " as a JSON boolean"...`


- Chun-Hung Hsiao


On March 13, 2018, 6:48 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66025/
> ---
> 
> (Updated March 13, 2018, 6:48 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-8656
> https://issues.apache.org/jira/browse/MESOS-8656
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previsouly when converting a JSON to a protobuf message in stout, we
> cannot handle the fields like below which are actually valid.
>   "int32": "-2147483647"
>   "int64": "-9223372036854775807"
>   "float": "1.5"
>   "bool": "true"
> The conversion will fail with an error like "Not expecting a JSON string
> for field 'int32'".
> 
> So in this patch, `Try operator()(const JSON::String& string)`
> was enhanced to be able to convert `JSON::String` to bool and numbers.
> This is to match Google's json -> protobuf behavior, see the doc below
> for details:
> https://developers.google.com/protocol-buffers/docs/proto3#json
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/protobuf.hpp 
> 4a1605e5130dbf7e8286dbb43d0d04ab4394e79a 
> 
> 
> Diff: https://reviews.apache.org/r/66025/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 66039: Check both disk and inode usage when slaves schedule gc.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66038, 66039]

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 13, 2018, 2:05 p.m., fei long wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66039/
> ---
> 
> (Updated March 13, 2018, 2:05 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-8665
> https://issues.apache.org/jira/browse/MESOS-8665
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check both disk and inode usage when slaves schedule gc.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 2f4ab157448eafc0f41372ee50255a76129e90db 
> 
> 
> Diff: https://reviews.apache.org/r/66039/diff/2/
> 
> 
> Testing
> ---
> 
> No new tests are added and all "make check" tests are passed.
> 
> 
> Thanks,
> 
> fei long
> 
>

Re: Review Request 65832: Displayed resource provider resources in GET_RESOURCE_PROVIDER response.

[Greg Mann]

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



This API change looks good to me. Since this is a public API change, could you 
email the dev list and notify folks of the change (and the one in the 
subsequent patch as well)?

- Greg Mann


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/65832/
> ---
> 
> (Updated Feb. 28, 2018, 12:55 p.m.)
> 
> 
> Review request for mesos, 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_RESOURCE_PROVIDER response.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 7d92cb8e3dc533081d973e488bd140c7d5ea2bbf 
>   include/mesos/v1/agent/agent.proto 59a9fd69df47c3605662529b5493cd3bf18c8397 
>   src/slave/http.cpp 7d7fa2b4ec2e1f8f65c5264ce72590d0d8195b9b 
>   src/tests/api_tests.cpp 9c172f751b11b3b50f23253d212c2eb603da9c2c 
> 
> 
> Diff: https://reviews.apache.org/r/65832/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

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

[Meng Zhu]

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

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 65833: Displayed resource provider resources in GET_AGENTS response.

[Greg Mann]

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



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.

- Greg Mann


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, 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 f40caa2a6338ff58a325444cfb72cd24f2c99472 
>   include/mesos/v1/master/master.proto 
> 67c9560eaa7bd6b20b73af07f7666f0d98c4453d 
>   src/common/protobuf_utils.cpp d2ada356be4c26290692bc99e7ec5121039bda4e 
>   src/tests/api_tests.cpp 9c172f751b11b3b50f23253d212c2eb603da9c2c 
> 
> 
> Diff: https://reviews.apache.org/r/65833/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63994: Added a new test for validation of offer operation IDs.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63994 was successfully built and tested.

Reviews applied: `['63991', '63992', '63994']`

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

- Mesos Reviewbot Windows


On March 13, 2018, 4:41 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63994/
> ---
> 
> (Updated March 13, 2018, 4:41 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 two new scheduler tests,
> 'OperationFeedbackValidationWithResourceProviderCapability' and
> 'OperationFeedbackValidationNoResourceProviderCapability',
> to verify that task and offer operation status updates are correctly
> sent when the 'id' field is set incorrectly on an operation.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d 
> 
> 
> Diff: https://reviews.apache.org/r/63994/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

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

[John Kordich via Review Board]

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


Ship it!




Ship It!

- John Kordich


On March 9, 2018, 10: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, 10: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 65591: Explicitly tracked resource providers in master.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On March 7, 2018, 11:36 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65591/
> ---
> 
> (Updated March 7, 2018, 11:36 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> 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
> -
> 
>   src/common/protobuf_utils.cpp 9c5fb97afb58f98013b79f3cbaea7dacc3603271 
>   src/master/http.cpp acf10a0f396234ec9f444c9b41515d657379d6ad 
>   src/master/master.hpp c9c8a968b6f56fe261ac9fe374e926a28d40fccb 
>   src/master/master.cpp e666b664dd125317cda5d16285d444b9c21e1f35 
> 
> 
> Diff: https://reviews.apache.org/r/65591/diff/8/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

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

Review request for mesos, Jie Yu and Jan Schlicht.


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


Repository: mesos


Description
---

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.


Diffs
-

  src/slave/constants.cpp 9b60bd0e808aab272039239db95bee71a3d910ab 
  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/slave_recovery_tests.cpp c856752fe1dc3f5b45adb21b65a736116184e10a 
  src/tests/slave_tests.cpp c2afaa573039e25fb59496765e6717e7643a692f 


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


Testing
---

`sudo make check`


Thanks,

Benjamin Bannier

Re: Review Request 63994: Added a new test for validation of offer operation IDs.

[Greg Mann]

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

(Updated March 13, 2018, 4:41 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 (updated)
---

This patch adds two new scheduler tests,
'OperationFeedbackValidationWithResourceProviderCapability' and
'OperationFeedbackValidationNoResourceProviderCapability',
to verify that task and offer operation status updates are correctly
sent when the 'id' field is set incorrectly on an operation.


Diffs
-

  src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d 


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


Testing
---

make check


Thanks,

Greg Mann

Re: Review Request 63994: Added a new test for validation of offer operation IDs.

[Greg Mann]

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

(Updated March 13, 2018, 4:39 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 a new test,
'SchedulerTest.OfferOperationFeedbackValidation',
to verify that task and offer operation status updates are correctly
sent when the 'id' field is set incorrectly on an operation.


Diffs (updated)
-

  src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d 


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

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


Testing
---

make check


Thanks,

Greg Mann

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

[Greg Mann]

> On March 7, 2018, 7:21 p.m., Gaston Kleiman wrote:
> > src/master/master.cpp
> > Line 4136 (original), 4184 (patched)
> > 
> >
> > If the operation has an operation ID, we'll always land on this 
> > `continue` even if the validations/checks pass.
> > 
> > This means that line #4187 will be skipped, and operations with an 
> > operatin ID won't be added to the object passed to the `_accept` 
> > continuation.
> 
> Greg Mann wrote:
> Ah thank you; this should be removed.

I elected to add another case to the conditional because I thought this was 
more readable; let me know what you think.


- Greg


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


On March 13, 2018, 4:38 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, 4:38 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/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

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

[Greg Mann]

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

(Updated March 13, 2018, 4:38 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 (updated)
-

  src/master/master.cpp 223ebf29ac4dd1dea9080e4bef4b2d4d064d847f 


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

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


Testing
---

make check


Thanks,

Greg Mann

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

[Greg Mann]

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

(Updated March 13, 2018, 4: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 (updated)
-

  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/

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


Testing
---

make check


Thanks,

Greg Mann

Re: Review Request 66039: Check both disk and inode usage when slaves schedule gc.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66039 was successfully built and tested.

Reviews applied: `['66038', '66039']`

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

- Mesos Reviewbot Windows


On March 13, 2018, 2:05 p.m., fei long wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66039/
> ---
> 
> (Updated March 13, 2018, 2:05 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-8665
> https://issues.apache.org/jira/browse/MESOS-8665
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check both disk and inode usage when slaves schedule gc.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 2f4ab157448eafc0f41372ee50255a76129e90db 
> 
> 
> Diff: https://reviews.apache.org/r/66039/diff/2/
> 
> 
> Testing
> ---
> 
> No new tests are added and all "make check" tests are passed.
> 
> 
> Thanks,
> 
> fei long
> 
>

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

[Greg Mann]

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


Fix it, then Ship it!





src/slave/http.cpp
Lines 2199-2218 (original), 2031-2039 (patched)


Not indented enough.



src/slave/http.cpp
Lines 2250-2276 (original), 2071-2080 (patched)


Not indented far enough; here and elsewhere. Could you double-check the 
indentation on these deferred continuations?



src/slave/http.cpp
Lines 3037-3044 (original), 2857-2864 (patched)


Not indented far enough.


- Greg Mann


On March 9, 2018, 1:06 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65313/
> ---
> 
> (Updated March 9, 2018, 1:06 p.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
> -
> 
>   src/slave/http.hpp c33adeb2ddd36e8be1324b3ddb1401bdf7e4e80b 
>   src/slave/http.cpp 7d7fa2b4ec2e1f8f65c5264ce72590d0d8195b9b 
>   src/slave/slave.cpp 8cb6899bf15fb697c3cb2784f63b7c2d5729d219 
> 
> 
> Diff: https://reviews.apache.org/r/65313/diff/7/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 66001: MESOS-6575: Add soft limit and kill to disk/xfs.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66001]

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 13, 2018, 1:45 p.m., Harold Dost wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66001/
> ---
> 
> (Updated March 13, 2018, 1:45 p.m.)
> 
> 
> Review request for mesos and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> New Flags for disk/xfs isolator
> - This patch adds a number of flags to handle switching the limit in the
>   `disk/xfs` isolator to allow apps to go over their limit and for mesos
> to kill them if they have gone over their limit.
> 
> New Flags:
> - xfs_disk_hard_limit_offset_pct - Use the `disk` as the soft limit and
>   set the hard limit to be some percentage above the soft limit.
> Allowing
>   for containers to surpass a desired allocation and making them
> killable.
> - xfs_disk_hard_limit_offset - Use the `disk` as the soft limit and set
>   the hard limit to some number of bytes specified above the
>   applications to be a soft limit instead of a hard limit.
> - xfs_kill_after_grace_period - This will kill tasks if they breach the
>   grace period configured using `xfs_quota -x -c "timer -p "`
> - xfs_kill_check_interval - The frequency with which a container will be
>   checked for soft limit violations.
> 
> Functionality
> - Add head room to the hard limit as a percentage or specified amount
>   for each container. This is specified at a flag level and not
>   customizable on a per container basis.
> - Provide the ability for an application to be killed after the
>   configured grace period for projects is violated.
> - Add an interval between which the watcher will check for violations.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp 
> 07e68a777aefba4dd35066f2eb207bba7f199d83 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
> 8d9f8f846866f9de377c59cb7fb311041283ba70 
>   src/slave/containerizer/mesos/isolators/xfs/utils.hpp 
> e034133629a9c1cf58b776f8da2a93421332cee0 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
> 2708524add1ff693b616d4fb241c4a0a3070520b 
>   src/slave/flags.hpp 0c67bf214ceb93ae7ff088bec2648fa26ddac59e 
>   src/slave/flags.cpp 943aaaf58b5f36555f0902019b8c5c6522ab7afc 
> 
> 
> Diff: https://reviews.apache.org/r/66001/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Harold Dost
> 
>

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

[Greg Mann]

> On March 9, 2018, 5:30 a.m., Greg Mann wrote:
> > src/slave/http.cpp
> > Lines 2641-2665 (original), 2431-2469 (patched)
> > 
> >
> > Why did you opt for a new templated method instead of the pre-existing 
> > lambda, here and elsewhere? Is it necessary because we pass the action as a 
> > template parameter?
> 
> Alexander Rojas wrote:
> It has to do more with the `Approvers::approved()` being parametrize, 
> which means that the type needs to be resolved on compilation time, so this 
> is a neat trick for that.
> 
> Greg Mann wrote:
> IMO it's unfortunate that we need to do this, since it makes this code 
> harder to read.
> 
> Actually, looking again at `ObjectApprovers`, I wonder if the action 
> really needs to be a template parameter. We could also do something like:
> ```
> class ObjectApprovers {
>   template 
>   bool approved(const authorization::Action& action, const Args&... args);
> 
> ...
> ```
> 
> Then a callsite would look like
> ```
> if (!approvers->approved(SOME_ACTION, someInfo)) {
> ```
> which seems OK to me. WDYT?

As we discussed in chat, we can get rid of some of these extra continuations if 
we duplicate some code. Let's get this stuff merged now to avoid more rebasing, 
then we can clean up later.


- Greg


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


On March 9, 2018, 1:06 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65313/
> ---
> 
> (Updated March 9, 2018, 1:06 p.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
> -
> 
>   src/slave/http.hpp c33adeb2ddd36e8be1324b3ddb1401bdf7e4e80b 
>   src/slave/http.cpp 7d7fa2b4ec2e1f8f65c5264ce72590d0d8195b9b 
>   src/slave/slave.cpp 8cb6899bf15fb697c3cb2784f63b7c2d5729d219 
> 
> 
> Diff: https://reviews.apache.org/r/65313/diff/7/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 65311: Added the ObjectApprovers to which unifies authorization logic.

[Greg Mann]

> On March 9, 2018, 10:16 p.m., Greg Mann wrote:
> > src/common/http.hpp
> > Lines 198 (patched)
> > 
> >
> > See the comment I left on https://reviews.apache.org/r/65313/ - I 
> > wonder if `action` really needs to be a template parameter here?
> 
> Alexander Rojas wrote:
> Strictly speaking it doesn't need to be a template, but then overrides 
> like:
> 
> ```c++
> template <>
> inline bool ObjectApprovers::approved(
> const Resource& resource)
> {
>   // ...
> }
> ```
> 
> Would have need to be dispatched to helper functions or handler in big 
> ifs in `approved()`.
> 
> The end result would be, you just move the part hard to read from one 
> side to the next.
> 
> My personal opinion is that the helper in the agent cleanup is more 
> readable an a better price to pay than a gigantic switch here.

As we discussed in chat, we can clean up the callsites in the agent a bit by 
duplicating code if we want. Let's leave this as-is right now.


- Greg


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


On March 8, 2018, 12:14 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65311/
> ---
> 
> (Updated March 8, 2018, 12:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Greg Mann.
> 
> 
> Bugs: MESOS-8434
> https://issues.apache.org/jira/browse/MESOS-8434
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the class `ObjectApprovers` which unifies the different
> mechanisms used in Mesos in order to perform authorization.
> 
> This new class will supersede the `Acceptor` interfaces and their
> logic.
> 
> Follow up patches will make use of this interface and remove
> deprecated code.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 750d3bfba1647624983108bdd23295a3b3091fe4 
>   src/common/http.cpp 8d3a4ad63bd74e0313082bad3d89c21fbf54f256 
> 
> 
> Diff: https://reviews.apache.org/r/65311/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 66001: MESOS-6575: Add soft limit and kill to disk/xfs.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66001 was successfully built and tested.

Reviews applied: `['66001']`

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

- Mesos Reviewbot Windows


On March 13, 2018, 1:45 p.m., Harold Dost wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66001/
> ---
> 
> (Updated March 13, 2018, 1:45 p.m.)
> 
> 
> Review request for mesos and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> New Flags for disk/xfs isolator
> - This patch adds a number of flags to handle switching the limit in the
>   `disk/xfs` isolator to allow apps to go over their limit and for mesos
> to kill them if they have gone over their limit.
> 
> New Flags:
> - xfs_disk_hard_limit_offset_pct - Use the `disk` as the soft limit and
>   set the hard limit to be some percentage above the soft limit.
> Allowing
>   for containers to surpass a desired allocation and making them
> killable.
> - xfs_disk_hard_limit_offset - Use the `disk` as the soft limit and set
>   the hard limit to some number of bytes specified above the
>   applications to be a soft limit instead of a hard limit.
> - xfs_kill_after_grace_period - This will kill tasks if they breach the
>   grace period configured using `xfs_quota -x -c "timer -p "`
> - xfs_kill_check_interval - The frequency with which a container will be
>   checked for soft limit violations.
> 
> Functionality
> - Add head room to the hard limit as a percentage or specified amount
>   for each container. This is specified at a flag level and not
>   customizable on a per container basis.
> - Provide the ability for an application to be killed after the
>   configured grace period for projects is violated.
> - Add an interval between which the watcher will check for violations.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp 
> 07e68a777aefba4dd35066f2eb207bba7f199d83 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
> 8d9f8f846866f9de377c59cb7fb311041283ba70 
>   src/slave/containerizer/mesos/isolators/xfs/utils.hpp 
> e034133629a9c1cf58b776f8da2a93421332cee0 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
> 2708524add1ff693b616d4fb241c4a0a3070520b 
>   src/slave/flags.hpp 0c67bf214ceb93ae7ff088bec2648fa26ddac59e 
>   src/slave/flags.cpp 943aaaf58b5f36555f0902019b8c5c6522ab7afc 
> 
> 
> Diff: https://reviews.apache.org/r/66001/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Harold Dost
> 
>

Re: Review Request 66038: Add usageWithInode to check both disk and inode usage.

[fei long]

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

(Updated March 13, 2018, 2:08 p.m.)


Review request for mesos and Gilbert Song.


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


Repository: mesos


Description (updated)
---

Add usageWithInode to check both disk and inode usage.


Diffs (updated)
-

  3rdparty/stout/include/stout/posix/fs.hpp 
269a4f50f1df8d68be9e11030f885cf2c254c9d8 


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

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


Testing
---

No new tests are added and all "make check" tests are passed.


Thanks,

fei long

Re: Review Request 66039: Check both disk and inode usage when slaves schedule gc.

[fei long]

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

(Updated March 13, 2018, 2:05 p.m.)


Review request for mesos and Gilbert Song.


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


Repository: mesos


Description (updated)
---

Check both disk and inode usage when slaves schedule gc.


Diffs (updated)
-

  src/slave/slave.cpp 2f4ab157448eafc0f41372ee50255a76129e90db 


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

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


Testing
---

No new tests are added and all "make check" tests are passed.


Thanks,

fei long

Re: Review Request 66001: MESOS-6575: Add soft limit and kill to disk/xfs.

[Harold Dost]

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

(Updated March 13, 2018, 1:45 p.m.)


Review request for mesos and James Peach.


Repository: mesos


Description (updated)
---

New Flags for disk/xfs isolator
- This patch adds a number of flags to handle switching the limit in the
  `disk/xfs` isolator to allow apps to go over their limit and for mesos
to kill them if they have gone over their limit.

New Flags:
- xfs_disk_hard_limit_offset_pct - Use the `disk` as the soft limit and
  set the hard limit to be some percentage above the soft limit.
Allowing
  for containers to surpass a desired allocation and making them
killable.
- xfs_disk_hard_limit_offset - Use the `disk` as the soft limit and set
  the hard limit to some number of bytes specified above the
  applications to be a soft limit instead of a hard limit.
- xfs_kill_after_grace_period - This will kill tasks if they breach the
  grace period configured using `xfs_quota -x -c "timer -p "`
- xfs_kill_check_interval - The frequency with which a container will be
  checked for soft limit violations.

Functionality
- Add head room to the hard limit as a percentage or specified amount
  for each container. This is specified at a flag level and not
  customizable on a per container basis.
- Provide the ability for an application to be killed after the
  configured grace period for projects is violated.
- Add an interval between which the watcher will check for violations.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/xfs/disk.hpp 
07e68a777aefba4dd35066f2eb207bba7f199d83 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
8d9f8f846866f9de377c59cb7fb311041283ba70 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp 
e034133629a9c1cf58b776f8da2a93421332cee0 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
2708524add1ff693b616d4fb241c4a0a3070520b 
  src/slave/flags.hpp 0c67bf214ceb93ae7ff088bec2648fa26ddac59e 
  src/slave/flags.cpp 943aaaf58b5f36555f0902019b8c5c6522ab7afc 


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

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


Testing
---


Thanks,

Harold Dost

Re: Review Request 66039: Check both disk and inode usage when slaves schedule gc.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66038', '66039']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
 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\mesos\src\java\jni\org_apache_mesos_state_Variable.cpp(44): 
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\org_apache_mesos_state_Variable.cpp(45): 
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\jvm\jvm.cpp(95): warning C4267: '=': 
conversion from 'size_t' to 'jint', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(463): warning C4244: 
'initializing': conversion from 'jchar' to 'char', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(493): warning C4244: 
'initializing': conversion from 'jlong' to 'long', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(654): warning C4244: 
'initializing': conversion from 'jchar' to 'char', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(693): warning C4244: 
'initializing': conversion from 'jlong' to 'long', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(770): warning C4244: 
'initializing': conversion from 'jchar' to 'char', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(809): warning C4244: 
'initializing': conversion from 'jlong' to 'long', 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) (16) ->
   (ClCompile target) -> 
 D:\DCOS\mesos\mesos\src\slave\slave.cpp(6746): error C2039: 
'usageWithInode': is not a member of 'fs' [D:\DCOS\mesos\src\mesos.vcxproj]
 D:\DCOS\mesos\mesos\src\slave\slave.cpp(6746): error C3861: 
'usageWithInode': identifier not found [D:\DCOS\mesos\src\mesos.vcxproj]
 D:\DCOS\mesos\mesos\src\slave\slave.cpp(6746): error C2228: left of 
'.onAny' must have class/struct/union [D:\DCOS\mesos\src\mesos.vcxproj]
 D:\DCOS\mesos\mesos\src\slave\slave.cpp(6799): error C2039: 
'usageWithInode': is not a member of 'fs' [D:\DCOS\mesos\src\mesos.vcxproj]
 D:\DCOS\mesos\mesos\src\slave\slave.cpp(6799): error C3861: 
'usageWithInode': identifier not found [D:\DCOS\mesos\src\mesos.vcxproj]
 D:\DCOS\mesos\mesos\src\slave\slave.cpp(6799): error C2228: left of 
'.onAny' must have class/struct/union [D:\DCOS\mesos\src\mesos.vcxproj]

214 Warning(s)
6 Error(s)

Time Elapsed 00:12:25.06
```

- Mesos Reviewbot Windows


On March 13, 2018, 2:05 p.m., fei long wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66039/
> ---
> 
> (Updated March 13, 2018, 2:05 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-8665
> https://issues.apache.org/jira/browse/MESOS-8665
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check both disk and inode usage when slaves schedule gc.
> Call the newly added fs::usageWithInode to determine 
> executorDirectoryMaxAllowedAge when slaves schedule gc.
> 
> 
> Diffs
> -
> 
>   

Review Request 66038: Add usageWithInode to check both disk and inode usage.

[fei long]

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

Review request for mesos and Gilbert Song.


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


Repository: mesos


Description
---

Add fs::usageWithInode to check both disk and inode usage, which returns 
max(diskUsage, inodeUsage).


Diffs
-

  3rdparty/stout/include/stout/posix/fs.hpp 
269a4f50f1df8d68be9e11030f885cf2c254c9d8 


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


Testing
---

No new tests are added and all "make check" tests are passed.


Thanks,

fei long

Review Request 66039: Check both disk and inode usage when slaves schedule gc.

[fei long]

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

Review request for mesos and Gilbert Song.


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


Repository: mesos


Description
---

Check both disk and inode usage when slaves schedule gc.
Call the newly added fs::usageWithInode to determine 
executorDirectoryMaxAllowedAge when slaves schedule gc.


Diffs
-

  src/slave/slave.cpp 2f4ab157448eafc0f41372ee50255a76129e90db 


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


Testing
---

No new tests are added and all "make check" tests are passed.


Thanks,

fei long

Re: Review Request 66035: Updated mesos-tidy setup to be based on upstream 6.0 release.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66035]

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 13, 2018, 9:49 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66035/
> ---
> 
> (Updated March 13, 2018, 9:49 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated mesos-tidy setup to be based on upstream 6.0 release.
> 
> 
> Diffs
> -
> 
>   support/mesos-tidy/Dockerfile f046a1410a3012509babb0558998f33b9e241c43 
> 
> 
> Diff: https://reviews.apache.org/r/66035/diff/1/
> 
> 
> Testing
> ---
> 
> * LLVM unit test suite passes
> * new image does not produce false positives
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66035: Updated mesos-tidy setup to be based on upstream 6.0 release.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66035 was successfully built and tested.

Reviews applied: `['66035']`

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

- Mesos Reviewbot Windows


On March 13, 2018, 2:49 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66035/
> ---
> 
> (Updated March 13, 2018, 2:49 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated mesos-tidy setup to be based on upstream 6.0 release.
> 
> 
> Diffs
> -
> 
>   support/mesos-tidy/Dockerfile f046a1410a3012509babb0558998f33b9e241c43 
> 
> 
> Diff: https://reviews.apache.org/r/66035/diff/1/
> 
> 
> Testing
> ---
> 
> * LLVM unit test suite passes
> * new image does not produce false positives
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 66035: Updated mesos-tidy setup to be based on upstream 6.0 release.

[Benjamin Bannier]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Updated mesos-tidy setup to be based on upstream 6.0 release.


Diffs
-

  support/mesos-tidy/Dockerfile f046a1410a3012509babb0558998f33b9e241c43 


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


Testing
---

* LLVM unit test suite passes
* new image does not produce false positives


Thanks,

Benjamin Bannier

Re: Review Request 65994: Made the master forward operation status updates to the schedulers.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [65362, 65300, 64618, 65993, 65994]

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 8, 2018, 10:07 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65994/
> ---
> 
> (Updated March 8, 2018, 10:07 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8189
> https://issues.apache.org/jira/browse/MESOS-8189
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the master forward operation status updates to the schedulers.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 223ebf29ac4dd1dea9080e4bef4b2d4d064d847f 
> 
> 
> Diff: https://reviews.apache.org/r/65994/diff/2/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 59987: Added protobuf map support to stout JSON<->protobuf conversion.

[Qian Zhang]

> On March 13, 2018, 10:07 a.m., Chun-Hung Hsiao wrote:
> > 3rdparty/stout/include/stout/protobuf.hpp
> > Lines 807-810 (patched)
> > 
> >
> > How about making this a lambda inside `protobuf()` so we don't expose 
> > this?

Agree!


- Qian


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


On March 13, 2018, 5:13 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59987/
> ---
> 
> (Updated March 13, 2018, 5:13 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
> ---
> 
> Map is a feature of proto2 syntax, but it can only be compiled
> with 3.0.0+ protobuf compiler, see the following discussion for
> details:
> 
> https://groups.google.com/forum/#!topic/protobuf/p4WxcplrlA4
> 
> We have already upgraded the compiler from 2.6.1 to 3.3.0 in
> MESOS-7228. This patch adds map support in the json <-> protobuf
> conversion in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/protobuf.hpp 
> 4a1605e5130dbf7e8286dbb43d0d04ab4394e79a 
> 
> 
> Diff: https://reviews.apache.org/r/59987/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 59987: Added protobuf map support to stout JSON<->protobuf conversion.

[Qian Zhang]

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

(Updated March 13, 2018, 5:13 p.m.)


Review request for mesos, Anand Mazumdar, Benjamin Mahler, Chun-Hung Hsiao, and 
Zhitao Li.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Map is a feature of proto2 syntax, but it can only be compiled
with 3.0.0+ protobuf compiler, see the following discussion for
details:

https://groups.google.com/forum/#!topic/protobuf/p4WxcplrlA4

We have already upgraded the compiler from 2.6.1 to 3.3.0 in
MESOS-7228. This patch adds map support in the json <-> protobuf
conversion in stout.


Diffs (updated)
-

  3rdparty/stout/include/stout/protobuf.hpp 
4a1605e5130dbf7e8286dbb43d0d04ab4394e79a 


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

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 66026: Updated `ProtobufTest.JSON` for parsing JSON::String to bools & numbers.

[Qian Zhang]

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

(Updated March 13, 2018, 3:09 p.m.)


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


Changes
---

Addressed review comments.


Summary (updated)
-

Updated `ProtobufTest.JSON` for parsing JSON::String to bools & numbers.


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


Repository: mesos


Description (updated)
---

Updated `ProtobufTest.JSON` for parsing JSON::String to bools & numbers.


Diffs (updated)
-

  3rdparty/stout/tests/protobuf_tests.cpp 
be35ad0d1e16501df67752a1818f79751419a43d 


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

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


Testing
---

sudo make check


Thanks,

Qian Zhang

Re: Review Request 66026: Updated `ProtobufTest.JSON` for parsing JSON::String to bools.

[Qian Zhang]

> On March 13, 2018, 9:41 a.m., Chun-Hung Hsiao wrote:
> > 3rdparty/stout/tests/protobuf_tests.cpp
> > Line 108 (original), 108 (patched)
> > 
> >
> > Would you mind using the C++-style string literals to avoid the 
> > escaping, like we do here? 
> > https://github.com/apache/mesos/blob/master/src/tests/storage_local_resource_provider_tests.cpp#L178
> > 
> > It would introduce inconsistency between this test and the remaining 
> > tests in this file. So it's fine if you don't want to do this.

BenM raised a similar comment in this patch: 
https://reviews.apache.org/r/59989/. And yes, I agree we should do that, I will 
post a separate patch to do that for all the tests in this file.


> On March 13, 2018, 9:41 a.m., Chun-Hung Hsiao wrote:
> > 3rdparty/stout/tests/protobuf_tests.cpp
> > Lines 141 (patched)
> > 
> >
> > Maybe `s/_expected/accepted/` to avoid naming confusion since we aren't 
> > expecting something equals to this string?

Agree.


> On March 13, 2018, 9:41 a.m., Chun-Hung Hsiao wrote:
> > 3rdparty/stout/tests/protobuf_tests.cpp
> > Lines 145-156 (patched)
> > 
> >
> > We probably want to make `d`, `f`, `repeated_double` and 
> > `repeated_float` strings as well.

Yes! And also `optional_default`.


- Qian


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


On March 12, 2018, 9:30 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66026/
> ---
> 
> (Updated March 12, 2018, 9:30 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-8656
> https://issues.apache.org/jira/browse/MESOS-8656
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `ProtobufTest.JSON` for parsing JSON::String to bools
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/protobuf_tests.cpp 
> be35ad0d1e16501df67752a1818f79751419a43d 
> 
> 
> Diff: https://reviews.apache.org/r/66026/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 66025: Converted `JSON::String` to bool and numbers.

[Qian Zhang]

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

(Updated March 13, 2018, 2:48 p.m.)


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


Changes
---

Addressed review comments.


Summary (updated)
-

Converted `JSON::String` to bool and numbers.


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


Repository: mesos


Description (updated)
---

Previsouly when converting a JSON to a protobuf message in stout, we
cannot handle the fields like below which are actually valid.
  "int32": "-2147483647"
  "int64": "-9223372036854775807"
  "float": "1.5"
  "bool": "true"
The conversion will fail with an error like "Not expecting a JSON string
for field 'int32'".

So in this patch, `Try operator()(const JSON::String& string)`
was enhanced to be able to convert `JSON::String` to bool and numbers.
This is to match Google's json -> protobuf behavior, see the doc below
for details:
https://developers.google.com/protocol-buffers/docs/proto3#json


Diffs (updated)
-

  3rdparty/stout/include/stout/protobuf.hpp 
4a1605e5130dbf7e8286dbb43d0d04ab4394e79a 


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

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


Testing
---

sudo make check


Thanks,

Qian Zhang

Re: Review Request 66025: Converted `JSON::String` to bool and integers.

[Qian Zhang]

> On March 13, 2018, 9:22 a.m., Chun-Hung Hsiao wrote:
> > 3rdparty/stout/include/stout/protobuf.hpp
> > Lines 454-538 (patched)
> > 
> >
> > As Ben mentioned, this is to match the "Notes" column of Google's JSON 
> > apping table ( 
> > https://developers.google.com/protocol-buffers/docs/proto3#json), except 
> > that the documentation missed the note that string values are also accepted 
> > for boolean fields.
> > 
> > Therefore, we probably should also include string -> float/double 
> > conversion. Given this, I'd suggest that we do the following instead to 
> > further simplify the logic:
> > 
> > ```
> > case ...: // All 32-bit and 64-bit ints, float an double
> > case ...: {
> >   Try number = JSON::parse(string.value);
> > 
> >   if (number.isError()) {
> > return Error(...);
> >   }
> > 
> >   return operator()(number);
> > }
> > case TYPE_BOOL: {
> >   Try boolean = JSON::parse(string.value);
> >   
> >   if (boolean.isError()) {
> > return Error(...);
> >   }
> >   
> >   return operator()(boolean);
> > }
> > ```

Agree, thanks!


- Qian


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


On March 12, 2018, 9:28 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66025/
> ---
> 
> (Updated March 12, 2018, 9:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-8656
> https://issues.apache.org/jira/browse/MESOS-8656
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previsouly when converting a JSON to a protobuf message in stout, we
> cannot handle the fields like below which are actually valid.
>   "int32": "-2147483647"
>   "int64": "-9223372036854775807"
>   "bool": "true"
> The conversion will fail with an error like "Not expecting a JSON string
> for field 'int32'".
> 
> So in this patch, `Try operator()(const JSON::String& string)`
> was enhanced to be able to convert `JSON::String` to bool and integers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/protobuf.hpp 
> 4a1605e5130dbf7e8286dbb43d0d04ab4394e79a 
> 
> 
> Diff: https://reviews.apache.org/r/66025/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 66025: Converted `JSON::String` to bool and integers.

[Qian Zhang]

> On March 13, 2018, 5:53 a.m., Benjamin Mahler wrote:
> > Can we clarify in the commit message that we're doing this to match 
> > Google's json -> protobuf behavior?

Sure.


- Qian


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


On March 12, 2018, 9:28 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66025/
> ---
> 
> (Updated March 12, 2018, 9:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-8656
> https://issues.apache.org/jira/browse/MESOS-8656
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previsouly when converting a JSON to a protobuf message in stout, we
> cannot handle the fields like below which are actually valid.
>   "int32": "-2147483647"
>   "int64": "-9223372036854775807"
>   "bool": "true"
> The conversion will fail with an error like "Not expecting a JSON string
> for field 'int32'".
> 
> So in this patch, `Try operator()(const JSON::String& string)`
> was enhanced to be able to convert `JSON::String` to bool and integers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/protobuf.hpp 
> 4a1605e5130dbf7e8286dbb43d0d04ab4394e79a 
> 
> 
> Diff: https://reviews.apache.org/r/66025/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 65886: Do not realpath executables from launcher_dir.

[Benjamin Peterson]

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

(Updated March 13, 2018, 6:33 a.m.)


Review request for mesos.


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


Repository: mesos


Description
---

Mesos realpaths some helper executables (e.g., mesos-executor) but not
others (mesos-executor and mesos-fetcher) before executing them. I
don't see any reason for the inconsistency here, and not doing it
saves some code.


Diffs
-

  src/slave/containerizer/fetcher.cpp a49411b7bac2d5a50a75d0b802842c2f61fe58c6 
  src/slave/slave.cpp e2facb3c15a2f907f6497c58a36842ed707f2c70 


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


Testing
---

`make check` passes.


Thanks,

Benjamin Peterson

Re: Review Request 66034: Remount several proc filesystem entries as read-only.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66034]

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 13, 2018, 1:29 a.m., Jason Lai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66034/
> ---
> 
> (Updated March 13, 2018, 1:29 a.m.)
> 
> 
> Review request for mesos, Eric Chung, Gilbert Song, Ian Downes, Jie Yu, James 
> Peach, and Zhitao Li.
> 
> 
> Bugs: MESOS-8654
> https://issues.apache.org/jira/browse/MESOS-8654
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Several entries under the proc FS within Mesos containers need to be
> remounted as readonly for improved security reasons.
> 
> The list should include the important ones introduced by Systemd's
> `ProtectKernelTunables` option:
> 
> * `/proc/bus`
> * `/proc/fs`
> * `/proc/irq`
> * `/proc/sys`
> * `/proc/sysrq-trigger`
> 
> It is particularly necessary to remount `/proc/sysrq-trigger` as
> read-only. Otherwise, it would be possible for processes running in
> containers as `root` to perform privileged operations, such as host
> reboot.
> 
> Extra mount options should include `nosuid,noexec,nodev` (see also
> `mount(2)` for detailed explanations of the options).
> 
> 
> Diffs
> -
> 
>   src/linux/fs.cpp ed26f80ef7315809a1df9f2c50b4fe3445810f8a 
> 
> 
> Diff: https://reviews.apache.org/r/66034/diff/1/
> 
> 
> Testing
> ---
> 
> The mount table of the container launched by the patched version of 
> `mesos-containerizer launch` include the entries listed above, with 
> `nosuid,noexec,nodev` mount points.
> ```
> $ sudo unshare -m -p -f /usr/local/libexec/mesos/mesos-containerizer launch 
> --launch_info="$(jq -c . launch_info.json)" --runtime_directory="$(pwd)"
> Marked '/' as rslave
> Prepared mount 
> '{"flags":20480,"source":"\/etc\/hostname","target":"\/home\/jlai\/containers\/rootfs\/etc\/hostname"}'
> Prepared mount 
> '{"flags":20480,"source":"\/etc\/hosts","target":"\/home\/jlai\/containers\/rootfs\/etc\/hosts"}'
> Prepared mount 
> '{"flags":20480,"source":"\/etc\/resolv.conf","target":"\/home\/jlai\/containers\/rootfs\/etc\/resolv.conf"}'
> Changing root to /home/jlai/containers/rootfs
> bash-4.4# findmnt -a
> TARGET  SOURCE  FSTYPE  OPTIONS
> /   alpine  overlay 
> rw,relatime,lowerdir=overlay/lower,upperdir=overlay/upper,workdir=overlay/work
> |-/etc/hostname /dev/dm-0[/etc/hostname]ext4
> rw,noatime,errors=panic,data=ordered
> |-/etc/hosts/dev/dm-0[/etc/hosts]   ext4
> rw,noatime,errors=panic,data=ordered
> |-/etc/resolv.conf  /dev/dm-0[/etc/resolv.conf] ext4
> rw,noatime,errors=panic,data=ordered
> |-/proc procproc
> rw,nosuid,nodev,noexec,relatime
> | |-/proc/bus   proc[/bus]  proc
> ro,nosuid,nodev,noexec,relatime
> | |-/proc/fsproc[/fs]   proc
> ro,nosuid,nodev,noexec,relatime
> | |-/proc/irq   proc[/irq]  proc
> ro,nosuid,nodev,noexec,relatime
> | |-/proc/sys   proc[/sys]  proc
> ro,nosuid,nodev,noexec,relatime
> | `-/proc/sysrq-trigger proc[/sysrq-trigger]proc
> ro,nosuid,nodev,noexec,relatime
> |-/sys  sysfs   sysfs   
> ro,nosuid,nodev,noexec,relatime
> `-/dev  tmpfs   tmpfs   
> rw,nosuid,noexec,mode=755
>   |-/dev/ptsdevpts  devpts  
> rw,nosuid,noexec,relatime,mode=600,ptmxmode=666
>   `-/dev/shmtmpfs   tmpfs   rw,nosuid,nodev
> ```
> 
> 
> Thanks,
> 
> Jason Lai
> 
>