Re: Review Request 52906: Simplify the comparison logic for `ExecutorInfo`.

[haosdent huang]

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

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


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

This is a follow up fix of https://reviews.apache.org/r/52817/.


Diffs
-

  src/common/type_utils.cpp c6cf4f1d4acd040d21434221e5469ca911404a39 

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


Testing
---

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


Thanks,

haosdent huang

Re: Review Request 52906: Simplify the comparison logic for `ExecutorInfo`.

[haosdent huang]

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

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


Review request for mesos, Anand Mazumdar and Vinod Kone.


Summary (updated)
-

Simplify the comparison logic for `ExecutorInfo`.


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


Repository: mesos


Description
---

This is a follow up fix of https://reviews.apache.org/r/52817/.


Diffs
-

  src/common/type_utils.cpp c6cf4f1d4acd040d21434221e5469ca911404a39 

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


Testing
---

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


Thanks,

haosdent huang

Re: Review Request 52906: Simplify the comparison log for `ExecutorInfo`.

[haosdent huang]

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

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


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Update testing done.


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


Repository: mesos


Description
---

This is a follow up fix of https://reviews.apache.org/r/52817/.


Diffs
-

  src/common/type_utils.cpp c6cf4f1d4acd040d21434221e5469ca911404a39 

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


Testing (updated)
---

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


Thanks,

haosdent huang

Review Request 52906: Simplify the comparison log for `ExecutorInfo`.

[haosdent huang]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

This is a follow up fix of https://reviews.apache.org/r/52817/.


Diffs
-

  src/common/type_utils.cpp c6cf4f1d4acd040d21434221e5469ca911404a39 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52817: Added utils method to check if `ExecutorInfo` different.

[haosdent huang]

> On Oct. 14, 2016, 11:09 p.m., Vinod Kone wrote:
> > src/common/type_utils.cpp, line 327
> > 
> >
> > didn't realize there is this new patch when i commited this chain!
> > 
> > anyway, i think  you can kill the if loop and change return to
> > 
> > ```
> > return left.has_type() == right.has_type() &&
> >(!left.has_type() || left.type() = right.type()) &&
> > &&
> >. &&
> > ```
> > 
> > this is how we do optional field checks. see examples in this file.

Sorry, I should update the ticket next time. A follow up fix 
https://reviews.apache.org/r/52906/


- haosdent


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


On Oct. 13, 2016, 6:36 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52817/
> ---
> 
> (Updated Oct. 13, 2016, 6:36 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6283
> https://issues.apache.org/jira/browse/MESOS-6283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added utils method to check if `ExecutorInfo` different.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp 1d09b4fb5d8db8005506a12e7a179f46ee376ca2 
>   src/common/type_utils.cpp cad243f7222e93030fa5a9c00955e9ef55f3c805 
> 
> Diff: https://reviews.apache.org/r/52817/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52058: Fixed fetcher to not call chown on sandbox again.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52828, 52058]

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. 14, 2016, 6:14 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52058/
> ---
> 
> (Updated Oct. 14, 2016, 6:14 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5218
> https://issues.apache.org/jira/browse/MESOS-5218
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fetcher code changes the ownership of entire sandbox directory
> recursively to the taskuser and as a resut also changes the
> ownership of files laid out by other entities in the sandbox.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> 11104d66e6dd05d8eb1d37a2e3250aca19278110 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> ad2070246b7fdb90aa6ed02326b5a7eb95365997 
> 
> Diff: https://reviews.apache.org/r/52058/diff/
> 
> 
> Testing
> ---
> 
> make check on linux and macos
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 52803: Changed agent to send TASK_GONE.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50235, 50416, 50417, 50418, 50422, 50699, 50700, 50701, 
50702, 50703, 50704, 50705, 50706, 50707, 50844, 50845, 50846, 51020, 51371, 
51374, 51375, 51376, 51377, 51021, 51706, 51653, 51707, 51805, 51845, 51891, 
51909, 51913, 51953, 51954, 51955, 51956, 51957, 51958, 52039, 52083, 52656, 
52657, 52658, 52659, 52693, 52719, 52720, 52721, 52722, 52723, 52740, 52746, 
52801, 52802, 52803]

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. 14, 2016, 4:02 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52803/
> ---
> 
> (Updated Oct. 14, 2016, 4:02 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6332
> https://issues.apache.org/jira/browse/MESOS-6332
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The agent previously sent TASK_LOST updates for tasks that are killed
> for various reasons, such as containerizer errors or QoS preemption. The
> agent now sends TASK_GONE to partition-aware frameworks instead.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 6d26797abf6d2b5e42b9e7743789e1edc62c9c1a 
>   src/tests/oversubscription_tests.cpp 
> b356fb62a4e068bc171a75a76001c6d0e76af92a 
>   src/tests/slave_tests.cpp 93b81d3e5b90d8036409e943f598c865fe335bcc 
> 
> Diff: https://reviews.apache.org/r/52803/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 52783: Added documentation for mesos-containerizer Linux capabilities support.

[Jie Yu]

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



I would like to have another separate docs/linux_capabilities.md to talk about 
how to use the framework API, and what will be the expected behaviors. Also, we 
need some example in the doc.

- Jie Yu


On Oct. 13, 2016, 12:22 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52783/
> ---
> 
> (Updated Oct. 13, 2016, 12:22 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6376
> https://issues.apache.org/jira/browse/MESOS-6376
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for mesos-containerizer Linux capabilities support.
> 
> 
> Diffs
> -
> 
>   docs/mesos-containerizer.md 76544625838845fe2817347fea483a60ab250f99 
> 
> Diff: https://reviews.apache.org/r/52783/diff/
> 
> 
> Testing
> ---
> 
> Checked with local renderer.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52887: Added a CHANGELOG description for partition-aware frameworks.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52887]

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. 14, 2016, 3:26 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52887/
> ---
> 
> (Updated Oct. 14, 2016, 3:26 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a CHANGELOG description for partition-aware frameworks.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 1b8fa346d916f941841bd34b264c8803e4286dc4 
> 
> Diff: https://reviews.apache.org/r/52887/diff/
> 
> 
> Testing
> ---
> 
> Visual inspection.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 52814: Added note to CHANGELOG about --runtime_dir.

[Kevin Klues]

> On Oct. 15, 2016, 12:08 a.m., Vinod Kone wrote:
> > CHANGELOG, line 18
> > 
> >
> > I would move this to "Additional API Changes" section since that's 
> > where we have been calling out important flags changes.

Got it. Will do next time.


- Kevin


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


On Oct. 15, 2016, 12:04 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52814/
> ---
> 
> (Updated Oct. 15, 2016, 12:04 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jie Yu, Till Toenshoff, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-6312
> https://issues.apache.org/jira/browse/MESOS-6312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added note to CHANGELOG about --runtime_dir.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 11e361b22254a7f892f5fa6b656480caf859d9ec 
> 
> Diff: https://reviews.apache.org/r/52814/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 52902: Updated default value of the '--runtime_dir' agent flag.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Oct. 15, 2016, 12:05 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52902/
> ---
> 
> (Updated Oct. 15, 2016, 12:05 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-6050
> https://issues.apache.org/jira/browse/MESOS-6050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the default value of '--runtime_dir' was hard coded to
> '/var/run/mesos'. However, this directory is typically only accessable
> to the 'root' user. This caused problems when launching an agent as a
> non-root user. We now check to see if the agent is launched as root or
> not, and change this default accordingly.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 064072197c4f8ea15b52b5e257cdb44879b34237 
>   src/slave/flags.cpp 7f79cfcc7939680c38a3d0cd57471cc9976aff7c 
> 
> Diff: https://reviews.apache.org/r/52902/diff/
> 
> 
> Testing
> ---
> 
> $ GTEST_FILTER="" make -j check
> $ src/mesos-tests
> 
> $ bin/mesos-local.sh
> ```
> Flags at startup: ... --runtime_dir="/tmp/mesos/runtime"
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 52814: Added note to CHANGELOG about --runtime_dir.

[Vinod Kone]

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


Fix it, then Ship it!





CHANGELOG (line 18)


I would move this to "Additional API Changes" section since that's where we 
have been calling out important flags changes.


- Vinod Kone


On Oct. 15, 2016, 12:04 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52814/
> ---
> 
> (Updated Oct. 15, 2016, 12:04 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jie Yu, Till Toenshoff, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-6312
> https://issues.apache.org/jira/browse/MESOS-6312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added note to CHANGELOG about --runtime_dir.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 11e361b22254a7f892f5fa6b656480caf859d9ec 
> 
> Diff: https://reviews.apache.org/r/52814/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 52902: Updated default value of the '--runtime_dir' agent flag.

[Kevin Klues]

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

(Updated Oct. 15, 2016, 12:05 a.m.)


Review request for mesos.


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


Repository: mesos


Description
---

Previously, the default value of '--runtime_dir' was hard coded to
'/var/run/mesos'. However, this directory is typically only accessable
to the 'root' user. This caused problems when launching an agent as a
non-root user. We now check to see if the agent is launched as root or
not, and change this default accordingly.


Diffs
-

  src/slave/constants.hpp 064072197c4f8ea15b52b5e257cdb44879b34237 
  src/slave/flags.cpp 7f79cfcc7939680c38a3d0cd57471cc9976aff7c 

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


Testing
---

$ GTEST_FILTER="" make -j check
$ src/mesos-tests

$ bin/mesos-local.sh
```
Flags at startup: ... --runtime_dir="/tmp/mesos/runtime"
```


Thanks,

Kevin Klues

Re: Review Request 52814: Added note to CHANGELOG about --runtime_dir.

[Kevin Klues]

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

(Updated Oct. 15, 2016, 12:04 a.m.)


Review request for mesos, Alexander Rukletsov, Jie Yu, Till Toenshoff, and 
Vinod Kone.


Changes
---

Updated to only change the CHANGELOG since we introduce 
https://reviews.apache.org/r/52902/ to fix the underlying issue.


Summary (updated)
-

Added note to CHANGELOG about --runtime_dir.


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


Repository: mesos


Description (updated)
---

Added note to CHANGELOG about --runtime_dir.


Diffs (updated)
-

  CHANGELOG 11e361b22254a7f892f5fa6b656480caf859d9ec 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 52901: Added documentation for default executor and LAUNCH_GROUP event.

[Vinod Kone]

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

(Updated Oct. 15, 2016, 12:03 a.m.)


Review request for mesos, Benjamin Mahler and Jie Yu.


Changes
---

benm's. NNFR.


Repository: mesos


Description
---

This is very minimal documentation mainly around the new framework
APIs and the default executor.


Diffs (updated)
-

  CHANGELOG 1b8fa346d916f941841bd34b264c8803e4286dc4 
  docs/app-framework-development-guide.md 
84521cdf32e75599988e55f5f50e94b032eed62f 
  docs/executor-http-api.md 50b4cb4efff48bcc56330b81bd7c4c217b8a22b5 
  docs/scheduler-http-api.md a27965abac1263542f0711d997804c0bcec5a716 

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


Testing
---


Thanks,

Vinod Kone

Review Request 52902: Updated default value of the '--runtime_dir' agent flag.

[Kevin Klues]

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

Review request for mesos.


Repository: mesos


Description
---

Previously, the default value of '--runtime_dir' was hard coded to
'/var/run/mesos'. However, this directory is typically only accessable
to the 'root' user. This caused problems when launching an agent as a
non-root user. We now check to see if the agent is launched as root or
not, and change this default accordingly.


Diffs
-

  src/slave/constants.hpp 064072197c4f8ea15b52b5e257cdb44879b34237 
  src/slave/flags.cpp 7f79cfcc7939680c38a3d0cd57471cc9976aff7c 

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


Testing
---

$ GTEST_FILTER="" make -j check
$ src/mesos-tests

$ bin/mesos-local.sh
```
Flags at startup: ... --runtime_dir="/tmp/mesos/runtime"
```


Thanks,

Kevin Klues

Re: Review Request 52901: Added documentation for default executor and LAUNCH_GROUP event.

[Vinod Kone]

> On Oct. 14, 2016, 11:23 p.m., Benjamin Mahler wrote:
> > CHANGELOG, lines 5-10
> > 
> >
> > It would be nice to distinguish the nested container support and the 
> > task group support in the changelog, i.e. we include nested container 
> > support for executors / operators to launch nested containers within an 
> > executor's container, and if you launch a task group using the default 
> > executor it will use a nested container for each task (using the new 
> > support for it).

Good idea. Done.


> On Oct. 14, 2016, 11:23 p.m., Benjamin Mahler wrote:
> > CHANGELOG, line 7
> > 
> >
> > "all or none of the tasks in the group" ?

done


> On Oct. 14, 2016, 11:23 p.m., Benjamin Mahler wrote:
> > docs/app-framework-development-guide.md, lines 18-24
> > 
> >
> > Is this related or did you want to pull this out into a separate commit?

i'll pull this and executor related change into a separate commit.


> On Oct. 14, 2016, 11:23 p.m., Benjamin Mahler wrote:
> > docs/app-framework-development-guide.md, line 329
> > 
> >
> > Not the freezer though.. right? Should we tell them about the cgroup 
> > level detail here or do they just need to know about the visible isolation 
> > semantics and namespaces?

I'll remove cgroups and talk about isolation semantics here. Thanks.


> On Oct. 14, 2016, 11:23 p.m., Benjamin Mahler wrote:
> > docs/app-framework-development-guide.md, line 335
> > 
> >
> > Do we want to call out that the other namespaces are not shared?

made it more generic.


> On Oct. 14, 2016, 11:23 p.m., Benjamin Mahler wrote:
> > docs/scheduler-http-api.md, line 275
> > 
> >
> > A little muddled?

rephrased.


> On Oct. 14, 2016, 11:23 p.m., Benjamin Mahler wrote:
> > docs/app-framework-development-guide.md, lines 364-370
> > 
> >
> > Related?

moved to a separate commit.


- Vinod


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


On Oct. 14, 2016, 10:15 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52901/
> ---
> 
> (Updated Oct. 14, 2016, 10:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is very minimal documentation mainly around the new framework
> APIs and the default executor.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 1b8fa346d916f941841bd34b264c8803e4286dc4 
>   docs/app-framework-development-guide.md 
> 84521cdf32e75599988e55f5f50e94b032eed62f 
>   docs/executor-http-api.md 50b4cb4efff48bcc56330b81bd7c4c217b8a22b5 
>   docs/scheduler-http-api.md a27965abac1263542f0711d997804c0bcec5a716 
> 
> Diff: https://reviews.apache.org/r/52901/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 52787: Set `runtime_dir` to a temporary folder in `mesos-local`.

[Kevin Klues]

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



This will no longer be necessary after https://reviews.apache.org/r/52902 lands.

- Kevin Klues


On Oct. 14, 2016, 4:26 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52787/
> ---
> 
> (Updated Oct. 14, 2016, 4:26 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Kevin Klues.
> 
> 
> Bugs: MESOS-6380
> https://issues.apache.org/jira/browse/MESOS-6380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set `runtime_dir` to a temporary folder in `mesos-local`.
> 
> 
> Diffs
> -
> 
>   bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
>   src/local/flags.hpp c77eff13e7a63630ff6c9428e57f3f6707e1f30f 
>   src/local/local.cpp 257179443827ffdfc946ed655a6840fcea70d454 
> 
> Diff: https://reviews.apache.org/r/52787/diff/
> 
> 
> Testing
> ---
> 
> To void this error when launch `mesos-local` without `sudo`
> 
> ```
>   message: 'Failed to launch container: Failed to make the containerizer 
> runtime directory 
> '/var/run/mesos/containers/f2d6947f-2916-4f1a-90dc-3d137b360b9c': Permission 
> denied; Abnormal executor termination: unknown container'
> ```
> 
> Test produce 
> 
> ```
> # Run with normal user.
> $ ./bin/mesos-local.sh
> 
> # Open a new session
> $ ./src/mesos-execute --master=127.0.0.1:5050 --name="test-single-1" 
> --command="sleep 2000"
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> W1014 00:05:22.457461 36489 scheduler.cpp:161]
> **
> Scheduler driver bound to loopback interface! Cannot communicate with remote 
> master(s). You might want to set 'LIBPROCESS_IP' environment variable to use 
> a routable IP address.
> **
> I1014 00:05:22.457690 36489 scheduler.cpp:176] Version: 1.1.0
> I1014 00:05:22.596534 36497 scheduler.cpp:465] New master detected at 
> master@127.0.0.1:5050
> Subscribed with ID 51e0d33f-d7b2-4c01-9bb3-10731cc7d1ab-
> Submitted task 'test-single-1' to agent 
> '8fba7ecb-b70e-48ac-9541-2070c4d9b1d5-S0'
> Received status update TASK_RUNNING for task 'test-single-1'
>   source: SOURCE_EXECUTOR
> ```
> 
> ```
> $ sudo make install
> $ mesos-local
> ...
> I1014 12:23:47.563256 4284416 slave.cpp:208] Mesos agent started on 
> (1)@127.0.0.1:5050
> I1014 12:23:47.563302 4284416 slave.cpp:209] Flags at startup: 
> --appc_simple_discovery_uri_prefix="http://; 
> --appc_store_dir="/tmp/mesos/store/appc" --authenticate_http_readonly="false" 
> --authenticate_http_readwrite="false" --authenticatee="crammd5" 
> --authentication_backoff_factor="1secs" --authorizer="local" 
> --container_disk_watch_interval="15secs" --containerizers="mesos" 
> --default_role="*" --disk_watch_interval="1mins" --docker="docker" 
> --docker_kill_orphans="true" --docker_registry="https://registry-1.docker.io; 
> --docker_remove_delay="6hrs" --docker_socket="/var/run/docker.sock" 
> --docker_stop_timeout="0ns" --docker_store_dir="/tmp/mesos/store/docker" 
> --docker_volume_checkpoint_dir="/var/run/mesos/isolators/docker/volume" 
> --enforce_container_disk_quota="false" 
> --executor_registration_timeout="1mins" 
> --executor_shutdown_grace_period="5secs" 
> --fetcher_cache_dir="/tmp/mesos/fetch" --fetcher_cache_size="2GB" 
> --frameworks_home="" --gc_delay="1weeks" --gc_disk_headroom="0.1" --hadoop_
 home="" --help="false" --hostname_lookup="true" --http_authenticators="basic" 
--http_command_executor="false" --image_provisioner_backend="copy" 
--initialize_driver_logging="true" --isolation="posix/cpu,posix/mem" 
--launcher="posix" --launcher_dir="/usr/local/libexec/mesos" --logbufsecs="0" 
--logging_level="INFO" --max_completed_executors_per_framework="150" 
--oversubscribed_resources_interval="15secs" 
--qos_correction_interval_min="0ns" --quiet="false" --recover="reconnect" 
--recovery_timeout="15mins" --registration_backoff_factor="1secs" 
--runtime_dir="/tmp/mesos/local/0" --sandbox_directory="/mnt/mesos/sandbox" 
--strict="true" --switch_user="true" --version="false" 
--work_dir="/tmp/mesos/local/0"
> ...
> 
> $ mesos-execute --master=127.0.0.1:5050 --name="test-single-1" 
> --command="sleep 2000"
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> W1014 12:25:39.922174 3211264 scheduler.cpp:161]
> **
> Scheduler driver bound to loopback interface! Cannot communicate with remote 
> master(s). You might want to set 'LIBPROCESS_IP' environment variable to use 
> a routable IP address.
> **
> I1014 12:25:39.922353 3211264 

Re: Review Request 50675: Libprocess: Enabled tests that pass on Windows.

[Joseph Wu]

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


Fix it, then Ship it!




I can fix the one test you missed.

---

Looks good!  On Windows:

>"3rdparty/stout/tests/Debug/stout_tests.exe"
```
[--] Global test environment tear-down
[==] 219 tests from 38 test cases ran. (4517 ms total)
[  PASSED  ] 219 tests.

  YOU HAVE 13 DISABLED TESTS
```

>"3rdparty/libprocess/src/tests/Debug/process_tests.exe"
```
[--] Global test environment tear-down
[==] 97 tests from 20 test cases ran. (767 ms total)
[  PASSED  ] 97 tests.

  YOU HAVE 44 DISABLED TESTS
```

---

Compared to OSX:

Stout:
```
[--] Global test environment tear-down
[==] 262 tests from 44 test cases ran. (425 ms total)
[  PASSED  ] 262 tests.
```

Libprocess:
```
[--] Global test environment tear-down
[==] 168 tests from 25 test cases ran. (645 ms total)
[  PASSED  ] 168 tests.

  YOU HAVE 2 DISABLED TESTS
```


3rdparty/libprocess/src/tests/process_tests.cpp (line 984)


Missed disabling this test.


- Joseph Wu


On Oct. 13, 2016, 6:10 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50675/
> ---
> 
> (Updated Oct. 13, 2016, 6:10 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A large number of libprocess test files are currently not being built on
> Windows. Many of these files contain tests for parts of libprocess that
> have already been ported, or require only trivial fixes to work (such as
> removing `#include`s on Windows). A small minority of the tests contain
> bugs that we should fix.
> 
> This commit will add these files to the build, fix some of the
> trivially-fixable tests, and disable tests that are known to fail
> because of bugs, including comments explaining why and links to JIRA
> issues where appropriate.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> c267117ab422310854d492493d711039b287d7e3 
>   3rdparty/libprocess/src/tests/future_tests.cpp 
> 383d260a4c0b17e9a0b5af002bb35070e3ec0b01 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 8d6c8c4507b93dd11f927bad2a8f0faeb957e388 
>   3rdparty/libprocess/src/tests/limiter_tests.cpp 
> 71a3d25d4040de9b0d3eb23612be80599f4b6a9a 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> 88526e67a20b7b58a6e14034215faa3ae9879fd0 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 3936f47dfcacb0f85a6fa0f68f1707159b1c320c 
>   3rdparty/libprocess/src/tests/reap_tests.cpp 
> 586d52891af191d43229707c89a91cf9cbb9007d 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> c8350cf8e512dca23933725e6edb3e3d94380211 
>   3rdparty/libprocess/src/tests/time_tests.cpp 
> 9d40eaee046eaa43199bb92a54892207e9c31ef2 
> 
> Diff: https://reviews.apache.org/r/50675/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 52901: Added documentation for default executor and LAUNCH_GROUP event.

[Benjamin Mahler]

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


Fix it, then Ship it!





CHANGELOG (lines 5 - 10)


It would be nice to distinguish the nested container support and the task 
group support in the changelog, i.e. we include nested container support for 
executors / operators to launch nested containers within an executor's 
container, and if you launch a task group using the default executor it will 
use a nested container for each task (using the new support for it).



CHANGELOG (line 7)


"all or none of the tasks in the group" ?



docs/app-framework-development-guide.md (lines 18 - 24)


Is this related or did you want to pull this out into a separate commit?



docs/app-framework-development-guide.md (line 329)


Not the freezer though.. right? Should we tell them about the cgroup level 
detail here or do they just need to know about the visible isolation semantics 
and namespaces?



docs/app-framework-development-guide.md (line 335)


Do we want to call out that the other namespaces are not shared?



docs/app-framework-development-guide.md (lines 364 - 370)


Related?



docs/scheduler-http-api.md (line 275)


A little muddled?


- Benjamin Mahler


On Oct. 14, 2016, 10:15 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52901/
> ---
> 
> (Updated Oct. 14, 2016, 10:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is very minimal documentation mainly around the new framework
> APIs and the default executor.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 1b8fa346d916f941841bd34b264c8803e4286dc4 
>   docs/app-framework-development-guide.md 
> 84521cdf32e75599988e55f5f50e94b032eed62f 
>   docs/executor-http-api.md 50b4cb4efff48bcc56330b81bd7c4c217b8a22b5 
>   docs/scheduler-http-api.md a27965abac1263542f0711d997804c0bcec5a716 
> 
> Diff: https://reviews.apache.org/r/52901/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 52877: Fixed wrong float serialization in JSON in locales different from C.

[Joris Van Remoortere]

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



I have a feeling that if you kept around a stringstream with the local set your 
benchmarks would look rather different.
I also suggest using callgrind to get the instruction count / # of library 
calls made.

- Joris Van Remoortere


On Oct. 14, 2016, 12:50 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52877/
> ---
> 
> (Updated Oct. 14, 2016, 12:50 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Bannier, and Michael Park.
> 
> 
> Bugs: MESOS-6349
> https://issues.apache.org/jira/browse/MESOS-6349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Resolves an isse with the JSON serialization functions where floats
> would use the process active locale as a decimal separator instead
> of the JSON conformant period (`.`).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/json.hpp 
> 9734561549eea867d57f14a0ab71febeb9dddc06 
>   3rdparty/stout/include/stout/jsonify.hpp 
> 8220d001001e8b9f247c05be58534f1174611aad 
> 
> Diff: https://reviews.apache.org/r/52877/diff/
> 
> 
> Testing
> ---
> 
> Three types of tests were used for this issue. The first just run our current
> test suite (`make check`). The second verifies that the proposed solutions
> are able to generate a locale independent output and the third consist of a
> benchmark of all the solutions.
> 
> The verification that the proposed solutions produce locale independent JSON,
> the following program was used (with manual verification of the generated
> output):
> 
> ```c++
> #include 
> 
> #include 
> #include 
> #include 
> 
> int main(int argc, char **argv) {
>   // Set locale to German.
>   std::setlocale(LC_ALL, "de_DE.UTF-8");
>   std::cout.imbue(std::locale("de_DE.UTF-8"));
> 
>   JSON::Object object;
>   object.values["float"] = 1234567890.12345;
> 
>   std::cout << stringify(object) << '\n';
>   return 0;
> }
> 
> ```
> 
> In order to test the different options the following benchmarking program was
> written using [Google benchmark](https://github.com/google/benchmark):
> 
> ```c++
> #include 
> #include 
> 
> #include 
> #include 
> #include 
> #include 
> 
> static void BM_Jsonify(benchmark::State& state) {
>   while (state.KeepRunning()) {
> state.PauseTiming();
> JSON::Object object;
> object.values["float00"] = 1234567890.12345;
> object.values["float01"] = 123456789.012345;
> object.values["float02"] = 12345678.9012345;
> object.values["float03"] = 1234567.89012345;
> object.values["float04"] = 123456.789012345;
> object.values["float05"] = 12345.6789012345;
> object.values["float06"] = 1234.56789012345;
> object.values["float07"] = 123.456789012345;
> object.values["float08"] = 12.3456789012345;
> object.values["float09"] = 1.23456789012345;
> object.values["float10"] = 1234567890.12345;
> object.values["float11"] = 123456789.012345;
> object.values["float12"] = 12345678.9012345;
> object.values["float13"] = 1234567.89012345;
> object.values["float14"] = 123456.789012345;
> object.values["float15"] = 12345.6789012345;
> object.values["float16"] = 1234.56789012345;
> object.values["float17"] = 123.456789012345;
> object.values["float18"] = 12.3456789012345;
> object.values["float19"] = 1.23456789012345;
> object.values["float20"] = 1234567890.12345;
> object.values["float21"] = 123456789.012345;
> object.values["float22"] = 12345678.9012345;
> object.values["float23"] = 1234567.89012345;
> object.values["float24"] = 123456.789012345;
> object.values["float25"] = 12345.6789012345;
> object.values["float26"] = 1234.56789012345;
> object.values["float27"] = 123.456789012345;
> object.values["float28"] = 12.3456789012345;
> object.values["float29"] = 1.23456789012345;
> object.values["float30"] = 1234567890.12345;
> object.values["float31"] = 123456789.012345;
> object.values["float32"] = 12345678.9012345;
> object.values["float33"] = 1234567.89012345;
> object.values["float34"] = 123456.789012345;
> object.values["float35"] = 12345.6789012345;
> object.values["float36"] = 1234.56789012345;
> object.values["float37"] = 123.456789012345;
> object.values["float38"] = 12.3456789012345;
> object.values["float39"] = 1.23456789012345;
> object.values["float40"] = 1234567890.12345;
> object.values["float41"] = 123456789.012345;
> object.values["float42"] = 12345678.9012345;
> object.values["float43"] = 1234567.89012345;
> object.values["float44"] = 123456.789012345;
> object.values["float45"] = 

Re: Review Request 52817: Added utils method to check if `ExecutorInfo` different.

[Vinod Kone]

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




src/common/type_utils.cpp (line 327)


didn't realize there is this new patch when i commited this chain!

anyway, i think  you can kill the if loop and change return to

```
return left.has_type() == right.has_type() &&
   (!left.has_type() || left.type() = right.type()) &&
    &&
   . &&
```

this is how we do optional field checks. see examples in this file.


- Vinod Kone


On Oct. 13, 2016, 6:36 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52817/
> ---
> 
> (Updated Oct. 13, 2016, 6:36 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6283
> https://issues.apache.org/jira/browse/MESOS-6283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added utils method to check if `ExecutorInfo` different.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp 1d09b4fb5d8db8005506a12e7a179f46ee376ca2 
>   src/common/type_utils.cpp cad243f7222e93030fa5a9c00955e9ef55f3c805 
> 
> Diff: https://reviews.apache.org/r/52817/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49833: Consistently used virtual inheritance for Flags classes in stout.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Oct. 6, 2016, 2:58 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49833/
> ---
> 
> (Updated Oct. 6, 2016, 2:58 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3335
> https://issues.apache.org/jira/browse/MESOS-3335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order for different `Flags` classes to be composable classes should
> always use virtual inheritance.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/flags_tests.cpp 
> 94ba915c40836e476cf6097274a85c55acd4d73b 
>   3rdparty/stout/tests/subcommand_tests.cpp 
> 9213d6b9faec30b5be320ab37ca29c2406c964ac 
> 
> Diff: https://reviews.apache.org/r/49833/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of the review chain ending in 
> https://reviews.apache.org/r/52388/ on various Linux configurations in 
> internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 49829: Consistently used virtual inheritance for Flags classes.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Oct. 6, 2016, 2:58 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49829/
> ---
> 
> (Updated Oct. 6, 2016, 2:58 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3335
> https://issues.apache.org/jira/browse/MESOS-3335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order for different `Flags` classes to be composable classes should
> always use virtual inheritance.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp fcf627b0dcbedd01700cc8c9acadf7ba0dae4faa 
>   src/docker/executor.hpp a49ec1b4045116741af6af08578791fb0440ad8f 
>   src/examples/balloon_framework.cpp 5613ff0c61e2d2f84684a206debc97dcb8b2c0d3 
>   src/examples/disk_full_framework.cpp 
> 1221f83d495f7c1ee1bcbcfe067e303db0a921eb 
>   src/examples/load_generator_framework.cpp 
> 5402e31b89b7ead1dc8fdc9065980b5b1c0d380c 
>   src/examples/long_lived_framework.cpp 
> 7c57eb5e4a34208504475013690ae8e3cae74155 
>   src/examples/no_executor_framework.cpp 
> 57425726aa5ca27c9579b0d8ecc0bb9eb9b26852 
>   src/examples/persistent_volume_framework.cpp 
> 3cc7cf0c4c97d90f2e70800d7a8d4ca64c2150d1 
>   src/launcher/executor.cpp 8a1051b886a0c95d19ff370e5c77d9c4033c8b61 
>   src/local/flags.hpp c77eff13e7a63630ff6c9428e57f3f6707e1f30f 
>   src/log/tool/benchmark.hpp e905e85301df089e3a260bff8e51c6aae574bd46 
>   src/log/tool/initialize.hpp 33d2c5c4e54cbdad30cb3102f36340b1cac3bc67 
>   src/log/tool/read.hpp 7ec82759efc90385ea3d1bc36bdc8ddb95dfa489 
>   src/log/tool/replica.hpp 9ca92ae1736a15b88a386cdf9f08c35253065fe5 
>   src/master/flags.hpp 708a629dab39e8cf161b0dd43420fcd761044102 
>   src/sched/flags.hpp 8782c7bef833cbf418a60a0d296352f1cbe63d7b 
>   src/scheduler/flags.hpp 6d0e95ed375726f8ab203e5fb40ef5bd299004e5 
>   src/slave/container_loggers/lib_logrotate.hpp 
> a8653d716a898f03cce83f7b88b666dc46c0ea90 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 70f30831819d7a0e6233fcb13a703dc6981324b6 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> e852c46a5027c7c911bb7f0c9364ff830f4df086 
>   src/slave/containerizer/mesos/launch.hpp 
> a9b6ee2f0da2c1c84c6a0642adbce20a9f0218dc 
>   src/slave/containerizer/mesos/mount.hpp 
> fcfe8601886e4f93386f71931a514ea7f44f19d3 
>   src/slave/flags.hpp 3952d04f6a00ac1dca1adf2bea7cc6e415620ce5 
>   src/tests/active_user_test_helper.hpp 
> b99d1e40acc65821b85c24cc4ac6f34c97678f9a 
>   src/tests/containerizer/capabilities_test_helper.hpp 
> 256ee3b5a986a5c59da7f479a64c63e02884adcb 
>   src/tests/flags.hpp 3066399152bf0e152f6876e001af65d4e945fb8f 
>   src/uri/fetcher.hpp 949363ecdb15f5b225db7f3ed67eda52c351bfa1 
> 
> Diff: https://reviews.apache.org/r/49829/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of the review chain ending in 
> https://reviews.apache.org/r/52388/ on various Linux configurations in 
> internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52387: Consistently used virtual inheritance for Flags classes in libprocess.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Oct. 6, 2016, 2:58 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52387/
> ---
> 
> (Updated Oct. 6, 2016, 2:58 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3335
> https://issues.apache.org/jira/browse/MESOS-3335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order for different `Flags` classes to be composable classes should
> always use virtual inheritance.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> c8350cf8e512dca23933725e6edb3e3d94380211 
> 
> Diff: https://reviews.apache.org/r/52387/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of the review chain ending in 
> https://reviews.apache.org/r/52388/ on various Linux configurations in 
> internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52856: Reverted incorrect changes in 1c2ee5c.

[Vinod Kone]

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


Fix it, then Ship it!





src/local/local.cpp (line 179)


can you add a comment here for posterity? it might not be clear for the 
next developer who comes across this.


- Vinod Kone


On Oct. 14, 2016, 4:26 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52856/
> ---
> 
> (Updated Oct. 14, 2016, 4:26 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-6380
> https://issues.apache.org/jira/browse/MESOS-6380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reverted incorrect changes in 1c2ee5c.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp 257179443827ffdfc946ed655a6840fcea70d454 
> 
> Diff: https://reviews.apache.org/r/52856/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52899: Added "os/wait.hpp" to stout's "os.hpp" header.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Oct. 14, 2016, 9:50 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52899/
> ---
> 
> (Updated Oct. 14, 2016, 9:50 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6310
> https://issues.apache.org/jira/browse/MESOS-6310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added "os/wait.hpp" to stout's "os.hpp" header.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os.hpp 
> c38e434d90c8c25570118c255f2eec72f96b348d 
> 
> Diff: https://reviews.apache.org/r/52899/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Review Request 52901: Added documentation for default executor and LAUNCH_GROUP event.

[Vinod Kone]

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

Review request for mesos, Benjamin Mahler and Jie Yu.


Repository: mesos


Description
---

This is very minimal documentation mainly around the new framework
APIs and the default executor.


Diffs
-

  CHANGELOG 1b8fa346d916f941841bd34b264c8803e4286dc4 
  docs/app-framework-development-guide.md 
84521cdf32e75599988e55f5f50e94b032eed62f 
  docs/executor-http-api.md 50b4cb4efff48bcc56330b81bd7c4c217b8a22b5 
  docs/scheduler-http-api.md a27965abac1263542f0711d997804c0bcec5a716 

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


Testing
---


Thanks,

Vinod Kone

Re: Review Request 52898: Tweaked description of framework checkpointing behavior.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Oct. 14, 2016, 9:36 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52898/
> ---
> 
> (Updated Oct. 14, 2016, 9:36 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tweaked description of framework checkpointing behavior.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto d0714317a9a88a41275cf73961ccb1a43d1c6b15 
>   include/mesos/v1/mesos.proto 74761a0e2701ff994bb708930a41c2402a25ee86 
> 
> Diff: https://reviews.apache.org/r/52898/diff/
> 
> 
> Testing
> ---
> 
> Visual inspection, no functional change.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 52899: Added "os/wait.hpp" to stout's "os.hpp" header.

[Kevin Klues]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added "os/wait.hpp" to stout's "os.hpp" header.


Diffs
-

  3rdparty/stout/include/stout/os.hpp c38e434d90c8c25570118c255f2eec72f96b348d 

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


Testing
---


Thanks,

Kevin Klues

Review Request 52898: Tweaked description of framework checkpointing behavior.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Tweaked description of framework checkpointing behavior.


Diffs
-

  include/mesos/mesos.proto d0714317a9a88a41275cf73961ccb1a43d1c6b15 
  include/mesos/v1/mesos.proto 74761a0e2701ff994bb708930a41c2402a25ee86 

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


Testing
---

Visual inspection, no functional change.


Thanks,

Neil Conway

Re: Review Request 50981: Fix volumes help message example mistake.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Aug. 11, 2016, 2:54 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50981/
> ---
> 
> (Updated Aug. 11, 2016, 2:54 p.m.)
> 
> 
> Review request for mesos, Guangya Liu and Jie Yu.
> 
> 
> Bugs: MESOS-6028
> https://issues.apache.org/jira/browse/MESOS-6028
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed docker_options to driver_options.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp b752d057a3d86482ef1a4baaf31052469e38dc76 
> 
> Diff: https://reviews.apache.org/r/50981/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

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

[Jiang Yan Xu]

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



This is conversation carried over from /r/52556/

Now that we're in the business of (re)naming things for consistency, 
understandability and unambiguity, how about we just follow some widely used 
conventions:
https://www.gnu.org/prep/standards/html_node/Directory-Variables.html

The motivation in the renaming in /r/52556/ seems to be because the number of 
binaries in the directory has grown and not all related to the launcher 
anymore. `--libexec_dir` or `MESOS_LIBEXEC_DIR` feels pretty clear for what it 
is.

BTW, when I first see the flag `--runtime_dir` on the agent, I thought it was 
for the "runtime binaries" as well. Would `--runstate_dir` or 
`MESOS_RUNSTATE_DIR` (from the linked doc) be better names? 

^^ @Jie

- Jiang Yan Xu


On Oct. 14, 2016, 10:18 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52880/
> ---
> 
> (Updated Oct. 14, 2016, 10:18 a.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/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 
>   src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 
> 
> Diff: https://reviews.apache.org/r/52880/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 40487: MESOS-3959: show slave hostname on executor page

[Alexander Rukletsov]

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


Fix it, then Ship it!




Looks like the file has been renamed. Thanks for the patch and sorry it fell 
through the cracks! I'll rebase, fix and commit shortly.


src/webui/master/static/slave_executor.html (line 47)


s/Slave/Agent


- Alexander Rukletsov


On Oct. 14, 2016, 8:19 p.m., Ian Babrou wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40487/
> ---
> 
> (Updated Oct. 14, 2016, 8:19 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-3959
> https://issues.apache.org/jira/browse/MESOS-3959
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3959: show slave hostname on executor page
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/slave_executor.html 
> 7c66405090f46f89bdd29806a58c05dc76c0ad23 
> 
> Diff: https://reviews.apache.org/r/40487/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Babrou
> 
>

Re: Review Request 40487: MESOS-3959: show slave hostname on executor page

[Ian Babrou]

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

(Updated Oct. 14, 2016, 8:19 p.m.)


Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

MESOS-3959: show slave hostname on executor page


Diffs
-

  src/webui/master/static/slave_executor.html 
7c66405090f46f89bdd29806a58c05dc76c0ad23 

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


Testing
---


Thanks,

Ian Babrou

Re: Review Request 52827: Added backend suffix to image layer rootfs path.

[Jie Yu]

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


Fix it, then Ship it!




Can you follow up with a test to test roll forward and backward case?


src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp (lines 
178 - 188)


I realized that that is not necessary. In 'recover', we did the same thing. 
We won't save the mapping if some layers are missing.


- Jie Yu


On Oct. 14, 2016, 3:36 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52827/
> ---
> 
> (Updated Oct. 14, 2016, 3:36 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
> https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously image layer rootfs path is in the format below regardless
> of which backend is used.
>   /layers//rootfs
> This introduced an issue: when agent is restarted with a different
> backend, we will wrongly handle the whiteout files since different
> backends(e.g., aufs and overlay) have different whiteout standard.
> 
> In this commit, we added backend suffix to image layer rootfs path
> for overlay backend like below.
>   /layers//rootfs.overlay
> For non-overlay backends, it is still in the previous format, this
> is because they share the same whiteout standard (aufs standard),
> and also used to handle backward compatibility.
> 
> So the expected result of this commit is:
> 1. If user switches backend from overlay to a non-overlay (or vice
> versa) when restarting agent, all the image layers of the previous
> backend in the store will just be ignored.
> 2. In the upgrade case, if user starts a new version of Mesos agent
> with overlay backend, then all the image layers in the store pulled
> by the old agent will just be ignored, but if user starts the new
> agent with a non-overlay backend, then all the image layers in the
> store can still be used.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> 9b09dca5cd8f9e60a90cf574300b250eb18ede35 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> 6456d90220a4026696a04f9969763cf682464353 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
> 949e0c16a361f22b00e267fcf81093690327041f 
>   src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
> a5cc952072274c61e8c515e8b1b7ea563344e950 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 1e2cc2dda3518d99ef1ad06e2b8ea7f78a4dcf72 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 52b9ea934a1dbe3312b8f08f5da8085e9e306de5 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 10fbc4149ac2e7503ffe7f2746fbd0e14a2365b4 
> 
> Diff: https://reviews.apache.org/r/52827/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 52855: Re-enabled the change user test in slave tests.

[Gilbert Song]

> On Oct. 14, 2016, 11:56 a.m., Gilbert Song wrote:
> > src/tests/slave_tests.cpp, lines 1163-1165
> > 
> >
> > Use `createDockerImage()`.

Please discard this comment.


- Gilbert


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


On Oct. 13, 2016, 8:36 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52855/
> ---
> 
> (Updated Oct. 13, 2016, 8:36 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-6391
> https://issues.apache.org/jira/browse/MESOS-6391
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that the sandbox of the command task is writable if
> the task specifies a non privileged user.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 6bc3835cbb62536ec933ef38c9e15138b8611e5f 
>   src/tests/slave_tests.cpp 93b81d3e5b90d8036409e943f598c865fe335bcc 
> 
> Diff: https://reviews.apache.org/r/52855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 52470: Filled missing executor info in tasks when `LAUNCH_GROUP`.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Oct. 13, 2016, 6:36 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52470/
> ---
> 
> (Updated Oct. 13, 2016, 6:36 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6283
> https://issues.apache.org/jira/browse/MESOS-6283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fixed the navigate error in Web UI because Web UI uses the
> executor id of the task to search the corresponding sandbox directory.
> Web UI uses the task id as the executor id if the executor id of the
> task is empty when searching the sandbox directory. It works fine when
> tasks are launched by `CommandExecutor` because the executor id of the
> task is equal to the task id in this case. However, when tasks are
> launched by `DefaultExecutor`, the executor id of the task is defined
> in the framework side and may different with the task id. So we need to
> fill the `ExecutorInfo` of the `TaskInfo` when `LAUNCH_GROUP` to avoid
> the Web UI uses incorrect executor id to search sandbox directory.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 05988d4a5cc387f6eafe6453170bcc8764ee1247 
>   include/mesos/v1/mesos.proto 08a536cf345663f81d03e53625c640f946ce5079 
>   src/master/master.hpp 43518b9bf1bfaa54e26acc7f2e70c4161c667a84 
>   src/master/master.cpp 7ef898781ce5c2349ffeaa3ce43e68dede19c852 
>   src/master/validation.cpp e5da3c9bf0a1042b42522f1ab74ce798fbb1738d 
>   src/tests/default_executor_tests.cpp 
> 9e0fd678a7f5e9c2288fbb11a60cf6f339efa24f 
>   src/tests/master_validation_tests.cpp 
> 99e350e0587e73e9ee25ef20dd369cd146bd446a 
> 
> Diff: https://reviews.apache.org/r/52470/diff/
> 
> 
> Testing
> ---
> 
> Add new test case `DefaultExecutorTest.ROOT_TaskUsesExecutor`.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52855: Re-enabled the change user test in slave tests.

[Gilbert Song]

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


Fix it, then Ship it!





src/tests/slave_tests.cpp (line 1045)


s/ROOT_RunTaskWithCommandInfoWithUser/ROOT_RunTaskWithCommandInfoUser



src/tests/slave_tests.cpp (lines 1106 - 1108)


Use `createDockerImage()`.



src/tests/slave_tests.cpp (lines 1106 - 1112)


Use `createContainerInfo()`.


- Gilbert Song


On Oct. 13, 2016, 8:36 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52855/
> ---
> 
> (Updated Oct. 13, 2016, 8:36 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-6391
> https://issues.apache.org/jira/browse/MESOS-6391
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that the sandbox of the command task is writable if
> the task specifies a non privileged user.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 6bc3835cbb62536ec933ef38c9e15138b8611e5f 
>   src/tests/slave_tests.cpp 93b81d3e5b90d8036409e943f598c865fe335bcc 
> 
> Diff: https://reviews.apache.org/r/52855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 52730: Added an abstraction for Envp pointer expected by exec routines.

[Alex Clemmer]

> On Oct. 11, 2016, 8:13 p.m., Alex Clemmer wrote:
> > Just a quick point here, there is a semantic mismatch between Windows and 
> > (all?) modern POSIX implementations in the C standard environment APIs. 
> > Much of this is captured in [1]; if I'm remembering right, I ended up 
> > concluding that it was likely that we were going to have to retire 
> > `os::environ` and `os::raw::environ` to replace it with a 
> > platform-independent environment API. Specifically this would probably 
> > entail replacing every function that expects a POSIX environment, with a 
> > corresponding function that takes the platform-independent environment 
> > object.
> > 
> > It looks like this review is most of the way there already... if you think 
> > this makes sense as a goal for the work you're doing here, I'm happy to add 
> > some suggestions about what would be helpful on the Windows side to resolve 
> > our issues.
> > 
> > 
> > [1] https://issues.apache.org/jira/browse/MESOS-5880
> 
> Jie Yu wrote:
> Alex, sure. I scan the code. Looks like after my chain here, the only 
> place that we need to change is `os::environment` which currently uses 
> `os::raw::environment`. I think we can move `os::raw::environment` to posix, 
> and split the impl. of `os::environment`. Let me know if that's what you have 
> in mind or not. Thanks for the heads up.

Following this up, this was checked in to unblock a bug. I'll come back and 
massage it later when I fix MESOS-5880. I have a long list of bugs to fix, so 
I'm not sure when that will be. :| Sorry, folks.


- Alex


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


On Oct. 11, 2016, 5:38 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52730/
> ---
> 
> (Updated Oct. 11, 2016, 5:38 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Kevin Klues.
> 
> 
> Bugs: MESOS-6323
> https://issues.apache.org/jira/browse/MESOS-6323
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an abstraction for Envp pointer expected by exec routines.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/raw/environment.hpp 
> 80cc45b24cfe6f6429d4d0ed4acdafba51c590eb 
> 
> Diff: https://reviews.apache.org/r/52730/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 52848: Cleaned up style in stout rmdir_tests.cpp.

[Alex Clemmer]

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


Ship it!




Ship It!

- Alex Clemmer


On Oct. 13, 2016, 11:48 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52848/
> ---
> 
> (Updated Oct. 13, 2016, 11:48 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Alex Clemmer, Joris Van Remoortere, 
> and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change a few test variables to `const` and made use of initializer lists
> as these variables are only modified once at the beginning of the tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 
> ffe234baac305e26b5a29cffcdd310350d10167e 
> 
> Diff: https://reviews.apache.org/r/52848/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX)
> 
> Windows:
> msbuild Mesos.sln /p:PreferredToolArchitecture=x64 /m /t:stout_tests
> 
> On an Administrator CmdPrompt:
> "3rdparty/stout/tests/Debug/stout_tests.exe"
> ```
> [ RUN  ] RmdirTest.RemoveDirectoryWithNoTargetSymbolicLink
> C:\mesos\3rdparty\stout\tests\os\rmdir_tests.cpp(285): error: 
> fs::symlink("tmp", link): '_stat' failed on path 'C:\tmp\vehk5d\tmp': No such 
> file or directory
> [  FAILED  ] RmdirTest.RemoveDirectoryWithNoTargetSymbolicLink (4 ms)
> 
> [--] Global test environment tear-down
> [==] 187 tests from 32 test cases ran. (2445 ms total)
> [  PASSED  ] 185 tests.
> [  FAILED  ] 2 tests, listed below:
> [  FAILED  ] NetTest.LinkDevice
> [  FAILED  ] RmdirTest.RemoveDirectoryWithNoTargetSymbolicLink
> ```
> ^ That was failing before; no change in this review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 52881: Made sure required capabilities are not dropped in capabilities test.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52881]

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. 14, 2016, 2:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52881/
> ---
> 
> (Updated Oct. 14, 2016, 2:16 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6386
> https://issues.apache.org/jira/browse/MESOS-6386
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The capabilities isolator test suites runs test as root where the
> files executed might not reside in directories accessible even to root
> after dropping all capabilities. We already ensured that the test
> agent would always permit `DAC_READ_SEARCH` so that we could move this
> one into the permitted set, but missed to ensure it was always present
> when tasks set capabilities. This could lead to situtations where
> e.g., `mesos-executor` could not be executed by the test.
> 
> This commit adds `DAC_READ_SEARCH` to the requested set for all
> situation where where drop all capabilities required for tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
> f040c209b4b4c87cef00b0569b7da7581f4ccf03 
> 
> Diff: https://reviews.apache.org/r/52881/diff/
> 
> 
> Testing
> ---
> 
> * as root: `./mesos-test 
> --gtest_filter='*/LinuxCapabilitiesIsolatorTest.ROOT_Ping/*'`
> * confirmed by inspection of the log output that all `launch` invocations 
> were able
>   to successly start the requested executables.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52828: Allow the chown code in fetcher to be executed.

[Megha Sharma]

> On Oct. 13, 2016, 11:47 p.m., Jiang Yan Xu wrote:
> > src/tests/slave_tests.cpp, line 932
> > 
> >
> > "test" worked? Could you verify?

"test" didn't work since we are chowning so needs a real user. Replaced it with 
"nobody".


- Megha


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


On Oct. 14, 2016, 6:14 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52828/
> ---
> 
> (Updated Oct. 14, 2016, 6:14 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: mesos-5218
> https://issues.apache.org/jira/browse/mesos-5218
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved the uri.size() check in fetcher so that the chown to
> task user of stdout/stderr in sandbox directory happens even
> when there is no uri to be fetched.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> 11104d66e6dd05d8eb1d37a2e3250aca19278110 
>   src/tests/slave_tests.cpp 93b81d3e5b90d8036409e943f598c865fe335bcc 
> 
> Diff: https://reviews.apache.org/r/52828/diff/
> 
> 
> Testing
> ---
> 
> make check on macOs and linux
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 52828: Allow the chown code in fetcher to be executed.

[Megha Sharma]

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

(Updated Oct. 14, 2016, 6:14 p.m.)


Review request for mesos and Jiang Yan Xu.


Bugs: mesos-5218
https://issues.apache.org/jira/browse/mesos-5218


Repository: mesos


Description
---

Moved the uri.size() check in fetcher so that the chown to
task user of stdout/stderr in sandbox directory happens even
when there is no uri to be fetched.


Diffs (updated)
-

  src/slave/containerizer/fetcher.cpp 11104d66e6dd05d8eb1d37a2e3250aca19278110 
  src/tests/slave_tests.cpp 93b81d3e5b90d8036409e943f598c865fe335bcc 

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


Testing
---

make check on macOs and linux


Thanks,

Megha Sharma

Re: Review Request 52058: Fixed fetcher to not call chown on sandbox again.

[Megha Sharma]

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

(Updated Oct. 14, 2016, 6:14 p.m.)


Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

Fetcher code changes the ownership of entire sandbox directory
recursively to the taskuser and as a resut also changes the
ownership of files laid out by other entities in the sandbox.


Diffs (updated)
-

  src/slave/containerizer/fetcher.cpp 11104d66e6dd05d8eb1d37a2e3250aca19278110 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
ad2070246b7fdb90aa6ed02326b5a7eb95365997 

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


Testing
---

make check on linux and macos


Thanks,

Megha Sharma

Re: Review Request 52787: Set `runtime_dir` to a temporary folder in `mesos-local`.

[haosdent huang]

> On Oct. 14, 2016, 5:48 p.m., Kevin Klues wrote:
> > This still doesn't have the right semantics:
> > ```
> > [klueska@core-dev build]$ ./bin/mesos-local.sh
> > I1014 10:43:56.385072 39941 master.cpp:382] Flags at startup: ... 
> > --runtime_dir="/tmp/mesos/0" ... --work_dir="/tmp/mesos/0"
> > ```
> > 
> > The `--runtime_dir` and `--work_dir` should have different values.
> > 
> > Also:
> > ```
> > [klueska@core-dev build]$ MESOS_RUNTIME_DIR=/tmp/kevin ./bin/mesos-local.sh
> > I1014 10:43:56.385072 39941 master.cpp:382] Flags at startup: ... 
> > --runtime_dir="/tmp/mesos/0" ... --work_dir="/tmp/mesos/0"
> > ```
> > 
> > I should be able to override the `--runtime_dir` with the environment 
> > variable.
> 
> Kevin Klues wrote:
> Changing `mesos-local-flags.in.sh` to have the following should address 
> both issues:
> 
> ```
> : ${MESOS_WORK_DIR:=/tmp/mesos/work_dir}
> : ${MESOS_RUNTIME_DIR:=/tmp/mesos/runtime_dir}
> 
> export MESOS_WORK_DIR
> export MESOS_RUNTIME_DIR
> ```
> 
> haosdent huang wrote:
> yep, now use `.sh` would have this problem. Use `src/mesos-local` would 
> be fine.

>The --runtime_dir and --work_dir should have different values.

Put both of them under the same folder `/tmp/mesos/0` should be safe? Because I 
saw their folders are not conflict.


- haosdent


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


On Oct. 14, 2016, 4:26 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52787/
> ---
> 
> (Updated Oct. 14, 2016, 4:26 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Kevin Klues.
> 
> 
> Bugs: MESOS-6380
> https://issues.apache.org/jira/browse/MESOS-6380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set `runtime_dir` to a temporary folder in `mesos-local`.
> 
> 
> Diffs
> -
> 
>   bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
>   src/local/flags.hpp c77eff13e7a63630ff6c9428e57f3f6707e1f30f 
>   src/local/local.cpp 257179443827ffdfc946ed655a6840fcea70d454 
> 
> Diff: https://reviews.apache.org/r/52787/diff/
> 
> 
> Testing
> ---
> 
> To void this error when launch `mesos-local` without `sudo`
> 
> ```
>   message: 'Failed to launch container: Failed to make the containerizer 
> runtime directory 
> '/var/run/mesos/containers/f2d6947f-2916-4f1a-90dc-3d137b360b9c': Permission 
> denied; Abnormal executor termination: unknown container'
> ```
> 
> Test produce 
> 
> ```
> # Run with normal user.
> $ ./bin/mesos-local.sh
> 
> # Open a new session
> $ ./src/mesos-execute --master=127.0.0.1:5050 --name="test-single-1" 
> --command="sleep 2000"
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> W1014 00:05:22.457461 36489 scheduler.cpp:161]
> **
> Scheduler driver bound to loopback interface! Cannot communicate with remote 
> master(s). You might want to set 'LIBPROCESS_IP' environment variable to use 
> a routable IP address.
> **
> I1014 00:05:22.457690 36489 scheduler.cpp:176] Version: 1.1.0
> I1014 00:05:22.596534 36497 scheduler.cpp:465] New master detected at 
> master@127.0.0.1:5050
> Subscribed with ID 51e0d33f-d7b2-4c01-9bb3-10731cc7d1ab-
> Submitted task 'test-single-1' to agent 
> '8fba7ecb-b70e-48ac-9541-2070c4d9b1d5-S0'
> Received status update TASK_RUNNING for task 'test-single-1'
>   source: SOURCE_EXECUTOR
> ```
> 
> ```
> $ sudo make install
> $ mesos-local
> ...
> I1014 12:23:47.563256 4284416 slave.cpp:208] Mesos agent started on 
> (1)@127.0.0.1:5050
> I1014 12:23:47.563302 4284416 slave.cpp:209] Flags at startup: 
> --appc_simple_discovery_uri_prefix="http://; 
> --appc_store_dir="/tmp/mesos/store/appc" --authenticate_http_readonly="false" 
> --authenticate_http_readwrite="false" --authenticatee="crammd5" 
> --authentication_backoff_factor="1secs" --authorizer="local" 
> --container_disk_watch_interval="15secs" --containerizers="mesos" 
> --default_role="*" --disk_watch_interval="1mins" --docker="docker" 
> --docker_kill_orphans="true" --docker_registry="https://registry-1.docker.io; 
> --docker_remove_delay="6hrs" --docker_socket="/var/run/docker.sock" 
> --docker_stop_timeout="0ns" --docker_store_dir="/tmp/mesos/store/docker" 
> --docker_volume_checkpoint_dir="/var/run/mesos/isolators/docker/volume" 
> --enforce_container_disk_quota="false" 
> --executor_registration_timeout="1mins" 
> --executor_shutdown_grace_period="5secs" 
> --fetcher_cache_dir="/tmp/mesos/fetch" --fetcher_cache_size="2GB" 
> --frameworks_home="" --gc_delay="1weeks" --gc_disk_headroom="0.1" --hadoop_
 home="" 

Re: Review Request 52787: Set `runtime_dir` to a temporary folder in `mesos-local`.

[haosdent huang]

> On Oct. 14, 2016, 5:48 p.m., Kevin Klues wrote:
> > This still doesn't have the right semantics:
> > ```
> > [klueska@core-dev build]$ ./bin/mesos-local.sh
> > I1014 10:43:56.385072 39941 master.cpp:382] Flags at startup: ... 
> > --runtime_dir="/tmp/mesos/0" ... --work_dir="/tmp/mesos/0"
> > ```
> > 
> > The `--runtime_dir` and `--work_dir` should have different values.
> > 
> > Also:
> > ```
> > [klueska@core-dev build]$ MESOS_RUNTIME_DIR=/tmp/kevin ./bin/mesos-local.sh
> > I1014 10:43:56.385072 39941 master.cpp:382] Flags at startup: ... 
> > --runtime_dir="/tmp/mesos/0" ... --work_dir="/tmp/mesos/0"
> > ```
> > 
> > I should be able to override the `--runtime_dir` with the environment 
> > variable.
> 
> Kevin Klues wrote:
> Changing `mesos-local-flags.in.sh` to have the following should address 
> both issues:
> 
> ```
> : ${MESOS_WORK_DIR:=/tmp/mesos/work_dir}
> : ${MESOS_RUNTIME_DIR:=/tmp/mesos/runtime_dir}
> 
> export MESOS_WORK_DIR
> export MESOS_RUNTIME_DIR
> ```

yep, now use `.sh` would have this problem. Use `src/mesos-local` would be fine.


- haosdent


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


On Oct. 14, 2016, 4:26 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52787/
> ---
> 
> (Updated Oct. 14, 2016, 4:26 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Kevin Klues.
> 
> 
> Bugs: MESOS-6380
> https://issues.apache.org/jira/browse/MESOS-6380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set `runtime_dir` to a temporary folder in `mesos-local`.
> 
> 
> Diffs
> -
> 
>   bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
>   src/local/flags.hpp c77eff13e7a63630ff6c9428e57f3f6707e1f30f 
>   src/local/local.cpp 257179443827ffdfc946ed655a6840fcea70d454 
> 
> Diff: https://reviews.apache.org/r/52787/diff/
> 
> 
> Testing
> ---
> 
> To void this error when launch `mesos-local` without `sudo`
> 
> ```
>   message: 'Failed to launch container: Failed to make the containerizer 
> runtime directory 
> '/var/run/mesos/containers/f2d6947f-2916-4f1a-90dc-3d137b360b9c': Permission 
> denied; Abnormal executor termination: unknown container'
> ```
> 
> Test produce 
> 
> ```
> # Run with normal user.
> $ ./bin/mesos-local.sh
> 
> # Open a new session
> $ ./src/mesos-execute --master=127.0.0.1:5050 --name="test-single-1" 
> --command="sleep 2000"
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> W1014 00:05:22.457461 36489 scheduler.cpp:161]
> **
> Scheduler driver bound to loopback interface! Cannot communicate with remote 
> master(s). You might want to set 'LIBPROCESS_IP' environment variable to use 
> a routable IP address.
> **
> I1014 00:05:22.457690 36489 scheduler.cpp:176] Version: 1.1.0
> I1014 00:05:22.596534 36497 scheduler.cpp:465] New master detected at 
> master@127.0.0.1:5050
> Subscribed with ID 51e0d33f-d7b2-4c01-9bb3-10731cc7d1ab-
> Submitted task 'test-single-1' to agent 
> '8fba7ecb-b70e-48ac-9541-2070c4d9b1d5-S0'
> Received status update TASK_RUNNING for task 'test-single-1'
>   source: SOURCE_EXECUTOR
> ```
> 
> ```
> $ sudo make install
> $ mesos-local
> ...
> I1014 12:23:47.563256 4284416 slave.cpp:208] Mesos agent started on 
> (1)@127.0.0.1:5050
> I1014 12:23:47.563302 4284416 slave.cpp:209] Flags at startup: 
> --appc_simple_discovery_uri_prefix="http://; 
> --appc_store_dir="/tmp/mesos/store/appc" --authenticate_http_readonly="false" 
> --authenticate_http_readwrite="false" --authenticatee="crammd5" 
> --authentication_backoff_factor="1secs" --authorizer="local" 
> --container_disk_watch_interval="15secs" --containerizers="mesos" 
> --default_role="*" --disk_watch_interval="1mins" --docker="docker" 
> --docker_kill_orphans="true" --docker_registry="https://registry-1.docker.io; 
> --docker_remove_delay="6hrs" --docker_socket="/var/run/docker.sock" 
> --docker_stop_timeout="0ns" --docker_store_dir="/tmp/mesos/store/docker" 
> --docker_volume_checkpoint_dir="/var/run/mesos/isolators/docker/volume" 
> --enforce_container_disk_quota="false" 
> --executor_registration_timeout="1mins" 
> --executor_shutdown_grace_period="5secs" 
> --fetcher_cache_dir="/tmp/mesos/fetch" --fetcher_cache_size="2GB" 
> --frameworks_home="" --gc_delay="1weeks" --gc_disk_headroom="0.1" --hadoop_
 home="" --help="false" --hostname_lookup="true" --http_authenticators="basic" 
--http_command_executor="false" --image_provisioner_backend="copy" 
--initialize_driver_logging="true" --isolation="posix/cpu,posix/mem" 

Re: Review Request 52787: Set `runtime_dir` to a temporary folder in `mesos-local`.

[Kevin Klues]

> On Oct. 14, 2016, 5:48 p.m., Kevin Klues wrote:
> > This still doesn't have the right semantics:
> > ```
> > [klueska@core-dev build]$ ./bin/mesos-local.sh
> > I1014 10:43:56.385072 39941 master.cpp:382] Flags at startup: ... 
> > --runtime_dir="/tmp/mesos/0" ... --work_dir="/tmp/mesos/0"
> > ```
> > 
> > The `--runtime_dir` and `--work_dir` should have different values.
> > 
> > Also:
> > ```
> > [klueska@core-dev build]$ MESOS_RUNTIME_DIR=/tmp/kevin ./bin/mesos-local.sh
> > I1014 10:43:56.385072 39941 master.cpp:382] Flags at startup: ... 
> > --runtime_dir="/tmp/mesos/0" ... --work_dir="/tmp/mesos/0"
> > ```
> > 
> > I should be able to override the `--runtime_dir` with the environment 
> > variable.

Changing `mesos-local-flags.in.sh` to have the following should address both 
issues:

```
: ${MESOS_WORK_DIR:=/tmp/mesos/work_dir}
: ${MESOS_RUNTIME_DIR:=/tmp/mesos/runtime_dir}

export MESOS_WORK_DIR
export MESOS_RUNTIME_DIR
```


- Kevin


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


On Oct. 14, 2016, 4:26 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52787/
> ---
> 
> (Updated Oct. 14, 2016, 4:26 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Kevin Klues.
> 
> 
> Bugs: MESOS-6380
> https://issues.apache.org/jira/browse/MESOS-6380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set `runtime_dir` to a temporary folder in `mesos-local`.
> 
> 
> Diffs
> -
> 
>   bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
>   src/local/flags.hpp c77eff13e7a63630ff6c9428e57f3f6707e1f30f 
>   src/local/local.cpp 257179443827ffdfc946ed655a6840fcea70d454 
> 
> Diff: https://reviews.apache.org/r/52787/diff/
> 
> 
> Testing
> ---
> 
> To void this error when launch `mesos-local` without `sudo`
> 
> ```
>   message: 'Failed to launch container: Failed to make the containerizer 
> runtime directory 
> '/var/run/mesos/containers/f2d6947f-2916-4f1a-90dc-3d137b360b9c': Permission 
> denied; Abnormal executor termination: unknown container'
> ```
> 
> Test produce 
> 
> ```
> # Run with normal user.
> $ ./bin/mesos-local.sh
> 
> # Open a new session
> $ ./src/mesos-execute --master=127.0.0.1:5050 --name="test-single-1" 
> --command="sleep 2000"
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> W1014 00:05:22.457461 36489 scheduler.cpp:161]
> **
> Scheduler driver bound to loopback interface! Cannot communicate with remote 
> master(s). You might want to set 'LIBPROCESS_IP' environment variable to use 
> a routable IP address.
> **
> I1014 00:05:22.457690 36489 scheduler.cpp:176] Version: 1.1.0
> I1014 00:05:22.596534 36497 scheduler.cpp:465] New master detected at 
> master@127.0.0.1:5050
> Subscribed with ID 51e0d33f-d7b2-4c01-9bb3-10731cc7d1ab-
> Submitted task 'test-single-1' to agent 
> '8fba7ecb-b70e-48ac-9541-2070c4d9b1d5-S0'
> Received status update TASK_RUNNING for task 'test-single-1'
>   source: SOURCE_EXECUTOR
> ```
> 
> ```
> $ sudo make install
> $ mesos-local
> ...
> I1014 12:23:47.563256 4284416 slave.cpp:208] Mesos agent started on 
> (1)@127.0.0.1:5050
> I1014 12:23:47.563302 4284416 slave.cpp:209] Flags at startup: 
> --appc_simple_discovery_uri_prefix="http://; 
> --appc_store_dir="/tmp/mesos/store/appc" --authenticate_http_readonly="false" 
> --authenticate_http_readwrite="false" --authenticatee="crammd5" 
> --authentication_backoff_factor="1secs" --authorizer="local" 
> --container_disk_watch_interval="15secs" --containerizers="mesos" 
> --default_role="*" --disk_watch_interval="1mins" --docker="docker" 
> --docker_kill_orphans="true" --docker_registry="https://registry-1.docker.io; 
> --docker_remove_delay="6hrs" --docker_socket="/var/run/docker.sock" 
> --docker_stop_timeout="0ns" --docker_store_dir="/tmp/mesos/store/docker" 
> --docker_volume_checkpoint_dir="/var/run/mesos/isolators/docker/volume" 
> --enforce_container_disk_quota="false" 
> --executor_registration_timeout="1mins" 
> --executor_shutdown_grace_period="5secs" 
> --fetcher_cache_dir="/tmp/mesos/fetch" --fetcher_cache_size="2GB" 
> --frameworks_home="" --gc_delay="1weeks" --gc_disk_headroom="0.1" --hadoop_
 home="" --help="false" --hostname_lookup="true" --http_authenticators="basic" 
--http_command_executor="false" --image_provisioner_backend="copy" 
--initialize_driver_logging="true" --isolation="posix/cpu,posix/mem" 
--launcher="posix" --launcher_dir="/usr/local/libexec/mesos" --logbufsecs="0" 
--logging_level="INFO" --max_completed_executors_per_framework="150" 

Re: Review Request 52881: Made sure required capabilities are not dropped in capabilities test.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Oct. 14, 2016, 2:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52881/
> ---
> 
> (Updated Oct. 14, 2016, 2:16 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6386
> https://issues.apache.org/jira/browse/MESOS-6386
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The capabilities isolator test suites runs test as root where the
> files executed might not reside in directories accessible even to root
> after dropping all capabilities. We already ensured that the test
> agent would always permit `DAC_READ_SEARCH` so that we could move this
> one into the permitted set, but missed to ensure it was always present
> when tasks set capabilities. This could lead to situtations where
> e.g., `mesos-executor` could not be executed by the test.
> 
> This commit adds `DAC_READ_SEARCH` to the requested set for all
> situation where where drop all capabilities required for tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
> f040c209b4b4c87cef00b0569b7da7581f4ccf03 
> 
> Diff: https://reviews.apache.org/r/52881/diff/
> 
> 
> Testing
> ---
> 
> * as root: `./mesos-test 
> --gtest_filter='*/LinuxCapabilitiesIsolatorTest.ROOT_Ping/*'`
> * confirmed by inspection of the log output that all `launch` invocations 
> were able
>   to successly start the requested executables.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52705: Added 'stout/wait.hpp' header.

[Kevin Klues]

> On Oct. 11, 2016, 8:48 p.m., Alex Clemmer wrote:
> > 3rdparty/stout/include/stout/wait.hpp, line 25
> > 
> >
> > Couple of questions here:
> > 
> > * This seems useful for POSIX platforms, but I don't really understand 
> > the implications of having this enabled on Windows. Windows will never 
> > support the signal semantics of POSIX, and in general my feeling is that 
> > I'd rather not define things on platforms that don't support those 
> > semantics, unless there is a particularly pertinent reason to. Thoughts?
> > * In the 2 or 3 places we do use `W_EXITCODE`, rather than defining it 
> > for Windows, we simply `#ifdef` the code out. So the precedent is to just 
> > not use the macro. Is there a compelling reason to change this?
> > 
> > 
> > Also, tiny, tiny nit question: is it style to have 2 spaces between 
> > `W_EXITCODE(ret, sig)` and `((ret) << 8 | (sig))`
> 
> Kevin Klues wrote:
> To answer both your bullets at once...
> If you look in `windows.hpp` all of the other `W*` macros from the posix 
> standard are defined in there (even though they may be meaningless on 
> windows). This is consistent with them all being defined in `sys/wait.h` on a 
> posix compliant system. This is why I separated the two by a simple `#ifdef`
> 
> Adding the code below to be added to both windows and poix systems:
> 
> ```
> #ifndef W_EXITCODE
> #define W_EXITCODE(ret, sig)  ((ret) << 8 | (sig))
> #endif
> ```
> 
> is just to cover a corner case where some libc variants don't actually 
> define this macro (because it's not technically posix compliant). Almost all 
> libc variants do define it, but a bug was filed against this because 
> (apparently) `musl` does not.
> 
> Given that adding this code makes both windows and any variant of libc 
> now define all of the `W*` macros symetrically, I think this is likely the 
> right way to do it for now. In the future when we completely strip out all 
> `W*` macros from windows, we can revisit this. 
> 
> ---
> 
> Regarding the formatting question. I just copy and pasted this from the 
> glibc header file. I don't think we have a preferred style for this, but I 
> tried it both ways just now, and it seems to be more readable as 2 spaces, so 
> I'll leave it.
> 
> Alex Clemmer wrote:
> I'm fine with this change being submitted because the applicable domain 
> of error is so small. But, I am hoping we can agree to not define 
> semantically empty macros in the future on Windows codepaths in the future. 
> So I will add some historical perspective to this thread. :)
> 
> The `W*` macros you bring up that made it into `windows.hpp` all made it 
> for extremely pragmatic reasons. We're not at all happy _per se_ with this 
> outcome.
> 
> For example, some of the macros are doing things like checking for the 
> results of signals that do not exist on Windows; our decision calculus, 
> essentially, was: (1) it hurt readability too much to `#ifdef` out their 
> usage in the codebase, (2) we were too time constrained with MesosCon 
> approaching to correctly factor them out correctly, and (3) the places where 
> we were checking these results had a negligible effect on Windows code paths 
> because the signal would never be triggered.
> 
> If `W_EXITCODE` is similarly justified, I don't quite see it. Maybe I'm 
> missing something. It looks like it's used twice on non-Windows codepaths.
> 
> I'm definitely sympathetic to the idea that we do not want to decrease 
> readability in the POSIX codepaths to help Windows, a minority use case, but 
> I think that dumping POSIX stuff into the Windows codepaths is a really 
> dangerous habit to get into, particularly when it's not needed. This has 
> caused me, personally, probably a hundred hours of debugging, all told.
> 
> Let me know if I'm missing something important here, but for now, my 
> recommendation is that we strongly avoid defining things we don't have a 
> strict operational justification for.

I agree on all fronts. We should always be mindful of this.


- Kevin


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


On Oct. 13, 2016, 12:08 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52705/
> ---
> 
> (Updated Oct. 13, 2016, 12:08 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6310
> https://issues.apache.org/jira/browse/MESOS-6310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This was motivated 

Re: Review Request 52787: Set `runtime_dir` to a temporary folder in `mesos-local`.

[Kevin Klues]

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



This still doesn't have the right semantics:
```
[klueska@core-dev build]$ ./bin/mesos-local.sh
I1014 10:43:56.385072 39941 master.cpp:382] Flags at startup: ... 
--runtime_dir="/tmp/mesos/0" ... --work_dir="/tmp/mesos/0"
```

The `--runtime_dir` and `--work_dir` should have different values.

Also:
```
[klueska@core-dev build]$ MESOS_RUNTIME_DIR=/tmp/kevin ./bin/mesos-local.sh
I1014 10:43:56.385072 39941 master.cpp:382] Flags at startup: ... 
--runtime_dir="/tmp/mesos/0" ... --work_dir="/tmp/mesos/0"
```

I should be able to override the `--runtime_dir` with the environment variable.

- Kevin Klues


On Oct. 14, 2016, 4:26 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52787/
> ---
> 
> (Updated Oct. 14, 2016, 4:26 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Kevin Klues.
> 
> 
> Bugs: MESOS-6380
> https://issues.apache.org/jira/browse/MESOS-6380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set `runtime_dir` to a temporary folder in `mesos-local`.
> 
> 
> Diffs
> -
> 
>   bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
>   src/local/flags.hpp c77eff13e7a63630ff6c9428e57f3f6707e1f30f 
>   src/local/local.cpp 257179443827ffdfc946ed655a6840fcea70d454 
> 
> Diff: https://reviews.apache.org/r/52787/diff/
> 
> 
> Testing
> ---
> 
> To void this error when launch `mesos-local` without `sudo`
> 
> ```
>   message: 'Failed to launch container: Failed to make the containerizer 
> runtime directory 
> '/var/run/mesos/containers/f2d6947f-2916-4f1a-90dc-3d137b360b9c': Permission 
> denied; Abnormal executor termination: unknown container'
> ```
> 
> Test produce 
> 
> ```
> # Run with normal user.
> $ ./bin/mesos-local.sh
> 
> # Open a new session
> $ ./src/mesos-execute --master=127.0.0.1:5050 --name="test-single-1" 
> --command="sleep 2000"
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> W1014 00:05:22.457461 36489 scheduler.cpp:161]
> **
> Scheduler driver bound to loopback interface! Cannot communicate with remote 
> master(s). You might want to set 'LIBPROCESS_IP' environment variable to use 
> a routable IP address.
> **
> I1014 00:05:22.457690 36489 scheduler.cpp:176] Version: 1.1.0
> I1014 00:05:22.596534 36497 scheduler.cpp:465] New master detected at 
> master@127.0.0.1:5050
> Subscribed with ID 51e0d33f-d7b2-4c01-9bb3-10731cc7d1ab-
> Submitted task 'test-single-1' to agent 
> '8fba7ecb-b70e-48ac-9541-2070c4d9b1d5-S0'
> Received status update TASK_RUNNING for task 'test-single-1'
>   source: SOURCE_EXECUTOR
> ```
> 
> ```
> $ sudo make install
> $ mesos-local
> ...
> I1014 12:23:47.563256 4284416 slave.cpp:208] Mesos agent started on 
> (1)@127.0.0.1:5050
> I1014 12:23:47.563302 4284416 slave.cpp:209] Flags at startup: 
> --appc_simple_discovery_uri_prefix="http://; 
> --appc_store_dir="/tmp/mesos/store/appc" --authenticate_http_readonly="false" 
> --authenticate_http_readwrite="false" --authenticatee="crammd5" 
> --authentication_backoff_factor="1secs" --authorizer="local" 
> --container_disk_watch_interval="15secs" --containerizers="mesos" 
> --default_role="*" --disk_watch_interval="1mins" --docker="docker" 
> --docker_kill_orphans="true" --docker_registry="https://registry-1.docker.io; 
> --docker_remove_delay="6hrs" --docker_socket="/var/run/docker.sock" 
> --docker_stop_timeout="0ns" --docker_store_dir="/tmp/mesos/store/docker" 
> --docker_volume_checkpoint_dir="/var/run/mesos/isolators/docker/volume" 
> --enforce_container_disk_quota="false" 
> --executor_registration_timeout="1mins" 
> --executor_shutdown_grace_period="5secs" 
> --fetcher_cache_dir="/tmp/mesos/fetch" --fetcher_cache_size="2GB" 
> --frameworks_home="" --gc_delay="1weeks" --gc_disk_headroom="0.1" --hadoop_
 home="" --help="false" --hostname_lookup="true" --http_authenticators="basic" 
--http_command_executor="false" --image_provisioner_backend="copy" 
--initialize_driver_logging="true" --isolation="posix/cpu,posix/mem" 
--launcher="posix" --launcher_dir="/usr/local/libexec/mesos" --logbufsecs="0" 
--logging_level="INFO" --max_completed_executors_per_framework="150" 
--oversubscribed_resources_interval="15secs" 
--qos_correction_interval_min="0ns" --quiet="false" --recover="reconnect" 
--recovery_timeout="15mins" --registration_backoff_factor="1secs" 
--runtime_dir="/tmp/mesos/local/0" --sandbox_directory="/mnt/mesos/sandbox" 
--strict="true" --switch_user="true" --version="false" 
--work_dir="/tmp/mesos/local/0"
> ...
> 
> $ mesos-execute 

Re: Review Request 52854: Fixed the sandbox owner for command tasks.

[Gilbert Song]

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


Ship it!





src/launcher/posix/executor.cpp (lines 99 - 100)


I guess you dont want to introduce another workaround in fetcher to `chown` 
files, right? And most likely we may not deprecate the command executor in a 
near term.



src/launcher/posix/executor.cpp (line 102)


Should we add a one-line comment for `not using recursive mode` to chown 
sandbox?


- Gilbert Song


On Oct. 13, 2016, 8:36 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52854/
> ---
> 
> (Updated Oct. 13, 2016, 8:36 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-6391
> https://issues.apache.org/jira/browse/MESOS-6391
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the task has a rootfs, the command executor will be run under root
> because it needs to perform pivot_root. Prior to this patch, if the
> task wants to run under an unprivileged user, the sandbox of that task
> will not be writable because it's owned by root.
> 
> This patch fixed the issue (MESOS-6391). The command executor now
> changes the owner (non-recursively) of the sandbox to match that of
> the task when rootfs is specified for the task.
> 
> 
> Diffs
> -
> 
>   src/launcher/posix/executor.cpp fdee17c5e19b94c350ee192522087051d9c9fe74 
> 
> Diff: https://reviews.apache.org/r/52854/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 52870: Ensured docker executor ignores health updates for terminated tasks.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Oct. 14, 2016, 12:42 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52870/
> ---
> 
> (Updated Oct. 14, 2016, 12:42 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and haosdent huang.
> 
> 
> Bugs: MESOS-5963
> https://issues.apache.org/jira/browse/MESOS-5963
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After the task has been terminated, its health updates become
> irrelevant and should be ignored. Now it is safe to start health
> checks even if the task has been asked to terminate.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 
> 
> Diff: https://reviews.apache.org/r/52870/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52705: Added 'stout/wait.hpp' header.

[Alex Clemmer]

> On Oct. 11, 2016, 8:48 p.m., Alex Clemmer wrote:
> > 3rdparty/stout/include/stout/wait.hpp, line 25
> > 
> >
> > Couple of questions here:
> > 
> > * This seems useful for POSIX platforms, but I don't really understand 
> > the implications of having this enabled on Windows. Windows will never 
> > support the signal semantics of POSIX, and in general my feeling is that 
> > I'd rather not define things on platforms that don't support those 
> > semantics, unless there is a particularly pertinent reason to. Thoughts?
> > * In the 2 or 3 places we do use `W_EXITCODE`, rather than defining it 
> > for Windows, we simply `#ifdef` the code out. So the precedent is to just 
> > not use the macro. Is there a compelling reason to change this?
> > 
> > 
> > Also, tiny, tiny nit question: is it style to have 2 spaces between 
> > `W_EXITCODE(ret, sig)` and `((ret) << 8 | (sig))`
> 
> Kevin Klues wrote:
> To answer both your bullets at once...
> If you look in `windows.hpp` all of the other `W*` macros from the posix 
> standard are defined in there (even though they may be meaningless on 
> windows). This is consistent with them all being defined in `sys/wait.h` on a 
> posix compliant system. This is why I separated the two by a simple `#ifdef`
> 
> Adding the code below to be added to both windows and poix systems:
> 
> ```
> #ifndef W_EXITCODE
> #define W_EXITCODE(ret, sig)  ((ret) << 8 | (sig))
> #endif
> ```
> 
> is just to cover a corner case where some libc variants don't actually 
> define this macro (because it's not technically posix compliant). Almost all 
> libc variants do define it, but a bug was filed against this because 
> (apparently) `musl` does not.
> 
> Given that adding this code makes both windows and any variant of libc 
> now define all of the `W*` macros symetrically, I think this is likely the 
> right way to do it for now. In the future when we completely strip out all 
> `W*` macros from windows, we can revisit this. 
> 
> ---
> 
> Regarding the formatting question. I just copy and pasted this from the 
> glibc header file. I don't think we have a preferred style for this, but I 
> tried it both ways just now, and it seems to be more readable as 2 spaces, so 
> I'll leave it.

I'm fine with this change being submitted because the applicable domain of 
error is so small. But, I am hoping we can agree to not define semantically 
empty macros in the future on Windows codepaths in the future. So I will add 
some historical perspective to this thread. :)

The `W*` macros you bring up that made it into `windows.hpp` all made it for 
extremely pragmatic reasons. We're not at all happy _per se_ with this outcome.

For example, some of the macros are doing things like checking for the results 
of signals that do not exist on Windows; our decision calculus, essentially, 
was: (1) it hurt readability too much to `#ifdef` out their usage in the 
codebase, (2) we were too time constrained with MesosCon approaching to 
correctly factor them out correctly, and (3) the places where we were checking 
these results had a negligible effect on Windows code paths because the signal 
would never be triggered.

If `W_EXITCODE` is similarly justified, I don't quite see it. Maybe I'm missing 
something. It looks like it's used twice on non-Windows codepaths.

I'm definitely sympathetic to the idea that we do not want to decrease 
readability in the POSIX codepaths to help Windows, a minority use case, but I 
think that dumping POSIX stuff into the Windows codepaths is a really dangerous 
habit to get into, particularly when it's not needed. This has caused me, 
personally, probably a hundred hours of debugging, all told.

Let me know if I'm missing something important here, but for now, my 
recommendation is that we strongly avoid defining things we don't have a strict 
operational justification for.


- Alex


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


On Oct. 13, 2016, 12:08 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52705/
> ---
> 
> (Updated Oct. 13, 2016, 12:08 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6310
> https://issues.apache.org/jira/browse/MESOS-6310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This was motivated by the need for a default definition of
> 'W_EXITCODE' (since it is not technically POSIX compliant).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am 

Re: Review Request 52877: Fixed wrong float serialization in JSON in locales different from C.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52877]

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. 14, 2016, 12:50 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52877/
> ---
> 
> (Updated Oct. 14, 2016, 12:50 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Bannier, and Michael Park.
> 
> 
> Bugs: MESOS-6349
> https://issues.apache.org/jira/browse/MESOS-6349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Resolves an isse with the JSON serialization functions where floats
> would use the process active locale as a decimal separator instead
> of the JSON conformant period (`.`).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/json.hpp 
> 9734561549eea867d57f14a0ab71febeb9dddc06 
>   3rdparty/stout/include/stout/jsonify.hpp 
> 8220d001001e8b9f247c05be58534f1174611aad 
> 
> Diff: https://reviews.apache.org/r/52877/diff/
> 
> 
> Testing
> ---
> 
> Three types of tests were used for this issue. The first just run our current
> test suite (`make check`). The second verifies that the proposed solutions
> are able to generate a locale independent output and the third consist of a
> benchmark of all the solutions.
> 
> The verification that the proposed solutions produce locale independent JSON,
> the following program was used (with manual verification of the generated
> output):
> 
> ```c++
> #include 
> 
> #include 
> #include 
> #include 
> 
> int main(int argc, char **argv) {
>   // Set locale to German.
>   std::setlocale(LC_ALL, "de_DE.UTF-8");
>   std::cout.imbue(std::locale("de_DE.UTF-8"));
> 
>   JSON::Object object;
>   object.values["float"] = 1234567890.12345;
> 
>   std::cout << stringify(object) << '\n';
>   return 0;
> }
> 
> ```
> 
> In order to test the different options the following benchmarking program was
> written using [Google benchmark](https://github.com/google/benchmark):
> 
> ```c++
> #include 
> #include 
> 
> #include 
> #include 
> #include 
> #include 
> 
> static void BM_Jsonify(benchmark::State& state) {
>   while (state.KeepRunning()) {
> state.PauseTiming();
> JSON::Object object;
> object.values["float00"] = 1234567890.12345;
> object.values["float01"] = 123456789.012345;
> object.values["float02"] = 12345678.9012345;
> object.values["float03"] = 1234567.89012345;
> object.values["float04"] = 123456.789012345;
> object.values["float05"] = 12345.6789012345;
> object.values["float06"] = 1234.56789012345;
> object.values["float07"] = 123.456789012345;
> object.values["float08"] = 12.3456789012345;
> object.values["float09"] = 1.23456789012345;
> object.values["float10"] = 1234567890.12345;
> object.values["float11"] = 123456789.012345;
> object.values["float12"] = 12345678.9012345;
> object.values["float13"] = 1234567.89012345;
> object.values["float14"] = 123456.789012345;
> object.values["float15"] = 12345.6789012345;
> object.values["float16"] = 1234.56789012345;
> object.values["float17"] = 123.456789012345;
> object.values["float18"] = 12.3456789012345;
> object.values["float19"] = 1.23456789012345;
> object.values["float20"] = 1234567890.12345;
> object.values["float21"] = 123456789.012345;
> object.values["float22"] = 12345678.9012345;
> object.values["float23"] = 1234567.89012345;
> object.values["float24"] = 123456.789012345;
> object.values["float25"] = 12345.6789012345;
> object.values["float26"] = 1234.56789012345;
> object.values["float27"] = 123.456789012345;
> object.values["float28"] = 12.3456789012345;
> object.values["float29"] = 1.23456789012345;
> object.values["float30"] = 1234567890.12345;
> object.values["float31"] = 123456789.012345;
> object.values["float32"] = 12345678.9012345;
> object.values["float33"] = 1234567.89012345;
> object.values["float34"] = 123456.789012345;
> object.values["float35"] = 12345.6789012345;
> object.values["float36"] = 1234.56789012345;
> object.values["float37"] = 123.456789012345;
> object.values["float38"] = 12.3456789012345;
> object.values["float39"] = 1.23456789012345;
> object.values["float40"] = 1234567890.12345;
> object.values["float41"] = 123456789.012345;
> object.values["float42"] = 12345678.9012345;
> object.values["float43"] = 1234567.89012345;
> object.values["float44"] = 123456.789012345;
> object.values["float45"] = 

Re: Review Request 52803: Changed agent to send TASK_GONE.

[Neil Conway]

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

(Updated Oct. 14, 2016, 4:02 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Tweak comment.


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


Repository: mesos


Description
---

The agent previously sent TASK_LOST updates for tasks that are killed
for various reasons, such as containerizer errors or QoS preemption. The
agent now sends TASK_GONE to partition-aware frameworks instead.


Diffs (updated)
-

  src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 
  src/tests/containerizer/docker_containerizer_tests.cpp 
6d26797abf6d2b5e42b9e7743789e1edc62c9c1a 
  src/tests/oversubscription_tests.cpp b356fb62a4e068bc171a75a76001c6d0e76af92a 
  src/tests/slave_tests.cpp 93b81d3e5b90d8036409e943f598c865fe335bcc 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 52671: Modified the `--network_cni_plugins_dir` flag.

[Avinash sridharan]

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

(Updated Oct. 14, 2016, 4:01 p.m.)


Review request for mesos, Jie Yu and Qian Zhang.


Changes
---

Addressed Jie's comments.


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


Repository: mesos


Description
---

The `--network_cni_plugins_dir` was initially designed to take in a
single directory where all the CNI plugins were expected to be
present. This however is limiting since the operator will have to
ensure that all 3rd party plugins are installed in the same location
which a very hard constraint.

To make things simpler we are therefore converting the
`--network_cni_plugins_dir` from a single directory into a search
path.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
1b22b28825e8160f659c3cbac37cc576f01666d5 
  src/slave/flags.cpp 7f79cfcc7939680c38a3d0cd57471cc9976aff7c 

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


Testing
---

make, make check and sudo ./bin/mesos-tests.sh --gtest_filter=Cni*

Also ran a single node cluster and tested the flags by moving the bridge plugin 
from directory to another.


Thanks,

Avinash sridharan

Re: Review Request 52827: Added backend suffix to image layer rootfs path.

[Qian Zhang]

> On Oct. 14, 2016, 7:34 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/provisioner/docker/paths.cpp, lines 61-64
> > 
> >
> > In fact, i think we should use aufs style whiteout by default because 
> > that's also the OCI standard. So the logic here should be:
> > ```
> > if (backend == "overlay") {
> >   return path::join(layerPath, "rootfs." + backend);
> > }
> > 
> > return path::join(layerPath, "rootfs");
> > ```
> > 
> > Also, I think we should define constants for "overlay" here as it has 
> > been referenced multiple times.

Thanks for the comments! And I will address the constants in a separate patch 
later.


- Qian


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


On Oct. 14, 2016, 11:36 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52827/
> ---
> 
> (Updated Oct. 14, 2016, 11:36 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
> https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously image layer rootfs path is in the format below regardless
> of which backend is used.
>   /layers//rootfs
> This introduced an issue: when agent is restarted with a different
> backend, we will wrongly handle the whiteout files since different
> backends(e.g., aufs and overlay) have different whiteout standard.
> 
> In this commit, we added backend suffix to image layer rootfs path
> for overlay backend like below.
>   /layers//rootfs.overlay
> For non-overlay backends, it is still in the previous format, this
> is because they share the same whiteout standard (aufs standard),
> and also used to handle backward compatibility.
> 
> So the expected result of this commit is:
> 1. If user switches backend from overlay to a non-overlay (or vice
> versa) when restarting agent, all the image layers of the previous
> backend in the store will just be ignored.
> 2. In the upgrade case, if user starts a new version of Mesos agent
> with overlay backend, then all the image layers in the store pulled
> by the old agent will just be ignored, but if user starts the new
> agent with a non-overlay backend, then all the image layers in the
> store can still be used.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> 9b09dca5cd8f9e60a90cf574300b250eb18ede35 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> 6456d90220a4026696a04f9969763cf682464353 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
> 949e0c16a361f22b00e267fcf81093690327041f 
>   src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
> a5cc952072274c61e8c515e8b1b7ea563344e950 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 1e2cc2dda3518d99ef1ad06e2b8ea7f78a4dcf72 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 52b9ea934a1dbe3312b8f08f5da8085e9e306de5 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 10fbc4149ac2e7503ffe7f2746fbd0e14a2365b4 
> 
> Diff: https://reviews.apache.org/r/52827/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 52827: Added backend suffix to image layer rootfs path.

[Qian Zhang]

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

(Updated Oct. 14, 2016, 11:36 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Addressed Jie's comments.


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


Repository: mesos


Description (updated)
---

Previously image layer rootfs path is in the format below regardless
of which backend is used.
  /layers//rootfs
This introduced an issue: when agent is restarted with a different
backend, we will wrongly handle the whiteout files since different
backends(e.g., aufs and overlay) have different whiteout standard.

In this commit, we added backend suffix to image layer rootfs path
for overlay backend like below.
  /layers//rootfs.overlay
For non-overlay backends, it is still in the previous format, this
is because they share the same whiteout standard (aufs standard),
and also used to handle backward compatibility.

So the expected result of this commit is:
1. If user switches backend from overlay to a non-overlay (or vice
versa) when restarting agent, all the image layers of the previous
backend in the store will just be ignored.
2. In the upgrade case, if user starts a new version of Mesos agent
with overlay backend, then all the image layers in the store pulled
by the old agent will just be ignored, but if user starts the new
agent with a non-overlay backend, then all the image layers in the
store can still be used.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
9b09dca5cd8f9e60a90cf574300b250eb18ede35 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
6456d90220a4026696a04f9969763cf682464353 
  src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
949e0c16a361f22b00e267fcf81093690327041f 
  src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
a5cc952072274c61e8c515e8b1b7ea563344e950 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
1e2cc2dda3518d99ef1ad06e2b8ea7f78a4dcf72 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
52b9ea934a1dbe3312b8f08f5da8085e9e306de5 
  src/tests/containerizer/provisioner_docker_tests.cpp 
10fbc4149ac2e7503ffe7f2746fbd0e14a2365b4 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 52869: Ensured command executor ignores health updates for terminated tasks.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Oct. 14, 2016, 12:41 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52869/
> ---
> 
> (Updated Oct. 14, 2016, 12:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and haosdent huang.
> 
> 
> Bugs: MESOS-5963
> https://issues.apache.org/jira/browse/MESOS-5963
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After the task has been terminated, its health updates
> become irrelevant and should be ignored.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 3e95d6029bea0ce6e0dfb39c24b795fe98d90d13 
> 
> Diff: https://reviews.apache.org/r/52869/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52868: Added pause/resume functionality to HealthChecker.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Oct. 14, 2016, 12:52 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52868/
> ---
> 
> (Updated Oct. 14, 2016, 12:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and haosdent huang.
> 
> 
> Bugs: MESOS-5963
> https://issues.apache.org/jira/browse/MESOS-5963
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
>   src/health-check/health_checker.cpp 
> 96ae1a733ff3d211b84d0893b4603873af1c89f0 
> 
> Diff: https://reviews.apache.org/r/52868/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 52887: Added a CHANGELOG description for partition-aware frameworks.

[Neil Conway]

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

Review request for mesos, Alexander Rukletsov and Vinod Kone.


Repository: mesos


Description
---

Added a CHANGELOG description for partition-aware frameworks.


Diffs
-

  CHANGELOG 1b8fa346d916f941841bd34b264c8803e4286dc4 

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


Testing
---

Visual inspection.


Thanks,

Neil Conway

Re: Review Request 52867: Used `Duration::create()` for double -> Duration conversion.

[Alexander Rukletsov]

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

(Updated Oct. 14, 2016, 3:26 p.m.)


Review request for mesos, Daniel Pravat, Gastón Kleiman, haosdent huang, and 
Joseph Wu.


Repository: mesos


Description
---

Additionally persist health check parameters from the `HealthCheck`
protobuf as class members to avoid code duplication.


Diffs (updated)
-

  src/health-check/health_checker.hpp 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
  src/health-check/health_checker.cpp 96ae1a733ff3d211b84d0893b4603873af1c89f0 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 52695: Harden libprocess

[Aaron Wood]

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

(Updated Oct. 14, 2016, 3:20 p.m.)


Review request for mesos, Michael Park and Neil Conway.


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


Repository: mesos


Description
---

Use a default set of flags to provide additional security and hardening to 
libprocess. Additionally, check and catch more warnings/errors.


Diffs
-

  3rdparty/libprocess/Makefile.am 020b0e1 

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


Testing
---

Compared the benchmarks with and without the flags being used. Also did a 
comparsion with the flags being used with and without optimizations and without 
the flags being used with and without optimizations. Overall the performance 
hit was very small with a 3-8% overhead (optimizations brings this down 
slightly). Most benchmarks were about 5% (or less) slower.


Thanks,

Aaron Wood

Re: Review Request 52696: Harden stout

[Aaron Wood]

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

(Updated Oct. 14, 2016, 3:20 p.m.)


Review request for mesos, Michael Park and Neil Conway.


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


Repository: mesos


Description
---

Use a default set of flags to provide additional security and hardening to 
stout. Additionally, check and catch more warnings/errors.


Diffs
-

  3rdparty/stout/Makefile.am fda069d 

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


Testing
---

Compared the benchmarks with and without the flags being used. Also did a 
comparsion with the flags being used with and without optimizations and without 
the flags being used with and without optimizations. Overall the performance 
hit was very small with a 3-8% overhead (optimizations brings this down 
slightly). Most benchmarks were about 5% (or less) slower.


Thanks,

Aaron Wood

Re: Review Request 52645: Harden Mesos

[Aaron Wood]

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

(Updated Oct. 14, 2016, 3:20 p.m.)


Review request for mesos, Michael Park and Neil Conway.


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


Repository: mesos


Description
---

Use a default set of flags to provide additional security and hardening to 
Mesos. Additionally, check and catch more warnings/errors.


Diffs
-

  configure.ac 034bb91 
  src/Makefile.am fd01e1d 

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


Testing
---

Compared the benchmarks with and without the flags being used. Also did a 
comparsion with the flags being used with and without optimizations and without 
the flags being used with and without optimizations. Overall the performance 
hit was very small with a 3-8% overhead (optimizations brings this down 
slightly). Most benchmarks were about 5% (or less) slower.


Thanks,

Aaron Wood

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/
---

Review request for mesos, 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 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. 14, 2016, 3:14 p.m.)


Review request for mesos, Michael Park and Neil Conway.


Changes
---

Targeted this RR at libprocess. There will be a separate one opened for stout.


Summary (updated)
-

Fix new sign comparison errors in libprocess produced by hardened flags


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


Repository: mesos


Description (updated)
---

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 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 52873: Cleaned up private members in HealthChecker class.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52863, 52864, 52865, 52866, 52867, 52868, 52869, 52870, 
52871, 52872, 52873]

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. 14, 2016, 12:45 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52873/
> ---
> 
> (Updated Oct. 14, 2016, 12:45 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
>   src/health-check/health_checker.cpp 
> 96ae1a733ff3d211b84d0893b4603873af1c89f0 
> 
> Diff: https://reviews.apache.org/r/52873/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 52881: Made sure required capabilities are not dropped in capabilities test.

[Benjamin Bannier]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

The capabilities isolator test suites runs test as root where the
files executed might not reside in directories accessible even to root
after dropping all capabilities. We already ensured that the test
agent would always permit `DAC_READ_SEARCH` so that we could move this
one into the permitted set, but missed to ensure it was always present
when tasks set capabilities. This could lead to situtations where
e.g., `mesos-executor` could not be executed by the test.

This commit adds `DAC_READ_SEARCH` to the requested set for all
situation where where drop all capabilities required for tests.


Diffs
-

  src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
f040c209b4b4c87cef00b0569b7da7581f4ccf03 

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


Testing
---

* as root: `./mesos-test 
--gtest_filter='*/LinuxCapabilitiesIsolatorTest.ROOT_Ping/*'`
* confirmed by inspection of the log output that all `launch` invocations were 
able
  to successly start the requested executables.


Thanks,

Benjamin Bannier

Re: Review Request 52867: Used `Duration::create()` for double -> Duration conversion.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Oct. 14, 2016, 12:51 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52867/
> ---
> 
> (Updated Oct. 14, 2016, 12:51 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Gastón Kleiman, haosdent huang, and 
> Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Additionally persist health check parameters from the `HealthCheck`
> protobuf as class members to avoid code duplication.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
>   src/health-check/health_checker.cpp 
> 96ae1a733ff3d211b84d0893b4603873af1c89f0 
> 
> Diff: https://reviews.apache.org/r/52867/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52873: Cleaned up private members in HealthChecker class.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Oct. 14, 2016, 12:45 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52873/
> ---
> 
> (Updated Oct. 14, 2016, 12:45 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
>   src/health-check/health_checker.cpp 
> 96ae1a733ff3d211b84d0893b4603873af1c89f0 
> 
> Diff: https://reviews.apache.org/r/52873/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Oct. 14, 2016, 12:38 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52866/
> ---
> 
> (Updated Oct. 14, 2016, 12:38 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
>   src/health-check/health_checker.cpp 
> 96ae1a733ff3d211b84d0893b4603873af1c89f0 
> 
> Diff: https://reviews.apache.org/r/52866/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52865: Refactored HealthChecker to never stop health checking.

[Gastón Kleiman]

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


Ship it!





src/tests/health_check_tests.cpp (line 997)


The changes to this file don't seem to be related to the rest of the patch.


- Gastón Kleiman


On Oct. 14, 2016, 12:37 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52865/
> ---
> 
> (Updated Oct. 14, 2016, 12:37 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Mahler, Gastón Kleiman, 
> and haosdent huang.
> 
> 
> Bugs: MESOS-5963
> https://issues.apache.org/jira/browse/MESOS-5963
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prior to this patch, HealthChecker would stop performing health
> checks after it marks the task for kill. Since tasks' lifecycle
> is managed by scheduler-executor, HealthChecker should never stop
> health checking on its own.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 
>   src/health-check/health_checker.hpp 
> 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
>   src/health-check/health_checker.cpp 
> 96ae1a733ff3d211b84d0893b4603873af1c89f0 
>   src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 
>   src/launcher/executor.cpp 3e95d6029bea0ce6e0dfb39c24b795fe98d90d13 
>   src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
> 
> Diff: https://reviews.apache.org/r/52865/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52864: Removed unnecessary sleep in HealthChecker.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Oct. 14, 2016, 12:35 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52864/
> ---
> 
> (Updated Oct. 14, 2016, 12:35 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After we have refactored the HealthChecker into a library from the
> binary, there is no more need to wait before failing the promise to
> ensure the message has been sent to the executor. HealthChecker
> lifetime is managed by an executor, hence it is their responsibility
> not to clean the instance until after the message with the `kill_task`
> flag is received.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.cpp 
> 96ae1a733ff3d211b84d0893b4603873af1c89f0 
> 
> Diff: https://reviews.apache.org/r/52864/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52863: Refactored HealthCheck validation for clarity.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Oct. 14, 2016, 12:35 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52863/
> ---
> 
> (Updated Oct. 14, 2016, 12:35 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.cpp 
> 96ae1a733ff3d211b84d0893b4603873af1c89f0 
> 
> Diff: https://reviews.apache.org/r/52863/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52608: Reordered the list of executor env variables in code and documentation.

[Gastón Kleiman]

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



Superseded by the chain starting with https://reviews.apache.org/r/52878/

- Gastón Kleiman


On Oct. 11, 2016, 6:54 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52608/
> ---
> 
> (Updated Oct. 11, 2016, 6:54 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, Jie Yu, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6288
> https://issues.apache.org/jira/browse/MESOS-6288
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reordered the list of executor env variables in code and documentation.
> 
> 
> Diffs
> -
> 
>   docs/executor-http-api.md 50b4cb4efff48bcc56330b81bd7c4c217b8a22b5 
>   src/slave/slave.cpp 119fb36c27b25739f2a86a55d48e964ca4a84ff7 
> 
> Diff: https://reviews.apache.org/r/52608/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 52556: Added the MESOS_RUNTIME_DIRECTORY executor env variable.

[Gastón Kleiman]

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



Superseded by the chain starting with https://reviews.apache.org/r/52878/

- Gastón Kleiman


On Oct. 11, 2016, 6:52 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52556/
> ---
> 
> (Updated Oct. 11, 2016, 6:52 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, Jie Yu, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6288
> https://issues.apache.org/jira/browse/MESOS-6288
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This environment variable points to the directory containing the
> binaries used by the Mesos Agent. It will eventually replace the
> `--launcher-dir` executor flag.
> 
> 
> Diffs
> -
> 
>   docs/executor-http-api.md 50b4cb4efff48bcc56330b81bd7c4c217b8a22b5 
>   src/launcher/default_executor.cpp 2454bd7df608254af72af01460782f5ab78a19c1 
>   src/launcher/executor.cpp 3e95d6029bea0ce6e0dfb39c24b795fe98d90d13 
>   src/slave/slave.cpp 119fb36c27b25739f2a86a55d48e964ca4a84ff7 
> 
> Diff: https://reviews.apache.org/r/52556/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

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/
---

Review request for mesos, Alexander Rukletsov, Anand Mazumdar, 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/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 
  src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 

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


Testing
---

`make check`


Thanks,

Gastón Kleiman

Review Request 52879: Updated the way in which the default and command executors load options.

[Gastón Kleiman]

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

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


Repository: mesos


Description
---

Updated the command and the default executor 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/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 
  src/launcher/executor.cpp 3e95d6029bea0ce6e0dfb39c24b795fe98d90d13 

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


Testing
---

`make check`


Thanks,

Gastón Kleiman

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/
---

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


Repository: mesos


Description
---

Removed outdated TODO in stout::flags.


Diffs
-

  3rdparty/stout/include/stout/flags.hpp 
21257b7ee6ed7a9a48a0ca1ad4edd8890ca2fa8f 

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


Testing
---


Thanks,

Gastón Kleiman

Re: Review Request 52871: Ensured default executor ignores health updates for terminated tasks.

[Alexander Rukletsov]

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

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


Review request for mesos, Anand Mazumdar, Benjamin Mahler, Gastón Kleiman, and 
haosdent huang.


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


Repository: mesos


Description
---

After the task has been terminated, its health updates become
irrelevant and should be ignored. Now it is safe to start health
checks even if the task has been asked to terminate. Also if the
default executor shuts down, we can safely pause all health checkers.


Diffs (updated)
-

  src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 52868: Added pause/resume functionality to HealthChecker.

[Alexander Rukletsov]

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

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


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


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/health-check/health_checker.hpp 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
  src/health-check/health_checker.cpp 96ae1a733ff3d211b84d0893b4603873af1c89f0 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 52867: Used `Duration::create()` for double -> Duration conversion.

[Alexander Rukletsov]

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

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


Review request for mesos, Daniel Pravat, Gastón Kleiman, haosdent huang, and 
Joseph Wu.


Summary (updated)
-

Used `Duration::create()` for double -> Duration conversion.


Repository: mesos


Description
---

Additionally persist health check parameters from the `HealthCheck`
protobuf as class members to avoid code duplication.


Diffs
-

  src/health-check/health_checker.hpp 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
  src/health-check/health_checker.cpp 96ae1a733ff3d211b84d0893b4603873af1c89f0 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Review Request 52877: Fixed wrong float serialization in JSON in locales different from C.

[Alexander Rojas]

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

Review request for mesos, Adam B, Benjamin Bannier, and Michael Park.


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


Repository: mesos


Description
---

Resolves an isse with the JSON serialization functions where floats
would use the process active locale as a decimal separator instead
of the JSON conformant period (`.`).


Diffs
-

  3rdparty/stout/include/stout/json.hpp 
9734561549eea867d57f14a0ab71febeb9dddc06 
  3rdparty/stout/include/stout/jsonify.hpp 
8220d001001e8b9f247c05be58534f1174611aad 

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


Testing
---

Three types of tests were used for this issue. The first just run our current
test suite (`make check`). The second verifies that the proposed solutions
are able to generate a locale independent output and the third consist of a
benchmark of all the solutions.

The verification that the proposed solutions produce locale independent JSON,
the following program was used (with manual verification of the generated
output):

```c++
#include 

#include 
#include 
#include 

int main(int argc, char **argv) {
  // Set locale to German.
  std::setlocale(LC_ALL, "de_DE.UTF-8");
  std::cout.imbue(std::locale("de_DE.UTF-8"));

  JSON::Object object;
  object.values["float"] = 1234567890.12345;

  std::cout << stringify(object) << '\n';
  return 0;
}

```

In order to test the different options the following benchmarking program was
written using [Google benchmark](https://github.com/google/benchmark):

```c++
#include 
#include 

#include 
#include 
#include 
#include 

static void BM_Jsonify(benchmark::State& state) {
  while (state.KeepRunning()) {
state.PauseTiming();
JSON::Object object;
object.values["float00"] = 1234567890.12345;
object.values["float01"] = 123456789.012345;
object.values["float02"] = 12345678.9012345;
object.values["float03"] = 1234567.89012345;
object.values["float04"] = 123456.789012345;
object.values["float05"] = 12345.6789012345;
object.values["float06"] = 1234.56789012345;
object.values["float07"] = 123.456789012345;
object.values["float08"] = 12.3456789012345;
object.values["float09"] = 1.23456789012345;
object.values["float10"] = 1234567890.12345;
object.values["float11"] = 123456789.012345;
object.values["float12"] = 12345678.9012345;
object.values["float13"] = 1234567.89012345;
object.values["float14"] = 123456.789012345;
object.values["float15"] = 12345.6789012345;
object.values["float16"] = 1234.56789012345;
object.values["float17"] = 123.456789012345;
object.values["float18"] = 12.3456789012345;
object.values["float19"] = 1.23456789012345;
object.values["float20"] = 1234567890.12345;
object.values["float21"] = 123456789.012345;
object.values["float22"] = 12345678.9012345;
object.values["float23"] = 1234567.89012345;
object.values["float24"] = 123456.789012345;
object.values["float25"] = 12345.6789012345;
object.values["float26"] = 1234.56789012345;
object.values["float27"] = 123.456789012345;
object.values["float28"] = 12.3456789012345;
object.values["float29"] = 1.23456789012345;
object.values["float30"] = 1234567890.12345;
object.values["float31"] = 123456789.012345;
object.values["float32"] = 12345678.9012345;
object.values["float33"] = 1234567.89012345;
object.values["float34"] = 123456.789012345;
object.values["float35"] = 12345.6789012345;
object.values["float36"] = 1234.56789012345;
object.values["float37"] = 123.456789012345;
object.values["float38"] = 12.3456789012345;
object.values["float39"] = 1.23456789012345;
object.values["float40"] = 1234567890.12345;
object.values["float41"] = 123456789.012345;
object.values["float42"] = 12345678.9012345;
object.values["float43"] = 1234567.89012345;
object.values["float44"] = 123456.789012345;
object.values["float45"] = 12345.6789012345;
object.values["float46"] = 1234.56789012345;
object.values["float47"] = 123.456789012345;
object.values["float48"] = 12.3456789012345;
object.values["float49"] = 1.23456789012345;

state.ResumeTiming();

benchmark::DoNotOptimize(stringify(object));
  }
}

BENCHMARK(BM_Jsonify);

BENCHMARK_MAIN();
```

The program creates a JSON object with 50 floats in it, and then it creates
its string representation. Since the algorithms in `json.hpp` and `jsonify.hpp`
are similar but not the same, they were benchmarked as different. While the
original solutions are non solutions since they produce erroneous results if
localization is active, they constitute a baseline for comparison with the
alternatives. These are the benchmarks for the original algorithms and their
alternatives:

1. 

Review Request 52873: Cleaned up private members in HealthChecker class.

[Alexander Rukletsov]

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

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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/health-check/health_checker.hpp 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
  src/health-check/health_checker.cpp 96ae1a733ff3d211b84d0893b4603873af1c89f0 

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


Testing
---

make check


Thanks,

Alexander Rukletsov

Review Request 52872: Used callback instead of `send()` for health status updates.

[Alexander Rukletsov]

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

Review request for mesos, Anand Mazumdar, Benjamin Mahler, Gastón Kleiman, and 
haosdent huang.


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


Repository: mesos


Description
---

Since HealthChecker is now used as a library only and does not live
in a separate OS process, there is no need to use libprocess message
sending for health status updates; a callback will do.


Diffs
-

  src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 
  src/health-check/health_checker.hpp 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
  src/health-check/health_checker.cpp 96ae1a733ff3d211b84d0893b4603873af1c89f0 
  src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 
  src/launcher/executor.cpp 3e95d6029bea0ce6e0dfb39c24b795fe98d90d13 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Review Request 52871: Ensured default executor ignores health updates for terminated tasks.

[Alexander Rukletsov]

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

Review request for mesos, Anand Mazumdar, Benjamin Mahler, Gastón Kleiman, and 
haosdent huang.


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


Repository: mesos


Description
---

After the task has been terminated, its health updates become
irrelevant and should be ignored. Now it is safe to start health
checks even if the task has been asked to terminate. Also if the
default executor shuts down, we can safely pause all health checkers.


Diffs
-

  src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Review Request 52870: Ensured docker executor ignores health updates for terminated tasks.

[Alexander Rukletsov]

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

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


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


Repository: mesos


Description
---

After the task has been terminated, its health updates become
irrelevant and should be ignored. Now it is safe to start health
checks even if the task has been asked to terminate.


Diffs
-

  src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Review Request 52869: Ensured command executor ignores health updates for terminated tasks.

[Alexander Rukletsov]

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

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


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


Repository: mesos


Description
---

After the task has been terminated, its health updates
become irrelevant and should be ignored.


Diffs
-

  src/launcher/executor.cpp 3e95d6029bea0ce6e0dfb39c24b795fe98d90d13 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Review Request 52868: Added pause/resume functionality to HealthChecker.

[Alexander Rukletsov]

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

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


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/health-check/health_checker.hpp 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
  src/health-check/health_checker.cpp 96ae1a733ff3d211b84d0893b4603873af1c89f0 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Review Request 52867: Used `Duration::create` for double -> Duration conversion.

[Alexander Rukletsov]

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

Review request for mesos, Daniel Pravat, Gastón Kleiman, haosdent huang, and 
Joseph Wu.


Repository: mesos


Description
---

Additionally persist health check parameters from the `HealthCheck`
protobuf as class members to avoid code duplication.


Diffs
-

  src/health-check/health_checker.hpp 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
  src/health-check/health_checker.cpp 96ae1a733ff3d211b84d0893b4603873af1c89f0 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

[Alexander Rukletsov]

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

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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/health-check/health_checker.hpp 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
  src/health-check/health_checker.cpp 96ae1a733ff3d211b84d0893b4603873af1c89f0 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Review Request 52865: Refactored HealthChecker to never stop health checking.

[Alexander Rukletsov]

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

Review request for mesos, Anand Mazumdar, Benjamin Mahler, Gastón Kleiman, and 
haosdent huang.


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


Repository: mesos


Description
---

Prior to this patch, HealthChecker would stop performing health
checks after it marks the task for kill. Since tasks' lifecycle
is managed by scheduler-executor, HealthChecker should never stop
health checking on its own.


Diffs
-

  src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 
  src/health-check/health_checker.hpp 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
  src/health-check/health_checker.cpp 96ae1a733ff3d211b84d0893b4603873af1c89f0 
  src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 
  src/launcher/executor.cpp 3e95d6029bea0ce6e0dfb39c24b795fe98d90d13 
  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Review Request 52863: Refactored HealthCheck validation for clarity.

[Alexander Rukletsov]

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

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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/health-check/health_checker.cpp 96ae1a733ff3d211b84d0893b4603873af1c89f0 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 52855: Re-enabled the change user test in slave tests.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52854, 52855]

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. 14, 2016, 3:36 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52855/
> ---
> 
> (Updated Oct. 14, 2016, 3:36 a.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-6391
> https://issues.apache.org/jira/browse/MESOS-6391
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that the sandbox of the command task is writable if
> the task specifies a non privileged user.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 6bc3835cbb62536ec933ef38c9e15138b8611e5f 
>   src/tests/slave_tests.cpp 93b81d3e5b90d8036409e943f598c865fe335bcc 
> 
> Diff: https://reviews.apache.org/r/52855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 52853: Fixed typo in a comment in hooks.hpp.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52853]

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. 14, 2016, 2:32 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52853/
> ---
> 
> (Updated Oct. 14, 2016, 2:32 a.m.)
> 
> 
> Review request for mesos and Kapil Arya.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp c8a58641e0768628a9028a70d8c28f7fb412 
> 
> Diff: https://reviews.apache.org/r/52853/diff/
> 
> 
> Testing
> ---
> 
> no functional change - make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 50127: Added DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch.

[Yubo Li]

> On 十月 14, 2016, 10:06 a.m., Guangya Liu wrote:
> > src/tests/containerizer/docker_containerizer_tests.cpp, lines 3994-3996
> > 
> >
> > Missed update here?
> 
> Guangya Liu wrote:
> ```
> EXPECT_TRUE(inspect->devices[i].access.read);
> EXPECT_TRUE(inspect->devices[i].access.write);
> EXPECT_TRUE(inspect->devices[i].access.mknod);
> ```

Sorry, just updated


- Yubo


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


On 十月 14, 2016, 10:07 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50127/
> ---
> 
> (Updated 十月 14, 2016, 10:07 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This added a testing case for end-to-end GPU support for docker
> containerizer.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 6d26797abf6d2b5e42b9e7743789e1edc62c9c1a 
> 
> Diff: https://reviews.apache.org/r/50127/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch" make -j 
> check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50127: Added DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch.

[Guangya Liu]

> On 十月 14, 2016, 10:06 a.m., Guangya Liu wrote:
> > src/tests/containerizer/docker_containerizer_tests.cpp, lines 3994-3996
> > 
> >
> > Missed update here?

```
EXPECT_TRUE(inspect->devices[i].access.read);
EXPECT_TRUE(inspect->devices[i].access.write);
EXPECT_TRUE(inspect->devices[i].access.mknod);
```


- Guangya


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


On 十月 14, 2016, 10:07 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50127/
> ---
> 
> (Updated 十月 14, 2016, 10:07 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This added a testing case for end-to-end GPU support for docker
> containerizer.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 6d26797abf6d2b5e42b9e7743789e1edc62c9c1a 
> 
> Diff: https://reviews.apache.org/r/50127/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch" make -j 
> check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50127: Added DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch.

[Yubo Li]

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

(Updated 十月 14, 2016, 10:07 a.m.)


Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.


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


Repository: mesos


Description
---

This added a testing case for end-to-end GPU support for docker
containerizer.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
6d26797abf6d2b5e42b9e7743789e1edc62c9c1a 

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


Testing
---

GTEST_FILTER="DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch" make -j 
check


Thanks,

Yubo Li