Re: Review Request 44439: Added device support in cgroups abstraction.

[Abhishek Dasgupta]

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

(Updated March 7, 2016, 6:27 a.m.)


Review request for mesos, Ben Mahler, Felix Abecassis, Kevin Klues, and Niklas 
Nielsen.


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


Repository: mesos


Description
---

There are some helper methods added to support device cgroups in cgroups 
abstraction to aid isolators controlling access to devices.


Diffs (updated)
-

  src/linux/cgroups.hpp 51ccefd1fd2d0989b9bd31342d3d1e3701f88ed2 
  src/linux/cgroups.cpp df18ed46a2a96871f67c7eb4233c3b4c27b7aa1c 
  src/tests/containerizer/cgroups_tests.cpp 
acaed9b3f8a04964092cef413133834d0cf5a145 

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


Testing
---

make check


Thanks,

Abhishek Dasgupta

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

[Avinash sridharan]

> On March 4, 2016, 3:30 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 134
> > 
> >
> > I don't understand this comment. We just made sure the plugin does not 
> > exist? So what does the comment imply "it can
> > // still be valid as long as operator puts the CNI plugin binary
> > // that it uses under '--network_cni_plugins_dir'." ?
> > 
> > I think at this point we should return an error. If can't find an 
> > executable for a named network, the behavior will become undefined. We 
> > should bail at this point.
> 
> Qian Zhang wrote:
> My point is, if we can not find a plugin for a named network during 
> initilization, log a warning message to let operator know this issue, and 
> afterward operator can put the plugin in the plugin directory without 
> restarting agent, then the named network can still work.
> 
> Avinash sridharan wrote:
> Lets not rely on the operator heeding WARNING messages and fixing the 
> problem. My concern is that this is a `FATAL` error since before the operator 
> can rectify the error if containers are launched the behavior becomes 
> undefined.
> 
> Qian Zhang wrote:
> Agree, let's return an error :-)
> 
> Qian Zhang wrote:
> After more thinking, I think in this case, it makes more sense to log a 
> warning message and ignore the network config file rather than bail at this 
> point, because there can be other valid network config files. If in the end 
> there is no any valid network config files, we should definitely bail.
> 
> Avinash sridharan wrote:
> I think we should not allow any errors in the configs/plugins passed by 
> the operator . Reason being that frameworks are going to learn about networks 
> out-of-band, and if there are config/plugin errors we will have to throw 
> errors during task launch. Why should we allow the system to proceed knowing 
> that this is going to lead to erroneous situation? The only way the operator 
> can fix this error is by restarting the slave (and fixing the config), so 
> might as well bail out sooner rather than later.
> 
> Qian Zhang wrote:
> Can you please let me know how this can lead to erroneous situation? If a 
> network config file is invalid for whatever reason, "network/isolator" will 
> NOT load it and just ignore it, so how can framework launches a task to join 
> an invalid network which is not loaded by the isolator? I do not think 
> framework user has such knowledge, or you think framework user will know all 
> the network config files (valid or invalid) under "--network_cni_config_dir" 
> in some way?
> 
> Avinash sridharan wrote:
> Frameworks would know only the network name. Its the responsibility of 
> the operator to install the right config for the given `name`. Hence the 
> erroneous case. The fact that config was not loaded for valid network name 
> cause inconsistency between the frameworks view of what is available and the 
> isolators view of what is configurable.
> 
> Qian Zhang wrote:
> What about framework specifies a wrong network name by mistake? Even in 
> `create()` method we ensure every network config file is valid and agent is 
> started successfuly, there is still a chance for framework to specify a wrong 
> network name (which is actually out of our control), right? So my point is, 
> we have to handle this erroneous in launching task case anyway.
> 
> And I do not quite understand what is `the frameworks view of what is 
> available`, can you please elaborate how framework can know what is 
> available? My thinking is, in future we may expose the available CNI networks 
> to framework via an HTTP endpoint or even via an offer as shared resources, 
> but the networks exposed in this way must be the ones which are valid, the 
> invalids will not be loaded by isolator, hense will not be exposed.

Let me try explaining my perspective differently. If the operator was to 
provide erroneous parameter to the isolator, the isolator would  bail out. My 
perspective is that an erroneous network config is an erroneous parameter being 
provided to the isolator and we should treat it so. I believe this would 
simplify any error handling that needs to be done when frameworks launch tasks 
on misconfigured networks.


- Avinash


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


On March 6, 2016, 4:03 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44269/
> ---
> 
> (Updated March 6, 2016, 4:03 p.m.)
> 
> 
> Review request for mesos, 

Review Request 44439: Added device support in cgroups abstraction.

[Abhishek Dasgupta]

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

Review request for mesos, Ben Mahler, Felix Abecassis, Kevin Klues, and Niklas 
Nielsen.


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


Repository: mesos


Description
---

There are some helper methods added to support device cgroups in cgroups 
abstraction to aid isolators controlling access to devices.


Diffs
-

  src/linux/cgroups.hpp 51ccefd1fd2d0989b9bd31342d3d1e3701f88ed2 
  src/linux/cgroups.cpp df18ed46a2a96871f67c7eb4233c3b4c27b7aa1c 
  src/tests/containerizer/cgroups_tests.cpp 
acaed9b3f8a04964092cef413133834d0cf5a145 

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


Testing
---

make check


Thanks,

Abhishek Dasgupta

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

[Qian Zhang]

> On March 4, 2016, 11:30 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 134
> > 
> >
> > I don't understand this comment. We just made sure the plugin does not 
> > exist? So what does the comment imply "it can
> > // still be valid as long as operator puts the CNI plugin binary
> > // that it uses under '--network_cni_plugins_dir'." ?
> > 
> > I think at this point we should return an error. If can't find an 
> > executable for a named network, the behavior will become undefined. We 
> > should bail at this point.
> 
> Qian Zhang wrote:
> My point is, if we can not find a plugin for a named network during 
> initilization, log a warning message to let operator know this issue, and 
> afterward operator can put the plugin in the plugin directory without 
> restarting agent, then the named network can still work.
> 
> Avinash sridharan wrote:
> Lets not rely on the operator heeding WARNING messages and fixing the 
> problem. My concern is that this is a `FATAL` error since before the operator 
> can rectify the error if containers are launched the behavior becomes 
> undefined.
> 
> Qian Zhang wrote:
> Agree, let's return an error :-)
> 
> Qian Zhang wrote:
> After more thinking, I think in this case, it makes more sense to log a 
> warning message and ignore the network config file rather than bail at this 
> point, because there can be other valid network config files. If in the end 
> there is no any valid network config files, we should definitely bail.
> 
> Avinash sridharan wrote:
> I think we should not allow any errors in the configs/plugins passed by 
> the operator . Reason being that frameworks are going to learn about networks 
> out-of-band, and if there are config/plugin errors we will have to throw 
> errors during task launch. Why should we allow the system to proceed knowing 
> that this is going to lead to erroneous situation? The only way the operator 
> can fix this error is by restarting the slave (and fixing the config), so 
> might as well bail out sooner rather than later.
> 
> Qian Zhang wrote:
> Can you please let me know how this can lead to erroneous situation? If a 
> network config file is invalid for whatever reason, "network/isolator" will 
> NOT load it and just ignore it, so how can framework launches a task to join 
> an invalid network which is not loaded by the isolator? I do not think 
> framework user has such knowledge, or you think framework user will know all 
> the network config files (valid or invalid) under "--network_cni_config_dir" 
> in some way?
> 
> Avinash sridharan wrote:
> Frameworks would know only the network name. Its the responsibility of 
> the operator to install the right config for the given `name`. Hence the 
> erroneous case. The fact that config was not loaded for valid network name 
> cause inconsistency between the frameworks view of what is available and the 
> isolators view of what is configurable.

What about framework specifies a wrong network name by mistake? Even in 
`create()` method we ensure every network config file is valid and agent is 
started successfuly, there is still a chance for framework to specify a wrong 
network name (which is actually out of our control), right? So my point is, we 
have to handle this erroneous in launching task case anyway.

And I do not quite understand what is `the frameworks view of what is 
available`, can you please elaborate how framework can know what is available? 
My thinking is, in future we may expose the available CNI networks to framework 
via an HTTP endpoint or even via an offer as shared resources, but the networks 
exposed in this way must be the ones which are valid, the invalids will not be 
loaded by isolator, hense will not be exposed.


- Qian


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


On March 7, 2016, 12:03 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44269/
> ---
> 
> (Updated March 7, 2016, 12:03 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
> ---
> 
> Added the framework of 'network/cni' isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION 
>   

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

[Avinash sridharan]

> On March 4, 2016, 3:30 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 134
> > 
> >
> > I don't understand this comment. We just made sure the plugin does not 
> > exist? So what does the comment imply "it can
> > // still be valid as long as operator puts the CNI plugin binary
> > // that it uses under '--network_cni_plugins_dir'." ?
> > 
> > I think at this point we should return an error. If can't find an 
> > executable for a named network, the behavior will become undefined. We 
> > should bail at this point.
> 
> Qian Zhang wrote:
> My point is, if we can not find a plugin for a named network during 
> initilization, log a warning message to let operator know this issue, and 
> afterward operator can put the plugin in the plugin directory without 
> restarting agent, then the named network can still work.
> 
> Avinash sridharan wrote:
> Lets not rely on the operator heeding WARNING messages and fixing the 
> problem. My concern is that this is a `FATAL` error since before the operator 
> can rectify the error if containers are launched the behavior becomes 
> undefined.
> 
> Qian Zhang wrote:
> Agree, let's return an error :-)
> 
> Qian Zhang wrote:
> After more thinking, I think in this case, it makes more sense to log a 
> warning message and ignore the network config file rather than bail at this 
> point, because there can be other valid network config files. If in the end 
> there is no any valid network config files, we should definitely bail.
> 
> Avinash sridharan wrote:
> I think we should not allow any errors in the configs/plugins passed by 
> the operator . Reason being that frameworks are going to learn about networks 
> out-of-band, and if there are config/plugin errors we will have to throw 
> errors during task launch. Why should we allow the system to proceed knowing 
> that this is going to lead to erroneous situation? The only way the operator 
> can fix this error is by restarting the slave (and fixing the config), so 
> might as well bail out sooner rather than later.
> 
> Qian Zhang wrote:
> Can you please let me know how this can lead to erroneous situation? If a 
> network config file is invalid for whatever reason, "network/isolator" will 
> NOT load it and just ignore it, so how can framework launches a task to join 
> an invalid network which is not loaded by the isolator? I do not think 
> framework user has such knowledge, or you think framework user will know all 
> the network config files (valid or invalid) under "--network_cni_config_dir" 
> in some way?

Frameworks would know only the network name. Its the responsibility of the 
operator to install the right config for the given `name`. Hence the erroneous 
case. The fact that config was not loaded for valid network name cause 
inconsistency between the frameworks view of what is available and the 
isolators view of what is configurable.


- Avinash


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


On March 6, 2016, 4:03 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44269/
> ---
> 
> (Updated March 6, 2016, 4:03 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
> ---
> 
> Added the framework of 'network/cni' isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44269/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44252: Update glog-0.3.3.patch to support PowerPC LE platform.

[Zhiwei Chen]

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

(Updated March 7, 2016, 10:05 a.m.)


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


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


Repository: mesos


Description
---

Update glog-0.3.3.patch to support PowerPC LE platform.


Diffs
-

  3rdparty/libprocess/3rdparty/glog-0.3.3.patch 
76b8c0fe3b4615371e265bab713d62c896b7c3d6 

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


Testing
---


Thanks,

Zhiwei Chen

Re: Review Request 44382: Update leveldb-1.4.patch to support PowerPC LE platform.

[Zhiwei Chen]

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

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


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


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


Repository: mesos


Description
---

Update leveldb-1.4.patch to support PowerPC LE platform.


Diffs
-

  3rdparty/leveldb-1.4.patch ad8c19b9caa856ff85978ba832d48df11b3a83f0 

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


Testing
---


Thanks,

Zhiwei Chen

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

[Qian Zhang]

> On March 4, 2016, 11:30 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 134
> > 
> >
> > I don't understand this comment. We just made sure the plugin does not 
> > exist? So what does the comment imply "it can
> > // still be valid as long as operator puts the CNI plugin binary
> > // that it uses under '--network_cni_plugins_dir'." ?
> > 
> > I think at this point we should return an error. If can't find an 
> > executable for a named network, the behavior will become undefined. We 
> > should bail at this point.
> 
> Qian Zhang wrote:
> My point is, if we can not find a plugin for a named network during 
> initilization, log a warning message to let operator know this issue, and 
> afterward operator can put the plugin in the plugin directory without 
> restarting agent, then the named network can still work.
> 
> Avinash sridharan wrote:
> Lets not rely on the operator heeding WARNING messages and fixing the 
> problem. My concern is that this is a `FATAL` error since before the operator 
> can rectify the error if containers are launched the behavior becomes 
> undefined.
> 
> Qian Zhang wrote:
> Agree, let's return an error :-)
> 
> Qian Zhang wrote:
> After more thinking, I think in this case, it makes more sense to log a 
> warning message and ignore the network config file rather than bail at this 
> point, because there can be other valid network config files. If in the end 
> there is no any valid network config files, we should definitely bail.
> 
> Avinash sridharan wrote:
> I think we should not allow any errors in the configs/plugins passed by 
> the operator . Reason being that frameworks are going to learn about networks 
> out-of-band, and if there are config/plugin errors we will have to throw 
> errors during task launch. Why should we allow the system to proceed knowing 
> that this is going to lead to erroneous situation? The only way the operator 
> can fix this error is by restarting the slave (and fixing the config), so 
> might as well bail out sooner rather than later.

Can you please let me know how this can lead to erroneous situation? If a 
network config file is invalid for whatever reason, "network/isolator" will NOT 
load it and just ignore it, so how can framework launches a task to join an 
invalid network which is not loaded by the isolator? I do not think framework 
user has such knowledge, or you think framework user will know all the network 
config files (valid or invalid) under "--network_cni_config_dir" in some way?


> On March 4, 2016, 11:30 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, lines 108-115
> > 
> >
> > isn't this the same as !nameValue.isSome() ?
> > 
> > Also LOG(WARNING) should be LOG(ERROR)
> 
> Qian Zhang wrote:
> Please see 
> https://github.com/apache/mesos/blob/0.27.1/src/docker/docker.cpp#L255:L260 
> for the existing way to handle the return value of `json.get().find()`.
> 
> Avinash sridharan wrote:
> Well... !isSome implies isNone or isError is true which is what you are 
> aiming for ? Will simplify things I guess.
> 
> Qian Zhang wrote:
> Let's follow the existing way, I think it is better to not introduce 
> inconsistence :-)
> 
> Avinash sridharan wrote:
> Inconsistence in terms of coding style is bad. But copying logic just for 
> the sake of consistency doesn't make sense. Just because a logic is 
> implemented a certain way doesn't mean we can't improve on it. I will leave 
> it up to you, but would prefer if we can shorten the code if possible.

Can you please elaborate how to shorten the code? Do you mean instead of 
calling `isNone()` and `isError()` we can simply call `isSome()`? I do not 
think we can do it, because `isNone()` and `isError()` have different purposes, 
we need `isNone()` to handle the case that there is no "name" in the JSON, and 
we need `isError()` to handle the case that there is an error to find "name" in 
the JSON. That's why we return different error for `isNone()` and `isError()` 
respectively.


- Qian


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


On March 7, 2016, 12:03 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44269/
> ---
> 
> (Updated March 7, 2016, 12:03 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> 

Re: Review Request 44372: Upgrade http-parser to 2.6.1 to support Power LE platform.

[Zhiwei Chen]

> On March 4, 2016, 3:37 p.m., haosdent huang wrote:
> > 3rdparty/libprocess/3rdparty/Makefile.am, line 44
> > 
> >
> > How about align the `` here.
> 
> Zhiwei Chen wrote:
> There are too many backslashes that did not column aligned, how about 
> create another patch to fix this?
> 
> I don't want to do things that not related to this patch.
> 
> Thanks.
> 
> haosdent huang wrote:
> Seems only few `` not align. 
> https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/Makefile.am

I mean in many files many backslashes, not only this file. I think you can 
create a code clean task do things like this.


- Zhiwei


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


On March 4, 2016, 3:24 p.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44372/
> ---
> 
> (Updated March 4, 2016, 3:24 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-4805
> https://issues.apache.org/jira/browse/MESOS-4805
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade http-parser to 2.6.1 to support Power LE platform.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 6eac4dc0f7189e209e7d7232419e4de4bc0875c0 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> b8351ad0181d885a984580ae8de208ea0524b0e7 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> ddf7e3d9bf76d4a03c33f02d52ec29812aef8509 
>   3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.patch 
> f9fac12437a6bedc66353fda1ce9c0d7a383225a 
>   3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.tar.gz 
> b811b63ce0ad6d71d9d296fed76656c023c76fc5 
>   3rdparty/libprocess/3rdparty/versions.am 
> 98195b8eb60b2673d610d8ab7ea31103f137debf 
>   3rdparty/libprocess/Makefile.am ac8cc8d29baccf6e3a17367540ddd1f28585ef6d 
>   3rdparty/libprocess/src/decoder.hpp 
> a20b5ba8fc50d834573d253948645cc863f030dd 
>   LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 
> 
> Diff: https://reviews.apache.org/r/44372/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 44423: Added HTTP command executor to make files.

[Qian Zhang]

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

(Updated March 7, 2016, 8:04 a.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description (updated)
---

Added HTTP command executor to make files. For now the content in 
http_command_executor.cpp is identical to executor.cpp, and it will be updated 
in the subsequent review.


Diffs
-

  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/launcher/http_command_executor.cpp PRE-CREATION 

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


Testing
---

Make check


Thanks,

Qian Zhang

Re: Review Request 44423: Added HTTP command executor to make files.

[Qian Zhang]

> On March 6, 2016, 11:59 p.m., Vinod Kone wrote:
> > Can you update the description saying that for now the content in 
> > http_command_executor.cpp is identical to executor.cpp and that it will 
> > updated in the subsequent review?

Sure.


- Qian


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


On March 5, 2016, 11:02 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44423/
> ---
> 
> (Updated March 5, 2016, 11:02 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3558
> https://issues.apache.org/jira/browse/MESOS-3558
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added HTTP command executor to make files.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/launcher/http_command_executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44423/diff/
> 
> 
> Testing
> ---
> 
> Make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44435: Fixed a bug that causes the task stuck in staging state.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44435]

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 6, 2016, 9:23 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44435/
> ---
> 
> (Updated March 6, 2016, 9:23 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4878
> https://issues.apache.org/jira/browse/MESOS-4878
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a bug that causes the task stuck in staging state.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> af3ff5750649497d8852b4761c78d4cae5455a02 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 15f0f93d2e5c19a22f6cc4a71a7d94be4aaec2c1 
> 
> Diff: https://reviews.apache.org/r/44435/diff/
> 
> 
> Testing
> ---
> 
> - Added a new test `"MesosContainerizerProvisionerTest.ProvisionFailed"`
> - make check
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

[Avinash sridharan]

> On March 4, 2016, 3:30 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, lines 108-115
> > 
> >
> > isn't this the same as !nameValue.isSome() ?
> > 
> > Also LOG(WARNING) should be LOG(ERROR)
> 
> Qian Zhang wrote:
> Please see 
> https://github.com/apache/mesos/blob/0.27.1/src/docker/docker.cpp#L255:L260 
> for the existing way to handle the return value of `json.get().find()`.
> 
> Avinash sridharan wrote:
> Well... !isSome implies isNone or isError is true which is what you are 
> aiming for ? Will simplify things I guess.
> 
> Qian Zhang wrote:
> Let's follow the existing way, I think it is better to not introduce 
> inconsistence :-)

Inconsistence in terms of coding style is bad. But copying logic just for the 
sake of consistency doesn't make sense. Just because a logic is implemented a 
certain way doesn't mean we can't improve on it. I will leave it up to you, but 
would prefer if we can shorten the code if possible.


> On March 4, 2016, 3:30 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 108
> > 
> >
> > Conver this to LOG(ERROR)
> 
> Qian Zhang wrote:
> I think it should be LOG(WARNING) if we are going to ignore something in 
> the `create()` method of an isolator, please see 
> `PortMappingIsolatorProcess::create()`:
> 
> https://github.com/apache/mesos/blob/0.27.1/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp#L1321

Semantically this is an error in the network configuration file. Please see my 
comments below about bailing out when a config/plugin error occurs.


> On March 4, 2016, 3:30 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 134
> > 
> >
> > I don't understand this comment. We just made sure the plugin does not 
> > exist? So what does the comment imply "it can
> > // still be valid as long as operator puts the CNI plugin binary
> > // that it uses under '--network_cni_plugins_dir'." ?
> > 
> > I think at this point we should return an error. If can't find an 
> > executable for a named network, the behavior will become undefined. We 
> > should bail at this point.
> 
> Qian Zhang wrote:
> My point is, if we can not find a plugin for a named network during 
> initilization, log a warning message to let operator know this issue, and 
> afterward operator can put the plugin in the plugin directory without 
> restarting agent, then the named network can still work.
> 
> Avinash sridharan wrote:
> Lets not rely on the operator heeding WARNING messages and fixing the 
> problem. My concern is that this is a `FATAL` error since before the operator 
> can rectify the error if containers are launched the behavior becomes 
> undefined.
> 
> Qian Zhang wrote:
> Agree, let's return an error :-)
> 
> Qian Zhang wrote:
> After more thinking, I think in this case, it makes more sense to log a 
> warning message and ignore the network config file rather than bail at this 
> point, because there can be other valid network config files. If in the end 
> there is no any valid network config files, we should definitely bail.

I think we should not allow any errors in the configs/plugins passed by the 
operator . Reason being that frameworks are going to learn about networks 
out-of-band, and if there are config/plugin errors we will have to throw errors 
during task launch. Why should we allow the system to proceed knowing that this 
is going to lead to erroneous situation? The only way the operator can fix this 
error is by restarting the slave (and fixing the config), so might as well bail 
out sooner rather than later.


- Avinash


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


On March 6, 2016, 4:03 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44269/
> ---
> 
> (Updated March 6, 2016, 4:03 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
> ---
> 
> Added the framework of 'network/cni' isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   

Review Request 44435: Fixed a bug that causes the task stuck in staging state.

[Shuai Lin]

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

Review request for mesos.


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


Repository: mesos


Description
---

Fixed a bug that causes the task stuck in staging state.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
af3ff5750649497d8852b4761c78d4cae5455a02 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
15f0f93d2e5c19a22f6cc4a71a7d94be4aaec2c1 

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


Testing
---

- Added a new test `"MesosContainerizerProvisionerTest.ProvisionFailed"`
- make check


Thanks,

Shuai Lin

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44004, 44200, 44269]

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 6, 2016, 4:03 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44269/
> ---
> 
> (Updated March 6, 2016, 4:03 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
> ---
> 
> Added the framework of 'network/cni' isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44269/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44433: Added empty line for list in maintenance doc.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44433]

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 6, 2016, 3:52 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44433/
> ---
> 
> (Updated March 6, 2016, 3:52 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added empty line for list in maintenance doc.
> 
> 
> Diffs
> -
> 
>   docs/maintenance.md 365c920719dbd0c5e61efe1975547a2848647bce 
> 
> Diff: https://reviews.apache.org/r/44433/diff/
> 
> 
> Testing
> ---
> 
> Document update.
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

[Qian Zhang]

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

(Updated March 7, 2016, 12:03 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
---

Added the framework of 'network/cni' isolator.


Diffs (updated)
-

  src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 44423: Added HTTP command executor to make files.

[Vinod Kone]

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


Ship it!




Can you update the description saying that for now the content in 
http_command_executor.cpp is identical to executor.cpp and that it will updated 
in the subsequent review?

- Vinod Kone


On March 5, 2016, 3:02 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44423/
> ---
> 
> (Updated March 5, 2016, 3:02 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3558
> https://issues.apache.org/jira/browse/MESOS-3558
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added HTTP command executor to make files.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/launcher/http_command_executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44423/diff/
> 
> 
> Testing
> ---
> 
> Make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

[Qian Zhang]

> On March 4, 2016, 11:30 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 134
> > 
> >
> > I don't understand this comment. We just made sure the plugin does not 
> > exist? So what does the comment imply "it can
> > // still be valid as long as operator puts the CNI plugin binary
> > // that it uses under '--network_cni_plugins_dir'." ?
> > 
> > I think at this point we should return an error. If can't find an 
> > executable for a named network, the behavior will become undefined. We 
> > should bail at this point.
> 
> Qian Zhang wrote:
> My point is, if we can not find a plugin for a named network during 
> initilization, log a warning message to let operator know this issue, and 
> afterward operator can put the plugin in the plugin directory without 
> restarting agent, then the named network can still work.
> 
> Avinash sridharan wrote:
> Lets not rely on the operator heeding WARNING messages and fixing the 
> problem. My concern is that this is a `FATAL` error since before the operator 
> can rectify the error if containers are launched the behavior becomes 
> undefined.
> 
> Qian Zhang wrote:
> Agree, let's return an error :-)

After more thinking, I think in this case, it makes more sense to log a warning 
message and ignore the network config file rather than bail at this point, 
because there can be other valid network config files. If in the end there is 
no any valid network config files, we should definitely bail.


> On March 4, 2016, 11:30 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 108
> > 
> >
> > Conver this to LOG(ERROR)

I think it should be LOG(WARNING) if we are going to ignore something in the 
`create()` method of an isolator, please see 
`PortMappingIsolatorProcess::create()`:
https://github.com/apache/mesos/blob/0.27.1/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp#L1321


> On March 4, 2016, 11:30 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 129
> > 
> >
> > Can we also check if the plugin is executable? Since later we are going 
> > to try and execute the plugin.
> 
> Avinash sridharan wrote:
> This might be slightly more involved then I thought. You will have to 
> figure out if the owner or group of the binary is root (since the assumption 
> is agent is running with sudo). If yes then check the permission of the user 
> and the group. If no then check the permission of others. os::permissions 
> should allow you to check all the permissions.

Yes, we should check it. And since the assumption is agent is running with root 
permission, we only need to check if any of user/group/other has "x" 
permission, if yes, root will be able to execute the plugin.


- Qian


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


On March 5, 2016, 1:07 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44269/
> ---
> 
> (Updated March 5, 2016, 1:07 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
> ---
> 
> Added the framework of 'network/cni' isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0eabfad66f43adff12d08dbf34ca3db1b801c49e 
>   src/Makefile.am 5b54fe04734afc5faa1bba3b84f43a1b8efe8753 
>   src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44269/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 44433: Added empty line for list in maintenance doc.

[Klaus Ma]

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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

Added empty line for list in maintenance doc.


Diffs
-

  docs/maintenance.md 365c920719dbd0c5e61efe1975547a2848647bce 

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


Testing
---

Document update.


Thanks,

Klaus Ma

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

[Qian Zhang]

> On March 5, 2016, 2:55 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 61
> > 
> >
> > Add a `+` at the end. While not necessary, makes it more readable.
> 
> Qian Zhang wrote:
> Agree.

There will be a compile error if adding a `+` at the end :-(


- Qian


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


On March 5, 2016, 1:07 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44269/
> ---
> 
> (Updated March 5, 2016, 1:07 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
> ---
> 
> Added the framework of 'network/cni' isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0eabfad66f43adff12d08dbf34ca3db1b801c49e 
>   src/Makefile.am 5b54fe04734afc5faa1bba3b84f43a1b8efe8753 
>   src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44269/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [37168]

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 6, 2016, 11:25 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 6, 2016, 11:25 a.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

[Klaus Ma]

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

(Updated March 6, 2016, 7:25 p.m.)


Review request for mesos, Adam B, Greg Mann, Joerg Schad, and Michael Park.


Changes
---

Address joerg84's comments.


Summary (updated)
-

Add an example framework using dynamic reservation.


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


Repository: mesos


Description
---

Provide example for dynamic reservation features.


Diffs (updated)
-

  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 43884: Added allocator metrics for used quotas.

[Klaus Ma]

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




src/master/allocator/mesos/metrics.hpp (line 98)


Add comments on keys, e.g. role : .


- Klaus Ma


On March 5, 2016, 12:01 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43884/
> ---
> 
> (Updated March 5, 2016, 12:01 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4723
> https://issues.apache.org/jira/browse/MESOS-4723
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metrics for used quotas.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 
> 70291075c00a9a557529c2562dedcfc6c6c3ec32 
>   src/master/allocator/mesos/metrics.hpp PRE-CREATION 
>   src/master/allocator/mesos/metrics.cpp PRE-CREATION 
>   src/tests/hierarchical_allocator_tests.cpp 
> 3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 
> 
> Diff: https://reviews.apache.org/r/43884/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 44431: Do not check overlayfs when create overlay backend.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44421, 44431]

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 6, 2016, 10:06 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44431/
> ---
> 
> (Updated March 6, 2016, 10:06 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Shuai Lin.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The overlayfs support was already checked when creating the
> backends creators.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
> 5cc0f8b5a8cd4c945023f874056a8184113186c5 
> 
> Diff: https://reviews.apache.org/r/44431/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 44431: Do not check overlayfs when create overlay backend.

[Guangya Liu]

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

Review request for mesos, Jie Yu and Shuai Lin.


Repository: mesos


Description
---

The overlayfs support was already checked when creating the
backends creators.


Diffs
-

  src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
5cc0f8b5a8cd4c945023f874056a8184113186c5 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

[Qian Zhang]

> On March 5, 2016, 2:55 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 45
> > 
> >
> > Remove the period at the end. We don't have terminal sentences in ERROR 
> > strings.

Nice catch!


> On March 5, 2016, 2:55 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 50
> > 
> >
> > Remove period.

Nice catch!


> On March 5, 2016, 2:55 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 61
> > 
> >
> > Add a `+` at the end. While not necessary, makes it more readable.

Agree.


- Qian


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


On March 5, 2016, 1:07 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44269/
> ---
> 
> (Updated March 5, 2016, 1:07 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
> ---
> 
> Added the framework of 'network/cni' isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0eabfad66f43adff12d08dbf34ca3db1b801c49e 
>   src/Makefile.am 5b54fe04734afc5faa1bba3b84f43a1b8efe8753 
>   src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44269/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>