Re: Review Request 69474: Added tests for agent/executor heartbeating.

2018-11-29 Thread Mesos Reviewbot Windows 

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



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

Reviews applied: `['69463', '69472', '69473', '69474']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3426):
 warning C4996: 'strerror': This function or variable may be unsafe. Consider 
using strerror_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3500):
 warning C4996: 'inet_ntoa': Use inet_ntop() or InetNtop() instead or define 
_WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3501):
 warning C4996: 'sprintf': This function or variable may be unsafe. Consider 
using sprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3479):
 warning C4101: 'addrstr': unreferenced local variable 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\recordio.c(170):
 warning C4267: '=': conversion from 'size_t' to 'int32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\mt_adaptor.c(496):
 warning C4244: '=': conversion from 'time_t' to 'int32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(256):
 warning C4090: 'function': different 'const' qualifiers 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(166):
 warning C4716: 'pthread_cond_broadcast': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(205):
 warning C4716: 'pthread_cond_wait': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(124):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(128):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(279):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(301):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data

Re: Review Request 69163: Set agent and/or resource provider ID in operation status updates.

2018-11-29 Thread Greg Mann 

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



In the final line of the commit message: s/not/no/


src/master/master.cpp
Lines 2254-2255 (patched)


It would make sense to me that we set these IDs if/when they are available. 
Currently I don't think it matters since the updates sent here do not need to 
be acknowledged; perhaps mention that in the comment?



src/slave/slave.cpp
Line 4354 (original), 4353-4359 (patched)


Why no RP ID? Here and below.


- Greg Mann


On Nov. 27, 2018, 6 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69163/
> ---
> 
> (Updated Nov. 27, 2018, 6 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gastón Kleiman, and James DeFelice.
> 
> 
> Bugs: MESOS-9293
> https://issues.apache.org/jira/browse/MESOS-9293
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch sets agent and/or resource provider ID operation status
> update messages. This is not always possible, e.g., some operations
> might fail validation so that no corresponding IDs can be extracted.
> 
> Since operations failing validation are currently directly rejected by
> the master without going through a status update manager, they are not
> retried either. If a master status update manager for operations is
> introduced at a later point it should be possible to forward
> acknowledgements for updates to the master's update manager (no agent
> ID, not resource provider ID).
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 1662125ed3e47b179ee32d08c1d3af75553066ba 
>   src/common/protobuf_utils.cpp a45607eed4c4bae5010bcc3f3ffeabd6d911062a 
>   src/master/master.cpp b4b02d8b4d7d6d1aabda1f97b9bf824419f76a9e 
>   src/resource_provider/manager.cpp 6c81c430e9e1205d71982a7fa2bcd9aa15fc01b2 
>   src/resource_provider/storage/provider.cpp 
> c137fa4f13edc58d93c03a9dd32fdf9d38b38316 
>   src/slave/slave.cpp 858b78620e1ef33f3587d0bd95a684917aaf5bbb 
>   src/tests/master_tests.cpp 651bb9ba9298fc3d7179ed86487aa55be519ce12 
>   src/tests/mesos.hpp 576f4bde88c069ee2fa0dd33912a034437338e7e 
> 
> 
> Diff: https://reviews.apache.org/r/69163/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

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

2018-11-29 Thread Joseph Wu 

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



The `PIPE` issue below is currently not a problem, since none of these ReadOnly 
handlers chunk their responses.


src/master/http.cpp
Lines 2392 (patched)


Micro nit: s/Note that this is/NOTE: This is/



src/master/http.cpp
Lines 2401 (patched)


You might want to leave a note here, or elsewhere, to state that only 
`BODY` type Responses should be used here.  When we copy the `Future` 
to multiple recipients, we effectively copy the `std::string body` that 
comprises the response text.  

This might also work for `PATH` type responses, but since we'd be copying 
the path to a file, we'd still be reading the same file twice.

The `PIPE` response would be the one that should be avoided.  When you copy 
the `Option reader`, this does not duplicate the stream.  So 
multiple readers would each get separate parts of the stream, and neither would 
get a cohesive response (or if lucky, one reader would read everything, and the 
other reader(s) would get nothing).


- Joseph Wu


On Nov. 21, 2018, 5:20 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68795/
> ---
> 
> (Updated Nov. 21, 2018, 5:20 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change will skip the computation of an HTTP response
> for requests with the same principal, endpoint and request
> headers where both requests are authenticated within the
> same batching windows.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 68ee2a6dcffbc772afec6e797b1af8da48f61937 
>   src/master/master.hpp 3b3c1a4e61de9503c8d038dd3bee623ded5914c9 
> 
> 
> Diff: https://reviews.apache.org/r/68795/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 69162: Added agent and resource provider IDs to operation status messages.

2018-11-29 Thread Greg Mann 

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


Fix it, then Ship it!




In commit message: s/This patch add agent and resource provide IDs to/This 
patch adds agent and resource provider IDs to/


include/mesos/mesos.proto
Lines 2401-2403 (patched)


I would be explicit here that if both fields are unset, then `uuid` should 
be unset also.


- Greg Mann


On Nov. 27, 2018, 6:03 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69162/
> ---
> 
> (Updated Nov. 27, 2018, 6:03 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gastón Kleiman, and James DeFelice.
> 
> 
> Bugs: MESOS-9293
> https://issues.apache.org/jira/browse/MESOS-9293
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch add agent and resource provide IDs to
> `UpdateOperationStatus` and `UpdateOperationStatusMessage`. With that
> frameworks are able to reconcile enough information after failover to
> construct operation acknowledgements.
> 
> We will add code to populate these fields in a follow-up patch.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 56107f47a46ac3679a57af0580c55ad0f98543f5 
>   include/mesos/v1/mesos.proto c6e7515a44eca0b057725c7b8196250072b56be5 
>   src/common/type_utils.cpp ef13eae47b88efc15f1b2d00852b6387c2fffcbc 
>   src/internal/devolve.cpp 491ed2aa131a92e958bfa71cccfc5f257cd4b3f9 
>   src/internal/evolve.cpp aa60efed5a28ed4d847a4a27aa8e994cda82175d 
> 
> 
> Diff: https://reviews.apache.org/r/69162/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69474: Added tests for agent/executor heartbeating.

2018-11-29 Thread Mesos Reviewbot Windows 

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



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

Reviews applied: `['69463', '69472', '69473', '69474']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3426):
 warning C4996: 'strerror': This function or variable may be unsafe. Consider 
using strerror_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3500):
 warning C4996: 'inet_ntoa': Use inet_ntop() or InetNtop() instead or define 
_WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3501):
 warning C4996: 'sprintf': This function or variable may be unsafe. Consider 
using sprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3479):
 warning C4101: 'addrstr': unreferenced local variable 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\recordio.c(170):
 warning C4267: '=': conversion from 'size_t' to 'int32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\mt_adaptor.c(496):
 warning C4244: '=': conversion from 'time_t' to 'int32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(256):
 warning C4090: 'function': different 'const' qualifiers 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(166):
 warning C4716: 'pthread_cond_broadcast': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(205):
 warning C4716: 'pthread_cond_wait': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(124):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(128):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(279):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(301):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data

Re: Review Request 69071: Narrowed interface of `ReadOnlyHandler` members.

2018-11-29 Thread Joseph Wu 

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


Ship it!




LGTM, pending the other parts of the chain.

- Joseph Wu


On Nov. 21, 2018, 5:19 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69071/
> ---
> 
> (Updated Nov. 21, 2018, 5:19 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the members of `ReadOnlyHandler` would take a full `Request` as 
> parameter, making it hard for clients to reason about which parts of the 
> request are used internally, and even harder to guarantee that behaviour into 
> the future.
> 
> This commit changes the interface so only the query parameters get passed.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 68ee2a6dcffbc772afec6e797b1af8da48f61937 
>   src/master/master.hpp 3b3c1a4e61de9503c8d038dd3bee623ded5914c9 
>   src/master/readonly_handler.cpp 8895374499dc6baa2c4d8a8dd86fddac4e39be29 
> 
> 
> Diff: https://reviews.apache.org/r/69071/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 69474: Added tests for agent/executor heartbeating.

2018-11-29 Thread Joseph Wu 

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

(Updated Nov. 29, 2018, 4:03 p.m.)


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


Changes
---

Fixed the flakiness in the `HeartbeatCalls` test by adding a little more 
synchronization between the executor driver and test thread.


Summary (updated)
-

Added tests for agent/executor heartbeating.


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


Repository: mesos


Description
---

This adds two separate tests which check if the Agent sends heartbeats
to HTTP executors, and if the HTTP executor driver sends heartbeats
to the agent.


Diffs (updated)
-

  src/tests/executor_http_api_tests.cpp 
a635e1a3c228b52f47ba81a67753ea49ce092143 


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

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


Testing (updated)
---

```
cmake --build . --target check
src/mesos-tests --gtest_filter="*Heartbeat*" --verbose --gtest_repeat=-1 
--gtest_break_on_failure
```


Thanks,

Joseph Wu

Re: Review Request 69411: Added new interface for constructing `cluster::Master`.

2018-11-29 Thread Joseph Wu 

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



I'll summarize a discussion we held offline, between BenM, Benno, and I.

The immediate goal behind changing the interface for `cluster::Master` (and 
possibly `cluster::Slave`) is to remove the need for dozens of `StartMaster` 
and `StartSlave` overloads in the test helpers.  Hopefully this change would 
also allow us to remove some extraneous lines scattered throughout the tests 
(like creating `Fetcher` objects for agents, even in tests that don't exercise 
the fetcher).

Currently, we do not use "builder" interfaces that widely in the mesos 
codebase.  Instead, we might want to use a struct instead.  i.e.
```
MasterOptions options;
options.flags.some_flag = non_default;

Try> master = StartMaster(options);
```

The struct approach would need to deal with vague ownership of some objects, as 
both the struct and `Owned` would be accessible from within 
the test body.  The `StartSlave` options struct may be more complicated than 
the master's, because there are several dependency chains in the agent.

- Joseph Wu


On Nov. 21, 2018, 5:21 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69411/
> ---
> 
> (Updated Nov. 21, 2018, 5:21 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review adds a new method `cluster::Master::create()` that
> allows constructing masters while only specifying the non-standard
> components required for the specific test, like this:
> 
> auto master = cluster::Master::build()
>   .withZookeeperUrl(url)
>   .withAllocator(allocator);
> 
> This is sometimes better known as "fluent interface".
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.hpp ad2b80e658d2f8afcefe9969d62cd33f0c475ce9 
>   src/tests/cluster.cpp 2b351ca70d8e80008e49722aa7d46918b5ecd9b0 
> 
> 
> Diff: https://reviews.apache.org/r/69411/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Review Request 69481: Deallocated the shared persistent volume's gid when it is removed.

2018-11-29 Thread Qian Zhang 

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

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


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


Repository: mesos


Description
---

Deallocated the shared persistent volume's gid when it is removed.


Diffs
-

  src/slave/slave.cpp 858b78620e1ef33f3587d0bd95a684917aaf5bbb 


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


Testing
---


Thanks,

Qian Zhang

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

2018-11-29 Thread Qian Zhang 

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

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


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


Repository: mesos


Description
---

Made non-root containers can access shared persistent volume.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
a5cf2da55c046c5c45e0c2ca3400f64de12de62b 
  src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
8f76944cd9058c0ba809443e32a3d8c8a26ac4a6 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
c7d753ac2e5575a8d687600bfb9e0617fa72c990 
  src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
deacc909a2d323671667cb646c019664bdb660e7 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
f91a2eeb835bb65a855eeb314d4c69e3b58fecae 
  src/slave/volume_gid_manager/volume_gid_manager.cpp PRE-CREATION 


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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 69479: Added the flag `--task_supplementary_groups` to command executor.

2018-11-29 Thread Qian Zhang 

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

(Updated Nov. 29, 2018, 5:08 p.m.)


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


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


Repository: mesos


Description
---

Added the flag `--task_supplementary_groups` to command executor.


Diffs
-

  src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421 


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


Testing
---


Thanks,

Qian Zhang

Review Request 69479: Added the flag `--task_supplementary_groups` to command executor.

2018-11-29 Thread Qian Zhang 

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

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


Repository: mesos


Description
---

Added the flag `--task_supplementary_groups` to command executor.


Diffs
-

  src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421 


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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 69478: Added `task_supplementary_groups` into `ContainerLaunchInfo`.

2018-11-29 Thread Qian Zhang 

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

(Updated Nov. 29, 2018, 5:06 p.m.)


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


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


Repository: mesos


Description
---

Added `task_supplementary_groups` into `ContainerLaunchInfo`.


Diffs
-

  include/mesos/slave/containerizer.proto 
5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9 


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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 69454: Added the autoconf `tar-pax` option.

2018-11-29 Thread Benjamin Bannier 

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


Ship it!




Thanks for the patch, James!

Could you update the commit message since this patch doesn't add a user-facing 
option, but rather changes a autoconf setting? Something like
```
Used POSIX.1-2001/pax tar format for distribution tarballs.
```

- Benjamin Bannier


On Nov. 27, 2018, 6:51 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69454/
> ---
> 
> (Updated Nov. 27, 2018, 6:51 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The default tar format used in `make dist` is v7, which only supports
> paths of up to 99 bytes in length. This causes errors when building
> the CentOS RPM and adding files from 3rd party packages.
> 
> 
> Diffs
> -
> 
>   configure.ac c193adf93531170fb31a31c296e3d4ae1d0300b2 
> 
> 
> Diff: https://reviews.apache.org/r/69454/diff/1/
> 
> 
> Testing
> ---
> 
> Ran `support/packaging/centos/build-rpm-docker.sh` and observed no tar errors.
> 
> Without this change, this is what happens:
> ```
> smelt:centos jpeach$ ./build-rpm-docker.sh
> ...
> tardir=mesos-1.8.0 && ${TAR-tar} chof - "$tardir" | GZIP=--best gzip -c 
> >mesos-1.8.0.tar.gz
> tar: 
> mesos-1.8.0/3rdparty/libprocess/3rdparty/glog-0.3.3/vsprojects/logging_unittest/logging_unittest.vcproj:
>  file name is too long (max 99); not dumped
> tar: 
> mesos-1.8.0/3rdparty/libprocess/3rdparty/glog-0.3.3/vsprojects/logging_unittest_static/logging_unittest_static.vcproj:
>  file name is too long (max 99); not dumped
> ...
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 69478: Added `task_supplementary_groups` into `ContainerLaunchInfo`.

2018-11-29 Thread Qian Zhang 

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

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


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


Repository: mesos


Description
---

Added `task_supplementary_groups` into `ContainerLaunchInfo`.


Diffs
-

  include/mesos/slave/containerizer.proto 
5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9 


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


Testing
---


Thanks,

Qian Zhang