Re: Review Request 53083: Made `cgroups::create` create cgroup recursively as default behaviour.

[haosdent huang]

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

(Updated Oct. 28, 2016, 5:15 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Jiang Yan Xu.


Changes
---

Rebase.


Repository: mesos


Description
---

Made `cgroups::create` create cgroup recursively as default behaviour.


Diffs (updated)
-

  src/linux/cgroups.hpp cfce09cb57501f2c988a8d997d7c6150280ed53d 
  src/linux/cgroups.cpp 630e2ec2d0eac2bb35488d0416883df1601224c8 
  src/tests/containerizer/cgroups_tests.cpp 
0afaec6ae948cabf1472bf01103210d8f9809cb1 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
e6c690c411f57138207044f31b4816bd4090c1b7 

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


Testing
---

```
sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*CGROUPS*" --verbose
sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*Cgroups*" --verbose
```


Thanks,

haosdent huang

Re: Review Request 53082: Refactored `ROOT_CGROUPS_CreateRemove` test case.

[haosdent huang]

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

(Updated Oct. 28, 2016, 5:14 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Jiang Yan Xu.


Changes
---

Rebase.


Repository: mesos


Description
---

This patch makes `ROOT_CGROUPS_CreateRemove` create cgroups with
`TEST_CGROUPS_ROOT` prefix in name to ensure cgroups would be clean up
during tear down.


Diffs (updated)
-

  src/tests/containerizer/cgroups_tests.cpp 
0afaec6ae948cabf1472bf01103210d8f9809cb1 

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


Testing
---

```
sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*CGROUPS*" --verbose
sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*Cgroups*" --verbose
```


Thanks,

haosdent huang

Re: Review Request 51185: Removed the expired TODO about non-recursive version `cgroups::get`.

[haosdent huang]

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

(Updated Oct. 28, 2016, 5:13 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, Zhengju Sha, and 
Jiang Yan Xu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed the expired TODO about non-recursive version `cgroups::get`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
2f6723c64261fb3295626d6479fe844fb23b0650 

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


Testing
---

```
sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*CGROUPS*" --verbose
sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*Cgroups*" --verbose
```


Thanks,

haosdent huang

Re: Review Request 51031: Added non-recursive version of `cgroups::get`.

[haosdent huang]

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

(Updated Oct. 28, 2016, 5:13 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, Zhengju Sha, and 
Jiang Yan Xu.


Changes
---

Address @jieyu's comments.


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


Repository: mesos


Description
---

In some cases, we just want to get the children cgroups instead of
retrieve descendant cgroups recursively. We added an argument to
`cgroups::get` to indicate whether to retrieve cgroups recursively but
made recursive retrieve the default behaviour. This patch fixed some
incorrect `TEST_CGROUPS_ROOT` checks as well.


Diffs (updated)
-

  src/linux/cgroups.hpp cfce09cb57501f2c988a8d997d7c6150280ed53d 
  src/linux/cgroups.cpp 630e2ec2d0eac2bb35488d0416883df1601224c8 
  src/tests/containerizer/cgroups_tests.cpp 
0afaec6ae948cabf1472bf01103210d8f9809cb1 

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


Testing
---

```
sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*" --verbose
```


Thanks,

haosdent huang

Review Request 53252: Added `hashset` constructor for type that supports `begin()/end()`.

[haosdent huang]

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

Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Add `hashset` constructor for any arbitrary type that supports
`begin()` and `end()`.


Diffs
-

  3rdparty/stout/include/stout/hashset.hpp 
8f633cad4d27c1e5f391279b7f3e8cb5fbf4bd03 

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


Testing
---


Thanks,

haosdent huang

Review Request 53253: Fixed the bug when search base hierarchy in `cgroups_tests.cpp`.

[haosdent huang]

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

Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Fixed the bug when search base hierarchy in `cgroups_tests.cpp`.


Diffs
-

  src/tests/containerizer/cgroups_tests.cpp 
0afaec6ae948cabf1472bf01103210d8f9809cb1 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 53058: Added tests for whole protobuf message based authorization.

[Alexander Rojas]

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

(Updated Oct. 28, 2016, 6:11 a.m.)


Review request for mesos, Adam B, Kapil Arya, and Till Toenshoff.


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


Repository: mesos


Description
---

Adds tests for the authorization of the authorization request
which makes use of whole protobuf messages.


Diffs (updated)
-

  src/tests/authorization_tests.cpp 5d7e17b67821357b8cb538798acc883945c8f8fd 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 52600: Enable multiple field based authorization in the authorizer interface.

[Alexander Rojas]

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

(Updated Oct. 28, 2016, 6:08 a.m.)


Review request for mesos, Adam B, Kapil Arya, and Till Toenshoff.


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


Repository: mesos


Description
---

Updates the authorizer interfaces and well as the local authorizer,
such that all actions which were limited to use a _role_ or a
_principal_ as an object, are able to use whole protobuf messages
as objects. This change enables more sophisticated authorization
mechanisms.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.hpp 
cb365c7d8d088f2810bde11b72dc20843a18fa51 
  include/mesos/authorizer/authorizer.proto 
b6a9f142eecbdfd59210872a92e3126f04de334c 
  src/authorizer/local/authorizer.cpp f1dff65d973fc84f4171f68fd0391a2343a96965 

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


Testing
---

tests in the last patch of the chain.


Thanks,

Alexander Rojas

Re: Review Request 51031: Added non-recursive version of `cgroups::get`.

[haosdent huang]

> On Oct. 25, 2016, 7:28 p.m., Jie Yu wrote:
> > src/tests/containerizer/cgroups_tests.cpp, lines 135-150
> > 
> >
> > We might need to cleanup this part. It's likely that `baseHierarchy` is 
> > not the parent of all the subsystems. Imagine a subsystem is not mounted 
> > initially. And we'll use TEST_CGROUPS_HIERARCHY as the base. However, if 
> > all other subsystems are mounted under /sys/fs/cgroup, then this test 
> > fixture will fail.
> > 
> > We can address that in the separate patch, but I'd like to cleanup this 
> > tech debt.

+1, I copy what we do in 
`ContainerizerTest::SetUp()` to here.


- haosdent


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


On Oct. 24, 2016, 2:39 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51031/
> ---
> 
> (Updated Oct. 24, 2016, 2:39 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, Zhengju Sha, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6035
> https://issues.apache.org/jira/browse/MESOS-6035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In some cases, we just want to get the children cgroups instead of
> retrieve descendant cgroups recursively. We added an argument to
> `cgroups::get` to indicate whether to retrieve cgroups recursively but
> made recursive retrieve the default behaviour. This patch fixed some
> incorrect `TEST_CGROUPS_ROOT` checks as well.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp cfce09cb57501f2c988a8d997d7c6150280ed53d 
>   src/linux/cgroups.cpp 630e2ec2d0eac2bb35488d0416883df1601224c8 
>   src/tests/containerizer/cgroups_tests.cpp 
> 0afaec6ae948cabf1472bf01103210d8f9809cb1 
> 
> Diff: https://reviews.apache.org/r/51031/diff/
> 
> 
> Testing
> ---
> 
> ```
> sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*" --verbose
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52765: Populated `recovered_slaves` in master `/state` endpoint.

[Anand Mazumdar]

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




src/master/http.cpp (lines 2652 - 2655)


You might want to create a helper overload function similar to the existing 
ones for `MasterInfo` etc. since `/slaves` endpoint would want to list the 
recovered agents too? I don't see it being added in this change.


- Anand Mazumdar


On Oct. 21, 2016, 6:40 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52765/
> ---
> 
> (Updated Oct. 21, 2016, 6:40 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Kunal Thakar.
> 
> 
> Bugs: MESOS-6177
> https://issues.apache.org/jira/browse/MESOS-6177
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Populated `recovered_slaves` in master `/state` endpoint.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 05d29906a73fb049c085abca05b75ec68c259d26 
> 
> Diff: https://reviews.apache.org/r/52765/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 52638: Populated `recovered_agents` field in `GetAgents` response.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On Oct. 21, 2016, 6:39 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52638/
> ---
> 
> (Updated Oct. 21, 2016, 6:39 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Kunal Thakar.
> 
> 
> Bugs: MESOS-6177
> https://issues.apache.org/jira/browse/MESOS-6177
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Populated `recovered_agents` field in `GetAgents` response.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 05d29906a73fb049c085abca05b75ec68c259d26 
> 
> Diff: https://reviews.apache.org/r/52638/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 52637: Added `recovered_agents` in `GetAgents` response.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On Oct. 21, 2016, 9:01 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52637/
> ---
> 
> (Updated Oct. 21, 2016, 9:01 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Kunal Thakar.
> 
> 
> Bugs: MESOS-6177
> https://issues.apache.org/jira/browse/MESOS-6177
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added infos of recovered but yet-to-reregister agents in the
> `GetAgents` response. This can be used by an operator or subscriber
> to know these agents are in this transitional state.
> One use case is to perform special handling on previously known
> tasks from these agents because these tasks are unknown to master yet.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 3553c683c17004ac1831ec90271aa8584c950e53 
>   include/mesos/v1/master/master.proto 
> 022b491b7d5c49c5aeddf4ffc97c148f55629c95 
> 
> Diff: https://reviews.apache.org/r/52637/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 53095: Make `slaves.recovered` in master a hashmap.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On Oct. 21, 2016, 6:38 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53095/
> ---
> 
> (Updated Oct. 21, 2016, 6:38 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Kunal Thakar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This information will be used to in both
> `GetAgents` API call and `/state` endpoint
> to return `AgentInfo` of recovered but unregistered
> slave, to facilitate operator to better handle
> such case.
> 
> The reason I want to return `AgentInfo` instead
> of `AgentID` is that hostname
> can be find in former, but not latter.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 6d2db9de52d35f3288c618d05138413ce709818b 
>   src/master/master.cpp 3f3ce93155069dd32731783ac4877ba6ee2519c0 
> 
> Diff: https://reviews.apache.org/r/53095/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 53116: Removed `ProvisionerProcess::__provision()`.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Oct. 25, 2016, 3:47 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53116/
> ---
> 
> (Updated Oct. 25, 2016, 3:47 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
> https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed `ProvisionerProcess::__provision()`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> c5481c585f2a55bb926340290b939f98af3c2135 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> de2c12140652244bd3de9763ced203b144688ab2 
> 
> Diff: https://reviews.apache.org/r/53116/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 53115: Implemented handling AUFS whiteouts for copy backend.

[Jie Yu]

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




src/slave/containerizer/mesos/provisioner/backends/copy.cpp (line 131)


I am not sure if we need to pass in 'image' to backend. If we added OCI 
support, do we need to add another check here?

I think we can simply assume the layer's whiteout is in aufs whiteout 
format, no matter what image type it is. I think APPC, we probably needs to do 
the same, right?

You probably add a NOTE there saying that we assume all image type uses 
aufs whiteout format.

In that way, no need to pass 'image' around to backend.



src/slave/containerizer/mesos/provisioner/backends/copy.cpp (line 150)


not yours, but can you fix the indentation here.



src/slave/containerizer/mesos/provisioner/backends/copy.cpp (line 211)


Hum, IIUC, looks like this is not entirely correct. According to this:
https://github.com/opencontainers/image-spec/blob/master/layer.md#whiteouts

"Files that are present in the same layer as a whiteout file can only be 
hidden by whiteout files in subsequent layers."

So we cannot cp first and then process whiteouts. Looks like before cp each 
layer, we need to do a pre-processing to handle whiteouts first (and remove the 
whiteouts), and then do the cp.



src/slave/containerizer/mesos/provisioner/backends/copy.cpp (lines 215 - 218)


Ditto on doing this in a less nesting way.



src/slave/containerizer/mesos/provisioner/backends/copy.cpp (line 218)


Ditto. string is not needed here.


- Jie Yu


On Oct. 25, 2016, 3:47 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53115/
> ---
> 
> (Updated Oct. 25, 2016, 3:47 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
> https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented handling AUFS whiteouts for copy backend.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/backend.hpp 
> 7257d3a962ecdf87fe9d52facbd6a2619311a018 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.hpp 
> a3c924195ae5eecc1caca9cd6fc0f6dc0df0a741 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.cpp 
> c92c6c7174421158b9190ed0bfb00c1c97ef0f7b 
>   src/slave/containerizer/mesos/provisioner/backends/bind.hpp 
> 54c88d940aa64d13114fc5d79ecbc1d474d169a6 
>   src/slave/containerizer/mesos/provisioner/backends/bind.cpp 
> 94dac40a12b6fd2e7d9733444d84763c77785402 
>   src/slave/containerizer/mesos/provisioner/backends/copy.hpp 
> 131d75521ca38afae651e8d885ebedad77d86a3e 
>   src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
> 9c5354e5fea488618ebdecf0aef9dd2fce555d20 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.hpp 
> 4d591c5f988d87e0e905f973df5ab15a3386d676 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
> b71a31323aef376c9a28e1d52322a1802fb212ad 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> de2c12140652244bd3de9763ced203b144688ab2 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> 727bf645dd9337a94f098ab0a816b7e0e0651083 
> 
> Diff: https://reviews.apache.org/r/53115/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 53249: Added MESOS-5788 to 1.1.0 CHANGELOG.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Added MESOS-5788 to 1.1.0 CHANGELOG.


Diffs
-

  CHANGELOG cf5ee8abc8359dd00fbc6ff93187d2b44fd6c40f 

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


Testing
---

Not a functional change.


Thanks,

Anand Mazumdar

Re: Review Request 52880: Added "launcher_dir" to the default executor flags.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52878, 53084, 53196, 53197, 52879, 52880]

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 Oct. 27, 2016, 5:34 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52880/
> ---
> 
> (Updated Oct. 27, 2016, 5:34 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent 
> huang, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6288
> https://issues.apache.org/jira/browse/MESOS-6288
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added "launcher_dir" to the default executor flags.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 769e998d80fde17bcb1ee6c5091ce13a1ad16137 
>   src/launcher/default_executor.hpp PRE-CREATION 
>   src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 
>   src/slave/slave.cpp 881c10ac61d1e4fdeabdc42c0a41508c36f49040 
> 
> Diff: https://reviews.apache.org/r/52880/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 53245: Added `MasterInfo` to the subscribed event.

[Joris Van Remoortere]

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


Fix it, then Ship it!





include/mesos/v1/scheduler/scheduler.proto (line 70)


Do you want to consider specifying which version sends this value?


- Joris Van Remoortere


On Oct. 27, 2016, 11:11 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53245/
> ---
> 
> (Updated Oct. 27, 2016, 11:11 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Bugs: MESOS-6497
> https://issues.apache.org/jira/browse/MESOS-6497
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This would be useful for schedulers that want more information
> about the master if they are using an alternate detector
> implementation that does not query Master ZK entry. Also,
> this is needed presently by the V0 -> V1 HTTP adapter. The
> old driver based schedulers used to get this information via
> the `Framework(Re-)registered` event.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler/scheduler.proto 
> 0576d903da0046c0bf88647c036b42fb260c0022 
>   include/mesos/v1/scheduler/scheduler.proto 
> a922a7f77e8fba4f6317765be4c3a36e65b096b6 
> 
> Diff: https://reviews.apache.org/r/53245/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 53245: Added `MasterInfo` to the subscribed event.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Oct. 27, 2016, 11:11 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53245/
> ---
> 
> (Updated Oct. 27, 2016, 11:11 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Bugs: MESOS-6497
> https://issues.apache.org/jira/browse/MESOS-6497
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This would be useful for schedulers that want more information
> about the master if they are using an alternate detector
> implementation that does not query Master ZK entry. Also,
> this is needed presently by the V0 -> V1 HTTP adapter. The
> old driver based schedulers used to get this information via
> the `Framework(Re-)registered` event.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler/scheduler.proto 
> 0576d903da0046c0bf88647c036b42fb260c0022 
>   include/mesos/v1/scheduler/scheduler.proto 
> a922a7f77e8fba4f6317765be4c3a36e65b096b6 
> 
> Diff: https://reviews.apache.org/r/53245/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 53245: Added `MasterInfo` to the subscribed event.

[Anand Mazumdar]

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

Review request for mesos, Joris Van Remoortere and Vinod Kone.


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


Repository: mesos


Description
---

This would be useful for schedulers that want more information
about the master if they are using an alternate detector
implementation that does not query Master ZK entry. Also,
this is needed presently by the V0 -> V1 HTTP adapter. The
old driver based schedulers used to get this information via
the `Framework(Re-)registered` event.


Diffs
-

  include/mesos/scheduler/scheduler.proto 
0576d903da0046c0bf88647c036b42fb260c0022 
  include/mesos/v1/scheduler/scheduler.proto 
a922a7f77e8fba4f6317765be4c3a36e65b096b6 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 53246: Populated `MasterInfo` evolving from v0 framework registered message.

[Anand Mazumdar]

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

Review request for mesos, Joris Van Remoortere and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/internal/evolve.cpp 8d5824fcb0f8026d290d0b47d4b6863f4e455fed 
  src/tests/scheduler_tests.cpp 6e876a7184204407c8ba4cb2300e095c29a6c8fb 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 53247: Populated `MasterInfo` in the v0 Java adapter.

[Anand Mazumdar]

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

Review request for mesos, Joris Van Remoortere and Vinod Kone.


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


Repository: mesos


Description
---

Populated `MasterInfo` in the v0 Java adapter.


Diffs
-

  src/examples/java/V1TestFramework.java 
ec269196671004d7faac500b7267cff756efe1fd 
  src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 
7febe95aa23960a3e1ca5e5be44e9ccf9fd840b8 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 53239: Changed master to make use of "retired" agent IDs.

[Neil Conway]

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

(Updated Oct. 27, 2016, 10:46 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Tweak metrics per TODO.


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


Repository: mesos


Description
---

A retired agent ID will never attempt to re-register in the future;
moreover, any tasks/executors being managed by that agent ID are no
longer running. We can take advantage of this knowledge to avoid waiting
for `agent_reregister_timeout` to expire after master failover.

This is particularly important when agent removal rate-limiting is in
use: if a power failure causes the master to fail at the same time that
many agent hosts lose power, when power returns the master will failover
and all the agents will register anew and receive new agent IDs. With
agent removal rate-limiting, it may take a long time for the master to
mark all the old agent IDs as unreachable; in the meantime, explicit
reconciliation will not return any results, potentially leaving
frameworks in limbo for an extended period.

Note that we currently mark retired agents as unreachable; in the near
future, that will change to marking such agents "gone", once support for
that feature is completed.


Diffs (updated)
-

  src/master/master.hpp 87186c6e733a686f96528b1722fda1c287e9c881 
  src/master/master.cpp 8692726d21812827f9e1fd9093d80fd260588ecb 
  src/tests/slave_recovery_tests.cpp 65fc18bc2732dc53581d39ee23368e347f0b2ca4 

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


Testing
---

`make check`

NOTE: Current implementation reuses `Master::markUnreachableAfterFailover`, 
which means we emit misleading log messages and increment the wrong metrics. 
Will adjust based on initial review comments.


Thanks,

Neil Conway

Re: Review Request 53227: Added the '--launcher_dir' flag to the default executor.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53227]

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 Oct. 27, 2016, 4:13 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53227/
> ---
> 
> (Updated Oct. 27, 2016, 4:13 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-6288
> https://issues.apache.org/jira/browse/MESOS-6288
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the '--launcher_dir' flag to the default executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 
>   src/slave/slave.cpp 881c10ac61d1e4fdeabdc42c0a41508c36f49040 
> 
> Diff: https://reviews.apache.org/r/53227/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 53202: Avoided CHECK failure with pre-1.0 agents.

[Jiang Yan Xu]

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


Ship it!




Ship It!

- Jiang Yan Xu


On Oct. 27, 2016, 2:11 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53202/
> ---
> 
> (Updated Oct. 27, 2016, 2:11 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6483
> https://issues.apache.org/jira/browse/MESOS-6483
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We don't guarantee compatibility with pre-1.0 agents. However, since it
> is easy to avoid a CHECK failure in the master when an old agent
> re-registers, it seems worth doing so.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 8692726d21812827f9e1fd9093d80fd260588ecb 
> 
> Diff: https://reviews.apache.org/r/53202/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Disabled the code that fills-in `frameworks.recovered`; verified that 
> `PartitionTest.DisconnectedFramework` dies with a `CHECK` failure if this RR 
> is not applied but passes this with RR applied.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 53202: Avoided CHECK failure with pre-1.0 agents.

[Neil Conway]

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

(Updated Oct. 27, 2016, 9:11 p.m.)


Review request for mesos, Vinod Kone and Jiang Yan Xu.


Changes
---

Tweak comment per review discussion.


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


Repository: mesos


Description
---

We don't guarantee compatibility with pre-1.0 agents. However, since it
is easy to avoid a CHECK failure in the master when an old agent
re-registers, it seems worth doing so.


Diffs (updated)
-

  src/master/master.cpp 8692726d21812827f9e1fd9093d80fd260588ecb 

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


Testing
---

`make check`

Disabled the code that fills-in `frameworks.recovered`; verified that 
`PartitionTest.DisconnectedFramework` dies with a `CHECK` failure if this RR is 
not applied but passes this with RR applied.


Thanks,

Neil Conway

Re: Review Request 52934: Explain read-only mode of persistent volumes in shared-resources.md.

[Anindya Sinha]

> On Oct. 17, 2016, 6:22 p.m., Neil Conway wrote:
> > docs/shared-resources.md, line 168
> > 
> >
> > I think we should discuss what happens if you try to write to a 
> > read-only volume.
> 
> Jiang Yan Xu wrote:
> SG.

We only allow persistent volumes as `RW`, they can never exist as `RO` (as in 
`Option validatePersistentVolume(const RepeatedPtrField& 
volumes)`. MESOS-6374 once implemented would not require 
`Resource::DiskInfo::Volume` to be included while doing a `CREATE` of 
persistent volumes (mode shall default to `RW`).

Tasks can request through the resources in `LAUNCH` if it wishes to use the 
volume as `RO` or `RW`. In case the task tries to write to a persistent volume 
it had requested `RO` access for, the task fails. Is there any other potential 
usage behavior that can be incorporated here as far as the task is concerned?


- Anindya


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


On Oct. 17, 2016, 6:15 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52934/
> ---
> 
> (Updated Oct. 17, 2016, 6:15 p.m.)
> 
> 
> Review request for mesos, (Disabled_DoNotUse) Anindya Sinha and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Explain read-only mode of persistent volumes in shared-resources.md.
> 
> 
> Diffs
> -
> 
>   docs/shared-resources.md 29e43389c530f816118a32dcd7c2f1b82b0af431 
> 
> Diff: https://reviews.apache.org/r/52934/diff/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 53208: Fixed Master that leaks empty entries in its hashmaps.

[Jiang Yan Xu]

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

(Updated Oct. 27, 2016, 1:27 p.m.)


Review request for mesos, (Disabled_DoNotUse) Anindya Sinha, Benjamin Mahler, 
Megha Sharma, and Neil Conway.


Bugs: MESOS-4975 and MESOS-6482
https://issues.apache.org/jira/browse/MESOS-4975
https://issues.apache.org/jira/browse/MESOS-6482


Repository: mesos


Description
---

This has also caused CHECK failures mentioned in MESOS-6482.


Diffs
-

  src/master/master.cpp 23ddb995b4ad0fcdb589974308a2e81ececdad31 

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


Testing (updated)
---

make check

MESOS-6482 exposes a CHECK failure which is going to be removed in /r/53202/, 
given that I just modified the `PartitionTest.DisconnectedFramework` to do a 
one-off verification: when the slave is marked unreachable after the framework 
is destroyed, without this patch the CHECK failure in MESOS-6482 occurs and 
with the patch it doesn't.


Thanks,

Jiang Yan Xu

Re: Review Request 53208: Fixed Master that leaks empty entries in its hashmaps.

[Anindya Sinha]

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


Ship it!




Ship It!

- Anindya Sinha


On Oct. 27, 2016, 7:06 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53208/
> ---
> 
> (Updated Oct. 27, 2016, 7:06 p.m.)
> 
> 
> Review request for mesos, (Disabled_DoNotUse) Anindya Sinha, Benjamin Mahler, 
> Megha Sharma, and Neil Conway.
> 
> 
> Bugs: MESOS-4975 and MESOS-6482
> https://issues.apache.org/jira/browse/MESOS-4975
> https://issues.apache.org/jira/browse/MESOS-6482
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This has also caused CHECK failures mentioned in MESOS-6482.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 23ddb995b4ad0fcdb589974308a2e81ececdad31 
> 
> Diff: https://reviews.apache.org/r/53208/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> No new test is added for this fix (MESOS-4975) along, but will add one for 
> MESOS-6482 (to verify that this patch fixes it).
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 53161: Implemented the conversion from AUFS whiteouts to OverlayFS whiteouts.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/utils.hpp (line 34)


I think this method should be moved to 
`src/slave/containerizer/mesos/provisioner/utils.hpp|cpp` since this is 
provision specific.

THe top level utils file is used for general utils used by Mesos 
containerizer.



src/slave/containerizer/mesos/utils.cpp (line 52)


I'd rename 'source' to 'directory'.



src/slave/containerizer/mesos/utils.cpp (line 55)


i'd prefer a new line above



src/slave/containerizer/mesos/utils.cpp (lines 62 - 65)


Can we do the following to reduce the level of nesting:
```
if (node->fts_info != FTS_F) {
  continue;
}

if (!strings::startsWith(node->fts_name, docker::spec::WHITEOUT_PREFIX)) {
  continue;
}

...
```



src/slave/containerizer/mesos/utils.cpp (line 65)


Do you need `string` here?



src/slave/containerizer/mesos/utils.cpp (lines 68 - 69)


I'd format like the following:
```
Try setxattr = os::setxattr(
path.dirname(),
"trusted.overlay.opaque",
"y",
0);
```



src/slave/containerizer/mesos/utils.cpp (line 74)


Why backslash?



src/slave/containerizer/mesos/utils.cpp (lines 92 - 99)


Can you double check if it is safe to remove files while walking the fts 
tree? To be more defensive, i would probably remove them after finishing the 
walk.


- Jie Yu


On Oct. 25, 2016, 3:46 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53161/
> ---
> 
> (Updated Oct. 25, 2016, 3:46 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
> https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the conversion from AUFS whiteouts to OverlayFS whiteouts.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> e192f86a1848b373f3aa73d29688a96375cac313 
>   src/slave/containerizer/mesos/utils.hpp 
> 178ebf3effac824e4788d7282795c18dc1cb5265 
>   src/slave/containerizer/mesos/utils.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53161/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 52886: Fix new sign comparison errors in stout produced by hardened flags

[Aaron Wood]

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

(Updated Oct. 27, 2016, 7:32 p.m.)


Review request for mesos, James Peach, Michael Park, and Neil Conway.


Changes
---

Addressed comments about casting/types.


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


Repository: mesos


Description
---

The hardening flags produced many new sign comparison errors in stout that need 
to be fixed for Mesos to compile/run.


Diffs (updated)
-

  3rdparty/stout/tests/cache_tests.cpp 0950c85 
  3rdparty/stout/tests/flags_tests.cpp da4deb9 
  3rdparty/stout/tests/hashmap_tests.cpp 2626d67 
  3rdparty/stout/tests/hashset_tests.cpp 66e59db 
  3rdparty/stout/tests/ip_tests.cpp b5a206f 
  3rdparty/stout/tests/json_tests.cpp 2bc4c88 
  3rdparty/stout/tests/linkedhashmap_tests.cpp 7a80769 
  3rdparty/stout/tests/multimap_tests.cpp 488991b 
  3rdparty/stout/tests/os/process_tests.cpp 4cb3b5f 
  3rdparty/stout/tests/os/sendfile_tests.cpp e221689 
  3rdparty/stout/tests/os/systems_tests.cpp 110ba5b 
  3rdparty/stout/tests/os_tests.cpp 0b7ee07 
  3rdparty/stout/tests/strings_tests.cpp 7dd3301 

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


Testing
---

Made sure compilation, tests, and benchmarks worked with both gcc and clang. 
Ran make && make check && make bench.


Thanks,

Aaron Wood

Re: Review Request 53230: Added test cases for HTTPS health check.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53230]

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 Oct. 27, 2016, 4:06 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53230/
> ---
> 
> (Updated Oct. 27, 2016, 4:06 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Bugs: MESOS-6493
> https://issues.apache.org/jira/browse/MESOS-6493
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for HTTPS health check.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp b1a8942eb1841f5f795b01e08cb017eedf62c8f4 
> 
> Diff: https://reviews.apache.org/r/53230/diff/
> 
> 
> Testing
> ---
> 
> ```
> sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*HealthCheckTest*" 
> --verbose
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53208: Fixed Master that leaks empty entries in its hashmaps.

[Jiang Yan Xu]

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

(Updated Oct. 27, 2016, 12:06 p.m.)


Review request for mesos, (Disabled_DoNotUse) Anindya Sinha, Benjamin Mahler, 
Megha Sharma, and Neil Conway.


Bugs: MESOS-4975 and MESOS-6482
https://issues.apache.org/jira/browse/MESOS-4975
https://issues.apache.org/jira/browse/MESOS-6482


Repository: mesos


Description
---

This has also caused CHECK failures mentioned in MESOS-6482.


Diffs
-

  src/master/master.cpp 23ddb995b4ad0fcdb589974308a2e81ececdad31 

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


Testing (updated)
---

make check

No new test is added for this fix (MESOS-4975) along, but will add one for 
MESOS-6482 (to verify that this patch fixes it).


Thanks,

Jiang Yan Xu

Review Request 53239: Changed master to make use of "retired" agent IDs.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

A retired agent ID will never attempt to re-register in the future;
moreover, any tasks/executors being managed by that agent ID are no
longer running. We can take advantage of this knowledge to avoid waiting
for `agent_reregister_timeout` to expire after master failover.

This is particularly important when agent removal rate-limiting is in
use: if a power failure causes the master to fail at the same time that
many agent hosts lose power, when power returns the master will failover
and all the agents will register anew and receive new agent IDs. With
agent removal rate-limiting, it may take a long time for the master to
mark all the old agent IDs as unreachable; in the meantime, explicit
reconciliation will not return any results, potentially leaving
frameworks in limbo for an extended period.

Note that we currently mark retired agents as unreachable; in the near
future, that will change to marking such agents "gone", once support for
that feature is completed.


Diffs
-

  src/master/master.hpp 87186c6e733a686f96528b1722fda1c287e9c881 
  src/master/master.cpp 23ddb995b4ad0fcdb589974308a2e81ececdad31 
  src/tests/slave_recovery_tests.cpp 65fc18bc2732dc53581d39ee23368e347f0b2ca4 

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


Testing
---

`make check`

NOTE: Current implementation reuses `Master::markUnreachableAfterFailover`, 
which means we emit misleading log messages and increment the wrong metrics. 
Will adjust based on initial review comments.


Thanks,

Neil Conway

Review Request 53238: Changed agent to report "retired" agent IDs on registration.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

If the agent observes that the boot ID has changed since the last time
it successfully (re-)registered with the master, we know that any agent
IDs that were previously in use in this `work_dir` must be associated
with different boot IDs; hence, any executors/tasks associated with
those agent IDs are no longer running, and those agent ID will never
attempt to re-register with the master. The agent now reports these
retired agent IDs to the master; the master will later take advantage of
this information, e.g., to learn that an agent that we think might
re-register after master failover will never re-register.


Diffs
-

  src/messages/messages.proto 7cbac56099689ffc378d83bae3a16abb49b50dd9 
  src/slave/slave.hpp c0a17657e684e87d555731d2e35ac15894c3c05b 
  src/slave/slave.cpp 881c10ac61d1e4fdeabdc42c0a41508c36f49040 
  src/slave/state.hpp a1c849690a5c2b3fc5ea3eb2e782e99a2b0f8044 
  src/slave/state.cpp a94bb8d7029295abef70d6595ebc732ac1ab87a8 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 53237: Improved slave recovery tests to check reconciliation, metrics.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Improved slave recovery tests to check reconciliation, metrics.


Diffs
-

  src/tests/slave_recovery_tests.cpp 65fc18bc2732dc53581d39ee23368e347f0b2ca4 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 53236: Cleaned up Master::markUnreachableAfterFailover.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Cleaned up Master::markUnreachableAfterFailover.


Diffs
-

  src/master/master.hpp 87186c6e733a686f96528b1722fda1c287e9c881 
  src/master/master.cpp 23ddb995b4ad0fcdb589974308a2e81ececdad31 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 53235: Cleaned up header includes and `using`.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Cleaned up header includes and `using`.


Diffs
-

  src/slave/state.hpp a1c849690a5c2b3fc5ea3eb2e782e99a2b0f8044 
  src/slave/state.cpp a94bb8d7029295abef70d6595ebc732ac1ab87a8 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 53234: Logged warning message if reading previous boot ID fails.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Not much we can do here, but as a general rule, we should never silently
ignore I/O errors.


Diffs
-

  src/slave/state.cpp a94bb8d7029295abef70d6595ebc732ac1ab87a8 

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


Testing
---

`make check`

Tested manually by doing `chmod 000` of the boot-id file.


Thanks,

Neil Conway

Review Request 53233: Simplified `State::recover` interface in the agent.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

`State::recover` previously returned `Result`, which implied
three possibilities: Error, None (if the `work_dir` doesn't exist), or a
`State` value. However, the agent code treated the latter two cases the
same way: if the `work_dir` doesn't exist, it is simpler to represent
this case by returning a `State` value whose fields are `None()`.


Diffs
-

  src/slave/slave.hpp c0a17657e684e87d555731d2e35ac15894c3c05b 
  src/slave/slave.cpp 881c10ac61d1e4fdeabdc42c0a41508c36f49040 
  src/slave/state.hpp a1c849690a5c2b3fc5ea3eb2e782e99a2b0f8044 
  src/slave/state.cpp a94bb8d7029295abef70d6595ebc732ac1ab87a8 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 53202: Avoided CHECK failure with pre-1.0 agents.

[Jiang Yan Xu]

> On Oct. 26, 2016, 2:43 p.m., Jiang Yan Xu wrote:
> > src/master/master.cpp, lines 6036-6037
> > 
> >
> > Assuming frameworks are not partition-aware based on the agent verison 
> > doesn't feel right.
> > 
> > Ultimately it doesn't seem to make a difference in terms of messages 
> > Mesos sends: if the framework is not connected, no update is sent in this 
> > method. Later when it reconnects and reconciles, Master checks its 
> > capability and decides: `(5) Task is unknown, slave is unreachable: 
> > TASK_UNREACHABLE` or `TASK_LOST` if the frameworks is not partition-aware.
> > 
> > Would it make sense to set the state to `TASK_UNREACHABLE` in this 
> > case? Looks like the only differences it makes are:
> > 
> > - - metrics: Regardless of framework capabilities, the agent is indeed 
> > unreachable: `TASK_UNREACHABLE` is more in line with the (1.1) master's 
> > logic and the metrics don't reflect 100% of what the master sends out 
> > anyways.
> > - documentation: we set the state because it makes sense to the master 
> > and not by guessing the framework's capabilities. also worth-mentioning is 
> > the fact that this doesn't violate the API semantics: partition-awareness 
> > is checked at reconciliaton time.
> 
> Neil Conway wrote:
> I don't think either option is ideal. I opted for `TASK_LOST` because:
> 
> (a) sending `TASK_LOST` is still the default behavior; 
> partition-awareness is an "experimental" feature in 1.1, and in any case is 
> guarded behind a capability.
> (b) if you have (very) old agents, you're more likely to have old 
> frameworks that haven't enabled partition-awareness.
> 
> Jiang Yan Xu wrote:
> OK I am not adamant about `TASK_LOST` vs `TASK_UNREACHABLE` as long as 
> it's not sent out but w.r.t the rationale:
> 
> a) I don't feel this has to do with the feature being "experimental", 
> imagine we are at 1.2 and we've promoted this feature as stable and we have 
> marked the following
> 
> ```
>   // In Mesos 1.2, this will only be sent when the framework does NOT
>   // opt-in to the PARTITION_AWARE capability.
>   TASK_LOST = 5;
> ```
> we still have this problem, would we solve it differently? If not, we 
> might as well don't take "experimental" into consideration here.
> 
> b) this is more about documentation but even if we use `TASK_LOST`, can 
> we avoid saying we are basing our decision on the likelyhood of old 
> frameworks because of old agents? i.e., my original comment: *we set the 
> state because it makes sense to the master and not by guessing the 
> framework's capabilities. also worth-mentioning is the fact that this doesn't 
> violate the API semantics: partition-awareness is checked at reconciliaton 
> time*.
> 
> Neil Conway wrote:
> Re: (a), we can only guarantee that `TASK_LOST` will not be sent to 
> partition-aware frameworks if all the agents in the cluster have been 
> upgraded to Mesos >= 1.2 -- otherwise, an old agent might generate a 
> `TASK_LOST` status update directly (e.g., due to an agent-local task launch 
> failure).
> 
> Re: (b) sure, that was only a minor point -- but I think we should err on 
> the side of not breaking old frameworks if we can't know the right thing to 
> do, at least for 1.1.

a) Ok I only just realized this now. Thanks for the clarification!
b) True, we should err on the side of not breaking old frameworks but here we 
don't break them either way. Can we update the comment to not say "assuming the 
framework isn't partition-aware" but rather something like "because TASK_LOST 
is still the default state (as of Mesos 1.1) in this situation, even if the 
disconnected framework is partition-aware"?

I just think the decision here is really made but by assuming the framework is 
not partition-aware.


- Jiang Yan


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


On Oct. 27, 2016, 10:16 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53202/
> ---
> 
> (Updated Oct. 27, 2016, 10:16 a.m.)
> 
> 
> Review request for mesos, Vinod Kone and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6483
> https://issues.apache.org/jira/browse/MESOS-6483
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We don't guarantee compatibility with pre-1.0 agents. However, since it
> is easy to avoid a CHECK failure in the master when an old agent
> re-registers, it seems worth doing so.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 23ddb995b4ad0fcdb589974308a2e81ececdad31 
> 
> Diff: 

Re: Review Request 52880: Added "launcher_dir" to the default executor flags.

[Gastón Kleiman]

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

(Updated Oct. 27, 2016, 5:34 p.m.)


Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent huang, 
Jie Yu, and Jiang Yan Xu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added "launcher_dir" to the default executor flags.


Diffs (updated)
-

  src/Makefile.am 769e998d80fde17bcb1ee6c5091ce13a1ad16137 
  src/launcher/default_executor.hpp PRE-CREATION 
  src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 
  src/slave/slave.cpp 881c10ac61d1e4fdeabdc42c0a41508c36f49040 

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


Testing
---

`make check`


Thanks,

Gastón Kleiman

Re: Review Request 52879: Cleaned up the way in which the executors load configuration options.

[Gastón Kleiman]

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

(Updated Oct. 27, 2016, 5:34 p.m.)


Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent huang, 
Jie Yu, and Jiang Yan Xu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Updated the executors so that they use only `stout::flags` to load
config options.

They used to use a mix of `stout::flags` and `os::getenv`.


Diffs (updated)
-

  src/CMakeLists.txt 639f8678ba23c4d9a2ea0bf84fbc3d6fc9286ef3 
  src/Makefile.am 769e998d80fde17bcb1ee6c5091ce13a1ad16137 
  src/docker/executor.hpp 8385631fd170f97b28c4ca3596255ab0546774d6 
  src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 
  src/exec/exec.cpp 1dc20390907253a466b7272b7f8b33ea14afb236 
  src/exec/flags.hpp PRE-CREATION 
  src/exec/flags.cpp PRE-CREATION 
  src/executor/executor.cpp 1d47b52d89eedee59d160badd6313d34e3bdb6d2 
  src/executor/flags.hpp PRE-CREATION 
  src/executor/flags.cpp PRE-CREATION 
  src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 
  src/launcher/executor.hpp 217680d99d6e8c31130d7dc714f8cd730f360852 
  src/launcher/executor.cpp 0544121e679db503fe4eaf23a24e315eb188a520 
  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 
  src/tests/containerizer.cpp 69b93c70dc883e1edff3e6c2d7c8da9ef05e42f8 

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


Testing
---

`make check`


Thanks,

Gastón Kleiman

Re: Review Request 53084: Support `LIBPROCESS_SSL_ENABLED` in the default executor and scheduler.

[Gastón Kleiman]

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

(Updated Oct. 27, 2016, 5:32 p.m.)


Review request for mesos, Alexander Rukletsov, haosdent huang, Jie Yu, Till 
Toenshoff, and Jiang Yan Xu.


Changes
---

Address all the feedback, keep it comin'!


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


Repository: mesos


Description
---

`LIBPROCESS_SSL_ENABLED` is preferred over `SSL_ENABLED`, and the latter
will be deprecated soon.


Diffs (updated)
-

  src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 
  src/scheduler/scheduler.cpp e282d419119dd1f01e170acf5cc2c6175b59791d 

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


Testing
---

make check


Thanks,

Gastón Kleiman

Re: Review Request 53208: Fixed Master that leaks empty entries in its hashmaps.

[Jiang Yan Xu]

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

(Updated Oct. 27, 2016, 10:27 a.m.)


Review request for mesos, (Disabled_DoNotUse) Anindya Sinha, Megha Sharma, and 
Neil Conway.


Changes
---

BenM's comments.


Bugs: MESOS-4975 and MESOS-6482
https://issues.apache.org/jira/browse/MESOS-4975
https://issues.apache.org/jira/browse/MESOS-6482


Repository: mesos


Description
---

This has also caused CHECK failures mentioned in MESOS-6482.


Diffs (updated)
-

  src/master/master.cpp 23ddb995b4ad0fcdb589974308a2e81ececdad31 

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


Testing
---

make check


Thanks,

Jiang Yan Xu

Re: Review Request 53053: Divided utils.hpp to utils.hpp and utils.cpp.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Oct. 25, 2016, 3:43 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53053/
> ---
> 
> (Updated Oct. 25, 2016, 3:43 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
> https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Divided utils.hpp to utils.hpp and utils.cpp.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 769e998d80fde17bcb1ee6c5091ce13a1ad16137 
>   src/slave/containerizer/mesos/utils.hpp 
> 178ebf3effac824e4788d7282795c18dc1cb5265 
>   src/slave/containerizer/mesos/utils.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53053/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 53202: Avoided CHECK failure with pre-1.0 agents.

[Neil Conway]

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

(Updated Oct. 27, 2016, 5:16 p.m.)


Review request for mesos, Vinod Kone and Jiang Yan Xu.


Changes
---

Add a warning message if partition-awareness cannot be determined accurately.


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


Repository: mesos


Description
---

We don't guarantee compatibility with pre-1.0 agents. However, since it
is easy to avoid a CHECK failure in the master when an old agent
re-registers, it seems worth doing so.


Diffs (updated)
-

  src/master/master.cpp 23ddb995b4ad0fcdb589974308a2e81ececdad31 

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


Testing
---

`make check`

Disabled the code that fills-in `frameworks.recovered`; verified that 
`PartitionTest.DisconnectedFramework` dies with a `CHECK` failure if this RR is 
not applied but passes this with RR applied.


Thanks,

Neil Conway

Re: Review Request 53202: Avoided CHECK failure with pre-1.0 agents.

[Neil Conway]

> On Oct. 26, 2016, 9:54 p.m., Jiang Yan Xu wrote:
> > src/master/master.cpp, line 6047
> > 
> >
> > If neither of the above is true, can we log a warning? We recommend 
> > aganist this situation but in operations there's always possbility of 
> > straggler hosts. A warning would be helpful.
> 
> Neil Conway wrote:
> Logging a warning seems a bit ugly because it seems like an ad-hoc place 
> to put a version compatibility check; if we have N places in the code that 
> contain such warnings, it seems like it will be annoying to maintain and 
> result in ugly log output.
> 
> I'd prefer to log a warning when an agent with an unsupported version 
> registers/re-registers with the master.
> 
> Jiang Yan Xu wrote:
> If we flat out reject connection from a component that doesn't meet the 
> version compatiblity requirement (a mechanism which we don't have today) and 
> log the client version and the reason for rejection there it would be clean, 
> but that's only one case. Since we are talking about a general vs. ad-hoc 
> place to put warnings, what happens with features we begin to deprecate but 
> are still in the deprecation window or those we clearly still need to support 
> but wish to give people heads-up about future deprecation, it is possible if 
> we do everything in one place? 
> 
> AFAIK we are currently just logging a warning when the code natrually 
> exercises the said logic, is this any different?

I think the question of deprecated features is a bit different. In this case, 
using a 0.28 agent with a 1.1 master is not supported; it _may_ work and 
probably will, but it would be good to educate users about that, rather than 
trying to track down specific situations in which 0.28 does something that is 
problematic.

Anyway, I'm fine with adding a warning here, but it would be good to think 
about how we want to handle this in general.


- Neil


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


On Oct. 26, 2016, 7:51 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53202/
> ---
> 
> (Updated Oct. 26, 2016, 7:51 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6483
> https://issues.apache.org/jira/browse/MESOS-6483
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We don't guarantee compatibility with pre-1.0 agents. However, since it
> is easy to avoid a CHECK failure in the master when an old agent
> re-registers, it seems worth doing so.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 23ddb995b4ad0fcdb589974308a2e81ececdad31 
> 
> Diff: https://reviews.apache.org/r/53202/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Disabled the code that fills-in `frameworks.recovered`; verified that 
> `PartitionTest.DisconnectedFramework` dies with a `CHECK` failure if this RR 
> is not applied but passes this with RR applied.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 52886: Fix new sign comparison errors in stout produced by hardened flags

[Aaron Wood]

> On Oct. 17, 2016, 3:39 p.m., James Peach wrote:
> > 3rdparty/stout/tests/ip_tests.cpp, line 50
> > 
> >
> > Probably better to just use the same type here. Since 
> > ``network.get().prefix()`` returns ``int``, consider 
> > ``numify(prefix).get()``.

Good idea!


- Aaron


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


On Oct. 21, 2016, 6:29 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52886/
> ---
> 
> (Updated Oct. 21, 2016, 6:29 p.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-6239
> https://issues.apache.org/jira/browse/MESOS-6239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The hardening flags produced many new sign comparison errors in stout that 
> need to be fixed for Mesos to compile/run.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/cache_tests.cpp 0950c85 
>   3rdparty/stout/tests/flags_tests.cpp 94ba915 
>   3rdparty/stout/tests/hashmap_tests.cpp 2626d67 
>   3rdparty/stout/tests/hashset_tests.cpp 66e59db 
>   3rdparty/stout/tests/ip_tests.cpp 59e69a5 
>   3rdparty/stout/tests/json_tests.cpp 2bc4c88 
>   3rdparty/stout/tests/linkedhashmap_tests.cpp 7a80769 
>   3rdparty/stout/tests/multimap_tests.cpp 488991b 
>   3rdparty/stout/tests/os/process_tests.cpp 4977d02 
>   3rdparty/stout/tests/os/sendfile_tests.cpp e221689 
>   3rdparty/stout/tests/os/systems_tests.cpp 110ba5b 
>   3rdparty/stout/tests/os_tests.cpp 6a7b836 
>   3rdparty/stout/tests/strings_tests.cpp 7dd3301 
> 
> Diff: https://reviews.apache.org/r/52886/diff/
> 
> 
> Testing
> ---
> 
> Made sure compilation, tests, and benchmarks worked with both gcc and clang. 
> Ran make && make check && make bench.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 53042: Added `FsTest.Xattr` test.

[Jie Yu]

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


Fix it, then Ship it!





3rdparty/stout/tests/os/filesystem_tests.cpp (line 456)


EXPECT_EQ


- Jie Yu


On Oct. 25, 2016, 3:42 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53042/
> ---
> 
> (Updated Oct. 25, 2016, 3:42 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
> https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `FsTest.Xattr` test.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> e3894ef6f21c15845085400a0d3426520411788e 
> 
> Diff: https://reviews.apache.org/r/53042/diff/
> 
> 
> Testing
> ---
> 
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from FsTest
> [ RUN  ] FsTest.Xattr
> [   OK ] FsTest.Xattr (2 ms)
> [--] 1 test from FsTest (2 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (2 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 53041: Added `setxattr()` and `getxattr()` in stout.

[Jie Yu]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/os/posix/xattr.hpp (lines 71 - 72)


I think James made a valid point there that errno might not be preserved 
across delete. So using a vector sounds better.


- Jie Yu


On Oct. 25, 2016, 3:41 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53041/
> ---
> 
> (Updated Oct. 25, 2016, 3:41 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
> https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `setxattr()` and `getxattr()` in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am 1eb9c146d3eebacaea32e0f724d145f0c1dd676f 
>   3rdparty/stout/include/stout/os.hpp 
> 96e8621b198a3ec4cce78e0a6ff5f271eda05ff1 
>   3rdparty/stout/include/stout/os/posix/xattr.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/xattr.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/xattr.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53041/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 52647: Fix new sign comparison errors in libprocess produced by hardened flags

[Aaron Wood]

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

(Updated Oct. 27, 2016, 4:51 p.m.)


Review request for mesos, James Peach, Michael Park, and Neil Conway.


Changes
---

Addressed comments about the encoder changes.


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


Repository: mesos


Description
---

The hardening flags produced many new sign comparison errors in libprocess that 
need to be fixed for Mesos to compile/run.


Diffs (updated)
-

  3rdparty/libprocess/src/decoder.hpp c79296b 
  3rdparty/libprocess/src/encoder.hpp 005d1cc 
  3rdparty/libprocess/src/process.cpp ab2b5a9 
  3rdparty/libprocess/src/tests/decoder_tests.cpp 4535614 
  3rdparty/libprocess/src/tests/encoder_tests.cpp 9e57375 
  3rdparty/libprocess/src/tests/http_tests.cpp 533104c 
  3rdparty/libprocess/src/tests/io_tests.cpp b85c79f 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 0dc1c62 

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


Testing
---

Made sure compilation, tests, and benchmarks worked with both gcc and clang. 
Ran `make && make check && make bench`.


Thanks,

Aaron Wood

Re: Review Request 52647: Fix new sign comparison errors in libprocess produced by hardened flags

[James Peach]

> On Oct. 18, 2016, 3:14 a.m., James Peach wrote:
> > 3rdparty/libprocess/src/tests/io_tests.cpp, line 284
> > 
> >
> > Can you just make ``length`` type ``ssize_t``?
> 
> Aaron Wood wrote:
> `length` is `ssize_t` (set on line 235)
> 
> James Peach wrote:
> Ok I see. It is bogus that ``length`` is ``ssize_t`` since ``io::read`` 
> is returning ``Future``. Just make ``length`` a ``size_t``.

Aaron and I discussed this. `length` needs to be `ssize_t` because it is the 
return from global `::write`. This means that casting `length` to an unsigned 
type is the most expedient solution. Alternatively, you could reuse `size` 
instead of `length`.


- James


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


On Oct. 21, 2016, 6:29 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52647/
> ---
> 
> (Updated Oct. 21, 2016, 6:29 p.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-6239
> https://issues.apache.org/jira/browse/MESOS-6239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The hardening flags produced many new sign comparison errors in libprocess 
> that need to be fixed for Mesos to compile/run.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp c79296b 
>   3rdparty/libprocess/src/encoder.hpp 005d1cc 
>   3rdparty/libprocess/src/process.cpp 18a8e20 
>   3rdparty/libprocess/src/tests/decoder_tests.cpp 4535614 
>   3rdparty/libprocess/src/tests/encoder_tests.cpp 9e57375 
>   3rdparty/libprocess/src/tests/http_tests.cpp 8d6c8c4 
>   3rdparty/libprocess/src/tests/io_tests.cpp b85c79f 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp c8350cf 
> 
> Diff: https://reviews.apache.org/r/52647/diff/
> 
> 
> Testing
> ---
> 
> Made sure compilation, tests, and benchmarks worked with both gcc and clang. 
> Ran `make && make check && make bench`.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52879: Cleaned up the way in which the executors load configuration options.

[Gastón Kleiman]

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

(Updated Oct. 27, 2016, 4:21 p.m.)


Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent huang, 
Jie Yu, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Updated the executors so that they use only `stout::flags` to load
config options.

They used to use a mix of `stout::flags` and `os::getenv`.


Diffs
-

  src/CMakeLists.txt 639f8678ba23c4d9a2ea0bf84fbc3d6fc9286ef3 
  src/Makefile.am 769e998d80fde17bcb1ee6c5091ce13a1ad16137 
  src/docker/executor.hpp 8385631fd170f97b28c4ca3596255ab0546774d6 
  src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 
  src/exec/exec.cpp 1dc20390907253a466b7272b7f8b33ea14afb236 
  src/exec/flags.hpp PRE-CREATION 
  src/exec/flags.cpp PRE-CREATION 
  src/executor/executor.cpp 1d47b52d89eedee59d160badd6313d34e3bdb6d2 
  src/executor/flags.hpp PRE-CREATION 
  src/executor/flags.cpp PRE-CREATION 
  src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 
  src/launcher/executor.hpp 217680d99d6e8c31130d7dc714f8cd730f360852 
  src/launcher/executor.cpp 0544121e679db503fe4eaf23a24e315eb188a520 
  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 
  src/tests/containerizer.cpp 69b93c70dc883e1edff3e6c2d7c8da9ef05e42f8 

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


Testing
---

`make check`


Thanks,

Gastón Kleiman

Re: Review Request 52878: Removed outdated TODO in stout::flags.

[Gastón Kleiman]

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

(Updated Oct. 27, 2016, 4:20 p.m.)


Review request for mesos, Alexander Rukletsov, Jie Yu, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Removed outdated TODO in stout::flags.


Diffs
-

  3rdparty/stout/include/stout/flags.hpp 
75925b8d25c8d4cc7ae9a5ed8db154d08a32 

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


Testing
---


Thanks,

Gastón Kleiman

Re: Review Request 53084: Support `LIBPROCESS_SSL_ENABLED` in the default executor and scheduler.

[Gastón Kleiman]

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

(Updated Oct. 27, 2016, 4:20 p.m.)


Review request for mesos, Alexander Rukletsov, haosdent huang, Jie Yu, Till 
Toenshoff, and Jiang Yan Xu.


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


Repository: mesos


Description
---

`LIBPROCESS_SSL_ENABLED` is preferred over `SSL_ENABLED`, and the latter
will be deprecated soon.


Diffs
-

  src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 
  src/scheduler/scheduler.cpp e282d419119dd1f01e170acf5cc2c6175b59791d 

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


Testing
---

make check


Thanks,

Gastón Kleiman

Re: Review Request 53197: Added parsers for 'SlaveID', 'ExecutorID' and 'FrameworkID'.

[Gastón Kleiman]

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

(Updated Oct. 27, 2016, 4:19 p.m.)


Review request for mesos, Alexander Rukletsov, haosdent huang, Jie Yu, Till 
Toenshoff, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Added parsers for 'SlaveID', 'ExecutorID' and 'FrameworkID'.


Diffs
-

  src/common/parse.hpp 0479a54a7bdeb34163ad988959b17f635dcea584 

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


Testing
---

`make check`

This is used in the next commit of the chain.


Thanks,

Gastón Kleiman

Re: Review Request 53227: Added the '--launcher_dir' flag to the default executor.

[Gastón Kleiman]

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



The idea of this patch is to resolve MESOS-6288 and to unblock MESOS-6119.

The chain starting with https://reviews.apache.org/r/52878/ cleans up the flags 
parsing in all the executors and all the executor libraries.

- Gastón Kleiman


On Oct. 27, 2016, 4:13 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53227/
> ---
> 
> (Updated Oct. 27, 2016, 4:13 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-6288
> https://issues.apache.org/jira/browse/MESOS-6288
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the '--launcher_dir' flag to the default executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 
>   src/slave/slave.cpp 881c10ac61d1e4fdeabdc42c0a41508c36f49040 
> 
> Diff: https://reviews.apache.org/r/53227/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Review Request 53227: Added the '--launcher_dir' flag to the default executor.

[Gastón Kleiman]

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

Review request for mesos and Alexander Rukletsov.


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


Repository: mesos


Description
---

Added the '--launcher_dir' flag to the default executor.


Diffs
-

  src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 
  src/slave/slave.cpp 881c10ac61d1e4fdeabdc42c0a41508c36f49040 

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


Testing
---

`make check`


Thanks,

Gastón Kleiman

Re: Review Request 53084: Support `LIBPROCESS_SSL_ENABLED` in the default executor and scheduler.

[Till Toenshoff]

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




src/launcher/default_executor.cpp (line 1033)


Shouldn't we be using a guarding `#ifdef` to make sure we are not allowing 
HTTPS on non SSL builds?



src/launcher/default_executor.cpp (line 1036)


Nice, thanks for adding the tracking ticket.


- Till Toenshoff


On Oct. 27, 2016, 3:49 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53084/
> ---
> 
> (Updated Oct. 27, 2016, 3:49 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, haosdent huang, Jie Yu, Till 
> Toenshoff, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `LIBPROCESS_SSL_ENABLED` is preferred over `SSL_ENABLED`, and the latter
> will be deprecated soon.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 
>   src/scheduler/scheduler.cpp e282d419119dd1f01e170acf5cc2c6175b59791d 
> 
> Diff: https://reviews.apache.org/r/53084/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 53230: Added test cases for HTTPS health check.

[haosdent huang]

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

(Updated Oct. 27, 2016, 4:06 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Update ticket number.


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


Repository: mesos


Description
---

Added test cases for HTTPS health check.


Diffs
-

  src/tests/health_check_tests.cpp b1a8942eb1841f5f795b01e08cb017eedf62c8f4 

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


Testing
---

```
sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*HealthCheckTest*" --verbose
```


Thanks,

haosdent huang

Review Request 53230: Added test cases for HTTPS health check.

[haosdent huang]

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

Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Repository: mesos


Description
---

Added test cases for HTTPS health check.


Diffs
-

  src/tests/health_check_tests.cpp b1a8942eb1841f5f795b01e08cb017eedf62c8f4 

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


Testing
---

```
sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*HealthCheckTest*" --verbose
```


Thanks,

haosdent huang

Re: Review Request 53197: Added parsers for 'SlaveID', 'ExecutorID' and 'FrameworkID'.

[Gastón Kleiman]

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

(Updated Oct. 27, 2016, 3:51 p.m.)


Review request for mesos, Alexander Rukletsov, haosdent huang, Jie Yu, Till 
Toenshoff, and Jiang Yan Xu.


Changes
---

Rebase.


Repository: mesos


Description
---

Added parsers for 'SlaveID', 'ExecutorID' and 'FrameworkID'.


Diffs (updated)
-

  src/common/parse.hpp 0479a54a7bdeb34163ad988959b17f635dcea584 

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


Testing
---

`make check`

This is used in the next commit of the chain.


Thanks,

Gastón Kleiman

Re: Review Request 53196: Fixed a typo in 'tests/containerizer.cpp'.

[Gastón Kleiman]

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

(Updated Oct. 27, 2016, 3:50 p.m.)


Review request for mesos, Alexander Rukletsov, haosdent huang, Jie Yu, Till 
Toenshoff, and Jiang Yan Xu.


Changes
---

Rebase.


Repository: mesos


Description
---

Fixed a typo in 'tests/containerizer.cpp'.


Diffs (updated)
-

  src/tests/containerizer.cpp 69b93c70dc883e1edff3e6c2d7c8da9ef05e42f8 

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


Testing
---


Thanks,

Gastón Kleiman

Re: Review Request 53084: Support `LIBPROCESS_SSL_ENABLED` in the default executor and scheduler.

[Gastón Kleiman]

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

(Updated Oct. 27, 2016, 3:49 p.m.)


Review request for mesos, Alexander Rukletsov, haosdent huang, Jie Yu, Till 
Toenshoff, and Jiang Yan Xu.


Changes
---

Make the behaviour consistent with what libprocess already does.


Summary (updated)
-

Support `LIBPROCESS_SSL_ENABLED` in the default executor and scheduler.


Repository: mesos


Description (updated)
---

`LIBPROCESS_SSL_ENABLED` is preferred over `SSL_ENABLED`, and the latter
will be deprecated soon.


Diffs (updated)
-

  src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 
  src/scheduler/scheduler.cpp e282d419119dd1f01e170acf5cc2c6175b59791d 

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


Testing
---

make check


Thanks,

Gastón Kleiman

Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

[Yubo Li]

> On 十月 26, 2016, 3:22 a.m., Kevin Klues wrote:
> > src/slave/containerizer/docker.cpp, line 700
> > 
> >
> > I would reword this to explicitly mention that tasks launched with the 
> > docker containerizer require these permissions in order to operate.

Added comments as follow:
```
// NOTE: GPU devices permissions are required to be `rmw` by default, that
// is because GPU tasks launched in the container may need to read/write/mknod
// to GPU devices in their lifecycle.
```


- Yubo


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


On 十月 24, 2016, 5 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated 十月 24, 2016, 5 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assigned Nvidia GPU devices to docker container based on
> GPUs allocated by Nvidia GPU allocator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 53202: Avoided CHECK failure with pre-1.0 agents.

[Neil Conway]

> On Oct. 26, 2016, 9:43 p.m., Jiang Yan Xu wrote:
> > src/master/master.cpp, lines 6036-6037
> > 
> >
> > Assuming frameworks are not partition-aware based on the agent verison 
> > doesn't feel right.
> > 
> > Ultimately it doesn't seem to make a difference in terms of messages 
> > Mesos sends: if the framework is not connected, no update is sent in this 
> > method. Later when it reconnects and reconciles, Master checks its 
> > capability and decides: `(5) Task is unknown, slave is unreachable: 
> > TASK_UNREACHABLE` or `TASK_LOST` if the frameworks is not partition-aware.
> > 
> > Would it make sense to set the state to `TASK_UNREACHABLE` in this 
> > case? Looks like the only differences it makes are:
> > 
> > - - metrics: Regardless of framework capabilities, the agent is indeed 
> > unreachable: `TASK_UNREACHABLE` is more in line with the (1.1) master's 
> > logic and the metrics don't reflect 100% of what the master sends out 
> > anyways.
> > - documentation: we set the state because it makes sense to the master 
> > and not by guessing the framework's capabilities. also worth-mentioning is 
> > the fact that this doesn't violate the API semantics: partition-awareness 
> > is checked at reconciliaton time.
> 
> Neil Conway wrote:
> I don't think either option is ideal. I opted for `TASK_LOST` because:
> 
> (a) sending `TASK_LOST` is still the default behavior; 
> partition-awareness is an "experimental" feature in 1.1, and in any case is 
> guarded behind a capability.
> (b) if you have (very) old agents, you're more likely to have old 
> frameworks that haven't enabled partition-awareness.
> 
> Jiang Yan Xu wrote:
> OK I am not adamant about `TASK_LOST` vs `TASK_UNREACHABLE` as long as 
> it's not sent out but w.r.t the rationale:
> 
> a) I don't feel this has to do with the feature being "experimental", 
> imagine we are at 1.2 and we've promoted this feature as stable and we have 
> marked the following
> 
> ```
>   // In Mesos 1.2, this will only be sent when the framework does NOT
>   // opt-in to the PARTITION_AWARE capability.
>   TASK_LOST = 5;
> ```
> we still have this problem, would we solve it differently? If not, we 
> might as well don't take "experimental" into consideration here.
> 
> b) this is more about documentation but even if we use `TASK_LOST`, can 
> we avoid saying we are basing our decision on the likelyhood of old 
> frameworks because of old agents? i.e., my original comment: *we set the 
> state because it makes sense to the master and not by guessing the 
> framework's capabilities. also worth-mentioning is the fact that this doesn't 
> violate the API semantics: partition-awareness is checked at reconciliaton 
> time*.

Re: (a), we can only guarantee that `TASK_LOST` will not be sent to 
partition-aware frameworks if all the agents in the cluster have been upgraded 
to Mesos >= 1.2 -- otherwise, an old agent might generate a `TASK_LOST` status 
update directly (e.g., due to an agent-local task launch failure).

Re: (b) sure, that was only a minor point -- but I think we should err on the 
side of not breaking old frameworks if we can't know the right thing to do, at 
least for 1.1.


- Neil


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


On Oct. 26, 2016, 7:51 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53202/
> ---
> 
> (Updated Oct. 26, 2016, 7:51 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6483
> https://issues.apache.org/jira/browse/MESOS-6483
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We don't guarantee compatibility with pre-1.0 agents. However, since it
> is easy to avoid a CHECK failure in the master when an old agent
> re-registers, it seems worth doing so.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 23ddb995b4ad0fcdb589974308a2e81ececdad31 
> 
> Diff: https://reviews.apache.org/r/53202/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Disabled the code that fills-in `frameworks.recovered`; verified that 
> `PartitionTest.DisconnectedFramework` dies with a `CHECK` failure if this RR 
> is not applied but passes this with RR applied.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 53226: Fixed HealthyTaskViaHTTPWithoutType test.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [53226]

Failed command: ./support/apply-review.sh -n -r 53226

Error:
2016-10-27 14:21:54 URL:https://reviews.apache.org/r/53226/diff/raw/ 
[2513/2513] -> "53226.patch" [1]
error: patch failed: src/tests/health_check_tests.cpp:1282
error: src/tests/health_check_tests.cpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/15840/console

- Mesos ReviewBot


On Oct. 27, 2016, 2:09 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53226/
> ---
> 
> (Updated Oct. 27, 2016, 2:09 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Bugs: MESOS-6293
> https://issues.apache.org/jira/browse/MESOS-6293
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp b1a8942eb1841f5f795b01e08cb017eedf62c8f4 
> 
> Diff: https://reviews.apache.org/r/53226/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 53226: Fixed HealthyTaskViaHTTPWithoutType test.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Oct. 27, 2016, 2:09 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53226/
> ---
> 
> (Updated Oct. 27, 2016, 2:09 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Bugs: MESOS-6293
> https://issues.apache.org/jira/browse/MESOS-6293
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp b1a8942eb1841f5f795b01e08cb017eedf62c8f4 
> 
> Diff: https://reviews.apache.org/r/53226/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 53226: Fixed HealthyTaskViaHTTPWithoutType test.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On Oct. 27, 2016, 2:09 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53226/
> ---
> 
> (Updated Oct. 27, 2016, 2:09 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Bugs: MESOS-6293
> https://issues.apache.org/jira/browse/MESOS-6293
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp b1a8942eb1841f5f795b01e08cb017eedf62c8f4 
> 
> Diff: https://reviews.apache.org/r/53226/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 53084: Updated the env variable used to enable SSL.

[Till Toenshoff]

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




src/launcher/default_executor.cpp (line 1037)


Within `3rdparty/libprocess/src/openssl.cpp` you will see that we actually 
take precedence on the `SSL_` variant over the `LIBPROCESS_SSL_` variant. In 
case of conflicts we are warning the user within `reinitialize`. Have a look at 
that logic please and see if you can play along a bit more in line.


- Till Toenshoff


On Oct. 24, 2016, 9:05 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53084/
> ---
> 
> (Updated Oct. 24, 2016, 9:05 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, haosdent huang, Jie Yu, Till 
> Toenshoff, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> According to the documentation it is `LIBPROCESS_SSL_ENABLED`, not
> `SSL_ENABLED`.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 
>   src/scheduler/scheduler.cpp e282d419119dd1f01e170acf5cc2c6175b59791d 
> 
> Diff: https://reviews.apache.org/r/53084/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Review Request 53226: Fixed HealthyTaskViaHTTPWithoutType test.

[Alexander Rukletsov]

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

Review request for mesos, Gastón Kleiman and haosdent huang.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/health_check_tests.cpp b1a8942eb1841f5f795b01e08cb017eedf62c8f4 

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


Testing
---

make check.


Thanks,

Alexander Rukletsov

Re: Review Request 52250: Added test cases for HTTP health check.

[haosdent huang]

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

(Updated Oct. 27, 2016, 1:51 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Repository: mesos


Description
---

Added test cases for HTTP health check.


Diffs
-

  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52786: Add the health check test helper.

[haosdent huang]

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

(Updated Oct. 27, 2016, 1:53 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Repository: mesos


Description
---

This patch updates the test helper for the health check. Via starts a
simple HTTP server which listens on given IP and port, it makes testing
HTTP health check and TCP health check easier.


Diffs
-

  src/Makefile.am 769e998d80fde17bcb1ee6c5091ce13a1ad16137 
  src/tests/CMakeLists.txt 18d6b15e65a462dcd638c4766c5adcf9fc10ba77 
  src/tests/health_check_test_helper.hpp PRE-CREATION 
  src/tests/health_check_test_helper.cpp PRE-CREATION 
  src/tests/test_helper_main.cpp 129a5e427b72fb577e55c3756d071f72ced9ff14 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52250: Added test cases for HTTP health check.

[haosdent huang]

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

(Updated Oct. 27, 2016, 1:53 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


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


Repository: mesos


Description
---

Added test cases for HTTP health check.


Diffs
-

  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52563: Minor clean ups in `health_check_tests.cpp` for consistency.

[Alexander Rukletsov]

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


Ship it!




I'll fix the outstanding issues and will commit this shortly.

- Alexander Rukletsov


On Oct. 21, 2016, 11:13 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52563/
> ---
> 
> (Updated Oct. 21, 2016, 11:13 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> * Replace `Testing` to `Tests` in comments.
> * Remove redundant `.Times(1)`.
> * Fix typos.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
> 
> Diff: https://reviews.apache.org/r/52563/diff/
> 
> 
> Testing
> ---
> 
> ```
> sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*HealthCheckTest.*" 
> --gtest_also_run_disabled_tests
> [--] Global test environment tear-down
> [==] 19 tests from 1 test case ran. (35264 ms total)
> [  PASSED  ] 19 tests.
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52563: Minor clean ups in `health_check_tests.cpp` for consistency.

[Alexander Rukletsov]

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




src/tests/health_check_tests.cpp (lines 357 - 361)


I don't see any reconciliation happening in this test.



src/tests/health_check_tests.cpp (lines 475 - 479)


Neither here.



src/tests/health_check_tests.cpp (lines 842 - 845)


I don't see endpoint verification.



src/tests/health_check_tests.cpp (lines 1178 - 1180)


No need to wrap.


- Alexander Rukletsov


On Oct. 21, 2016, 11:13 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52563/
> ---
> 
> (Updated Oct. 21, 2016, 11:13 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> * Replace `Testing` to `Tests` in comments.
> * Remove redundant `.Times(1)`.
> * Fix typos.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
> 
> Diff: https://reviews.apache.org/r/52563/diff/
> 
> 
> Testing
> ---
> 
> ```
> sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*HealthCheckTest.*" 
> --gtest_also_run_disabled_tests
> [--] Global test environment tear-down
> [==] 19 tests from 1 test case ran. (35264 ms total)
> [  PASSED  ] 19 tests.
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53172: Fixed the broken metrics information of master in WebUI.

[haosdent huang]

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




src/webui/master/static/js/controllers.js (lines 327 - 337)


Because `$scope` need to be used in ansible controllers, I put it here.


- haosdent huang


On Oct. 27, 2016, 4:54 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53172/
> ---
> 
> (Updated Oct. 27, 2016, 4:54 a.m.)
> 
> 
> Review request for mesos, Joseph Wu, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6446
> https://issues.apache.org/jira/browse/MESOS-6446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After introduced redirection on `/master/state` endpoint to the leading
> master in `c9153336`, the manual redirection logic in WebUI in broken
> because browser would redirect requests automatically when receives
> `307 Temporary Redirect`. Because the metrics information for tasks is
> only available on the leading master, it would miss in WebUI if the
> master we accessed is not a leading master. In this patch, we retrieve
> the leading master from `/master/state` endpoint and ensure the
> requests to `/metrics/snapshot` endpoint always send to the leading
> master.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/js/controllers.js 
> 3dead4f20e2dbeec8447e22ad287dd6ed2378345 
> 
> Diff: https://reviews.apache.org/r/53172/diff/
> 
> 
> Testing
> ---
> 
> ...
> 
> 
> Test screen record.
> 
> ![webui_metrics](https://issues.apache.org/jira/secure/attachment/12835172/webui_metrics.gif)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52561: Renamed `flags` to `agentFlags` in health check test cases.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Oct. 27, 2016, 12:07 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52561/
> ---
> 
> (Updated Oct. 27, 2016, 12:07 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch renames `slave` to `agent` in health check test cases as
> well.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
> 
> Diff: https://reviews.apache.org/r/52561/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52560: Avoided temporary `MockDocker` pointers in health check test cases.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Oct. 27, 2016, 12:06 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52560/
> ---
> 
> (Updated Oct. 27, 2016, 12:06 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided temporary `MockDocker` pointers in health check test cases.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
> 
> Diff: https://reviews.apache.org/r/52560/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52251: Added test cases for TCP health check.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Oct. 27, 2016, 12:14 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52251/
> ---
> 
> (Updated Oct. 27, 2016, 12:14 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Bugs: MESOS-6279
> https://issues.apache.org/jira/browse/MESOS-6279
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for TCP health check.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
> 
> Diff: https://reviews.apache.org/r/52251/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52250: Added test cases for HTTP health check.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Oct. 27, 2016, 12:14 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52250/
> ---
> 
> (Updated Oct. 27, 2016, 12:14 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Bugs: MESOS-6278
> https://issues.apache.org/jira/browse/MESOS-6278
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for HTTP health check.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
> 
> Diff: https://reviews.apache.org/r/52250/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52786: Add the health check test helper.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Oct. 27, 2016, 12:14 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52786/
> ---
> 
> (Updated Oct. 27, 2016, 12:14 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Bugs: MESOS-6278
> https://issues.apache.org/jira/browse/MESOS-6278
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the test helper for the health check. Via starts a
> simple HTTP server which listens on given IP and port, it makes testing
> HTTP health check and TCP health check easier.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 769e998d80fde17bcb1ee6c5091ce13a1ad16137 
>   src/tests/CMakeLists.txt 18d6b15e65a462dcd638c4766c5adcf9fc10ba77 
>   src/tests/health_check_test_helper.hpp PRE-CREATION 
>   src/tests/health_check_test_helper.cpp PRE-CREATION 
>   src/tests/test_helper_main.cpp 129a5e427b72fb577e55c3756d071f72ced9ff14 
> 
> Diff: https://reviews.apache.org/r/52786/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52786: Add the health check test helper.

[haosdent huang]

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

(Updated Oct. 27, 2016, 12:14 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Rebase.


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


Repository: mesos


Description
---

This patch updates the test helper for the health check. Via starts a
simple HTTP server which listens on given IP and port, it makes testing
HTTP health check and TCP health check easier.


Diffs (updated)
-

  src/Makefile.am 769e998d80fde17bcb1ee6c5091ce13a1ad16137 
  src/tests/CMakeLists.txt 18d6b15e65a462dcd638c4766c5adcf9fc10ba77 
  src/tests/health_check_test_helper.hpp PRE-CREATION 
  src/tests/health_check_test_helper.cpp PRE-CREATION 
  src/tests/test_helper_main.cpp 129a5e427b72fb577e55c3756d071f72ced9ff14 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52250: Added test cases for HTTP health check.

[haosdent huang]

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

(Updated Oct. 27, 2016, 12:14 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added test cases for HTTP health check.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52251: Added test cases for TCP health check.

[haosdent huang]

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

(Updated Oct. 27, 2016, 12:14 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added test cases for TCP health check.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52561: Renamed `flags` to `agentFlags` in health check test cases.

[haosdent huang]

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

(Updated Oct. 27, 2016, 12:07 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Rebase.


Repository: mesos


Description
---

This patch renames `slave` to `agent` in health check test cases as
well.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52560: Avoided temporary `MockDocker` pointers in health check test cases.

[haosdent huang]

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

(Updated Oct. 27, 2016, 12:06 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Rebase.


Repository: mesos


Description
---

Avoided temporary `MockDocker` pointers in health check test cases.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52251: Added test cases for TCP health check.

[haosdent huang]

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

(Updated Oct. 27, 2016, 12:05 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added test cases for TCP health check.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52250: Added test cases for HTTP health check.

[haosdent huang]

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

(Updated Oct. 27, 2016, 12:05 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added test cases for HTTP health check.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52250: Added test cases for HTTP health check.

[haosdent huang]

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

(Updated Oct. 27, 2016, 12:03 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added test cases for HTTP health check.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52786: Add the health check test helper.

[haosdent huang]

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

(Updated Oct. 27, 2016, 12:02 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Rebase.


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


Repository: mesos


Description
---

This patch updates the test helper for the health check. Via starts a
simple HTTP server which listens on given IP and port, it makes testing
HTTP health check and TCP health check easier.


Diffs (updated)
-

  src/Makefile.am 769e998d80fde17bcb1ee6c5091ce13a1ad16137 
  src/tests/CMakeLists.txt 18d6b15e65a462dcd638c4766c5adcf9fc10ba77 
  src/tests/health_check_test_helper.hpp PRE-CREATION 
  src/tests/health_check_test_helper.cpp PRE-CREATION 
  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
  src/tests/test_helper_main.cpp 129a5e427b72fb577e55c3756d071f72ced9ff14 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52786: Add the health check test helper.

[haosdent huang]

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

(Updated Oct. 27, 2016, 11:54 a.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Address @alexr's comments.


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


Repository: mesos


Description
---

This patch updates the test helper for the health check. Via starts a
simple HTTP server which listens on given IP and port, it makes testing
HTTP health check and TCP health check easier.


Diffs (updated)
-

  src/Makefile.am 769e998d80fde17bcb1ee6c5091ce13a1ad16137 
  src/tests/CMakeLists.txt 18d6b15e65a462dcd638c4766c5adcf9fc10ba77 
  src/tests/health_check_test_helper.hpp PRE-CREATION 
  src/tests/health_check_test_helper.cpp PRE-CREATION 
  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
  src/tests/test_helper_main.cpp 129a5e427b72fb577e55c3756d071f72ced9ff14 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52250: Added test cases for HTTP health check.

[haosdent huang]

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

(Updated Oct. 27, 2016, 11:54 a.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Address @alexr's comments.


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


Repository: mesos


Description
---

Added test cases for HTTP health check.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52251: Added test cases for TCP health check.

[Alexander Rukletsov]

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




src/tests/health_check_tests.cpp (line 1463)


s/healthy task/healthy non-contained task



src/tests/health_check_tests.cpp (lines 1463 - 1464)


Please say something like:

// NOTE: This test is almost identical to HealthyTaskViaHTTP with the 
difference being TCP health check.



src/tests/health_check_tests.cpp (line 1472)


kill this line



src/tests/health_check_tests.cpp (line 1492)


const



src/tests/health_check_tests.cpp (line 1496)


const



src/tests/health_check_tests.cpp (line 1516)


statusHealthy



src/tests/health_check_tests.cpp (lines 1520 - 1521)


`WillRepeatedly`



src/tests/health_check_tests.cpp (lines 1621 - 1622)


Please add
// To emulate a task responsive to TCP health checks, starts Netcat in the 
docker "alpine" image.
//
// NOTE: This test is almost identical to 
ROOT_INTERNET_CURL_HealthyTaskViaHTTPWithContainerImage with the difference 
being TCP health check.



src/tests/health_check_tests.cpp (line 1635)


kill this line.



src/tests/health_check_tests.cpp (line 1655)


const



src/tests/health_check_tests.cpp (line 1658)


const



src/tests/health_check_tests.cpp (line 1666)


please be consistent and use "alpine"



src/tests/health_check_tests.cpp (line 1684)


statusHealthy



src/tests/health_check_tests.cpp (lines 1688 - 1689)


WillRepeatedly...



src/tests/health_check_tests.cpp (lines 1832 - 1833)


// NOTE: This test is almost identical to 
ROOT_DOCKER_DockerHealthyTaskViaHTTP with the difference being TCP health check.



src/tests/health_check_tests.cpp (line 1879)


const



src/tests/health_check_tests.cpp (line 1882)


const



src/tests/health_check_tests.cpp (lines 1921 - 1922)


WillRepeatedly...


- Alexander Rukletsov


On Oct. 21, 2016, 11:12 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52251/
> ---
> 
> (Updated Oct. 21, 2016, 11:12 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Bugs: MESOS-6279
> https://issues.apache.org/jira/browse/MESOS-6279
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for TCP health check.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
> 
> Diff: https://reviews.apache.org/r/52251/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52250: Added test cases for HTTP health check.

[Alexander Rukletsov]

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




src/tests/health_check_tests.cpp (lines 1636 - 1637)


WillRepeatedly...


- Alexander Rukletsov


On Oct. 21, 2016, 11:12 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52250/
> ---
> 
> (Updated Oct. 21, 2016, 11:12 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Bugs: MESOS-6278
> https://issues.apache.org/jira/browse/MESOS-6278
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for HTTP health check.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
> 
> Diff: https://reviews.apache.org/r/52250/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52250: Added test cases for HTTP health check.

[Alexander Rukletsov]

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




src/tests/health_check_tests.cpp (line 1388)


s/healthy task/healthy non-contained task



src/tests/health_check_tests.cpp (line 1397)


You don't need this blank line.



src/tests/health_check_tests.cpp (line 1417)


const



src/tests/health_check_tests.cpp (line 1421)


const



src/tests/health_check_tests.cpp (line 1442)


s/statusHealth/statusHealthy



src/tests/health_check_tests.cpp (lines 1446 - 1447)


`.WillRepeatedly(Return()); // Ignore subsequent updates.`



src/tests/health_check_tests.cpp (lines 1463 - 1464)


Add the same sentence from the next test:
// To emulate a task responsive to HTTP health checks, starts Netcat in the 
docker "alpine" image.



src/tests/health_check_tests.cpp (line 1477)


kill this.



src/tests/health_check_tests.cpp (line 1497)


const



src/tests/health_check_tests.cpp (line 1500)


const



src/tests/health_check_tests.cpp (line 1526)


s/statusHealth/statusHealthy



src/tests/health_check_tests.cpp (lines 1530 - 1531)


`.WillRepeatedly(Return()); // Ignore subsequent updates.`



src/tests/health_check_tests.cpp (line 1594)


const



src/tests/health_check_tests.cpp (line 1597)


const



src/tests/health_check_tests.cpp (line 1608)


why not "library/alpine" like in the previous test?



src/tests/health_check_tests.cpp (line 1632)


s/statusHealth/statusHealthy


- Alexander Rukletsov


On Oct. 21, 2016, 11:12 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52250/
> ---
> 
> (Updated Oct. 21, 2016, 11:12 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Bugs: MESOS-6278
> https://issues.apache.org/jira/browse/MESOS-6278
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for HTTP health check.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
> 
> Diff: https://reviews.apache.org/r/52250/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53172: Fixed the broken metrics information of master in WebUI.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53193, 53172]

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 Oct. 27, 2016, 4:54 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53172/
> ---
> 
> (Updated Oct. 27, 2016, 4:54 a.m.)
> 
> 
> Review request for mesos, Joseph Wu, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6446
> https://issues.apache.org/jira/browse/MESOS-6446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After introduced redirection on `/master/state` endpoint to the leading
> master in `c9153336`, the manual redirection logic in WebUI in broken
> because browser would redirect requests automatically when receives
> `307 Temporary Redirect`. Because the metrics information for tasks is
> only available on the leading master, it would miss in WebUI if the
> master we accessed is not a leading master. In this patch, we retrieve
> the leading master from `/master/state` endpoint and ensure the
> requests to `/metrics/snapshot` endpoint always send to the leading
> master.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/js/controllers.js 
> 3dead4f20e2dbeec8447e22ad287dd6ed2378345 
> 
> Diff: https://reviews.apache.org/r/53172/diff/
> 
> 
> Testing
> ---
> 
> ...
> 
> 
> Test screen record.
> 
> ![webui_metrics](https://issues.apache.org/jira/secure/attachment/12835172/webui_metrics.gif)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>