Re: Review Request 41333: Added helper functions to filter usage slack resources.

[Klaus Ma]

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




include/mesos/resources.hpp (line 174)


`isThrottleable`?


- Klaus Ma


On March 15, 2016, 10:33 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41333/
> ---
> 
> (Updated March 15, 2016, 10:33 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
> https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper functions to filter usage slack resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp bb343ad852576a75615a93ef850b413bf77698e0 
>   include/mesos/v1/resources.hpp 719110fbbf39f1755460ac0b32e3893656054a4e 
>   src/common/resources.cpp 1f23dc83f7330c305a836d698f114b7eaf3d7ba1 
>   src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
>   src/v1/resources.cpp c6f125ec317e2da537a6456f5cff2da0a48701d8 
> 
> Diff: https://reviews.apache.org/r/41333/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 43615: Update test suite to use the reworked MesosTest helpers.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43613, 43614, 43629, 43630, 43615]

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

- Mesos ReviewBot


On March 16, 2016, 12:21 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43615/
> ---
> 
> (Updated March 16, 2016, 12:21 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Includes the following changes:
> 
> * Added the `` header where appropriate.
> * Added the namespace `using process::Owned;` where appropriate.
> * Generally replaced `Try` with `Try`.  
> And `Try` with `Try`.
> * Added the (now required) `MasterDetector` argument to all slaves.  Before, 
> this was fetched from the first master in `Cluster`.
> * Removed `Shutdown();` from all tests.
> * Replaced `Stop(...)` with the appropriate master/slave destruction calls.
> * Wrap various slave objects in `Owned` (i.e. containerizers, isolators, 
> launchers, etc).
> * Replace `CHECK` in tests with `ASSERT`.
> 
> 
> Diffs
> -
> 
>   src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
>   src/tests/command_executor_tests.cpp 
> 0d2fcf6d4b8d9a925eb6748e6bd33cf279b8f7f8 
>   src/tests/container_logger_tests.cpp 
> 00f4129e46aa9268fbb66da25b34e61004fa87b2 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 8afaa4dab3984e9866b7b223e8e2e70ef83a39dc 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 8e1dbe306a088eb16cd3b9c6174b95fad5685da4 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> e72239a55724f1aeeec5362cc370c93dbeca7164 
>   src/tests/containerizer/isolator_tests.cpp 
> 342037ce0a5f8caa4e3cf1550b8f9a7cc328acf9 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> 03879d99c371f296f8d9904666911b34209c114d 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 15f0f93d2e5c19a22f6cc4a71a7d94be4aaec2c1 
>   src/tests/containerizer/port_mapping_tests.cpp 
> a1427fd0157dee343b643f3272dba8ffea61f7b0 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> b2663a83afccbb156f10b4703f29ee0f638384bf 
>   src/tests/containerizer/runtime_isolator_tests.cpp 
> 1625614f560a4c9a644b1b9e9568199f30373ab5 
>   src/tests/credentials_tests.cpp 7edcc857e0f6f8e80e265deeec59d6349d392224 
>   src/tests/disk_quota_tests.cpp 413e562026a4fc9779f616e921ae2fa2ca51e012 
>   src/tests/exception_tests.cpp 6b71316d545e97f14a45daa14d0fd95204befd3b 
>   src/tests/executor_http_api_tests.cpp 
> 2fc0893f5f5e80a783296fb31b30abe86d92df1b 
>   src/tests/fault_tolerance_tests.cpp 
> 67f362747ac4301a74693aeaa5631b7dd866e782 
>   src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf 
>   src/tests/health_check_tests.cpp 9605859b665645654bbdb2688455cae2d692a057 
>   src/tests/hook_tests.cpp bb287c77493209e663a37e7f88d09a0459855f7d 
>   src/tests/http_fault_tolerance_tests.cpp 
> 7c7f3d90210148176e83553346100a506f263591 
>   src/tests/master_allocator_tests.cpp 
> cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_authorization_tests.cpp 
> 6a4de4709960d7ca505e99396e14a1bb51d6902d 
>   src/tests/master_contender_detector_tests.cpp 
> 255ab8119a04b55bb4f1b61dee19c4be64499376 
>   src/tests/master_quota_tests.cpp f1277f03f5b5cf735fea7ba324a27614a829cd50 
>   src/tests/master_slave_reconciliation_tests.cpp 
> d41178eb41df519073fc0890c5716bbc9fed6ad2 
>   src/tests/master_tests.cpp e8c39e775d6ca218ce74cfc6bb50c7576d73e90e 
>   src/tests/master_validation_tests.cpp 
> c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 
>   src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 
>   src/tests/monitor_tests.cpp 869c9e032817e8859a968232d4a61556a3d53d45 
>   src/tests/oversubscription_tests.cpp 
> e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
>   src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> b5f425ab7f2b4a540418d761bea145565467bd2b 
>   src/tests/persistent_volume_tests.cpp 
> e9215de2e073025f67cdc73e8a8de38cf030671f 
>   src/tests/rate_limiting_tests.cpp 352a5e7a58b500c25c7c8a421047c4e79434e38e 
>   src/tests/reconciliation_tests.cpp e8f3f29836652d20a6ee1bb5231a15e71eb76990 
>   src/tests/registrar_zookeeper_tests.cpp 
> 3df9779ee5d076e16f6a538326693a36f986b6d0 
>   src/tests/repair_tests.cpp 

Re: Review Request 42036: Windows: Added `slave/flags.cpp` to Windows build.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On March 11, 2016, 9:51 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42036/
> ---
> 
> (Updated March 11, 2016, 9:51 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, and 
> Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added `slave/flags.cpp` to Windows build.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt e9f7c3a447aad79ab19d53879888a413a587408f 
> 
> Diff: https://reviews.apache.org/r/42036/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 44836: Update TaskId in long_lived_framework to use fixed length taskid.

[Jay Guo]

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

(Updated March 16, 2016, 2:27 a.m.)


Review request for mesos and haosdent huang.


Changes
---

rebase master


Repository: mesos


Description
---

Current WebUI sorts taskId lexically, which results in non-intuitive ordering.
This patch updates long_lived_framework to use `01`, `02` instead.


Diffs (updated)
-

  src/examples/long_lived_framework.cpp 
289a0b9dd3d1ce30f20dd9bb381126bff30c 

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


Testing
---

make check

In addition, `long_lived_framework` is ran and we confirm correct sorting of 
`TaskID` in WebUI


Thanks,

Jay Guo

Re: Review Request 44758: Upgrade to clang-format-3.8 (MESOS-4906).

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44758]

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

- Mesos ReviewBot


On March 16, 2016, 1:37 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44758/
> ---
> 
> (Updated March 16, 2016, 1:37 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4906
> https://issues.apache.org/jira/browse/MESOS-4906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade to clang-format-3.8 (MESOS-4906).
> 
> 
> Diffs
> -
> 
>   docs/clang-format.md 7f1c1dfd70e1fe9bfa186df1bdda7bdcf867db04 
>   support/clang-format 499d0e749e14e50256ae649afa0ced2b04589a0e 
> 
> Diff: https://reviews.apache.org/r/44758/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 44846: Deprecated the plain text credential format.

[Greg Mann]

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



FYI, this review: https://reviews.apache.org/r/44515/ adds command-line 
credentials to the agent with the `--http_credentials` flag. Depending on which 
one of these lands first, the other one should be updated to remain consistent.

- Greg Mann


On March 15, 2016, 2:01 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44846/
> ---
> 
> (Updated March 15, 2016, 2:01 p.m.)
> 
> 
> Review request for mesos, Adam B and Joerg Schad.
> 
> 
> Bugs: MESOS-2281
> https://issues.apache.org/jira/browse/MESOS-2281
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Deprecated the plain text credential format.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 761238c48332bcce0bff6c411225fdb4176ddca6 
>   docs/authentication.md e7c0bf3ed331411f607e7622419f14853006a480 
>   docs/configuration.md 739d4ff9aeeb1ba70ce11033168d63d37b6ef56b 
>   docs/upgrades.md 83b839f7fb996385baaa6ef1081dc1116cd6e338 
>   src/credentials/credentials.hpp aad17c21bb0ce98907bbfa22b890b66130e081e4 
>   src/master/flags.cpp e6fea6421ea1a16b9cd78b0e42b830829b95ad61 
>   src/slave/flags.cpp 4d10818105627738e258116647ccada374e3d7b9 
> 
> Diff: https://reviews.apache.org/r/44846/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 44755: Added getAppcImage for Appc provisioning tests.

[Anand Mazumdar]

> On March 15, 2016, 4:56 p.m., Jie Yu wrote:
> > src/tests/containerizer/provisioner_appc_tests.cpp, lines 587-610
> > 
> >
> > Can you move this function to the top of this file and make it 'static'?
> 
> Anand Mazumdar wrote:
> Wondering why does this function needs to be a global and can't be part 
> of the test fixture i.e. `AppcImageFetcherTest`?
> 
> Jojy Varghese wrote:
> The intend is to use it in all fixtures. Eg, for integegration tests, we 
> have a separate fixture.

I see. If the function has static scope, how would we use it across separate 
fixtures i.e. in some other test file? 

This is assuming that the integration tests would be part of a separate test 
file since we typically avoid having multiple test fixtures in a single file.


- Anand


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


On March 15, 2016, 6:26 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44755/
> ---
> 
> (Updated March 15, 2016, 6:26 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper function to get an Appc image protobuf object with given
> parameters.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 069af8a0b5e031d9032fd13154cee250f279b404 
> 
> Diff: https://reviews.apache.org/r/44755/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44755: Added getAppcImage for Appc provisioning tests.

[Jojy Varghese]

> On March 15, 2016, 4:56 p.m., Jie Yu wrote:
> > src/tests/containerizer/provisioner_appc_tests.cpp, lines 587-610
> > 
> >
> > Can you move this function to the top of this file and make it 'static'?
> 
> Anand Mazumdar wrote:
> Wondering why does this function needs to be a global and can't be part 
> of the test fixture i.e. `AppcImageFetcherTest`?
> 
> Jojy Varghese wrote:
> The intend is to use it in all fixtures. Eg, for integegration tests, we 
> have a separate fixture.
> 
> Anand Mazumdar wrote:
> I see. If the function has static scope, how would we use it across 
> separate fixtures i.e. in some other test file? 
> 
> This is assuming that the integration tests would be part of a separate 
> test file since we typically avoid having multiple test fixtures in a single 
> file.

Right now the integration tests are in the same file (in another patch). Not 
sure if having multiple fixtures in a test file is a no-no (eg. 
provisioner_docker_tests).


- Jojy


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


On March 15, 2016, 6:26 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44755/
> ---
> 
> (Updated March 15, 2016, 6:26 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper function to get an Appc image protobuf object with given
> parameters.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 069af8a0b5e031d9032fd13154cee250f279b404 
> 
> Diff: https://reviews.apache.org/r/44755/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44654: Fixed hard-coded executor shutdown grace period in executor library.

[Ben Mahler]

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



Looks great! Main thing is documenting why we don't assume the environment 
variable is present, and which version of the agent sets it.


src/exec/exec.cpp (line 110)


If you'd like to add the `private` qualifier, why isn't `kill` left as 
protected?



src/exec/exec.cpp (line 707)


A comment about why we don't require it to be set like the others above 
(backwards-compatibility) would be helpful for the reader here. Also would be 
good to mention that agents that are 0.29.0 (or did those patches land in 
0.28.0?) and above set this, so that we don't have to do a deeper investigation 
later if we want to figure out which agent version sets this.


- Ben Mahler


On March 15, 2016, 2:15 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44654/
> ---
> 
> (Updated March 15, 2016, 2:15 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4911
> https://issues.apache.org/jira/browse/MESOS-4911
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor shutdown grace period, which configured on the agent, is
> propagated to executors via the `MESOS_EXECUTOR_SHUTDOWN_GRACE_PERIOD`
> environment variable. The executor library uses this timeout to delay
> the hard shutdown of the related executor.
> 
> 
> Diffs
> -
> 
>   src/exec/exec.cpp 741786132f3a8cc43f5b9ced262429038832a946 
> 
> Diff: https://reviews.apache.org/r/44654/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44755: Added getAppcImage for Appc provisioning tests.

[Jojy Varghese]

> On March 15, 2016, 4:56 p.m., Jie Yu wrote:
> > src/tests/containerizer/provisioner_appc_tests.cpp, lines 587-610
> > 
> >
> > Can you move this function to the top of this file and make it 'static'?
> 
> Anand Mazumdar wrote:
> Wondering why does this function needs to be a global and can't be part 
> of the test fixture i.e. `AppcImageFetcherTest`?

The intend is to use it in all fixtures. Eg, for integegration tests, we have a 
separate fixture.


- Jojy


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


On March 12, 2016, 5:11 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44755/
> ---
> 
> (Updated March 12, 2016, 5:11 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper function to get an Appc image protobuf object with given
> parameters.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 069af8a0b5e031d9032fd13154cee250f279b404 
> 
> Diff: https://reviews.apache.org/r/44755/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44755: Added getAppcImage for Appc provisioning tests.

[Jojy Varghese]

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

(Updated March 15, 2016, 6:26 p.m.)


Review request for mesos and Jie Yu.


Changes
---

review addressed.


Repository: mesos


Description
---

Added helper function to get an Appc image protobuf object with given
parameters.


Diffs (updated)
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
069af8a0b5e031d9032fd13154cee250f279b404 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 44514: Implemented prepare() method of "network/cni" isolator.

[Jie Yu]

> On March 11, 2016, 8:02 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.hpp, line 85
> > 
> >
> > I would suggest we have a `Info` for each container.
> > 
> > ```
> > struct Info
> > {
> >   ...
> > };
> > ```
> > 
> > Remember that for any field you put into the Info struct, you need to 
> > be able to 'recover' it in recover function. It's also possible that 
> > ExecutorInfo/TaskInfo are not available during recover (orphans due to 
> > wiped meta data). You need to think about how to recover (e.g., the name of 
> > the network) that the container has joined.
> 
> Avinash sridharan wrote:
> We also need to recover the IP address associated with the container. 
> Since this has to be returned as part of `NetworkINfo` . Need to figure out 
> how to recover this?
> 
> Jie Yu wrote:
> I am not sure. Why do we need to store/recover IP address? When destroy a 
> container, the CNI plugin only needs container ID (which can be the same as 
> our container Id), the namespace handle and the networkconfig file. The rest 
> can all be fixed.
> 
> I think the trick part is: when there is an orphan container (no 
> executorinfo/taskinfo), how can we figure out its network namespace handle 
> and networconfig file so that we can destroy it (call CNI plugin DEL). We 
> need to know the name of the network to get the networkcofig file, and the 
> pid to get the network namespace handle.
> 
> Qian Zhang wrote:
> Yes, that is the tricky part. For orphan container (e.g., the container 
> created by the framework without checkpoint enabled), when agent restarts, in 
> the recover() we only have container ID which is not enough to invoke CNI 
> plugin DEL. Totally we need:
> 1) Container ID.
> 2) Network namespace path.
> 3) Network configuration.
> 4) Name of the interface inside the container.
> 
> I think currently we only have 1).
> For 2), my idea is, in `isolate()`, before calling CNI plugin, we do a 
> bind mount (`/var/run/netns/[pid]` -> `proc/[pid]/ns/net`), and then a symbol 
> link (`/var/run/mesos/cni/netns/[containerId]` -> `/var/run/netns/[pid]`). 
> And then in `recover()`, since we have `containerId` for orphan containers, 
> we should be able to figure out the `pid` based on the symbol link, and then 
> get the network namespace path `/proc/[pid]/ns/net`.
> For 3), actually when we call CNI plugin for a container in `isolate()`, 
> the CNI plugin (actually the CNI `host-local` IPAM plugin) will create a file 
> under `/var/lib/cni/networks/[neworkName]/`, the name of the file is the IP 
> assigned to the container, and the content of file is the container ID. So if 
> a container is assigned an IP from a CNI network, it must have a file under 
> `/var/lib/cni/networks/[neworkName]/`, we may leverage this information to 
> figure out the name of CNI networks the orphan container joins. But this may 
> only work for the CNI network which uses `host-local` IPAM plugin, for `dhcp` 
> IPAM plugin, I think it will not write any files for containers.
> For 4), I do not have idea for it yet :-(
> 
> Or another solution would be, when we create the symbol link in 
> `isolate()`, we may encode more info into the name of symbol link, e.g., 
> `/var/run/mesos/cni/netns/[containerId]-[net1]_[ifName in 
> net1]-[net2]_[ifName in net2]...`, however this seems ugly to me.
> 
> Avinash sridharan wrote:
> For (2) why not bind mount directly to 
> /var/run/mesos/cni/netns/[continaerId] and create a simlink to 
> /var/run/netns/. Also, Jie mentioned it would be a good idea to prefix the 
> `[containerId]` with a conanical string like mesos-cni. For (3) I don't think 
> we should rely on files created by CNI plugins. There is no guarantee that 
> the IPAM plugin would maintain those files. Instead if we have already bind 
> moutned the network namespace, the namespace will persist in the kernel even 
> after the container has exited (became an orphan). At recover we just need to 
> walk to list of `containerID` stored under /var/run/mesos/cni/netns and for 
> each network namespace we can run the `ip netns comamnd` to retrieve the link 
> and IP address for that network namespace.
> 
> Qian Zhang wrote:
> For 2), what symlink do you want to create under `/var/run/netns`? 
> `/var/run/netns/[pid] -> /var/run/mesos/cni/netns/[containerId]`? Or 
> `/var/run/netns/[containerId] -> /var/run/mesos/cni/netns/[contiainerId]`? 
> And you can take a look at 
> https://github.com/apache/mesos/blob/0.27.2/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp#L2205:L2243,
>  What it does is same as what I proposed above.
> 
> And good point for 3), I agree in `recover()`, we can run `ip netns exec 
> [namespace] ip addr` to retrieve the link and IP, but the problem is how we 
> can know which CNI networks 

Re: Review Request 44545: Separated standalone and zookeeper classes.

[Anurag Singh]

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

(Updated March 15, 2016, 6:17 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Instead of keeping standalone and zookeper contender/detector class
definitions and implementations in the same file, separated them. Also
made the necessary changes in users of class headers to point to the
new locations.


Diffs (updated)
-

  src/Makefile.am 7ee5a653fc96907021f14ab28f74c0b3ed0649d9 
  src/local/local.cpp f8599e7378e9a0065bbd01ad8f23f11debb30c91 
  src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
  src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
  src/master/contenders/contender.hpp PRE-CREATION 
  src/master/contenders/contender.cpp PRE-CREATION 
  src/master/contenders/standalone.cpp PRE-CREATION 
  src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
  src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
  src/master/detectors/detector.hpp PRE-CREATION 
  src/master/detectors/detector.cpp PRE-CREATION 
  src/master/detectors/standalone.cpp PRE-CREATION 
  src/master/main.cpp 7c1656bcc266f6c94cb4befad37fa813a218b2fa 
  src/master/master.hpp 124d43931a5c8a00ee0aaa604feb1761795209f2 
  src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 
  src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
  src/tests/cluster.cpp e5796d3cc17f814bec8f02dccf40515b65cfea91 
  src/tests/containerizer/external_containerizer_test.cpp 
8e1dbe306a088eb16cd3b9c6174b95fad5685da4 
  src/tests/containerizer/isolator_tests.cpp 
342037ce0a5f8caa4e3cf1550b8f9a7cc328acf9 
  src/tests/fault_tolerance_tests.cpp 349669d6aa0ead63b2ebcfcc2f769c99a6db0192 
  src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf 
  src/tests/master_allocator_tests.cpp cba7c36471f93b678d94e1da0251a28a893696b1 
  src/tests/master_authorization_tests.cpp 
6a4de4709960d7ca505e99396e14a1bb51d6902d 
  src/tests/master_contender_detector_tests.cpp 
255ab8119a04b55bb4f1b61dee19c4be64499376 
  src/tests/master_slave_reconciliation_tests.cpp 
d41178eb41df519073fc0890c5716bbc9fed6ad2 
  src/tests/master_tests.cpp e8c39e775d6ca218ce74cfc6bb50c7576d73e90e 
  src/tests/mesos.hpp 908a53a146208a6d41c0bda5208415e39e2eae05 
  src/tests/oversubscription_tests.cpp e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
  src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
  src/tests/persistent_volume_tests.cpp 
e9215de2e073025f67cdc73e8a8de38cf030671f 
  src/tests/reconciliation_tests.cpp e8f3f29836652d20a6ee1bb5231a15e71eb76990 
  src/tests/reservation_tests.cpp c88091635949037fcfc6be504043764f9af35f79 
  src/tests/scheduler_event_call_tests.cpp 
8c02ceeb3ec1783cb2f63f100700508e70f586e4 
  src/tests/scheduler_http_api_tests.cpp 
dfb0f51fec67a3951e396eab28eedb0dbf9493ae 
  src/tests/scheduler_tests.cpp fa42fb42f2d18060a867ade547cebbdcaead07d4 
  src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 
  src/tests/slave_tests.cpp 124e9587180f2a55e659d966d1c9060234c19457 

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


Testing
---


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

[Anurag Singh]

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

(Updated March 15, 2016, 6:16 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs (updated)
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44289/.


Thanks,

Anurag Singh

Re: Review Request 44288: Changed MasterDetector/Contender namespace.

[Anurag Singh]

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

(Updated March 15, 2016, 6:17 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Also modified users of MasterContender and MasterDetector to use this
namespace.


Diffs (updated)
-

  include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
  include/mesos/v1/scheduler.hpp 765935e97b6c1686ab464a5cf1cf2dfd816f51f1 
  src/cli/resolve.cpp 257e29034abf32491511f9a4e476b6859714829d 
  src/local/local.cpp f8599e7378e9a0065bbd01ad8f23f11debb30c91 
  src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
  src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
  src/master/main.cpp 7c1656bcc266f6c94cb4befad37fa813a218b2fa 
  src/master/master.hpp 124d43931a5c8a00ee0aaa604feb1761795209f2 
  src/master/master.cpp d0380db3b90a9166607445f8dd50cc63d547228e 
  src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
  src/scheduler/scheduler.cpp 35f479483740baed3f7bdbf45bf1d5bba3b9febc 
  src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 
  src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
  src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
  src/tests/cluster.cpp e5796d3cc17f814bec8f02dccf40515b65cfea91 
  src/tests/fault_tolerance_tests.cpp 349669d6aa0ead63b2ebcfcc2f769c99a6db0192 
  src/tests/master_allocator_tests.cpp cba7c36471f93b678d94e1da0251a28a893696b1 
  src/tests/master_authorization_tests.cpp 
6a4de4709960d7ca505e99396e14a1bb51d6902d 
  src/tests/master_contender_detector_tests.cpp 
255ab8119a04b55bb4f1b61dee19c4be64499376 
  src/tests/master_slave_reconciliation_tests.cpp 
d41178eb41df519073fc0890c5716bbc9fed6ad2 
  src/tests/master_tests.cpp e8c39e775d6ca218ce74cfc6bb50c7576d73e90e 
  src/tests/mesos.hpp 908a53a146208a6d41c0bda5208415e39e2eae05 
  src/tests/mesos.cpp 7cca4ed4753fa365b209d1d22f0df4650b19bc6a 
  src/tests/oversubscription_tests.cpp e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
  src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
  src/tests/persistent_volume_tests.cpp 
e9215de2e073025f67cdc73e8a8de38cf030671f 
  src/tests/reconciliation_tests.cpp e8f3f29836652d20a6ee1bb5231a15e71eb76990 
  src/tests/reservation_tests.cpp c88091635949037fcfc6be504043764f9af35f79 
  src/tests/scheduler_event_call_tests.cpp 
8c02ceeb3ec1783cb2f63f100700508e70f586e4 
  src/tests/scheduler_http_api_tests.cpp 
dfb0f51fec67a3951e396eab28eedb0dbf9493ae 
  src/tests/scheduler_tests.cpp fa42fb42f2d18060a867ade547cebbdcaead07d4 
  src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 
  src/tests/slave_tests.cpp 124e9587180f2a55e659d966d1c9060234c19457 

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


Testing
---

See https://reviews.apache.org/r/44289/.


Thanks,

Anurag Singh

Re: Review Request 44544: Moved contender and detector definitions into separate directories.

[Anurag Singh]

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

(Updated March 15, 2016, 6:17 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Moved contender and detector definitions into separate directories.


Diffs (updated)
-

  src/master/contenders/contender.hpp PRE-CREATION 
  src/master/contenders/contender.cpp PRE-CREATION 
  src/master/detectors/detector.hpp PRE-CREATION 
  src/master/detectors/detector.cpp PRE-CREATION 

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


Testing
---


Thanks,

Anurag Singh

Re: Review Request 44543: Removed unnecessary MasterContender and MasterDetector definitions.

[Anurag Singh]

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

(Updated March 15, 2016, 6:17 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

MasterContender and MasterDetector are now defined in
include/mesos/master/contender.hpp and detector.hpp.


Diffs (updated)
-

  src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
  src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 

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


Testing
---


Thanks,

Anurag Singh

Re: Review Request 44546: Moved functions in promises to a common header file.

[Anurag Singh]

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

(Updated March 15, 2016, 6:18 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Moved functions in promises to a common header file.


Diffs (updated)
-

  src/master/detectors/standalone.cpp PRE-CREATION 
  src/master/detectors/zookeeper.cpp 9274435802d6292b183be48f42b43999476e016e 

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


Testing
---


Thanks,

Anurag Singh

Re: Review Request 44670: Added master_detector and master_contender flags.

[Anurag Singh]

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

(Updated March 15, 2016, 6:18 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

The master_detector and master_contender flags allow modules to be
used for specifying the MasterContender and MasterDetector
implementations to use.


Diffs (updated)
-

  src/master/flags.hpp f8d2cc4c6c8dab00e34ca737dbcb5b9ca3870d6d 
  src/master/flags.cpp e6fea6421ea1a16b9cd78b0e42b830829b95ad61 
  src/master/main.cpp 7c1656bcc266f6c94cb4befad37fa813a218b2fa 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp 5bacf109bcd9c9eae1b7ec3c32095c72899ebdbd 
  src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 

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


Testing
---

In addition to all unit tests passing, we are currently using this 
functionality in our environment with a custom consensus stack. In our world, 
we have a C++ plugin that calls out to an HTTP REST service (implemented in 
Java/Scala, not that it matters).


Thanks,

Anurag Singh

Re: Review Request 44669: Added createFromModule methods to MasterContender and MasterDetector.

[Anurag Singh]

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

(Updated March 15, 2016, 6:18 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

The createFromModule will be used to create a MasterContender/Detector
from a module (specified using the --modules flag on the command
line).


Diffs (updated)
-

  src/master/contenders/contender.cpp PRE-CREATION 
  src/master/detectors/detector.cpp PRE-CREATION 

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


Testing
---


Thanks,

Anurag Singh

Re: Review Request 44547: Added functions in promises to the collect header.

[Anurag Singh]

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

(Updated March 15, 2016, 6:18 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Added functions in promises to the collect header.


Diffs (updated)
-

  3rdparty/libprocess/include/process/collect.hpp 
5a92b72eb7668494dc832ec446a41b3d673a20cc 

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


Testing
---


Thanks,

Anurag Singh

Re: Review Request 44289: Added support for contender and detector modules.

[Anurag Singh]

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

(Updated March 15, 2016, 6:18 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Added support for contender and detector modules.


Diffs (updated)
-

  include/mesos/module/contender.hpp PRE-CREATION 
  include/mesos/module/detector.hpp PRE-CREATION 
  src/Makefile.am 7ee5a653fc96907021f14ab28f74c0b3ed0649d9 
  src/examples/test_contender_module.cpp PRE-CREATION 
  src/examples/test_detector_module.cpp PRE-CREATION 
  src/local/local.cpp f8599e7378e9a0065bbd01ad8f23f11debb30c91 
  src/master/contenders/zookeeper.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
  src/master/detectors/zookeeper.cpp PRE-CREATION 
  src/module/manager.cpp 8c9aaf7cd00c904daba9994a99df9e1329831c01 
  src/tests/module.hpp 4b32f29f2ce76100433621a5cb6b8cc87c9b38f8 
  src/tests/module.cpp 8cc305c0ef606b07eea39d548d3165a2bb2b042a 

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


Testing
---

In addition to all unit tests passing, we are currently using this 
functionality in our environment with a custom consensus stack. In our world, 
we have a C++ plugin that calls out to an HTTP REST service (implemented in 
Java/Scala, not that it matters).


Thanks,

Anurag Singh