Re: Review Request 37275: [2/2]Generate make batch file to build project in windows.

2015-09-16 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [37273, 37275]

All tests passed.

- Mesos ReviewBot


On Sept. 16, 2015, 3:51 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37275/
> ---
> 
> (Updated Sept. 16, 2015, 3:51 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Generate make batch file to build project in windows.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
> 
> Diff: https://reviews.apache.org/r/37275/diff/
> 
> 
> Testing
> ---
> 
> # Build steps:
> cmake ..  -DREBUNDLED=FALSE
> make
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Timothy Chen

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

Ship it!


Ship It!

- Timothy Chen


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 38124: Add V1 Support for QuiesceOffers

2015-09-16 Thread Guangya Liu

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

(Updated 九月 16, 2015, 6:01 a.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
Kone.


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


Repository: mesos


Description
---

Add V1 Support for QuiesceOffers


Diffs (updated)
-

  include/mesos/v1/scheduler/scheduler.proto 
0118b46afca8a5adecb0e65981dd44bb98333bab 
  src/master/http.cpp f7ce9aa56b453c1d37b99dd836d956ab292ab62e 
  src/master/validation.cpp f97eba650f91e975859c2bbf0614ee5d39cf035e 
  src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 

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


Testing
---


Thanks,

Guangya Liu



Re: Review Request 37714: Updated Multimap and multihashmap so their signatures resemble that of hashmap and hashset.

2015-09-16 Thread Alexander Rojas

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

(Updated Sept. 16, 2015, 8:14 a.m.)


Review request for mesos, Joerg Schad, Michael Park, and Jan Schlicht.


Changes
---

Addresses Mpark and Till's comments.


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


Repository: mesos


Description
---

Adds extra template parameters to `multihashmap` which offer control over the 
hash function to use as well as the equality operator.

Implements initializer_list, copy and move constructors for both, 
`multihashmap` and `Multimap` in a similar way as it was done for `hashmap` and 
`hashset`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/multihashmap.hpp 
d9e4031cee64e48ad50541c04ca11e7861d0a17c 
  3rdparty/libprocess/3rdparty/stout/include/stout/multimap.hpp 
fb3e7a1d0377001389980302342813217f49cf5f 
  3rdparty/libprocess/3rdparty/stout/tests/multimap_tests.cpp 
535cd2d10e3074c86c149ce85b205e73ca42ddd3 

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


Testing
---

make check


Thanks,

Alexander Rojas



Re: Review Request 38011: Maintenance Primitives: Use the parse<RepeatedPtrField> helper instead of a plural MachineID protobuf.

2015-09-16 Thread Guangya Liu

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



src/common/protobuf_utils.hpp (line 115)


This should also be removed.


- Guangya Liu


On 九月 15, 2015, 5:47 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38011/
> ---
> 
> (Updated 九月 15, 2015, 5:47 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Hindman, Artem 
> Harutyunyan, Joris Van Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This removes the MachineIDs protobuf and changes it to instead use the 
> RepeatedFieldPtr.
> This also changes the maintenance primitives (alpha) API for /machine/up and 
> /machine/down, which both take an array instead of an object now.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   include/mesos/v1/mesos.proto bcce340777f037053e1eab2d75c351e53420b9da 
>   src/common/protobuf_utils.hpp 86474eac116306e50f276bc0539de9bca66133e0 
>   src/common/protobuf_utils.cpp 08612700c456017638a9978e5fe9cfa466294c46 
>   src/master/http.cpp f7ce9aa56b453c1d37b99dd836d956ab292ab62e 
>   src/master/maintenance.hpp 8d134aa44cac9003f6821bc418a22254869f2d6c 
>   src/master/maintenance.cpp 87308a659db05f0676bd02a56ff41fe9d953ba71 
>   src/tests/master_maintenance_tests.cpp 
> 44785057f129a3e6a69f399f7d6db59d9d5c2e91 
>   src/tests/registrar_tests.cpp aa49c86c94446f17f705039075050296b4af1955 
> 
> Diff: https://reviews.apache.org/r/38011/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 38124: Add V1 Support for QuiesceOffers

2015-09-16 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [37532, 37873, 38119, 38120, 38121, 38124]

All tests passed.

- Mesos ReviewBot


On Sept. 16, 2015, 6:01 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38124/
> ---
> 
> (Updated Sept. 16, 2015, 6:01 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add V1 Support for QuiesceOffers
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler/scheduler.proto 
> 0118b46afca8a5adecb0e65981dd44bb98333bab 
>   src/master/http.cpp f7ce9aa56b453c1d37b99dd836d956ab292ab62e 
>   src/master/validation.cpp f97eba650f91e975859c2bbf0614ee5d39cf035e 
>   src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 
> 
> Diff: https://reviews.apache.org/r/38124/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-09-16 Thread Isabel Jimenez

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

Ship it!


Ship It!

- Isabel Jimenez


On Sept. 16, 2015, 1:03 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 16, 2015, 1:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 38056: Fix webui task informations not update bug.

2015-09-16 Thread Philip Norman

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



src/webui/master/static/js/controllers.js (line 217)


These metrics are not available on master, only directly from the slave 
nodes. 

The webui redirects the user to master, so these metrics will never be 
available in the UI, so I can't see a situation where this would work.

It's not clear to me how this data could be gathered from master, and it 
would be impractical to poll slaves directly, even assuming that the CORS 
restrictions were fixed.


- Philip Norman


On Sept. 14, 2015, 4:42 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38056/
> ---
> 
> (Updated Sept. 14, 2015, 4:42 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-3282
> https://issues.apache.org/jira/browse/MESOS-3282
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix webui task informations not update bug.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/js/controllers.js 
> fbf86965efdb16f11787c24fb50d20af0837c62d 
> 
> Diff: https://reviews.apache.org/r/38056/diff/
> 
> 
> Testing
> ---
> 
> Manual test, test scenarios:
> 
> * Normal scenarios updated after the new task is finished
> * After mesos-master down, could updated after reconnected
> 
> ![task_infomartions.png](https://issues.apache.org/jira/secure/attachment/12753783/task_infomartions.png)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 38278: Updating unversioned Executor protobuf

2015-09-16 Thread Guangya Liu

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

Ship it!


Ship It!

- Guangya Liu


On 九月 15, 2015, 10:24 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38278/
> ---
> 
> (Updated 九月 15, 2015, 10:24 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adding last changes of the executor HTTP API design to the unversioned 
> protobuf
> 
> 
> Diffs
> -
> 
>   include/mesos/executor/executor.proto 52c84b3 
> 
> Diff: https://reviews.apache.org/r/38278/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>



Re: Review Request 38168: Replace hard-coded reap interval with a constant for 3rdparty

2015-09-16 Thread Alexander Rukletsov

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

Ship it!


Last minor nit: we tend to use past tense in summary and prepend the commit 
message with "libprocess" for libprocess changes. Also, not a trailing period. 
That's how it may be rewritten:
"libprocess: Replaced hard-coded reap interval with a constant."

Description is in present, but you don't need any in this case.

Last, but not least: though not that many people adhere to this style, it's 
sometimes benefitial to mention on what platforms did you run `make check` or 
`make distcheck`. Shouldn't make a difference for this patch though.

- Alexander Rukletsov


On Sept. 9, 2015, 2:59 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38168/
> ---
> 
> (Updated Sept. 9, 2015, 2:59 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-1935
> https://issues.apache.org/jira/browse/MESOS-1935
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replace hard-coded reap interval with a constant for 3rdparty
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/reap_tests.cpp 
> 642ab97b28a6085dc9837f14ff36a3124387a03b 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> ab7515325e5db0a4fd222bb982f51243d7b7e39d 
> 
> Diff: https://reviews.apache.org/r/38168/diff/
> 
> 
> Testing
> ---
> 
> make distcheck
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 38056: Fix webui task informations not update bug.

2015-09-16 Thread haosdent huang


> On Sept. 16, 2015, 10:41 a.m., Philip Norman wrote:
> > src/webui/master/static/js/controllers.js, line 224
> > 
> >
> > These metrics are not available on master, only directly from the slave 
> > nodes. 
> > 
> > The webui redirects the user to master, so these metrics will never be 
> > available in the UI, so I can't see a situation where this would work.
> > 
> > It's not clear to me how this data could be gathered from master, and 
> > it would be impractical to poll slaves directly, even assuming that the 
> > CORS restrictions were fixed.

Thank you very much for your review @philipnrmn . Yes, I need change 
s/slave/master. Because I use mesos-local when I debug this, it would start the 
master and slave at same network port. Let me update it.


- haosdent


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


On Sept. 14, 2015, 4:42 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38056/
> ---
> 
> (Updated Sept. 14, 2015, 4:42 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-3282
> https://issues.apache.org/jira/browse/MESOS-3282
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix webui task informations not update bug.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/js/controllers.js 
> fbf86965efdb16f11787c24fb50d20af0837c62d 
> 
> Diff: https://reviews.apache.org/r/38056/diff/
> 
> 
> Testing
> ---
> 
> Manual test, test scenarios:
> 
> * Normal scenarios updated after the new task is finished
> * After mesos-master down, could updated after reconnected
> 
> ![task_infomartions.png](https://issues.apache.org/jira/secure/attachment/12753783/task_infomartions.png)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 38056: Fix webui task informations not update bug.

2015-09-16 Thread haosdent huang

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

(Updated Sept. 16, 2015, 3:03 p.m.)


Review request for mesos, Adam B, Joerg Schad, Michael Park, and Philip Norman.


Changes
---

Update according @Philip Norman reviews, and test it under cluster mode.


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


Repository: mesos


Description
---

Fix webui task informations not update bug.


Diffs (updated)
-

  src/webui/master/static/js/controllers.js 
c4340ab94812433a2eadb1e5183b0765330cb844 

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


Testing
---

Manual test, test scenarios:

* Normal scenarios updated after the new task is finished
* After mesos-master down, could updated after reconnected

![task_infomartions.png](https://issues.apache.org/jira/secure/attachment/12753783/task_infomartions.png)


Thanks,

haosdent huang



Re: Review Request 38056: Fix webui task informations not update bug.

2015-09-16 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [38056]

All tests passed.

- Mesos ReviewBot


On Sept. 16, 2015, 3:03 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38056/
> ---
> 
> (Updated Sept. 16, 2015, 3:03 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, Michael Park, and Philip 
> Norman.
> 
> 
> Bugs: MESOS-3282
> https://issues.apache.org/jira/browse/MESOS-3282
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix webui task informations not update bug.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/js/controllers.js 
> c4340ab94812433a2eadb1e5183b0765330cb844 
> 
> Diff: https://reviews.apache.org/r/38056/diff/
> 
> 
> Testing
> ---
> 
> Manual test, test scenarios:
> 
> * Normal scenarios updated after the new task is finished
> * After mesos-master down, could updated after reconnected
> 
> ![task_infomartions.png](https://issues.apache.org/jira/secure/attachment/12753783/task_infomartions.png)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 38168: libprocess: Replaced hard-coded reap interval with a constant.

2015-09-16 Thread Guangya Liu


> On 九月 16, 2015, 3:09 p.m., Alexander Rukletsov wrote:
> > Last minor nit: we tend to use past tense in summary and prepend the commit 
> > message with "libprocess" for libprocess changes. Also, not a trailing 
> > period. That's how it may be rewritten:
> > "libprocess: Replaced hard-coded reap interval with a constant."
> > 
> > Description is in present, but you don't need any in this case.
> > 
> > Last, but not least: though not that many people adhere to this style, it's 
> > sometimes benefitial to mention on what platforms did you run `make check` 
> > or `make distcheck`. Shouldn't make a difference for this patch though.

Thanks Alex, updated.


- Guangya


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


On 九月 16, 2015, 4:07 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38168/
> ---
> 
> (Updated 九月 16, 2015, 4:07 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-1935
> https://issues.apache.org/jira/browse/MESOS-1935
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> libprocess: Replaced hard-coded reap interval with a constant.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/reap_tests.cpp 
> 642ab97b28a6085dc9837f14ff36a3124387a03b 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> ab7515325e5db0a4fd222bb982f51243d7b7e39d 
> 
> Diff: https://reviews.apache.org/r/38168/diff/
> 
> 
> Testing
> ---
> 
> OS: Ubuntu 14.04
> make distcheck
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 38046: mesos: Replaced hard-coded reap interval with a constant.

2015-09-16 Thread Guangya Liu


> On 九月 16, 2015, 3:12 p.m., Alexander Rukletsov wrote:
> > Looks good! Similar to my comment on https://reviews.apache.org/r/38168/, 
> > please adjust the summary to something like "mesos: Replaced hard-coded 
> > reap interval with a constant."

Thanks Alex, updated.


- Guangya


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


On 九月 16, 2015, 4:07 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38046/
> ---
> 
> (Updated 九月 16, 2015, 4:07 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-1935
> https://issues.apache.org/jira/browse/MESOS-1935
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> mesos: Replace hard-coded reap interval with a constant.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/launch_tests.cpp 
> d211fc0f665988068c67836ef80916828a0df2bd 
>   src/tests/slave_recovery_tests.cpp 6aae14a3c39b9aee76147b691b0170946e1120b5 
>   src/tests/slave_tests.cpp 5c1a3d36a5f67629aef275eeae12956c524e8102 
> 
> Diff: https://reviews.apache.org/r/38046/diff/
> 
> 
> Testing
> ---
> 
> Platform: Ubuntu 14.04
> make distcheck
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 38168: libprocess: Replaced hard-coded reap interval with a constant.

2015-09-16 Thread Guangya Liu

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

(Updated 九月 16, 2015, 4:07 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
---

Address Alex's comments.


Summary (updated)
-

libprocess: Replaced hard-coded reap interval with a constant.


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


Repository: mesos


Description (updated)
---

libprocess: Replaced hard-coded reap interval with a constant.


Diffs
-

  3rdparty/libprocess/src/tests/reap_tests.cpp 
642ab97b28a6085dc9837f14ff36a3124387a03b 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
ab7515325e5db0a4fd222bb982f51243d7b7e39d 

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


Testing (updated)
---

OS: Ubuntu 14.04
make distcheck


Thanks,

Guangya Liu



Re: Review Request 38046: mesos: Replaced hard-coded reap interval with a constant.

2015-09-16 Thread Guangya Liu

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

(Updated 九月 16, 2015, 4:07 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
---

Address Alex's comments.


Summary (updated)
-

mesos: Replaced hard-coded reap interval with a constant.


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


Repository: mesos


Description (updated)
---

mesos: Replace hard-coded reap interval with a constant.


Diffs
-

  src/tests/containerizer/launch_tests.cpp 
d211fc0f665988068c67836ef80916828a0df2bd 
  src/tests/slave_recovery_tests.cpp 6aae14a3c39b9aee76147b691b0170946e1120b5 
  src/tests/slave_tests.cpp 5c1a3d36a5f67629aef275eeae12956c524e8102 

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


Testing (updated)
---

Platform: Ubuntu 14.04
make distcheck


Thanks,

Guangya Liu



Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Timothy Chen


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners
> 
> Jie Yu wrote:
> OK, i got confused a little bit. WE only need to specify a single image 
> provisioner.
> 
> `--image_provisioner=bind`

sounds good to me.


- Timothy


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Vinod Kone


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners
> 
> Jie Yu wrote:
> OK, i got confused a little bit. WE only need to specify a single image 
> provisioner.
> 
> `--image_provisioner=bind`
> 
> Timothy Chen wrote:
> sounds good to me.
> 
> Jiang Yan Xu wrote:
> About naming, I would prefer leaving the word "backend" in the second 
> flag: `--image_provisioner_backend`, just because the code is written this 
> way. The code has a fontend called `Provisioner` and the backend called 
> `Backend`. Having the flag seemingly refer to the previous when it's in fact 
> referring to the latter is very confusing.
> 
> I actually like the frontend being called "Provider" and the backend 
> called "Provisioner" better but to do the change more thoroughly we should be 
> renaming current `Provisioner` to `Provider` and the `Backend` to 
> `Provisioner`, which is a larger undertaking.
> 
> Timothy Chen wrote:
> this is probably the best patch to change all the names if we want to. 
> Backend in comparison with Provisioner, definitely I feel like Provisioner is 
> more natural.

+1 for changing backend to provisioner and provisioner to provider. my original 
inspiration for the suggestion was vagrant.

https://docs.vagrantup.com/v2/provisioning/index.html
https://docs.vagrantup.com/v2/providers/index.html


- Vinod


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jiang Yan Xu


> On Sept. 16, 2015, 10:54 a.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners
> 
> Jie Yu wrote:
> OK, i got confused a little bit. WE only need to specify a single image 
> provisioner.
> 
> `--image_provisioner=bind`
> 
> Timothy Chen wrote:
> sounds good to me.
> 
> Jiang Yan Xu wrote:
> About naming, I would prefer leaving the word "backend" in the second 
> flag: `--image_provisioner_backend`, just because the code is written this 
> way. The code has a fontend called `Provisioner` and the backend called 
> `Backend`. Having the flag seemingly refer to the previous when it's in fact 
> referring to the latter is very confusing.
> 
> I actually like the frontend being called "Provider" and the backend 
> called "Provisioner" better but to do the change more thoroughly we should be 
> renaming current `Provisioner` to `Provider` and the `Backend` to 
> `Provisioner`, which is a larger undertaking.
> 
> Timothy Chen wrote:
> this is probably the best patch to change all the names if we want to. 
> Backend in comparison with Provisioner, definitely I feel like Provisioner is 
> more natural.
> 
> Vinod Kone wrote:
> +1 for changing backend to provisioner and provisioner to provider. my 
> original inspiration for the suggestion was vagrant.
> 
> https://docs.vagrantup.com/v2/provisioning/index.html
> https://docs.vagrantup.com/v2/providers/index.html
> 
> Jie Yu wrote:
> I think `Store` could be renamed to `Provider` because that's the one 
> that 'provides' image layers.
> 
> I would still keeps the frontend being called `Provisioner` because it's 
> the one that take layers from the `Provider` and constructs (provisions) a 
> root filesystem.
> 
> Jie Yu wrote:
> OK, I don't think we should spend too much time on this naming. How about 
> just keep the class name unchanged and use `--image_provisioner_backend` 
> instead:
> 
> ```
> --image_providers=APPC,DOCKER
> --image_provisioner_backend=bind
> ```
> 
> Jie Yu wrote:
> We pretty much need to use 'auto' for --image_provisioner_backend in the 
> future. If only one layer is returned, user 'bind' backend. Otherwise, use 
> overlayfs. If overlayfs is not available, use copy.

+1 for the above flags for now. It's probably too large of an undertaking to be 
justified.


- Jiang Yan


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


On Sept. 15, 2015, 6:28 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 15, 2015, 6:28 p.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 

Re: Review Request 38031: [3/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-16 Thread Ben Mahler

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


Have you surveyed how other libraires handle this? For example, the scala play 
framework seems to use BigDecimal (128 bit floating point) for all numbers, 
which can represent all 64 bit signed / unsigned integral values without loss. 
Have you considered this as well?

It would be great to outline the options we have here in the ticket so we can 
decide on the tradeoffs. If you've already done this, can you point me to it? :)

- Ben Mahler


On Sept. 15, 2015, 5:24 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38031/
> ---
> 
> (Updated Sept. 15, 2015, 5:24 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Michael Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-3345
> https://issues.apache.org/jira/browse/MESOS-3345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> * Changes JSON::Number to keep track of whether it is floating, signed 
> integral, or unsigned integral.
> * Changes how JSON::Number is stringified, to ensure that stringified doubles 
> are parsed as JSON doubles.
> * Added one test for integer precision between String <-> JSON <-> Protobuf 
> conversions.
> * Added one test for JSON::Number comparisons.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 
> f28138c4682c41e94ab6c7641a78d66b2f9daa5f 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 57d5fdf45273c620655b44b5f5572290cffa4bf6 
>   3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
> 850650c269e9be24c0f1ae81b8aa8725f8a0c151 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
> c56d6a3098293eb3659b3066f10e875927ec3ac3 
> 
> Diff: https://reviews.apache.org/r/38031/diff/
> 
> 
> Testing
> ---
> 
> No testing done until the last patch in the chain.
> 
> However, this patch does breaks some libprocess and mesos tests (because 
> JSON::Number is stored differently), which are fixed in the following two 
> reviews.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 38031: [3/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-16 Thread Ben Mahler


> On Sept. 16, 2015, 6:01 p.m., Ben Mahler wrote:
> > Have you surveyed how other libraires handle this? For example, the scala 
> > play framework seems to use BigDecimal (128 bit floating point) for all 
> > numbers, which can represent all 64 bit signed / unsigned integral values 
> > without loss. Have you considered this as well?
> > 
> > It would be great to outline the options we have here in the ticket so we 
> > can decide on the tradeoffs. If you've already done this, can you point me 
> > to it? :)
> 
> Joris Van Remoortere wrote:
> Hi Ben! THanks for bringing this up.
> 
> When you say "handle this", what do you mean exactly? The issue that 
> Joseph is dealing with is the round-trip loss when we transit through 
> pico-json. The modification to Number are just to maintain type information 
> when possible.
> 
> I believe we would have to choose another parsing library if we wanted to 
> use 128 bit floats, or am I missing something? I'm definitely open to 
> discussing alternative libraries; but I would like to patch this up in the 
> mean time so that we don't need to rush the larger discussion. What are your 
> thoughts?

Improving over the current state sounds good, just trying to gauge if this is 
how we want this to look long term: Looking at the diff, it seems like we still 
are not able to represent every unsigned 64 bit integer? I'm a bit worried 
about that, we can get away with it for now, but given we may use uint64s for 
representing identifiers, this needs to be non-lossy longer term, no?


- Ben


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


On Sept. 15, 2015, 5:24 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38031/
> ---
> 
> (Updated Sept. 15, 2015, 5:24 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Michael Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-3345
> https://issues.apache.org/jira/browse/MESOS-3345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> * Changes JSON::Number to keep track of whether it is floating, signed 
> integral, or unsigned integral.
> * Changes how JSON::Number is stringified, to ensure that stringified doubles 
> are parsed as JSON doubles.
> * Added one test for integer precision between String <-> JSON <-> Protobuf 
> conversions.
> * Added one test for JSON::Number comparisons.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 
> f28138c4682c41e94ab6c7641a78d66b2f9daa5f 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 57d5fdf45273c620655b44b5f5572290cffa4bf6 
>   3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
> 850650c269e9be24c0f1ae81b8aa8725f8a0c151 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
> c56d6a3098293eb3659b3066f10e875927ec3ac3 
> 
> Diff: https://reviews.apache.org/r/38031/diff/
> 
> 
> Testing
> ---
> 
> No testing done until the last patch in the chain.
> 
> However, this patch does breaks some libprocess and mesos tests (because 
> JSON::Number is stored differently), which are fixed in the following two 
> reviews.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 38011: Maintenance Primitives: Use the parse<RepeatedPtrField> helper instead of a plural MachineID protobuf.

2015-09-16 Thread Joseph Wu

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

(Updated Sept. 16, 2015, 12:38 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Hindman, Artem 
Harutyunyan, Joris Van Remoortere, and Michael Park.


Changes
---

Remove stale todo.


Repository: mesos


Description
---

This removes the MachineIDs protobuf and changes it to instead use the 
RepeatedFieldPtr.
This also changes the maintenance primitives (alpha) API for /machine/up and 
/machine/down, which both take an array instead of an object now.


Diffs (updated)
-

  include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
  include/mesos/v1/mesos.proto bcce340777f037053e1eab2d75c351e53420b9da 
  src/common/protobuf_utils.hpp 86474eac116306e50f276bc0539de9bca66133e0 
  src/common/protobuf_utils.cpp 08612700c456017638a9978e5fe9cfa466294c46 
  src/master/http.cpp f7ce9aa56b453c1d37b99dd836d956ab292ab62e 
  src/master/maintenance.hpp 8d134aa44cac9003f6821bc418a22254869f2d6c 
  src/master/maintenance.cpp 87308a659db05f0676bd02a56ff41fe9d953ba71 
  src/tests/master_maintenance_tests.cpp 
44785057f129a3e6a69f399f7d6db59d9d5c2e91 
  src/tests/registrar_tests.cpp aa49c86c94446f17f705039075050296b4af1955 

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


Testing
---

`make check`


Thanks,

Joseph Wu



Re: Review Request 38363: Refactored container Launcher to accept namespaces dynamically.

2015-09-16 Thread Niklas Nielsen

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

Ship it!


Ship It!

- Niklas Nielsen


On Sept. 14, 2015, 1:55 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38363/
> ---
> 
> (Updated Sept. 14, 2015, 1:55 p.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3387
> https://issues.apache.org/jira/browse/MESOS-3387
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This prepares MesosContainerizer to allow isolators to specify namespaces for
> each individual containerizer. Thus, allowing isolators to decide whether or 
> not
> to enable isolation for a particular container.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/launcher.hpp 
> 8cc832e99838683c14d8158bdf52a267299508c7 
>   src/slave/containerizer/launcher.cpp 
> 5267eecbdf31c88cd3c7dc7172b5aff7bac534b0 
>   src/slave/containerizer/linux_launcher.hpp 
> bf6bf3f27dac007429c224d4f6798ac68316accb 
>   src/slave/containerizer/linux_launcher.cpp 
> 12acf4d08cb7fb1161820cf4bb480fd1bc2c6858 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/isolator_tests.cpp 
> d34c82a7f24da6f60cfb22790f516dc6065b1f6f 
>   src/tests/containerizer/launcher.hpp 
> 6f020f94b042eef0e6a69ba8c26dfb697f3e81a3 
> 
> Diff: https://reviews.apache.org/r/38363/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Vinod Kone


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```

one singular one plural? can we name the latter --image_provisioners?


- Vinod


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 38031: [3/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-16 Thread Joris Van Remoortere


> On Sept. 16, 2015, 6:01 p.m., Ben Mahler wrote:
> > Have you surveyed how other libraires handle this? For example, the scala 
> > play framework seems to use BigDecimal (128 bit floating point) for all 
> > numbers, which can represent all 64 bit signed / unsigned integral values 
> > without loss. Have you considered this as well?
> > 
> > It would be great to outline the options we have here in the ticket so we 
> > can decide on the tradeoffs. If you've already done this, can you point me 
> > to it? :)

Hi Ben! THanks for bringing this up.

When you say "handle this", what do you mean exactly? The issue that Joseph is 
dealing with is the round-trip loss when we transit through pico-json. The 
modification to Number are just to maintain type information when possible.

I believe we would have to choose another parsing library if we wanted to use 
128 bit floats, or am I missing something? I'm definitely open to discussing 
alternative libraries; but I would like to patch this up in the mean time so 
that we don't need to rush the larger discussion. What are your thoughts?


- Joris


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


On Sept. 15, 2015, 5:24 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38031/
> ---
> 
> (Updated Sept. 15, 2015, 5:24 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Michael Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-3345
> https://issues.apache.org/jira/browse/MESOS-3345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> * Changes JSON::Number to keep track of whether it is floating, signed 
> integral, or unsigned integral.
> * Changes how JSON::Number is stringified, to ensure that stringified doubles 
> are parsed as JSON doubles.
> * Added one test for integer precision between String <-> JSON <-> Protobuf 
> conversions.
> * Added one test for JSON::Number comparisons.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 
> f28138c4682c41e94ab6c7641a78d66b2f9daa5f 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 57d5fdf45273c620655b44b5f5572290cffa4bf6 
>   3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
> 850650c269e9be24c0f1ae81b8aa8725f8a0c151 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
> c56d6a3098293eb3659b3066f10e875927ec3ac3 
> 
> Diff: https://reviews.apache.org/r/38031/diff/
> 
> 
> Testing
> ---
> 
> No testing done until the last patch in the chain.
> 
> However, this patch does breaks some libprocess and mesos tests (because 
> JSON::Number is stored differently), which are fixed in the following two 
> reviews.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Timothy Chen


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners
> 
> Jie Yu wrote:
> OK, i got confused a little bit. WE only need to specify a single image 
> provisioner.
> 
> `--image_provisioner=bind`
> 
> Timothy Chen wrote:
> sounds good to me.
> 
> Jiang Yan Xu wrote:
> About naming, I would prefer leaving the word "backend" in the second 
> flag: `--image_provisioner_backend`, just because the code is written this 
> way. The code has a fontend called `Provisioner` and the backend called 
> `Backend`. Having the flag seemingly refer to the previous when it's in fact 
> referring to the latter is very confusing.
> 
> I actually like the frontend being called "Provider" and the backend 
> called "Provisioner" better but to do the change more thoroughly we should be 
> renaming current `Provisioner` to `Provider` and the `Backend` to 
> `Provisioner`, which is a larger undertaking.
> 
> Timothy Chen wrote:
> this is probably the best patch to change all the names if we want to. 
> Backend in comparison with Provisioner, definitely I feel like Provisioner is 
> more natural.
> 
> Vinod Kone wrote:
> +1 for changing backend to provisioner and provisioner to provider. my 
> original inspiration for the suggestion was vagrant.
> 
> https://docs.vagrantup.com/v2/provisioning/index.html
> https://docs.vagrantup.com/v2/providers/index.html
> 
> Jie Yu wrote:
> I think `Store` could be renamed to `Provider` because that's the one 
> that 'provides' image layers.
> 
> I would still keeps the frontend being called `Provisioner` because it's 
> the one that take layers from the `Provider` and constructs (provisions) a 
> root filesystem.
> 
> Jie Yu wrote:
> OK, I don't think we should spend too much time on this naming. How about 
> just keep the class name unchanged and use `--image_provisioner_backend` 
> instead:
> 
> ```
> --image_providers=APPC,DOCKER
> --image_provisioner_backend=bind
> ```
> 
> Jie Yu wrote:
> We pretty much need to use 'auto' for --image_provisioner_backend in the 
> future. If only one layer is returned, user 'bind' backend. Otherwise, use 
> overlayfs. If overlayfs is not available, use copy.
> 
> Jiang Yan Xu wrote:
> +1 for the above flags for now. It's probably too large of an undertaking 
> to be justified.

I think we need to keep the flag even when auto is available, as it's obvious 
from previous conversations with users they need to be able to choose.
But yes we can always rename later.


- Timothy


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   

Re: Review Request 38031: [3/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-16 Thread Joseph Wu


> On Sept. 16, 2015, 11:01 a.m., Ben Mahler wrote:
> > Have you surveyed how other libraires handle this? For example, the scala 
> > play framework seems to use BigDecimal (128 bit floating point) for all 
> > numbers, which can represent all 64 bit signed / unsigned integral values 
> > without loss. Have you considered this as well?
> > 
> > It would be great to outline the options we have here in the ticket so we 
> > can decide on the tradeoffs. If you've already done this, can you point me 
> > to it? :)
> 
> Joris Van Remoortere wrote:
> Hi Ben! THanks for bringing this up.
> 
> When you say "handle this", what do you mean exactly? The issue that 
> Joseph is dealing with is the round-trip loss when we transit through 
> pico-json. The modification to Number are just to maintain type information 
> when possible.
> 
> I believe we would have to choose another parsing library if we wanted to 
> use 128 bit floats, or am I missing something? I'm definitely open to 
> discussing alternative libraries; but I would like to patch this up in the 
> mean time so that we don't need to rush the larger discussion. What are your 
> thoughts?
> 
> Ben Mahler wrote:
> Improving over the current state sounds good, just trying to gauge if 
> this is how we want this to look long term: Looking at the diff, it seems 
> like we still are not able to represent every unsigned 64 bit integer? I'm a 
> bit worried about that, we can get away with it for now, but given we may use 
> uint64s for representing identifiers, this needs to be non-lossy longer term, 
> no?

Ben, are you seeing a problem with large uint64's in this diff?  If so, I'd 
like to fix it.

As Joris (and I, in the code) mentioned, the loss that occurs between INT64_MAX 
and UINT64_MAX is due to a limitation within PicoJson.  We would need to 
change/patch the json library to fix this.  (If this is the case, I'd recommend 
moving this dicussion to the JIRA.)


- Joseph


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


On Sept. 15, 2015, 10:24 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38031/
> ---
> 
> (Updated Sept. 15, 2015, 10:24 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Michael Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-3345
> https://issues.apache.org/jira/browse/MESOS-3345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> * Changes JSON::Number to keep track of whether it is floating, signed 
> integral, or unsigned integral.
> * Changes how JSON::Number is stringified, to ensure that stringified doubles 
> are parsed as JSON doubles.
> * Added one test for integer precision between String <-> JSON <-> Protobuf 
> conversions.
> * Added one test for JSON::Number comparisons.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 
> f28138c4682c41e94ab6c7641a78d66b2f9daa5f 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 57d5fdf45273c620655b44b5f5572290cffa4bf6 
>   3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
> 850650c269e9be24c0f1ae81b8aa8725f8a0c151 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
> c56d6a3098293eb3659b3066f10e875927ec3ac3 
> 
> Diff: https://reviews.apache.org/r/38031/diff/
> 
> 
> Testing
> ---
> 
> No testing done until the last patch in the chain.
> 
> However, this patch does breaks some libprocess and mesos tests (because 
> JSON::Number is stored differently), which are fixed in the following two 
> reviews.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Timothy Chen


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners
> 
> Jie Yu wrote:
> OK, i got confused a little bit. WE only need to specify a single image 
> provisioner.
> 
> `--image_provisioner=bind`
> 
> Timothy Chen wrote:
> sounds good to me.
> 
> Jiang Yan Xu wrote:
> About naming, I would prefer leaving the word "backend" in the second 
> flag: `--image_provisioner_backend`, just because the code is written this 
> way. The code has a fontend called `Provisioner` and the backend called 
> `Backend`. Having the flag seemingly refer to the previous when it's in fact 
> referring to the latter is very confusing.
> 
> I actually like the frontend being called "Provider" and the backend 
> called "Provisioner" better but to do the change more thoroughly we should be 
> renaming current `Provisioner` to `Provider` and the `Backend` to 
> `Provisioner`, which is a larger undertaking.

this is probably the best patch to change all the names if we want to. Backend 
in comparison with Provisioner, definitely I feel like Provisioner is more 
natural.


- Timothy


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 38370: Updated docker executor to set container IP in TaskStatus::NetworkInfo.

2015-09-16 Thread Niklas Nielsen

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



src/docker/executor.cpp (lines 162 - 164)


Was "Docker.NetworkSettings.IPAddress" in 0.24.0? If so, don't we want to 
deprecate this over a release cycle? (Supporting both in the interim)


- Niklas Nielsen


On Sept. 15, 2015, 2:11 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38370/
> ---
> 
> (Updated Sept. 15, 2015, 2:11 p.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docker executor to set container IP in TaskStatus::NetworkInfo.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a628922cc49b1dc8a37621a6a8b7ef6842cfb051 
> 
> Diff: https://reviews.apache.org/r/38370/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 38031: [3/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-16 Thread Ben Mahler


> On Sept. 16, 2015, 6:01 p.m., Ben Mahler wrote:
> > Have you surveyed how other libraires handle this? For example, the scala 
> > play framework seems to use BigDecimal (128 bit floating point) for all 
> > numbers, which can represent all 64 bit signed / unsigned integral values 
> > without loss. Have you considered this as well?
> > 
> > It would be great to outline the options we have here in the ticket so we 
> > can decide on the tradeoffs. If you've already done this, can you point me 
> > to it? :)
> 
> Joris Van Remoortere wrote:
> Hi Ben! THanks for bringing this up.
> 
> When you say "handle this", what do you mean exactly? The issue that 
> Joseph is dealing with is the round-trip loss when we transit through 
> pico-json. The modification to Number are just to maintain type information 
> when possible.
> 
> I believe we would have to choose another parsing library if we wanted to 
> use 128 bit floats, or am I missing something? I'm definitely open to 
> discussing alternative libraries; but I would like to patch this up in the 
> mean time so that we don't need to rush the larger discussion. What are your 
> thoughts?
> 
> Ben Mahler wrote:
> Improving over the current state sounds good, just trying to gauge if 
> this is how we want this to look long term: Looking at the diff, it seems 
> like we still are not able to represent every unsigned 64 bit integer? I'm a 
> bit worried about that, we can get away with it for now, but given we may use 
> uint64s for representing identifiers, this needs to be non-lossy longer term, 
> no?
> 
> Joseph Wu wrote:
> Ben, are you seeing a problem with large uint64's in this diff?  If so, 
> I'd like to fix it.
> 
> As Joris (and I, in the code) mentioned, the loss that occurs between 
> INT64_MAX and UINT64_MAX is due to a limitation within PicoJson.  We would 
> need to change/patch the json library to fix this.  (If this is the case, I'd 
> recommend moving this dicussion to the JIRA.)

> the loss that occurs between INT64_MAX and UINT64_MAX is due to a limitation 
> within PicoJson.

This is what I was referring to when I mentioned not being able to represent 
every uint64.


- Ben


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


On Sept. 15, 2015, 5:24 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38031/
> ---
> 
> (Updated Sept. 15, 2015, 5:24 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Michael Park, and Vinod Kone.
> 
> 
> Bugs: MESOS-3345
> https://issues.apache.org/jira/browse/MESOS-3345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> * Changes JSON::Number to keep track of whether it is floating, signed 
> integral, or unsigned integral.
> * Changes how JSON::Number is stringified, to ensure that stringified doubles 
> are parsed as JSON doubles.
> * Added one test for integer precision between String <-> JSON <-> Protobuf 
> conversions.
> * Added one test for JSON::Number comparisons.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 
> f28138c4682c41e94ab6c7641a78d66b2f9daa5f 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 57d5fdf45273c620655b44b5f5572290cffa4bf6 
>   3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
> 850650c269e9be24c0f1ae81b8aa8725f8a0c151 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
> c56d6a3098293eb3659b3066f10e875927ec3ac3 
> 
> Diff: https://reviews.apache.org/r/38031/diff/
> 
> 
> Testing
> ---
> 
> No testing done until the last patch in the chain.
> 
> However, this patch does breaks some libprocess and mesos tests (because 
> JSON::Number is stored differently), which are fixed in the following two 
> reviews.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 37505: Fix broken health check in docker executor.

2015-09-16 Thread Timothy Chen

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


By the way your patch fails mesos style check. Did you run the mesos style 
python script?

- Timothy Chen


On Sept. 14, 2015, 12:59 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37505/
> ---
> 
> (Updated Sept. 14, 2015, 12:59 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3136
> https://issues.apache.org/jira/browse/MESOS-3136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix broken health check in docker executor.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp aaf8884a20901ad3a440d545d4b6316b87fc3403 
>   src/docker/docker.cpp 553e831029454d6d423842915b8bbfcaf19fa7f9 
>   src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e 
>   src/slave/containerizer/docker.cpp 289d4ec0fba9071dfe0cbf5391b5391d4566dd9c 
>   src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea 
> 
> Diff: https://reviews.apache.org/r/37505/diff/
> 
> 
> Testing
> ---
> 
> # Add two new test cases, HealthCheckTest.ROOT_DOCKER_DockerHealthyTask and 
> HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange
> # Docker health check command is run through "docker exec"
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthyTask" --verbose
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange" 
> --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners
> 
> Jie Yu wrote:
> OK, i got confused a little bit. WE only need to specify a single image 
> provisioner.
> 
> `--image_provisioner=bind`
> 
> Timothy Chen wrote:
> sounds good to me.
> 
> Jiang Yan Xu wrote:
> About naming, I would prefer leaving the word "backend" in the second 
> flag: `--image_provisioner_backend`, just because the code is written this 
> way. The code has a fontend called `Provisioner` and the backend called 
> `Backend`. Having the flag seemingly refer to the previous when it's in fact 
> referring to the latter is very confusing.
> 
> I actually like the frontend being called "Provider" and the backend 
> called "Provisioner" better but to do the change more thoroughly we should be 
> renaming current `Provisioner` to `Provider` and the `Backend` to 
> `Provisioner`, which is a larger undertaking.
> 
> Timothy Chen wrote:
> this is probably the best patch to change all the names if we want to. 
> Backend in comparison with Provisioner, definitely I feel like Provisioner is 
> more natural.
> 
> Vinod Kone wrote:
> +1 for changing backend to provisioner and provisioner to provider. my 
> original inspiration for the suggestion was vagrant.
> 
> https://docs.vagrantup.com/v2/provisioning/index.html
> https://docs.vagrantup.com/v2/providers/index.html

I think `Store` could be renamed to `Provider` because that's the one that 
'provides' image layers.

I would still keeps the frontend being called `Provisioner` because it's the 
one that take layers from the `Provider` and constructs (provisions) a root 
filesystem.


- Jie


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners

OK, i got confused a little bit. WE only need to specify a single image 
provisioner.

`--image_provisioner=bind`


- Jie


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu

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


Chatted with Viond about the flag naming. We prefer the following:

```
--image_providers=APPC,DOCKER
--image_provisioner=bind,copy,overlayfs
```

- Jie Yu


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners
> 
> Jie Yu wrote:
> OK, i got confused a little bit. WE only need to specify a single image 
> provisioner.
> 
> `--image_provisioner=bind`
> 
> Timothy Chen wrote:
> sounds good to me.
> 
> Jiang Yan Xu wrote:
> About naming, I would prefer leaving the word "backend" in the second 
> flag: `--image_provisioner_backend`, just because the code is written this 
> way. The code has a fontend called `Provisioner` and the backend called 
> `Backend`. Having the flag seemingly refer to the previous when it's in fact 
> referring to the latter is very confusing.
> 
> I actually like the frontend being called "Provider" and the backend 
> called "Provisioner" better but to do the change more thoroughly we should be 
> renaming current `Provisioner` to `Provider` and the `Backend` to 
> `Provisioner`, which is a larger undertaking.
> 
> Timothy Chen wrote:
> this is probably the best patch to change all the names if we want to. 
> Backend in comparison with Provisioner, definitely I feel like Provisioner is 
> more natural.
> 
> Vinod Kone wrote:
> +1 for changing backend to provisioner and provisioner to provider. my 
> original inspiration for the suggestion was vagrant.
> 
> https://docs.vagrantup.com/v2/provisioning/index.html
> https://docs.vagrantup.com/v2/providers/index.html
> 
> Jie Yu wrote:
> I think `Store` could be renamed to `Provider` because that's the one 
> that 'provides' image layers.
> 
> I would still keeps the frontend being called `Provisioner` because it's 
> the one that take layers from the `Provider` and constructs (provisions) a 
> root filesystem.
> 
> Jie Yu wrote:
> OK, I don't think we should spend too much time on this naming. How about 
> just keep the class name unchanged and use `--image_provisioner_backend` 
> instead:
> 
> ```
> --image_providers=APPC,DOCKER
> --image_provisioner_backend=bind
> ```

We pretty much need to use 'auto' for --image_provisioner_backend in the 
future. If only one layer is returned, user 'bind' backend. Otherwise, use 
overlayfs. If overlayfs is not available, use copy.


- Jie


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   

Re: Review Request 38011: Maintenance Primitives: Use the parse<RepeatedPtrField> helper instead of a plural MachineID protobuf.

2015-09-16 Thread Joseph Wu


> On Sept. 15, 2015, 11:58 p.m., Guangya Liu wrote:
> > src/common/protobuf_utils.hpp, line 115
> > 
> >
> > This should also be removed.

Thanks for spotting this :)


- Joseph


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


On Sept. 15, 2015, 10:47 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38011/
> ---
> 
> (Updated Sept. 15, 2015, 10:47 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Hindman, Artem 
> Harutyunyan, Joris Van Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This removes the MachineIDs protobuf and changes it to instead use the 
> RepeatedFieldPtr.
> This also changes the maintenance primitives (alpha) API for /machine/up and 
> /machine/down, which both take an array instead of an object now.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   include/mesos/v1/mesos.proto bcce340777f037053e1eab2d75c351e53420b9da 
>   src/common/protobuf_utils.hpp 86474eac116306e50f276bc0539de9bca66133e0 
>   src/common/protobuf_utils.cpp 08612700c456017638a9978e5fe9cfa466294c46 
>   src/master/http.cpp f7ce9aa56b453c1d37b99dd836d956ab292ab62e 
>   src/master/maintenance.hpp 8d134aa44cac9003f6821bc418a22254869f2d6c 
>   src/master/maintenance.cpp 87308a659db05f0676bd02a56ff41fe9d953ba71 
>   src/tests/master_maintenance_tests.cpp 
> 44785057f129a3e6a69f399f7d6db59d9d5c2e91 
>   src/tests/registrar_tests.cpp aa49c86c94446f17f705039075050296b4af1955 
> 
> Diff: https://reviews.apache.org/r/38011/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 38366: Added helper to model Labels message for state.json.

2015-09-16 Thread Niklas Nielsen

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

Ship it!


Ship It!

- Niklas Nielsen


On Sept. 14, 2015, 1:55 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38366/
> ---
> 
> (Updated Sept. 14, 2015, 1:55 p.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, and Niklas Nielsen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also updated Task modelling to show labels only if Task.has_labels() is true. 
> This way, the "labels" field won't shown if there are no labels. This makes 
> it consistent with the modelling of rest of the "optional" fields.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 61ad5314fb14cab0d1cec4fb855fb89b7ac8cf60 
>   src/common/http.cpp 9c0d31e261788c492224345b9e0c32643ddb4156 
>   src/common/protobuf_utils.cpp 08612700c456017638a9978e5fe9cfa466294c46 
>   src/master/master.cpp 5589eca4317b597de509f3387cfc349083b361ac 
>   src/tests/common/http_tests.cpp bf8712b11339b409514ab86c1f32eaf7e9c9a2f1 
> 
> Diff: https://reviews.apache.org/r/38366/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 38370: Updated docker executor to set container IP in TaskStatus::NetworkInfo.

2015-09-16 Thread Niklas Nielsen


> On Sept. 15, 2015, 6:57 p.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [38363, 38364, 38365]
> > 
> > Failed command: ./support/apply-review.sh -n -r 38365
> > 
> > Error:
> >  2015-09-16 01:57:34 URL:https://reviews.apache.org/r/38365/diff/raw/ 
> > [16233/16233] -> "38365.patch" [1]
> > error: patch failed: 
> > src/slave/containerizer/isolators/filesystem/linux.cpp:243
> > error: src/slave/containerizer/isolators/filesystem/linux.cpp: patch does 
> > not apply
> > Failed to apply patch

Do you need to rebase?


- Niklas


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


On Sept. 15, 2015, 2:11 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38370/
> ---
> 
> (Updated Sept. 15, 2015, 2:11 p.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docker executor to set container IP in TaskStatus::NetworkInfo.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a628922cc49b1dc8a37621a6a8b7ef6842cfb051 
> 
> Diff: https://reviews.apache.org/r/38370/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 38370: Updated docker executor to set container IP in TaskStatus::NetworkInfo.

2015-09-16 Thread Mesos ReviewBot

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


Bad patch!

Reviews applied: [38363, 38364, 38365]

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

Error:
 2015-09-16 20:48:28 URL:https://reviews.apache.org/r/38365/diff/raw/ 
[16233/16233] -> "38365.patch" [1]
error: patch failed: src/slave/containerizer/isolators/filesystem/linux.cpp:243
error: src/slave/containerizer/isolators/filesystem/linux.cpp: patch does not 
apply
Failed to apply patch

- Mesos ReviewBot


On Sept. 16, 2015, 7:52 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38370/
> ---
> 
> (Updated Sept. 16, 2015, 7:52 p.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docker executor to set container IP in TaskStatus::NetworkInfo.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a628922cc49b1dc8a37621a6a8b7ef6842cfb051 
> 
> Diff: https://reviews.apache.org/r/38370/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 38367: Added NetworkInfo message to ContainerInfo and TaskStatus.

2015-09-16 Thread Vinod Kone


> On Sept. 14, 2015, 10:19 p.m., Vinod Kone wrote:
> > src/common/http.cpp, lines 153-194
> > 
> >
> > Why custom models? Can we not use the protobuf to json converters?
> 
> Kapil Arya wrote:
> This is mostly due to Labels. The way they are setup right now, we get 
> sth like:
> ```
> { "labels": { "labels": [ { "key": "k", "value": "v" } ... ] } }
> ```
> 
> The explicit modelling removes the extra label from within the JSON to 
> make it more readable.

we have a goal to back our admin endpoints (like state.json) with protobufs so 
that it is easy to convey the schema to the operators. so i would prefer we use 
standard protobuf to json converters.

but, looks like "Labels" is already being modeled in a custom way in the state 
endpoint. so i'm fine if you want to keep it consistent. feel free to drop the 
issue.


- Vinod


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


On Sept. 16, 2015, 2:05 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38367/
> ---
> 
> (Updated Sept. 16, 2015, 2:05 a.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows the frameworks to specify an intent to enable ip-per-container. 
> The
> IP information is supplied back to the framework as well as state.json 
> endpoints
> by including NetworkInfo inside TaskStatus.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   src/common/http.hpp 61ad5314fb14cab0d1cec4fb855fb89b7ac8cf60 
>   src/common/http.cpp 9c0d31e261788c492224345b9e0c32643ddb4156 
>   src/slave/slave.cpp 44865bd2f7ab822829f93780ba1b883ffedb9842 
>   src/tests/master_tests.cpp dd65fccf89566b367fd0da781a60b6b6b35e5d5b 
>   src/tests/slave_tests.cpp 447c43c96ce0c043e37319bb213723ed33820f8a 
> 
> Diff: https://reviews.apache.org/r/38367/diff/
> 
> 
> Testing
> ---
> 
> Added new tests and ran make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 38011: Maintenance Primitives: Use the parse<RepeatedPtrField> helper instead of a plural MachineID protobuf.

2015-09-16 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [38011]

All tests passed.

- Mesos ReviewBot


On Sept. 16, 2015, 7:38 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38011/
> ---
> 
> (Updated Sept. 16, 2015, 7:38 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Hindman, Artem 
> Harutyunyan, Joris Van Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This removes the MachineIDs protobuf and changes it to instead use the 
> RepeatedFieldPtr.
> This also changes the maintenance primitives (alpha) API for /machine/up and 
> /machine/down, which both take an array instead of an object now.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   include/mesos/v1/mesos.proto bcce340777f037053e1eab2d75c351e53420b9da 
>   src/common/protobuf_utils.hpp 86474eac116306e50f276bc0539de9bca66133e0 
>   src/common/protobuf_utils.cpp 08612700c456017638a9978e5fe9cfa466294c46 
>   src/master/http.cpp f7ce9aa56b453c1d37b99dd836d956ab292ab62e 
>   src/master/maintenance.hpp 8d134aa44cac9003f6821bc418a22254869f2d6c 
>   src/master/maintenance.cpp 87308a659db05f0676bd02a56ff41fe9d953ba71 
>   src/tests/master_maintenance_tests.cpp 
> 44785057f129a3e6a69f399f7d6db59d9d5c2e91 
>   src/tests/registrar_tests.cpp aa49c86c94446f17f705039075050296b4af1955 
> 
> Diff: https://reviews.apache.org/r/38011/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 37505: Fix broken health check in docker executor.

2015-09-16 Thread Timothy Chen

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

Ship it!


Ship It!

- Timothy Chen


On Sept. 14, 2015, 12:59 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37505/
> ---
> 
> (Updated Sept. 14, 2015, 12:59 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3136
> https://issues.apache.org/jira/browse/MESOS-3136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix broken health check in docker executor.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp aaf8884a20901ad3a440d545d4b6316b87fc3403 
>   src/docker/docker.cpp 553e831029454d6d423842915b8bbfcaf19fa7f9 
>   src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e 
>   src/slave/containerizer/docker.cpp 289d4ec0fba9071dfe0cbf5391b5391d4566dd9c 
>   src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea 
> 
> Diff: https://reviews.apache.org/r/37505/diff/
> 
> 
> Testing
> ---
> 
> # Add two new test cases, HealthCheckTest.ROOT_DOCKER_DockerHealthyTask and 
> HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange
> # Docker health check command is run through "docker exec"
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthyTask" --verbose
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange" 
> --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-09-16 Thread Gilbert Song

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

Ship it!


This improvement brings convenience.

- Gilbert Song


On Sept. 16, 2015, 1:03 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 16, 2015, 1:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 37826: Introduced conversion of JSON arrays to repeated protobufs.

2015-09-16 Thread Gilbert Song

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

Ship it!


Ship It!

- Gilbert Song


On Sept. 10, 2015, 6:31 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37826/
> ---
> 
> (Updated Sept. 10, 2015, 6:31 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Michael Park.
> 
> 
> Bugs: MESOS-3312
> https://issues.apache.org/jira/browse/MESOS-3312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 57d5fdf45273c620655b44b5f5572290cffa4bf6 
> 
> Diff: https://reviews.apache.org/r/37826/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Re: Review Request 38367: Added NetworkInfo message to ContainerInfo and TaskStatus.

2015-09-16 Thread Niklas Nielsen

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


To Vinod's point; can we use JSON::protobuf() and override the fields we want 
custom formatting for?


include/mesos/mesos.proto (line 1392)


s/upto/up to/


- Niklas Nielsen


On Sept. 15, 2015, 7:05 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38367/
> ---
> 
> (Updated Sept. 15, 2015, 7:05 p.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows the frameworks to specify an intent to enable ip-per-container. 
> The
> IP information is supplied back to the framework as well as state.json 
> endpoints
> by including NetworkInfo inside TaskStatus.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   src/common/http.hpp 61ad5314fb14cab0d1cec4fb855fb89b7ac8cf60 
>   src/common/http.cpp 9c0d31e261788c492224345b9e0c32643ddb4156 
>   src/slave/slave.cpp 44865bd2f7ab822829f93780ba1b883ffedb9842 
>   src/tests/master_tests.cpp dd65fccf89566b367fd0da781a60b6b6b35e5d5b 
>   src/tests/slave_tests.cpp 447c43c96ce0c043e37319bb213723ed33820f8a 
> 
> Diff: https://reviews.apache.org/r/38367/diff/
> 
> 
> Testing
> ---
> 
> Added new tests and ran make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 38368: Replaced slaveTaskStatusLabelDecorator hook with slaveTaskStatusDecorator hook.

2015-09-16 Thread Niklas Nielsen

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



src/examples/test_hook_module.cpp (line 203)


newline



src/hook/manager.cpp (line 248)


newline



src/tests/common/http_tests.cpp (line 111)


Instead of stringify, does it make sense to add an operator overload?



src/tests/hook_tests.cpp (line 524)


newline


- Niklas Nielsen


On Sept. 15, 2015, 7:05 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38368/
> ---
> 
> (Updated Sept. 15, 2015, 7:05 p.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows the hook to not only update TaskStatus::labels, but also
> TaskStatus::container_status.
> 
> Enhanced example hook module and relevant test to reflect the change.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp d90baccba4ac73eb777c8848e40ba737e756032f 
>   src/examples/test_hook_module.cpp bc13a8ad0308668f31310b3aa65243bfb41b87b5 
>   src/hook/manager.hpp 30d8321f459cacdfc0397ab7cd4e81710655351a 
>   src/hook/manager.cpp 754c238fcf728d6aa5b897ed5d9f46c251345334 
>   src/slave/slave.cpp 44865bd2f7ab822829f93780ba1b883ffedb9842 
>   src/tests/common/http_tests.cpp bf8712b11339b409514ab86c1f32eaf7e9c9a2f1 
>   src/tests/hook_tests.cpp deb4343089a30073e8f1f811731b075f7d968846 
> 
> Diff: https://reviews.apache.org/r/38368/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jiang Yan Xu

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

Ship it!



src/slave/containerizer/provisioner/appc/store.cpp (line 49)


Yup this is a better approach!



src/slave/containerizer/provisioner/provisioner.cpp (line 324)


This is used as an alias? `const &` so it's more idiomatic?



src/slave/flags.cpp (lines 62 - 65)


This is the first flag IIRC that we require that capital letters be used. 
It's no big deal but my original code made it OK to use lower case "appc" is 
thoughtful and less error-prone for operators at little cost for code 
complexity.



src/slave/flags.cpp (line 67)


Assuming this is going to be changed to --image_provisioner_backend.



src/tests/containerizer/provisioner_appc_tests.cpp (lines 284 - 289)


I was using this to test these paths::XYZ methods: Code being tested is 
using these methods and the test itself is using literals to reconsturct the 
path.


- Jiang Yan Xu


On Sept. 15, 2015, 6:28 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 15, 2015, 6:28 p.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 38137: Added Docker provisioner and local store

2015-09-16 Thread Gilbert Song

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



src/slave/containerizer/provisioners/docker/metadata_manager.hpp (line 64)


Create `an` Image. Not a big deal:)



src/slave/containerizer/provisioners/docker/metadata_manager.cpp (line 221)


`getImageLayerRootfsPath`

This method name is clear.



src/slave/containerizer/provisioners/docker/paths.cpp (lines 36 - 39)


Is it necessary to attach the following comments?

// Creates a temporary directory using the specified path
// template. The template may be any path with _6_ `Xs' appended to
// it, for example /tmp/temp.XX. The trailing `Xs' are replaced
// with a unique alphanumeric combination.



src/slave/containerizer/provisioners/docker/provisioner.cpp (lines 101 - 110)


{string _root} is suggested.



src/slave/containerizer/provisioners/docker/provisioner.cpp (line 104)


This looks good to me. If the directory exists, no need to try.



src/slave/containerizer/provisioners/docker/store.hpp (lines 53 - 59)


Comments of returned list scenario, with a given specified image. Of 
course, no need to do so if we decided to merge APPC and DOCKER provisioner 
together.


- Gilbert Song


On Sept. 11, 2015, 7:42 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38137/
> ---
> 
> (Updated Sept. 11, 2015, 7:42 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang 
> Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Joining all the commits around provisioner and local store into one review so 
> it's easier to review, as patches
> are changing code on top of each other.
> 
> All the commits are going to committed together.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8963cea 
>   src/slave/containerizer/provisioner.hpp 9e0e0b8 
>   src/slave/containerizer/provisioner.cpp 2ac9008 
>   src/slave/containerizer/provisioners/docker/local_store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/local_store.cpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/message.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/message.proto PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/metadata_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/metadata_manager.cpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/paths.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/paths.cpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/provisioner.cpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963 
>   src/slave/flags.cpp b676bac 
>   src/tests/containerizer/docker_provisioner_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38137/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 38137: Added Docker provisioner and local store

2015-09-16 Thread Gilbert Song

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


Read through all the codes. A discussion is needed to determine whether it is 
necessary to unify DOCKER and APPC together. Some parts of docker provisioner 
are almost copied from appc provisioner, but some of them are new for docker.

I am not pretty clear about why are Store and LocalStore implemented seperately?

Some improvement in Docker provisioner can be applied to appc provisioner.

- Gilbert Song


On Sept. 11, 2015, 7:42 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38137/
> ---
> 
> (Updated Sept. 11, 2015, 7:42 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang 
> Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Joining all the commits around provisioner and local store into one review so 
> it's easier to review, as patches
> are changing code on top of each other.
> 
> All the commits are going to committed together.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8963cea 
>   src/slave/containerizer/provisioner.hpp 9e0e0b8 
>   src/slave/containerizer/provisioner.cpp 2ac9008 
>   src/slave/containerizer/provisioners/docker/local_store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/local_store.cpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/message.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/message.proto PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/metadata_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/metadata_manager.cpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/paths.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/paths.cpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/provisioner.cpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963 
>   src/slave/flags.cpp b676bac 
>   src/tests/containerizer/docker_provisioner_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38137/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu


> On Sept. 16, 2015, 9:39 p.m., Jiang Yan Xu wrote:
> > src/tests/containerizer/provisioner_appc_tests.cpp, lines 290-295
> > 
> >
> > I was using this to test these paths::XYZ methods: Code being tested is 
> > using these methods and the test itself is using literals to reconsturct 
> > the path.

DIscussed offline. Path tests should be in a different test. This test is to 
test provisioning with APPC images.


- Jie


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 38365: Updated Isolator::prepare to return list of required namespaces.

2015-09-16 Thread Kapil Arya

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

(Updated Sept. 16, 2015, 6 p.m.)


Review request for mesos, Connor Doyle, Jie Yu, and Niklas Nielsen.


Changes
---

rebased


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


Repository: mesos


Description
---

This allows the Isolators to decide whether or not to provide resource isolation
on a per-container level.


Diffs (updated)
-

  include/mesos/slave/isolator.hpp 4db23dc242039607e6444d15aa800207b415ca02 
  include/mesos/slave/isolator.proto 12ea6ac3552c70a172ae9e8506f4b5d96457a3ec 
  src/slave/containerizer/isolator.hpp fbb7c8ab908192ae64f34e466c0c24705b3a134b 
  src/slave/containerizer/isolator.cpp 7973100ea1a58938c50962120b9ecb6722b2ee4e 
  src/slave/containerizer/isolators/filesystem/linux.hpp 
6cfe9fa2971d50f545587b57721f75a981f6d5ed 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
dbdbf8722d088ef671b705c0c42191d0f9e05be9 
  src/slave/containerizer/isolators/filesystem/shared.hpp 
a21bc79d342ece50c4924fc0ebd2186e57b3e899 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
4b4520e30ce1d1818bd3a13260f6dd55ab3900c9 
  src/slave/containerizer/isolators/namespaces/pid.hpp 
b22f5ba8e3743bb243ed2c5d204ab4ba21088630 
  src/slave/containerizer/isolators/namespaces/pid.cpp 
35cb6645c9abc0cf533b844e2b2cccf4374bfd68 
  src/slave/containerizer/isolators/network/port_mapping.hpp 
4bca0b81bf69fb4cd75e05aacd02d3e818e32d09 
  src/slave/containerizer/isolators/network/port_mapping.cpp 
34ba2294b0bd7d57aa9de073692a2ea8ec62681d 
  src/slave/containerizer/mesos/containerizer.cpp 
1b83a8725b35435531038e37188b4c97189cef03 
  src/tests/containerizer/isolator_tests.cpp 
d34c82a7f24da6f60cfb22790f516dc6065b1f6f 

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


Testing
---

make check


Thanks,

Kapil Arya



Re: Review Request 38367: Added NetworkInfo message to ContainerInfo and TaskStatus.

2015-09-16 Thread Kapil Arya

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

(Updated Sept. 16, 2015, 6 p.m.)


Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Vinod Kone.


Changes
---

rebased; addressed issues.


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


Repository: mesos


Description
---

This allows the frameworks to specify an intent to enable ip-per-container. The
IP information is supplied back to the framework as well as state.json endpoints
by including NetworkInfo inside TaskStatus.


Diffs (updated)
-

  include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
  src/common/http.hpp 61ad5314fb14cab0d1cec4fb855fb89b7ac8cf60 
  src/common/http.cpp 9c0d31e261788c492224345b9e0c32643ddb4156 
  src/slave/slave.cpp 44865bd2f7ab822829f93780ba1b883ffedb9842 
  src/tests/master_tests.cpp dd65fccf89566b367fd0da781a60b6b6b35e5d5b 
  src/tests/slave_tests.cpp 447c43c96ce0c043e37319bb213723ed33820f8a 

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


Testing
---

Added new tests and ran make check.


Thanks,

Kapil Arya



Re: Review Request 38366: Added helper to model Labels message for state.json.

2015-09-16 Thread Kapil Arya

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

(Updated Sept. 16, 2015, 6 p.m.)


Review request for mesos, Connor Doyle, Jie Yu, and Niklas Nielsen.


Changes
---

rebased


Repository: mesos


Description
---

Also updated Task modelling to show labels only if Task.has_labels() is true. 
This way, the "labels" field won't shown if there are no labels. This makes it 
consistent with the modelling of rest of the "optional" fields.


Diffs (updated)
-

  src/common/http.hpp 61ad5314fb14cab0d1cec4fb855fb89b7ac8cf60 
  src/common/http.cpp 9c0d31e261788c492224345b9e0c32643ddb4156 
  src/common/protobuf_utils.cpp 08612700c456017638a9978e5fe9cfa466294c46 
  src/master/master.cpp f26271c5b21685916c0654488ac1464f21d72e9a 
  src/tests/common/http_tests.cpp bf8712b11339b409514ab86c1f32eaf7e9c9a2f1 

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


Testing
---

make check


Thanks,

Kapil Arya



Re: Review Request 38363: Refactored container Launcher to accept namespaces dynamically.

2015-09-16 Thread Kapil Arya

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

(Updated Sept. 16, 2015, 6 p.m.)


Review request for mesos, Connor Doyle, Jie Yu, and Niklas Nielsen.


Changes
---

rebased


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


Repository: mesos


Description
---

This prepares MesosContainerizer to allow isolators to specify namespaces for
each individual containerizer. Thus, allowing isolators to decide whether or not
to enable isolation for a particular container.


Diffs (updated)
-

  src/slave/containerizer/launcher.hpp 8cc832e99838683c14d8158bdf52a267299508c7 
  src/slave/containerizer/launcher.cpp 5267eecbdf31c88cd3c7dc7172b5aff7bac534b0 
  src/slave/containerizer/linux_launcher.hpp 
bf6bf3f27dac007429c224d4f6798ac68316accb 
  src/slave/containerizer/linux_launcher.cpp 
12acf4d08cb7fb1161820cf4bb480fd1bc2c6858 
  src/slave/containerizer/mesos/containerizer.cpp 
1b83a8725b35435531038e37188b4c97189cef03 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
  src/tests/containerizer/isolator_tests.cpp 
d34c82a7f24da6f60cfb22790f516dc6065b1f6f 
  src/tests/containerizer/launcher.hpp 6f020f94b042eef0e6a69ba8c26dfb697f3e81a3 

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


Testing
---

make check.


Thanks,

Kapil Arya



Re: Review Request 38368: Replaced slaveTaskStatusLabelDecorator hook with slaveTaskStatusDecorator hook.

2015-09-16 Thread Kapil Arya

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

(Updated Sept. 16, 2015, 6 p.m.)


Review request for mesos, Connor Doyle, Jie Yu, and Niklas Nielsen.


Changes
---

rebased; addressed issues


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


Repository: mesos


Description
---

This allows the hook to not only update TaskStatus::labels, but also
TaskStatus::container_status.

Enhanced example hook module and relevant test to reflect the change.


Diffs (updated)
-

  include/mesos/hook.hpp d90baccba4ac73eb777c8848e40ba737e756032f 
  src/examples/test_hook_module.cpp bc13a8ad0308668f31310b3aa65243bfb41b87b5 
  src/hook/manager.hpp 30d8321f459cacdfc0397ab7cd4e81710655351a 
  src/hook/manager.cpp 754c238fcf728d6aa5b897ed5d9f46c251345334 
  src/slave/slave.cpp 44865bd2f7ab822829f93780ba1b883ffedb9842 
  src/tests/hook_tests.cpp deb4343089a30073e8f1f811731b075f7d968846 

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


Testing
---

make check.


Thanks,

Kapil Arya



Re: Review Request 38364: Minor refactor for MesosContainerizer.

2015-09-16 Thread Kapil Arya

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

(Updated Sept. 16, 2015, 6 p.m.)


Review request for mesos, Connor Doyle, Jie Yu, and Niklas Nielsen.


Changes
---

rebased


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


Repository: mesos


Description
---

This change moves two different pieces of code, that each iterate over list of 
ContainerPrepareInfos, close together for readability. It becomes even more 
relevant when looking at https://reviews.apache.org/r/38365/ that iterates over 
yet another member from ContainerPrepareInfo.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
1b83a8725b35435531038e37188b4c97189cef03 

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


Testing
---

make  check


Thanks,

Kapil Arya



Re: Review Request 38370: Updated docker executor to set container IP in TaskStatus::NetworkInfo.

2015-09-16 Thread Kapil Arya

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

(Updated Sept. 16, 2015, 6 p.m.)


Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy 
Chen.


Changes
---

rebased


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


Repository: mesos


Description
---

Updated docker executor to set container IP in TaskStatus::NetworkInfo.


Diffs (updated)
-

  src/docker/executor.cpp 6809e4ac0018f827db27e976164c549e3a0ff823 
  src/tests/containerizer/docker_containerizer_tests.cpp 
a628922cc49b1dc8a37621a6a8b7ef6842cfb051 

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


Testing
---

make check.


Thanks,

Kapil Arya



Re: Review Request 38367: Added NetworkInfo message to ContainerInfo and TaskStatus.

2015-09-16 Thread Vinod Kone

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

Ship it!



include/mesos/mesos.proto (lines 1165 - 1167)


I don't think you need to mention about how this field is populated (hook 
etc). Also the comment about the network_info.ip belongs where 
ContainerStatus/NetworkInfo is defined.



include/mesos/mesos.proto (line 1371)


s/not/no/


- Vinod Kone


On Sept. 16, 2015, 2:05 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38367/
> ---
> 
> (Updated Sept. 16, 2015, 2:05 a.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows the frameworks to specify an intent to enable ip-per-container. 
> The
> IP information is supplied back to the framework as well as state.json 
> endpoints
> by including NetworkInfo inside TaskStatus.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4720cab4a89db76a48bc9563bba4f5bf1c 
>   src/common/http.hpp 61ad5314fb14cab0d1cec4fb855fb89b7ac8cf60 
>   src/common/http.cpp 9c0d31e261788c492224345b9e0c32643ddb4156 
>   src/slave/slave.cpp 44865bd2f7ab822829f93780ba1b883ffedb9842 
>   src/tests/master_tests.cpp dd65fccf89566b367fd0da781a60b6b6b35e5d5b 
>   src/tests/slave_tests.cpp 447c43c96ce0c043e37319bb213723ed33820f8a 
> 
> Diff: https://reviews.apache.org/r/38367/diff/
> 
> 
> Testing
> ---
> 
> Added new tests and ran make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-09-16 Thread Joseph Wu

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

Ship it!


Ship It!

- Joseph Wu


On Sept. 15, 2015, 6:03 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 15, 2015, 6:03 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 38368: Replaced slaveTaskStatusLabelDecorator hook with slaveTaskStatusDecorator hook.

2015-09-16 Thread Niklas Nielsen

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

Ship it!


Ship It!

- Niklas Nielsen


On Sept. 16, 2015, 3 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38368/
> ---
> 
> (Updated Sept. 16, 2015, 3 p.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows the hook to not only update TaskStatus::labels, but also
> TaskStatus::container_status.
> 
> Enhanced example hook module and relevant test to reflect the change.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp d90baccba4ac73eb777c8848e40ba737e756032f 
>   src/examples/test_hook_module.cpp bc13a8ad0308668f31310b3aa65243bfb41b87b5 
>   src/hook/manager.hpp 30d8321f459cacdfc0397ab7cd4e81710655351a 
>   src/hook/manager.cpp 754c238fcf728d6aa5b897ed5d9f46c251345334 
>   src/slave/slave.cpp 44865bd2f7ab822829f93780ba1b883ffedb9842 
>   src/tests/hook_tests.cpp deb4343089a30073e8f1f811731b075f7d968846 
> 
> Diff: https://reviews.apache.org/r/38368/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 38253: Add containerId to ResourceUsage to enable QoS controller to target a container

2015-09-16 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [38253]

All tests passed.

- Mesos ReviewBot


On Sept. 15, 2015, 1:08 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38253/
> ---
> 
> (Updated Sept. 15, 2015, 1:08 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2875
> https://issues.apache.org/jira/browse/MESOS-2875
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We should ensure that we are addressing the container which the QoS 
> controller intended to kill. Without this check, we may run into a scenario 
> where the executor has terminated and one with the same id has started in the 
> interim i.e. running in a different container than the one the QoS controller 
> targeted.
> 
> This most likely requires us to add containerId to the ResourceUsage message 
> and encode the containerID in the QoS Correction message.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4 
>   include/mesos/slave/oversubscription.proto fa69a95 
>   src/slave/slave.cpp 44865bd 
>   src/tests/oversubscription_tests.cpp 0c5edaf 
> 
> Diff: https://reviews.apache.org/r/38253/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 38046: mesos: Replaced hard-coded reap interval with a constant.

2015-09-16 Thread Ben Mahler

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

Ship it!


Ship It!

- Ben Mahler


On Sept. 16, 2015, 4:07 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38046/
> ---
> 
> (Updated Sept. 16, 2015, 4:07 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-1935
> https://issues.apache.org/jira/browse/MESOS-1935
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> mesos: Replace hard-coded reap interval with a constant.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/launch_tests.cpp 
> d211fc0f665988068c67836ef80916828a0df2bd 
>   src/tests/slave_recovery_tests.cpp 6aae14a3c39b9aee76147b691b0170946e1120b5 
>   src/tests/slave_tests.cpp 5c1a3d36a5f67629aef275eeae12956c524e8102 
> 
> Diff: https://reviews.apache.org/r/38046/diff/
> 
> 
> Testing
> ---
> 
> Platform: Ubuntu 14.04
> make distcheck
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 38253: Add containerId to ResourceUsage to enable QoS controller to target a container

2015-09-16 Thread Niklas Nielsen

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



include/mesos/mesos.proto (line 820)


The comment doesn't add a ton of context; let's kill this one or specify 
that it is the container id for the executor specified in the executor_info 
field.



include/mesos/slave/oversubscription.proto (lines 41 - 42)


How about something like:

NOTE: Framework ID and either executor id or executor id **and** 
container_id must be set to kill a running container. In the latter case, the 
caller can be assured that the kill request doesn't target a new executor 
reusing an old executor id.



src/slave/slave.cpp (line 4380)


Great! Let's include the container id's as well for debugging :)


- Niklas Nielsen


On Sept. 15, 2015, 6:08 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38253/
> ---
> 
> (Updated Sept. 15, 2015, 6:08 a.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2875
> https://issues.apache.org/jira/browse/MESOS-2875
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We should ensure that we are addressing the container which the QoS 
> controller intended to kill. Without this check, we may run into a scenario 
> where the executor has terminated and one with the same id has started in the 
> interim i.e. running in a different container than the one the QoS controller 
> targeted.
> 
> This most likely requires us to add containerId to the ResourceUsage message 
> and encode the containerID in the QoS Correction message.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4 
>   include/mesos/slave/oversubscription.proto fa69a95 
>   src/slave/slave.cpp 44865bd 
>   src/tests/oversubscription_tests.cpp 0c5edaf 
> 
> Diff: https://reviews.apache.org/r/38253/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 38003: MESOS-3351 (duplicated slave id in master after master failover)

2015-09-16 Thread Klaus Ma


> On Sept. 15, 2015, 10:32 p.m., Vinod Kone wrote:
> > src/tests/master_tests.cpp, line 3637
> > 
> >
> > Does this test reliably fail (i.e., every time) without the code change 
> > in master.cpp?

Nop; the repro rate is about 90% (9 in 10 times). The root cause is master host 
re-used port; but if master did not re-use port, this issue will not trigger. 
For example, I can not reproduce this issue in Ubuntu 14.04 by default setting; 
but it's easy repro in OS X.


> On Sept. 15, 2015, 10:32 p.m., Vinod Kone wrote:
> > src/tests/master_tests.cpp, line 3636
> > 
> >
> > Also add a CHECK_NE() check with both the slave ids?

No sure whether it's necessary; if duplicated slave ids in master, master will 
ask the second slave (with the same id) to shutdown; in this case, it will 
failed when waiting for re-register message.


> On Sept. 15, 2015, 10:32 p.m., Vinod Kone wrote:
> > src/tests/master_tests.cpp, lines 3607-3608
> > 
> >
> > Why specify a mock executor and test containerizer? There's a 
> > StartSlave() overload that takes just the detector (and optionally flags), 
> > which you can use?

Yes, it's only for detector; let me try to use StartSlave with detector only.


- Klaus


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


On Sept. 14, 2015, 6:08 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38003/
> ---
> 
> (Updated Sept. 14, 2015, 6:08 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3351
> https://issues.apache.org/jira/browse/MESOS-3351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> __Phenomenon:__
> In some race condition, the slave was shutdown when after master failover.
> 
> __Root Cause:__
> The slave was shutdown because of duplicated SlavID: in master, the SlaveID 
> is genereated by masterInfo.id + "-S" + nextSlaveId; when master failover, 
> nextSlaveId was reset to 0 and masterInfo.id (generated by date + ip + port + 
> pid) maybe un-changed which lead to duplicated SlaveID. 
> 
> __Solution/Fix:__
> Generate masterInfo.id by UUID instead of "date + ip + port + pid".
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 5589eca 
>   src/tests/master_tests.cpp 8a6b98b 
> 
> Diff: https://reviews.apache.org/r/38003/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 38046: mesos: Replaced hard-coded reap interval with a constant.

2015-09-16 Thread Ben Mahler

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


Whoops, it would be better for all of these to be process::MAX_REAP_INTERVAL() 
since we'd like to move away from using entire the entire process namespace in 
the .cpp files.

- Ben Mahler


On Sept. 16, 2015, 4:07 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38046/
> ---
> 
> (Updated Sept. 16, 2015, 4:07 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-1935
> https://issues.apache.org/jira/browse/MESOS-1935
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> mesos: Replace hard-coded reap interval with a constant.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/launch_tests.cpp 
> d211fc0f665988068c67836ef80916828a0df2bd 
>   src/tests/slave_recovery_tests.cpp 6aae14a3c39b9aee76147b691b0170946e1120b5 
>   src/tests/slave_tests.cpp 5c1a3d36a5f67629aef275eeae12956c524e8102 
> 
> Diff: https://reviews.apache.org/r/38046/diff/
> 
> 
> Testing
> ---
> 
> Platform: Ubuntu 14.04
> make distcheck
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 38046: mesos: Replaced hard-coded reap interval with a constant.

2015-09-16 Thread Guangya Liu


> On 九月 16, 2015, 11:04 p.m., Ben Mahler wrote:
> > Whoops, it would be better for all of these to be 
> > process::MAX_REAP_INTERVAL() since we'd like to move away from using entire 
> > the entire process namespace in the .cpp files.

OK, I will try to file another bug to resolve this. Thanks.


- Guangya


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


On 九月 16, 2015, 4:07 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38046/
> ---
> 
> (Updated 九月 16, 2015, 4:07 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-1935
> https://issues.apache.org/jira/browse/MESOS-1935
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> mesos: Replace hard-coded reap interval with a constant.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/launch_tests.cpp 
> d211fc0f665988068c67836ef80916828a0df2bd 
>   src/tests/slave_recovery_tests.cpp 6aae14a3c39b9aee76147b691b0170946e1120b5 
>   src/tests/slave_tests.cpp 5c1a3d36a5f67629aef275eeae12956c524e8102 
> 
> Diff: https://reviews.apache.org/r/38046/diff/
> 
> 
> Testing
> ---
> 
> Platform: Ubuntu 14.04
> make distcheck
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 38253: Add containerId to ResourceUsage to enable QoS controller to target a container

2015-09-16 Thread Klaus Ma


> On Sept. 14, 2015, 6:07 p.m., Niklas Nielsen wrote:
> > include/mesos/slave/oversubscription.proto, lines 46-47
> > 
> >
> > We need to add a comment about the semantics of executor_id vs 
> > container_id. For example, what happens if both is set? Or do they need to 
> > be set together? (Loooks like it from the code below)

Addressed; add comments that executor id must be set if kill specified 
container.


> On Sept. 14, 2015, 6:07 p.m., Niklas Nielsen wrote:
> > src/slave/slave.cpp, lines 4367-4368
> > 
> >
> > Style nit:
> > 
> > ```
> >   const ContainerID& containerId =
> >   kill.has_container_id() ? kill.container_id() : 
> > executor->containerId;
> > ```
> > 
> > Also, what happens if the container id is wrong? Think we need to do 
> > some validation here.

Addressed; if containerId was wrong, log warn message and ignore the kill 
request.


> On Sept. 14, 2015, 6:07 p.m., Niklas Nielsen wrote:
> > src/tests/oversubscription_tests.cpp, line 859
> > 
> >
> > Mind adding a TODO about doing a full blown object comparison?

Do you mean full comparison on metrics?


- Klaus


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


On Sept. 15, 2015, 1:08 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38253/
> ---
> 
> (Updated Sept. 15, 2015, 1:08 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2875
> https://issues.apache.org/jira/browse/MESOS-2875
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We should ensure that we are addressing the container which the QoS 
> controller intended to kill. Without this check, we may run into a scenario 
> where the executor has terminated and one with the same id has started in the 
> interim i.e. running in a different container than the one the QoS controller 
> targeted.
> 
> This most likely requires us to add containerId to the ResourceUsage message 
> and encode the containerID in the QoS Correction message.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4 
>   include/mesos/slave/oversubscription.proto fa69a95 
>   src/slave/slave.cpp 44865bd 
>   src/tests/oversubscription_tests.cpp 0c5edaf 
> 
> Diff: https://reviews.apache.org/r/38253/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 37931: Update indent for slave.cpp

2015-09-16 Thread Ben Mahler

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

Ship it!


Ship It!

- Ben Mahler


On Sept. 15, 2015, 11:25 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37931/
> ---
> 
> (Updated Sept. 15, 2015, 11:25 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The status message for unknown checkpointed resource need to be
> updated to adjust the indent.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 8cfad7b86675b10c98524174975828bbcf47aed2 
> 
> Diff: https://reviews.apache.org/r/37931/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Review Request 38444: Only unmount sandbox if the mount exists in LinuxFilesystemIsolator.

2015-09-16 Thread Jie Yu

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

Review request for mesos, Timothy Chen and Jiang Yan Xu.


Repository: mesos


Description
---

Only unmount sandbox if the mount exists in LinuxFilesystemIsolator.

This is for upgrade concern. Containers created by the old agent do not have 
sandbox mounts.


Diffs
-

  src/slave/containerizer/isolators/filesystem/linux.cpp 
297a296d536fc72dc8ce9687c381cd666d8cadd2 

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


Testing
---

sudo make check


Thanks,

Jie Yu



Re: Review Request 38253: Add containerId to ResourceUsage to enable QoS controller to target a container

2015-09-16 Thread Niklas Nielsen


> On Sept. 14, 2015, 11:07 a.m., Niklas Nielsen wrote:
> > src/tests/oversubscription_tests.cpp, line 859
> > 
> >
> > Mind adding a TODO about doing a full blown object comparison?
> 
> Klaus Ma wrote:
> Do you mean full comparison on metrics?

Scratch that :) Think we are fine. Will drop the issue


- Niklas


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


On Sept. 15, 2015, 6:08 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38253/
> ---
> 
> (Updated Sept. 15, 2015, 6:08 a.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2875
> https://issues.apache.org/jira/browse/MESOS-2875
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We should ensure that we are addressing the container which the QoS 
> controller intended to kill. Without this check, we may run into a scenario 
> where the executor has terminated and one with the same id has started in the 
> interim i.e. running in a different container than the one the QoS controller 
> targeted.
> 
> This most likely requires us to add containerId to the ResourceUsage message 
> and encode the containerID in the QoS Correction message.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b1deed4 
>   include/mesos/slave/oversubscription.proto fa69a95 
>   src/slave/slave.cpp 44865bd 
>   src/tests/oversubscription_tests.cpp 0c5edaf 
> 
> Diff: https://reviews.apache.org/r/38253/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 38168: libprocess: Replaced hard-coded reap interval with a constant.

2015-09-16 Thread Ben Mahler

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

Ship it!


Ship It!

- Ben Mahler


On Sept. 16, 2015, 4:07 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38168/
> ---
> 
> (Updated Sept. 16, 2015, 4:07 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-1935
> https://issues.apache.org/jira/browse/MESOS-1935
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> libprocess: Replaced hard-coded reap interval with a constant.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/reap_tests.cpp 
> 642ab97b28a6085dc9837f14ff36a3124387a03b 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> ab7515325e5db0a4fd222bb982f51243d7b7e39d 
> 
> Diff: https://reviews.apache.org/r/38168/diff/
> 
> 
> Testing
> ---
> 
> OS: Ubuntu 14.04
> make distcheck
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-09-16 Thread Timothy Chen


> On Sept. 16, 2015, 10:42 p.m., Ben Mahler wrote:
> > Looks like you're missing the change to decoder. On that note, what happens 
> > when we receive a response with a code that we haven't added to our enum?

You're right I missed the decoding part! Let me populate all the codes in the 
enum and set it correctly.


- Timothy


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


On Sept. 16, 2015, 1:03 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 16, 2015, 1:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 38046: mesos: Replaced hard-coded reap interval with a constant.

2015-09-16 Thread Ben Mahler


> On Sept. 16, 2015, 11:04 p.m., Ben Mahler wrote:
> > Whoops, it would be better for all of these to be 
> > process::MAX_REAP_INTERVAL() since we'd like to move away from using entire 
> > the entire process namespace in the .cpp files.
> 
> Guangya Liu wrote:
> OK, I will try to file another bug to resolve this. Thanks.

No need, I will fix it.


- Ben


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


On Sept. 16, 2015, 4:07 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38046/
> ---
> 
> (Updated Sept. 16, 2015, 4:07 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-1935
> https://issues.apache.org/jira/browse/MESOS-1935
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> mesos: Replace hard-coded reap interval with a constant.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/launch_tests.cpp 
> d211fc0f665988068c67836ef80916828a0df2bd 
>   src/tests/slave_recovery_tests.cpp 6aae14a3c39b9aee76147b691b0170946e1120b5 
>   src/tests/slave_tests.cpp 5c1a3d36a5f67629aef275eeae12956c524e8102 
> 
> Diff: https://reviews.apache.org/r/38046/diff/
> 
> 
> Testing
> ---
> 
> Platform: Ubuntu 14.04
> make distcheck
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-09-16 Thread Ben Mahler

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


Looks like you're missing the change to decoder. On that note, what happens 
when we receive a response with a code that we haven't added to our enum?

- Ben Mahler


On Sept. 16, 2015, 1:03 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 16, 2015, 1:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 38370: Updated docker executor to set container IP in TaskStatus::NetworkInfo.

2015-09-16 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [38363, 38364, 38365, 38366, 38367, 38368, 38370]

All tests passed.

- Mesos ReviewBot


On Sept. 16, 2015, 10 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38370/
> ---
> 
> (Updated Sept. 16, 2015, 10 p.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docker executor to set container IP in TaskStatus::NetworkInfo.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 6809e4ac0018f827db27e976164c549e3a0ff823 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a628922cc49b1dc8a37621a6a8b7ef6842cfb051 
> 
> Diff: https://reviews.apache.org/r/38370/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-09-16 Thread Ben Mahler


> On Sept. 16, 2015, 10:42 p.m., Ben Mahler wrote:
> > Looks like you're missing the change to decoder. On that note, what happens 
> > when we receive a response with a code that we haven't added to our enum?
> 
> Timothy Chen wrote:
> You're right I missed the decoding part! Let me populate all the codes in 
> the enum and set it correctly.

Keep in mind that we still have to handle codes we don't have in our enum (some 
are non-standard, we might receive something random, etc).


- Ben


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


On Sept. 16, 2015, 1:03 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 16, 2015, 1:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Review Request 38443: Added layerid information to ManifestResponse

2015-09-16 Thread Jojy Varghese

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

Review request for mesos and Timothy Chen.


Repository: mesos


Description
---

Added layerid information to ManifestResponse


Diffs
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
  src/tests/containerizer/provisioner_docker_tests.cpp 
1b0c30450a07d01d1a38d460bc7d921c80be7e3b 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 38444: Only unmount sandbox if the mount exists in LinuxFilesystemIsolator.

2015-09-16 Thread Jiang Yan Xu

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

Ship it!



src/slave/containerizer/isolators/filesystem/linux.cpp (line 750)


This condition being false is actually rare and only happens when people 
have upgraded from a version of linux filesystem isolation from "head" or when 
the host mount table has been tampered with.

Worth a log line?


- Jiang Yan Xu


On Sept. 16, 2015, 5:52 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38444/
> ---
> 
> (Updated Sept. 16, 2015, 5:52 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only unmount sandbox if the mount exists in LinuxFilesystemIsolator.
> 
> This is for upgrade concern. Containers created by the old agent do not have 
> sandbox mounts.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 297a296d536fc72dc8ce9687c381cd666d8cadd2 
> 
> Diff: https://reviews.apache.org/r/38444/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 38443: Added layerid information to ManifestResponse

2015-09-16 Thread Timothy Chen

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



src/slave/containerizer/provisioner/docker/registry_client.cpp (line 499)


If history array contains non JSON Object from the response I think it will 
just crash the slave.

I realize in other places too, this becomes a bit tricky when docker 
registry changes their impl, or when user is targeting some other version of 
the registry I tihnk we shouldn't crash the whole slave.


- Timothy Chen


On Sept. 17, 2015, 12:27 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38443/
> ---
> 
> (Updated Sept. 17, 2015, 12:27 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added layerid information to ManifestResponse
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 1b0c30450a07d01d1a38d460bc7d921c80be7e3b 
> 
> Diff: https://reviews.apache.org/r/38443/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Re: Review Request 38444: Only unmount sandbox if the mount exists in LinuxFilesystemIsolator.

2015-09-16 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [38444]

All tests passed.

- Mesos ReviewBot


On Sept. 17, 2015, 12:52 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38444/
> ---
> 
> (Updated Sept. 17, 2015, 12:52 a.m.)
> 
> 
> Review request for mesos, Jojy Varghese, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only unmount sandbox if the mount exists in LinuxFilesystemIsolator.
> 
> This is for upgrade concern. Containers created by the old agent do not have 
> sandbox mounts.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 297a296d536fc72dc8ce9687c381cd666d8cadd2 
> 
> Diff: https://reviews.apache.org/r/38444/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 38378: Minor cleanup in perf_tests.cpp.

2015-09-16 Thread Guangya Liu

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

Ship it!


Ship It!

- Guangya Liu


On 九月 14, 2015, 11:08 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38378/
> ---
> 
> (Updated 九月 14, 2015, 11:08 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/perf_tests.cpp 
> bef475e4b573f02b69f2c4dd6d1c6a6e10f34c38 
> 
> Diff: https://reviews.apache.org/r/38378/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>



Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-09-16 Thread Timothy Chen


> On Sept. 16, 2015, 10:42 p.m., Ben Mahler wrote:
> > Looks like you're missing the change to decoder. On that note, what happens 
> > when we receive a response with a code that we haven't added to our enum?
> 
> Timothy Chen wrote:
> You're right I missed the decoding part! Let me populate all the codes in 
> the enum and set it correctly.
> 
> Ben Mahler wrote:
> Keep in mind that we still have to handle codes we don't have in our enum 
> (some are non-standard, we might receive something random, etc).

definitely, the existing code that checks the http map will still exist.


- Timothy


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


On Sept. 16, 2015, 1:03 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 16, 2015, 1:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-09-16 Thread Guangya Liu


> On Sept. 16, 2015, 1:43 a.m., Guangya Liu wrote:
> >
> 
> Timothy Chen wrote:
> it looks like the structs are ordered by response code, so just moving 
> this to the right place assuming so

Got it, this can make the code clear. Thanks.


- Guangya


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


On Sept. 16, 2015, 1:03 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Sept. 16, 2015, 1:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jiang Yan Xu


> On Sept. 16, 2015, 10:54 a.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners
> 
> Jie Yu wrote:
> OK, i got confused a little bit. WE only need to specify a single image 
> provisioner.
> 
> `--image_provisioner=bind`
> 
> Timothy Chen wrote:
> sounds good to me.

About naming, I would prefer leaving the word "backend" in the second flag: 
`--image_provisioner_backend`, just because the code is written this way. The 
code has a fontend called `Provisioner` and the backend called `Backend`. 
Having the flag seemingly refer to the previous when it's in fact referring to 
the latter is very confusing.

I actually like the frontend being called "Provider" and the backend called 
"Provisioner" better but to do the change more thoroughly we should be renaming 
current `Provisioner` to `Provider` and the `Backend` to `Provisioner`, which 
is a larger undertaking.


- Jiang Yan


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


On Sept. 15, 2015, 6:28 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 15, 2015, 6:28 p.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners
> 
> Jie Yu wrote:
> OK, i got confused a little bit. WE only need to specify a single image 
> provisioner.
> 
> `--image_provisioner=bind`
> 
> Timothy Chen wrote:
> sounds good to me.
> 
> Jiang Yan Xu wrote:
> About naming, I would prefer leaving the word "backend" in the second 
> flag: `--image_provisioner_backend`, just because the code is written this 
> way. The code has a fontend called `Provisioner` and the backend called 
> `Backend`. Having the flag seemingly refer to the previous when it's in fact 
> referring to the latter is very confusing.
> 
> I actually like the frontend being called "Provider" and the backend 
> called "Provisioner" better but to do the change more thoroughly we should be 
> renaming current `Provisioner` to `Provider` and the `Backend` to 
> `Provisioner`, which is a larger undertaking.
> 
> Timothy Chen wrote:
> this is probably the best patch to change all the names if we want to. 
> Backend in comparison with Provisioner, definitely I feel like Provisioner is 
> more natural.
> 
> Vinod Kone wrote:
> +1 for changing backend to provisioner and provisioner to provider. my 
> original inspiration for the suggestion was vagrant.
> 
> https://docs.vagrantup.com/v2/provisioning/index.html
> https://docs.vagrantup.com/v2/providers/index.html
> 
> Jie Yu wrote:
> I think `Store` could be renamed to `Provider` because that's the one 
> that 'provides' image layers.
> 
> I would still keeps the frontend being called `Provisioner` because it's 
> the one that take layers from the `Provider` and constructs (provisions) a 
> root filesystem.

OK, I don't think we should spend too much time on this naming. How about just 
keep the class name unchanged and use `--image_provisioner_backend` instead:

```
--image_providers=APPC,DOCKER
--image_provisioner_backend=bind
```


- Jie


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 

Re: Review Request 38370: Updated docker executor to set container IP in TaskStatus::NetworkInfo.

2015-09-16 Thread Kapil Arya


> On Sept. 16, 2015, 3:44 p.m., Niklas Nielsen wrote:
> > src/docker/executor.cpp, lines 162-164
> > 
> >
> > Was "Docker.NetworkSettings.IPAddress" in 0.24.0? If so, don't we want 
> > to deprecate this over a release cycle? (Supporting both in the interim)

done!


- Kapil


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


On Sept. 16, 2015, 3:52 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38370/
> ---
> 
> (Updated Sept. 16, 2015, 3:52 p.m.)
> 
> 
> Review request for mesos, Connor Doyle, Jie Yu, Niklas Nielsen, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-3013
> https://issues.apache.org/jira/browse/MESOS-3013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docker executor to set container IP in TaskStatus::NetworkInfo.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a628922cc49b1dc8a37621a6a8b7ef6842cfb051 
> 
> Diff: https://reviews.apache.org/r/38370/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 37505: Fix broken health check in docker executor.

2015-09-16 Thread Timothy Chen

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


Overall looks good to me, some minor comments around style and wording, but I 
can fix them.
Let me test this patch locally first and if it looks good I'll merge.

- Timothy Chen


On Sept. 14, 2015, 12:59 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37505/
> ---
> 
> (Updated Sept. 14, 2015, 12:59 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3136
> https://issues.apache.org/jira/browse/MESOS-3136
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix broken health check in docker executor.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp aaf8884a20901ad3a440d545d4b6316b87fc3403 
>   src/docker/docker.cpp 553e831029454d6d423842915b8bbfcaf19fa7f9 
>   src/docker/executor.cpp 6647075b55e5a79264e3556bb906a1f26a2d673e 
>   src/slave/containerizer/docker.cpp 289d4ec0fba9071dfe0cbf5391b5391d4566dd9c 
>   src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea 
> 
> Diff: https://reviews.apache.org/r/37505/diff/
> 
> 
> Testing
> ---
> 
> # Add two new test cases, HealthCheckTest.ROOT_DOCKER_DockerHealthyTask and 
> HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange
> # Docker health check command is run through "docker exec"
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthyTask" --verbose
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange" 
> --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>