Re: Review Request 25270: Enable bridge network in Mesos

2014-09-16 Thread Benjamin Hindman

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

Ship it!



src/tests/docker_containerizer_tests.cpp
https://reviews.apache.org/r/25270/#comment93221

Awesome test! Can we add a comment above that explains in text how this 
differs from the other bridged test? In particular, it would be nice for people 
to understand that you have two tests here, both testing bridged networking, 
one which tests for an executor, the other which tests for a task and also 
makes sure the port mapping works. Thanks Tim!



src/tests/docker_containerizer_tests.cpp
https://reviews.apache.org/r/25270/#comment93227

Let's move this down to where it's actually used. ;-)



src/tests/docker_containerizer_tests.cpp
https://reviews.apache.org/r/25270/#comment93218

Separating these stanzas would make them easier to read (as we do when 
setting up other protobufs).



src/tests/docker_tests.cpp
https://reviews.apache.org/r/25270/#comment93225

Just a random question, what happens if /mnt/mesos/sandbox doesn't exist?



src/tests/docker_tests.cpp
https://reviews.apache.org/r/25270/#comment93223

Please split these up like above!



src/tests/docker_tests.cpp
https://reviews.apache.org/r/25270/#comment93224

Please use AWAIT_EXPECT_FAILED, otherwise when this code actually becomes 
asynchronous this test is going to start non-deterministically failing.


- Benjamin Hindman


On Sept. 11, 2014, 5:48 p.m., Timothy Chen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25270/
 ---
 
 (Updated Sept. 11, 2014, 5:48 p.m.)
 
 
 Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy St. Clair.
 
 
 Bugs: MESOS-1621
 https://issues.apache.org/jira/browse/MESOS-1621
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Review: https://reviews.apache.org/r/25270
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/docker_containerizer_tests.cpp 
 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
   src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 
 
 Diff: https://reviews.apache.org/r/25270/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Timothy Chen
 




Re: Review Request 25270: Enable bridge network in Mesos

2014-09-16 Thread Timothy Chen

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

(Updated Sept. 16, 2014, 11:04 p.m.)


Review request for drill, Benjamin Hindman, Jie Yu, and Timothy St. Clair.


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


Repository: mesos-git


Description
---

Review: https://reviews.apache.org/r/25270


Diffs (updated)
-

  include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
  src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e 
  src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
  src/tests/docker_containerizer_tests.cpp 
8654f9c787bd207f6a7b821651e0c083bea9dc8a 
  src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 

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


Testing
---

make check


Thanks,

Timothy Chen



Re: Review Request 25270: Enable bridge network in Mesos

2014-09-16 Thread Timothy Chen

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

(Updated Sept. 16, 2014, 11:04 p.m.)


Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy St. Clair.


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


Repository: mesos-git


Description
---

Review: https://reviews.apache.org/r/25270


Diffs
-

  include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
  src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e 
  src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
  src/tests/docker_containerizer_tests.cpp 
8654f9c787bd207f6a7b821651e0c083bea9dc8a 
  src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 

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


Testing
---

make check


Thanks,

Timothy Chen



Re: Review Request 25270: Enable bridge network in Mesos

2014-09-15 Thread Derek Zhang

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

Ship it!


Ship It!

- Derek Zhang


On Sept. 11, 2014, 5:48 p.m., Timothy Chen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25270/
 ---
 
 (Updated Sept. 11, 2014, 5:48 p.m.)
 
 
 Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy St. Clair.
 
 
 Bugs: MESOS-1621
 https://issues.apache.org/jira/browse/MESOS-1621
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Review: https://reviews.apache.org/r/25270
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/docker_containerizer_tests.cpp 
 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
   src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 
 
 Diff: https://reviews.apache.org/r/25270/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Timothy Chen
 




Re: Review Request 25270: Enable bridge network in Mesos

2014-09-15 Thread Bhuvan Arumugam


 On Sept. 15, 2014, 6 a.m., Derek Zhang wrote:
  Ship It!

any more volunteers to bless  submit this patch?


- Bhuvan


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


On Sept. 11, 2014, 5:48 p.m., Timothy Chen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25270/
 ---
 
 (Updated Sept. 11, 2014, 5:48 p.m.)
 
 
 Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy St. Clair.
 
 
 Bugs: MESOS-1621
 https://issues.apache.org/jira/browse/MESOS-1621
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Review: https://reviews.apache.org/r/25270
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/docker_containerizer_tests.cpp 
 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
   src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 
 
 Diff: https://reviews.apache.org/r/25270/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Timothy Chen
 




Re: Review Request 25270: Enable bridge network in Mesos

2014-09-14 Thread Bhuvan Arumugam

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



src/tests/docker_containerizer_tests.cpp
https://reviews.apache.org/r/25270/#comment92903

Timothy Chen: do you intend to use the image mesosphere/test-executor?


- Bhuvan Arumugam


On Sept. 11, 2014, 5:48 p.m., Timothy Chen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25270/
 ---
 
 (Updated Sept. 11, 2014, 5:48 p.m.)
 
 
 Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy St. Clair.
 
 
 Bugs: MESOS-1621
 https://issues.apache.org/jira/browse/MESOS-1621
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Review: https://reviews.apache.org/r/25270
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/docker_containerizer_tests.cpp 
 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
   src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 
 
 Diff: https://reviews.apache.org/r/25270/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Timothy Chen
 




Re: Review Request 25270: Enable bridge network in Mesos

2014-09-14 Thread Timothy Chen


 On Sept. 15, 2014, 12:44 a.m., Bhuvan Arumugam wrote:
  src/tests/docker_containerizer_tests.cpp, line 243
  https://reviews.apache.org/r/25270/diff/5/?file=685937#file685937line243
 
  Timothy Chen: do you intend to use the image mesosphere/test-executor?

No I actually intend to use the new image, I don't have access to update that 
image so I created a new one.
The test-executor is a native go implementation of the api, and was lacking 
what libprocess was doing which is to publish PID with publish host ip, and it 
used to only subscribe as 0.0.0.0 which won't work with bridge network.


- Timothy


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


On Sept. 11, 2014, 5:48 p.m., Timothy Chen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25270/
 ---
 
 (Updated Sept. 11, 2014, 5:48 p.m.)
 
 
 Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy St. Clair.
 
 
 Bugs: MESOS-1621
 https://issues.apache.org/jira/browse/MESOS-1621
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Review: https://reviews.apache.org/r/25270
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/docker_containerizer_tests.cpp 
 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
   src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 
 
 Diff: https://reviews.apache.org/r/25270/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Timothy Chen
 




Re: Review Request 25270: Enable bridge network in Mesos

2014-09-11 Thread Timothy Chen


 On Sept. 9, 2014, 6:48 p.m., Benjamin Hindman wrote:
  src/tests/docker_containerizer_tests.cpp, line 243
  https://reviews.apache.org/r/25270/diff/4/?file=680521#file680521line243
 
  Ultimately it would be great to get this under the 'mesos' registry so 
  other folks can easily make changes to the executor too.

Yes however I don't know who has access to mesos yet, we'll do this later.


- Timothy


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


On Sept. 5, 2014, 4:55 p.m., Timothy Chen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25270/
 ---
 
 (Updated Sept. 5, 2014, 4:55 p.m.)
 
 
 Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy St. Clair.
 
 
 Bugs: MESOS-1621
 https://issues.apache.org/jira/browse/MESOS-1621
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Review: https://reviews.apache.org/r/25270
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/common/resources.cpp 29fc765f1a4c023d91e348cacd5b6333d351a5ac 
   src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e 
   src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525 
   src/tests/docker_containerizer_tests.cpp 
 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
   src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 
 
 Diff: https://reviews.apache.org/r/25270/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Timothy Chen
 




Re: Review Request 25270: Enable bridge network in Mesos

2014-09-11 Thread Timothy Chen

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

(Updated Sept. 11, 2014, 5:48 p.m.)


Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy St. Clair.


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


Repository: mesos-git


Description
---

Review: https://reviews.apache.org/r/25270


Diffs (updated)
-

  include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
  src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e 
  src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
  src/tests/docker_containerizer_tests.cpp 
8654f9c787bd207f6a7b821651e0c083bea9dc8a 
  src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 

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


Testing
---

make check


Thanks,

Timothy Chen



Re: Review Request 25270: Enable bridge network in Mesos

2014-09-11 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [25270]

All tests passed.

- Mesos ReviewBot


On Sept. 11, 2014, 5:48 p.m., Timothy Chen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25270/
 ---
 
 (Updated Sept. 11, 2014, 5:48 p.m.)
 
 
 Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy St. Clair.
 
 
 Bugs: MESOS-1621
 https://issues.apache.org/jira/browse/MESOS-1621
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Review: https://reviews.apache.org/r/25270
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/docker_containerizer_tests.cpp 
 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
   src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 
 
 Diff: https://reviews.apache.org/r/25270/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Timothy Chen
 




Re: Review Request 25270: Enable bridge network in Mesos

2014-09-09 Thread Benjamin Hindman

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


Looking good!


include/mesos/mesos.proto
https://reviews.apache.org/r/25270/#comment91819

Any reason not to do s/NetworkMode/Network/ instead?



include/mesos/mesos.proto
https://reviews.apache.org/r/25270/#comment91824

Do you want to make this an enum too? Maybe not, but what happens if 
someone puts in 'TCP' instead of 'tcp'?



src/docker/docker.cpp
https://reviews.apache.org/r/25270/#comment91826

How about keeping this close to where we actually 'argv.push(network)' 
below please?



src/docker/docker.cpp
https://reviews.apache.org/r/25270/#comment91828





src/tests/docker_containerizer_tests.cpp
https://reviews.apache.org/r/25270/#comment91832

Ultimately it would be great to get this under the 'mesos' registry so 
other folks can easily make changes to the executor too.



src/tests/docker_tests.cpp
https://reviews.apache.org/r/25270/#comment91835

Is there something you actually want to test for? Like being able to send 
to a mapped port and have it show up on a command listening on the internal 
port? Is there 'nc' or 'telnet' in busybox that you can assume to get the 
output of any data sent to the internal port?


- Benjamin Hindman


On Sept. 5, 2014, 4:55 p.m., Timothy Chen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25270/
 ---
 
 (Updated Sept. 5, 2014, 4:55 p.m.)
 
 
 Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy St. Clair.
 
 
 Bugs: MESOS-1621
 https://issues.apache.org/jira/browse/MESOS-1621
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Review: https://reviews.apache.org/r/25270
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/common/resources.cpp 29fc765f1a4c023d91e348cacd5b6333d351a5ac 
   src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e 
   src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525 
   src/tests/docker_containerizer_tests.cpp 
 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
   src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 
 
 Diff: https://reviews.apache.org/r/25270/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Timothy Chen
 




Re: Review Request 25270: Enable bridge network in Mesos

2014-09-06 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [25270]

All tests passed.

- Mesos ReviewBot


On Sept. 5, 2014, 4:55 p.m., Timothy Chen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25270/
 ---
 
 (Updated Sept. 5, 2014, 4:55 p.m.)
 
 
 Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy St. Clair.
 
 
 Bugs: MESOS-1621
 https://issues.apache.org/jira/browse/MESOS-1621
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Review: https://reviews.apache.org/r/25270
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/common/resources.cpp 29fc765f1a4c023d91e348cacd5b6333d351a5ac 
   src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e 
   src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525 
   src/tests/docker_containerizer_tests.cpp 
 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
   src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 
 
 Diff: https://reviews.apache.org/r/25270/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Timothy Chen
 




Re: Review Request 25270: Enable bridge network in Mesos

2014-09-05 Thread Tom Arnfeld


 On Sept. 3, 2014, 10:59 a.m., Tom Arnfeld wrote:
  What's the reason for not also supporting the `port` resource type? For 
  example, the Hadoop framework uses this 
  https://github.com/mesos/hadoop/blob/master/src/main/java/org/apache/hadoop/mapred/ResourcePolicy.java#L458-L472.
   I noticed there's a TODO here 
  https://github.com/apache/mesos/blob/master/src/docker/docker.cpp#L256 to 
  support the ports resource.
  
  I think it'd be nice to support both this kind of PortMapping behaviour for 
  explicit mappings, as you've done in the DockerInfo protobuf, but if I just 
  need any old port that's available to bind to a resource type would be 
  useful.
 
 Timothy Chen wrote:
 What do you think that will look like with docker run? I'm trying to 
 think how a allocated port will then be translated into a mapping to the 
 container/host.

So if you're using the port resource, you're basically securing a single port 
for use by your container and your contianer only, from the host. I imagine 
this being identical to the current external containerizer implementations, 
using `-p 1234:1234` to map the allocated port on the host to the same port 
inside the container.


- Tom


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


On Sept. 4, 2014, 10:04 p.m., Timothy Chen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25270/
 ---
 
 (Updated Sept. 4, 2014, 10:04 p.m.)
 
 
 Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy St. Clair.
 
 
 Bugs: MESOS-1621
 https://issues.apache.org/jira/browse/MESOS-1621
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Review: https://reviews.apache.org/r/25270
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e 
   src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525 
   src/tests/docker_containerizer_tests.cpp 
 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
 
 Diff: https://reviews.apache.org/r/25270/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Timothy Chen
 




Re: Review Request 25270: Enable bridge network in Mesos

2014-09-05 Thread Timothy Chen


 On Sept. 3, 2014, 10:59 a.m., Tom Arnfeld wrote:
  What's the reason for not also supporting the `port` resource type? For 
  example, the Hadoop framework uses this 
  https://github.com/mesos/hadoop/blob/master/src/main/java/org/apache/hadoop/mapred/ResourcePolicy.java#L458-L472.
   I noticed there's a TODO here 
  https://github.com/apache/mesos/blob/master/src/docker/docker.cpp#L256 to 
  support the ports resource.
  
  I think it'd be nice to support both this kind of PortMapping behaviour for 
  explicit mappings, as you've done in the DockerInfo protobuf, but if I just 
  need any old port that's available to bind to a resource type would be 
  useful.
 
 Timothy Chen wrote:
 What do you think that will look like with docker run? I'm trying to 
 think how a allocated port will then be translated into a mapping to the 
 container/host.
 
 Tom Arnfeld wrote:
 So if you're using the port resource, you're basically securing a single 
 port for use by your container and your contianer only, from the host. I 
 imagine this being identical to the current external containerizer 
 implementations, using `-p 1234:1234` to map the allocated port on the host 
 to the same port inside the container.

Hi Tom, thanks for the comment, I chatted with several folks and yes the 
direction I'm going now is to have explicit mapping like you defined. I have 
that exactly in my very last update in this reviewboard.
What I'm also going to do besides just allowing explicit mapping, is to verify 
if you want to map a host port you must have a resources in your task info that 
has port resources for that port.


- Timothy


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


On Sept. 4, 2014, 10:04 p.m., Timothy Chen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25270/
 ---
 
 (Updated Sept. 4, 2014, 10:04 p.m.)
 
 
 Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy St. Clair.
 
 
 Bugs: MESOS-1621
 https://issues.apache.org/jira/browse/MESOS-1621
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Review: https://reviews.apache.org/r/25270
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e 
   src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525 
   src/tests/docker_containerizer_tests.cpp 
 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
 
 Diff: https://reviews.apache.org/r/25270/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Timothy Chen
 




Re: Review Request 25270: Enable bridge network in Mesos

2014-09-05 Thread Timothy Chen

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

(Updated Sept. 5, 2014, 7:50 a.m.)


Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy St. Clair.


Changes
---

Added port validation


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


Repository: mesos-git


Description
---

Review: https://reviews.apache.org/r/25270


Diffs (updated)
-

  include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
  src/common/resources.cpp 29fc765f1a4c023d91e348cacd5b6333d351a5ac 
  src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e 
  src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525 
  src/tests/docker_containerizer_tests.cpp 
8654f9c787bd207f6a7b821651e0c083bea9dc8a 
  src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 

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


Testing
---

make check


Thanks,

Timothy Chen



Re: Review Request 25270: Enable bridge network in Mesos

2014-09-05 Thread Mesos ReviewBot

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


Bad patch!

Reviews applied: [25270]

Failed command: ./support/mesos-style.py

Error:
 Checking 504 files using filter 
--filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/tab,+whitespace/todo
src/tests/docker_tests.cpp:248:  Lines should be = 80 characters long  
[whitespace/line_length] [2]
Total errors found: 1

- Mesos ReviewBot


On Sept. 5, 2014, 7:50 a.m., Timothy Chen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25270/
 ---
 
 (Updated Sept. 5, 2014, 7:50 a.m.)
 
 
 Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy St. Clair.
 
 
 Bugs: MESOS-1621
 https://issues.apache.org/jira/browse/MESOS-1621
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Review: https://reviews.apache.org/r/25270
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/common/resources.cpp 29fc765f1a4c023d91e348cacd5b6333d351a5ac 
   src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e 
   src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525 
   src/tests/docker_containerizer_tests.cpp 
 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
   src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 
 
 Diff: https://reviews.apache.org/r/25270/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Timothy Chen
 




Re: Review Request 25270: Enable bridge network in Mesos

2014-09-05 Thread Timothy Chen

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

(Updated Sept. 5, 2014, 4:55 p.m.)


Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy St. Clair.


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


Repository: mesos-git


Description
---

Review: https://reviews.apache.org/r/25270


Diffs (updated)
-

  include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
  src/common/resources.cpp 29fc765f1a4c023d91e348cacd5b6333d351a5ac 
  src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e 
  src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525 
  src/tests/docker_containerizer_tests.cpp 
8654f9c787bd207f6a7b821651e0c083bea9dc8a 
  src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 

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


Testing
---

make check


Thanks,

Timothy Chen



Re: Review Request 25270: Enable bridge network in Mesos

2014-09-04 Thread Timothy Chen

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

(Updated Sept. 4, 2014, 10:04 p.m.)


Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy St. Clair.


Summary (updated)
-

Enable bridge network in Mesos


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


Repository: mesos-git


Description (updated)
---

Review: https://reviews.apache.org/r/25270


Diffs (updated)
-

  include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
  src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e 
  src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525 
  src/tests/docker_containerizer_tests.cpp 
8654f9c787bd207f6a7b821651e0c083bea9dc8a 

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


Testing
---

make check


Thanks,

Timothy Chen