Review Request 56867: WIP: Added upgrade guide for 1.2.x.

[Adam B]

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

Review request for mesos.


Repository: mesos


Description
---

Added upgrade guide for 1.2.x.


Diffs
-

  docs/upgrades.md 74f4367020d7025bdcfbbf50a166a644ee5050bc 

Diff: https://reviews.apache.org/r/56867/diff/


Testing
---


Thanks,

Adam B

Re: Review Request 55888: Test to ensure non-authorized users cannot launch tasks on agents.

[Anindya Sinha]

> On Feb. 21, 2017, 12:18 a.m., Jiang Yan Xu wrote:
> > Hey looks like slave_authorization_tests.cpp is more suitable place to put 
> > this test?

Fair enough. Moved the test to `slave_authorization_tests.cpp` in the latest 
update.


- Anindya


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


On Feb. 8, 2017, 3:54 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55888/
> ---
> 
> (Updated Feb. 8, 2017, 3:54 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6953
> https://issues.apache.org/jira/browse/MESOS-6953
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Test to ensure non-authorized users cannot launch tasks on agents.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 7f357a653335b559b5c78d4618926715dc4a63c7 
> 
> Diff: https://reviews.apache.org/r/55888/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 55888: Test to ensure non-authorized users cannot launch tasks on agents.

[Anindya Sinha]

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

(Updated Feb. 21, 2017, 7:21 a.m.)


Review request for mesos, Adam B, Alexander Rojas, and Jiang Yan Xu.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Test to ensure non-authorized users cannot launch tasks on agents.


Diffs (updated)
-

  src/tests/slave_authorization_tests.cpp 
61c44460eb79931dd77c5100eb33a31a182c1710 

Diff: https://reviews.apache.org/r/55888/diff/


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 55888: Test to ensure non-authorized users cannot launch tasks on agents.

[Anindya Sinha]

> On Feb. 20, 2017, 7:50 a.m., Jiang Yan Xu wrote:
> > src/tests/slave_tests.cpp, line 4965
> > 
> >
> > s/users/user/

`users` is fine since there are 2 users, i.e. `bar` and the current user.


- Anindya


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


On Feb. 8, 2017, 3:54 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55888/
> ---
> 
> (Updated Feb. 8, 2017, 3:54 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6953
> https://issues.apache.org/jira/browse/MESOS-6953
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Test to ensure non-authorized users cannot launch tasks on agents.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 7f357a653335b559b5c78d4618926715dc4a63c7 
> 
> Diff: https://reviews.apache.org/r/55888/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 55887: Check task user before allowing a task to be launched on the agent.

[Anindya Sinha]

> On Feb. 20, 2017, 7:07 a.m., Jiang Yan Xu wrote:
> > src/slave/slave.cpp, line 1713
> > 
> >
> > s/tasks(s) // 
> > because it's already in `taskOrTaskGroup`?
> > 
> > Also s/WARNING/ERROR/ because this is actually an error, if we do log 
> > it.

I changed to `ERROR`, however nothing is logged on the master when 
authorization fails. So we might as well do the same on the agent?


> On Feb. 20, 2017, 7:07 a.m., Jiang Yan Xu wrote:
> > src/slave/slave.cpp, line 1722
> > 
> >
> > Why not TASK_ERROR?

AFAICT, `TASK_ERROR` is only generated from the master and never from the 
agents; agents only generate `TASK_FAILED` on a failure. So, I left this status 
update that the agent generates as `TASK_FAILED`.


- Anindya


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


On Feb. 8, 2017, 3:53 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55887/
> ---
> 
> (Updated Feb. 8, 2017, 3:53 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6953
> https://issues.apache.org/jira/browse/MESOS-6953
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for action `run_tasks` on the agent's flag `acl`. Based on
> the ACL configured for `run_tasks`, a task to be launched on the agent
> will be (dis)allowed to launch on the agent.
> If a task or task group cannot be launched due to failed authorization,
> a `TASK_FAILED` Status Update shall be sent with a reason code of
> `REASON_TASK_UNAUTHORIZED` or `REASON_TASK_GROUP_UNAUTHORIZED` as
> applicable.
> Note that in case of a task group, all tasks fail if any of the tasks
> within the task group encounter the authorization error.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 5049eb783b8ad7b9599f20c3701f7d3d654b4491 
>   src/slave/slave.cpp 92564ff8fff06d1cb17192d374d355b4bb7d39d8 
> 
> Diff: https://reviews.apache.org/r/55887/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 55887: Check task user before allowing a task to be launched on the agent.

[Anindya Sinha]

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

(Updated Feb. 21, 2017, 7:20 a.m.)


Review request for mesos, Adam B, Alexander Rojas, and Jiang Yan Xu.


Changes
---

Addressed review comments.


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


Repository: mesos


Description (updated)
---

Added support for action `run_tasks` on the agent's flag `acl`. Based on
the ACL configured for `run_tasks`, a task to be launched on the agent
can be (dis)allowed to launch on the agent.
If a task or task group cannot be launched due to failed authorization,
a `TASK_FAILED` Status Update shall be sent with a reason code of
`REASON_TASK_UNAUTHORIZED` or `REASON_TASK_GROUP_UNAUTHORIZED` as
applicable.
Note that in case of a task group, all tasks fail if any of the tasks
within the task group encounter the authorization error.


Diffs (updated)
-

  src/slave/slave.hpp 3b0aea4e3e9a17501077beccbccaab4abbe11af2 
  src/slave/slave.cpp 7564e8d39530794131dbbc928fcbc59fb65ef471 
  src/tests/master_slave_reconciliation_tests.cpp 
1c7a3d686e2f924ad14c75fcab2ccafaab6d7b81 
  src/tests/mock_slave.hpp 1acb961b642e1e0b4677db7b8fc6150d480eb751 
  src/tests/mock_slave.cpp 50c04bff9e6f0f202af4c07b4036e021d3833ebf 
  src/tests/slave_tests.cpp 16bb14b170d724bd8424778a76de28b0efccc6ed 

Diff: https://reviews.apache.org/r/55887/diff/


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 56814: Wrapped IOSwitchboard.connect() in a dispatch.

[Jie Yu]

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


Ship it!




Good catch!

- Jie Yu


On Feb. 18, 2017, 6:40 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56814/
> ---
> 
> (Updated Feb. 18, 2017, 6:40 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7144
> https://issues.apache.org/jira/browse/MESOS-7144
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the IOSwitchboard is implemented as a MesosIsolatorProcess, most
> of its API calls are automatically dispatched onto its underlying
> process by an Isolator wrapper. However, the IOSwitchboard also
> includes an additional connect() call which is not accessed through
> the Isolator wrapper. As such, we need to wrap it in a dispatch call
> manually.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.hpp 
> 7a7ad2da047ef316f4382e45526d503c87a903a0 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> b948f3c44dd2e1af2228ca1579f24ae3a94160b0 
> 
> Diff: https://reviews.apache.org/r/56814/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check
> sudo src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 56812: Updated agent handlers to use 'AuthenticationContext'.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On Feb. 17, 2017, 7:02 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56812/
> ---
> 
> (Updated Feb. 17, 2017, 7:02 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7003
> https://issues.apache.org/jira/browse/MESOS-7003
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the HTTP endpoint handlers in the
> agent process to accept an `AuthenticationContext`
> instead of an `Option& principal`.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp af70b6f294a04f23b04cd1d8c36c1c3e86d7d5e6 
>   src/slave/slave.hpp 3b0aea4e3e9a17501077beccbccaab4abbe11af2 
>   src/slave/slave.cpp 7564e8d39530794131dbbc928fcbc59fb65ef471 
> 
> Diff: https://reviews.apache.org/r/56812/diff/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 56804: Cleaned up weights handling code.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On Feb. 17, 2017, 4:14 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56804/
> ---
> 
> (Updated Feb. 17, 2017, 4:14 p.m.)
> 
> 
> Review request for mesos, Adam B and Yongqiao Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed a redundant statement, fixed log message style, cleaned up
> comments.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 5534674fa2740b0a6e37fc1df34b582f1e691133 
>   src/master/weights_handler.cpp da0b995e1866e7eb57b8ef885864c6c6d66405d6 
>   src/tests/dynamic_weights_tests.cpp 
> ce577ce6c71bbfffba7db7c72523a9fd2d68141b 
> 
> Diff: https://reviews.apache.org/r/56804/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 54784: Added authorization tests when trying to attach to a container input.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On Feb. 15, 2017, 6:31 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54784/
> ---
> 
> (Updated Feb. 15, 2017, 6:31 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-6886
> https://issues.apache.org/jira/browse/MESOS-6886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces a test where a principal is allowed to launch a container
> session, but not to attach to its input, therefore its attempts should
> fail.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp aa7d89a710400d1460bc9139ea563cfdd45bd58f 
> 
> Diff: https://reviews.apache.org/r/54784/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 54783: Adds authorization tests of launching container sessions API.

[Adam B]

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


Ship it!




Looks like a fine test of unauthorized access. Authorized access is tested by 
`AgentAPITest.LaunchNestedContainerSession`

- Adam B


On Feb. 15, 2017, 6:30 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54783/
> ---
> 
> (Updated Feb. 15, 2017, 6:30 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-6886
> https://issues.apache.org/jira/browse/MESOS-6886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces a test which ensures that unauthorized users are unable to
> launch nested container sessions.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp aa7d89a710400d1460bc9139ea563cfdd45bd58f 
> 
> Diff: https://reviews.apache.org/r/54783/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 56619: Updated 'Files' handlers to use 'AuthenticationContext'.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On Feb. 17, 2017, 7 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56619/
> ---
> 
> (Updated Feb. 17, 2017, 7 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7003
> https://issues.apache.org/jira/browse/MESOS-7003
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the HTTP endpoint handlers in the
> Mesos `Files` process to accept an `AuthenticationContext`
> instead of an `Option& principal`.
> 
> 
> Diffs
> -
> 
>   src/files/files.hpp 8cffc26fc7d674187e55663f23f1e10bed40229e 
>   src/files/files.cpp 8327f8002fbfa3be77a4bbe4aa83a73d0f170f7a 
> 
> Diff: https://reviews.apache.org/r/56619/diff/
> 
> 
> Testing
> ---
> 
> Testing information can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 56618: Updated common Mesos code to use 'AuthenticationContext'.

[Adam B]

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



Lookin good


include/mesos/authorizer/authorizer.proto (lines 29 - 30)


"currently only a value" is no longer accurate


- Adam B


On Feb. 17, 2017, 2:34 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56618/
> ---
> 
> (Updated Feb. 17, 2017, 2:34 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7003
> https://issues.apache.org/jira/browse/MESOS-7003
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates common Mesos HTTP-related helpers,
> as well as the `authorization::Subject` protobuf
> message, to make use of the `AuthenticationContext`
> type instead of an `Option principal`.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 9cc75b0db17b2d0bab3f449f795cbf505c5b0f15 
>   src/common/http.hpp 3d5ab59ddc4dce4d791c1b439f5e1459d1a724a4 
>   src/common/http.cpp abfbf7248beb2d4068be06b7f5f829d7617f943e 
> 
> Diff: https://reviews.apache.org/r/56618/diff/
> 
> 
> Testing
> ---
> 
> Testing information can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 56617: Updated libprocess handlers to use 'AuthenticationContext'.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On Feb. 13, 2017, 7:41 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56617/
> ---
> 
> (Updated Feb. 13, 2017, 7:41 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7003
> https://issues.apache.org/jira/browse/MESOS-7003
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the HTTP endpoint handlers in libprocess
> to make use of the `AuthenticationContext` instead of an
> `Option& principal`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/logging.hpp 
> b5c2515605d5a9fab304f1425d29082f6167cea4 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> d9c64b7a4660182a5a23beed50a8e42b89d8b2aa 
>   3rdparty/libprocess/include/process/profiler.hpp 
> d19d647aa0d1172f09940fae1d1dfa83fd07b924 
>   3rdparty/libprocess/src/logging.cpp 
> cf7b43d2339df57ce899677419968d7e00ad8c38 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> 29c8c1da4e1424f029ccdf9fab890e8c1ec0ccb8 
>   3rdparty/libprocess/src/profiler.cpp 
> 2b3b6d5220aed9568f9fef51045cf322aa2934d7 
> 
> Diff: https://reviews.apache.org/r/56617/diff/
> 
> 
> Testing
> ---
> 
> Testing information can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 56623: Implemented 'AuthenticationContext' in libprocess.

[Adam B]

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



Looks good for a foundation, but I haven't looked at the rest of the patch 
chain yet


3rdparty/libprocess/include/process/authenticator.hpp (line 36)


"claims" or "labels"? I kinda like "claims"


- Adam B


On Feb. 17, 2017, 6:54 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56623/
> ---
> 
> (Updated Feb. 17, 2017, 6:54 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7003
> https://issues.apache.org/jira/browse/MESOS-7003
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a new struct, `AuthenticationContext`, to
> libprocess to represent an authenticated entity in the system.
> The new type contains a `principal` and a map containing
> arbitrary key-value pairs.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> e5489c6cb4adc8a822e7dd4515542618c36136f9 
>   3rdparty/libprocess/include/process/http.hpp 
> 3a406963c1defc037d58971f2400e40855503fbb 
>   3rdparty/libprocess/include/process/process.hpp 
> ceedbe86ccba5c24bf1e9e7f032d7dbcf6acb0f3 
>   3rdparty/libprocess/src/authenticator.cpp 
> cfedb6f7674e0f6690e77a633cdd1bd494c7d2c7 
>   3rdparty/libprocess/src/authenticator_manager.cpp 
> a22acd026a001788dc39b8005a56577e33c6800b 
>   3rdparty/libprocess/src/http.cpp 0e37936eec5b2910e54d40788c26f98665cbe1f1 
>   3rdparty/libprocess/src/process.cpp 
> 3ad485f9136cd9edf0a6db76ff1f475039236391 
> 
> Diff: https://reviews.apache.org/r/56623/diff/
> 
> 
> Testing
> ---
> 
> Testing information can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 55888: Test to ensure non-authorized users cannot launch tasks on agents.

[Jiang Yan Xu]

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



Hey looks like slave_authorization_tests.cpp is more suitable place to put this 
test?

- Jiang Yan Xu


On Feb. 7, 2017, 7:54 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55888/
> ---
> 
> (Updated Feb. 7, 2017, 7:54 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6953
> https://issues.apache.org/jira/browse/MESOS-6953
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Test to ensure non-authorized users cannot launch tasks on agents.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 7f357a653335b559b5c78d4618926715dc4a63c7 
> 
> Diff: https://reviews.apache.org/r/55888/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 55576: Fixes FutureTest.After3 flakiness.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [55576]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Feb. 20, 2017, 1:41 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55576/
> ---
> 
> (Updated Feb. 20, 2017, 1:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Hindman, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-6907
> https://issues.apache.org/jira/browse/MESOS-6907
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a flakiness in `FutureTest.After3` caused by the way
> libprocess processes timers.
> 
> Timers are not clear before setting `clock::settle` to 
> `false`, which make cause a race condition for callers which
> expect timers' thunks to be destroyed after the timer is
> called and the clock is settled.
> 
> The issue is addressed by explicitly calling 
> `std::list::clear()` on the `timedout` timer's list
> once the timers are executed.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/clock.cpp 6116c82377b1c2f33c5f4bc14890d35f34cc973d 
>   3rdparty/libprocess/src/tests/future_tests.cpp 
> 380755f9a2329d548969cfb2332c79aacbf7fff2 
> 
> Diff: https://reviews.apache.org/r/55576/diff/
> 
> 
> Testing
> ---
> 
> ```sh
> # Put the machine under stress.
> $ stress -c 4 -t 2600 -d 2 -i 2 &
> $ ./${MESOS_SRC_ROOT}/build/3rdparty/libprocess/libprocess-tests \
>   --gtest_filter="FutureTest.After3" --gtest_repeat=100 
> --gtest_break_on_failure
> $ ./${MESOS_SRC_ROOT}/build/build/src/mesos-tests
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 56493: Port_mapping isolator: do not depend on interface speed.

[Jie Yu]

> On Feb. 17, 2017, 10:51 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/port_mapping.cpp, lines 
> > 1601-1602
> > 
> >
> > I would still return Error here.
> 
> Pierre Cheynier wrote:
> I do not agree on that part. Here is what I experience and it's the 
> reason of this review:
> ```
> $ ls -la /sys/class/net/team0/speed
> -r--r--r-- 1 root root 4096 Feb 20 12:17 /sys/class/net/team0/speed
> # file /sys/class/net/team0/speed
> /sys/class/net/team0/speed: ERROR: cannot read (Invalid argument)
> # cat /sys/class/net/team0/speed
> cat: /sys/class/net/team0/speed: Invalid argument
> ```
> 
> Jie Yu wrote:
> Ah, ok, let me change this back.

Fixed.


- Jie


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


On Feb. 17, 2017, 3:51 p.m., Pierre Cheynier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56493/
> ---
> 
> (Updated Feb. 17, 2017, 3:51 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7074
> https://issues.apache.org/jira/browse/MESOS-7074
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since a lot of environments do not provide interface speed in
> /sys/class/net//speed`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> f6f2bfe1d5d3c113036ad44a480f97bbb462a269 
> 
> Diff: https://reviews.apache.org/r/56493/diff/
> 
> 
> Testing
> ---
> 
> ./configure
> make -j 8
> make check
> 
> 
> Thanks,
> 
> Pierre Cheynier
> 
>

Re: Review Request 56493: Port_mapping isolator: do not depend on interface speed.

[Jie Yu]

> On Feb. 17, 2017, 10:51 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/port_mapping.cpp, lines 
> > 1601-1602
> > 
> >
> > I would still return Error here.
> 
> Pierre Cheynier wrote:
> I do not agree on that part. Here is what I experience and it's the 
> reason of this review:
> ```
> $ ls -la /sys/class/net/team0/speed
> -r--r--r-- 1 root root 4096 Feb 20 12:17 /sys/class/net/team0/speed
> # file /sys/class/net/team0/speed
> /sys/class/net/team0/speed: ERROR: cannot read (Invalid argument)
> # cat /sys/class/net/team0/speed
> cat: /sys/class/net/team0/speed: Invalid argument
> ```

Ah, ok, let me change this back.


- Jie


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


On Feb. 17, 2017, 3:51 p.m., Pierre Cheynier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56493/
> ---
> 
> (Updated Feb. 17, 2017, 3:51 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7074
> https://issues.apache.org/jira/browse/MESOS-7074
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since a lot of environments do not provide interface speed in
> /sys/class/net//speed`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> f6f2bfe1d5d3c113036ad44a480f97bbb462a269 
> 
> Diff: https://reviews.apache.org/r/56493/diff/
> 
> 
> Testing
> ---
> 
> ./configure
> make -j 8
> make check
> 
> 
> Thanks,
> 
> Pierre Cheynier
> 
>

Re: Review Request 56847: Removed unnecessary leveldb build rules.

[haosdent huang]

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


Ship it!




I adjust to only build `libleveldb.a`

```
-all: $(SHARED_LIBS) $(SHARED_PROGRAMS) $(STATIC_OUTDIR)/libleveldb.a 
$(STATIC_OUTDIR)/libmemenv.a $(STATIC_PROGRAMS)
+all: $(STATIC_OUTDIR)/libleveldb.a
``

- haosdent huang


On Feb. 20, 2017, 4:45 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56847/
> ---
> 
> (Updated Feb. 20, 2017, 4:45 p.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Bugs: MESOS-7146
> https://issues.apache.org/jira/browse/MESOS-7146
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After upgraded to leveldb 1.19, the default CXXFLAGS of Mesos conflict
> with the require flags when building `db/c_test.c` with clang in macOS.
> This patch disables unnecessary rules in leveldb and bypass build
> failures in macOS.
> 
> 
> Diffs
> -
> 
>   3rdparty/leveldb-1.19.patch e281e9572438d91de5746114f19aa339e732bcf9 
> 
> Diff: https://reviews.apache.org/r/56847/diff/
> 
> 
> Testing
> ---
> 
> `make -j8` in macOS.
> `make -j24 GTEST_FILTER="*"` in CentOS 7.
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 56847: Removed unnecessary leveldb build rules.

[Tomasz Janiszewski]

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

(Updated Feb. 20, 2017, 4:45 p.m.)


Review request for mesos and haosdent huang.


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


Repository: mesos


Description (updated)
---

After upgraded to leveldb 1.19, the default CXXFLAGS of Mesos conflict
with the require flags when building `db/c_test.c` with clang in macOS.
This patch disables unnecessary rules in leveldb and bypass build
failures in macOS.


Diffs (updated)
-

  3rdparty/leveldb-1.19.patch e281e9572438d91de5746114f19aa339e732bcf9 

Diff: https://reviews.apache.org/r/56847/diff/


Testing (updated)
---

`make -j8` in macOS.
`make -j24 GTEST_FILTER="*"` in CentOS 7.


Thanks,

Tomasz Janiszewski

Re: Review Request 56847: Removed unnecessary leveldb build rules.

[Tomasz Janiszewski]

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

(Updated Feb. 20, 2017, 4:35 p.m.)


Review request for mesos and haosdent huang.


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


Repository: mesos


Description
---

After upgraded to leveldb 1.19, the default CXXFLAGS of Mesos conflict
with the require flags when building `db/c_test.c` with clang in macOS.
This patch disables unnecessary rules in leveldb and bypass build
failures in macOS.


Diffs
-

  3rdparty/leveldb-1.19.patch e281e9572438d91de5746114f19aa339e732bcf9 

Diff: https://reviews.apache.org/r/56847/diff/


Testing
---


Thanks,

Tomasz Janiszewski

Re: Review Request 56847: Removed unnecessary leveldb build rules.

[Tomasz Janiszewski]

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

(Updated Feb. 20, 2017, 4:33 p.m.)


Review request for mesos and haosdent huang.


Summary (updated)
-

Removed unnecessary leveldb build rules.


Repository: mesos


Description (updated)
---

After upgraded to leveldb 1.19, the default CXXFLAGS of Mesos conflict
with the require flags when building `db/c_test.c` with clang in macOS.
This patch disables unnecessary rules in leveldb and bypass build
failures in macOS.


Diffs
-

  3rdparty/leveldb-1.19.patch e281e9572438d91de5746114f19aa339e732bcf9 

Diff: https://reviews.apache.org/r/56847/diff/


Testing
---


Thanks,

Tomasz Janiszewski

Re: Review Request 56847: Remove unecessary leveldb build rule.

[haosdent huang]

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



According to my test, apply this change should be enough.
```
-all: $(SHARED_LIBS) $(SHARED_PROGRAMS) $(STATIC_OUTDIR)/libleveldb.a 
$(STATIC_OUTDIR)/libmemenv.a $(STATIC_PROGRAMS)
+all: $(STATIC_OUTDIR)/libleveldb.a
```

- haosdent huang


On Feb. 20, 2017, 11:52 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56847/
> ---
> 
> (Updated Feb. 20, 2017, 11:52 a.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Remove unecessary leveldb build rule.
> 
> 
> Diffs
> -
> 
>   3rdparty/leveldb-1.19.patch e281e9572438d91de5746114f19aa339e732bcf9 
> 
> Diff: https://reviews.apache.org/r/56847/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 51053: Update leveldb to 1.19.

[haosdent huang]

> On Aug. 22, 2016, 2:29 a.m., Vinod Kone wrote:
> > Have you also done compatibility tests mentioned in the ticket?
> 
> Tomasz Janiszewski wrote:
> No. It's minior upgrade from 1.18 to 1.19 but I can got thur 
> https://docs.google.com/document/d/1fv2OMvH6hVm6waacOejSrTJwUuDQeXlqqPDZjBmbcKU/edit#
>  and check if everything is OK.
> 
> haosdent huang wrote:
> I could help to run from 1.14 to 1.19 if it is necessary. :)
> 
> Tomasz Janiszewski wrote:
> I took care of it. Folloed steps presented by Haosdent it works the same. 
> Benchamrks were run as presented in ticket comments `MESOS_BENCHMARK=1 
> GTEST_FILTER="*BENCHMARK*.*/1" make check`. All tests performed on Ubuntu 
> 16.04 4.4.0-34-generic x86_64.
> I've got a problem with running `support/test-upgrade.py` it failed with 
> `mesos 1.0.0 framework failed` framework output ends with `Registered!`
> 
> haosdent huang wrote:
> As I test, `support/test-upgrade.py` requires pass all path with absolute 
> paths, otherwise would get 
> 
> ```
> sh: ./mesos_1.4/build/src/test-executor: No such file or directory
> ```
> 
> in stderr of tasks.
> 
> After use absolute paths as the parameters of `support/test-upgrade.py`, 
> everything works fine.
> 
> Alexander Rojas wrote:
> This patch managed to break the build in OS-X. On a first run it will 
> break with the following message:
> 
> ```
> checking if clang supports -c -o file.o... yes
> checking for poll.h... yes
> checking sys/select.h usability... checking if clang supports -fno-rtti 
> -fno-exceptions... yes
> checking if clang supports -c -o file.o... yes
> (cached) yes
> checking for clang option to produce PIC... checking whether the clang 
> linker (/usr/bin/ld) supports shared libraries... -fno-common -DPIC
> checking if clang PIC flag -fno-common -DPIC works... yes
> checking dynamic linker characteristics... mkdir out-shared
> clang++ -stdlib=libc++ -nostdinc++ 
> -I/usr/local/opt/llvm@3.8/lib/llvm-3.8/include/c++/v1 
> -Wno-deprecated-declarations  -fvisibility-inlines-hidden -fcolor-diagnostics 
> -Wno-unused-local-typedef -std=c++11 -stdlib=libc++ -DGTEST_USE_OWN_TR1
> _TUPLE=1 -DGTEST_LANG_CXX11 -I. -I./include -std=c++0x  -DOS_MACOSX 
> -DLEVELDB_PLATFORM_POSIX -DLEVELDB_ATOMIC_PRESENT -stdlib=libc++ -nostdinc++ 
> -I/usr/local/opt/llvm@3.8/lib/llvm-3.8/include/c++/v1 
> -Wno-deprecated-declarations  -fvisibil
> ity-inlines-hidden -fcolor-diagnostics -Wno-unused-local-typedef 
> -std=c++11 -stdlib=libc++ -DGTEST_USE_OWN_TR1_TUPLE=1 -DGTEST_LANG_CXX11 
> -fPIC -fPIC -c db/db_bench.cc -o out-shared/db/db_bench.o
> yes
> checking sys/select.h presence... error: unable to open output file 
> 'out-shared/db/db_bench.o': 'No such file or directory'
> 1 error generated.
> make[4]: *** [out-shared/db/db_bench.o] Error 1
> make[3]: *** [leveldb-1.19/out-static/libleveldb.a] Error 2
> make[3]: *** Waiting for unfinished jobs
> ```
> 
> if one re-runs the make command, then it fails with the following error:
> 
> ```
> clang -I. -I./include -DOS_MACOSX -DLEVELDB_PLATFORM_POSIX 
> -DLEVELDB_ATOMIC_PRESENT -stdlib=libc++ -nostdinc++ 
> -I/usr/local/opt/llvm@3.8/lib/llvm-3.8/include/c++/v1 
> -Wno-deprecated-declarations  -fvisibility-inlines-hidden -fcolor-diagnostics 
> -Wno-unused-local-typedef -std=c++11 -stdlib=libc++ 
> -DGTEST_USE_OWN_TR1_TUPLE=1 -DGTEST_LANG_CXX11 -fPIC -c db/c_test.c -o 
> out-static/db/c_test.o
> error: invalid argument '-std=c++11' not allowed with 'C/ObjC'
> ```
> 
> This one indicates that the C compiler is using `CXXFLAGS` instead of 
> `CFLAGS` or that `CFLAGS` are being wrongly generated. Running a thir time 
> the make command throes the following output:
> 
> ```
> clang -I. -I./include -DOS_MACOSX -DLEVELDB_PLATFORM_POSIX 
> -DLEVELDB_ATOMIC_PRESENT -stdlib=libc++ -nostdinc++ 
> -I/usr/local/opt/llvm@3.8/lib/llvm-3.8/include/c++/v1 
> -Wno-deprecated-declarations  -fvisibility-inlines-hidden -fcolor-diagno$
> tics -Wno-unused-local-typedef -std=c++11 -stdlib=libc++ 
> -DGTEST_USE_OWN_TR1_TUPLE=1 -DGTEST_LANG_CXX11 -fPIC -c db/c_test.c -o 
> out-static/db/c_test.o
> clang++ -L/usr/local/opt/subversion/lib -L/usr/local/opt/openssl/lib 
> -L/usr/local/opt/libevent/lib -L/usr/local/opt/apr/libexec/lib 
> -L/usr/local/opt/llvm/lib -Wl,-rpath,/usr/local/opt/llvm/lib  
> -fcolor-diagnostics  -stdlib=libc++ -nostdi$
> c++ -I/usr/local/opt/llvm@3.8/lib/llvm-3.8/include/c++/v1 
> -Wno-deprecated-declarations  -fvisibility-inlines-hidden -fcolor-diagnostics 
> -Wno-unused-local-typedef -std=c++11 -stdlib=libc++ 
> -DGTEST_USE_OWN_TR1_TUPLE=1 -DGTEST_LANG_CXX11 -I$
>  -I./include -std=c++0x  -DOS_MACOSX -DLEVELDB_PLATFORM_POSIX 
> -DLEVELDB_ATOMIC_PRESENT -stdlib=libc++ -nostdinc++ 
> -I/usr/local/opt/llvm@3.8/lib/llvm-3.8/include/c++/v1 
> -Wno-deprecated-declarations  

Re: Review Request 51053: Update leveldb to 1.19.

[Alexander Rojas]

> On Aug. 22, 2016, 4:29 a.m., Vinod Kone wrote:
> > Have you also done compatibility tests mentioned in the ticket?
> 
> Tomasz Janiszewski wrote:
> No. It's minior upgrade from 1.18 to 1.19 but I can got thur 
> https://docs.google.com/document/d/1fv2OMvH6hVm6waacOejSrTJwUuDQeXlqqPDZjBmbcKU/edit#
>  and check if everything is OK.
> 
> haosdent huang wrote:
> I could help to run from 1.14 to 1.19 if it is necessary. :)
> 
> Tomasz Janiszewski wrote:
> I took care of it. Folloed steps presented by Haosdent it works the same. 
> Benchamrks were run as presented in ticket comments `MESOS_BENCHMARK=1 
> GTEST_FILTER="*BENCHMARK*.*/1" make check`. All tests performed on Ubuntu 
> 16.04 4.4.0-34-generic x86_64.
> I've got a problem with running `support/test-upgrade.py` it failed with 
> `mesos 1.0.0 framework failed` framework output ends with `Registered!`
> 
> haosdent huang wrote:
> As I test, `support/test-upgrade.py` requires pass all path with absolute 
> paths, otherwise would get 
> 
> ```
> sh: ./mesos_1.4/build/src/test-executor: No such file or directory
> ```
> 
> in stderr of tasks.
> 
> After use absolute paths as the parameters of `support/test-upgrade.py`, 
> everything works fine.

This patch managed to break the build in OS-X. On a first run it will break 
with the following message:

```
checking if clang supports -c -o file.o... yes
checking for poll.h... yes
checking sys/select.h usability... checking if clang supports -fno-rtti 
-fno-exceptions... yes
checking if clang supports -c -o file.o... yes
(cached) yes
checking for clang option to produce PIC... checking whether the clang linker 
(/usr/bin/ld) supports shared libraries... -fno-common -DPIC
checking if clang PIC flag -fno-common -DPIC works... yes
checking dynamic linker characteristics... mkdir out-shared
clang++ -stdlib=libc++ -nostdinc++ 
-I/usr/local/opt/llvm@3.8/lib/llvm-3.8/include/c++/v1 
-Wno-deprecated-declarations  -fvisibility-inlines-hidden -fcolor-diagnostics 
-Wno-unused-local-typedef -std=c++11 -stdlib=libc++ -DGTEST_USE_OWN_TR1
_TUPLE=1 -DGTEST_LANG_CXX11 -I. -I./include -std=c++0x  -DOS_MACOSX 
-DLEVELDB_PLATFORM_POSIX -DLEVELDB_ATOMIC_PRESENT -stdlib=libc++ -nostdinc++ 
-I/usr/local/opt/llvm@3.8/lib/llvm-3.8/include/c++/v1 
-Wno-deprecated-declarations  -fvisibil
ity-inlines-hidden -fcolor-diagnostics -Wno-unused-local-typedef -std=c++11 
-stdlib=libc++ -DGTEST_USE_OWN_TR1_TUPLE=1 -DGTEST_LANG_CXX11 -fPIC -fPIC -c 
db/db_bench.cc -o out-shared/db/db_bench.o
yes
checking sys/select.h presence... error: unable to open output file 
'out-shared/db/db_bench.o': 'No such file or directory'
1 error generated.
make[4]: *** [out-shared/db/db_bench.o] Error 1
make[3]: *** [leveldb-1.19/out-static/libleveldb.a] Error 2
make[3]: *** Waiting for unfinished jobs
```

if one re-runs the make command, then it fails with the following error:

```
clang -I. -I./include -DOS_MACOSX -DLEVELDB_PLATFORM_POSIX 
-DLEVELDB_ATOMIC_PRESENT -stdlib=libc++ -nostdinc++ 
-I/usr/local/opt/llvm@3.8/lib/llvm-3.8/include/c++/v1 
-Wno-deprecated-declarations  -fvisibility-inlines-hidden -fcolor-diagnostics 
-Wno-unused-local-typedef -std=c++11 -stdlib=libc++ -DGTEST_USE_OWN_TR1_TUPLE=1 
-DGTEST_LANG_CXX11 -fPIC -c db/c_test.c -o out-static/db/c_test.o
error: invalid argument '-std=c++11' not allowed with 'C/ObjC'
```

This one indicates that the C compiler is using `CXXFLAGS` instead of `CFLAGS` 
or that `CFLAGS` are being wrongly generated. Running a thir time the make 
command throes the following output:

```
clang -I. -I./include -DOS_MACOSX -DLEVELDB_PLATFORM_POSIX 
-DLEVELDB_ATOMIC_PRESENT -stdlib=libc++ -nostdinc++ 
-I/usr/local/opt/llvm@3.8/lib/llvm-3.8/include/c++/v1 
-Wno-deprecated-declarations  -fvisibility-inlines-hidden -fcolor-diagno$
tics -Wno-unused-local-typedef -std=c++11 -stdlib=libc++ 
-DGTEST_USE_OWN_TR1_TUPLE=1 -DGTEST_LANG_CXX11 -fPIC -c db/c_test.c -o 
out-static/db/c_test.o
clang++ -L/usr/local/opt/subversion/lib -L/usr/local/opt/openssl/lib 
-L/usr/local/opt/libevent/lib -L/usr/local/opt/apr/libexec/lib 
-L/usr/local/opt/llvm/lib -Wl,-rpath,/usr/local/opt/llvm/lib  
-fcolor-diagnostics  -stdlib=libc++ -nostdi$
c++ -I/usr/local/opt/llvm@3.8/lib/llvm-3.8/include/c++/v1 
-Wno-deprecated-declarations  -fvisibility-inlines-hidden -fcolor-diagnostics 
-Wno-unused-local-typedef -std=c++11 -stdlib=libc++ -DGTEST_USE_OWN_TR1_TUPLE=1 
-DGTEST_LANG_CXX11 -I$
 -I./include -std=c++0x  -DOS_MACOSX -DLEVELDB_PLATFORM_POSIX 
-DLEVELDB_ATOMIC_PRESENT -stdlib=libc++ -nostdinc++ 
-I/usr/local/opt/llvm@3.8/lib/llvm-3.8/include/c++/v1 
-Wno-deprecated-declarations  -fvisibility-inlines-hidden -fcolor-dia$
nostics -Wno-unused-local-typedef -std=c++11 -stdlib=libc++ 
-DGTEST_USE_OWN_TR1_TUPLE=1 -DGTEST_LANG_CXX11 -fPIC db/corruption_test.cc 
out-static/db/builder.o out-static/db/c.o out-static/db/db_impl.o 
out-static/db/db_iter.o out-static/d$
/dbformat.o out-static/db/dumpfile.o 

Re: Review Request 56847: Remove unecessary leveldb build rule.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [56847]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Feb. 20, 2017, 11:52 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56847/
> ---
> 
> (Updated Feb. 20, 2017, 11:52 a.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Remove unecessary leveldb build rule.
> 
> 
> Diffs
> -
> 
>   3rdparty/leveldb-1.19.patch e281e9572438d91de5746114f19aa339e732bcf9 
> 
> Diff: https://reviews.apache.org/r/56847/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 55576: Fixes FutureTest.After3 flakiness.

[Alexander Rojas]

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

(Updated Feb. 20, 2017, 2:41 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Hindman, and Joris Van 
Remoortere.


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


Repository: mesos


Description (updated)
---

Fixed a flakiness in `FutureTest.After3` caused by the way
libprocess processes timers.

Timers are not clear before setting `clock::settle` to 
`false`, which make cause a race condition for callers which
expect timers' thunks to be destroyed after the timer is
called and the clock is settled.

The issue is addressed by explicitly calling 
`std::list::clear()` on the `timedout` timer's list
once the timers are executed.


Diffs (updated)
-

  3rdparty/libprocess/src/clock.cpp 6116c82377b1c2f33c5f4bc14890d35f34cc973d 
  3rdparty/libprocess/src/tests/future_tests.cpp 
380755f9a2329d548969cfb2332c79aacbf7fff2 

Diff: https://reviews.apache.org/r/55576/diff/


Testing
---

```sh
# Put the machine under stress.
$ stress -c 4 -t 2600 -d 2 -i 2 &
$ ./${MESOS_SRC_ROOT}/build/3rdparty/libprocess/libprocess-tests \
  --gtest_filter="FutureTest.After3" --gtest_repeat=100 
--gtest_break_on_failure
$ ./${MESOS_SRC_ROOT}/build/build/src/mesos-tests
```


Thanks,

Alexander Rojas

Review Request 56853: Update OCI tests with the latest OCI image spec.

[Qian Zhang]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Update OCI tests with the latest OCI image spec.


Diffs
-

  src/tests/containerizer/oci_spec_tests.cpp 
29a8f23caae3b13c5abfcec484ab8161b334713d 

Diff: https://reviews.apache.org/r/56853/diff/


Testing
---

make -j8 V=0 check GTEST_FILTER="OCISpecTest.*"


Thanks,

Qian Zhang

Review Request 56852: Updated OCI spec parsing & validation code with latest OCI image spec.

[Qian Zhang]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Updated OCI spec parsing & validation code with latest OCI image spec.


Diffs
-

  include/mesos/oci/spec.hpp ea4f29e3f5a552b671738d664882f08db682ad90 
  src/oci/spec.cpp 0a88edb404836205f11e61ba3edb771df8b618ef 

Diff: https://reviews.apache.org/r/56852/diff/


Testing
---


Thanks,

Qian Zhang

Review Request 56851: Updated OCI protobuf messages with latest OCI image spec.

[Qian Zhang]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Updated OCI protobuf messages with latest OCI image spec.


Diffs
-

  include/mesos/oci/spec.proto 97f3cf33fa89a0a15a6b7d52c6ca9f82c66bb28f 

Diff: https://reviews.apache.org/r/56851/diff/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 56742: Updated CHANGELOG for Mesos 1.2.0 release.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [56742]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Feb. 20, 2017, 9:22 a.m., Adam B wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56742/
> ---
> 
> (Updated Feb. 20, 2017, 9:22 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated CHANGELOG for Mesos 1.2.0 release.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 1478ad4dd3b20fc768fd679f81d803acf0995ad2 
> 
> Diff: https://reviews.apache.org/r/56742/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Adam B
> 
>

Re: Review Request 56493: Port_mapping isolator: do not depend on interface speed.

[Pierre Cheynier]

> On Feb. 17, 2017, 10:51 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/port_mapping.cpp, line 1597
> > 
> >
> > We don't add period in log lines.

Sorry for that. I see that you've already merged.


> On Feb. 17, 2017, 10:51 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/port_mapping.cpp, lines 
> > 1601-1602
> > 
> >
> > I would still return Error here.

I do not agree on that part. Here is what I experience and it's the reason of 
this review:
```
$ ls -la /sys/class/net/team0/speed
-r--r--r-- 1 root root 4096 Feb 20 12:17 /sys/class/net/team0/speed
# file /sys/class/net/team0/speed
/sys/class/net/team0/speed: ERROR: cannot read (Invalid argument)
# cat /sys/class/net/team0/speed
cat: /sys/class/net/team0/speed: Invalid argument
```


- Pierre


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


On Feb. 17, 2017, 3:51 p.m., Pierre Cheynier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56493/
> ---
> 
> (Updated Feb. 17, 2017, 3:51 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7074
> https://issues.apache.org/jira/browse/MESOS-7074
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since a lot of environments do not provide interface speed in
> /sys/class/net//speed`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> f6f2bfe1d5d3c113036ad44a480f97bbb462a269 
> 
> Diff: https://reviews.apache.org/r/56493/diff/
> 
> 
> Testing
> ---
> 
> ./configure
> make -j 8
> make check
> 
> 
> Thanks,
> 
> Pierre Cheynier
> 
>

Re: Review Request 56847: Remove unecessary leveldb build rule.

[Tomasz Janiszewski]

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

(Updated Feb. 20, 2017, 11:52 a.m.)


Review request for mesos and haosdent huang.


Repository: mesos


Description
---

Remove unecessary leveldb build rule.


Diffs (updated)
-

  3rdparty/leveldb-1.19.patch e281e9572438d91de5746114f19aa339e732bcf9 

Diff: https://reviews.apache.org/r/56847/diff/


Testing
---


Thanks,

Tomasz Janiszewski

Review Request 56847: Remove unecessary leveldb build rule.

[Tomasz Janiszewski]

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

Review request for mesos and haosdent huang.


Repository: mesos


Description
---

Remove unecessary leveldb build rule.


Diffs
-

  3rdparty/leveldb-1.19.patch e281e9572438d91de5746114f19aa339e732bcf9 

Diff: https://reviews.apache.org/r/56847/diff/


Testing
---


Thanks,

Tomasz Janiszewski

Re: Review Request 56537: Added a test to ensure framework can upgrade to support MULTI_ROLE.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [56537]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Feb. 20, 2017, 5:56 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56537/
> ---
> 
> (Updated Feb. 20, 2017, 5:56 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-7035
> https://issues.apache.org/jira/browse/MESOS-7035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A framework can be upgraded to be MULTI_ROLE capable even with task
> running, as long as it does not change role while upgrading.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 64443dc6e2ca4ab8f37269a0dced49908526649b 
> 
> Diff: https://reviews.apache.org/r/56537/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 56742: Updated CHANGELOG for Mesos 1.2.0 release.

[Adam B]

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

(Updated Feb. 20, 2017, 1:22 a.m.)


Review request for mesos, Jie Yu and Vinod Kone.


Changes
---

Updates!


Repository: mesos


Description
---

Updated CHANGELOG for Mesos 1.2.0 release.


Diffs (updated)
-

  CHANGELOG 1478ad4dd3b20fc768fd679f81d803acf0995ad2 

Diff: https://reviews.apache.org/r/56742/diff/


Testing
---


Thanks,

Adam B

Re: Review Request 56836: Added test for a task specifying on termination policy.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [56830, 56831, 56832, 56833, 56834, 56835, 56836]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Feb. 20, 2017, 5:01 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56836/
> ---
> 
> (Updated Feb. 20, 2017, 5:01 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7068
> https://issues.apache.org/jira/browse/MESOS-7068
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for a task specifying on termination policy.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> eaf63946735b28b21bfb7e16aca5924a5a82 
> 
> Diff: https://reviews.apache.org/r/56836/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>