Re: Review Request 68053: Call any function in a specified namespace.

[Sergey Urbanovich]

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

(Updated July 31, 2018, 5:34 a.m.)


Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, Jie Yu, and Qian 
Zhang.


Changes
---

Added NamespaceRunner::ProcessingQueue. It uses mutex and condition variable to 
solve the problems with process::Queue. Please let me know if you think that 
libprocess would be a better place for this class.


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


Repository: mesos


Description
---

The NamespaceRunner runs any function in a specified namespace. To do
that it manages a separate thread which would be re-associated with
that namespace.


Diffs (updated)
-

  src/linux/ns.hpp 0b4136bd3cc2d3e0cfee163d89469558e699b5f2 
  src/tests/containerizer/ns_tests.cpp fa4349e29b975550e05b00a1f848a24cd8e4f0de 


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

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


Testing
---

bin/mesos-tests.sh --verbose --gtest_filter="NsTest*" --gtest_break_on_failure 
--gtest_repeat=100


Thanks,

Sergey Urbanovich

Review Request 68122: Fixed couple of typos in the allocator.

[Meng Zhu]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
35992474eacb8b14ae57e1dc23307e1542f63cb5 


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


Testing
---

Not needed.


Thanks,

Meng Zhu

Review Request 68118: Introduced helpers to track agent resources in the allocator.

[Meng Zhu]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

These helpers consolidate and simplify the agent
resource bookkeeping logic in the allocator.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
0cd2fac17f09110c46b8540523a9c935f156f858 
  src/master/allocator/mesos/hierarchical.cpp 
35992474eacb8b14ae57e1dc23307e1542f63cb5 


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


Testing
---

make check


Thanks,

Meng Zhu

Review Request 68123: Avoided unnecessary `Resources::allocations()` call in the allocator.

[Meng Zhu]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
35992474eacb8b14ae57e1dc23307e1542f63cb5 


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


Testing
---

make check


Thanks,

Meng Zhu

Review Request 68119: Introduced a helper `updateSlaveResources` in the allocator.

[Meng Zhu]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

The helper encapsulates slave resources update logic. It also
avoids redundant updates if the agent resources stay the same.

Also, removed the direct setters in the Slave class to avoid
inconsistent updates.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
0cd2fac17f09110c46b8540523a9c935f156f858 
  src/master/allocator/mesos/hierarchical.cpp 
35992474eacb8b14ae57e1dc23307e1542f63cb5 


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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 68120: UI: Pull up the leader URL generation to a top-level function.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68120 was successfully built and tested.

Reviews applied: `['68120']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2007/mesos-review-68120

- Mesos Reviewbot Windows


On July 31, 2018, 2:40 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68120/
> ---
> 
> (Updated July 31, 2018, 2:40 a.m.)
> 
> 
> Review request for mesos, Armand Grillet and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This path pulls up the leader URL prefix generation to a top-level
> function in order to have it re-used across controllers.
> 
> Having this logic in just one place makes it easier to modify it,
> e.g., to make it possible to use the UI via a reverse proxy.
> 
> 
> Diffs
> -
> 
>   src/webui/app/controllers.js 7c228a1629d7b1dcce56b432f042f02d3ec1583b 
>   src/webui/app/home.html ff965952d7a8d84c9dcfd5d6e7e045cd72bc 
> 
> 
> Diff: https://reviews.apache.org/r/68120/diff/1/
> 
> 
> Testing
> ---
> 
> manual testing
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68104: Implemented authorization for agent `GET_RESOURCE_PROVIDER` calls.

[Chun-Hung Hsiao]

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




src/slave/http.cpp
Lines 1818 (patched)


Conventionally we only indent this by 4 extra spaces aligning with "return".



src/slave/http.cpp
Lines 1821-1822 (patched)


The following can be fit into 80 characters:
```
[this, acceptType](const Owned& approvers) -> 
Response {
```
Or for here it seems harmless to just use `[=]`. I'm fine with either 
though.



src/slave/http.cpp
Lines 1824 (patched)


Should we return an empty list of resource provider infos or return a 403 
Forbidden?



src/tests/api_tests.cpp
Lines 7002-7004 (original), 7002-7021 (patched)


How about the following:
```
slave::Flags slaveFlags = CreateSlaveFlags();
slaveFlags.authenticate_http_readwrite = true;

{
  // `DEFAULT_CREDENTIAL_2` is not allowed to view any resource provider.
  mesos::ACL::ViewResourceProvider* acl =
slaveFlags.acls->add_view_resource_providers();
  acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL_2.principal());
  acl->mutable_resource_providers()->set_type(mesos::ACL::Entity::NONE);
{

Try> slave = StartSlave(&detector, slaveFlags);
```



src/tests/api_tests.cpp
Lines 7046-7049 (patched)


I'm a bit against checking content of the failure string since it's 
slightly hard to maintain. Can we avoid this?


- Chun-Hung Hsiao


On July 30, 2018, 8:57 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68104/
> ---
> 
> (Updated July 30, 2018, 8:57 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-8314
> https://issues.apache.org/jira/browse/MESOS-8314
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented authorization for agent `GET_RESOURCE_PROVIDER` calls.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp ab5864d9fd2fde478ed7da2ca7ed8abedc72c7c5 
>   src/tests/api_tests.cpp 182622a62d350ebefc891a385de3f2d35a7c0243 
> 
> 
> Diff: https://reviews.apache.org/r/68104/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 68103: Added an authorizer action for viewing of resource provider information.

[Chun-Hung Hsiao]

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




include/mesos/authorizer/acls.proto
Lines 561 (patched)


Let's use a sigular term for consistency: `ViewResourceProvider`.



include/mesos/authorizer/acls.proto
Lines 652 (patched)


`repeated ACL.ViewResourceProvider view_resource_providers`



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


`VIEW_RESOURCE_PROVIDER`



src/authorizer/local/authorizer.cpp
Lines 419-422 (patched)


How about merging this into the above?



src/authorizer/local/authorizer.cpp
Lines 1772 (patched)


s/`PruneImages`/`ViewResourceProvider`/



src/tests/authorization_tests.cpp
Lines 4715 (patched)


s/`ViewResourceProviders`/`ViewResourceProvider`/



src/tests/authorization_tests.cpp
Lines 4721 (patched)


```
// ... can view resource provider information.
```



src/tests/authorization_tests.cpp
Lines 4728 (patched)


Ditto.



src/tests/authorization_tests.cpp
Lines 4754 (patched)


```
// ... with invalid ACLs.
```


- Chun-Hung Hsiao


On July 30, 2018, 8:57 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68103/
> ---
> 
> (Updated July 30, 2018, 8:57 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-8314
> https://issues.apache.org/jira/browse/MESOS-8314
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an authorizer action for viewing of resource provider information.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 1777c04316bfd7493894f0e782f170fe8437aafe 
>   include/mesos/authorizer/authorizer.proto 
> 8b5fa09f389ca4c26f8390d35af32c4cdd561418 
>   src/authorizer/local/authorizer.cpp 
> abf5b4663cd517fb6d69b5373dd0e6520e87cf8e 
>   src/tests/authorization_tests.cpp 41ecac29a53f6ad9553cbf0a1121e1f3cff50df8 
> 
> 
> Diff: https://reviews.apache.org/r/68103/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 68120: UI: Pull up the leader URL generation to a top-level function.

[Benjamin Mahler]

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

Review request for mesos, Armand Grillet and Gastón Kleiman.


Repository: mesos


Description
---

This path pulls up the leader URL prefix generation to a top-level
function in order to have it re-used across controllers.

Having this logic in just one place makes it easier to modify it,
e.g., to make it possible to use the UI via a reverse proxy.


Diffs
-

  src/webui/app/controllers.js 7c228a1629d7b1dcce56b432f042f02d3ec1583b 
  src/webui/app/home.html ff965952d7a8d84c9dcfd5d6e7e045cd72bc 


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


Testing
---

manual testing


Thanks,

Benjamin Mahler

Re: Review Request 67187: Tested per-framework task state metrics.

[Greg Mann]

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

(Updated July 31, 2018, 1:53 a.m.)


Review request for mesos, Gastón Kleiman and Gilbert Song.


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


Repository: mesos


Description
---

This patch adds `MasterTest.TaskStateMetrics`, which verifies that
per-framework task state metrics for both terminal and active task
states report correct values, even after agent reregistration.


Diffs (updated)
-

  src/tests/master_tests.cpp 44b0ac39f87c6415e130c5e7f505428642739311 


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

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


Testing
---

The new test was run ~10,000 times with no failures.


Thanks,

Greg Mann

Re: Review Request 68114: Fixed a gRPC compilation issue for Clang.

[Mesos Reviewbot Windows]

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



FAIL: Mesos binaries failed to build.

Reviews applied: `['68091', '68074', '68092', '68114']`

Failed command: `cmake.exe --build . --config Release -- /maxcpucount`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2006/mesos-review-68114

Relevant logs:

- 
[mesos-binaries-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2006/mesos-review-68114/logs/mesos-binaries-cmake-stdout.log):

```
 
d:\dcos\mesos\3rdparty\libarchive-3.3.2\src\libarchive-3.3.2\libarchive\archive_read_support_format_7zip.c(1426):
 warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss 
of data 
[D:\DCOS\mesos\3rdparty\libarchive-3.3.2\src\libarchive-3.3.2-build\libarchive\archive_static.vcxproj]
 [D:\DCOS\mesos\3rdparty\libarchive-3.3.2.vcxproj]
 
d:\dcos\mesos\3rdparty\libarchive-3.3.2\src\libarchive-3.3.2\libarchive\archive_read_support_format_7zip.c(1428):
 warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss 
of data 
[D:\DCOS\mesos\3rdparty\libarchive-3.3.2\src\libarchive-3.3.2-build\libarchive\archive_static.vcxproj]
 [D:\DCOS\mesos\3rdparty\libarchive-3.3.2.vcxproj]
 
d:\dcos\mesos\3rdparty\libarchive-3.3.2\src\libarchive-3.3.2\libarchive\archive_write_add_filter_bzip2.c(197):
 warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss 
of data 
[D:\DCOS\mesos\3rdparty\libarchive-3.3.2\src\libarchive-3.3.2-build\libarchive\archive_static.vcxproj]
 [D:\DCOS\mesos\3rdparty\libarchive-3.3.2.vcxproj]
 
d:\dcos\mesos\3rdparty\libarchive-3.3.2\src\libarchive-3.3.2\libarchive\archive_write_add_filter_bzip2.c(251):
 warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss 
of data 
[D:\DCOS\mesos\3rdparty\libarchive-3.3.2\src\libarchive-3.3.2-build\libarchive\archive_static.vcxproj]
 [D:\DCOS\mesos\3rdparty\libarchive-3.3.2.vcxproj]
 
d:\dcos\mesos\3rdparty\libarchive-3.3.2\src\libarchive-3.3.2\libarchive\archive_write_add_filter_bzip2.c(322):
 warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss 
of data 
[D:\DCOS\mesos\3rdparty\libarchive-3.3.2\src\libarchive-3.3.2-build\libarchive\archive_static.vcxproj]
 [D:\DCOS\mesos\3rdparty\libarchive-3.3.2.vcxproj]
 
d:\dcos\mesos\3rdparty\libarchive-3.3.2\src\libarchive-3.3.2\libarchive\archive_write_set_format_7zip.c(1805):
 warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss 
of data 
[D:\DCOS\mesos\3rdparty\libarchive-3.3.2\src\libarchive-3.3.2-build\libarchive\archive_static.vcxproj]
 [D:\DCOS\mesos\3rdparty\libarchive-3.3.2.vcxproj]
 
d:\dcos\mesos\3rdparty\libarchive-3.3.2\src\libarchive-3.3.2\libarchive\archive_write_set_format_7zip.c(1809):
 warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss 
of data 
[D:\DCOS\mesos\3rdparty\libarchive-3.3.2\src\libarchive-3.3.2-build\libarchive\archive_static.vcxproj]
 [D:\DCOS\mesos\3rdparty\libarchive-3.3.2.vcxproj]
 
d:\dcos\mesos\3rdparty\libarchive-3.3.2\src\libarchive-3.3.2\libarchive\archive_write_set_format_7zip.c(1838):
 warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss 
of data 
[D:\DCOS\mesos\3rdparty\libarchive-3.3.2\src\libarchive-3.3.2-build\libarchive\archive_static.vcxproj]
 [D:\DCOS\mesos\3rdparty\libarchive-3.3.2.vcxproj]
 
d:\dcos\mesos\3rdparty\libarchive-3.3.2\src\libarchive-3.3.2\libarchive\archive_write_set_format_7zip.c(1842):
 warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss 
of data 
[D:\DCOS\mesos\3rdparty\libarchive-3.3.2\src\libarchive-3.3.2-build\libarchive\archive_static.vcxproj]
 [D:\DCOS\mesos\3rdparty\libarchive-3.3.2.vcxproj]


   "D:\DCOS\mesos\ALL_BUILD.vcxproj" (default target) (1) ->
   
"D:\DCOS\mesos\src\slave\resource_estimators\fixed_resource_estimator.vcxproj" 
(default target) (9) ->
   "D:\DCOS\mesos\3rdparty\libprocess\src\process.vcxproj" (default target) 
(27) ->
   (ClCompile target) -> 
 d:\dcos\mesos\mesos\3rdparty\stout\include\stout\exit.hpp(69): warning 
C4722: '__Exit::~__Exit': destructor never returns, potential memory leak 
[D:\DCOS\mesos\3rdparty\libprocess\src\process.vcxproj]
 d:\dcos\mesos\mesos\3rdparty\stout\include\stout\exit.hpp(69): warning 
C4722: '__Exit::~__Exit': destructor never returns, potential memory leak 
[D:\DCOS\mesos\3rdparty\libprocess\src\process.vcxproj]


   "D:\DCOS\mesos\ALL_BUILD.vcxproj" (default target) (1) ->
   
"D:\DCOS\mesos\src\slave\resource_estimators\fixed_resource_estimator.vcxproj" 
(default target) (9) ->
   "D:\DCOS\mesos\src\mesos-protobufs.vcxproj" (default target) (18) ->
  

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

[Andrew Schwartzmeyer]

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




cmake/CompilationConfigure.cmake
Lines 538-541 (patched)


What targets need `ENABLE_SECCOMP_ISOLATOR` defined? Presumably this is new 
code, so we don't have to leave this as a to-do. (And I'm guessing just 
`libmesos PRIVATE`?)


- Andrew Schwartzmeyer


On July 23, 2018, 9:17 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68016/
> ---
> 
> (Updated July 23, 2018, 9:17 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Gilbert Song, Jie Yu, James 
> Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9032
> https://issues.apache.org/jira/browse/MESOS-9032
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This library is needed to implement Seccomp syscall filtering in the
> Mesos containerizer. This patch introduces `seccomp-isolator` build
> flag, which is used to include or exclude sources related to Seccomp
> from the build. Since Seccomp is a Linux-specific feature, the flag
> is disabled by default. Enabling `seccomp-isolator` means either:
> 
> 1. Compiling and linking against the bundled version of libseccomp from
>sources (default).
> 
> 2. Linking against the libseccomp installed in the OS,
>if `--with-libseccomp` build flag is provided.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt b58996d2ed7521e42d6298d174cc8c494b84eb8f 
>   3rdparty/Makefile.am 6123b8a0164d50f0bd771d2e5f485379811e7fcb 
>   3rdparty/cmake/Versions.cmake 0a897d808cd9e05ac0d1a4e1ca61b8f3538f0c4a 
>   3rdparty/versions.am 57792800c4b4b67dd55a316ce00480db6c253d34 
>   cmake/CompilationConfigure.cmake f80c265d117c70acda95da6b943cdd3e4e477daa 
>   configure.ac e86c496140ee9732ad3204ef22ae8ce60cd5c079 
>   src/CMakeLists.txt 10b0946d6f49c7e9c201bad6f9f1b41cc8460fe5 
>   src/Makefile.am ecb95ef152fd37e5e83c4f6415489f4cf3959578 
> 
> 
> Diff: https://reviews.apache.org/r/68016/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 67932: Use `wclayer` from `hcsshim` to extract file layers.

[Andrew Schwartzmeyer]

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




src/common/command_utils.cpp
Lines 186 (patched)


?



src/common/command_utils.cpp
Lines 188-191 (patched)


Trying to think of a cleaner way to do this...



src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
Line 381 (original), 388 (patched)


Did we change behavior here? This is in a loop, so the original code was 
constantly adding to the front, whereas now we're adding the back (IMHO 
original code smells a bit funny).



src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
Line 416 (original), 433-455 (patched)


Goodness, but it looks like the best we can do here.



src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
Line 430 (original), 469 (patched)


Why do we have to do this now?



src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 379-380 (patched)


I take it that the `wclayer` command in `wclayer_remove` is supposed to 
remove the directory?



src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 490 (patched)


Last I remember, `flags.docker_store_dir` was some Linux path; did we get 
that fixed (or am I misremembering)?


- Andrew Schwartzmeyer


On July 18, 2018, 2:27 a.m., Liangyu Zhao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67932/
> ---
> 
> (Updated July 18, 2018, 2:27 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `tar` command cannot work successfully on Windows, so use `wclayer`
> instead. Note that the folder generated from extraction also cannot be
> deleted by `rmdir`, so the GC is also changed to use `wclayer remove`.
> 
> 
> Diffs
> -
> 
>   src/common/command_utils.hpp 525f9c1c37b74f7e4cc71fdc8d52944226998ddc 
>   src/common/command_utils.cpp 7dfcc9ff74bcf044d47b803ebc42cf63fba89d17 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> a5683e3fe15dd35596122fcc0c580ae9d3adf7f2 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 85aad25ac8ecfda125be85fb46d882c3982f3930 
> 
> 
> Diff: https://reviews.apache.org/r/67932/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Liangyu Zhao
> 
>

Re: Review Request 68074: Autotools: Always build gRPC in Mesos.

[Chun-Hung Hsiao]

> On July 30, 2018, 10:16 a.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Line 80 (original), 80 (patched)
> > 
> >
> > If you want to keep separate patches for modifying autotools and cmake, 
> > this should be in the next patch in the chain, 
> > https://reviews.apache.org/r/68092/.

I'll merge the two patches.


> On July 30, 2018, 10:16 a.m., Benjamin Bannier wrote:
> > configure.ac
> > Line 1 (original), 1 (patched)
> > 
> >
> > Any reason we cannot just merge this patch with 
> > https://reviews.apache.org/r/68092/?

I'm just trying not to create a big patch which might be hard to review. I 
could merge these two as you wish :)


> On July 30, 2018, 10:16 a.m., Benjamin Bannier wrote:
> > src/Makefile.am
> > Line 1529 (original), 1521 (patched)
> > 
> >
> > Can we merge this with above list now?

I'm following the same pattern we do for other libraries: first adding cpp 
files then adding the header/proto files. Should I just don't bother 
maintaining the consistency?


- Chun-Hung


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


On July 27, 2018, 7:12 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68074/
> ---
> 
> (Updated July 27, 2018, 7:12 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-8395
> https://issues.apache.org/jira/browse/MESOS-8395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Autotools: Always build gRPC in Mesos.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 26e5d55561731ff03639df31562bb835d9687339 
>   cmake/CompilationConfigure.cmake f80c265d117c70acda95da6b943cdd3e4e477daa 
>   configure.ac 58003b6e823c8457f4b5a8c8d670be43bf96de8d 
>   include/csi/spec.hpp 2e9b870617a6604e5d3e93eedce119ec029eea05 
>   src/Makefile.am 71f9052f60fc65d8183faa7ab9764d3e6388ddc9 
>   src/python/native_common/ext_modules.py.in 
> dc4d91a8b8e3640e2191a53e657a5e4588fa911e 
>   src/resource_provider/local.cpp ae23c2000b7941aad92d3b83e4e905bdf042ebe8 
> 
> 
> Diff: https://reviews.apache.org/r/68074/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67187: Tested per-framework task state metrics.

[Greg Mann]

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

(Updated July 31, 2018, 12:22 a.m.)


Review request for mesos, Gastón Kleiman and Gilbert Song.


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


Repository: mesos


Description
---

This patch adds `MasterTest.TaskStateMetrics`, which verifies that
per-framework task state metrics for both terminal and active task
states report correct values, even after agent reregistration.


Diffs (updated)
-

  src/tests/master_tests.cpp 44b0ac39f87c6415e130c5e7f505428642739311 


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

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


Testing
---

The new test was run ~10,000 times with no failures.


Thanks,

Greg Mann

Re: Review Request 67187: Tested per-framework task state metrics.

[Greg Mann]

> On July 25, 2018, 9:06 p.m., Gastón Kleiman wrote:
> > src/tests/master_tests.cpp
> > Lines 9113-9117 (patched)
> > 
> >
> > Can we use the new helpers here?
> > 
> > ```
> >   testing::Sequence taskSequence1;
> >   Future running1;
> > 
> >   EXPECT_CALL(
> >   sched,
> >   statusUpdate(&driver, AllOf(
> >   TaskStatusTaskIdEq(task1.id()),
> >   TaskStatusStateEq(TASK_STARTING
> > .InSequence(taskSequence1)
> > 
> >   EXPECT_CALL(
> >  sched,
> >  statusUpdate(&driver, AllOf(
> >  TaskStatusTaskIdEq(task1.id()),
> >  TaskStatusStateEq(TASK_RUNNING
> >.InSequence(taskSequence1)
> >.WillOnce(FutureArg<1>(&running1));
> > ```

Thx for this suggestion!!


- Greg


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


On July 31, 2018, 12:22 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67187/
> ---
> 
> (Updated July 31, 2018, 12:22 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Gilbert Song.
> 
> 
> Bugs: MESOS-8847
> https://issues.apache.org/jira/browse/MESOS-8847
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds `MasterTest.TaskStateMetrics`, which verifies that
> per-framework task state metrics for both terminal and active task
> states report correct values, even after agent reregistration.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 44b0ac39f87c6415e130c5e7f505428642739311 
> 
> 
> Diff: https://reviews.apache.org/r/67187/diff/7/
> 
> 
> Testing
> ---
> 
> The new test was run ~10,000 times with no failures.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 68114: Fixed a gRPC compilation issue for Clang.

[Chun-Hung Hsiao]

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

Review request for mesos and Benjamin Bannier.


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


Repository: mesos


Description
---

When compiling gRPC with Clang, there are some array-out-of-bound
warnings due to the use of GLIBC's `__strcmp_cg` macro in the c-ares
library. With `-Werror` on, these warnings would stop gRPC from
compiling. This patch ignores such errors.


Diffs
-

  3rdparty/Makefile.am 26e5d55561731ff03639df31562bb835d9687339 


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


Testing
---

`sudo make check` with clang on ubuntu 16.04, which triggers the warnings.


Thanks,

Chun-Hung Hsiao

Re: Review Request 67930: Get tests ready for Windows UCR development.

[Andrew Schwartzmeyer]

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


Fix it, then Ship it!





src/tests/uri_fetcher_tests.cpp
Lines 281-283 (patched)


nit: `static constexpr char DOCKER_IMAGE[]` (like above)


- Andrew Schwartzmeyer


On July 18, 2018, 2:27 a.m., Liangyu Zhao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67930/
> ---
> 
> (Updated July 18, 2018, 2:27 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled Internet test environment on Windows. Disabled Internet
> HealthCheckTests on Windows, since they require complete development.
> Modified DockerFetcherPluginTest to fetch `microsoft/nanoserver` for
> more extensive test for fetcher on Windows.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp 3b84c0a413193badbbdb6d3ee71d74f3ab85c90b 
>   src/tests/health_check_tests.cpp 34102e2e3ff90c194bea83ae8a702181121d6522 
>   src/tests/uri_fetcher_tests.cpp 816d8ec2503ea79d069c062dfa2f01224b263bf6 
> 
> 
> Diff: https://reviews.apache.org/r/67930/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Liangyu Zhao
> 
>

Re: Review Request 67931: Support Image Manifest Version 2 Schema 2.

[Andrew Schwartzmeyer]

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




3rdparty/CMakeLists.txt
Lines 721-736 (original), 721-736 (patched)


Note for others: this was forced by an upstream build change in curl.



include/mesos/docker/spec.hpp
Lines 40-42 (patched)


+1



include/mesos/uri/fetcher.hpp
Lines 101 (patched)


I am not convinced we need to be passing this as a shared_ptr, wouldn't 
const-ref be just fine?



src/Makefile.am
Lines 692-697 (original), 695-703 (patched)


Whitespace I'll fix up when committing...



src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
Lines 467-505 (patched)


I believe you could accomplish what you're doing here with `goto 
fail/succeed` by refactoring this into a function (or even an in-place lambda) 
where the `goto`s would turn into `return`s. But I'm not sure if it'd be any 
cleaner. I have nothing against `goto` when used right, and this seems 
reasonable.



src/uri/fetcher.hpp
Line 46 (original), 43 (patched)


This can go.



src/uri/fetchers/docker.cpp
Lines 791-818 (patched)


Hey that worked out nicely.



src/uri/fetchers/docker.cpp
Lines 1016-1017 (patched)


Are we just skipping a failed blob here and trying to process the rest? 
Does that make sense to do?


- Andrew Schwartzmeyer


On July 30, 2018, 3:40 p.m., Liangyu Zhao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67931/
> ---
> 
> (Updated July 30, 2018, 3:40 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support parsing schema 2 and fetching blob with multiple URLs as
> specified in schema 2. Update `curl` version to 7.61.0 because of bug
> encountered in version 7.57.0.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt b58996d2ed7521e42d6298d174cc8c494b84eb8f 
>   3rdparty/cmake/Versions.cmake 0a897d808cd9e05ac0d1a4e1ca61b8f3538f0c4a 
>   include/mesos/docker/spec.hpp 2879414dc42ffe633ac74b51e1bb116698c41162 
>   include/mesos/docker/v2_2.hpp PRE-CREATION 
>   include/mesos/docker/v2_2.proto PRE-CREATION 
>   include/mesos/uri/fetcher.hpp 760d6b33234d8efdc533c0c6f05e83a5c7d1f56b 
>   src/CMakeLists.txt 10b0946d6f49c7e9c201bad6f9f1b41cc8460fe5 
>   src/Makefile.am 228e168c22f3fd0367f029c506171c6979b31c07 
>   src/docker/spec.cpp 96fbf1f9cf1c2c4b2383607a97990f3a9156e6d9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 98129d006cda9b65804b518619b6addc8990410a 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> a5683e3fe15dd35596122fcc0c580ae9d3adf7f2 
>   src/uri/fetcher.hpp fbf64c6767dea3aa0798f68db8322ce47cd8ad36 
>   src/uri/fetcher.cpp 489c7ce0198ee6803dcc8eb9710b292fa743a0e8 
>   src/uri/fetchers/docker.hpp cdbab9a5684a68a729be12bc06d331acca137da5 
>   src/uri/fetchers/docker.cpp 55ca118660872a933a2dc186723bec6a39ee80f7 
> 
> 
> Diff: https://reviews.apache.org/r/67931/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Liangyu Zhao
> 
>

Re: Review Request 68054: Added networking statistics to cni isolator.

[Sergey Urbanovich]

> On July 28, 2018, 12:29 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
> > Lines 1506-1507 (patched)
> > 
> >
> > No need for this temp variable

I got the following error without the temp variable. What would be the best way 
to use `lambda::bind()` in that case?

```
../src/slave/containerizer/mesos/isolators/network/cni/cni.cpp:1513:26: error: 
no matching member function for call to 'run'
  return namespaceRunner.run(netns, "net", 
lambda::bind(&NetworkCniIsolatorProcess::_usage, ifNames));
 ^~~
../src/linux/ns.hpp:210:22: note: candidate template ignored: could not match 
'function' against '_Bind'
  process::Future run(
 ^
```


- Sergey


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


On July 27, 2018, 11:36 p.m., Sergey Urbanovich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68054/
> ---
> 
> (Updated July 27, 2018, 11:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-5647
> https://issues.apache.org/jira/browse/MESOS-5647
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On receiving a request for getting `usage` for a given container
> the `network/cni` isolator uses getifaddrs(3) glibc function. The
> function returns basic networking metrics for each networking
> interface in the container networking namespace. It should work
> right out of the box on all modern Linux-based systems.
> 
> To get more networking metrics please use Netlink Protocol Library.
> However, you will have to open NETLINK sockets in each networking
> namespace and manage them from the `network/cni` isolator.
> 
> JIRA: https://issues.apache.org/jira/browse/MESOS-5647
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 31ec4ddb1049b7259b0784e5e40b002e29f6a8da 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f46c962d8f044092aaa113fafc536c6b25bab45c 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 90d2d4103c8136d2dd883318acc135f7efca80d8 
> 
> 
> Diff: https://reviews.apache.org/r/68054/diff/3/
> 
> 
> Testing
> ---
> 
> bin/mesos-tests.sh --verbose 
> --gtest_filter="CniIsolatorTest.ROOT_VerifyResourceStatistics" 
> --gtest_break_on_failure --gtest_repeat=100
> 
> 
> Thanks,
> 
> Sergey Urbanovich
> 
>

Re: Review Request 67931: Support Image Manifest Version 2 Schema 2.

[Liangyu Zhao via Review Board]

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

(Updated July 30, 2018, 3:40 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Joseph Wu.


Changes
---

Remove dependency so 30 -> 31.


Repository: mesos


Description
---

Support parsing schema 2 and fetching blob with multiple URLs as
specified in schema 2. Update `curl` version to 7.61.0 because of bug
encountered in version 7.57.0.


Diffs
-

  3rdparty/CMakeLists.txt b58996d2ed7521e42d6298d174cc8c494b84eb8f 
  3rdparty/cmake/Versions.cmake 0a897d808cd9e05ac0d1a4e1ca61b8f3538f0c4a 
  include/mesos/docker/spec.hpp 2879414dc42ffe633ac74b51e1bb116698c41162 
  include/mesos/docker/v2_2.hpp PRE-CREATION 
  include/mesos/docker/v2_2.proto PRE-CREATION 
  include/mesos/uri/fetcher.hpp 760d6b33234d8efdc533c0c6f05e83a5c7d1f56b 
  src/CMakeLists.txt 10b0946d6f49c7e9c201bad6f9f1b41cc8460fe5 
  src/Makefile.am 228e168c22f3fd0367f029c506171c6979b31c07 
  src/docker/spec.cpp 96fbf1f9cf1c2c4b2383607a97990f3a9156e6d9 
  src/slave/containerizer/mesos/containerizer.cpp 
98129d006cda9b65804b518619b6addc8990410a 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
a5683e3fe15dd35596122fcc0c580ae9d3adf7f2 
  src/uri/fetcher.hpp fbf64c6767dea3aa0798f68db8322ce47cd8ad36 
  src/uri/fetcher.cpp 489c7ce0198ee6803dcc8eb9710b292fa743a0e8 
  src/uri/fetchers/docker.hpp cdbab9a5684a68a729be12bc06d331acca137da5 
  src/uri/fetchers/docker.cpp 55ca118660872a933a2dc186723bec6a39ee80f7 


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


Testing
---


Thanks,

Liangyu Zhao

Re: Review Request 68111: Added 'MesosCon 2018 CFP is now open!' blog post.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68111 was successfully built and tested.

Reviews applied: `['68111']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2005/mesos-review-68111

- Mesos Reviewbot Windows


On July 30, 2018, 1:38 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68111/
> ---
> 
> (Updated July 30, 2018, 1:38 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'MesosCon 2018 CFP is now open!' blog post.
> 
> 
> Diffs
> -
> 
>   site/source/blog/2018-07-30-mesoscon-2018-cfp-is-now-open.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68111/diff/3/
> 
> 
> Testing
> ---
> 
> Used `./mesos-website-dev.sh` to verify that the website is properly rendered.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 68054: Added networking statistics to cni isolator.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68054 was successfully built and tested.

Reviews applied: `['68052', '68053', '68054']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2004/mesos-review-68054

- Mesos Reviewbot Windows


On July 27, 2018, 11:36 p.m., Sergey Urbanovich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68054/
> ---
> 
> (Updated July 27, 2018, 11:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-5647
> https://issues.apache.org/jira/browse/MESOS-5647
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On receiving a request for getting `usage` for a given container
> the `network/cni` isolator uses getifaddrs(3) glibc function. The
> function returns basic networking metrics for each networking
> interface in the container networking namespace. It should work
> right out of the box on all modern Linux-based systems.
> 
> To get more networking metrics please use Netlink Protocol Library.
> However, you will have to open NETLINK sockets in each networking
> namespace and manage them from the `network/cni` isolator.
> 
> JIRA: https://issues.apache.org/jira/browse/MESOS-5647
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 31ec4ddb1049b7259b0784e5e40b002e29f6a8da 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f46c962d8f044092aaa113fafc536c6b25bab45c 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 90d2d4103c8136d2dd883318acc135f7efca80d8 
> 
> 
> Diff: https://reviews.apache.org/r/68054/diff/3/
> 
> 
> Testing
> ---
> 
> bin/mesos-tests.sh --verbose 
> --gtest_filter="CniIsolatorTest.ROOT_VerifyResourceStatistics" 
> --gtest_break_on_failure --gtest_repeat=100
> 
> 
> Thanks,
> 
> Sergey Urbanovich
> 
>

Re: Review Request 67932: Use `wclayer` from `hcsshim` to extract file layers.

[Andrew Schwartzmeyer]

> On July 16, 2018, 12:48 p.m., Mesos Reviewbot Windows wrote:
> > FAIL: Some of the unit tests failed. Please check the relevant logs.
> > 
> > Reviews applied: `['67930', '67931', '67932']`
> > 
> > Failed command: `Start-MesosCITesting`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1935/mesos-review-67932
> > 
> > Relevant logs:
> > 
> > - 
> > [stout-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1935/mesos-review-67932/logs/stout-tests-cmake-stdout.log):
> > 
> > ```
> >  d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1056): 
> > warning C4267: '=': conversion from 'size_t' to 'Int32', possible loss of 
> > data 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
> >  d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1191): 
> > warning C4267: '=': conversion from 'size_t' to 'Int32', possible loss of 
> > data 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
> >  d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1417): 
> > warning C4996: 'strcat': This function or variable may be unsafe. Consider 
> > using strcat_s instead. To disable deprecation, use 
> > _CRT_SECURE_NO_WARNINGS. See online help for details. 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
> >  d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1418): 
> > warning C4996: 'strcat': This function or variable may be unsafe. Consider 
> > using strcat_s instead. To disable deprecation, use 
> > _CRT_SECURE_NO_WARNINGS. See online help for details. 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
> >  d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1423): 
> > warning C4996: 'setmode': The POSIX name for this item is deprecated. 
> > Instead, use the ISO C and C++ conformant name: _setmode. See online help 
> > for details. 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
> >  d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1425): 
> > 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\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
> >  d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1431): 
> > warning C4996: 'fdopen': The POSIX name for this item is deprecated. 
> > Instead, use the ISO C and C++ conformant name: _fdopen. See online help 
> > for details. 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
> > 
> > 
> >"D:\DCOS\mesos\3rdparty\stout\tests\stout-tests.vcxproj" (default 
> > target) (1) ->
> >"D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj" (default target) (8) ->
> >(CustomBuild target) -> 
> >  CUSTOMBUILD : error : downloading 
> > 'https://github.com/mesos/3rdparty/raw/master/curl-7.60.0.tar.gz' failed 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : The requested URL returned error : 404 Not Found 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : error : downloading 
> > 'https://github.com/mesos/3rdparty/raw/master/curl-7.60.0.tar.gz' failed 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : The requested URL returned error : 404 Not Found 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : error : downloading 
> > 'https://github.com/mesos/3rdparty/raw/master/curl-7.60.0.tar.gz' failed 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : The requested URL returned error : 404 Not Found 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : error : downloading 
> > 'https://github.com/mesos/3rdparty/raw/master/curl-7.60.0.tar.gz' failed 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : The requested URL returned error : 404 Not Found 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : error : downloading 
> > 'https://github.com/mesos/3rdparty/raw/master/curl-7.60.0.tar.gz' failed 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : The requested URL returned error : 404 Not Found 
> > [D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
> >  CUSTOMBUILD : error : downloading 
> > 'https://github.com/mesos/3rdparty/raw/master/curl

Re: Review Request 67932: Use `wclayer` from `hcsshim` to extract file layers.

[Andrew Schwartzmeyer]

> On July 18, 2018, 3:10 a.m., Mesos Reviewbot Windows wrote:
> > Bad review!
> > 
> > Error:
> > Circular dependency detected for review 67931. Please fix the 'depends_on' 
> > field.

Did this get fixed?


- Andrew


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


On July 18, 2018, 2:27 a.m., Liangyu Zhao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67932/
> ---
> 
> (Updated July 18, 2018, 2:27 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `tar` command cannot work successfully on Windows, so use `wclayer`
> instead. Note that the folder generated from extraction also cannot be
> deleted by `rmdir`, so the GC is also changed to use `wclayer remove`.
> 
> 
> Diffs
> -
> 
>   src/common/command_utils.hpp 525f9c1c37b74f7e4cc71fdc8d52944226998ddc 
>   src/common/command_utils.cpp 7dfcc9ff74bcf044d47b803ebc42cf63fba89d17 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> a5683e3fe15dd35596122fcc0c580ae9d3adf7f2 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 85aad25ac8ecfda125be85fb46d882c3982f3930 
> 
> 
> Diff: https://reviews.apache.org/r/67932/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Liangyu Zhao
> 
>

Re: Review Request 67931: Support Image Manifest Version 2 Schema 2.

[Andrew Schwartzmeyer]

> On July 17, 2018, 11:16 a.m., Andrew Schwartzmeyer wrote:
> > include/mesos/docker/spec.hpp
> > Lines 124-126 (patched)
> > 
> >
> > I think this and the commit before need to be re-ordered, as this 
> > commit is introducing logic required and used by the previous commit.

I think you fixed this?


> On July 17, 2018, 11:16 a.m., Andrew Schwartzmeyer wrote:
> > include/mesos/docker/spec.hpp
> > Lines 141 (patched)
> > 
> >
> > Dumb question, but is there an S1/S2 for V1 as well?
> 
> Liangyu Zhao wrote:
> I don't think so.

There is, but it's not in use, so I'll drop this.


> On July 17, 2018, 11:16 a.m., Andrew Schwartzmeyer wrote:
> > include/mesos/docker/spec.hpp
> > Lines 141-164 (patched)
> > 
> >
> > I'm not sure, but would this be better as a variant?
> > 
> > ```
> > std::unique_ptr > v2_2::ImageManifest>> manifest;
> > ```
> > 
> > (Declared at point of use.)
> > 
> > P.S. Why do we need pointers?
> 
> Liangyu Zhao wrote:
> I haven't used the `std::Variant` before. I think it is a better choice.
> 
> The reason why I used pointers is trying to avoid copying the object. 
> This is just a minor optimization of efficiency.

Avoid pointers whenever possible; the minor optimization isn't worth the 
potential bug cost. Plus with move semantics the optimization probably won't 
even matter.


> On July 17, 2018, 11:16 a.m., Andrew Schwartzmeyer wrote:
> > src/uri/fetcher.cpp
> > Lines 88 (patched)
> > 
> >
> > Why does this function throw away `urls`?
> 
> Liangyu Zhao wrote:
> This is just a default parent class function. Currently, only 
> `DockerFetcherPulgin` supports multiple URLs fetch, so it overrides this 
> function. As for other plugins, this is the default way of handling extra 
> URLs. Commented at `include\mesos\uri\fetcher.hpp` line 89.

Gotcha.


- Andrew


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


On July 18, 2018, 2:27 a.m., Liangyu Zhao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67931/
> ---
> 
> (Updated July 18, 2018, 2:27 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support parsing schema 2 and fetching blob with multiple URLs as
> specified in schema 2. Update `curl` version to 7.61.0 because of bug
> encountered in version 7.57.0.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt b58996d2ed7521e42d6298d174cc8c494b84eb8f 
>   3rdparty/cmake/Versions.cmake 0a897d808cd9e05ac0d1a4e1ca61b8f3538f0c4a 
>   include/mesos/docker/spec.hpp 2879414dc42ffe633ac74b51e1bb116698c41162 
>   include/mesos/docker/v2_2.hpp PRE-CREATION 
>   include/mesos/docker/v2_2.proto PRE-CREATION 
>   include/mesos/uri/fetcher.hpp 760d6b33234d8efdc533c0c6f05e83a5c7d1f56b 
>   src/CMakeLists.txt 10b0946d6f49c7e9c201bad6f9f1b41cc8460fe5 
>   src/Makefile.am 228e168c22f3fd0367f029c506171c6979b31c07 
>   src/docker/spec.cpp 96fbf1f9cf1c2c4b2383607a97990f3a9156e6d9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 98129d006cda9b65804b518619b6addc8990410a 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> a5683e3fe15dd35596122fcc0c580ae9d3adf7f2 
>   src/uri/fetcher.hpp fbf64c6767dea3aa0798f68db8322ce47cd8ad36 
>   src/uri/fetcher.cpp 489c7ce0198ee6803dcc8eb9710b292fa743a0e8 
>   src/uri/fetchers/docker.hpp cdbab9a5684a68a729be12bc06d331acca137da5 
>   src/uri/fetchers/docker.cpp 55ca118660872a933a2dc186723bec6a39ee80f7 
> 
> 
> Diff: https://reviews.apache.org/r/67931/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Liangyu Zhao
> 
>

Re: Review Request 67916: Patched Google Test with upstream bugfix.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67916 was successfully built and tested.

Reviews applied: `['67916']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2003/mesos-review-67916

- Mesos Reviewbot Windows


On July 30, 2018, 11:50 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67916/
> ---
> 
> (Updated July 30, 2018, 11:50 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-8990
> https://issues.apache.org/jira/browse/MESOS-8990
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Per MESOS-8990, our Google Test dependency needs a patch from
> upstream, https://github.com/google/googletest/pull/1620, in order to
> continue building with the next version of MSVC (and potentially other
> compilers).
> 
> This patch file was generated by cherry-picking `f66ab00` from
> `master` onto `release-1.8.0` in the Google Test repo, and resolving
> the merge conflict.
> 
> Additionally, we need to define `GTEST_LANG_CXX11=1` when including
> the GoogleTest headers such that the patch is used.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt d8d113c17d124b763659dcc5ace9066499de6cd4 
>   3rdparty/googletest-release-1.8.0.patch PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67916/diff/2/
> 
> 
> Testing
> ---
> 
> Repro'ed MESOS-8990 on Windows with Visual Studio Preview, and the following 
> (temporary) CMake changes:
> 
> ```
>  # Set the default standard to C++11 for all targets.
> -set(CMAKE_CXX_STANDARD 11)
> -set(CMAKE_CXX_STANDARD_REQUIRED ON)
> +# set(CMAKE_CXX_STANDARD 11)
> +# set(CMAKE_CXX_STANDARD_REQUIRED ON)
>  # Do not use, for example, `-std=gnu++11`.
> -set(CMAKE_CXX_EXTENSIONS OFF)
> +# set(CMAKE_CXX_EXTENSIONS OFF)
> +add_compile_options(/std:c++latest)
> +add_definitions(-D_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING)
> +add_definitions(-D_HAS_AUTO_PTR_ETC=1)
> +add_definitions(-D_HAS_TR1_NAMESPACE=1)
> +add_definitions(-D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING)
> +add_definitions(-D_SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING)
> ```
> 
> After adding the necessary compile interface definition, `ninja tests` 
> successfully built on Windows with this patch applied, after not building 
> before the patch.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 68048: Added documentation for per-framework metrics.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On July 25, 2018, 4:06 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68048/
> ---
> 
> (Updated July 25, 2018, 4:06 p.m.)
> 
> 
> Review request for mesos and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for per-framework metrics.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 336f3aa7d2440e826765ae2bf022a5f6acac2927 
> 
> 
> Diff: https://reviews.apache.org/r/68048/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 68111: Added 'MesosCon 2018 CFP is now open!' blog post.

[Gastón Kleiman]

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

(Updated July 30, 2018, 1:38 p.m.)


Review request for mesos, Greg Mann and Vinod Kone.


Changes
---

Reformatted markdown, fixed dates, added a paragraph explaining what's MesosCon.


Repository: mesos


Description
---

Added 'MesosCon 2018 CFP is now open!' blog post.


Diffs (updated)
-

  site/source/blog/2018-07-30-mesoscon-2018-cfp-is-now-open.md PRE-CREATION 


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

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


Testing
---

Used `./mesos-website-dev.sh` to verify that the website is properly rendered.


Thanks,

Gastón Kleiman

Re: Review Request 68111: Added 'MesosCon 2018 CFP is now open!' blog post.

[Benjamin Bannier]

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



I believe we should add some more content to explain what MesosCon is.

Also, please consistently rewrap at a reasonable column limit, and possible 
join all lines in a paragraph.


site/source/blog/2018-07-30-mesoscon-2018-cfp-is-now-open.md
Lines 12 (patched)


Should we add a paragraph here explaining what MesosCon is? Something along 
the lines of

> MesosCon is the gathering of the Mesos community to discuss ...



site/source/blog/2018-07-30-mesoscon-2018-cfp-is-now-open.md
Lines 13 (patched)


The dates look off here.

Also, `s/we're/we are/`.



site/source/blog/2018-07-30-mesoscon-2018-cfp-is-now-open.md
Lines 23 (patched)


2017?



site/source/blog/2018-07-30-mesoscon-2018-cfp-is-now-open.md
Lines 25 (patched)


`s/you're/you are/`

`s/don't/do not/`

`s/We'll/We would/`



site/source/blog/2018-07-30-mesoscon-2018-cfp-is-now-open.md
Lines 34 (patched)


Use ticks `'`, not quotes `’`.


- Benjamin Bannier


On July 30, 2018, 10:17 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68111/
> ---
> 
> (Updated July 30, 2018, 10:17 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'MesosCon 2018 CFP is now open!' blog post.
> 
> 
> Diffs
> -
> 
>   site/source/blog/2018-07-30-mesoscon-2018-cfp-is-now-open.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68111/diff/2/
> 
> 
> Testing
> ---
> 
> Used `./mesos-website-dev.sh` to verify that the website is properly rendered.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 68111: Added 'MesosCon 2018 CFP is now open!' blog post.

[Gastón Kleiman]

> On July 30, 2018, 1:16 p.m., Vinod Kone wrote:
> > site/source/blog/2018-07-30-mesoscon-2018-cfp-is-now-open.md
> > Lines 23 (patched)
> > 
> >
> > s/2018/2017/

Good catch! Thanks =).


- Gastón


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


On July 30, 2018, 1:17 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68111/
> ---
> 
> (Updated July 30, 2018, 1:17 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'MesosCon 2018 CFP is now open!' blog post.
> 
> 
> Diffs
> -
> 
>   site/source/blog/2018-07-30-mesoscon-2018-cfp-is-now-open.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68111/diff/2/
> 
> 
> Testing
> ---
> 
> Used `./mesos-website-dev.sh` to verify that the website is properly rendered.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 68111: Added 'MesosCon 2018 CFP is now open!' blog post.

[Gastón Kleiman]

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

(Updated July 30, 2018, 1:17 p.m.)


Review request for mesos, Greg Mann and Vinod Kone.


Changes
---

Fixed typo.


Repository: mesos


Description
---

Added 'MesosCon 2018 CFP is now open!' blog post.


Diffs (updated)
-

  site/source/blog/2018-07-30-mesoscon-2018-cfp-is-now-open.md PRE-CREATION 


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

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


Testing
---

Used `./mesos-website-dev.sh` to verify that the website is properly rendered.


Thanks,

Gastón Kleiman

Re: Review Request 68111: Added 'MesosCon 2018 CFP is now open!' blog post.

[Vinod Kone]

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


Fix it, then Ship it!





site/source/blog/2018-07-30-mesoscon-2018-cfp-is-now-open.md
Lines 23 (patched)


s/2018/2017/


- Vinod Kone


On July 30, 2018, 8:12 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68111/
> ---
> 
> (Updated July 30, 2018, 8:12 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'MesosCon 2018 CFP is now open!' blog post.
> 
> 
> Diffs
> -
> 
>   site/source/blog/2018-07-30-mesoscon-2018-cfp-is-now-open.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68111/diff/1/
> 
> 
> Testing
> ---
> 
> Used `./mesos-website-dev.sh` to verify that the website is properly rendered.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Review Request 68111: Added 'MesosCon 2018 CFP is now open!' blog post.

[Gastón Kleiman]

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

Review request for mesos, Greg Mann and Vinod Kone.


Repository: mesos


Description
---

Added 'MesosCon 2018 CFP is now open!' blog post.


Diffs
-

  site/source/blog/2018-07-30-mesoscon-2018-cfp-is-now-open.md PRE-CREATION 


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


Testing
---

Used `./mesos-website-dev.sh` to verify that the website is properly rendered.


Thanks,

Gastón Kleiman

Re: Review Request 68088: Replaced `cgroups::DESTROY_TIMEOUT` with new agent flag.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68088 was successfully built and tested.

Reviews applied: `['68088']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2002/mesos-review-68088

- Mesos Reviewbot Windows


On July 30, 2018, 5:50 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68088/
> ---
> 
> (Updated July 30, 2018, 5:50 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jason Lai.
> 
> 
> Bugs: MESOS-8038
> https://issues.apache.org/jira/browse/MESOS-8038
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new agent flag can be used to reconfigure how long a container
> destroy is allowed to take on Mesos containerizer.
> 
> The default is also increased to 5 min based on suggestion from Gilbert
> because certain containers could have deep system calls which may not
> finish within previous 1 min timeout.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 6a6f0e6df208bc0b0a888d132b3befd062755851 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 8fba6f9d335cc43a7cd0fcb51ed047ba0e7027ed 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 3bddcece7028745cec6623ac33dbfcaced629629 
>   src/slave/flags.hpp eeb9708f9ec76d83b6719541f4a012544c7c0cbe 
>   src/slave/flags.cpp 58cdc0f1100fe244e5bf1036e1ccf39478d5d478 
> 
> 
> Diff: https://reviews.apache.org/r/68088/diff/2/
> 
> 
> Testing
> ---
> 
> `make` and `./bin/mesos-slave.sh --help`
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 67916: Patched Google Test with upstream bugfix.

[Andrew Schwartzmeyer]

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

(Updated July 30, 2018, 11:50 a.m.)


Review request for mesos, Benjamin Bannier, Joseph Wu, and Till Toenshoff.


Changes
---

Fixed build by adding compile interface definition.


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


Repository: mesos


Description (updated)
---

Per MESOS-8990, our Google Test dependency needs a patch from
upstream, https://github.com/google/googletest/pull/1620, in order to
continue building with the next version of MSVC (and potentially other
compilers).

This patch file was generated by cherry-picking `f66ab00` from
`master` onto `release-1.8.0` in the Google Test repo, and resolving
the merge conflict.

Additionally, we need to define `GTEST_LANG_CXX11=1` when including
the GoogleTest headers such that the patch is used.


Diffs (updated)
-

  3rdparty/CMakeLists.txt d8d113c17d124b763659dcc5ace9066499de6cd4 
  3rdparty/googletest-release-1.8.0.patch PRE-CREATION 


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

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


Testing (updated)
---

Repro'ed MESOS-8990 on Windows with Visual Studio Preview, and the following 
(temporary) CMake changes:

```
 # Set the default standard to C++11 for all targets.
-set(CMAKE_CXX_STANDARD 11)
-set(CMAKE_CXX_STANDARD_REQUIRED ON)
+# set(CMAKE_CXX_STANDARD 11)
+# set(CMAKE_CXX_STANDARD_REQUIRED ON)
 # Do not use, for example, `-std=gnu++11`.
-set(CMAKE_CXX_EXTENSIONS OFF)
+# set(CMAKE_CXX_EXTENSIONS OFF)
+add_compile_options(/std:c++latest)
+add_definitions(-D_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING)
+add_definitions(-D_HAS_AUTO_PTR_ETC=1)
+add_definitions(-D_HAS_TR1_NAMESPACE=1)
+add_definitions(-D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING)
+add_definitions(-D_SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING)
```

After adding the necessary compile interface definition, `ninja tests` 
successfully built on Windows with this patch applied, after not building 
before the patch.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 67916: Patched Google Test with upstream bugfix.

[Andrew Schwartzmeyer]

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

(Updated July 30, 2018, 11:02 a.m.)


Review request for mesos, Benjamin Bannier, Joseph Wu, and Till Toenshoff.


Changes
---

Does not fix MESOS-8990 (yet).


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


Repository: mesos


Description
---

Per MESOS-8990, our Google Test dependency needs a patch from
upstream, https://github.com/google/googletest/pull/1620, in order to
continue building with the next version of MSVC (and potentially other
compilers).

This patch file was generated by cherry-picking `f66ab00` from
`master` onto `release-1.8.0` in the Google Test repo, and resolving
the merge conflict.


Diffs
-

  3rdparty/CMakeLists.txt b58996d2ed7521e42d6298d174cc8c494b84eb8f 
  3rdparty/googletest-release-1.8.0.patch PRE-CREATION 


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


Testing (updated)
---

Repro'ed MESOS-8990 on Windows with Visual Studio Preview, and the following 
(temporary) CMake changes:

```
 # Set the default standard to C++11 for all targets.
-set(CMAKE_CXX_STANDARD 11)
-set(CMAKE_CXX_STANDARD_REQUIRED ON)
+# set(CMAKE_CXX_STANDARD 11)
+# set(CMAKE_CXX_STANDARD_REQUIRED ON)
 # Do not use, for example, `-std=gnu++11`.
-set(CMAKE_CXX_EXTENSIONS OFF)
+# set(CMAKE_CXX_EXTENSIONS OFF)
+add_compile_options(/std:c++latest)
+add_definitions(-D_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING)
+add_definitions(-D_HAS_AUTO_PTR_ETC=1)
+add_definitions(-D_HAS_TR1_NAMESPACE=1)
+add_definitions(-D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING)
+add_definitions(-D_SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING)
```

Unfortunately, it still repro'ed with this patch applied too, so I need to 
figure out what we missed.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 68098: Fixed rapidjson make install directory structure.

[Benjamin Mahler]

> On July 30, 2018, 9:22 a.m., Benno Evers wrote:
> > Looks good, except for two typos in the commit message (`no_base` -> 
> > `nobase`, `strucutre` -> `structure`)
> 
> Benjamin Mahler wrote:
> Thanks for catching those!

ah I accidentally lost my edits to the commit message typos prior to pushing :(


- Benjamin


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


On July 29, 2018, 9:46 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68098/
> ---
> 
> (Updated July 29, 2018, 9:46 p.m.)
> 
> 
> Review request for mesos, Benno Evers and James Peach.
> 
> 
> Bugs: MESOS-9115
> https://issues.apache.org/jira/browse/MESOS-9115
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The rapidjson headers were getting flattened into the same top-level
> include directory. The `no_base` prefix will preserve the directory
> strucutre of the headers, however we would need a Makefile further
> down for this to work (as done with stout and libprocess). Instead,
> we use separate variables as suggested here:
> 
> https://www.gnu.org/software/automake/manual/html_node/Alternative.html
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 26e5d55561731ff03639df31562bb835d9687339 
> 
> 
> Diff: https://reviews.apache.org/r/68098/diff/1/
> 
> 
> Testing
> ---
> 
> $ make install DESTDIR=tmp
> ...
> $ cd tmp/ && find include/
> [bmahler@core-dev local]$ find include
> include
> include/elfio
> include/elfio/elf_types.hpp
> include/elfio/elfio.hpp
> include/elfio/elfio_dump.hpp
> include/elfio/elfio_dynamic.hpp
> include/elfio/elfio_header.hpp
> include/elfio/elfio_note.hpp
> include/elfio/elfio_relocation.hpp
> include/elfio/elfio_section.hpp
> include/elfio/elfio_segment.hpp
> include/elfio/elfio_strings.hpp
> include/elfio/elfio_symbols.hpp
> include/elfio/elfio_utils.hpp
> include/picojson.h
> include/rapidjson
> include/rapidjson/error
> include/rapidjson/error/en.h
> include/rapidjson/error/error.h
> include/rapidjson/internal
> include/rapidjson/internal/biginteger.h
> include/rapidjson/internal/diyfp.h
> include/rapidjson/internal/dtoa.h
> include/rapidjson/internal/ieee754.h
> include/rapidjson/internal/itoa.h
> include/rapidjson/internal/meta.h
> include/rapidjson/internal/pow10.h
> include/rapidjson/internal/regex.h
> include/rapidjson/internal/stack.h
> include/rapidjson/internal/strfunc.h
> include/rapidjson/internal/strtod.h
> include/rapidjson/internal/swap.h
> include/rapidjson/msinttypes
> include/rapidjson/msinttypes/inttypes.h
> include/rapidjson/msinttypes/stdint.h
> include/rapidjson/allocators.h
> include/rapidjson/document.h
> include/rapidjson/encodedstream.h
> include/rapidjson/encodings.h
> include/rapidjson/filereadstream.h
> include/rapidjson/filewritestream.h
> include/rapidjson/fwd.h
> include/rapidjson/istreamwrapper.h
> include/rapidjson/memorybuffer.h
> include/rapidjson/memorystream.h
> include/rapidjson/ostreamwrapper.h
> include/rapidjson/pointer.h
> include/rapidjson/prettywriter.h
> include/rapidjson/rapidjson.h
> include/rapidjson/reader.h
> include/rapidjson/schema.h
> include/rapidjson/stream.h
> include/rapidjson/stringbuffer.h
> include/rapidjson/writer.h
> include/stout
> ...
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68098: Fixed rapidjson make install directory structure.

[Benjamin Mahler]

> On July 30, 2018, 9:22 a.m., Benno Evers wrote:
> > Looks good, except for two typos in the commit message (`no_base` -> 
> > `nobase`, `strucutre` -> `structure`)

Thanks for catching those!


- Benjamin


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


On July 29, 2018, 9:46 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68098/
> ---
> 
> (Updated July 29, 2018, 9:46 p.m.)
> 
> 
> Review request for mesos, Benno Evers and James Peach.
> 
> 
> Bugs: MESOS-9115
> https://issues.apache.org/jira/browse/MESOS-9115
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The rapidjson headers were getting flattened into the same top-level
> include directory. The `no_base` prefix will preserve the directory
> strucutre of the headers, however we would need a Makefile further
> down for this to work (as done with stout and libprocess). Instead,
> we use separate variables as suggested here:
> 
> https://www.gnu.org/software/automake/manual/html_node/Alternative.html
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 26e5d55561731ff03639df31562bb835d9687339 
> 
> 
> Diff: https://reviews.apache.org/r/68098/diff/1/
> 
> 
> Testing
> ---
> 
> $ make install DESTDIR=tmp
> ...
> $ cd tmp/ && find include/
> [bmahler@core-dev local]$ find include
> include
> include/elfio
> include/elfio/elf_types.hpp
> include/elfio/elfio.hpp
> include/elfio/elfio_dump.hpp
> include/elfio/elfio_dynamic.hpp
> include/elfio/elfio_header.hpp
> include/elfio/elfio_note.hpp
> include/elfio/elfio_relocation.hpp
> include/elfio/elfio_section.hpp
> include/elfio/elfio_segment.hpp
> include/elfio/elfio_strings.hpp
> include/elfio/elfio_symbols.hpp
> include/elfio/elfio_utils.hpp
> include/picojson.h
> include/rapidjson
> include/rapidjson/error
> include/rapidjson/error/en.h
> include/rapidjson/error/error.h
> include/rapidjson/internal
> include/rapidjson/internal/biginteger.h
> include/rapidjson/internal/diyfp.h
> include/rapidjson/internal/dtoa.h
> include/rapidjson/internal/ieee754.h
> include/rapidjson/internal/itoa.h
> include/rapidjson/internal/meta.h
> include/rapidjson/internal/pow10.h
> include/rapidjson/internal/regex.h
> include/rapidjson/internal/stack.h
> include/rapidjson/internal/strfunc.h
> include/rapidjson/internal/strtod.h
> include/rapidjson/internal/swap.h
> include/rapidjson/msinttypes
> include/rapidjson/msinttypes/inttypes.h
> include/rapidjson/msinttypes/stdint.h
> include/rapidjson/allocators.h
> include/rapidjson/document.h
> include/rapidjson/encodedstream.h
> include/rapidjson/encodings.h
> include/rapidjson/filereadstream.h
> include/rapidjson/filewritestream.h
> include/rapidjson/fwd.h
> include/rapidjson/istreamwrapper.h
> include/rapidjson/memorybuffer.h
> include/rapidjson/memorystream.h
> include/rapidjson/ostreamwrapper.h
> include/rapidjson/pointer.h
> include/rapidjson/prettywriter.h
> include/rapidjson/rapidjson.h
> include/rapidjson/reader.h
> include/rapidjson/schema.h
> include/rapidjson/stream.h
> include/rapidjson/stringbuffer.h
> include/rapidjson/writer.h
> include/stout
> ...
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68088: Replaced `cgroups::DESTROY_TIMEOUT` with new agent flag.

[Jason Lai]

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


Ship it!




Ship It!

- Jason Lai


On July 30, 2018, 5:50 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68088/
> ---
> 
> (Updated July 30, 2018, 5:50 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jason Lai.
> 
> 
> Bugs: MESOS-8038
> https://issues.apache.org/jira/browse/MESOS-8038
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new agent flag can be used to reconfigure how long a container
> destroy is allowed to take on Mesos containerizer.
> 
> The default is also increased to 5 min based on suggestion from Gilbert
> because certain containers could have deep system calls which may not
> finish within previous 1 min timeout.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 6a6f0e6df208bc0b0a888d132b3befd062755851 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 8fba6f9d335cc43a7cd0fcb51ed047ba0e7027ed 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 3bddcece7028745cec6623ac33dbfcaced629629 
>   src/slave/flags.hpp eeb9708f9ec76d83b6719541f4a012544c7c0cbe 
>   src/slave/flags.cpp 58cdc0f1100fe244e5bf1036e1ccf39478d5d478 
> 
> 
> Diff: https://reviews.apache.org/r/68088/diff/2/
> 
> 
> Testing
> ---
> 
> `make` and `./bin/mesos-slave.sh --help`
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 68088: Replaced `cgroups::DESTROY_TIMEOUT` with new agent flag.

[Zhitao Li]

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

(Updated July 30, 2018, 10:50 a.m.)


Review request for mesos, Gilbert Song and Jason Lai.


Changes
---

Squash both diffs.


Summary (updated)
-

Replaced `cgroups::DESTROY_TIMEOUT` with new agent flag.


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


Repository: mesos


Description (updated)
---

The new agent flag can be used to reconfigure how long a container
destroy is allowed to take on Mesos containerizer.

The default is also increased to 5 min based on suggestion from Gilbert
because certain containers could have deep system calls which may not
finish within previous 1 min timeout.


Diffs (updated)
-

  src/linux/cgroups.hpp 6a6f0e6df208bc0b0a888d132b3befd062755851 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
8fba6f9d335cc43a7cd0fcb51ed047ba0e7027ed 
  src/slave/containerizer/mesos/linux_launcher.cpp 
3bddcece7028745cec6623ac33dbfcaced629629 
  src/slave/flags.hpp eeb9708f9ec76d83b6719541f4a012544c7c0cbe 
  src/slave/flags.cpp 58cdc0f1100fe244e5bf1036e1ccf39478d5d478 


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

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


Testing
---

`make` and `./bin/mesos-slave.sh --help`


Thanks,

Zhitao Li

Re: Review Request 68092: CMake: Always build gRPC in Mesos.

[Benjamin Bannier]

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


Fix it, then Ship it!





3rdparty/CMakeLists.txt
Line 1 (original), 1 (patched)


Any reason we cannot just merge this patch with 
https://reviews.apache.org/r/68074/?



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


I couldn't find out how `PROTOC_GRPC` is set and wheter it can ever be 
false. Do we still need this conditional?

Here and below.


- Benjamin Bannier


On July 27, 2018, 9:12 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68092/
> ---
> 
> (Updated July 27, 2018, 9:12 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-8395
> https://issues.apache.org/jira/browse/MESOS-8395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Always build gRPC in Mesos.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 9b0dfe0ab2187d6d57bb267939aa7c1dbf6c3e8d 
>   cmake/CompilationConfigure.cmake f80c265d117c70acda95da6b943cdd3e4e477daa 
>   src/CMakeLists.txt 10b0946d6f49c7e9c201bad6f9f1b41cc8460fe5 
>   src/cmake/MesosProtobuf.cmake 9fadc9f0616a00f3c30c1fa81a44e0023ccf865f 
>   src/examples/CMakeLists.txt 6a6ee7c3029552ef12726d1c36ccabd8c79ba1f8 
>   src/tests/CMakeLists.txt 695b6f543847d4270f7004a1768919904975d7a4 
> 
> 
> Diff: https://reviews.apache.org/r/68092/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check with CMake
> Built with Ninja
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 68074: Autotools: Always build gRPC in Mesos.

[Benjamin Bannier]

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


Fix it, then Ship it!





cmake/CompilationConfigure.cmake
Line 80 (original), 80 (patched)


If you want to keep separate patches for modifying autotools and cmake, 
this should be in the next patch in the chain, 
https://reviews.apache.org/r/68092/.



configure.ac
Line 1 (original), 1 (patched)


Any reason we cannot just merge this patch with 
https://reviews.apache.org/r/68092/?



src/Makefile.am
Line 1529 (original), 1521 (patched)


Can we merge this with above list now?


- Benjamin Bannier


On July 27, 2018, 9:12 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68074/
> ---
> 
> (Updated July 27, 2018, 9:12 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-8395
> https://issues.apache.org/jira/browse/MESOS-8395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Autotools: Always build gRPC in Mesos.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 26e5d55561731ff03639df31562bb835d9687339 
>   cmake/CompilationConfigure.cmake f80c265d117c70acda95da6b943cdd3e4e477daa 
>   configure.ac 58003b6e823c8457f4b5a8c8d670be43bf96de8d 
>   include/csi/spec.hpp 2e9b870617a6604e5d3e93eedce119ec029eea05 
>   src/Makefile.am 71f9052f60fc65d8183faa7ab9764d3e6388ddc9 
>   src/python/native_common/ext_modules.py.in 
> dc4d91a8b8e3640e2191a53e657a5e4588fa911e 
>   src/resource_provider/local.cpp ae23c2000b7941aad92d3b83e4e905bdf042ebe8 
> 
> 
> Diff: https://reviews.apache.org/r/68074/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 68091: Always build gRPC in libprocess.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On July 27, 2018, 9:07 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68091/
> ---
> 
> (Updated July 27, 2018, 9:07 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-8395
> https://issues.apache.org/jira/browse/MESOS-8395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Always build gRPC in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> da350b1d36b9f545372999f38acc5022a9ca1034 
>   3rdparty/libprocess/Makefile.am abf667373aa65c9bb196d05a48d5d826d4d375e2 
>   3rdparty/libprocess/configure.ac f125e43800bcb6477b3c2ea5802040fa38411565 
>   3rdparty/libprocess/src/CMakeLists.txt 
> bb8af6d09d4d5207c7eb75c32bbd2952fd4e75a6 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> 5814bc615a8bf164ab563d9c32860f7d4e9325e0 
> 
> 
> Diff: https://reviews.apache.org/r/68091/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo make check` for standalone libprocess build.
> 
> NOTE: Mesos will fail to build if gRPC is not enabled.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 68105: WIP: Bumped bundled zookeeper.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the current review.

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

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2001/mesos-review-68105

Relevant logs:

- 
[apply-review-68105-stdout.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2001/mesos-review-68105/logs/apply-review-68105-stdout.log):

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

- Mesos Reviewbot Windows


On July 30, 2018, 9:06 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68105/
> ---
> 
> (Updated July 30, 2018, 9:06 a.m.)
> 
> 
> Review request for Mesos Reviewbot and Mesos Reviewbot Windows.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Bumped bundled zookeeper.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake c65d7b3d2d48ce9d0b4777aeb12fdf5b605803ac 
>   3rdparty/zookeeper-3.4.8.patch edfc3b5cf20568ded024137e3d4f7ab2fe221c2d 
>   3rdparty/zookeeper-3.4.8.tar.gz a23d68becf596b0246371fc244f018e4c71ca5d9 
> 
> 
> Diff: https://reviews.apache.org/r/68105/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 68104: Implemented authorization for agent `GET_RESOURCE_PROROVIDER` calls.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['68103', '68104']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2000/mesos-review-68104

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2000/mesos-review-68104/logs/mesos-tests-stdout.log):

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

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (32 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (36 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (70 ms 
total)

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

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

[--] Global test environment tear-down
[==] 1013 tests from 98 test cases ran. (511084 ms total)
[  PASSED  ] 1011 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] ContentType/AgentAPITest.GetResourceProviders/0, where GetParam() 
= application/x-protobuf
[  FAILED  ] ContentType/AgentAPITest.GetResourceProviders/1, where GetParam() 
= application/json

 2 FAILED TESTS
  YOU HAVE 222 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2000/mesos-review-68104/logs/mesos-tests-stderr.log):

```
I0730 10:01:14.117033 41656 master.cpp:10917] Updating the state of task 
97e2133f-50e7-45af-8e46-3750b3129849 of framework 
fcb1d046-be24-4dd5-9005-2d1e2953f587- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0730 10:01:14.117033 46456 slave.cpp:3939] Shutting down framework 
fcb1d046-be24-4dd5-9005-2d1e2953f587-
I0730 10:01:14.117033 46456 slave.cpp:6658] Shutting down executor 
'97e2133f-50e7-45af-8e46-3750b3129849' of framework 
fcb1d046-be24-4dd5-9005-2d1e2953f587- at executor(1)@192.10.1.6:51061
I0730 10:01:14.119036 41656 master.cpp:11016] Removing task 
97e2133f-50e7-45af-8e46-3750b3129849 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of fraI0730 10:01:13.954027 45360 exec.cpp:162] Version: 1.7.0
I0730 10:01:13.980051 38316 exec.cpp:236] Executor registered on agent 
fcb1d046-be24-4dd5-9005-2d1e2953f587-S0
I0730 10:01:13.984037 44996 executor.cpp:182] Received SUBSCRIBED event
I0730 10:01:13.988037 44996 executor.cpp:186] Subscribed executor on 
windows-02.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net
I0730 10:01:13.989035 44996 executor.cpp:182] Received LAUNCH event
I0730 10:01:13.993038 44996 executor.cpp:679] Starting task 
97e2133f-50e7-45af-8e46-3750b3129849
I0730 10:01:14.072033 44996 executor.cpp:499] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0730 10:01:14.081030 44996 executor.cpp:693] Forked command at 48112
I0730 10:01:14.120071 43776 exec.cpp:445] Executor asked to shutdown
I0730 10:01:14.121038 44996 executor.cpp:182] Received SHUTDOWN event
I0730 10:01:14.121038 44996 executor.cpp:796] Shutting down
I0730 10:01:14.121038 44996 executor.cpp:909] Sending SIGTERM to process tree 
at pid 48mework fcb1d046-be24-4dd5-9005-2d1e2953f587- on agent 
fcb1d046-be24-4dd5-9005-2d1e2953f587-S0 at slave(462)@192.10.1.6:49351 
(windows-02.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net)
I0730 10:01:14.120071 46456 slave.cpp:931] Agent terminating
W0730 10:01:14.120071 46456 slave.cpp:3935] Ignoring shutdown framework 
fcb1d046-be24-4dd5-9005-2d1e2953f587- because it is terminating
I0730 10:01:14.123036 47368 master.cpp:1330] Agent 
fcb1d046-be24-4dd5-9005-2d1e2953f587-S0 at slave(462)@192.10.1.6:49351 
(windows-02.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net) disconnected
I0730 10:01:14.123036 47368 master.cpp:3340] Disconnecting agent 
fcb1d046-be24-4dd5-9005-2d1e2953f587-S0 at slave(462)@192.10.1.6:49351 
(windows-02.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net)
I0730 10:01:14.123036 47368 master.cpp:3359] Deactivating agent 
f

Re: Review Request 68098: Fixed rapidjson make install directory structure.

[Benno Evers]

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


Ship it!




Looks good, except for two typos in the commit message (`no_base` -> `nobase`, 
`strucutre` -> `structure`)

- Benno Evers


On July 29, 2018, 9:46 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68098/
> ---
> 
> (Updated July 29, 2018, 9:46 p.m.)
> 
> 
> Review request for mesos, Benno Evers and James Peach.
> 
> 
> Bugs: MESOS-9115
> https://issues.apache.org/jira/browse/MESOS-9115
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The rapidjson headers were getting flattened into the same top-level
> include directory. The `no_base` prefix will preserve the directory
> strucutre of the headers, however we would need a Makefile further
> down for this to work (as done with stout and libprocess). Instead,
> we use separate variables as suggested here:
> 
> https://www.gnu.org/software/automake/manual/html_node/Alternative.html
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 26e5d55561731ff03639df31562bb835d9687339 
> 
> 
> Diff: https://reviews.apache.org/r/68098/diff/1/
> 
> 
> Testing
> ---
> 
> $ make install DESTDIR=tmp
> ...
> $ cd tmp/ && find include/
> [bmahler@core-dev local]$ find include
> include
> include/elfio
> include/elfio/elf_types.hpp
> include/elfio/elfio.hpp
> include/elfio/elfio_dump.hpp
> include/elfio/elfio_dynamic.hpp
> include/elfio/elfio_header.hpp
> include/elfio/elfio_note.hpp
> include/elfio/elfio_relocation.hpp
> include/elfio/elfio_section.hpp
> include/elfio/elfio_segment.hpp
> include/elfio/elfio_strings.hpp
> include/elfio/elfio_symbols.hpp
> include/elfio/elfio_utils.hpp
> include/picojson.h
> include/rapidjson
> include/rapidjson/error
> include/rapidjson/error/en.h
> include/rapidjson/error/error.h
> include/rapidjson/internal
> include/rapidjson/internal/biginteger.h
> include/rapidjson/internal/diyfp.h
> include/rapidjson/internal/dtoa.h
> include/rapidjson/internal/ieee754.h
> include/rapidjson/internal/itoa.h
> include/rapidjson/internal/meta.h
> include/rapidjson/internal/pow10.h
> include/rapidjson/internal/regex.h
> include/rapidjson/internal/stack.h
> include/rapidjson/internal/strfunc.h
> include/rapidjson/internal/strtod.h
> include/rapidjson/internal/swap.h
> include/rapidjson/msinttypes
> include/rapidjson/msinttypes/inttypes.h
> include/rapidjson/msinttypes/stdint.h
> include/rapidjson/allocators.h
> include/rapidjson/document.h
> include/rapidjson/encodedstream.h
> include/rapidjson/encodings.h
> include/rapidjson/filereadstream.h
> include/rapidjson/filewritestream.h
> include/rapidjson/fwd.h
> include/rapidjson/istreamwrapper.h
> include/rapidjson/memorybuffer.h
> include/rapidjson/memorystream.h
> include/rapidjson/ostreamwrapper.h
> include/rapidjson/pointer.h
> include/rapidjson/prettywriter.h
> include/rapidjson/rapidjson.h
> include/rapidjson/reader.h
> include/rapidjson/schema.h
> include/rapidjson/stream.h
> include/rapidjson/stringbuffer.h
> include/rapidjson/writer.h
> include/stout
> ...
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Review Request 68105: WIP: Bumped bundled zookeeper.

[Benjamin Bannier]

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

Review request for Mesos Reviewbot and Mesos Reviewbot Windows.


Repository: mesos


Description
---

WIP: Bumped bundled zookeeper.


Diffs
-

  3rdparty/cmake/Versions.cmake c65d7b3d2d48ce9d0b4777aeb12fdf5b605803ac 
  3rdparty/zookeeper-3.4.8.patch edfc3b5cf20568ded024137e3d4f7ab2fe221c2d 
  3rdparty/zookeeper-3.4.8.tar.gz a23d68becf596b0246371fc244f018e4c71ca5d9 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 68104: Implemented authorization for agent `GET_RESOURCE_PROROVIDER` calls.

[Benjamin Bannier]

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

Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.


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


Repository: mesos


Description
---

Implemented authorization for agent `GET_RESOURCE_PROROVIDER` calls.


Diffs
-

  src/slave/http.cpp ab5864d9fd2fde478ed7da2ca7ed8abedc72c7c5 
  src/tests/api_tests.cpp 182622a62d350ebefc891a385de3f2d35a7c0243 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 68103: Added an authorizer action for viewing of resource provider information.

[Benjamin Bannier]

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

Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.


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


Repository: mesos


Description
---

Added an authorizer action for viewing of resource provider information.


Diffs
-

  include/mesos/authorizer/acls.proto 1777c04316bfd7493894f0e782f170fe8437aafe 
  include/mesos/authorizer/authorizer.proto 
8b5fa09f389ca4c26f8390d35af32c4cdd561418 
  src/authorizer/local/authorizer.cpp abf5b4663cd517fb6d69b5373dd0e6520e87cf8e 
  src/tests/authorization_tests.cpp 41ecac29a53f6ad9553cbf0a1121e1f3cff50df8 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier