Re: Review Request 50857: Modified a scheduler test to run with SSL enabled.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45991, 45995, 45996, 45999, 46094, 46146, 40266, 50621, 
40410, 40411, 40413, 40268, 40512, 50736, 50737, 50969, 50857]

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 Aug. 10, 2016, 10:37 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50857/
> ---
> 
> (Updated Aug. 10, 2016, 10:37 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-3753
> https://issues.apache.org/jira/browse/MESOS-3753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the test `SchedulerTest.Teardown` to
> be parametrized by both `ContentType` and SSL configuration,
> and renames it to `SchedulerSSLTest.RunTaskAndTeardown`.
> This allows the test to verify the scheduler's behavior with
> SSL both enabled and disabled.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp ccd0f2f602a7a1a9a44b68fd0f59bdc8e0fa58b1 
> 
> Diff: https://reviews.apache.org/r/50857/diff/
> 
> 
> Testing
> ---
> 
> This test is currently flaky in the SSL-enabled configurations and will 
> produce a segfault on my OSX machine after 50-100 repetitions. To reproduce, 
> try:
> 
> `GTEST_REPEAT=1000 GTEST_BREAK_ON_FAILURE=1 
> GTEST_FILTER="ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/2" 
> bin/mesos-tests.sh`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 49571: Added a benchmark test for allocations.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45960, 45961, 45962, 45963, 45964, 45966, 45967, 49571]

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 Aug. 10, 2016, 11:09 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49571/
> ---
> 
> (Updated Aug. 10, 2016, 11:09 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5771
> https://issues.apache.org/jira/browse/MESOS-5771
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allocations test has the following resource configurations:
> (1) REGULAR: Offers from every slave have regular resources.
> (2) SHARED: Offers from every slave include a shared resource.
> (3) REGULAR: Offers from every alternate slave contain only regular
> resources; and offers from every other alternate slave contains
> a shared resource.
> 
> This test is parameterized based on number of agents, number of
> frameworks and resource configuration.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> cbed333f497016fe2811f755028796012b41db77 
> 
> Diff: https://reviews.apache.org/r/49571/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> Allocations benchmark test results
> ==
> Support of shared resources has a small impact (roughly 5%-7%) on runtime 
> performance in allocations as compared to HEAD (without shared resources). 
> Also, there is no visible impact in performance when shared resources are 
> added in the tests.
> 
> Following is a snapshot with 1000 agents and 200 frameworks.
> 
> With the patch (and no shared resources)
> 
> [ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/9
> Using 1000 agents and 200 frameworks with resource type 0
> Added 200 frameworks in 6572us
> Added 1000 agents in 1.820284secs
> round 0 allocate() took 1.602476secs to make 1000 offers
> round 50 allocate() took 1.586638secs to make 1000 offers
> round 100 allocate() took 1.588735secs to make 1000 offers
> round 150 allocate() took 1.581553secs to make 1000 offers
> round 199 allocate() took 1.595088secs to make 1000 offers
> 
> With the patch (and shared resources on all agents)
> ---
> [ RUN  ] 
> AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/10
> Using 1000 agents and 200 frameworks with resource type 1
> Added 200 frameworks in 6869us
> Added 1000 agents in 1.847318secs
> round 0 allocate() took 1.606464secs to make 1000 offers
> round 50 allocate() took 1.603727secs to make 1000 offers
> round 100 allocate() took 1.630528secs to make 1000 offers
> round 150 allocate() took 1.60693secs to make 1000 offers
> round 199 allocate() took 1.613754secs to make 1000 offers
> 
> With the patch (and shared resources on alternate agents)
> -
> [ RUN  ] 
> AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/11
> Using 1000 agents and 200 frameworks with resource type 2
> Added 200 frameworks in 6903us
> Added 1000 agents in 1.856714secs
> round 0 allocate() took 1.621012secs to make 1000 offers
> round 50 allocate() took 1.60385secs to make 1000 offers
> round 100 allocate() took 1.64289secs to make 1000 offers
> round 150 allocate() took 1.654631secs to make 1000 offers
> round 199 allocate() took 1.609998secs to make 1000 offers
> 
> Based on HEAD, with all regular resources (no shared resources in HEAD 
> supported)
> -
> [ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/9
> Using 1000 agents and 200 frameworks with resource type 0
> Added 200 frameworks in 6801us
> Added 1000 agents in 1.721447secs
> round 0 allocate() took 1.502953secs to make 1000 offers
> round 50 allocate() took 1.520157secs to make 1000 offers
> round 100 allocate() took 1.517221secs to make 1000 offers
> round 150 allocate() took 1.526446secs to make 1000 offers
> round 199 allocate() took 1.538005secs to make 1000 offers
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

[Alexander Rukletsov]

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




src/health-check/health_checker.hpp (line 275)


It's good to have a default. Let's create a class constant 
`DEFAULT_HTTP_SCHEME`.



src/health-check/health_checker.hpp (lines 276 - 278)


This belongs into the `validate()` function.



src/health-check/health_checker.hpp (line 280)


Do you think we should add "/" if the user has not specified anything?



src/health-check/health_checker.hpp (line 282)


Again, it's great we can deduce the domain for the user, but let's put into 
the class constant, e.g. `DEFAULT_DOMAIN`.



src/health-check/health_checker.hpp (line 304)


We can remove "with reason", what do you think?



src/health-check/health_checker.hpp (lines 320 - 333)


This is used once, maybe it makes sense to replace it with a lambda then? 
The function is short enough, it's declaration is much longer than the function 
itself.



src/health-check/health_checker.hpp (line 330)


Why do you need to discard original future here?



src/health-check/health_checker.hpp (line 332)


It's unclear, what exactly is pending. How about: "CURL has not returned 
after " + stringify(timeout) + "; aborting"



src/health-check/health_checker.hpp (line 356)


We should be consistent: either wrap `curl` in '' everywhere or nowhere.



src/health-check/health_checker.hpp (lines 376 - 377)


Let's swap these conditions : )



src/health-check/health_checker.hpp (lines 392 - 393)


I would omit "with reason", ':' is enough.

Let's make sure we test these paths.



src/health-check/health_checker.cpp (lines 57 - 59)


Let's introduce a separate `validate` function. See my comments in the 
previous review.


- Alexander Rukletsov


On Aug. 7, 2016, 6:21 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated Aug. 7, 2016, 6:21 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
> Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> b28a9cf3c6c9217c0b2453ac6e34d88e1f648d61 
>   src/health-check/health_checker.cpp 
> 585a0b565d948cfa292bad818a710501a4ce0daf 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> * Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
> HealthCheckTest.HealthyTaskNotMatchHttpStatuses
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 43558: Speed up ExamplesTest.PersistentVolumeFramework.

[haosdent huang]

> On Aug. 10, 2016, 7:23 p.m., Benjamin Mahler wrote:
> > src/examples/persistent_volume_framework.cpp, lines 402-410
> > 
> >
> > Can you give some context on why lowering these speeds up the test? How 
> > much does it speed it up?

Hi, @bmahler The most effective snippet is

```
export MESOS_AUTHENTICATION_TIMEOUT=200ms
```

which we committed before. I test this patch again, seems only bring little 
speed up.

Before apply the patch,

```
[   OK ] ExamplesTest.PersistentVolumeFramework (3505 ms)
[--] 1 test from ExamplesTest (3510 ms total)
```

After apply the patch,

```
[   OK ] ExamplesTest.PersistentVolumeFramework (2493 ms)
[--] 1 test from ExamplesTest (2497 ms total)
```

Do you think we should discard this?


- haosdent


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


On Aug. 10, 2016, 5:21 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43558/
> ---
> 
> (Updated Aug. 10, 2016, 5:21 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Jie Yu.
> 
> 
> Bugs: MESOS-4663
> https://issues.apache.org/jira/browse/MESOS-4663
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Speed up ExamplesTest.PersistentVolumeFramework.
> 
> 
> Diffs
> -
> 
>   src/examples/persistent_volume_framework.cpp 
> fe2837cfffb6dd251ccb9c93197f623d0c55fb36 
> 
> Diff: https://reviews.apache.org/r/43558/diff/
> 
> 
> Testing
> ---
> 
> Repeat test in CentOS 7.1
> ```
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="ExamplesTest.PersistentVolumeFramework" --gtest_repeat=200 
> --gtest_break_on_failure
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49351: Updated 'HealthCheck' protobuf validation.

[Alexander Rukletsov]

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




src/docker/executor.cpp (lines 470 - 471)


This block is for command checks only. Let's prepend it with the comment, 
why do we need to do extra movements in this case (wrapping command into 
`docker exec`).



src/health-check/health_checker.hpp 


I like your idea to introduce a function per health check type (what you do 
in r/36816). Do you want to introduce them and do dispatch here?



src/health-check/health_checker.cpp (lines 53 - 59)


Let's extract validation into a separate function/namespace. We can 
additionally use this function to implement `Option 
validateHealthCheck(const HealthCheck& healthCheck);` in the master. I've filed 
MESOS-6025 for this.


- Alexander Rukletsov


On Aug. 7, 2016, 6:20 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49351/
> ---
> 
> (Updated Aug. 7, 2016, 6:20 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
> Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated 'HealthCheck' protobuf validation.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 445628c9164facdd8bd812c5b45e3b2b886ebf0e 
>   src/health-check/health_checker.hpp 
> b28a9cf3c6c9217c0b2453ac6e34d88e1f648d61 
>   src/health-check/health_checker.cpp 
> 585a0b565d948cfa292bad818a710501a4ce0daf 
> 
> Diff: https://reviews.apache.org/r/49351/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 43558: Speed up ExamplesTest.PersistentVolumeFramework.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43558]

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 Aug. 10, 2016, 5:21 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43558/
> ---
> 
> (Updated Aug. 10, 2016, 5:21 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Jie Yu.
> 
> 
> Bugs: MESOS-4663
> https://issues.apache.org/jira/browse/MESOS-4663
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Speed up ExamplesTest.PersistentVolumeFramework.
> 
> 
> Diffs
> -
> 
>   src/examples/persistent_volume_framework.cpp 
> fe2837cfffb6dd251ccb9c93197f623d0c55fb36 
> 
> Diff: https://reviews.apache.org/r/43558/diff/
> 
> 
> Testing
> ---
> 
> Repeat test in CentOS 7.1
> ```
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="ExamplesTest.PersistentVolumeFramework" --gtest_repeat=200 
> --gtest_break_on_failure
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50877: Added more cases for sorter benchmark test.

[Benjamin Mahler]

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


Fix it, then Ship it!





src/tests/sorter_tests.cpp (line 699)


Should we remove this assertion and the one below? Doesn't seem related to 
this code.


- Benjamin Mahler


On Aug. 7, 2016, 10:04 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50877/
> ---
> 
> (Updated Aug. 7, 2016, 10:04 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch is adding more benchmark test cases for sorter include
> the following:
> 
> 1) Added time elapse for unallocated all resources for all clients.
> 2) Added time elapse for removing all agents.
> 3) Added time elapse for removing all clients.
> 
> 
> Diffs
> -
> 
>   src/tests/sorter_tests.cpp d6578c0df9e860087aa0bc57242b6fc302c7063f 
> 
> Diff: https://reviews.apache.org/r/50877/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> The result is that the `unallocated` consumed quite a lot of time, the root 
> cause is that the `contains` is time consuming when `unallocated` resources 
> for clients.
> 
> ```
>  ./bin/mesos-tests.sh --benchmark  
> --gtest_filter="*Sorter_BENCHMARK_Test.FullSort/0"
> [==] 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/0
> Using 1000 agents and 1 clients
> Added 1 clients in 124us
> Added 1000 agents in 29667us
> Added allocations for 1000 agents in 81757us
> Full sort of 1 clients took 86us
> No-op sort of 1 clients took 5us
> Removed allocations for 1000 agents in 5.834111secs
> Removed 1000 agents in 41694us
> Removed 1 clients in 38us
> [   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/0 (5990 ms)
> [--] 1 test from AgentAndClientCount/Sorter_BENCHMARK_Test (5990 ms 
> total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (6012 ms total)
> [  PASSED  ] 1 test.
> ```
> 
> ```
> ./bin/mesos-tests.sh --benchmark  
> --gtest_filter="*Sorter_BENCHMARK_Test.FullSort/35"
> [==] 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/35
> Using 5 agents and 1000 clients
> Added 1000 clients in 21453us
> Added 5 agents in 1.572453secs
> Added allocations for 5 agents in 5.358785secs
> Full sort of 1000 clients took 32856us
> No-op sort of 1000 clients took 347us
> Removed allocations for 5 agents in 5.1378020833mins
> Removed 5 agents in 1.919385secs
> Removed 1000 clients in 21738us
> [   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35 (317208 ms)
> [--] 1 test from AgentAndClientCount/Sorter_BENCHMARK_Test (317208 ms 
> total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (317231 ms total)
> [  PASSED  ] 1 test.
> ```
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Joseph Wu]

> On Aug. 10, 2016, 12:02 p.m., Kevin Klues wrote:
> > support/mesos-style.py, lines 228-234
> > 
> >
> > I know we didn't talk about this, but I realized recently that we 
> > actually *have* to run pylint inside the virtual environment, otherwise it 
> > runs using the system python, which is not what we want. Especially for 
> > import libraries.
> 
> Haris Choudhary wrote:
> There are two approaches to this:
> 
> 1) We can either activate the virtualenv from the CLI and than run 
> pylint. But that means if the virtualenv is not created within the CLI we 
> will have to create it and activate it.
> 
> 2) We can integrate the virtualenv to the project wide bootstrap and thus 
> ensuring the the virtualenv is created for the project on bootstrapping 
> mesos. This seems to be the better way to do it however might require 
> significant changes as opposed to (1). It'd be desirable to have a 
> project-wide virtualenv at some point however even if we choose to not do so 
> right now. A thing to note here is that if we integrate it to the project 
> wide bootstrap, we'll need users to have virtualenv installed for mesos 
> instead of only the CLI.
> 
> Kevin Klues wrote:
> I'd prefer #2, but it's a bigger change and has the downside that the 
> virtualenv dependency will become immediate if we add it to the top-level 
> bootstrap. Curious what @vinodkone thinks.
> 
> Kevin Klues wrote:
> I think this should suffice for now (including fixes to the indentation):
> ```
> cli_dir = os.path.abspath(self.source_dirs[0])
> source_files = ' '.join(source_paths)
> 
> p = subprocess.Popen(
> ['source {virtualenv}/bin/activate; \
>  PYTHONPATH={lib_dir}:{bin_dir} pylint --rcfile={config} 
> --ignore={ignore} {files}'.\
>  format(virtualenv=os.path.join(cli_dir, '.virtualenv'),
> lib_dir=os.path.join(cli_dir, 'lib'),
> bin_dir=os.path.join(cli_dir, 'bin'),
> config=os.path.join(cli_dir, 'pylint.config'),
> ignore=os.path.join(cli_dir, 'bin', 'mesos'),
> files=source_files)],
> shell=True, stdout=subprocess.PIPE)
> ```
> 
> Kevin Klues wrote:
> Though,a ctually, this should come in a subsequent commit. Just fix the 
> indenting and the other comments from Vinod and I'll add this in as part of 
> my commit that actually populates the `cli_dir`.

I would prefer something like this:

```
p = subprocess.Popen([
'pylint', 
'--rcfile=%s' % os.path.join(cli_dir, 'pylint.config'),
'--ignore=%s' % os.path.join(cli_dir, 'bin', 'mesos'),
] + source_paths,
env={ "PYTHONPATH" : "%s:%s" % (lib_path, bin_path)},
stdout=subprocess.PIPE)
```

No `shell=True`.  As for how to activate a virtualenv... I'm not sure.  But it 
should still be possible without using `shell=True`, I hope.


- Joseph


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


On Aug. 10, 2016, 3:43 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 10, 2016, 3:43 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50878: Avoid subtract resource when check contains for non persistent volume.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On Aug. 7, 2016, 3:40 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50878/
> ---
> 
> (Updated Aug. 7, 2016, 3:40 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When check resources contains logic, there wil be logic of subtracting
> the resources which is being checked. But this is not a must if the
> resources is not a persistent volume as the non persistent volume
> resources will be merged by default.
> 
> This fix is adding persistent volume check before subtrat resources,
> so that the non persistent volume resources will not be subtracted.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 2470c0280db7d48d9484c42bc2150e53e7ce6e1c 
>   src/v1/resources.cpp 6c4e3b299e701d477947dd7427c31d2ae64c05ae 
> 
> Diff: https://reviews.apache.org/r/50878/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> The test result is that sorter benchmark improved 40%+ when `unallocated` 
> resources, the resources benchmark test improved 40%+ when checking contain 
> for `scalar` and `shared` resources.
> 
> Sorter benchmark test before fix:
> ```
> ./bin/mesos-tests.sh --benchmark  
> --gtest_filter="*Sorter_BENCHMARK_Test.FullSort/35"
> [==] 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/35
> Using 5 agents and 1000 clients
> Added 1000 clients in 24290us
> Added 5 agents in 1.483639secs
> Added allocations for 5 agents in 5.007551secs
> Full sort of 1000 clients took 33151us
> No-op sort of 1000 clients took 351us
> Removed allocations for 5 agents in 5.5673474833mins
> Removed 5 agents in 2.050195secs
> Removed 1000 clients in 19310us
> [   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35 (342673 ms)
> [--] 1 test from AgentAndClientCount/Sorter_BENCHMARK_Test (342673 ms 
> total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (342697 ms total)
> [  PASSED  ] 1 test.
> ```
> 
> Sorter benchmark test after fix:
> ```
> ./bin/mesos-tests.sh --benchmark  
> --gtest_filter="*Sorter_BENCHMARK_Test.FullSort/35"
> [==] 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/35
> Using 5 agents and 1000 clients
> Added 1000 clients in 20536us
> Added 5 agents in 1.592772secs
> Added allocations for 5 agents in 5.257316secs
> Full sort of 1000 clients took 35049us
> No-op sort of 1000 clients took 349us
> Removed allocations for 5 agents in 3.2523637667mins
> Removed 5 agents in 1.835446secs
> Removed 1000 clients in 19347us
> [   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35 (203915 ms)
> [--] 1 test from AgentAndClientCount/Sorter_BENCHMARK_Test (203916 ms 
> total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (203941 ms total)
> [  PASSED  ] 1 test.
> 
> ```
> 
> Apply patch https://reviews.apache.org/r/50551/
> 
> Resources benchmark test before fix:
> ```
> ./bin/mesos-tests.sh --benchmark  
> --gtest_filter="*Resources_BENCHMARK_Test.Contains/*”
> [==] Running 4 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 4 tests from ResourcesOperators/Resources_BENCHMARK_Test
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Contains/0
> Took 867642us to perform 5 'initial.contains(r)' operations on cpus(*):1; 
> gpus(*):1; mem(*):128; disk(*):256 with initial resources cpus(*):1; 
> gpus(*):1; mem(*):128; disk(*):256
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Contains/0 (870 ms)
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Contains/1
> Took 28.222642secs to perform 10 'initial.contains(r)' operations on cpus(0, 
> principal_0, {key_0: value_0}):1; gpus(... with initial resources cpus(0, 
> principal_0, {key_0: value_0}):1; gpus(...
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Contains/1 (28280 ms)
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Contains/2
> Took 480321us to perform 1000 'initial.contains(r)' operations on 
> ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1... with initial resources 
> ports(*):[3-5]
> [   OK ] 

Re: Review Request 50907: Abstracted mesos-style.py to wrap the cpp linter in a class.

[Joseph Wu]

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


Fix it, then Ship it!




Since you're making the linter object into some sort of interface, you should 
minimally add comments for object and override-able functions.


support/mesos-style.py (lines 11 - 12)


Would be nice to have a comment here, describing:

* General function of the class
* What fields/functions to override



support/mesos-style.py (lines 43 - 44)


Would be nice to have a comment here, describing:

* When this is called
* What the expected arguments are


- Joseph Wu


On Aug. 8, 2016, 12:49 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50907/
> ---
> 
> (Updated Aug. 8, 2016, 12:49 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-6006
> https://issues.apache.org/jira/browse/MESOS-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, mesos-style.py was just a collection of functions that
> checked the style of relevant files in the mesos code base.  However,
> the script assumed that we always wanted to run cpplint over every
> file we were checking. Since we are planning on adding a python linter
> to the codebase soon, it makes sense to abstract the common
> functionality from this script into a class so that a cpp-based linter
> and a python-based linter can inherit the same set of common
> functionality.
> 
> This commit builds this abstraction and implements a 'CppLinter()' in
> terms of it.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50907/diff/
> 
> 
> Testing
> ---
> 
> Ran `support/mesos-style.py` over the whole code base.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Kevin Klues]

> On Aug. 10, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/mesos-style.py, lines 228-234
> > 
> >
> > I know we didn't talk about this, but I realized recently that we 
> > actually *have* to run pylint inside the virtual environment, otherwise it 
> > runs using the system python, which is not what we want. Especially for 
> > import libraries.
> 
> Haris Choudhary wrote:
> There are two approaches to this:
> 
> 1) We can either activate the virtualenv from the CLI and than run 
> pylint. But that means if the virtualenv is not created within the CLI we 
> will have to create it and activate it.
> 
> 2) We can integrate the virtualenv to the project wide bootstrap and thus 
> ensuring the the virtualenv is created for the project on bootstrapping 
> mesos. This seems to be the better way to do it however might require 
> significant changes as opposed to (1). It'd be desirable to have a 
> project-wide virtualenv at some point however even if we choose to not do so 
> right now. A thing to note here is that if we integrate it to the project 
> wide bootstrap, we'll need users to have virtualenv installed for mesos 
> instead of only the CLI.
> 
> Kevin Klues wrote:
> I'd prefer #2, but it's a bigger change and has the downside that the 
> virtualenv dependency will become immediate if we add it to the top-level 
> bootstrap. Curious what @vinodkone thinks.
> 
> Kevin Klues wrote:
> I think this should suffice for now (including fixes to the indentation):
> ```
> cli_dir = os.path.abspath(self.source_dirs[0])
> source_files = ' '.join(source_paths)
> 
> p = subprocess.Popen(
> ['source {virtualenv}/bin/activate; \
>  PYTHONPATH={lib_dir}:{bin_dir} pylint --rcfile={config} 
> --ignore={ignore} {files}'.\
>  format(virtualenv=os.path.join(cli_dir, '.virtualenv'),
> lib_dir=os.path.join(cli_dir, 'lib'),
> bin_dir=os.path.join(cli_dir, 'bin'),
> config=os.path.join(cli_dir, 'pylint.config'),
> ignore=os.path.join(cli_dir, 'bin', 'mesos'),
> files=source_files)],
> shell=True, stdout=subprocess.PIPE)
> ```

Though,a ctually, this should come in a subsequent commit. Just fix the 
indenting and the other comments from Vinod and I'll add this in as part of my 
commit that actually populates the `cli_dir`.


- Kevin


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


On Aug. 10, 2016, 10:43 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 10, 2016, 10:43 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Kevin Klues]

> On Aug. 10, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/mesos-style.py, lines 228-234
> > 
> >
> > I know we didn't talk about this, but I realized recently that we 
> > actually *have* to run pylint inside the virtual environment, otherwise it 
> > runs using the system python, which is not what we want. Especially for 
> > import libraries.
> 
> Haris Choudhary wrote:
> There are two approaches to this:
> 
> 1) We can either activate the virtualenv from the CLI and than run 
> pylint. But that means if the virtualenv is not created within the CLI we 
> will have to create it and activate it.
> 
> 2) We can integrate the virtualenv to the project wide bootstrap and thus 
> ensuring the the virtualenv is created for the project on bootstrapping 
> mesos. This seems to be the better way to do it however might require 
> significant changes as opposed to (1). It'd be desirable to have a 
> project-wide virtualenv at some point however even if we choose to not do so 
> right now. A thing to note here is that if we integrate it to the project 
> wide bootstrap, we'll need users to have virtualenv installed for mesos 
> instead of only the CLI.
> 
> Kevin Klues wrote:
> I'd prefer #2, but it's a bigger change and has the downside that the 
> virtualenv dependency will become immediate if we add it to the top-level 
> bootstrap. Curious what @vinodkone thinks.

I think this should suffice for now (including fixes to the indentation):
```
cli_dir = os.path.abspath(self.source_dirs[0])
source_files = ' '.join(source_paths)

p = subprocess.Popen(
['source {virtualenv}/bin/activate; \
 PYTHONPATH={lib_dir}:{bin_dir} pylint --rcfile={config} 
--ignore={ignore} {files}'.\
 format(virtualenv=os.path.join(cli_dir, '.virtualenv'),
lib_dir=os.path.join(cli_dir, 'lib'),
bin_dir=os.path.join(cli_dir, 'bin'),
config=os.path.join(cli_dir, 'pylint.config'),
ignore=os.path.join(cli_dir, 'bin', 'mesos'),
files=source_files)],
shell=True, stdout=subprocess.PIPE)
```


- Kevin


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


On Aug. 10, 2016, 10:43 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 10, 2016, 10:43 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50940: Removed a limitation.

[Qian Zhang]

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


Ship it!




Ship It!

- Qian Zhang


On Aug. 10, 2016, 9:12 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50940/
> ---
> 
> (Updated Aug. 10, 2016, 9:12 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-5985
> https://issues.apache.org/jira/browse/MESOS-5985
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed a limitation.
> 
> 
> Diffs
> -
> 
>   docs/cni.md 7cceca566e3bd1d1981afd0b41ffc3c14596d851 
> 
> Diff: https://reviews.apache.org/r/50940/diff/
> 
> 
> Testing
> ---
> 
> Launched website and verfied the changes.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [50271, 50270, 50269, 50889, 50266]

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

Error:
2016-08-10 23:58:36 URL:https://reviews.apache.org/r/50270/diff/raw/ 
[9465/9465] -> "50270.patch" [1]
error: patch failed: src/launcher/executor.cpp:110
error: src/launcher/executor.cpp: patch does not apply
error: patch failed: src/slave/flags.cpp:92
error: src/slave/flags.cpp: patch does not apply

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

- Mesos ReviewBot


On Aug. 10, 2016, 5:03 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Aug. 10, 2016, 5:03 p.m.)
> 
> 
> 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 e945514fbdd49b3b35e200a7fc2e1cd446c1af91 
>   src/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
>   src/slave/containerizer/mesos/isolators/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> 827c9f06b97191a5b74bb8fa2ef716c4282e7518 
> 
> 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
> 
>

Re: Review Request 50851: Decode Protobuf long int form strings.

[Joseph Wu]

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



Can you add the reason/motivation for this change in your patch description?


3rdparty/stout/include/stout/protobuf.hpp (lines 443 - 459)


I wonder how far we want to follow the protobuf mapping recommendations.  
Technically, protobuf3 takes strings for all types of numbers :)

I'm fine with just large numbers however.



3rdparty/stout/include/stout/protobuf.hpp (line 447)


Unfortunately, `std::stoll` and similar functions will throw exceptions if 
they fail.  And Mesos does not use exceptions.

You'll need to parse the string some other way (i.e. `strtoimax`) and 
return an `Error` if the parsing fails.



3rdparty/stout/tests/protobuf_tests.cpp (lines 334 - 336)


This should be checking for equality, rather than parse success:

EXPECT_SOME_EQ(, json);



3rdparty/stout/tests/protobuf_tests.cpp (lines 342 - 343)


This is actually a Protobuf -> JSON -> String conversion.

You'll want to split these two apart and have one comparision for each step.


- Joseph Wu


On Aug. 5, 2016, 7:33 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50851/
> ---
> 
> (Updated Aug. 5, 2016, 7:33 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Joseph Wu.
> 
> 
> Bugs: MESOS-5995
> https://issues.apache.org/jira/browse/MESOS-5995
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Decode Protobuf long int form strings.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/protobuf.hpp 
> 91305e104c01d649bd435a27b15954036c27 
>   3rdparty/stout/tests/protobuf_tests.cpp 
> 8877e8934e0f7875bfedcfa88b491ce4b13ca44f 
> 
> Diff: https://reviews.apache.org/r/50851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Kevin Klues]

> On Aug. 10, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/mesos-style.py, lines 228-234
> > 
> >
> > I know we didn't talk about this, but I realized recently that we 
> > actually *have* to run pylint inside the virtual environment, otherwise it 
> > runs using the system python, which is not what we want. Especially for 
> > import libraries.
> 
> Haris Choudhary wrote:
> There are two approaches to this:
> 
> 1) We can either activate the virtualenv from the CLI and than run 
> pylint. But that means if the virtualenv is not created within the CLI we 
> will have to create it and activate it.
> 
> 2) We can integrate the virtualenv to the project wide bootstrap and thus 
> ensuring the the virtualenv is created for the project on bootstrapping 
> mesos. This seems to be the better way to do it however might require 
> significant changes as opposed to (1). It'd be desirable to have a 
> project-wide virtualenv at some point however even if we choose to not do so 
> right now. A thing to note here is that if we integrate it to the project 
> wide bootstrap, we'll need users to have virtualenv installed for mesos 
> instead of only the CLI.

I'd prefer #2, but it's a bigger change and has the downside that the 
virtualenv dependency will become immediate if we add it to the top-level 
bootstrap. Curious what @vinodkone thinks.


- Kevin


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


On Aug. 10, 2016, 10:43 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 10, 2016, 10:43 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

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

[Ammar Askar]

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

(Updated Aug. 10, 2016, 11:28 p.m.)


Review request for mesos, Greg Mann and Vinod Kone.


Changes
---

rebased


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 d6fe4d0 

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 50002: Allow all flags load methods to specify a prefix.

[Ammar Askar]

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

(Updated Aug. 10, 2016, 11:24 p.m.)


Review request for mesos, Greg Mann and Vinod Kone.


Changes
---

Rebased


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


Repository: mesos


Description
---

Allow all flags load methods to specify a prefix.

This also refactors the prefix logic into one place, so that's nice.
Required for the actual fix for passing work_dir in local.


Diffs (updated)
-

  3rdparty/stout/include/stout/flags/flags.hpp dd93627 
  3rdparty/stout/tests/flags_tests.cpp 848d707 

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


Testing
---

Added additional tests to flags_tests.cpp to ensure that prefix works on the 
other methods.

`make check` passes


Thanks,

Ammar Askar

Re: Review Request 50857: Modified a scheduler test to run with SSL enabled.

[Joseph Wu]

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




src/tests/scheduler_tests.cpp (line 1516)


`::tr1` ?



src/tests/scheduler_tests.cpp (line 1534)


`None()`



src/tests/scheduler_tests.cpp (line 1545)


`None()`


- Joseph Wu


On Aug. 10, 2016, 3:37 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50857/
> ---
> 
> (Updated Aug. 10, 2016, 3:37 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-3753
> https://issues.apache.org/jira/browse/MESOS-3753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies the test `SchedulerTest.Teardown` to
> be parametrized by both `ContentType` and SSL configuration,
> and renames it to `SchedulerSSLTest.RunTaskAndTeardown`.
> This allows the test to verify the scheduler's behavior with
> SSL both enabled and disabled.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp ccd0f2f602a7a1a9a44b68fd0f59bdc8e0fa58b1 
> 
> Diff: https://reviews.apache.org/r/50857/diff/
> 
> 
> Testing
> ---
> 
> This test is currently flaky in the SSL-enabled configurations and will 
> produce a segfault on my OSX machine after 50-100 repetitions. To reproduce, 
> try:
> 
> `GTEST_REPEAT=1000 GTEST_BREAK_ON_FAILURE=1 
> GTEST_FILTER="ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/2" 
> bin/mesos-tests.sh`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 50969: Updated SSL environment variables in HTTP API libraries.

[Joseph Wu]

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




src/executor/executor.cpp (lines 210 - 216)


Let's re-use the parsed value from libprocess.

```
#ifdef USE_SSL_SOCKET
#include 
#endif // USE_SSL_SOCKET

...

#ifdef USE_SSL_SOCKET
if (process::network::openssl::flags().enabled) {
  scheme = "https";
}
#endif // USE_SSL_SOCKET
```

Similar below.


- Joseph Wu


On Aug. 10, 2016, 3:39 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50969/
> ---
> 
> (Updated Aug. 10, 2016, 3:39 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-3753
> https://issues.apache.org/jira/browse/MESOS-3753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the SSL-related environment variable
> names used in the HTTP API scheduler and executor libraries.
> We recently added the `LIBPROCESS_` prefix to these, but
> still also look for the older `SSL_` format.
> 
> 
> Diffs
> -
> 
>   src/executor/executor.cpp d1ec817c53d0224a483fec545031016cf90620da 
>   src/scheduler/scheduler.cpp 276ed10bd439c4a7830447bec5053292fb2ca4e7 
> 
> Diff: https://reviews.apache.org/r/50969/diff/
> 
> 
> Testing
> ---
> 
> Tested after the following patch in this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

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

[Vinod Kone]

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



can you rebase?

- Vinod Kone


On July 21, 2016, 1:38 a.m., Ammar Askar wrote:
> 
> ---
> 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
> -
> 
>   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 49571: Added a benchmark test for allocations.

[Anindya Sinha]

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

(Updated Aug. 10, 2016, 11:09 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Review comments and rebased.


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


Repository: mesos


Description (updated)
---

Allocations test has the following resource configurations:
(1) REGULAR: Offers from every slave have regular resources.
(2) SHARED: Offers from every slave include a shared resource.
(3) REGULAR: Offers from every alternate slave contain only regular
resources; and offers from every other alternate slave contains
a shared resource.

This test is parameterized based on number of agents, number of
frameworks and resource configuration.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
cbed333f497016fe2811f755028796012b41db77 

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


Testing (updated)
---

All tests passed.

Allocations benchmark test results
==
Support of shared resources has a small impact (roughly 5%-7%) on runtime 
performance in allocations as compared to HEAD (without shared resources). 
Also, there is no visible impact in performance when shared resources are added 
in the tests.

Following is a snapshot with 1000 agents and 200 frameworks.

With the patch (and no shared resources)

[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/9
Using 1000 agents and 200 frameworks with resource type 0
Added 200 frameworks in 6572us
Added 1000 agents in 1.820284secs
round 0 allocate() took 1.602476secs to make 1000 offers
round 50 allocate() took 1.586638secs to make 1000 offers
round 100 allocate() took 1.588735secs to make 1000 offers
round 150 allocate() took 1.581553secs to make 1000 offers
round 199 allocate() took 1.595088secs to make 1000 offers

With the patch (and shared resources on all agents)
---
[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/10
Using 1000 agents and 200 frameworks with resource type 1
Added 200 frameworks in 6869us
Added 1000 agents in 1.847318secs
round 0 allocate() took 1.606464secs to make 1000 offers
round 50 allocate() took 1.603727secs to make 1000 offers
round 100 allocate() took 1.630528secs to make 1000 offers
round 150 allocate() took 1.60693secs to make 1000 offers
round 199 allocate() took 1.613754secs to make 1000 offers

With the patch (and shared resources on alternate agents)
-
[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/11
Using 1000 agents and 200 frameworks with resource type 2
Added 200 frameworks in 6903us
Added 1000 agents in 1.856714secs
round 0 allocate() took 1.621012secs to make 1000 offers
round 50 allocate() took 1.60385secs to make 1000 offers
round 100 allocate() took 1.64289secs to make 1000 offers
round 150 allocate() took 1.654631secs to make 1000 offers
round 199 allocate() took 1.609998secs to make 1000 offers

Based on HEAD, with all regular resources (no shared resources in HEAD 
supported)
-
[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/9
Using 1000 agents and 200 frameworks with resource type 0
Added 200 frameworks in 6801us
Added 1000 agents in 1.721447secs
round 0 allocate() took 1.502953secs to make 1000 offers
round 50 allocate() took 1.520157secs to make 1000 offers
round 100 allocate() took 1.517221secs to make 1000 offers
round 150 allocate() took 1.526446secs to make 1000 offers
round 199 allocate() took 1.538005secs to make 1000 offers


Thanks,

Anindya Sinha

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

[Vinod Kone]

> On Aug. 6, 2016, 12:29 a.m., Vinod Kone wrote:
> > src/local/local.cpp, lines 179-180
> > 
> >
> > not sure if 'propagated_flags' is the right name.
> > 
> > also, wondering if it would be intuitive to just do
> > 
> > os::setenv("MESOS_WORK_DIR") = flags.work_dir;
> 
> Ammar Askar wrote:
> >not sure if 'propagated_flags' is the right name.
> 
> The reasoning was they are being propogated from the local main()'s flags 
> to the agent's and master's flags.
> 
> 
> >also, wondering if it would be intuitive to just do
> 
> That would certainly work, the reason I didn't do it that way and opted 
> to take the approach with the map is to make this more extensible in the 
> future. So in case more flags need to be added to the local run. And if they 
> only needed to be provided to a master or just the agents, it's much easier 
> and clearer than fiddling with environment variables, in my opinion.

sgtm.


- Vinod


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


On July 21, 2016, 1:38 a.m., Ammar Askar wrote:
> 
> ---
> 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
> -
> 
>   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 50737: Parametrized libprocess HTTPTests by SSL configuration.

[Joseph Wu]

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




3rdparty/libprocess/src/tests/http_tests.cpp (line 144)


Per a comment in the previous review, let's change this to 
`WithParamInterface`.



3rdparty/libprocess/src/tests/http_tests.cpp (line 157)


This should be `None()`



3rdparty/libprocess/src/tests/http_tests.cpp (line 168)


`None()`



3rdparty/libprocess/src/tests/http_tests.cpp (lines 181 - 184)


Per a comment in the previous review, let's change this to `"http"` and 
`"https"`.


- Joseph Wu


On Aug. 10, 2016, 3:41 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50737/
> ---
> 
> (Updated Aug. 10, 2016, 3:41 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5966
> https://issues.apache.org/jira/browse/MESOS-5966
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch parametrizes the HTTP tests in libprocess so
> that they run with SSL both enabled and disabled when
> the library was compiled with SSL support.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 24b266df5f17e28e0c95326f5d1ea451952500e8 
> 
> Diff: https://reviews.apache.org/r/50737/diff/
> 
> 
> Testing
> ---
> 
> Tested on OSX with `GTEST_REPEAT=100 GTEST_BREAK_ON_FAILURE=1 
> GTEST_FILTER="*HTTPTest*" 3rdparty/libprocess/libprocess-tests`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 50954: Added documentation to Appc runtime support.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50954]

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 Aug. 10, 2016, 4:39 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50954/
> ---
> 
> (Updated Aug. 10, 2016, 4:39 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5837
> https://issues.apache.org/jira/browse/MESOS-5837
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation to Appc runtime support.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md 0feb0286218d1774f47640a7e41f0f65c6181db0 
>   docs/mesos-containerizer.md 76544625838845fe2817347fea483a60ab250f99 
> 
> Diff: https://reviews.apache.org/r/50954/diff/
> 
> 
> Testing
> ---
> 
> Tested documentation update.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 50625: Renamed the filter for tests that depend on "perf".

[Neil Conway]

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

(Updated Aug. 10, 2016, 11:05 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

added bug -- @vinodkone


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


Repository: mesos


Description
---

Previously, any test case that had "Perf" in its name would only be run
when the perf(1) command was installed. This is inconsistent with other
name-based filters for test cases (e.g., "ROOT_" or "CGROUPS_"). It also
meant that the filter matched "Registrar_BENCHMARK_Test.Performance",
which was a bug (the test does not depend on perf).

Instead, use "PERF_" to indicate tests that can only be run when perf(1)
is installed.


Diffs
-

  src/tests/containerizer/cgroups_tests.cpp 
d766f0f0bfabd87904311d36f69aedf7651415b6 
  src/tests/containerizer/isolator_tests.cpp 
488747347f71a6a1bb6bc01477143d077d4fd3eb 
  src/tests/containerizer/perf_tests.cpp 
3dc5b0fa90dd941408a73585197b5309e3204576 
  src/tests/environment.cpp 0d22c4be42e2c3c53b2370be0c11544aea55621b 
  src/tests/slave_recovery_tests.cpp 470fb26a2985f912b2b91d647cd7a27b8748c2a5 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 50736: Added SSL support to libprocess HTTP request helpers.

[Joseph Wu]

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




3rdparty/libprocess/src/http.cpp (lines 1373 - 1375)


It will be cleaner (and easier to read at the call sites) to pass in the 
`scheme` directly.

i.e. `const Option& scheme`
Same for below.

It's a little unfortunate that the scheme argument will be last, even 
though it is the front of the URL.  But I wouldn't want to go through a big 
refactor for this change :)


- Joseph Wu


On Aug. 10, 2016, 3:41 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50736/
> ---
> 
> (Updated Aug. 10, 2016, 3:41 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5966
> https://issues.apache.org/jira/browse/MESOS-5966
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order to test libprocess HTTP functionality with SSL
> both enabled and disabled, the HTTP helper functions that
> libprocess uses in tests must be parametrized by SSL
> configuration.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 404196bb198c1ff958b55d72fb29c5fe92dba429 
>   3rdparty/libprocess/src/http.cpp 298bd460afdeccd18e201d8a505b961fd4cf3d3e 
> 
> Diff: https://reviews.apache.org/r/50736/diff/
> 
> 
> Testing
> ---
> 
> Find testing information in the subsequent patch in this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 50769: Fixed a broken link to CNI repo.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 9, 2016, 6:11 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50769/
> ---
> 
> (Updated Aug. 9, 2016, 6:11 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5985
> https://issues.apache.org/jira/browse/MESOS-5985
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a broken link to CNI repo.
> 
> 
> Diffs
> -
> 
>   docs/networking.md f6652f58b02edee08e0b2410c23b2beb4d25e83b 
> 
> Diff: https://reviews.apache.org/r/50769/diff/
> 
> 
> Testing
> ---
> 
> Launched the mesos website and verified the link.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 50769: Fixed a broken link to CNI repo.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Aug. 10, 2016, 1:11 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50769/
> ---
> 
> (Updated Aug. 10, 2016, 1:11 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5985
> https://issues.apache.org/jira/browse/MESOS-5985
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a broken link to CNI repo.
> 
> 
> Diffs
> -
> 
>   docs/networking.md f6652f58b02edee08e0b2410c23b2beb4d25e83b 
> 
> Diff: https://reviews.apache.org/r/50769/diff/
> 
> 
> Testing
> ---
> 
> Launched the mesos website and verified the link.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Haris Choudhary]

> On Aug. 10, 2016, 4:53 a.m., Vinod Kone wrote:
> > support/mesos-style.py, line 242
> > 
> >
> > Don't follow this comment?

I meant that as a note to myself. Forgot to remove it. I was talking about how 
pylint does not give us access to the number of errors and so we need to parse 
the output to obtain our result.


- Haris


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


On Aug. 10, 2016, 10:43 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 10, 2016, 10:43 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Haris Choudhary]

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

(Updated Aug. 10, 2016, 10:43 p.m.)


Review request for mesos, Kevin Klues and Vinod Kone.


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


Repository: mesos


Description
---

It currently doesn't run over any files in the code base, but we will
be adding the new python CLI in a subsequent commit, which will use
this new linter.


Diffs (updated)
-

  support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 

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


Testing
---


Thanks,

Haris Choudhary

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Haris Choudhary]

> On Aug. 10, 2016, 4:53 a.m., Vinod Kone wrote:
> > support/mesos-style.py, lines 228-234
> > 
> >
> > this indentation looks weird? or is it just RB?

I fixed that. Hope its better now.


- Haris


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


On Aug. 10, 2016, 10:43 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 10, 2016, 10:43 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 50736: Added SSL support to libprocess HTTP request helpers.

[Greg Mann]

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

(Updated Aug. 10, 2016, 10:41 p.m.)


Review request for mesos, Anand Mazumdar, Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description
---

In order to test libprocess HTTP functionality with SSL
both enabled and disabled, the HTTP helper functions that
libprocess uses in tests must be parametrized by SSL
configuration.


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp 
404196bb198c1ff958b55d72fb29c5fe92dba429 
  3rdparty/libprocess/src/http.cpp 298bd460afdeccd18e201d8a505b961fd4cf3d3e 

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


Testing
---

Find testing information in the subsequent patch in this chain.


Thanks,

Greg Mann

Re: Review Request 50737: Parametrized libprocess HTTPTests by SSL configuration.

[Greg Mann]

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

(Updated Aug. 10, 2016, 10:41 p.m.)


Review request for mesos, Anand Mazumdar, Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description
---

This patch parametrizes the HTTP tests in libprocess so
that they run with SSL both enabled and disabled when
the library was compiled with SSL support.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/http_tests.cpp 
24b266df5f17e28e0c95326f5d1ea451952500e8 

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


Testing
---

Tested on OSX with `GTEST_REPEAT=100 GTEST_BREAK_ON_FAILURE=1 
GTEST_FILTER="*HTTPTest*" 3rdparty/libprocess/libprocess-tests`


Thanks,

Greg Mann

Re: Review Request 50969: Updated SSL environment variables in HTTP API libraries.

[Greg Mann]

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

(Updated Aug. 10, 2016, 10:39 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


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


Repository: mesos


Description
---

This patch updates the SSL-related environment variable
names used in the HTTP API scheduler and executor libraries.
We recently added the `LIBPROCESS_` prefix to these, but
still also look for the older `SSL_` format.


Diffs
-

  src/executor/executor.cpp d1ec817c53d0224a483fec545031016cf90620da 
  src/scheduler/scheduler.cpp 276ed10bd439c4a7830447bec5053292fb2ca4e7 

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


Testing
---

Tested after the following patch in this chain.


Thanks,

Greg Mann

Re: Review Request 50857: Modified a scheduler test to run with SSL enabled.

[Greg Mann]

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

(Updated Aug. 10, 2016, 10:37 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Summary (updated)
-

Modified a scheduler test to run with SSL enabled.


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


Repository: mesos


Description
---

This patch modifies the test `SchedulerTest.Teardown` to
be parametrized by both `ContentType` and SSL configuration,
and renames it to `SchedulerSSLTest.RunTaskAndTeardown`.
This allows the test to verify the scheduler's behavior with
SSL both enabled and disabled.


Diffs
-

  src/tests/scheduler_tests.cpp ccd0f2f602a7a1a9a44b68fd0f59bdc8e0fa58b1 

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


Testing
---

This test is currently flaky in the SSL-enabled configurations and will produce 
a segfault on my OSX machine after 50-100 repetitions. To reproduce, try:

`GTEST_REPEAT=1000 GTEST_BREAK_ON_FAILURE=1 
GTEST_FILTER="ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/2" 
bin/mesos-tests.sh`


Thanks,

Greg Mann

Review Request 50969: Updated SSL environment variables in HTTP API libraries.

[Greg Mann]

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

Review request for mesos, Joseph Wu and Vinod Kone.


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


Repository: mesos


Description
---

This patch updates the SSL-related environment variable
names used in the HTTP API scheduler and executor libraries.
We recently added the `LIBPROCESS_` prefix to these, but
still also look for the older `SSL_` format.


Diffs
-

  src/executor/executor.cpp d1ec817c53d0224a483fec545031016cf90620da 
  src/scheduler/scheduler.cpp 276ed10bd439c4a7830447bec5053292fb2ca4e7 

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


Testing
---

Tested after the following patch in this chain.


Thanks,

Greg Mann

Re: Review Request 50857: WIP: Added an HTTP scheduler test with SSL enabled.

[Greg Mann]

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

(Updated Aug. 10, 2016, 10:33 p.m.)


Review request for mesos, Benjamin Mahler and Vinod Kone.


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


Repository: mesos


Description (updated)
---

This patch modifies the test `SchedulerTest.Teardown` to
be parametrized by both `ContentType` and SSL configuration,
and renames it to `SchedulerSSLTest.RunTaskAndTeardown`.
This allows the test to verify the scheduler's behavior with
SSL both enabled and disabled.


Diffs (updated)
-

  src/tests/scheduler_tests.cpp ccd0f2f602a7a1a9a44b68fd0f59bdc8e0fa58b1 

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


Testing (updated)
---

This test is currently flaky in the SSL-enabled configurations and will produce 
a segfault on my OSX machine after 50-100 repetitions. To reproduce, try:

`GTEST_REPEAT=1000 GTEST_BREAK_ON_FAILURE=1 
GTEST_FILTER="ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/2" 
bin/mesos-tests.sh`


Thanks,

Greg Mann

Re: Review Request 50737: Parametrized libprocess HTTPTests by SSL configuration.

[Greg Mann]

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

(Updated Aug. 10, 2016, 9:49 p.m.)


Review request for mesos, Anand Mazumdar, Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description (updated)
---

This patch parametrizes the HTTP tests in libprocess so
that they run with SSL both enabled and disabled when
the library was compiled with SSL support.


Diffs
-

  3rdparty/libprocess/src/tests/http_tests.cpp 
8a0d61eb57f9ae972eedf3481f131844b283abc5 

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


Testing
---

Tested on OSX with `GTEST_REPEAT=100 GTEST_BREAK_ON_FAILURE=1 
GTEST_FILTER="*HTTPTest*" 3rdparty/libprocess/libprocess-tests`


Thanks,

Greg Mann

Re: Review Request 50922: Moved the implementation of health check to health_checker.cpp.

[Alexander Rukletsov]

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


Fix it, then Ship it!





src/health-check/health_checker.hpp 


This should stay as long as you keep the c-tor definition in the .hpp.



src/health-check/health_checker.hpp (line 25)


owned.hpp and time.hpp are missing, as well as stout/nothing.hpp



src/health-check/health_checker.hpp 


We usually declare the *Process class after the wrapper.



src/health-check/health_checker.hpp (lines 35 - 44)


We can move the c-tor definition to the .cpp as well.



src/health-check/health_checker.cpp (line 39)


According our style guide, the header should be included first.



src/health-check/health_checker.cpp (line 230)


Extra blank line.


- Alexander Rukletsov


On Aug. 9, 2016, 6:13 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50922/
> ---
> 
> (Updated Aug. 9, 2016, 6:13 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Gastón 
> Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved the implementation of health check to health_checker.cpp.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> b28a9cf3c6c9217c0b2453ac6e34d88e1f648d61 
>   src/health-check/health_checker.cpp 
> 585a0b565d948cfa292bad818a710501a4ce0daf 
> 
> Diff: https://reviews.apache.org/r/50922/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50751: Removed CgroupsPerfEventIsolatorProcess.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49819, 49820, 49821, 49823, 49824, 49825, 49827, 50627, 
50642, 50643, 49817, 49828, 49849, 49850, 49851, 49852, 49853, 49854, 49855, 
50758, 50733, 50748, 50749, 50750, 50751]

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 Aug. 10, 2016, 4:36 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50751/
> ---
> 
> (Updated Aug. 10, 2016, 4:36 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5975
> https://issues.apache.org/jira/browse/MESOS-5975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed CgroupsPerfEventIsolatorProcess.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 6088c26d3465c3abb908495da3fa5b98c3062d16 
>   src/Makefile.am cf76a699c33debb129722b7cba63a4e9e09f84f7 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
>   src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp 
> 4abde12af68a26b94b3706cdb38bf9890d811039 
>   src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
> 31f35385691681ef5da14be747edfb5f57c5d05a 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
>   src/tests/containerizer/isolator_tests.cpp 
> 827c9f06b97191a5b74bb8fa2ef716c4282e7518 
> 
> Diff: https://reviews.apache.org/r/50751/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Haris Choudhary]

> On Aug. 10, 2016, 7:02 p.m., Kevin Klues wrote:
> > support/mesos-style.py, lines 228-234
> > 
> >
> > I know we didn't talk about this, but I realized recently that we 
> > actually *have* to run pylint inside the virtual environment, otherwise it 
> > runs using the system python, which is not what we want. Especially for 
> > import libraries.

There are two approaches to this:

1) We can either activate the virtualenv from the CLI and than run pylint. But 
that means if the virtualenv is not created within the CLI we will have to 
create it and activate it.

2) We can integrate the virtualenv to the project wide bootstrap and thus 
ensuring the the virtualenv is created for the project on bootstrapping mesos. 
This seems to be the better way to do it however might require significant 
changes as opposed to (1). It'd be desirable to have a project-wide virtualenv 
at some point however even if we choose to not do so right now. A thing to note 
here is that if we integrate it to the project wide bootstrap, we'll need users 
to have virtualenv installed for mesos instead of only the CLI.


- Haris


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


On Aug. 8, 2016, 10:10 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 8, 2016, 10:10 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 40413: Libprocess Reinit: Move ReaperProcess instantiation into process.cpp.

[Joseph Wu]

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

(Updated Aug. 10, 2016, 1:17 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
and Vinod Kone.


Changes
---

Rebase on whitespace change.


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


Repository: mesos


Description
---

The reaper singleton must be unified with `process::initialize` so 
that it also falls under the scope of reinitialization.  The singleton 
must also not be guarded by `Once`.


Diffs (updated)
-

  3rdparty/libprocess/include/process/reap.hpp 
d7e0fa381df63a9ca7d438d5cea0631e1b0ec7ee 
  3rdparty/libprocess/src/process.cpp 629f1644bc0a263972ec9efc41890c33f9406a34 
  3rdparty/libprocess/src/reap.cpp 5fc2a4d67a3a6fe56005fc2c2d16d4983d53b83a 

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


Testing
---

Tests done in a subsequent review.


Thanks,

Joseph Wu

Re: Review Request 50964: Windows: Fix build in src/launcher/executor.*.

[Joseph Wu]

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

(Updated Aug. 10, 2016, 1:08 p.m.)


Review request for mesos, Anand Mazumdar and Artem Harutyunyan.


Changes
---

Finished debugging build on Windows.  Needed to add some more headers.


Repository: mesos


Description
---

The namespace was refactored in some other cleanup:
https://reviews.apache.org/r/50411/


Diffs (updated)
-

  src/launcher/windows/executor.hpp bae44f5188e2ca46a744a033eb3d00998b3192d6 
  src/launcher/windows/executor.cpp a7b8125e67ad6cd825fad11fd9788255c4922843 

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


Testing
---

Equivalent of `make` on Windows 10.


Thanks,

Joseph Wu

Re: Review Request 50964: Windows: Fix build in src/launcher/executor.*.

[Anand Mazumdar]

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


Ship it!




Thanks for the prompt fix!

- Anand Mazumdar


On Aug. 10, 2016, 7:29 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50964/
> ---
> 
> (Updated Aug. 10, 2016, 7:29 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Artem Harutyunyan.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The namespace was refactored in some other cleanup:
> https://reviews.apache.org/r/50411/
> 
> 
> Diffs
> -
> 
>   src/launcher/windows/executor.hpp bae44f5188e2ca46a744a033eb3d00998b3192d6 
>   src/launcher/windows/executor.cpp a7b8125e67ad6cd825fad11fd9788255c4922843 
> 
> Diff: https://reviews.apache.org/r/50964/diff/
> 
> 
> Testing
> ---
> 
> Equivalent of `make` on Windows 10.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 50964: Windows: Fix build in src/launcher/executor.*.

[Joseph Wu]

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

Review request for mesos, Anand Mazumdar and Artem Harutyunyan.


Repository: mesos


Description
---

The namespace was refactored in some other cleanup:
https://reviews.apache.org/r/50411/


Diffs
-

  src/launcher/windows/executor.hpp bae44f5188e2ca46a744a033eb3d00998b3192d6 
  src/launcher/windows/executor.cpp a7b8125e67ad6cd825fad11fd9788255c4922843 

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


Testing
---

Equivalent of `make` on Windows 10.


Thanks,

Joseph Wu

Re: Review Request 50914: Removed the external containerizer.

[Vinod Kone]

> On Aug. 10, 2016, 7:21 p.m., Vinod Kone wrote:
> > src/Makefile.am, line 2263
> > 
> >
> > this should be deleted as well. this is causing the ASF CI to fail.

I pushed the fix.


- Vinod


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


On Aug. 9, 2016, 8:12 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50914/
> ---
> 
> (Updated Aug. 9, 2016, 8:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-3370
> https://issues.apache.org/jira/browse/MESOS-3370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed the external containerizer.
> 
> 
> Diffs
> -
> 
>   configure.ac f82c6314d65a8ea3eb118bab8e110063cae46121 
>   docs/committers.md a4cf837e3058686347bc721dc7f0745f88c4fa81 
>   docs/configuration.md d6a7eb02b9f3e0011d52ccfd3093167bb6adccd3 
>   docs/containerizer-internals.md 1839c7290c2c7681c516b3514face7496ffb2521 
>   docs/external-containerizer.md eece9e73d47c13cc50074ef77235147f09cd380a 
>   docs/fetcher.md 688984552ae243eb59a3c1bfb7abd3898e4fac67 
>   docs/home.md 80451f4a9b2327dc728b9454938b6205043bebbb 
>   docs/sandbox.md db973e311b8562aa25fcdcdabefa1f342d25e26e 
>   include/mesos/containerizer/containerizer.proto 
> 4d8e82791c263c6578bf06f4e94ebe853042be59 
>   src/CMakeLists.txt 1286ee08fe2d60867326a1f2585f054c20b52208 
>   src/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
>   src/examples/python/test-containerizer.in 
> 5066b45d11ce0418d6c8ae8374b5103cb0eaa249 
>   src/examples/python/test_containerizer.py 
> 8e154b0e8e0db3cfc5d3f695d509b0e49bcec652 
>   src/slave/containerizer/containerizer.cpp 
> ba3b3f62fe0cf755fdbebf52350d9069cb2efca8 
>   src/slave/containerizer/external_containerizer.hpp 
> feeb0278a3d8fec8c7177a9a3dc443ee0c198c5c 
>   src/slave/containerizer/external_containerizer.cpp 
> 9ee137a7731e66754bbf86424071765962423969 
>   src/slave/flags.hpp 58fba4a22d988ac6612fc3af8a9346f0b8f8bb51 
>   src/slave/flags.cpp b8ecc98721c52dcd59a0cc1333421d4f024fbe96 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 226eac56725e908040abda306a2e5e3c96bce9d7 
> 
> Diff: https://reviews.apache.org/r/50914/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 50960: Added appc uri fetcher tests.

[Srinivas Brahmaroutu]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added tests to fetch using all three modes that 'rkt fetch' supports,
fetch with meta discovery  ex: 'coreos.com/hello:latest', 
fetch from a specific  location ex: 'https://github.com/xxx/hello.aci'
fetch from docker registry ex: 'docker://hello-world'.


Diffs
-

  src/tests/uri_fetcher_tests.cpp 072c09b8081e5d5dded907b1a0ffb6d739d86911 

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


Testing
---

make check


Thanks,

Srinivas Brahmaroutu

Re: Review Request 43558: Speed up ExamplesTest.PersistentVolumeFramework.

[Benjamin Mahler]

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




src/examples/persistent_volume_framework.cpp (lines 402 - 410)


Can you give some context on why lowering these speeds up the test? How 
much does it speed it up?


- Benjamin Mahler


On Aug. 10, 2016, 5:21 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43558/
> ---
> 
> (Updated Aug. 10, 2016, 5:21 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Jie Yu.
> 
> 
> Bugs: MESOS-4663
> https://issues.apache.org/jira/browse/MESOS-4663
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Speed up ExamplesTest.PersistentVolumeFramework.
> 
> 
> Diffs
> -
> 
>   src/examples/persistent_volume_framework.cpp 
> fe2837cfffb6dd251ccb9c93197f623d0c55fb36 
> 
> Diff: https://reviews.apache.org/r/43558/diff/
> 
> 
> Testing
> ---
> 
> Repeat test in CentOS 7.1
> ```
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="ExamplesTest.PersistentVolumeFramework" --gtest_repeat=200 
> --gtest_break_on_failure
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50959: Added appc fetcher plugin to use rkt tool.

[Srinivas Brahmaroutu]

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

(Updated Aug. 10, 2016, 7:21 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description (updated)
---

AppcFetcherPlugin returns a '.aci' file of a container image.

Fetch supports all three modes that 'rkt fetch' supports, fetch with
meta discovery  ex: 'coreos.com/hello:latest', fetch from a specific 
location ex: 'https://github.com/xxx/hello.aci' or fetch from docker 
registry ex: 'docker://hello-world'.

Added new fetched plugin for Appc fetch algorithm. The new 
AppcFetcherPlugin implements two methods. Method 'rktFetch' is used to
fetch a container image into a store specified at a directory location 
using 'rkt fetch' command and method 'rktExport' exports the image 
fetched into the directory location as a '.aci' file that is in tar 
file format, usinf 'rkt image export' command.


Diffs
-

  src/Makefile.am cf76a699c33debb129722b7cba63a4e9e09f84f7 
  src/uri/fetcher.hpp 949363ecdb15f5b225db7f3ed67eda52c351bfa1 
  src/uri/fetcher.cpp 198bd29993381758183edfce8faafba36da2d9ae 
  src/uri/fetchers/appc.hpp PRE-CREATION 
  src/uri/fetchers/appc.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Srinivas Brahmaroutu

Re: Review Request 50914: Removed the external containerizer.

[Vinod Kone]

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




src/Makefile.am (line 2258)


this should be deleted as well. this is causing the ASF CI to fail.


- Vinod Kone


On Aug. 9, 2016, 8:12 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50914/
> ---
> 
> (Updated Aug. 9, 2016, 8:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-3370
> https://issues.apache.org/jira/browse/MESOS-3370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed the external containerizer.
> 
> 
> Diffs
> -
> 
>   configure.ac f82c6314d65a8ea3eb118bab8e110063cae46121 
>   docs/committers.md a4cf837e3058686347bc721dc7f0745f88c4fa81 
>   docs/configuration.md d6a7eb02b9f3e0011d52ccfd3093167bb6adccd3 
>   docs/containerizer-internals.md 1839c7290c2c7681c516b3514face7496ffb2521 
>   docs/external-containerizer.md eece9e73d47c13cc50074ef77235147f09cd380a 
>   docs/fetcher.md 688984552ae243eb59a3c1bfb7abd3898e4fac67 
>   docs/home.md 80451f4a9b2327dc728b9454938b6205043bebbb 
>   docs/sandbox.md db973e311b8562aa25fcdcdabefa1f342d25e26e 
>   include/mesos/containerizer/containerizer.proto 
> 4d8e82791c263c6578bf06f4e94ebe853042be59 
>   src/CMakeLists.txt 1286ee08fe2d60867326a1f2585f054c20b52208 
>   src/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
>   src/examples/python/test-containerizer.in 
> 5066b45d11ce0418d6c8ae8374b5103cb0eaa249 
>   src/examples/python/test_containerizer.py 
> 8e154b0e8e0db3cfc5d3f695d509b0e49bcec652 
>   src/slave/containerizer/containerizer.cpp 
> ba3b3f62fe0cf755fdbebf52350d9069cb2efca8 
>   src/slave/containerizer/external_containerizer.hpp 
> feeb0278a3d8fec8c7177a9a3dc443ee0c198c5c 
>   src/slave/containerizer/external_containerizer.cpp 
> 9ee137a7731e66754bbf86424071765962423969 
>   src/slave/flags.hpp 58fba4a22d988ac6612fc3af8a9346f0b8f8bb51 
>   src/slave/flags.cpp b8ecc98721c52dcd59a0cc1333421d4f024fbe96 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 226eac56725e908040abda306a2e5e3c96bce9d7 
> 
> Diff: https://reviews.apache.org/r/50914/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 50266: Introduced linux capabilities API.

[Benjamin Bannier]

> On July 22, 2016, 2:32 a.m., Jie Yu wrote:
> > src/linux/capabilities.hpp, line 196
> > 
> >
> > I'd prefer this method returns `Set`.
> > 
> > We should have a general way to convert `CapabilityInfo` to 
> > `Set`, and vise versa, instead of having different return type 
> > (or parameter type) mixed in this file.
> 
> Benjamin Bannier wrote:
> If we'd just return a `Set` we'd need to use an empty set for both the 
> case of no supported capabilities, and the error case. What about using a 
> `Try` instead?

I made this a member function, as we discussed OTR. This allows us to return a 
`Set` since this approach requires a vaild `Capabilities`.


> On July 22, 2016, 2:32 a.m., Jie Yu wrote:
> > src/linux/capabilities.hpp, lines 124-134
> > 
> >
> > I'd suggest that we don't create a class `Capabilities`. Instead, let's 
> > put all methods under namesapce `capabilities`. Looks like we just need to 
> > cache `lastCapability` (`/proc/sys/kernel/cap_last_cap`), which should not 
> > change. There's no reason to keep a class for that.
> > 
> > ```
> > namespace capabilities {
> > Try initialize();
> > Try get();
> > Try set(const ProcessCapabilities& set);
> > ...
> > }
> > ```
> 
> Benjamin Bannier wrote:
> Are you suggesting we create a global variable to hold `lastCapability`? 
> It looks that one could only be set in `capabilities::initialize`, so we'd 
> require a way to denote unset/invalid state, so we'd either need something 
> like e.g., an `Option` which opens the door the nasty destruction order bugs, 
> or semantically worse, use e.g., a signed type to store an unsigned value and 
> use values < 0 to denote invalid values.
> 
> As much as I would like to get rid of an extra class here, using a class 
> to control that state appears simpler to me.

Dropping this as discussed offline.


> On July 22, 2016, 2:32 a.m., Jie Yu wrote:
> > src/linux/capabilities.hpp, lines 136-137
> > 
> >
> > I don't think this is needed. `getAllSupportedCapabilities` is 
> > sufficient.

This can indeed be removed now that `getAllSupportedCapabilities` is a member 
function.


> On July 22, 2016, 2:32 a.m., Jie Yu wrote:
> > src/linux/capabilities.cpp, line 191
> > 
> >
> > As I mentioned above, this should be the `initialize` function.

Dropping this as we keep on using a `Capabilities` class instead of free 
function.s


> On July 22, 2016, 2:32 a.m., Jie Yu wrote:
> > src/linux/capabilities.hpp, line 171
> > 
> >
> > This function is pretty high level. I don't think it belongs here. 
> > Looking at other capability libraries like 
> > https://github.com/syndtr/gocapability/tree/master/capability, it's not 
> > there. Let's move this out of this patch first.

We discussed pulling this function out of this patch (I guess a similar 
argument could be made for `Capabilities::keepCapabilitiesOnSetUid`), and 
literally inlining them at the call sites.

While the latter could be implemented as a free function, the former requires 
`Capabilities` state, so one would either need to pass such state in, or keep 
it a member. Regard inlining, it strongly looks to me as if that way we would 
impose dealing with very low-level code on users (e.g., preparing arguments for 
`capset`). It appears as if `capabilities` or `Capabilities` might just be the 
right spot for this code; otherwise it seems to me we wouldn't provide a 
complete toolbox.

Of course it is possible to simply rip this code out for the moment, and 
potentially add it later.

What do you think?


- Benjamin


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


On Aug. 10, 2016, 9:14 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50266/
> ---
> 
> (Updated Aug. 10, 2016, 9:14 p.m.)
> 
> 
> 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

Re: Review Request 50266: Introduced linux capabilities API.

[Benjamin Bannier]

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

(Updated Aug. 10, 2016, 9:14 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Last changes addressing review comments.


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 (updated)
-

  src/CMakeLists.txt 1286ee08fe2d60867326a1f2585f054c20b52208 
  src/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
  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

Re: Review Request 50910: Added a python linter to mesos-style.cpp.

[Kevin Klues]

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




support/mesos-style.py (lines 228 - 234)


I know we didn't talk about this, but I realized recently that we actually 
*have* to run pylint inside the virtual environment, otherwise it runs using 
the system python, which is not what we want. Especially for import 
libraries.


- Kevin Klues


On Aug. 8, 2016, 10:10 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50910/
> ---
> 
> (Updated Aug. 8, 2016, 10:10 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: Mesos-6006
> https://issues.apache.org/jira/browse/Mesos-6006
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It currently doesn't run over any files in the code base, but we will
> be adding the new python CLI in a subsequent commit, which will use
> this new linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 061cbe711bd9407b7341fa146f8c723eaa9fc438 
> 
> Diff: https://reviews.apache.org/r/50910/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Review Request 50959: Added appc fetcher plugin to use rkt tool.

[Srinivas Brahmaroutu]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

AppcFetcherPlugin returns a '.aci' file of a container image.

Fetch supports all three modes that 'rkt fetch' supports, fetch with meta
discovery  ex: 'coreos.com/hello:latest', fetch from a specific location
ex: 'https://github.com/xxx/hello.aci' or fetch from docker registry
ex: 'docker://hello-world'.

Added new fetched plugin for Appc fetch algorithm. The new AppcFetcherPlugin
implements two methods. Method 'rktFetch' is used to fetch a container image
into a store specified at a directory location using 'rkt fetch' command and
method 'rktExport' exports the image fetched into the directory location as
a '.aci' file that is in tar file format, usinf 'rkt image export' command.


Diffs
-

  src/Makefile.am cf76a699c33debb129722b7cba63a4e9e09f84f7 
  src/uri/fetcher.hpp 949363ecdb15f5b225db7f3ed67eda52c351bfa1 
  src/uri/fetcher.cpp 198bd29993381758183edfce8faafba36da2d9ae 
  src/uri/fetchers/appc.hpp PRE-CREATION 
  src/uri/fetchers/appc.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Srinivas Brahmaroutu

Review Request 50958: Added tests to invoke the fetcher plugins by name.

[Srinivas Brahmaroutu]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Add a test case for each of the existing fetcher plugins to
invoke them by name.


Diffs
-

  src/tests/uri_fetcher_tests.cpp 072c09b8081e5d5dded907b1a0ffb6d739d86911 

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


Testing
---

make check


Thanks,

Srinivas Brahmaroutu

Re: Review Request 50887: Trimmed unneeded extra space between right angle brackets.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Aug. 8, 2016, 4:13 p.m., Gaojin CAO wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50887/
> ---
> 
> (Updated Aug. 8, 2016, 4:13 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Guangya Liu, and haosdent huang.
> 
> 
> Bugs: MESOS-5830
> https://issues.apache.org/jira/browse/MESOS-5830
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos codebase has been evolved and no more need an extra space
> between two right angle brackets, e.g. vector, to
> work around C++98 parsing deficiencies(see MESOS-5830).
> 
> 
> Diffs
> -
> 
>   include/mesos/log/log.hpp 0f69848bd72372e7a58f6e3789e86bf826eaeab7 
>   include/mesos/zookeeper/contender.hpp 
> 0ef9d56c15c4c5dd1d36f2866324ac95d55901d1 
>   include/mesos/zookeeper/detector.hpp 
> a953997b98e1242bea10efc791e2f31fa3252593 
>   src/authentication/cram_md5/auxprop.cpp 
> 85403417c8169c0796ffa16484aa85da2d5220ba 
>   src/cli/mesos.cpp 81e2325c1978d56487479848966115ffd14b7cb0 
>   src/cli/resolve.cpp ac6be05e64c5e66703081068d992ebb0d6ca6c70 
>   src/files/files.cpp 793d68ec700c792fc9dd73b09ee2616108b88f4f 
>   src/java/jni/org_apache_mesos_Log.cpp 
> 04f8afd99b4383e7941a93bfd565806a9988c870 
>   src/java/jni/org_apache_mesos_state_AbstractState.cpp 
> 3101352e42e8505a93d8760ad657d9fe35761080 
>   src/launcher/executor.cpp 9333dc0832cd04305e307ce750195c0fbc860ab2 
>   src/linux/ns.hpp e905aefe5ab178c8471cfe801e6432540d3dea67 
>   src/log/consensus.cpp 27f2c58c4c54cc3fe5b92873261096b6011ff15d 
>   src/log/coordinator.hpp 9fa4f1988c531a9765c9522a017b39fafb0a9cea 
>   src/log/coordinator.cpp ecf457ff266f447938ee6d2872a8ef3b6d50517e 
>   src/log/log.cpp 0940f9288509409c1d593c27da48cfa2a50545c9 
>   src/log/main.cpp 171c312562442836a2cf8f61f0a83615c18d0971 
>   src/log/network.hpp 8750880e6cecb6edf4efab4f2b9afe3d8a4d5964 
>   src/log/recover.hpp 97793b6e1a26f92b4ae20b82e7da18d063776ce6 
>   src/log/recover.cpp eaf3fd334f7b66f9ba05f3856ee885bab9af62f6 
>   src/log/tool/benchmark.cpp da35b08b5448c965cedd3d9349d06b4a99fabfce 
>   src/master/registrar.cpp 4d1eaa04fcc7f3330bf3abab8533428848691f2c 
>   src/slave/gc.hpp 9a09e7b3585217754a26597b77ea51e706ecb933 
>   src/slave/gc.cpp a0af68b8d713a31bea533b24637861e81ca167e3 
>   src/slave/state.cpp 91dee118d1e6866fb2b968765cfe7c6f1b5a3325 
>   src/slave/status_update_manager.cpp 
> 9c67b8526915cb529b5c5afd5b5e4a451bf94d61 
>   src/tests/allocator.hpp c4d835afe180bcc2404cc3772af5f7c4885771bb 
>   src/tests/containerizer.hpp ded331bc60078196abf9e1d338c23b9b962a3ad2 
>   src/tests/containerizer.cpp 105ca9c0416b16fd26d396a4fdef7fbc8a43b8a4 
>   src/tests/containerizer/composing_containerizer_tests.cpp 
> e7e3b622b6606a812aef046c761bf92368d34af2 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4e5b1fd1628504d346cced545f7911d6b6443773 
>   src/tests/containerizer/docker_tests.cpp 
> 7b73a4939da33d0b7dd08fdcf56525b6403eada3 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 226eac56725e908040abda306a2e5e3c96bce9d7 
>   src/tests/containerizer/isolator_tests.cpp 
> 827c9f06b97191a5b74bb8fa2ef716c4282e7518 
>   src/tests/containerizer/launcher.hpp 
> 49f8ecb47abb098bf76017f79b7d00380401efe4 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 94d6d7589a4d9accff6f43ad6abbf05bf3be7ee5 
>   src/tests/disk_quota_tests.cpp f379a6e351fbcf8b41d7db6ed62865ce55b0060e 
>   src/tests/environment.cpp 6960c2a0165c4bb34851a0412c6b22b7da645540 
>   src/tests/exception_tests.cpp a9b1c23d3e8460c5c67c8f399346f85f2ac8addf 
>   src/tests/health_check_tests.cpp 47c35d15a259d470f21f81303879077787c125e7 
>   src/tests/master_allocator_tests.cpp 
> 32e3a4638273fa1c792c3c9425e0b2ff8f6bed03 
>   src/tests/master_authorization_tests.cpp 
> 16f1ab46a92137de01313a29dac2b18f18f5e816 
>   src/tests/master_contender_detector_tests.cpp 
> 2970986830d0cfaedbf17f3b002c40ac5c28bd2a 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 03405e33d3f1cccbd5298810f5cd818c1dac8593 
>   src/tests/mesos.hpp 9174a3810d0da34b99dc257e9c77f83107fdd9f5 
>   src/tests/reconciliation_tests.cpp 492560c430eba45e3a9f7a767ee761dfa0cacb05 
>   src/tests/registrar_tests.cpp ba7c66ff83f09d68fb5324a3f85d2875068a35a6 
>   src/tests/registrar_zookeeper_tests.cpp 
> 98a5e312a8b2701dc8dc9b6593178bb52cd5e13c 
>   src/tests/slave_recovery_tests.cpp af442cac40fb7b34e362dbfb2d864e7c8a010087 
>   src/tests/state_tests.cpp 4ba42673eedfb4ab5633e2250a79bdde667eff95 
>   src/tests/status_update_manager_tests.cpp 
> f5313a124c59eba3da86a173817d0c2a6a943e17 
>   src/tests/zookeeper.cpp 

Re: Review Request 50899: Removed unneeded extra space in stout code base.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Aug. 8, 2016, 4:17 p.m., Gaojin CAO wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50899/
> ---
> 
> (Updated Aug. 8, 2016, 4:17 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Guangya Liu, and haosdent huang.
> 
> 
> Bugs: MESOS-5830
> https://issues.apache.org/jira/browse/MESOS-5830
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch is part of MESOS-5830, specially working on cleaning up
> unneeded space in project stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags.hpp 
> 169f959ab2770744d615cc235062ba2becc8af64 
>   3rdparty/stout/include/stout/interval.hpp 
> e6deeac39855488548678c59ed4c69646fc02602 
>   3rdparty/stout/include/stout/json.hpp 
> 51b1ada87846d4d25cbc8a6bbb97ba91e02a9cab 
>   3rdparty/stout/include/stout/linkedhashmap.hpp 
> 6e06260bcce2dbe0f9134f3d0fd7a2b47b0d3286 
>   3rdparty/stout/include/stout/os/osx.hpp 
> ca77bb0bbda0cee07d85435350dc4ccc176d4013 
>   3rdparty/stout/include/stout/os/sunos.hpp 
> ec8e1f7562d5747113631351e6d8ff8989cf0fed 
>   3rdparty/stout/include/stout/os/sysctl.hpp 
> 6db7bf6f139770ce978d171f451e932072cdaf5f 
>   3rdparty/stout/include/stout/proc.hpp 
> b543432c1a8419093c508c1d0e72b9757df04463 
>   3rdparty/stout/include/stout/protobuf.hpp 
> 91305e104c01d649bd435a27b15954036c27 
>   3rdparty/stout/include/stout/strings.hpp 
> 7f7f1cffcebfe16cb986917b1d90c1ae4a480989 
>   3rdparty/stout/tests/flags_tests.cpp 
> 77f3a6af110da1ffcdf2b7ab2b66431a6b5c91d3 
>   3rdparty/stout/tests/ip_tests.cpp 4d1f1c9a4dd35da2a21bef1349a662af44bb4ba1 
>   3rdparty/stout/tests/mac_tests.cpp 1ff60cf5506c1855547400aa20107ec5086d8431 
>   3rdparty/stout/tests/multimap_tests.cpp 
> 2f606eb16fbaa3c755e597a05e97cd30d3bfd9f4 
>   3rdparty/stout/tests/option_tests.cpp 
> 36abe137bdae1e6aaf65f39bf302d9e7a76fefdd 
>   3rdparty/stout/tests/os/process_tests.cpp 
> b536e664d22370e30c49a8f953189e92a1358709 
>   3rdparty/stout/tests/os_tests.cpp e67444077eae55fd25945b451164b5bcc37552b0 
>   3rdparty/stout/tests/proc_tests.cpp 
> 17b909368405d9ddb7e7127e89d0422a9a2baa11 
>   3rdparty/stout/tests/strings_tests.cpp 
> b54a9dbf162403310b8bba687442e184a473f5a6 
> 
> Diff: https://reviews.apache.org/r/50899/diff/
> 
> 
> Testing
> ---
> 
> Pass `make` and `make tests`.
> 
> 
> Thanks,
> 
> Gaojin CAO
> 
>

Re: Review Request 50900: Removed unneeded extra space in libprocess code base.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Aug. 8, 2016, 4:19 p.m., Gaojin CAO wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50900/
> ---
> 
> (Updated Aug. 8, 2016, 4:19 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Guangya Liu, and haosdent huang.
> 
> 
> Bugs: MESOS-5830
> https://issues.apache.org/jira/browse/MESOS-5830
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch is part of MESOS-5830, specially working on cleaning up
> unneeded space in project libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/async.hpp 
> e7d846288791221f4dfa03d7de89d9bde177f1ca 
>   3rdparty/libprocess/include/process/protobuf.hpp 
> 695c6e6572852d2c4afbaacc6b63771ed035fd65 
>   3rdparty/libprocess/include/process/reap.hpp 
> 1a9709c618c5ddc9d2b7492cc1855a11f1fc4fb9 
>   3rdparty/libprocess/include/process/sequence.hpp 
> c9a46f28b770023ca621696493521445762a3fd1 
>   3rdparty/libprocess/include/process/statistics.hpp 
> 13aa464329c2cc77b017868a1dcbf5b77c53635d 
>   3rdparty/libprocess/src/reap.cpp f8d2fdc3449744cfd6a974170484b676e8f3ac7b 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 9fac45e21bd03ccea19d0ef7d8bb2efc408e60b5 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 8a0d61eb57f9ae972eedf3481f131844b283abc5 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> 4ddd35b61f222ca674e41875a6d0ee43d12cbfbd 
>   3rdparty/libprocess/src/tests/reap_tests.cpp 
> 0cbe9f139d59ac154695d84638c551fbe098cb10 
>   3rdparty/libprocess/src/tests/shared_tests.cpp 
> 8f94cca107ebe1fdefc19fb1f65e4c205f96 
>   3rdparty/libprocess/src/tests/statistics_tests.cpp 
> db9817ff182f37b7a04358fb59b8b800b16a7e1e 
> 
> Diff: https://reviews.apache.org/r/50900/diff/
> 
> 
> Testing
> ---
> 
> Pass `make` and `make tests`.
> 
> 
> Thanks,
> 
> Gaojin CAO
> 
>

Review Request 50957: Added fetch method based on plugin name.

[Srinivas Brahmaroutu]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added plugins by name into the Fetcher. Added a new fetch
method to select plugin by name which ignores scheme and 
directly invokes fetch on the plugin.


Diffs
-

  include/mesos/uri/fetcher.hpp 3add35c8c0e559203acb540a288d0b51ac817519 
  src/uri/fetcher.cpp 198bd29993381758183edfce8faafba36da2d9ae 

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


Testing
---

make check


Thanks,

Srinivas Brahmaroutu

Re: Review Request 50700: Added a list of "unreachable" agents to the registry.

[Vinod Kone]

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




src/master/registry.proto (line 45)


lets just do SlaveID for now.


- Vinod Kone


On Aug. 2, 2016, 1:01 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50700/
> ---
> 
> (Updated Aug. 2, 2016, 1:01 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4049
> https://issues.apache.org/jira/browse/MESOS-4049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These are agents that have failed health checks.
> 
> 
> Diffs
> -
> 
>   src/master/registry.proto 9bf9998bcf521b962f3fe8741d02620b1907f577 
> 
> Diff: https://reviews.apache.org/r/50700/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 50956: Added name to uri fetcher plugins.

[Srinivas Brahmaroutu]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added name to uri fetcher plugins.


Diffs
-

  include/mesos/uri/fetcher.hpp 3add35c8c0e559203acb540a288d0b51ac817519 
  src/uri/fetchers/copy.hpp 64e686c3249a34c9c68f1bf0ccde1683516d9f26 
  src/uri/fetchers/copy.cpp f095ad65b50405c5e0869e74652ce9529332dd0c 
  src/uri/fetchers/curl.hpp 447e01b9a566df133fd397cc4ea80150761f4653 
  src/uri/fetchers/curl.cpp cc3f9eec42eb841ecd320ffc83aa7884dc67c415 
  src/uri/fetchers/docker.hpp 6cb57be97d9494ea59ce9759e3d52e37b19d6c43 
  src/uri/fetchers/docker.cpp 72f70b8d17c947d5e9d8eb70c1c6bd6046bf1cd2 
  src/uri/fetchers/hadoop.hpp 1689f607a2bcff09887e5aa4d6e03de0ac020260 
  src/uri/fetchers/hadoop.cpp 3c69d43d7bc21061adb9c392da976dc5584e74d4 

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


Testing
---

make check


Thanks,

Srinivas Brahmaroutu

Re: Review Request 43558: Speed up ExamplesTest.PersistentVolumeFramework.

[haosdent huang]

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

(Updated Aug. 10, 2016, 5:21 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Speed up ExamplesTest.PersistentVolumeFramework.


Diffs (updated)
-

  src/examples/persistent_volume_framework.cpp 
fe2837cfffb6dd251ccb9c93197f623d0c55fb36 

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


Testing
---

Repeat test in CentOS 7.1
```
sudo ./bin/mesos-tests.sh 
--gtest_filter="ExamplesTest.PersistentVolumeFramework" --gtest_repeat=200 
--gtest_break_on_failure
```


Thanks,

haosdent huang

Re: Review Request 50266: Introduced linux capabilities API.

[Benjamin Bannier]

> On July 22, 2016, 2:32 a.m., Jie Yu wrote:
> > src/linux/capabilities.hpp, lines 179-180
> > 
> >
> > Do we still need this given that we only accept 
> > `_LINUX_CAPABILITY_VERSION_3`?

Good point. We only support `_LINUX_CAPABILITY_VERSION_3` so there is no need 
to store it.


> On July 22, 2016, 2:32 a.m., Jie Yu wrote:
> > src/linux/capabilities.hpp, lines 124-134
> > 
> >
> > I'd suggest that we don't create a class `Capabilities`. Instead, let's 
> > put all methods under namesapce `capabilities`. Looks like we just need to 
> > cache `lastCapability` (`/proc/sys/kernel/cap_last_cap`), which should not 
> > change. There's no reason to keep a class for that.
> > 
> > ```
> > namespace capabilities {
> > Try initialize();
> > Try get();
> > Try set(const ProcessCapabilities& set);
> > ...
> > }
> > ```

Are you suggesting we create a global variable to hold `lastCapability`? It 
looks that one could only be set in `capabilities::initialize`, so we'd require 
a way to denote unset/invalid state, so we'd either need something like e.g., 
an `Option` which opens the door the nasty destruction order bugs, or 
semantically worse, use e.g., a signed type to store an unsigned value and use 
values < 0 to denote invalid values.

As much as I would like to get rid of an extra class here, using a class to 
control that state appears simpler to me.


> On July 22, 2016, 2:32 a.m., Jie Yu wrote:
> > src/linux/capabilities.hpp, line 196
> > 
> >
> > I'd prefer this method returns `Set`.
> > 
> > We should have a general way to convert `CapabilityInfo` to 
> > `Set`, and vise versa, instead of having different return type 
> > (or parameter type) mixed in this file.

If we'd just return a `Set` we'd need to use an empty set for both the case of 
no supported capabilities, and the error case. What about using a 
`Try` instead?


- Benjamin


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


On Aug. 10, 2016, 7:03 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50266/
> ---
> 
> (Updated Aug. 10, 2016, 7:03 p.m.)
> 
> 
> 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 1286ee08fe2d60867326a1f2585f054c20b52208 
>   src/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
>   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
> 
>

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

[Benjamin Bannier]

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

(Updated Aug. 10, 2016, 7:03 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


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 (updated)
-

  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 9333dc0832cd04305e307ce750195c0fbc860ab2 
  src/slave/flags.hpp 58fba4a22d988ac6612fc3af8a9346f0b8f8bb51 
  src/slave/flags.cpp b8ecc98721c52dcd59a0cc1333421d4f024fbe96 

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 50266: Introduced linux capabilities API.

[Benjamin Bannier]

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

(Updated Aug. 10, 2016, 7:03 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Used `Set` instead of manually adjusting bitmasks in various places.


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 (updated)
-

  src/CMakeLists.txt 1286ee08fe2d60867326a1f2585f054c20b52208 
  src/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
  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

Re: Review Request 50271: Created an isolator for Linux capabilities.

[Benjamin Bannier]

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

(Updated Aug. 10, 2016, 7:03 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


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 (updated)
-

  include/mesos/slave/isolator.proto e945514fbdd49b3b35e200a7fc2e1cd446c1af91 
  src/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
  src/slave/containerizer/mesos/containerizer.cpp 
86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
  src/slave/containerizer/mesos/isolators/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/capabilities.cpp PRE-CREATION 
  src/tests/containerizer/isolator_tests.cpp 
827c9f06b97191a5b74bb8fa2ef716c4282e7518 

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

Re: Review Request 50733: Removed CgroupsCpushareIsolatorProcess.

[haosdent huang]

> On Aug. 8, 2016, 3:17 p.m., Qian Zhang wrote:
> >

Hi, @qianzhang Because I think all the test cases are associate with 
`CgroupsIsolator`, I think it would be better to update the test cases name to 
make it more clear and exactly.


- haosdent


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


On Aug. 10, 2016, 4:35 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50733/
> ---
> 
> (Updated Aug. 10, 2016, 4:35 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5973
> https://issues.apache.org/jira/browse/MESOS-5973
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed CgroupsCpushareIsolatorProcess.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 6088c26d3465c3abb908495da3fa5b98c3062d16 
>   src/Makefile.am cf76a699c33debb129722b7cba63a4e9e09f84f7 
>   src/docker/docker.cpp e07c4aa723f57db7f8bc31d4af5d32a30ebf98f2 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
>   src/slave/containerizer/mesos/isolators/cgroups/cpushare.hpp 
> 74982a610b6c0a74734165a0c6aa8c9f72f54deb 
>   src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp 
> 221e814a448c4b5df9dab98de597451a24e2b89c 
>   src/tests/containerizer/isolator_tests.cpp 
> 827c9f06b97191a5b74bb8fa2ef716c4282e7518 
> 
> Diff: https://reviews.apache.org/r/50733/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50751: Removed CgroupsPerfEventIsolatorProcess.

[haosdent huang]

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

(Updated Aug. 10, 2016, 4:36 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed CgroupsPerfEventIsolatorProcess.


Diffs (updated)
-

  src/CMakeLists.txt 6088c26d3465c3abb908495da3fa5b98c3062d16 
  src/Makefile.am cf76a699c33debb129722b7cba63a4e9e09f84f7 
  src/slave/containerizer/mesos/containerizer.cpp 
86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
  src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp 
4abde12af68a26b94b3706cdb38bf9890d811039 
  src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
31f35385691681ef5da14be747edfb5f57c5d05a 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
  src/tests/containerizer/isolator_tests.cpp 
827c9f06b97191a5b74bb8fa2ef716c4282e7518 

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


Testing
---


Thanks,

haosdent huang

Review Request 50954: Added documentation to Appc runtime support.

[Srinivas Brahmaroutu]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added documentation to Appc runtime support.


Diffs
-

  docs/container-image.md 0feb0286218d1774f47640a7e41f0f65c6181db0 
  docs/mesos-containerizer.md 76544625838845fe2817347fea483a60ab250f99 

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


Testing
---

Tested documentation update.


Thanks,

Srinivas Brahmaroutu

Re: Review Request 50750: Removed CgroupsNetClsIsolatorProcess.

[haosdent huang]

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

(Updated Aug. 10, 2016, 4:36 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed CgroupsNetClsIsolatorProcess.


Diffs (updated)
-

  src/CMakeLists.txt 6088c26d3465c3abb908495da3fa5b98c3062d16 
  src/Makefile.am cf76a699c33debb129722b7cba63a4e9e09f84f7 
  src/slave/containerizer/mesos/containerizer.cpp 
86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b3fd8c85476bf46368bd79f052b7923ad9d32199 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
bebefbba7153d4b0e9a8d7179cfb642e6e802bea 
  src/tests/containerizer/isolator_tests.cpp 
827c9f06b97191a5b74bb8fa2ef716c4282e7518 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50748: Removed CgroupsMemIsolatorProcess.

[haosdent huang]

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

(Updated Aug. 10, 2016, 4:35 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed CgroupsMemIsolatorProcess.


Diffs (updated)
-

  src/CMakeLists.txt 6088c26d3465c3abb908495da3fa5b98c3062d16 
  src/Makefile.am cf76a699c33debb129722b7cba63a4e9e09f84f7 
  src/docker/docker.cpp e07c4aa723f57db7f8bc31d4af5d32a30ebf98f2 
  src/slave/containerizer/mesos/containerizer.cpp 
86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
  src/slave/containerizer/mesos/isolators/cgroups/mem.hpp 
b3ce6ed2505312bdd2d800164c2f57cd7625c9fa 
  src/slave/containerizer/mesos/isolators/cgroups/mem.cpp 
0a4f38ded5cffd438e2a3b1e7d066c3077557f0d 
  src/tests/containerizer/isolator_tests.cpp 
827c9f06b97191a5b74bb8fa2ef716c4282e7518 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50749: Removed CgroupsDevicesIsolatorProcess.

[haosdent huang]

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

(Updated Aug. 10, 2016, 4:35 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed CgroupsDevicesIsolatorProcess.


Diffs (updated)
-

  src/CMakeLists.txt 6088c26d3465c3abb908495da3fa5b98c3062d16 
  src/Makefile.am cf76a699c33debb129722b7cba63a4e9e09f84f7 
  src/slave/containerizer/mesos/containerizer.cpp 
86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
  src/slave/containerizer/mesos/isolators/cgroups/devices.hpp 
7ad96440b21a380c5d9af27b0168e9abf47769af 
  src/slave/containerizer/mesos/isolators/cgroups/devices.cpp 
f1b5e75d23780c0e1d53487852422b02c88de9e8 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50733: Removed CgroupsCpushareIsolatorProcess.

[haosdent huang]

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

(Updated Aug. 10, 2016, 4:35 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed CgroupsCpushareIsolatorProcess.


Diffs (updated)
-

  src/CMakeLists.txt 6088c26d3465c3abb908495da3fa5b98c3062d16 
  src/Makefile.am cf76a699c33debb129722b7cba63a4e9e09f84f7 
  src/docker/docker.cpp e07c4aa723f57db7f8bc31d4af5d32a30ebf98f2 
  src/slave/containerizer/mesos/containerizer.cpp 
86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
  src/slave/containerizer/mesos/isolators/cgroups/cpushare.hpp 
74982a610b6c0a74734165a0c6aa8c9f72f54deb 
  src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp 
221e814a448c4b5df9dab98de597451a24e2b89c 
  src/tests/containerizer/isolator_tests.cpp 
827c9f06b97191a5b74bb8fa2ef716c4282e7518 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49854: Implemented `DevicesSubsystem`.

[haosdent huang]

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

(Updated Aug. 10, 2016, 4:34 p.m.)


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


Changes
---

Address @qianzhang's comment.


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


Repository: mesos


Description
---

Implemented `DevicesSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
2659252d8cffcefc233bc85fb4707c8147272737 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49855: Enabled cgroups unified isolator in isolation.

[haosdent huang]

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

(Updated Aug. 10, 2016, 4:35 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Enabled cgroups unified isolator in isolation.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
86a8d8a85b6a33c87798108cb65af85bb9bbbc77 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49853: Implemented `PerfEventSubsystem`.

[haosdent huang]

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

(Updated Aug. 10, 2016, 4:33 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `PerfEventSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
2659252d8cffcefc233bc85fb4707c8147272737 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49852: Implemented `NetClsSubsystem`.

[haosdent huang]

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

(Updated Aug. 10, 2016, 4:32 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `NetClsSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
2659252d8cffcefc233bc85fb4707c8147272737 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b3fd8c85476bf46368bd79f052b7923ad9d32199 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
bebefbba7153d4b0e9a8d7179cfb642e6e802bea 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

[haosdent huang]

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

(Updated Aug. 10, 2016, 4:32 p.m.)


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


Changes
---

Address?@qianxhang's comment


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


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
b191b2a52a9645fc902a35ed52909b2142f0b4c0 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
2659252d8cffcefc233bc85fb4707c8147272737 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49849: Implemented `CpuSubsystem`.

[haosdent huang]

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

(Updated Aug. 10, 2016, 4:31 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CpuSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
2659252d8cffcefc233bc85fb4707c8147272737 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49828: Added default methods implementations for `Subsystem` base class.

[haosdent huang]

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

(Updated Aug. 10, 2016, 4:31 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added default methods implementations for `Subsystem` base class.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

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

[haosdent huang]

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

(Updated Aug. 10, 2016, 4:31 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 
b191b2a52a9645fc902a35ed52909b2142f0b4c0 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
2659252d8cffcefc233bc85fb4707c8147272737 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49850: Implemented `CpuacctSubsystem`.

[haosdent huang]

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

(Updated Aug. 10, 2016, 4:31 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CpuacctSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
2659252d8cffcefc233bc85fb4707c8147272737 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Review Request 50758: Updated `UserCgroupIsolatorTest` to use `CgroupsIsolatorProcess`.

[haosdent huang]

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

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


Repository: mesos


Description
---

Updated `UserCgroupIsolatorTest` to use `CgroupsIsolatorProcess`.


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
827c9f06b97191a5b74bb8fa2ef716c4282e7518 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Guangya Liu]

> On 八月 10, 2016, 3:08 p.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/isolators/gpu/allocator.cpp, line 153
> > 
> >
> > What about update the message here as well by highlighting `mesos 
> > containerizer`?
> > 
> > Such as 
> > 
> > ```
> > return Error("'--nvidia_gpus_devices' can only be specified if the "
> >  "`--isolation` flag contains 'gpu/nvidia' with `mesos` "
> >  "containerizer");
> > ```

Correct a typo here: I prefer that we use `'` but not ``` for the error message.

```
return Error("'--nvidia_gpus_devices' can only be specified if the "
 "'--isolation' flag contains 'gpu/nvidia' with 'mesos' "
 "containerizer");
```


- Guangya


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


On 八月 10, 2016, 10:33 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50947/
> ---
> 
> (Updated 八月 10, 2016, 10:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5975
> https://issues.apache.org/jira/browse/MESOS-5975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
> docker and external containerizers do not need it anymore. This removed
> the 'gpu/nvidia' isolator check for docker and external containerizers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
> c1a87e9e5c07529bc1d077f68477108a40506806 
> 
> Diff: https://reviews.apache.org/r/50947/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Guangya Liu]

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




src/slave/containerizer/mesos/isolators/gpu/allocator.cpp (line 147)


The `external containerizers` was already removed in 
https://reviews.apache.org/r/50914/



src/slave/containerizer/mesos/isolators/gpu/allocator.cpp (lines 148 - 155)


What about 

```
// Don't allow the `--nvidia-gpu_devices` flag without the GPU isolator
// when using `mesos` containerizer.
if (strings::contains(flags.containerizers, "mesos") &&
flags.nvidia_gpu_devices.isSome() &&
isolators.count("gpu/nvidia") == 0) {
  return Error("'--nvidia_gpus_devices' can only be specified if the"
   " `--isolation` flag contains 'gpu/nvidia'");
}
```



src/slave/containerizer/mesos/isolators/gpu/allocator.cpp (line 152)


Not yours, but we prefer adding the space in the first line, which is 

```
return Error("'--nvidia_gpus_devices' can only be specified if the "
```



src/slave/containerizer/mesos/isolators/gpu/allocator.cpp (line 153)


What about update the message here as well by highlighting `mesos 
containerizer`?

Such as 

```
return Error("'--nvidia_gpus_devices' can only be specified if the "
 "`--isolation` flag contains 'gpu/nvidia' with `mesos` "
 "containerizer");
```


- Guangya Liu


On 八月 10, 2016, 10:33 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50947/
> ---
> 
> (Updated 八月 10, 2016, 10:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5975
> https://issues.apache.org/jira/browse/MESOS-5975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
> docker and external containerizers do not need it anymore. This removed
> the 'gpu/nvidia' isolator check for docker and external containerizers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
> c1a87e9e5c07529bc1d077f68477108a40506806 
> 
> Diff: https://reviews.apache.org/r/50947/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50872: Removed the redundant `mesos::` namespace prefix + minor style fixes.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Aug. 6, 2016, 1:35 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50872/
> ---
> 
> (Updated Aug. 6, 2016, 1:35 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed the redundant `mesos::` namespace prefix + minor style fixes.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.hpp 1e2d49b6a465c13dd055e54f0d4c49d22afc15c6 
>   src/internal/evolve.cpp 64818ccbbc4d0fcf6744e3f9a30c17c5332a 
> 
> Diff: https://reviews.apache.org/r/50872/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 50123: Added GPU scheduler for docker containerizer process.

[Yubo Li]

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

(Updated 八月 10, 2016, 10:34 a.m.)


Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.


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


Repository: mesos


Description
---

This added 'NvidiaGpuAllocator' to docker containerizer process so that
docker containerizer process is ready to use it to allocate GPUs to task
with 'gpus' resource.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 43ca4317d608b3b43dd7bd0d1b55c721e7364885 
  src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 
  src/tests/mesos.hpp 9174a3810d0da34b99dc257e9c77f83107fdd9f5 
  src/tests/mesos.cpp 30492d7e3b4c5e9ae9d2b2446cadba62d43a3c65 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50841: Added GPU scheduling logic to docker containerizer process.

[Yubo Li]

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

(Updated 八月 10, 2016, 10:34 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Added control logic to allocate/deallocate GPUs to GPU-related task
when the task is started/terminated.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 43ca4317d608b3b43dd7bd0d1b55c721e7364885 
  src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50125: Added mesos-docker-executor support for devices control.

[Yubo Li]

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

(Updated 八月 10, 2016, 10:33 a.m.)


Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.


Summary (updated)
-

Added mesos-docker-executor support for devices control.


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


Repository: mesos


Description (updated)
---

Added a new flag '--devices' to mesos-docker-executor, and gave its
feature to control devices exposition, isolation, and access permission.


Diffs (updated)
-

  src/docker/executor.hpp 7b63d784d6b8685912598b77fb38cf6e70646ae3 
  src/docker/executor.cpp 445628c9164facdd8bd812c5b45e3b2b886ebf0e 
  src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50128: Added helper functions to 'Docker::Device'.

[Yubo Li]

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

(Updated 八月 10, 2016, 10:34 a.m.)


Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.


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


Repository: mesos


Description
---

Wrapped helper functions 'serialize()' and 'parse()' to 'Docker::Device'
to handle data tranformation between 'Docker::Device' structure and
string.


Diffs (updated)
-

  src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
  src/docker/docker.cpp e8d2cb9662af34d75c9e2d822004f58fac76e7e0 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50599: Passed allocated GPUs to 'devices' entry of 'docker::Flags'.

[Yubo Li]

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

(Updated 八月 10, 2016, 10:33 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


Summary (updated)
-

Passed allocated GPUs to 'devices' entry of 'docker::Flags'.


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


Repository: mesos


Description (updated)
---

Added a new string entry 'devices' to 'docker::Flags' to
indicate host devices exposed into docker container, and
passed allocated GPUs to it.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 

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


Testing
---

make check


Thanks,

Yubo Li

Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
c1a87e9e5c07529bc1d077f68477108a40506806 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50127: Added DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch.

[Yubo Li]

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

(Updated 八月 10, 2016, 10:32 a.m.)


Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.


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


Repository: mesos


Description
---

This added a testing case for end-to-end GPU support for docker
containerizer.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
4e5b1fd1628504d346cced545f7911d6b6443773 

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


Testing
---

GTEST_FILTER="DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch" make -j 
check


Thanks,

Yubo Li