Re: Review Request 50285: Replaced boost::lexical_cast with numify when paring Value.

2016-07-20 Thread Klaus Ma

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

(Updated July 21, 2016, 2:52 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Update v1/value.cpp


Repository: mesos


Description
---

Replaced boost::lexical_cast with numify when paring Value.


Diffs (updated)
-

  src/common/values.cpp 7b8a8ea00bd4bd1a82c33be4438c26b2731b1dd8 
  src/v1/values.cpp ff1926f9ab26c3ac3ab62b6df3ce305f5d12f12e 

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


Testing
---

make && make check


Thanks,

Klaus Ma



Review Request 50285: Replaced boost::lexical_cast with numify when paring Value.

2016-07-20 Thread Klaus Ma

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Replaced boost::lexical_cast with numify when paring Value.


Diffs
-

  src/common/values.cpp 7b8a8ea00bd4bd1a82c33be4438c26b2731b1dd8 

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


Testing
---

make && make check


Thanks,

Klaus Ma



Review Request 50284: Updated upgrades.md with fetcher user changes.

2016-07-20 Thread Greg Mann

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

The fetcher was changed to assume the same user as
a task when fetching files. This patch updates
upgrades.md accordingly.


Diffs
-

  docs/upgrades.md 1c871c4389539b7b9f5a93afdd5d986d3330b169 

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


Testing
---


Thanks,

Greg Mann



Review Request 50283: Updated CHANGELOG with fetcher user changes.

2016-07-20 Thread Greg Mann

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

The fetcher was changed to assume the same user as
a task when fetching files. This patch updates the
CHANGELOG accordingly.


Diffs
-

  CHANGELOG b9aa8943a7cf1709175bddebe5c3d022a7862c36 

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


Testing
---


Thanks,

Greg Mann



Re: Review Request 50268: Target shutdownFramework to associated agents.

2016-07-20 Thread Mesos ReviewBot

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On July 20, 2016, 10:09 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50268/
> ---
> 
> (Updated July 20, 2016, 10:09 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5874
> https://issues.apache.org/jira/browse/MESOS-5874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Target shutdownFramework to associated agents.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 78a8889313179b4509a6657e6c61d9f6d3bb99c0 
>   src/slave/slave.cpp da643e6e50b2f313705d2f862c961291aa5d2f22 
> 
> Diff: https://reviews.apache.org/r/50268/diff/
> 
> 
> Testing
> ---
> 
> 5 frameworks on 1 agent with 2 agents in cluster
> pre: ShutDownFrameworkMessage sent to both agents. 
> post: ShutdownFrameworkMessage sent to agent with executors associated with 
> framework.
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>



Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-20 Thread Zhitao Li

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

(Updated July 21, 2016, 4:33 a.m.)


Review request for mesos, Adam B, Benjamin Bannier, Greg Mann, and Till 
Toenshoff.


Changes
---

Add test for disabled authentication of readonly endpoints.
Properly unset authentication for `Slave` in test.


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


Repository: mesos


Description (updated)
---

Changes included:
- separate flags for readonly and readwrite endpoints;
- helper function for registering http authenticator;
- fixing existing tests.

Next step:
- docs fix.
- refactor common helper.


Diffs (updated)
-

  src/local/local.cpp a543aef117fea62660d55435be4d66d30f8ee860 
  src/master/constants.hpp 410c388c8f8ad98777c6587fc0b06807639e782a 
  src/master/flags.hpp a5dd11b624d19a9ea3508031ded4c0199098afd1 
  src/master/flags.cpp ca3e80bf9467328892be89718e5e0a1a05264ab8 
  src/master/main.cpp 9775b8a1e5fe51670789805557339bd0737a02b7 
  src/master/master.hpp 6decff6f4b9c3434de030fd5c06df4c683a7abad 
  src/master/master.cpp 370fd8712062dc75bb81824cb99ccc7920acbf78 
  src/slave/constants.hpp 10319396a6694e17137876b95ac6866c3d2ebcbd 
  src/slave/flags.hpp e798dbf2554a85310d71697d873bca4445a6161a 
  src/slave/flags.cpp 166a6516362a23bc5012aaa2dd45edfc6446de48 
  src/slave/main.cpp 4624392d30cf391015dcd63f447fe2414a47a16a 
  src/slave/slave.hpp ffe4220c9289419ab1b1a2c1f499b6eac3c01e4b 
  src/slave/slave.cpp 3e7131170e1f9bf682fb0c603d2ca39f514d87d9 
  src/tests/cluster.hpp 55dbaaef6c703676859e39e50bb1c31b942d0929 
  src/tests/cluster.cpp e1be275fccf130181ed18fd1c5a1b7b18979d7aa 
  src/tests/dynamic_weights_tests.cpp 6aa0102d3821c1b9d09672c706d6d8850f3729c3 
  src/tests/logging_tests.cpp 8712d332de50ee70584e6cb8c730cc243f4ba504 
  src/tests/main.cpp 1425a04c6f6ba9e512b44f083bdd66e3140925cf 
  src/tests/master_quota_tests.cpp 639f4c4146c61c0713e2945fea4fd6ffe1f3e726 
  src/tests/master_tests.cpp 252b8f9f2740256aaf54f24efe961d49fce53932 
  src/tests/mesos.hpp e4eccfc3810bed3649a3ab80e252849470de4c72 
  src/tests/mesos.cpp d073d79c5797ecb021f0294ab6586a000f3ca600 
  src/tests/metrics_tests.cpp e470e75e2457b01d24b50fd4ddefffb7553bd485 
  src/tests/persistent_volume_endpoints_tests.cpp 
fdd10a76dbfaf8bcae021b9d8b976f43748117fe 
  src/tests/reservation_endpoints_tests.cpp 
48c002d1dc371c285b9421ef5a2c57250d270fa8 
  src/tests/slave_tests.cpp 60f9e1644efaeba893f4ff38b6d5a07087d1a355 

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


Testing
---

`make check` on Mac OS.


Thanks,

Zhitao Li



Review Request 50280: Simplificed removing whitspace by strings::replace().

2016-07-20 Thread Klaus Ma

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Simplificed removing whitspace by strings::replace().


Diffs
-

  src/common/values.cpp 7b8a8ea00bd4bd1a82c33be4438c26b2731b1dd8 
  src/v1/values.cpp ff1926f9ab26c3ac3ab62b6df3ce305f5d12f12e 

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


Testing
---

make && make test


Thanks,

Klaus Ma



Re: Review Request 49239: Added logging when Offer::Operation::Launch has no tasks.

2016-07-20 Thread Jose Guilherme Vanz

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

(Updated July 21, 2016, 1:04 a.m.)


Review request for mesos and Anand Mazumdar.


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


Repository: mesos


Description
---

The master.cpp has been updated adding a warning message when some
offer is accepted by the framework but any task is defined to be
executed


Diffs (updated)
-

  src/master/master.cpp 370fd87 

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


Testing
---


Thanks,

Jose Guilherme Vanz



Review Request 50281: Added logging when Offer::Operation::Launch has no tasks.

2016-07-20 Thread Jose Guilherme Vanz

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

Review request for mesos.


Repository: mesos


Description
---

The master.cpp has been updated adding a warning message when some
offer is accepted by the framework but any task is defined to be
executed


Diffs
-

  src/master/master.cpp 370fd8712062dc75bb81824cb99ccc7920acbf78 

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


Testing
---


Thanks,

Jose Guilherme Vanz



Re: Review Request 49819: Implemented `CgroupsIsolatorProcess::prepare`.

2016-07-20 Thread haosdent huang


> On July 18, 2016, 2:24 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, line 307
> > 
> >
> > I think here you need to call `onAny()` rather than `then()`. You can 
> > take a look at the following code as a reference:
> > 
> > https://github.com/apache/mesos/blob/1.0.0-rc2/src/linux/cgroups.cpp#L1648:L1649
> 
> haosdent huang wrote:
> We could not use `onAny` here because it break the future chain. We only 
> could use `onAny` when return `void`.
> 
> For example, suppose we have these three methods:
> 
> ```
> Future X::A()
> {
>   return xxx.then(defer(self(), &Self::B));
> }
> 
> Future X::B();
> {
>   return xxx.then(defer(self(), &Self::C));
> }
> 
> Future X::C()
> {
>   return T(xxx);
> }
> ```
> 
> If we change
> 
> ```
> Future X::A()
> {
>   return xxx.then(defer(self(), &Self::B));
> }
> ```
> 
> to use `onAny`
> 
> ```
> Future X::A()
> {
>   return xxx.onAny(defer(self(), &Self::B)).then([]() -> T { return T(); 
> });
> }
> ```
> 
> then the `Future` return by `X::A()` would not wait for `X::C()` complete.

A possible workaround is

```
xxx.onAny();
return xxx.then([]() -> Nothing() { return Nothing(); });
```


- haosdent


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


On July 20, 2016, 6:37 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49819/
> ---
> 
> (Updated July 20, 2016, 6:37 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `CgroupsIsolatorProcess::prepare`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4a9f55bf3b217405bf90943f27a976422877a99e 
> 
> Diff: https://reviews.apache.org/r/49819/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 50259: Updated docker recovery to use abstraction provided by docker inspect.

2016-07-20 Thread Mesos ReviewBot

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



Bad patch!

Reviews applied: [50259, 50258, 50128, 50127, 50126, 50125, 50124, 50123]

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

Error:
2016-07-21 03:13:10 URL:https://reviews.apache.org/r/50258/diff/raw/ 
[10180/10180] -> "50258.patch" [1]
Total errors found: 0
Checking 3 files
Error: No line in the commit message summary may exceed 72 characters.

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

- Mesos ReviewBot


On July 20, 2016, 7:35 p.m., Rajat Phull wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50259/
> ---
> 
> (Updated July 20, 2016, 7:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, Yubo Li, and Vikrama 
> Ditya.
> 
> 
> Bugs: Mesos-5795
> https://issues.apache.org/jira/browse/Mesos-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docker recovery to use abstraction provided by docker inspect.
> 
> Builds on top of reviewrequest #50258
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 43ca4317d608b3b43dd7bd0d1b55c721e7364885 
>   src/slave/containerizer/docker.cpp f1ecf3b25d85597f6c3dcaa47968860ed119dbd5 
> 
> Diff: https://reviews.apache.org/r/50259/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="NvidiaGpuDockerContainerizerTest.ROOT_DOCKER_LaunchWithGpuRecovery"
>  make -j check
> 
> 
> Thanks,
> 
> Rajat Phull
> 
>



Re: Review Request 49819: Implemented `CgroupsIsolatorProcess::prepare`.

2016-07-20 Thread haosdent huang


> On July 18, 2016, 2:24 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, line 307
> > 
> >
> > I think here you need to call `onAny()` rather than `then()`. You can 
> > take a look at the following code as a reference:
> > 
> > https://github.com/apache/mesos/blob/1.0.0-rc2/src/linux/cgroups.cpp#L1648:L1649

We could not use `onAny` here because it break the future chain. We only could 
use `onAny` when return `void`.

For example, suppose we have these three methods:

```
Future X::A()
{
  return xxx.then(defer(self(), &Self::B));
}

Future X::B();
{
  return xxx.then(defer(self(), &Self::C));
}

Future X::C()
{
  return T(xxx);
}
```

If we change

```
Future X::A()
{
  return xxx.then(defer(self(), &Self::B));
}
```

to use `onAny`

```
Future X::A()
{
  return xxx.onAny(defer(self(), &Self::B)).then([]() -> T { return T(); });
}
```

then the `Future` return by `X::A()` would not wait for `X::C()` complete.


- haosdent


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


On July 20, 2016, 6:37 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49819/
> ---
> 
> (Updated July 20, 2016, 6:37 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `CgroupsIsolatorProcess::prepare`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4a9f55bf3b217405bf90943f27a976422877a99e 
> 
> Diff: https://reviews.apache.org/r/49819/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 50062: Refactored `makePortRanges` for allocator benchmark test.

2016-07-20 Thread Guangya Liu

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

(Updated 七月 21, 2016, 2:07 a.m.)


Review request for mesos, Benjamin Mahler and Jiang Yan Xu.


Summary (updated)
-

Refactored `makePortRanges` for allocator benchmark test.


Repository: mesos


Description (updated)
---

This patch refactored the helper function of `makePortRanges` as this:

1) Renamed `makePortRanges` to `makePorts`, this function creates a
   "ports(*)" resource for the given ranges.
2) Added a new helper function `fragment`, it helps to fragments the
   given range bounds into a number of subranges.
3) Keep `makeRange` unchanged but updated its parameter using `uint64_t`.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
befb94adb15e051abae135c7697538bd0cf83852 

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


Testing
---

make
make check


Thanks,

Guangya Liu



Re: Review Request 50172: Made `ports` resource configurable in sorter benchmark test.

2016-07-20 Thread Guangya Liu

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

(Updated 七月 21, 2016, 1:43 a.m.)


Review request for mesos and Benjamin Mahler.


Repository: mesos


Description (updated)
---

This patch added three new functions to sorter benchmark test to
generate a \`ports\` resource which falls into a specifed ports
range bounds.

1) makePorts: Creates a "ports(*)" resource for the given ranges.
2) fragment: Fragments the given range bounds into a number of
   subranges.
3) makeRange: Returns the range bounds.


Diffs (updated)
-

  src/tests/sorter_tests.cpp b0e5ef8a55bbcca3553a221bd5691a9c801a04f7 

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


Testing
---

```
./bin/mesos-tests.sh --benchmark 
--gtest_filter="AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/1"
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from AgentAndClientCount/Sorter_BENCHMARK_Test
[ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/1
Using 1000 agents and 50 clients
Added 50 clients in 872us
Added 1000 agents in 26457us
Added allocations for 1000 agents in 79697us
Full sort of 50 clients took 1321us
No-op sort of 50 clients took 28us
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/1 (115 ms)
[--] 1 test from AgentAndClientCount/Sorter_BENCHMARK_Test (115 ms 
total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (133 ms total)
[  PASSED  ] 1 test.
```


Thanks,

Guangya Liu



Re: Review Request 50003: Propagate work_dir flag from local runs to agents/masters.

2016-07-20 Thread Ammar Askar

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

(Updated July 21, 2016, 1:38 a.m.)


Review request for mesos, Greg Mann and Vinod Kone.


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


Repository: mesos


Description
---

Propagate work_dir flag from local runs to agents/masters.


Diffs (updated)
-

  src/local/flags.hpp f0af0d2 
  src/local/local.cpp a543aef 

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


Testing
---

Manually tested that `mesos local` and `mesos-local` run now. Not sure if a 
regression test for this would be good, please advise.


Thanks,

Ammar Askar



Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-20 Thread Zhitao Li

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

(Updated July 21, 2016, 1:25 a.m.)


Review request for mesos, Adam B, Benjamin Bannier, Greg Mann, and Till 
Toenshoff.


Changes
---

Incluede changes for agent and testing too.


Summary (updated)
-

Separate AuthN for readonly and readwrite endpoints in Mesos.


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


Repository: mesos


Description (updated)
---

Changes included:
- separate flags for readonly and readwrite endpoints;
- helper function for registering http authenticator;
- fixing existing tests.

Next step:
- test for readonly endpoints;
- docs fix.
- refactor common helper.


Diffs (updated)
-

  src/local/local.cpp a543aef117fea62660d55435be4d66d30f8ee860 
  src/master/constants.hpp 410c388c8f8ad98777c6587fc0b06807639e782a 
  src/master/flags.hpp a5dd11b624d19a9ea3508031ded4c0199098afd1 
  src/master/flags.cpp ca3e80bf9467328892be89718e5e0a1a05264ab8 
  src/master/main.cpp 9775b8a1e5fe51670789805557339bd0737a02b7 
  src/master/master.hpp 6decff6f4b9c3434de030fd5c06df4c683a7abad 
  src/master/master.cpp 370fd8712062dc75bb81824cb99ccc7920acbf78 
  src/slave/constants.hpp 10319396a6694e17137876b95ac6866c3d2ebcbd 
  src/slave/flags.hpp e798dbf2554a85310d71697d873bca4445a6161a 
  src/slave/flags.cpp 166a6516362a23bc5012aaa2dd45edfc6446de48 
  src/slave/main.cpp 4624392d30cf391015dcd63f447fe2414a47a16a 
  src/slave/slave.hpp ffe4220c9289419ab1b1a2c1f499b6eac3c01e4b 
  src/slave/slave.cpp 3e7131170e1f9bf682fb0c603d2ca39f514d87d9 
  src/tests/cluster.hpp 55dbaaef6c703676859e39e50bb1c31b942d0929 
  src/tests/cluster.cpp e1be275fccf130181ed18fd1c5a1b7b18979d7aa 
  src/tests/dynamic_weights_tests.cpp 6aa0102d3821c1b9d09672c706d6d8850f3729c3 
  src/tests/logging_tests.cpp 8712d332de50ee70584e6cb8c730cc243f4ba504 
  src/tests/main.cpp 1425a04c6f6ba9e512b44f083bdd66e3140925cf 
  src/tests/master_quota_tests.cpp 639f4c4146c61c0713e2945fea4fd6ffe1f3e726 
  src/tests/mesos.hpp e4eccfc3810bed3649a3ab80e252849470de4c72 
  src/tests/mesos.cpp d073d79c5797ecb021f0294ab6586a000f3ca600 
  src/tests/metrics_tests.cpp e470e75e2457b01d24b50fd4ddefffb7553bd485 
  src/tests/persistent_volume_endpoints_tests.cpp 
fdd10a76dbfaf8bcae021b9d8b976f43748117fe 
  src/tests/reservation_endpoints_tests.cpp 
48c002d1dc371c285b9421ef5a2c57250d270fa8 

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


Testing
---

`make check` on Mac OS.


Thanks,

Zhitao Li



Review Request 50277: Separate readonly and readwrite realms in libprocess.

2016-07-20 Thread Zhitao Li

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

Review request for mesos, Adam B, Benjamin Bannier, Greg Mann, and Till 
Toenshoff.


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


Repository: mesos


Description
---

Separate readonly and readwrite realms in libprocess.


Diffs
-

  3rdparty/libprocess/include/process/gtest.hpp 
21f66edf0b8c8a20fb4497b1a66dcdde5c328148 
  3rdparty/libprocess/include/process/process.hpp 
2e85d315aa479477bf0ac000e617761c8aa43f9e 
  3rdparty/libprocess/src/process.cpp c20a72b74b9b4d4729d83d2a5a162529fa35d682 
  3rdparty/libprocess/src/tests/main.cpp 
a03dbc4ab46747003d8b11d22f2dc136293264ec 
  3rdparty/libprocess/src/tests/metrics_tests.cpp 
d4d99de6d3bd9249e7b65e6be5841fbdb670a744 
  3rdparty/libprocess/src/tests/profiler_tests.cpp 
bf7a37536a7d15a03d1a88258f8cfb1f4b56bce8 

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


Testing
---


Thanks,

Zhitao Li



Re: Review Request 50260: Documented the executor envirment variable 'MESOS_SANDBOX'.

2016-07-20 Thread Gilbert Song

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

(Updated July 20, 2016, 5:58 p.m.)


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


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


Repository: mesos


Description
---

Documented the executor envirment variable 'MESOS_SANDBOX'.


Diffs (updated)
-

  docs/executor-http-api.md 17ba6f6a0ecbd271e6b1739ffaf92fba83c0fac1 

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


Testing
---

gist


Thanks,

Gilbert Song



Re: Review Request 50254: Added an example test for the V0/V1 Mesos java implementation.

2016-07-20 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [50247, 50248, 50250, 50251, 50252, 50253, 50254]

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

- Mesos ReviewBot


On July 20, 2016, 7:03 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50254/
> ---
> 
> (Updated July 20, 2016, 7:03 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Bugs: MESOS-5788
> https://issues.apache.org/jira/browse/MESOS-5788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an example test for the V0/V1 Mesos java implementation.
> 
> 
> Diffs
> -
> 
>   configure.ac d2136909b7305498ae901a5ea00133142b77f9e6 
>   src/Makefile.am cc83525a4455bbb0e654d346921d66ed2436411d 
>   src/examples/java/V1TestFramework.java PRE-CREATION 
>   src/examples/java/v1-test-framework.in PRE-CREATION 
>   src/tests/examples_tests.cpp ac513ce9aa3c8f366fe81ba937e3dc0d51a26940 
>   src/tests/java_v0_framework_test.sh PRE-CREATION 
>   src/tests/java_v1_framework_test.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50254/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 48616: Add v1 changes for shared resources.

2016-07-20 Thread Anindya Sinha

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

(Updated July 21, 2016, 12:13 a.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

v1 changes based on review comments in https://reviews.apache.org/r/45959/.


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


Repository: mesos


Description
---

Add v1 changes for shared resources.


Diffs (updated)
-

  include/mesos/v1/resources.hpp a5ba8fec4c9c3643646308f75a4b28cefe0b3df3 
  src/v1/resources.cpp 6d4ec75fbb7edab013563142aaf13d5302cc50af 

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


Testing
---


Thanks,

Anindya Sinha



Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

2016-07-20 Thread Anindya Sinha


> On July 20, 2016, 2:36 a.m., Klaus Ma wrote:
> > include/mesos/resources.hpp, line 124
> > 
> >
> > Seems `Option` is not necessary.

sharedCount is None when it is is regular non-shared resource, and is an 
integer for a shared resource which references to the number of copies of the 
shared resource. So, I think `Option` should be fine here.


> On July 20, 2016, 2:36 a.m., Klaus Ma wrote:
> > include/mesos/resources.hpp, line 484
> > 
> >
> > Why not `list`? It may avoid un-necessary resizing.

`std::list` does a memory allocation for every insert and a deallocation on 
every remove. `std::vector` reserves memory for certain number of entries 
(dependent on platform and implementation) but not on every insert (and hence 
memory allocation is done in a batch, and same for memory deallocation).
To alleviate move/copy operations on insert and remove from `std::vector`, we 
always do a `std::vector::push_back()` on insertion which is O(1), and for 
erase we swap the entry with the last entry using `std::vector::back()` and 
remove the last entry with a `std::vector::pop_back()` resulting in O(1).


> On July 20, 2016, 2:36 a.m., Klaus Ma wrote:
> > include/mesos/resources.hpp, line 84
> > 
> >
> > I think this's dangous, the developer need to check this converting and 
> > decide which `operator` is used. If miss used, it's hard to debuging.

Can you please explain why this is dangerous? This operator implicitly converts 
a `Resource_` to return the corresponding `Resource`.
Since `Resource_` is nested private class (and is hence hidden), callers would 
care about `Resource` only (this would ensure backward compatibility with 
existing code).


> On July 20, 2016, 2:36 a.m., Klaus Ma wrote:
> > src/common/resources.cpp, line 882
> > 
> >
> > one `isShared()` is enough. `isShared` is checked in `subtractable`.

Updated in `Resources::Resource_& Resources::Resource_::operator+=(const 
Resource_& that)` as well as in `Resources::Resource_& 
Resources::Resource_::operator-=(const Resource_& that)`.


- Anindya


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


On July 19, 2016, 10:51 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45959/
> ---
> 
> (Updated July 19, 2016, 10:51 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-4892
> https://issues.apache.org/jira/browse/MESOS-4892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A new class Resource_ is added that allows 'Resources' to group
> identical shared resource objects together into a single 'Resource_'
> object and tracked by its shared count. Non-shared resource objects
> are not grouped.
> 
> For resource addition and subtraction, the shared count is adjusted for
> shared resources as follows:
> a) Addition: If shared resource is absent from original, then the
>resource is added initialized with a consumer count of 1. Otherwise,
>the share count for the shared resource is incremented.
> b) Subtraction: If shared resource's share count is already 1, then
>the shared resource is removed from the original. Otherwise, its
>consumer count is decremented.
> 
> Note that v1 changes for shared resources are in the next commit.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp a557e97c65194d4aad879fb88d8edefd1c95b8d8 
>   src/common/resources.cpp b1bd2784aefdebf91892638b40b215373b998574 
>   src/master/validation.cpp 50ba372d1301d16d1738a3e4f4882b51f9ce06cd 
>   src/tests/mesos.hpp e4eccfc3810bed3649a3ab80e252849470de4c72 
>   src/tests/resources_tests.cpp 4111e080b84079e100b731c9a56861b204f17388 
> 
> Diff: https://reviews.apache.org/r/45959/diff/
> 
> 
> Testing
> ---
> 
> New tests added to demonstrate arithmetic operations for shared resources 
> with consumer counts.
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>



Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

2016-07-20 Thread Anindya Sinha

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

(Updated July 21, 2016, 12:12 a.m.)


Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Jiang Yan 
Xu.


Changes
---

updates based on review comments.


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


Repository: mesos


Description
---

A new class Resource_ is added that allows 'Resources' to group
identical shared resource objects together into a single 'Resource_'
object and tracked by its shared count. Non-shared resource objects
are not grouped.

For resource addition and subtraction, the shared count is adjusted for
shared resources as follows:
a) Addition: If shared resource is absent from original, then the
   resource is added initialized with a consumer count of 1. Otherwise,
   the share count for the shared resource is incremented.
b) Subtraction: If shared resource's share count is already 1, then
   the shared resource is removed from the original. Otherwise, its
   consumer count is decremented.

Note that v1 changes for shared resources are in the next commit.


Diffs (updated)
-

  include/mesos/resources.hpp a557e97c65194d4aad879fb88d8edefd1c95b8d8 
  src/common/resources.cpp b1bd2784aefdebf91892638b40b215373b998574 
  src/master/validation.cpp 52002beac29c371411348cb026a216e99ac96ab2 
  src/tests/mesos.hpp e4eccfc3810bed3649a3ab80e252849470de4c72 
  src/tests/resources_tests.cpp 4111e080b84079e100b731c9a56861b204f17388 

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


Testing
---

New tests added to demonstrate arithmetic operations for shared resources with 
consumer counts.
Tests successful.


Thanks,

Anindya Sinha



Re: Review Request 47095: Added tests for MESOS_SANDBOX env for unified containerizer.

2016-07-20 Thread Gilbert Song

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



@Shuai, did you pull the master upstream before rebasing?

- Gilbert Song


On July 19, 2016, 11:10 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47095/
> ---
> 
> (Updated July 19, 2016, 11:10 p.m.)
> 
> 
> Review request for mesos, Guangya Liu and Jie Yu.
> 
> 
> Bugs: MESOS-5312
> https://issues.apache.org/jira/browse/MESOS-5312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests for MESOS_SANDBOX env for unified containerizer.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> e3c8daab4fc688150a4f222e05f9b1bd9aee1912 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> ffe3382da8b1199e257a72ca9034fbccec9494b1 
> 
> Diff: https://reviews.apache.org/r/47095/diff/
> 
> 
> Testing
> ---
> 
> "make check" on ubuntu 14.04 64bit with gcc 4.8.4
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>



Re: Review Request 43561: Improve Ranges parsing to handle single values.

2016-07-20 Thread Benjamin Mahler


> On March 21, 2016, 2:02 p.m., Joris Van Remoortere wrote:
> > src/common/values.cpp, line 613
> > 
> >
> > @benm I wish we had support for iterating over these splicers eg:
> > `foreachtoken(temp, ",\n", [](const string& token) { ... });`
> > 
> > These helpers become rather heavy (at 3 nested) considering their 
> > utility.
> 
> Klaus Ma wrote:
> @joris, I think `foreachtoken` is also nested loop. When doing the 
> patches, there're three options to me: FSM, regex and nested loop; FSM & 
> regex seems hard to maintain, so I used nested loop.
> 
> Klaus Ma wrote:
> `foreachtoken(temp, ",\n", [](const string& token) { ... });` is a good 
> helper function; but no performance improvement. Any comments?
> 
> Klaus Ma wrote:
> @joris, I logged MESOS-5234 to trace this; I'll update patches 
> accordingly.
> 
> Klaus Ma wrote:
> @joris, had a patch for `foreachtoken`; would you help to review it?
> 
> https://reviews.apache.org/r/46425/

Joris why not do this?

```
foreach (const string& token, strings::token(s, ",")) {
  ...
}
```

By the way, we're cleaning up this code in https://reviews.apache.org/r/49223/ 
and other patches. This patch should become trivial.


- Benjamin


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


On June 11, 2016, 2:29 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated June 11, 2016, 2:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 43561: Improve Ranges parsing to handle single values.

2016-07-20 Thread Benjamin Mahler

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



Can you rebase this on top of https://reviews.apache.org/r/49223/?

That should make this a much simpler patch, yes?

- Benjamin Mahler


On June 11, 2016, 2:29 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated June 11, 2016, 2:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 49223: Fixed Value parsing code to only accept the canonical formats.

2016-07-20 Thread Benjamin Mahler

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



Sorry for the delay, mostly some bugs around use of tokenize rather than split, 
and some cleanup suggestions.

Be sure to update the two values.cpp files in the same way, based on the 
comments.


src/common/values.cpp (lines 577 - 583)


This can just be:

```
// Remove all spaces.
string temp = strings::replace(text, " ", "");
```

Feel free to simplify this in another patch.



src/common/values.cpp (lines 589 - 594)


Do we still need this? Seems to me that we want to do the following:

```
// Ranges. E.g. [1-2,4-5]
if (strings::startsWith(temp, "[")) {
  if (!strings::endsWith(temp, "]")) {
return Error("Missing a closing bracket");
  }

  ...
}

if (strings::startsWith(temp, "{")) {
  if (!strings::endsWith(temp, "}")) {
return Error("Missing a closing brace");
  }
  
  ..
}
```

Also, why is this looking at parentheses? I only see brackets (ranges) and 
braces (set) being used.



src/common/values.cpp (line 604)


Looks like you want a strings::split here? strings::tokenize will ignore 
empty fields:

```
// Using tokenize makes these two equivalent.
[1-25-6]
[1-2,5-6]
```

Can you add a test that we don't allow the extra commas since it's not the 
canonical format?

Also, why did you keep tokenizing on newlines? AFAICT they are not part of 
the canonical format?



src/common/values.cpp (lines 604 - 606)


You don't need the temporary variable?

```
foreach (const string& token, strings::split(temp, ",")) {
  ...
}
```



src/common/values.cpp (line 607)


Seems like we need strings::split here to avoid accepting ranges like 
`--1--2-`



src/common/values.cpp (line 609)


This message seems confusing. How about:

Invalid range '1 to 2'; expected format is 'begin-end'



src/common/values.cpp (lines 614 - 615)


We use `numify` now, mind updating this and removing the include?

Feel free to post a cleanup patch in front of this one.



src/common/values.cpp (line 635)


Ditto comments from Ranges here. (tokenize and newline)



src/common/values.cpp (lines 635 - 636)


You don't need the temporary variable?

```
foreach (const string& token, strings::split(temp, ",")) {
  ...
}
```



src/common/values.cpp (lines 637 - 638)


Please document in the description that this isn't a complete validation 
against the canonical format because we aren't validating text yet. The current 
summary of this commit seems to suggest that after applying this patch we'll 
**only** accept the canonical format (but that's not quite true).



src/common/values.cpp (line 649)


Ditto about numify here (but please do it in a separate patch in 
front of this one).



src/tests/values_tests.cpp (lines 88 - 91)


Can we also test the braces?

```
  // Only a single pair of braces are allowed.
  EXPECT_ERROR(parse("{a,b}{c,d}"));
  EXPECT_ERROR(parse("{a,b},{c,d}"));
  EXPECT_ERROR(parse("{a,b,{c,d}}"));
```

Since this isn't rejected just yet, please update the description to 
reflect that this patch isn't doing a complete validation against the canonical 
format. We can do the following to make it clear to the reader of the test that 
we don't fully validate yet:

```
  // TODO(klaus1982): Only a single pair of braces should be allowed,
  // however we do not validate the set entries as conforming to the
  // Values.Text format yet.
  //
  // EXPECT_ERROR(parse("{a,b}{c,d}"));
  // EXPECT_ERROR(parse("{a,b},{c,d}"));
  // EXPECT_ERROR(parse("{a,b,{c,d}}"));
```


- Benjamin Mahler


On July 16, 2016, 2:39 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated July 16, 2016, 2:39 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
>

Re: Review Request 50261: Updated allocator test helper function `makeLabels` more readable.

2016-07-20 Thread Benjamin Mahler

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


Ship it!




Nice cleanup, thanks for following up on my email even though this isn't your 
code!


src/tests/hierarchical_allocator_tests.cpp (line 3518)


Perhaps createLabels instead of makeLabels to be consistent with the 
existing createLabel naming?


- Benjamin Mahler


On July 20, 2016, 9:10 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50261/
> ---
> 
> (Updated July 20, 2016, 9:10 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated allocator test helper function `makeLabels` more readable.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 3ddce7ab19613831527f010524b8454fecfb9927 
> 
> Diff: https://reviews.apache.org/r/50261/diff/
> 
> 
> Testing
> ---
> 
> Discussion for this: http://markmail.org/thread/7sjfbfxi7tkhu35p
> 
> make
> make check
> 
> ```
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.ResourceLabels/1
> Using 1000 agents and 50 frameworks
> Added 50 frameworks in 10012us
> Added 1000 agents in 3.337114secs
> round 0 allocate() took 2.498978secs to make 1000 offers
> round 1 allocate() took 2.459849secs to make 1000 offers
> round 2 allocate() took 2.434381secs to make 1000 offers
> round 3 allocate() took 2.488205secs to make 1000 offers
> round 4 allocate() took 2.479436secs to make 1000 offers
> ```
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 50260: Documented the executor envirment variable 'MESOS_SANDBOX'.

2016-07-20 Thread Jie Yu

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




docs/executor-http-api.md (line 328)


on the host filesystem (deprecated)


- Jie Yu


On July 20, 2016, 7:32 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50260/
> ---
> 
> (Updated July 20, 2016, 7:32 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented the executor envirment variable 'MESOS_SANDBOX'.
> 
> 
> Diffs
> -
> 
>   docs/executor-http-api.md 17ba6f6a0ecbd271e6b1739ffaf92fba83c0fac1 
> 
> Diff: https://reviews.apache.org/r/50260/diff/
> 
> 
> Testing
> ---
> 
> gist
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>



Re: Review Request 50200: Made the agent fetch files as the task user.

2016-07-20 Thread Greg Mann


> On July 20, 2016, 10:38 p.m., Jie Yu wrote:
> > src/launcher/fetcher.cpp, lines 513-514
> > 
> >
> > ```
> > CHECK_SOME(os::su(...))
> >   << ...
> > ```
> > 
> > BTW, no idea why we use CHECK_SOME here. Looks to me that we should 
> > check the error and `return EXIT_FAILURE` here. Can you follow up with a 
> > patch to fix all occurance of CHECK_SOME here?

I fixed the instances of CHECK_SOME in my patches, and I'll leave a link here 
shortly to a patch replacing the remaining occurrences.


- Greg


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


On July 20, 2016, 11:02 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50200/
> ---
> 
> (Updated July 20, 2016, 11:02 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Joerg Schad.
> 
> 
> Bugs: mesos-5845
> https://issues.apache.org/jira/browse/mesos-5845
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To ensure that a task cannot fetch root-protected
> files from the local filesystem when running as a
> non-root user, this patch changes the fetcher to
> fetch files as the task user.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 0539b0182bd4a7178f1031ab4fee8fc79eda 
>   src/tests/fetcher_tests.cpp d38ce6e750dc828ef5af4a27fac76327cc4cb56c 
> 
> Diff: https://reviews.apache.org/r/50200/diff/
> 
> 
> Testing
> ---
> 
> A new test was added to the fetcher tests: 
> `FetcherTest.ROOT_RootProtectedFileURI`.
> 
> `sudo make check` was used to test on both OSX and CentOS 7.
> 
> Note that two of the fetcher tests fail for me when run as root on OSX. I saw 
> the following on my OSX 10.10.5 system:
> ```
> [  FAILED  ] FetcherCacheTest.LocalUncachedExtract
> [  FAILED  ] FetcherCacheHttpTest.HttpMixed
> ```
> 
> These failures are already tracked here: 
> https://issues.apache.org/jira/browse/MESOS-4890
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 50200: Made the agent fetch files as the task user.

2016-07-20 Thread Greg Mann

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

(Updated July 20, 2016, 11:02 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Joerg Schad.


Changes
---

Addressed comments.


Bugs: mesos-5845
https://issues.apache.org/jira/browse/mesos-5845


Repository: mesos


Description
---

To ensure that a task cannot fetch root-protected
files from the local filesystem when running as a
non-root user, this patch changes the fetcher to
fetch files as the task user.


Diffs (updated)
-

  src/launcher/fetcher.cpp 0539b0182bd4a7178f1031ab4fee8fc79eda 
  src/tests/fetcher_tests.cpp d38ce6e750dc828ef5af4a27fac76327cc4cb56c 

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


Testing
---

A new test was added to the fetcher tests: 
`FetcherTest.ROOT_RootProtectedFileURI`.

`sudo make check` was used to test on both OSX and CentOS 7.

Note that two of the fetcher tests fail for me when run as root on OSX. I saw 
the following on my OSX 10.10.5 system:
```
[  FAILED  ] FetcherCacheTest.LocalUncachedExtract
[  FAILED  ] FetcherCacheHttpTest.HttpMixed
```

These failures are already tracked here: 
https://issues.apache.org/jira/browse/MESOS-4890


Thanks,

Greg Mann



Re: Review Request 50199: Refactored fetcher cache directory creation.

2016-07-20 Thread Greg Mann

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

(Updated July 20, 2016, 11:01 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Joerg Schad.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

The fetcher's launcher creates the fetcher cache
directory for each user immediately before an artifact
is fetched. In order to allow this directory to be
created by a different user than the user doing the
fetching, this patch factors out this directory
creation and places it before all fetches occur.


Diffs (updated)
-

  src/launcher/fetcher.cpp 0539b0182bd4a7178f1031ab4fee8fc79eda 

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


Testing
---

`sudo make check` on both OSX and CentOS 7 was done at the end of this patch 
chain.


Thanks,

Greg Mann



Re: Review Request 50199: Refactored fetcher cache directory creation.

2016-07-20 Thread Greg Mann


> On July 20, 2016, 10:26 p.m., Jie Yu wrote:
> > src/launcher/fetcher.cpp, lines 512-514
> > 
> >
> > Can we instead use:
> > ```
> > CHECK_SOME(createCacheDirectory(fetcherInfo.get()))
> >   << "...";
> > ```

I changed this to an `EXIT` statement, per your suggestion on the following 
patch.


- Greg


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


On July 20, 2016, 11:01 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50199/
> ---
> 
> (Updated July 20, 2016, 11:01 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Joerg Schad.
> 
> 
> Bugs: MESOS-5845
> https://issues.apache.org/jira/browse/MESOS-5845
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The fetcher's launcher creates the fetcher cache
> directory for each user immediately before an artifact
> is fetched. In order to allow this directory to be
> created by a different user than the user doing the
> fetching, this patch factors out this directory
> creation and places it before all fetches occur.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 0539b0182bd4a7178f1031ab4fee8fc79eda 
> 
> Diff: https://reviews.apache.org/r/50199/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on both OSX and CentOS 7 was done at the end of this patch 
> chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 50200: Made the agent fetch files as the task user.

2016-07-20 Thread Jie Yu

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


Fix it, then Ship it!





src/launcher/fetcher.cpp (lines 513 - 514)


```
CHECK_SOME(os::su(...))
  << ...
```

BTW, no idea why we use CHECK_SOME here. Looks to me that we should check 
the error and `return EXIT_FAILURE` here. Can you follow up with a patch to fix 
all occurance of CHECK_SOME here?



src/tests/fetcher_tests.cpp (lines 131 - 133)


No need for temp var here:
```
AWAIT_FAILED(fether.fetch(
containerId,
commandInfo,
os::getcwd(),
None(),
slaveId,
flags);
```


- Jie Yu


On July 20, 2016, 8:01 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50200/
> ---
> 
> (Updated July 20, 2016, 8:01 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Joerg Schad.
> 
> 
> Bugs: mesos-5845
> https://issues.apache.org/jira/browse/mesos-5845
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To ensure that a task cannot fetch root-protected
> files from the local filesystem when running as a
> non-root user, this patch changes the fetcher to
> fetch files as the task user.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 0539b0182bd4a7178f1031ab4fee8fc79eda 
>   src/tests/fetcher_tests.cpp d38ce6e750dc828ef5af4a27fac76327cc4cb56c 
> 
> Diff: https://reviews.apache.org/r/50200/diff/
> 
> 
> Testing
> ---
> 
> A new test was added to the fetcher tests: 
> `FetcherTest.ROOT_RootProtectedFileURI`.
> 
> `sudo make check` was used to test on both OSX and CentOS 7.
> 
> Note that two of the fetcher tests fail for me when run as root on OSX. I saw 
> the following on my OSX 10.10.5 system:
> ```
> [  FAILED  ] FetcherCacheTest.LocalUncachedExtract
> [  FAILED  ] FetcherCacheHttpTest.HttpMixed
> ```
> 
> These failures are already tracked here: 
> https://issues.apache.org/jira/browse/MESOS-4890
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 50199: Refactored fetcher cache directory creation.

2016-07-20 Thread Jie Yu

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


Fix it, then Ship it!





src/launcher/fetcher.cpp (lines 506 - 508)


Can we instead use:
```
CHECK_SOME(createCacheDirectory(fetcherInfo.get()))
  << "...";
```



src/launcher/fetcher.cpp (line 430)


for such case, we usually do the following to avoid more level of nesting:
```
if (!fetcherInfo.has_cache_directory()) {
  return Nothing();
}

foreach (...)
```



src/launcher/fetcher.cpp (line 436)


s/result/mkdir/



src/launcher/fetcher.cpp (lines 444 - 445)


I'd prefer
```
Try chown = os:chown(
fetcherInfo.user(),
fetcherInfo.cache_directory(),
false);

if (chown.isError()) {
  return chown;
}
```


- Jie Yu


On July 20, 2016, 10:07 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50199/
> ---
> 
> (Updated July 20, 2016, 10:07 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Joerg Schad.
> 
> 
> Bugs: MESOS-5845
> https://issues.apache.org/jira/browse/MESOS-5845
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The fetcher's launcher creates the fetcher cache
> directory for each user immediately before an artifact
> is fetched. In order to allow this directory to be
> created by a different user than the user doing the
> fetching, this patch factors out this directory
> creation and places it before all fetches occur.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 0539b0182bd4a7178f1031ab4fee8fc79eda 
> 
> Diff: https://reviews.apache.org/r/50199/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on both OSX and CentOS 7 was done at the end of this patch 
> chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Review Request 50269: Added basic tests for capabilities API.

2016-07-20 Thread Benjamin Bannier

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

This test is based on the work in https://reviews.apache.org/r/46371/.


Diffs
-

  src/Makefile.am cc83525a4455bbb0e654d346921d66ed2436411d 
  src/tests/capabilities_test_helper.hpp PRE-CREATION 
  src/tests/capabilities_test_helper.cpp PRE-CREATION 
  src/tests/capabilities_test_helper_main.cpp PRE-CREATION 
  src/tests/capabilities_tests.cpp PRE-CREATION 

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


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)


Thanks,

Benjamin Bannier



Review Request 50271: Created an isolator for Linux capabilities.

2016-07-20 Thread Benjamin Bannier

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

This isolator evaluates agent allowed capabilities and passes net
capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed via a new field in
`ContainerLaunchInfo`.


Diffs
-

  include/mesos/slave/isolator.proto a971a582da2ec121ed8dd4ef0b7f5ee47a0c3d03 
  src/Makefile.am cc83525a4455bbb0e654d346921d66ed2436411d 
  src/slave/containerizer/mesos/containerizer.cpp 
1c9e17b23d7f2ee49e00b80053e4de1797d73c73 
  src/slave/containerizer/mesos/isolators/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/capabilities.cpp PRE-CREATION 
  src/tests/containerizer/isolator_tests.cpp 
488747347f71a6a1bb6bc01477143d077d4fd3eb 

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


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)


Thanks,

Benjamin Bannier



Review Request 50266: Introduced linux capabilities API.

2016-07-20 Thread Benjamin Bannier

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

This change introduces basic API for linux capabilities. This is not a
comprehensive API but is strictly limited to the need for securing Mesos
containers using linux capabilities.

This patch is based on the work in https://reviews.apache.org/r/46370/.


Diffs
-

  src/CMakeLists.txt bde76f7840afe55f20d7551b3f7e5fe522f7f326 
  src/Makefile.am cc83525a4455bbb0e654d346921d66ed2436411d 
  src/linux/capabilities.hpp PRE-CREATION 
  src/linux/capabilities.cpp PRE-CREATION 

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


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)


Thanks,

Benjamin Bannier



Review Request 50270: Introduced linux capabilities support for mesos containerizer.

2016-07-20 Thread Benjamin Bannier

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

This change introduces linux capability based security for unified
containerizer. A new agent flag \`allowed_capabilities\` has been
introduced to override the default capabilities of the user or the
capabilities requested by the user.

This feature is only available on linux.

This patch is based on https://reviews.apache.org/r/46798/.


Diffs
-

  src/common/parse.hpp 5dc795d7f54209abe64ad48360f538faac7616f0 
  src/internal/devolve.hpp 3812fd654d6cdceccf31b3f7c1a067cf2922e06f 
  src/internal/devolve.cpp a2ad4641fcadef4003e487683fc0a73aeece7647 
  src/internal/evolve.hpp 1e2d49b6a465c13dd055e54f0d4c49d22afc15c6 
  src/internal/evolve.cpp 64818ccbbc4d0fcf6744e3f9a30c17c5332a 
  src/launcher/executor.cpp 5a5f95f04a6ce096079b67397cb324575409f795 
  src/slave/flags.hpp e798dbf2554a85310d71697d873bca4445a6161a 
  src/slave/flags.cpp 166a6516362a23bc5012aaa2dd45edfc6446de48 

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


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)


Thanks,

Benjamin Bannier



Re: Review Request 50268: Target shutdownFramework to associated agents.

2016-07-20 Thread Jacob Janco

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

(Updated July 20, 2016, 10:09 p.m.)


Review request for mesos.


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


Repository: mesos


Description
---

Target shutdownFramework to associated agents.


Diffs
-

  src/master/master.cpp 78a8889313179b4509a6657e6c61d9f6d3bb99c0 
  src/slave/slave.cpp da643e6e50b2f313705d2f862c961291aa5d2f22 

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


Testing
---

5 frameworks on 1 agent with 2 agents in cluster
pre: ShutDownFrameworkMessage sent to both agents. 
post: ShutdownFrameworkMessage sent to agent with executors associated with 
framework.


Thanks,

Jacob Janco



Re: Review Request 50268: Target shutdownFramework to associated agents.

2016-07-20 Thread Jacob Janco

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

(Updated July 20, 2016, 10:08 p.m.)


Review request for mesos.


Repository: mesos


Description
---

Target shutdownFramework to associated agents.


Diffs
-

  src/master/master.cpp 78a8889313179b4509a6657e6c61d9f6d3bb99c0 
  src/slave/slave.cpp da643e6e50b2f313705d2f862c961291aa5d2f22 

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


Testing (updated)
---

5 frameworks on 1 agent with 2 agents in cluster
pre: ShutDownFrameworkMessage sent to both agents. 
post: ShutdownFrameworkMessage sent to agent with executors associated with 
framework.


Thanks,

Jacob Janco



Re: Review Request 50199: Refactored fetcher cache directory creation.

2016-07-20 Thread Greg Mann


> On July 20, 2016, 10:06 p.m., Joerg Schad wrote:
> > src/launcher/fetcher.cpp, line 441
> > 
> >
> > Can it happen that dir already exists?

Indeed it can! While we were safe with the existing code, I tweaked this call 
to `os::mkdir` to explicitly set `recursive = true`, so that we won't get an 
error if the directory already exists. Thanks Joerg!!


- Greg


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


On July 20, 2016, 10:07 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50199/
> ---
> 
> (Updated July 20, 2016, 10:07 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Joerg Schad.
> 
> 
> Bugs: MESOS-5845
> https://issues.apache.org/jira/browse/MESOS-5845
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The fetcher's launcher creates the fetcher cache
> directory for each user immediately before an artifact
> is fetched. In order to allow this directory to be
> created by a different user than the user doing the
> fetching, this patch factors out this directory
> creation and places it before all fetches occur.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 0539b0182bd4a7178f1031ab4fee8fc79eda 
> 
> Diff: https://reviews.apache.org/r/50199/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on both OSX and CentOS 7 was done at the end of this patch 
> chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 50199: Refactored fetcher cache directory creation.

2016-07-20 Thread Greg Mann

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

(Updated July 20, 2016, 10:07 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Joerg Schad.


Changes
---

Addressed comment.


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


Repository: mesos


Description
---

The fetcher's launcher creates the fetcher cache
directory for each user immediately before an artifact
is fetched. In order to allow this directory to be
created by a different user than the user doing the
fetching, this patch factors out this directory
creation and places it before all fetches occur.


Diffs (updated)
-

  src/launcher/fetcher.cpp 0539b0182bd4a7178f1031ab4fee8fc79eda 

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


Testing
---

`sudo make check` on both OSX and CentOS 7 was done at the end of this patch 
chain.


Thanks,

Greg Mann



Re: Review Request 50199: Refactored fetcher cache directory creation.

2016-07-20 Thread Joerg Schad

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




src/launcher/fetcher.cpp (line 435)


Can it happen that dir already exists?


- Joerg Schad


On July 20, 2016, 7:50 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50199/
> ---
> 
> (Updated July 20, 2016, 7:50 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Joerg Schad.
> 
> 
> Bugs: MESOS-5845
> https://issues.apache.org/jira/browse/MESOS-5845
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The fetcher's launcher creates the fetcher cache
> directory for each user immediately before an artifact
> is fetched. In order to allow this directory to be
> created by a different user than the user doing the
> fetching, this patch factors out this directory
> creation and places it before all fetches occur.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 0539b0182bd4a7178f1031ab4fee8fc79eda 
> 
> Diff: https://reviews.apache.org/r/50199/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on both OSX and CentOS 7 was done at the end of this patch 
> chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Review Request 50268: Target shutdownFramework to associated agents.

2016-07-20 Thread Jacob Janco

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

Review request for mesos.


Repository: mesos


Description
---

Target shutdownFramework to associated agents.


Diffs
-

  src/master/master.cpp 78a8889313179b4509a6657e6c61d9f6d3bb99c0 
  src/slave/slave.cpp da643e6e50b2f313705d2f862c961291aa5d2f22 

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


Testing
---


Thanks,

Jacob Janco



Re: Review Request 50267: Updated upgrades.md about deprecated SSL env variables.

2016-07-20 Thread Vinod Kone

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


Fix it, then Ship it!





docs/upgrades.md (line 182)


s/we found that/it was found that/

s/we deprecated those environment variables. Users should use the 
corresponding/`SSL_*` environment variables are deprecated in favor of the 
corresponding `LIBPROCESS_SSL_*` variables./ ?


- Vinod Kone


On July 20, 2016, 10:01 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50267/
> ---
> 
> (Updated July 20, 2016, 10:01 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated upgrades.md about deprecated SSL env variables.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 431e6b3fab1a066e8f84e2a83ce961ddfb51f647 
> 
> Diff: https://reviews.apache.org/r/50267/diff/
> 
> 
> Testing
> ---
> 
> Preview here:
> https://github.com/jieyu/mesos/blob/WIP/docs/upgrades.md
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 49239: Added logging when Offer::Operation::Launch has no tasks.

2016-07-20 Thread Anand Mazumdar

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



Sorry for the late turn around on this one.


src/master/master.cpp (lines 3379 - 3380)


How about:

```
Implicitly declining offers: << accept.offer_ids() << in ACCEPT call for 
framework << framework->id() << as the launch operation specified no tasks
```


- Anand Mazumdar


On June 27, 2016, 12:43 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49239/
> ---
> 
> (Updated June 27, 2016, 12:43 a.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Bugs: MESOS-5565
> https://issues.apache.org/jira/browse/MESOS-5565
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The master.cpp has been updated adding a warning message when some
> offer is accepted by the framework but any task is defined to be
> executed
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp d89c049326358bad509bb1455c81eb11610eeeb8 
> 
> Diff: https://reviews.apache.org/r/49239/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>



Review Request 50267: Updated upgrades.md about deprecated SSL env variables.

2016-07-20 Thread Jie Yu

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

Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

Updated upgrades.md about deprecated SSL env variables.


Diffs
-

  docs/upgrades.md 431e6b3fab1a066e8f84e2a83ce961ddfb51f647 

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


Testing
---

Preview here:
https://github.com/jieyu/mesos/blob/WIP/docs/upgrades.md


Thanks,

Jie Yu



Re: Review Request 50265: Updated 1.0 CHANGELOG about an SSL fix.

2016-07-20 Thread Vinod Kone

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


Ship it!




Ship It!

- Vinod Kone


On July 20, 2016, 9:59 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50265/
> ---
> 
> (Updated July 20, 2016, 9:59 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated 1.0 CHANGELOG about an SSL fix.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 2f618db007ee66127c408f8f19429d23c26996bc 
> 
> Diff: https://reviews.apache.org/r/50265/diff/
> 
> 
> Testing
> ---
> 
> trivial
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 50265: Updated 1.0 CHANGELOG about an SSL fix.

2016-07-20 Thread Jie Yu

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

(Updated July 20, 2016, 9:59 p.m.)


Review request for mesos, Joris Van Remoortere and Vinod Kone.


Changes
---

Call out the deprecation.


Repository: mesos


Description
---

Updated 1.0 CHANGELOG about an SSL fix.


Diffs (updated)
-

  CHANGELOG 2f618db007ee66127c408f8f19429d23c26996bc 

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


Testing
---

trivial


Thanks,

Jie Yu



Re: Review Request 50257: Used LIBPROCESS_SSL_ instead of SSL_ as the prefix for ssl support.

2016-07-20 Thread Jie Yu


> On July 20, 2016, 9:39 p.m., Adam B wrote:
> > Won't this break upgrades for any user already using these environment 
> > variables?!?
> 
> Adam B wrote:
> Nevermind.. premature panic. Now I see
> ```
>   // To be backward compatible, for each environment variable prefixed
>   // by SSL_, we generate the corresponding LIBPROCESS_SSL_ version.
> ```
> So even if we use the `SSL_` variety, libprocess will use the values, but 
> subprocesses will not see them?
> 
> Jie Yu wrote:
> No, it won't. see the doc.

subprocess will still see them. we don't change/add/update env variables during 
fork.

users should use LIBPROCESS_SSL instead to avoid conflicts with openssl.


- Jie


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


On July 20, 2016, 8:34 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50257/
> ---
> 
> (Updated July 20, 2016, 8:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-5863
> https://issues.apache.org/jira/browse/MESOS-5863
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used LIBPROCESS_SSL_ instead of SSL_ as the prefix for ssl support.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> a929cc90fc9ee975f3635957518ced4eb70bdfeb 
>   3rdparty/libprocess/src/openssl.hpp 
> 68f88970610293107b8349c216c34a32d5df9105 
>   3rdparty/libprocess/src/openssl.cpp 
> 63916ff66b4daa29120b7e6b12b329b68f746694 
>   3rdparty/libprocess/src/process.cpp 
> 9661386afd4fddd1877d55941fa403afc9230280 
>   3rdparty/libprocess/src/tests/ssl_client.cpp 
> 6b43cfc77681b5aca76da2638443155f682cd92f 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 72432ec8f91eb4d628df73e19edbaceb8a2c0fb9 
> 
> Diff: https://reviews.apache.org/r/50257/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Review Request 50265: Updated 1.0 CHANGELOG about an SSL fix.

2016-07-20 Thread Jie Yu

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

Review request for mesos, Joris Van Remoortere and Vinod Kone.


Repository: mesos


Description
---

Updated 1.0 CHANGELOG about an SSL fix.


Diffs
-

  CHANGELOG 2f618db007ee66127c408f8f19429d23c26996bc 

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


Testing
---

trivial


Thanks,

Jie Yu



Re: Review Request 50257: Used LIBPROCESS_SSL_ instead of SSL_ as the prefix for ssl support.

2016-07-20 Thread Adam B

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



Won't this break upgrades for any user already using these environment 
variables?!?

- Adam B


On July 20, 2016, 1:34 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50257/
> ---
> 
> (Updated July 20, 2016, 1:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-5863
> https://issues.apache.org/jira/browse/MESOS-5863
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used LIBPROCESS_SSL_ instead of SSL_ as the prefix for ssl support.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> a929cc90fc9ee975f3635957518ced4eb70bdfeb 
>   3rdparty/libprocess/src/openssl.hpp 
> 68f88970610293107b8349c216c34a32d5df9105 
>   3rdparty/libprocess/src/openssl.cpp 
> 63916ff66b4daa29120b7e6b12b329b68f746694 
>   3rdparty/libprocess/src/process.cpp 
> 9661386afd4fddd1877d55941fa403afc9230280 
>   3rdparty/libprocess/src/tests/ssl_client.cpp 
> 6b43cfc77681b5aca76da2638443155f682cd92f 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 72432ec8f91eb4d628df73e19edbaceb8a2c0fb9 
> 
> Diff: https://reviews.apache.org/r/50257/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 50257: Used LIBPROCESS_SSL_ instead of SSL_ as the prefix for ssl support.

2016-07-20 Thread Jie Yu


> On July 20, 2016, 9:39 p.m., Adam B wrote:
> > Won't this break upgrades for any user already using these environment 
> > variables?!?
> 
> Adam B wrote:
> Nevermind.. premature panic. Now I see
> ```
>   // To be backward compatible, for each environment variable prefixed
>   // by SSL_, we generate the corresponding LIBPROCESS_SSL_ version.
> ```
> So even if we use the `SSL_` variety, libprocess will use the values, but 
> subprocesses will not see them?

No, it won't. see the doc.


- Jie


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


On July 20, 2016, 8:34 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50257/
> ---
> 
> (Updated July 20, 2016, 8:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-5863
> https://issues.apache.org/jira/browse/MESOS-5863
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used LIBPROCESS_SSL_ instead of SSL_ as the prefix for ssl support.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> a929cc90fc9ee975f3635957518ced4eb70bdfeb 
>   3rdparty/libprocess/src/openssl.hpp 
> 68f88970610293107b8349c216c34a32d5df9105 
>   3rdparty/libprocess/src/openssl.cpp 
> 63916ff66b4daa29120b7e6b12b329b68f746694 
>   3rdparty/libprocess/src/process.cpp 
> 9661386afd4fddd1877d55941fa403afc9230280 
>   3rdparty/libprocess/src/tests/ssl_client.cpp 
> 6b43cfc77681b5aca76da2638443155f682cd92f 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 72432ec8f91eb4d628df73e19edbaceb8a2c0fb9 
> 
> Diff: https://reviews.apache.org/r/50257/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 50257: Used LIBPROCESS_SSL_ instead of SSL_ as the prefix for ssl support.

2016-07-20 Thread Adam B


> On July 20, 2016, 2:39 p.m., Adam B wrote:
> > Won't this break upgrades for any user already using these environment 
> > variables?!?

Nevermind.. premature panic. Now I see
```
  // To be backward compatible, for each environment variable prefixed
  // by SSL_, we generate the corresponding LIBPROCESS_SSL_ version.
```
So even if we use the `SSL_` variety, libprocess will use the values, but 
subprocesses will not see them?


- Adam


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


On July 20, 2016, 1:34 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50257/
> ---
> 
> (Updated July 20, 2016, 1:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-5863
> https://issues.apache.org/jira/browse/MESOS-5863
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used LIBPROCESS_SSL_ instead of SSL_ as the prefix for ssl support.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> a929cc90fc9ee975f3635957518ced4eb70bdfeb 
>   3rdparty/libprocess/src/openssl.hpp 
> 68f88970610293107b8349c216c34a32d5df9105 
>   3rdparty/libprocess/src/openssl.cpp 
> 63916ff66b4daa29120b7e6b12b329b68f746694 
>   3rdparty/libprocess/src/process.cpp 
> 9661386afd4fddd1877d55941fa403afc9230280 
>   3rdparty/libprocess/src/tests/ssl_client.cpp 
> 6b43cfc77681b5aca76da2638443155f682cd92f 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 72432ec8f91eb4d628df73e19edbaceb8a2c0fb9 
> 
> Diff: https://reviews.apache.org/r/50257/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in master.

2016-07-20 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [50223]

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

- Mesos ReviewBot


On July 20, 2016, 3:21 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50223/
> ---
> 
> (Updated July 20, 2016, 3:21 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Bannier, Greg Mann, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-5851
> https://issues.apache.org/jira/browse/MESOS-5851
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes included:
> - separate flags for readonly and readwrite endpoints;
> - helper function for registering http authenticator;
> - fixing existing tests.
> 
> Open question:
> - location and name of helper function.
> 
> Next step:
> - test for readonly endpoints;
> - similar change to agent endpoints;
> - docs/configuration.md fix;
> - docs/endpoints/index.md fix.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp a543aef117fea62660d55435be4d66d30f8ee860 
>   src/master/constants.hpp 410c388c8f8ad98777c6587fc0b06807639e782a 
>   src/master/flags.hpp a5dd11b624d19a9ea3508031ded4c0199098afd1 
>   src/master/flags.cpp ca3e80bf9467328892be89718e5e0a1a05264ab8 
>   src/master/main.cpp 9775b8a1e5fe51670789805557339bd0737a02b7 
>   src/master/master.hpp 6decff6f4b9c3434de030fd5c06df4c683a7abad 
>   src/master/master.cpp 370fd8712062dc75bb81824cb99ccc7920acbf78 
>   src/tests/cluster.hpp 55dbaaef6c703676859e39e50bb1c31b942d0929 
>   src/tests/cluster.cpp e1be275fccf130181ed18fd1c5a1b7b18979d7aa 
>   src/tests/dynamic_weights_tests.cpp 
> 6aa0102d3821c1b9d09672c706d6d8850f3729c3 
>   src/tests/master_quota_tests.cpp 639f4c4146c61c0713e2945fea4fd6ffe1f3e726 
>   src/tests/mesos.cpp d073d79c5797ecb021f0294ab6586a000f3ca600 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> fdd10a76dbfaf8bcae021b9d8b976f43748117fe 
>   src/tests/reservation_endpoints_tests.cpp 
> 48c002d1dc371c285b9421ef5a2c57250d270fa8 
> 
> Diff: https://reviews.apache.org/r/50223/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>



Review Request 50261: Updated allocator test helper function `makeLabels` more readable.

2016-07-20 Thread Guangya Liu

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Updated allocator test helper function `makeLabels` more readable.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
3ddce7ab19613831527f010524b8454fecfb9927 

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


Testing
---

Discussion for this: http://markmail.org/thread/7sjfbfxi7tkhu35p

make
make check

```
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test
[ RUN  ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.ResourceLabels/1
Using 1000 agents and 50 frameworks
Added 50 frameworks in 10012us
Added 1000 agents in 3.337114secs
round 0 allocate() took 2.498978secs to make 1000 offers
round 1 allocate() took 2.459849secs to make 1000 offers
round 2 allocate() took 2.434381secs to make 1000 offers
round 3 allocate() took 2.488205secs to make 1000 offers
round 4 allocate() took 2.479436secs to make 1000 offers
```


Thanks,

Guangya Liu



Re: Review Request 50262: Updated the document about SSL env variables.

2016-07-20 Thread Joseph Wu

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


Fix it, then Ship it!





docs/ssl.md (line 24)


Do you want to note that `SSL_*` may collide with other programs and lead 
to unexpected results?


- Joseph Wu


On July 20, 2016, 2:04 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50262/
> ---
> 
> (Updated July 20, 2016, 2:04 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Bugs: MESOS-5863
> https://issues.apache.org/jira/browse/MESOS-5863
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the document about SSL env variables.
> 
> 
> Diffs
> -
> 
>   docs/ssl.md 64f63b7c23ff97c027c24fad10ab7fa46ccc8117 
> 
> Diff: https://reviews.apache.org/r/50262/diff/
> 
> 
> Testing
> ---
> 
> Preview here:
> https://github.com/jieyu/mesos/blob/WIP/docs/ssl.md
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Review Request 50262: Updated the document about SSL env variables.

2016-07-20 Thread Jie Yu

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

Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description
---

Updated the document about SSL env variables.


Diffs
-

  docs/ssl.md 64f63b7c23ff97c027c24fad10ab7fa46ccc8117 

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


Testing
---

Preview here:
https://github.com/jieyu/mesos/blob/WIP/docs/ssl.md


Thanks,

Jie Yu



Re: Review Request 50257: Used LIBPROCESS_SSL_ instead of SSL_ as the prefix for ssl support.

2016-07-20 Thread Joris Van Remoortere


> On July 20, 2016, 9 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/openssl.cpp, line 312
> > 
> >
> > Don't forget the change the documentation too!

It's reportedly on it's way. Has to be a separate patch still right?


- Joris


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


On July 20, 2016, 8:34 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50257/
> ---
> 
> (Updated July 20, 2016, 8:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-5863
> https://issues.apache.org/jira/browse/MESOS-5863
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used LIBPROCESS_SSL_ instead of SSL_ as the prefix for ssl support.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> a929cc90fc9ee975f3635957518ced4eb70bdfeb 
>   3rdparty/libprocess/src/openssl.hpp 
> 68f88970610293107b8349c216c34a32d5df9105 
>   3rdparty/libprocess/src/openssl.cpp 
> 63916ff66b4daa29120b7e6b12b329b68f746694 
>   3rdparty/libprocess/src/process.cpp 
> 9661386afd4fddd1877d55941fa403afc9230280 
>   3rdparty/libprocess/src/tests/ssl_client.cpp 
> 6b43cfc77681b5aca76da2638443155f682cd92f 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 72432ec8f91eb4d628df73e19edbaceb8a2c0fb9 
> 
> Diff: https://reviews.apache.org/r/50257/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 50257: Used LIBPROCESS_SSL_ instead of SSL_ as the prefix for ssl support.

2016-07-20 Thread Joseph Wu

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




3rdparty/libprocess/src/openssl.cpp (line 312)


Don't forget the change the documentation too!


- Joseph Wu


On July 20, 2016, 1:34 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50257/
> ---
> 
> (Updated July 20, 2016, 1:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-5863
> https://issues.apache.org/jira/browse/MESOS-5863
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used LIBPROCESS_SSL_ instead of SSL_ as the prefix for ssl support.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> a929cc90fc9ee975f3635957518ced4eb70bdfeb 
>   3rdparty/libprocess/src/openssl.hpp 
> 68f88970610293107b8349c216c34a32d5df9105 
>   3rdparty/libprocess/src/openssl.cpp 
> 63916ff66b4daa29120b7e6b12b329b68f746694 
>   3rdparty/libprocess/src/process.cpp 
> 9661386afd4fddd1877d55941fa403afc9230280 
>   3rdparty/libprocess/src/tests/ssl_client.cpp 
> 6b43cfc77681b5aca76da2638443155f682cd92f 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 72432ec8f91eb4d628df73e19edbaceb8a2c0fb9 
> 
> Diff: https://reviews.apache.org/r/50257/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 50257: Used LIBPROCESS_SSL_ instead of SSL_ as the prefix for ssl support.

2016-07-20 Thread Joris Van Remoortere

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


Fix it, then Ship it!





3rdparty/libprocess/src/openssl.cpp (lines 306 - 308)


Do you want to report an error or warning if both are set but the values 
are different?



3rdparty/libprocess/src/tests/ssl_tests.cpp (line 323)


`LIBPROCESS_` missing.
Consider emphasising `IS`?



3rdparty/libprocess/src/tests/ssl_tests.cpp (line 357)


`LIBPROCESS_` missing.
Same thing with consider emphasizing `IS`.


- Joris Van Remoortere


On July 20, 2016, 8:34 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50257/
> ---
> 
> (Updated July 20, 2016, 8:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-5863
> https://issues.apache.org/jira/browse/MESOS-5863
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used LIBPROCESS_SSL_ instead of SSL_ as the prefix for ssl support.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> a929cc90fc9ee975f3635957518ced4eb70bdfeb 
>   3rdparty/libprocess/src/openssl.hpp 
> 68f88970610293107b8349c216c34a32d5df9105 
>   3rdparty/libprocess/src/openssl.cpp 
> 63916ff66b4daa29120b7e6b12b329b68f746694 
>   3rdparty/libprocess/src/process.cpp 
> 9661386afd4fddd1877d55941fa403afc9230280 
>   3rdparty/libprocess/src/tests/ssl_client.cpp 
> 6b43cfc77681b5aca76da2638443155f682cd92f 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 72432ec8f91eb4d628df73e19edbaceb8a2c0fb9 
> 
> Diff: https://reviews.apache.org/r/50257/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 50200: Made the agent fetch files as the task user.

2016-07-20 Thread Greg Mann


> On July 20, 2016, 5:45 p.m., Gilbert Song wrote:
> > src/tests/fetcher_tests.cpp, line 107
> > 
> >
> > `nobody` seems safe enough since it should exist in all Unix system.
> 
> Joseph Wu wrote:
> Side note: one of our tests creates a user:
> 
> https://github.com/apache/mesos/blob/db305bb3502cc90412bd3548cc6481fd829a5ee5/src/tests/containerizer/isolator_tests.cpp#L1563-L1592

Joseph, thanks for that reference! Interesting... ideally, I _would_ to make a 
new user for the purposes of this test. However, I'm not sure if there's a 
platform-independent way to do this, so it might require an `#ifdef`? OSX 
doesn't have `useradd`, for example. Since both linux and OSX do have the 
"nobody" user, this single piece of code works for those platforms, so it might 
be preferable?


- Greg


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


On July 20, 2016, 8:01 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50200/
> ---
> 
> (Updated July 20, 2016, 8:01 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Joerg Schad.
> 
> 
> Bugs: mesos-5845
> https://issues.apache.org/jira/browse/mesos-5845
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To ensure that a task cannot fetch root-protected
> files from the local filesystem when running as a
> non-root user, this patch changes the fetcher to
> fetch files as the task user.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 0539b0182bd4a7178f1031ab4fee8fc79eda 
>   src/tests/fetcher_tests.cpp d38ce6e750dc828ef5af4a27fac76327cc4cb56c 
> 
> Diff: https://reviews.apache.org/r/50200/diff/
> 
> 
> Testing
> ---
> 
> A new test was added to the fetcher tests: 
> `FetcherTest.ROOT_RootProtectedFileURI`.
> 
> `sudo make check` was used to test on both OSX and CentOS 7.
> 
> Note that two of the fetcher tests fail for me when run as root on OSX. I saw 
> the following on my OSX 10.10.5 system:
> ```
> [  FAILED  ] FetcherCacheTest.LocalUncachedExtract
> [  FAILED  ] FetcherCacheHttpTest.HttpMixed
> ```
> 
> These failures are already tracked here: 
> https://issues.apache.org/jira/browse/MESOS-4890
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 50257: Used LIBPROCESS_SSL_ instead of SSL_ as the prefix for ssl support.

2016-07-20 Thread Jie Yu

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

(Updated July 20, 2016, 8:34 p.m.)


Review request for mesos and Joris Van Remoortere.


Changes
---

Fixed the tests.


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


Repository: mesos


Description
---

Used LIBPROCESS_SSL_ instead of SSL_ as the prefix for ssl support.


Diffs (updated)
-

  3rdparty/libprocess/include/process/ssl/gtest.hpp 
a929cc90fc9ee975f3635957518ced4eb70bdfeb 
  3rdparty/libprocess/src/openssl.hpp 68f88970610293107b8349c216c34a32d5df9105 
  3rdparty/libprocess/src/openssl.cpp 63916ff66b4daa29120b7e6b12b329b68f746694 
  3rdparty/libprocess/src/process.cpp 9661386afd4fddd1877d55941fa403afc9230280 
  3rdparty/libprocess/src/tests/ssl_client.cpp 
6b43cfc77681b5aca76da2638443155f682cd92f 
  3rdparty/libprocess/src/tests/ssl_tests.cpp 
72432ec8f91eb4d628df73e19edbaceb8a2c0fb9 

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


Testing (updated)
---

make check


Thanks,

Jie Yu



Re: Review Request 48693: Added tests for `UUID::fromString`.

2016-07-20 Thread Anand Mazumdar

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


Ship it!




Ship It!

- Anand Mazumdar


On June 21, 2016, 12:14 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48693/
> ---
> 
> (Updated June 21, 2016, 12:14 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests for `UUID::fromString`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/uuid_tests.cpp 
> b061b827e2461c2c8216c0c00099b19ad486d351 
> 
> Diff: https://reviews.apache.org/r/48693/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 48692: Cleaned up header includes in stout/uuid.

2016-07-20 Thread Anand Mazumdar

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


Ship it!




LGTM though I can understand why it was done previously this way for 
convenience.

- Anand Mazumdar


On June 21, 2016, 12:14 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48692/
> ---
> 
> (Updated June 21, 2016, 12:14 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up header includes in stout/uuid.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/uuid.hpp 
> a57896c78a35de677546ff7da116d669cfad1d66 
> 
> Diff: https://reviews.apache.org/r/48692/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 50199: Refactored fetcher cache directory creation.

2016-07-20 Thread Joerg Schad


> On July 20, 2016, 8:14 a.m., Joerg Schad wrote:
> > src/launcher/fetcher.cpp, line 514
> > 
> >
> > I personally would still use this cacheDirectory as parameter to 
> > createCacheDirectory for the following reason: we need to extract it anyhow 
> > and otherwise we do that at two different places. Feel free to drop if you 
> > have a different opinion.
> 
> Greg Mann wrote:
> With the current diff, the only extra work done in `createCacheDirectory` 
> is `if (fetcherInfo.has_cache_directory())`, which seems OK to me. I think 
> that we could definitely refactor the various fetching functions in this file 
> to clean things up, but perhaps we could do that in separate patches.

It is no issue :-), it just felt weird when reading the code that you first 
create the directory and afterwards extract the directory name from the 
protobuf.


- Joerg


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


On July 20, 2016, 7:50 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50199/
> ---
> 
> (Updated July 20, 2016, 7:50 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Joerg Schad.
> 
> 
> Bugs: MESOS-5845
> https://issues.apache.org/jira/browse/MESOS-5845
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The fetcher's launcher creates the fetcher cache
> directory for each user immediately before an artifact
> is fetched. In order to allow this directory to be
> created by a different user than the user doing the
> fetching, this patch factors out this directory
> creation and places it before all fetches occur.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 0539b0182bd4a7178f1031ab4fee8fc79eda 
> 
> Diff: https://reviews.apache.org/r/50199/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on both OSX and CentOS 7 was done at the end of this patch 
> chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 50200: Made the agent fetch files as the task user.

2016-07-20 Thread Greg Mann

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

(Updated July 20, 2016, 8:01 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Joerg Schad.


Changes
---

Addressed comment.


Bugs: mesos-5845
https://issues.apache.org/jira/browse/mesos-5845


Repository: mesos


Description
---

To ensure that a task cannot fetch root-protected
files from the local filesystem when running as a
non-root user, this patch changes the fetcher to
fetch files as the task user.


Diffs (updated)
-

  src/launcher/fetcher.cpp 0539b0182bd4a7178f1031ab4fee8fc79eda 
  src/tests/fetcher_tests.cpp d38ce6e750dc828ef5af4a27fac76327cc4cb56c 

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


Testing
---

A new test was added to the fetcher tests: 
`FetcherTest.ROOT_RootProtectedFileURI`.

`sudo make check` was used to test on both OSX and CentOS 7.

Note that two of the fetcher tests fail for me when run as root on OSX. I saw 
the following on my OSX 10.10.5 system:
```
[  FAILED  ] FetcherCacheTest.LocalUncachedExtract
[  FAILED  ] FetcherCacheHttpTest.HttpMixed
```

These failures are already tracked here: 
https://issues.apache.org/jira/browse/MESOS-4890


Thanks,

Greg Mann



Re: Review Request 50249: Modified 'agent' to accecpt request with charset in Content-Type.

2016-07-20 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [50246, 50249]

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

- Mesos ReviewBot


On July 20, 2016, 6:13 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50249/
> ---
> 
> (Updated July 20, 2016, 6:13 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5858
> https://issues.apache.org/jira/browse/MESOS-5858
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Earlier operator v1 api was not supporting charset utf-8
> specified in 'Content Type'. In this patch, mesos agent
> is modified to support contentType with and without
> charset utf-8 specified.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 2cbf3c57e294f0d47d64915371b4a6bbf9cdfc0a 
>   src/slave/slave.hpp ffe4220c9289419ab1b1a2c1f499b6eac3c01e4b 
> 
> Diff: https://reviews.apache.org/r/50249/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo GTEST_FILTER="*AgentAPITest.*" make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>



Re: Review Request 50199: Refactored fetcher cache directory creation.

2016-07-20 Thread Greg Mann

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

(Updated July 20, 2016, 7:50 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Joerg Schad.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

The fetcher's launcher creates the fetcher cache
directory for each user immediately before an artifact
is fetched. In order to allow this directory to be
created by a different user than the user doing the
fetching, this patch factors out this directory
creation and places it before all fetches occur.


Diffs (updated)
-

  src/launcher/fetcher.cpp 0539b0182bd4a7178f1031ab4fee8fc79eda 

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


Testing
---

`sudo make check` on both OSX and CentOS 7 was done at the end of this patch 
chain.


Thanks,

Greg Mann



Re: Review Request 50199: Refactored fetcher cache directory creation.

2016-07-20 Thread Greg Mann


> On July 20, 2016, 8:14 a.m., Joerg Schad wrote:
> > src/launcher/fetcher.cpp, line 436
> > 
> >
> > Do we really want to check and fail on this condition? In the previous 
> > iteration you would have skipped creating the directory in that case.
> > Also judging from the code ``  const Option cacheDirectory =
> > fetcherInfo.get().has_cache_directory() ?
> >   Option::some(fetcherInfo.get().cache_directory()) :
> > Option::none();``` below, it seems it is valid that 
> > there is no cache_directory.
> 
> Joerg Schad wrote:
> just seen your comment on the previous patch set. Does it in that case 
> make sense to add a check and change the logic of the above mentioned code?

So actually, I do think that I should remove this check :) sorry for the 
confusion...

I'm inclined to leave the code that you mentioned as-is; while our current 
implementation does guarantee that `FetcherInfo` will always contain a cache 
directory, there's no reason that this _must_ be the case if the cache is being 
bypassed, so I don't think we should enforce it in the fetcher binary itself. 
Since `fetchThroughCache` performs the necessary check when the cache _is_ 
being used, I think the code you quoted is OK. However, in order to make it so 
the fetcher binary does not assume that the cache directory is always included, 
I should remove this CHECK.


> On July 20, 2016, 8:14 a.m., Joerg Schad wrote:
> > src/launcher/fetcher.cpp, line 510
> > 
> >
> > Does the name `result` make sense at this point?
> > Maybe something like `cacheDirectory`?

I think this is OK, since `result` doesn't contain the cache directory itself, 
the `Try` is just used to propagate an error. I wonder if an `Option` 
would be better?


> On July 20, 2016, 8:14 a.m., Joerg Schad wrote:
> > src/launcher/fetcher.cpp, line 438
> > 
> >
> > This comment should move one line down or? (as you now have the create 
> > logic in the loop as well).

I expanded this comment a bit so that it makes sense (I hope) in its current 
location.


> On July 20, 2016, 8:14 a.m., Joerg Schad wrote:
> > src/launcher/fetcher.cpp, line 514
> > 
> >
> > I personally would still use this cacheDirectory as parameter to 
> > createCacheDirectory for the following reason: we need to extract it anyhow 
> > and otherwise we do that at two different places. Feel free to drop if you 
> > have a different opinion.

With the current diff, the only extra work done in `createCacheDirectory` is 
`if (fetcherInfo.has_cache_directory())`, which seems OK to me. I think that we 
could definitely refactor the various fetching functions in this file to clean 
things up, but perhaps we could do that in separate patches.


- Greg


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


On July 20, 2016, 7:50 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50199/
> ---
> 
> (Updated July 20, 2016, 7:50 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Joerg Schad.
> 
> 
> Bugs: MESOS-5845
> https://issues.apache.org/jira/browse/MESOS-5845
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The fetcher's launcher creates the fetcher cache
> directory for each user immediately before an artifact
> is fetched. In order to allow this directory to be
> created by a different user than the user doing the
> fetching, this patch factors out this directory
> creation and places it before all fetches occur.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 0539b0182bd4a7178f1031ab4fee8fc79eda 
> 
> Diff: https://reviews.apache.org/r/50199/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on both OSX and CentOS 7 was done at the end of this patch 
> chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Review Request 50259: Updated docker recovery to use abstraction provided by docker inspect.

2016-07-20 Thread Rajat Phull

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

Review request for mesos, Benjamin Mahler, Kevin Klues, Yubo Li, and Vikrama 
Ditya.


Bugs: Mesos-5795
https://issues.apache.org/jira/browse/Mesos-5795


Repository: mesos


Description
---

Updated docker recovery to use abstraction provided by docker inspect.

Builds on top of reviewrequest #50258


Diffs
-

  src/slave/containerizer/docker.hpp 43ca4317d608b3b43dd7bd0d1b55c721e7364885 
  src/slave/containerizer/docker.cpp f1ecf3b25d85597f6c3dcaa47968860ed119dbd5 

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


Testing
---

GTEST_FILTER="NvidiaGpuDockerContainerizerTest.ROOT_DOCKER_LaunchWithGpuRecovery"
 make -j check


Thanks,

Rajat Phull



Review Request 50260: Documented the executor envirment variable 'MESOS_SANDBOX'.

2016-07-20 Thread Gilbert Song

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

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


Repository: mesos


Description
---

Documented the executor envirment variable 'MESOS_SANDBOX'.


Diffs
-

  docs/executor-http-api.md 17ba6f6a0ecbd271e6b1739ffaf92fba83c0fac1 

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


Testing
---

gist


Thanks,

Gilbert Song



Review Request 50258: Updated docker recovery to account for GPU resources.

2016-07-20 Thread Rajat Phull

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

Review request for mesos, Benjamin Mahler, Kevin Klues, Yubo Li, and Vikrama 
Ditya.


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


Repository: mesos


Description
---

This change is to account for allocated GPU devices during docker recovery 
invoked as part of slave recovery.
On docker container recovery, the list of devices is taken from the docker 
inspect output. The nvidia 
devices of the format /dev/nvidia# are further narrowed down to get the minor 
number of the GPU.
The allocation module is invoked to account for the GPUs used by the recovered 
containers.

Note: The next review request #50259 builds on top of this and uses the 
abstraction provided by docker inspect


Diffs
-

  src/slave/containerizer/docker.hpp 43ca4317d608b3b43dd7bd0d1b55c721e7364885 
  src/slave/containerizer/docker.cpp f1ecf3b25d85597f6c3dcaa47968860ed119dbd5 
  src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
7944c0da716b2438fa2b40f5504aefa346e31046 

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


Testing
---

GTEST_FILTER="NvidiaGpuDockerContainerizerTest.ROOT_DOCKER_LaunchWithGpuRecovery"
 make -j check


Thanks,

Rajat Phull



Review Request 50254: Added an example test for the V0/V1 Mesos java implementation.

2016-07-20 Thread Anand Mazumdar

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

Review request for mesos, Joris Van Remoortere and Vinod Kone.


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


Repository: mesos


Description
---

Added an example test for the V0/V1 Mesos java implementation.


Diffs
-

  configure.ac d2136909b7305498ae901a5ea00133142b77f9e6 
  src/Makefile.am cc83525a4455bbb0e654d346921d66ed2436411d 
  src/examples/java/V1TestFramework.java PRE-CREATION 
  src/examples/java/v1-test-framework.in PRE-CREATION 
  src/tests/examples_tests.cpp ac513ce9aa3c8f366fe81ba937e3dc0d51a26940 
  src/tests/java_v0_framework_test.sh PRE-CREATION 
  src/tests/java_v1_framework_test.sh PRE-CREATION 

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


Testing
---

make check


Thanks,

Anand Mazumdar



Review Request 50253: Added native implementation for the V0 Mesos Adapter.

2016-07-20 Thread Anand Mazumdar

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

Review request for mesos, Joris Van Remoortere and Vinod Kone.


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


Repository: mesos


Description
---

This change adds the C++ implementation for the JAVA V0 to V1 Mesos
implementation adapter (driver + scheduler).


Diffs
-

  src/Makefile.am cc83525a4455bbb0e654d346921d66ed2436411d 
  src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Anand Mazumdar



Review Request 50251: Added java implementations for the V0/V1 implementation for Mesos.

2016-07-20 Thread Anand Mazumdar

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

Review request for mesos, Joris Van Remoortere and Vinod Kone.


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


Repository: mesos


Description
---

Added java implementations for the V0/V1 implementation for Mesos.


Diffs
-

  src/Makefile.am cc83525a4455bbb0e654d346921d66ed2436411d 
  src/java/src/org/apache/mesos/v1/scheduler/JNIMesos.java PRE-CREATION 
  src/java/src/org/apache/mesos/v1/scheduler/V0Mesos.java PRE-CREATION 

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


Testing
---

make check


Thanks,

Anand Mazumdar



Review Request 50252: Added native implementation for v1 Mesos interface.

2016-07-20 Thread Anand Mazumdar

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

Review request for mesos, Joris Van Remoortere and Vinod Kone.


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


Repository: mesos


Description
---

This change adds the native C++ implementation for the v1
Java class `JNIMesos` used for interacting with Mesos.


Diffs
-

  src/Makefile.am cc83525a4455bbb0e654d346921d66ed2436411d 
  src/java/jni/org_apache_mesos_v1_scheduler_JNIMesos.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Anand Mazumdar



Review Request 50250: Added v1 Scheduler/Mesos interface in Java.

2016-07-20 Thread Anand Mazumdar

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

Review request for mesos, Joris Van Remoortere and Vinod Kone.


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


Repository: mesos


Description
---

This change adds an interface for the v1 Scheduler + Mesos
interface that the schedulers can use to connect to Mesos.


Diffs
-

  src/Makefile.am cc83525a4455bbb0e654d346921d66ed2436411d 
  src/java/src/org/apache/mesos/v1/scheduler/Mesos.java PRE-CREATION 
  src/java/src/org/apache/mesos/v1/scheduler/Scheduler.java PRE-CREATION 

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


Testing
---

make check


Thanks,

Anand Mazumdar



Review Request 50247: Added a abstract base class for scheduler library.

2016-07-20 Thread Anand Mazumdar

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

Review request for mesos, Joris Van Remoortere and Vinod Kone.


Repository: mesos


Description
---

This change adds an abstract base class `MesosBase` that
would be used by implementations for interacting with
Mesos via the Scheduler API. This is needed later for
implementing the V0->V1 Scheduler Adapter.


Diffs
-

  include/mesos/v1/scheduler.hpp 18e7a95f8342ea155f9e339945b05810b6bd82b5 

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


Testing
---

make check


Thanks,

Anand Mazumdar



Review Request 50248: Added helper functions for v1 JNI `construct()`/`convert()`.

2016-07-20 Thread Anand Mazumdar

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

Review request for mesos, Joris Van Remoortere and Vinod Kone.


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


Repository: mesos


Description
---

These would be used later in the chain for the making JNI
calls to v1 Mesos implementation in native code.


Diffs
-

  src/java/jni/construct.cpp e5c070c819698729780b487ebd214ee43f15ae87 
  src/java/jni/convert.cpp 45ff488b921c0254761ec0a7b5c06608aa86ca3f 

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


Testing
---

make check


Thanks,

Anand Mazumdar



Review Request 50257: Used LIBPROCESS_SSL_ instead of SSL_ as the prefix for ssl support.

2016-07-20 Thread Jie Yu

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

Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description
---

Used LIBPROCESS_SSL_ instead of SSL_ as the prefix for ssl support.


Diffs
-

  3rdparty/libprocess/include/process/ssl/gtest.hpp 
a929cc90fc9ee975f3635957518ced4eb70bdfeb 
  3rdparty/libprocess/src/openssl.hpp 68f88970610293107b8349c216c34a32d5df9105 
  3rdparty/libprocess/src/openssl.cpp 63916ff66b4daa29120b7e6b12b329b68f746694 
  3rdparty/libprocess/src/process.cpp 9661386afd4fddd1877d55941fa403afc9230280 
  3rdparty/libprocess/src/tests/ssl_client.cpp 
6b43cfc77681b5aca76da2638443155f682cd92f 
  3rdparty/libprocess/src/tests/ssl_tests.cpp 
72432ec8f91eb4d628df73e19edbaceb8a2c0fb9 

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


Testing
---

The test does not pass at the moment because some potential bug in ssl path.


Thanks,

Jie Yu



Re: Review Request 49814: Implemented `CgroupsIsolatorProcess::create`.

2016-07-20 Thread haosdent huang

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

(Updated July 20, 2016, 6:36 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qianzhang's comments.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::create`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang



Re: Review Request 49825: Implemented `CgroupsIsolatorProcess::status`.

2016-07-20 Thread haosdent huang

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

(Updated July 20, 2016, 6:39 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qianzhang's comments.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::status`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang



Re: Review Request 49824: Implemented `CgroupsIsolatorProcess::usage`.

2016-07-20 Thread haosdent huang

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

(Updated July 20, 2016, 6:38 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qianzhang's comments.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::usage`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang



Re: Review Request 49823: Implemented `CgroupsIsolatorProcess::update`.

2016-07-20 Thread haosdent huang

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

(Updated July 20, 2016, 6:38 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qianzhang's comments.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::update`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang



Re: Review Request 49821: Implemented `CgroupsIsolatorProcess::watch`.

2016-07-20 Thread haosdent huang

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

(Updated July 20, 2016, 6:38 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qianzhang's comments.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::watch`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang



Re: Review Request 49820: Implemented `CgroupsIsolatorProcess::isolate`.

2016-07-20 Thread haosdent huang

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

(Updated July 20, 2016, 6:37 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qianzhang's comments.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::isolate`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang



Re: Review Request 49819: Implemented `CgroupsIsolatorProcess::prepare`.

2016-07-20 Thread haosdent huang

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

(Updated July 20, 2016, 6:37 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qianzhang's comments.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::prepare`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang



Re: Review Request 49817: Implemented `CgroupsIsolatorProcess::recover`.

2016-07-20 Thread haosdent huang

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

(Updated July 20, 2016, 6:36 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::recover`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang



Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in master.

2016-07-20 Thread Greg Mann

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




src/master/flags.cpp (lines 225 - 226)


Enclose the flag names in backticks



src/master/flags.cpp (line 238)


Do we really want `--authenticate_http` to be an alias for the readonly 
flag as well? Since `--authenticate_http` only covered the readwrite endpoints 
in 0.28, perhaps we should only use the alias for the readwrite flag.



src/master/main.cpp (line 278)


I think here, I would advocate extending the READONLY/READWRITE distinction 
to libprocess as well. `libprocess::initialize` could accept two realms, one 
for readonly endpoints and the other for readwrite.



src/master/master.hpp (line 1895)


Can we use `string&` here instead of `const char*`?



src/master/master.cpp (lines 418 - 420)


Fits on one line?



src/tests/cluster.hpp (line 105)


The `/files/*` endpoints should belong to the readonly realm, right?


- Greg Mann


On July 20, 2016, 3:21 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50223/
> ---
> 
> (Updated July 20, 2016, 3:21 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Bannier, Greg Mann, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-5851
> https://issues.apache.org/jira/browse/MESOS-5851
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes included:
> - separate flags for readonly and readwrite endpoints;
> - helper function for registering http authenticator;
> - fixing existing tests.
> 
> Open question:
> - location and name of helper function.
> 
> Next step:
> - test for readonly endpoints;
> - similar change to agent endpoints;
> - docs/configuration.md fix;
> - docs/endpoints/index.md fix.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp a543aef117fea62660d55435be4d66d30f8ee860 
>   src/master/constants.hpp 410c388c8f8ad98777c6587fc0b06807639e782a 
>   src/master/flags.hpp a5dd11b624d19a9ea3508031ded4c0199098afd1 
>   src/master/flags.cpp ca3e80bf9467328892be89718e5e0a1a05264ab8 
>   src/master/main.cpp 9775b8a1e5fe51670789805557339bd0737a02b7 
>   src/master/master.hpp 6decff6f4b9c3434de030fd5c06df4c683a7abad 
>   src/master/master.cpp 370fd8712062dc75bb81824cb99ccc7920acbf78 
>   src/tests/cluster.hpp 55dbaaef6c703676859e39e50bb1c31b942d0929 
>   src/tests/cluster.cpp e1be275fccf130181ed18fd1c5a1b7b18979d7aa 
>   src/tests/dynamic_weights_tests.cpp 
> 6aa0102d3821c1b9d09672c706d6d8850f3729c3 
>   src/tests/master_quota_tests.cpp 639f4c4146c61c0713e2945fea4fd6ffe1f3e726 
>   src/tests/mesos.cpp d073d79c5797ecb021f0294ab6586a000f3ca600 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> fdd10a76dbfaf8bcae021b9d8b976f43748117fe 
>   src/tests/reservation_endpoints_tests.cpp 
> 48c002d1dc371c285b9421ef5a2c57250d270fa8 
> 
> Diff: https://reviews.apache.org/r/50223/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>



Review Request 50249: Modified 'agent' to accecpt request with charset in Content-Type.

2016-07-20 Thread Abhishek Dasgupta

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

Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


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


Repository: mesos


Description
---

Earlier operator v1 api was not supporting charset utf-8
specified in 'Content Type'. In this patch, mesos agent
is modified to support contentType with and without
charset utf-8 specified.


Diffs
-

  src/slave/http.cpp 2cbf3c57e294f0d47d64915371b4a6bbf9cdfc0a 
  src/slave/slave.hpp ffe4220c9289419ab1b1a2c1f499b6eac3c01e4b 

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


Testing
---

On Ubuntu 16.04:
sudo GTEST_FILTER="*AgentAPITest.*" make -j4 check


Thanks,

Abhishek Dasgupta



Re: Review Request 50246: Modified 'master' to accecpt request with charset in Content-Type.

2016-07-20 Thread Abhishek Dasgupta

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

(Updated July 20, 2016, 6:11 p.m.)


Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


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


Repository: mesos


Description (updated)
---

Earlier operator v1 api was not supporting charset utf-8
specified in 'Content Type'. In this patch, mesos master
is modified to support contentType with and without
charset utf-8 specified.


Diffs
-

  src/master/http.cpp 1cbec976ccedae81def388c62889624aaf9a794e 
  src/master/master.hpp 6decff6f4b9c3434de030fd5c06df4c683a7abad 

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


Testing
---

On Ubuntu 16.04:
sudo GTEST_FILTER="*MasterAPITest.*" make -j4 check

Manual Testing:
1. sudo  ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
2. Ran Poster application in firefox and sent a POST data for GET_VERSION call 
to http://127.0.0.1:5050/api/v1 with contentType as application/json; 
charset=utf-8


Thanks,

Abhishek Dasgupta



Review Request 50246: Modified 'master' to accecpt request with charset in Content-Type.

2016-07-20 Thread Abhishek Dasgupta

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

Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


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


Repository: mesos


Description
---

Modified 'master' to accecpt request with charset in Content-Type.


Diffs
-

  src/master/http.cpp 1cbec976ccedae81def388c62889624aaf9a794e 
  src/master/master.hpp 6decff6f4b9c3434de030fd5c06df4c683a7abad 

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


Testing
---

On Ubuntu 16.04:
sudo GTEST_FILTER="*MasterAPITest.*" make -j4 check

Manual Testing:
1. sudo  ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
2. Ran Poster application in firefox and sent a POST data for GET_VERSION call 
to http://127.0.0.1:5050/api/v1 with contentType as application/json; 
charset=utf-8


Thanks,

Abhishek Dasgupta



Re: Review Request 49348: Added implementation to Appc Runtime Isolator.

2016-07-20 Thread Gilbert Song

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


Ship it!




This is great. Thanks, Srini!

- Gilbert Song


On July 19, 2016, 2:11 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49348/
> ---
> 
> (Updated July 19, 2016, 2:11 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added implementation to Appc Runtime Isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/appc/runtime.cpp 
> 19c68e8a25d9ceedc5dfd562e287d6b6a56a9d3a 
> 
> Diff: https://reviews.apache.org/r/49348/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>



Re: Review Request 50200: Made the agent fetch files as the task user.

2016-07-20 Thread Joseph Wu


> On July 20, 2016, 10:45 a.m., Gilbert Song wrote:
> > src/tests/fetcher_tests.cpp, line 107
> > 
> >
> > `nobody` seems safe enough since it should exist in all Unix system.

Side note: one of our tests creates a user:
https://github.com/apache/mesos/blob/db305bb3502cc90412bd3548cc6481fd829a5ee5/src/tests/containerizer/isolator_tests.cpp#L1563-L1592


- Joseph


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


On July 19, 2016, 4:10 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50200/
> ---
> 
> (Updated July 19, 2016, 4:10 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Joerg Schad.
> 
> 
> Bugs: mesos-5845
> https://issues.apache.org/jira/browse/mesos-5845
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To ensure that a task cannot fetch root-protected
> files from the local filesystem when running as a
> non-root user, this patch changes the fetcher to
> fetch files as the task user.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 0539b0182bd4a7178f1031ab4fee8fc79eda 
>   src/tests/fetcher_tests.cpp d38ce6e750dc828ef5af4a27fac76327cc4cb56c 
> 
> Diff: https://reviews.apache.org/r/50200/diff/
> 
> 
> Testing
> ---
> 
> A new test was added to the fetcher tests: 
> `FetcherTest.ROOT_RootProtectedFileURI`.
> 
> `sudo make check` was used to test on both OSX and CentOS 7.
> 
> Note that two of the fetcher tests fail for me when run as root on OSX. I saw 
> the following on my OSX 10.10.5 system:
> ```
> [  FAILED  ] FetcherCacheTest.LocalUncachedExtract
> [  FAILED  ] FetcherCacheHttpTest.HttpMixed
> ```
> 
> These failures are already tracked here: 
> https://issues.apache.org/jira/browse/MESOS-4890
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

2016-07-20 Thread Gilbert Song

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


Fix it, then Ship it!





src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 199)


Not sure how the indentation should be here, since we do it differently in 
our code base. So I will give a ship it now.


- Gilbert Song


On July 19, 2016, 2:12 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated July 19, 2016, 2:12 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1c9e17b23d7f2ee49e00b80053e4de1797d73c73 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 6825742546a87c8148097e6e13a94592e3b6754e 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>



Re: Review Request 50003: Propagate work_dir flag from local runs to agents/masters.

2016-07-20 Thread Greg Mann

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




src/local/flags.hpp (line 50)


Hi Ammar: sorry for the extra comment here, but I actually just discovered 
the helper function `os::temp()`. We use it 
[here](https://github.com/apache/mesos/blob/master/src/slave/flags.cpp#L129). 
Since it looks like this is our accepted way to set default temporary 
locations, we should follow that pattern here as well.


- Greg Mann


On July 20, 2016, 5:10 a.m., Ammar Askar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50003/
> ---
> 
> (Updated July 20, 2016, 5:10 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-5613
> https://issues.apache.org/jira/browse/MESOS-5613
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Propagate work_dir flag from local runs to agents/masters.
> 
> 
> Diffs
> -
> 
>   src/local/flags.hpp f0af0d2 
>   src/local/local.cpp a543aef 
> 
> Diff: https://reviews.apache.org/r/50003/diff/
> 
> 
> Testing
> ---
> 
> Manually tested that `mesos local` and `mesos-local` run now. Not sure if a 
> regression test for this would be good, please advise.
> 
> 
> Thanks,
> 
> Ammar Askar
> 
>



Re: Review Request 50200: Made the agent fetch files as the task user.

2016-07-20 Thread Gilbert Song

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


Fix it, then Ship it!





src/tests/fetcher_tests.cpp (line 107)


`nobody` seems safe enough since it should exist in all Unix system.



src/tests/fetcher_tests.cpp (lines 115 - 117)


Let's remove them if not used.


- Gilbert Song


On July 19, 2016, 4:10 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50200/
> ---
> 
> (Updated July 19, 2016, 4:10 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Joerg Schad.
> 
> 
> Bugs: mesos-5845
> https://issues.apache.org/jira/browse/mesos-5845
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To ensure that a task cannot fetch root-protected
> files from the local filesystem when running as a
> non-root user, this patch changes the fetcher to
> fetch files as the task user.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 0539b0182bd4a7178f1031ab4fee8fc79eda 
>   src/tests/fetcher_tests.cpp d38ce6e750dc828ef5af4a27fac76327cc4cb56c 
> 
> Diff: https://reviews.apache.org/r/50200/diff/
> 
> 
> Testing
> ---
> 
> A new test was added to the fetcher tests: 
> `FetcherTest.ROOT_RootProtectedFileURI`.
> 
> `sudo make check` was used to test on both OSX and CentOS 7.
> 
> Note that two of the fetcher tests fail for me when run as root on OSX. I saw 
> the following on my OSX 10.10.5 system:
> ```
> [  FAILED  ] FetcherCacheTest.LocalUncachedExtract
> [  FAILED  ] FetcherCacheHttpTest.HttpMixed
> ```
> 
> These failures are already tracked here: 
> https://issues.apache.org/jira/browse/MESOS-4890
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 50177: Add systemd watchdog support.

2016-07-20 Thread David Robinson

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



Overall it LGTM.


configure.ac (line 615)


This doesn't read correctly and it sort of states the obvious, consider 
simplying it:

# Check for systemd and link it in if present.



src/linux/systemd.cpp (lines 198 - 200)


The terminology here is a little confusing. The 'watchdog daemon' is the 
process running this function, and we're not pinging the watchdog we're sending 
a message to systemd. Consider rewording, eg:

// If the WATCHDOG_USEC environment variable is set systemd expects a 
watchdog message every WATCHDOG_USEC / 2 microseconds. The message indicates 
liveliness to systemd.


- David Robinson


On July 19, 2016, 1:14 a.m., Lawrence Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50177/
> ---
> 
> (Updated July 19, 2016, 1:14 a.m.)
> 
> 
> Review request for mesos, David Robinson and Ian Downes.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds systemd watchdog support (see 
> http://0pointer.de/blog/projects/watchdog.html for context).
> 
> 
> Diffs
> -
> 
>   configure.ac d2136909b7305498ae901a5ea00133142b77f9e6 
>   src/linux/systemd.hpp 91134f1d4b100759e45931bd09ca4e1e1aeaaf8a 
>   src/linux/systemd.cpp 619aa2778da5f99d3a078a8e1208bdaa9dc77581 
>   src/slave/main.cpp 4624392d30cf391015dcd63f447fe2414a47a16a 
> 
> Diff: https://reviews.apache.org/r/50177/diff/
> 
> 
> Testing
> ---
> 
> Tested by sending SIGSTOP to running mesos and verifying via journalctl that 
> it was killed by the watchdog.
> 
> TODO: write actual test by using and forking a mock binary that initializes 
> systemd and sending SIGSTOP to that binary. test also needs a unit file for 
> that binary and for systemd to run so we can verify that systemd killed it.
> 
> 
> File Attachments
> 
> 
> mesos gets owned by watchdog
>   
> https://reviews.apache.org/media/uploaded/files/2016/07/19/3c73b595-d28e-45d4-adfe-697801ba02cc__Screen_Shot_2016-07-18_at_2.09.31_PM.png
> 
> 
> Thanks,
> 
> Lawrence Wu
> 
>



Re: Review Request 46626: Added example framework for testing disk quota enforcement.

2016-07-20 Thread Artem Harutyunyan


> On July 19, 2016, 1:40 p.m., Joseph Wu wrote:
> > LGTM, modulo the examples tests patch, which is presumably coming up next :)

https://reviews.apache.org/r/50217/


- Artem


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


On July 19, 2016, 6:23 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46626/
> ---
> 
> (Updated July 19, 2016, 6:23 p.m.)
> 
> 
> Review request for Joseph Wu.
> 
> 
> Bugs: MESOS-5855
> https://issues.apache.org/jira/browse/MESOS-5855
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added example framework for testing disk quota enforcement.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am cc83525a4455bbb0e654d346921d66ed2436411d 
>   src/examples/disk_full_framework.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46626/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>



  1   2   >