Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

[Travis Hegner]

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

(Updated Feb. 2, 2016, 6:52 p.m.)


Review request for mesos and Benjamin Hindman.


Repository: mesos


Description
---

Fixes [MESOS-4370]


Diffs
-

  src/docker/docker.cpp a831726 

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


Testing (updated)
---

This patch will first query the docker API for the HostConfig.NetworkMode, 
which is populated with the network name. (Essentially what was passed in --net 
 to the docker run command). This name is then used as a key in 
NetworkSettings.Networks..IPAddress to get the IP address that is 
currently in use by the container.

It appears that even though the docker API has been set up to allow for 
multiple networks, our testing has indicated that it's still only applying one 
network to the container (the last one via the --net argument on the run line). 
I can only speculate that the docker API will change again in the near future, 
but I can't speculate how, so at least this fixes the problem as it stands 
right now.

Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04.


Thanks,

Travis Hegner

Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [43093]

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

Error:
2016-02-02 20:06:11 URL:https://reviews.apache.org/r/43093/diff/raw/ 
[1765/1765] -> "43093.patch" [1]
Total errors found: 0
Checking 1 files
Error: Commit message summary (the first line) must end in a period.

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

- Mesos ReviewBot


On Feb. 2, 2016, 6:52 p.m., Travis Hegner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43093/
> ---
> 
> (Updated Feb. 2, 2016, 6:52 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes [MESOS-4370]
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp a831726 
> 
> Diff: https://reviews.apache.org/r/43093/diff/
> 
> 
> Testing
> ---
> 
> This patch will first query the docker API for the HostConfig.NetworkMode, 
> which is populated with the network name. (Essentially what was passed in 
> --net  to the docker run command). This name is then used as a key in 
> NetworkSettings.Networks..IPAddress to get the IP address that is 
> currently in use by the container.
> 
> It appears that even though the docker API has been set up to allow for 
> multiple networks, our testing has indicated that it's still only applying 
> one network to the container (the last one via the --net argument on the run 
> line). I can only speculate that the docker API will change again in the near 
> future, but I can't speculate how, so at least this fixes the problem as it 
> stands right now.
> 
> Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Travis Hegner
> 
>

Re: Review Request 42587: Implemented the `NetClsHandleMgr` class.

[Avinash sridharan]

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

(Updated Feb. 2, 2016, 6:16 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Implemented the `NetClsHandleMgr` class.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b4bc52114389d1c1efce2830f4292bd89bb0de7c 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 

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


Testing
---

make


Thanks,

Avinash sridharan

Re: Review Request 42618: Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.

[Avinash sridharan]

> On Feb. 1, 2016, 8:14 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp, lines 440-443
> > 
> >
> > Can you try to see if writing a decimal number if fine as well? If not, 
> > we should move the helper function into cpp as a file local helper.

Writing a decimal number works. Have moved this into a helper function in 
cgroups, as a separate patch.


> On Feb. 1, 2016, 8:14 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp, lines 491-497
> > 
> >
> > I don't think this is necessary.

Agreed.


- Avinash


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


On Feb. 2, 2016, 6:32 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42618/
> ---
> 
> (Updated Feb. 2, 2016, 6:32 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b4bc52114389d1c1efce2830f4292bd89bb0de7c 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
> 
> Diff: https://reviews.apache.org/r/42618/diff/
> 
> 
> Testing
> ---
> 
> make and make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

[Travis Hegner]

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

Review request for mesos and Benjamin Hindman.


Repository: mesos


Description
---

Fixes [MESOS-4370]


Diffs
-

  src/docker/docker.cpp a831726 

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


Testing
---

This patch will first query the docker API for the HostConfig.NetworkMode, 
which is populated with the network name. (Essentially what was passed in --net 
 to the docker run command). This name is then used as a key in 
NetworkSettings.Networks..IPAddress to get the IP address that is 
currently in use by the container.

It appears that even though the docker API has been set up to allow for 
multiple networks, our testing has indicated that it's still only applying one 
network to the container (the last one via the --net argument on the run line). 
I can only speculate that the docker API will change again in the near future, 
but I can't speculate how, so at least this fixes the problem as it stands 
right now.

Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04


Thanks,

Travis Hegner

Re: Review Request 42780: Changed the NetClsIsolatorTest to check for net_cls handles.

[Avinash sridharan]

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

(Updated Feb. 2, 2016, 7:07 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Changed the NetClsIsolatorTest to check for net_cls handles.


Diffs (updated)
-

  src/tests/containerizer/isolator_tests.cpp 
8d101df957fd36adac388310eddba2db1f98c029 

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


Testing
---

make and make check


Thanks,

Avinash sridharan

Re: Review Request 43037: Support env var in docker runtime isolator.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 96)


s/launchEnvironment/environment/



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 108)


Ditto below.



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 110)


Should that be container_config?



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (lines 116 - 117)


I would prefer:
```
foreach(const string& env,
containerConfig.docker().manifest().config().env()) {
}
```



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 122)


You want to print the containerId here as well. So i'll need to pass in the 
'containerId' to this helper method.



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 126)


Add an extra line above.



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 137)


indentation.


- Jie Yu


On Feb. 2, 2016, 5:18 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43037/
> ---
> 
> (Updated Feb. 2, 2016, 5:18 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4383
> https://issues.apache.org/jira/browse/MESOS-4383
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support env var in docker runtime isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4b504dbb58823ce7675f1d2048dcc7a27c05663d 
>   src/slave/containerizer/mesos/isolators/docker/runtime.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43037/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 42618: Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.

[Avinash sridharan]

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

(Updated Feb. 2, 2016, 6:32 p.m.)


Review request for mesos and Jie Yu.


Summary (updated)
-

Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.


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


Repository: mesos


Description (updated)
---

Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b4bc52114389d1c1efce2830f4292bd89bb0de7c 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 

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


Testing
---

make and make check


Thanks,

Avinash sridharan

Re: Review Request 43022: Added protobuf fields for docker runtime isolator.

[Jie Yu]

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




include/mesos/slave/isolator.proto (line 85)


Why 'required' here. I would rather keep it 'optional' in case in the 
future, we have v2/v3/v4 manifest.



include/mesos/slave/isolator.proto (line 105)


This is not docker specific right?



include/mesos/slave/isolator.proto (lines 108 - 111)


```
If specified, it'll become the launch command for the custom executor, or 
the launch command for the user task in the case of a command task.
```



include/mesos/slave/isolator.proto (line 114)


Why Docker specific here? It can be used by, for example, appc as well, 
right? Just mention that it's the working directory for the container.



include/mesos/slave/isolator.proto (lines 118 - 122)


Let's not change the numbering here.


- Jie Yu


On Feb. 2, 2016, 5:18 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43022/
> ---
> 
> (Updated Feb. 2, 2016, 5:18 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4517
> https://issues.apache.org/jira/browse/MESOS-4517
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added protobuf fields for docker runtime isolator.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.proto 8b7c54afc4467084b5d22387e55689d06078b504 
> 
> Diff: https://reviews.apache.org/r/43022/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 42587: Implemented the `NetClsHandleMgr` class.

[Avinash sridharan]

> On Feb. 1, 2016, 10:46 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp, lines 137-139
> > 
> >
> > Hum, having two data fields here looks a little confusing to me. How 
> > about just merging them:
> > 
> > ```
> > // A map for used handles: primary -> bitset for secondary.
> > hashmap> used;
> > ```

Used a typedef for bitset<0x+1> to improve readability.


> On Feb. 1, 2016, 10:46 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp, lines 73-78
> > 
> >
> > I got confused here. minor is a uint16 value. Why this check is 
> > necessary?

Removed this validation. Always set the first element of the bitmask when 
initializing it, so no longer need the check on the secondary handles.


> On Feb. 1, 2016, 10:46 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp, lines 82-96
> > 
> >
> > For testin, I would probably just expose a 'isUsed' method:
> > ```
> > bool isUsed(const NetClsHandle& handle);
> > ```

Removed usage(). Will introduce isUsed with the unit-test patch.


- Avinash


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


On Feb. 2, 2016, 6:16 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42587/
> ---
> 
> (Updated Feb. 2, 2016, 6:16 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the `NetClsHandleMgr` class.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b4bc52114389d1c1efce2830f4292bd89bb0de7c 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
> 
> Diff: https://reviews.apache.org/r/42587/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43036: Plugged in docker runtime isolator.

[Jie Yu]

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




src/slave/containerizer/mesos/containerizer.cpp (lines 840 - 841)


Does this compile? Should that be:
```
ContainerConfig::Docker* docker = containerConfig.mutable_docker();
docker->mutable_manifest()->CopyFrom(
provisionInfo->dockerManifest.get());
```



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (lines 78 - 79)


No need for this.



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (lines 81 - 83)


I would move this up as the first check so that you don't have to do 
`executorInfo.has_container()` in later checks


- Jie Yu


On Feb. 2, 2016, 5:18 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43036/
> ---
> 
> (Updated Feb. 2, 2016, 5:18 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4517
> https://issues.apache.org/jira/browse/MESOS-4517
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Plugged in docker runtime isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4b504dbb58823ce7675f1d2048dcc7a27c05663d 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43036/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 43020: Exposed accurate ProvisionInfo to command executor.

[Jie Yu]

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




src/slave/containerizer/mesos/containerizer.cpp (line 724)


Add some more comments here:
```
For command tasks (i.e., taskInfo.isSome()), the filesystem image for the 
task is specified in a volume (COMMAND_EXECUTOR_ROOTFS_CONTAINER_PATH). We need 
to pass the provisionInfo from that image to isolators through 'prepare'.
```



src/slave/containerizer/mesos/containerizer.cpp (line 726)


I would do the following:

```
Owned _provisionInfo(new 
ProvisionInfo(provisionInfo));
```



src/slave/containerizer/mesos/containerizer.cpp (lines 738 - 739)


Merge these in one line.



src/slave/containerizer/mesos/containerizer.cpp (lines 738 - 751)


```
futures.push_back(
  provisioner->provision(containerId, image)
.then([=](const ProvisionInfo& info) -> Future {
  volume->set_host_path(info.rootfs);
  
  // Add some comments here about why we need to do this!
  if (taskInfo.isSome() &&
  volume->container_path() ==
COMMAND_EXECUTOR_ROOTFS_CONTAINER_PATH) {
_provisionInfo.reset(new ProvisionInfo(info));
  }
}));
```



src/slave/containerizer/mesos/containerizer.cpp (line 740)


capturing reference here is not correct. It's already a pointer, no need to 
capturing the reference of a pointer.



src/slave/containerizer/mesos/containerizer.cpp (lines 758 - 761)


No need for this.



src/slave/containerizer/mesos/containerizer.cpp (line 772)


`*_provisionInfo` here.


- Jie Yu


On Feb. 2, 2016, 5:18 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43020/
> ---
> 
> (Updated Feb. 2, 2016, 5:18 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed accurate ProvisionInfo to command executor.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4b504dbb58823ce7675f1d2048dcc7a27c05663d 
> 
> Diff: https://reviews.apache.org/r/43020/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 42588: Added unit-test for `NetClsHandleMgr`.

[Avinash sridharan]

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

(Updated Feb. 2, 2016, 6:25 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added unit-test for `NetClsHandleMgr`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b4bc52114389d1c1efce2830f4292bd89bb0de7c 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
  src/tests/containerizer/isolator_tests.cpp 
8d101df957fd36adac388310eddba2db1f98c029 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 43021: Implemented docker runtime isolator interface.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 2, 2016, 5:18 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43021/
> ---
> 
> (Updated Feb. 2, 2016, 5:18 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4517
> https://issues.apache.org/jira/browse/MESOS-4517
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented docker runtime isolator interface.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 6b2c7cbffa57d44b913419ad68fc66eaeca169d0 
>   src/Makefile.am fac17f4bac3b2ddda0384dec8b0ed6f960bd24d1 
>   src/slave/containerizer/mesos/isolators/docker/runtime.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43021/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 43021: Implemented docker runtime isolator interface.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/docker/runtime.hpp (line 26)


s/Isolator/isolator/



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 23)


We try to avoid using this now. Let's use 'using process::XXX' explicitly 
here.


- Jie Yu


On Feb. 2, 2016, 5:18 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43021/
> ---
> 
> (Updated Feb. 2, 2016, 5:18 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4517
> https://issues.apache.org/jira/browse/MESOS-4517
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented docker runtime isolator interface.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 6b2c7cbffa57d44b913419ad68fc66eaeca169d0 
>   src/Makefile.am fac17f4bac3b2ddda0384dec8b0ed6f960bd24d1 
>   src/slave/containerizer/mesos/isolators/docker/runtime.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43021/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 42181: Added a Slave constructor to pass process ID manually.

[Vinod Kone]

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




src/tests/cluster.cpp (lines 409 - 430)


instead of doing if else blocks can you use a ternary operator for the 1st 
argument in the constructor?

```
slave.slave = new Slave::Slave(
  id.isSome() ? id.get() : process::ID::generate("slave"),
  ...
  ...);
```

infact instead of declaring an overloaded constructor in slave.hpp and 
chaining constructors, can you make 'id' an argument with default value?

```
Slave(...,
  const std::string& id = process::ID::generate("slave"))
```

Thoughts?


- Vinod Kone


On Jan. 22, 2016, 8:27 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42181/
> ---
> 
> (Updated Jan. 22, 2016, 8:27 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4255
> https://issues.apache.org/jira/browse/MESOS-4255
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a constructor to Slave that takes in the process ID string 
> as an argument. This is necessary for testing recovery of HTTP based 
> executors. (Previously, every invocation of `StartSlave` used to generate a 
> new ID via `ID::generate`). There was no process delegate set via 
> `process::initialize` that led to problems for the existing HTTP based 
> executor to connect back to the slave.
> 
> Also, modified the tests to introduce a new `StartSlave` function that takes 
> this process ID as argument.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
>   src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 
>   src/tests/cluster.hpp 576dcb8b7e27d1905425aa0f7cb319c19c17c15c 
>   src/tests/cluster.cpp 1a3a038e78999bca2fc3f08725f4ffaf4290bcb9 
>   src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
>   src/tests/mesos.cpp 365ebe8335c37bfdb983a5424d4c995fa9b76a22 
> 
> Diff: https://reviews.apache.org/r/42181/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 43075: Fixed definition of operator<< for `Call::Type` and `Event::Type`.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Feb. 2, 2016, 9:33 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43075/
> ---
> 
> (Updated Feb. 2, 2016, 9:33 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The types were defined in the `mesos::scheduler` namespace, but the
> overload of `operator<<` was only defined in the `mesos` namespace.
> Because of how ADL works, this usually resulted in the overload
> not being found.
> 
> Old output: "Dropping 2 call from framework ..."
> 
> New output: "Dropping TEARDOWN call from framework ..."
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler/scheduler.hpp 
> cadd7df11bb6d4210c5abdab5c04586e6686af1d 
>   src/sched/sched.cpp 8e51752536041b896e42d44e6e9eae909b0a1d68 
> 
> Diff: https://reviews.apache.org/r/43075/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43075: Fixed error message when dropping a scheduler::Call message.

[Vinod Kone]

> On Feb. 2, 2016, 7:54 p.m., Vinod Kone wrote:
> > src/master/master.cpp, line 1801
> > 
> >
> > The output stream operator overload for Call (unversioned) defined in 
> > "mesos/scheduler/scheduler.hpp". Not sure why the compiler unable to find 
> > that overload (defined in `mesos` namespace).
> > 
> > It seems to work fine for log statements (that print v1::Call) in 
> > scheduler.cpp. The overload for v1::Call was defined in 
> > "mesos/include/mesos/v1/scheduler/scheduler.cpp" though, which is in the 
> > same namespace as the log statements in scheduler.cpp.
> 
> Neil Conway wrote:
> I _believe_ what is going on here is that the overload of `operator<<` 
> for `mesos::scheduler::Call::Type` is defined in the `mesos` namespace, not 
> the `mesos::scheduler` namespace. Since ADL only considers the "closest 
> enclosing namespace" of the argument (see 
> http://en.cppreference.com/w/cpp/language/adl , case 2(d)), it looks for an 
> overload in `mesos::scheduler` but not `mesos`.
> 
> Defining the overload of `operator<<` in `mesos::scheduler` seems to fix 
> the problem. I think it is generally good practice to group types and 
> functions on those types together into the same namespace, anyway. I'll 
> submit a revised RR doing that shortly.

Yup, that's what I figured out after reading SO. Just doing a `make check` to 
confirm.


- Vinod


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


On Feb. 2, 2016, 5:12 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43075/
> ---
> 
> (Updated Feb. 2, 2016, 5:12 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Old output: "Dropping 2 call from framework ..."
> 
> New output: "Dropping TEARDOWN call from framework ..."
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 
> 
> Diff: https://reviews.apache.org/r/43075/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42843: Ensure `ServiceUnavailable` is not received for `Subscribe` calls.

[Anand Mazumdar]

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

(Updated Feb. 2, 2016, 9:56 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Modified description.


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


Repository: mesos


Description (updated)
---

The agent does not return an `ServiceUnavailable` response for `Subscribe` 
calls. Added a `CHECK` to ensure this in the executor library.


Diffs
-

  src/executor/executor.cpp 92334ffb8af83b1c86c44bd406147893c7c6daa3 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 42844: Drop `404 NotFound` responses in the executor library.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Jan. 27, 2016, 4:14 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42844/
> ---
> 
> (Updated Jan. 27, 2016, 4:14 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4255
> https://issues.apache.org/jira/browse/MESOS-4255
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, we did not use to drop `404 NotFound` responses in the library 
> and send `Event::Error` to executor. However, this can be trigerred upon an 
> agent restart when it has not yet set up HTTP routes.
> 
> 
> Diffs
> -
> 
>   src/executor/executor.cpp 92334ffb8af83b1c86c44bd406147893c7c6daa3 
> 
> Diff: https://reviews.apache.org/r/42844/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 43075: Fixed definition of operator<< for `Call::Type` and `Event::Type`.

[Neil Conway]

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

(Updated Feb. 2, 2016, 9:33 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

New approach, per discussion with Vinod.


Summary (updated)
-

Fixed definition of operator<< for `Call::Type` and `Event::Type`.


Repository: mesos


Description (updated)
---

The types were defined in the `mesos::scheduler` namespace, but the
overload of `operator<<` was only defined in the `mesos` namespace.
Because of how ADL works, this usually resulted in the overload
not being found.

Old output: "Dropping 2 call from framework ..."

New output: "Dropping TEARDOWN call from framework ..."


Diffs (updated)
-

  include/mesos/scheduler/scheduler.hpp 
cadd7df11bb6d4210c5abdab5c04586e6686af1d 
  src/sched/sched.cpp 8e51752536041b896e42d44e6e9eae909b0a1d68 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 43075: Fixed definition of operator<< for `Call::Type` and `Event::Type`.

[Neil Conway]

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

(Updated Feb. 2, 2016, 9:36 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Minor fixes: add explicit header include, make style consistent with v1 version


Repository: mesos


Description
---

The types were defined in the `mesos::scheduler` namespace, but the
overload of `operator<<` was only defined in the `mesos` namespace.
Because of how ADL works, this usually resulted in the overload
not being found.

Old output: "Dropping 2 call from framework ..."

New output: "Dropping TEARDOWN call from framework ..."


Diffs (updated)
-

  include/mesos/scheduler/scheduler.hpp 
cadd7df11bb6d4210c5abdab5c04586e6686af1d 
  src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 
  src/sched/sched.cpp 8e51752536041b896e42d44e6e9eae909b0a1d68 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

[Travis Hegner]

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

(Updated Feb. 2, 2016, 5:06 p.m.)


Review request for mesos and Kapil Arya.


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


Repository: mesos


Description
---

Fixes [MESOS-4370]


Diffs
-

  src/docker/docker.cpp a831726 

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


Testing
---

This patch will first query the docker API for the HostConfig.NetworkMode, 
which is populated with the network name. (Essentially what was passed in --net 
 to the docker run command). This name is then used as a key in 
NetworkSettings.Networks..IPAddress to get the IP address that is 
currently in use by the container.

It appears that even though the docker API has been set up to allow for 
multiple networks, our testing has indicated that it's still only applying one 
network to the container (the last one via the --net argument on the run line). 
I can only speculate that the docker API will change again in the near future, 
but I can't speculate how, so at least this fixes the problem as it stands 
right now.

Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04.


Thanks,

Travis Hegner

Re: Review Request 43075: Fixed error message when dropping a scheduler::Call message.

[Neil Conway]

> On Feb. 2, 2016, 7:54 p.m., Vinod Kone wrote:
> > src/master/master.cpp, line 1801
> > 
> >
> > The output stream operator overload for Call (unversioned) defined in 
> > "mesos/scheduler/scheduler.hpp". Not sure why the compiler unable to find 
> > that overload (defined in `mesos` namespace).
> > 
> > It seems to work fine for log statements (that print v1::Call) in 
> > scheduler.cpp. The overload for v1::Call was defined in 
> > "mesos/include/mesos/v1/scheduler/scheduler.cpp" though, which is in the 
> > same namespace as the log statements in scheduler.cpp.

I _believe_ what is going on here is that the overload of `operator<<` for 
`mesos::scheduler::Call::Type` is defined in the `mesos` namespace, not the 
`mesos::scheduler` namespace. Since ADL only considers the "closest enclosing 
namespace" of the argument (see http://en.cppreference.com/w/cpp/language/adl , 
case 2(d)), it looks for an overload in `mesos::scheduler` but not `mesos`.

Defining the overload of `operator<<` in `mesos::scheduler` seems to fix the 
problem. I think it is generally good practice to group types and functions on 
those types together into the same namespace, anyway. I'll submit a revised RR 
doing that shortly.


- Neil


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


On Feb. 2, 2016, 5:12 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43075/
> ---
> 
> (Updated Feb. 2, 2016, 5:12 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Old output: "Dropping 2 call from framework ..."
> 
> New output: "Dropping TEARDOWN call from framework ..."
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 
> 
> Diff: https://reviews.apache.org/r/43075/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 43118: Corrected mistakes in docs for volume/reservation HTTP endpoints.

[Neil Conway]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Corrected mistakes in docs for volume/reservation HTTP endpoints.


Diffs
-

  docs/persistent-volume.md 9657c3f75dcc17dfd4d6391b35da3dbc0a6c547d 
  docs/reservation.md 25337109ff19240f926667961a59323bbfeb9956 

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


Testing
---

Previewed in site-docker.


Thanks,

Neil Conway

Review Request 43105: Ensured the allocator does not double account resources.

[Alexander Rukletsov]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

If `addFramework()` is called before `addSlave()` double accounting
of resources is possible. We have not observed bugs because Mesos
currently always adds a framework in the allocator after all agents
with the framework's tasks have been added.


Diffs
-

  include/mesos/master/allocator.hpp e163669c9c4e4c98572968f18987704b60722a79 
  src/master/allocator/mesos/hierarchical.cpp 
1a07d69016407e5aad2209586da37fecbcddb765 

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


Testing
---

make check


Thanks,

Alexander Rukletsov

Review Request 43107: Passed agent flag --cgroup_net_cls_primary_handle to net_cls isolator.

[Avinash sridharan]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

The `cgroup/net_cls` isolator will use the primary handle, passed by the 
operator
using the --cgroup_net_cls_primary_handle, to initialize the `NetClsHandleMgr`.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b4bc52114389d1c1efce2830f4292bd89bb0de7c 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 

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


Testing
---

make and make check

Launched a slave by passing invalid flags to --cgroup_net_cls_primary_handle 
and made sure the slave correctly detects these invalid handles. 

Launched a slave with valid handle to make sure that the slave launches 
correctly.


Thanks,

Avinash sridharan

Re: Review Request 42843: Ensure `ServiceUnavailable` is not received for `Subscribe` calls.

[Anand Mazumdar]

> On Feb. 2, 2016, 10:10 p.m., Vinod Kone wrote:
> > src/executor/executor.cpp, line 502
> > 
> >
> > Looks like HttpProxy can send ServiceUnavailable too for some cases 
> > https://github.com/apache/mesos/blob/b8122f110f826e97f263b982f8fdd14ef53c24f6/3rdparty/libprocess/src/process.cpp#L1137
> > 
> > maybe CHECK isn't prudent here?

Aha, good catch. Let me discard this change.


- Anand


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


On Feb. 2, 2016, 9:56 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42843/
> ---
> 
> (Updated Feb. 2, 2016, 9:56 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4255
> https://issues.apache.org/jira/browse/MESOS-4255
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The agent does not return an `ServiceUnavailable` response for `Subscribe` 
> calls. Added a `CHECK` to ensure this in the executor library.
> 
> 
> Diffs
> -
> 
>   src/executor/executor.cpp 92334ffb8af83b1c86c44bd406147893c7c6daa3 
> 
> Diff: https://reviews.apache.org/r/42843/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 42185: Added an example executor based on the new V1 API.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Jan. 27, 2016, 4:13 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42185/
> ---
> 
> (Updated Jan. 27, 2016, 4:13 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4255
> https://issues.apache.org/jira/browse/MESOS-4255
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a custom executor based on the new executor library.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am bdb34020043f64c07721b3e8b29b221e5b99 
>   src/examples/test_http_executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42185/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 42781: Added the --cgroups_net_cls_primary_handle flag to the slave.

[Avinash sridharan]

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

(Updated Feb. 2, 2016, 11:11 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added the --cgroups_net_cls_primary_handle flag to the slave.


Diffs (updated)
-

  src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
  src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
  src/slave/flags.hpp 23ec158f4a95fa76d657d9eb6a4a6fe30d57e5b6 
  src/slave/flags.cpp 75d7429a4e3e1d6259296257c0ace1ade365ac2b 

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


Testing
---

Tested the flag by launching a mesos agent and checking that the slave takes 
the `--cgroups_net_cls_major_handles` flag.


Thanks,

Avinash sridharan

Review Request 42982: Defined a virtual `status` method for Containerizer.

[Avinash sridharan]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Defined a virtual `status` method for Containerizer.


Diffs
-

  src/slave/containerizer/containerizer.hpp 
08d1ed3cf50f50b5b6e71df60bffbd141490ad32 

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


Testing
---

make


Thanks,

Avinash sridharan

Review Request 42983: Implemented the `status` method in `MesosContainerizer`.

[Avinash sridharan]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Implemented the `status` method in `MesosContainerizer`.


Diffs
-

  src/slave/containerizer/mesos/containerizer.hpp 
811ab7937279c4a55da450c136f9fcb1303ea0d5 
  src/slave/containerizer/mesos/containerizer.cpp 
4b504dbb58823ce7675f1d2048dcc7a27c05663d 

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


Testing
---

make


Thanks,

Avinash sridharan

Re: Review Request 42781: Added the --cgroups_net_cls_primary_handle flag to the slave.

[Avinash sridharan]

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

(Updated Feb. 2, 2016, 10:39 p.m.)


Review request for mesos and Jie Yu.


Summary (updated)
-

Added the --cgroups_net_cls_primary_handle flag to the slave.


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


Repository: mesos


Description (updated)
---

Added the --cgroups_net_cls_primary_handle flag to the slave.


Diffs (updated)
-

  src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
  src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
  src/slave/flags.hpp 23ec158f4a95fa76d657d9eb6a4a6fe30d57e5b6 
  src/slave/flags.cpp 75d7429a4e3e1d6259296257c0ace1ade365ac2b 

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


Testing
---

Tested the flag by launching a mesos agent and checking that the slave takes 
the `--cgroups_net_cls_major_handles` flag.


Thanks,

Avinash sridharan

Re: Review Request 42185: Added an example executor based on the new V1 API.

[Vinod Kone]

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




src/examples/test_http_executor.cpp (line 154)


I think it makes sense to send a TASK_RUNNING before sending TASK_FINISHED 
as the test_executor does. That's a more realistic behavior of a custome 
executor.


- Vinod Kone


On Jan. 27, 2016, 4:13 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42185/
> ---
> 
> (Updated Jan. 27, 2016, 4:13 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4255
> https://issues.apache.org/jira/browse/MESOS-4255
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a custom executor based on the new executor library.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am bdb34020043f64c07721b3e8b29b221e5b99 
>   src/examples/test_http_executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42185/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 42186: Added tests for recovery for HTTP based executors.

[Vinod Kone]

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




src/tests/slave_recovery_tests.cpp (line 420)


s/executor/HTTP based executor/



src/tests/slave_recovery_tests.cpp (line 434)


Can you add a comment here on why you do this for posterity?



src/tests/slave_recovery_tests.cpp (line 460)


Add a TODO here to use a command http executor instead of the custom test 
http executor.



src/tests/slave_recovery_tests.cpp (line 462)


s/default/http/



src/tests/slave_recovery_tests.cpp (line 506)


Was surprised to see a TASK_FINISHED here instead of TASK_RUNNING. I 
commented on the dependent review to fix that. That should change this test 
slightly.



src/tests/slave_recovery_tests.cpp (line 1046)


I think you should start the slave with fixed id like in the above test, to 
ensure the executor tries to connect to the right endpoint.



src/tests/slave_recovery_tests.cpp (line 1072)


ditto. see above.


- Vinod Kone


On Jan. 27, 2016, 4:14 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42186/
> ---
> 
> (Updated Jan. 27, 2016, 4:14 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4255
> https://issues.apache.org/jira/browse/MESOS-4255
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp 6683a081ac231f1e275a3cdb8ee841da430a9f66 
> 
> Diff: https://reviews.apache.org/r/42186/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 42782: Added a unit-test to test net_cls major handles set from command line.

[Avinash sridharan]

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

(Updated Feb. 2, 2016, 11:14 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added a unit-test to test net_cls major handles set from command line.


Diffs (updated)
-

  src/tests/containerizer/isolator_tests.cpp 
8d101df957fd36adac388310eddba2db1f98c029 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 42947: Implemented the `status` method in `CgroupNetClsIsolatorProcess`.

[Avinash sridharan]

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

(Updated Feb. 2, 2016, 11:19 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Implemented the `status` method in `CgroupNetClsIsolatorProcess`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b4bc52114389d1c1efce2830f4292bd89bb0de7c 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 43082: Added new flag to command executor for command passing.

[Jie Yu]

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




src/launcher/executor.cpp (line 143)


Why 'executorLaunchCommand'? This is the task command, right?

I would simply use 'command' for this naming here.



src/launcher/executor.cpp (lines 143 - 164)


I would restructure the code here for better readability:
```
// Determine the command to launch the task.
CommandInfo command;

if (taskCommand.isSome()) {
  ...
  command = parse.get();
} else if (task.has_command()) {
  command = task.command();
} else {
  CHECK_SOME(override)
<< "Expecting task " << task.task_id()
<< " to have a command!";
}

if (override.isNone()) {
  // TODO(jieyu): For now, ...
  if (command.shell()) {
CHECK(command.has_value()) << "...";
  } else {
CHECK(command.has_value()) << "...";
  }
}
```



src/launcher/executor.cpp (line 179)


YOu forgot to update this!



src/launcher/executor.cpp (line 277)


You may want to rename it to 'commandString'.



src/launcher/executor.cpp (line 746)


Can you add a TODO here to deprecate this flag since no one is using it, 
and it'll cause confusing to your newly added flag.



src/launcher/executor.cpp (lines 767 - 769)


If specified, this is the overrided command for launching the task (instead 
of the command from TaskInfo).


- Jie Yu


On Feb. 2, 2016, 5:18 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43082/
> ---
> 
> (Updated Feb. 2, 2016, 5:18 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4004
> https://issues.apache.org/jira/browse/MESOS-4004
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new flag to command executor for command passing.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 356d311fdf97b2c4663c60e13ede7cdb71a264c7 
> 
> Diff: https://reviews.apache.org/r/43082/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 43121: Updated Doxygen main page to use relative links.

[Kapil Arya]

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

Review request for mesos, Joseph Wu and Neil Conway.


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


Repository: mesos


Description
---

Previously, it was using absolute links which made it harder to test
links locally.


Diffs
-

  src/main.dox d5b29cf96b18ba84ac39f1747c4a174a5bfbc2e1 

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


Testing
---


Thanks,

Kapil Arya

Review Request 43122: Updated Rakefile to automatically generate doxygen pages.

[Kapil Arya]

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

Review request for mesos, Joseph Wu and Neil Conway.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  site/Rakefile 0ce4b7975f95ab6930f0b2674191930df9ab5b20 

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 43120: Removed site/Gemfile.lock.

[Kevin Klues]

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



You can't just remove this file.  It specifies the versions of everything that 
should be installed in a fresh docker image upon calling 'bundle insgtall'.  If 
you saw that it was being overwritten, it means you had a (possibly 
incompatible) newer version of some tools already installed on your system, 
which was causing newer versions of things to be pulled in.  We need this 
committed so that we have proper versioning for known good versions of the 
tools installed on a fresh install.

- Kevin Klues


On Feb. 3, 2016, 12:15 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43120/
> ---
> 
> (Updated Feb. 3, 2016, 12:15 a.m.)
> 
> 
> Review request for mesos, Joseph Wu and Neil Conway.
> 
> 
> Bugs: MESOS-4584
> https://issues.apache.org/jira/browse/MESOS-4584
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This file is automatically generated by `bundle install` and can cause
> conflicts if pre-generated.
> 
> 
> Diffs
> -
> 
>   site/Gemfile.lock 065107ec25f0eff28e2a09051b61152d49f77f9a 
> 
> Diff: https://reviews.apache.org/r/43120/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 42586: Defined the NetClsHandleMgr class.

[Avinash sridharan]

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

(Updated Feb. 3, 2016, 12:43 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This class will be responsible for keeping track of the free and allocated
net_cls handles.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b4bc52114389d1c1efce2830f4292bd89bb0de7c 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 

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


Testing
---

make


Thanks,

Avinash sridharan

Re: Review Request 42587: Implemented the `NetClsHandleMgr` class.

[Avinash sridharan]

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

(Updated Feb. 3, 2016, 12:44 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Implemented the `NetClsHandleMgr` class.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b4bc52114389d1c1efce2830f4292bd89bb0de7c 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 

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


Testing
---

make


Thanks,

Avinash sridharan

Re: Review Request 43120: Removed site/Gemfile.lock.

[Kapil Arya]

> On Feb. 2, 2016, 7:27 p.m., Kevin Klues wrote:
> > You can't just remove this file.  It specifies the versions of everything 
> > that should be installed in a fresh docker image upon calling 'bundle 
> > insgtall'.  If you saw that it was being overwritten, it means you had a 
> > (possibly incompatible) newer version of some tools already installed on 
> > your system, which was causing newer versions of things to be pulled in.  
> > We need this committed so that we have proper versioning for known good 
> > versions of the tools installed on a fresh install.

The problem was that it wasn't getting overwritten :-). I had newer versions of 
tools; more specifically, eventmachine version 1.0.9 was installed. However, 
this file expected version 1.0.3. So either I had to update or remove this file 
by hand before running rake. I wonder if anyone else has run into such problems 
and if there are better alternatives.

For me, I ended up not using the docker image and just used the stuff from 
mesos source tree to generate the publish folder,  but I understand that this 
might fail for others. In that case, we can just drop this RR and continue with 
the other RRs :-).


- Kapil


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


On Feb. 2, 2016, 7:15 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43120/
> ---
> 
> (Updated Feb. 2, 2016, 7:15 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Neil Conway.
> 
> 
> Bugs: MESOS-4584
> https://issues.apache.org/jira/browse/MESOS-4584
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This file is automatically generated by `bundle install` and can cause
> conflicts if pre-generated.
> 
> 
> Diffs
> -
> 
>   site/Gemfile.lock 065107ec25f0eff28e2a09051b61152d49f77f9a 
> 
> Diff: https://reviews.apache.org/r/43120/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 42841: WIP: Introducing appc image fetcher.

[Jojy Varghese]

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

(Updated Feb. 3, 2016, 1:31 a.m.)


Review request for Jie Yu.


Repository: mesos


Description
---

WIP: Introducing appc image fetcher.


Diffs (updated)
-

  src/CMakeLists.txt 6b2c7cbffa57d44b913419ad68fc66eaeca169d0 
  src/Makefile.am fac17f4bac3b2ddda0384dec8b0ed6f960bd24d1 
  src/slave/containerizer/mesos/provisioner/appc/fetcher.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp PRE-CREATION 
  src/slave/flags.hpp 23ec158f4a95fa76d657d9eb6a4a6fe30d57e5b6 
  src/slave/flags.cpp 24a23325cc255d0d7b7af7ed096b6d3012ad75c7 

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


Testing
---

make check; local image server


Thanks,

Jojy Varghese

Review Request 43127: Introduced Appc image cache.

[Jojy Varghese]

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

Review request for mesos and Jie Yu.


Bugs: MESOS-4439 and MESOS-4575
https://issues.apache.org/jira/browse/MESOS-4439
https://issues.apache.org/jira/browse/MESOS-4575


Repository: mesos


Description
---

Image cache will cache metadata about all Appc images stores in the store's
image directory. This cache could be useful to:
  - Quickly query if an image is present in the store.
  - By sharing the cache with other components like fetcher, an image can be
  checked if its present before fetching it.


Diffs
-

  src/slave/containerizer/mesos/provisioner/appc/cache.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/appc/cache.cpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
73c4df858a70da3d4cc4a1cb15092165f6ff8fe4 
  src/tests/containerizer/provisioner_appc_tests.cpp 
e3d08d9e49df93d5290099c8bfd917f60c93e51b 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 43118: Corrected mistakes in docs for volume/reservation HTTP endpoints.

[Neil Conway]

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

(Updated Feb. 3, 2016, 12:39 a.m.)


Review request for mesos, Greg Mann and Jie Yu.


Changes
---

Fix per Greg's review.


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


Repository: mesos


Description
---

Corrected mistakes in docs for volume/reservation HTTP endpoints.


Diffs (updated)
-

  docs/persistent-volume.md 9657c3f75dcc17dfd4d6391b35da3dbc0a6c547d 
  docs/reservation.md 25337109ff19240f926667961a59323bbfeb9956 

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


Testing
---

Previewed in site-docker.


Thanks,

Neil Conway

Re: Review Request 42618: Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.

[Avinash sridharan]

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

(Updated Feb. 3, 2016, 12:48 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b4bc52114389d1c1efce2830f4292bd89bb0de7c 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 

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


Testing
---

make and make check


Thanks,

Avinash sridharan

Re: Review Request 42781: Added the --cgroups_net_cls_primary_handle flag to the slave.

[Avinash sridharan]

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

(Updated Feb. 3, 2016, 12:51 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added the --cgroups_net_cls_primary_handle flag to the slave.


Diffs (updated)
-

  src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
  src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
  src/slave/flags.hpp 23ec158f4a95fa76d657d9eb6a4a6fe30d57e5b6 
  src/slave/flags.cpp 24a23325cc255d0d7b7af7ed096b6d3012ad75c7 

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


Testing
---

Tested the flag by launching a mesos agent and checking that the slave takes 
the `--cgroups_net_cls_major_handles` flag.


Thanks,

Avinash sridharan

Re: Review Request 43105: Ensured the allocator does not double account resources.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42633, 42636, 42657, 42658, 42672, 41950, 42911, 43105]

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 Feb. 2, 2016, 11:13 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43105/
> ---
> 
> (Updated Feb. 2, 2016, 11:13 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If `addFramework()` is called before `addSlave()` double accounting
> of resources is possible. We have not observed bugs because Mesos
> currently always adds a framework in the allocator after all agents
> with the framework's tasks have been added.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp e163669c9c4e4c98572968f18987704b60722a79 
>   src/master/allocator/mesos/hierarchical.cpp 
> 1a07d69016407e5aad2209586da37fecbcddb765 
> 
> Diff: https://reviews.apache.org/r/43105/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 43022: Added protobuf fields for docker runtime isolator.

[Jie Yu]

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




include/mesos/slave/isolator.proto (line 115)


Let's use working_directory here.


- Jie Yu


On Feb. 2, 2016, 5:18 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43022/
> ---
> 
> (Updated Feb. 2, 2016, 5:18 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4517
> https://issues.apache.org/jira/browse/MESOS-4517
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added protobuf fields for docker runtime isolator.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.proto 8b7c54afc4467084b5d22387e55689d06078b504 
> 
> Diff: https://reviews.apache.org/r/43022/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 43083: Supported working dir in docker runtime isolator.

[Jie Yu]

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




src/slave/containerizer/mesos/containerizer.cpp (lines 1029 - 1034)


We shouldn't allow executor to cd into an arbitrary directory if filesystem 
isolation is not used (because that'll create security issue).

I would do the following:
```
if (rootfs.isSome()) {
  launchFlags.directory = workingDir.isSome()
? workingDir.get()
: flags.sandbox_directory;
} else {
  // NOTE: If the executor shares the host filesystem, we
  // should not allow them to 'cd' into an arbitrary directory
  // because that'll create security issues.
  if (workingDir.isSome()) {
LOG(WARNING) << "Ignore working directory '" << workingDir.get()
 << "' specified in container launch info for container "
 << containerId << " since the executor is using the "
 << "host filesystem";
  }
  launchFlags.directory = directory;
}
```



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (lines 112 - 113)


I think we should distinguish between sandbox_direcotry and 
working_directory. For instance, for the marathon docker image, 
sandbox_directory is still /mnt/mesos/sandbox while  working_directory is 
/marathon.

So please add another flag to command executor.



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (lines 114 - 121)


if (!containerConfig.has_task_info()) {
  // Custom executor case.
  Option dir = getWorkingDirectory(containerConfig);
  if (dir.isSome()) {
launchInfo.set_working_directory(dir.get());
  }
}



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (lines 357 - 359)


I would make this function very simple: get the working directory specified 
in the docker image.

I'll move the logics of setting --working_directory to 
getExecutorLaunchCommand.



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (lines 361 - 363)


Ditto on container_config?


- Jie Yu


On Feb. 2, 2016, 5:18 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43083/
> ---
> 
> (Updated Feb. 2, 2016, 5:18 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4005
> https://issues.apache.org/jira/browse/MESOS-4005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported working dir in docker runtime isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4b504dbb58823ce7675f1d2048dcc7a27c05663d 
>   src/slave/containerizer/mesos/isolators/docker/runtime.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43083/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 43019: Fixed non-camel case in protobuf field.

[Jie Yu]

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




include/mesos/slave/isolator.proto (lines 71 - 72)


Can we go through a deprecation cycle to do the renaming. In other words, 
can we keep the existing fields and add two new fields. We set both in 0.28 so 
that we don't break the build for existing isolators.


- Jie Yu


On Feb. 2, 2016, 5:18 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43019/
> ---
> 
> (Updated Feb. 2, 2016, 5:18 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed non-camel case in protobuf field.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.proto 8b7c54afc4467084b5d22387e55689d06078b504 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4b504dbb58823ce7675f1d2048dcc7a27c05663d 
>   src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp 
> 57168c837aabf85c85866ca9d4b5480f281e3143 
>   src/slave/containerizer/mesos/isolators/cgroups/mem.cpp 
> 34aa660204911ce4f22c7735af36c9f1af8b12b9 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 24ff8194bb5ea86a31224d2d2c20d367472168aa 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
> bce5ab9a1913ee7cebaf55b5ee0b42fa4a325e87 
>   src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
> 59ffd13920fb1c4fb8dbf04a2823d9cee75a8f27 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> d5dd93d892e3cde2011e208a050eec2b31891f54 
>   src/tests/containerizer/isolator_tests.cpp 
> 8d101df957fd36adac388310eddba2db1f98c029 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 182fe9217a5da9af603d6f9c203a1689eff4ca1b 
> 
> Diff: https://reviews.apache.org/r/43019/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 42588: Added unit-test for `NetClsHandleMgr`.

[Avinash sridharan]

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

(Updated Feb. 3, 2016, 12:45 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added unit-test for `NetClsHandleMgr`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b4bc52114389d1c1efce2830f4292bd89bb0de7c 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
  src/tests/containerizer/isolator_tests.cpp 
8d101df957fd36adac388310eddba2db1f98c029 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 42780: Changed the NetClsIsolatorTest to check for net_cls handles.

[Avinash sridharan]

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

(Updated Feb. 3, 2016, 12:49 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Changed the NetClsIsolatorTest to check for net_cls handles.


Diffs (updated)
-

  src/tests/containerizer/isolator_tests.cpp 
8d101df957fd36adac388310eddba2db1f98c029 

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


Testing
---

make and make check


Thanks,

Avinash sridharan

Re: Review Request 42586: Defined the NetClsHandleMgr class.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 74)


s/NetClsHandleMgr/NetClsHandleManager/



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 83)


s/allocMajor/allocPrimary/



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 91)


Two things:
1) Can you `s/_netClsHandles/used/`?
2) Can you just use bitset<0x1> here? No need for the constant.


- Jie Yu


On Feb. 3, 2016, 12:43 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42586/
> ---
> 
> (Updated Feb. 3, 2016, 12:43 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This class will be responsible for keeping track of the free and allocated
> net_cls handles.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b4bc52114389d1c1efce2830f4292bd89bb0de7c 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
> 
> Diff: https://reviews.apache.org/r/42586/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Review Request 43096: Added helper function in cgroup for supporting net_cls subsystem.

[Avinash sridharan]

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

Review request for mesos.


Repository: mesos


Description
---

Added helper function in cgroup for supporting net_cls subsystem.


Diffs
-

  src/linux/cgroups.hpp 83b3e401ac69194bef496087dee32e0532b834ab 
  src/linux/cgroups.cpp bbc1fb3c60e53dffb1e8029942725e7102eb9aee 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 43096: Added helper function in cgroup for supporting net_cls subsystem.

[Avinash sridharan]

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

(Updated Feb. 2, 2016, 7:05 p.m.)


Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added helper function in cgroup for supporting net_cls subsystem.


Diffs (updated)
-

  src/linux/cgroups.hpp 83b3e401ac69194bef496087dee32e0532b834ab 
  src/linux/cgroups.cpp bbc1fb3c60e53dffb1e8029942725e7102eb9aee 

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


Testing
---

make and make check.


Thanks,

Avinash sridharan

Re: Review Request 43083: Supported working dir in docker runtime isolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43019, 43020, 43021, 43022, 43036, 43037, 43081, 43082, 43083]

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 Feb. 2, 2016, 5:18 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43083/
> ---
> 
> (Updated Feb. 2, 2016, 5:18 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4005
> https://issues.apache.org/jira/browse/MESOS-4005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported working dir in docker runtime isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4b504dbb58823ce7675f1d2048dcc7a27c05663d 
>   src/slave/containerizer/mesos/isolators/docker/runtime.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43083/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 43118: Corrected mistakes in docs for volume/reservation HTTP endpoints.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43118]

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 Feb. 3, 2016, 12:39 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43118/
> ---
> 
> (Updated Feb. 3, 2016, 12:39 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Jie Yu.
> 
> 
> Bugs: MESOS-4421
> https://issues.apache.org/jira/browse/MESOS-4421
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Corrected mistakes in docs for volume/reservation HTTP endpoints.
> 
> 
> Diffs
> -
> 
>   docs/persistent-volume.md 9657c3f75dcc17dfd4d6391b35da3dbc0a6c547d 
>   docs/reservation.md 25337109ff19240f926667961a59323bbfeb9956 
> 
> Diff: https://reviews.apache.org/r/43118/diff/
> 
> 
> Testing
> ---
> 
> Previewed in site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42780: Changed the NetClsIsolatorTest to check for net_cls handles.

[Avinash sridharan]

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

(Updated Feb. 2, 2016, 7:09 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Changed the NetClsIsolatorTest to check for net_cls handles.


Diffs (updated)
-

  src/tests/containerizer/isolator_tests.cpp 
8d101df957fd36adac388310eddba2db1f98c029 

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


Testing
---

make and make check


Thanks,

Avinash sridharan

Re: Review Request 43075: Fixed error message when dropping a scheduler::Call message.

[Vinod Kone]

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




src/master/master.cpp (line 1801)


The output stream operator overload for Call (unversioned) defined in 
"mesos/scheduler/scheduler.hpp". Not sure why the compiler unable to find that 
overload (defined in `mesos` namespace).

It seems to work fine for log statements (that print v1::Call) in 
scheduler.cpp. The overload for v1::Call was defined in 
"mesos/include/mesos/v1/scheduler/scheduler.cpp" though, which is in the same 
namespace as the log statements in scheduler.cpp.


- Vinod Kone


On Feb. 2, 2016, 5:12 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43075/
> ---
> 
> (Updated Feb. 2, 2016, 5:12 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Old output: "Dropping 2 call from framework ..."
> 
> New output: "Dropping TEARDOWN call from framework ..."
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 
> 
> Diff: https://reviews.apache.org/r/43075/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43081: Supported entrypoint and cmd in docker runtime isolator.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 105)


s/executorLaunchCommand/command/



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (lines 156 - 157)


.. which will be used to launch the executor.



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 161)


Do you want to use contianer_config() here?



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 165)


For command task case,



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (lines 165 - 166)


if no need to change the launch command for the user task



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (lines 172 - 175)


1. If 'shell' is true, we will ignore Entrypoint and Cmd from the docker 
image (row 5-8).



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (lines 176 - 178)


If 'shell' is false and 'value' is set, we will ignore the Entrypoint and 
Cmd from the docker image as well (row 3-4).



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (lines 229 - 272)


OK, you pretty much duplicated the logics here below, right? Can we merge 
them? My suggestion earlier should allow you to merge them?



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 233)


container_config() here?



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 276)


return Error?


- Jie Yu


On Feb. 2, 2016, 5:18 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43081/
> ---
> 
> (Updated Feb. 2, 2016, 5:18 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4004
> https://issues.apache.org/jira/browse/MESOS-4004
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported entrypoint and cmd in docker runtime isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4b504dbb58823ce7675f1d2048dcc7a27c05663d 
>   src/slave/containerizer/mesos/isolators/docker/runtime.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43081/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 43072: Use full screen width for Mesos UI using Bootstraps (v3.3.6) container-fluid

[haosdent huang]

> On Feb. 2, 2016, 2:45 a.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [43072]
> > 
> > Failed command: ./support/apply-review.sh -n -r 43072
> > 
> > Error:
> > 2016-02-02 02:45:28 URL:https://reviews.apache.org/r/43072/diff/raw/ 
> > [226634/226634] -> "43072.patch" [1]
> > Traceback (most recent call last):
> >   File "support/apply-reviews.py", line 342, in 
> > reviewboard()
> >   File "support/apply-reviews.py", line 321, in reviewboard
> > apply_review()
> >   File "support/apply-reviews.py", line 139, in apply_review
> > commit_patch()
> >   File "support/apply-reviews.py", line 180, in commit_patch
> > data = patch_data()
> >   File "support/apply-reviews.py", line 199, in patch_data
> > return reviewboard_data()
> >   File "support/apply-reviews.py", line 252, in reviewboard_data
> > email=user.get('email'))
> > UnicodeEncodeError: 'ascii' codec can't encode character u'\xf8' in 
> > position 11: ordinal not in range(128)
> > 
> > Full log: https://builds.apache.org/job/mesos-reviewbot/11187/console
> 
> haosdent huang wrote:
> I think we could ignore this error message.
> 
> Bernd Mathiske wrote:
> I'd rather not ignore the error message. How can there be an out-of-range 
> char in HTML and CSS files?
> 
> haosdent huang wrote:
> I afraid this may caused by boostrap have some special characters like 
> `ok:before{content:"\e013"}`.
> 
> Bernd Mathiske wrote:
> This should be an encoding using printable cahrs (<=128) only.
> 
> haosdent huang wrote:
> LoL, seems `Michael Lunøe` cause the problem. Need fix the 
> apply_reviews.py

Need do changes in apply_reviews.py to support no-ascii commit messages.
```
diff --git a/support/apply-reviews.py b/support/apply-reviews.py
index ea5e43a..36bfb1e 100755
--- a/support/apply-reviews.py
+++ b/support/apply-reviews.py
@@ -185,7 +185,7 @@ def commit_patch():
   else:
 amend = '-e'

-  cmd = 'git commit --author \'{author}\' {_amend} -am \'{message}\''\
+  cmd = u'git commit --author \'{author}\' {_amend} -am \'{message}\''\
 .format(author=quote(data['author']),
 _amend=amend,
 message=quote(data['message']))
@@ -248,7 +248,7 @@ def reviewboard_data():
   user = url_to_json(reviewboard_user_url(
 review.get('links').get('submitter').get('title'))).get('user')

-  author = '{author} <{email}>'.format(author=user.get('fullname'),
+  author = u'{author} <{email}>'.format(author=user.get('fullname'),
email=user.get('email'))
   message = '\n\n'.join(['{summary}',
   '{description}',
```


- haosdent


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


On Feb. 1, 2016, 11:36 p.m., Michael Lunøe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43072/
> ---
> 
> (Updated Feb. 1, 2016, 11:36 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Thomas Rampelberg.
> 
> 
> Bugs: MESOS-2585
> https://issues.apache.org/jira/browse/MESOS-2585
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The solution to our problem is solved by updating the Bootstrap CSS file, as 
> container-fluid was introduced later in Bootstrap.
> 
> I have updated to lastest stable release of Bootstrap v3.3.6. Looking through 
> the change log, the only thing that we are using in the Mesos UI and changed 
> is the "hide" class (deprecated), so instances of these have been replaced by 
> its successor ".hidden" class.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 6b18056 
>   src/webui/master/static/css/bootstrap-3.0.3.min.css c547283 
>   src/webui/master/static/css/bootstrap-3.3.6.min.css PRE-CREATION 
>   src/webui/master/static/framework.html 9b28820 
>   src/webui/master/static/home.html d6cde1e 
>   src/webui/master/static/index.html 25caf53 
>   src/webui/master/static/slave.html bc46885 
>   src/webui/master/static/slave_executor.html 9d582d5 
>   src/webui/master/static/slave_framework.html 96d788f 
> 
> Diff: https://reviews.apache.org/r/43072/diff/
> 
> 
> Testing
> ---
> 
> - Mobile: http://cl.ly/2R2O0m1a1G3Q/Image%202016-02-01%20at%2015.21.53.png
> - 1680 x 1050px screen: 
> http://cl.ly/0K1o112u3L2I/Image%202016-02-01%20at%2015.23.37.png
> - 2880 x 1800px screen: 
> http://cl.ly/1M0L3M2X0J2i/Image%202016-02-01%20at%2015.22.50.png
> 
> Visual test in latest Chrome, Firefox, Safari and IE. No functional changes 
> introduced.
> 
> 
> Thanks,
> 
> Michael Lunøe
> 
>

Re: Review Request 43072: Use full screen width for Mesos UI using Bootstraps (v3.3.6) container-fluid

[haosdent huang]

> On Feb. 2, 2016, 2:45 a.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [43072]
> > 
> > Failed command: ./support/apply-review.sh -n -r 43072
> > 
> > Error:
> > 2016-02-02 02:45:28 URL:https://reviews.apache.org/r/43072/diff/raw/ 
> > [226634/226634] -> "43072.patch" [1]
> > Traceback (most recent call last):
> >   File "support/apply-reviews.py", line 342, in 
> > reviewboard()
> >   File "support/apply-reviews.py", line 321, in reviewboard
> > apply_review()
> >   File "support/apply-reviews.py", line 139, in apply_review
> > commit_patch()
> >   File "support/apply-reviews.py", line 180, in commit_patch
> > data = patch_data()
> >   File "support/apply-reviews.py", line 199, in patch_data
> > return reviewboard_data()
> >   File "support/apply-reviews.py", line 252, in reviewboard_data
> > email=user.get('email'))
> > UnicodeEncodeError: 'ascii' codec can't encode character u'\xf8' in 
> > position 11: ordinal not in range(128)
> > 
> > Full log: https://builds.apache.org/job/mesos-reviewbot/11187/console
> 
> haosdent huang wrote:
> I think we could ignore this error message.
> 
> Bernd Mathiske wrote:
> I'd rather not ignore the error message. How can there be an out-of-range 
> char in HTML and CSS files?
> 
> haosdent huang wrote:
> I afraid this may caused by boostrap have some special characters like 
> `ok:before{content:"\e013"}`.
> 
> Bernd Mathiske wrote:
> This should be an encoding using printable cahrs (<=128) only.

LoL, seems `Michael Lunøe` cause the problem. Need fix the apply_reviews.py


- haosdent


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


On Feb. 1, 2016, 11:36 p.m., Michael Lunøe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43072/
> ---
> 
> (Updated Feb. 1, 2016, 11:36 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Thomas Rampelberg.
> 
> 
> Bugs: MESOS-2585
> https://issues.apache.org/jira/browse/MESOS-2585
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The solution to our problem is solved by updating the Bootstrap CSS file, as 
> container-fluid was introduced later in Bootstrap.
> 
> I have updated to lastest stable release of Bootstrap v3.3.6. Looking through 
> the change log, the only thing that we are using in the Mesos UI and changed 
> is the "hide" class (deprecated), so instances of these have been replaced by 
> its successor ".hidden" class.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 6b18056 
>   src/webui/master/static/css/bootstrap-3.0.3.min.css c547283 
>   src/webui/master/static/css/bootstrap-3.3.6.min.css PRE-CREATION 
>   src/webui/master/static/framework.html 9b28820 
>   src/webui/master/static/home.html d6cde1e 
>   src/webui/master/static/index.html 25caf53 
>   src/webui/master/static/slave.html bc46885 
>   src/webui/master/static/slave_executor.html 9d582d5 
>   src/webui/master/static/slave_framework.html 96d788f 
> 
> Diff: https://reviews.apache.org/r/43072/diff/
> 
> 
> Testing
> ---
> 
> - Mobile: http://cl.ly/2R2O0m1a1G3Q/Image%202016-02-01%20at%2015.21.53.png
> - 1680 x 1050px screen: 
> http://cl.ly/0K1o112u3L2I/Image%202016-02-01%20at%2015.23.37.png
> - 2880 x 1800px screen: 
> http://cl.ly/1M0L3M2X0J2i/Image%202016-02-01%20at%2015.22.50.png
> 
> Visual test in latest Chrome, Firefox, Safari and IE. No functional changes 
> introduced.
> 
> 
> Thanks,
> 
> Michael Lunøe
> 
>

Re: Review Request 42781: Added the --cgroups_net_cls_primary_handle flag to the slave.

[Avinash sridharan]

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

(Updated Feb. 3, 2016, 7:47 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added the --cgroups_net_cls_primary_handle flag to the slave.


Diffs (updated)
-

  src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
  src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
  src/slave/flags.hpp 23ec158f4a95fa76d657d9eb6a4a6fe30d57e5b6 
  src/slave/flags.cpp 24a23325cc255d0d7b7af7ed096b6d3012ad75c7 

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


Testing
---

Tested the flag by launching a mesos agent and checking that the slave takes 
the `--cgroups_net_cls_major_handles` flag.


Thanks,

Avinash sridharan

Re: Review Request 42780: Changed the NetClsIsolatorTest to check for net_cls handles.

[Avinash sridharan]

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

(Updated Feb. 3, 2016, 7:47 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Changed the NetClsIsolatorTest to check for net_cls handles.


Diffs (updated)
-

  src/tests/containerizer/isolator_tests.cpp 
8d101df957fd36adac388310eddba2db1f98c029 

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


Testing
---

make and make check


Thanks,

Avinash sridharan

Re: Review Request 42618: Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.

[Avinash sridharan]

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

(Updated Feb. 3, 2016, 7:46 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Modified the `cgroup/net_cls` isolator to use the `NetClsHandleMgr`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b4bc52114389d1c1efce2830f4292bd89bb0de7c 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 

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


Testing
---

make and make check


Thanks,

Avinash sridharan

Re: Review Request 42587: Implemented the `NetClsHandleManager` class.

[Anand Mazumdar]

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



Primarily fly-by style comments.


src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 58)


We generally leave a line before the comment and the TODO to correctly 
demarcate where the comment ends and where the TODO begins.

```
// comments
//
// TODO(asridharan): 
//
```



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 91)


What does `handles.size()` returns. I am assuming its an `unsigned in`t and 
hence should fail compilation with `-Wsign-compare` on GCC.

Why don't we just do `size_t count = 1`?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 95)


Should fit on one line?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 96)


I see that we name `NetClsHandles` as `handles`.

Why not name `NetClsHandle` as just `handle`?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 120)


Remove period at the end.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 131)


How about simplifying this to:

```
if (condition) {
  return Nothing();
}

return Error(...);
```



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 154)


Remove period at the end.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 165)


How about simplifying this to:

```
if (condition) {
  return Nothing();
}

return Error(...);
```


- Anand Mazumdar


On Feb. 3, 2016, 6:46 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42587/
> ---
> 
> (Updated Feb. 3, 2016, 6:46 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the `NetClsHandleManager` class.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b4bc52114389d1c1efce2830f4292bd89bb0de7c 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
> 
> Diff: https://reviews.apache.org/r/42587/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42594: Introduced framework registry operations.

[Yongqiao Wang]

> On Feb. 1, 2016, 9:03 a.m., Jian Qiu wrote:
> > src/master/master.hpp, line 1638
> > 
> >
> > hashset* slaveIDs, bool strict

Those parameters do not be used, so it does not need to name them.


> On Feb. 1, 2016, 9:03 a.m., Jian Qiu wrote:
> > src/master/master.hpp, line 1629
> > 
> >
> > When to use readmit if framwork is not allowed to be updated

According to the original ideas, ReadmitFramework function will be called when 
framework re-register. If the registry do not mutated when framework 
re-register, then we can remove this function.


- Yongqiao


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


On Feb. 1, 2016, 7:24 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42594/
> ---
> 
> (Updated Feb. 1, 2016, 7:24 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Guangya Liu, Klaus Ma, Klaus Ma, Qian 
> Zhang, Jian Qiu, and Vinod Kone.
> 
> 
> Bugs: MESOS-1719
> https://issues.apache.org/jira/browse/MESOS-1719
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced framework registry operations.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 
> 
> Diff: https://reviews.apache.org/r/42594/diff/
> 
> 
> Testing
> ---
> 
> Make && Make check successfully!
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 42594: Introduced framework registry operations.

[Yongqiao Wang]

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

(Updated Feb. 2, 2016, 8:53 a.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Klaus Ma, Klaus Ma, Qian 
Zhang, Jian Qiu, and Vinod Kone.


Changes
---

Addressed Jian's comments.


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


Repository: mesos


Description
---

Introduced framework registry operations.


Diffs (updated)
-

  src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 

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


Testing
---

Make && Make check successfully!


Thanks,

Yongqiao Wang

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

[Abhishek Dasgupta]

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

(Updated Feb. 2, 2016, 8:58 a.m.)


Review request for mesos and Guangya Liu.


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


Repository: mesos


Description
---

Set Docker labels based on TaskInfo labels.


Diffs (updated)
-

  src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
  src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
  src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
  src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
  src/tests/containerizer/docker_containerizer_tests.cpp 
645bdcf095145097d8b8c65d592c787417883145 
  src/tests/containerizer/docker_tests.cpp 
f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
  src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
  src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 

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


Testing
---

The following test cases in docker_tests have been changed:
DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
docker run command through arguments.


Thanks,

Abhishek Dasgupta

Review Request 43091: Renamed a variable and updated a comment for clarity.

[Alexander Rukletsov]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  include/mesos/master/allocator.hpp e163669c9c4e4c98572968f18987704b60722a79 
  src/master/allocator/mesos/allocator.hpp 
581eaad376e7b2febe0b6359014617b935a677a3 
  src/master/allocator/mesos/hierarchical.hpp 
1f0c440565ee0f9926d168734df30fd740a53f88 
  src/master/allocator/mesos/hierarchical.cpp 
1a07d69016407e5aad2209586da37fecbcddb765 
  src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 
  src/tests/allocator.hpp 206e9ac3a83038a691f7929bdd627042b0f363b0 

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


Testing
---

`make check`


Thanks,

Alexander Rukletsov

Re: Review Request 42222: Added a comment on allocator recovery.

[Alexander Rukletsov]

> On Jan. 15, 2016, 6:50 a.m., Ben Mahler wrote:
> > Thanks Alex! I ended up going over the structure of the recover code and 
> > left some higher level comments. There also appears to be a bug that will 
> > crash the master that I marked as an issue :)
> > 
> > Have we convinced ourselves that we want to act on partial state when quota 
> > is *not* set? It seems to complicate things here, and acting on partial 
> > state w/o quota has some issues as well?
> 
> Alexander Rukletsov wrote:
> Thanks a bunch for a thorough review, Ben! I have opened a separate 
> ticket to track the bug and other comments you had left: 
> https://issues.apache.org/jira/browse/MESOS-4417.

> Have we convinced ourselves that we want to act on partial state when quota 
> is not set?

I think it's fine, because the allocator does not provide any guarantees 
without quota, neither it enforces the sharing scheme computed by the sorter. 
There can be other scenarios other than recovery that may lead to biased 
cluster sharing. Do you have any specific use cases in mind why shall we 
*always* recover?


> On Jan. 15, 2016, 6:50 a.m., Ben Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 149
> > 
> >
> > It now seems odd to start the allocation loop within initialize rather 
> > than within recover, we'll do the following:
> > 
> > 
> > intialize -> unpauses, starts allocation loop but the allocation loop 
> > should do nothing
> > recover -> pauses allocations, waits for condition to be met, resumes 
> > allocation.
> > 
> > 
> > It seems a simpler way to think about this would be to just start 
> > allocating after the recovery condition is met. This would consist of (1) 
> > started = true* and (2) the initial delay to 'batch' that starts the delay 
> > loop.
> > 
> > * Although keeping the pause concept may be useful if we want to 
> > support pausing/resuming allocation from endpoints.

When we were thinking about how to implement it in the most consistent way, we 
have figured out, that the widely used pattern is:
`initialize()` -> [optional] `recover()`. Both `initialize()` and `recover()` 
are called from the client code. Hence we decided it would be great to ensure 
things work if `recover()` has not been called. We can also always call 
`recover()` even though there is no quota to recover.

Anyway I favour to keep the pause concept regardless which workflow we opt for.


> On Jan. 15, 2016, 6:50 a.m., Ben Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 168
> > 
> >
> > Should this comment be moved down?
> > 
> > I'm also not quite sure what it's expressing, is it a re-hash of the 
> > lifecycle documentation?
> > 
> > ```
> >* Because it is hard to define recovery for a running allocator, this
> >* method should be called after `initialize()`, but before actual
> >* allocation starts (i.e. `addSlave()` is called).
> > ```
> > 
> > For example:
> > 
> > ```
> > CHECK(initialized);
> > 
> > // Recovery should be called after initialize and before other calls.
> > CHECK_EQ(0u, slaves.size());
> > CHECK_EQ(0, quotaRoleSorter->count());
> > ```
> > 
> > Could we extend our existing 'initialized' state boolean into an enum 
> > to express UNINITIALIZED, INITIALIZED, RECOVERED? As far as I can tell, the 
> > existing methods with `CHECK(initialized)` are also interested in checking 
> > that recover was called?

I think the question here is whether we want recovery be optional or not. If it 
is optional, tracking state transitions may become tedious.


> On Jan. 15, 2016, 6:50 a.m., Ben Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 171-172
> > 
> >
> > Not for this review, but looks like we should update `Sorter::count` to 
> > return a size_t.
> > 
> > The following CHECK implies that `_expectedAgentCount` should just be a 
> > size_t?
> > 
> > ```
> > CHECK(_expectedAgentCount >= 0);
> > ```
> > 
> > With a size_t we don't have to do this kind of check :)

This is a general question: shall we use `size_t` for non-negative integers or 
follow google style guide (and protobufs!) and use int. I'm not sure we have an 
agreement on this one.


> On Jan. 15, 2016, 6:50 a.m., Ben Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 468
> > 
> >
> > Hm.. did you need the static cast here?

I believe so, one is `size_t` and the other one is `int`.


- Alexander


---
This is an automatically generated e-mail. To reply, visit:

Re: Review Request 42586: Defined the NetClsHandleMgr class.

[Avinash sridharan]

> On Feb. 1, 2016, 5:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp, lines 65-75
> > 
> >
> > Why do we need this?

Was using this data structure to pass on the usage of handles for a given major 
handle. It was useful in unit-testing, but thought will be useful in 
implementing a usage interface to the NetClsHandleMgr class. Should I keep it?


> On Feb. 1, 2016, 5:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp, lines 108-113
> > 
> >
> > We do you need this in this patch?

There are quite a few places where we are throwing erros and need to convert 
the major and minor handle into a hex string, hence thought implement this as a 
static function (maybe inline) . Should I keep this?


> On Feb. 1, 2016, 5:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp, line 40
> > 
> >
> > 2 blank lines above please.
> 
> Guangya Liu wrote:
> Does there are any policies for this when need one blank line and when 
> need two blank line?

The coding guide lines give the following rule:
"Elements outside classes (classes, structs, global functions, etc.) should be 
spaced apart by two empty lines."

So from that perspective this makes sense.


- Avinash


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


On Feb. 1, 2016, 2:20 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42586/
> ---
> 
> (Updated Feb. 1, 2016, 2:20 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This class will be responsible for keeping track of the free and allocated
> net_cls handles.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b4bc52114389d1c1efce2830f4292bd89bb0de7c 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
> 
> Diff: https://reviews.apache.org/r/42586/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42910: Added a note about revocable resources beyond quota in the user doc.

[Alexander Rukletsov]

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

(Updated Feb. 2, 2016, 10:27 a.m.)


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


Repository: mesos


Description
---

See summary.


Diffs
-

  docs/quota.md 2762557ad1cec0655a27c4dda4016e07c7f63cd9 

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


Testing
---

None.


Thanks,

Alexander Rukletsov

Re: Review Request 42633: Corrected a comment in the allocator.

[Alexander Rukletsov]

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

(Updated Feb. 2, 2016, 10:28 a.m.)


Review request for mesos, Ben Mahler and Qian Zhang.


Repository: mesos


Description
---

Revocable resources may come from different sources: usage slack (aka
oversubscribed resources), allocation slack (aka optimistic offers).
We should not use "oversubscribed" and "revocable" interchangeably.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
1f0c440565ee0f9926d168734df30fd740a53f88 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 42637: Cleaned up formatting in allocator tests.

[Alexander Rukletsov]

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

(Updated Feb. 2, 2016, 10:29 a.m.)


Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
953712149bd951789beb29c72779c4ac65aa48dc 

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


Testing
---

make check


Thanks,

Alexander Rukletsov

Re: Review Request 42594: Introduced framework registry operations.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42591, 42594]

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 Feb. 2, 2016, 8:53 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42594/
> ---
> 
> (Updated Feb. 2, 2016, 8:53 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Guangya Liu, Klaus Ma, Klaus Ma, Qian 
> Zhang, Jian Qiu, and Vinod Kone.
> 
> 
> Bugs: MESOS-1719
> https://issues.apache.org/jira/browse/MESOS-1719
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced framework registry operations.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 
> 
> Diff: https://reviews.apache.org/r/42594/diff/
> 
> 
> Testing
> ---
> 
> Make && Make check successfully!
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 42971: Marked an unused function parameter appropriately in stout.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 1, 2016, 10:08 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42971/
> ---
> 
> (Updated Feb. 1, 2016, 10:08 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Marked an unused function parameter appropriately in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 
> a3dc7697f5f041a503e4aa84df67c915c43b76b5 
> 
> Diff: https://reviews.apache.org/r/42971/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42972: Fixed a minor bug in stout's Flags package.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 2, 2016, 1:53 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42972/
> ---
> 
> (Updated Feb. 2, 2016, 1:53 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unintended inequality comparison between pointer value and 0.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
> 2a188459465a5203c56d788a74e69d403790c5bf 
> 
> Diff: https://reviews.apache.org/r/42972/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42973: Marked some unused function parameters in libprocess appropriately.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 1, 2016, 10:08 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42973/
> ---
> 
> (Updated Feb. 1, 2016, 10:08 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Marked some unused function parameters in libprocess appropriately.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/event.hpp 
> a03824c061c4a0eb865b163999a763635e56744c 
>   3rdparty/libprocess/include/process/filter.hpp 
> 2f7e6fb4be8725c8a91f56f42baed071a3ef10c3 
>   3rdparty/libprocess/include/process/process.hpp 
> 1e4126f1082c24debd9d713b490d18f5ed83f0be 
> 
> Diff: https://reviews.apache.org/r/42973/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42974: Removed unused variable in libprocess.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 1, 2016, 10:08 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42974/
> ---
> 
> (Updated Feb. 1, 2016, 10:08 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unused variable in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> d8a74d7637d20c81f384e974e4fdeba22effb437 
> 
> Diff: https://reviews.apache.org/r/42974/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43091: Renamed a variable and updated a comment for clarity.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43091]

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 Feb. 2, 2016, 2:54 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43091/
> ---
> 
> (Updated Feb. 2, 2016, 2:54 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4443
> https://issues.apache.org/jira/browse/MESOS-4443
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp e163669c9c4e4c98572968f18987704b60722a79 
>   src/master/allocator/mesos/allocator.hpp 
> 581eaad376e7b2febe0b6359014617b935a677a3 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1f0c440565ee0f9926d168734df30fd740a53f88 
>   src/master/allocator/mesos/hierarchical.cpp 
> 1a07d69016407e5aad2209586da37fecbcddb765 
>   src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 
>   src/tests/allocator.hpp 206e9ac3a83038a691f7929bdd627042b0f363b0 
> 
> Diff: https://reviews.apache.org/r/43091/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 43083: Supported working dir in docker runtime isolator.

[Gilbert Song]

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

(Updated Feb. 2, 2016, 9:18 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Supported working dir in docker runtime isolator.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
4b504dbb58823ce7675f1d2048dcc7a27c05663d 
  src/slave/containerizer/mesos/isolators/docker/runtime.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/runtime.cpp PRE-CREATION 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song

Re: Review Request 43021: Implemented docker runtime isolator interface.

[Gilbert Song]

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

(Updated Feb. 2, 2016, 9:18 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Implemented docker runtime isolator interface.


Diffs
-

  src/CMakeLists.txt 6b2c7cbffa57d44b913419ad68fc66eaeca169d0 
  src/Makefile.am fac17f4bac3b2ddda0384dec8b0ed6f960bd24d1 
  src/slave/containerizer/mesos/isolators/docker/runtime.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/runtime.cpp PRE-CREATION 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song

Re: Review Request 43019: Fixed non-camel case in protobuf field.

[Gilbert Song]

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

(Updated Feb. 2, 2016, 9:18 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


Repository: mesos


Description
---

Fixed non-camel case in protobuf field.


Diffs
-

  include/mesos/slave/isolator.proto 8b7c54afc4467084b5d22387e55689d06078b504 
  src/slave/containerizer/mesos/containerizer.cpp 
4b504dbb58823ce7675f1d2048dcc7a27c05663d 
  src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp 
57168c837aabf85c85866ca9d4b5480f281e3143 
  src/slave/containerizer/mesos/isolators/cgroups/mem.cpp 
34aa660204911ce4f22c7735af36c9f1af8b12b9 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
24ff8194bb5ea86a31224d2d2c20d367472168aa 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
bce5ab9a1913ee7cebaf55b5ee0b42fa4a325e87 
  src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
59ffd13920fb1c4fb8dbf04a2823d9cee75a8f27 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
d5dd93d892e3cde2011e208a050eec2b31891f54 
  src/tests/containerizer/isolator_tests.cpp 
8d101df957fd36adac388310eddba2db1f98c029 
  src/tests/containerizer/port_mapping_tests.cpp 
182fe9217a5da9af603d6f9c203a1689eff4ca1b 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song

Re: Review Request 43020: Exposed accurate ProvisionInfo to command executor.

[Gilbert Song]

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

(Updated Feb. 2, 2016, 9:18 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


Repository: mesos


Description
---

Exposed accurate ProvisionInfo to command executor.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
4b504dbb58823ce7675f1d2048dcc7a27c05663d 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song

Re: Review Request 43081: Supported entrypoint and cmd in docker runtime isolator.

[Gilbert Song]

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

(Updated Feb. 2, 2016, 9:18 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Supported entrypoint and cmd in docker runtime isolator.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
4b504dbb58823ce7675f1d2048dcc7a27c05663d 
  src/slave/containerizer/mesos/isolators/docker/runtime.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/runtime.cpp PRE-CREATION 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song

Re: Review Request 43036: Plugged in docker runtime isolator.

[Gilbert Song]

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

(Updated Feb. 2, 2016, 9:18 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Plugged in docker runtime isolator.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
4b504dbb58823ce7675f1d2048dcc7a27c05663d 
  src/slave/containerizer/mesos/isolators/docker/runtime.cpp PRE-CREATION 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song

Re: Review Request 43037: Support env var in docker runtime isolator.

[Gilbert Song]

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

(Updated Feb. 2, 2016, 9:18 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Support env var in docker runtime isolator.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
4b504dbb58823ce7675f1d2048dcc7a27c05663d 
  src/slave/containerizer/mesos/isolators/docker/runtime.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/runtime.cpp PRE-CREATION 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song

Re: Review Request 43082: Added new flag to command executor for command passing.

[Gilbert Song]

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

(Updated Feb. 2, 2016, 9:18 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Added new flag to command executor for command passing.


Diffs
-

  src/launcher/executor.cpp 356d311fdf97b2c4663c60e13ede7cdb71a264c7 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song

Re: Review Request 43022: Added protobuf fields for docker runtime isolator.

[Gilbert Song]

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

(Updated Feb. 2, 2016, 9:18 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Added protobuf fields for docker runtime isolator.


Diffs
-

  include/mesos/slave/isolator.proto 8b7c54afc4467084b5d22387e55689d06078b504 

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


Testing
---

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song

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

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42516]

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

- Mesos ReviewBot


On Feb. 2, 2016, 1:57 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 2, 2016, 1:57 p.m.)
> 
> 
> Review request for mesos and TimothyIL TimothyIL.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 194750e92020753e60154083a47bdc3398d31466 
>   include/mesos/v1/mesos.proto 1102bbc92f46f97c1915c03a71c7cf829003e0ed 
>   src/docker/docker.cpp a83172674b385a59cfd6f344836740a5719f954e 
> 
> 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 43019: Fixed non-camel case in protobuf field.

[Jie Yu]

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



So unfortunate that this didn't get into 0.27. We'll break the isolator 
interface again:( Can you update the upgrades.md and mention that change? 
Thanks!


src/tests/containerizer/port_mapping_tests.cpp (line 692)


You forget this? Please do a grep to find all occurances.


- Jie Yu


On Feb. 2, 2016, 5:18 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43019/
> ---
> 
> (Updated Feb. 2, 2016, 5:18 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed non-camel case in protobuf field.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.proto 8b7c54afc4467084b5d22387e55689d06078b504 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4b504dbb58823ce7675f1d2048dcc7a27c05663d 
>   src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp 
> 57168c837aabf85c85866ca9d4b5480f281e3143 
>   src/slave/containerizer/mesos/isolators/cgroups/mem.cpp 
> 34aa660204911ce4f22c7735af36c9f1af8b12b9 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 24ff8194bb5ea86a31224d2d2c20d367472168aa 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
> bce5ab9a1913ee7cebaf55b5ee0b42fa4a325e87 
>   src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
> 59ffd13920fb1c4fb8dbf04a2823d9cee75a8f27 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> d5dd93d892e3cde2011e208a050eec2b31891f54 
>   src/tests/containerizer/isolator_tests.cpp 
> 8d101df957fd36adac388310eddba2db1f98c029 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 182fe9217a5da9af603d6f9c203a1689eff4ca1b 
> 
> Diff: https://reviews.apache.org/r/43019/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 42586: Defined the NetClsHandleMgr class.

[Avinash sridharan]

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

(Updated Feb. 2, 2016, 5:52 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

This class will be responsible for keeping track of the free and allocated
net_cls handles.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b4bc52114389d1c1efce2830f4292bd89bb0de7c 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 

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


Testing
---

make


Thanks,

Avinash sridharan

Re: Review Request 42586: Defined the NetClsHandleManager class.

[Avinash sridharan]

> On Feb. 1, 2016, 5:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp, lines 51-52
> > 
> >
> > `s/majHandle/_major/`
> > `s/minHandle/_minor/`
> > 
> > Also, remove the space right after `NetClsHandle`

Using primary and secondary instead of major and minor.


> On Feb. 1, 2016, 5:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp, lines 65-75
> > 
> >
> > Why do we need this?
> 
> Avinash sridharan wrote:
> Was using this data structure to pass on the usage of handles for a given 
> major handle. It was useful in unit-testing, but thought will be useful in 
> implementing a usage interface to the NetClsHandleMgr class. Should I keep it?

Removed this structure. We no longer need it.


> On Feb. 1, 2016, 5:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp, lines 78-85
> > 
> >
> > I would suggest that we don't define a new data structure for this. Can 
> > you just inline it in NetClsHandleManager?

Simplified the data structure. We no longer need an explicit NetClsHandles 
structure.


> On Feb. 1, 2016, 5:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp, lines 108-113
> > 
> >
> > We do you need this in this patch?
> 
> Avinash sridharan wrote:
> There are quite a few places where we are throwing erros and need to 
> convert the major and minor handle into a hex string, hence thought implement 
> this as a static function (maybe inline) . Should I keep this?

Removed this structure from this patch. Will re-introduce it when we introduce 
the implementation.


> On Feb. 1, 2016, 5:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp, line 129
> > 
> >
> > What's the key for this map?

Added a comment to elaborate on the key.


- Avinash


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


On Feb. 3, 2016, 6:44 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42586/
> ---
> 
> (Updated Feb. 3, 2016, 6:44 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This class will be responsible for keeping track of the free and allocated
> net_cls handles.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b4bc52114389d1c1efce2830f4292bd89bb0de7c 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
> 
> Diff: https://reviews.apache.org/r/42586/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42586: Defined the NetClsHandleManager class.

[Avinash sridharan]

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

(Updated Feb. 3, 2016, 6:44 a.m.)


Review request for mesos and Jie Yu.


Summary (updated)
-

Defined the NetClsHandleManager class.


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


Repository: mesos


Description
---

This class will be responsible for keeping track of the free and allocated
net_cls handles.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b4bc52114389d1c1efce2830f4292bd89bb0de7c 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 

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


Testing
---

make


Thanks,

Avinash sridharan

Re: Review Request 42587: Implemented the `NetClsHandleManager` class.

[Avinash sridharan]

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

(Updated Feb. 3, 2016, 6:46 a.m.)


Review request for mesos and Jie Yu.


Summary (updated)
-

Implemented the `NetClsHandleManager` class.


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


Repository: mesos


Description (updated)
---

Implemented the `NetClsHandleManager` class.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b4bc52114389d1c1efce2830f4292bd89bb0de7c 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 

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


Testing
---

make


Thanks,

Avinash sridharan

Review Request 43134: Added output operator for unversioned executor protobuf.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

This change adds the missing output operator for the unversioned executor 
protobuf. These are not used in the source code currently and hence were 
missing. The executor library uses the `v1` protobufs and the output operator 
for them was already added.


Diffs
-

  include/mesos/executor/executor.hpp a7d40f6acaf24e086fb46ecfb74e97612ba326d4 

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


Testing
---

make check


Thanks,

Anand Mazumdar