Re: Review Request 42618: Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 10:24 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
d8cebd7be97170c799016f3c60d91d2a4de9510d 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
f96c3274610269eacc9e91fec21a47a06bfeea62 

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


Testing
---

make and make check


Thanks,

Avinash sridharan

Re: Review Request 42947: Implemented the `status` method in `CgroupNetClsIsolatorProcess`.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 10:32 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Implemented the `status` method in `CgroupNetClsIsolatorProcess`.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
d8cebd7be97170c799016f3c60d91d2a4de9510d 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
f96c3274610269eacc9e91fec21a47a06bfeea62 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 42993: Support for multiple frameworks sharing the same resource.

[Anindya Sinha]

> On Feb. 1, 2016, 3:51 a.m., Guangya Liu wrote:
> > It is better add some unit tests in `hierarchical_allocator_tests.cpp` to 
> > cover the update in allocator for shareable resources.

Added unit tests but included in https://reviews.apache.org/r/42996.


- Anindya


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


On Feb. 5, 2016, 10:57 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42993/
> ---
> 
> (Updated Feb. 5, 2016, 10:57 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The allocator tracks allocated shared resources separately from the
> remaining allocated resources so that multiple frameworks see the
> same view of the shared resource at any given time. This avoids
> unnecessary copies of the same shared resource when multiple
> frameworks use such resources simultaneously.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> a9d2c23162892e0f97d89a076d2311091d91 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 4669149b81de39b4bb921ef7cd6787aa583f6e40 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 18797e42a9c2bd20392020237cfae600a5ffe12c 
>   src/master/allocator/sorter/sorter.hpp 
> a0a779b81f6d048271f15256b38ff907ae144b83 
>   src/master/master.cpp e1ca81dab85a7ab1391eca0d6bd995548bf79c22 
> 
> Diff: https://reviews.apache.org/r/42993/diff/
> 
> 
> Testing
> ---
> 
> make check done.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 43156: Added test: dynamic reservations with same role, different principals.

[Guangya Liu]

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


Ship it!




Ship It!

- Guangya Liu


On 二月 5, 2016, 5:47 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43156/
> ---
> 
> (Updated 二月 5, 2016, 5:47 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4590
> https://issues.apache.org/jira/browse/MESOS-4590
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test: dynamic reservations with same role, different principals.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_endpoints_tests.cpp 
> 35c093567b07a11ca6eee85e2ff91894a460a7af 
> 
> Diff: https://reviews.apache.org/r/43156/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42735: Make bash scripts portable.

[David Forsythe]

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

(Updated Feb. 5, 2016, 4:51 p.m.)


Review request for mesos, Artem Harutyunyan and Ian Downes.


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


Repository: mesos


Description
---

Make bash scripts portable.


Diffs (updated)
-

  support/atexit.sh 90696a2d426ff013d1aaec7800c6dc880bc2a00c 
  support/coverage.sh df81f9a45d043d506f47e9d6fac6893bf044144a 
  support/docker_build.sh 55d402e35d6d391fd483c47b69b64c1ff99569d3 
  support/hooks/commit-msg d173dfdf380b08a9b349589dfd33c53b3cdccc60 
  support/release.sh 633bbace091bddd582b9fcef5d35b331a0f169f0 
  support/tag.sh 9d37c81a13cb0c281f4a53884c6c55e09c341d85 
  support/vote.sh 218a38512d4dc8e160d975a99c577e4411879eee 

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


Testing
---


Thanks,

David Forsythe

Re: Review Request 42982: Defined a virtual `status` method for Containerizer.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 3:35 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Defined a virtual `status` method for Containerizer.


Diffs
-

  src/slave/containerizer/containerizer.hpp 
08d1ed3cf50f50b5b6e71df60bffbd141490ad32 

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


Testing
---

make


Thanks,

Avinash sridharan

Re: Review Request 42982: Defined a virtual `status` method for Containerizer.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 3:35 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Defined a virtual `status` method for Containerizer.


Diffs (updated)
-

  src/slave/containerizer/containerizer.hpp 
08d1ed3cf50f50b5b6e71df60bffbd141490ad32 

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


Testing
---

make


Thanks,

Avinash sridharan

Re: Review Request 42618: Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 3:31 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Making sure comments are tw=70


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


Repository: mesos


Description
---

Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
85004aea4c1ee4b25e106f3ce40025c69f1ce030 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
bf6c88341dedc0a37546c04f38197c892b498684 

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


Testing
---

make and make check


Thanks,

Avinash sridharan

Re: Review Request 42618: Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 3:27 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
85004aea4c1ee4b25e106f3ce40025c69f1ce030 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
bf6c88341dedc0a37546c04f38197c892b498684 

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


Testing
---

make and make check


Thanks,

Avinash sridharan

Re: Review Request 42782: Added a unit-test to test net_cls major handles set from command line.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 3:34 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added a unit-test to test net_cls major handles set from command line.


Diffs (updated)
-

  src/tests/containerizer/isolator_tests.cpp 
67322abc776cbd501385932676852a79b74ef248 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 42947: Implemented the `status` method in `CgroupNetClsIsolatorProcess`.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 3:47 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Removed patch dependency.


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


Repository: mesos


Description
---

Implemented the `status` method in `CgroupNetClsIsolatorProcess`.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
85004aea4c1ee4b25e106f3ce40025c69f1ce030 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
bf6c88341dedc0a37546c04f38197c892b498684 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 42782: Added a unit-test to test net_cls major handles set from command line.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42982, 42983, 43096, 42618, 42780, 42781, 43107, 42782]

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

- Mesos ReviewBot


On Feb. 5, 2016, 3:34 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42782/
> ---
> 
> (Updated Feb. 5, 2016, 3:34 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4344
> https://issues.apache.org/jira/browse/MESOS-4344
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a unit-test to test net_cls major handles set from command line.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> 67322abc776cbd501385932676852a79b74ef248 
> 
> Diff: https://reviews.apache.org/r/42782/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43107: Passed agent flag --cgroup_net_cls_primary_handle to net_cls isolator.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 3:34 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

The `cgroup/net_cls` isolator will use the primary handle, passed by the
operator using the --cgroup_net_cls_primary_handle, to initialize the
`NetClsHandleManager`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
bf6c88341dedc0a37546c04f38197c892b498684 

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


Testing
---

make and make check

Launched a slave by passing invalid flags to --cgroup_net_cls_primary_handle 
and made sure the slave correctly detects these invalid handles. 

Launched a slave with valid handle to make sure that the slave launches 
correctly.


Thanks,

Avinash sridharan

Re: Review Request 42781: Added the --cgroups_net_cls_primary_handle flag to the slave.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 3:32 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added the --cgroups_net_cls_primary_handle flag to the slave.


Diffs (updated)
-

  src/slave/flags.hpp 23ec158f4a95fa76d657d9eb6a4a6fe30d57e5b6 
  src/slave/flags.cpp 24a23325cc255d0d7b7af7ed096b6d3012ad75c7 

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


Testing
---

Tested the flag by launching a mesos agent and checking that the slave takes 
the `--cgroups_net_cls_major_handles` flag.


Thanks,

Avinash sridharan

Re: Review Request 43193: Fixed parsing docker image env vars.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 5, 2016, 10:14 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43193/
> ---
> 
> (Updated Feb. 5, 2016, 10:14 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4587
> https://issues.apache.org/jira/browse/MESOS-4587
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow equal character in the value of environemnt variables.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e9e983499bffd6f8f29bb4b71715f737d1 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
> 
> Diff: https://reviews.apache.org/r/43193/diff/
> 
> 
> Testing
> ---
> 
> - Start mesos master, slave and marathon, add a marathon task with the docker 
> image `jupyter/all-spark-notebook`, the one the reporter has problem to 
> launch beacuse it has equal signs in env vars. 
> ```json
> {
>   "id": "/testapp/app1",
>   "cmd": "env && sleep 300",
>   "cpus": 0.1,
>   "mem": 18,
>   "requirePorts": false,
>   "instances": 1,
>   "executor": "",
>   "container": {
> "type": "DOCKER",
> "docker": {
>   "image": "jupyter/all-spark-notebook",
>   "network": "BRIDGE",
>   "portMappings": [
> {
>   "containerPort": ,
>   "hostPort": 31000,
>   "protocol": "tcp"
> }
>   ],
>   "privileged": false
> }
>   }
> }
> ```
> - Without this patch, the task fails to launch with:
> ```sh
> E0204 18:04:38.538508 14843 slave.cpp:3703] Container 
> '0dd269c4-cdeb-4a24-ae5c-86b75fae8737' for executor 
> 'testapp_app1.c19685e6-cb69-11e5-99d6-024234e5bbc1' of framework 
> aaf4cc37-4809-4495-a9bd-18c1407442e0- failed to start: Unable to create 
> image: Unexpected Env format for 'ContainerConfig.Env'
> ```
>   
> - With this patch, the task starts successfully.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Review Request 43156: Added test: dynamic reservations with same role, different principals.

[Neil Conway]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Added test: dynamic reservations with same role, different principals.


Diffs
-

  src/tests/reservation_endpoints_tests.cpp 
35c093567b07a11ca6eee85e2ff91894a460a7af 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 42735: Make bash scripts portable.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On Feb. 5, 2016, 4:51 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42735/
> ---
> 
> (Updated Feb. 5, 2016, 4:51 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Ian Downes.
> 
> 
> Bugs: MESOS-4502
> https://issues.apache.org/jira/browse/MESOS-4502
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Make bash scripts portable.
> 
> 
> Diffs
> -
> 
>   support/atexit.sh 90696a2d426ff013d1aaec7800c6dc880bc2a00c 
>   support/coverage.sh df81f9a45d043d506f47e9d6fac6893bf044144a 
>   support/docker_build.sh 55d402e35d6d391fd483c47b69b64c1ff99569d3 
>   support/hooks/commit-msg d173dfdf380b08a9b349589dfd33c53b3cdccc60 
>   support/release.sh 633bbace091bddd582b9fcef5d35b331a0f169f0 
>   support/tag.sh 9d37c81a13cb0c281f4a53884c6c55e09c341d85 
>   support/vote.sh 218a38512d4dc8e160d975a99c577e4411879eee 
> 
> Diff: https://reviews.apache.org/r/42735/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 43193: Fixed parsing docker image env vars.

[Shuai Lin]

> On Feb. 5, 2016, 5:18 p.m., Jie Yu wrote:
> > Ship It!

Jie Yu, Can you shepherd/commit it?


- Shuai


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


On Feb. 5, 2016, 10:14 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43193/
> ---
> 
> (Updated Feb. 5, 2016, 10:14 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4587
> https://issues.apache.org/jira/browse/MESOS-4587
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow equal character in the value of environemnt variables.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e9e983499bffd6f8f29bb4b71715f737d1 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
> 
> Diff: https://reviews.apache.org/r/43193/diff/
> 
> 
> Testing
> ---
> 
> - Start mesos master, slave and marathon, add a marathon task with the docker 
> image `jupyter/all-spark-notebook`, the one the reporter has problem to 
> launch beacuse it has equal signs in env vars. 
> ```json
> {
>   "id": "/testapp/app1",
>   "cmd": "env && sleep 300",
>   "cpus": 0.1,
>   "mem": 18,
>   "requirePorts": false,
>   "instances": 1,
>   "executor": "",
>   "container": {
> "type": "DOCKER",
> "docker": {
>   "image": "jupyter/all-spark-notebook",
>   "network": "BRIDGE",
>   "portMappings": [
> {
>   "containerPort": ,
>   "hostPort": 31000,
>   "protocol": "tcp"
> }
>   ],
>   "privileged": false
> }
>   }
> }
> ```
> - Without this patch, the task fails to launch with:
> ```sh
> E0204 18:04:38.538508 14843 slave.cpp:3703] Container 
> '0dd269c4-cdeb-4a24-ae5c-86b75fae8737' for executor 
> 'testapp_app1.c19685e6-cb69-11e5-99d6-024234e5bbc1' of framework 
> aaf4cc37-4809-4495-a9bd-18c1407442e0- failed to start: Unable to create 
> image: Unexpected Env format for 'ContainerConfig.Env'
> ```
>   
> - With this patch, the task starts successfully.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 43168: Added working dir flag to command executor.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 5, 2016, 1:24 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43168/
> ---
> 
> (Updated Feb. 5, 2016, 1:24 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4005
> https://issues.apache.org/jira/browse/MESOS-4005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added working dir flag to command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp b214a3f876a023c1d68dab4288b18fe5068b2b5a 
> 
> Diff: https://reviews.apache.org/r/43168/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 43193: Fixed parsing docker image env vars.

[Shuai Lin]

> On Feb. 5, 2016, 5:18 p.m., Jie Yu wrote:
> > Ship It!
> 
> Shuai Lin wrote:
> Jie Yu, Can you shepherd/commit it?

oops, just saw it already commited, thanks!


- Shuai


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


On Feb. 5, 2016, 10:14 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43193/
> ---
> 
> (Updated Feb. 5, 2016, 10:14 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4587
> https://issues.apache.org/jira/browse/MESOS-4587
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow equal character in the value of environemnt variables.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e9e983499bffd6f8f29bb4b71715f737d1 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
> 
> Diff: https://reviews.apache.org/r/43193/diff/
> 
> 
> Testing
> ---
> 
> - Start mesos master, slave and marathon, add a marathon task with the docker 
> image `jupyter/all-spark-notebook`, the one the reporter has problem to 
> launch beacuse it has equal signs in env vars. 
> ```json
> {
>   "id": "/testapp/app1",
>   "cmd": "env && sleep 300",
>   "cpus": 0.1,
>   "mem": 18,
>   "requirePorts": false,
>   "instances": 1,
>   "executor": "",
>   "container": {
> "type": "DOCKER",
> "docker": {
>   "image": "jupyter/all-spark-notebook",
>   "network": "BRIDGE",
>   "portMappings": [
> {
>   "containerPort": ,
>   "hostPort": 31000,
>   "protocol": "tcp"
> }
>   ],
>   "privileged": false
> }
>   }
> }
> ```
> - Without this patch, the task fails to launch with:
> ```sh
> E0204 18:04:38.538508 14843 slave.cpp:3703] Container 
> '0dd269c4-cdeb-4a24-ae5c-86b75fae8737' for executor 
> 'testapp_app1.c19685e6-cb69-11e5-99d6-024234e5bbc1' of framework 
> aaf4cc37-4809-4495-a9bd-18c1407442e0- failed to start: Unable to create 
> image: Unexpected Env format for 'ContainerConfig.Env'
> ```
>   
> - With this patch, the task starts successfully.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 43072: Use full screen width for Mesos UI using Bootstraps (v3.3.6) container-fluid

[Michael Lunøe]

> On Feb. 2, 2016, 2:45 a.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [43072]
> > 
> > Failed command: ./support/apply-review.sh -n -r 43072
> > 
> > Error:
> > 2016-02-02 02:45:28 URL:https://reviews.apache.org/r/43072/diff/raw/ 
> > [226634/226634] -> "43072.patch" [1]
> > Traceback (most recent call last):
> >   File "support/apply-reviews.py", line 342, in 
> > reviewboard()
> >   File "support/apply-reviews.py", line 321, in reviewboard
> > apply_review()
> >   File "support/apply-reviews.py", line 139, in apply_review
> > commit_patch()
> >   File "support/apply-reviews.py", line 180, in commit_patch
> > data = patch_data()
> >   File "support/apply-reviews.py", line 199, in patch_data
> > return reviewboard_data()
> >   File "support/apply-reviews.py", line 252, in reviewboard_data
> > email=user.get('email'))
> > UnicodeEncodeError: 'ascii' codec can't encode character u'\xf8' in 
> > position 11: ordinal not in range(128)
> > 
> > Full log: https://builds.apache.org/job/mesos-reviewbot/11187/console
> 
> haosdent huang wrote:
> I think we could ignore this error message.
> 
> Bernd Mathiske wrote:
> I'd rather not ignore the error message. How can there be an out-of-range 
> char in HTML and CSS files?
> 
> haosdent huang wrote:
> I afraid this may caused by boostrap have some special characters like 
> `ok:before{content:"\e013"}`.
> 
> Bernd Mathiske wrote:
> This should be an encoding using printable cahrs (<=128) only.
> 
> haosdent huang wrote:
> LoL, seems `Michael Lunøe` cause the problem. Need fix the 
> apply_reviews.py
> 
> haosdent huang wrote:
> Need do changes in apply_reviews.py to support no-ascii commit messages.
> ```
> diff --git a/support/apply-reviews.py b/support/apply-reviews.py
> index ea5e43a..36bfb1e 100755
> --- a/support/apply-reviews.py
> +++ b/support/apply-reviews.py
> @@ -185,7 +185,7 @@ def commit_patch():
>else:
>  amend = '-e'
> 
> -  cmd = 'git commit --author \'{author}\' {_amend} -am \'{message}\''\
> +  cmd = u'git commit --author \'{author}\' {_amend} -am \'{message}\''\
>  .format(author=quote(data['author']),
>  _amend=amend,
>  message=quote(data['message']))
> @@ -248,7 +248,7 @@ def reviewboard_data():
>user = url_to_json(reviewboard_user_url(
>  review.get('links').get('submitter').get('title'))).get('user')
> 
> -  author = '{author} <{email}>'.format(author=user.get('fullname'),
> +  author = u'{author} <{email}>'.format(author=user.get('fullname'),
> email=user.get('email'))
>message = '\n\n'.join(['{summary}',
>'{description}',
> ```
> 
> Vinod Kone wrote:
> Thanks haosdent for the diff. I committed a patch based on this (and 
> attributed it to you).

Where do I see this patch, so I know when to re-open this?


- Michael


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


On Feb. 1, 2016, 11:36 p.m., Michael Lunøe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43072/
> ---
> 
> (Updated Feb. 1, 2016, 11:36 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Thomas Rampelberg.
> 
> 
> Bugs: MESOS-2585
> https://issues.apache.org/jira/browse/MESOS-2585
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The solution to our problem is solved by updating the Bootstrap CSS file, as 
> container-fluid was introduced later in Bootstrap.
> 
> I have updated to lastest stable release of Bootstrap v3.3.6. Looking through 
> the change log, the only thing that we are using in the Mesos UI and changed 
> is the "hide" class (deprecated), so instances of these have been replaced by 
> its successor ".hidden" class.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 6b18056 
>   src/webui/master/static/css/bootstrap-3.0.3.min.css c547283 
>   src/webui/master/static/css/bootstrap-3.3.6.min.css PRE-CREATION 
>   src/webui/master/static/framework.html 9b28820 
>   src/webui/master/static/home.html d6cde1e 
>   src/webui/master/static/index.html 25caf53 
>   src/webui/master/static/slave.html bc46885 
>   src/webui/master/static/slave_executor.html 9d582d5 
>   src/webui/master/static/slave_framework.html 96d788f 
> 
> Diff: https://reviews.apache.org/r/43072/diff/
> 
> 
> Testing
> ---
> 
> - Mobile: http://cl.ly/2R2O0m1a1G3Q/Image%202016-02-01%20at%2015.21.53.png
> - 1680 x 1050px screen: 
> http://cl.ly/0K1o112u3L2I/Image%202016-02-01%20at%2015.23.37.png
> - 2880 x 1800px screen: 

Re: Review Request 43240: Removed implicit, value changing conversion.

[Jie Yu]

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


Fix it, then Ship it!





src/log/tool/benchmark.cpp (line 209)


can you use 0xff here?


- Jie Yu


On Feb. 5, 2016, 3:28 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43240/
> ---
> 
> (Updated Feb. 5, 2016, 3:28 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Whether `char` is signed or not is up to the implementation, so be more 
> explicit
> here. Also, since we want a `char` with all bits set, use a value 
> representation
> that more clearly communicates that.
> 
> 
> Diffs
> -
> 
>   src/log/tool/benchmark.cpp 9cb60bd66f53e0d191d96a429bfca89d91412339 
> 
> Diff: https://reviews.apache.org/r/43240/diff/
> 
> 
> Testing
> ---
> 
> When compiling with trunk clang, compilation fails with
> 
> ../../src/log/tool/benchmark.cpp|209 col 47 warning| implicit conversion 
> from 'int' to 'value_type' (aka 'char') changes value from 255 to -1 
> [-Wconstant-conversion]
> 
> After inserting the explicit cast the code compiles fine.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43240: Removed implicit, value changing conversion.

[Benjamin Bannier]

> On Feb. 5, 2016, 6:31 p.m., Jie Yu wrote:
> > src/log/tool/benchmark.cpp, line 209
> > 
> >
> > can you use 0xff here?

Yes, I can and should.


- Benjamin


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


On Feb. 5, 2016, 6:41 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43240/
> ---
> 
> (Updated Feb. 5, 2016, 6:41 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Whether `char` is signed or not is up to the implementation, so be more 
> explicit
> here. Also, since we want a `char` with all bits set, use a value 
> representation
> that more clearly communicates that.
> 
> 
> Diffs
> -
> 
>   src/log/tool/benchmark.cpp 9cb60bd66f53e0d191d96a429bfca89d91412339 
> 
> Diff: https://reviews.apache.org/r/43240/diff/
> 
> 
> Testing
> ---
> 
> When compiling with trunk clang, compilation fails with
> 
> ../../src/log/tool/benchmark.cpp|209 col 47 warning| implicit conversion 
> from 'int' to 'value_type' (aka 'char') changes value from 255 to -1 
> [-Wconstant-conversion]
> 
> After inserting the explicit cast the code compiles fine.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43240: Removed implicit, value changing conversion.

[Benjamin Bannier]

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

(Updated Feb. 5, 2016, 6:41 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Addressed review comments.


Repository: mesos


Description (updated)
---

Whether `char` is signed or not is up to the implementation, so be more explicit
here. Also, since we want a `char` with all bits set, use a value representation
that more clearly communicates that.


Diffs (updated)
-

  src/log/tool/benchmark.cpp 9cb60bd66f53e0d191d96a429bfca89d91412339 

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


Testing
---

When compiling with trunk clang, compilation fails with

../../src/log/tool/benchmark.cpp|209 col 47 warning| implicit conversion 
from 'int' to 'value_type' (aka 'char') changes value from 255 to -1 
[-Wconstant-conversion]

After inserting the explicit cast the code compiles fine.


Thanks,

Benjamin Bannier

Re: Review Request 43083: Supported working dir in docker runtime isolator.

[Gilbert Song]

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

(Updated Feb. 5, 2016, 12:13 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Supported working dir in docker runtime isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
dc0868e24486644371b7c285fdf16b9aeac4ca5b 
  src/slave/containerizer/mesos/isolators/docker/runtime.hpp 
6e6ec86d04b157321141cdf5214033e83ce74548 
  src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
c8a93725fa04476c935a8ba18ad2a145c36fb5fa 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

[Guangya Liu]

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




src/docker/docker.cpp (lines 538 - 540)


The message needs some update to be more clear, such as:

`Your docker version is smaller than 1.6.0, all of the TaskInfo labels 
 will be ignored`

BTW: The log messag do not need a period to the end.


- Guangya Liu


On 二月 5, 2016, 7:15 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated 二月 5, 2016, 7:15 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 42947: Implemented the `status` method in `CgroupNetClsIsolatorProcess`.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 3:46 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Implemented the `status` method in `CgroupNetClsIsolatorProcess`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
85004aea4c1ee4b25e106f3ce40025c69f1ce030 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
bf6c88341dedc0a37546c04f38197c892b498684 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 42751: Tweaked some resource test cases.

[Michael Park]

> On Feb. 5, 2016, 2:33 a.m., Guangya Liu wrote:
> > src/tests/resources_tests.cpp, lines 327-328
> > 
> >
> > Just a question, do we have some guidelines for when to use `ASSERT_EQ` 
> > and when to use `EXPECT_EQ` ?

We use `ASSERT_*` in cases where the violation of such condition invalidates 
the rest of the test. For example, in this case, if `cpus->type()` is not a 
`Value::SCALAR`, then we violate `cpus->scalar()`'s preconditions and therefore 
is invalid. We use `EXPECT_*` if the condition is one that we want to test and, 
if even it fails, it doesn't mean the rest of the test is non-sensical.

Take for example:

```
std::vector x = f(0, 1, 2);

// `ASSERT_*` here since if we have less than 3 elements, `x[2]` is invalid.
ASSERT_GE(x.size(), 3);

// Each of these are independent test cases.
// For example, even if x[0] != 0, it doesn't invalidate the rest of our tests.
EXPECT_EQ(0, x[0]);
EXPECT_EQ(1, x[1]);
EXPECT_EQ(2, x[2]);
```


- Michael


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


On Feb. 5, 2016, 2:19 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42751/
> ---
> 
> (Updated Feb. 5, 2016, 2:19 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We should check that two reservations with the same role but different
> principals are considered distinct.
> 
> 
> Diffs
> -
> 
>   src/tests/resources_tests.cpp 4b25e82c13e4f46c73803f773db90f269c09c48a 
> 
> Diff: https://reviews.apache.org/r/42751/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42588: Added unit-test for `NetClsHandleManager`.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (lines 171 - 173)


Why error here? THis should be false, right? I'll fix it for you.


- Jie Yu


On Feb. 5, 2016, 2:16 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42588/
> ---
> 
> (Updated Feb. 5, 2016, 2:16 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unit-test for `NetClsHandleManager`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> 85004aea4c1ee4b25e106f3ce40025c69f1ce030 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> bf6c88341dedc0a37546c04f38197c892b498684 
>   src/tests/containerizer/isolator_tests.cpp 
> 67322abc776cbd501385932676852a79b74ef248 
> 
> Diff: https://reviews.apache.org/r/42588/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42157: Changed ProvisionerAppcTest to use AppcStoreTest suite.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Jan. 22, 2016, 5:36 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42157/
> ---
> 
> (Updated Jan. 22, 2016, 5:36 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change will enable ProvisionerAppcTest suite to reuse common code like
> test image creation.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> e3d08d9e49df93d5290099c8bfd917f60c93e51b 
> 
> Diff: https://reviews.apache.org/r/42157/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 42995: Allow tasks to set persistent volume as readonly or readwrite resource.

[Anindya Sinha]

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

(Updated Feb. 5, 2016, 10:57 p.m.)


Review request for mesos and Adam B.


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


Repository: mesos


Description
---

If the task is the first consumer of the shared persistent volume, then
set the ownership of the persistent volume to match that of the task.
Otherwise, allow the task to be executed only if the ownership of the
persistent volume matches that of the task.
Allow the task to specify the persistent volume access to be read-only
or read-write. Note that the persistent volume is always created as
read-write.


Diffs (updated)
-

  src/examples/persistent_shared_volume_framework.cpp PRE-CREATION 
  src/master/master.cpp e1ca81dab85a7ab1391eca0d6bd995548bf79c22 
  src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
  src/master/validation.cpp 66898e914c7b4ab83c4580be67530f355cfb05ca 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
b9615becc87e6c69c5c56b6ae76ccae180c26917 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
01c0ad6dbb6d509e62e769365586b3d23dcb240d 

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


Testing
---

make check done.


Thanks,

Anindya Sinha

Re: Review Request 42992: Support sharing of resources through reference counting of resources.

[Anindya Sinha]

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

(Updated Feb. 5, 2016, 10:57 p.m.)


Review request for mesos and Adam B.


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


Repository: mesos


Description
---

Added new Offer::Operation of SHARE and UNSHARE for resources.
Added ShareInfo within Resources protobuf to allow for sharing of resources
and keep track of consumers of such resources.
Shareable resources keeps track of its consumers and makes decision based on
that during task launch and task termination.
Allow DESTROY or UNSHARE for shared volumes only if reference count is 0.


Diffs (updated)
-

  include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
  include/mesos/resources.hpp 6bfac2e7e8799e74d87c7570fc5eef320ba76eb1 
  include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
  include/mesos/v1/resources.hpp 5a88c0756db2ea8db0f5df7ea3019b511ea135af 
  src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13 
  src/common/resources_utils.cpp 70e6f025d89383084ab8b2cda23ab1cd55d959b2 
  src/master/allocator/mesos/hierarchical.cpp 
a9d2c23162892e0f97d89a076d2311091d91 
  src/master/allocator/sorter/drf/sorter.cpp 
18797e42a9c2bd20392020237cfae600a5ffe12c 
  src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 
  src/master/master.cpp e1ca81dab85a7ab1391eca0d6bd995548bf79c22 
  src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
  src/master/validation.cpp 66898e914c7b4ab83c4580be67530f355cfb05ca 
  src/v1/resources.cpp be4a5d153e9313cb71a6e85d1ed25a358537f2b7 

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


Testing
---

make check done.


Thanks,

Anindya Sinha

Re: Review Request 42996: Add unit tests for sharing of resources.

[Anindya Sinha]

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

(Updated Feb. 5, 2016, 10:57 p.m.)


Review request for mesos and Adam B.


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


Repository: mesos


Description
---

Add unit tests for sharing of resources.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
0acfc098750ff8ff9505207b983a34c1ccf3ad06 
  src/tests/master_validation_tests.cpp 
6fae01fa1833ae05ec82618a4ae28ac5bd275bd5 
  src/tests/mesos.hpp e07d8aa6f1e507a91ce70763aafa134bdd9a7ec8 
  src/tests/persistent_volume_tests.cpp 
cbf2bcedd5b4c14107d3b50a74f161aa9395d7d0 
  src/tests/resources_tests.cpp 4b25e82c13e4f46c73803f773db90f269c09c48a 

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


Testing
---

make check done.


Thanks,

Anindya Sinha

Re: Review Request 42994: Added a persistent volume test framework for shared volumes.

[Anindya Sinha]

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

(Updated Feb. 5, 2016, 10:57 p.m.)


Review request for mesos and Adam B.


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


Repository: mesos


Description
---

Added a persistent volume test framework for shared volumes.


Diffs (updated)
-

  src/Makefile.am 22f51319a1c06da02cac5822d42315e3027cf500 
  src/examples/persistent_shared_volume_framework.cpp PRE-CREATION 
  src/tests/examples_tests.cpp a9b685b0290f23461f43b12f05f6071fa46412a6 
  src/tests/persistent_shared_volume_framework_test.sh PRE-CREATION 

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


Testing
---

make check done.


Thanks,

Anindya Sinha

Re: Review Request 42993: Support for multiple frameworks sharing the same resource.

[Anindya Sinha]

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

(Updated Feb. 5, 2016, 10:57 p.m.)


Review request for mesos and Adam B.


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


Repository: mesos


Description
---

The allocator tracks allocated shared resources separately from the
remaining allocated resources so that multiple frameworks see the
same view of the shared resource at any given time. This avoids
unnecessary copies of the same shared resource when multiple
frameworks use such resources simultaneously.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
a9d2c23162892e0f97d89a076d2311091d91 
  src/master/allocator/sorter/drf/sorter.hpp 
4669149b81de39b4bb921ef7cd6787aa583f6e40 
  src/master/allocator/sorter/drf/sorter.cpp 
18797e42a9c2bd20392020237cfae600a5ffe12c 
  src/master/allocator/sorter/sorter.hpp 
a0a779b81f6d048271f15256b38ff907ae144b83 
  src/master/master.cpp e1ca81dab85a7ab1391eca0d6bd995548bf79c22 

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


Testing
---

make check done.


Thanks,

Anindya Sinha

Re: Review Request 43220: Added script to generate docs from endpoint help strings.

[Kevin Klues]

> On Feb. 5, 2016, 5:17 a.m., Ben Mahler wrote:
> > support/generate-endpoint-help.py, lines 225-227
> > 
> >
> > Do you need the empty check? Also I think we've been doing `if 
> > new_name` in our python style.

Unfortunately, I do need the empty check because runing join() with an empty 
string at the end produces an extra, unwanted "/"


- Kevin


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


On Feb. 5, 2016, 3:30 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43220/
> ---
> 
> (Updated Feb. 5, 2016, 3:30 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-3831
> https://issues.apache.org/jira/browse/MESOS-3831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Invoke this script from anywhere within the mesos source tree
> (including in the build directory) to autogenerate documentation for
> all endpoint strings and install them into the docs/endpoint folder.
> In a subsequent commit we commit all of these autogenerated files back
> to the source repo and add a link in home.md to find them.
> 
> 
> Diffs
> -
> 
>   support/generate-endpoint-help.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43220/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 42618: Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.

[Avinash sridharan]

> On Feb. 5, 2016, 12:31 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp, line 200
> > 
> >
> > What's defaultPrimary? Who is the user? Can you just do the following:
> > 
> > ```
> > if (!primaries.empty()) {
> >   handleManager = NetClsHandleManager(primaries.get());
> > }
> > ```

Have converted defaultPrimary as a helper function in 
`CgroupsNetClsIsolatorProcess` . Since the defaultPrimary is a concept specific 
to the isolator, and it needs to store the ranges, on which the defaultPrimary 
operates, as well. Hence, have introduced an `IntervalSet` field in 
`CgroupsNetClsIsolatorProcess` as well.


> On Feb. 5, 2016, 12:31 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp, line 408
> > 
> >
> > Can you make 'defaultPrimiary' a helper method instead of a field 
> > member. The field member will go away in the future once we introduce other 
> > allocation policies like one primary per framweork.

Please see my comments above about introducing an IntervalSet field 
in `CgroupsNetClsIsolatorProcess`.


- Avinash


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


On Feb. 5, 2016, 3:31 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42618/
> ---
> 
> (Updated Feb. 5, 2016, 3:31 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> 85004aea4c1ee4b25e106f3ce40025c69f1ce030 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> bf6c88341dedc0a37546c04f38197c892b498684 
> 
> Diff: https://reviews.apache.org/r/42618/diff/
> 
> 
> Testing
> ---
> 
> make and make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43096: Added helper function in cgroup for supporting net_cls subsystem.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 10:22 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebase.


Repository: mesos


Description
---

Added helper function in cgroup for supporting net_cls subsystem.


Diffs (updated)
-

  src/linux/cgroups.hpp 83b3e401ac69194bef496087dee32e0532b834ab 
  src/linux/cgroups.cpp bbc1fb3c60e53dffb1e8029942725e7102eb9aee 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 42947: Implemented the `status` method in `CgroupNetClsIsolatorProcess`.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 10:28 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Implemented the `status` method in `CgroupNetClsIsolatorProcess`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
d8cebd7be97170c799016f3c60d91d2a4de9510d 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
f96c3274610269eacc9e91fec21a47a06bfeea62 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 42782: Added a unit-test to test net_cls major handles set from command line.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 10:27 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added a unit-test to test net_cls major handles set from command line.


Diffs (updated)
-

  src/tests/containerizer/isolator_tests.cpp 
84fe4fb61ac032e68d2ab22e85f6b41481c04644 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 43107: Passed agent flag --cgroup_net_cls_primary_handle to net_cls isolator.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 10:27 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

The `cgroup/net_cls` isolator will use the primary handle, passed by the
operator using the --cgroup_net_cls_primary_handle, to initialize the
`NetClsHandleManager`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
f96c3274610269eacc9e91fec21a47a06bfeea62 

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


Testing
---

make and make check

Launched a slave by passing invalid flags to --cgroup_net_cls_primary_handle 
and made sure the slave correctly detects these invalid handles. 

Launched a slave with valid handle to make sure that the slave launches 
correctly.


Thanks,

Avinash sridharan

Re: Review Request 42156: Refactored up the common image creation in appc tests.

[Jie Yu]

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




src/tests/containerizer/provisioner_appc_tests.cpp (line 55)


Can you add a TODO to move AppcSpecTest to a separate test file 
(appc_spec_tests.cpp) like we did for docker.



src/tests/containerizer/provisioner_appc_tests.cpp (line 127)


s/baseDirectory/storeDir/

s/prepareSimpleImageDirectory/createTestImage/

Please also add a comment about what this method is doing? What it is 
returning?



src/tests/containerizer/provisioner_appc_tests.cpp (line 129)


No need to do this check. os::mkdir will be a no-op if the directory 
already exists.



src/tests/containerizer/provisioner_appc_tests.cpp (line 177)


Could you please use paths::getImagePath here instead?



src/tests/containerizer/provisioner_appc_tests.cpp (lines 179 - 180)


THis is not 'Rootfs' dir, right?

Can you do the following:
1) create rootfs dir (spec::getImageRootfsPath)
2) create tmp under rootfs
3) create the test file



src/tests/containerizer/provisioner_appc_tests.cpp (lines 193 - 194)


Please use spec::getImageManifestPath


- Jie Yu


On Jan. 22, 2016, 6:47 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42156/
> ---
> 
> (Updated Jan. 22, 2016, 6:47 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change will enable other tests to reuse the common test image creation
> logic.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> e3d08d9e49df93d5290099c8bfd917f60c93e51b 
> 
> Diff: https://reviews.apache.org/r/42156/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

> On Feb. 5, 2016, 2:20 p.m., Qian Zhang wrote:
> > 1. Besides changing the code, you may also need to update the following doc 
> > to describe the newly introduced env var "LIBPROCESS_MAX_WORKER_THREADS"
> > https://github.com/apache/mesos/blob/master/docs/configuration.md#libprocess-options
> > 2. Have you test the patch with executor from E2E? For example, start Mesos 
> > agent with "LIBPROCESS_MAX_WORKER_THREADS" in its 
> > "--executor_environment_variables" option, launch a task, and check if the 
> > libprocess worker thread number of the related executor is the expected 
> > number.

1. Done.
2. No. Can someone point me to a simple test to view executor logs?


- Maged


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


On Feb. 5, 2016, 9:46 p.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated Feb. 5, 2016, 9:46 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, and Qian Zhang.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
>   docs/configuration.md 4b5a394 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 42997: Offer shareable resources to frameworks only if opted in.

[Anindya Sinha]

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

(Updated Feb. 5, 2016, 10:57 p.m.)


Review request for mesos and Adam B.


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


Repository: mesos


Description
---

Added a new capability SHAREABLE_RESOURCES that frameworks need to opt in
if they are interested in receiving shared resources in their offers.


Diffs (updated)
-

  include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
  include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
  src/examples/persistent_shared_volume_framework.cpp PRE-CREATION 
  src/master/allocator/mesos/hierarchical.hpp 
1f0c440565ee0f9926d168734df30fd740a53f88 
  src/master/allocator/mesos/hierarchical.cpp 
a9d2c23162892e0f97d89a076d2311091d91 
  src/master/master.cpp e1ca81dab85a7ab1391eca0d6bd995548bf79c22 
  src/tests/hierarchical_allocator_tests.cpp 
0acfc098750ff8ff9505207b983a34c1ccf3ad06 

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


Testing
---

make check done.


Thanks,

Anindya Sinha

Re: Review Request 42998: Added docs for shareable resources.

[Anindya Sinha]

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

(Updated Feb. 5, 2016, 10:59 p.m.)


Review request for mesos and Adam B.


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


Repository: mesos


Description
---

Added docs for shareable resources.


Diffs (updated)
-

  docs/home.md a2000a35a6eeaa7b36cb1796532263f5a703ac88 
  docs/shareable-resources.md PRE-CREATION 

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


Testing
---

Just documentation changes in this RR.


Thanks,

Anindya Sinha

Re: Review Request 43220: Added script to generate docs from endpoint help strings.

[Kevin Klues]

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

(Updated Feb. 5, 2016, 11:17 p.m.)


Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.


Changes
---

Updated to address bmahler's comments.


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


Repository: mesos


Description
---

Invoke this script from anywhere within the mesos source tree
(including in the build directory) to autogenerate documentation for
all endpoint strings and install them into the docs/endpoint folder.
In a subsequent commit we commit all of these autogenerated files back
to the source repo and add a link in home.md to find them.


Diffs (updated)
-

  support/generate-endpoint-help.py PRE-CREATION 

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


Testing (updated)
---

Ran and generated files from it.  Then rebuilt the website and verified that 
everything looked as it should.


Thanks,

Kevin Klues

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

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

(Updated Feb. 5, 2016, 9:45 p.m.)


Review request for mesos and Klaus Ma.


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


Repository: mesos


Description
---

Added env var to set upper bound on number of libprocess worker threads.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp d8a74d7 
  docs/configuration.md 4b5a394 

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


Testing
---

Passed make check on x86_64 Ubuntu 14.04.

Invocations of mesos-master, mesos-slave, and mesos-tests with the 
LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
"-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
The last 3 tests generated warnings.


Thanks,

Maged Michael

Re: Review Request 42781: Added the --cgroups_net_cls_primary_handle flag to the slave.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 5, 2016, 3:32 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42781/
> ---
> 
> (Updated Feb. 5, 2016, 3:32 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4344
> https://issues.apache.org/jira/browse/MESOS-4344
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the --cgroups_net_cls_primary_handle flag to the slave.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.hpp 23ec158f4a95fa76d657d9eb6a4a6fe30d57e5b6 
>   src/slave/flags.cpp 24a23325cc255d0d7b7af7ed096b6d3012ad75c7 
> 
> Diff: https://reviews.apache.org/r/42781/diff/
> 
> 
> Testing
> ---
> 
> Tested the flag by launching a mesos agent and checking that the slave takes 
> the `--cgroups_net_cls_major_handles` flag.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43239: Added Resources::size().

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Feb. 5, 2016, 2:18 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43239/
> ---
> 
> (Updated Feb. 5, 2016, 2:18 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Resources::size().
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6bfac2e7e8799e74d87c7570fc5eef320ba76eb1 
>   include/mesos/v1/resources.hpp 5a88c0756db2ea8db0f5df7ea3019b511ea135af 
> 
> Diff: https://reviews.apache.org/r/43239/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42781: Added the --cgroups_net_cls_primary_handle flag to the slave.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 10:26 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added the --cgroups_net_cls_primary_handle flag to the slave.


Diffs (updated)
-

  src/slave/flags.hpp 23ec158f4a95fa76d657d9eb6a4a6fe30d57e5b6 
  src/slave/flags.cpp 24a23325cc255d0d7b7af7ed096b6d3012ad75c7 

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


Testing
---

Tested the flag by launching a mesos agent and checking that the slave takes 
the `--cgroups_net_cls_major_handles` flag.


Thanks,

Avinash sridharan

Review Request 43267: Returned "ServiceUnavailable" for slave's /state during recovery.

[Vinod Kone]

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

Review request for mesos, Anand Mazumdar and Ben Mahler.


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


Repository: mesos


Description
---

This will help operators hitting the /state endpoint by not returning incomplete
data when slave is still recoverying.


Diffs
-

  src/slave/http.cpp 9167030e77efc7e1d0dc0c6bd800d20ba9c55e3c 
  src/tests/slave_tests.cpp b2b1fd4be933512c3dffa8c1c579b59782a37d77 

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


Testing
---

make -j3 check GTEST_FILTER="*StateEndpointUnavailableDuringRecovery*"

./bin/mesos-tests.sh --gtest_filter="*StateEndpointUnavailableDuringRecovery*" 
--gtest_repeat=1000 --gtest_break_on_failure

./bin/mesos-tests.sh


Thanks,

Vinod Kone

Re: Review Request 43261: Refactor process::initialize environment variable parsing into Flags.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43260, 43261]

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

- Mesos ReviewBot


On Feb. 5, 2016, 9:05 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43261/
> ---
> 
> (Updated Feb. 5, 2016, 9:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4609
> https://issues.apache.org/jira/browse/MESOS-4609
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moves the environment variables `LIBPROCESS_` + (`IP`, `ADVERTISE_IP`, 
> `PORT`, `ADVERTISE_PORT`) into a Flags object along with the validation logic 
> for ports.  Help strings of the flags are synced with the docs in 
> configuration.md.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 4a5302085917db14654a2f942d85261b934473f7 
> 
> Diff: https://reviews.apache.org/r/43261/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 43272: Modify the containerizer, fetcher, and container logger's environment.

[Joseph Wu]

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

Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Till 
Toenshoff.


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


Repository: mesos


Description
---

Delegates some of the environment logic in the containerizer and fetcher to the 
`subprocess_environment` helper.
Changes the logrotate container logger to use the default `environment` for 
`subprocess`.


Diffs
-

  src/slave/container_loggers/lib_logrotate.cpp 
01a3ff031b70366443214e2895803cd4b2606ad8 
  src/slave/containerizer/containerizer.cpp 
59904684cdeb17ef2b42092a3558802c42bfb6bd 
  src/slave/containerizer/fetcher.cpp 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 

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


Testing
---

make check (OSX)


Thanks,

Joseph Wu

Review Request 43271: Modify subprocess to deal with LIBPROCESS_PORT specially.

[Joseph Wu]

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

Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Till 
Toenshoff.


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


Repository: mesos


Description
---

* Adds a helper method for getting the current environment plus considerations 
for libprocess.
* Changes the default behavior of `process::subprocess` to use the above helper 
when given `environment = None()`.
* Adds a warning inside `process::subprocess` if `LIBPROCESS_PORT` conflicts 
with the current process's `LIBPROCESS_PORT`.


Diffs
-

  3rdparty/libprocess/include/process/subprocess.hpp 
bb50cc3070245a294fa16efe44f14ae893bc5518 
  3rdparty/libprocess/src/subprocess.cpp 
ff477e37a9619c780bddd5a8e629fa981b729715 

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


Testing
---

make

Tests are run in the next review.


Thanks,

Joseph Wu

Review Request 43261: Refactor process::initialize environment variable parsing into Flags.

[Joseph Wu]

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

Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Till 
Toenshoff.


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


Repository: mesos


Description
---

Moves the environment variables `LIBPROCESS_` + (`IP`, `ADVERTISE_IP`, `PORT`, 
`ADVERTISE_PORT`) into a Flags object along with the validation logic for 
ports.  Help strings of the flags are synced with the docs in configuration.md.


Diffs
-

  3rdparty/libprocess/src/process.cpp 4a5302085917db14654a2f942d85261b934473f7 

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


Testing
---

make check (OSX)


Thanks,

Joseph Wu

Re: Review Request 43156: Added test: dynamic reservations with same role, different principals.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43156]

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

- Mesos ReviewBot


On Feb. 5, 2016, 5:47 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43156/
> ---
> 
> (Updated Feb. 5, 2016, 5:47 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4590
> https://issues.apache.org/jira/browse/MESOS-4590
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test: dynamic reservations with same role, different principals.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_endpoints_tests.cpp 
> 35c093567b07a11ca6eee85e2ff91894a460a7af 
> 
> Diff: https://reviews.apache.org/r/43156/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42983: Implemented the `status` method in `MesosContainerizer`.

[Avinash sridharan]

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

(Updated Feb. 5, 2016, 8 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Made logs more descipritive.


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


Repository: mesos


Description
---

Implemented the `status` method in `MesosContainerizer`.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
811ab7937279c4a55da450c136f9fcb1303ea0d5 
  src/slave/containerizer/mesos/containerizer.cpp 
ed8a54543cd2e51bd9967745ab2351f909b7cf35 

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


Testing
---

make


Thanks,

Avinash sridharan

Re: Review Request 43240: Removed implicit, value changing conversion.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43240]

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

- Mesos ReviewBot


On Feb. 5, 2016, 5:41 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43240/
> ---
> 
> (Updated Feb. 5, 2016, 5:41 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Whether `char` is signed or not is up to the implementation, so be more 
> explicit
> here. Also, since we want a `char` with all bits set, use a value 
> representation
> that more clearly communicates that.
> 
> 
> Diffs
> -
> 
>   src/log/tool/benchmark.cpp 9cb60bd66f53e0d191d96a429bfca89d91412339 
> 
> Diff: https://reviews.apache.org/r/43240/diff/
> 
> 
> Testing
> ---
> 
> When compiling with trunk clang, compilation fails with
> 
> ../../src/log/tool/benchmark.cpp|209 col 47 warning| implicit conversion 
> from 'int' to 'value_type' (aka 'char') changes value from 255 to -1 
> [-Wconstant-conversion]
> 
> After inserting the explicit cast the code compiles fine.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43072: Use full screen width for Mesos UI using Bootstraps (v3.3.6) container-fluid

[Vinod Kone]

> On Feb. 2, 2016, 2:45 a.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [43072]
> > 
> > Failed command: ./support/apply-review.sh -n -r 43072
> > 
> > Error:
> > 2016-02-02 02:45:28 URL:https://reviews.apache.org/r/43072/diff/raw/ 
> > [226634/226634] -> "43072.patch" [1]
> > Traceback (most recent call last):
> >   File "support/apply-reviews.py", line 342, in 
> > reviewboard()
> >   File "support/apply-reviews.py", line 321, in reviewboard
> > apply_review()
> >   File "support/apply-reviews.py", line 139, in apply_review
> > commit_patch()
> >   File "support/apply-reviews.py", line 180, in commit_patch
> > data = patch_data()
> >   File "support/apply-reviews.py", line 199, in patch_data
> > return reviewboard_data()
> >   File "support/apply-reviews.py", line 252, in reviewboard_data
> > email=user.get('email'))
> > UnicodeEncodeError: 'ascii' codec can't encode character u'\xf8' in 
> > position 11: ordinal not in range(128)
> > 
> > Full log: https://builds.apache.org/job/mesos-reviewbot/11187/console
> 
> haosdent huang wrote:
> I think we could ignore this error message.
> 
> Bernd Mathiske wrote:
> I'd rather not ignore the error message. How can there be an out-of-range 
> char in HTML and CSS files?
> 
> haosdent huang wrote:
> I afraid this may caused by boostrap have some special characters like 
> `ok:before{content:"\e013"}`.
> 
> Bernd Mathiske wrote:
> This should be an encoding using printable cahrs (<=128) only.
> 
> haosdent huang wrote:
> LoL, seems `Michael Lunøe` cause the problem. Need fix the 
> apply_reviews.py
> 
> haosdent huang wrote:
> Need do changes in apply_reviews.py to support no-ascii commit messages.
> ```
> diff --git a/support/apply-reviews.py b/support/apply-reviews.py
> index ea5e43a..36bfb1e 100755
> --- a/support/apply-reviews.py
> +++ b/support/apply-reviews.py
> @@ -185,7 +185,7 @@ def commit_patch():
>else:
>  amend = '-e'
> 
> -  cmd = 'git commit --author \'{author}\' {_amend} -am \'{message}\''\
> +  cmd = u'git commit --author \'{author}\' {_amend} -am \'{message}\''\
>  .format(author=quote(data['author']),
>  _amend=amend,
>  message=quote(data['message']))
> @@ -248,7 +248,7 @@ def reviewboard_data():
>user = url_to_json(reviewboard_user_url(
>  review.get('links').get('submitter').get('title'))).get('user')
> 
> -  author = '{author} <{email}>'.format(author=user.get('fullname'),
> +  author = u'{author} <{email}>'.format(author=user.get('fullname'),
> email=user.get('email'))
>message = '\n\n'.join(['{summary}',
>'{description}',
> ```
> 
> Vinod Kone wrote:
> Thanks haosdent for the diff. I committed a patch based on this (and 
> attributed it to you).
> 
> Michael Lunøe wrote:
> Where do I see this patch, so I know when to re-open this?

It's here 
https://github.com/apache/mesos/commit/c93c5009cc0004388b8e248478a2291ff9d0c595.

Not sure what you mean by "re-open this"? Your patch is committed too.


- Vinod


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


On Feb. 1, 2016, 11:36 p.m., Michael Lunøe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43072/
> ---
> 
> (Updated Feb. 1, 2016, 11:36 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Thomas Rampelberg.
> 
> 
> Bugs: MESOS-2585
> https://issues.apache.org/jira/browse/MESOS-2585
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The solution to our problem is solved by updating the Bootstrap CSS file, as 
> container-fluid was introduced later in Bootstrap.
> 
> I have updated to lastest stable release of Bootstrap v3.3.6. Looking through 
> the change log, the only thing that we are using in the Mesos UI and changed 
> is the "hide" class (deprecated), so instances of these have been replaced by 
> its successor ".hidden" class.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 6b18056 
>   src/webui/master/static/css/bootstrap-3.0.3.min.css c547283 
>   src/webui/master/static/css/bootstrap-3.3.6.min.css PRE-CREATION 
>   src/webui/master/static/framework.html 9b28820 
>   src/webui/master/static/home.html d6cde1e 
>   src/webui/master/static/index.html 25caf53 
>   src/webui/master/static/slave.html bc46885 
>   src/webui/master/static/slave_executor.html 9d582d5 
>   src/webui/master/static/slave_framework.html 96d788f 
> 
> Diff: https://reviews.apache.org/r/43072/diff/
> 
> 
> Testing
> ---
> 
> 

Review Request 43263: Updated site building instrucutions in site/README.md.

[Kapil Arya]

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

Review request for mesos.


Repository: mesos


Description
---

Updated site building instrucutions in site/README.md.


Diffs
-

  site/README.md 71ca3ebbc645c6c188223895d12fb436b55038f9 

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 43122: Updated doxygen target for site/Rakefile.

[Kapil Arya]

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

(Updated Feb. 5, 2016, 4:19 p.m.)


Review request for mesos, Joseph Wu and Neil Conway.


Summary (updated)
-

Updated doxygen target for site/Rakefile.


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


Repository: mesos


Description (updated)
---

See summary


Diffs (updated)
-

  site/Rakefile 0ce4b7975f95ab6930f0b2674191930df9ab5b20 

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 43200: Updated role documentation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43199, 43200]

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

- Mesos ReviewBot


On Feb. 4, 2016, 7:55 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43200/
> ---
> 
> (Updated Feb. 4, 2016, 7:55 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4452
> https://issues.apache.org/jira/browse/MESOS-4452
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated role documentation.
> 
> Added information on the distinction between roles and principals.
> 
> 
> Diffs
> -
> 
>   docs/roles.md c84a483259922be01d1686befd01b7b1c4005bbd 
> 
> Diff: https://reviews.apache.org/r/43200/diff/
> 
> 
> Testing
> ---
> 
> Viewed in the mesos website container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 43105: Ensured the allocator does not double account resources.

[Guangya Liu]

> On 二月 4, 2016, 3:46 p.m., Guangya Liu wrote:
> > This case may happen when master is recovering, framework recovery start 
> > before some agent. Can you please add a unit test to cover this code 
> > change? It could be register framework first, then addslave and check the 
> > result.
> 
> Alexander Rukletsov wrote:
> Nope, this can't happen, because in Mesos we reconstruct framework state 
> through reconnecting agents.

What about in the case when framework failover? Does there are any potential 
steps/operations which can cause `addFramework()` be called before 
`addSlave()`? Thanks.


- Guangya


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


On 二月 2, 2016, 11:13 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43105/
> ---
> 
> (Updated 二月 2, 2016, 11:13 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If `addFramework()` is called before `addSlave()` double accounting
> of resources is possible. We have not observed bugs because Mesos
> currently always adds a framework in the allocator after all agents
> with the framework's tasks have been added.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp e163669c9c4e4c98572968f18987704b60722a79 
>   src/master/allocator/mesos/hierarchical.cpp 
> 1a07d69016407e5aad2209586da37fecbcddb765 
> 
> Diff: https://reviews.apache.org/r/43105/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 43083: Supported working dir in docker runtime isolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43019, 43020, 43021, 43022, 43036, 43037, 43081, 43082, 
43166, 43167, 43168, 43083]

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

- Mesos ReviewBot


On Feb. 5, 2016, 8:13 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43083/
> ---
> 
> (Updated Feb. 5, 2016, 8:13 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4005
> https://issues.apache.org/jira/browse/MESOS-4005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported working dir in docker runtime isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> dc0868e24486644371b7c285fdf16b9aeac4ca5b 
>   src/slave/containerizer/mesos/isolators/docker/runtime.hpp 
> 6e6ec86d04b157321141cdf5214033e83ce74548 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> c8a93725fa04476c935a8ba18ad2a145c36fb5fa 
> 
> Diff: https://reviews.apache.org/r/43083/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

[Abhishek Dasgupta]

> On Feb. 5, 2016, 9:54 a.m., haosdent huang wrote:
> > src/tests/containerizer/docker_tests.cpp, line 129
> > 
> >
> > Why adjust await time here? I think 15 seconds should be enough to get 
> > version, start container and inspect.

If Labels are tested , then docker version may need another some time to 
connect to docker. Anyway, while testing, I found 15 seconds are two less time 
for downloading image, inspecting versions and so I increased it to 30.


- Abhishek


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


On Feb. 5, 2016, 7:15 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 5, 2016, 7:15 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43193: Fixed parsing docker image env vars.

[Shuai Lin]

> On Feb. 5, 2016, 8:47 a.m., Guangya Liu wrote:
> > But the unit test does passed 
> > https://github.com/apache/mesos/blob/master/src/tests/containerizer/docker_tests.cpp#L578-L579
> >  even without your fix, can you please clarify why the unit test passed for 
> > validating `JAVA_DEBIAN_VERSION`? It is also an env with "=".

The bug is trigged when there is '=' in the value part of the env. 

The env you mentioned is `JAVA_DEBIAN_VERSION=8u66-b01-1~bpo8+1`, which have 
key `JAVA_DEBIAN_VERSION` and value `8u66-b01-1~bpo8+1`. There is no '=' in the 
value part. 

Thanks for the question, I'v added an extra env which has '=' in its value in 
that test case.


- Shuai


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


On Feb. 4, 2016, 6:07 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43193/
> ---
> 
> (Updated Feb. 4, 2016, 6:07 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4587
> https://issues.apache.org/jira/browse/MESOS-4587
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow equal character in the value of environemnt variables.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e9e983499bffd6f8f29bb4b71715f737d1 
> 
> Diff: https://reviews.apache.org/r/43193/diff/
> 
> 
> Testing
> ---
> 
> - Start mesos master, slave and marathon, add a marathon task with the docker 
> image `jupyter/all-spark-notebook`, the one the reporter has problem to 
> launch beacuse it has equal signs in env vars. 
> ```json
> {
>   "id": "/testapp/app1",
>   "cmd": "env && sleep 300",
>   "cpus": 0.1,
>   "mem": 18,
>   "requirePorts": false,
>   "instances": 1,
>   "executor": "",
>   "container": {
> "type": "DOCKER",
> "docker": {
>   "image": "jupyter/all-spark-notebook",
>   "network": "BRIDGE",
>   "portMappings": [
> {
>   "containerPort": ,
>   "hostPort": 31000,
>   "protocol": "tcp"
> }
>   ],
>   "privileged": false
> }
>   }
> }
> ```
> - Without this patch, the task fails to launch with:
> ```sh
> E0204 18:04:38.538508 14843 slave.cpp:3703] Container 
> '0dd269c4-cdeb-4a24-ae5c-86b75fae8737' for executor 
> 'testapp_app1.c19685e6-cb69-11e5-99d6-024234e5bbc1' of framework 
> aaf4cc37-4809-4495-a9bd-18c1407442e0- failed to start: Unable to create 
> image: Unexpected Env format for 'ContainerConfig.Env'
> ```
>   
> - With this patch, the task starts successfully.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 43193: Fixed parsing docker image env vars.

[Shuai Lin]

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

(Updated Feb. 5, 2016, 10:14 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Added asserts the newly added docker tests env.


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


Repository: mesos


Description
---

Allow equal character in the value of environemnt variables.


Diffs (updated)
-

  src/docker/docker.cpp b4b8d3e9e983499bffd6f8f29bb4b71715f737d1 
  src/tests/containerizer/docker_tests.cpp 
f0ad20e5a8252ad761e8fa42724caed31b8fae9d 

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


Testing
---

- Start mesos master, slave and marathon, add a marathon task with the docker 
image `jupyter/all-spark-notebook`, the one the reporter has problem to launch 
beacuse it has equal signs in env vars. 
```json
{
  "id": "/testapp/app1",
  "cmd": "env && sleep 300",
  "cpus": 0.1,
  "mem": 18,
  "requirePorts": false,
  "instances": 1,
  "executor": "",
  "container": {
"type": "DOCKER",
"docker": {
  "image": "jupyter/all-spark-notebook",
  "network": "BRIDGE",
  "portMappings": [
{
  "containerPort": ,
  "hostPort": 31000,
  "protocol": "tcp"
}
  ],
  "privileged": false
}
  }
}
```
- Without this patch, the task fails to launch with:
```sh
E0204 18:04:38.538508 14843 slave.cpp:3703] Container 
'0dd269c4-cdeb-4a24-ae5c-86b75fae8737' for executor 
'testapp_app1.c19685e6-cb69-11e5-99d6-024234e5bbc1' of framework 
aaf4cc37-4809-4495-a9bd-18c1407442e0- failed to start: Unable to create 
image: Unexpected Env format for 'ContainerConfig.Env'
```
  
- With this patch, the task starts successfully.


Thanks,

Shuai Lin

Re: Review Request 43193: Fixed parsing docker image env vars.

[Guangya Liu]

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



But the unit test does passed 
https://github.com/apache/mesos/blob/master/src/tests/containerizer/docker_tests.cpp#L578-L579
 even without your fix, can you please clarify why the unit test passed for 
validating `JAVA_DEBIAN_VERSION`? It is also an env with "=".

- Guangya Liu


On 二月 4, 2016, 6:07 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43193/
> ---
> 
> (Updated 二月 4, 2016, 6:07 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4587
> https://issues.apache.org/jira/browse/MESOS-4587
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow equal character in the value of environemnt variables.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e9e983499bffd6f8f29bb4b71715f737d1 
> 
> Diff: https://reviews.apache.org/r/43193/diff/
> 
> 
> Testing
> ---
> 
> - Start mesos master, slave and marathon, add a marathon task with the docker 
> image `jupyter/all-spark-notebook`, the one the reporter has problem to 
> launch beacuse it has equal signs in env vars. 
> ```json
> {
>   "id": "/testapp/app1",
>   "cmd": "env && sleep 300",
>   "cpus": 0.1,
>   "mem": 18,
>   "requirePorts": false,
>   "instances": 1,
>   "executor": "",
>   "container": {
> "type": "DOCKER",
> "docker": {
>   "image": "jupyter/all-spark-notebook",
>   "network": "BRIDGE",
>   "portMappings": [
> {
>   "containerPort": ,
>   "hostPort": 31000,
>   "protocol": "tcp"
> }
>   ],
>   "privileged": false
> }
>   }
> }
> ```
> - Without this patch, the task fails to launch with:
> ```sh
> E0204 18:04:38.538508 14843 slave.cpp:3703] Container 
> '0dd269c4-cdeb-4a24-ae5c-86b75fae8737' for executor 
> 'testapp_app1.c19685e6-cb69-11e5-99d6-024234e5bbc1' of framework 
> aaf4cc37-4809-4495-a9bd-18c1407442e0- failed to start: Unable to create 
> image: Unexpected Env format for 'ContainerConfig.Env'
> ```
>   
> - With this patch, the task starts successfully.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 43193: Fixed parsing docker image env vars.

[Shuai Lin]

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

(Updated Feb. 5, 2016, 9:50 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Updated docker tests to cover this scenario.


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


Repository: mesos


Description
---

Allow equal character in the value of environemnt variables.


Diffs (updated)
-

  src/docker/docker.cpp b4b8d3e9e983499bffd6f8f29bb4b71715f737d1 
  src/tests/containerizer/docker_tests.cpp 
f0ad20e5a8252ad761e8fa42724caed31b8fae9d 

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


Testing
---

- Start mesos master, slave and marathon, add a marathon task with the docker 
image `jupyter/all-spark-notebook`, the one the reporter has problem to launch 
beacuse it has equal signs in env vars. 
```json
{
  "id": "/testapp/app1",
  "cmd": "env && sleep 300",
  "cpus": 0.1,
  "mem": 18,
  "requirePorts": false,
  "instances": 1,
  "executor": "",
  "container": {
"type": "DOCKER",
"docker": {
  "image": "jupyter/all-spark-notebook",
  "network": "BRIDGE",
  "portMappings": [
{
  "containerPort": ,
  "hostPort": 31000,
  "protocol": "tcp"
}
  ],
  "privileged": false
}
  }
}
```
- Without this patch, the task fails to launch with:
```sh
E0204 18:04:38.538508 14843 slave.cpp:3703] Container 
'0dd269c4-cdeb-4a24-ae5c-86b75fae8737' for executor 
'testapp_app1.c19685e6-cb69-11e5-99d6-024234e5bbc1' of framework 
aaf4cc37-4809-4495-a9bd-18c1407442e0- failed to start: Unable to create 
image: Unexpected Env format for 'ContainerConfig.Env'
```
  
- With this patch, the task starts successfully.


Thanks,

Shuai Lin

Re: Review Request 43193: Fixed parsing docker image env vars.

[Guangya Liu]

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




src/tests/containerizer/docker_tests.cpp (line 507)


Is this enough? I think that you may also want to add some logic to 
`EXPECT_EQ` to verify those env vars.


- Guangya Liu


On 二月 5, 2016, 9:50 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43193/
> ---
> 
> (Updated 二月 5, 2016, 9:50 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4587
> https://issues.apache.org/jira/browse/MESOS-4587
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow equal character in the value of environemnt variables.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e9e983499bffd6f8f29bb4b71715f737d1 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
> 
> Diff: https://reviews.apache.org/r/43193/diff/
> 
> 
> Testing
> ---
> 
> - Start mesos master, slave and marathon, add a marathon task with the docker 
> image `jupyter/all-spark-notebook`, the one the reporter has problem to 
> launch beacuse it has equal signs in env vars. 
> ```json
> {
>   "id": "/testapp/app1",
>   "cmd": "env && sleep 300",
>   "cpus": 0.1,
>   "mem": 18,
>   "requirePorts": false,
>   "instances": 1,
>   "executor": "",
>   "container": {
> "type": "DOCKER",
> "docker": {
>   "image": "jupyter/all-spark-notebook",
>   "network": "BRIDGE",
>   "portMappings": [
> {
>   "containerPort": ,
>   "hostPort": 31000,
>   "protocol": "tcp"
> }
>   ],
>   "privileged": false
> }
>   }
> }
> ```
> - Without this patch, the task fails to launch with:
> ```sh
> E0204 18:04:38.538508 14843 slave.cpp:3703] Container 
> '0dd269c4-cdeb-4a24-ae5c-86b75fae8737' for executor 
> 'testapp_app1.c19685e6-cb69-11e5-99d6-024234e5bbc1' of framework 
> aaf4cc37-4809-4495-a9bd-18c1407442e0- failed to start: Unable to create 
> image: Unexpected Env format for 'ContainerConfig.Env'
> ```
>   
> - With this patch, the task starts successfully.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

[Abhishek Dasgupta]

> On Feb. 5, 2016, 9:54 a.m., haosdent huang wrote:
> > src/tests/containerizer/docker_tests.cpp, line 129
> > 
> >
> > Why adjust await time here? I think 15 seconds should be enough to get 
> > version, start container and inspect.
> 
> Abhishek Dasgupta wrote:
> If Labels are tested , then docker version may need another some time to 
> connect to docker. Anyway, while testing, I found 15 seconds are two less 
> time for downloading image, inspecting versions and so I increased it to 30.

If Labels are tested , then docker version may need some more time to connect 
to docker. Anyway, while testing, I found 15 seconds are too less time for 
downloading image, inspecting versions and so I increased it to 30 to be in 
safe side.


- Abhishek


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


On Feb. 5, 2016, 10:52 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 5, 2016, 10:52 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43193: Fixed parsing docker image env vars.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43193]

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

- Mesos ReviewBot


On Feb. 5, 2016, 10:14 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43193/
> ---
> 
> (Updated Feb. 5, 2016, 10:14 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4587
> https://issues.apache.org/jira/browse/MESOS-4587
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow equal character in the value of environemnt variables.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e9e983499bffd6f8f29bb4b71715f737d1 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
> 
> Diff: https://reviews.apache.org/r/43193/diff/
> 
> 
> Testing
> ---
> 
> - Start mesos master, slave and marathon, add a marathon task with the docker 
> image `jupyter/all-spark-notebook`, the one the reporter has problem to 
> launch beacuse it has equal signs in env vars. 
> ```json
> {
>   "id": "/testapp/app1",
>   "cmd": "env && sleep 300",
>   "cpus": 0.1,
>   "mem": 18,
>   "requirePorts": false,
>   "instances": 1,
>   "executor": "",
>   "container": {
> "type": "DOCKER",
> "docker": {
>   "image": "jupyter/all-spark-notebook",
>   "network": "BRIDGE",
>   "portMappings": [
> {
>   "containerPort": ,
>   "hostPort": 31000,
>   "protocol": "tcp"
> }
>   ],
>   "privileged": false
> }
>   }
> }
> ```
> - Without this patch, the task fails to launch with:
> ```sh
> E0204 18:04:38.538508 14843 slave.cpp:3703] Container 
> '0dd269c4-cdeb-4a24-ae5c-86b75fae8737' for executor 
> 'testapp_app1.c19685e6-cb69-11e5-99d6-024234e5bbc1' of framework 
> aaf4cc37-4809-4495-a9bd-18c1407442e0- failed to start: Unable to create 
> image: Unexpected Env format for 'ContainerConfig.Env'
> ```
>   
> - With this patch, the task starts successfully.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43032]

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

- Mesos ReviewBot


On Feb. 5, 2016, 10:52 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 5, 2016, 10:52 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

[haosdent huang]

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


Fix it, then Ship it!




Ship It!


src/docker/docker.cpp (lines 538 - 540)


indent seems missing 1 space here.



src/tests/containerizer/docker_tests.cpp (line 129)


Why adjust await time here? I think 15 seconds should be enough to get 
version, start container and inspect.


- haosdent huang


On Feb. 5, 2016, 7:15 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Feb. 5, 2016, 7:15 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43193: Fixed parsing docker image env vars.

[Guangya Liu]

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


Ship it!




Thanks for the update

- Guangya Liu


On 二月 5, 2016, 10:14 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43193/
> ---
> 
> (Updated 二月 5, 2016, 10:14 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4587
> https://issues.apache.org/jira/browse/MESOS-4587
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow equal character in the value of environemnt variables.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e9e983499bffd6f8f29bb4b71715f737d1 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
> 
> Diff: https://reviews.apache.org/r/43193/diff/
> 
> 
> Testing
> ---
> 
> - Start mesos master, slave and marathon, add a marathon task with the docker 
> image `jupyter/all-spark-notebook`, the one the reporter has problem to 
> launch beacuse it has equal signs in env vars. 
> ```json
> {
>   "id": "/testapp/app1",
>   "cmd": "env && sleep 300",
>   "cpus": 0.1,
>   "mem": 18,
>   "requirePorts": false,
>   "instances": 1,
>   "executor": "",
>   "container": {
> "type": "DOCKER",
> "docker": {
>   "image": "jupyter/all-spark-notebook",
>   "network": "BRIDGE",
>   "portMappings": [
> {
>   "containerPort": ,
>   "hostPort": 31000,
>   "protocol": "tcp"
> }
>   ],
>   "privileged": false
> }
>   }
> }
> ```
> - Without this patch, the task fails to launch with:
> ```sh
> E0204 18:04:38.538508 14843 slave.cpp:3703] Container 
> '0dd269c4-cdeb-4a24-ae5c-86b75fae8737' for executor 
> 'testapp_app1.c19685e6-cb69-11e5-99d6-024234e5bbc1' of framework 
> aaf4cc37-4809-4495-a9bd-18c1407442e0- failed to start: Unable to create 
> image: Unexpected Env format for 'ContainerConfig.Env'
> ```
>   
> - With this patch, the task starts successfully.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

[Abhishek Dasgupta]

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

(Updated Feb. 5, 2016, 10:52 a.m.)


Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Set Docker labels based on TaskInfo labels.


Diffs (updated)
-

  src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
  src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
  src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
  src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
  src/tests/containerizer/docker_containerizer_tests.cpp 
645bdcf095145097d8b8c65d592c787417883145 
  src/tests/containerizer/docker_tests.cpp 
f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
  src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
  src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 

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


Testing
---

The following test cases in docker_tests have been changed:
DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
docker run command through arguments.


Thanks,

Abhishek Dasgupta

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Qian Zhang]

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



1. Besides changing the code, you may also need to update the following doc to 
describe the newly introduced env var "LIBPROCESS_MAX_WORKER_THREADS"
https://github.com/apache/mesos/blob/master/docs/configuration.md#libprocess-options
2. Have you test the patch with executor from E2E? For example, start Mesos 
agent with "LIBPROCESS_MAX_WORKER_THREADS" in its 
"--executor_environment_variables" option, launch a task, and check if the 
libprocess worker thread number of the related executor is the expected number.

- Qian Zhang


On Feb. 5, 2016, 11:39 a.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated Feb. 5, 2016, 11:39 a.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Review Request 43269: MasterContender/MasterDetector loadable as modules.

[Mark Cavage]

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

Review request for mesos.


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


Repository: mesos


Description
---

MasterContender/MasterDetector loadable as modules.

There is a large set of changes here, mostly due to a namespace rename. The 
previous version had the MasterContender/Detector APIs under an internal 
namespace, which didn't seem appropriate for an external contract. So the 
actual code changes that were impactful are relatively surgical, with then a 
wide ranging downstream cleanup to make the existing code base reference the 
new location.


Diffs
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 
  include/mesos/module/contender.hpp PRE-CREATION 
  include/mesos/module/detector.hpp PRE-CREATION 
  include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
  src/Makefile.am 22f51319a1c06da02cac5822d42315e3027cf500 
  src/cli/resolve.cpp 257e29034abf32491511f9a4e476b6859714829d 
  src/examples/test_contender_module.cpp PRE-CREATION 
  src/examples/test_detector_module.cpp PRE-CREATION 
  src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
  src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
  src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
  src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
  src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
  src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
  src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 
  src/master/master.cpp e1ca81dab85a7ab1391eca0d6bd995548bf79c22 
  src/module/manager.cpp 6ae99504005581b22a44768949b1d305cec517d9 
  src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
  src/scheduler/scheduler.cpp 99a7d0dfff7b0c61decc9ff6d9e6d46ef13a7e75 
  src/slave/slave.hpp a3830ff460a6f6c5661fb8a0172fae303b245889 
  src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
  src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
  src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
  src/tests/fault_tolerance_tests.cpp 982468f851cd9d95eb6cde7c57f2d737d46a827c 
  src/tests/master_allocator_tests.cpp cba7c36471f93b678d94e1da0251a28a893696b1 
  src/tests/master_authorization_tests.cpp 
29c89fb11da792c3e71eb880a19657ea225b3cc8 
  src/tests/master_contender_detector_tests.cpp 
6375586c31b1fd406529bf299dad6e321b945de8 
  src/tests/master_slave_reconciliation_tests.cpp 
d41178eb41df519073fc0890c5716bbc9fed6ad2 
  src/tests/master_tests.cpp 0357b1c259472213181a65e5adbe6d5caa1698ad 
  src/tests/module.hpp 4b32f29f2ce76100433621a5cb6b8cc87c9b38f8 
  src/tests/module.cpp 246f3a402d4fe3b273c459f6e02c009f3de65f3e 
  src/tests/oversubscription_tests.cpp c857c2bd5135d0e30edfe6f5e856fe6641b8dcfb 
  src/tests/partition_tests.cpp c5badbe90e302793bfbf3f16373efe241decb7d5 
  src/tests/persistent_volume_tests.cpp 
cbf2bcedd5b4c14107d3b50a74f161aa9395d7d0 
  src/tests/reconciliation_tests.cpp 1cbc3230d003a84277b91da6470828ebf73ef897 
  src/tests/reservation_tests.cpp d2ef15934556cb879f31850d52712aec77231fc7 
  src/tests/scheduler_event_call_tests.cpp 
bd8920fa9d5475e5f6533c8424ebff1588bfe645 
  src/tests/scheduler_http_api_tests.cpp 
9eb1de7d9541395b92b951f0fe0ddbb2f219fe30 
  src/tests/slave_recovery_tests.cpp bccdf37ced5de8e759c6abb91337e7bfecc77b77 
  src/tests/slave_tests.cpp b2b1fd4be933512c3dffa8c1c579b59782a37d77 

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


Testing
---

In addition to all unit tests passing, we are currently using this 
functionality in our environment with a custom consensus stack. In our world, 
we have a C++ plugin that calls out to an HTTP REST service (implemented in 
Java/Scala, not that it matters).


Thanks,

Mark Cavage

Review Request 43276: Fix compilation on Ubuntu 15.

[Joseph Wu]

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

Review request for mesos, Artem Harutyunyan, Michael Park, and Neil Conway.


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


Repository: mesos


Description
---

A few signed-unsigned comparisons introduced by 
https://reviews.apache.org/r/42751/


Diffs
-

  src/tests/resources_tests.cpp 3a338835757001d2987f80016a9029085cd7ff3b 

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


Testing
---

make check (Ubuntu15)


Thanks,

Joseph Wu

Re: Review Request 43258: Modified agent to get container status from containerizer.

[Avinash sridharan]

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

(Updated Feb. 6, 2016, 1:26 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Modified agent to get container status from containerizer.


Diffs (updated)
-

  src/slave/slave.hpp 632640293f14fb97aea6d7dd2c462c1f604638fb 
  src/slave/slave.cpp 1f4c8368feb0ce19963577582ce745acfb21aa9f 

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


Testing
---

make and make check


Thanks,

Avinash sridharan

Re: Review Request 43121: Updated Doxygen main page to use relative links.

[Ben Mahler]

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


Ship it!




Ship It!

- Ben Mahler


On Feb. 3, 2016, 12:15 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43121/
> ---
> 
> (Updated Feb. 3, 2016, 12:15 a.m.)
> 
> 
> Review request for mesos, Joseph Wu and Neil Conway.
> 
> 
> Bugs: MESOS-4584
> https://issues.apache.org/jira/browse/MESOS-4584
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, it was using absolute links which made it harder to test
> links locally.
> 
> 
> Diffs
> -
> 
>   src/main.dox d5b29cf96b18ba84ac39f1747c4a174a5bfbc2e1 
> 
> Diff: https://reviews.apache.org/r/43121/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 43220: Added script to generate docs from endpoint help strings.

[Ben Mahler]

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



Looking great, just some minor thoughts around ways to make this a bit easier 
on the reader of the code. Curious to hear your thoughts!


support/generate-endpoint-help.py (line 35)


typo: cause



support/generate-endpoint-help.py (line 36)


typo: we



support/generate-endpoint-help.py (lines 55 - 60)


Do we need the name indexing? It looks like we could just store the Popen 
objects in a list for cleanup purposes. Could start_master just return the 
Popen object instead of the index into this map?

Since we only create the master followed by the agent, we don't have both 
subprocesses alive at the same time, could we simplify if we assume we won't 
have more than a single active subprocess for now?



support/generate-endpoint-help.py (lines 76 - 92)


Would be nice to indent consistently here if possible, should we wrap the 
add_argument calls on a newline?



support/generate-endpoint-help.py (lines 77 - 79)


Should this be a 4 space indent?



support/generate-endpoint-help.py (lines 100 - 111)


Do we need these? They seem to exist only to rename os.makedirs and 
shutil.rmtree?

For the os.makedirs error handling, we could just do the following in the 
caller code to make the 'already exists' case explicit:

```
if not os.path.exists(directory):
os.makedirs(directory)
```



support/generate-endpoint-help.py (line 103)


Should this be a 2 space indent?



support/generate-endpoint-help.py (lines 114 - 127)


Could we avoid the string -> list conversion by just starting with the 
command in list form?

It looks like the caller code doesn't need to use a string.

After that change, this could take s/cmd/args/



support/generate-endpoint-help.py (lines 133 - 134)


Where does this happen? Looks like we don't delete the key from the map 
after the kill?



support/generate-endpoint-help.py (lines 186 - 188)


A maintenance endpoint might be a good example to include in general, since 
these endpoints have a slash in the name?



support/generate-endpoint-help.py (lines 242 - 243)


Could we prefix this sentence with a bold NOTE?



support/generate-endpoint-help.py (line 255)


Let's omit any reference to mesosphere ;)



support/generate-endpoint-help.py (line 264)


Ditto here ;)



support/generate-endpoint-help.py (lines 270 - 280)


Still a little tricky for me to figure this out, could the comment include 
an example?

One thought that comes to mind is, should this just return a 'list' of 
links for a given process, and the caller loops over processes calling this? 
Then the caller loops over the link list and stringifies as needed. Just 
thinking that perhaps this function should not be doing the overall string 
construction for the caller as that might be a bit tougher to follow.

Curious to see if we can make this easier for dummies like me :)



support/generate-endpoint-help.py (line 304)


relative_path


- Ben Mahler


On Feb. 5, 2016, 11:17 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43220/
> ---
> 
> (Updated Feb. 5, 2016, 11:17 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-3831
> https://issues.apache.org/jira/browse/MESOS-3831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Invoke this script from anywhere within the mesos source tree
> (including in the build directory) to autogenerate documentation for
> all endpoint strings and install them into the docs/endpoint folder.
> In a subsequent commit we commit all of these autogenerated files back
> to the source repo and add a link in home.md to find them.
> 
> 
> Diffs
> -
> 
>   support/generate-endpoint-help.py PRE-CREATION 
> 
> Diff: 

Re: Review Request 43258: Modified agent to get container status from containerizer.

[Guangya Liu]

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




src/slave/slave.cpp (lines 3228 - 3234)


I think that the current format is right, why update?

If you want to update, it should be the following, but I think there is no 
need to update here.
_statusUpdate(
None(), <
update,
pid,
executor->id,
executor->containerId,
executor->checkpoint);


- Guangya Liu


On 二月 6, 2016, 1:26 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43258/
> ---
> 
> (Updated 二月 6, 2016, 1:26 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4490
> https://issues.apache.org/jira/browse/MESOS-4490
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modified agent to get container status from containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 632640293f14fb97aea6d7dd2c462c1f604638fb 
>   src/slave/slave.cpp 1f4c8368feb0ce19963577582ce745acfb21aa9f 
> 
> Diff: https://reviews.apache.org/r/43258/diff/
> 
> 
> Testing
> ---
> 
> make and make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42751: Tweaked some resource test cases.

[Guangya Liu]

> On 二月 5, 2016, 2:33 a.m., Guangya Liu wrote:
> > src/tests/resources_tests.cpp, lines 327-328
> > 
> >
> > Just a question, do we have some guidelines for when to use `ASSERT_EQ` 
> > and when to use `EXPECT_EQ` ?
> 
> Michael Park wrote:
> We use `ASSERT_*` in cases where the violation of such condition 
> invalidates the rest of the test. For example, in this case, if 
> `cpus->type()` is not a `Value::SCALAR`, then we violate `cpus->scalar()`'s 
> preconditions and therefore is invalid. We use `EXPECT_*` if the condition is 
> one that we want to test and, if even it fails, it doesn't mean the rest of 
> the test is non-sensical.
> 
> Take for example:
> 
> ```
> std::vector x = f(0, 1, 2);
> 
> // `ASSERT_*` here since if we have less than 3 elements, `x[2]` is 
> invalid.
> ASSERT_GE(x.size(), 3);
> 
> // Each of these are independent test cases.
> // For example, even if x[0] != 0, it doesn't invalidate the rest of our 
> tests.
> EXPECT_EQ(0, x[0]);
> EXPECT_EQ(1, x[1]);
> EXPECT_EQ(2, x[2]);
> ```

Thanks Michael for the explanation, clear now.


- Guangya


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


On 二月 5, 2016, 2:19 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42751/
> ---
> 
> (Updated 二月 5, 2016, 2:19 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We should check that two reservations with the same role but different
> principals are considered distinct.
> 
> 
> Diffs
> -
> 
>   src/tests/resources_tests.cpp 4b25e82c13e4f46c73803f773db90f269c09c48a 
> 
> Diff: https://reviews.apache.org/r/42751/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43199: Updated authorization documentation.

[Greg Mann]

> On Feb. 4, 2016, 7:43 p.m., Neil Conway wrote:
> > docs/authorization.md, line 223
> > 
> >
> > s/framework/principal/ , I'd think.

Fixed this here, and elsewhere.


- Greg


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


On Feb. 6, 2016, 2:40 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43199/
> ---
> 
> (Updated Feb. 6, 2016, 2:40 a.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4452
> https://issues.apache.org/jira/browse/MESOS-4452
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated authorization documentation.
> 
> Added information about the distinction between roles and principals, as well 
> as a real-world authorization example.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md dbbfd60cb35cbb67e47b6a468d4f4ab824981e5d 
> 
> Diff: https://reviews.apache.org/r/43199/diff/
> 
> 
> Testing
> ---
> 
> Viewed in the mesos website container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 43200: Updated role documentation.

[Greg Mann]

> On Feb. 5, 2016, 7:07 a.m., Guangya Liu wrote:
> > docs/roles.md, line 21
> > 
> >
> > Maybe `Configure Mesos to provide guaranteed resource allocations for 
> > use by a role.` is better? You can refer to `home.md` to check the 
> > explanation for `quota.md`

I like using the word "guaranteed", but I don't think that saying "for use by a 
role" makes sense when we are actually trying to define what a "role" is. I 
changed the text to use "guaranteed"; let me know what you think!


- Greg


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


On Feb. 6, 2016, 2:43 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43200/
> ---
> 
> (Updated Feb. 6, 2016, 2:43 a.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4452
> https://issues.apache.org/jira/browse/MESOS-4452
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated role documentation.
> 
> Added information on the distinction between roles and principals.
> 
> 
> Diffs
> -
> 
>   docs/roles.md c84a483259922be01d1686befd01b7b1c4005bbd 
> 
> Diff: https://reviews.apache.org/r/43200/diff/
> 
> 
> Testing
> ---
> 
> Viewed in the mesos website container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 43200: Updated role documentation.

[Guangya Liu]

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




docs/roles.md (line 22)


I saw that we actually already have `role` reference in line 16 and line 19 
in this file.

So is it possible to also hightlihgt `role` in line 22? e.g. `a group of 
frameworks in one role`?


- Guangya Liu


On 二月 6, 2016, 2:43 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43200/
> ---
> 
> (Updated 二月 6, 2016, 2:43 a.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4452
> https://issues.apache.org/jira/browse/MESOS-4452
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated role documentation.
> 
> Added information on the distinction between roles and principals.
> 
> 
> Diffs
> -
> 
>   docs/roles.md c84a483259922be01d1686befd01b7b1c4005bbd 
> 
> Diff: https://reviews.apache.org/r/43200/diff/
> 
> 
> Testing
> ---
> 
> Viewed in the mesos website container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42618: Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 5, 2016, 10:24 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42618/
> ---
> 
> (Updated Feb. 5, 2016, 10:24 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> d8cebd7be97170c799016f3c60d91d2a4de9510d 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> f96c3274610269eacc9e91fec21a47a06bfeea62 
> 
> Diff: https://reviews.apache.org/r/42618/diff/
> 
> 
> Testing
> ---
> 
> make and make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43276: Fix compilation on Ubuntu 15.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Feb. 6, 2016, 12:34 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43276/
> ---
> 
> (Updated Feb. 6, 2016, 12:34 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-4479
> https://issues.apache.org/jira/browse/MESOS-4479
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A few signed-unsigned comparisons introduced by 
> https://reviews.apache.org/r/42751/
> 
> 
> Diffs
> -
> 
>   src/tests/resources_tests.cpp 3a338835757001d2987f80016a9029085cd7ff3b 
> 
> Diff: https://reviews.apache.org/r/43276/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu15)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 43199: Updated authorization documentation.

[Greg Mann]

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

(Updated Feb. 6, 2016, 2:40 a.m.)


Review request for mesos, Neil Conway and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Updated authorization documentation.

Added information about the distinction between roles and principals, as well 
as a real-world authorization example.


Diffs (updated)
-

  docs/authorization.md dbbfd60cb35cbb67e47b6a468d4f4ab824981e5d 

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


Testing
---

Viewed in the mesos website container: 
https://github.com/mesosphere/mesos-website-container


Thanks,

Greg Mann

Re: Review Request 43200: Updated role documentation.

[Greg Mann]

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

(Updated Feb. 6, 2016, 2:43 a.m.)


Review request for mesos, Neil Conway and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Updated role documentation.

Added information on the distinction between roles and principals.


Diffs (updated)
-

  docs/roles.md c84a483259922be01d1686befd01b7b1c4005bbd 

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


Testing
---

Viewed in the mesos website container: 
https://github.com/mesosphere/mesos-website-container


Thanks,

Greg Mann

Re: Review Request 43200: Updated role documentation.

[Guangya Liu]

> On 二月 5, 2016, 7:07 a.m., Guangya Liu wrote:
> > docs/roles.md, line 21
> > 
> >
> > Maybe `Configure Mesos to provide guaranteed resource allocations for 
> > use by a role.` is better? You can refer to `home.md` to check the 
> > explanation for `quota.md`
> 
> Greg Mann wrote:
> I like using the word "guaranteed", but I don't think that saying "for 
> use by a role" makes sense when we are actually trying to define what a 
> "role" is. I changed the text to use "guaranteed"; let me know what you think!

My only concern is that the description in `roles.md` and `home.md` will have 
different explanation for `Quota`, I think that we should keep them consistent.

What about update your latest document as "for a group of frameworks in one 
role"?


- Guangya


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


On 二月 6, 2016, 2:43 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43200/
> ---
> 
> (Updated 二月 6, 2016, 2:43 a.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4452
> https://issues.apache.org/jira/browse/MESOS-4452
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated role documentation.
> 
> Added information on the distinction between roles and principals.
> 
> 
> Diffs
> -
> 
>   docs/roles.md c84a483259922be01d1686befd01b7b1c4005bbd 
> 
> Diff: https://reviews.apache.org/r/43200/diff/
> 
> 
> Testing
> ---
> 
> Viewed in the mesos website container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 43072: Use full screen width for Mesos UI using Bootstraps (v3.3.6) container-fluid

[Michael Lunøe]

> On Feb. 2, 2016, 2:45 a.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [43072]
> > 
> > Failed command: ./support/apply-review.sh -n -r 43072
> > 
> > Error:
> > 2016-02-02 02:45:28 URL:https://reviews.apache.org/r/43072/diff/raw/ 
> > [226634/226634] -> "43072.patch" [1]
> > Traceback (most recent call last):
> >   File "support/apply-reviews.py", line 342, in 
> > reviewboard()
> >   File "support/apply-reviews.py", line 321, in reviewboard
> > apply_review()
> >   File "support/apply-reviews.py", line 139, in apply_review
> > commit_patch()
> >   File "support/apply-reviews.py", line 180, in commit_patch
> > data = patch_data()
> >   File "support/apply-reviews.py", line 199, in patch_data
> > return reviewboard_data()
> >   File "support/apply-reviews.py", line 252, in reviewboard_data
> > email=user.get('email'))
> > UnicodeEncodeError: 'ascii' codec can't encode character u'\xf8' in 
> > position 11: ordinal not in range(128)
> > 
> > Full log: https://builds.apache.org/job/mesos-reviewbot/11187/console
> 
> haosdent huang wrote:
> I think we could ignore this error message.
> 
> Bernd Mathiske wrote:
> I'd rather not ignore the error message. How can there be an out-of-range 
> char in HTML and CSS files?
> 
> haosdent huang wrote:
> I afraid this may caused by boostrap have some special characters like 
> `ok:before{content:"\e013"}`.
> 
> Bernd Mathiske wrote:
> This should be an encoding using printable cahrs (<=128) only.
> 
> haosdent huang wrote:
> LoL, seems `Michael Lunøe` cause the problem. Need fix the 
> apply_reviews.py
> 
> haosdent huang wrote:
> Need do changes in apply_reviews.py to support no-ascii commit messages.
> ```
> diff --git a/support/apply-reviews.py b/support/apply-reviews.py
> index ea5e43a..36bfb1e 100755
> --- a/support/apply-reviews.py
> +++ b/support/apply-reviews.py
> @@ -185,7 +185,7 @@ def commit_patch():
>else:
>  amend = '-e'
> 
> -  cmd = 'git commit --author \'{author}\' {_amend} -am \'{message}\''\
> +  cmd = u'git commit --author \'{author}\' {_amend} -am \'{message}\''\
>  .format(author=quote(data['author']),
>  _amend=amend,
>  message=quote(data['message']))
> @@ -248,7 +248,7 @@ def reviewboard_data():
>user = url_to_json(reviewboard_user_url(
>  review.get('links').get('submitter').get('title'))).get('user')
> 
> -  author = '{author} <{email}>'.format(author=user.get('fullname'),
> +  author = u'{author} <{email}>'.format(author=user.get('fullname'),
> email=user.get('email'))
>message = '\n\n'.join(['{summary}',
>'{description}',
> ```
> 
> Vinod Kone wrote:
> Thanks haosdent for the diff. I committed a patch based on this (and 
> attributed it to you).
> 
> Michael Lunøe wrote:
> Where do I see this patch, so I know when to re-open this?
> 
> Vinod Kone wrote:
> It's here 
> https://github.com/apache/mesos/commit/c93c5009cc0004388b8e248478a2291ff9d0c595.
> 
> Not sure what you mean by "re-open this"? Your patch is committed too.

Oh! Cool!


- Michael


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


On Feb. 1, 2016, 11:36 p.m., Michael Lunøe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43072/
> ---
> 
> (Updated Feb. 1, 2016, 11:36 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Thomas Rampelberg.
> 
> 
> Bugs: MESOS-2585
> https://issues.apache.org/jira/browse/MESOS-2585
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The solution to our problem is solved by updating the Bootstrap CSS file, as 
> container-fluid was introduced later in Bootstrap.
> 
> I have updated to lastest stable release of Bootstrap v3.3.6. Looking through 
> the change log, the only thing that we are using in the Mesos UI and changed 
> is the "hide" class (deprecated), so instances of these have been replaced by 
> its successor ".hidden" class.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 6b18056 
>   src/webui/master/static/css/bootstrap-3.0.3.min.css c547283 
>   src/webui/master/static/css/bootstrap-3.3.6.min.css PRE-CREATION 
>   src/webui/master/static/framework.html 9b28820 
>   src/webui/master/static/home.html d6cde1e 
>   src/webui/master/static/index.html 25caf53 
>   src/webui/master/static/slave.html bc46885 
>   src/webui/master/static/slave_executor.html 9d582d5 
>   src/webui/master/static/slave_framework.html 96d788f 
> 
> Diff: 

Re: Review Request 42157: Changed ProvisionerAppcTest to use AppcStoreTest suite.

[Jojy Varghese]

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

(Updated Feb. 6, 2016, 1:35 a.m.)


Review request for mesos and Jie Yu.


Changes
---

rebased.


Repository: mesos


Description
---

This change will enable ProvisionerAppcTest suite to reuse common code like
test image creation.


Diffs (updated)
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
012dba4e24b9a94dc8da0d329baf4bec2d33ffca 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 42841: WIP: Introducing appc image fetcher.

[Jojy Varghese]

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

(Updated Feb. 6, 2016, 1:36 a.m.)


Review request for Jie Yu.


Changes
---

rebased.


Repository: mesos


Description
---

WIP: Introducing appc image fetcher.


Diffs (updated)
-

  src/CMakeLists.txt 4a2954498efa48a4eb43f82827ff1d6f5f65d389 
  src/Makefile.am 22f51319a1c06da02cac5822d42315e3027cf500 
  src/slave/containerizer/mesos/provisioner/appc/fetcher.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp PRE-CREATION 
  src/slave/flags.hpp 23ec158f4a95fa76d657d9eb6a4a6fe30d57e5b6 
  src/slave/flags.cpp 24a23325cc255d0d7b7af7ed096b6d3012ad75c7 

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


Testing
---

make check; local image server


Thanks,

Jojy Varghese

Re: Review Request 43263: Updated site building instrucutions in site/README.md.

[Ben Mahler]

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



Thanks Kapil!


site/README.md (lines 27 - 31)


Does this need to be removed by the user at some point? Can this be 
permanently set? Tricky for me to figure out what this is for, would help to 
explain why this needs to be set in the comment here.



site/README.md (lines 52 - 58)


Would be great to include exact commands to run so that folks doing this 
don't have to think as hard, would that be do-able here?


- Ben Mahler


On Feb. 6, 2016, 12:13 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43263/
> ---
> 
> (Updated Feb. 6, 2016, 12:13 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated site building instrucutions in site/README.md.
> 
> 
> Diffs
> -
> 
>   site/README.md 71ca3ebbc645c6c188223895d12fb436b55038f9 
> 
> Diff: https://reviews.apache.org/r/43263/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 43263: Updated site building instrucutions in site/README.md.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On Feb. 5, 2016, 9:19 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43263/
> ---
> 
> (Updated Feb. 5, 2016, 9:19 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated site building instrucutions in site/README.md.
> 
> 
> Diffs
> -
> 
>   site/README.md 71ca3ebbc645c6c188223895d12fb436b55038f9 
> 
> Diff: https://reviews.apache.org/r/43263/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 42156: Refactored up the common image creation in appc tests.

[Jojy Varghese]

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

(Updated Feb. 6, 2016, 1:35 a.m.)


Review request for mesos and Jie Yu.


Changes
---

review addressed.


Repository: mesos


Description
---

This change will enable other tests to reuse the common test image creation
logic.


Diffs (updated)
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
012dba4e24b9a94dc8da0d329baf4bec2d33ffca 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 43122: Updated doxygen target for site/Rakefile.

[Ben Mahler]

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



Hm.. a bit tough to tell what this change is about, mind elaborating in the 
description?

- Ben Mahler


On Feb. 5, 2016, 9:19 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43122/
> ---
> 
> (Updated Feb. 5, 2016, 9:19 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Neil Conway.
> 
> 
> Bugs: MESOS-4584
> https://issues.apache.org/jira/browse/MESOS-4584
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary
> 
> 
> Diffs
> -
> 
>   site/Rakefile 0ce4b7975f95ab6930f0b2674191930df9ab5b20 
> 
> Diff: https://reviews.apache.org/r/43122/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 43276: Fix compilation on Ubuntu 15.

[Benjamin Bannier]

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


Ship it!




LGTM! Compiles and tests successfully under both gcc-5.3.0 (homebrew) and clang 
(today's trunk).

- Benjamin Bannier


On Feb. 6, 2016, 1:34 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43276/
> ---
> 
> (Updated Feb. 6, 2016, 1:34 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-4479
> https://issues.apache.org/jira/browse/MESOS-4479
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A few signed-unsigned comparisons introduced by 
> https://reviews.apache.org/r/42751/
> 
> 
> Diffs
> -
> 
>   src/tests/resources_tests.cpp 3a338835757001d2987f80016a9029085cd7ff3b 
> 
> Diff: https://reviews.apache.org/r/43276/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu15)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>