Re: Review Request 33109: Allow setting environment variables in mesos-execute

2015-05-12 Thread Adam B


 On May 11, 2015, 10:58 p.m., Adam B wrote:
  src/cli/execute.cpp, lines 203-204
  https://reviews.apache.org/r/33109/diff/5-6/?file=939687#file939687line203
 
  You can keep this as one statement, just wrap with the '=' at the start 
  of the newline.

Correction... wrap after the '='. (Sorry for the confusion)


- Adam


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


On April 25, 2015, 2:11 a.m., haosdent huang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/33109/
 ---
 
 (Updated April 25, 2015, 2:11 a.m.)
 
 
 Review request for mesos and Adam B.
 
 
 Bugs: MESOS-2023
 https://issues.apache.org/jira/browse/MESOS-2023
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Allow setting environment variables in mesos-execute
 
 
 Diffs
 -
 
   include/mesos/type_utils.hpp cdf5864389a72002b538c263d70bcade2bdffa45 
   src/cli/execute.cpp 84f70dccbc2c5dd43f68105d967f4488c82f582b 
   src/common/parse.hpp 547b32041f39f0ff0c38179b66a32b2239134abc 
 
 Diff: https://reviews.apache.org/r/33109/diff/
 
 
 Testing
 ---
 
 ### Test without env
 
 ```
 $ ./src/mesos-execute  --master=127.0.0.1:5050 --name=test --command='echo $a'
 ```
 
 The log from mesos, we could see the output is empty
 
 ```
 Registered executor on localhostI0423 01:27:37.180140 14061 slave.cpp:1540] 
 Sending queued task 'test' to executor 'test' of framework 
 20150423-012731-16777343-5050-14024-
 Starting task test
 Forked command at 14133
 sh -c 'echo $a'
 
 I0423 01:27:37.183823 14064 slave.cpp:2507] Handling status update 
 TASK_RUNNING (UUID: be8664ab-a4ca-409e-87e4-de3756506c4a) for task test of 
 framework 20150423-012731-16777343-5050-14024- from 
 executor(1)@127.0.0.1:33700
 ```
 
 ### Test with env from command
 
 ```
 $ ./src/mesos-execute  --master=127.0.0.1:5050 --name=test --command='echo 
 $a' --env={\a\: \stdin\}
 ```
 
 The log from mesos, we could see the output is stdin
 
 ```
 Registered executor on localhost
 Starting task test
 sh -c 'echo $a'
 Forked command at 14783
 stdin
 I0423 01:30:41.750128 14063 slave.cpp:2507] Handling status update 
 TASK_RUNNING (UUID: e8b6cf52-9635-4393-94de-b16869e50c91) for task test of 
 framework 20150423-012731-16777343-5050-14024-0001 from 
 executor(1)@127.0.0.1:40051
 ```
 
 ### Test with env from file
 
 ```
 $ cat /tmp/env
 {a: file}
 ```
 
 ```
 $ ./src/mesos-execute  --master=127.0.0.1:5050 --name=test --command='echo 
 $a' --env=file:///tmp/env
 ```
 
 The log from mesos, we could see the output is file
 
 ```
 Registered executor on localhost
 Starting task testsh -c 'echo $a'
 Forked command at 15258
 file
 I0423 01:32:49.817705 14065 slave.cpp:2507] Handling status update 
 TASK_RUNNING (UUID: 9e8945d0-4921-41e8-a8e2-d64ec36eea16) for task test of 
 framework 20150423-012731-16777343-5050-14024-0002 from 
 executor(1)@127.0.0.1:56334
 ```
 
 
 Thanks,
 
 haosdent huang
 




Re: Review Request 33718: Extended documentation on Mesos hooks.

2015-05-12 Thread Adam B

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

Ship it!


Looks great! Sorry I was so slow. Didn't realize it would be so short.


docs/modules.md
https://reviews.apache.org/r/33718/#comment134341

Does it get loaded too by --modules, even if it's not selected by --hooks? 
If so, I would say load it instead of introduce it. If not, introduce 
seems like a perfect term.


- Adam B


On May 8, 2015, 4:19 p.m., Alexander Rukletsov wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/33718/
 ---
 
 (Updated May 8, 2015, 4:19 p.m.)
 
 
 Review request for mesos, Adam B, Kapil Arya, Niklas Nielsen, and Till 
 Toenshoff.
 
 
 Bugs: MESOS-2680
 https://issues.apache.org/jira/browse/MESOS-2680
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Mentions necessary flags and adds a usage example.
 
 
 Diffs
 -
 
   docs/modules.md a8b471541cdfa584eeb89fbe96643f93c712cfd4 
 
 Diff: https://reviews.apache.org/r/33718/diff/
 
 
 Testing
 ---
 
 none: docs update.
 
 @Adam: as a native speaker, do you mind checking the language?
 
 
 Thanks,
 
 Alexander Rukletsov
 




Re: Review Request 33109: Allow setting environment variables in mesos-execute

2015-05-12 Thread haosdent huang

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

(Updated May 12, 2015, 6:24 a.m.)


Review request for mesos and Adam B.


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


Repository: mesos


Description
---

Allow setting environment variables in mesos-execute


Diffs (updated)
-

  .clang-format PRE-CREATION 
  .gitignore-template 934cad71a370d5c64d9a0994f775f39db74f5851 
  3rdparty/libprocess/3rdparty/stout/README.md 
588f7393c43d528834aab8b6a14e4aee281417f3 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
79239d738d0607364f8c3d7addfd54a642bdffc0 
  3rdparty/libprocess/3rdparty/stout/include/stout/format.hpp 
abe1a1d43659bda8e6af4f76c0ae4d1ad4b1c8c5 
  3rdparty/libprocess/3rdparty/stout/include/stout/hashset.hpp 
21b059937f55927644ac3b19e1b5ccb1b45b6237 
  3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 
8784e76d79c7fb8d1a88dda4b0d5cbf6cdb12772 
  3rdparty/libprocess/3rdparty/stout/include/stout/lambda.hpp 
6cbb2ac50dd23d78465e8690112fc01114ada07b 
  3rdparty/libprocess/3rdparty/stout/include/stout/memory.hpp 
dfaef77fa1af857c13e5d8705505dd86f5108578 
  3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 
ea79b501d9ed7b7da9636ce9c9c590738a586993 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/fork.hpp 
d72b4c19573be6f4eebfadef3c36cff350312bbb 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/pstree.hpp 
9426895f2bf3f77a5ba7ec87b97a2cbfe79adbf4 
  3rdparty/libprocess/3rdparty/stout/include/stout/tuple.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
60c0336082caf30b2d7943d85ef3bb7a534648d8 
  3rdparty/libprocess/configure.ac c5106cd09901781ca77d8c02c73919553a085876 
  3rdparty/libprocess/include/Makefile.am 
8aab0593f296c7aae71289f9bd6cf3eb3578a721 
  3rdparty/libprocess/include/process/c++11/defer.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/c++11/deferred.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/c++11/delay.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/c++11/dispatch.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/c++11/executor.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/collect.hpp 
c713c1bbc13ee50ae8c9773ee971a1b565be50f4 
  3rdparty/libprocess/include/process/defer.hpp 
7c04736a3d29bf2c87057efb393ab3f9bdaa10eb 
  3rdparty/libprocess/include/process/deferred.hpp 
3746d692f2c9a42f7865f83b74ed64b847181ffa 
  3rdparty/libprocess/include/process/delay.hpp 
29e353246e270f853ed33c1fb677d35cdfcf41ca 
  3rdparty/libprocess/include/process/dispatch.hpp 
617fd43394074fa3e1f0e656ff175e651137c86c 
  3rdparty/libprocess/include/process/event.hpp 
ad4a8f4f1de1f78f9af89954fd9c038eb2756ce4 
  3rdparty/libprocess/include/process/executor.hpp 
157a1d29fa6e64f823b0ab40ec51889bf8947f60 
  3rdparty/libprocess/include/process/future.hpp 
c22d6c85cd52f7b91e47b6aee3dc4ed62978d245 
  3rdparty/libprocess/include/process/http.hpp 
058fa02eeecdf31023db731734257a924d770079 
  3rdparty/libprocess/include/process/metrics/counter.hpp 
f9cab39907a7c5643576214c5dc00858808d1a56 
  3rdparty/libprocess/include/process/metrics/gauge.hpp 
7d02cd5c2947b382b67fedc4ee912281a0c78b3b 
  3rdparty/libprocess/include/process/metrics/metric.hpp 
a7be2d793b75d6bf1f28ddd8d5360540323228f4 
  3rdparty/libprocess/include/process/metrics/timer.hpp 
b6f9fbd51279a702aa6875587311c06f6b6c5720 
  3rdparty/libprocess/include/process/mutex.hpp 
99dd14f2883658e483807a4323d85bf80e05e7cb 
  3rdparty/libprocess/include/process/owned.hpp 
054111316d6874026ac84496aa3f66fece11f861 
  3rdparty/libprocess/include/process/queue.hpp 
df8efc0752197a3fef2121e4355e128ff22a19db 
  3rdparty/libprocess/include/process/run.hpp 
a0d7286ce8470c2513258a2d6167a4a258a3f7bf 
  3rdparty/libprocess/include/process/shared.hpp 
d80fb7f236ffe12ede40b56668f0e992d41f8aec 
  3rdparty/libprocess/include/process/subprocess.hpp 
37cab7755d2890619b64e1ca09e0b7ad0e72cf76 
  3rdparty/libprocess/include/process/timeseries.hpp 
ec0ac67bed5563bd85694d9ce9230450823f3b49 
  3rdparty/libprocess/src/http.cpp 67983ee4daf0601202c9360fe40ed28e9ef74755 
  3rdparty/libprocess/src/io.cpp 4944e2873ea858401cec592305e92f49cdfcb2a5 
  3rdparty/libprocess/src/libev_poll.cpp 
6191be3bb9f5df80ae21c2d5da8e575f7df00e84 
  3rdparty/libprocess/src/process.cpp 588bd3e40a0b350ceca15bb8f3f78290ba41d173 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d29cd29d8c0544671a09d204ca8ba4f24340e2de 
  3rdparty/libprocess/src/tests/process_tests.cpp 
67e582cc250a9767a389e2bd0cc68985477f3ffb 
  CHANGELOG f8d998dd31018546ae1ec646aeb128b9fb708f8d 
  bootstrap 779b33cdcb88b2417769bb046a17b47cd6042f2d 
  configure.ac fa53bbd416dffc3987f8ec28bd9a8ce2b32d6831 
  docs/clang-format.md 909a253b699fca37ae4ea0f8c9d680c9a5f012ed 
  docs/mesos-c++-style-guide.md 13312f6f4fe1788791479bd768f60df0a8e80e69 
  

Re: Review Request 30339: Use flags.hooks.isSome() before calling hooks.

2015-05-12 Thread Niklas Nielsen


 On March 4, 2015, 4:03 p.m., Niklas Nielsen wrote:
 

Do you want this in? If so, please update the review :)


- Niklas


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


On Jan. 27, 2015, 4:42 p.m., Kapil Arya wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/30339/
 ---
 
 (Updated Jan. 27, 2015, 4:42 p.m.)
 
 
 Review request for mesos, Niklas Nielsen and Till Toenshoff.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Call hook manager only if hooks were specified on the commandline.
 
 
 Diffs
 -
 
   src/master/master.cpp bda8fda9bc2e52ccc1d75e2541e4604989515e13 
   src/slave/slave.hpp 70bd8c1fde4ea09fa54c76aa93424a1adb0309f6 
   src/slave/slave.cpp fca83b3977b95ddda30f9830da10e124b5c605e6 
   src/tests/hook_tests.cpp 44f73effdce2d03627215418007ccbc3263a0c52 
 
 Diff: https://reviews.apache.org/r/30339/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Kapil Arya
 




Re: Review Request 29748: Added tests for dynamic reservation.

2015-05-12 Thread Michael Park

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

(Updated May 12, 2015, 5:13 p.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, and Jie Yu.


Changes
---

Addressed Jie's comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 
  src/tests/reservation_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Michael Park



Re: Review Request 32398: Persisted the reservation state on the slave.

2015-05-12 Thread Michael Park

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

(Updated May 12, 2015, 6:44 p.m.)


Review request for mesos, Alexander Rukletsov and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

* Added `isDynamicallyReserved(resource)` which returns true if the resource is 
a dynamic reservation.
* Added the `isDynamicallyReserved` condition onto `needCheckpointing`.
* Updated the `applyCheckpointedResources` to consider dynamic reservations.
* Added tests.


Diffs (updated)
-

  include/mesos/resources.hpp 4c036d36e0e8ab3852786dd32b2d983d45891624 
  src/common/resources.cpp 235930ff2dbb3ea49a3a0696dc070f2bd56fba4b 
  src/common/resources_utils.cpp fe04d57227fa193d6d11d2f76529c46aea74c6a1 
  src/tests/reservation_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Michael Park



Re: Review Request 33152: Moved the slave shutdown test into slave_tests.cpp.

2015-05-12 Thread Vinod Kone


 On April 17, 2015, 12:01 p.m., Alexander Rojas wrote:
  src/tests/slave_tests.cpp, lines 146-148
  https://reviews.apache.org/r/33152/diff/1/?file=926674#file926674line146
 
  One line break too many.

forgot to publish the updated diff?


- Vinod


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


On April 14, 2015, 1:46 a.m., Ben Mahler wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/33152/
 ---
 
 (Updated April 14, 2015, 1:46 a.m.)
 
 
 Review request for mesos and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 See summary.
 
 
 Diffs
 -
 
   src/tests/fault_tolerance_tests.cpp 
 a637c32f004638a110390b22cf5b626e904097cf 
   src/tests/slave_tests.cpp b826000e0a4221690f956ea51f49ad4c99d5e188 
 
 Diff: https://reviews.apache.org/r/33152/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Ben Mahler
 




Re: Review Request 29748: Added tests for dynamic reservation.

2015-05-12 Thread Michael Park

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

(Updated May 12, 2015, 6:41 p.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, and Jie Yu.


Changes
---

Addressed Jie's comment about redundant tests.

Removed tests:
* `DropReserveNonMatchingRole`
* `DropReserveNonMatchingPrincipal`
* `DropReserveFrameworkMissingPrincipal`

Since they'are already covered in `src/tests/master_validation_tests.cpp`.

Migrated tests:
* `DropReserveAsStaticReservation`
* `DropUnreserveStaticReservation`

to `src/tests/master_validation_tests.cpp` as
* `TEST_F(ReserveOperationValidationTest, StaticReservation)`
* `TEST_F(UnreserveOperationValidationTest, StaticReservation)`


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 
  src/tests/master_validation_tests.cpp 
1366bcd229f4cb62df7d181c42dae4152435bb14 
  src/tests/reservation_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Michael Park



Re: Review Request 32398: Persisted the reservation state on the slave.

2015-05-12 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [32139, 32140, 32149, 32150, 29748, 32398]

All tests passed.

- Mesos ReviewBot


On May 12, 2015, 6:44 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/32398/
 ---
 
 (Updated May 12, 2015, 6:44 p.m.)
 
 
 Review request for mesos, Alexander Rukletsov and Jie Yu.
 
 
 Bugs: MESOS-2491
 https://issues.apache.org/jira/browse/MESOS-2491
 
 
 Repository: mesos
 
 
 Description
 ---
 
 * Added `isDynamicallyReserved(resource)` which returns true if the resource 
 is a dynamic reservation.
 * Added the `isDynamicallyReserved` condition onto `needCheckpointing`.
 * Updated the `applyCheckpointedResources` to consider dynamic reservations.
 * Added tests.
 
 
 Diffs
 -
 
   include/mesos/resources.hpp 4c036d36e0e8ab3852786dd32b2d983d45891624 
   src/common/resources.cpp 235930ff2dbb3ea49a3a0696dc070f2bd56fba4b 
   src/common/resources_utils.cpp fe04d57227fa193d6d11d2f76529c46aea74c6a1 
   src/tests/reservation_tests.cpp PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/32398/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Michael Park
 




Re: Review Request 33271: Update style guide to disallow capturing temporaries by reference.

2015-05-12 Thread Michael Park

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

Ship it!


Left a minor suggestion but otherwise it looks good to me!


docs/mesos-c++-style-guide.md
https://reviews.apache.org/r/33271/#comment134451

Perhaps `s/temporaries/*temporaries*/` to emphasize that it's only 
temporaries that we disallow?


- Michael Park


On April 29, 2015, 9:34 p.m., Joris Van Remoortere wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/33271/
 ---
 
 (Updated April 29, 2015, 9:34 p.m.)
 
 
 Review request for mesos, Bernd Mathiske, Cody Maloney, Joerg Schad, Michael 
 Park, and Till Toenshoff.
 
 
 Bugs: MESOS-2629
 https://issues.apache.org/jira/browse/MESOS-2629
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Follow up from r32630.
 
 
 Diffs
 -
 
   docs/mesos-c++-style-guide.md 451d2475e72e07e0a5deec4e62af9d6746d94bc0 
 
 Diff: https://reviews.apache.org/r/33271/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Joris Van Remoortere
 




Re: Review Request 33876: Added usages() to resource monitor

2015-05-12 Thread Niklas Nielsen

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

(Updated May 12, 2015, 1:55 p.m.)


Review request for mesos, Jie Yu and Vinod Kone.


Changes
---

Adressed Jie's comments.


Repository: mesos


Description
---

Added usages() to resource monitor to enable internal components tapping into 
resource statistics.


Diffs (updated)
-

  src/slave/monitor.hpp 69c60a10187f8ea617c6be9738b28e8103e0ed27 
  src/slave/monitor.cpp 398af010564e999b46e38560ba1e652261a9420c 

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


Testing
---

make check


Thanks,

Niklas Nielsen



Re: Review Request 33241: docs: Add documentation on observability metrics.

2015-05-12 Thread Ricardo Cervera-Navarro


 On April 30, 2015, 4:09 p.m., Joe Smith wrote:
  docs/metrics.md, line 221
  https://reviews.apache.org/r/33241/diff/3/?file=938848#file938848line221
 
  master/slave_shutdowns_completed
  
  These are slaves which were not heard from despite the slave-removal 
  rate limit, and have been removed from the Master's slave registry. This is 
  essentially the slave failing its health check.

Added


 On April 30, 2015, 4:09 p.m., Joe Smith wrote:
  docs/metrics.md, line 225
  https://reviews.apache.org/r/33241/diff/3/?file=938848#file938848line225
 
  These are slaves that are able to cleanly re-join the cluster and 
  connect back to the master after the master is disconnected.

Added


 On April 30, 2015, 4:09 p.m., Joe Smith wrote:
  docs/metrics.md, line 232
  https://reviews.apache.org/r/33241/diff/3/?file=938848#file938848line232
 
  These are slaves which are being removed for various reasons, including 
  Maintenance. This metric will be broken out for 
  https://issues.apache.org/jira/browse/MESOS-2485

Added in here for now


 On April 30, 2015, 4:09 p.m., Joe Smith wrote:
  docs/metrics.md, line 246
  https://reviews.apache.org/r/33241/diff/3/?file=938848#file938848line246
 
  Slaves which have failed their health check and are scheduled to be 
  removed. They will not be immediately removed due to the Slave Removal 
  Rate-Limit, but master/slave_shutdowns_completed will start increasing as 
  they do get removed.

Added


 On April 30, 2015, 4:09 p.m., Joe Smith wrote:
  docs/metrics.md, line 255
  https://reviews.apache.org/r/33241/diff/3/?file=938848#file938848line255
 
  This happens when the slave removal rate limit allows for a slave to 
  reconnect and send a PONG to the master before being removed.

Added


- Ricardo


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


On May 12, 2015, 1:43 p.m., Ricardo Cervera-Navarro wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/33241/
 ---
 
 (Updated May 12, 2015, 1:43 p.m.)
 
 
 Review request for mesos.
 
 
 Bugs: MESOS-2621
 https://issues.apache.org/jira/browse/MESOS-2621
 
 
 Repository: mesos
 
 
 Description
 ---
 
 docs: Add documentation on observability metrics.
 
 
 Diffs
 -
 
   docs/home.md 6ab61f85aa7d62e0f19267b886dffb4e0aa826ea 
   docs/metrics.md PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/33241/diff/
 
 
 Testing
 ---
 
 - Previewed in markdown editor.
 - Replaced page content using the Element Inspector in Chrome to ensure that 
 the tables look good.
 
 
 File Attachments
 
 
 mesosmetrics2.webarchive
   
 https://reviews.apache.org/media/uploaded/files/2015/04/27/b89946c8-f709-4bca-b610-affd584c62f3__mesosmetrics2.webarchive
 
 
 Thanks,
 
 Ricardo Cervera-Navarro
 




Re: Review Request 33241: docs: Add documentation on observability metrics.

2015-05-12 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [33241]

All tests passed.

- Mesos ReviewBot


On May 12, 2015, 8:43 p.m., Ricardo Cervera-Navarro wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/33241/
 ---
 
 (Updated May 12, 2015, 8:43 p.m.)
 
 
 Review request for mesos.
 
 
 Bugs: MESOS-2621
 https://issues.apache.org/jira/browse/MESOS-2621
 
 
 Repository: mesos
 
 
 Description
 ---
 
 docs: Add documentation on observability metrics.
 
 
 Diffs
 -
 
   docs/home.md 6ab61f85aa7d62e0f19267b886dffb4e0aa826ea 
   docs/metrics.md PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/33241/diff/
 
 
 Testing
 ---
 
 - Previewed in markdown editor.
 - Replaced page content using the Element Inspector in Chrome to ensure that 
 the tables look good.
 
 
 File Attachments
 
 
 mesosmetrics2.webarchive
   
 https://reviews.apache.org/media/uploaded/files/2015/04/27/b89946c8-f709-4bca-b610-affd584c62f3__mesosmetrics2.webarchive
 
 
 Thanks,
 
 Ricardo Cervera-Navarro
 




Re: Review Request 30609: Added a function that reports file size, not following links.

2015-05-12 Thread Bernd Mathiske

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

(Updated May 12, 2015, 2:23 p.m.)


Review request for mesos, Benjamin Hindman, Till Toenshoff, and Timothy Chen.


Changes
---

Fixed issues. Thanks, Tim!!


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


Repository: mesos


Description
---

This returns a file's size (on UNIXes as reported by lstat(), not stat()). It 
is desired that in case of a link, the size of the link, not the size of the 
referenced file, is returned.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/stat.hpp 
270c4c848fc0460dcdb9a90823281d735f4550c2 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
343f95be7f316170b37c9358627f3c2090f0e29e 

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


Testing
---

Wrote a simple test that creates a file and tests its size, and also checks if 
a non-existing file yields an error.


Thanks,

Bernd Mathiske



Re: Review Request 29748: Added tests for dynamic reservation.

2015-05-12 Thread Michael Park


 On May 11, 2015, 11:32 p.m., Jie Yu wrote:
  src/tests/reservation_tests.cpp, line 365
  https://reviews.apache.org/r/29748/diff/17/?file=954334#file954334line365
 
  YOu do need a snake_case checker:)

Sigh... Sorry :( Fixed.


 On May 11, 2015, 11:32 p.m., Jie Yu wrote:
  src/tests/reservation_tests.cpp, lines 399-400
  https://reviews.apache.org/r/29748/diff/17/?file=954334#file954334line399
 
  Hum, have the following 3 tests already been captured in 
  ReserveOperationValidationTest?

Yes, they're captured in the following test cases. What would you like to see 
done here?

```cpp
TEST_F(ReserveOperationValidationTest, NonMatchingRole)
TEST_F(ReserveOperationValidationTest, NonMatchingPrincipal)
TEST_F(ReserveOperationValidationTest, FrameworkMissingPrincipal)
```


 On May 11, 2015, 11:32 p.m., Jie Yu wrote:
  src/tests/reservation_tests.cpp, line 944
  https://reviews.apache.org/r/29748/diff/17/?file=954334#file954334line944
 
  2 lines apart between two top level declarations. Please make sure this 
  is the case in this file.

Fixed.


- Michael


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


On May 12, 2015, 5:13 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29748/
 ---
 
 (Updated May 12, 2015, 5:13 p.m.)
 
 
 Review request for mesos, Alexander Rukletsov, Ben Mahler, and Jie Yu.
 
 
 Bugs: MESOS-2489
 https://issues.apache.org/jira/browse/MESOS-2489
 
 
 Repository: mesos
 
 
 Description
 ---
 
 See summary.
 
 
 Diffs
 -
 
   src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 
   src/tests/reservation_tests.cpp PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/29748/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Michael Park
 




Re: Review Request 31444: Support chrooting in MesosContainerizer launch helper.

2015-05-12 Thread Ian Downes

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

(Updated May 12, 2015, 5:22 p.m.)


Review request for mesos, Chi Zhang, Dominic Hamon, Jay Buffington, Jie Yu, and 
James Peach.


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


Repository: mesos


Description
---

Optionally take a path that the launch helper should chroot to before exec'ing 
the executor. It is assumed that the work directory is mounted to the 
appropriate location under the chroot. In particular, the path to the executor 
must be relative to the chroot.

Configuration that should be private to the chroot is done during the launch, 
e.g. mounting proc and statically configuring basic devices. It is assumed that 
other configuration, e.g., preparing the image, mounting in volumes or 
persistent resources, is done by the caller.

Mounts can be made to the chroot (e.g., updating the volumes or persistent 
resources) and they will propagate in to the container but mounts made inside 
the container will not propagate out to the host.

It currently assumes that at least {{chroot}}/tmp is writeable and that mount 
points {{chroot}}/{tmp,dev,proc,sys} exist in the chroot.

This is specific to Linux.


Diffs (updated)
-

  src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 
  src/linux/fs.cpp 1c9cf3f2ffead37148e4f6a81cefdbb97f679b09 
  src/slave/containerizer/mesos/launch.hpp 
7c8b535746b5ce9add00afef86fdb6faefb5620e 
  src/slave/containerizer/mesos/launch.cpp 
2f2d60e2011f60ec711d3b29fd2c157e30c83c34 
  src/tests/launch_tests.cpp PRE-CREATION 

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


Testing
---

Manual testing only so far. This is harder to automate because we need a 
self-contained chroot to execute something in... Suggestions welcome.


Thanks,

Ian Downes



Re: Review Request 32891: Support for entering and configuring a Linux chroot.

2015-05-12 Thread Ian Downes

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

(Updated May 12, 2015, 5:22 p.m.)


Review request for mesos, Chi Zhang, Jay Buffington, Jie Yu, and James Peach.


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


Repository: mesos


Description
---

Support for entering and configuring a Linux chroot.


Diffs (updated)
-

  src/linux/fs.hpp d7832a4b3761c48be6c1ccef58a30ee31c70dc1b 
  src/linux/fs.cpp 1c9cf3f2ffead37148e4f6a81cefdbb97f679b09 

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


Testing
---


Thanks,

Ian Downes



Re: Review Request 34016: Change the type of signaledWrapper to unique_ptr

2015-05-12 Thread haosdent huang

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

(Updated May 13, 2015, 2:21 a.m.)


Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Change the type of signaledWrapper to unique_ptr


Diffs (updated)
-

  src/slave/slave.cpp bf290bfd7d9a59ce7197ce34cbd8cf42e7dd17a3 

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


Testing
---

make check


Thanks,

haosdent huang



Re: Review Request 34128: Enable different IP/Port for external access.

2015-05-12 Thread Timothy Chen

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



3rdparty/libprocess/src/process.cpp
https://reviews.apache.org/r/34128/#comment134536

I think we should at least prefix the env variable, like 
LIBPROCESS_PUBLIC_IP and LIBPROCESS_PUBLIC_PORT so we don't clash with other 
env variables from other projects.



3rdparty/libprocess/src/process.cpp
https://reviews.apache.org/r/34128/#comment134539

I think overriding __address__ static is dangerous, since we use this 
variable in other places for other purposes, such as checking if we are 
communicating to a remote address, etc.

I suggest we create a local address struct just for binding for listening 
to public traffic.


- Timothy Chen


On May 12, 2015, 11:23 p.m., Anindya Sinha wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34128/
 ---
 
 (Updated May 12, 2015, 11:23 p.m.)
 
 
 Review request for mesos.
 
 
 Bugs: MESOS-809
 https://issues.apache.org/jira/browse/MESOS-809
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Expose environment variables PUBLIC_IP and PUBLIC_PORT as the IP and
 port which libprocess would advertise (if set). If not set, it defaults to
 the IP and port on which it binded to.
 
 
 Diffs
 -
 
   3rdparty/libprocess/src/process.cpp 
 588bd3e40a0b350ceca15bb8f3f78290ba41d173 
 
 Diff: https://reviews.apache.org/r/34128/diff/
 
 
 Testing
 ---
 
 Testing:
   make test
 
 
 Thanks,
 
 Anindya Sinha
 




Review Request 34135: Add filesystem/ isolators for persistent volumes.

2015-05-12 Thread Ian Downes

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

Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Repository: mesos


Description
---

Moved code from Mesos Containerizer to filesystem isolators
 - filesystem/posix (symlinks, doesn't support container rootfs)
 - filesystem/linux (bind mounts, does support container rootfs)

The filesystem/posix isolator will be automatically included if no filesystem/ 
isolator is specified.


Diffs
-

  src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 
  src/slave/containerizer/isolators/filesystem/linux.hpp PRE-CREATION 
  src/slave/containerizer/isolators/filesystem/linux.cpp PRE-CREATION 
  src/slave/containerizer/isolators/filesystem/posix.hpp PRE-CREATION 
  src/slave/containerizer/isolators/filesystem/posix.cpp PRE-CREATION 
  src/slave/containerizer/linux_launcher.cpp 
b9e22e3c70bed0c29e2ca8632411789d33f779a8 
  src/slave/containerizer/mesos/containerizer.cpp 
b644b9c74bc23cf78c0a53284544be6cdaef2f8a 

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


Testing
---

existing persistent volumes tests.


Thanks,

Ian Downes



Review Request 34140: Appc image store

2015-05-12 Thread Ian Downes

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

Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Repository: mesos


Description
---

Images are fetched into the store (after discovery). Stored images are
currently kept indefinitely.


Diffs
-

  src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 
  src/slave/containerizer/provisioners/appc/store.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/appc/store.cpp PRE-CREATION 
  src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
  src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 

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


Testing
---


Thanks,

Ian Downes



Review Request 34141: AppC provsioning backend.

2015-05-12 Thread Ian Downes

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

Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Repository: mesos


Description
---

Simple copy backend that forms the rootfs by copying each layer.


Diffs
-

  src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 
  src/slave/containerizer/provisioners/appc/backend.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/appc/backend.cpp PRE-CREATION 
  src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
  src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 

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


Testing
---


Thanks,

Ian Downes



Review Request 34137: Add support for container image provisioners.

2015-05-12 Thread Ian Downes

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

Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Repository: mesos


Description
---

The MesosContainerizer can optionally provision a root filesystem for the 
containers.


Diffs
-

  src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 
  src/slave/containerizer/mesos/containerizer.hpp 
3e18617b0dbac58176bfd41dc550898eb6a4a79e 
  src/slave/containerizer/mesos/containerizer.cpp 
b644b9c74bc23cf78c0a53284544be6cdaef2f8a 
  src/slave/containerizer/provisioner.hpp PRE-CREATION 
  src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
  src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 

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


Testing
---


Thanks,

Ian Downes



Re: Review Request 33155: Added tests for slave removal metrics.

2015-05-12 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [33152, 33153, 33154, 33155]

All tests passed.

- Mesos ReviewBot


On May 12, 2015, 11:32 p.m., Ben Mahler wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/33155/
 ---
 
 (Updated May 12, 2015, 11:32 p.m.)
 
 
 Review request for mesos and Vinod Kone.
 
 
 Bugs: MESOS-2485
 https://issues.apache.org/jira/browse/MESOS-2485
 
 
 Repository: mesos
 
 
 Description
 ---
 
 I updated the existing tests to validate the metrics. However, it blocks the 
 tests due to the slave metrics never getting satisfied. Added a TODO.
 
 
 Diffs
 -
 
   src/tests/master_tests.cpp 75ffadae64ece4e3ff53abeefa5f6e8e3690d402 
   src/tests/partition_tests.cpp 1018e479a77a6b533f2dd392fedbdccb80e3ac1f 
   src/tests/slave_tests.cpp 04e79ec061e767a82d40242be7884967942c50b6 
 
 Diff: https://reviews.apache.org/r/33155/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Ben Mahler
 




Re: Review Request 34016: Change the type of signaledWrapper to unique_ptr

2015-05-12 Thread haosdent huang


 On May 11, 2015, 10:19 p.m., Ben Mahler wrote:
  src/slave/slave.cpp, line 155
  https://reviews.apache.org/r/34016/diff/1/?file=954524#file954524line155
 
  unique_ptr is not a POD, so this will still try to run the destructor 
  of the function during exit of the program.
  
  Can you just use a raw pointer here and leak it? (Not ideal but we use 
  this in many places)
 
 haosdent huang wrote:
 Yes, I use raw pointer before. But got this error
 ```
 ../../src/slave/slave.cpp:519:19: error: cannot convert 
 ‘process::_Deferredstd::_Bindstd::_Mem_fnvoid (std::functionvoid(int, 
 int)::*)(int, int)const(std::functionvoid(int, int), 
 std::_Placeholder1, std::_Placeholder2) ’ to ‘std::functionvoid(int, 
 int)*’ in assignment   signaledWrapper = defer(self(), Slave::signaled, 
 lambda::_1, lambda::_2);
 ```
 So I use unique_ptr. Let me remove unique_ptr and add force convert here.
 
 haosdent huang wrote:
 But still have this error.
 ```
 ../../src/slave/slave.cpp:520:55: error: taking address of temporary 
 [-fpermissive]
self(), Slave::signaled, lambda::_1, lambda::_2);
^
 ```
 So I use `new` here?
 
 Ben Mahler wrote:
 yes, you need a copy on the heap, so `new function...(defer(...));` 
 should work

But this heap object would never release util Slave exit, is it OK?


- haosdent


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


On May 12, 2015, 1:26 a.m., haosdent huang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34016/
 ---
 
 (Updated May 12, 2015, 1:26 a.m.)
 
 
 Review request for mesos and Ben Mahler.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Change the type of signaledWrapper to unique_ptr
 
 
 Diffs
 -
 
   src/slave/slave.cpp bf290bfd7d9a59ce7197ce34cbd8cf42e7dd17a3 
 
 Diff: https://reviews.apache.org/r/34016/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 haosdent huang
 




Review Request 34134: Add container rootfs to Isolator::prepare().

2015-05-12 Thread Ian Downes

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

Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Repository: mesos


Description
---

Add container rootfs to Isolator::prepare().


Diffs
-

  include/mesos/slave/isolator.hpp b4fe1ffba9ec4e5905f98954bec884a6f5623aeb 
  src/slave/containerizer/isolator.cpp d99f47e3920c3edf5261d49f1cb77e96df01ab2c 
  src/slave/containerizer/isolators/cgroups/cpushare.hpp 
ff4a9dbdb1b655e71bf87dcee8fe62433d396f52 
  src/slave/containerizer/isolators/cgroups/cpushare.cpp 
6a5b2b5c6e2844fe1a10815956569194b6f56681 
  src/slave/containerizer/isolators/cgroups/mem.hpp 
c8e1ba15de399deb9c53c03fbc08e2b2cb514b36 
  src/slave/containerizer/isolators/cgroups/mem.cpp 
2c218b2b83cf42f54dbc7ec4c2ba8960b6e194de 
  src/slave/containerizer/isolators/cgroups/perf_event.hpp 
6679719bff24a8fed1aaa62cbef90dfd5a9ee9c0 
  src/slave/containerizer/isolators/cgroups/perf_event.cpp 
37967b5db654bc1c377b8bd08fa1956bf8cc8110 
  src/slave/containerizer/isolators/filesystem/shared.hpp 
68ed54d8ef9387faa0d968d49f4ebdc5b3e647b4 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
101d6da66e750263fa288b43df2bbf27aaf3adf8 
  src/slave/containerizer/isolators/namespaces/pid.hpp 
187cbe802523973bf28d4e39aa99852e78a9d611 
  src/slave/containerizer/isolators/namespaces/pid.cpp 
b426d0806ddf8284a2d9bfe4bb58e6a5e1305ab6 
  src/slave/containerizer/isolators/posix.hpp 
9b43f023708d0d40821da42dc8ec9b046e4b10cc 
  src/slave/containerizer/isolators/posix/disk.hpp 
f3f79d8bf83075db06865ea1aca7e8eb3bf872e6 
  src/slave/containerizer/isolators/posix/disk.cpp 
caf81e8bc30c0877f4d87c12a0f1073b9553d454 
  src/slave/containerizer/mesos/containerizer.cpp 
b644b9c74bc23cf78c0a53284544be6cdaef2f8a 
  src/tests/containerizer_tests.cpp 3c9f9585f40a6d0d257152932e54d4d12ad067fe 
  src/tests/isolator.hpp 7db13cdce7226103366e1ff7ca873294017ccead 
  src/tests/isolator_tests.cpp 24c71b7906a92bdc84a38e88d6084ab09e3cf2ab 

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


Testing
---


Thanks,

Ian Downes



Re: Review Request 34129: Add 2 optional args public_ip and public_port for libprocess to advertise.

2015-05-12 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [34129]

All tests passed.

- Mesos ReviewBot


On May 12, 2015, 11:30 p.m., Anindya Sinha wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34129/
 ---
 
 (Updated May 12, 2015, 11:30 p.m.)
 
 
 Review request for mesos and Cosmin Lehene.
 
 
 Bugs: MESOS-809
 https://issues.apache.org/jira/browse/MESOS-809
 
 
 Repository: mesos
 
 
 Description
 ---
 
 If set, these IP/Port shall be advertised by libprocess (although bind is
 not done on this IP/Port). If not set, libprocess advertises the IP/Port
 on which bind was done.
 
 Command line arguments added:
 public_ip: Public IP address to reach mesos. Defaults to the command line 
 argument `ip`.
 public_port: Public port to reach mesos. Defaults to the command line 
 argument `port`.
 
 
 Diffs
 -
 
   docs/configuration.md 54c4e31ed6dfed3c23d492c19a301ce119a0519b 
   src/master/main.cpp 18f8c3178459da0cbf23a1817ec49cd9d3998bfd 
 
 Diff: https://reviews.apache.org/r/34129/diff/
 
 
 Testing
 ---
 
 Testing:
   make test
 
 
 Thanks,
 
 Anindya Sinha
 




Review Request 34152: Master flag validation now supports zookeeper

2015-05-12 Thread Tom Arnfeld

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

Review request for mesos.


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


Repository: mesos


Description
---

The mesos-execute cli tool does custom validation on the --master flag
to improve user error handling, but currently does not support ZooKeeper.


Diffs
-

  src/cli/execute.cpp dbd19e67f56a150f54180ad13e6402842eb68e17 

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


Testing
---

Compiled and tested locally on a live cluster, with both ZooKeeper and 
standalone host:port.


Thanks,

Tom Arnfeld



Re: Review Request 30643: Optionally specify executor for mesos execute.

2015-05-12 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [30643]

All tests passed.

- Mesos ReviewBot


On May 13, 2015, 12:22 a.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/30643/
 ---
 
 (Updated May 13, 2015, 12:22 a.m.)
 
 
 Review request for mesos, Timothy Chen and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Provided executor will be used to run the task rather than deferring to the 
 slave to determine an executor to use (currently the local included command 
 executor).
 
 This is helpful for testing containers with filesystem isolation as you can 
 fetch an executor into the work directory rather than using the command 
 executor on the host filesystem. You can, of course, still use the command 
 executor, but it will be copied/fetched.
 
 +tnachen - can you please verify I've preserved the override functionality in 
 the command executor? Is that still required?
 
 
 Diffs
 -
 
   src/cli/execute.cpp dbd19e67f56a150f54180ad13e6402842eb68e17 
   src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 
   src/launcher/executor.cpp de6f1b104a765a8e53934154e78872b03695b24c 
 
 Diff: https://reviews.apache.org/r/30643/diff/
 
 
 Testing
 ---
 
 make check
 
 manually used 'mesos execute' and specified command executor, both directly 
 as the value and fetching from file:///. Tested enviroment variables too.
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 33746: Improve logging of clone flags for linux_launcher.

2015-05-12 Thread Ian Downes

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

(Updated May 12, 2015, 5:23 p.m.)


Review request for mesos, Chi Zhang and Jie Yu.


Repository: mesos


Description
---

Improve logging of clone flags for linux_launcher.


Diffs (updated)
-

  src/linux/ns.hpp 87ff82c2efdf26f38b81f8440640bf04def05931 
  src/slave/containerizer/linux_launcher.cpp 
b9e22e3c70bed0c29e2ca8632411789d33f779a8 

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


Testing
---

Example output:

I0501 05:13:25.414758  7838 linux_launcher.cpp:212] Cloning child process with 
flags = CLONE_NEWNS | CLONE_NEWPID


Thanks,

Ian Downes



Re: Review Request 30643: Optionally specify executor for mesos execute.

2015-05-12 Thread Ian Downes

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

(Updated May 12, 2015, 5:22 p.m.)


Review request for mesos, Timothy Chen and Vinod Kone.


Repository: mesos


Description
---

Provided executor will be used to run the task rather than deferring to the 
slave to determine an executor to use (currently the local included command 
executor).

This is helpful for testing containers with filesystem isolation as you can 
fetch an executor into the work directory rather than using the command 
executor on the host filesystem. You can, of course, still use the command 
executor, but it will be copied/fetched.

+tnachen - can you please verify I've preserved the override functionality in 
the command executor? Is that still required?


Diffs (updated)
-

  src/cli/execute.cpp dbd19e67f56a150f54180ad13e6402842eb68e17 
  src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 
  src/launcher/executor.cpp de6f1b104a765a8e53934154e78872b03695b24c 

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


Testing
---

make check

manually used 'mesos execute' and specified command executor, both directly as 
the value and fetching from file:///. Tested enviroment variables too.


Thanks,

Ian Downes



Re: Review Request 33919: Integrated resources estimator with the slave.

2015-05-12 Thread Jie Yu


 On May 7, 2015, 6:16 p.m., Niklas Nielsen wrote:
  src/messages/messages.proto, line 339
  https://reviews.apache.org/r/33919/diff/1/?file=951655#file951655line339
 
  Is the slave id going to be implicit from the sender PID in the master?
  Also, do we need a time stamp?

Yeah, we need a slave_id. Added.

Not sure about the timestamp yet, will add it later once it's needed.


 On May 7, 2015, 6:16 p.m., Niklas Nielsen wrote:
  src/slave/flags.cpp, line 442
  https://reviews.apache.org/r/33919/diff/1/?file=951657#file951657line442
 
  s/allowed/enabled/?
  
  Should we also add a comment about selecting estimator and qos 
  controller?

changed that to --resource_estimator per BenM's comments.


- Jie


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


On May 13, 2015, 12:37 a.m., Jie Yu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/33919/
 ---
 
 (Updated May 13, 2015, 12:37 a.m.)
 
 
 Review request for mesos, Ben Mahler, Joris Van Remoortere, Niklas Nielsen, 
 and Vinod Kone.
 
 
 Bugs: MESOS-2653
 https://issues.apache.org/jira/browse/MESOS-2653
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Integrated resources estimator with the slave.
 
 This patch hooks the resources estimator with the slave. Slave will simply 
 forward the estimation from the resources estimator to the master.
 
 
 Diffs
 -
 
   src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 
   src/local/local.cpp dda25ab348c6430360c4b88e1d93dc70d14738d2 
   src/messages/messages.proto 98d859f3db6013a2e155d838f590a0cde6dc5ed5 
   src/slave/constants.hpp fd1c1aba0aa62372ab399bee5709ce81b8e92cec 
   src/slave/constants.cpp 2a99b1105af0e2d62b956fa96988f477937a46bd 
   src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
   src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 
   src/slave/main.cpp c62d3ab9825bf952071e8e312d383a0cb46547d2 
   src/slave/resource_estimator.hpp PRE-CREATION 
   src/slave/resource_estimator.cpp PRE-CREATION 
   src/slave/slave.hpp adb52b587ae151a51cf21ebd4923aaec1548ef10 
   src/slave/slave.cpp 1b17441d7153f25bd16e6b1d863ddca265480eb5 
   src/tests/cluster.hpp 95061665974a7aca98c374735dd9520278d89043 
   src/tests/mesos.hpp 563b833f8563ee6687ef95143815ed38dff71657 
   src/tests/mesos.cpp bc082e8d91deb2c5dd64bbc3f0a8a50fa7d19264 
   src/tests/oversubscription_tests.cpp PRE-CREATION 
   src/tests/slave_tests.cpp 04e79ec061e767a82d40242be7884967942c50b6 
 
 Diff: https://reviews.apache.org/r/33919/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Jie Yu
 




Re: Review Request 33154: Added reason metrics for slave removals.

2015-05-12 Thread Ben Mahler


 On May 12, 2015, 7:31 p.m., Vinod Kone wrote:
  LGTM. Can you add a comment on removeSlave() on why it takes a const ref of 
  'reason' per Alex's comments?

I've just removed the const reference instead, to take a copy.


- Ben


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


On May 12, 2015, 2:51 a.m., Ben Mahler wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/33154/
 ---
 
 (Updated May 12, 2015, 2:51 a.m.)
 
 
 Review request for mesos and Vinod Kone.
 
 
 Bugs: MESOS-2485
 https://issues.apache.org/jira/browse/MESOS-2485
 
 
 Repository: mesos
 
 
 Description
 ---
 
 See [MESOS-2485](https://issues.apache.org/jira/browse/MESOS-2485).
 
 
 Diffs
 -
 
   include/mesos/mesos.proto db4fc8c001dd68bc3b9ca83650170c4f26db18c7 
   src/master/master.hpp af41216f4a8b81e58916c607a843965aeb4dfb5e 
   src/master/master.cpp ec32cd655c5a4fded0b682b320a6e0867e4e6468 
   src/master/metrics.hpp ee3982e38f23b0dcf92d6b1c39b650c3582c16a2 
   src/master/metrics.cpp 973f0517400786fb16f86914d1d077c88965c9da 
 
 Diff: https://reviews.apache.org/r/33154/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Ben Mahler
 




Re: Review Request 33154: Added reason metrics for slave removals.

2015-05-12 Thread Ben Mahler

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

(Updated May 12, 2015, 11:28 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Removed a misleading const reference.


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


Repository: mesos


Description
---

See [MESOS-2485](https://issues.apache.org/jira/browse/MESOS-2485).


Diffs (updated)
-

  include/mesos/mesos.proto db4fc8c001dd68bc3b9ca83650170c4f26db18c7 
  src/master/master.hpp af41216f4a8b81e58916c607a843965aeb4dfb5e 
  src/master/master.cpp ec32cd655c5a4fded0b682b320a6e0867e4e6468 
  src/master/metrics.hpp ee3982e38f23b0dcf92d6b1c39b650c3582c16a2 
  src/master/metrics.cpp 973f0517400786fb16f86914d1d077c88965c9da 

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


Testing
---

make check


Thanks,

Ben Mahler



Review Request 34128: Enable different IP/Port for external access.

2015-05-12 Thread Anindya Sinha

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

Review request for mesos.


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


Repository: mesos


Description
---

Expose environment variables PUBLIC_IP and PUBLIC_PORT as the IP and
port which libprocess would advertise (if set). If not set, it defaults to
the IP and port on which it binded to.


Diffs
-

  3rdparty/libprocess/src/process.cpp 588bd3e40a0b350ceca15bb8f3f78290ba41d173 

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


Testing
---


Thanks,

Anindya Sinha



Re: Review Request 33155: Added tests for slave removal metrics.

2015-05-12 Thread Ben Mahler

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

(Updated May 12, 2015, 11:32 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Added a missing call to stop the slave; the tests are no longer commented out 
now.

Note that I can't test the newly added replaced slave metric since we cannot 
restart a slave with the same PID currently.


Summary (updated)
-

Added tests for slave removal metrics.


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


Repository: mesos


Description
---

I updated the existing tests to validate the metrics. However, it blocks the 
tests due to the slave metrics never getting satisfied. Added a TODO.


Diffs (updated)
-

  src/tests/master_tests.cpp 75ffadae64ece4e3ff53abeefa5f6e8e3690d402 
  src/tests/partition_tests.cpp 1018e479a77a6b533f2dd392fedbdccb80e3ac1f 
  src/tests/slave_tests.cpp 04e79ec061e767a82d40242be7884967942c50b6 

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


Testing
---

make check


Thanks,

Ben Mahler



Re: Review Request 33152: Moved the slave shutdown test into slave_tests.cpp.

2015-05-12 Thread Ben Mahler

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

(Updated May 12, 2015, 10:22 p.m.)


Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/fault_tolerance_tests.cpp 3a27d82b2e24cc3821c846a0fe0458645b46a496 
  src/tests/slave_tests.cpp 04e79ec061e767a82d40242be7884967942c50b6 

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


Testing
---

make check


Thanks,

Ben Mahler



Re: Review Request 34128: Enable different IP/Port for external access.

2015-05-12 Thread Anindya Sinha

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

(Updated May 12, 2015, 11:23 p.m.)


Review request for mesos.


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


Repository: mesos


Description
---

Expose environment variables PUBLIC_IP and PUBLIC_PORT as the IP and
port which libprocess would advertise (if set). If not set, it defaults to
the IP and port on which it binded to.


Diffs
-

  3rdparty/libprocess/src/process.cpp 588bd3e40a0b350ceca15bb8f3f78290ba41d173 

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


Testing (updated)
---

Testing:
  make test


Thanks,

Anindya Sinha



Review Request 34129: Add 2 optional args public_ip and public_port for libprocess to advertise.

2015-05-12 Thread Anindya Sinha

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

Review request for mesos.


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


Repository: mesos


Description
---

If set, these IP/Port shall be advertised by libprocess (although bind is
not done on this IP/Port). If not set, libprocess advertises the IP/Port
on which bind was done.

Command line arguments added:
public_ip: Public IP address to reach mesos. Defaults to the command line 
argument `ip`.
public_port: Public port to reach mesos. Defaults to the command line argument 
`port`.


Diffs
-

  docs/configuration.md 54c4e31ed6dfed3c23d492c19a301ce119a0519b 
  src/master/main.cpp 18f8c3178459da0cbf23a1817ec49cd9d3998bfd 

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


Testing
---

Testing:
  make test


Thanks,

Anindya Sinha



Re: Review Request 33153: Moved a partition test into partition_tests.cpp.

2015-05-12 Thread Ben Mahler

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

(Updated May 12, 2015, 11:28 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebased, no need for review.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/fault_tolerance_tests.cpp 3a27d82b2e24cc3821c846a0fe0458645b46a496 
  src/tests/partition_tests.cpp 1018e479a77a6b533f2dd392fedbdccb80e3ac1f 

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


Testing
---

make check


Thanks,

Ben Mahler



Re: Review Request 34128: Enable different IP/Port for external access.

2015-05-12 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [34128]

All tests passed.

- Mesos ReviewBot


On May 12, 2015, 11:23 p.m., Anindya Sinha wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34128/
 ---
 
 (Updated May 12, 2015, 11:23 p.m.)
 
 
 Review request for mesos.
 
 
 Bugs: MESOS-809
 https://issues.apache.org/jira/browse/MESOS-809
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Expose environment variables PUBLIC_IP and PUBLIC_PORT as the IP and
 port which libprocess would advertise (if set). If not set, it defaults to
 the IP and port on which it binded to.
 
 
 Diffs
 -
 
   3rdparty/libprocess/src/process.cpp 
 588bd3e40a0b350ceca15bb8f3f78290ba41d173 
 
 Diff: https://reviews.apache.org/r/34128/diff/
 
 
 Testing
 ---
 
 Testing:
   make test
 
 
 Thanks,
 
 Anindya Sinha
 




Re: Review Request 33919: Integrated resources estimator with the slave.

2015-05-12 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [33918, 33919]

All tests passed.

- Mesos ReviewBot


On May 13, 2015, 12:37 a.m., Jie Yu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/33919/
 ---
 
 (Updated May 13, 2015, 12:37 a.m.)
 
 
 Review request for mesos, Ben Mahler, Joris Van Remoortere, Niklas Nielsen, 
 and Vinod Kone.
 
 
 Bugs: MESOS-2653
 https://issues.apache.org/jira/browse/MESOS-2653
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Integrated resources estimator with the slave.
 
 This patch hooks the resources estimator with the slave. Slave will simply 
 forward the estimation from the resources estimator to the master.
 
 
 Diffs
 -
 
   src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 
   src/local/local.cpp dda25ab348c6430360c4b88e1d93dc70d14738d2 
   src/messages/messages.proto 98d859f3db6013a2e155d838f590a0cde6dc5ed5 
   src/slave/constants.hpp fd1c1aba0aa62372ab399bee5709ce81b8e92cec 
   src/slave/constants.cpp 2a99b1105af0e2d62b956fa96988f477937a46bd 
   src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
   src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 
   src/slave/main.cpp c62d3ab9825bf952071e8e312d383a0cb46547d2 
   src/slave/resource_estimator.hpp PRE-CREATION 
   src/slave/resource_estimator.cpp PRE-CREATION 
   src/slave/slave.hpp adb52b587ae151a51cf21ebd4923aaec1548ef10 
   src/slave/slave.cpp 1b17441d7153f25bd16e6b1d863ddca265480eb5 
   src/tests/cluster.hpp 95061665974a7aca98c374735dd9520278d89043 
   src/tests/mesos.hpp 563b833f8563ee6687ef95143815ed38dff71657 
   src/tests/mesos.cpp bc082e8d91deb2c5dd64bbc3f0a8a50fa7d19264 
   src/tests/oversubscription_tests.cpp PRE-CREATION 
   src/tests/slave_tests.cpp 04e79ec061e767a82d40242be7884967942c50b6 
 
 Diff: https://reviews.apache.org/r/33919/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Jie Yu
 




Re: Review Request 31444: Support chrooting in MesosContainerizer launch helper.

2015-05-12 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [32891, 32978, 31444]

All tests passed.

- Mesos ReviewBot


On May 13, 2015, 12:22 a.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/31444/
 ---
 
 (Updated May 13, 2015, 12:22 a.m.)
 
 
 Review request for mesos, Chi Zhang, Dominic Hamon, Jay Buffington, Jie Yu, 
 and James Peach.
 
 
 Bugs: MESOS-2350
 https://issues.apache.org/jira/browse/MESOS-2350
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Optionally take a path that the launch helper should chroot to before 
 exec'ing the executor. It is assumed that the work directory is mounted to 
 the appropriate location under the chroot. In particular, the path to the 
 executor must be relative to the chroot.
 
 Configuration that should be private to the chroot is done during the launch, 
 e.g. mounting proc and statically configuring basic devices. It is assumed 
 that other configuration, e.g., preparing the image, mounting in volumes or 
 persistent resources, is done by the caller.
 
 Mounts can be made to the chroot (e.g., updating the volumes or persistent 
 resources) and they will propagate in to the container but mounts made inside 
 the container will not propagate out to the host.
 
 It currently assumes that at least {{chroot}}/tmp is writeable and that mount 
 points {{chroot}}/{tmp,dev,proc,sys} exist in the chroot.
 
 This is specific to Linux.
 
 
 Diffs
 -
 
   src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 
   src/linux/fs.cpp 1c9cf3f2ffead37148e4f6a81cefdbb97f679b09 
   src/slave/containerizer/mesos/launch.hpp 
 7c8b535746b5ce9add00afef86fdb6faefb5620e 
   src/slave/containerizer/mesos/launch.cpp 
 2f2d60e2011f60ec711d3b29fd2c157e30c83c34 
   src/tests/launch_tests.cpp PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/31444/diff/
 
 
 Testing
 ---
 
 Manual testing only so far. This is harder to automate because we need a 
 self-contained chroot to execute something in... Suggestions welcome.
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 33746: Improve logging of clone flags for linux_launcher.

2015-05-12 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [33746]

All tests passed.

- Mesos ReviewBot


On May 13, 2015, 12:23 a.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/33746/
 ---
 
 (Updated May 13, 2015, 12:23 a.m.)
 
 
 Review request for mesos, Chi Zhang and Jie Yu.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Improve logging of clone flags for linux_launcher.
 
 
 Diffs
 -
 
   src/linux/ns.hpp 87ff82c2efdf26f38b81f8440640bf04def05931 
   src/slave/containerizer/linux_launcher.cpp 
 b9e22e3c70bed0c29e2ca8632411789d33f779a8 
 
 Diff: https://reviews.apache.org/r/33746/diff/
 
 
 Testing
 ---
 
 Example output:
 
 I0501 05:13:25.414758  7838 linux_launcher.cpp:212] Cloning child process 
 with flags = CLONE_NEWNS | CLONE_NEWPID
 
 
 Thanks,
 
 Ian Downes