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 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 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 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
> 
>

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 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 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 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 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 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

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 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
> 
>

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 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 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 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 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: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 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 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 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 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 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 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 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 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 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 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 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 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 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
> 
>

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 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 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

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 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 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:
> ```
> 

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
> 
>

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
> 
>

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

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
> 
>

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 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 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
> 
>

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

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

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 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 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 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 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 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
> 
>

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
> 
>

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 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

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 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]

> 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
> 
>

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 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 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 ..

Let me know if you think we should do something specific in this code, or it 
was just a general remark about NetworkInfo


- Ezra


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


On March 29, 2016, 8:34 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, 8:34 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 44257: Upgrade protobuf to 2.6.1 to support PowerPC LE platform [stout].

[Zhiwei Chen]

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

(Updated March 29, 2016, 3:14 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 [stout].


Diffs
-

  3rdparty/libprocess/3rdparty/stout/README.md 
c534835db7baca1138791f2c700e95ff73052d85 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 
3d1f13082a65f9b1694ee7c65ba0cec131c18c5a 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 
fb11b1147b3a1872f60e90d0691723f9b2985427 

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


Testing (updated)
---

../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 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, 3:14 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
-

  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 
98195b8eb60b2673d610d8ab7ea31103f137debf 

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


Testing (updated)
---

../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 45367: Upgrade protobuf to 2.6.1 to support PowerPC LE platform [mesos].

[Zhiwei Chen]

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

(Updated March 29, 2016, 3:14 p.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 (updated)
---

../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 42516: Add support for user-defined networks.

[Guangya Liu]

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




src/docker/docker.cpp (lines 553 - 572)


I prefer the following format:

case ContainerInfo::DockerInfo::USER: {
  // User defined networks require docker version >= 1.9.0.
  Try validateVersion =
this->validateVersion(Version(1, 9, 0));

  if (validateVersion.isError()) {
return Failure("User defined networks require Docker "
   "version 1.9.0 or higher");
  }

  if (containerInfo.network_infos_size() == 0) {
return Failure("No network info found in container info");
  }

  const NetworkInfo& networkInfo = containerInfo.network_infos(0);
  if(!networkInfo.has_name()){
return Failure("No network name found in network info");
  }
  network = networkInfo.name();
  break;
}

The main changes are:
1) Add a period to the end of the comments in L554, also keep 2 spaces.
2) Add a blank space after L556.
2) Put `break` into the `{}` block.
3) Updated indent in L559.


- Guangya Liu


On 三月 29, 2016, 7:31 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 三月 29, 2016, 7:31 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 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, 3:04 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]

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

(Updated March 29, 2016, 7:31 a.m.)


Review request for mesos and Timothy Chen.


Changes
---

Fix Indentation


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 29, 2016, 7:51 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 553-572
> > 
> >
> > I prefer the following format:
> > 
> > case ContainerInfo::DockerInfo::USER: {
> >   // User defined networks require docker version >= 1.9.0.
> >   Try validateVersion =
> > this->validateVersion(Version(1, 9, 0));
> > 
> >   if (validateVersion.isError()) {
> > return Failure("User defined networks require Docker "
> >"version 1.9.0 or higher");
> >   }
> > 
> >   if (containerInfo.network_infos_size() == 0) {
> > return Failure("No network info found in container info");
> >   }
> > 
> >   const NetworkInfo& networkInfo = containerInfo.network_infos(0);
> >   if(!networkInfo.has_name()){
> > return Failure("No network name found in network info");
> >   }
> >   network = networkInfo.name();
> >   break;
> > }
> > 
> > The main changes are:
> > 1) Add a period to the end of the comments in L554, also keep 2 spaces.
> > 2) Add a blank space after L556.
> > 2) Put `break` into the `{}` block.
> > 3) Updated indent in L559.

Thanks. Fixed.


- Ezra


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


On March 29, 2016, 7:31 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, 7:31 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 45399: Fixed capitalization of Watchdog enum.

[Joerg Schad]

> 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?

Before the enum name WATCHDOG was all capitalized (vs. Setsid).


- Joerg


---
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 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`.

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


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


On March 28, 2016, 10:51 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 28, 2016, 10:51 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, 8:34 a.m.)


Review request for mesos and Timothy Chen.


Changes
---

Fixing comments from Guangya Liu


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.

[Guangya Liu]

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




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.



src/docker/docker.cpp (line 570)


blank line here.


- Guangya Liu


On 三月 29, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 三月 29, 2016, 8:34 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]

> 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.

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.


- Ezra


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


On March 29, 2016, 8:34 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, 8:34 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 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, 3:11 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 45397: Changed name of http-parser enum to 'flags_enum'.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45397]

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 28, 2016, 9:58 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45397/
> ---
> 
> (Updated March 28, 2016, 9:58 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 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, 8:56 a.m.)


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


Changes
---

Address @neilc comments.


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, 8:56 a.m.)


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


Changes
---

Address @neilc comments.


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]

> On March 29, 2016, 3:30 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp, line 127
> > 
> >
> > What does "flat" mean, here? Also, is it worth adding a `CHECK` that 
> > all paths are absolute?

`flat` means the path is not like "/a/../c" or "/a//b". Do you think we need 
add a method like `path::simplify()` to flat the path passed in?


- haosdent


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


On March 29, 2016, 8:56 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42860/
> ---
> 
> (Updated March 29, 2016, 8:56 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, Joris Van Remoortere, and 
> Neil Conway.
> 
> 
> Bugs: MESOS-4521
> https://issues.apache.org/jira/browse/MESOS-4521
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add paths::overlapping to check whether paths are overlapping.
> 
> 
> Diffs
> -
> 
>   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 45410: Documented when to invoke `send` using the scheduler library.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45408, 45409, 45410]

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, 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 44706: Implemented isolate() method of "network/cni" isolator.

[Qian Zhang]

> On March 29, 2016, 1:33 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 354
> > 
> >
> > Should we return a Failure here  instead?

I think we should not return a Failure because there can be containers which 
does not opt in CNI networks in which case "network/cni" isolator should act as 
a no-op.


> On March 29, 2016, 1:33 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 396
> > 
> >
> > There's a problem with the current code. 'collect' (in isolate()) on 
> > 'attach' will fail if any of the call to the plugin (ADD) fails. And we'll 
> > call 'cleanup' right after that, it's likely that another call to the 
> > plugin (ADD) is still pending. I don't think calling DEL while an ADD is 
> > still pending is gonna work.
> > 
> > Therefore, I think we should use 'await' in isolate instead of 
> > 'collect' so that we can make sure all of them are in termimal state before 
> > we return the result.

Agree, will fix it soon.


> On March 29, 2016, 1:33 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 451
> > 
> >
> > You want to read 'stderr' as well.

But as per CNI spec (https://github.com/appc/cni/blob/master/SPEC.md#result), 
CNI plugins will never write to "stderr", they will always write to "stdout" in 
both success and failure cases.


- Qian


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


On March 29, 2016, 3:04 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44706/
> ---
> 
> (Updated March 29, 2016, 3:04 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
> -
> 
>   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 45419: Cleaned up ModuleManager.

[Benjamin Bannier]

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




src/module/manager.hpp (line 24)


Not yours, but doesn't seem to be used.



src/module/manager.hpp (line 33)


Not yours, but doesn't seem to be used.



src/module/manager.hpp (lines 34 - 36)


Not yours, but could you update this with the complete include list? I see 
use of definitions from at least `stout/errorbase.hpp`, `stout/foreach.hpp`, 
`stout/none.hpp`, `stout/option.hpp` and `stout/try.hpp`.



src/module/manager.cpp (line 27)


We don't seem to include headers both in the header and implementation file.



src/module/manager.cpp (lines 28 - 29)


Not yours, but doesn't seem to be used.



src/module/manager.cpp (lines 31 - 32)


Not yours, but could you sort these alphabetically?



src/module/manager.cpp (line 32)


Not yours, but doesn't seem to be used.



src/module/manager.cpp (line 34)


Could you makes these complete? I see the implementation is using 
definitions from at least `stout/check.hpp`, `stout/nothing.hpp`, 
`stout/synchronized.hpp`, and `process/owned.hpp`.



src/module/manager.cpp 


Usually we separate declarations outside of classes etc. by two empty 
lines, so I think this line should not have been removed. Note that there is 
some inconsitency here, since strictly following this rule would also make use 
set the declarations of above globals apart by two empty lines.


- Benjamin Bannier


On March 29, 2016, 11: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, 11: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 45371: Upgrade libev to 4.22 to support PowerPC LE platform [mesos].

[Zhiwei Chen]

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

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


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


Repository: mesos


Description
---

Upgrade libev to 4.22 to support PowerPC LE platform [mesos].


Diffs (updated)
-

  3rdparty/cmake/Versions.cmake 6530fccc43a11bfb423a2e2e39c954d114c5d902 
  LICENSE 4cc7f5b321a468bdbdac189b4cf8dc065735ebaa 
  src/python/native_common/ext_modules.py.in 
c335bd83024bc07b6243dd59d775e7f29adc7520 

Diff: https://reviews.apache.org/r/45371/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 44378: Upgrade libev to 4.22 to support PowerPC LE platform [libprocess].

[Zhiwei Chen]

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

(Updated March 29, 2016, 6:34 p.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 (updated)
-

  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 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 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 45381: Pass containerizer to ResourceMonitor and ResourceMonitorProcess.

[Jay Guo]

> On March 28, 2016, 3:52 p.m., Guangya Liu wrote:
> > src/slave/slave.hpp, lines 398-399
> > 
> >
> > A question here: What is the use of this API?
> 
> Jay Guo wrote:
> This line of code is leaked from another implementation. I intended to 
> retrieve a list container ids from slave. Anyhow, as we communicated with 
> Jie, they decided that the whole logic should be kept in slave process. So 
> this patch should be discarded. Thanks for reviewing!
> 
> Guangya Liu wrote:
> I think that you do not need to discard this patch but continue update 
> this patch should be good enough.

OK, will do.


- Jay


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


On March 28, 2016, 11:54 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45381/
> ---
> 
> (Updated March 28, 2016, 11:54 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is used by ResourceMonitorProcess to collect ContainerStatus
> internally, to avoid dispatching this action back and forth between
> slave process and monitor process.
> 
> 
> Diffs
> -
> 
>   src/slave/monitor.hpp 70a7c88fefd4577f53f85e3eccdd6b69ab6f3acd 
>   src/slave/monitor.cpp 5c1dd354595e67e5eb603c5d49850f84b84b 
>   src/slave/slave.hpp 3ba335fcd31a92af9609023daae328b7f4bf5e59 
>   src/slave/slave.cpp f383605a52f31d7b805ad6153adc409dcb40f83a 
>   src/tests/containerizer.hpp efc1ca87c00b04d70efd1e3f6acf4e763132d6b0 
>   src/tests/containerizer.cpp 4c7f5a26d38222daf013bd3ca9a133e12d7bf338 
>   src/tests/monitor_tests.cpp 5dcb2481ff2f1a7caf54036bc3e60c78feb982b1 
> 
> Diff: https://reviews.apache.org/r/45381/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> https://issues.apache.org/jira/browse/MESOS-4891
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

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/#review125847
---



Bad patch!

Reviews applied: [44378, 45371]

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

Error:
2016-03-29 09:32:18 URL:https://reviews.apache.org/r/45371/diff/raw/ 
[1888/1888] -> "45371.patch" [1]
error: patch failed: 3rdparty/cmake/Versions.cmake:5
error: 3rdparty/cmake/Versions.cmake: patch does not apply

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

- Mesos ReviewBot


On March 29, 2016, 2 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, 2 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 
> 6b07aefc58a1daa235b35e83832e47d1878e2e94 
>   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 
> 98195b8eb60b2673d610d8ab7ea31103f137debf 
> 
> 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 45342: Make the Action enum optional to support upgrades (MESOS-5031).

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45342]

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:04 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45342/
> ---
> 
> (Updated March 29, 2016, 2:04 a.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, and Vinod Kone.
> 
> 
> Bugs: MESOS-5031
> https://issues.apache.org/jira/browse/MESOS-5031
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix tries to make the Action enum in Authorization optional
> to support upgrades. See MESOS-4997 for details.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 944a493e0979c7ffbd99f3a67785a10425fd9040 
>   src/authorizer/local/authorizer.cpp 
> 0f0d9276337858984f0b19a82ffca74ee84dc650 
> 
> Diff: https://reviews.apache.org/r/45342/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

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 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 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

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 42516: Add support for user-defined networks.

[Guangya Liu]

> On 三月 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.

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 ;-)


- Guangya


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


On 三月 29, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 三月 29, 2016, 8:34 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 45381: Pass containerizer to ResourceMonitor and ResourceMonitorProcess.

[Jay Guo]

> On March 28, 2016, 11:02 p.m., Jie Yu wrote:
> > Chatted with BenM on this. I think we should just kill the ResourceMonitor 
> > and move all the logics to the agent. We still need to keep the 
> > /monitor/statistics.json and /monitor/statistics endpoints for backwards 
> > compatibility. But since slave properly sets up the delagates, this should 
> > just work.

OK. Although I'm thinking to still keep monitor as a separate cpp file, instead 
of bloating slave.cpp further, which is already 6000+ LoC.


- Jay


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


On March 28, 2016, 11:54 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45381/
> ---
> 
> (Updated March 28, 2016, 11:54 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is used by ResourceMonitorProcess to collect ContainerStatus
> internally, to avoid dispatching this action back and forth between
> slave process and monitor process.
> 
> 
> Diffs
> -
> 
>   src/slave/monitor.hpp 70a7c88fefd4577f53f85e3eccdd6b69ab6f3acd 
>   src/slave/monitor.cpp 5c1dd354595e67e5eb603c5d49850f84b84b 
>   src/slave/slave.hpp 3ba335fcd31a92af9609023daae328b7f4bf5e59 
>   src/slave/slave.cpp f383605a52f31d7b805ad6153adc409dcb40f83a 
>   src/tests/containerizer.hpp efc1ca87c00b04d70efd1e3f6acf4e763132d6b0 
>   src/tests/containerizer.cpp 4c7f5a26d38222daf013bd3ca9a133e12d7bf338 
>   src/tests/monitor_tests.cpp 5dcb2481ff2f1a7caf54036bc3e60c78feb982b1 
> 
> Diff: https://reviews.apache.org/r/45381/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> https://issues.apache.org/jira/browse/MESOS-4891
> 
> 
> Thanks,
> 
> Jay Guo
> 
>