Re: Review Request 45471: Fix a test case bug due to upgrade http-parser to 2.6.1.

[Zhiwei Chen]

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

(Updated March 30, 2016, 1:35 p.m.)


Review request for mesos, Greg Mann and Vinod Kone.


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


Repository: mesos


Description
---

Fix a test case bug due to upgrade http-parser to 2.6.1.


Diffs
-

  3rdparty/libprocess/src/tests/ssl_tests.cpp 
53a0ed03226030104af43a45c6ccfbfc0f4b7e9f 

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


Testing
---

../configure --enable-libevent --enable-ssl

sudo make dist check


Thanks,

Zhiwei Chen

Re: Review Request 45471: Fix a test case bug due to upgrade http-parser to 2.6.1.

[Zhiwei Chen]

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

(Updated March 30, 2016, 1:33 p.m.)


Review request for mesos, Greg Mann and Vinod Kone.


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


Repository: mesos


Description
---

Fix a test case bug due to upgrade http-parser to 2.6.1.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/ssl_tests.cpp 
53a0ed03226030104af43a45c6ccfbfc0f4b7e9f 

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


Testing
---

../configure --enable-libevent --enable-ssl

sudo make dist check


Thanks,

Zhiwei Chen

Review Request 45471: Fix a test case bug due to upgrade http-parser to 2.6.1.

[Zhiwei Chen]

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

Review request for mesos, Greg Mann and Vinod Kone.


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


Repository: mesos


Description
---

Fix a test case bug due to upgrade http-parser to 2.6.1.


Diffs
-

  3rdparty/libprocess/src/tests/ssl_tests.cpp 
4d237815a03828b915e821c3af78132e2915c610 

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


Testing
---

../configure --enable-libevent --enable-ssl

sudo make dist check


Thanks,

Zhiwei Chen

Re: Review Request 45455: Added test for recovering orphaned docker containers.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45453, 45454, 45455]

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

- Mesos ReviewBot


On March 29, 2016, 10:44 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45455/
> ---
> 
> (Updated March 29, 2016, 10:44 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-3573
> https://issues.apache.org/jira/browse/MESOS-3573
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> b9c26b92c44e8ca718a5eb333855199bdf2a8e81 
> 
> Diff: https://reviews.apache.org/r/45455/diff/
> 
> 
> Testing
> ---
> 
> make check (gtest_repeat=100)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 42516: Add support for user-defined networks.

[Ezra Silvera]

> On March 29, 2016, 6:01 p.m., Avinash sridharan wrote:
> > src/docker/docker.cpp, line 568
> > 
> >
> > Maybe s/ Found .../Found multiple networks specified for this 
> > container. We can only attach docker containers to a single network/
> > 
> > We try to avoid variable names in `Error` since it becomes cryptic. 
> > Also, I guess the limiation is of the `DockerContainerizer` rather than 
> > `Docker`?

No, the limitation is from Docker. During "run/create" container you can 
specify only single netwrok. Only this command is supported n the contenerizer 
as it consume resources. From Docker you can later on call "network connect 
..." which allows you to connect the containe to aditional/multiple network. 
1) The "connect" command is not supported by the contenerizer
2) The implementation we now reviw is for the "run" command for which docker 
supports only a single network.


- Ezra


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


On March 29, 2016, 4:33 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 29, 2016, 4:33 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Re: Review Request 45231: Removed the use of subprocess setup functions [2/7].

[haosdent huang]

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



@joerg84 Your works related to fork performance looks awesome, sorry to public 
the review after it submitted. I am trying to rebase my patches today and see 
these wired changes related to comments.


src/tests/containerizer/isolator_tests.cpp (line 279)


`asingle` should be `a single`?



src/tests/containerizer/isolator_tests.cpp (line 561)


`preventblocking` should be `prevent blocking`?



src/tests/containerizer/isolator_tests.cpp (line 612)


`testwill` should be `test will`


- haosdent huang


On March 28, 2016, 4:51 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45231/
> ---
> 
> (Updated March 28, 2016, 4:51 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review follows the previous one and removes most
> (see following reviews) usages setup functions throughout
> the code.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
>   src/health-check/main.cpp 36955dd5f77a056213762d92c39be4ca7dd5780c 
>   src/slave/container_loggers/lib_logrotate.cpp 
> cf5f238eb3b6217b848cf23f8c9cd1848bf9a9f0 
>   src/slave/containerizer/docker.cpp 0133628f5b37dc0d3aa12f210c61dfc07deac18b 
>   src/slave/containerizer/external_containerizer.cpp 
> fe368dd52644b89e47ffd4d947de290b3998fb1a 
>   src/slave/containerizer/fetcher.cpp 
> 2b1e022c8dcd9734663f20fe963a8de0d6233715 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e7f7e7fd1304e14dbfaab8b53cea16efc0417911 
>   src/slave/containerizer/mesos/isolators/posix/disk.cpp 
> b38d83fbb29f46552ffbda7b17cbc85af15550e1 
>   src/slave/containerizer/mesos/launcher.hpp 
> 8afac20c5a4093162f764412dbf16c4230dcb141 
>   src/slave/containerizer/mesos/launcher.cpp 
> ba012b1d577a770948a1bfcad5ad2d4977f81b2f 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> 1763d8d48614e0a1e187505d90920e97c8e476fb 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 550c58263df257989568debe2858ef599e5f96e8 
>   src/tests/containerizer/isolator_tests.cpp 
> df506fc793e480e825b476e43c683ef8bcf676b2 
>   src/tests/containerizer/launch_tests.cpp 
> e0f934cf93f51316b35f34d34f3d0c5f004565d9 
>   src/tests/containerizer/launcher.hpp 
> 36405a97893f352147af4794fb39b6e621d95b58 
>   src/tests/containerizer/launcher.cpp 
> a92d9890f0931425d69ef8ce0896d081b8722079 
>   src/tests/containerizer/ns_tests.cpp 
> 4bf45e970bede713fa4ffce205627b149232fe2b 
>   src/tests/slave_tests.cpp 1f1a31020096efa5db698e86ac74e61dfdb4b94a 
> 
> Diff: https://reviews.apache.org/r/45231/diff/
> 
> 
> Testing
> ---
> 
> tested complete chain (see https://reviews.apache.org/r/45236/).
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45231: Removed the use of subprocess setup functions [2/7].

[haosdent huang]

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




src/tests/containerizer/isolator_tests.cpp (line 166)


I think here should be `one second`?


- haosdent huang


On March 28, 2016, 4:51 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45231/
> ---
> 
> (Updated March 28, 2016, 4:51 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review follows the previous one and removes most
> (see following reviews) usages setup functions throughout
> the code.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
>   src/health-check/main.cpp 36955dd5f77a056213762d92c39be4ca7dd5780c 
>   src/slave/container_loggers/lib_logrotate.cpp 
> cf5f238eb3b6217b848cf23f8c9cd1848bf9a9f0 
>   src/slave/containerizer/docker.cpp 0133628f5b37dc0d3aa12f210c61dfc07deac18b 
>   src/slave/containerizer/external_containerizer.cpp 
> fe368dd52644b89e47ffd4d947de290b3998fb1a 
>   src/slave/containerizer/fetcher.cpp 
> 2b1e022c8dcd9734663f20fe963a8de0d6233715 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e7f7e7fd1304e14dbfaab8b53cea16efc0417911 
>   src/slave/containerizer/mesos/isolators/posix/disk.cpp 
> b38d83fbb29f46552ffbda7b17cbc85af15550e1 
>   src/slave/containerizer/mesos/launcher.hpp 
> 8afac20c5a4093162f764412dbf16c4230dcb141 
>   src/slave/containerizer/mesos/launcher.cpp 
> ba012b1d577a770948a1bfcad5ad2d4977f81b2f 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> 1763d8d48614e0a1e187505d90920e97c8e476fb 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 550c58263df257989568debe2858ef599e5f96e8 
>   src/tests/containerizer/isolator_tests.cpp 
> df506fc793e480e825b476e43c683ef8bcf676b2 
>   src/tests/containerizer/launch_tests.cpp 
> e0f934cf93f51316b35f34d34f3d0c5f004565d9 
>   src/tests/containerizer/launcher.hpp 
> 36405a97893f352147af4794fb39b6e621d95b58 
>   src/tests/containerizer/launcher.cpp 
> a92d9890f0931425d69ef8ce0896d081b8722079 
>   src/tests/containerizer/ns_tests.cpp 
> 4bf45e970bede713fa4ffce205627b149232fe2b 
>   src/tests/slave_tests.cpp 1f1a31020096efa5db698e86ac74e61dfdb4b94a 
> 
> Diff: https://reviews.apache.org/r/45231/diff/
> 
> 
> Testing
> ---
> 
> tested complete chain (see https://reviews.apache.org/r/45236/).
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 44950: Add XFS disk isolator documentation.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [44950, 44949, 44948, 44947, 44946, 44945]

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

Error:
2016-03-30 02:51:59 URL:https://reviews.apache.org/r/44945/diff/raw/ 
[3292/3292] -> "44945.patch" [1]
error: patch failed: configure.ac:920
error: configure.ac: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/12208/console

- Mesos ReviewBot


On March 29, 2016, 8:51 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44950/
> ---
> 
> (Updated March 29, 2016, 8:51 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOs-4828
> https://issues.apache.org/jira/browse/MESOs-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add XFS disk isolator documentation.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 9ad0c2a201130ac23b22b1a844396dc97ec5ff70 
>   docs/mesos-containerizer.md 15fb5bdbe74e059614b8948108f32cd04b623305 
> 
> Diff: https://reviews.apache.org/r/44950/diff/
> 
> 
> Testing
> ---
> 
> Make check. Source inspection.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 45082: Implemented cleanup() method of "network/cni" isolator.

[Qian Zhang]

> On March 22, 2016, 11:37 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 644-645
> > 
> >
> > These two CHECKS don't make sense. What if the plugin got deleted or 
> > there was a bug in the plugin because of which it wasn't able to delete the 
> > interfaces or release the IP addresses. The Agent should not die because of 
> > an error in a 3rd party plugin.
> 
> Qian Zhang wrote:
> If the plugin got deleted, then `Subprocess.isError()` will be true and 
> we will return a `Failure`, if there was a bug in the plugin, then 
> `Subprocess.status()` will be non-zero and we will read its output for the 
> detailed error message. For either of these cases, agent will not die.
> 
> Avinash sridharan wrote:
> The fact that we are failing on the plugin returning an error seems very 
> dangerous. A malicious plugin can start crashing the Agent which is just bad 
> behavior. We should avoid this at all costs. CHECKS are more defensive to 
> test an internal logic, they should not be used for any external inputs. I 
> would rather throw an error and get out.

We do not have these two CHECKs in the latest patch anymore :-)


> On March 22, 2016, 11:37 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 647-649
> > 
> >
> > This is a bit odd, __disconnect always returns an error ? The plugin 
> > can return error codes and error logs which we should be propagating 
> > upstream through failure semantics.
> 
> Qian Zhang wrote:
> The reason that `__disconnect()` always returns a `Failure` is, it will 
> be only called when the exit code of plugin is not 0 (i.e., plugin fails for 
> a reason), in case of plugin success, we will return `Nothing()` in 
> `_disconnect()`. So we only need `__disconnect()` in case of plugin failure 
> to get the output (i.e., detailed failure reason) of plugin and propagate it 
> upstream.
> 
> Avinash sridharan wrote:
> I would suggest s/Failed to execute the CNI plugin/CNI plugin failed to 
> detach network/ . "Failed to execute ..." implies that we not able to launch 
> the plugin, which might not be the case here.

Agree!


> On March 22, 2016, 11:37 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 653
> > 
> >
> > Why are we returning a Future over here? We should be 
> > returning a Future there is no list since you are deletnig the 
> > entire container directory in one shot. This seems a bit odd.
> 
> Qian Zhang wrote:
> I tend to agree with you, however, I see CPU share isolator does the 
> similar thing: 
> https://github.com/apache/mesos/blob/0.28.0/src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp#L536:L547,
>  maybe we should change it as well?

I have made `_cleanup()` as a void method so we do not need to return 
`Future` anymore.


> On March 22, 2016, 11:37 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 686
> > 
> >
> > This is dangerous we should be returning a const reference passed down 
> > in the call as a return argument. Just do a return list() over 
> > here. 
> > 
> > PS: Please see my comment on returning Future above.

Ditto.


- Qian


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


On March 29, 2016, 6:59 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45082/
> ---
> 
> (Updated March 29, 2016, 6:59 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented cleanup() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> b1b7205f4f10b6dc256fcc4ecb3210105c1240b4 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 7cda5715814a0cfc4b394eb04437831e6dc44e3f 
> 
> Diff: https://reviews.apache.org/r/45082/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 45466: Removed the redundant `NULL` check when deleting `Credential`.

[Anand Mazumdar]

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

Review request for mesos, Klaus Ma, Neil Conway, and Vinod Kone.


Repository: mesos


Description
---

See summary. Also moved all `delete` statements together in the order they were 
declared in `.hpp` file.


Diffs
-

  src/sched/sched.cpp 701e024ac57fc6e2f96d7d83e65cb68252d41654 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 45440: Added some metrics to the long-lived-framework example.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45067, 45440]

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

- Mesos ReviewBot


On March 29, 2016, 7:08 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45440/
> ---
> 
> (Updated March 29, 2016, 7:08 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5062
> https://issues.apache.org/jira/browse/MESOS-5062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds metrics to gauge the health of the framework.  This includes:
> 
> * uptime_secs = How long the framework has been running.
> * registered  = If the framework is registered.
> * offers_received = A counter used to determine if the framework is starved 
> or not.
> * tasks_launched  = Number of tasks launched.
> * abnormal_terminations = Number of terminal status updates which were not 
> `TASK_FINISHED`.
> 
> Also adds an endpoint `/framework/counters` which returns the list of metrics 
> which are "counters".
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 
> 
> Diff: https://reviews.apache.org/r/45440/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Also deployed this version on a test cluster.  See the previous review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 45435: Modified scheduler library to properly handle SSL connections.

[Anand Mazumdar]

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

(Updated March 30, 2016, 2:03 a.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Review comments from Joseph


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


Repository: mesos


Description
---

This fixes the scheduler library to properly set the scheme
as `https` if SSL is enabled. If not, default to `http` as
is the case now. Would follow up with a similar change for the 
executor library.


Diffs (updated)
-

  src/scheduler/scheduler.cpp 13972449363b633f21ddec7649b1b170703c773a 

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


Testing
---

make check

Used the command scheduler to verify requests are made via HTTPS. Adding tests 
for this would be handled as part of MESOS-3753.

GLOG_v=1 SSL_CERT_FILE=/home/faceoff/work/mesos-priv/build/cert.pem 
SSL_KEY_FILE=/home/faceoff/work/mesos-priv/build/key.pem SSL_ENABLED=1 
./src/mesos-execute --command="sleep 1" --master=127.0.1.1:5050 --name=task1
Enter PEM pass phrase:
WARNING: Logging before InitGoogleLogging() is written to STDERR
I0329 10:33:02.122915 28647 process.cpp:986] libprocess is initialized on 
127.0.1.1:46244 for 8 cpus
I0329 10:33:02.125663 28663 logging.cpp:195] Logging to STDERR
I0329 10:33:02.125758 28663 scheduler.cpp:172] Version: 0.29.0
I0329 10:33:02.131425 28667 scheduler.cpp:449] New master detected at 
master@127.0.1.1:5050
I0329 10:33:02.168808 28664 scheduler.cpp:338] Connected with the master at 
https://127.0.1.1:5050/master/api/v1/scheduler
I0329 10:33:02.170920 28669 scheduler.cpp:231] Sending SUBSCRIBE call to 
https://127.0.1.1:5050/master/api/v1/scheduler
I0329 10:33:02.225026 28670 scheduler.cpp:640] Enqueuing event SUBSCRIBED 
received from https://127.0.1.1:5050/master/api/v1/scheduler
Subscribed with ID 'c4347d67-05d4-4458-994d-043c25189236-0001
I0329 10:33:02.225764 28663 scheduler.cpp:640] Enqueuing event HEARTBEAT 
received from https://127.0.1.1:5050/master/api/v1/scheduler
I0329 10:33:02.226172 28665 scheduler.cpp:640] Enqueuing event OFFERS received 
from https://127.0.1.1:5050/master/api/v1/scheduler
I0329 10:33:02.226687 28664 resources.cpp:574] Parsing resources as JSON 
failed: cpus:1;mem:128
Trying semicolon-delimited string format instead
task task1 submitted to agent fac82f38-c437-4058-b3e1-ce664070611b-S0
I0329 10:33:02.227926 28670 scheduler.cpp:231] Sending ACCEPT call to 
https://127.0.1.1:5050/master/api/v1/scheduler
I0329 10:33:02.362051 28667 scheduler.cpp:640] Enqueuing event UPDATE received 
from https://127.0.1.1:5050/master/api/v1/scheduler
Received status update TASK_RUNNING for task task1
I0329 10:33:02.362877 28663 scheduler.cpp:231] Sending ACKNOWLEDGE call to 
https://127.0.1.1:5050/master/api/v1/scheduler
I0329 10:33:03.378164 28669 scheduler.cpp:640] Enqueuing event UPDATE received 
from https://127.0.1.1:5050/master/api/v1/scheduler
Received status update TASK_FINISHED for task task1
I0329 10:33:03.379533 28663 scheduler.cpp:231] Sending ACKNOWLEDGE call to 
https://127.0.1.1:5050/master/api/v1/scheduler


Thanks,

Anand Mazumdar

Re: Review Request 45435: Modified scheduler library to properly handle SSL connections.

[Anand Mazumdar]

> On March 30, 2016, 12:23 a.m., Joseph Wu wrote:
> > src/scheduler/scheduler.cpp, line 437
> > 
> >
> > This flag is a `bool`, so we accept two forms:
> > "1" and "true".
> > 
> > See: 
> > https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp#L54
> > 
> > ---
> > 
> > Also, you can compare an Option without checking `.isSome()`.

Fixed.

Also, we can't use an `Option` to directly compare here. Since "1" is basically 
a `const char[2]` and not a `std::string`. Makes sense?


- Anand


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


On March 30, 2016, 2:03 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45435/
> ---
> 
> (Updated March 30, 2016, 2:03 a.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-3976
> https://issues.apache.org/jira/browse/MESOS-3976
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fixes the scheduler library to properly set the scheme
> as `https` if SSL is enabled. If not, default to `http` as
> is the case now. Would follow up with a similar change for the 
> executor library.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 13972449363b633f21ddec7649b1b170703c773a 
> 
> Diff: https://reviews.apache.org/r/45435/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Used the command scheduler to verify requests are made via HTTPS. Adding 
> tests for this would be handled as part of MESOS-3753.
> 
> GLOG_v=1 SSL_CERT_FILE=/home/faceoff/work/mesos-priv/build/cert.pem 
> SSL_KEY_FILE=/home/faceoff/work/mesos-priv/build/key.pem SSL_ENABLED=1 
> ./src/mesos-execute --command="sleep 1" --master=127.0.1.1:5050 --name=task1
> Enter PEM pass phrase:
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> I0329 10:33:02.122915 28647 process.cpp:986] libprocess is initialized on 
> 127.0.1.1:46244 for 8 cpus
> I0329 10:33:02.125663 28663 logging.cpp:195] Logging to STDERR
> I0329 10:33:02.125758 28663 scheduler.cpp:172] Version: 0.29.0
> I0329 10:33:02.131425 28667 scheduler.cpp:449] New master detected at 
> master@127.0.1.1:5050
> I0329 10:33:02.168808 28664 scheduler.cpp:338] Connected with the master at 
> https://127.0.1.1:5050/master/api/v1/scheduler
> I0329 10:33:02.170920 28669 scheduler.cpp:231] Sending SUBSCRIBE call to 
> https://127.0.1.1:5050/master/api/v1/scheduler
> I0329 10:33:02.225026 28670 scheduler.cpp:640] Enqueuing event SUBSCRIBED 
> received from https://127.0.1.1:5050/master/api/v1/scheduler
> Subscribed with ID 'c4347d67-05d4-4458-994d-043c25189236-0001
> I0329 10:33:02.225764 28663 scheduler.cpp:640] Enqueuing event HEARTBEAT 
> received from https://127.0.1.1:5050/master/api/v1/scheduler
> I0329 10:33:02.226172 28665 scheduler.cpp:640] Enqueuing event OFFERS 
> received from https://127.0.1.1:5050/master/api/v1/scheduler
> I0329 10:33:02.226687 28664 resources.cpp:574] Parsing resources as JSON 
> failed: cpus:1;mem:128
> Trying semicolon-delimited string format instead
> task task1 submitted to agent fac82f38-c437-4058-b3e1-ce664070611b-S0
> I0329 10:33:02.227926 28670 scheduler.cpp:231] Sending ACCEPT call to 
> https://127.0.1.1:5050/master/api/v1/scheduler
> I0329 10:33:02.362051 28667 scheduler.cpp:640] Enqueuing event UPDATE 
> received from https://127.0.1.1:5050/master/api/v1/scheduler
> Received status update TASK_RUNNING for task task1
> I0329 10:33:02.362877 28663 scheduler.cpp:231] Sending ACKNOWLEDGE call to 
> https://127.0.1.1:5050/master/api/v1/scheduler
> I0329 10:33:03.378164 28669 scheduler.cpp:640] Enqueuing event UPDATE 
> received from https://127.0.1.1:5050/master/api/v1/scheduler
> Received status update TASK_FINISHED for task task1
> I0329 10:33:03.379533 28663 scheduler.cpp:231] Sending ACKNOWLEDGE call to 
> https://127.0.1.1:5050/master/api/v1/scheduler
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45439: Completed implementation of the Nvidia GPU isolator.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [45439, 44364, 44366, 44365, 44363, 44361, 44360, 44779, 
44778, 44777, 44776]

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

Error:
2016-03-30 01:08:28 URL:https://reviews.apache.org/r/44778/diff/raw/ 
[1015/1015] -> "44778.patch" [1]
error: patch failed: src/common/parse.hpp:87
error: src/common/parse.hpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/12206/console

- Mesos ReviewBot


On March 29, 2016, 6:59 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45439/
> ---
> 
> (Updated March 29, 2016, 6:59 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4625
> https://issues.apache.org/jira/browse/MESOS-4625
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current implementation includes some code for generic device
> isolation. Once we start isolating more devices, we should abstract
> this out and figure out an architecture to support "sub-device"
> isolation.
> 
> This was joint work with Rob Todd from Nvidia .
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45439/diff/
> 
> 
> Testing
> ---
> 
> Test to come in subsequent patch.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 42516: Add support for user-defined networks.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42516]

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

- Mesos ReviewBot


On March 29, 2016, 4:33 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 29, 2016, 4:33 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Re: Review Request 44366: Added GPUs as an explicit resource.

[Ben Mahler]

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


Ship it!




Went over this with kevin, we'll be making some minor adjustments to the 
comments.


src/slave/containerizer/containerizer.cpp (line 105)


how about "millis" to try to make the intent of the modulo logic a bit more 
clear?



src/slave/containerizer/containerizer.cpp (lines 117 - 119)


We would probably need this TODO in all of our nvidia gpu blocks :)


- Ben Mahler


On March 14, 2016, 7:39 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44366/
> ---
> 
> (Updated March 14, 2016, 7:39 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4865
> https://issues.apache.org/jira/browse/MESOS-4865
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, we enforce that the number of GPUs specified in the 'gpus'
> resource parameter equal the number of GPUs passed in via the
> --nvidia_gpu_devices flag. In the future, we will generalize this via
> autodiscovery of GPUs and support for GPU types other than Nvidia.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp bb343ad852576a75615a93ef850b413bf77698e0 
>   include/mesos/v1/resources.hpp 719110fbbf39f1755460ac0b32e3893656054a4e 
>   src/common/resources.cpp 1f23dc83f7330c305a836d698f114b7eaf3d7ba1 
>   src/slave/containerizer/containerizer.cpp 
> f6fc7863d0c215611f170dc0c89aa229407b5137 
>   src/v1/resources.cpp c6f125ec317e2da537a6456f5cff2da0a48701d8 
> 
> Diff: https://reviews.apache.org/r/44366/diff/
> 
> 
> Testing
> ---
> 
> ./bin/mesos-slave.sh --master=127.0.0.1:5050 --resources="gpus:string"
> Failed to determine slave resources: Bad type for resource gpus value string 
> type TEXT
> 
> ./bin/mesos-slave.sh --master=127.0.0.1:5050 --resources="gpus:4.9"
> Failed to determine slave resources: The `gpus` resource must specified as an 
> unsigned integer
>
> ./bin/mesos-slave.sh --master=127.0.0.1:5050 --resources="gpus:4.0"
> Failed to determine slave resources: When specifying the `gpus` resource, you 
> must also specify a list of GPUs via the `--nvidia_gpu_devices` flag
> 
> ./bin/mesos-slave.sh --master=127.0.0.1:5050 --resources="gpus:4.0" 
> --nvidia_gpu_devices=1,2,3
> Failed to determine slave resources: The number of GPUs passed in the 
> `--nvidia_gpu_devices` flag must match the number of GPUs specified in the 
> `gpus` resource
> 
> ./bin/mesos-slave.sh --master=127.0.0.1:5050 --resources="gpus:4.0" 
> --nvidia_gpu_devices=1,2,3,4
> SUCCESS
> 
> **NOTE**: I didn't set the `--isolation` flag here, so the agent actually 
> started up correctly (i.e. it didn't error out saying that the Nvidia GPU 
> isolator is currently unsupported).  This is the correct behaviour, and it 
> properly exercises the code added in this patch.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 45435: Modified scheduler library to properly handle SSL connections.

[Joseph Wu]

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


Fix it, then Ship it!




The approach LGTM.


src/scheduler/scheduler.cpp (line 437)


This flag is a `bool`, so we accept two forms:
"1" and "true".

See: 
https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp#L54

---

Also, you can compare an Option without checking `.isSome()`.


- Joseph Wu


On March 29, 2016, 10:35 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45435/
> ---
> 
> (Updated March 29, 2016, 10:35 a.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-3976
> https://issues.apache.org/jira/browse/MESOS-3976
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fixes the scheduler library to properly set the scheme
> as `https` if SSL is enabled. If not, default to `http` as
> is the case now. Would follow up with a similar change for the 
> executor library.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 13972449363b633f21ddec7649b1b170703c773a 
> 
> Diff: https://reviews.apache.org/r/45435/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Used the command scheduler to verify requests are made via HTTPS. Adding 
> tests for this would be handled as part of MESOS-3753.
> 
> GLOG_v=1 SSL_CERT_FILE=/home/faceoff/work/mesos-priv/build/cert.pem 
> SSL_KEY_FILE=/home/faceoff/work/mesos-priv/build/key.pem SSL_ENABLED=1 
> ./src/mesos-execute --command="sleep 1" --master=127.0.1.1:5050 --name=task1
> Enter PEM pass phrase:
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> I0329 10:33:02.122915 28647 process.cpp:986] libprocess is initialized on 
> 127.0.1.1:46244 for 8 cpus
> I0329 10:33:02.125663 28663 logging.cpp:195] Logging to STDERR
> I0329 10:33:02.125758 28663 scheduler.cpp:172] Version: 0.29.0
> I0329 10:33:02.131425 28667 scheduler.cpp:449] New master detected at 
> master@127.0.1.1:5050
> I0329 10:33:02.168808 28664 scheduler.cpp:338] Connected with the master at 
> https://127.0.1.1:5050/master/api/v1/scheduler
> I0329 10:33:02.170920 28669 scheduler.cpp:231] Sending SUBSCRIBE call to 
> https://127.0.1.1:5050/master/api/v1/scheduler
> I0329 10:33:02.225026 28670 scheduler.cpp:640] Enqueuing event SUBSCRIBED 
> received from https://127.0.1.1:5050/master/api/v1/scheduler
> Subscribed with ID 'c4347d67-05d4-4458-994d-043c25189236-0001
> I0329 10:33:02.225764 28663 scheduler.cpp:640] Enqueuing event HEARTBEAT 
> received from https://127.0.1.1:5050/master/api/v1/scheduler
> I0329 10:33:02.226172 28665 scheduler.cpp:640] Enqueuing event OFFERS 
> received from https://127.0.1.1:5050/master/api/v1/scheduler
> I0329 10:33:02.226687 28664 resources.cpp:574] Parsing resources as JSON 
> failed: cpus:1;mem:128
> Trying semicolon-delimited string format instead
> task task1 submitted to agent fac82f38-c437-4058-b3e1-ce664070611b-S0
> I0329 10:33:02.227926 28670 scheduler.cpp:231] Sending ACCEPT call to 
> https://127.0.1.1:5050/master/api/v1/scheduler
> I0329 10:33:02.362051 28667 scheduler.cpp:640] Enqueuing event UPDATE 
> received from https://127.0.1.1:5050/master/api/v1/scheduler
> Received status update TASK_RUNNING for task task1
> I0329 10:33:02.362877 28663 scheduler.cpp:231] Sending ACKNOWLEDGE call to 
> https://127.0.1.1:5050/master/api/v1/scheduler
> I0329 10:33:03.378164 28669 scheduler.cpp:640] Enqueuing event UPDATE 
> received from https://127.0.1.1:5050/master/api/v1/scheduler
> Received status update TASK_FINISHED for task task1
> I0329 10:33:03.379533 28663 scheduler.cpp:231] Sending ACKNOWLEDGE call to 
> https://127.0.1.1:5050/master/api/v1/scheduler
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 44365: Added flag to specify available Nvidia GPUs on an agent's command line.

[Kevin Klues]

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

(Updated March 30, 2016, 12:19 a.m.)


Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.


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


Repository: mesos


Description
---

Added flag to specify available Nvidia GPUs on an agent's command line.


Diffs (updated)
-

  docs/configuration.md 6fc18754c88df16162d83a31e0ecede324bec971 
  src/slave/flags.hpp 9ee7f349cc2bb3fa76141645f4a06fad57664367 
  src/slave/flags.cpp 0c13ab6eb3bc8754da11104466935062cf20ed87 

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


Testing
---

./bin/mesos-slave.sh --master=127.0.0.1:5050 --nvidia_gpu_devices=1,2,string
Failed to load flag 'nvidia_gpu_devices': Failed to load value '1,2,string': 
Expecting all elements to be unsigned integers

./bin/mesos-slave.sh --master=127.0.0.1:5050 --nvidia_gpu_devices=1,2.0,3   
Failed to load flag 'nvidia_gpu_devices': Failed to load value '1,2,3.0': 
Expecting all elements to be unsigned integers

./bin/mesos-slave.sh --master=127.0.0.1:5050 --nvidia_gpu_devices=1,2,3
SUCCESS


Thanks,

Kevin Klues

Re: Review Request 44365: Added flag to specify available Nvidia GPUs on an agent's command line.

[Kevin Klues]

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

(Updated March 30, 2016, 12:16 a.m.)


Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.


Changes
---

Rebased to newest master. Fixed comments as per bmahler's suggestion.


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


Repository: mesos


Description
---

Added flag to specify available Nvidia GPUs on an agent's command line.


Diffs (updated)
-

  docs/configuration.md 6fc18754c88df16162d83a31e0ecede324bec971 
  src/slave/flags.hpp 9ee7f349cc2bb3fa76141645f4a06fad57664367 
  src/slave/flags.cpp 0c13ab6eb3bc8754da11104466935062cf20ed87 

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


Testing
---

./bin/mesos-slave.sh --master=127.0.0.1:5050 --nvidia_gpu_devices=1,2,string
Failed to load flag 'nvidia_gpu_devices': Failed to load value '1,2,string': 
Expecting all elements to be unsigned integers

./bin/mesos-slave.sh --master=127.0.0.1:5050 --nvidia_gpu_devices=1,2.0,3   
Failed to load flag 'nvidia_gpu_devices': Failed to load value '1,2,3.0': 
Expecting all elements to be unsigned integers

./bin/mesos-slave.sh --master=127.0.0.1:5050 --nvidia_gpu_devices=1,2,3
SUCCESS


Thanks,

Kevin Klues

Re: Review Request 45435: Modified scheduler library to properly handle SSL connections.

[Vinod Kone]

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


Ship it!




LGTM. I'll let Joseph Wu take a look and ship it as well.

- Vinod Kone


On March 29, 2016, 5:35 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45435/
> ---
> 
> (Updated March 29, 2016, 5:35 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-3976
> https://issues.apache.org/jira/browse/MESOS-3976
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fixes the scheduler library to properly set the scheme
> as `https` if SSL is enabled. If not, default to `http` as
> is the case now. Would follow up with a similar change for the 
> executor library.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 13972449363b633f21ddec7649b1b170703c773a 
> 
> Diff: https://reviews.apache.org/r/45435/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Used the command scheduler to verify requests are made via HTTPS. Adding 
> tests for this would be handled as part of MESOS-3753.
> 
> GLOG_v=1 SSL_CERT_FILE=/home/faceoff/work/mesos-priv/build/cert.pem 
> SSL_KEY_FILE=/home/faceoff/work/mesos-priv/build/key.pem SSL_ENABLED=1 
> ./src/mesos-execute --command="sleep 1" --master=127.0.1.1:5050 --name=task1
> Enter PEM pass phrase:
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> I0329 10:33:02.122915 28647 process.cpp:986] libprocess is initialized on 
> 127.0.1.1:46244 for 8 cpus
> I0329 10:33:02.125663 28663 logging.cpp:195] Logging to STDERR
> I0329 10:33:02.125758 28663 scheduler.cpp:172] Version: 0.29.0
> I0329 10:33:02.131425 28667 scheduler.cpp:449] New master detected at 
> master@127.0.1.1:5050
> I0329 10:33:02.168808 28664 scheduler.cpp:338] Connected with the master at 
> https://127.0.1.1:5050/master/api/v1/scheduler
> I0329 10:33:02.170920 28669 scheduler.cpp:231] Sending SUBSCRIBE call to 
> https://127.0.1.1:5050/master/api/v1/scheduler
> I0329 10:33:02.225026 28670 scheduler.cpp:640] Enqueuing event SUBSCRIBED 
> received from https://127.0.1.1:5050/master/api/v1/scheduler
> Subscribed with ID 'c4347d67-05d4-4458-994d-043c25189236-0001
> I0329 10:33:02.225764 28663 scheduler.cpp:640] Enqueuing event HEARTBEAT 
> received from https://127.0.1.1:5050/master/api/v1/scheduler
> I0329 10:33:02.226172 28665 scheduler.cpp:640] Enqueuing event OFFERS 
> received from https://127.0.1.1:5050/master/api/v1/scheduler
> I0329 10:33:02.226687 28664 resources.cpp:574] Parsing resources as JSON 
> failed: cpus:1;mem:128
> Trying semicolon-delimited string format instead
> task task1 submitted to agent fac82f38-c437-4058-b3e1-ce664070611b-S0
> I0329 10:33:02.227926 28670 scheduler.cpp:231] Sending ACCEPT call to 
> https://127.0.1.1:5050/master/api/v1/scheduler
> I0329 10:33:02.362051 28667 scheduler.cpp:640] Enqueuing event UPDATE 
> received from https://127.0.1.1:5050/master/api/v1/scheduler
> Received status update TASK_RUNNING for task task1
> I0329 10:33:02.362877 28663 scheduler.cpp:231] Sending ACKNOWLEDGE call to 
> https://127.0.1.1:5050/master/api/v1/scheduler
> I0329 10:33:03.378164 28669 scheduler.cpp:640] Enqueuing event UPDATE 
> received from https://127.0.1.1:5050/master/api/v1/scheduler
> Received status update TASK_FINISHED for task task1
> I0329 10:33:03.379533 28663 scheduler.cpp:231] Sending ACKNOWLEDGE call to 
> https://127.0.1.1:5050/master/api/v1/scheduler
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 44365: Added flag to specify available Nvidia GPUs on an agent's command line.

[Ben Mahler]

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


Fix it, then Ship it!





docs/configuration.md (lines 1308 - 1309)


Could we add something to clarify that this is relevant when "gpus" are 
specified within --resources?


- Ben Mahler


On March 14, 2016, 7:38 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44365/
> ---
> 
> (Updated March 14, 2016, 7:38 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4864
> https://issues.apache.org/jira/browse/MESOS-4864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added flag to specify available Nvidia GPUs on an agent's command line.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 739d4ff9aeeb1ba70ce11033168d63d37b6ef56b 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 5bacf109bcd9c9eae1b7ec3c32095c72899ebdbd 
> 
> Diff: https://reviews.apache.org/r/44365/diff/
> 
> 
> Testing
> ---
> 
> ./bin/mesos-slave.sh --master=127.0.0.1:5050 --nvidia_gpu_devices=1,2,string
> Failed to load flag 'nvidia_gpu_devices': Failed to load value '1,2,string': 
> Expecting all elements to be unsigned integers
> 
> ./bin/mesos-slave.sh --master=127.0.0.1:5050 --nvidia_gpu_devices=1,2.0,3   
> Failed to load flag 'nvidia_gpu_devices': Failed to load value '1,2,3.0': 
> Expecting all elements to be unsigned integers
> 
> ./bin/mesos-slave.sh --master=127.0.0.1:5050 --nvidia_gpu_devices=1,2,3
> SUCCESS
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Review Request 45461: Create tempfiles in test temporary directory.

[Michael Browning]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Create tempfiles in test temporary directory.

Previously, some tests were creating tempfiles outside of the temporary 
directory created by the test case and then manually removing them, which could 
lead to their being leaked if the test were interrupted.


Diffs
-

  src/tests/fetcher_tests.cpp 84c8d487bbf61c77452bd293062a80fa132e069f 

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


Testing
---

This change merely creates all temporary files used in the process of running 
fetcher tests inside of the temp directories created by the test case, so no 
further testing was performed.


Thanks,

Michael Browning

Re: Review Request 45410: Documented when to invoke `send` using the scheduler library.

[Vinod Kone]

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


Ship it!




Oh so you added it here. Cool.

Can you specificy somewhere that the library automatically reconnects with the 
master after a disconnection?

- Vinod Kone


On March 29, 2016, 12:19 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45410/
> ---
> 
> (Updated March 29, 2016, 12:19 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4950
> https://issues.apache.org/jira/browse/MESOS-4950
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Trivial change to add a comment to invoke `send` only after
> receiving the `connected` callback. If not, all calls would
> be dropped while disconnected.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp 765935e97b6c1686ab464a5cf1cf2dfd816f51f1 
> 
> Diff: https://reviews.apache.org/r/45410/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45409: Added test for `reconnect` functionality.

[Vinod Kone]

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




src/tests/scheduler_tests.cpp (line 1513)


s/callback./callback followed by a connected callback./



src/tests/scheduler_tests.cpp (line 1538)


You should do a Clock::pause() and Clock::settle() to ensure all pending 
events resulting from `reconnect()` are processed.


- Vinod Kone


On March 29, 2016, 12:19 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45409/
> ---
> 
> (Updated March 29, 2016, 12:19 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4950
> https://issues.apache.org/jira/browse/MESOS-4950
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a trivial test for testing the `reconnect`
> method on the scheduler library.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 917058f4dcf32ddaaeda8a3ff21898571f4829dd 
> 
> Diff: https://reviews.apache.org/r/45409/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45408: Introduced a `reconnect` method on the scheduler library.

[Vinod Kone]

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




include/mesos/v1/scheduler.hpp (line 50)


s/events/calls and events/ ?



include/mesos/v1/scheduler.hpp (lines 70 - 73)


Can you add more meat to this comment?

For example, error event is not sent if the master is disconnected but the 
call is silently dropped. Also mention that clients should send() a call only 
after they have received a connected() callback and that they shouldn't send a 
call if they received a disconnected() callback until they  get a connected() 
callback again.

You can put some of this comment above the Mesos constructor.



include/mesos/v1/scheduler.hpp (line 85)


Also mention that clients will get a disconnected() and connected() 
callback after making this call?


- Vinod Kone


On March 29, 2016, 12:18 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45408/
> ---
> 
> (Updated March 29, 2016, 12:18 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4950
> https://issues.apache.org/jira/browse/MESOS-4950
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a `reconnect` method that allows schedulers
> to force a reconnection with the master. This is useful when
> the scheduler detects that there is a one-way partition with
> the master (e.g., lack of `HEARTBEAT` events) and wants to
> trigger a reconnection rather than relying on the `disconnected`
> callback.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp 765935e97b6c1686ab464a5cf1cf2dfd816f51f1 
>   src/scheduler/scheduler.cpp 13972449363b633f21ddec7649b1b170703c773a 
> 
> Diff: https://reviews.apache.org/r/45408/diff/
> 
> 
> Testing
> ---
> 
> make check (added a test later in the review chain)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

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

[Maged Michael]

> On March 29, 2016, 12:52 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/process.cpp, line 2193
> > 
> >
> > since this value will not longer always map to the number of `cpus`, 
> > can we rename this to something more meaningful such as 
> > `num_worker_threads`?

Done. I also changed `cpus` in lines 846, 986 and 987.


> On March 29, 2016, 12:52 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/process.cpp, line 2208
> > 
> >
> > please use full words for variable names (i.e. `num`)

Changed to `number`.


> On March 29, 2016, 12:52 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 2210-2212
> > 
> >
> > It seems valuable to log the number of threads we will use regardless 
> > of whether they were over-ridden or not.
> > 
> > Let's log the actual number outside of the `if` block. This will also 
> > simplify the message (e.g. `Using X libprocess worker threads`

The number of thread is logged in lines 986 and 987. The added logging here is 
just for the override.
What do you think?


> On March 29, 2016, 12:52 p.m., Joris Van Remoortere wrote:
> > docs/configuration.md, line 1761
> > 
> >
> > Shall we explain what the default is?

Added ", which is the maximum of 8 and the number of cores on the machine".


- Maged


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


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

Re: Review Request 45008: Moved command scheduler to use the scheduler library.

[Vinod Kone]

> On March 18, 2016, 5:34 a.m., Guangya Liu wrote:
> > src/cli/execute.cpp, line 456
> > 
> >
> > Can you please add some comments here for why `devolve`here?
> > 
> > Do we have plan to update all `status` use V1?

I think Anand did it because mesos::internal::protobuf::isTerminalState() takes 
a v0 state.


- Vinod


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


On March 28, 2016, 10:32 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45008/
> ---
> 
> (Updated March 28, 2016, 10:32 p.m.)
> 
> 
> Review request for mesos, Guangya Liu and Vinod Kone.
> 
> 
> Bugs: MESOS-3559
> https://issues.apache.org/jira/browse/MESOS-3559
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change moves the command scheduler (`mesos-execute`) to use
> the scheduler library instead of the driver.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/45008/diff/
> 
> 
> Testing
> ---
> 
> make check, an example run:
> 
> ```
> build git:(mesos-3559) : ./src/mesos-execute --command="sleep 1" 
> --master=127.0.1.1:5050 --name=task1
> I0317 19:04:43.974733 21678 scheduler.cpp:172] Version: 0.29.0
> Subscribed with ID '7decbfe9-8b46-48f5-8573-0a5a3e465a1f-0001
> task task1 submitted to agent 5a54dc00-7fd8-477f-a666-1b1ab86a0033-S0
> Received status update TASK_RUNNING for task task1
> Received status update TASK_FINISHED for task task1
> E0317 19:04:45.148203 21680 scheduler.cpp:585] End-Of-File received from 
> master. The master closed the event stream
> 
> build git:(mesos-3559) : echo $?
> 0
> ```
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45008: Moved command scheduler to use the scheduler library.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On March 28, 2016, 10:32 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45008/
> ---
> 
> (Updated March 28, 2016, 10:32 p.m.)
> 
> 
> Review request for mesos, Guangya Liu and Vinod Kone.
> 
> 
> Bugs: MESOS-3559
> https://issues.apache.org/jira/browse/MESOS-3559
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change moves the command scheduler (`mesos-execute`) to use
> the scheduler library instead of the driver.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/45008/diff/
> 
> 
> Testing
> ---
> 
> make check, an example run:
> 
> ```
> build git:(mesos-3559) : ./src/mesos-execute --command="sleep 1" 
> --master=127.0.1.1:5050 --name=task1
> I0317 19:04:43.974733 21678 scheduler.cpp:172] Version: 0.29.0
> Subscribed with ID '7decbfe9-8b46-48f5-8573-0a5a3e465a1f-0001
> task task1 submitted to agent 5a54dc00-7fd8-477f-a666-1b1ab86a0033-S0
> Received status update TASK_RUNNING for task task1
> Received status update TASK_FINISHED for task task1
> E0317 19:04:45.148203 21680 scheduler.cpp:585] End-Of-File received from 
> master. The master closed the event stream
> 
> build git:(mesos-3559) : echo $?
> 0
> ```
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

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

[Maged Michael]

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

(Updated March 29, 2016, 11:14 p.m.)


Review request for mesos, Guangya Liu, Klaus Ma, and Qian Zhang.


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


Repository: mesos


Description
---

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


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp feaffa4 
  docs/configuration.md 9ad0c2a 

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


Testing
---

Passed make check on x86_64 Ubuntu 14.04.

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


Thanks,

Maged Michael

Re: Review Request 45398: Added CHANGELOG for python module changes.

[Vinod Kone]

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


Ship it!




- Vinod Kone


On March 29, 2016, 2:21 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45398/
> ---
> 
> (Updated March 29, 2016, 2:21 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added CHANGELOG for python module changes.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 4c73a2638074ac9f11de1438a5dd3d70e8201229 
> 
> Diff: https://reviews.apache.org/r/45398/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 44361: Added configure flags to build with Nvidia GPU support.

[Kevin Klues]

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

(Updated March 29, 2016, 11:08 p.m.)


Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.


Changes
---

Addressed bmahler's comments. I also updated the test description to reflect 
tests for passing relative paths to --with-nvml-include and --with-nvml-lib.


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


Repository: mesos


Description
---

This is the initial commit to begin adding native support for GPUs in
Mesos. This initial version will only include support for Nvidia GPUs
that can be managed by the Nvidia Management Library (nvml).

The configure flags added in this commit can be used to enable Nvidia
GPU support, as well as specify the installation directories of the
nvml header and library files if not already installed in standard
include/library paths on the system.

In a subsequent commit, we will use these configure flags to
conditionally build support for Nvidia GPUs into Mesos.


Diffs (updated)
-

  configure.ac 9ec4bc1cff3b0b46dd2e7ece2c1f2d19ffb8 

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


Testing (updated)
---

I ran `bootstrap` to generate configure.

I then ran:

```
mkdir build; cd build
../configure --enable-nvidia-gpu-support
../configure --enable-nvidia-gpu-support 
--with-nvml-include=
../configure --enable-nvidia-gpu-support --with-nvml-include=
../configure --enable-nvidia-gpu-support --with-nvml-include=
../configure --enable-nvidia-gpu-support 
--with-nvml-include=
../configure --enable-nvidia-gpu-support --with-nvml-lib=
../configure --enable-nvidia-gpu-support --with-nvml-lib=
../configure --enable-nvidia-gpu-support --with-nvml-include= 
--with-nvml-lib=
../configure --enable-nvidia-gpu-support --with-nvml-include= 
--with-nvml-lib=
../configure --enable-nvidia-gpu-support --with-nvml-include= 
--with-nvml-lib=
../configure --enable-nvidia-gpu-support --with-nvml-include= 
--with-nvml-lib=
```

And verified the proper errors / successes in each case (only the last one is a 
success).

The exact command I ran in the success case for my configuration was:
```
../configure --enable-nvidia-gpu-support 
--with-nvml-include=/opt/nvidia-gdk/usr/include 
--with-nvml-lib=/opt/nvidia-gdk/usr/src/gdk/nvml/lib
```


Thanks,

Kevin Klues

Re: Review Request 44361: Added configure flags to build with Nvidia GPU support.

[Ben Mahler]

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


Fix it, then Ship it!





configure.ac (line 215)


NVML



configure.ac (line 221)


s/libs/libraries/ ?



configure.ac (lines 948 - 955)


Could we tell the user explicitly when the path is absolute?


- Ben Mahler


On March 14, 2016, 7:37 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44361/
> ---
> 
> (Updated March 14, 2016, 7:37 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4861
> https://issues.apache.org/jira/browse/MESOS-4861
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the initial commit to begin adding native support for GPUs in
> Mesos. This initial version will only include support for Nvidia GPUs
> that can be managed by the Nvidia Management Library (nvml).
> 
> The configure flags added in this commit can be used to enable Nvidia
> GPU support, as well as specify the installation directories of the
> nvml header and library files if not already installed in standard
> include/library paths on the system.
> 
> In a subsequent commit, we will use these configure flags to
> conditionally build support for Nvidia GPUs into Mesos.
> 
> 
> Diffs
> -
> 
>   configure.ac 8e4f03593df4a8ba13f00292963e351acc3f71c1 
> 
> Diff: https://reviews.apache.org/r/44361/diff/
> 
> 
> Testing
> ---
> 
> I ran `bootstrap` to generate configure.
> 
> I then ran:
> 
> ```
> mkdir build; cd build
> ../configure --enable-nvidia-gpu-support
> ../configure --enable-nvidia-gpu-support --with-nvml-include=
> ../configure --enable-nvidia-gpu-support --with-nvml-include=
> ../configure --enable-nvidia-gpu-support --with-nvml-lib=
> ../configure --enable-nvidia-gpu-support --with-nvml-lib=
> ../configure --enable-nvidia-gpu-support --with-nvml-include= 
> --with-nvml-lib=
> ../configure --enable-nvidia-gpu-support 
> --with-nvml-include= --with-nvml-lib=
> ../configure --enable-nvidia-gpu-support --with-nvml-include= 
> --with-nvml-lib=
> ../configure --enable-nvidia-gpu-support 
> --with-nvml-include= --with-nvml-lib=
> ```
> 
> And verified the proper errors / successes in each case (only the last one is 
> a success).
> 
> The exact command I ran in the success case for my configuration was:
> ```
> ../configure --enable-nvidia-gpu-support 
> --with-nvml-include=/opt/nvidia-gdk/usr/include 
> --with-nvml-lib=/opt/nvidia-gdk/usr/src/gdk/nvml/lib
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Review Request 45455: Added test for recovering orphaned docker containers.

[Anand Mazumdar]

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

Review request for mesos, Jie Yu and Timothy Chen.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
b9c26b92c44e8ca718a5eb333855199bdf2a8e81 

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


Testing
---

make check (gtest_repeat=100)


Thanks,

Anand Mazumdar

Review Request 45453: Minor spacing cleanups in docker containerizer.

[Anand Mazumdar]

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

Review request for mesos, Jie Yu and Timothy Chen.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/slave/containerizer/docker.cpp c5007a311ae9c1766dd4522ccbddbdb506d4ae4e 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 45454: Cleanup orphaned docker containers owned by previous agent instance.

[Anand Mazumdar]

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

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


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


Repository: mesos


Description
---

This change modifies the docker containerizer to cleanup docker
containers left from another agent instance. The containers can
become orphans due to any of the scenarios mentioned here:
http://bit.ly/1RxCpPl

This change modifies the logic to invoke docker `ps` on all
containers on the agent instead of limiting itself to the
current slaveID. This change also means that running multiple
agent instances on the same host might not work well for docker
containers from now on i.e. another agent instance might
cleanup the docker containers that belong to another instance.
The cgroup isolators/linux launcher for the Mesos containerizer
anyways don't recommend running multiple instances of the agent
on the same host.

In case one still wants to run multiple agent instances on a
test cluster using the docker containerizer, we can use the
`--no-docker_kill_orphans` flag and then kill the docker
containers manually using a script.


Diffs
-

  src/slave/containerizer/docker.hpp 89d450e10a84f24ddd46d517e2b4b46ab02c4fda 
  src/slave/containerizer/docker.cpp c5007a311ae9c1766dd4522ccbddbdb506d4ae4e 

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


Testing
---

make check (Test is added as part of the next review in the chain)

Would follow up with an email on user@ to notify them about the potential
gotchas with running multiple agent instances in production.


Thanks,

Anand Mazumdar

Review Request 45450: Add JSON::Object::at() helper API.

[James Peach]

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

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


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


Repository: mesos


Description
---

JSON::Object::find() interprets the '.' as a path into nested JSON
objects. Add the at() helper that just indexes the top level keys
of a JSON object with an uninterpreted key. This is helpful for
indexing keys that contain periods.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 
87d018386ea59b16bf69b529426b6567727ed545 
  3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
4ea610c3cd373f05843954763173b9bd51d8d5f5 

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


Testing
---

Make check.


Thanks,

James Peach

Review Request 45451: Fix the local Docker puller for private registries.

[James Peach]

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

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


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


Repository: mesos


Description
---

If you use the unified containerizer and the local docker puller with
a Docker image from a private registry, the local puller fails to find
any layers in the image's repository manifest. This happens because the
top layer repository is qualified by the private registry name.

The fix is to also try the fully-qualified repository name if we have
a registry name.


Diffs
-

  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
74254c19a9a31cb76f09eaf1b8684610dd0d7afc 

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


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 45434: Added test cases for '/files' endpoint authentication.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45434]

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

- Mesos ReviewBot


On March 29, 2016, 4:36 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45434/
> ---
> 
> (Updated March 29, 2016, 4:36 p.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current authentication tests of the `/files/` endpoint only test the case 
> of missing credentials. This commit adds the additional case of wrong 
> credentials.
> 
> 
> Diffs
> -
> 
>   src/tests/files_tests.cpp 2f0f84cd78e7f382b70b97c8711f71cea64ce0a5 
> 
> Diff: https://reviews.apache.org/r/45434/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 45429: Added authentication to the '/registry' endpoint.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45429]

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

- Mesos ReviewBot


On March 29, 2016, 3:46 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45429/
> ---
> 
> (Updated March 29, 2016, 3:46 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4933
> https://issues.apache.org/jira/browse/MESOS-4933
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp e777ea2938a23db8b407676a0f7e635e63d032fa 
>   src/master/main.cpp 58561cffa440aaf1293e9ffe19b5685e6d2f1952 
>   src/master/registrar.hpp f087569bd6774ac16093f9204e6870e4d4404420 
>   src/master/registrar.cpp def40b94689c363617a7cfbcce82ea8d357cb345 
>   src/tests/cluster.cpp 2da0bd7612d571277e76d0a95ad8e776434af323 
>   src/tests/registrar_tests.cpp 39caf9bb950c0b229a66becb039c7a830a18f6bc 
> 
> Diff: https://reviews.apache.org/r/45429/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 44945: Add autoconf tests for XFS project quotas.

[James Peach]

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

(Updated March 29, 2016, 8:55 p.m.)


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


Changes
---

Rebased and updated.


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


Repository: mesos


Description
---

Add autoconf tests for XFS project quotas.


Diffs (updated)
-

  configure.ac 9ec4bc1cff3b0b46dd2e7ece2c1f2d19ffb8 

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


Testing
---

Make check. Manual verification.


Thanks,

James Peach

Re: Review Request 44947: Add tests for XFS project quota utilities.

[James Peach]

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

(Updated March 29, 2016, 8:55 p.m.)


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


Changes
---

Rebased and updated.


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


Repository: mesos


Description
---

Add tests for XFS project quota utilities.


Diffs (updated)
-

  src/Makefile.am 21e2965bb7cd9b88a5c787dd3efe0673c71cdc4f 
  src/tests/cluster.cpp 2da0bd7612d571277e76d0a95ad8e776434af323 
  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 
  src/tests/environment.cpp 7617e43587cb81104786d06f753f08565a6c2d0a 

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


Testing
---

Make check. Manual testing. These tests.


Thanks,

James Peach

Re: Review Request 44946: Add utility functions to manipulate XFS project quotas.

[James Peach]

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

(Updated March 29, 2016, 8:55 p.m.)


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


Changes
---

Rebased and updated.


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


Repository: mesos


Description
---

Add utility functions to manipulate XFS project quotas.


Diffs (updated)
-

  src/Makefile.am 21e2965bb7cd9b88a5c787dd3efe0673c71cdc4f 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 

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


Testing
---

Make check. Manual verification. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

[James Peach]

> On March 24, 2016, 4:54 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.cpp, line 95
> > 
> >
> > This method aborts early when a error occurs, I think it's useful to 
> > note this.
> > 
> > Plus, I still think what we need in the isolator is a generic 
> > filesystem walk functionality. I'll help with creating something that 
> > utilize `nftw`: 
> > http://pubs.opengroup.org/onlinepubs/9699919799/functions/nftw.html

Replaced with ``fts(3)``.


> On March 24, 2016, 4:54 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.cpp, lines 501-503
> > 
> >
> > If the new resources have no disk resources, shouldn't we cleanup the 
> > sandbox?

It's not clear to me what this means. If you had a disk resource and now you 
don't are you unrestricted or do you have zero disk? I think it is better to 
only remove the project ID in cleanup, but maybe we should still adjust the 
quota here?


> On March 24, 2016, 4:54 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.cpp, line 568
> > 
> >
> > It seems that we don't need to modify `totalProjectIds` (which can be a 
> > const) here, this change doesn't survive agent restart anyways and we use 
> > `freeProjectIds` to assign IDs.

I think it is better to handle this consistently. We use ``totalProjectIds`` to 
bound the set of possible IDs and this ID is not longer possible.


- James


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


On March 22, 2016, 11:24 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44948/
> ---
> 
> (Updated March 22, 2016, 11:24 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOs-4828
> https://issues.apache.org/jira/browse/MESOs-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 6552e48eab2708a28dd69adba3ec759cb5aeca4c 
>   src/slave/containerizer/mesos/containerizer.cpp 
> ee7a265975323ca891114a286357c8e42901560c 
>   src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
>   src/slave/flags.hpp 3067c128f1fded4f130f4d45f191584c2f30ad9c 
>   src/slave/flags.cpp ce028825ad99f54a231b4b18dde277b63aa0525c 
> 
> Diff: https://reviews.apache.org/r/44948/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44948: Add XFS disk resource isolator.

[James Peach]

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

(Updated March 29, 2016, 8:54 p.m.)


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


Changes
---

Rebased and updated.


Bugs: MESOs-4828
https://issues.apache.org/jira/browse/MESOs-4828


Repository: mesos


Description
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am 21e2965bb7cd9b88a5c787dd3efe0673c71cdc4f 
  src/slave/containerizer/mesos/containerizer.cpp 
605c52e1118f35232ddf59ee90d2343569061a29 
  src/slave/containerizer/mesos/isolators/xfs/disk.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp PRE-CREATION 
  src/slave/flags.hpp 9ee7f349cc2bb3fa76141645f4a06fad57664367 
  src/slave/flags.cpp fd9fbbaa911cc77a21574ba314c50ac226fa49ce 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated March 29, 2016, 8:52 p.m.)


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


Changes
---

Rebased and updated.


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


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 

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


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44950: Add XFS disk isolator documentation.

[James Peach]

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

(Updated March 29, 2016, 8:51 p.m.)


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


Changes
---

Rebased and updated.


Bugs: MESOs-4828
https://issues.apache.org/jira/browse/MESOs-4828


Repository: mesos


Description
---

Add XFS disk isolator documentation.


Diffs (updated)
-

  docs/configuration.md 9ad0c2a201130ac23b22b1a844396dc97ec5ff70 
  docs/mesos-containerizer.md 15fb5bdbe74e059614b8948108f32cd04b623305 

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


Testing
---

Make check. Source inspection.


Thanks,

James Peach

Re: Review Request 45438: Moved realm initialization from constructor to initializer list.

[Till Toenshoff]

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


Ship it!




Ship It!

- Till Toenshoff


On March 29, 2016, 6:45 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45438/
> ---
> 
> (Updated March 29, 2016, 6:45 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved realm initialization from constructor to initializer list.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp a8b27518bd50b951cc1a6dc077ac21eba07376cd 
> 
> Diff: https://reviews.apache.org/r/45438/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45051: Fixed flakiness in ContainerLoggerTest.LOGROTATE_RotateInSandbox.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On March 29, 2016, 7:32 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45051/
> ---
> 
> (Updated March 29, 2016, 7:32 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Vinod Kone.
> 
> 
> Bugs: MESOS-4961
> https://issues.apache.org/jira/browse/MESOS-4961
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This removes a race between the test and the global `ReapProcess`, which were 
> originally both calling `waitpid` on the logger subprocesses.  The test now 
> defers the reaping to the `ReapProcess` and just waits for all logger 
> subprocesses to exit naturally via `os::exists(pid)`.
> 
> Since the `ReapProcess` reaps on an interval, a `Clock::advance` was inserted 
> to skip the reap delay.
> 
> 
> Diffs
> -
> 
>   src/tests/container_logger_tests.cpp 
> 71101c31cee6a400b89cf285cf0a105d2d1534a8 
> 
> Diff: https://reviews.apache.org/r/45051/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> bin/mesos-tests.sh 
> --gtest_filter="ContainerLoggerTest.LOGROTATE_RotateInSandbox" 
> --gtest_repeat=1000 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 45051: Fixed flakiness in ContainerLoggerTest.LOGROTATE_RotateInSandbox.

[Joseph Wu]

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

(Updated March 29, 2016, 12:32 p.m.)


Review request for mesos, Artem Harutyunyan and Vinod Kone.


Changes
---

Changed this to a proper fix.


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


Repository: mesos


Description (updated)
---

This removes a race between the test and the global `ReapProcess`, which were 
originally both calling `waitpid` on the logger subprocesses.  The test now 
defers the reaping to the `ReapProcess` and just waits for all logger 
subprocesses to exit naturally via `os::exists(pid)`.

Since the `ReapProcess` reaps on an interval, a `Clock::advance` was inserted 
to skip the reap delay.


Diffs (updated)
-

  src/tests/container_logger_tests.cpp 71101c31cee6a400b89cf285cf0a105d2d1534a8 

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


Testing (updated)
---

make check

bin/mesos-tests.sh 
--gtest_filter="ContainerLoggerTest.LOGROTATE_RotateInSandbox" 
--gtest_repeat=1000 --gtest_break_on_failure


Thanks,

Joseph Wu

Re: Review Request 45067: Updated the long-lived-framework example.

[Joseph Wu]

> On March 28, 2016, 5:31 p.m., Vinod Kone wrote:
> > src/examples/long_lived_framework.cpp, line 77
> > 
> >
> > s/. Reject/. Reject/ <-- extraneous space

Oops, that's a habit (and the Apache license uses 2 spaces :)


> On March 28, 2016, 5:31 p.m., Vinod Kone wrote:
> > src/examples/long_lived_framework.cpp, lines 95-96
> > 
> >
> > So if resources are not enough we do not explicitly decline the offer?
> > 
> > How about:
> > 
> > ```
> > // Reject offer if the offer belongs to a different slave than the one 
> > currently running the executor(s).
> > if (slaveID.isSome() && slaveID != offers[i].slave_id()) {
> >   // decline offer.
> > }
> > 
> > // Reject the offer if the resources are not enough.
> > if (!Resources(offers[i].resources()).flatten().contains(TASK_RESOURCES 
> > + EXECUTOR_RESOURCES)) {
> >   // decline offer.
> > }
> > 
> > // Launch the task.
> > 
> > ```
> > 
> > Also, are you expecting the executor to be multi-task? If yes, the 
> > resources check above should only include task resources? 
> > 
> > Since you do not control whether the executor is single or multi-task, 
> > I would recommend to launch each task with an unique executor id.

We actually do expect the same executor to be used for each task.  (The 
executor's ID is "default".)

There are really 5 cases here:
1) Executor running & offer from different slave.
2) Executor running & offer not big enough for a task.
3) Executor running & offer from same slave & offer big enough for task.
4) Executor not running & offer not big enough for task + executor.
5) Executor not running & offer big enough for task + executor.


> On March 28, 2016, 5:31 p.m., Vinod Kone wrote:
> > src/examples/long_lived_framework.cpp, line 173
> > 
> >
> > why is this called "residency" instead of "slaveID" ?

I wanted a variable name that somehow expressed where the `long-lived-executor` 
was "living".  But `slaveId` would work too.


> On March 28, 2016, 5:31 p.m., Vinod Kone wrote:
> > src/examples/long_lived_framework.cpp, line 196
> > 
> >
> > why does the framework assume master lives on the same machine?

Hm, I guess the location of the master doesn't really play a role here.  
Removed from comment.


> On March 28, 2016, 5:31 p.m., Vinod Kone wrote:
> > src/examples/long_lived_framework.cpp, line 151
> > 
> >
> > I thought we could compare option directly with a value instead of 
> > doing a .get()?

Yup, my mistake.


> On March 28, 2016, 5:31 p.m., Vinod Kone wrote:
> > src/examples/long_lived_framework.cpp, line 160
> > 
> >
> > seems weird that you are setting residency/slaveID to None() if one 
> > executor is lost. isn't it still possible for other executors to be running 
> > on that slave? do we want to launch new executors on a different slave in 
> > that case?
> > 
> > I would recommend to keep track of running executors with 
> > `set`.

As mentioned above, this framework only launches on executor with ID "default".


- Joseph


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


On March 29, 2016, 12:08 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45067/
> ---
> 
> (Updated March 29, 2016, 12:08 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5062
> https://issues.apache.org/jira/browse/MESOS-5062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives the example `long-lived-framework` enough options to run outside 
> of the build environment.
> 
> This also updates:
> 
> * The style of the framework code.
> * Gives the `ExecutorInfo` some resources (needed for some cgroups isolators).
> * Restricts the framework to one agent.  Otherwise, it would grab a small 
> chunk of every machine in the cluster.
> * Adds filters for declined offers.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 
> 
> Diff: https://reviews.apache.org/r/45067/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Ran this on the master node on a Mesos cluster:
> ```
> 

Review Request 45440: Added some metrics to the long-lived-framework example.

[Joseph Wu]

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

Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and Vinod 
Kone.


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


Repository: mesos


Description
---

Adds metrics to gauge the health of the framework.  This includes:

* uptime_secs = How long the framework has been running.
* registered  = If the framework is registered.
* offers_received = A counter used to determine if the framework is starved or 
not.
* tasks_launched  = Number of tasks launched.
* abnormal_terminations = Number of terminal status updates which were not 
`TASK_FINISHED`.

Also adds an endpoint `/framework/counters` which returns the list of metrics 
which are "counters".


Diffs
-

  src/examples/long_lived_framework.cpp 
ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 

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


Testing
---

make check

Also deployed this version on a test cluster.  See the previous review.


Thanks,

Joseph Wu

Re: Review Request 45067: Updated the long-lived-framework example.

[Joseph Wu]

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

(Updated March 29, 2016, 12:08 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and Vinod 
Kone.


Changes
---

Address comments from Vinod.  Reworked how/when offers are accepted.


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


Repository: mesos


Description
---

This gives the example `long-lived-framework` enough options to run outside of 
the build environment.

This also updates:

* The style of the framework code.
* Gives the `ExecutorInfo` some resources (needed for some cgroups isolators).
* Restricts the framework to one agent.  Otherwise, it would grab a small chunk 
of every machine in the cluster.
* Adds filters for declined offers.


Diffs (updated)
-

  src/examples/long_lived_framework.cpp 
ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 

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


Testing
---

make check

Ran this on the master node on a Mesos cluster:
```
./long-lived-framework --master=zk://localhost:2181/mesos 
--executor_uri="https://s3.amazonaws.com/url/to/long-lived-executor; 
--executor_command="LD_LIBRARY_PATH=/path/to/libmesos && ./long-lived-executor"
```


Thanks,

Joseph Wu

Re: Review Request 45439: Completed implementation of the Nvidia GPU isolator.

[Kevin Klues]

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



This review supercedes: https://reviews.apache.org/r/44979

- Kevin Klues


On March 29, 2016, 6:59 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45439/
> ---
> 
> (Updated March 29, 2016, 6:59 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4625
> https://issues.apache.org/jira/browse/MESOS-4625
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current implementation includes some code for generic device
> isolation. Once we start isolating more devices, we should abstract
> this out and figure out an architecture to support "sub-device"
> isolation.
> 
> This was joint work with Rob Todd from Nvidia .
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45439/diff/
> 
> 
> Testing
> ---
> 
> Test to come in subsequent patch.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 44360: Added a script to install the Nvidia GDK on a host.

[Ben Mahler]

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


Ship it!




Looks great, thanks! I'll just touch up some minor typos.

- Ben Mahler


On March 14, 2016, 7:35 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44360/
> ---
> 
> (Updated March 14, 2016, 7:35 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4860
> https://issues.apache.org/jira/browse/MESOS-4860
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This script can be used to install the Nvidia GPU Deployment Kit (GDK)
> on a mesos development machine. The purpose of the GDK is to provide
> stubs for compiling code against Nvidia's latest drivers.  It includes
> all the necessary header files (nvml.h) and library files
> (libnvidia-ml.so) necessary to build mesos with Nvidia GPU support.
> However, it does not actually contain any driver code, and it can't be
> used to interact with any GPUs installed on a system. For that, you
> will have to find the latest released drivers themselves.
> 
> We provide this script only for convenience, so that developers
> working on non-GPU equipped systems can build code with Nvidia GPU
> support. It is a simple wrapper around the official GDK installer
> script provided by Nvidia, which can be downloaded here:
> 
>   https://developer.nvidia.com/gpu-deployment-kit
> 
> The script itself takes a few parameters. Run it as
> 'support/install-nvidia-gdk.sh --help' to see its usage semantics.
> 
> 
> Diffs
> -
> 
>   support/install-nvidia-gdk.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44360/diff/
> 
> 
> Testing
> ---
> 
> I ran the script on my Mac to verify that it fails out cleanly.
> 
> I ran it on a linux box to exercise each of the different flags that can be 
> passed to it.
> ```
> install-nvidia-gdk.sh --install-dir=
> install-nvidia-gdk.sh --install-dir=
> install-nvidia-gdk.sh --install-dir= --update-ldcache
> install-nvidia-gdk.sh --install-dir= --update-ldcache
> ```
> 
> With each combination, I verified that the proper error messages were 
> reported, or that the installation was successful.
> 
> As per one of the comments below, I also temporarily changed the download URL 
> for the installer to make sure that the `wget` call errored out cleanly.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 45438: Moved realm initialization from constructor to initializer list.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On March 29, 2016, 6:45 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45438/
> ---
> 
> (Updated March 29, 2016, 6:45 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved realm initialization from constructor to initializer list.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp a8b27518bd50b951cc1a6dc077ac21eba07376cd 
> 
> Diff: https://reviews.apache.org/r/45438/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 45438: Moved realm initialization from constructor to initializer list.

[Joerg Schad]

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

Review request for mesos and Till Toenshoff.


Repository: mesos


Description
---

Moved realm initialization from constructor to initializer list.


Diffs
-

  src/files/files.cpp a8b27518bd50b951cc1a6dc077ac21eba07376cd 

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


Testing
---

sudo make check


Thanks,

Joerg Schad

Re: Review Request 44777: Added a flags parser for vector to src/common/parse.hpp.

[Ben Mahler]

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


Ship it!





src/common/parse.hpp (line 123)


Do we need this temporary?

```
foreach (const std::string& token, strings::tokenize(value, ",")) {
```



src/common/parse.hpp (line 128)


Unfortunately numify doesn't follow our error composition approach of the 
callee not repeating information available to the caller, so if we followed our 
composition pattern here we would get:

```
return Error("Failed to numify token '" + token + "'"
 ": " + numify.error());

// because numify is repeating caller-available
// information, this yields:
//
// "Failed to numify 'a': Failed to convert 'a' to number"
```

But let's still do this for now, so that this continue to be meaningful 
once we update numify logging to not repeat the arguments available in the 
caller.


- Ben Mahler


On March 18, 2016, 12:14 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44777/
> ---
> 
> (Updated March 18, 2016, 12:14 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4926
> https://issues.apache.org/jira/browse/MESOS-4926
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a flags parser for vector to src/common/parse.hpp.
> 
> 
> Diffs
> -
> 
>   src/common/parse.hpp 9535fad0d50b469fbb4dc6ac5cf5c89b40d29b47 
> 
> Diff: https://reviews.apache.org/r/44777/diff/
> 
> 
> Testing
> ---
> 
> Ran `make` and `make check`
> 
> This is also tested out in a subsequent commit when adding support for the 
> `--nvidia_gpu_devices` flag, which uses this functionality.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 44776: Did a general cleanup of s/.get()./->/ in the resources abstraction.

[Ben Mahler]

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


Ship it!




Ship It!

- Ben Mahler


On March 14, 2016, 7:36 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44776/
> ---
> 
> (Updated March 14, 2016, 7:36 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: 4928
> https://issues.apache.org/jira/browse/4928
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Did a general cleanup of s/.get()./->/ in the resources abstraction.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 1f23dc83f7330c305a836d698f114b7eaf3d7ba1 
>   src/v1/resources.cpp c6f125ec317e2da537a6456f5cff2da0a48701d8 
> 
> Diff: https://reviews.apache.org/r/44776/diff/
> 
> 
> Testing
> ---
> 
> Ran `make` and `make check`
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 45423: Added note about preventing resource autodetecting to documentation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45423]

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

- Mesos ReviewBot


On March 29, 2016, 2:02 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45423/
> ---
> 
> (Updated March 29, 2016, 2:02 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When manually specifying disk resources as explained in the multiple-disk
> documentation, Mesos stops autodetecting the Root disk. This can be
> unexpected behavior for users.
> 
> 
> Diffs
> -
> 
>   docs/multiple-disk.md 5c15b64b8b7737159cfba21223a84bdfcac75a0b 
> 
> Diff: https://reviews.apache.org/r/45423/diff/
> 
> 
> Testing
> ---
> 
> Viewed as gist: https://gist.github.com/joerg84/ee8a6b5a92e71d6bf704
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 44808: Fixup POSIX build by removing headers from load.*.

[Daniel Pravat]

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


Ship it!




Ship It!

- Daniel Pravat


On March 14, 2016, 9:06 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44808/
> ---
> 
> (Updated March 14, 2016, 9:06 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixup POSIX build by removing headers from load.*.
> 
> 
> Diffs
> -
> 
>   src/slave/qos_controllers/load.hpp 098a6d0b2dfc54b5b95a261a780eea70a838c12d 
>   src/slave/qos_controllers/load.cpp dd44f9209ad283bfea95f16a8c1017e309757f23 
> 
> Diff: https://reviews.apache.org/r/44808/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 42516: Add support for user-defined networks.

[Avinash sridharan]

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




src/docker/docker.cpp (line 568)


Maybe s/ Found .../Found multiple networks specified for this container. We 
can only attach docker containers to a single network/

We try to avoid variable names in `Error` since it becomes cryptic. Also, I 
guess the limiation is of the `DockerContainerizer` rather than `Docker`?


- Avinash sridharan


On March 29, 2016, 4:33 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 29, 2016, 4:33 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Re: Review Request 45082: Implemented cleanup() method of "network/cni" isolator.

[Avinash sridharan]

> On March 22, 2016, 3:37 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 644-645
> > 
> >
> > These two CHECKS don't make sense. What if the plugin got deleted or 
> > there was a bug in the plugin because of which it wasn't able to delete the 
> > interfaces or release the IP addresses. The Agent should not die because of 
> > an error in a 3rd party plugin.
> 
> Qian Zhang wrote:
> If the plugin got deleted, then `Subprocess.isError()` will be true and 
> we will return a `Failure`, if there was a bug in the plugin, then 
> `Subprocess.status()` will be non-zero and we will read its output for the 
> detailed error message. For either of these cases, agent will not die.

The fact that we are failing on the plugin returning an error seems very 
dangerous. A malicious plugin can start crashing the Agent which is just bad 
behavior. We should avoid this at all costs. CHECKS are more defensive to test 
an internal logic, they should not be used for any external inputs. I would 
rather throw an error and get out.


> On March 22, 2016, 3:37 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 647-649
> > 
> >
> > This is a bit odd, __disconnect always returns an error ? The plugin 
> > can return error codes and error logs which we should be propagating 
> > upstream through failure semantics.
> 
> Qian Zhang wrote:
> The reason that `__disconnect()` always returns a `Failure` is, it will 
> be only called when the exit code of plugin is not 0 (i.e., plugin fails for 
> a reason), in case of plugin success, we will return `Nothing()` in 
> `_disconnect()`. So we only need `__disconnect()` in case of plugin failure 
> to get the output (i.e., detailed failure reason) of plugin and propagate it 
> upstream.

I would suggest s/Failed to execute the CNI plugin/CNI plugin failed to detach 
network/ . "Failed to execute ..." implies that we not able to launch the 
plugin, which might not be the case here.


- Avinash


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


On March 29, 2016, 10:59 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45082/
> ---
> 
> (Updated March 29, 2016, 10:59 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented cleanup() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> b1b7205f4f10b6dc256fcc4ecb3210105c1240b4 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 7cda5715814a0cfc4b394eb04437831e6dc44e3f 
> 
> Diff: https://reviews.apache.org/r/45082/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 45383: Implemented recover() method of "network/cni" isolator.

[Avinash sridharan]

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




src/slave/containerizer/mesos/isolators/network/cni/cni.hpp (line 103)


Why do we need to use a hashmap over here? Why not a vector or a list? 
Hashmap's are expensive in terms of memory. Usually a container won't be 
associated with more than one network, so seems pretty inefficient?



src/slave/containerizer/mesos/isolators/network/cni/cni.hpp (line 139)


Maybe s/recoverInfo/_recover/



src/slave/containerizer/mesos/isolators/network/cni/cni.hpp (line 151)


Should have pointed in the earlier patches, why do we need `Info` to be a 
smart pointer. We can have it as an object. We are not going to share this 
information with any other class or thread.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 136)


Why do we have this change as part of this patch? Can we move this out to a 
different patch?



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 288 - 290)


Do we need this loop? `infos.clear` below should destroy the hashmap which 
in turn would destroy any of the residing objects (`networkinfos`)



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 


I think we should have this in a separate patch?



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 809)


Isn't this a problem? We are returning `Nothing` here, which implies that 
in the `recover` method the container information is assumed to have been 
stored correctly. However, there is NO container informationt that has been 
stored!! I thought we should never run into this issue, since we should be 
creating the directories before calling the CNI plugin?



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 824 - 827)


What if the isolator dies right after creating the directories, but after 
invoking the plugin? If we just return `Nothing` we might end up leaking state 
information from the CNI plugin (IPAM). I think we should return `Error` here 
and ask the isolator to cleanup the containers and start over.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 870)


This error doesn't make sense. Maybe:
"Found multiple interfaces for a given CNI network. This should not be 
allowed" ?



src/slave/containerizer/mesos/isolators/network/cni/spec.cpp (line 53)


Why not just return `parse` ?


- Avinash sridharan


On March 29, 2016, 11 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45383/
> ---
> 
> (Updated March 29, 2016, 11 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recover() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> b1b7205f4f10b6dc256fcc4ecb3210105c1240b4 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 7cda5715814a0cfc4b394eb04437831e6dc44e3f 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.hpp 
> 6a3c33645bab73edaf5af4d298a671852ea59c46 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.cpp 
> 5b5f904def9ef6dcc4462a03a2d024ad4eb3d946 
> 
> Diff: https://reviews.apache.org/r/45383/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 42861: Ensure two Mount Disk resources do not have the same root path.

[haosdent huang]

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

(Updated March 29, 2016, 5:33 p.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, Joris Van Remoortere, and 
Neil Conway.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Ensure two Mount Disk resources do not have the same root path.


Diffs (updated)
-

  src/slave/slave.cpp fc77f594d16a9fb2ca001e089d74e2c0ffeb5baa 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 42860: Add paths::overlapping to check whether paths are overlapping.

[haosdent huang]

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

(Updated March 29, 2016, 5:33 p.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, Joris Van Remoortere, and 
Neil Conway.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Add paths::overlapping to check whether paths are overlapping.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp 
ef538045a8b7a1e3d8962c869317d86a85e0259f 
  3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp 
6dff5e76e0e15098c5a262adc50bfcb65f933697 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 42861: Ensure two Mount Disk resources do not have the same root path.

[haosdent huang]

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

(Updated March 29, 2016, 5:29 p.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, Joris Van Remoortere, and 
Neil Conway.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Ensure two Mount Disk resources do not have the same root path.


Diffs (updated)
-

  src/slave/slave.cpp fc77f594d16a9fb2ca001e089d74e2c0ffeb5baa 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45419: Cleaned up ModuleManager.

[Kapil Arya]

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


Ship it!





src/module/manager.cpp (lines 41 - 42)


Should we use the following instead?
```
namespace mesos {
namespace module {
```

Although there is no consistensy in the code base, the above format seems 
to be used more often :).


- Kapil Arya


On March 29, 2016, 11:41 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45419/
> ---
> 
> (Updated March 29, 2016, 11:41 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Kapil Arya.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removes unused includes and usings from the ModuleManager.
> 
> 
> Diffs
> -
> 
>   src/module/manager.hpp 9944af0 
>   src/module/manager.cpp 8c9aaf7 
> 
> Diff: https://reviews.apache.org/r/45419/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 44706: Implemented isolate() method of "network/cni" isolator.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 468)


You don't need this PIPE if you don't care about stderr. I'll fix it for 
you.


- Jie Yu


On March 29, 2016, 10:58 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44706/
> ---
> 
> (Updated March 29, 2016, 10:58 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented isolate() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 7bda2ac684f38da94e334f0cef843614687ae4aa 
>   src/Makefile.am 6552e48eab2708a28dd69adba3ec759cb5aeca4c 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> b1b7205f4f10b6dc256fcc4ecb3210105c1240b4 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 7cda5715814a0cfc4b394eb04437831e6dc44e3f 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44706/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 45399: Fixed capitalization of Watchdog enum.

[Jie Yu]

> On March 29, 2016, 5:02 a.m., Jie Yu wrote:
> > Can you provide more context for this change? For instance, why change 
> > Watchdog and not Setsid?
> 
> Joerg Schad wrote:
> Before the enum name WATCHDOG was all capitalized (vs. Setsid).

oops, i should not do reviews late night:)


- Jie


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


On March 28, 2016, 9:41 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45399/
> ---
> 
> (Updated March 28, 2016, 9:41 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed capitalization of Watchdog enum.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> da806640a238e168bd24ea837cbb711041fe1d12 
>   3rdparty/libprocess/src/subprocess.cpp 
> be32962e94b605ee2e15d35010acd05d58c2b2d3 
> 
> Diff: https://reviews.apache.org/r/45399/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45383: Implemented recover() method of "network/cni" isolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44004, 44200, 44549, 44555, 44622, 44514, 44706, 45082, 45383]

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

- Mesos ReviewBot


On March 29, 2016, 11 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45383/
> ---
> 
> (Updated March 29, 2016, 11 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recover() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> b1b7205f4f10b6dc256fcc4ecb3210105c1240b4 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 7cda5715814a0cfc4b394eb04437831e6dc44e3f 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.hpp 
> 6a3c33645bab73edaf5af4d298a671852ea59c46 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.cpp 
> 5b5f904def9ef6dcc4462a03a2d024ad4eb3d946 
> 
> Diff: https://reviews.apache.org/r/45383/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 45434: Added test cases for '/files' endpoint authentication.

[Jan Schlicht]

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

Review request for mesos, Adam B and Greg Mann.


Repository: mesos


Description
---

The current authentication tests of the `/files/` endpoint only test the case 
of missing credentials. This commit adds the additional case of wrong 
credentials.


Diffs
-

  src/tests/files_tests.cpp 2f0f84cd78e7f382b70b97c8711f71cea64ce0a5 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 42516: Add support for user-defined networks.

[Ezra Silvera]

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

(Updated March 29, 2016, 4:33 p.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Changes
---

Don't allow multiple network_info elements during "docker run"


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
  include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
  src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera

Re: Review Request 42516: Add support for user-defined networks.

[Ezra Silvera]

> On March 28, 2016, 3:13 p.m., Avinash sridharan wrote:
> > src/docker/docker.cpp, line 562
> > 
> >
> > We should make sure the user is not trying to specify  more than one 
> > network for this container (multiple `NetworkInfo`). Docker 1.9 supports 
> > multiple user networks but the container can be connected to more than one 
> > network only after being started (using `docker network connect`) which is 
> > kind of useless. Docker 1.10 apparently allows you to attach container to 
> > multiple network but not using the `docker run` command, so doesn't fit the 
> > model for `DockerContainerizer`.
> 
> Ezra Silvera wrote:
> I'm not sure I follow you point ...  Indeed in Docker run it's not 
> allowed to specify multiple networks this is the reason we use the first 
> element in the  array.  In general if you are not going to populate this 
> structure through inspect I'm not sure you even need an array there ..
> 
> Ezra Silvera wrote:
> Let me know if you think we should do something specific in this code, or 
> it was just a general remark about NetworkInfo
> 
> Avinash sridharan wrote:
> Should have been clearer. The `NetworkInfo` field in `ContainerInfo` is a 
> repeated field. So the framework can potentially ask the container to join 
> multiple networks (this is supported in CNI 
> https://issues.apache.org/jira/browse/MESOS-4641) . We should explicitly 
> disallow it over here. So may be an `if` condition :
> if (containerInfo.network_infos_size() > 1) {
>return Failure();
> }
> 
> Avinash sridharan wrote:
> Just to add. The reason I think raising this error is important is that 
> we should explicitly inform the Framework that they are trying to do 
> something that is not supported, rather than making an assumption that the 
> first network specified by the `Framework` was the network it intended the 
> container to join. This is ofcourse valid only in case multiple user-networks 
> have been specified.

I fully agree. I currently print just a warning   I also think we should fail 
the operation!


- Ezra


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


On March 29, 2016, 11:08 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 29, 2016, 11:08 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Re: Review Request 45397: Changed name of http-parser enum to 'flags_enum'.

[Greg Mann]

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

(Updated March 29, 2016, 4:15 p.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Changed name of http-parser enum to 'flags_enum'.


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
04a4cf4241b8188c093f4942d27b050fa7b20397 
  3rdparty/libprocess/3rdparty/Makefile.am 
385302a491aba5a38ff74c0debfc2a423b0c5a8a 
  3rdparty/libprocess/3rdparty/http-parser-2.6.1.patch PRE-CREATION 

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


Testing
---

`../configure --enable-libevent --enable-ssl && make check` was used to text on 
OSX.


Thanks,

Greg Mann

Re: Review Request 44378: Upgrade libev to 4.22 to support PowerPC LE platform [libprocess].

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45371, 44378]

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

- Mesos ReviewBot


On March 29, 2016, 10:34 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44378/
> ---
> 
> (Updated March 29, 2016, 10:34 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-4803
> https://issues.apache.org/jira/browse/MESOS-4803
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade libev to 4.22 to support PowerPC LE platform [libprocess].
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 04a4cf4241b8188c093f4942d27b050fa7b20397 
>   3rdparty/libprocess/3rdparty/libev-4.15.patch 
> bbd83e6928e6caba3bc5a9739823d60923cfaa2a 
>   3rdparty/libprocess/3rdparty/libev-4.15.tar.gz 
> 4c282b573aa9331fd16197ef286faf323b6515eb 
>   3rdparty/libprocess/3rdparty/libev-4.22.patch PRE-CREATION 
>   3rdparty/libprocess/3rdparty/libev-4.22.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/versions.am 
> 462a867f4c4e8fce0b62f95bac05ae069281393f 
> 
> Diff: https://reviews.apache.org/r/44378/diff/
> 
> 
> Testing
> ---
> 
> ../configure --enable-libevent --enable-ssl
> 
> sudo make dist check
> 
> sudo ./src/mesos-tests --benchmark
> 
> sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 45429: Added authentication to the '/registry' endpoint.

[Jan Schlicht]

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

(Updated March 29, 2016, 5:46 p.m.)


Review request for mesos, Adam B, Joerg Schad, and Till Toenshoff.


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


Repository: mesos


Description (updated)
---

See summary.


Diffs
-

  src/local/local.cpp e777ea2938a23db8b407676a0f7e635e63d032fa 
  src/master/main.cpp 58561cffa440aaf1293e9ffe19b5685e6d2f1952 
  src/master/registrar.hpp f087569bd6774ac16093f9204e6870e4d4404420 
  src/master/registrar.cpp def40b94689c363617a7cfbcce82ea8d357cb345 
  src/tests/cluster.cpp 2da0bd7612d571277e76d0a95ad8e776434af323 
  src/tests/registrar_tests.cpp 39caf9bb950c0b229a66becb039c7a830a18f6bc 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 45429: Added authentication to the '/registry' endpoint.

[Jan Schlicht]

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

(Updated March 29, 2016, 5:46 p.m.)


Review request for mesos, Adam B, Joerg Schad, and Till Toenshoff.


Changes
---

Added tests case of missing credentials.


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


Repository: mesos


Description (updated)
---

Added authentication to the '/registry' endpoint.


Diffs (updated)
-

  src/local/local.cpp e777ea2938a23db8b407676a0f7e635e63d032fa 
  src/master/main.cpp 58561cffa440aaf1293e9ffe19b5685e6d2f1952 
  src/master/registrar.hpp f087569bd6774ac16093f9204e6870e4d4404420 
  src/master/registrar.cpp def40b94689c363617a7cfbcce82ea8d357cb345 
  src/tests/cluster.cpp 2da0bd7612d571277e76d0a95ad8e776434af323 
  src/tests/registrar_tests.cpp 39caf9bb950c0b229a66becb039c7a830a18f6bc 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 45419: Cleaned up ModuleManager.

[Till Toenshoff]

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

(Updated March 29, 2016, 3:41 p.m.)


Review request for mesos, Benjamin Bannier and Kapil Arya.


Changes
---

Addressed Benjamin's comments.


Repository: mesos


Description (updated)
---

Removes unused includes and usings from the ModuleManager.


Diffs (updated)
-

  src/module/manager.hpp 9944af0 
  src/module/manager.cpp 8c9aaf7 

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


Testing
---

make check


Thanks,

Till Toenshoff

Re: Review Request 45397: Changed name of http-parser enum to 'flags_enum'.

[Greg Mann]

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

(Updated March 29, 2016, 3:23 p.m.)


Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Changed name of http-parser enum to 'flags_enum'.


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
04a4cf4241b8188c093f4942d27b050fa7b20397 
  3rdparty/libprocess/3rdparty/Makefile.am 
385302a491aba5a38ff74c0debfc2a423b0c5a8a 
  3rdparty/libprocess/3rdparty/http-parser-2.6.1.patch PRE-CREATION 

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


Testing
---

`../configure --enable-libevent --enable-ssl && make check` was used to text on 
OSX.


Thanks,

Greg Mann

Re: Review Request 42516: Add support for user-defined networks.

[Avinash sridharan]

> On March 28, 2016, 3:13 p.m., Avinash sridharan wrote:
> > src/docker/docker.cpp, line 562
> > 
> >
> > We should make sure the user is not trying to specify  more than one 
> > network for this container (multiple `NetworkInfo`). Docker 1.9 supports 
> > multiple user networks but the container can be connected to more than one 
> > network only after being started (using `docker network connect`) which is 
> > kind of useless. Docker 1.10 apparently allows you to attach container to 
> > multiple network but not using the `docker run` command, so doesn't fit the 
> > model for `DockerContainerizer`.
> 
> Ezra Silvera wrote:
> I'm not sure I follow you point ...  Indeed in Docker run it's not 
> allowed to specify multiple networks this is the reason we use the first 
> element in the  array.  In general if you are not going to populate this 
> structure through inspect I'm not sure you even need an array there ..
> 
> Ezra Silvera wrote:
> Let me know if you think we should do something specific in this code, or 
> it was just a general remark about NetworkInfo
> 
> Avinash sridharan wrote:
> Should have been clearer. The `NetworkInfo` field in `ContainerInfo` is a 
> repeated field. So the framework can potentially ask the container to join 
> multiple networks (this is supported in CNI 
> https://issues.apache.org/jira/browse/MESOS-4641) . We should explicitly 
> disallow it over here. So may be an `if` condition :
> if (containerInfo.network_infos_size() > 1) {
>return Failure();
> }

Just to add. The reason I think raising this error is important is that we 
should explicitly inform the Framework that they are trying to do something 
that is not supported, rather than making an assumption that the first network 
specified by the `Framework` was the network it intended the container to join. 
This is ofcourse valid only in case multiple user-networks have been specified.


- Avinash


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


On March 29, 2016, 11:08 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 29, 2016, 11:08 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Re: Review Request 42516: Add support for user-defined networks.

[Avinash sridharan]

> On March 28, 2016, 3:13 p.m., Avinash sridharan wrote:
> > src/docker/docker.cpp, line 562
> > 
> >
> > We should make sure the user is not trying to specify  more than one 
> > network for this container (multiple `NetworkInfo`). Docker 1.9 supports 
> > multiple user networks but the container can be connected to more than one 
> > network only after being started (using `docker network connect`) which is 
> > kind of useless. Docker 1.10 apparently allows you to attach container to 
> > multiple network but not using the `docker run` command, so doesn't fit the 
> > model for `DockerContainerizer`.
> 
> Ezra Silvera wrote:
> I'm not sure I follow you point ...  Indeed in Docker run it's not 
> allowed to specify multiple networks this is the reason we use the first 
> element in the  array.  In general if you are not going to populate this 
> structure through inspect I'm not sure you even need an array there ..
> 
> Ezra Silvera wrote:
> Let me know if you think we should do something specific in this code, or 
> it was just a general remark about NetworkInfo

Should have been clearer. The `NetworkInfo` field in `ContainerInfo` is a 
repeated field. So the framework can potentially ask the container to join 
multiple networks (this is supported in CNI 
https://issues.apache.org/jira/browse/MESOS-4641) . We should explicitly 
disallow it over here. So may be an `if` condition :
if (containerInfo.network_infos_size() > 1) {
   return Failure();
}


- Avinash


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


On March 29, 2016, 11:08 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 29, 2016, 11:08 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Review Request 45429: Added authentication to the '/registry' endpoint.

[Jan Schlicht]

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

Review request for mesos, Adam B and Till Toenshoff.


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


Repository: mesos


Description
---

Added authentication to the '/registry' endpoint.


Diffs
-

  src/local/local.cpp e777ea2938a23db8b407676a0f7e635e63d032fa 
  src/master/main.cpp 58561cffa440aaf1293e9ffe19b5685e6d2f1952 
  src/master/registrar.hpp f087569bd6774ac16093f9204e6870e4d4404420 
  src/master/registrar.cpp def40b94689c363617a7cfbcce82ea8d357cb345 
  src/tests/cluster.cpp 2da0bd7612d571277e76d0a95ad8e776434af323 
  src/tests/registrar_tests.cpp 39caf9bb950c0b229a66becb039c7a830a18f6bc 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 45429: Added authentication to the '/registry' endpoint.

[Jan Schlicht]

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

(Updated March 29, 2016, 5:05 p.m.)


Review request for mesos, Adam B and Till Toenshoff.


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


Repository: mesos


Description (updated)
---

See summary.


Diffs
-

  src/local/local.cpp e777ea2938a23db8b407676a0f7e635e63d032fa 
  src/master/main.cpp 58561cffa440aaf1293e9ffe19b5685e6d2f1952 
  src/master/registrar.hpp f087569bd6774ac16093f9204e6870e4d4404420 
  src/master/registrar.cpp def40b94689c363617a7cfbcce82ea8d357cb345 
  src/tests/cluster.cpp 2da0bd7612d571277e76d0a95ad8e776434af323 
  src/tests/registrar_tests.cpp 39caf9bb950c0b229a66becb039c7a830a18f6bc 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 45398: Added CHANGELOG for python module changes.

[Steve Niemitz]

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

(Updated March 29, 2016, 2:21 p.m.)


Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Added CHANGELOG for python module changes.


Diffs (updated)
-

  CHANGELOG 4c73a2638074ac9f11de1438a5dd3d70e8201229 

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


Testing
---


Thanks,

Steve Niemitz

Re: Review Request 45419: Cleaned up ModuleManager.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45419]

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

- Mesos ReviewBot


On March 29, 2016, 9:17 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45419/
> ---
> 
> (Updated March 29, 2016, 9:17 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Kapil Arya.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removes unused includes and some unsused usings from the ModuleManager.
> 
> 
> Diffs
> -
> 
>   src/module/manager.hpp 9944af0daf6c9cb5a8ff338099401b1db88ee237 
>   src/module/manager.cpp 8c9aaf7cd00c904daba9994a99df9e1329831c01 
> 
> Diff: https://reviews.apache.org/r/45419/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 45398: Added CHANGELOG for python module changes.

[Steve Niemitz]

> On March 28, 2016, 9:55 p.m., Vinod Kone wrote:
> > Can you also update the release guide 
> > (https://github.com/apache/mesos/blob/0af8bd6fe28e3cad82d7e05ae3e308c3b2febf29/docs/release-guide.md)
> >  regarding how to upload these new packages to pypi?

As far as I can tell, only mesos.interface is on pypi, the native modules 
aren't.


> On March 28, 2016, 9:55 p.m., Vinod Kone wrote:
> > CHANGELOG, line 26
> > 
> >
> > How about
> > 
> > // This change also removes un-necessary 3rd party dependencies from 
> > `mesos.executor` and `mesos.scheduler`.

 sure


> On March 28, 2016, 9:55 p.m., Vinod Kone wrote:
> > CHANGELOG, line 28
> > 
> >
> > s/modules/modules,/

There shouldn't be a comma there, the way its written is correct.


- Steve


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


On March 28, 2016, 9:13 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45398/
> ---
> 
> (Updated March 28, 2016, 9:13 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added CHANGELOG for python module changes.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 4c73a2638074ac9f11de1438a5dd3d70e8201229 
> 
> Diff: https://reviews.apache.org/r/45398/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 45423: Added note about preventing resource autodetecting to documentation.

[Joerg Schad]

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

(Updated March 29, 2016, 2:02 p.m.)


Review request for mesos, Joris Van Remoortere and Neil Conway.


Changes
---

Addressed review.


Summary (updated)
-

Added note about preventing resource autodetecting to documentation.


Repository: mesos


Description
---

When manually specifying disk resources as explained in the multiple-disk
documentation, Mesos stops autodetecting the Root disk. This can be
unexpected behavior for users.


Diffs (updated)
-

  docs/multiple-disk.md 5c15b64b8b7737159cfba21223a84bdfcac75a0b 

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


Testing
---

Viewed as gist: https://gist.github.com/joerg84/ee8a6b5a92e71d6bf704


Thanks,

Joerg Schad

Re: Review Request 45423: Added note about preventing resource autodetection to documentation.

[Neil Conway]

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


Fix it, then Ship it!





docs/multiple-disk.md (line 31)


suggested: "manually (via the `--resources` flag)"



docs/multiple-disk.md (line 33)


suggested: "manually specify it using the format described below."


- Neil Conway


On March 29, 2016, 10:24 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45423/
> ---
> 
> (Updated March 29, 2016, 10:24 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When manually specifying disk resources as explained in the multiple-disk
> documentation, Mesos stops autodetecting the Root disk. This can be
> unexpected behavior for users.
> 
> 
> Diffs
> -
> 
>   docs/multiple-disk.md 5c15b64b8b7737159cfba21223a84bdfcac75a0b 
> 
> Diff: https://reviews.apache.org/r/45423/diff/
> 
> 
> Testing
> ---
> 
> Viewed as gist: https://gist.github.com/joerg84/ee8a6b5a92e71d6bf704
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

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

[Joris Van Remoortere]

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




3rdparty/libprocess/src/process.cpp (line 2193)


since this value will not longer always map to the number of `cpus`, can we 
rename this to something more meaningful such as `num_worker_threads`?



3rdparty/libprocess/src/process.cpp (line 2204)


I don't think we tend to use `auto` for this pattern. Do you mind 
explicitly stating the type?



3rdparty/libprocess/src/process.cpp (line 2208)


please use full words for variable names (i.e. `num`)



3rdparty/libprocess/src/process.cpp (lines 2210 - 2212)


It seems valuable to log the number of threads we will use regardless of 
whether they were over-ridden or not.

Let's log the actual number outside of the `if` block. This will also 
simplify the message (e.g. `Using X libprocess worker threads`



docs/configuration.md (line 1761)


Shall we explain what the default is?


- Joris Van Remoortere


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

Re: Review Request 45399: Fixed capitalization of Watchdog enum.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On March 28, 2016, 9:41 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45399/
> ---
> 
> (Updated March 28, 2016, 9:41 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed capitalization of Watchdog enum.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> da806640a238e168bd24ea837cbb711041fe1d12 
>   3rdparty/libprocess/src/subprocess.cpp 
> be32962e94b605ee2e15d35010acd05d58c2b2d3 
> 
> Diff: https://reviews.apache.org/r/45399/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45367: Upgrade protobuf to 2.6.1 to support PowerPC LE platform [mesos].

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44257, 45368, 45367]

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

- Mesos ReviewBot


On March 29, 2016, 7:14 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45367/
> ---
> 
> (Updated March 29, 2016, 7:14 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, James Peach, Kapil Arya, 
> Neil Conway, and Vinod Kone.
> 
> 
> Bugs: MESOS-4678
> https://issues.apache.org/jira/browse/MESOS-4678
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade protobuf to 2.6.1 to support PowerPC LE platform [mesos].
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
>   LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 
>   configure.ac 9ec4bc1cff3b0b46dd2e7ece2c1f2d19ffb8 
>   src/java/mesos.pom.in 7615d61eb6fedfa0ead785cd360946c56ccf80af 
>   src/python/interface/setup.py.in d73996734c3a3c70c3a6c0c697bb6733c241c091 
>   src/python/native_common/ext_modules.py.in 
> c335bd83024bc07b6243dd59d775e7f29adc7520 
>   src/python/protocol/setup.py.in 4c50fbbf1ce11c4c42c848364523225ee7ea5a3b 
> 
> Diff: https://reviews.apache.org/r/45367/diff/
> 
> 
> Testing
> ---
> 
> ../configure --enable-libevent --enable-ssl
> 
> sudo make dist check
> 
> sudo ./src/mesos-tests --benchmark
> 
> sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 45424: Fixed a typo in "future.hpp".

[Joerg Schad]

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


Ship it!




Ship It!

- Joerg Schad


On March 29, 2016, 11:49 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45424/
> ---
> 
> (Updated March 29, 2016, 11:49 a.m.)
> 
> 
> Review request for mesos and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 2531fa9ab7a665072bc9df053fc215c6a10ccd23 
> 
> Diff: https://reviews.apache.org/r/45424/diff/
> 
> 
> Testing
> ---
> 
> None: Not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 45424: Fixed a typo in "future.hpp".

[Alexander Rukletsov]

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

Review request for mesos and Joerg Schad.


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/include/process/future.hpp 
2531fa9ab7a665072bc9df053fc215c6a10ccd23 

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


Testing
---

None: Not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 42516: Add support for user-defined networks.

[Ezra Silvera]

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

(Updated March 29, 2016, 11:08 a.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Changes
---

Typo Warnning -->  Warning


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
  include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
  src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera

Re: Review Request 45383: Implemented recover() method of "network/cni" isolator.

[Qian Zhang]

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

(Updated March 29, 2016, 7 p.m.)


Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


Repository: mesos


Description
---

Implemented recover() method of "network/cni" isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
b1b7205f4f10b6dc256fcc4ecb3210105c1240b4 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
7cda5715814a0cfc4b394eb04437831e6dc44e3f 
  src/slave/containerizer/mesos/isolators/network/cni/paths.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni/paths.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni/spec.hpp 
6a3c33645bab73edaf5af4d298a671852ea59c46 
  src/slave/containerizer/mesos/isolators/network/cni/spec.cpp 
5b5f904def9ef6dcc4462a03a2d024ad4eb3d946 

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 45082: Implemented cleanup() method of "network/cni" isolator.

[Qian Zhang]

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

(Updated March 29, 2016, 6:59 p.m.)


Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


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


Repository: mesos


Description
---

Implemented cleanup() method of "network/cni" isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
b1b7205f4f10b6dc256fcc4ecb3210105c1240b4 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
7cda5715814a0cfc4b394eb04437831e6dc44e3f 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 44706: Implemented isolate() method of "network/cni" isolator.

[Qian Zhang]

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

(Updated March 29, 2016, 6:58 p.m.)


Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


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


Repository: mesos


Description
---

Implemented isolate() method of "network/cni" isolator.


Diffs (updated)
-

  src/CMakeLists.txt 7bda2ac684f38da94e334f0cef843614687ae4aa 
  src/Makefile.am 6552e48eab2708a28dd69adba3ec759cb5aeca4c 
  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
b1b7205f4f10b6dc256fcc4ecb3210105c1240b4 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
7cda5715814a0cfc4b394eb04437831e6dc44e3f 
  src/slave/containerizer/mesos/isolators/network/cni/paths.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni/paths.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 42516: Add support for user-defined networks.

[Ezra Silvera]

> On March 29, 2016, 8:54 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, line 567
> > 
> >
> > Can you please add some comments here: If there are multiple network 
> > infos, the docker containerizer will only take the first network info.
> > 
> > It is a tricky part here, if an operator defined many network infos and 
> > only the first one do not have network name, error will return here.
> 
> Ezra Silvera wrote:
> We need to resolve this "many network infos" once and for all :-)  
> In Docker you can't specify more then a single network during the 
> run/create command. We are handling here the run command so in this case we 
> expect a single NetworkInfo element, anything else will be an error. 
> Currently we choose to just ignore it, we can actually fail the operation for 
> elements > 1 if you think it will be more clear.
> 
> Guangya Liu wrote:
> I think that logging some warning message may be good enough if there are 
> multiple networks.
> 
> Can you please also add @jieyu as he is also tracking this issue and we 
> can get some comments from him ;-)

Add a warnning. Keep in mind that Only single network_info can be used for 
Docker run.


- Ezra


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


On March 29, 2016, 10:46 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 29, 2016, 10:46 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Re: Review Request 42516: Add support for user-defined networks.

[Ezra Silvera]

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

(Updated March 29, 2016, 10:46 a.m.)


Review request for mesos and Timothy Chen.


Changes
---

Add warnning for multiple network_infos.


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
  include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
  src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera

Re: Review Request 45368: Upgrade protobuf to 2.6.1 to support PowerPC LE platform [libprocess].

[Zhiwei Chen]

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

(Updated March 29, 2016, 6:47 p.m.)


Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
and Vinod Kone.


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


Repository: mesos


Description
---

Upgrade protobuf to 2.6.1 to support PowerPC LE platform [libprocess].


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/protobuf-2.5.0.tar.gz 
e600ac57be4c88efb5f146e4b3ec226d8f685033 
  3rdparty/libprocess/3rdparty/protobuf-2.6.1.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/versions.am 
462a867f4c4e8fce0b62f95bac05ae069281393f 

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


Testing
---

../configure --enable-libevent --enable-ssl

sudo make dist check

sudo ./src/mesos-tests --benchmark

sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build


Thanks,

Zhiwei Chen