Re: Review Request 71535: Fixed certificate write functions for windows.

2019-09-23 Thread Till Toenshoff via Review Board

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

(Updated Sept. 24, 2019, 12:41 a.m.)


Review request for mesos, Greg Mann and Joseph Wu.


Repository: mesos


Description (updated)
---

The former implementation failed on Windows. By switching
to the recommended `BIO_` API for serializing the x509 data
to disk, we avoid that problem.


Diffs
-

  3rdparty/libprocess/src/ssl/utilities.cpp 
c4a8ab4f83939554ca3eb7b4eb86c42c8a539b48 


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


Testing
---

Tested "manually" within a private project on Windows.


Thanks,

Till Toenshoff



Re: Review Request 71535: Fixed certificate write functions for windows.

2019-09-23 Thread Till Toenshoff via Review Board

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

(Updated Sept. 24, 2019, 12:40 a.m.)


Review request for mesos, Greg Mann and Joseph Wu.


Repository: mesos


Description (updated)
---

Fixed certificate write functions for windows.


Diffs (updated)
-

  3rdparty/libprocess/src/ssl/utilities.cpp 
c4a8ab4f83939554ca3eb7b4eb86c42c8a539b48 


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

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


Testing
---

Tested "manually" within a private project on Windows.


Thanks,

Till Toenshoff



Review Request 71535: Fixed certificate write functions for windows.

2019-09-23 Thread Till Toenshoff via Review Board

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

Review request for mesos, Greg Mann and Joseph Wu.


Repository: mesos


Description
---

The former implementation failed on Windows. By switching
to the recommended `BIO_` API for serializing the x509 data
to disk, we avoid that problem.


Diffs
-

  3rdparty/libprocess/src/ssl/utilities.cpp 
c4a8ab4f83939554ca3eb7b4eb86c42c8a539b48 


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


Testing
---

Tested "manually" within a private project on Windows.


Thanks,

Till Toenshoff



Re: Review Request 71497: Introduced new names for SSL-related libprocess flags.

2019-09-20 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Sept. 19, 2019, 2:35 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71497/
> ---
> 
> (Updated Sept. 19, 2019, 2:35 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Till Toenshoff.
> 
> 
> Bugs: MESOS-9972
> https://issues.apache.org/jira/browse/MESOS-9972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `LIBPROCESS_SSL_REQUIRE_CERT` flag was renamed to
> `LIBPROCESS_SSL_REQUIRE_CLIENT_CERT`.
> 
> The `LIBPROCESS_SSL_VERIFY_CERT` flag was renamed to
> `LIBPROCESS_SSL_VERIFY_SERVER_CERT`.
> 
> The new names better describe the actual effect of both flags, and
> make upgrades easier by allowing operators to only enable verification
> on agents that are new enough to contain the updated hostname
> validation code paths.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/flags.hpp 
> 1a0e3820cc8cd1459625f46a54b194133500f11e 
>   3rdparty/libprocess/src/openssl.hpp 
> 271cc95238d287c06df36478554502e8b7205b09 
>   3rdparty/libprocess/src/openssl.cpp 
> 5854711971c9ebc4d676edc43af5ab5cfd5ea4c6 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 9d5ab679165a709f7c3740020961ec89a7db4f54 
>   docs/ssl.md 90a2eb9800b7d8d9aa9d7b1060a6e5eb9e124b02 
>   docs/upgrades.md e630731c332fdd7df788f96644a8084f30b5c621 
> 
> 
> Diff: https://reviews.apache.org/r/71497/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 71478: Windows: Moved definition out of inline function call.

2019-09-19 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Sept. 12, 2019, 7:35 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71478/
> ---
> 
> (Updated Sept. 12, 2019, 7:35 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MSVC does not deal with #ifdefs from inside function calls.
> So here, the `#if defined(...)` was taken literally and is
> considered a syntax error by MSVC.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> 5854711971c9ebc4d676edc43af5ab5cfd5ea4c6 
> 
> 
> Diff: https://reviews.apache.org/r/71478/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 71509: Windows: Fixed AllocationRoleEnvironmentVariable tests.

2019-09-18 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Sept. 18, 2019, 10:57 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71509/
> ---
> 
> (Updated Sept. 18, 2019, 10:57 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Qian Zhang, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-9874
> https://issues.apache.org/jira/browse/MESOS-9874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests were using a shell script, which can easily be
> converted to a batch script on Windows.
> 
> 
> Diffs
> -
> 
>   src/tests/command_executor_tests.cpp 
> e9726b0b6c4623d5196186cd43722952f77669da 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 5e31a95149bdc0de64eeab3c289f7816077c82c4 
>   src/tests/default_executor_tests.cpp 
> e5c0bf2df4b35fb3920104e1b72bea7797724933 
> 
> 
> Diff: https://reviews.apache.org/r/71509/diff/1/
> 
> 
> Testing
> ---
> 
> cmake --build . --target check (Windows)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 71497: Introduced new names for SSL-related libprocess flags.

2019-09-18 Thread Till Toenshoff via Review Board

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




3rdparty/libprocess/src/openssl.cpp
Lines 545-547 (patched)


We are stating that there would be a deprecation; does that mean at some 
point this flag won't work anymore? If so when?

Our experience with such changes shows that we will very likely never kill 
the old name to make sure we stay reliably compatible. The only realistic 
option appears to be Mesos 2.0 for a removal of the old flag. Let's create a 
blocker ticket for 2.0, add that as a comment referencing the JIRA and we are 
golden.



3rdparty/libprocess/src/openssl.cpp
Line 593 (original), 615 (patched)


s/compatility/compatibility/



3rdparty/libprocess/src/openssl.cpp
Line 594 (original), 616 (patched)


s/be//



docs/ssl.md
Lines 100 (patched)


s/below//



docs/ssl.md
Lines 117 (patched)


s/below//


- Till Toenshoff


On Sept. 17, 2019, 12:50 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71497/
> ---
> 
> (Updated Sept. 17, 2019, 12:50 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `LIBPROCESS_SSL_REQUIRE_CERT` flag was renamed to
> `LIBPROCESS_SSL_REQUIRE_CLIENT_CERT`.
> 
> The `LIBPROCESS_SSL_VERIFY_CERT` flag was renamed to
> `LIBPROCESS_SSL_VERIFY_SERVER_CERT`.
> 
> The new names better describe the actual effect of both flags, and
> make upgrades easier by allowing operators to only enable verification
> on agents that are new enough to contain the updated hostname
> validation code paths.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/flags.hpp 
> 1a0e3820cc8cd1459625f46a54b194133500f11e 
>   3rdparty/libprocess/src/openssl.hpp 
> 271cc95238d287c06df36478554502e8b7205b09 
>   3rdparty/libprocess/src/openssl.cpp 
> 5854711971c9ebc4d676edc43af5ab5cfd5ea4c6 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 9d5ab679165a709f7c3740020961ec89a7db4f54 
>   docs/ssl.md 90a2eb9800b7d8d9aa9d7b1060a6e5eb9e124b02 
> 
> 
> Diff: https://reviews.apache.org/r/71497/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 71430: Included new Python CLI in distribution tarball.

2019-09-17 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Sept. 4, 2019, 7:48 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71430/
> ---
> 
> (Updated Sept. 4, 2019, 7:48 a.m.)
> 
> 
> Review request for mesos, Benno Evers and Till Toenshoff.
> 
> 
> Bugs: MESOS-9958
> https://issues.apache.org/jira/browse/MESOS-9958
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Included new Python CLI in distribution tarball.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5daee632d52e882acc15b90f655a53dab23eaaf6 
> 
> 
> Diff: https://reviews.apache.org/r/71430/diff/1/
> 
> 
> Testing
> ---
> 
> * confirmed that the new CLI source files are contained in the distribution 
> tarball
> * one can currently still not run `make distcheck` with `--enable-new-cli` 
> since the Mesos lib egg build process puts output files into the source dir
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 71496: Removed an outdated reference to the 'libprocess' hostname validation.

2019-09-17 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Sept. 17, 2019, 12:35 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71496/
> ---
> 
> (Updated Sept. 17, 2019, 12:35 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed a reference to the 'libprocess' hostname validation scheme,
> which was removed to 'legacy' during development.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> 5854711971c9ebc4d676edc43af5ab5cfd5ea4c6 
> 
> 
> Diff: https://reviews.apache.org/r/71496/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 71395: Implemented snapshot bot.

2019-09-05 Thread Till Toenshoff via Review Board

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




support/mesos-snapshot.sh
Lines 23 (patched)


Very good point and I missed it in my review.


- Till Toenshoff


On Aug. 28, 2019, 4:41 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71395/
> ---
> 
> (Updated Aug. 28, 2019, 4:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Bugs: MESOS-9955
> https://issues.apache.org/jira/browse/MESOS-9955
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Dockerized the snapshot building process.
> 
> 
> Diffs
> -
> 
>   support/jenkins/snapshotbot.sh PRE-CREATION 
>   support/mesos-snapshot.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71395/diff/1/
> 
> 
> Testing
> ---
> 
> https://builds.apache.org/view/M-R/view/Mesos/job/Mesos-Snapshot-Jar-Test/16/console
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 71395: Implemented snapshot bot.

2019-09-05 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Aug. 28, 2019, 4:41 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71395/
> ---
> 
> (Updated Aug. 28, 2019, 4:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Bugs: MESOS-9955
> https://issues.apache.org/jira/browse/MESOS-9955
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Dockerized the snapshot building process.
> 
> 
> Diffs
> -
> 
>   support/jenkins/snapshotbot.sh PRE-CREATION 
>   support/mesos-snapshot.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71395/diff/1/
> 
> 
> Testing
> ---
> 
> https://builds.apache.org/view/M-R/view/Mesos/job/Mesos-Snapshot-Jar-Test/16/console
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 71393: Skiped GPG signing during `maven-install` in Makefile.

2019-09-05 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Aug. 28, 2019, 4:40 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71393/
> ---
> 
> (Updated Aug. 28, 2019, 4:40 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Bugs: MESOS-9955
> https://issues.apache.org/jira/browse/MESOS-9955
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> GPG signing is not needed for snapshot JARs (since they are
> typically done by CI). But our snapshot building workflow,
> i.e., support/snapshot.sh, relies on `make maven-install` which forces
> GPG signing to happen. This allows our snapshot script to skip GPG
> signing by doing `mvn deploy -Dgpg.skip`.
> 
> Note that GPG signing still happens for RCs and release JARs (which
> are typically done by humans) when `mvn deploy` (without `-Dgpg.skip`)
> is called (e.g., in support/vote.sh).
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5daee632d52e882acc15b90f655a53dab23eaaf6 
> 
> 
> Diff: https://reviews.apache.org/r/71393/diff/1/
> 
> 
> Testing
> ---
> 
> https://builds.apache.org/view/M-R/view/Mesos/job/Mesos-Snapshot-Jar-Test/16/console
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 71394: Skipped GPG signing for snapshot builds.

2019-09-05 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Aug. 28, 2019, 4:40 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71394/
> ---
> 
> (Updated Aug. 28, 2019, 4:40 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Bugs: MESOS-9955
> https://issues.apache.org/jira/browse/MESOS-9955
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Skipped GPG signing for snapshot builds.
> 
> 
> Diffs
> -
> 
>   support/snapshot.sh c30aca02ec999320e2b3f0f3ce36c04aa0425ae3 
> 
> 
> Diff: https://reviews.apache.org/r/71394/diff/1/
> 
> 
> Testing
> ---
> 
> https://builds.apache.org/view/M-R/view/Mesos/job/Mesos-Snapshot-Jar-Test/16/console
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 71392: Improved snapshot script to deduce the Mesos version.

2019-09-05 Thread Till Toenshoff via Review Board

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




support/snapshot.sh
Line 36 (original), 34 (patched)


This is a bit unfortunate I feel as "SNAPSHOT" does not help that much 
determining the version --- how about we use the HEAD SHA instead?

```
TAG=$VERSION-$SHA
```



support/snapshot.sh
Line 56 (original), 53 (patched)


Why are we hardcoding `-j3` here and below instead of using e.g. `$(nproc)`?


- Till Toenshoff


On Aug. 28, 2019, 4:39 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71392/
> ---
> 
> (Updated Aug. 28, 2019, 4:39 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Bugs: MESOS-9955
> https://issues.apache.org/jira/browse/MESOS-9955
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the script expected the version to be provided as an
> argument.
> 
> 
> Diffs
> -
> 
>   support/snapshot.sh c30aca02ec999320e2b3f0f3ce36c04aa0425ae3 
> 
> 
> Diff: https://reviews.apache.org/r/71392/diff/1/
> 
> 
> Testing
> ---
> 
> https://builds.apache.org/view/M-R/view/Mesos/job/Mesos-Snapshot-Jar-Test/16/console
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 71390: Fixed the snapshot script to be non-interactive.

2019-09-05 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Aug. 28, 2019, 4:38 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71390/
> ---
> 
> (Updated Aug. 28, 2019, 4:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Bugs: MESOS-9955
> https://issues.apache.org/jira/browse/MESOS-9955
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the snapshot script to be non-interactive.
> 
> 
> Diffs
> -
> 
>   support/snapshot.sh c30aca02ec999320e2b3f0f3ce36c04aa0425ae3 
> 
> 
> Diff: https://reviews.apache.org/r/71390/diff/1/
> 
> 
> Testing
> ---
> 
> https://builds.apache.org/view/M-R/view/Mesos/job/Mesos-Snapshot-Jar-Test/16/console
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 71241: Added a `distcheck` target to the cmake build.

2019-09-05 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Sept. 3, 2019, 2:51 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71241/
> ---
> 
> (Updated Sept. 3, 2019, 2:51 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a `distcheck` target to the cmake build which mimics the
> target of the same name provided by the autotools build. `distcheck`
> depends on the `dist` target to create a distribution archive and then
> ensures that with the distributed sources the `check` targets succeeds.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt dc50dd457585c14a456ca710da4a16a0d45ef17a 
>   cmake/distcheck.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71241/diff/3/
> 
> 
> Testing
> ---
> 
> `ninja distcheck` passes, no artifacts left around but tarball.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 71240: Added a `dist` target to the cmake build.

2019-09-05 Thread Till Toenshoff via Review Board

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




cmake/dist.sh
Lines 30 (patched)


Will fail with my setup as my repo-dir is called different than "mesos".


- Till Toenshoff


On Sept. 3, 2019, 1:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71240/
> ---
> 
> (Updated Sept. 3, 2019, 1:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a `dist` target to the cmake build, analogous to the
> target provided by the autotools build.
> 
> While cmake already provides a `package_source` target to create a
> source archive it does not take care of only including relevant files,
> but instead adds all files to the archive, e.g., including build or
> backup files which should not be part of a release. For that reason this
> patch introduces a script which performs a temporary `git clone` from
> the checked out git repository and creates the archive from that clean
> tree.
> 
> This patch also removes a hardcoded list of ignored files which was
> by design not exhaustive.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt dc50dd457585c14a456ca710da4a16a0d45ef17a 
>   cmake/dist.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71240/diff/2/
> 
> 
> Testing
> ---
> 
> `ninja dist` produces a tarball, no artifacts left around but tarball.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 71240: Added a `dist` target to the cmake build.

2019-09-05 Thread Till Toenshoff via Review Board

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




cmake/dist.sh
Lines 37 (patched)


s/Sucessfully/Successfully/


- Till Toenshoff


On Sept. 3, 2019, 1:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71240/
> ---
> 
> (Updated Sept. 3, 2019, 1:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a `dist` target to the cmake build, analogous to the
> target provided by the autotools build.
> 
> While cmake already provides a `package_source` target to create a
> source archive it does not take care of only including relevant files,
> but instead adds all files to the archive, e.g., including build or
> backup files which should not be part of a release. For that reason this
> patch introduces a script which performs a temporary `git clone` from
> the checked out git repository and creates the archive from that clean
> tree.
> 
> This patch also removes a hardcoded list of ignored files which was
> by design not exhaustive.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt dc50dd457585c14a456ca710da4a16a0d45ef17a 
>   cmake/dist.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71240/diff/2/
> 
> 
> Testing
> ---
> 
> `ninja dist` produces a tarball, no artifacts left around but tarball.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 71240: Added a `dist` target to the cmake build.

2019-09-05 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Sept. 3, 2019, 1:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71240/
> ---
> 
> (Updated Sept. 3, 2019, 1:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a `dist` target to the cmake build, analogous to the
> target provided by the autotools build.
> 
> While cmake already provides a `package_source` target to create a
> source archive it does not take care of only including relevant files,
> but instead adds all files to the archive, e.g., including build or
> backup files which should not be part of a release. For that reason this
> patch introduces a script which performs a temporary `git clone` from
> the checked out git repository and creates the archive from that clean
> tree.
> 
> This patch also removes a hardcoded list of ignored files which was
> by design not exhaustive.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt dc50dd457585c14a456ca710da4a16a0d45ef17a 
>   cmake/dist.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71240/diff/2/
> 
> 
> Testing
> ---
> 
> `ninja dist` produces a tarball, no artifacts left around but tarball.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 71207: Revert "Updated cpplint.py to be less verbose when there is no linting issue."

2019-08-26 Thread Till Toenshoff via Review Board

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


Ship it!




- Till Toenshoff


On Aug. 20, 2019, 11:49 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71207/
> ---
> 
> (Updated Aug. 20, 2019, 11:49 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This reverts commit c0f8f56d5a93f3fb870e448fedfd22f1491356ca.
> 
> This patch was necessary when we were running cpplint via
> `support/mesos-style.py` to prevent it from cluttering up the hook
> output. When running under pre-commit linter output is not shown if no
> errors occur so we can undo our change to stay closer to upstream.
> 
> 
> Diffs
> -
> 
>   support/cpplint.py 66ec8b3636a8d3ba57becd8560b4fe394e7119d8 
> 
> 
> Diff: https://reviews.apache.org/r/71207/diff/3/
> 
> 
> Testing
> ---
> 
> * used for development for a couple of months
> * reports no issues in the current source tree
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 71209: Enabled a number of additional pre-commit checks.

2019-08-26 Thread Till Toenshoff via Review Board

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


Ship it!




I'ld love to see the commit description to tell us which checks now got 
activated exactly - the more we explain, the better i feel.

- Till Toenshoff


On Aug. 19, 2019, 7:20 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71209/
> ---
> 
> (Updated Aug. 19, 2019, 7:20 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled a number of additional pre-commit checks.
> 
> 
> Diffs
> -
> 
>   support/pre-commit-config.yaml PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71209/diff/5/
> 
> 
> Testing
> ---
> 
> * used for development for a couple of months
> * reports no issues in the current source tree as indentified issues were 
> fixed
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 71300: Removed mesos-style transition script.

2019-08-26 Thread Till Toenshoff via Review Board

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


Ship it!




- Till Toenshoff


On Aug. 26, 2019, 4:40 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71300/
> ---
> 
> (Updated Aug. 26, 2019, 4:40 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed mesos-style transition script.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py cd490bd3deeb8477295bbad010f36d984ce17d27 
> 
> 
> Diff: https://reviews.apache.org/r/71300/diff/3/
> 
> 
> Testing
> ---
> 
> n/a
> 
> THIS PATCH SHOULD ONLY BE COMMITTED AFTER THE PRECEEDING CHAIN HAS BEEN 
> LANDED FOR SOME TIME TO GIVE CONTRIBUTORS A CHANCE TO ADJUST THEIR WORKFLOW.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 71208: Revert "Updated cpplint to be compatible with Python 3."

2019-08-26 Thread Till Toenshoff via Review Board

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


Ship it!




- Till Toenshoff


On July 30, 2019, 9:01 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71208/
> ---
> 
> (Updated July 30, 2019, 9:01 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This reverts commit 89db66e3df831eaa50fffb4149a3894097505c14.
> 
> This patch was necessary when we were running cpplint in the python3
> environment used e.g., also for bindings and other scripts. With
> pre-commit we have freedom to choose the Python environment needed so we
> can undo our adjustments here to stay closer to upstream.
> 
> 
> Diffs
> -
> 
>   support/cpplint.py 66ec8b3636a8d3ba57becd8560b4fe394e7119d8 
> 
> 
> Diff: https://reviews.apache.org/r/71208/diff/3/
> 
> 
> Testing
> ---
> 
> * used for development for a couple of months
> * reports no issues in the current source tree
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 71206: Removed old mesos-style and references.

2019-08-26 Thread Till Toenshoff via Review Board

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


Ship it!




- Till Toenshoff


On Aug. 20, 2019, 11:49 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71206/
> ---
> 
> (Updated Aug. 20, 2019, 11:49 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch removes references to `support/mesos-style.py` which was
> replaced with a pre-commit setup in a previous commit. We also remove
> the tool itself.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 8a48afe780f23736c9b7abeb7337977521cecfa5 
>   support/build-virtualenv 7dc03b054f7663979e4eb4b11ad51d759b7f1ad3 
>   support/hooks/commit-msg a0c218deee3fb4b7594fe39b76c1025045ba0725 
>   support/hooks/post-rewrite 1ab14abf711d1923a7ae69beb33581317009a94a 
>   support/hooks/pre-commit 519567bf5f20a74b273c8d8514577fe4342dc45d 
>   support/mesos-split.py 0a77c257386ffe576abd12f59f926640836ad900 
>   support/mesos-style.py cd490bd3deeb8477295bbad010f36d984ce17d27 
> 
> 
> Diff: https://reviews.apache.org/r/71206/diff/5/
> 
> 
> Testing
> ---
> 
> n/a
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 71205: Switched commit hooks to pre-commit.

2019-08-26 Thread Till Toenshoff via Review Board

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


Fix it, then Ship it!




When running the script, I noticed this
```
[INFO] Installing environment for local.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
[INFO] Installing environment for local.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
```
Why do we seemingly repeat that step? After this all further steps were non 
duplicated.


docs/advanced-contribution.md
Line 67 (original), 68 (patched)


"The hooks are set up with `./support/setup_dev.sh`"

We have to invoke it, so it does not really do much automatically, I'ld 
argue.



support/setup-dev.sh
Lines 57 (patched)


We generally strive for capitalizing product names - so let's have it this 
`Mesos` way everywhere please :)



support/setup-dev.sh
Lines 62 (patched)


Shouldnt there be an `exit(1)` here so we spare our users three more error 
messages?


- Till Toenshoff


On Aug. 20, 2019, 11:48 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71205/
> ---
> 
> (Updated Aug. 20, 2019, 11:48 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch switches commit hooks to be orchestrated by the pre-commit
> tool mirroring the previous linters invoked through git commit
> hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
> 
> Using pre-commit removes the burden of maintaining
> `support/mesos-style.py`, making sure that hooks have the expected
> environment (e.g., Python version, Node installed). Additionally,
> upstream provides a number of additional linters which are not hard to
> add to Mesos' hooks.
> 
> 
> Diffs
> -
> 
>   docs/advanced-contribution.md 573138d3642e901f46ee35db58299627f188b94c 
>   support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b 
>   support/pre-commit-config.yaml PRE-CREATION 
>   support/setup-dev.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4 
>   support/setup-dev.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71205/diff/8/
> 
> 
> Testing
> ---
> 
> * used successfully for a couple of months
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 71299: Added separate script to install developer setup.

2019-08-26 Thread Till Toenshoff via Review Board

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


Ship it!




Excellent structure - pretty much a move code only review, making it easy for 
us - Thanks!!


support/setup-dev.sh
Lines 29-48 (patched)


These still are rather unfortunate, I feel.


- Till Toenshoff


On Aug. 20, 2019, 11:48 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71299/
> ---
> 
> (Updated Aug. 20, 2019, 11:48 a.m.)
> 
> 
> Review request for mesos, Benno Evers and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch breaks the installation of developer tools (i.e., linter
> configuration files and git hooks) out of `./bootstrap`. This not only
> simplifies and streamlines the setup, but will allow us to add
> developer-only features without breaking users who are just interested
> in building a distribution tarball.
> 
> 
> Diffs
> -
> 
>   bootstrap 7be2cc95c7ace55d710315427f38284cc5b2af4c 
>   bootstrap.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4 
>   docs/advanced-contribution.md 573138d3642e901f46ee35db58299627f188b94c 
>   docs/beginner-contribution.md 471f5dd0da30d921cc3b29484d2b02f164f8ca75 
>   docs/clang-format.md 4289813c2ca78e1b963aa22a596abd6e8aa3a28b 
>   support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b 
>   support/llvm/README.md 188930224ba8ff53dbef8770bc56625be936bb76 
>   support/setup-dev.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71299/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 71204: Added gitlint config.

2019-08-26 Thread Till Toenshoff via Review Board

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


Ship it!




- Till Toenshoff


On Aug. 14, 2019, 11:24 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71204/
> ---
> 
> (Updated Aug. 14, 2019, 11:24 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a config for the gitlint tool which is slated to replace
> a custom commit-msg hook once we switch our hook infrastructure to the
> pre-commit tool.
> 
> 
> Diffs
> -
> 
>   bootstrap 7be2cc95c7ace55d710315427f38284cc5b2af4c 
>   bootstrap.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4 
>   support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b 
>   support/gitlint PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71204/diff/7/
> 
> 
> Testing
> ---
> 
> n/a
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 71203: Added check script to check for license headers.

2019-08-26 Thread Till Toenshoff via Review Board

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


Ship it!





support/check-license.py
Lines 25-26 (patched)


Nifty python code :)


- Till Toenshoff


On Aug. 19, 2019, 7:20 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71203/
> ---
> 
> (Updated Aug. 19, 2019, 7:20 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This check adds a script which validates that source files have valid
> license headers. This will allow us to reuse this functionality with
> e.g., the pre-commit tool.
> 
> At the moment the code added here is not invoked from
> `support/mesos-style.py` since it will be removed in a follow-up commit.
> 
> 
> Diffs
> -
> 
>   support/check-license.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71203/diff/5/
> 
> 
> Testing
> ---
> 
> * tested against files with license headers present or absent
> * tested against all Python and C++ source files in the repo
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 70096: Moved cpplint configuration into dedicated file.

2019-08-26 Thread Till Toenshoff via Review Board

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


Ship it!





bootstrap
Lines 84 (patched)


Too bad cpplint does not allow for hidden config files like e.g. 
`.cpplintrc` - but this code tells us; 
https://github.com/cpplint/cpplint/blob/master/cpplint.py#L6220-L6228


- Till Toenshoff


On Aug. 14, 2019, 11:25 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70096/
> ---
> 
> (Updated Aug. 14, 2019, 11:25 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With this change we not only reduce the amount of code in
> `support/mesos-style.py` in favor of a configuration supported by
> upstream, but we also make it easier to interoperate with editor
> integrations for cpplint.
> 
> 
> Diffs
> -
> 
>   bootstrap 7be2cc95c7ace55d710315427f38284cc5b2af4c 
>   bootstrap.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4 
>   support/CPPLINT.cfg PRE-CREATION 
>   support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b 
>   support/mesos-style.py cd490bd3deeb8477295bbad010f36d984ce17d27 
> 
> 
> Diff: https://reviews.apache.org/r/70096/diff/7/
> 
> 
> Testing
> ---
> 
> * confirmed that `./support/mesos-style.py src/executor/executor.cpp` still 
> does what is expected
> * no new warnings when running over the whole codebase
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 71338: Moved OpenSSL-related ifdef to a central location.

2019-08-21 Thread Till Toenshoff via Review Board

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


Fix it, then Ship it!




Thanks Benno for fixing my ugly patch.


3rdparty/libprocess/src/openssl.cpp
Lines 55 (patched)


As nicely noted by BenjaminB in my previous less beautiful RR, we might 
want to explain what version this was; "(OpenSSL 1.1.0)" as done above.


- Till Toenshoff


On Aug. 21, 2019, 1:34 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71338/
> ---
> 
> (Updated Aug. 21, 2019, 1:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A previous commit introduced an preprocessor directive that would
> split up code between brackets, confusing syntax highlighters and
> making the logic hard to read.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> 4aeab855b29837dc6bf93104afc62f111c38626c 
> 
> 
> Diff: https://reviews.apache.org/r/71338/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 71336: Fixed deprecation warning when building against OpenSSL 1.1.x.

2019-08-21 Thread Till Toenshoff via Review Board

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

(Updated Aug. 21, 2019, 11:45 a.m.)


Review request for mesos, Benno Evers and Benjamin Mahler.


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


Repository: mesos


Description
---

OpenSSL 1.1.x deprecates 'ASN1_STRING_get_data' and replaces it
by 'ASN1_STRING_get0_data'.


Diffs
-

  3rdparty/libprocess/src/openssl.cpp ee9a049d06bdf6365930aa92873dd97af197eba0 


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


Testing
---

```
cmake .. -DUNBUNDLED_ZOOKEEPER=ON -DUNBUNDLED_LIBEVENT=ON 
-DUNBUNDLED_LEVELDB=ON -DUNBUNDLED_LIBARCHIVE=ON -DENABLE_SSL=ON 
-DENABLE_LIBEVENT=ON -DOPENSSL_ROOT_DIR=/usr/local/opt/openssl@1.1 
-DLIBEVENT_ROOT_DIR=/Users/till/usr -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -G Ninja
ninja tests -j12
```


Thanks,

Till Toenshoff



Review Request 71336: Fixed deprecation warning when building against OpenSSL 1.1.x.

2019-08-21 Thread Till Toenshoff via Review Board

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

Review request for mesos, Benno Evers and Benjamin Mahler.


Repository: mesos


Description
---

OpenSSL 1.1.x deprecates 'ASN1_STRING_get_data' and replaces it
by 'ASN1_STRING_get0_data'.


Diffs
-

  3rdparty/libprocess/src/openssl.cpp ee9a049d06bdf6365930aa92873dd97af197eba0 


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


Testing
---

```
cmake .. -DUNBUNDLED_ZOOKEEPER=ON -DUNBUNDLED_LIBEVENT=ON 
-DUNBUNDLED_LEVELDB=ON -DUNBUNDLED_LIBARCHIVE=ON -DENABLE_SSL=ON 
-DENABLE_LIBEVENT=ON -DOPENSSL_ROOT_DIR=/usr/local/opt/openssl@1.1 
-DLIBEVENT_ROOT_DIR=/Users/till/usr -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -G Ninja
ninja tests -j12
```


Thanks,

Till Toenshoff



Re: Review Request 71232: Renamed cmake parameter for parallel test execution.

2019-08-02 Thread Till Toenshoff via Review Board

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


Fix it, then Ship it!





cmake/MesosConfigure.cmake
Line 49 (original), 49 (patched)


s/no/not/


- Till Toenshoff


On Aug. 2, 2019, 7:15 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71232/
> ---
> 
> (Updated Aug. 2, 2019, 7:15 a.m.)
> 
> 
> Review request for mesos, Benno Evers and Till Toenshoff.
> 
> 
> Bugs: MESOS-6382
> https://issues.apache.org/jira/browse/MESOS-6382
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Jenkins setup (which uses `support/docker-build.sh` under the
> covers) is parameterized with the  `CONFIGURATION` environment variable.
> While in we pass configure-style flags for both autotools and cmake
> builds to it in the Jenkins
> configuration, the script performs transformations so that
> configure-style flags are transformed to cmake-style (replace `_` with
> `-`, uppercase flags, replace `--` with `-D`).
> 
> We disable parallel test execution in Jenkins by passing
> `--disable-parallel-test-execution` which with the transformations in
> `support/docker-build.sh` leads to a cmake arg
> `-DDISABLE_PARALLEL_TEST_EXECUTION=1`. This patch renames the cmake arg
> from a default enabled `ENABLE_PARALLEL_TEST_EXECUTION` to a default
> disabled `DISABLE_PARALLEL_TEST_EXECUTION` to support this workflow.
> 
> 
> Diffs
> -
> 
>   cmake/MesosConfigure.cmake ca8d406e1be9d6ab50c6c9dc4d2de97189b17b9a 
> 
> 
> Diff: https://reviews.apache.org/r/71232/diff/1/
> 
> 
> Testing
> ---
> 
> Tested ctest behavior with both the new flag enabled and disabled.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Review Request 71138: Updated configure.ac to correct openssl/libevent setup.

2019-07-22 Thread Till Toenshoff via Review Board

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Fixes a problem introduced by configure.ac reordering in commit 4df2b62.


Diffs
-

  configure.ac 0e3058c2bcd730a0992ca497809ab09c58ed6fa1 


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


Testing
---

Tested within DC/OS build CI which failed without this fix.


Thanks,

Till Toenshoff



Re: Review Request 71123: Made openssl configuration in 'configure.ac' occur earlier.

2019-07-19 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On July 19, 2019, 10:49 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71123/
> ---
> 
> (Updated July 19, 2019, 10:49 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joseph Wu, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `AC_CHECK_LIBS()` call has a side effect of adding the
> found libraries to the linker command line by default. In
> addition, our custom configuration code also adds any
> non-default include and library search paths to `LDFLAGS`
> and `CPPFLAGS`.
> 
> Since many of our third-party dependencies depend themselves
> on openssl, the configuration check for openssl should be
> early during the configuration so that later libraries can
> be built against the same version of openssl that is used
> for the Mesos build itself.
> 
> According to the instructions given at
> 
>   https://www.gnu.org/software/autoconf/manual/autoconf-2.67
> /html_node/Libraries.html
> 
> reordering the calls to `AC_CHECK_LIB()` is a preferrable solution
> to passing in `-lssl -lcrypto` as the `other-libraries` argument.
> 
> 
> Diffs
> -
> 
>   configure.ac 4635bb38b6b65ea0454cfbde31b681a6ce232e10 
> 
> 
> Diff: https://reviews.apache.org/r/71123/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70991: Added ability to pass custom SSL context to `Socket::connect()`.

2019-07-04 Thread Till Toenshoff via Review Board


> On July 4, 2019, 11:53 p.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/src/openssl.cpp
> > Line 806 (original), 807 (patched)
> > 
> >
> > s/libprocess/legacy/

Actually, please make sure you dont end up with 
```
// i.e. the lagacy 'legacy' scheme
```
:D


- Till


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


On July 4, 2019, 7 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70991/
> ---
> 
> (Updated July 4, 2019, 7 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-9878
> https://issues.apache.org/jira/browse/MESOS-9878
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Users of libprocess can now pass a custom SSL context when
> connecting a generic socket via the `Socket::connect()`
> function.
> 
> Additionally the API of `Socket::connect()` was also reworked
> according to the following boundary conditions requested by
> libprocess maintainers:
> 
>  * When libprocess is compiled without SSL support, neither the
>declaration of the TLS configuration object nor the `connnect()`
>overload that accepts the TLS configuration should be available.
>  * Passing just the servername is not an acceptable short-hand for
>using the default TLS configuration together with that servername.
>  * When the incorrect overload is selected (i.e. passing TLS config
>to a poll socket or omitting TLS configuration for a TLS socket),
>the program should abort.
> 
> This following changes are introduced according to the requirements
> above:
> 
>  * A new class `openssl::TLSClientConfig` is introduced when libprocess
>is compiled with ssl support.
>  * A new overload
>`Socket::connect(const Address&, const TLSClientConfig&)` is
>introduced when libprocess is compiled with ssl support.
>  * All call sites are adjusted to check the socket kind before calling
>`connect()`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> 1ddcc2d5a30f7bf3914138e497a9b228b515cd29 
>   3rdparty/libprocess/include/process/socket.hpp 
> 4f0f6e9aa6e95e826e3de96e518a7200ad7a8f83 
>   3rdparty/libprocess/include/process/ssl/tls_config.hpp PRE-CREATION 
>   3rdparty/libprocess/src/http.cpp 3e73ee936f5c6329f41704a179f3d88ab65dfb6d 
>   3rdparty/libprocess/src/openssl.hpp 
> 17bec246e516261f8d772f1647c17f092fae82d1 
>   3rdparty/libprocess/src/openssl.cpp 
> 19d25a89f7dda1f6c66dd1ffc5051e35457d26b0 
>   3rdparty/libprocess/src/poll_socket.hpp 
> 15b7902ba2b10fad63e2ba7b8d5081d4b9e2d1c7 
>   3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp 
> 6ef5a86566af3439cfe0b06ab3576076623f7be0 
>   3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 
> 7e2229a9ed815727500bd457356e5531607fa6cf 
>   3rdparty/libprocess/src/posix/poll_socket.cpp 
> 74acb6942682a9d9626df81b303eba0a1c24ecf7 
>   3rdparty/libprocess/src/process.cpp 
> 799666f03d6a78708aa9336c2dd04bc9b5023aa0 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 4d372943a2d417d24d06444ec2e72909fb348017 
>   3rdparty/libprocess/src/tests/socket_tests.cpp 
> b09ae23a551c6587656b2d5f6f58c5267e8e0088 
>   3rdparty/libprocess/src/tests/ssl_client.cpp 
> de87b3b89c84d17f2ebba1f09e9ec682f139aace 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 5d360221937e68da185754f0633fa41a217c7107 
> 
> 
> Diff: https://reviews.apache.org/r/70991/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70991: Added ability to pass custom SSL context to `Socket::connect()`.

2019-07-04 Thread Till Toenshoff via Review Board

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


Fix it, then Ship it!




Awesome work Benno - thanks!

Can't wait to see these great additions landing.


3rdparty/libprocess/include/process/ssl/tls_config.hpp
Lines 59 (patched)


```
 (See also the comments on the typedefs above.)
```

Looking at the callback type comments is somewhat implicit, i feel - we can 
do without that part.



3rdparty/libprocess/include/process/ssl/tls_config.hpp
Lines 65-69 (patched)


I would suggest to split this up into smaller digestable sentences - native 
speaker please?!

Otherwise maybe like this?

```
// Returns `TLSClientConfig` object configured with provided `servername` as
// well as the global libprocess SSL context. The callbacks for `verify` and
// `configure_socket` get setup following the SSL behaviour configured via
// the `LIBPROCESS_SSL_*` environment variables.

```



3rdparty/libprocess/include/process/ssl/tls_config.hpp
Lines 80 (patched)


s/libprocess/legacy/



3rdparty/libprocess/src/openssl.cpp
Line 806 (original), 807 (patched)


s/libprocess/legacy/


- Till Toenshoff


On July 4, 2019, 7 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70991/
> ---
> 
> (Updated July 4, 2019, 7 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-9878
> https://issues.apache.org/jira/browse/MESOS-9878
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Users of libprocess can now pass a custom SSL context when
> connecting a generic socket via the `Socket::connect()`
> function.
> 
> Additionally the API of `Socket::connect()` was also reworked
> according to the following boundary conditions requested by
> libprocess maintainers:
> 
>  * When libprocess is compiled without SSL support, neither the
>declaration of the TLS configuration object nor the `connnect()`
>overload that accepts the TLS configuration should be available.
>  * Passing just the servername is not an acceptable short-hand for
>using the default TLS configuration together with that servername.
>  * When the incorrect overload is selected (i.e. passing TLS config
>to a poll socket or omitting TLS configuration for a TLS socket),
>the program should abort.
> 
> This following changes are introduced according to the requirements
> above:
> 
>  * A new class `openssl::TLSClientConfig` is introduced when libprocess
>is compiled with ssl support.
>  * A new overload
>`Socket::connect(const Address&, const TLSClientConfig&)` is
>introduced when libprocess is compiled with ssl support.
>  * All call sites are adjusted to check the socket kind before calling
>`connect()`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> 1ddcc2d5a30f7bf3914138e497a9b228b515cd29 
>   3rdparty/libprocess/include/process/socket.hpp 
> 4f0f6e9aa6e95e826e3de96e518a7200ad7a8f83 
>   3rdparty/libprocess/include/process/ssl/tls_config.hpp PRE-CREATION 
>   3rdparty/libprocess/src/http.cpp 3e73ee936f5c6329f41704a179f3d88ab65dfb6d 
>   3rdparty/libprocess/src/openssl.hpp 
> 17bec246e516261f8d772f1647c17f092fae82d1 
>   3rdparty/libprocess/src/openssl.cpp 
> 19d25a89f7dda1f6c66dd1ffc5051e35457d26b0 
>   3rdparty/libprocess/src/poll_socket.hpp 
> 15b7902ba2b10fad63e2ba7b8d5081d4b9e2d1c7 
>   3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp 
> 6ef5a86566af3439cfe0b06ab3576076623f7be0 
>   3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 
> 7e2229a9ed815727500bd457356e5531607fa6cf 
>   3rdparty/libprocess/src/posix/poll_socket.cpp 
> 74acb6942682a9d9626df81b303eba0a1c24ecf7 
>   3rdparty/libprocess/src/process.cpp 
> 799666f03d6a78708aa9336c2dd04bc9b5023aa0 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 4d372943a2d417d24d06444ec2e72909fb348017 
>   3rdparty/libprocess/src/tests/socket_tests.cpp 
> b09ae23a551c6587656b2d5f6f58c5267e8e0088 
>   3rdparty/libprocess/src/tests/ssl_client.cpp 
> de87b3b89c84d17f2ebba1f09e9ec682f139aace 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 5d360221937e68da185754f0633fa41a217c7107 
> 
> 
> Diff: https://reviews.apache.org/r/70991/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70991: Updated `Socket::connect()` API according to maintainer feedback.

2019-07-04 Thread Till Toenshoff via Review Board

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




3rdparty/libprocess/src/poll_socket.hpp
Lines 16-18 (patched)


No need for #ifdef.



3rdparty/libprocess/src/poll_socket.hpp
Lines 16-18 (patched)


No need for #ifdef.


- Till Toenshoff


On July 2, 2019, 5:50 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70991/
> ---
> 
> (Updated July 2, 2019, 5:50 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-9878
> https://issues.apache.org/jira/browse/MESOS-9878
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reworked the API of `Socket::connect()` according to the following
> boundary conditions requested by libprocess maintainers:
> 
>  * It shall be possible to use custom connection options when
>connecting with a SSL socket.
>  * When libprocess is compiled without SSL support, neither the
>declaration of the TLS configuration object nor the `connnect()`
>overload that accepts the TLS configuration should be available.
>  * Passing just the servername is not an acceptable short-hand for
>using the default TLS configuration together with that servername.
>  * When the incorrect overload is selected (i.e. passing TLS config
>to a poll socket or omitting TLS configuration for a TLS socket),
>the program should abort.
> 
> This commit changes the API of `Socket::connect()` according to the
> requirements above. In particular:
> 
>  * A new class `openssl::TLSClientConfig` is introduced when libprocess
>is compiled with ssl support.
>  * A new overload
>`Socket::connect(const Address&, const TLSClientConfig&)` is
>introduced when libprocess is compiled with ssl support.
>  * All call sites are adjusted to check the socket kind before calling
>`connect()`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> 1ddcc2d5a30f7bf3914138e497a9b228b515cd29 
>   3rdparty/libprocess/include/process/socket.hpp 
> 4f0f6e9aa6e95e826e3de96e518a7200ad7a8f83 
>   3rdparty/libprocess/include/process/ssl/tls_config.hpp PRE-CREATION 
>   3rdparty/libprocess/src/http.cpp 3e73ee936f5c6329f41704a179f3d88ab65dfb6d 
>   3rdparty/libprocess/src/openssl.hpp 
> 17bec246e516261f8d772f1647c17f092fae82d1 
>   3rdparty/libprocess/src/openssl.cpp 
> 19d25a89f7dda1f6c66dd1ffc5051e35457d26b0 
>   3rdparty/libprocess/src/poll_socket.hpp 
> 15b7902ba2b10fad63e2ba7b8d5081d4b9e2d1c7 
>   3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp 
> 6ef5a86566af3439cfe0b06ab3576076623f7be0 
>   3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 
> 7e2229a9ed815727500bd457356e5531607fa6cf 
>   3rdparty/libprocess/src/posix/poll_socket.cpp 
> 74acb6942682a9d9626df81b303eba0a1c24ecf7 
>   3rdparty/libprocess/src/process.cpp 
> 799666f03d6a78708aa9336c2dd04bc9b5023aa0 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 4d372943a2d417d24d06444ec2e72909fb348017 
>   3rdparty/libprocess/src/tests/socket_tests.cpp 
> b09ae23a551c6587656b2d5f6f58c5267e8e0088 
>   3rdparty/libprocess/src/tests/ssl_client.cpp 
> de87b3b89c84d17f2ebba1f09e9ec682f139aace 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 5d360221937e68da185754f0633fa41a217c7107 
> 
> 
> Diff: https://reviews.apache.org/r/70991/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70921: Added OpenSSL-related changes to CHANGELOG.

2019-07-04 Thread Till Toenshoff via Review Board

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


Fix it, then Ship it!




Ship It!


CHANGELOG
Lines 9 (patched)


s/should/will/



CHANGELOG
Lines 27 (patched)


How about:

... will now forego TLS certificate verification ...



CHANGELOG
Lines 27 (patched)


How about:

... will now forego TLS certificate verification ...



docs/upgrades.md
Lines 516 (patched)


s/change/configure/



docs/upgrades.md
Lines 526 (patched)


How about?
```  * For incoming connections, certificates are not verified unless 
`LIBPROCESS_SSL_REQUIRE_CERT` is set to true.
```



docs/upgrades.md
Lines 527 (patched)


This is really hard to parse but right now I am unable to come up with 
something easier for the eye -- lets try to improve this with some native 
speaker help.


- Till Toenshoff


On June 21, 2019, 3:02 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70921/
> ---
> 
> (Updated June 21, 2019, 3:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joseph Wu, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added OpenSSL-related changes to CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG ecd01d6f415859eb6b9cfa3468db02d1810a3998 
>   docs/upgrades.md 4a818df4993093770b49efb675b8078c42144241 
> 
> 
> Diff: https://reviews.apache.org/r/70921/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70993: Added warnings about known problems with libevent epoll backend.

2019-07-04 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On July 2, 2019, 5:53 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70993/
> ---
> 
> (Updated July 2, 2019, 5:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
> 
> 
> Bugs: MESOS-9867
> https://issues.apache.org/jira/browse/MESOS-9867
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some SSL options are known to cause issues in combination with
> older versions of libevent. Detect and warn about this situation.
> 
> See MESOS-9867 for details.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 851a842114bb19abb00a318c85824067d68d71f6 
>   3rdparty/libprocess/src/openssl.cpp 
> 19d25a89f7dda1f6c66dd1ffc5051e35457d26b0 
> 
> 
> Diff: https://reviews.apache.org/r/70993/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70933: Moved an inline duration for slow DNS logging into a const variable.

2019-07-04 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On June 24, 2019, 12:22 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70933/
> ---
> 
> (Updated June 24, 2019, 12:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved the 100ms threshold that was used for printing warning
> messages about slow reverse DNS lookups into a named variable.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> 19d25a89f7dda1f6c66dd1ffc5051e35457d26b0 
> 
> 
> Diff: https://reviews.apache.org/r/70933/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70797: Added unit tests for hostname validation.

2019-07-04 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On July 2, 2019, 5:53 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70797/
> ---
> 
> (Updated July 2, 2019, 5:53 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Joseph Wu, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adds some unit tests to verify the newly added 
> `hostname_validation_scheme` flag is working as intended.
> 
> While going through the existing tests to look for candidates
> that would benefit from being tested for both hostname validation
> schemes, I noticed a number of existing tests where test setup
> did not quite match the comment or test name. I fixed these up
> in this review as well.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> 6cdd7815f4389cd398defe56260a73eb710a4d8f 
>   3rdparty/libprocess/src/tests/ssl_client.cpp 
> de87b3b89c84d17f2ebba1f09e9ec682f139aace 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 5d360221937e68da185754f0633fa41a217c7107 
> 
> 
> Diff: https://reviews.apache.org/r/70797/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70797: Added unit tests for hostname validation.

2019-07-04 Thread Till Toenshoff via Review Board

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




3rdparty/libprocess/src/tests/ssl_tests.cpp
Line 318 (original), 368 (patched)


s/incorrect/not matching/


- Till Toenshoff


On July 2, 2019, 5:53 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70797/
> ---
> 
> (Updated July 2, 2019, 5:53 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Joseph Wu, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adds some unit tests to verify the newly added 
> `hostname_validation_scheme` flag is working as intended.
> 
> While going through the existing tests to look for candidates
> that would benefit from being tested for both hostname validation
> schemes, I noticed a number of existing tests where test setup
> did not quite match the comment or test name. I fixed these up
> in this review as well.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> 6cdd7815f4389cd398defe56260a73eb710a4d8f 
>   3rdparty/libprocess/src/tests/ssl_client.cpp 
> de87b3b89c84d17f2ebba1f09e9ec682f139aace 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 5d360221937e68da185754f0633fa41a217c7107 
> 
> 
> Diff: https://reviews.apache.org/r/70797/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70992: Recorded Socket API change in CHANGELOG.

2019-07-04 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On July 4, 2019, 2:28 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70992/
> ---
> 
> (Updated July 4, 2019, 2:28 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joseph Wu, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Recorded Socket API change in CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG ecd01d6f415859eb6b9cfa3468db02d1810a3998 
> 
> 
> Diff: https://reviews.apache.org/r/70992/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70795: Updated SSL docs to include new libprocess flag.

2019-07-01 Thread Till Toenshoff via Review Board


> On June 21, 2019, 1:34 a.m., Till Toenshoff wrote:
> > docs/ssl.md
> > Lines 194 (patched)
> > 
> >
> > I wonder if we should already start a deprecation of the `libprocess` 
> > scheme - that would be:
> > - announcing that `openssl` will be standard soon on compatible boxes
> > - announcing it to be gone at some point
> > 
> > Or am I too eager for unification here?
> 
> Benno Evers wrote:
> It's actually a pretty big change - the 'libprocess' behaviour was built, 
> I assume, to "magically" work with normal certificates w/o IP addresses 
> despite libprocess only knowing about IP addresses. In DC/OS we don't notice 
> most of it, since there all our certificates *do* contain the correct IP 
> address, but at least quite a few unit tests will break by switching the 
> default.
> 
> So I actually agree we should do this deprecation, but I'm not sure about 
> the timeline.
> 
> Benno Evers wrote:
> Created https://issues.apache.org/jira/browse/MESOS-9857 to track the 
> change.

Great - next we would update all relevant documentation with a deprecation note 
and a reference of that ticket.

Right now I am contemplating doing this in a single run, right away, instead of 
multiple phases. Multiple phases would  which would allow us to have that 
`libprocess` default without having to warn about it. 

What do you think?

We would ...
- add a comment note here
- add an SSL flags description note
- possibly have the flags validation output a deprecation warning
- anything I forgot here?


- Till


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


On June 21, 2019, 3:05 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70795/
> ---
> 
> (Updated June 21, 2019, 3:05 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Jan-Philip 
> Gehrcke, Joseph Wu, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a description of the new `--hostname_validation_scheme` flag
> and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_SCHEME`
> environment variable.
> 
> 
> Diffs
> -
> 
>   docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 
> 
> 
> Diff: https://reviews.apache.org/r/70795/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70948: Attempting to get Jenkins pipeline build expiry fixed.

2019-06-25 Thread Till Toenshoff via Review Board

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

(Updated June 26, 2019, 12:16 a.m.)


Review request for mesos, Benjamin Bannier, Gilbert Song, and Vinod Kone.


Repository: mesos


Description
---

Attempting to get Jenkins pipeline build expiry fixed.


Diffs (updated)
-

  support/jenkins/Jenkinsfile-packaging-centos 
aa8f37f6cd661e767e55ca75095a91827ebedb21 


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

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


Testing
---

none - cannot test this without committing it right now

info for the option in use is taken from: 
https://stackoverflow.com/a/53141684/91282


Thanks,

Till Toenshoff



Review Request 70948: Attempting to get Jenkins pipeline build expiry fixed.

2019-06-25 Thread Till Toenshoff via Review Board

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

Review request for mesos, Benjamin Bannier, Gilbert Song, and Vinod Kone.


Repository: mesos


Description
---

Attempting to get Jenkins pipeline build expiry fixed.


Diffs
-

  support/jenkins/Jenkinsfile-packaging-centos 
aa8f37f6cd661e767e55ca75095a91827ebedb21 


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


Testing
---

none - cannot test this without committing it right now

info for the option in use is taken from: 
https://stackoverflow.com/a/53141684/91282


Thanks,

Till Toenshoff



Re: Review Request 70795: Updated SSL docs to include new libprocess flag.

2019-06-20 Thread Till Toenshoff via Review Board

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


Fix it, then Ship it!




Great! 

Please don't forget to update the review description to match the changed 
naming.


docs/ssl.md
Lines 176 (patched)


s/can not/cannot/



docs/ssl.md
Lines 194 (patched)


I wonder if we should already start a deprecation of the `libprocess` 
scheme - that would be:
- announcing that `openssl` will be standard soon on compatible boxes
- announcing it to be gone at some point

Or am I too eager for unification here?


- Till Toenshoff


On June 20, 2019, 5:27 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70795/
> ---
> 
> (Updated June 20, 2019, 5:27 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a description of the new `--hostname_validation_algorithm` flag
> and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_ALGORITHM`
> environment variable.
> 
> 
> Diffs
> -
> 
>   docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 
> 
> 
> Diff: https://reviews.apache.org/r/70795/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70749: Introduced optional new scheme for hostname validation.

2019-06-20 Thread Till Toenshoff via Review Board

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




3rdparty/libprocess/src/openssl.cpp
Lines 147 (patched)


s/algorithm/scheme/?



3rdparty/libprocess/src/openssl.cpp
Lines 153 (patched)


s/algorithm/scheme/?

Also, we should tell the user which version he needs at least here, nice 
touch ;)



3rdparty/libprocess/src/openssl.cpp
Lines 565-566 (patched)


Should we 
```
EXIT(EXIT_FAILURE)
 << "Unknown value for hostname_validation_scheme: "
   << ssl_flags->hostname_validation_scheme;  
```
instead here and below to be perfectly explicit and consistent?



3rdparty/libprocess/src/openssl.cpp
Lines 576 (patched)


s/algorithm/scheme/?



3rdparty/libprocess/src/openssl.cpp
Line 755 (original), 788 (patched)


Maybe
```
// When using the OpenSSL built-in scheme,...
```



3rdparty/libprocess/src/openssl.cpp
Lines 795 (patched)


s/algorithm/scheme/



3rdparty/libprocess/src/openssl.cpp
Lines 808 (patched)


I know you just moved it, but where do these 100ms come from and how could 
we be more explicit about that choice?

I would suggest to use a const with some explaining comment how that value 
was chosen - can we please? :D



3rdparty/libprocess/src/openssl.cpp
Lines 984-985 (patched)


This sounds like a great idea worth spending 1 more cycle on -- can we 
create and reference a ticket that explains this jazz as nicely as we do here 
in the code?

My thought is that being open about this idea in JIRA,  we would provide 
more chances of getting community support for it.



3rdparty/libprocess/src/openssl.cpp
Lines 989 (patched)


Shall we explain why this is the `right` way - aka best practices?



3rdparty/libprocess/src/openssl.cpp
Lines 1016 (patched)


s/ip/IP/



3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp
Lines 444-448 (patched)


Great comment - thanks!



3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp
Lines 569 (patched)


Now that we log the `peer_hostname`, I wonder if it made sense to also log 
the `peer_ip` like this - what do you think? If you agree, then we should even 
try to render a single log-line out of those two - it will be noisy enough on 
level 2.



3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp
Lines 1129 (patched)


I feel `peername` comes in a bit surprising here if one does not know about 
`getpeername()`.

Maybe s/peername/IP address/?



3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp
Lines 1136 (patched)


As mentioned in some other place of this chain, let's add a neat JIRA on 
this plan.


- Till Toenshoff


On June 20, 2019, 5:48 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70749/
> ---
> 
> (Updated June 20, 2019, 5:48 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-9809
> https://issues.apache.org/jira/browse/MESOS-9809
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit introduces a new libprocess SSL flag
> `hostname_validation_scheme`, which can be used to select
> between the previous hostname validation behaviour and a new
> option to use standardized OpenSSL algorithms to handle
> hostname validation as part of the handshake.
> 
> As a nice side-effect, the new scheme gets rid of reverse DNS
> lookups during TLS connection establishment, which used to be
> a common source of hard-to-debug unresponsiveness in Mesos
> components.
> 
> See `docs/ssl.md` in the follow-up commit for details of and
> differences between the schemes.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/flags.hpp 
> f3483f97f93bb29117b2c78f0f2ed9735d9c4b3a 
>   3rdparty/libprocess/src/openssl.hpp 
> 17bec246e516261f8d772f1647c17f092fae82d1 
>   3rdparty/libprocess/src/openssl.cpp 
> 

Re: Review Request 70796: Fixed Mesos unit tests after Address API change.

2019-06-20 Thread Till Toenshoff via Review Board

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


Ship it!




- Till Toenshoff


On June 19, 2019, 2:44 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70796/
> ---
> 
> (Updated June 19, 2019, 2:44 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joseph Wu, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adjusts some Mesos unit tests after renaming
> `Address::hostname()` to `Address::lookup_hostname()`.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 097f1b77a59e29c6690210773d1556ebf2bb701e 
> 
> 
> Diff: https://reviews.apache.org/r/70796/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70885: Renamed 'libprocess::network::Address::hostname()'.

2019-06-20 Thread Till Toenshoff via Review Board

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


Ship it!




Thanks a bunch - much better signal for a pricy function now :)

- Till Toenshoff


On June 19, 2019, 2:44 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70885/
> ---
> 
> (Updated June 19, 2019, 2:44 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed member function `hostname()` to `lookup_hostname()`,
> since the former name hides the fact that a call to this
> function involves a synchronous network access in order
> to make a reverse DNS lookup.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/address.hpp 
> e740e840c38381bafd7a1a7fcde5f963832ac1fb 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 97aaf3ed3d4fab6d717d5c9b6d12402562ac6b46 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 6b8496aeeed79ae1bd39d7013f4f403b248fdd4c 
> 
> 
> Diff: https://reviews.apache.org/r/70885/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70883: Added optional 'peer_hostname' argument to Socket::connect().

2019-06-20 Thread Till Toenshoff via Review Board

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


Ship it!




We have discussed this patch a bit out of band and agreed that passing the 
`peer_hostname` as an additional argument into the `process::connect` function 
and socket stack below was the most sensible we can do. Alternatives like 
passing as a new `network::Address` member has surprising 
conversion/typecasting results while exploiting the idea of a network address. 
Adding a setter to our `Socket` might be another option but here we would have 
to supply a noop member of the non SSL variant. With both variants we would 
have widened the scope of this argument to areas where it does not play a role.


3rdparty/libprocess/include/process/socket.hpp
Lines 371 (patched)


Too bad we can't default.


- Till Toenshoff


On June 19, 2019, 2:34 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70883/
> ---
> 
> (Updated June 19, 2019, 2:34 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Socket::connect() function now takes an optional string
> as an additional argument. This is to prepare support for proper
> TLS hostname validation.
> 
> With TCP, a connection is always made to a specific IP address,
> with the hostname just serving as an artifact to help humans
> remember that address.
> 
> With TLS, the roles are switched: A connection is made to a
> specific hostname (which is recorded in a TLS certificate),
> with the IP address just being a network-layer artifact to
> help packets route to that hostname.
> 
> Therefore, a connecting TLS socket must be aware of the
> hostname it is supposed to connect to.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 029605eaff72e80206cb7dfd64c2f898084155e0 
>   3rdparty/libprocess/include/process/socket.hpp 
> 4f0f6e9aa6e95e826e3de96e518a7200ad7a8f83 
>   3rdparty/libprocess/src/http.cpp 3e73ee936f5c6329f41704a179f3d88ab65dfb6d 
>   3rdparty/libprocess/src/poll_socket.hpp 
> 15b7902ba2b10fad63e2ba7b8d5081d4b9e2d1c7 
>   3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp 
> 6ef5a86566af3439cfe0b06ab3576076623f7be0 
>   3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 
> 29a1bf71c1df9d80370455a6269ecea0ec4193b0 
>   3rdparty/libprocess/src/posix/poll_socket.cpp 
> 74acb6942682a9d9626df81b303eba0a1c24ecf7 
>   3rdparty/libprocess/src/windows/poll_socket.cpp 
> 565b0088dc2b270193e615655f57f48419eb2c12 
> 
> 
> Diff: https://reviews.apache.org/r/70883/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70810: Updated SSL docs with suggested runtime configuration.

2019-06-20 Thread Till Toenshoff via Review Board

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


Ship it!




Awesome - thanks!

- Till Toenshoff


On June 20, 2019, 5:32 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70810/
> ---
> 
> (Updated June 20, 2019, 5:32 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-9791
> https://issues.apache.org/jira/browse/MESOS-9791
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated SSL docs with suggested runtime configuration.
> 
> 
> Diffs
> -
> 
>   docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 
> 
> 
> Diff: https://reviews.apache.org/r/70810/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70748: Changed semantics of some libprocess TLS flags.

2019-06-20 Thread Till Toenshoff via Review Board

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


Fix it, then Ship it!




Great - thanks so much!


3rdparty/libprocess/src/openssl.cpp
Line 90 (original), 90 (patched)


`verify_cert` effectively becomes a TLS client mode specific setting - we 
should explain that here. In my mind, in TLS server mode, `verify_cert` stricly 
follows `require_cert` in its state.



3rdparty/libprocess/src/openssl.cpp
Line 95 (original), 95 (patched)


This is a TLS server mode only setting by now - we need to call that out I 
feel. In TLS client mode, we have `require_cert` enabled implicitly.



3rdparty/libprocess/src/openssl.cpp
Line 96 (original), 96 (patched)


Additional to the updates above, we also need to call this out in the 
CHANGELOG. Can you please include that in this chain?



3rdparty/libprocess/src/openssl.cpp
Lines 743-745 (patched)


This is so important, I would like to add more meat to it;

Let's clarify that the OpenSSL handshake itself will do this, before we 
even reach our additional verification. Also, the use of `remote` confuses me - 
do local servers have an exception here? 

Maybe something like this?
```
  // NOTE: Even without this check, the OpenSSL handshake will
  // not complete when connecting to servers that do not present
  // a certificate, unless an anonymous cipher is used.
```



3rdparty/libprocess/src/tests/ssl_tests.cpp
Lines 296-297 (patched)


Nice, thank you!


- Till Toenshoff


On June 19, 2019, 2:28 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70748/
> ---
> 
> (Updated June 19, 2019, 2:28 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, 
> and Till Toenshoff.
> 
> 
> Bugs: MESOS-9810
> https://issues.apache.org/jira/browse/MESOS-9810
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit slightly updates the semants of the
> `LIBPROCESS_SSL_VERIFY_CERT` and `LIBPROCESS_SSL_REQUIRE_CERT`
> environment variables. The former now only applies to connections
> in client mode and the latter now only applies to connections in
> server mode.
> 
> In particular, in TLS server mode we now *only* verify client
> certificates when `LIBPROCESS_SSL_REQUIRE_CERT` is set to `true`,
> regardless of the value of `LIBPROCESS_SSL_VERIFY_CERT`.
> 
> In addition, when in SSL client mode and  `LIBPROCESS_SSL_VERIFY_CERT`
> has been set to `true`, enforce that the server actually presents a
> certificate that can be verified. Note that this is expected to be
> not a behavioural change in practice, since the TLS specification
> already states that a server MUST always send a certificate unless an
> anonymous cipher is used, and most TLS ciphersuites are configured to
> exclude anonymous ciphers.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.hpp 
> 17bec246e516261f8d772f1647c17f092fae82d1 
>   3rdparty/libprocess/src/openssl.cpp 
> e7dbd67913fa8e7fbbf60dee428e7e38895f86ce 
>   3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 
> 29a1bf71c1df9d80370455a6269ecea0ec4193b0 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 6b8496aeeed79ae1bd39d7013f4f403b248fdd4c 
> 
> 
> Diff: https://reviews.apache.org/r/70748/diff/4/
> 
> 
> Testing
> ---
> 
> So far, mostly manual testing.
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70747: Updated some flag description for libprocess SSL flags.

2019-06-19 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On May 29, 2019, 1:50 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70747/
> ---
> 
> (Updated May 29, 2019, 1:50 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed `SSL` to `TLS` in some flag descriptions where the
> flag was referring to versions of the TLS protocol.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> e7dbd67913fa8e7fbbf60dee428e7e38895f86ce 
> 
> 
> Diff: https://reviews.apache.org/r/70747/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70748: Disallow verification of empty TLS server certificates.

2019-06-13 Thread Till Toenshoff via Review Board

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




3rdparty/libprocess/src/openssl.cpp
Line 738 (original), 745 (patched)


This means that when a certificate gets provided, we always validate it. 
Entirely straightforward but may be too much of a potential problem source in 
the real world, I fear. 

IIUC then client certificate validation is exactly a non goal of this 
review-chain as client certificates in real world scenarios often times do not 
match. My feeling is that we have a great chance of reducing potential problem 
sources by working more like this;

CLIENT mode (we initiate a connection)
- when we receive a peer certificate, we verify it
- when we receive no peer certificate, we error out
SERVER mode (we accept a connection)
- `require` is set to false
  - when we receive a peer certificate, we ignore it
  - when we receive no peer certificate, we ignore it

We would return early as done for verify above like this;
```
  if (mode == Mode::SERVER && !ssl_flags->require_cert) {
return Nothing();
  }
```

Then relax this part towards;
```
  if (cert == nullptr) {
return Error("Peer did not provide certificate");
  }
```


- Till Toenshoff


On June 6, 2019, 11:12 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70748/
> ---
> 
> (Updated June 6, 2019, 11:12 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, 
> and Till Toenshoff.
> 
> 
> Bugs: MESOS-9810
> https://issues.apache.org/jira/browse/MESOS-9810
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When in SSL client mode and `LIBPROCESS_SSL_VERIFY_CERT=true` has
> been set, enforce that the server actually presents a certificate
> that can be verified.
> 
> Note that in most cases, the TLS stack would rejected the connection
> before the code ever reaches `openssl::verify()`, since the TLS
> specification that a server MUST always send a certificate unless
> an anonymous cipher is used.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.hpp 
> 17bec246e516261f8d772f1647c17f092fae82d1 
>   3rdparty/libprocess/src/openssl.cpp 
> e7dbd67913fa8e7fbbf60dee428e7e38895f86ce 
>   3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 
> 29a1bf71c1df9d80370455a6269ecea0ec4193b0 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 6b8496aeeed79ae1bd39d7013f4f403b248fdd4c 
> 
> 
> Diff: https://reviews.apache.org/r/70748/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70811: Fixed compilation error on Mac OS.

2019-06-11 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On June 7, 2019, 5:21 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70811/
> ---
> 
> (Updated June 7, 2019, 5:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gilbert Song, James Peach, and 
> Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds missing switch case to fix compilation error introduced
> in `bc5a57122635`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> a69a68823842171bb15ddf34a504e5ce4af232b0 
> 
> 
> Diff: https://reviews.apache.org/r/70811/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 70811: Fixed compilation error on Mac OS.

2019-06-11 Thread Till Toenshoff via Review Board

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




src/slave/containerizer/mesos/launch.cpp
Line 508 (original), 509 (patched)


I am 100% with BenB here - silently ignoring seems a ticket for hard to 
explain / debug problems. Let's please return an `Error` suggesting that this 
setting was not supported on macOS instead.


- Till Toenshoff


On June 7, 2019, 5:21 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70811/
> ---
> 
> (Updated June 7, 2019, 5:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gilbert Song, James Peach, and 
> Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds missing switch case to fix compilation error introduced
> in `bc5a57122635`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> a69a68823842171bb15ddf34a504e5ce4af232b0 
> 
> 
> Diff: https://reviews.apache.org/r/70811/diff/1/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Review Request 70829: Added logrotate option to force Jenkins into removing older artefacts.

2019-06-11 Thread Till Toenshoff via Review Board

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

Review request for mesos, Benjamin Bannier and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  support/jenkins/Jenkinsfile-packaging-centos 
de688f45f6530c48f13930cbd78f772873a1bc84 


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


Testing
---

Untested but based on documentation and hope: 
https://support.cloudbees.com/hc/en-us/articles/115000237071-How-do-I-set-discard-old-builds-for-a-Multi-Branch-Pipeline-Job-

This should remove old logfiles and artefacts after 30 days.


Thanks,

Till Toenshoff



Re: Review Request 70562: Add LIBPROCESS_SSL_ENABLE_TLS_V1_3

2019-04-29 Thread Till Toenshoff via Review Board

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


Ship it!




Thanks so much Stephane. Looks slick -- Benno will go ahead and land this with 
the small fix he mentioned already (thanks Benno :))

- Till Toenshoff


On April 29, 2019, 6:43 a.m., Stéphane Cottin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70562/
> ---
> 
> (Updated April 29, 2019, 6:43 a.m.)
> 
> 
> Review request for mesos and Benno Evers.
> 
> 
> Bugs: MESOS-9730
> https://issues.apache.org/jira/browse/MESOS-9730
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When building mesos with libopenssl >= 1.1.1, TLS1.3 is enabled by default. 
> This causes major communication issues between executors and agents.
> 
> This patch adds new `LIBPROCESS_SSL_ENABLE_TLS_V1_3` env var, disabled by 
> default.
> Should be changed to enabled by default when full openssl >= 1.1 support will 
> land.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/flags.hpp 3806266fb 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp e173b3224 
>   3rdparty/libprocess/src/openssl.hpp 0c4192f90 
>   3rdparty/libprocess/src/openssl.cpp a4d503642 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 5e994498d 
> 
> 
> Diff: https://reviews.apache.org/r/70562/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Stéphane Cottin
> 
>



Re: Review Request 70563: document LIBPROCESS_SSL_ENABLE_TLS_V1_3

2019-04-29 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On April 29, 2019, 6:43 a.m., Stéphane Cottin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70563/
> ---
> 
> (Updated April 29, 2019, 6:43 a.m.)
> 
> 
> Review request for mesos and Benno Evers.
> 
> 
> Bugs: MESOS-9730
> https://issues.apache.org/jira/browse/MESOS-9730
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> update documentation about `LIBPROCESS_SSL_ENABLE_TLS_V1_3` and TLS1.3
> 
> 
> Diffs
> -
> 
>   docs/ssl.md e6c118121 
> 
> 
> Diff: https://reviews.apache.org/r/70563/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Stéphane Cottin
> 
>



Re: Review Request 70526: Rearranged 'Downloads' page and updated bintray URL.

2019-04-24 Thread Till Toenshoff via Review Board

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


Ship it!




As discussed out of band, we will try to preserve all the older releases when 
transitioning to the new URL - thanks a bunch Benno!

- Till Toenshoff


On April 23, 2019, 2:32 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70526/
> ---
> 
> (Updated April 23, 2019, 2:32 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Till Toenshoff.
> 
> 
> Bugs: MESOS-9697
> https://issues.apache.org/jira/browse/MESOS-9697
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the bintray link on the `Downloads` page to point
> to the `apache/mesos` account instead of the `mesos`
> account.
> 
> In addition, several minor formatting changes were done:
> 
>   * Added a space after the colon in the `Getting older
> Mesos binaries` section.
> 
>   * Moved links to the getting started guide to the top
> of the document.
> 
>   * Used a list to present the download links to the latest
> stable release.
> 
>   * Used `` instead of `` tags for the link to the
> ASF git repository.
> 
> 
> Diffs
> -
> 
>   site/source/downloads.html.erb d5a165c71378a7c268b9c2e73b60902d632463fb 
> 
> 
> Diff: https://reviews.apache.org/r/70526/diff/1/
> 
> 
> Testing
> ---
> 
> Visual inspection after using Firefox to edit the HTML: 
> https://drive.google.com/file/d/1PuLGmb3h3pj3xNXciVGWPQzGBHJqHvk5/view?usp=sharing
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 70489: Documented docker manifest v2 schema2 support.

2019-04-16 Thread Till Toenshoff via Review Board

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




docs/container-image.md
Lines 188-190 (patched)


maybe a bit more like this?

```
For privacte registries, the operator needs to configure `curl` with the 
location of required CA certificates.
```


- Till Toenshoff


On April 16, 2019, 11:39 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70489/
> ---
> 
> (Updated April 16, 2019, 11:39 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Jie Yu, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented docker manifest v2 schema2 support.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md 3c4520ad4a5391b1202319bb8db27b4d1c07a96b 
> 
> 
> Diff: https://reviews.apache.org/r/70489/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>



Re: Review Request 70323: Bumped nokogiri and rack site dependencies.

2019-03-27 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On March 27, 2019, 4:21 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70323/
> ---
> 
> (Updated March 27, 2019, 4:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This bumps nokogiri to >=1.8.5 for CVE-2018-16471 and rack to
> >=1.6.11 for CVE-2018-16471.
> 
> 
> Diffs
> -
> 
>   site/Gemfile.lock b3f364945b7dea7cd4696cbb7e956c3ed7bfb59a 
> 
> 
> Diff: https://reviews.apache.org/r/70323/diff/1/
> 
> 
> Testing
> ---
> 
> `bundle install && bundle exec middleman`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 70233: Allowed for optionally unbundled ZooKeeper for CMake builds.

2019-03-20 Thread Till Toenshoff via Review Board


> On March 19, 2019, 11:07 p.m., Joseph Wu wrote:
> > 3rdparty/cmake/FindZOOKEEPER.cmake
> > Lines 50 (patched)
> > 
> >
> > I wonder if it is worth adding a note here, restating bits of the blurb 
> > here:
> > 
> > https://github.com/apache/zookeeper/blob/master/zookeeper-client/zookeeper-client-c/README#L26-L32
> > 
> > We don't have a note in the autotools build.

Yeah, why not - its a good note...


> On March 19, 2019, 11:07 p.m., Joseph Wu wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 115 (patched)
> > 
> >
> > Using a Windows style path here is... a bit funny to me.  Nothing wrong 
> > with it though.

You are perfectly right, the last few times I followed this pattern, I smiled 
:D.


- Till


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


On March 19, 2019, 12:11 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70233/
> ---
> 
> (Updated March 19, 2019, 12:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9662
> https://issues.apache.org/jira/browse/MESOS-9662
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allowed for optionally unbundled ZooKeeper for CMake builds.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
>   3rdparty/cmake/FindZOOKEEPER.cmake PRE-CREATION 
>   cmake/CompilationConfigure.cmake d9c1e40a59eebbb295aa993682edb8ccbf2b48f7 
> 
> 
> Diff: https://reviews.apache.org/r/70233/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>



Re: Review Request 70233: Allowed for optionally unbundled ZooKeeper for CMake builds.

2019-03-18 Thread Till Toenshoff via Review Board

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

(Updated March 19, 2019, 12:11 a.m.)


Review request for mesos, Benjamin Bannier and Joseph Wu.


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


Repository: mesos


Description
---

Allowed for optionally unbundled ZooKeeper for CMake builds.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
  3rdparty/cmake/FindZOOKEEPER.cmake PRE-CREATION 
  cmake/CompilationConfigure.cmake d9c1e40a59eebbb295aa993682edb8ccbf2b48f7 


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

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


Testing
---


Thanks,

Till Toenshoff



Re: Review Request 70233: Allowed for optionally unbundled ZooKeeper for CMake builds.

2019-03-18 Thread Till Toenshoff via Review Board

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

(Updated March 19, 2019, 12:03 a.m.)


Review request for mesos, Benjamin Bannier and Joseph Wu.


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


Repository: mesos


Description
---

Allowed for optionally unbundled ZooKeeper for CMake builds.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
  3rdparty/cmake/FindZOOKEEPER.cmake PRE-CREATION 
  cmake/CompilationConfigure.cmake d9c1e40a59eebbb295aa993682edb8ccbf2b48f7 


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

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


Testing
---


Thanks,

Till Toenshoff



Review Request 70233: Allowed for optionally unbundled ZooKeeper for CMake builds.

2019-03-18 Thread Till Toenshoff via Review Board

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

Review request for mesos, Benjamin Bannier and Joseph Wu.


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


Repository: mesos


Description
---

Allowed for optionally unbundled ZooKeeper for CMake builds.


Diffs
-

  3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
  3rdparty/cmake/FindZOOKEEPER.cmake PRE-CREATION 
  cmake/CompilationConfigure.cmake d9c1e40a59eebbb295aa993682edb8ccbf2b48f7 


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


Testing
---


Thanks,

Till Toenshoff



Re: Review Request 70131: Added a comment around recovery for `ContainerLogger`s.

2019-03-06 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On March 5, 2019, 10:42 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70131/
> ---
> 
> (Updated March 5, 2019, 10:42 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Joseph Wu, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `ContainerLogger` interface does not currently use a
> "prepare-recover-cleanup" pattern and hence neither allows
> stateful loggers nor provides synchronization on container
> termination. Capture this in a comment to the interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/container_logger.hpp 
> 4e6d15a503e533a82bfac393da447f6f129dd66d 
> 
> 
> Diff: https://reviews.apache.org/r/70131/diff/1/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Re: Review Request 70047: Updated build specific artefact generation.

2019-03-05 Thread Till Toenshoff via Review Board

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

(Updated March 6, 2019, 1:12 a.m.)


Review request for mesos, Benjamin Bannier and Joseph Wu.


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


Repository: mesos


Description
---

For autotools, we extracted additional build info like the git branch
and sha during the automake phase handing them into libbuild via
commandline defines.

CMake builds however used a configuration file for this purpose.

This patch updates both build systems to make use of
build_git_config.hpp.in for build specific git information.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
  configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
  src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
  src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
  src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 
  src/common/git_version.hpp.in PRE-CREATION 


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

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


Testing
---

Manually tested both cmake and autotools.

First configure run:
```
[...]
checking for src/common/build_git_config.hpp... no
configure: creating src/common/build_git_config.hpp
[...]

```

Subsequent configure runs:
```
[...]
checking for src/common/build_git_config.hpp... yes
[...]
```

Final build from `support/packaging/centos/build-docker-rpmbuild.sh` installed 
and ran agent;
```
Installed:
  mesos.x86_64 0:1.8.0-0.1.pre.20190226gitc125541.el7

Complete!
[root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
I0226 01:00:48.581748   157 main.cpp:350] Build: 2019-02-26 00:36:49 by centos
I0226 01:00:48.581817   157 main.cpp:351] Version: 1.8.0
I0226 01:00:48.581823   157 main.cpp:358] Git SHA: 
c125541b8e4f2c2f6f56fe7e1c2e0b26d5bbfc62
I0226 01:00:48.587003   157 systemd.cpp:240] systemd version `219` detected
I0226 01:00:48.587026   157 main.cpp:453] Initializing systemd state
```


Thanks,

Till Toenshoff



Re: Review Request 70049: Assured a set USER within RPM build.

2019-03-05 Thread Till Toenshoff via Review Board

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

(Updated March 6, 2019, 1:01 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

Assured a set USER within RPM build.


Diffs (updated)
-

  support/packaging/centos/build_rpm.sh 
4c5b9617cfcef591b33d11f7f84170c0781d1fa1 


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

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


Testing
---

Installed resulting package and ran agent;
```
[root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
I0225 02:30:16.11352897 main.cpp:350] Build: 2019-02-25 01:59:55 by centos
I0225 02:30:16.11360697 main.cpp:351] Version: 1.8.0
```


Thanks,

Till Toenshoff



Re: Review Request 65116: Getting Started using the Mesos Binaries.

2019-03-05 Thread Till Toenshoff via Review Board

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


Ship it!




Thanks Senthil!

Fixing below while landing


docs/binary-packages.md
Lines 29 (patched)


s/Subsitute/Substitute/



site/source/downloads.html.erb
Lines 47 (patched)


s/mesos/Mesos/g


- Till Toenshoff


On March 5, 2019, 10:30 p.m., Senthil Kumaran wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65116/
> ---
> 
> (Updated March 5, 2019, 10:30 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Kapil Arya.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Getting Started using the Mesos Binaries.
> 
> 
> Diffs
> -
> 
>   docs/binary-packages.md PRE-CREATION 
>   site/source/downloads.html.erb 259ca43a7 
>   site/source/getting-started.html.md 2b1b18f61 
> 
> 
> Diff: https://reviews.apache.org/r/65116/diff/3/
> 
> 
> Testing
> ---
> 
> ```
> $ curl -L 
> "https://dl.bintray.com/mesos/el//7/x86_64/mesos-1.4.1-1.el7.x86_64.rpm; -o 
> mesos-1.4.1-1.el7.x86_64.rpm
>   % Total% Received % Xferd  Average Speed   TimeTime Time  
> Current
>  Dload  Upload   Total   SpentLeft  Speed
>   0 00 00 0  0  0 --:--:-- --:--:-- --:--:-- 0
> 100 43.4M  100 43.4M0 0  49.5M  0 --:--:-- --:--:-- --:--:-- 49.5M
> ```
> 
> And
> 
> ```
> $ sudo yum install mesos-1.4.1-1.el7.x86_64.rpm
> Loaded plugins: langpacks, product-id, search-disabled-repos
> Examining mesos-1.4.1-1.el7.x86_64.rpm: mesos-1.4.1-1.el7.x86_64
> Marking mesos-1.4.1-1.el7.x86_64.rpm to be installed
> Resolving Dependencies
> --> Running transaction check
> ---> Package mesos.x86_64 0:1.4.1-1.el7 will be installed
> --> Processing Dependency: cyrus-sasl-md5 for package: 
> mesos-1.4.1-1.el7.x86_64
> rhui-microsoft-azure-rhel7
>  | 2.9 kB  00:00:00
> rhui-rhel-7-server-dotnet-rhui-debug-rpms 
>  | 3.8 kB  00:00:00
> rhui-rhel-7-server-dotnet-rhui-rpms   
>  | 4.0 kB  00:00:00
> rhui-rhel-7-server-dotnet-rhui-source-rpms
>  | 3.8 kB  00:00:00
> rhui-rhel-7-server-rhui-debug-rpms
>  | 3.3 kB  00:00:00
> rhui-rhel-7-server-rhui-extras-debug-rpms 
>  | 3.2 kB  00:00:00
> rhui-rhel-7-server-rhui-extras-rpms   
>  | 3.4 kB  00:00:00
> rhui-rhel-7-server-rhui-extras-source-rpms
>  | 3.8 kB  00:00:00
> rhui-rhel-7-server-rhui-optional-debug-rpms   
>  | 3.3 kB  00:00:00
> rhui-rhel-7-server-rhui-optional-rpms 
>  | 3.5 kB  00:00:00
> rhui-rhel-7-server-rhui-optional-source-rpms  
>  | 3.8 kB  00:00:00
> rhui-rhel-7-server-rhui-rh-common-debug-rpms  
>  | 3.8 kB  00:00:00
> rhui-rhel-7-server-rhui-rh-common-rpms
>  | 3.8 kB  00:00:00
> rhui-rhel-7-server-rhui-rh-common-source-rpms 
>  | 3.8 kB  00:00:00
> rhui-rhel-7-server-rhui-rpms  
>  | 3.5 kB  00:00:00
> rhui-rhel-7-server-rhui-source-rpms   
>  | 3.8 kB  00:00:00
> rhui-rhel-7-server-rhui-supplementary-debug-rpms  
>  | 3.7 kB  00:00:00
> rhui-rhel-7-server-rhui-supplementary-rpms
>  | 3.4 kB  00:00:00
> rhui-rhel-7-server-rhui-supplementary-source-rpms 
>  | 3.8 kB  00:00:00
> rhui-rhel-server-rhui-rhscl-7-debug-rpms  
>  | 3.4 kB  00:00:00
> rhui-rhel-server-rhui-rhscl-7-rpms
>  | 3.5 kB  00:00:00
> 

Re: Review Request 70047: Updated build specific artefact generation.

2019-03-05 Thread Till Toenshoff via Review Board

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

(Updated March 5, 2019, 6:46 p.m.)


Review request for mesos, Benjamin Bannier and Joseph Wu.


Changes
---

addressed comments


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


Repository: mesos


Description
---

For autotools, we extracted additional build info like the git branch
and sha during the automake phase handing them into libbuild via
commandline defines.

CMake builds however used a configuration file for this purpose.

This patch updates both build systems to make use of
build_git_config.hpp.in for build specific git information.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
  configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
  src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
  src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
  src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 
  src/common/git_version.hpp.in PRE-CREATION 


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

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


Testing
---

Manually tested both cmake and autotools.

First configure run:
```
[...]
checking for src/common/build_git_config.hpp... no
configure: creating src/common/build_git_config.hpp
[...]

```

Subsequent configure runs:
```
[...]
checking for src/common/build_git_config.hpp... yes
[...]
```

Final build from `support/packaging/centos/build-docker-rpmbuild.sh` installed 
and ran agent;
```
Installed:
  mesos.x86_64 0:1.8.0-0.1.pre.20190226gitc125541.el7

Complete!
[root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
I0226 01:00:48.581748   157 main.cpp:350] Build: 2019-02-26 00:36:49 by centos
I0226 01:00:48.581817   157 main.cpp:351] Version: 1.8.0
I0226 01:00:48.581823   157 main.cpp:358] Git SHA: 
c125541b8e4f2c2f6f56fe7e1c2e0b26d5bbfc62
I0226 01:00:48.587003   157 systemd.cpp:240] systemd version `219` detected
I0226 01:00:48.587026   157 main.cpp:453] Initializing systemd state
```


Thanks,

Till Toenshoff



Re: Review Request 70089: Removed non-ascii source code characters.

2019-03-03 Thread Till Toenshoff via Review Board

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


Ship it!




Thanks - these keep creeping in.

- Till Toenshoff


On March 3, 2019, 1:01 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70089/
> ---
> 
> (Updated March 3, 2019, 1:01 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed non-ascii source code characters.
> 
> 
> Diffs
> -
> 
>   3rdparty/patch.exe.manifest cebcdadc49ab8e9a919fd3c50c4e823900b8fe37 
>   CHANGELOG c1ce30c21dd89e1b9dad4673c1505a3d8f3b0e97 
>   LICENSE 592b4d1369ea358a9889d768675a98f0466e2c9c 
>   docs/csi.md ae6079d9de566859963453429d8bdaee8ad8e898 
>   docs/isolators/cgroups-cpu.md ce29a890ee01f01eb01b2fe1f64c8b4ad83b4be7 
>   docs/quota.md 28021f481ba7c637519b3822c7dbe88dba440e0f 
>   include/mesos/scheduler/scheduler.proto 
> f6a780a7b75878b9e74402a28a25bb868f7ac36f 
>   include/mesos/v1/scheduler/scheduler.proto 
> fcfec5e417463103e98dd6917722b4fde41cac7c 
> 
> 
> Diff: https://reviews.apache.org/r/70089/diff/1/
> 
> 
> Testing
> ---
> 
> * `make check`
> * regenerated and inspected site
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 70049: Assured a set USER within RPM build.

2019-03-01 Thread Till Toenshoff via Review Board


> On Feb. 28, 2019, 8:52 p.m., Benjamin Bannier wrote:
> > support/packaging/centos/build_rpm.sh
> > Lines 29 (patched)
> > 
> >
> > What about only setting this if unset?
> > 
> > export USER=${USER:-centos}
> > 
> > That way this script would do the expected thing if invoked by somebody 
> > building her own packages, and we wouldn't necessarily assume that we run 
> > this in a Docker container.

magificient.


- Till


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


On Feb. 25, 2019, 2:31 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70049/
> ---
> 
> (Updated Feb. 25, 2019, 2:31 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-9605
> https://issues.apache.org/jira/browse/MESOS-9605
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assured a set USER within RPM build.
> 
> 
> Diffs
> -
> 
>   support/packaging/centos/build_rpm.sh 
> 4c5b9617cfcef591b33d11f7f84170c0781d1fa1 
> 
> 
> Diff: https://reviews.apache.org/r/70049/diff/1/
> 
> 
> Testing
> ---
> 
> Installed resulting package and ran agent;
> ```
> [root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
> I0225 02:30:16.11352897 main.cpp:350] Build: 2019-02-25 01:59:55 by centos
> I0225 02:30:16.11360697 main.cpp:351] Version: 1.8.0
> ```
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>



Re: Review Request 70047: Updated build specific artefact generation.

2019-02-28 Thread Till Toenshoff via Review Board


> On Feb. 26, 2019, 8:22 p.m., Joseph Wu wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 630-637 (original), 663-667 (patched)
> > 
> >
> > I don't think the CMake build needs to follow the autotools one here.  
> > For one, the packaging for CMake does not need to configure more than once 
> > (assuming we finish the CMake packaging ;).
> > 
> > A comment mentioning the difference should be enough.  i.e. 
> > ```
> > # NOTE: The `build_git_config.hpp.in` file is specific to the autotools 
> > build.
> > # `build_config.hpp.in` includes the definitions for the CMake build.
> > ```
> 
> Till Toenshoff wrote:
> There is indeed only one reason to do this, it is because we might want 
> zero suprises when finally switching over -- waiting for my shepherd to make 
> a call here :)

I totally omitted the source-distribution argument when stating the above -- so 
while academically speaking we wont need this for cmake based package creation 
(as done via CPack and not rpmbuild), we will still need it to include a git 
sha artefact in our source tarballs/packages.


- Till


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


On Feb. 26, 2019, 11:05 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70047/
> ---
> 
> (Updated Feb. 26, 2019, 11:05 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9605
> https://issues.apache.org/jira/browse/MESOS-9605
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools, we extracted additional build info like the git branch
> and sha during the automake phase handing them into libbuild via
> commandline defines.
> 
> CMake builds however used a configuration file for this purpose.
> 
> This patch updates both build systems to make use of
> build_git_config.hpp.in for build specific git information.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
>   configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
>   src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
>   src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
>   src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 
>   src/common/build_git_config.hpp.in PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70047/diff/4/
> 
> 
> Testing
> ---
> 
> Manually tested both cmake and autotools.
> 
> First configure run:
> ```
> [...]
> checking for src/common/build_git_config.hpp... no
> configure: creating src/common/build_git_config.hpp
> [...]
> 
> ```
> 
> Subsequent configure runs:
> ```
> [...]
> checking for src/common/build_git_config.hpp... yes
> [...]
> ```
> 
> Final build from `support/packaging/centos/build-docker-rpmbuild.sh` 
> installed and ran agent;
> ```
> Installed:
>   mesos.x86_64 0:1.8.0-0.1.pre.20190226gitc125541.el7
> 
> Complete!
> [root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
> I0226 01:00:48.581748   157 main.cpp:350] Build: 2019-02-26 00:36:49 by centos
> I0226 01:00:48.581817   157 main.cpp:351] Version: 1.8.0
> I0226 01:00:48.581823   157 main.cpp:358] Git SHA: 
> c125541b8e4f2c2f6f56fe7e1c2e0b26d5bbfc62
> I0226 01:00:48.587003   157 systemd.cpp:240] systemd version `219` detected
> I0226 01:00:48.587026   157 main.cpp:453] Initializing systemd state
> ```
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>



Re: Review Request 70047: Updated build specific artefact generation.

2019-02-28 Thread Till Toenshoff via Review Board


> On March 1, 2019, 12:02 a.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 630-637 (original), 663-667 (patched)
> > 
> >
> > The idea (at least on the autotools side which currently supports 
> > packaging) is to generate an artifact specifiying git info which is 
> > included with the distribution tarball.
> > 
> > AFAICT, unless we completely rip out autotools while implementing cmake 
> > distribution tarballs, we'll need to implement something like this to 
> > support showing git info when building from distribution tarballs, even in 
> > the cmake build (which would probably emit this also during packaging time).
> > 
> > Suggest to keep as is.

Yup!


> On March 1, 2019, 12:02 a.m., Benjamin Bannier wrote:
> > configure.ac
> > Lines 2829 (patched)
> > 
> >
> > Lets use `presence` instead of `for`,
> > 
> > checking src/common/build_git_config.hpp presence ...

ok


> On March 1, 2019, 12:02 a.m., Benjamin Bannier wrote:
> > configure.ac
> > Lines 2831 (patched)
> > 
> >
> > Even though there is some nesting I'd prefer more general `AS_IF` and 
> > friends.
> > 
> > Here and below.

ok


> On March 1, 2019, 12:02 a.m., Benjamin Bannier wrote:
> > configure.ac
> > Lines 2836 (patched)
> > 
> >
> > `s/creating/generating/`

ok


> On March 1, 2019, 12:02 a.m., Benjamin Bannier wrote:
> > configure.ac
> > Lines 2843 (patched)
> > 
> >
> > Do you understand why this command could print something to stderr? It 
> > seems it should be possible to treat the absent case as error and define 
> > the variable with a value immediately.
> > 
> > Also, what do you think about instead
> > ```
> > git --git-dir=${srcdir}"/.git/ rev-parse HEAD ...
> > ```
> > 
> > We could put the git dir into a variable.
> > 
> > Here and below.

> Do you understand why this command could print something to stderr?

Yes - well not the head SHA, that should indeed always work.

Let me take the tag for an example;
```
lobomacpro4:~/Development/mesos-private (ci/till/build-config-revamp ?) $ git 
describe --exact --tags
fatal: no tag exactly matches '9ed7e160f003b5e22bf9c41e0104e0efca1df682'
lobomacpro4:~/Development/mesos-private (ci/till/build-config-revamp ?) $ git 
describe --exact --tags 2>/dev/null
lobomacpro4:~/Development/mesos-private (ci/till/build-config-revamp ?) $ echo 
$?
128
```

Then imagine a detached checkout, that would similarly fail for the branch 
detection.

```
lobomacpro4:~/Development/mesos-private (no branch! ?) $ git symbolic-ref HEAD
fatal: ref HEAD is not a symbolic ref
lobomacpro4:~/Development/mesos-private (no branch! ?) $ git symbolic-ref HEAD 
2>/dev/null
lobomacpro4:~/Development/mesos-private (no branch! ?) $ echo $?
128
```

So I would argue that the current approach for error handling is rather well 
adapted.


> > `git --git-dir=${srcdir}"/.git/ rev-parse HEAD ...`

Much better, thanks!


> On March 1, 2019, 12:02 a.m., Benjamin Bannier wrote:
> > src/common/build_git_config.hpp.in
> > Lines 1 (patched)
> > 
> >
> > Since this isn't about the build but git version, what do you think 
> > abot calling this file e.g., `git_version.hpp.in` or similar?

Yes - good point.


> On March 1, 2019, 12:02 a.m., Benjamin Bannier wrote:
> > src/common/build_git_config.hpp.in
> > Lines 23-25 (patched)
> > 
> >
> > Not really a fan of how we directly emit a CPP macro from autoconf, but 
> > without config headers I cannot really think of a objectively better 
> > approach :(

I wish you could! :(


- Till


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


On Feb. 26, 2019, 11:05 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70047/
> ---
> 
> (Updated Feb. 26, 2019, 11:05 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9605
> https://issues.apache.org/jira/browse/MESOS-9605
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools, we extracted additional build info like the git branch
> and sha during the automake phase 

Re: Review Request 70047: Updated build specific artefact generation.

2019-02-28 Thread Till Toenshoff via Review Board


> On Feb. 26, 2019, 8:22 p.m., Joseph Wu wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 630-637 (original), 663-667 (patched)
> > 
> >
> > I don't think the CMake build needs to follow the autotools one here.  
> > For one, the packaging for CMake does not need to configure more than once 
> > (assuming we finish the CMake packaging ;).
> > 
> > A comment mentioning the difference should be enough.  i.e. 
> > ```
> > # NOTE: The `build_git_config.hpp.in` file is specific to the autotools 
> > build.
> > # `build_config.hpp.in` includes the definitions for the CMake build.
> > ```

There is indeed only one reason to do this, it is because we might want zero 
suprises when finally switching over -- waiting for my shepherd to make a call 
here :)


- Till


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


On Feb. 26, 2019, 11:05 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70047/
> ---
> 
> (Updated Feb. 26, 2019, 11:05 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9605
> https://issues.apache.org/jira/browse/MESOS-9605
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools, we extracted additional build info like the git branch
> and sha during the automake phase handing them into libbuild via
> commandline defines.
> 
> CMake builds however used a configuration file for this purpose.
> 
> This patch updates both build systems to make use of
> build_git_config.hpp.in for build specific git information.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
>   configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
>   src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
>   src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
>   src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 
>   src/common/build_git_config.hpp.in PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70047/diff/4/
> 
> 
> Testing
> ---
> 
> Manually tested both cmake and autotools.
> 
> First configure run:
> ```
> [...]
> checking for src/common/build_git_config.hpp... no
> configure: creating src/common/build_git_config.hpp
> [...]
> 
> ```
> 
> Subsequent configure runs:
> ```
> [...]
> checking for src/common/build_git_config.hpp... yes
> [...]
> ```
> 
> Final build from `support/packaging/centos/build-docker-rpmbuild.sh` 
> installed and ran agent;
> ```
> Installed:
>   mesos.x86_64 0:1.8.0-0.1.pre.20190226gitc125541.el7
> 
> Complete!
> [root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
> I0226 01:00:48.581748   157 main.cpp:350] Build: 2019-02-26 00:36:49 by centos
> I0226 01:00:48.581817   157 main.cpp:351] Version: 1.8.0
> I0226 01:00:48.581823   157 main.cpp:358] Git SHA: 
> c125541b8e4f2c2f6f56fe7e1c2e0b26d5bbfc62
> I0226 01:00:48.587003   157 systemd.cpp:240] systemd version `219` detected
> I0226 01:00:48.587026   157 main.cpp:453] Initializing systemd state
> ```
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>



Re: Review Request 70047: Updated build specific artefact generation.

2019-02-26 Thread Till Toenshoff via Review Board

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

(Updated Feb. 26, 2019, 11:05 a.m.)


Review request for mesos, Benjamin Bannier and Joseph Wu.


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


Repository: mesos


Description
---

For autotools, we extracted additional build info like the git branch
and sha during the automake phase handing them into libbuild via
commandline defines.

CMake builds however used a configuration file for this purpose.

This patch updates both build systems to make use of
build_git_config.hpp.in for build specific git information.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
  configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
  src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
  src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
  src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 
  src/common/build_git_config.hpp.in PRE-CREATION 


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

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


Testing
---

Manually tested both cmake and autotools.

First configure run:
```
[...]
checking for src/common/build_git_config.hpp... no
configure: creating src/common/build_git_config.hpp
[...]

```

Subsequent configure runs:
```
[...]
checking for src/common/build_git_config.hpp... yes
[...]
```

Final build from `support/packaging/centos/build-docker-rpmbuild.sh` installed 
and ran agent;
```
Installed:
  mesos.x86_64 0:1.8.0-0.1.pre.20190226gitc125541.el7

Complete!
[root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
I0226 01:00:48.581748   157 main.cpp:350] Build: 2019-02-26 00:36:49 by centos
I0226 01:00:48.581817   157 main.cpp:351] Version: 1.8.0
I0226 01:00:48.581823   157 main.cpp:358] Git SHA: 
c125541b8e4f2c2f6f56fe7e1c2e0b26d5bbfc62
I0226 01:00:48.587003   157 systemd.cpp:240] systemd version `219` detected
I0226 01:00:48.587026   157 main.cpp:453] Initializing systemd state
```


Thanks,

Till Toenshoff



Re: Review Request 70047: Updated build specific artefact generation.

2019-02-25 Thread Till Toenshoff via Review Board

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

(Updated Feb. 26, 2019, 2:20 a.m.)


Review request for mesos, Benjamin Bannier and Joseph Wu.


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


Repository: mesos


Description (updated)
---

For autotools, we extracted additional build info like the git branch
and sha during the automake phase handing them into libbuild via
commandline defines.

CMake builds however used a configuration file for this purpose.

This patch updates both build systems to make use of
build_git_config.hpp.in for build specific git information.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
  configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
  src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
  src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
  src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 
  src/common/build_git_config.hpp.in PRE-CREATION 


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

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


Testing
---

Manually tested both cmake and autotools.

First configure run:
```
[...]
checking for src/common/build_git_config.hpp... no
configure: creating src/common/build_git_config.hpp
[...]

```

Subsequent configure runs:
```
[...]
checking for src/common/build_git_config.hpp... yes
[...]
```

Final build from `support/packaging/centos/build-docker-rpmbuild.sh` installed 
and ran agent;
```
Installed:
  mesos.x86_64 0:1.8.0-0.1.pre.20190226gitc125541.el7

Complete!
[root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
I0226 01:00:48.581748   157 main.cpp:350] Build: 2019-02-26 00:36:49 by centos
I0226 01:00:48.581817   157 main.cpp:351] Version: 1.8.0
I0226 01:00:48.581823   157 main.cpp:358] Git SHA: 
c125541b8e4f2c2f6f56fe7e1c2e0b26d5bbfc62
I0226 01:00:48.587003   157 systemd.cpp:240] systemd version `219` detected
I0226 01:00:48.587026   157 main.cpp:453] Initializing systemd state
```


Thanks,

Till Toenshoff



Re: Review Request 70047: Updated build specific artefact generation.

2019-02-25 Thread Till Toenshoff via Review Board

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

(Updated Feb. 26, 2019, 2:18 a.m.)


Review request for mesos, Benjamin Bannier and Joseph Wu.


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


Repository: mesos


Description (updated)
---

For autotools, we extracted additional build info like the git branch
and sha during the automake phase handing them into libbuild via
commandline supplied defines.

CMake builds however used a configuration file for this purpose.

This patch updates both build systems to make use of
build_git_config.hpp.in for build specific git information. The data
is persisted only once for the initial configuration / cmake run.
Subsequent invocations will not update build_git_config.hpp. We do
this to make sure that snapshots can get built without having git
specific information available at build time.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
  configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
  src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
  src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
  src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 
  src/common/build_git_config.hpp.in PRE-CREATION 


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

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


Testing (updated)
---

Manually tested both cmake and autotools.

First configure run:
```
[...]
checking for src/common/build_git_config.hpp... no
configure: creating src/common/build_git_config.hpp
[...]

```

Subsequent configure runs:
```
[...]
checking for src/common/build_git_config.hpp... yes
[...]
```

Final build from `support/packaging/centos/build-docker-rpmbuild.sh` installed 
and ran agent;
```
Installed:
  mesos.x86_64 0:1.8.0-0.1.pre.20190226gitc125541.el7

Complete!
[root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
I0226 01:00:48.581748   157 main.cpp:350] Build: 2019-02-26 00:36:49 by centos
I0226 01:00:48.581817   157 main.cpp:351] Version: 1.8.0
I0226 01:00:48.581823   157 main.cpp:358] Git SHA: 
c125541b8e4f2c2f6f56fe7e1c2e0b26d5bbfc62
I0226 01:00:48.587003   157 systemd.cpp:240] systemd version `219` detected
I0226 01:00:48.587026   157 main.cpp:453] Initializing systemd state
```


Thanks,

Till Toenshoff



Re: Review Request 70055: Output Review Request URL in Reviewbot output.

2019-02-25 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Feb. 25, 2019, 11:44 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70055/
> ---
> 
> (Updated Feb. 25, 2019, 11:44 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The URL makes it easy to get to the Review from log messages.
> 
> 
> Diffs
> -
> 
>   support/verify-reviews.py ab4888ae04ded555abb75c7cea468afa0ccb2ea8 
> 
> 
> Diff: https://reviews.apache.org/r/70055/diff/1/
> 
> 
> Testing
> ---
> 
> ./support/verify-reviews.py -u foo -p bar --skip-verify
> 
> 
> This just made sure I didn't make any syntax errors.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 70047: Updated build specific artefact generation.

2019-02-24 Thread Till Toenshoff via Review Board

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

(Updated Feb. 25, 2019, 2:35 a.m.)


Review request for mesos, Benjamin Bannier and Joseph Wu.


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


Repository: mesos


Description
---

For autotools, we extracted additional build info like the git branch
and sha during the automake phase handing them into libbuild via
commandline defines.

CMake builds however used a configuration file for this purpose.

This patch updates both build systems to make use of
build_config.hpp.in for build specific configuration artefacts.


Diffs
-

  cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
  configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
  src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
  src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
  src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 


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


Testing (updated)
---

manually tested both cmake and autotools.

WIP: A test using the centos RPM toolchain does not yield the intended result;
```
[root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
I0225 02:30:16.11352897 main.cpp:350] Build: 2019-02-25 01:59:55 by centos
I0225 02:30:16.11360697 main.cpp:351] Version: 1.8.0
I0225 02:30:16.12117397 systemd.cpp:240] systemd version `219` detected
I0225 02:30:16.12119997 main.cpp:453] Initializing systemd state
E0225 02:30:16.12125197 main.cpp:462] EXIT with status 1: Failed to 
initialize systemd: Failed to locate systemd runtime directory: 
/run/systemd/system
```

It still lacks the SHA!


Thanks,

Till Toenshoff



Re: Review Request 70049: Assured a set USER within RPM build.

2019-02-24 Thread Till Toenshoff via Review Board

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

(Updated Feb. 25, 2019, 2:31 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

Assured a set USER within RPM build.


Diffs
-

  support/packaging/centos/build_rpm.sh 
4c5b9617cfcef591b33d11f7f84170c0781d1fa1 


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


Testing (updated)
---

Installed resulting package and ran agent;
```
[root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
I0225 02:30:16.11352897 main.cpp:350] Build: 2019-02-25 01:59:55 by centos
I0225 02:30:16.11360697 main.cpp:351] Version: 1.8.0
```


Thanks,

Till Toenshoff



Review Request 70049: Assured a set USER within RPM build.

2019-02-24 Thread Till Toenshoff via Review Board

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

Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

Assured a set USER within RPM build.


Diffs
-

  support/packaging/centos/build_rpm.sh 
4c5b9617cfcef591b33d11f7f84170c0781d1fa1 


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


Testing
---


Thanks,

Till Toenshoff



Review Request 70047: Updated build specific artefact generation.

2019-02-24 Thread Till Toenshoff via Review Board

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

Review request for mesos, Benjamin Bannier and Joseph Wu.


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


Repository: mesos


Description
---

For autotools, we extracted additional build info like the git branch
and sha during the automake phase handing them into libbuild via
commandline defines.

CMake builds however used a configuration file for this purpose.

This patch updates both build systems to make use of
build_config.hpp.in for build specific configuration artefacts.


Diffs
-

  cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
  configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
  src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
  src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
  src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 


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


Testing
---

manually tested both cmake and autotools.


Thanks,

Till Toenshoff



Re: Review Request 70045: Skipped tests when verifying `master` as part of review verification.

2019-02-24 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Feb. 24, 2019, 10:03 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70045/
> ---
> 
> (Updated Feb. 24, 2019, 10:03 p.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to flakes test results reported by reviewbot need manual
> verification so we can skip executing tests when validating the
> `master` branch. This saves some execution time during validation.
> 
> Incidentally with this patch a previous comment matches now matches
> what is happening.
> 
> 
> Diffs
> -
> 
>   support/jenkins/reviewbot.sh 3cdd7f1c909c930a561ce2890212118579a47b73 
> 
> 
> Diff: https://reviews.apache.org/r/70045/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 70000: Fixed verify-reviews.py to not abort review verification prematurely.

2019-02-19 Thread Till Toenshoff via Review Board

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


Fix it, then Ship it!





support/verify-reviews.py
Lines 182-186 (patched)


We could now reference https://issues.apache.org/jira/browse/MESOS-9583 :)


- Till Toenshoff


On Feb. 19, 2019, 10:43 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7/
> ---
> 
> (Updated Feb. 19, 2019, 10:43 a.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Bugs: MESOS-9582
> https://issues.apache.org/jira/browse/MESOS-9582
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously `support/verify-reviews.py` would abort prematurely
> whenever a review a patch could not be applied. This was due to
> the `shell` function used to call `support/apply-reviews.py` invoking
> `exit(1)` on the first error which stopped iteration over all
> outstanding reviews.
> 
> In this patch that explicit call to `exit` is removed, and instead we
> let a possible `subprocess.CalledProcessError` propagate up for it to
> be handled at a higher level. Currently this will post a note on the
> review in question to (1) notify the submitter, and (2) prevent the
> review from being checked again.
> 
> With the changes here the script will not stop verify reviews in such
> cases.
> 
> 
> Diffs
> -
> 
>   support/verify-reviews.py a88a91f8ecb3794846d2d5eddee27ada770d55b9 
> 
> 
> Diff: https://reviews.apache.org/r/7/diff/2/
> 
> 
> Testing
> ---
> 
> Ran the script (with a dummy user and password) on today's reviewboard state. 
> The script attempted to post a review on the last patch in the chain instead 
> of aborting (see the `TODO` in the code on why we weren't able to diagnose 
> the issue in the faulty patch with the current implementation).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 69999: Fixed some lint issues in verify-reviews.py.

2019-02-19 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Feb. 17, 2019, 5:48 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6/
> ---
> 
> (Updated Feb. 17, 2019, 5:48 p.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Bugs: MESOS-9582
> https://issues.apache.org/jira/browse/MESOS-9582
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed some lint issues in verify-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/verify-reviews.py a88a91f8ecb3794846d2d5eddee27ada770d55b9 
> 
> 
> Diff: https://reviews.apache.org/r/6/diff/1/
> 
> 
> Testing
> ---
> 
> https://reviews.apache.org/r/7/
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 69998: Reformated verify-reviews.py with yapf.

2019-02-19 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Feb. 17, 2019, 5:48 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69998/
> ---
> 
> (Updated Feb. 17, 2019, 5:48 p.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Bugs: MESOS-9582
> https://issues.apache.org/jira/browse/MESOS-9582
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reformated verify-reviews.py with yapf.
> 
> 
> Diffs
> -
> 
>   support/verify-reviews.py a88a91f8ecb3794846d2d5eddee27ada770d55b9 
> 
> 
> Diff: https://reviews.apache.org/r/69998/diff/1/
> 
> 
> Testing
> ---
> 
> Tested as part of https://reviews.apache.org/r/7/
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 69986: Added a log line for simplified debugging.

2019-02-15 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Feb. 14, 2019, 3:24 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69986/
> ---
> 
> (Updated Feb. 14, 2019, 3:24 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a log line for simplified debugging.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 2a9ea448d7f963f86e8b2909d83e82b498e4104c 
> 
> 
> Diff: https://reviews.apache.org/r/69986/diff/1/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Review Request 69931: Empty test commit.

2019-02-08 Thread Till Toenshoff via Review Board

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

Review request for mesos.


Repository: mesos


Description
---

Empty test commit.


Diffs
-

  configure.ac 21115e56e1f5faaff667389f59a357e314f1da8f 


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


Testing
---


Thanks,

Till Toenshoff



Re: Review Request 69697: Reverted cleanup step of `verify-reviews.py`.

2019-02-08 Thread Till Toenshoff via Review Board


> On Jan. 16, 2019, 5:04 p.m., Vinod Kone wrote:
> > support/verify-reviews.py
> > Line 174 (original), 171 (patched)
> > 
> >
> > Do you guys understand why this was failing in the Azure CI in the 
> > first place? Originally, the HEAD variable was calculated outside 
> > `cleanup`. But looks like at some point (during refactor for python3) it 
> > was pushed inside `cleanup`. Do you know why?
> > 
> > Also `git checkout HEAD -- HEAD` is wrong. I don't think that works. 
> > Was this tested?

We are assuming this stems from the fact that the working directory of the 
invocations differ - https://reviews.apache.org/r/69817 determines the working 
directory, we expect/hope/pray that fixes things.


- Till


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


On Jan. 9, 2019, 9:30 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69697/
> ---
> 
> (Updated Jan. 9, 2019, 9:30 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9514
> https://issues.apache.org/jira/browse/MESOS-9514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reverts the improvements made on the support script `verify-reviews.py`
> in 3badf7179992e61f30f5a79da9d481dd451c7c2f. Sadly, these changes have
> broken our Azure based Mesos CI as seen in MESOS-9514.
> 
> After comparing the code before and after the commit, we can see that
> the line failing in CI, `HEAD = shell("git rev-parse HEAD")`, is
> misplaced. It was previously located in the exit process in the
> `cleanup()` function and it is now back there, surrounded by a
> try/catch for error handling.
> 
> 
> Diffs
> -
> 
>   support/verify-reviews.py 72f98b234d9a2a84decb0569998d74e4c730122d 
> 
> 
> Diff: https://reviews.apache.org/r/69697/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 69817: Refactored 'support/verify-reviews.py' to be closer to commit 7412179.

2019-02-08 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Jan. 23, 2019, 11:47 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69817/
> ---
> 
> (Updated Jan. 23, 2019, 11:47 a.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The goal of this commit is to make 'verify-reviews.py' as close as
> possible to its last known working state (commit
> 74121798f24fca372180b8c4bc00b4df07d46240).
> 
> The diff between 'verify-reviews.py' at this commit and 7412179 is
> available here: https://www.diffchecker.com/cmfaM83O All the new
> features added since 7412179 have been added again and the code
> has been made Python 3 compatible again.
> 
> The goal is to have a diff as minimal as possible with improvements
> regarding logs so that we do not face CI issues anymore. Changes
> have been made regarding how we run commands from the script so
> that they are run from the repository instead of where the
> script is being called.
> 
> 
> Diffs
> -
> 
>   support/verify-reviews.py 71326d34bb649e27a3a2901867d31a2a1fffd4e9 
> 
> 
> Diff: https://reviews.apache.org/r/69817/diff/1/
> 
> 
> Testing
> ---
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 69839: Fixed scheduler library on multiple SUBSCRIBE requests per connection.

2019-01-29 Thread Till Toenshoff via Review Board

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

(Updated Jan. 30, 2019, 4:34 a.m.)


Review request for mesos, Alexander Rukletsov and Vinod Kone.


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


Repository: mesos


Description (updated)
---

The HTTP scheduler API dictates that on a single connection, the
scheduler may only send a single SUBSCRIBE request. Due to recent
authentication related changes, this contract got broken. This patch
restores the contract and adds a test validating that the library is
enforcing it.


Diffs (updated)
-

  src/scheduler/scheduler.cpp cb24ba9c8e1d04b8c62bdf07b12758a61b3bf036 
  src/tests/scheduler_tests.cpp b571bb1d20744b943580677a26db4c12c7c311d1 


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

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


Testing
---

manual testing; 
Running the included test without patching `scheduler.cpp` -> fails as the 
master does in fact receive two SUBSCRIBE requests.

`make check`


Thanks,

Till Toenshoff



Re: Review Request 69839: Fixed scheduler library on multiple SUBSCRIBE requests per connection.

2019-01-25 Thread Till Toenshoff via Review Board

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

(Updated Jan. 25, 2019, 3:41 p.m.)


Review request for mesos, Alexander Rukletsov and Vinod Kone.


Summary (updated)
-

Fixed scheduler library on multiple SUBSCRIBE requests per connection.


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


Repository: mesos


Description (updated)
---

The HTTP scheduler API dictates that on a single connection, the scheduler may
only send a single SUBSCRIBE request. Due to recent authentication related
changes, this contract got broken. This patch restores the contract and adds a
test validating that the library is enforcing it.


Diffs
-

  src/scheduler/scheduler.cpp cb24ba9c8e1d04b8c62bdf07b12758a61b3bf036 
  src/tests/scheduler_tests.cpp b571bb1d20744b943580677a26db4c12c7c311d1 


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


Testing
---

manual testing; 
Running the included test without patching `scheduler.cpp` -> fails as the 
master does in fact receive two SUBSCRIBE requests.

`make check`


Thanks,

Till Toenshoff



Review Request 69839: Fix scheduler library on multiple SUBSCRIBE requests per connection.

2019-01-25 Thread Till Toenshoff via Review Board

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

Review request for mesos, Alexander Rukletsov and Vinod Kone.


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


Repository: mesos


Description
---

The HTTP scheduler API dictates that on a single connection, the scheduler
may only send a single SUBSCRIBE request. Due to some authentication
related refactorings, this contract got broken.
Adds an additional test validating that the library was enforcing the
above contract.


Diffs
-

  src/scheduler/scheduler.cpp cb24ba9c8e1d04b8c62bdf07b12758a61b3bf036 
  src/tests/scheduler_tests.cpp b571bb1d20744b943580677a26db4c12c7c311d1 


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


Testing
---

manual testing; 
Running the included test without patching `scheduler.cpp` -> fails as the 
master does in fact receive two SUBSCRIBE requests.

`make check`


Thanks,

Till Toenshoff



Re: Review Request 69721: Broke up `SSLTest.ProtocolMismatch` into smaller tests.

2019-01-15 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Jan. 14, 2019, 7:54 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69721/
> ---
> 
> (Updated Jan. 14, 2019, 7:54 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-5189
> https://issues.apache.org/jira/browse/MESOS-5189
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch breaks up the different iterations in
> `SSLTest.ProtocolMismatch` into smaller tests
> `SSLProtocol/SSLProtocolTest.Mismatch/N`. While this slightly
> increases the time to run all combinations sequentially, it leads to
> much shorter aggregated run time in parallel test execution.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> bfb5eab255230b7fbfdefaa62be27a8f8c4e9517 
> 
> 
> Diff: https://reviews.apache.org/r/69721/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 69720: Made `SSLTest` an unparameterized test suite.

2019-01-15 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Jan. 14, 2019, 7:54 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69720/
> ---
> 
> (Updated Jan. 14, 2019, 7:54 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-5189
> https://issues.apache.org/jira/browse/MESOS-5189
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This general and reusable testing class is made unparameterized since
> deriving from a parameterized test is cumbersome (e.g., requires manual
> disambiguation of possibly private types and methods inherited from the
> respective `::testing::WithParamInterface` base class).
> 
> This patch makes `SSLTest` an unparameterized test suite which eases
> reuse. We are able to remove a number of redundant test
> parameterizations along the way.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> 23d7aee963b6fb489403a94500d39e3413c7fcdd 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> bfb5eab255230b7fbfdefaa62be27a8f8c4e9517 
> 
> 
> Diff: https://reviews.apache.org/r/69720/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 69694: Tester.

2019-01-09 Thread Till Toenshoff via Review Board

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

(Updated Jan. 10, 2019, 3:52 a.m.)


Review request for mesos.


Repository: mesos


Description
---

Tester.


Diffs (updated)
-

  CMakeLists.txt f2885faa25f1d718d0f451aa2199e3f7692317a3 
  configure.ac 6778f119570def1838e26cddf7b0192bfe6e37d4 


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

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


Testing
---

Dont review - just a test!


Thanks,

Till Toenshoff



  1   2   3   >