Re: Review Request 39837: Passing os environment variables when start docker executor.

2015-11-01 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [39837]

All tests passed.

- Mesos ReviewBot


On Nov. 1, 2015, 9:38 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39837/
> ---
> 
> (Updated Nov. 1, 2015, 9:38 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3815
> https://issues.apache.org/jira/browse/MESOS-3815
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Passing os environment variables when start docker executor.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 5e5e0f93123b7f0bda6c8afeb4df271e796e4637 
> 
> Diff: https://reviews.apache.org/r/39837/diff/
> 
> 
> Testing
> ---
> 
> manually test.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Review Request 39837: Passing os environment variables when start docker executor.

2015-11-01 Thread haosdent huang

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

Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.


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


Repository: mesos


Description
---

Passing os environment variables when start docker executor.


Diffs
-

  src/slave/containerizer/docker.cpp 5e5e0f93123b7f0bda6c8afeb4df271e796e4637 

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


Testing
---

manually test.


Thanks,

haosdent huang



Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-11-01 Thread Qian Zhang

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



src/master/allocator/mesos/hierarchical.cpp (line 1005)


For this TODO, what do we plan to do in future? Include the dynamic 
reserved resources for this role on this agent in ```roleConsumedResources```? 
And what about the static reserved resources?



src/master/allocator/mesos/hierarchical.cpp (line 1035)


Can you please clarify a bit about this TODO? What do we plan to do about 
it in future?


- Qian Zhang


On Oct. 30, 2015, 3:29 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39401/
> ---
> 
> (Updated Oct. 30, 2015, 3:29 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3718
> https://issues.apache.org/jira/browse/MESOS-3718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39401/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Review Request 39839: RegistryClient refactor: Changed getManifest interface

2015-11-01 Thread Jojy Varghese

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Replaced path and tag parameters with Image::Name


Diffs
-

  src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
ad57d8592c5f9df5115b575855ed2e99a0597359 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
  src/tests/containerizer/provisioner_docker_tests.cpp 
99c2af7d06dd7908e8d77e1bfb3e20622fb3eb4c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 39837: Passing os environment variables when start docker executor.

2015-11-01 Thread Timothy Chen

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



src/slave/containerizer/docker.cpp 


We intentionally exclude os enviornments as you can see here for a reason.

There are a lot of environment variables that are default to the OS, that 
will break when docker containers run and we need to not include them unless 
it's specifically specified by the user with executor environement variables or 
taskinfo.

The fix should be carefully picking the ones we need only.


- Timothy Chen


On Nov. 1, 2015, 9:38 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39837/
> ---
> 
> (Updated Nov. 1, 2015, 9:38 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3815
> https://issues.apache.org/jira/browse/MESOS-3815
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Passing os environment variables when start docker executor.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 5e5e0f93123b7f0bda6c8afeb4df271e796e4637 
> 
> Diff: https://reviews.apache.org/r/39837/diff/
> 
> 
> Testing
> ---
> 
> manually test.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Review Request 39832: RegistryClient refactor: fully qualified name for URL

2015-11-01 Thread Jojy Varghese

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Removed "using http::URL" to be more explicit and match the rest of http types.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 

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


Testing (updated)
---

make check


Thanks,

Jojy Varghese



Re: Review Request 39250: Puller refactor: moved untar to a common place

2015-11-01 Thread Mesos ReviewBot

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


Bad patch!

Reviews applied: [39838, 39832, 39839, 38579, 39015, 39016, 39017, 39053, 
39112, 39340, 39840, 38580]

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

Error:
 2015-11-01 15:50:03 URL:https://reviews.apache.org/r/38580/diff/raw/ 
[36966/36966] -> "38580.patch" [1]
Successfully applied: Added docker registry RemotePuller

Integrated remote puller with store. Tests will follow.


Review: https://reviews.apache.org/r/38580
Checking 12 files using filter 
--filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/operators,+whitespace/semicolon,+whitespace/tab,+whitespace/todo
src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp:548:  
Redundant blank line at the start of a code block should be deleted.  
[whitespace/blank_line] [2]
Total errors found: 1
Failed to commit patch

- Mesos ReviewBot


On Nov. 1, 2015, 3:39 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39250/
> ---
> 
> (Updated Nov. 1, 2015, 3:39 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Puller refactor: moved untar to a common place
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f314f20d989ed0125e15d2a14d0f8e56c56976d5 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 8010b8aa75a2fc2e4b537f915f9dca028e407b63 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
>   src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39250/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Re: Review Request 39053: RegistryClient refactor: priv method const'ness

2015-11-01 Thread Jojy Varghese

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

(Updated Nov. 1, 2015, 3:33 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased.


Repository: mesos


Description
---

RegistryClient refactor: priv method const'ness


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 39112: RegistryClient refactor: fixed variable names

2015-11-01 Thread Jojy Varghese

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

(Updated Nov. 1, 2015, 3:34 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased.


Repository: mesos


Description
---

RegistryClient refactor: fixed variable names. This patch will serve as 
catch-all for any variable name related changes in the refactor.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
  src/tests/containerizer/provisioner_docker_tests.cpp 
99c2af7d06dd7908e8d77e1bfb3e20622fb3eb4c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 39017: RegistryClient refactor: changed Manifest struct

2015-11-01 Thread Jojy Varghese

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

(Updated Nov. 1, 2015, 3:33 p.m.)


Review request for mesos and Ben Mahler.


Summary (updated)
-

RegistryClient refactor: changed Manifest struct


Repository: mesos


Description (updated)
---

Added factory method.
 Removed digest.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
ad57d8592c5f9df5115b575855ed2e99a0597359 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 39837: Passing os environment variables when start docker executor.

2015-11-01 Thread Jojy Varghese


> On Nov. 1, 2015, 4:29 p.m., Timothy Chen wrote:
> > src/slave/containerizer/docker.cpp, line 302
> > 
> >
> > We intentionally exclude os enviornments as you can see here for a 
> > reason.
> > 
> > There are a lot of environment variables that are default to the OS, 
> > that will break when docker containers run and we need to not include them 
> > unless it's specifically specified by the user with executor environement 
> > variables or taskinfo.
> > 
> > The fix should be carefully picking the ones we need only.
> 
> haosdent huang wrote:
> How about only include `MESOS_` and `LIBPROCESS_` prefix environment 
> variables default?

In this particular case, it looks like we DONT include the SSL environment 
variables in the executor process which breaks it. So maybe we can address this 
particular issue for now by adding SSL env variables. Also we need a better way 
to express the idea of adding specific environment variables in the executor 
process(maybe something like a "include" and "exclude" filters)


- Jojy


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


On Nov. 1, 2015, 9:38 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39837/
> ---
> 
> (Updated Nov. 1, 2015, 9:38 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3815
> https://issues.apache.org/jira/browse/MESOS-3815
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Passing os environment variables when start docker executor.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 5e5e0f93123b7f0bda6c8afeb4df271e796e4637 
> 
> Diff: https://reviews.apache.org/r/39837/diff/
> 
> 
> Testing
> ---
> 
> manually test.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 39840: RegistryClient refactor: pulled up streaming read

2015-11-01 Thread Jojy Varghese

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

(Updated Nov. 1, 2015, 3:52 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

removed overloaded methd.


Repository: mesos


Description
---

Made reading the stream from the PIPE common.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 39015: RegistryClient refactor: expanded abbreviated names.

2015-11-01 Thread Jojy Varghese

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

(Updated Nov. 1, 2015, 3:29 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased.


Repository: mesos


Description
---

RegistryClient refactor: expanded abbreviated names.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
ad57d8592c5f9df5115b575855ed2e99a0597359 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 39016: RegistryClient refactor: refactored lambdas

2015-11-01 Thread Jojy Varghese

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

(Updated Nov. 1, 2015, 3:30 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased.


Repository: mesos


Description
---

RegistryClient refactor: refactored lambdas as per review comments.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 

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


Testing
---

Make check.


Thanks,

Jojy Varghese



Re: Review Request 38579: Refactored registry client

2015-11-01 Thread Jojy Varghese

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

(Updated Nov. 1, 2015, 3:28 p.m.)


Review request for mesos, Ben Mahler and Timothy Chen.


Changes
---

rebased.


Repository: mesos


Description
---

- Cleanup and broke big methods into smaller chunks.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
ad57d8592c5f9df5115b575855ed2e99a0597359 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
  src/tests/containerizer/provisioner_docker_tests.cpp 
99c2af7d06dd7908e8d77e1bfb3e20622fb3eb4c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 39837: Passing os environment variables when start docker executor.

2015-11-01 Thread haosdent huang


> On Nov. 1, 2015, 4:29 p.m., Timothy Chen wrote:
> > src/slave/containerizer/docker.cpp, line 302
> > 
> >
> > We intentionally exclude os enviornments as you can see here for a 
> > reason.
> > 
> > There are a lot of environment variables that are default to the OS, 
> > that will break when docker containers run and we need to not include them 
> > unless it's specifically specified by the user with executor environement 
> > variables or taskinfo.
> > 
> > The fix should be carefully picking the ones we need only.

How about only include `MESOS_` and `LIBPROCESS_` prefix environment variables 
default?


- haosdent


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


On Nov. 1, 2015, 9:38 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39837/
> ---
> 
> (Updated Nov. 1, 2015, 9:38 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3815
> https://issues.apache.org/jira/browse/MESOS-3815
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Passing os environment variables when start docker executor.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 5e5e0f93123b7f0bda6c8afeb4df271e796e4637 
> 
> Diff: https://reviews.apache.org/r/39837/diff/
> 
> 
> Testing
> ---
> 
> manually test.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 39837: Passing os environment variables when start docker executor.

2015-11-01 Thread Jojy Varghese

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


How did you test this? Could you please elaborate the test steps? Or even 
better - add a test case?

- Jojy Varghese


On Nov. 1, 2015, 9:38 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39837/
> ---
> 
> (Updated Nov. 1, 2015, 9:38 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3815
> https://issues.apache.org/jira/browse/MESOS-3815
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Passing os environment variables when start docker executor.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 5e5e0f93123b7f0bda6c8afeb4df271e796e4637 
> 
> Diff: https://reviews.apache.org/r/39837/diff/
> 
> 
> Testing
> ---
> 
> manually test.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Review Request 39838: RegistryClient refactor: fixed getManifest test

2015-11-01 Thread Jojy Varghese

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Test was broken due to refactor changes that changed getManifest API.


Diffs
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
99c2af7d06dd7908e8d77e1bfb3e20622fb3eb4c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 39250: Puller refactor: moved untar to a common place

2015-11-01 Thread Jojy Varghese

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

(Updated Nov. 1, 2015, 3:39 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

rebased.


Repository: mesos


Description
---

Puller refactor: moved untar to a common place


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
f314f20d989ed0125e15d2a14d0f8e56c56976d5 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
8010b8aa75a2fc2e4b537f915f9dca028e407b63 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
  src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 38580: Added docker registry RemotePuller

2015-11-01 Thread Jojy Varghese

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

(Updated Nov. 1, 2015, 3:38 p.m.)


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


Changes
---

rebased.


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/CMakeLists.txt d107e329cc6887cd9d4ce3706dfc6ce6080d0289 
  src/Makefile.am d6eb302f0e812a777f51f421deef89140871a1db 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
87d80026939a326bd1169f46906e36d6ef19f546 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
f314f20d989ed0125e15d2a14d0f8e56c56976d5 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
8010b8aa75a2fc2e4b537f915f9dca028e407b63 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
f61f9e5e9e97a771a03b75ff17cfcd3e3ecbc9b2 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
ad57d8592c5f9df5115b575855ed2e99a0597359 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 
  src/slave/containerizer/mesos/provisioner/docker/remote_puller.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp 
PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
bb02d650e16d45fcf337a7954f7a26143fb2c69f 
  src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
  src/slave/flags.cpp ed9b0b8313f5a5e53f3715af5300d9fcaa936df8 
  src/tests/containerizer/provisioner_docker_tests.cpp 
99c2af7d06dd7908e8d77e1bfb3e20622fb3eb4c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 38747: Adding digest utilities

2015-11-01 Thread Jojy Varghese

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

(Updated Nov. 1, 2015, 3:38 p.m.)


Review request for mesos, Ben Mahler, Gilbert Song, and Timothy Chen.


Changes
---

rebased


Repository: mesos


Description
---

Added SHA256, SHA512 implementation.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 404a7438e7cd53d9295fe6025b5af5fb83df939b 
  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
99c9754b6a5c7ceb12808a782da9cb9fb3fc731e 
  3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 39837: Passing os environment variables when start docker executor.

2015-11-01 Thread haosdent huang


> On Nov. 1, 2015, 5:56 p.m., Jojy Varghese wrote:
> > How did you test this? Could you please elaborate the test steps? Or even 
> > better - add a test case?

Need add a test case.


- haosdent


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


On Nov. 1, 2015, 9:38 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39837/
> ---
> 
> (Updated Nov. 1, 2015, 9:38 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3815
> https://issues.apache.org/jira/browse/MESOS-3815
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Passing os environment variables when start docker executor.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 5e5e0f93123b7f0bda6c8afeb4df271e796e4637 
> 
> Diff: https://reviews.apache.org/r/39837/diff/
> 
> 
> Testing
> ---
> 
> manually test.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 39841: WIP: Used cgroups::enabled() to check for the availability of the freezer.

2015-11-01 Thread Artem Harutyunyan

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

(Updated Nov. 1, 2015, 5:36 p.m.)


Review request for mesos, Jie Yu and Joris Van Remoortere.


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


Repository: mesos


Description
---

WIP: Used cgroups::enabled() to check for the availability of the freezer.


Diffs
-

  src/slave/containerizer/linux_launcher.cpp 
c0adb34771fdb5a85d087296a8f98b890254ddf7 

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


Testing (updated)
---

This should solve the problem reported in 
https://issues.apache.org/jira/browse/MESOS-3814, however with this test in 
Docker still fail. The reason is that `freezer` subsytem is actually enabled in 
docker, however the Linux launcher fails because the file system is not mounted.


Thanks,

Artem Harutyunyan



Re: Review Request 39841: WIP: Used cgroups::enabled() to check for the availability of the freezer.

2015-11-01 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [39841]

All tests passed.

- Mesos ReviewBot


On Nov. 2, 2015, 1:36 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39841/
> ---
> 
> (Updated Nov. 2, 2015, 1:36 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3814
> https://issues.apache.org/jira/browse/MESOS-3814
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Used cgroups::enabled() to check for the availability of the freezer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> c0adb34771fdb5a85d087296a8f98b890254ddf7 
> 
> Diff: https://reviews.apache.org/r/39841/diff/
> 
> 
> Testing
> ---
> 
> This should solve the problem reported in 
> https://issues.apache.org/jira/browse/MESOS-3814, however with this test in 
> Docker still fail. The reason is that `freezer` subsytem is actually enabled 
> in docker, however the Linux launcher fails because the file system is not 
> mounted.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>



Review Request 39841: WIP: Used cgroups::enabled() to check for the availability of the freezer.

2015-11-01 Thread Artem Harutyunyan

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

Review request for mesos, Jie Yu and Joris Van Remoortere.


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


Repository: mesos


Description
---

WIP: Used cgroups::enabled() to check for the availability of the freezer.


Diffs
-

  src/slave/containerizer/linux_launcher.cpp 
c0adb34771fdb5a85d087296a8f98b890254ddf7 

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


Testing
---


Thanks,

Artem Harutyunyan



Re: Review Request 39604: Added function that verifies prerequisites for using Linux launcher.

2015-11-01 Thread Artem Harutyunyan


> On Oct. 30, 2015, 2:16 p.m., Jie Yu wrote:
> > src/slave/containerizer/linux_launcher.cpp, line 190
> > 
> >
> > OK, this is problematic and almost caused us an incident at Twitter.
> > 
> > Mesos is able to create 'freezer' hierarchy if it's not already 
> > mounted. In some configurations, the host won't pre-mount all cgroups 
> > hierarchies. We should remove that check here.
> 
> Jie Yu wrote:
> Chatted with Artem, a better way is to check: cgroups::enabled("freezer") 
> here

https://reviews.apache.org/r/39841/


- Artem


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


On Oct. 23, 2015, 4:48 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39604/
> ---
> 
> (Updated Oct. 23, 2015, 4:48 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Joris Van Remoortere, and Kapil 
> Arya.
> 
> 
> Bugs: MESOS-3800
> https://issues.apache.org/jira/browse/MESOS-3800
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We recently switched to using LinuxLauncher by default 
> (https://github.com/apache/mesos/blame/master/src/slave/containerizer/mesos/containerizer.cpp#L217-L234).
>  This causes problems with running the slave in a Docker container. 
> 
> This patch introduces a function for checking whether the machine is suited 
> for running Linux launcher. For the time being it checks whether the freezer 
> cgroup subsystem enabled, as the Linux launcher evolves we might want to add 
> more checks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.hpp 
> 35b3315498d690ed66616617aa7d51455371fb5b 
>   src/slave/containerizer/linux_launcher.cpp 
> c03b89eb0678825b03a052874d6262f377a39e13 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d1fc5a460e7313828014eea999cf4e63dde01921 
> 
> Diff: https://reviews.apache.org/r/39604/diff/
> 
> 
> Testing
> ---
> 
> - Ran Mesoss tests in a Docker container where cgroup was not available.
> - Ran the new Jenkins script (https://reviews.apache.org/r/37787/).
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>



Review Request 39843: Update declineOffer use Call::DECLINE to decline offer

2015-11-01 Thread Guangya Liu

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

The unit test is already covered in scheduler_tests.cpp by
TEST_P(SchedulerTest, Decline)


Diffs
-

  src/sched/sched.cpp 9c5e3b8e42605f4647d897ba02ea3a17e494f355 

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


Testing
---

make
make check


Thanks,

Guangya Liu



Re: Review Request 39837: Passing os environment variables when start docker executor.

2015-11-01 Thread haosdent huang


> On Nov. 1, 2015, 4:29 p.m., Timothy Chen wrote:
> > src/slave/containerizer/docker.cpp, line 302
> > 
> >
> > We intentionally exclude os enviornments as you can see here for a 
> > reason.
> > 
> > There are a lot of environment variables that are default to the OS, 
> > that will break when docker containers run and we need to not include them 
> > unless it's specifically specified by the user with executor environement 
> > variables or taskinfo.
> > 
> > The fix should be carefully picking the ones we need only.
> 
> haosdent huang wrote:
> How about only include `MESOS_` and `LIBPROCESS_` prefix environment 
> variables default?
> 
> Jojy Varghese wrote:
> In this particular case, it looks like we DONT include the SSL 
> environment variables in the executor process which breaks it. So maybe we 
> can address this particular issue for now by adding SSL env variables. Also 
> we need a better way to express the idea of adding specific environment 
> variables in the executor process(maybe something like a "include" and 
> "exclude" filters)

Agree. So let me change to only include SSL environment variables.


- haosdent


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


On Nov. 1, 2015, 9:38 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39837/
> ---
> 
> (Updated Nov. 1, 2015, 9:38 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3815
> https://issues.apache.org/jira/browse/MESOS-3815
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Passing os environment variables when start docker executor.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 5e5e0f93123b7f0bda6c8afeb4df271e796e4637 
> 
> Diff: https://reviews.apache.org/r/39837/diff/
> 
> 
> Testing
> ---
> 
> manually test.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-11-01 Thread Guangya Liu


> On 十月 25, 2015, 1:39 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1035
> > 
> >
> > I know that we have design to exclue the reserved resource from quota, 
> > but why not include the current role's reserved as available?
> 
> Alexander Rukletsov wrote:
> If you mean dynamically reserved resources than yes, we can do that. If 
> you mean statically reserved than nope, we decided not to conflate them with 
> quota.

I mean both static and dynamic reservation, I think that both of the 
reservations for one role should be included in the role's quota, can you 
pleaes clairify why not include static reservation in one role's quota?


> On 十月 25, 2015, 1:39 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1058
> > 
> >
> > Where does those offerable resource offered to master?
> 
> Alexander Rukletsov wrote:
> 
> https://github.com/apache/mesos/blob/master/src/master/allocator/mesos/hierarchical.cpp#L956
> 
> Dropping the issue, feel free to reopen if I haven't answered your 
> question.

I did not see that those resources are offered to master via the offer 
callback, but I see that you already updated code diff to resolve this. ;-)


> On 十月 25, 2015, 1:39 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 982
> > 
> >
> > s/unsatisfiedRoleQuotas/unAllocatedRoleQuotas?
> 
> Alexander Rukletsov wrote:
> Dynamic reservations may not be allocated, but should still count towards 
> quota.

But this is actually not allocated role quota, can we use a more meaningful 
name? The name of `unsatisfiedRoleQuotas` is more likely to be the value that 
the role quota needed to fill in its quota guarantee value. Comments?


- Guangya


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


On 十月 29, 2015, 7:29 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39401/
> ---
> 
> (Updated 十月 29, 2015, 7:29 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3718
> https://issues.apache.org/jira/browse/MESOS-3718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39401/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Re: Review Request 39340: RegistryClient: Added streaming response read

2015-11-01 Thread Jojy Varghese

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

(Updated Nov. 1, 2015, 3:35 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased.


Repository: mesos


Description
---

RegistryClient: Added streaming response read


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Review Request 39840: RegistryClient refactor: pulled up streaming read

2015-11-01 Thread Jojy Varghese

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Made reading the stream from the PIPE common.


Diffs
-

  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e4d2c22cf6627c1c76ebafeeb84b2bbf6b8c238c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 39388: Explicitly set the `LIBPROCESS_IP` env variable for docker containers.

2015-11-01 Thread Timothy Chen

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



src/docker/docker.cpp (line 433)


I think there are two problems here to address:
- --net is not always set to host, and if it's not it's causing problem as 
LIBPROCESS_IP takes precedence and also causes framework to not be able to 
connect
- docker code here in src/docker/docker.cpp is meant to be an abstraction 
to run docker, which we use to run executors and docker tasks. It shouldn't 
really have logic like this embedded especialy when this is not the desire 
effect for all docker containers we ever launch from Mesos. And really this 
problem and setting has to be two cases 1) We run a executor in the docker 
container 2) --net is host. We need to specifically solve this outside of the 
docker abstraction.


- Timothy Chen


On Oct. 17, 2015, 11:18 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39388/
> ---
> 
> (Updated Oct. 17, 2015, 11:18 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3740
> https://issues.apache.org/jira/browse/MESOS-3740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 56d63dc75637c9f89a239af371f476a85a570696 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4bb65afd0ee61cafef68e064a697fdce65d60058 
> 
> Diff: https://reviews.apache.org/r/39388/diff/
> 
> 
> Testing
> ---
> 
> Added `DockerContainerizerTest.ROOT_DOCKER_LaunchWithLibprocessIP` test which 
> fails without the changes made to `src/docker/docker.cpp`.
> 
> 
> Thanks,
> 
> Michael Park
> 
>



Re: Review Request 39388: Explicitly set the `LIBPROCESS_IP` env variable for docker containers.

2015-11-01 Thread Timothy Chen


> On Oct. 18, 2015, 4:10 a.m., haosdent huang wrote:
> > src/docker/docker.cpp, line 430
> > 
> >
> > Seems we already have this env in 
> > https://github.com/apache/mesos/blob/master/src/slave/containerizer/containerizer.cpp#L254
> 
> Klaus Ma wrote:
> I think those environment are used for `docker run` instead of passing 
> them to the container.
> 
> Michael Park wrote:
> That one is used to pass the `LIBPROCES_IP` to the `subprocess` call when 
> we're launching a process, whereas this is used to pass the `LIBPROCESS_IP` 
> to `docker->run` when we're launching a container via `-e`.
> 
> haosdent huang wrote:
> Thank you very much for explanation, got it!
> 
> haosdent huang wrote:
> Doe we change the line in docker executor could solve this problem?
> ```
> diff --git a/src/docker/executor.cpp b/src/docker/executor.cpp
> index 1e49013..a125078 100644
> --- a/src/docker/executor.cpp
> +++ b/src/docker/executor.cpp
> @@ -147,7 +147,7 @@ public:
>  sandboxDirectory,
>  mappedDirectory,
>  task.resources() + task.executor().resources(),
> -None(),
> +os::environment(),
>  path::join(sandboxDirectory, "stdout"),
>  path::join(sandboxDirectory, "stderr"))
>.onAny(defer(
> ```
> 
> haosdent huang wrote:
> Or how about serialize the environment variables to docker flag and pass 
> it to `docker->run` if `os::environment` noisy?
> ```
> diff --git a/src/slave/containerizer/docker.cpp 
> b/src/slave/containerizer/docker.cpp
> index 7022958..d9b23d2 100644
> --- a/src/slave/containerizer/docker.cpp
> +++ b/src/slave/containerizer/docker.cpp
> @@ -911,7 +911,7 @@ Future 
> DockerContainerizerProcess::launchExecutorProcess(
>Subprocess::PIPE(),
>Subprocess::PATH(path::join(container->directory, "stdout")),
>Subprocess::PATH(path::join(container->directory, "stderr")),
> -  dockerFlags(flags, container->name(), container->directory),
> +  dockerFlags(flags, container->name(), container->directory, 
> environment),
>environment,
>lambda::bind(, container->directory));
> ```

We should just solve the cases we know required and avoid passing environment 
variables if don't need to. We have found various problems passing down env 
variables and doing os::environment or environment will cause more pain in the 
future.


- Timothy


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


On Oct. 17, 2015, 11:18 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39388/
> ---
> 
> (Updated Oct. 17, 2015, 11:18 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3740
> https://issues.apache.org/jira/browse/MESOS-3740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 56d63dc75637c9f89a239af371f476a85a570696 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4bb65afd0ee61cafef68e064a697fdce65d60058 
> 
> Diff: https://reviews.apache.org/r/39388/diff/
> 
> 
> Testing
> ---
> 
> Added `DockerContainerizerTest.ROOT_DOCKER_LaunchWithLibprocessIP` test which 
> fails without the changes made to `src/docker/docker.cpp`.
> 
> 
> Thanks,
> 
> Michael Park
> 
>



Re: Review Request 39388: Explicitly set the `LIBPROCESS_IP` env variable for docker containers.

2015-11-01 Thread haosdent huang


> On Oct. 18, 2015, 4:10 a.m., haosdent huang wrote:
> > src/docker/docker.cpp, line 430
> > 
> >
> > Seems we already have this env in 
> > https://github.com/apache/mesos/blob/master/src/slave/containerizer/containerizer.cpp#L254
> 
> Klaus Ma wrote:
> I think those environment are used for `docker run` instead of passing 
> them to the container.
> 
> Michael Park wrote:
> That one is used to pass the `LIBPROCES_IP` to the `subprocess` call when 
> we're launching a process, whereas this is used to pass the `LIBPROCESS_IP` 
> to `docker->run` when we're launching a container via `-e`.
> 
> haosdent huang wrote:
> Thank you very much for explanation, got it!
> 
> haosdent huang wrote:
> Doe we change the line in docker executor could solve this problem?
> ```
> diff --git a/src/docker/executor.cpp b/src/docker/executor.cpp
> index 1e49013..a125078 100644
> --- a/src/docker/executor.cpp
> +++ b/src/docker/executor.cpp
> @@ -147,7 +147,7 @@ public:
>  sandboxDirectory,
>  mappedDirectory,
>  task.resources() + task.executor().resources(),
> -None(),
> +os::environment(),
>  path::join(sandboxDirectory, "stdout"),
>  path::join(sandboxDirectory, "stderr"))
>.onAny(defer(
> ```
> 
> haosdent huang wrote:
> Or how about serialize the environment variables to docker flag and pass 
> it to `docker->run` if `os::environment` noisy?
> ```
> diff --git a/src/slave/containerizer/docker.cpp 
> b/src/slave/containerizer/docker.cpp
> index 7022958..d9b23d2 100644
> --- a/src/slave/containerizer/docker.cpp
> +++ b/src/slave/containerizer/docker.cpp
> @@ -911,7 +911,7 @@ Future 
> DockerContainerizerProcess::launchExecutorProcess(
>Subprocess::PIPE(),
>Subprocess::PATH(path::join(container->directory, "stdout")),
>Subprocess::PATH(path::join(container->directory, "stderr")),
> -  dockerFlags(flags, container->name(), container->directory),
> +  dockerFlags(flags, container->name(), container->directory, 
> environment),
>environment,
>lambda::bind(, container->directory));
> ```
> 
> Timothy Chen wrote:
> We should just solve the cases we know required and avoid passing 
> environment variables if don't need to. We have found various problems 
> passing down env variables and doing os::environment or environment will 
> cause more pain in the future.

Got it. Thank you very much.


- haosdent


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


On Oct. 17, 2015, 11:18 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39388/
> ---
> 
> (Updated Oct. 17, 2015, 11:18 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3740
> https://issues.apache.org/jira/browse/MESOS-3740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 56d63dc75637c9f89a239af371f476a85a570696 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4bb65afd0ee61cafef68e064a697fdce65d60058 
> 
> Diff: https://reviews.apache.org/r/39388/diff/
> 
> 
> Testing
> ---
> 
> Added `DockerContainerizerTest.ROOT_DOCKER_LaunchWithLibprocessIP` test which 
> fails without the changes made to `src/docker/docker.cpp`.
> 
> 
> Thanks,
> 
> Michael Park
> 
>



Re: Review Request 37336: Simplified the caller interface to process::Subprocess

2015-11-01 Thread Marco Massenzio

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

(Updated Nov. 2, 2015, 7:22 a.m.)


Review request for mesos, Joris Van Remoortere and Michael Park.


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


Repository: mesos


Description
---

Jira: MESOS-3035

The original API for `process::Subprocess` still left a lot of legwork
to do for the caller; we have now added an `execute()` method
that returns a `Future` (also introduced with this patch) 
which
contains useful information about the command invocation; the exit code;
stdout and, optionally, stderr too.

Once the Future completes, if successful, the caller will be able to 
retrieve
stdout/stderr; whether the command was successful; and whether it received 
a signal


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess.hpp 
f17816e813d5efce1d3bb1ff1e850eeda3ba 
  3rdparty/libprocess/src/subprocess.cpp 
459825c188d56d25f6e2832e5a170d806e257d6b 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
ac600a551fb1a7782ff33cce204b7819497ef54a 

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


Testing
---

make check


Thanks,

Marco Massenzio