Re: Review Request 55139: Implemented parse methods for OCI image spec.

[Qian Zhang]

> On Feb. 5, 2017, 12:02 p.m., Jie Yu wrote:
> > src/oci/spec.cpp, lines 163-166
> > 
> >
> > what if `digest.isNone()`?
> 
> Qian Zhang wrote:
> `digest` is a required field, so it can not be `None()` here.
> 
> Jie Yu wrote:
> I'd prefer to be a bit more defensive here, and self contained. Either 
> add a CHECK here, or doing explicit check of `isNone()`. I prefer the latter.

OK, let me do an explicit check of `isNone()`.


- Qian


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


On Feb. 6, 2017, 10:43 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55139/
> ---
> 
> (Updated Feb. 6, 2017, 10:43 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6681
> https://issues.apache.org/jira/browse/MESOS-6681
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented parse methods for OCI image spec.
> 
> 
> Diffs
> -
> 
>   include/mesos/oci/spec.hpp PRE-CREATION 
>   src/CMakeLists.txt c09bcde3c14a73ab0b296c400f79f6a775da2470 
>   src/Makefile.am 78d55d86b1e1238beda3163d3240c85609ea982d 
>   src/oci/spec.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/55139/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 56360: Fixed consistency in error messages.

[Jay Guo]

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

(Updated Feb. 7, 2017, 3:34 p.m.)


Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Fixed consistency in error messages.


Diffs (updated)
-

  src/master/validation.cpp 2beee167439fe39d4f595c6b858fb6175321fbcd 

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


Testing
---


Thanks,

Jay Guo

Re: Review Request 56375: Added a test to ensure offers with different roles cannot be combined.

[Jay Guo]

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

(Updated Feb. 7, 2017, 3:34 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Guangya Liu.


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


Repository: mesos


Description
---

Multi-role framework cannot combine offers allocated to different
roles of that framework in a single launchTask call.


Diffs (updated)
-

  src/tests/master_tests.cpp 3b4123b49ee32c902a5d2a01fcc7026da21fdd18 

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


Testing
---

Added a test
make check GTEST_FILTER="MasterTest.LaunchDifferentRoleLost"


Thanks,

Jay Guo

Re: Review Request 54650: Added validation for roles in ACCEPT call.

[Jay Guo]

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

(Updated Feb. 7, 2017, 3:34 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Guangya Liu.


Changes
---

remove TODO


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


Repository: mesos


Description
---

For a multi-role framework, it may receive offers for different roles
in that framework. However, we disallow multiple offers with different
roles being accepted in single ACCEPT call.


Diffs (updated)
-

  src/master/master.cpp 98c39b279e7b9830d02efc8ec6a4469afc15d62a 
  src/master/validation.cpp 2beee167439fe39d4f595c6b858fb6175321fbcd 

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


Testing
---

make check


Thanks,

Jay Guo

Re: Review Request 56370: Added a test to ensure multi-role framework receiving offers.

[Jay Guo]

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

(Updated Feb. 7, 2017, 3:29 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Guangya Liu.


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


Repository: mesos


Description
---

This test ensures that multi-role framework should receive offers for
each of its roles.


Diffs (updated)
-

  src/tests/master_tests.cpp 3b4123b49ee32c902a5d2a01fcc7026da21fdd18 

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


Testing
---

Added a test.

make check GTEST_FILTER="MasterTest.MultiRoleFrameworkReceivesOffers"


Thanks,

Jay Guo

Review Request 56375: Added a test to ensure offers with different roles cannot be combined.

[Jay Guo]

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

Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Guangya Liu.


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


Repository: mesos


Description
---

Multi-role framework cannot combine offers allocated to different
roles of that framework in a single launchTask call.


Diffs
-

  src/tests/master_tests.cpp 3b4123b49ee32c902a5d2a01fcc7026da21fdd18 

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


Testing
---

Added a test
make check GTEST_FILTER="MasterTest.LaunchDifferentRoleLost"


Thanks,

Jay Guo

Re: Review Request 54135: Added test for offer rescind in quota update.

[Zhitao Li]

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

(Updated Feb. 7, 2017, 7:15 a.m.)


Review request for mesos, Alexander Rukletsov and Xiaojian Huang.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added test for offer rescind in quota update.


Diffs (updated)
-

  src/tests/master_quota_tests.cpp b25f5911579c435549b9bc65994627414357dcb6 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 53993: Updated quota doc to support quota update.

[Zhitao Li]

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

(Updated Feb. 7, 2017, 7:15 a.m.)


Review request for mesos, Alexander Rukletsov and Xiaojian Huang.


Changes
---

Rebase and review comments.


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


Repository: mesos


Description
---

Updated quota doc to support quota update.


Diffs (updated)
-

  docs/quota.md 36b4853b524ba7b28788fe1f3983dd43d95072d4 

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


Testing
---

https://github.com/zhitaoli/mesos/blob/quota_update_sept/docs/quota.md


Thanks,

Zhitao Li

Re: Review Request 53991: Added master API call for `UPDATE_QUOTA`.

[Zhitao Li]

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

(Updated Feb. 7, 2017, 7:14 a.m.)


Review request for mesos, Alexander Rukletsov and Xiaojian Huang.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added master API call for `UPDATE_QUOTA`.


Diffs (updated)
-

  include/mesos/master/master.proto a2228db8902c297f137b8106a7b3d2babbc35017 
  include/mesos/v1/master/master.proto cfdca7400d98233c6320eebd8784e4f51c43ebbd 
  src/master/http.cpp d881ad6dba9ba96057988db265faf0b3013c9b05 
  src/master/master.hpp a7e3c739f76d724070e1ae51c3093636a79e8008 
  src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
  src/master/validation.cpp 2beee167439fe39d4f595c6b858fb6175321fbcd 
  src/tests/api_tests.cpp aa7d89a710400d1460bc9139ea563cfdd45bd58f 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 52103: Implemented quota update through `PUT` method.

[Zhitao Li]

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

(Updated Feb. 7, 2017, 7:14 a.m.)


Review request for mesos, Alexander Rukletsov and Xiaojian Huang.


Changes
---

Rebase.


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


Repository: mesos


Description
---

This patch implemented atomic quota update through
`PUT` methood on the `/quota` endpoint.

Changes included:
- a new virtual function on the allocator interface-
- implementation in hierarchical allocator;
- implementation in quota_handler;
- some integration tests for updating a quota.

More tests around resource rescind and operator API
implementation will be sent in later patches.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
71a40537b673e44ecdd5327d9a9f083faa7fc13a 
  src/master/allocator/mesos/allocator.hpp 
e3c86181677302dbfc3b213715196122f96e312c 
  src/master/allocator/mesos/hierarchical.hpp 
896abcdf0727f986eef3a1a9304a0e4847094057 
  src/master/allocator/mesos/hierarchical.cpp 
fc93ade661bedb6c915d1ee634b02cf28b9b24d9 
  src/master/http.cpp d881ad6dba9ba96057988db265faf0b3013c9b05 
  src/master/master.hpp a7e3c739f76d724070e1ae51c3093636a79e8008 
  src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
  src/tests/allocator.hpp 32c291213d18d1c8fe5d9e8194b92c10716b9961 
  src/tests/master_quota_tests.cpp b25f5911579c435549b9bc65994627414357dcb6 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 53691: Implemented some quota functionality tests.

[Zhitao Li]

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

(Updated Feb. 7, 2017, 7:13 a.m.)


Review request for mesos, Alexander Rukletsov and Xiaojian Huang.


Changes
---

Rebase and review comments.


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


Repository: mesos


Description (updated)
---

This implements some tests in previous todos for
quota functionality.


Diffs (updated)
-

  src/tests/master_quota_tests.cpp b25f5911579c435549b9bc65994627414357dcb6 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 54650: Added validation for roles in ACCEPT call.

[Benjamin Bannier]

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


Ship it!





src/master/validation.cpp (line 1462)


I created https://issues.apache.org/jira/browse/MESOS-7072 to clean up the 
use of `T*` for in parameters here.



src/master/validation.cpp (line 1473)


Small nit: `role()` returns a `const string&`, and we just store the result 
in `_role` to have a convenient short-hand. If we'd bind the result of `role()` 
to a `const string&` instead of creating a copy we could avoid needless 
creation of strings for all but the first element (where we store a copy in 
`role`).


- Benjamin Bannier


On Feb. 7, 2017, 3:10 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54650/
> ---
> 
> (Updated Feb. 7, 2017, 3:10 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Guangya Liu.
> 
> 
> Bugs: MESOS-6637
> https://issues.apache.org/jira/browse/MESOS-6637
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For a multi-role framework, it may receive offers for different roles
> in that framework. However, we disallow multiple offers with different
> roles being accepted in single ACCEPT call.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 2beee167439fe39d4f595c6b858fb6175321fbcd 
> 
> Diff: https://reviews.apache.org/r/54650/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 53679: Implemented capacity heuristic check related tests.

[Zhitao Li]

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

(Updated Feb. 7, 2017, 7:11 a.m.)


Review request for mesos, Alexander Rukletsov, Xiaojian Huang, and Kunal Thakar.


Changes
---

Rebase and review comments.


Summary (updated)
-

Implemented capacity heuristic check related tests.


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


Repository: mesos


Description (updated)
---

Implemented capacity heuristic check related tests.


Diffs (updated)
-

  src/tests/master_quota_tests.cpp b25f5911579c435549b9bc65994627414357dcb6 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 52284: Implemented more quota validation tests.

[Zhitao Li]

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

(Updated Feb. 7, 2017, 7:10 a.m.)


Review request for mesos, Alexander Rukletsov and Xiaojian Huang.


Changes
---

Rebase and review comments.


Summary (updated)
-

Implemented more quota validation tests.


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


Repository: mesos


Description (updated)
---

Implemented more quota validation tests.


Diffs (updated)
-

  src/tests/master_quota_tests.cpp b25f5911579c435549b9bc65994627414357dcb6 

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


Testing
---


Thanks,

Zhitao Li

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 Feb. 7, 2017, 5:02 a.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Rebased.


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

  src/tests/hierarchical_allocator_tests.cpp 
c681d03c3f94f7d071143366a5aad0421108ebec 
  src/tests/resources_utils.hpp 1f41f02babce5c8174ea2223f4dc7470452fbaf1 
  src/tests/resources_utils.cpp 2cef55f7312d671307e097c2c4960c8dcf45c1ff 

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 10%) 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 6907us
Added 1000 agents in 2.057098secs
round 0 allocate() took 1.689164secs to make 1000 offers
round 50 allocate() took 1.672373secs to make 1000 offers
round 100 allocate() took 1.680571secs to make 1000 offers
round 150 allocate() took 1.674683secs to make 1000 offers
round 199 allocate() took 1.671525secs 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 6888us
Added 1000 agents in 2.096218secs
round 0 allocate() took 1.704491secs to make 1000 offers
round 50 allocate() took 1.718623secs to make 1000 offers
round 100 allocate() took 1.716224secs to make 1000 offers
round 150 allocate() took 1.707343secs to make 1000 offers
round 199 allocate() took 1.727467secs 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 7304us
Added 1000 agents in 2.071009secs
round 0 allocate() took 1.689045secs to make 1000 offers
round 50 allocate() took 1.691524secs to make 1000 offers
round 100 allocate() took 1.688873secs to make 1000 offers
round 150 allocate() took 1.688713secs to make 1000 offers
round 199 allocate() took 1.691223secs 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 55359: Consolidate update of allocations in `updateAllocation()`.

[Anindya Sinha]

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

(Updated Feb. 7, 2017, 5:02 a.m.)


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


Changes
---

Addressed comment and rebased.


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


Repository: mesos


Description
---

We handle shared resources for `LAUNCH` operations separately in the
`updateAllocation()` API to add additional copies of shared resources.
But the updates to the allocations in the allocator and sorters
can be consolidated together.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
56d6791baa64189523df668749f4a7ab67d6b363 

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


Testing
---

Tests passed.


Thanks,

Anindya Sinha

Review Request 56370: Added a test to ensure multi-role framework receiving offers.

[Jay Guo]

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

Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Guangya Liu.


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


Repository: mesos


Description
---

This test ensures that multi-role framework should receive offers for
each of its roles.


Diffs
-

  src/tests/master_tests.cpp 3b4123b49ee32c902a5d2a01fcc7026da21fdd18 

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


Testing
---

Added a test.

make check GTEST_FILTER="MasterTest.MultiRoleFrameworkReceivesOffers"


Thanks,

Jay Guo

Review Request 56367: Stout: Windows: patch `os::killtree` to terminate job objects.

[Andrew Schwartzmeyer]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description
---

Before we deprecate `os::killtree`, we need to it at least work
with the new job object semantics of the `WindowsLauncher`.
Given the PID of the "process tree" to kill, this maps it to the
name used for the job object, opens a new handle to it, and calls
`os::kill_job` to terminate it.


Diffs
-

  3rdparty/stout/include/stout/os/windows/killtree.hpp 
2be2f8c3d58ee64410f87ee4a4b2bb54fe014748 

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 56363: Stout: Add predicate `find` semantics to `hashmap`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description
---

`Option hashmap::findIf(lambda::function predicate)`
filters `hashmap` values given a predicate (for more advanced kinds of
`Value`, say compare structs on a field).

`bool hashmap::containsIf(lambda::function predicate)`
checks whether a `Value` matching predicate exists in the `hashmap`
(just passes to `findIf` and acts like `hashmap::contains`).


Diffs
-

  3rdparty/stout/include/stout/hashmap.hpp 
539bbfd9250f2f372c2b3211e2379ad2229fa35e 

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 56366: Windows: Refactor executor to use `SharedHandle` semantics.

[Andrew Schwartzmeyer]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description
---

The executor now owns the `SharedHandle` for the process and job object,
so that their lifetime is correctly tied to the executor and their
handles will be closed properly.


Diffs
-

  src/launcher/executor.cpp 0c770bb18ae8bd8df85589b5262f457ab50574a9 
  src/launcher/windows/executor.hpp 6f02912c477105819b4c27ddf248b7289799eaa0 
  src/launcher/windows/executor.cpp b51fde7376c2083119f342ea599b446944ede000 

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 56365: Windows: Refactor `WindowsLauncher` to use Job Objects.

[Andrew Schwartzmeyer]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description
---

This commit refactors the existing `WindowsLauncher` (copied from
the `PosixLauncher`) to treat a group of processes associated with
a task/container as a "Job Object" instead of a root PID and
the tree containing its children. The latter is not a reasonable
approach on Windows, and has been the source of subtle bugs.

The Job Object approach creates a named job with a one-to-one
mapping to the containerizer, and assigns the first process
started for the task to the job object. After being assigned,
the Windows kernel ensures all spawned child processes
(specifically those made with the `CreateProcess` system call)
are also assigned to the named job object. Thus this job object
can then be used to query the process group's resource usage,
kill the process group, and set limits on the process group.

So instead of seeing a process group as a tree and referring to it
by the root process's PID, the `WindowsLauncher` sees a process group
as a named Job Object, the same way the Windows kernel sees it.
However, the containerizer code which interacts with the launcher
still refers to a task group by the singular PID, and so we have
a sort of shim which maps the initial PID to the name of the job object.
This is a an unfortunate consequence of the shared containerizer
code being originally written for POSIX-like systems.

This abstraction sets us up for implementing resource usage limits
on the process group as a whole.


Diffs
-

  src/slave/containerizer/mesos/windows_launcher.hpp PRE-CREATION 
  src/slave/containerizer/mesos/windows_launcher.cpp PRE-CREATION 

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 56368: Libprocess: Windows: Remove `CREATE_JOB` parent hook.

[Andrew Schwartzmeyer]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description
---

Libprocess: Windows: Remove `CREATE_JOB` parent hook.


Diffs
-

  3rdparty/libprocess/src/subprocess.cpp 
bc1b99b8f84332f42623c35fc2b8d5186ba8af70 

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


Testing
---

This is the last patch in a series. Applied all together, they were tested with 
`make && make check` on Linux, and `support\windows-build.bat` on Windows. All 
tests pass (minus flaky ones which sometimes fail but that's unrelated).


Thanks,

Andrew Schwartzmeyer

Review Request 56364: Windows: Stout: Rewrite Job Object wrappers.

[Andrew Schwartzmeyer]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description
---

`os::create_job` now returns a `Try` instead of a raw
`HANDLE`, forcing ownership of the job object handle onto the caller
of the function. `create_job` requires a `std::string name` for the
job object, which is mapped from a PID using `os::name_job`.

The assignment of a process to the job object is now done via
`Try os::assign_job(SharedHandle, pid_t)`.

The equivalent of killing a process tree with job object semantics
is simply to terminate the job object. This is done via
`os::kill_job(SharedHandle)`.


Diffs
-

  3rdparty/stout/include/stout/windows/os.hpp 
b5172fca96c4151f4b1ebb6d343022558f45fc34 

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 56362: Windows: Create the `WindowsLauncher`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description
---

Instead of deriving `WindowsLauncher` from `PosixLauncher`,
this commit implements a separate `WindowsLauncher` derived
from `Launcher` parallel to the `PosixLauncher`.

The purpose of this is to setup for refactoring the `WindowsLauncher`
into a `Launcher` that uses Windows' "Job Objects,"
which are similar to Linux's cgroups, and enable us to reason
about a group of processes. It is necessary to do this in the
`WindowsLaucher` because the current implementation uses the POSIX
idea of a process hierarchy to list and kill all processes
associated with a task. We have found this model to not work on
Windows, and the solution is to use Job Objects.

Because this is preparation work, it is a straight copy of the
current "working" implementation. That is, the `PosixLauncher`
has been copied to create the `WindowsLauncher`, instead of
the latter deriving the former.


Diffs
-

  src/CMakeLists.txt c09bcde3c14a73ab0b296c400f79f6a775da2470 
  src/slave/containerizer/mesos/containerizer.cpp 
d2b4f75a55dbe4746bc2dfc180335fa831a554ef 
  src/slave/containerizer/mesos/launcher.hpp 
79f6eea0ee8e564e90b36208672df150dbc5d540 
  src/slave/containerizer/mesos/launcher.cpp 
5114c130efbfb252dde1e85c081f5174e66f57af 
  src/slave/containerizer/mesos/windows_launcher.hpp PRE-CREATION 
  src/slave/containerizer/mesos/windows_launcher.cpp PRE-CREATION 

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 56359: Add framework principal to the slave state endpoint

[Jeff Malnick]

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

Review request for mesos and Adam B.


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


Repository: mesos


Description
---

This commit adds the framework principal to the slave state endpoint. The 
documentation for the agent state API needs to be updated to reflect this 
change.


Diffs
-

  src/slave/http.cpp d68c9adc1db43c2e853c8b2290705fdc7739d45c 

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


Testing
---

Tests are in progress.


Thanks,

Jeff Malnick

Review Request 56360: Fixed consistency in error messages.

[Jay Guo]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Fixed consistency in error messages.


Diffs
-

  src/master/validation.cpp 2beee167439fe39d4f595c6b858fb6175321fbcd 

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


Testing
---


Thanks,

Jay Guo

Re: Review Request 54650: Added validation for roles in ACCEPT call.

[Jay Guo]

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

(Updated Feb. 7, 2017, 10:10 a.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Guangya Liu.


Changes
---

rebase and address comments from BenM and Guangya


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


Repository: mesos


Description
---

For a multi-role framework, it may receive offers for different roles
in that framework. However, we disallow multiple offers with different
roles being accepted in single ACCEPT call.


Diffs (updated)
-

  src/master/validation.cpp 2beee167439fe39d4f595c6b858fb6175321fbcd 

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


Testing
---

make check


Thanks,

Jay Guo

Re: Review Request 55160: Added test for DockerContainerizer when `cgroups_enable_cfs` is set.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On Feb. 6, 2017, 9:29 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55160/
> ---
> 
> (Updated Feb. 6, 2017, 9:29 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-6134
> https://issues.apache.org/jira/browse/MESOS-6134
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for DockerContainerizer when `cgroups_enable_cfs` is set.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 31d63b1f239055d82470ace9024b584a2096dce4 
> 
> Diff: https://reviews.apache.org/r/55160/diff/
> 
> 
> Testing
> ---
> 
> Run the new test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 56327: Updated Suppress and Revive proto to support per role.

[Guangya Liu]

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

(Updated 二月 7, 2017, 2:08 a.m.)


Review request for mesos, Benjamin Mahler and Jay Guo.


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


Repository: mesos


Description
---

Updated Suppress and Revive proto to support per role.


Diffs (updated)
-

  include/mesos/scheduler/scheduler.proto 
5f4635d523286754a61aa99e18e79d6c1db9463f 
  include/mesos/v1/scheduler/scheduler.proto 
096c76dfffe03c0e2d6abe84d438c396cc1b0be9 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 56284: Added ProvisionerDockerLocalStoreTest.MissingLayer test.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [56174, 56284]

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 Feb. 6, 2017, 8:40 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56284/
> ---
> 
> (Updated Feb. 6, 2017, 8:40 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-7045
> https://issues.apache.org/jira/browse/MESOS-7045
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new test verifies that that local Docker puller and store will skip 
> layers that are already pulled.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 99e0820fa47303896f70c81cfb91cd34f0474e55 
> 
> Diff: https://reviews.apache.org/r/56284/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 56209: Hashed unacknowledged updates by UUID string in command executor.

[Vinod Kone]

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



Why this change? I can't tell from the linked ticket or the description.

- Vinod Kone


On Feb. 2, 2017, 9:56 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56209/
> ---
> 
> (Updated Feb. 2, 2017, 9:56 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-6906
> https://issues.apache.org/jira/browse/MESOS-6906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 0c770bb18ae8bd8df85589b5262f457ab50574a9 
> 
> Diff: https://reviews.apache.org/r/56209/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/56218/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 56208: Updated checks library with general check support.

[Vinod Kone]

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




src/checks/checker.hpp (line 60)


s/prior/prior to/
s/a check/the check/



src/checks/checker.hpp (line 71)


const & ?



src/checks/checker.hpp (line 88)


Any reason why this is defined here instead of inside the cpp file?



src/checks/checker.hpp (line 96)


const &?



src/checks/checker.hpp (line 105)


s/performSingleCheck/performCheck/ ?

similar to how you don't say `singleCommandCheck` or `singleHttpCheck` below



src/checks/checker.hpp (line 126)


const?



src/checks/checker.cpp (line 123)


extraneous back tick.



src/checks/checker.cpp (line 141)


put it on the previous line?



src/checks/checker.cpp (line 273)


is the equality operator defined for the above statement to be true?



src/checks/checker.cpp (line 274)


i.e.,



src/checks/checker.cpp (line 300)


s/external/s/ like you did below for http check?


- Vinod Kone


On Feb. 2, 2017, 9:56 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56208/
> ---
> 
> (Updated Feb. 2, 2017, 9:56 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-6906
> https://issues.apache.org/jira/browse/MESOS-6906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for general checks, i.e. defined by CheckInfo, in
> checking library. A general check can be either an command or
> an HTTP request. The library performs the requested check at
> the specified interval and sends the result to the framework
> via a task status update. If the current result is the same as
> the previous result, no status update is sent.
> 
> 
> Diffs
> -
> 
>   src/checks/checker.hpp dc293f3d3613dec716510d269829f8a6f406c277 
>   src/checks/checker.cpp 8716e4cc684e6c4b6b76d8ca53221be06d10b2a6 
>   src/checks/health_checker.hpp 95da1ff7dd6b222a93076633eb3757ec9aa43cf6 
>   src/checks/health_checker.cpp 58380dc18896f659aa9c4fb4bb567a55bba97f6b 
> 
> Diff: https://reviews.apache.org/r/56208/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/56218/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 56341: Changed docker/runtime isolator's handling of Environment.

[Joseph Wu]

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

(Updated Feb. 6, 2017, 5:19 p.m.)


Review request for mesos, Jie Yu and Kevin Klues.


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


Repository: mesos


Description
---

This commit adds more special-casing in the `docker/runtime` isolator
for the command executor.  The command executor will generally break
when the `docker/runtime` isolator provides environment variables
directly to the executor.  This is because the environment variables
are provided in the context of the container image, rather than the
host.

For example, a container image may provide an environment variable
like `LD_LIBRARY_PATH=/image/specific/location`, whereas the default
executor expects to find libraries in the host's environment.  If the
image's environment end up in the command executor at launch time, 
the command executor may simply fail to launch.


Diffs
-

  src/launcher/executor.cpp 0c770bb18ae8bd8df85589b5262f457ab50574a9 
  src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
2d816e512c95ed2922c9578ba796908c5ce23da4 

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


Testing (updated)
---

make check

sudo src/mesos-tests --gtest_filter="*ROOT*"

All the tests broken by https://reviews.apache.org/r/56339/ (earlier in the 
chain) are now passing.


Thanks,

Joseph Wu

Re: Review Request 56340: Disabled ability to launch default executor with ContainerInfo.

[Joseph Wu]

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

(Updated Feb. 6, 2017, 5:18 p.m.)


Review request for mesos, Jie Yu and Kevin Klues.


Changes
---

Change a string in another validation test.  The TaskGroup in that test is now 
invalidated slightly earlier than before (and with a different string).


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


Repository: mesos


Description
---

The default executor used to launch TaskGroups is generated by the
agent.  The agent generates the executor's `CommandInfo`, hence why
the user may not specify the `CommandInfo` in the `LAUNCH_TASK_GROUP`
call.

This commit adds similar restrictions for `ContainerInfo` plus the
default executor.  The `CommandInfo` constructed by the agent expects
to be run in the same environment as the agent process.  This commit
prevents the user from specifying a `DockerInfo` or a container image
along with the default executor.

If the user explicitly wants to use the default executor, they could
always package the default executor's binary and libraries into a
container and launch it like any other custom executor.


Diffs (updated)
-

  include/mesos/mesos.proto 53885cbc63ac6658a749da5e05bb2301392f84dd 
  include/mesos/v1/mesos.proto c4ca6dea787cfe4661c9f0d9afb770bceb1c2639 
  src/master/validation.cpp 2beee167439fe39d4f595c6b858fb6175321fbcd 
  src/tests/master_validation_tests.cpp 
1f833aa8d6cacbd7bf502fb66c556e4e3d4f79e2 

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


Testing (updated)
---

src/mesos-tests --gtest_filter="ExecutorValidationTest.ExecutorType"

NOTE: The change to TaskGroupValidationTest.ExecutorUsesDockerContainerInfo is 
because the tested case now fails during "general" validation of the 
ExecutorInfo.  We still need the checks around `Option 
validateExecutor(TaskGroupInfo, ExecutorInfo, Framework*, Slave*, Resources)` 
because those checks are valid for custom executors.


Thanks,

Joseph Wu

Re: Review Request 55359: Consolidate update of allocations in `updateAllocation()`.

[Jiang Yan Xu]

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



LGTM. Just need to take another look after rebasing to ship it.


src/master/allocator/mesos/hierarchical.cpp (lines 682 - 683)


"contained" is ambiguous, I think it's better if we convey the idea of "at 
least one copy".

How about:

```
Ensure that offered resources contain at least one copy of the consumed 
shared resource (guaranteed by master validation).
```



src/master/allocator/mesos/hierarchical.cpp (lines 696 - 697)


So `additional` now encompasses multiple launch operations but it feels 
like it's still valuable to log the taskIds in this call for debugging?



src/master/allocator/mesos/hierarchical.cpp (lines 732 - 733)


So now we have two reasons:

```
The agent's total resources shouldn't contain:
1. The additionally allocated shared resources.
2. `AllocationInfo` as set in `updatedOfferedResources`.
```

Maybe spell them out like this?


- Jiang Yan Xu


On Feb. 3, 2017, 2:01 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55359/
> ---
> 
> (Updated Feb. 3, 2017, 2:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6444
> https://issues.apache.org/jira/browse/MESOS-6444
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We handle shared resources for `LAUNCH` operations separately in the
> `updateAllocation()` API to add additional copies of shared resources.
> But the updates to the allocations in the allocator and sorters
> can be consolidated together.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5f540569043df9d2bb75416c8c36bb4dd7bd68a1 
> 
> Diff: https://reviews.apache.org/r/55359/diff/
> 
> 
> Testing
> ---
> 
> Tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 56354: Fixed incorrect `CHECK_EQ`s in `WindowsFD`.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On Feb. 6, 2017, 11:23 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56354/
> ---
> 
> (Updated Feb. 6, 2017, 11:23 p.m.)
> 
> 
> Review request for mesos and Alex Clemmer.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We store both the `CRT` and `HANDLE` in `WindowsFD` whether we construct
> from `CRT` or `HANDLE` by using `_get_osfhandle` and `_open_osfhandle`.
> 
> Thus, accessing the `CRT` via `crt()` does not require that it be in the
> `CRT` state, but rather `CRT` or `HANDLE`, for example.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> 24d3661aad72817d8b9e3cd88fe6178ab60832bd 
> 
> Diff: https://reviews.apache.org/r/56354/diff/
> 
> 
> Testing
> ---
> 
> Ran `.\support\windows-build.bat` on a Windows VM.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 56354: Fixed incorrect `CHECK_EQ`s in `WindowsFD`.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Feb. 6, 2017, 3:23 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56354/
> ---
> 
> (Updated Feb. 6, 2017, 3:23 p.m.)
> 
> 
> Review request for mesos and Alex Clemmer.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We store both the `CRT` and `HANDLE` in `WindowsFD` whether we construct
> from `CRT` or `HANDLE` by using `_get_osfhandle` and `_open_osfhandle`.
> 
> Thus, accessing the `CRT` via `crt()` does not require that it be in the
> `CRT` state, but rather `CRT` or `HANDLE`, for example.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> 24d3661aad72817d8b9e3cd88fe6178ab60832bd 
> 
> Diff: https://reviews.apache.org/r/56354/diff/
> 
> 
> Testing
> ---
> 
> Ran `.\support\windows-build.bat` on a Windows VM.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 56270: Made the default executor launch multiple task groups.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Feb. 4, 2017, 5:53 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56270/
> ---
> 
> (Updated Feb. 4, 2017, 5:53 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6296
> https://issues.apache.org/jira/browse/MESOS-6296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a task fails or is explicitly killed by the scheduler, the
> default on termination policy kills the particular task group.
> The executor commits suicide when there are no active tasks.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 97eee05cac8cb1f62d43e2aecc08a8e54e49eac3 
>   src/tests/default_executor_tests.cpp 
> e8e0aa2f0d2508de6db03685c70d1ed9c1da3659 
> 
> Diff: https://reviews.apache.org/r/56270/diff/
> 
> 
> Testing
> ---
> 
> make check. Also, modified an existing test to launch/kill multiple task 
> groups.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 56269: Introduced a `Container` struct on the default executor.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Feb. 4, 2017, 5:51 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56269/
> ---
> 
> (Updated Feb. 4, 2017, 5:51 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6296
> https://issues.apache.org/jira/browse/MESOS-6296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This helps us in consolidating various metadata associatd with
> an active child container. Also, this would be used in the future
> for figuring out the task group corresponding to a child container
> and then killing the other child containers belonging to the task
> group when one of the tasks fail; or if a task in the task group
> is explicitly killed by the scheduler.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 97eee05cac8cb1f62d43e2aecc08a8e54e49eac3 
> 
> Diff: https://reviews.apache.org/r/56269/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 56268: Made `kill()` not invoke `shutdown()` in the default executor.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Feb. 4, 2017, 5:52 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56268/
> ---
> 
> (Updated Feb. 4, 2017, 5:52 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6296
> https://issues.apache.org/jira/browse/MESOS-6296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This ensures that when we add support for the default executor
> launch multiple task groups, kill for a particular task should
> _only_ result in the killing of a task group and not the entire
> executor as was the case currently.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 97eee05cac8cb1f62d43e2aecc08a8e54e49eac3 
>   src/tests/default_executor_tests.cpp 
> e8e0aa2f0d2508de6db03685c70d1ed9c1da3659 
> 
> Diff: https://reviews.apache.org/r/56268/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 56354: Fixed incorrect `CHECK_EQ`s in `WindowsFD`.

[Alex Clemmer]

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


Ship it!




Ship It!

- Alex Clemmer


On Feb. 6, 2017, 11:23 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56354/
> ---
> 
> (Updated Feb. 6, 2017, 11:23 p.m.)
> 
> 
> Review request for mesos and Alex Clemmer.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We store both the `CRT` and `HANDLE` in `WindowsFD` whether we construct
> from `CRT` or `HANDLE` by using `_get_osfhandle` and `_open_osfhandle`.
> 
> Thus, accessing the `CRT` via `crt()` does not require that it be in the
> `CRT` state, but rather `CRT` or `HANDLE`, for example.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> 24d3661aad72817d8b9e3cd88fe6178ab60832bd 
> 
> Diff: https://reviews.apache.org/r/56354/diff/
> 
> 
> Testing
> ---
> 
> Ran `.\support\windows-build.bat` on a Windows VM.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 56354: Fixed incorrect `CHECK_EQ`s in `WindowsFD`.

[Michael Park]

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

(Updated Feb. 6, 2017, 3:23 p.m.)


Review request for mesos and Alex Clemmer.


Repository: mesos


Description
---

We store both the `CRT` and `HANDLE` in `WindowsFD` whether we construct
from `CRT` or `HANDLE` by using `_get_osfhandle` and `_open_osfhandle`.

Thus, accessing the `CRT` via `crt()` does not require that it be in the
`CRT` state, but rather `CRT` or `HANDLE`, for example.


Diffs
-

  3rdparty/stout/include/stout/os/windows/fd.hpp 
24d3661aad72817d8b9e3cd88fe6178ab60832bd 

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


Testing (updated)
---

Ran `.\support\windows-build.bat` on a Windows VM.


Thanks,

Michael Park

Review Request 56354: Fixed incorrect `CHECK_EQ`s in `WindowsFD`.

[Michael Park]

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

Review request for mesos and Alex Clemmer.


Repository: mesos


Description
---

We store both the `CRT` and `HANDLE` in `WindowsFD` whether we construct
from `CRT` or `HANDLE` by using `_get_osfhandle` and `_open_osfhandle`.

Thus, accessing the `CRT` via `crt()` does not require that it be in the
`CRT` state, but rather `CRT` or `HANDLE`, for example.


Diffs
-

  3rdparty/stout/include/stout/os/windows/fd.hpp 
24d3661aad72817d8b9e3cd88fe6178ab60832bd 

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


Testing
---


Thanks,

Michael Park

Re: Review Request 56352: Optimized and simplified `DRFSorter::allocated()`.

[Neil Conway]

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

(Updated Feb. 6, 2017, 11:21 p.m.)


Review request for mesos and Michael Park.


Repository: mesos


Description
---

When an allocation is made to a client, we want to update the share and
the count of allocations made to that client. This was previously done
separately, resulting in two std::set lookups, removals, and
reinsertions. By improving `DRFSorter::updateShare` to support
incrementing the number of client-allocations, we can do both updates
with a single sequence of std::set lookup/remove/insert operations.


Diffs
-

  src/master/allocator/sorter/drf/sorter.hpp 
5bed53a1641fee0c09862f77b394a0e9ec131990 
  src/master/allocator/sorter/drf/sorter.cpp 
5681a5d78a7bdde820c3a8633d742d9d6412f1c7 

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


Testing (updated)
---

`make check`

Running the sorter benchmarks with an optimized build, this results in a modest 
improvement (~10%). With the change:

```
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/0 (343 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/1 (333 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/2 (329 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/3 (331 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/4 (335 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/5 (346 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/6 (1660 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/7 (1739 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/8 (1682 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/9 (1673 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/10 (1694 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/11 (1733 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/12 (3223 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/13 (3311 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/14 (3319 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/15 (3303 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/16 (3368 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/17 (3410 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/18 (6538 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/19 (6646 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/20 (6532 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/21 (6668 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/22 (6689 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/23 (6754 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/24 (9652 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/25 (9886 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/26 (9930 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/27 (10024 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/28 (10094 ms)
```

Without the change:

```
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/0 (346 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/1 (339 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/2 (346 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/3 (352 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/4 (343 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/5 (356 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/6 (1646 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/7 (1723 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/8 (1734 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/9 (1712 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/10 (1767 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/11 (2333 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/12 (3382 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/13 (3341 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/14 (3328 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/15 (3609 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/16 (3442 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/17 (3408 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/18 (6780 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/19 (6662 ms)
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/20 (7044 

Re: Review Request 56341: Changed docker/runtime isolator's handling of Environment.

[Joseph Wu]

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

(Updated Feb. 6, 2017, 3:19 p.m.)


Review request for mesos, Jie Yu and Kevin Klues.


Changes
---

Changed approach, as the docker/runtime isolator is already full of 
command-executor special-casing.  This also doesn't break other isolators that 
need to affect the command executor rather than the command task.


Summary (updated)
-

Changed docker/runtime isolator's handling of Environment.


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


Repository: mesos


Description (updated)
---

This commit adds more special-casing in the `docker/runtime` isolator
for the command executor.  The command executor will generally break
when the `docker/runtime` isolator provides environment variables
directly to the executor.  This is because the environment variables
are provided in the context of the container image, rather than the
host.

For example, a container image may provide an environment variable
like `LD_LIBRARY_PATH=/image/specific/location`, whereas the default
executor expects to find libraries in the host's environment.  If the
image's environment end up in the command executor at launch time, 
the command executor may simply fail to launch.


Diffs (updated)
-

  src/launcher/executor.cpp 0c770bb18ae8bd8df85589b5262f457ab50574a9 
  src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
2d816e512c95ed2922c9578ba796908c5ce23da4 

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


Testing (updated)
---

In progress...


Thanks,

Joseph Wu

Re: Review Request 55888: Test to ensure non-authorized users cannot launch tasks on agents.

[Anindya Sinha]

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

(Updated Feb. 6, 2017, 11:17 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Test to ensure non-authorized users cannot launch tasks on agents.


Diffs (updated)
-

  src/tests/slave_tests.cpp 7f357a653335b559b5c78d4618926715dc4a63c7 

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Review Request 56352: Optimized and simplified `DRFSorter::allocated()`.

[Neil Conway]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

When an allocation is made to a client, we want to update the share and
the count of allocations made to that client. This was previously done
separately, resulting in two std::set lookups, removals, and
reinsertions. By improving `DRFSorter::updateShare` to support
incrementing the number of client-allocations, we can do both updates
with a single sequence of std::set lookup/remove/insert operations.


Diffs
-

  src/master/allocator/sorter/drf/sorter.hpp 
5bed53a1641fee0c09862f77b394a0e9ec131990 
  src/master/allocator/sorter/drf/sorter.cpp 
5681a5d78a7bdde820c3a8633d742d9d6412f1c7 

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


Testing
---

`make check`

Running the sorter benchmarks with an optimized build, this results in a modest 
improvement (~10%).


Thanks,

Neil Conway

Review Request 56351: Renamed a private member function in DRFSorter.

[Neil Conway]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Renamed `update(const string&)` to `updateShare(const string&)`. The
sorter interface already includes two different member functions named
`update` that do different things, so being a bit more explicit seems
helpful.


Diffs
-

  src/master/allocator/sorter/drf/sorter.hpp 
5bed53a1641fee0c09862f77b394a0e9ec131990 
  src/master/allocator/sorter/drf/sorter.cpp 
5681a5d78a7bdde820c3a8633d742d9d6412f1c7 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 56350: Const-ified several methods in the sorter interface.

[Neil Conway]

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

Review request for mesos.


Repository: mesos


Description
---

Const-ified several methods in the sorter interface.


Diffs
-

  src/master/allocator/sorter/drf/sorter.hpp 
5bed53a1641fee0c09862f77b394a0e9ec131990 
  src/master/allocator/sorter/drf/sorter.cpp 
5681a5d78a7bdde820c3a8633d742d9d6412f1c7 
  src/master/allocator/sorter/sorter.hpp 
737b13f66f989ac23c9a2fb6467a51f84339c773 

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


Testing
---

`make check`

Note that this changes the sorter interface. I can add a note to the 
`CHANGELOG` if warranted.


Thanks,

Neil Conway

Review Request 56349: Clarified comments in sorter.

[Neil Conway]

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

Review request for mesos.


Repository: mesos


Description
---

Clarified comments in sorter.


Diffs
-

  src/master/allocator/sorter/drf/sorter.hpp 
5bed53a1641fee0c09862f77b394a0e9ec131990 

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


Testing
---

`make check` (no functional changes).


Thanks,

Neil Conway

Re: Review Request 56308: Modified active containers key to be `TaskID` in the default executor.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Feb. 4, 2017, 5:49 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56308/
> ---
> 
> (Updated Feb. 4, 2017, 5:49 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6296
> https://issues.apache.org/jira/browse/MESOS-6296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We have more occurences of look up by `TaskID` then the other way
> around. Hence, modified the active containers hashmap to be keyed
> on `TaskID` instead.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 97eee05cac8cb1f62d43e2aecc08a8e54e49eac3 
> 
> Diff: https://reviews.apache.org/r/56308/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 56347: Stout: Explicitly delete `SharedHandle` default constructor.

[Andrew Schwartzmeyer]

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

Review request for mesos.


Repository: mesos


Description
---

Stout: Explicitly delete `SharedHandle` default constructor.


Diffs
-

  3rdparty/stout/include/stout/windows.hpp 
3bc973b88ea39948780df2ecc7c3692f7e07e1a9 

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


Testing
---

make && make check


Thanks,

Andrew Schwartzmeyer

Review Request 56339: Changed test DockerArchive to include environment variables.

[Joseph Wu]

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

Review request for mesos, Jie Yu and Kevin Klues.


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


Repository: mesos


Description
---

This subtly modifies all tests using the `docker/runtime` isolator
to fail if environment variables from inside the DockerArchive
are passed into the Mesos executor's environment.  This applies
for all executors (default, command, and docker), but mainly
affects the command executor.

The environment variables are `LD_LIBRARY_PATH`, `LIBPROCESS_IP`,
and `LIBPROCESS_PORT`; all of which are set to `invalid`.  This
either causes linking problems or will force libprocess to exit.


Diffs
-

  src/tests/containerizer/docker_archive.hpp 
b36dbdba7acf0587e18aced3581f68a1269b04d2 

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


Testing
---

This causes some tests to fail, particularly many of the 
`DockerRuntimeIsolatorTest` in `runtime_isolator_tests.cpp`.

A variety of other tests which use `DockerArchive` will also fail.


Thanks,

Joseph Wu

Review Request 56341: Added special case for command executor's environment.

[Joseph Wu]

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

Review request for mesos, Jie Yu and Kevin Klues.


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


Repository: mesos


Description
---

The environment variables given to the command executor will sometimes
modify the behavior of the command executor, rather than the command
task (which is often the intended target of the environment variables).
For example, specifying `LIBPROCESS_IP=invalid` will cause the command 
executor to terminate itself as soon as it starts.  We do not filter 
out environment variables because these types of errors generally fall
under "user error" or "operator error".

The command executor inherits environment variables from:
  * (User) Variables inside a `CommandInfo`;
  * (Operator) Hooks and isolators installed/enabled on the agent;
  * (Operator) The agent flag `--executor_environment_variables`;
  * And Mesos-specific variables provided by the agent.

This commit adds a special case for the command executor.  Isolator
environment variables should be passed into the command executor
separately, as the command executor explicitly depends on environment
variables from the agent.  In some cases, environment variables from
the isolator will cause the command executor to fail; while some 
variables are necessary for the command executor to function.

For example, when running a container image with the unified 
containerizer, if the container image contains the environment variable
`LIBPROCESS_IP=invalid`, the command executor will fail.

For a counter example, when SSL is enabled on the agent, without
downgrade support, you may need to add hooks/isolators to supply the
relevant `LIBPROCESS_SSL_*` environment variables.  Otherwise, the
command executor will not be able to register.


Diffs
-

  src/launcher/executor.cpp 0c770bb18ae8bd8df85589b5262f457ab50574a9 
  src/slave/containerizer/mesos/containerizer.cpp 
d2b4f75a55dbe4746bc2dfc180335fa831a554ef 

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


Testing
---

In progress...

NOTE: As mentioned in the commit description, this patch addresses one problem 
(unified containerizer env vars + command task), while exacerbating another 
(SSL + command task).


Thanks,

Joseph Wu

Review Request 56340: Disabled ability to launch default executor with ContainerInfo.

[Joseph Wu]

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

Review request for mesos, Jie Yu and Kevin Klues.


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


Repository: mesos


Description
---

The default executor used to launch TaskGroups is generated by the
agent.  The agent generates the executor's `CommandInfo`, hence why
the user may not specify the `CommandInfo` in the `LAUNCH_TASK_GROUP`
call.

This commit adds similar restrictions for `ContainerInfo` plus the
default executor.  The `CommandInfo` constructed by the agent expects
to be run in the same environment as the agent process.  This commit
prevents the user from specifying a `DockerInfo` or a container image
along with the default executor.

If the user explicitly wants to use the default executor, they could
always package the default executor's binary and libraries into a
container and launch it like any other custom executor.


Diffs
-

  include/mesos/mesos.proto 53885cbc63ac6658a749da5e05bb2301392f84dd 
  include/mesos/v1/mesos.proto c4ca6dea787cfe4661c9f0d9afb770bceb1c2639 
  src/master/validation.cpp 2beee167439fe39d4f595c6b858fb6175321fbcd 
  src/tests/master_validation_tests.cpp 
1f833aa8d6cacbd7bf502fb66c556e4e3d4f79e2 

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


Testing
---

src/mesos-tests --gtest_filter="ExecutorValidationTest.ExecutorType"


Thanks,

Joseph Wu

Re: Review Request 55160: Added test for DockerContainerizer when `cgroups_enable_cfs` is set.

[Zhitao Li]

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

(Updated Feb. 6, 2017, 9:29 p.m.)


Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.


Changes
---

@haosdent's comments.


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


Repository: mesos


Description
---

Added test for DockerContainerizer when `cgroups_enable_cfs` is set.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
31d63b1f239055d82470ace9024b584a2096dce4 

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


Testing
---

Run the new test.


Thanks,

Zhitao Li

Re: Review Request 55461: Made resource reservation validation multi-role aware.

[Benjamin Mahler]

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




src/master/master.cpp (lines 3942 - 3947)


This isn't quite correct since we need to check the capability to determine 
which field to examine.

Per my suggestion below, can we pass the FrameworkInfo in rather than just 
the roles?

Also note that there is a `getRoles` helper in protobuf utils to simplify 
this. We should add a `set roles` field to the `Framework` struct in 
the master though.



src/master/validation.hpp (lines 223 - 227)


Hm.. any reason this shouldn't follow the pattern used in the create 
validation right below where we take framework info? That seems to make it a 
bit clearer that the optional framework means it's none when done by operator.

E.g.

```
// Validates the RESERVE operation.
Option validate(
const Offer::Operation::Reserve& reserve,
const Option& principal,
const Option& frameworkInfo = None());
```



src/master/validation.cpp (lines 1500 - 1503)


Hm.. do you know which call sites need to be updated? It seems like we need 
to do the sweep now to ensure the call-sites are correct, no?



src/master/validation.cpp (lines 1509 - 1513)


This condition isn't quite the `*` role, this would be a framework with no 
roles. The `*` role case is now the 1 element `*` set, but it seems to me this 
check should be looking at the resource.role() rather than the framework's 
roles, no?


- Benjamin Mahler


On Jan. 16, 2017, 12:27 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55461/
> ---
> 
> (Updated Jan. 16, 2017, 12:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jay Guo, and Guangya Liu.
> 
> 
> Bugs: MESOS-6730
> https://issues.apache.org/jira/browse/MESOS-6730
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This updates the resource reservation validation for frameworks which
> can have multiple roles. During a deprecation period 'FrameworkInfo'
> will have fields for both 'role' and 'roles', however the validation
> function works with just an optional set of roles. Here an empty set
> captures the previous semantics of either having an empty 'role' field
> or 'role' set as '*'. This forces the callers to properly construct a
> set of framework roles from the available information. An optional set
> is used in order to accommodate callers which have no information
> about the framework's roles, and ultimately disables validation taking
> that information into account.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp b863ff6e93931c3d1ee056248084c7f44caf2fd9 
>   src/master/validation.hpp 57e81779ff7444904c2ad7bad33aaf9167b98d05 
>   src/master/validation.cpp 96aa36585ded4bd7cf98526f710ccbc4f23b1f0f 
>   src/tests/master_validation_tests.cpp 
> e5d55e03648cb218d42adc594d6fa7d40ea9bcbb 
> 
> Diff: https://reviews.apache.org/r/55461/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 54821: Refactored Docker::run() to make it only aware of docker cli options.

[Zhitao Li]

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

(Updated Feb. 6, 2017, 9:28 p.m.)


Review request for mesos, Xiaojian Huang, haosdent huang, and Jie Yu.


Changes
---

Style fixes.


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


Repository: mesos


Description
---

This patch creates a wrapper struct for all recognizable docker cli
options, and separate logic of creating these options to a different
common function.

This also enables us to overcome gmock's 10 argument limit.

No logic change happens in this refactoring patch.


Diffs (updated)
-

  src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
  src/docker/docker.cpp b63b060ba1c1d39dc1702368cf667831edbd39bd 
  src/docker/executor.cpp d8c72b081fb9a37b3927b73cea3b47956a076691 
  src/slave/containerizer/docker.cpp 886528f0f706ef2a3c07246406f3ee1ebc45565d 
  src/tests/containerizer/docker_containerizer_tests.cpp 
31d63b1f239055d82470ace9024b584a2096dce4 
  src/tests/containerizer/docker_tests.cpp 
9667d434486c1832f180a297a39a3d5dae6a26bd 
  src/tests/mock_docker.hpp 829a760d54ad8c7b88256ae5df4c88c9fb18df71 
  src/tests/mock_docker.cpp 02b6065a01e7e52b0edb38676dfb1bb475584502 

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


Testing
---

`make check` with ROOT and DOCKER filter.


Thanks,

Zhitao Li

Re: Review Request 56330: Enabled suppress offer per role.

[Benjamin Mahler]

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




include/mesos/allocator/allocator.hpp (lines 352 - 353)


Hm.. it doesn't look like any of the comments in this header make use of 
@param.



src/master/allocator/mesos/hierarchical.cpp (lines 1177 - 1191)


How about something like:

```
const set& roles =
  role.isSome() ? {role.get()} : framework.roles;
  
foreach (const string& role, roles) {
  ...
}
```



src/master/allocator/mesos/hierarchical.cpp (line 1178)


This CHECK can fail if the frameworks sets a Suppress.role to something not 
present in their FrameworkInfo.roles or FrameworkInfo.role fields.

We have two options:

(1) Drop it here with a warning when the role is not one of the framework's 
roles.

(2) Keep the CHECK but have the master validate that Suppress.role is one 
of the frameworks roles. If not, the master drops it as an invalid call.

The right approach here seems to be (2) as it's consistent with how we 
handle other calls.


- Benjamin Mahler


On Feb. 6, 2017, 3:13 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56330/
> ---
> 
> (Updated Feb. 6, 2017, 3:13 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jay Guo.
> 
> 
> Bugs: MESOS-6638
> https://issues.apache.org/jira/browse/MESOS-6638
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled suppress offer per role.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 71a40537b673e44ecdd5327d9a9f083faa7fc13a 
>   src/master/allocator/mesos/allocator.hpp 
> e3c86181677302dbfc3b213715196122f96e312c 
>   src/master/allocator/mesos/hierarchical.hpp 
> 896abcdf0727f986eef3a1a9304a0e4847094057 
>   src/master/allocator/mesos/hierarchical.cpp 
> 3429a6591e5041240736dd033acc23253d9942c8 
>   src/master/master.cpp 98c39b279e7b9830d02efc8ec6a4469afc15d62a 
>   src/tests/allocator.hpp 32c291213d18d1c8fe5d9e8194b92c10716b9961 
>   src/tests/hierarchical_allocator_tests.cpp 
> c681d03c3f94f7d071143366a5aad0421108ebec 
> 
> Diff: https://reviews.apache.org/r/56330/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> Will add a test case to enable suppress per role in follow up patches.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 56328: Augmented master `Suppress` API to accept `Call::Suppress`.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On Feb. 6, 2017, 3:13 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56328/
> ---
> 
> (Updated Feb. 6, 2017, 3:13 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jay Guo.
> 
> 
> Bugs: MESOS-6638
> https://issues.apache.org/jira/browse/MESOS-6638
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Augmented master `Suppress` API to accept `Call::Suppress`.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp d881ad6dba9ba96057988db265faf0b3013c9b05 
>   src/master/master.hpp ceb9bf7e4be7e0e43a166d7706bd0ab4fd9ed578 
>   src/master/master.cpp 98c39b279e7b9830d02efc8ec6a4469afc15d62a 
> 
> Diff: https://reviews.apache.org/r/56328/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 56327: Updated Suppress and Revive proto to support per role.

[Benjamin Mahler]

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


Ship it!





include/mesos/scheduler/scheduler.proto (line 286)


s/offer/offers/
s/spcified/specified/

Also could you mention the case where role is unset and that revives offers 
for all of the roles the framework is subscribed to?



include/mesos/scheduler/scheduler.proto (line 373)


Ditto here.



include/mesos/v1/scheduler/scheduler.proto (line 276)


Ditto here.



include/mesos/v1/scheduler/scheduler.proto (line 363)


Ditto here.


- Benjamin Mahler


On Feb. 6, 2017, 3:13 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56327/
> ---
> 
> (Updated Feb. 6, 2017, 3:13 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jay Guo.
> 
> 
> Bugs: MESOS-6638
> https://issues.apache.org/jira/browse/MESOS-6638
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Suppress and Revive proto to support per role.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler/scheduler.proto 
> 5f4635d523286754a61aa99e18e79d6c1db9463f 
>   include/mesos/v1/scheduler/scheduler.proto 
> 096c76dfffe03c0e2d6abe84d438c396cc1b0be9 
> 
> Diff: https://reviews.apache.org/r/56327/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [56178]

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 Feb. 6, 2017, 7:32 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56178/
> ---
> 
> (Updated Feb. 6, 2017, 7:32 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7022
> https://issues.apache.org/jira/browse/MESOS-7022
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This updates the local authorizer so that MULTI_ROLE frameworks can be
> authorized.
> 
> For non-MULTI_ROLE frameworks we continue to support use of the
> deprecated 'value' field in the authorization request's 'Object';
> however for MULTI_ROLE frameworks the 'value' field will not be set,
> and authorizers still relying on it should be updated to instead use
> the object's 'framework_info' field to extract roles to authorize
> against from.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> b98e1fcdf2ee5ec1f6ac0be6f8accdefaa390a09 
>   src/master/master.cpp 98c39b279e7b9830d02efc8ec6a4469afc15d62a 
> 
> Diff: https://reviews.apache.org/r/56178/diff/
> 
> 
> Testing
> ---
> 
> Tested on various configurations in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 56284: Added ProvisionerDockerLocalStoreTest.MissingLayer test.

[Ilya Pronin]

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

(Updated Feb. 6, 2017, 8:40 p.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Changes
---

Used less fragile approach suggested by Jie in 
[56174](https://reviews.apache.org/r/56174/).


Summary (updated)
-

Added ProvisionerDockerLocalStoreTest.MissingLayer test.


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


Repository: mesos


Description (updated)
---

The new test verifies that that local Docker puller and store will skip layers 
that are already pulled.


Diffs (updated)
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
99e0820fa47303896f70c81cfb91cd34f0474e55 

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


Testing
---

`make check`


Thanks,

Ilya Pronin

Re: Review Request 56338: Removed an unnecessary copy of operations in the allocator.

[Jiang Yan Xu]

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


Ship it!




Ship It!

- Jiang Yan Xu


On Feb. 6, 2017, 11:47 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56338/
> ---
> 
> (Updated Feb. 6, 2017, 11:47 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This was accidentally introduced during the changes to make the
> allocator support MULTI_ROLE frameworks.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 3429a6591e5041240736dd033acc23253d9942c8 
> 
> Diff: https://reviews.apache.org/r/56338/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Review Request 56338: Removed an unnecessary copy of operations in the allocator.

[Benjamin Mahler]

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

Review request for mesos and Jiang Yan Xu.


Repository: mesos


Description
---

This was accidentally introduced during the changes to make the
allocator support MULTI_ROLE frameworks.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
3429a6591e5041240736dd033acc23253d9942c8 

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


Testing
---

make check


Thanks,

Benjamin Mahler

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

[Benjamin Bannier]

> On Feb. 3, 2017, 11:26 a.m., Adam B wrote:
> > src/authorizer/local/authorizer.cpp, line 244
> > 
> >
> > Seems like framework_info is always set, so how/why would we ever fall 
> > through to the other cases?
> 
> Benjamin Bannier wrote:
> Yes, this is currently dead code. I was also wondering whether it should 
> be removed, but decided against it since it provides some level of redundancy 
> as long as `value` still exists and code in the master and in authorizers 
> might not evolve consistently.
> 
> Do you believe it should be removed?
> 
> Alexander Rojas wrote:
> The main reason the `object->value` is still there, is that the local 
> authorizer is a reference implementation for module writers who want to build 
> their own modules, as such, it does provide a reference. I myself will vote 
> to remove the `value` field if possible. However, we makred as deprecated in 
> November 2016 which means we need it there until June (at the same time it 
> had said it is supposed to be removed on 1.2).
> 
> Adam B wrote:
> Alright. Please add a comment explaining why we're keeping dead code 
> around, so I am not tempted to delete it next time I see it.

Yes, a comment is in order here, updated the patch. I also slipped in a `TODO` 
to remove this branch when `value` is purged.


- Benjamin


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


On Feb. 6, 2017, 8:32 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56178/
> ---
> 
> (Updated Feb. 6, 2017, 8:32 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7022
> https://issues.apache.org/jira/browse/MESOS-7022
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This updates the local authorizer so that MULTI_ROLE frameworks can be
> authorized.
> 
> For non-MULTI_ROLE frameworks we continue to support use of the
> deprecated 'value' field in the authorization request's 'Object';
> however for MULTI_ROLE frameworks the 'value' field will not be set,
> and authorizers still relying on it should be updated to instead use
> the object's 'framework_info' field to extract roles to authorize
> against from.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> b98e1fcdf2ee5ec1f6ac0be6f8accdefaa390a09 
>   src/master/master.cpp 98c39b279e7b9830d02efc8ec6a4469afc15d62a 
> 
> Diff: https://reviews.apache.org/r/56178/diff/
> 
> 
> Testing
> ---
> 
> Tested on various configurations in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

[Benjamin Bannier]

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

(Updated Feb. 6, 2017, 8:32 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.


Changes
---

Added a comment as suggested by Adam.


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


Repository: mesos


Description
---

This updates the local authorizer so that MULTI_ROLE frameworks can be
authorized.

For non-MULTI_ROLE frameworks we continue to support use of the
deprecated 'value' field in the authorization request's 'Object';
however for MULTI_ROLE frameworks the 'value' field will not be set,
and authorizers still relying on it should be updated to instead use
the object's 'framework_info' field to extract roles to authorize
against from.


Diffs (updated)
-

  src/authorizer/local/authorizer.cpp b98e1fcdf2ee5ec1f6ac0be6f8accdefaa390a09 
  src/master/master.cpp 98c39b279e7b9830d02efc8ec6a4469afc15d62a 

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


Testing
---

Tested on various configurations in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 54650: Added validation for roles in ACCEPT call.

[Benjamin Mahler]

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


Fix it, then Ship it!




There seems to be a bug, otherwise looks good.

Feel free to split out the offer id printing fix.


src/master/validation.cpp (line 1308)


If you want to pull these two consistency fixes out I could get that 
committed for you.



src/master/validation.cpp (line 1401)


How about validateAllocationRole?



src/master/validation.cpp (line 1415)


This line looks wrong? You could replace it with a 'continue' but it's also 
fine if you just remove it.


- Benjamin Mahler


On Dec. 13, 2016, 4:41 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54650/
> ---
> 
> (Updated Dec. 13, 2016, 4:41 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Guangya Liu.
> 
> 
> Bugs: MESOS-6637
> https://issues.apache.org/jira/browse/MESOS-6637
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For a multi-role framework, it may receive offers for different roles
> in that framework. However, we disallow multiple offers with different
> roles being accepted in single ACCEPT call.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 96aa36585ded4bd7cf98526f710ccbc4f23b1f0f 
> 
> Diff: https://reviews.apache.org/r/54650/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

[Adam B]

> On Feb. 3, 2017, 2:26 a.m., Adam B wrote:
> > src/authorizer/local/authorizer.cpp, line 244
> > 
> >
> > Seems like framework_info is always set, so how/why would we ever fall 
> > through to the other cases?
> 
> Benjamin Bannier wrote:
> Yes, this is currently dead code. I was also wondering whether it should 
> be removed, but decided against it since it provides some level of redundancy 
> as long as `value` still exists and code in the master and in authorizers 
> might not evolve consistently.
> 
> Do you believe it should be removed?
> 
> Alexander Rojas wrote:
> The main reason the `object->value` is still there, is that the local 
> authorizer is a reference implementation for module writers who want to build 
> their own modules, as such, it does provide a reference. I myself will vote 
> to remove the `value` field if possible. However, we makred as deprecated in 
> November 2016 which means we need it there until June (at the same time it 
> had said it is supposed to be removed on 1.2).

Alright. Please add a comment explaining why we're keeping dead code around, so 
I am not tempted to delete it next time I see it.


- Adam


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


On Feb. 6, 2017, 7:44 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56178/
> ---
> 
> (Updated Feb. 6, 2017, 7:44 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7022
> https://issues.apache.org/jira/browse/MESOS-7022
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This updates the local authorizer so that MULTI_ROLE frameworks can be
> authorized.
> 
> For non-MULTI_ROLE frameworks we continue to support use of the
> deprecated 'value' field in the authorization request's 'Object';
> however for MULTI_ROLE frameworks the 'value' field will not be set,
> and authorizers still relying on it should be updated to instead use
> the object's 'framework_info' field to extract roles to authorize
> against from.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> b98e1fcdf2ee5ec1f6ac0be6f8accdefaa390a09 
>   src/master/master.cpp 98c39b279e7b9830d02efc8ec6a4469afc15d62a 
> 
> Diff: https://reviews.apache.org/r/56178/diff/
> 
> 
> Testing
> ---
> 
> Tested on various configurations in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 55162: Stout: Added style fixes and some useful error messages.

[Andrew Schwartzmeyer]

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



These changes won't be necessary with the `WindowsLauncher` patches.

- Andrew Schwartzmeyer


On Jan. 29, 2017, 7:20 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55162/
> ---
> 
> (Updated Jan. 29, 2017, 7:20 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Added style fixes and some useful error messages.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/killtree.hpp 
> 2be2f8c3d58ee64410f87ee4a4b2bb54fe014748 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 5cd92545a49648e39e8eb7cf131895e9cfc97902 
> 
> Diff: https://reviews.apache.org/r/55162/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 55749: Added CMake to standard documentation.

[Andrew Schwartzmeyer]

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


Ship it!




- Andrew Schwartzmeyer


On Jan. 29, 2017, 7:22 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55749/
> ---
> 
> (Updated Jan. 29, 2017, 7:22 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Bugs: MESOS-3139
> https://issues.apache.org/jira/browse/MESOS-3139
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added CMake to standard documentation.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
> 
> Diff: https://reviews.apache.org/r/55749/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 55901: Added support for command health checks to the default executor.

[Vinod Kone]

> On Jan. 28, 2017, 1:17 a.m., Vinod Kone wrote:
> > src/checks/health_checker.hpp, line 137
> > 
> >
> > we don't typically do `const` for POD types.
> > 
> > s/_agentSpawnsCommandContainer/
> 
> Gastón Kleiman wrote:
> Looks lke the regex was truncated ;-).
> 
> Vinod Kone wrote:
> How about "localCommandCheck" (vs "remoteCommandCheck")?
> 
> Alexander Rukletsov wrote:
> We are not consistent about `const` that's true, but I think we are 
> starting to use it more and more often. Unfortunately, I don't have extensive 
> data at hand, here are some links:
> 
> https://github.com/apache/mesos/blob/30f7f3e1965885406d6d3e49348bee7e7d0df9d5/src/slave/slave.hpp#L970
> 
> https://github.com/apache/mesos/blob/30f7f3e1965885406d6d3e49348bee7e7d0df9d5/src/slave/containerizer/mesos/isolators/volume/sandbox_path.hpp#L55

Making a POD member variable const is different from making it a const in 
function argument. We definitely do the former but not the latter IIRC.


- Vinod


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


On Feb. 3, 2017, 7:52 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55901/
> ---
> 
> (Updated Feb. 3, 2017, 7:52 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent 
> huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6280
> https://issues.apache.org/jira/browse/MESOS-6280
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for command health checks to the default executor.
> 
> 
> Diffs
> -
> 
>   src/checks/health_checker.hpp 95da1ff7dd6b222a93076633eb3757ec9aa43cf6 
>   src/checks/health_checker.cpp 58380dc18896f659aa9c4fb4bb567a55bba97f6b 
>   src/launcher/default_executor.cpp 97eee05cac8cb1f62d43e2aecc08a8e54e49eac3 
>   src/tests/health_check_tests.cpp 8418cd91484fd26734de16255b37f3ebf574f5eb 
> 
> Diff: https://reviews.apache.org/r/55901/diff/
> 
> 
> Testing
> ---
> 
> Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It 
> passes on Linux, but not on macOS.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [56178]

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 Feb. 6, 2017, 3:44 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56178/
> ---
> 
> (Updated Feb. 6, 2017, 3:44 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7022
> https://issues.apache.org/jira/browse/MESOS-7022
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This updates the local authorizer so that MULTI_ROLE frameworks can be
> authorized.
> 
> For non-MULTI_ROLE frameworks we continue to support use of the
> deprecated 'value' field in the authorization request's 'Object';
> however for MULTI_ROLE frameworks the 'value' field will not be set,
> and authorizers still relying on it should be updated to instead use
> the object's 'framework_info' field to extract roles to authorize
> against from.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> b98e1fcdf2ee5ec1f6ac0be6f8accdefaa390a09 
>   src/master/master.cpp 98c39b279e7b9830d02efc8ec6a4469afc15d62a 
> 
> Diff: https://reviews.apache.org/r/56178/diff/
> 
> 
> Testing
> ---
> 
> Tested on various configurations in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 55543: Fail the build if %PreferredToolArchitecture% is not set to `x64`.

[Andrew Schwartzmeyer]

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


Ship it!




So many MSVC problems.

- Andrew Schwartzmeyer


On Jan. 29, 2017, 7:21 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55543/
> ---
> 
> (Updated Jan. 29, 2017, 7:21 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Bugs: MESOS-6720
> https://issues.apache.org/jira/browse/MESOS-6720
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Before building Mesos on a Windows machine, it is necessary to set
> `%PreferredToolArchitecture%` to the value `x64`. This is necessary to
> work around (at least) two bugs in the MSVC backend: in particular, the
> linker can sometimes take hours or days to link `mesos-x.x.x.lib`, and
> the build system occasionally finds it self spuriously unable to find
> file `mesos-x.x.x.lib` to link against.
> 
> These issues are well-known and documented (e.g., in the official Mesos
> "getting started" document), but it is better to simply refuse to build
> Mesos at all on Windows unless that environment variable is set.
> 
> This commit will introduce such a check.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 560935b81603dc58c167918d36e2ae0a4060673d 
> 
> Diff: https://reviews.apache.org/r/55543/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 55139: Implemented parse methods for OCI image spec.

[Jie Yu]

> On Feb. 5, 2017, 4:02 a.m., Jie Yu wrote:
> > src/oci/spec.cpp, lines 163-166
> > 
> >
> > what if `digest.isNone()`?
> 
> Qian Zhang wrote:
> `digest` is a required field, so it can not be `None()` here.

I'd prefer to be a bit more defensive here, and self contained. Either add a 
CHECK here, or doing explicit check of `isNone()`. I prefer the latter.


- Jie


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


On Feb. 6, 2017, 2:43 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55139/
> ---
> 
> (Updated Feb. 6, 2017, 2:43 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6681
> https://issues.apache.org/jira/browse/MESOS-6681
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented parse methods for OCI image spec.
> 
> 
> Diffs
> -
> 
>   include/mesos/oci/spec.hpp PRE-CREATION 
>   src/CMakeLists.txt c09bcde3c14a73ab0b296c400f79f6a775da2470 
>   src/Makefile.am 78d55d86b1e1238beda3163d3240c85609ea982d 
>   src/oci/spec.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/55139/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 55901: Added support for command health checks to the default executor.

[Alexander Rukletsov]

> On Jan. 28, 2017, 1:17 a.m., Vinod Kone wrote:
> > src/checks/health_checker.hpp, line 137
> > 
> >
> > we don't typically do `const` for POD types.
> > 
> > s/_agentSpawnsCommandContainer/
> 
> Gastón Kleiman wrote:
> Looks lke the regex was truncated ;-).
> 
> Vinod Kone wrote:
> How about "localCommandCheck" (vs "remoteCommandCheck")?

We are not consistent about `const` that's true, but I think we are starting to 
use it more and more often. Unfortunately, I don't have extensive data at hand, 
here are some links:
https://github.com/apache/mesos/blob/30f7f3e1965885406d6d3e49348bee7e7d0df9d5/src/slave/slave.hpp#L970
https://github.com/apache/mesos/blob/30f7f3e1965885406d6d3e49348bee7e7d0df9d5/src/slave/containerizer/mesos/isolators/volume/sandbox_path.hpp#L55


- Alexander


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


On Feb. 3, 2017, 7:52 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55901/
> ---
> 
> (Updated Feb. 3, 2017, 7:52 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent 
> huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6280
> https://issues.apache.org/jira/browse/MESOS-6280
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for command health checks to the default executor.
> 
> 
> Diffs
> -
> 
>   src/checks/health_checker.hpp 95da1ff7dd6b222a93076633eb3757ec9aa43cf6 
>   src/checks/health_checker.cpp 58380dc18896f659aa9c4fb4bb567a55bba97f6b 
>   src/launcher/default_executor.cpp 97eee05cac8cb1f62d43e2aecc08a8e54e49eac3 
>   src/tests/health_check_tests.cpp 8418cd91484fd26734de16255b37f3ebf574f5eb 
> 
> Diff: https://reviews.apache.org/r/55901/diff/
> 
> 
> Testing
> ---
> 
> Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It 
> passes on Linux, but not on macOS.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 56236: Add `syntax = "proto2"` to some files automatically.

[haosdent huang]

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



When we going to commit this, note that we need to send to 
d...@mesos.apache.org about this. So other develpers would not foget to add 
`syntax 2` in protobuf files..

- haosdent huang


On Feb. 2, 2017, 8:12 p.m., Anthony Sottile wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56236/
> ---
> 
> (Updated Feb. 2, 2017, 8:12 p.m.)
> 
> 
> Review request for mesos, Deshi Xiao, Yong Tang, and Zhitao Li.
> 
> 
> Bugs: MESOS-5186 and MESOS-6138
> https://issues.apache.org/jira/browse/MESOS-5186
> https://issues.apache.org/jira/browse/MESOS-6138
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> I used this pipeline:
> 
> # Find all proto files with the 15-line license and add the syntax line
> # after that
> git grep -n 'limitations under the License' -- '*.proto' |
> grep ':15' | cut -d':' -f1 |
> xargs sed -i '16isyntax = "proto2";'
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 775c14b70c94c90d31d0123f45188fd29651f9f4 
>   include/mesos/allocator/allocator.proto 
> 9fc8baccb135c5aa58f64847307978139139a46e 
>   include/mesos/appc/spec.proto 2dbb3504596dda229c42353098879350c3a7c960 
>   include/mesos/authentication/authentication.proto 
> a24d53512a8ec27e97cf543cd64129c8a096ac67 
>   include/mesos/authorizer/acls.proto 
> fd25e5449ad659ee0269cfc3f47b9939ac022fb4 
>   include/mesos/authorizer/authorizer.proto 
> 8b860a3e8e0b1c660a8fefc97f10f5acc0501920 
>   include/mesos/docker/spec.proto 734395cbde81bc4ff99f1310fc4b913beeb5b11f 
>   include/mesos/docker/v1.proto 209fb4afe5e4ea1cc776bfe9cb36865c9b05aaf0 
>   include/mesos/docker/v2.proto 9b5bfb5d03ae8115c1b20a8c640640d6dbaa257d 
>   include/mesos/executor/executor.proto 
> e746608176245bcabf265925111b8df9cab8ca55 
>   include/mesos/fetcher/fetcher.proto 
> 7f204e1a0b70ecdcd4247e1c0699585243a97b40 
>   include/mesos/maintenance/maintenance.proto 
> 4afae5c6958a5da91b3e649b496b9757d951ca0c 
>   include/mesos/master/master.proto a2228db8902c297f137b8106a7b3d2babbc35017 
>   include/mesos/mesos.proto 53885cbc63ac6658a749da5e05bb2301392f84dd 
>   include/mesos/module/hook.proto f7b91f813f7919098763790211d3b90aed0b8340 
>   include/mesos/module/module.proto 1c6c4b0762f5a4e71cf64fd9bc859e380222b591 
>   include/mesos/quota/quota.proto 78e12d56c4f41613ec6b731d86093f48a23b4509 
>   include/mesos/scheduler/scheduler.proto 
> 5f4635d523286754a61aa99e18e79d6c1db9463f 
>   include/mesos/slave/containerizer.proto 
> c70d437ac3f8f813fcb71c04275cc8eeeb946065 
>   include/mesos/slave/oversubscription.proto 
> e7346089d5699194b761c81ab9610ad33d83ba55 
>   include/mesos/state/state.proto 7a7d68e6abc85c0ead04f5771e878d10348f44b8 
>   include/mesos/uri/uri.proto 55023b681665e712990d45ff4a1dbc4ccf64edbf 
>   include/mesos/v1/agent/agent.proto a98acb7b0c66f7e17dcecf61ed22bfde70a33651 
>   include/mesos/v1/allocator/allocator.proto 
> 093f18f554470b14905db157bcc1e33303423eaa 
>   include/mesos/v1/executor/executor.proto 
> 754e62aee5f822526eb9661aa255444c3f84dd1d 
>   include/mesos/v1/maintenance/maintenance.proto 
> 27f7c2b3460fe3a7e1a845cd962cb9979a9ccc0b 
>   include/mesos/v1/master/master.proto 
> cfdca7400d98233c6320eebd8784e4f51c43ebbd 
>   include/mesos/v1/mesos.proto c4ca6dea787cfe4661c9f0d9afb770bceb1c2639 
>   include/mesos/v1/quota/quota.proto 424e6b4bf0064d146cfeeb19de77f0cce67882b9 
>   include/mesos/v1/scheduler/scheduler.proto 
> 096c76dfffe03c0e2d6abe84d438c396cc1b0be9 
>   src/master/registry.proto eab9821b6d159d14bf62b347d6188608ae5c 
>   src/messages/flags.proto 08406b69acaa52eba58e6091b9c8b3bc03981044 
>   src/messages/log.proto 12c2d83d88bb5b282bc44d49655a67e671b12ba2 
>   src/messages/messages.proto 7a2f37b78a8edcd372558f77f15e6b249742e321 
>   src/messages/state.proto 857a88873681c9cd7263d0063e8f17f08c5cd64b 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.proto 
> 4c5b03c1a802c177f64ea8bb1e31fa58603991bb 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.proto 
> 4b3850037ec01969cabf16b94745c1802bf4de62 
>   src/slave/containerizer/mesos/provisioner/docker/message.proto 
> c93c7a92ec152bd9747a70392adfe6a0e863e839 
> 
> Diff: https://reviews.apache.org/r/56236/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Sottile
> 
>

Re: Review Request 56330: Enabled suppress offer per role.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [56327, 56328, 56330]

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 Feb. 6, 2017, 3:13 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56330/
> ---
> 
> (Updated Feb. 6, 2017, 3:13 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jay Guo.
> 
> 
> Bugs: MESOS-6638
> https://issues.apache.org/jira/browse/MESOS-6638
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled suppress offer per role.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 71a40537b673e44ecdd5327d9a9f083faa7fc13a 
>   src/master/allocator/mesos/allocator.hpp 
> e3c86181677302dbfc3b213715196122f96e312c 
>   src/master/allocator/mesos/hierarchical.hpp 
> 896abcdf0727f986eef3a1a9304a0e4847094057 
>   src/master/allocator/mesos/hierarchical.cpp 
> 3429a6591e5041240736dd033acc23253d9942c8 
>   src/master/master.cpp 98c39b279e7b9830d02efc8ec6a4469afc15d62a 
>   src/tests/allocator.hpp 32c291213d18d1c8fe5d9e8194b92c10716b9961 
>   src/tests/hierarchical_allocator_tests.cpp 
> c681d03c3f94f7d071143366a5aad0421108ebec 
> 
> Diff: https://reviews.apache.org/r/56330/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> Will add a test case to enable suppress per role in follow up patches.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

[Benjamin Bannier]

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

(Updated Feb. 6, 2017, 4:44 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.


Changes
---

Added a comment on deprecation.


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


Repository: mesos


Description
---

This updates the local authorizer so that MULTI_ROLE frameworks can be
authorized.

For non-MULTI_ROLE frameworks we continue to support use of the
deprecated 'value' field in the authorization request's 'Object';
however for MULTI_ROLE frameworks the 'value' field will not be set,
and authorizers still relying on it should be updated to instead use
the object's 'framework_info' field to extract roles to authorize
against from.


Diffs (updated)
-

  src/authorizer/local/authorizer.cpp b98e1fcdf2ee5ec1f6ac0be6f8accdefaa390a09 
  src/master/master.cpp 98c39b279e7b9830d02efc8ec6a4469afc15d62a 

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


Testing
---

Tested on various configurations in internal CI.


Thanks,

Benjamin Bannier

Review Request 56330: Enabled suppress offer per role.

[Guangya Liu]

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

Review request for mesos, Benjamin Mahler and Jay Guo.


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


Repository: mesos


Description
---

Enabled suppress offer per role.


Diffs
-

  include/mesos/allocator/allocator.hpp 
71a40537b673e44ecdd5327d9a9f083faa7fc13a 
  src/master/allocator/mesos/allocator.hpp 
e3c86181677302dbfc3b213715196122f96e312c 
  src/master/allocator/mesos/hierarchical.hpp 
896abcdf0727f986eef3a1a9304a0e4847094057 
  src/master/allocator/mesos/hierarchical.cpp 
3429a6591e5041240736dd033acc23253d9942c8 
  src/master/master.cpp 98c39b279e7b9830d02efc8ec6a4469afc15d62a 
  src/tests/allocator.hpp 32c291213d18d1c8fe5d9e8194b92c10716b9961 
  src/tests/hierarchical_allocator_tests.cpp 
c681d03c3f94f7d071143366a5aad0421108ebec 

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


Testing
---

make
make check

Will add a test case to enable suppress per role in follow up patches.


Thanks,

Guangya Liu

Review Request 56328: Augmented master `Suppress` API to accept `Call::Suppress`.

[Guangya Liu]

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

Review request for mesos, Benjamin Mahler and Jay Guo.


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


Repository: mesos


Description
---

Augmented master `Suppress` API to accept `Call::Suppress`.


Diffs
-

  src/master/http.cpp d881ad6dba9ba96057988db265faf0b3013c9b05 
  src/master/master.hpp ceb9bf7e4be7e0e43a166d7706bd0ab4fd9ed578 
  src/master/master.cpp 98c39b279e7b9830d02efc8ec6a4469afc15d62a 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Review Request 56327: Updated Suppress and Revive proto to support per role.

[Guangya Liu]

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

Review request for mesos, Benjamin Mahler and Jay Guo.


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


Repository: mesos


Description
---

Updated Suppress and Revive proto to support per role.


Diffs
-

  include/mesos/scheduler/scheduler.proto 
5f4635d523286754a61aa99e18e79d6c1db9463f 
  include/mesos/v1/scheduler/scheduler.proto 
096c76dfffe03c0e2d6abe84d438c396cc1b0be9 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 55887: Check task user before allowing a task to be launched on the agent.

[Alexander Rojas]

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




src/slave/slave.cpp (line 1659)


`authorizeTask()` returns a `Future`, which means you cannot be sure 
is going to be ready on return. That doesn't mean you could block here.  I 
recomend to check 
[here](https://github.com/apache/mesos/blob/30f7f3e1965885406d6d3e49348bee7e7d0df9d5/src/master/master.cpp#L3345-L3380)
 to follow the pattern we use in these cases. Something like:

```c++
list authorizations;
foreach (const TaskInfo& _task, tasks) {
  authorizations.insert(authorizeTask(_task, framework));
}

// Await calls the `.then` clause when all the futures in
// the list reach a final state.
await(authorizations)
  .then([](const list& authorizations) {
// Add here the actual body of the foreach
  });
```


- Alexander Rojas


On Jan. 25, 2017, 1:31 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55887/
> ---
> 
> (Updated Jan. 25, 2017, 1:31 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6953
> https://issues.apache.org/jira/browse/MESOS-6953
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for action `run_tasks` on the agent's flag `acl`. Based on
> the ACL configured for `run_tasks`, a task to be launched on the agent
> will be (dis)allowed to launch on the agent.
> If a task or task group cannot be launched due to failed authorization,
> a `TASK_FAILED` Status Update shall be sent with a reason code of
> `REASON_TASK_UNAUTHORIZED` or `REASON_TASK_GROUP_UNAUTHORIZED` as
> applicable.
> Note that in case of a task group, all tasks fail if any of the tasks
> within the task group encounter the authorization error.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 0dadbe50be15f89b791da55fa10f1b434693ee0f 
>   src/slave/slave.cpp 0548b04073c0ba4adfc4433d75fd18c2ba79d891 
> 
> Diff: https://reviews.apache.org/r/55887/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 55139: Implemented parse methods for OCI image spec.

[Qian Zhang]

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

(Updated Feb. 6, 2017, 10:43 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Addressed Jie's comments.


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


Repository: mesos


Description
---

Implemented parse methods for OCI image spec.


Diffs (updated)
-

  include/mesos/oci/spec.hpp PRE-CREATION 
  src/CMakeLists.txt c09bcde3c14a73ab0b296c400f79f6a775da2470 
  src/Makefile.am 78d55d86b1e1238beda3163d3240c85609ea982d 
  src/oci/spec.cpp PRE-CREATION 

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 55139: Implemented parse methods for OCI image spec.

[Qian Zhang]

> On Feb. 5, 2017, 12:02 p.m., Jie Yu wrote:
> > src/oci/spec.cpp, lines 163-166
> > 
> >
> > what if `digest.isNone()`?

`digest` is a required field, so it can not be `None()` here.


> On Feb. 5, 2017, 12:02 p.m., Jie Yu wrote:
> > src/oci/spec.cpp, lines 182-185
> > 
> >
> > What if `platform.isNone()`?

Ditto.


- Qian


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


On Feb. 1, 2017, 9:13 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55139/
> ---
> 
> (Updated Feb. 1, 2017, 9:13 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6681
> https://issues.apache.org/jira/browse/MESOS-6681
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented parse methods for OCI image spec.
> 
> 
> Diffs
> -
> 
>   include/mesos/oci/spec.hpp PRE-CREATION 
>   src/CMakeLists.txt 09ef1aee680c6b91476092dbf61d4ca3a8d256bb 
>   src/Makefile.am 6c9be54fe0ce3faaa26e2f090773d1b77bb6e430 
>   src/oci/spec.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/55139/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 56326: Size of the favicon optimized.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [56326]

Failed command: python support/apply-reviews.py -n -r 56326

Error:
2017-02-06 13:30:40 URL:https://reviews.apache.org/r/56326/diff/raw/ [299/299] 
-> "56326.patch" [1]
error: missing binary patch data for 'src/webui/master/static/ico/favicon.ico'
error: binary patch does not apply to 'src/webui/master/static/ico/favicon.ico'
error: src/webui/master/static/ico/favicon.ico: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/17002/console

- Mesos Reviewbot


On Feb. 6, 2017, 1 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56326/
> ---
> 
> (Updated Feb. 6, 2017, 1 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The favicon keeps the same images as before but its size has been reduced (in 
> a lossless manner) from 361 KB to 115 KB.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/ico/favicon.ico 
> 11dd3de5f124550be141bce9a9a96a516a9c9b7b 
> 
> Diff: https://reviews.apache.org/r/56326/diff/
> 
> 
> Testing
> ---
> 
> None. I took care of converting 'favicon.ico' to '.png' images using 
> imagemagick, optimized their size using ImageOptim, and rebundled them as 
> 'favicon.ico' using imagemagick again.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 53741: Show maintainance schedule info in the WebUI.

[Tomasz Janiszewski]

> On Feb. 6, 2017, 9:30 a.m., haosdent huang wrote:
> > I would fix these issues for you. Please take a look. Thanks a lot for your 
> > contribution!

@haosdent Thanks for taking care of it.


- Tomasz


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


On Feb. 6, 2017, 9:29 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53741/
> ---
> 
> (Updated Feb. 6, 2017, 9:29 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Joseph Wu.
> 
> 
> Bugs: MESOS-6443
> https://issues.apache.org/jira/browse/MESOS-6443
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Create new page with Maintenance schedule. Schedule is downloaded on
> page refresh. Schedule is not live like stats and tasks so there is
> no need to poll it periodically.
> Diable sorting when data-key is not defined in table header.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/index.html 7c6a8ad56437c2315fc563d469ed0426286b48ce 
>   src/webui/master/static/js/app.js 558a67ddf59a39db43023b4f7aa26b7e319924b0 
>   src/webui/master/static/js/controllers.js 
> 07bc612a4d7a6b4b418de964303e8fb7083b5d31 
>   src/webui/master/static/maintenance.html PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53741/diff/
> 
> 
> Testing
> ---
> 
> [Screenshot](https://issues.apache.org/jira/secure/attachment/12838845/mesos_webui_maintenance_schedule.png)
> 
> Testing done maually on Ubuntu/Chrome. Perfomrance testing done with 500 
> entires schedule generated with 
> [generate_schedule.py](https://gist.github.com/janisz/e4dcb001f19aa4b466f9112a6dd16853)
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Review Request 56326: Size of the favicon optimized.

[Armand Grillet]

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

Review request for mesos and Till Toenshoff.


Repository: mesos


Description
---

The favicon keeps the same images as before but its size has been reduced (in a 
lossless manner) from 361 KB to 115 KB.


Diffs
-

  src/webui/master/static/ico/favicon.ico 
11dd3de5f124550be141bce9a9a96a516a9c9b7b 

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


Testing
---

None. I took care of converting 'favicon.ico' to '.png' images using 
imagemagick, optimized their size using ImageOptim, and rebundled them as 
'favicon.ico' using imagemagick again.


Thanks,

Armand Grillet

Re: Review Request 55901: Added support for command health checks to the default executor.

[Alexander Rukletsov]

> On Feb. 3, 2017, 1:34 p.m., Alexander Rukletsov wrote:
> > src/tests/health_check_tests.cpp, lines 2154-2155
> > 
> >
> > Formatting.
> > 
> > Also it is probably a good idea to explicitly say you don't care about 
> > further updates.
> 
> Gastón Kleiman wrote:
> Fixed the formatting. We don't have such a comment in most of the other 
> tests in this file.

Oh, under say I mean `.WillRepeatedly(Return());`


- Alexander


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


On Feb. 3, 2017, 7:52 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55901/
> ---
> 
> (Updated Feb. 3, 2017, 7:52 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent 
> huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6280
> https://issues.apache.org/jira/browse/MESOS-6280
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for command health checks to the default executor.
> 
> 
> Diffs
> -
> 
>   src/checks/health_checker.hpp 95da1ff7dd6b222a93076633eb3757ec9aa43cf6 
>   src/checks/health_checker.cpp 58380dc18896f659aa9c4fb4bb567a55bba97f6b 
>   src/launcher/default_executor.cpp 97eee05cac8cb1f62d43e2aecc08a8e54e49eac3 
>   src/tests/health_check_tests.cpp 8418cd91484fd26734de16255b37f3ebf574f5eb 
> 
> Diff: https://reviews.apache.org/r/55901/diff/
> 
> 
> Testing
> ---
> 
> Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It 
> passes on Linux, but not on macOS.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55901: Added support for command health checks to the default executor.

[Alexander Rukletsov]

> On Feb. 3, 2017, 1:34 p.m., Alexander Rukletsov wrote:
> > src/tests/health_check_tests.cpp, lines 2118-2119
> > 
> >
> > Why do we need to expilictly create `containerizer`?
> 
> Gastón Kleiman wrote:
> Because the implicit `containerizer` is started in local mode. 
> `LaunchNestedContainerSession` (used by cmd health checks) tries to start a 
> IO switchboard, which doesn't work in local mode yet.

Please capture this in a comment.


- Alexander


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


On Feb. 3, 2017, 7:52 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55901/
> ---
> 
> (Updated Feb. 3, 2017, 7:52 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent 
> huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6280
> https://issues.apache.org/jira/browse/MESOS-6280
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for command health checks to the default executor.
> 
> 
> Diffs
> -
> 
>   src/checks/health_checker.hpp 95da1ff7dd6b222a93076633eb3757ec9aa43cf6 
>   src/checks/health_checker.cpp 58380dc18896f659aa9c4fb4bb567a55bba97f6b 
>   src/launcher/default_executor.cpp 97eee05cac8cb1f62d43e2aecc08a8e54e49eac3 
>   src/tests/health_check_tests.cpp 8418cd91484fd26734de16255b37f3ebf574f5eb 
> 
> Diff: https://reviews.apache.org/r/55901/diff/
> 
> 
> Testing
> ---
> 
> Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It 
> passes on Linux, but not on macOS.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

[Alexander Rojas]

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




src/master/master.cpp (line 2177)


Probably add a _TODO_ requesting to remove these lines after end of 
deprecation period.


- Alexander Rojas


On Feb. 3, 2017, 1:10 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56178/
> ---
> 
> (Updated Feb. 3, 2017, 1:10 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7022
> https://issues.apache.org/jira/browse/MESOS-7022
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This updates the local authorizer so that MULTI_ROLE frameworks can be
> authorized.
> 
> For non-MULTI_ROLE frameworks we continue to support use of the
> deprecated 'value' field in the authorization request's 'Object';
> however for MULTI_ROLE frameworks the 'value' field will not be set,
> and authorizers still relying on it should be updated to instead use
> the object's 'framework_info' field to extract roles to authorize
> against from.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> b98e1fcdf2ee5ec1f6ac0be6f8accdefaa390a09 
>   src/master/master.cpp 284566ca72bd5c6bd581db9b65d404f86aa7bf61 
> 
> Diff: https://reviews.apache.org/r/56178/diff/
> 
> 
> Testing
> ---
> 
> Tested on various configurations in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

[Alexander Rojas]

> On Feb. 3, 2017, 11:26 a.m., Adam B wrote:
> > src/authorizer/local/authorizer.cpp, line 244
> > 
> >
> > Seems like framework_info is always set, so how/why would we ever fall 
> > through to the other cases?
> 
> Benjamin Bannier wrote:
> Yes, this is currently dead code. I was also wondering whether it should 
> be removed, but decided against it since it provides some level of redundancy 
> as long as `value` still exists and code in the master and in authorizers 
> might not evolve consistently.
> 
> Do you believe it should be removed?

The main reason the `object->value` is still there, is that the local 
authorizer is a reference implementation for module writers who want to build 
their own modules, as such, it does provide a reference. I myself will vote to 
remove the `value` field if possible. However, we makred as deprecated in 
November 2016 which means we need it there until June (at the same time it had 
said it is supposed to be removed on 1.2).


- Alexander


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


On Feb. 3, 2017, 1:10 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56178/
> ---
> 
> (Updated Feb. 3, 2017, 1:10 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7022
> https://issues.apache.org/jira/browse/MESOS-7022
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This updates the local authorizer so that MULTI_ROLE frameworks can be
> authorized.
> 
> For non-MULTI_ROLE frameworks we continue to support use of the
> deprecated 'value' field in the authorization request's 'Object';
> however for MULTI_ROLE frameworks the 'value' field will not be set,
> and authorizers still relying on it should be updated to instead use
> the object's 'framework_info' field to extract roles to authorize
> against from.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> b98e1fcdf2ee5ec1f6ac0be6f8accdefaa390a09 
>   src/master/master.cpp 284566ca72bd5c6bd581db9b65d404f86aa7bf61 
> 
> Diff: https://reviews.apache.org/r/56178/diff/
> 
> 
> Testing
> ---
> 
> Tested on various configurations in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53741: Show maintainance schedule info in the WebUI.

[haosdent huang]

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


Fix it, then Ship it!




I would fix these issues for you. Please take a look. Thanks a lot for your 
contribution!


src/webui/master/static/index.html (line 48)


Nit: remove the extra space.
```
  
Maintenance
  
```



src/webui/master/static/js/app.js (line 23)


Moved this after `framework` to keep alphabectically.

```
.when('/frameworks/:id',
  {templateUrl: 'static/framework.html', controller: 
'FrameworkCtrl'})
.when('/maintenance',
  {templateUrl: 'static/maintenance.html', controller: 
'MaintenanceCtrl'})

```



src/webui/master/static/js/controllers.js (line 477)


```
$http.jsonp('/master/maintenance/schedule?jsonp=JSON_CALLBACK')
```



src/webui/master/static/maintenance.html (lines 21 - 27)


Need to use the custom directives by this way.
```
  


  
  


  
```



src/webui/master/static/maintenance.html (line 30)


Nit: Indent should be 2.

``
{{machine_ids.hostname ? machine_ids.hostname : machine_ids.ip}}
```


- haosdent huang


On Feb. 6, 2017, 9:29 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53741/
> ---
> 
> (Updated Feb. 6, 2017, 9:29 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Joseph Wu.
> 
> 
> Bugs: MESOS-6443
> https://issues.apache.org/jira/browse/MESOS-6443
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Create new page with Maintenance schedule. Schedule is downloaded on
> page refresh. Schedule is not live like stats and tasks so there is
> no need to poll it periodically.
> Diable sorting when data-key is not defined in table header.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/index.html 7c6a8ad56437c2315fc563d469ed0426286b48ce 
>   src/webui/master/static/js/app.js 558a67ddf59a39db43023b4f7aa26b7e319924b0 
>   src/webui/master/static/js/controllers.js 
> 07bc612a4d7a6b4b418de964303e8fb7083b5d31 
>   src/webui/master/static/maintenance.html PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53741/diff/
> 
> 
> Testing
> ---
> 
> [Screenshot](https://issues.apache.org/jira/secure/attachment/12838845/mesos_webui_maintenance_schedule.png)
> 
> Testing done maually on Ubuntu/Chrome. Perfomrance testing done with 500 
> entires schedule generated with 
> [generate_schedule.py](https://gist.github.com/janisz/e4dcb001f19aa4b466f9112a6dd16853)
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 53741: Show maintainance schedule info in the WebUI.

[Tomasz Janiszewski]

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

(Updated Feb. 6, 2017, 9:29 a.m.)


Review request for mesos, haosdent huang and Joseph Wu.


Summary (updated)
-

Show maintainance schedule info in the WebUI.


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


Repository: mesos


Description
---

Create new page with Maintenance schedule. Schedule is downloaded on
page refresh. Schedule is not live like stats and tasks so there is
no need to poll it periodically.
Diable sorting when data-key is not defined in table header.


Diffs
-

  src/webui/master/static/index.html 7c6a8ad56437c2315fc563d469ed0426286b48ce 
  src/webui/master/static/js/app.js 558a67ddf59a39db43023b4f7aa26b7e319924b0 
  src/webui/master/static/js/controllers.js 
07bc612a4d7a6b4b418de964303e8fb7083b5d31 
  src/webui/master/static/maintenance.html PRE-CREATION 

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


Testing
---

[Screenshot](https://issues.apache.org/jira/secure/attachment/12838845/mesos_webui_maintenance_schedule.png)

Testing done maually on Ubuntu/Chrome. Perfomrance testing done with 500 
entires schedule generated with 
[generate_schedule.py](https://gist.github.com/janisz/e4dcb001f19aa4b466f9112a6dd16853)


Thanks,

Tomasz Janiszewski

Re: Review Request 56214: Hashed unacknowledged updates by UUID string in default executor.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On Feb. 2, 2017, 9:57 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56214/
> ---
> 
> (Updated Feb. 2, 2017, 9:57 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-6906
> https://issues.apache.org/jira/browse/MESOS-6906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 97eee05cac8cb1f62d43e2aecc08a8e54e49eac3 
> 
> Diff: https://reviews.apache.org/r/56214/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/56218/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 56209: Hashed unacknowledged updates by UUID string in command executor.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On Feb. 2, 2017, 9:56 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56209/
> ---
> 
> (Updated Feb. 2, 2017, 9:56 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-6906
> https://issues.apache.org/jira/browse/MESOS-6906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 0c770bb18ae8bd8df85589b5262f457ab50574a9 
> 
> Diff: https://reviews.apache.org/r/56209/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/56218/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>