Re: Review Request 58015: Add name to contributors.yaml file for first contribution.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58011, 58012, 58013, 58014, 58015]

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 March 29, 2017, 1 a.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58015/
> ---
> 
> (Updated March 29, 2017, 1 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-7226
> https://issues.apache.org/jira/browse/MESOS-7226
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add name to contributors.yaml file for first contribution.
> 
> 
> Diffs
> -
> 
>   docs/contributors.yaml 725f50c3d781b187dc61039e84fdd4ada6d874fa 
> 
> 
> Diff: https://reviews.apache.org/r/58015/diff/1/
> 
> 
> Testing
> ---
> 
> ## CentOS 7
> 
> Full build with all changes, made sure build was successful with no errors.
> 
> ## Windows
> 
> Full build with all changes, made sure build was successful with no errors.
> 
> 
> ## Timing improvements with this change
> 
>  Build times on Windows (no PCH):
>   1. Full build time (everything including 3rd party products): 24:49.47 
> (1489.47s total)
>   2. Time to rebuild all of Mesos itself: 21:8.63 (1268.63s total)
>   3. Time for a build with no changes: 0:30.47 (30.47s total)
>   4. Modified file 3rdparty/stout/include/stout/os/os.hpp (added a comment)
>   5. Time for an incremental build: 36:55.55 (2215.55s total)
>  (Very odd that an incremental takes longer than a full build!)
>   6. Time for an incremental build (just mesos-agent): 0:14:24.73 (864.73s 
> total)
>   7. Time for an incremental build, mesos-agent only: 0:16:42.57 (1002.57s 
> total)
>  (Includes speedup to linker flags)
> 
> 
>  Build times on Windows (with PCH):
>   1. Full build time (everything including 3rd party products): 19:54.49 
> (1194.49s total)
>   2. Time to rebuild all of Mesos itself: 0:15:49.58 (949.58s total)
>   3. Time for a build with no changes: 0:0:21.72 (21.72s total)
>   4. Modified file 3rdparty/stout/include/stout/os/os.hpp (added a comment)
>   5. Time for an incremental build: 0:34:40.64 (2080.64s total)
>   6. Time for an incremental build (just mesos-agent): 0:11:53.57 (713.57s 
> total)
>   7. Time for an incremental build, mesos-agent only: 0:10:43.10 (643.10s 
> total)
>  (Includes speedup to linker flags)
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>

Review Request 58015: Add name to contributors.yaml file for first contribution.

[Jeff Coffler]

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

Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li 
Li.


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


Repository: mesos


Description
---

Add name to contributors.yaml file for first contribution.


Diffs
-

  docs/contributors.yaml 725f50c3d781b187dc61039e84fdd4ada6d874fa 


Diff: https://reviews.apache.org/r/58015/diff/1/


Testing
---

## CentOS 7

Full build with all changes, made sure build was successful with no errors.

## Windows

Full build with all changes, made sure build was successful with no errors.


## Timing improvements with this change

 Build times on Windows (no PCH):
  1. Full build time (everything including 3rd party products): 24:49.47 
(1489.47s total)
  2. Time to rebuild all of Mesos itself: 21:8.63 (1268.63s total)
  3. Time for a build with no changes: 0:30.47 (30.47s total)
  4. Modified file 3rdparty/stout/include/stout/os/os.hpp (added a comment)
  5. Time for an incremental build: 36:55.55 (2215.55s total)
 (Very odd that an incremental takes longer than a full build!)
  6. Time for an incremental build (just mesos-agent): 0:14:24.73 (864.73s 
total)
  7. Time for an incremental build, mesos-agent only: 0:16:42.57 (1002.57s 
total)
 (Includes speedup to linker flags)


 Build times on Windows (with PCH):
  1. Full build time (everything including 3rd party products): 19:54.49 
(1194.49s total)
  2. Time to rebuild all of Mesos itself: 0:15:49.58 (949.58s total)
  3. Time for a build with no changes: 0:0:21.72 (21.72s total)
  4. Modified file 3rdparty/stout/include/stout/os/os.hpp (added a comment)
  5. Time for an incremental build: 0:34:40.64 (2080.64s total)
  6. Time for an incremental build (just mesos-agent): 0:11:53.57 (713.57s 
total)
  7. Time for an incremental build, mesos-agent only: 0:10:43.10 (643.10s total)
 (Includes speedup to linker flags)


Thanks,

Jeff Coffler

Review Request 58014: CMake: Setup cotire for 'mesos-agent' target.

[Jeff Coffler]

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

Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li 
Li.


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


Repository: mesos


Description
---

This sets several cotire variables in the 'src' directory to enable
pre-compiled headers for the 'mesos-agent' target. The excluded
headers were removed due to namespace issues or breaking incremental
builds.

All cotire variables are conditionally set if option
ENABLE_PRECOMPILED_HEADERS is set. In this commit, this is ON for
WIN32 builds, off for all others.


Diffs
-

  cmake/CompilationConfigure.cmake 5936be07557395ad08b06aa376b5a29f9b11c143 
  src/CMakeLists.txt b67b512cafd90558abf712a872310ab913fa38ae 


Diff: https://reviews.apache.org/r/58014/diff/1/


Testing
---

Testing done at end of chain


Thanks,

Jeff Coffler

Review Request 58013: WIN32: Add compile/link flags to improve incremental link times.

[Jeff Coffler]

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

Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li 
Li.


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


Repository: mesos


Description
---

When working on precompiled headers, we recognized that adding
some compile/link flags resulted in a 20% reduction in incremental
linkage time. This is NOT directly dependent on precompiled headers
(the improvement comes regardless of PCH).


Diffs
-

  cmake/CompilationConfigure.cmake 5936be07557395ad08b06aa376b5a29f9b11c143 


Diff: https://reviews.apache.org/r/58013/diff/1/


Testing
---

Testing done at end of chain


Thanks,

Jeff Coffler

Review Request 58012: Fix code issues to facilitate use of precompiled headers on Windows.

[Jeff Coffler]

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

Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li 
Li.


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


Repository: mesos


Description
---

Precompiled headers often include new include files that were not
otherwise included in a module. If two include files have the same
class name (with different scopoing), this causes a conflict.

Fully qualify class names and fix other code issues to allow for
precompiled headers on Windows.


Diffs
-

  src/internal/evolve.cpp 5ef23067c3f03c7972e24968d4a84b3cf61726a0 
  src/slave/containerizer/docker.cpp dfab26224e8679b493d770657f83c711a1d442ef 
  src/slave/windows_ctrlhandler.hpp 25b14a446175cd302d1840b1f542989ef8315018 


Diff: https://reviews.apache.org/r/58012/diff/1/


Testing
---

Testing done at end of chain


Thanks,

Jeff Coffler

Review Request 58011: CMake: Add Cotire module (version 1.7.9).

[Jeff Coffler]

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

Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li 
Li.


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


Repository: mesos


Description
---

Add CMake's cotire module to allow for use of PCH (precompiled
headers) during compilation, reducing compilation speed.


Diffs
-

  3rdparty/cmake/cotire.cmake PRE-CREATION 
  CMakeLists.txt 0d1e17b863856287b18fd026e1ae71ec35e5ad83 
  LICENSE f11970cada909e7bbb685f96cb9ef0ff24e01924 


Diff: https://reviews.apache.org/r/58011/diff/1/


Testing
---

Testing done at end of chain


Thanks,

Jeff Coffler

Re: Review Request 57972: Added base stout Environment class to mesos-tests Environment class.

[Joseph Wu]

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




src/tests/environment.hpp
Line 34 (original), 35 (patched)


I'd expect there to be a substantial amount of deletions in 
`src/tests/environment.cpp`.


- Joseph Wu


On March 27, 2017, 3:15 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57972/
> ---
> 
> (Updated March 27, 2017, 3:15 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Bugs: MESOS-6731
> https://issues.apache.org/jira/browse/MESOS-6731
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added base stout Environment class to mesos-tests Environment class.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.hpp 06b6d54fc76b327e3e26914eb61c16619a36de42 
>   src/tests/main.cpp 5d062c3451bdfb5d5fc459ac7c071ab18e6d8043 
> 
> 
> Diff: https://reviews.apache.org/r/57972/diff/1/
> 
> 
> Testing
> ---
> 
> Ran make check on Linux and the tests on Windows.
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 57824: Filtered stout tests with symlinks when unable to create symlinks.

[Joseph Wu]

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




3rdparty/stout/include/stout/tests/environment.hpp
Lines 17-18 (patched)


Try `__STOUT_TESTS_ENVIRONMENT_HPP__` instead.



3rdparty/stout/include/stout/tests/environment.hpp
Lines 196-198 (patched)


The test filters constitute the main difference between the test 
environment of Stout vs Libprocess vs Mesos.  If we're going to re-use the code 
effectively, the vector of filters should either be:
A) an argument of `Environment` 's constructor; or
B) added via a object method (i.e. `Environment::AddFilter(...)`)

Doing (A) might result in some rather beefy constructors in 
Libprocess/Mesos tests.  I prefer this approach because it does not add a 
method that could be mis-used later on.

Doing (B) would require moving most of the other logic in the `Environment` 
constructor into `Environment::SetUp()`.



3rdparty/stout/include/stout/tests/environment.hpp
Lines 218-224 (patched)


You don't have include these two if they are empty.



3rdparty/stout/include/stout/tests/environment.hpp
Lines 226-248 (patched)


Leave this out for now.  I have a TODO to move this temporary directory 
helper out of the `Environment` entirely, so that we don't need a global 
variable in some of the tests (at the Mesos level).



3rdparty/stout/tests/environment.hpp
Lines 1 (patched)


Delete this file.



3rdparty/stout/tests/environment.cpp
Lines 1 (patched)


Delete this file.


- Joseph Wu


On March 27, 2017, 3:13 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57824/
> ---
> 
> (Updated March 27, 2017, 3:13 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Bugs: MESOS-6731
> https://issues.apache.org/jira/browse/MESOS-6731
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Filtered stout tests with symlinks when unable to create symlinks.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/tests/environment.hpp PRE-CREATION 
>   3rdparty/stout/tests/environment.hpp PRE-CREATION 
>   3rdparty/stout/tests/environment.cpp PRE-CREATION 
>   3rdparty/stout/tests/main.cpp f41b29c087402d5dbed1144504980746e0e9b6da 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> e445daf9ed4e4c0d44dbb95e1cbebd0342c1acbc 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 
> ed43b44663cbf04d7ddb449fd9f42b8de210bc6e 
>   3rdparty/stout/tests/os_tests.cpp 8b9531443f625ae0e1f00d29e9e45299f181880e 
> 
> 
> Diff: https://reviews.apache.org/r/57824/diff/2/
> 
> 
> Testing
> ---
> 
> Ran make check on Linux and the tests on Windows.
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 57167: Updated quota handler logic for hierarchical roles.

[Neil Conway]

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

(Updated March 28, 2017, 11:42 p.m.)


Review request for mesos and Michael Park.


Changes
---

Address review comments.


Repository: mesos


Description
---

The quota'd resources for a nested role are "included" within the
quota'd resources for that role's parent. Hence, the quota of a node
must always be greater than or equal to the sum of the quota'd resources
of that role's children.

When creating and removing quota, we must ensure that this invariant is
not violated.

When computing the cluster capacity heuristic, we must ensure that we do
not "double-count" quota'd resources: e.g., if the cluster has a total
capacity of 100 CPUs, role "x" has a quota guarantee of 80 CPUs, and
role "x/y" has a quota guarantee of 40 CPUs, this does NOT violate the
cluster capacity heuristic.


Diffs (updated)
-

  src/master/quota_handler.cpp 36ea1acca47014b2fb7a3b597b857c8ec9e2ab67 
  src/tests/hierarchical_allocator_tests.cpp 
e343dc37bd7136f0f6dd5dbc22a25cabe715038d 
  src/tests/master_quota_tests.cpp e418f22ea1773f4356ced44b8d57a80e826c8837 


Diff: https://reviews.apache.org/r/57167/diff/9/

Changes: https://reviews.apache.org/r/57167/diff/8-9/


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 57167: Updated quota handler logic for hierarchical roles.

[Neil Conway]

> On March 28, 2017, 11:06 p.m., Michael Park wrote:
> > src/master/quota_handler.cpp
> > Lines 111-115 (patched)
> > 
> >
> > Is this `CHECK` here to require that we only `insert` once per 
> > `role`...? If yes, it seems that the comment is a bit confusing to me? 
> > Could you clarify the intended semantics here?

Clarified the wording of this comment.


> On March 28, 2017, 11:06 p.m., Michael Park wrote:
> > src/master/quota_handler.cpp
> > Lines 469-487 (original), 605-618 (patched)
> > 
> >
> > Couldn't this be just:
> > 
> > ```cpp
> > vector components = strings::split(request.url.path, "/", 3u);
> > if (components.size() < 3u) {
> >   return BadRequest(
> >   "Failed to parse request path '" + request.url.path +
> >   "': 3 tokens ('master', 'quota', 'role') required, found " +
> >   stringify(tokens.size()) + " token(s)"));
> > }
> > 
> > CHECK_EQ(3u, components.size());
> > const string& role = components.back();
> > /* ... */
> > ```

I'm not sure this is actually more readable; leaving as-is for now, will 
discuss offline.


- Neil


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


On March 23, 2017, 12:53 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57167/
> ---
> 
> (Updated March 23, 2017, 12:53 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The quota'd resources for a nested role are "included" within the
> quota'd resources for that role's parent. Hence, the quota of a node
> must always be greater than or equal to the sum of the quota'd resources
> of that role's children.
> 
> When creating and removing quota, we must ensure that this invariant is
> not violated.
> 
> When computing the cluster capacity heuristic, we must ensure that we do
> not "double-count" quota'd resources: e.g., if the cluster has a total
> capacity of 100 CPUs, role "x" has a quota guarantee of 80 CPUs, and
> role "x/y" has a quota guarantee of 40 CPUs, this does NOT violate the
> cluster capacity heuristic.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 36ea1acca47014b2fb7a3b597b857c8ec9e2ab67 
>   src/tests/hierarchical_allocator_tests.cpp 
> e343dc37bd7136f0f6dd5dbc22a25cabe715038d 
>   src/tests/master_quota_tests.cpp e418f22ea1773f4356ced44b8d57a80e826c8837 
> 
> 
> Diff: https://reviews.apache.org/r/57167/diff/8/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 57976: Refactor default executor for `launchTaskSubprocess`.

[Andrew Schwartzmeyer]

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

(Updated March 28, 2017, 11:28 p.m.)


Review request for mesos, John Kordich, Joseph Wu, Li Li, and Michael Park.


Changes
---

Update test results.


Repository: mesos


Description
---

This commit reverses the file split done in e821978.
Since `launchTaskPosix` and `launchTaskWindows` were reconciled
using `Subprocess`, the files were pulled back into just
`launcher/executor.cpp` with `launchTaskSubprocess`.


Diffs
-

  src/Makefile.am 071656ad7354a802e8292140a7181cb70b68fe9e 
  src/launcher/CMakeLists.txt f63f544f92924b92ef41382c40acabef59a56d8b 
  src/launcher/executor.hpp c7c134aed26d2116295d66100b3d6efaf610736c 
  src/launcher/executor.cpp a4bac9d932b6574eb2e1b4a27aa44eaaca6aaa62 
  src/launcher/posix/executor.hpp 2dd9766aa5b6e0550269ccaa79209d0a483fee76 
  src/launcher/posix/executor.cpp 7c4ef10390e7ecfe63e2fd0c813f91c896fc7a8d 


Diff: https://reviews.apache.org/r/57976/diff/1/


Testing (updated)
---

# CentOS 7:
## `make check`
```
[--] Global test environment tear-down
[==] 1534 tests from 173 test cases ran. (434228 ms total)
[  PASSED  ] 1533 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] LdcacheTest.Parse
```
Assuming that `LdcacheTest.Parse` is unrelated.

# Windows 10:
## `./3rdparty/stout/tests/Debug/stout-tests.exe`
```
[==] 230 tests from 39 test cases ran. (4361 ms total)
[  PASSED  ] 228 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] Base64Test.EncodeURLSafe
[  FAILED  ] Base64Test.DecodeURLSafe
```
These are [known failures](https://issues.apache.org/jira/browse/MESOS-7236).

## `./3rdparty/libprocess/src/tests/Debug/libprocess-tests.exe`
```
[--] Global test environment tear-down
[==] 116 tests from 26 test cases ran. (10006 ms total)
[  PASSED  ] 116 tests.
```

## `.\src\mesos-tests.exe --gtest_filter="MesosContainerizer/*"`
```
[--] 7 tests from MesosContainerizer/DefaultExecutorTest (110253 ms 
total)

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

## Entire `.\src\mesos-tests.exe`

```
[--] Global test environment tear-down
[==] 558 tests from 57 test cases ran. (66481401 ms total)
[  PASSED  ] 543 tests.
[  FAILED  ] 15 tests, listed below:
[  FAILED  ] CommandExecutorCheckTest.CommandCheckStatusChange
[  FAILED  ] CommandExecutorCheckTest.CommandCheckTimeout
[  FAILED  ] CommandExecutorCheckTest.CommandCheckAndHealthCheckNoShadowing
[  FAILED  ] HealthCheckTest.HealthyTask
[  FAILED  ] HealthCheckTest.HealthyTaskNonShell
[  FAILED  ] HealthCheckTest.HealthStatusChange
[  FAILED  ] HealthCheckTest.ConsecutiveFailures
[  FAILED  ] HealthCheckTest.EnvironmentSetup
[  FAILED  ] HealthCheckTest.GracePeriod
[  FAILED  ] HealthCheckTest.CheckCommandTimeout
[  FAILED  ] HealthCheckTest.HealthyToUnhealthyTransitionWithinGracePeriod
[  FAILED  ] HealthCheckTest.HealthyTaskViaTCP
[  FAILED  ] CombinedAuthenticatorTest.MultipleAuthenticators
[  FAILED  ] SlaveTest.StatisticsEndpointRunningExecutor
[  FAILED  ] CopyFetcherPluginTest.FetchExistingFile
```

The `FetchExistingFile` is a [known 
failure](https://issues.apache.org/jira/browse/MESOS-7311).

The `CommandExecutorCheckTest` failures are new, and I've traced them to the 
difference between:

```
.\mesos-containerizer.exe launch --help=false 
--launch_info={"command":{"shell":true,"value":"ping 127.0.0.1 -n 1"}}
```

and

```
.\mesos-containerizer.exe launch --help=false 
--launch_info="{\"command\":{\"shell\":true,\"value\":\"ping 127.0.0.1 -n 
1\"}}"
```

so I need to introduce some quoting to `launchTaskSubprocess`.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 57167: Updated quota handler logic for hierarchical roles.

[Michael Park]

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


Fix it, then Ship it!





src/master/quota_handler.cpp
Lines 111-115 (patched)


Is this `CHECK` here to require that we only `insert` once per `role`...? 
If yes, it seems that the comment is a bit confusing to me? Could you clarify 
the intended semantics here?



src/master/quota_handler.cpp
Lines 153-155 (patched)


Let's still put the members at the bottom.



src/master/quota_handler.cpp
Lines 469-487 (original), 605-618 (patched)


Couldn't this be just:

```cpp
vector components = strings::split(request.url.path, "/", 3u);
if (components.size() < 3u) {
  return BadRequest(
  "Failed to parse request path '" + request.url.path +
  "': 3 tokens ('master', 'quota', 'role') required, found " +
  stringify(tokens.size()) + " token(s)"));
}

CHECK_EQ(3u, components.size());
const string& role = components.back();
/* ... */
```



src/tests/master_quota_tests.cpp
Lines 1800-1816 (patched)


How about something like this?
```cpp
map expected = {{parent.role(), parent.guarantee()},
   {child.role(), child.guarantee()}};

map actual = {
  {status->infos(0).role(), status->infos(0).guarantee()},
  {status->infos(1).role(), status->infos(1).guarantee()}
};

EXPECT_EQ(expected, actual);
```


- Michael Park


On March 22, 2017, 5:53 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57167/
> ---
> 
> (Updated March 22, 2017, 5:53 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The quota'd resources for a nested role are "included" within the
> quota'd resources for that role's parent. Hence, the quota of a node
> must always be greater than or equal to the sum of the quota'd resources
> of that role's children.
> 
> When creating and removing quota, we must ensure that this invariant is
> not violated.
> 
> When computing the cluster capacity heuristic, we must ensure that we do
> not "double-count" quota'd resources: e.g., if the cluster has a total
> capacity of 100 CPUs, role "x" has a quota guarantee of 80 CPUs, and
> role "x/y" has a quota guarantee of 40 CPUs, this does NOT violate the
> cluster capacity heuristic.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 36ea1acca47014b2fb7a3b597b857c8ec9e2ab67 
>   src/tests/hierarchical_allocator_tests.cpp 
> e343dc37bd7136f0f6dd5dbc22a25cabe715038d 
>   src/tests/master_quota_tests.cpp e418f22ea1773f4356ced44b8d57a80e826c8837 
> 
> 
> Diff: https://reviews.apache.org/r/57167/diff/8/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 57911: Added UNKNOWN DiskInfo.Source type.

[Benjamin Bannier]

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

(Updated March 28, 2017, 9:21 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Addressed comment from Jie.


Repository: mesos


Description
---

We introduce an explicit UNKNOWN enum kind to allow explicit handling
of unknown enum values (e.g., when the sending and receiving end use
different versions of a message using the enum).

This commit also migrates pattern matching of values of this enum from
if statements to switch statements so that compiler diagnostics can be
used to identify unhandled cases when other types are added in the
future.


Diffs (updated)
-

  include/mesos/mesos.proto 9a66967ab459f75f21faf21be644f39b3fad670b 
  include/mesos/v1/mesos.proto 115f1b4d9e129f83a3aed62a95eb11faa12e04d1 
  src/common/resources.cpp ca1add1dbbe04fa775004010ebc847254c198bd7 
  src/slave/paths.cpp 38ad1993aa36a627ec97a7865488677495ee4c5a 
  src/slave/slave.cpp c8479d7e8eb915284f0ea8cf75f47acd679dee7e 
  src/v1/resources.cpp 6008a9d2572d04612f0da2d54834be548d366f29 


Diff: https://reviews.apache.org/r/57911/diff/3/

Changes: https://reviews.apache.org/r/57911/diff/2-3/


Testing
---

make check (OS X, Linux)


Thanks,

Benjamin Bannier

Re: Review Request 57911: Added UNKNOWN DiskInfo.Source type.

[Jie Yu]

> On March 28, 2017, 5:44 p.m., Jie Yu wrote:
> > src/slave/paths.cpp
> > Lines 497-498 (patched)
> > 
> >
> > I'll do:
> > ```
> > FAIL() << "Unsupported DiskInfo.Source.type";
> > ```
> 
> Benjamin Bannier wrote:
> `FAIL` comes from gtest which is currently not available when compiling 
> non-test code (e.g., not in the include path). Should I add it?
> 
> I went with `CHECK_NE` here as I saw it used for similar hard checks in 
> non-test code. It is provided by glog which we use here.

Ah, use LOG(FATAL) then


- Jie


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


On March 28, 2017, 6:06 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57911/
> ---
> 
> (Updated March 28, 2017, 6:06 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We introduce an explicit UNKNOWN enum kind to allow explicit handling
> of unknown enum values (e.g., when the sending and receiving end use
> different versions of a message using the enum).
> 
> This commit also migrates pattern matching of values of this enum from
> if statements to switch statements so that compiler diagnostics can be
> used to identify unhandled cases when other types are added in the
> future.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 9a66967ab459f75f21faf21be644f39b3fad670b 
>   include/mesos/v1/mesos.proto 115f1b4d9e129f83a3aed62a95eb11faa12e04d1 
>   src/common/resources.cpp ca1add1dbbe04fa775004010ebc847254c198bd7 
>   src/slave/paths.cpp 38ad1993aa36a627ec97a7865488677495ee4c5a 
>   src/slave/slave.cpp c8479d7e8eb915284f0ea8cf75f47acd679dee7e 
>   src/v1/resources.cpp 6008a9d2572d04612f0da2d54834be548d366f29 
> 
> 
> Diff: https://reviews.apache.org/r/57911/diff/2/
> 
> 
> Testing
> ---
> 
> make check (OS X, Linux)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 57911: Added UNKNOWN DiskInfo.Source type.

[Benjamin Bannier]

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

(Updated March 28, 2017, 8:06 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Replaced one more instance of `if` with `switch`.


Summary (updated)
-

Added UNKNOWN DiskInfo.Source type.


Repository: mesos


Description
---

We introduce an explicit UNKNOWN enum kind to allow explicit handling
of unknown enum values (e.g., when the sending and receiving end use
different versions of a message using the enum).

This commit also migrates pattern matching of values of this enum from
if statements to switch statements so that compiler diagnostics can be
used to identify unhandled cases when other types are added in the
future.


Diffs (updated)
-

  include/mesos/mesos.proto 9a66967ab459f75f21faf21be644f39b3fad670b 
  include/mesos/v1/mesos.proto 115f1b4d9e129f83a3aed62a95eb11faa12e04d1 
  src/common/resources.cpp ca1add1dbbe04fa775004010ebc847254c198bd7 
  src/slave/paths.cpp 38ad1993aa36a627ec97a7865488677495ee4c5a 
  src/slave/slave.cpp c8479d7e8eb915284f0ea8cf75f47acd679dee7e 
  src/v1/resources.cpp 6008a9d2572d04612f0da2d54834be548d366f29 


Diff: https://reviews.apache.org/r/57911/diff/2/

Changes: https://reviews.apache.org/r/57911/diff/1-2/


Testing
---

make check (OS X, Linux)


Thanks,

Benjamin Bannier

Re: Review Request 57911: WIP: Added UNKNOWN DiskInfo.Source type.

[Benjamin Bannier]

> On March 28, 2017, 7:44 p.m., Jie Yu wrote:
> > src/slave/paths.cpp
> > Lines 497-498 (patched)
> > 
> >
> > I'll do:
> > ```
> > FAIL() << "Unsupported DiskInfo.Source.type";
> > ```

`FAIL` comes from gtest which is currently not available when compiling 
non-test code (e.g., not in the include path). Should I add it?

I went with `CHECK_NE` here as I saw it used for similar hard checks in 
non-test code. It is provided by glog which we use here.


- Benjamin


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


On March 24, 2017, 4:39 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57911/
> ---
> 
> (Updated March 24, 2017, 4:39 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We introduce an explicit UNKNOWN enum kind to allow explicit handling
> of unknown enum values (e.g., when the sending and receiving end use
> different versions of a message using the enum).
> 
> This commit also migrates pattern matching of values of this enum from
> if statements to switch statements so that compiler diagnostics can be
> used to identify unhandled cases when other types are added in the
> future.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 9a66967ab459f75f21faf21be644f39b3fad670b 
>   include/mesos/v1/mesos.proto 115f1b4d9e129f83a3aed62a95eb11faa12e04d1 
>   src/common/resources.cpp ca1add1dbbe04fa775004010ebc847254c198bd7 
>   src/slave/paths.cpp 38ad1993aa36a627ec97a7865488677495ee4c5a 
>   src/slave/slave.cpp c6ee4fa9ad216fe82e97095e1808bb1fb1585398 
>   src/v1/resources.cpp 6008a9d2572d04612f0da2d54834be548d366f29 
> 
> 
> Diff: https://reviews.apache.org/r/57911/diff/1/
> 
> 
> Testing
> ---
> 
> make check (OS X, Linux)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 57911: WIP: Added UNKNOWN DiskInfo.Source type.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/paths.cpp
Lines 497-498 (patched)


I'll do:
```
FAIL() << "Unsupported DiskInfo.Source.type";
```


- Jie Yu


On March 24, 2017, 3:39 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57911/
> ---
> 
> (Updated March 24, 2017, 3:39 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We introduce an explicit UNKNOWN enum kind to allow explicit handling
> of unknown enum values (e.g., when the sending and receiving end use
> different versions of a message using the enum).
> 
> This commit also migrates pattern matching of values of this enum from
> if statements to switch statements so that compiler diagnostics can be
> used to identify unhandled cases when other types are added in the
> future.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 9a66967ab459f75f21faf21be644f39b3fad670b 
>   include/mesos/v1/mesos.proto 115f1b4d9e129f83a3aed62a95eb11faa12e04d1 
>   src/common/resources.cpp ca1add1dbbe04fa775004010ebc847254c198bd7 
>   src/slave/paths.cpp 38ad1993aa36a627ec97a7865488677495ee4c5a 
>   src/slave/slave.cpp c6ee4fa9ad216fe82e97095e1808bb1fb1585398 
>   src/v1/resources.cpp 6008a9d2572d04612f0da2d54834be548d366f29 
> 
> 
> Diff: https://reviews.apache.org/r/57911/diff/1/
> 
> 
> Testing
> ---
> 
> make check (OS X, Linux)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 57984: Fixed indents.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 28, 2017, 8:10 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57984/
> ---
> 
> (Updated March 28, 2017, 8:10 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed indents.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 43e9d26167c1f405329ea05224c22e7b8c65315f 
> 
> 
> Diff: https://reviews.apache.org/r/57984/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 57994: Fixed a regression hiding previously exposed master and agent flags.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [57994]

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 March 28, 2017, 2:01 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57994/
> ---
> 
> (Updated March 28, 2017, 2:01 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Michael Park.
> 
> 
> Bugs: MESOS-7316
> https://issues.apache.org/jira/browse/MESOS-7316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In f441eb9 we in a number of places changed  how 'Flag's were added to
> 'Flags' by moving from ad-hoc invocations of 'FlagsBase::add' on
> particular instances to proper 'Flags' member variables. This was needed
> to ensure 'Flags' instances could always safely be copied. For that we
> introduced local derived 'Flags' classes to support localized parsing
> needs. At the same time, this implementation strategy led to these these
> local variables not being accessible through instances of the original
> class anymore (this was inevitable when making 'Flags' classes properly
> copyable), which e.g., causes a regression in the flags displayed in a
> master's '/flags' endpoint.
> 
> This commit moves the flags into the respective base class removing the
> local classes so that all passed flags are exposed to users.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp 41a0edfaecf04759f1efa62a9851fbeeb214e84c 
>   src/master/flags.cpp b7a129b27bf752bf238d214534364403853c1b36 
>   src/master/main.cpp fa7ba1310142a3bef71379ba37fded9b8390aae9 
>   src/slave/flags.hpp 224fac1d06d5a3914d4d1408e880458ac5be010e 
>   src/slave/flags.cpp 76881536e0058880f5720fbf3c1cebc684508235 
>   src/slave/main.cpp 81d61b14accca7611d84db92663a63d5777edd83 
> 
> 
> Diff: https://reviews.apache.org/r/57994/diff/1/
> 
> 
> Testing
> ---
> 
> Tested on a number of platforms in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 57994: Fixed a regression hiding previously exposed master and agent flags.

[Benjamin Bannier]

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

Review request for mesos, Anand Mazumdar and Michael Park.


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


Repository: mesos


Description
---

In f441eb9 we in a number of places changed  how 'Flag's were added to
'Flags' by moving from ad-hoc invocations of 'FlagsBase::add' on
particular instances to proper 'Flags' member variables. This was needed
to ensure 'Flags' instances could always safely be copied. For that we
introduced local derived 'Flags' classes to support localized parsing
needs. At the same time, this implementation strategy led to these these
local variables not being accessible through instances of the original
class anymore (this was inevitable when making 'Flags' classes properly
copyable), which e.g., causes a regression in the flags displayed in a
master's '/flags' endpoint.

This commit moves the flags into the respective base class removing the
local classes so that all passed flags are exposed to users.


Diffs
-

  src/master/flags.hpp 41a0edfaecf04759f1efa62a9851fbeeb214e84c 
  src/master/flags.cpp b7a129b27bf752bf238d214534364403853c1b36 
  src/master/main.cpp fa7ba1310142a3bef71379ba37fded9b8390aae9 
  src/slave/flags.hpp 224fac1d06d5a3914d4d1408e880458ac5be010e 
  src/slave/flags.cpp 76881536e0058880f5720fbf3c1cebc684508235 
  src/slave/main.cpp 81d61b14accca7611d84db92663a63d5777edd83 


Diff: https://reviews.apache.org/r/57994/diff/1/


Testing
---

Tested on a number of platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 57989: Removing deprecated ACL `ShutdownFramework`.

[Mesos Reviewbot]

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



Bad review!

Reviews applied: [57989, 57472, 57166, 56805, 57165, 57164]

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

- Mesos Reviewbot


On March 28, 2017, 9:35 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57989/
> ---
> 
> (Updated March 28, 2017, 9:35 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7259
> https://issues.apache.org/jira/browse/MESOS-7259
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `ShutdownFramework` message from `acls.proto` was deprecated more
> than six months ago in favor of `TeardownFramework`. Since its
> deprecation cycle is already over, this patch removes the message.
> 
> Note that this only removes the message support from the ACLs, since
> the associated action was removed long ago.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 875ae02008cb22515396cf17c1d2e7da0046d068 
>   docs/upgrades.md e7f95aadeee330b3a9dbafc0f716c13df1bc8252 
>   include/mesos/authorizer/acls.proto 
> 46e66e8ed42240e5258890cdaf5aebe8800efcf0 
>   src/authorizer/local/authorizer.cpp 
> be8037299601427e5d5e79f58f77eea3f89579d0 
>   src/tests/teardown_tests.cpp 239a87ee0be8605a3f13196933038ef752ad9d4b 
> 
> 
> Diff: https://reviews.apache.org/r/57989/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 57731: Fixed a test in MasterAuthorizationTest.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [57520, 57534, 57535, 57710, 57730, 57731]

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 March 17, 2017, 3:55 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57731/
> ---
> 
> (Updated March 17, 2017, 3:55 p.m.)
> 
> 
> Review request for mesos, Anindya Sinha and Vinod Kone.
> 
> 
> Bugs: MESOS-7097
> https://issues.apache.org/jira/browse/MESOS-7097
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - `MasterAuthorizationTest.PendingExecutorInfoDiffersOnDifferentSlaves`
>   used to assume the mock authorizer is only called for tasks
>   authorization but with the new `regsiter_agents` ACL this is no
>   longer true.
> 
> 
> Diffs
> -
> 
>   src/tests/master_authorization_tests.cpp 
> 1a0285a3f345ef21a8256d4123d8bb684f538da4 
> 
> 
> Diff: https://reviews.apache.org/r/57731/diff/4/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 55065: Added support for s390x architecture

[Till Toenshoff]

> On March 22, 2017, 2:45 p.m., Till Toenshoff wrote:
> > src/linux/ldcache.cpp
> > Line 120 (original), 120 (patched)
> > 
> >
> > Can you elaborate why this change is safe for all systems?
> 
> Ayanampudi Varsha wrote:
> We have tried this on Big endian system where 16 bit flag was causing 
> issue.
> Test case LdcacheTest.Parse failed at condition ASSERT_GT(cache->size(), 
> 1u); as size was returned as 0. This happens because the 16 bits after the 
> field "flags" is unused. 
> On x86_64, I have verified that this "cache->size()" is same with 16 bit 
> and 32 bit flag.
> Observed that the other fields in structure are all 32bit/64bit.
> An alternative would be to make changes only specific to s390x by putting 
> a condition. Please suggest
> 
> Till Toenshoff wrote:
> You may actually have found a bug in our ldcache parsing. After looking 
> into the glibc sources, I think this field should have been 32bit also for 
> x86_64 instead of 16bit - I suspect missing struct-packing for this bug to 
> not have surfaced earlier. For confirming this without mixing it with your 
> platform specific docker-repository change, I would like you to split this 
> patch into two.
> 
> So please add another patch that only fixes `flags` attribute size into 
> 32bit. Please make sure you add the following reviewers: tillt, klueska.
> 
> Remove that 32bit change from this patch and update it according to my 
> and Kevin's review comments.
> 
> Thanks a bunch for your work - this is highly appreciated.
> 
> Till Toenshoff wrote:
> Ow, for further reference of this `flags` change, here are the internal 
> glibc structures: 
> https://github.com/lattera/glibc/blob/master/sysdeps/generic/dl-cache.h#L80
> 
> Ayanampudi Varsha wrote:
> As I am planning to drop the docker_registry customization change, can I 
> resubmit removing docker_registry change in same patch and continue in the 
> same review board ticket. please suggest

Certainly Ayanampudi. Please do as you suggested...


- Till


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


On March 27, 2017, 9:25 a.m., Ayanampudi Varsha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55065/
> ---
> 
> (Updated March 27, 2017, 9:25 a.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Bugs: MESOS-6742
> https://issues.apache.org/jira/browse/MESOS-6742
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for s390x architecture
> There are 2 issues addressed in this patch:
> 1. LdcacheTest.Parse test case fails on s390x machines.
> 2. From the value of flag docker_registry in slave/flags.cpp, amd64 images 
> get downloaded due to which test cases fail on s390x with "Exec format Error"
> 
> 
> Diffs
> -
> 
>   src/linux/ldcache.cpp 6260496672cd0226a24ea1b34003a48af9678a63 
>   src/slave/flags.cpp 1eccea920338032173be4df6c374ec50dbd2eaf3 
> 
> 
> Diff: https://reviews.apache.org/r/55065/diff/1/
> 
> 
> Testing
> ---
> 
> make check 
> make check GTEST_FILTER="LdcacheTest.Parse"
> 
> 
> Thanks,
> 
> Ayanampudi Varsha
> 
>

Re: Review Request 55065: Added support for s390x architecture

[Ayanampudi Varsha]

> On March 27, 2017, 7:01 p.m., Kevin Klues wrote:
> > src/slave/flags.cpp
> > Lines 152-158 (original), 152-169 (patched)
> > 
> >
> > We typically don't create duplicate entries like this when simply 
> > customizing the default value. Instead we use a lambda with the #ifdef's 
> > inside of it. For example,
> > 
> > https://github.com/apache/mesos/blob/master/src/slave/flags.cpp#L212
> 
> Till Toenshoff wrote:
> I am not sure we really need to come up with a lambda for such static 
> default - see 
> https://github.com/apache/mesos/blob/42f43ceb3b4b375357281030fb3816e548dac763/src/slave/flags.cpp#L670
> Let's not make this more complicated than needed.
> 
> Kevin Klues wrote:
> Sure that makes sense. No need for a lambda in this case, but the #ifdef 
> should only be around the new default value parameter, not a duplicate of the 
> entire flags field.

On a second thought, as user can anyways configure to local path if required, 
as mentioned in flag description.
I would like to drop these changes.


- Ayanampudi


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


On March 27, 2017, 9:25 a.m., Ayanampudi Varsha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55065/
> ---
> 
> (Updated March 27, 2017, 9:25 a.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Bugs: MESOS-6742
> https://issues.apache.org/jira/browse/MESOS-6742
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for s390x architecture
> There are 2 issues addressed in this patch:
> 1. LdcacheTest.Parse test case fails on s390x machines.
> 2. From the value of flag docker_registry in slave/flags.cpp, amd64 images 
> get downloaded due to which test cases fail on s390x with "Exec format Error"
> 
> 
> Diffs
> -
> 
>   src/linux/ldcache.cpp 6260496672cd0226a24ea1b34003a48af9678a63 
>   src/slave/flags.cpp 1eccea920338032173be4df6c374ec50dbd2eaf3 
> 
> 
> Diff: https://reviews.apache.org/r/55065/diff/1/
> 
> 
> Testing
> ---
> 
> make check 
> make check GTEST_FILTER="LdcacheTest.Parse"
> 
> 
> Thanks,
> 
> Ayanampudi Varsha
> 
>

Re: Review Request 55065: Added support for s390x architecture

[Ayanampudi Varsha]

> On March 22, 2017, 2:45 p.m., Till Toenshoff wrote:
> > src/linux/ldcache.cpp
> > Line 120 (original), 120 (patched)
> > 
> >
> > Can you elaborate why this change is safe for all systems?
> 
> Ayanampudi Varsha wrote:
> We have tried this on Big endian system where 16 bit flag was causing 
> issue.
> Test case LdcacheTest.Parse failed at condition ASSERT_GT(cache->size(), 
> 1u); as size was returned as 0. This happens because the 16 bits after the 
> field "flags" is unused. 
> On x86_64, I have verified that this "cache->size()" is same with 16 bit 
> and 32 bit flag.
> Observed that the other fields in structure are all 32bit/64bit.
> An alternative would be to make changes only specific to s390x by putting 
> a condition. Please suggest
> 
> Till Toenshoff wrote:
> You may actually have found a bug in our ldcache parsing. After looking 
> into the glibc sources, I think this field should have been 32bit also for 
> x86_64 instead of 16bit - I suspect missing struct-packing for this bug to 
> not have surfaced earlier. For confirming this without mixing it with your 
> platform specific docker-repository change, I would like you to split this 
> patch into two.
> 
> So please add another patch that only fixes `flags` attribute size into 
> 32bit. Please make sure you add the following reviewers: tillt, klueska.
> 
> Remove that 32bit change from this patch and update it according to my 
> and Kevin's review comments.
> 
> Thanks a bunch for your work - this is highly appreciated.
> 
> Till Toenshoff wrote:
> Ow, for further reference of this `flags` change, here are the internal 
> glibc structures: 
> https://github.com/lattera/glibc/blob/master/sysdeps/generic/dl-cache.h#L80

As I am planning to drop the docker_registry customization change, can I 
resubmit removing docker_registry change in same patch and continue in the same 
review board ticket. please suggest


- Ayanampudi


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


On March 27, 2017, 9:25 a.m., Ayanampudi Varsha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55065/
> ---
> 
> (Updated March 27, 2017, 9:25 a.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Bugs: MESOS-6742
> https://issues.apache.org/jira/browse/MESOS-6742
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for s390x architecture
> There are 2 issues addressed in this patch:
> 1. LdcacheTest.Parse test case fails on s390x machines.
> 2. From the value of flag docker_registry in slave/flags.cpp, amd64 images 
> get downloaded due to which test cases fail on s390x with "Exec format Error"
> 
> 
> Diffs
> -
> 
>   src/linux/ldcache.cpp 6260496672cd0226a24ea1b34003a48af9678a63 
>   src/slave/flags.cpp 1eccea920338032173be4df6c374ec50dbd2eaf3 
> 
> 
> Diff: https://reviews.apache.org/r/55065/diff/1/
> 
> 
> Testing
> ---
> 
> make check 
> make check GTEST_FILTER="LdcacheTest.Parse"
> 
> 
> Thanks,
> 
> Ayanampudi Varsha
> 
>

Re: Review Request 56853: Update OCI tests with the latest OCI image spec.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [56851, 56852, 56853]

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 March 28, 2017, 8:20 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56853/
> ---
> 
> (Updated March 28, 2017, 8:20 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
> ---
> 
> Update OCI tests with the latest OCI image spec.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/oci_spec_tests.cpp 
> 29a8f23caae3b13c5abfcec484ab8161b334713d 
> 
> 
> Diff: https://reviews.apache.org/r/56853/diff/3/
> 
> 
> Testing
> ---
> 
> make -j8 V=0 check GTEST_FILTER="OCISpecTest.*"
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 57989: Removing deprecated ACL `ShutdownFramework`.

[Alexander Rojas]

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

Review request for mesos, Adam B and Alexander Rukletsov.


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


Repository: mesos


Description
---

The `ShutdownFramework` message from `acls.proto` was deprecated more
than six months ago in favor of `TeardownFramework`. Since its
deprecation cycle is already over, this patch removes the message.

Note that this only removes the message support from the ACLs, since
the associated action was removed long ago.


Diffs
-

  CHANGELOG 875ae02008cb22515396cf17c1d2e7da0046d068 
  docs/upgrades.md e7f95aadeee330b3a9dbafc0f716c13df1bc8252 
  include/mesos/authorizer/acls.proto 46e66e8ed42240e5258890cdaf5aebe8800efcf0 
  src/authorizer/local/authorizer.cpp be8037299601427e5d5e79f58f77eea3f89579d0 
  src/tests/teardown_tests.cpp 239a87ee0be8605a3f13196933038ef752ad9d4b 


Diff: https://reviews.apache.org/r/57989/diff/1/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 56852: Updated OCI spec parsing & validation code with latest OCI image spec.

[Qian Zhang]

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

(Updated March 28, 2017, 4:49 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Removed the unnecessary media type validation, because as per the spec, an 
encountered mediaType that is unknown should be safely ignored.


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


Repository: mesos


Description
---

Updated OCI spec parsing & validation code with latest OCI image spec.


Diffs (updated)
-

  include/mesos/oci/spec.hpp ea4f29e3f5a552b671738d664882f08db682ad90 
  src/oci/spec.cpp 0a88edb404836205f11e61ba3edb771df8b618ef 


Diff: https://reviews.apache.org/r/56852/diff/5/

Changes: https://reviews.apache.org/r/56852/diff/4-5/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 57534: Added and implemented RegisterAgent ACL.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On March 28, 2017, 10:24 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57534/
> ---
> 
> (Updated March 28, 2017, 10:24 a.m.)
> 
> 
> Review request for mesos, Adam B, Anindya Sinha, Alexander Rojas, and Greg 
> Mann.
> 
> 
> Bugs: MESOS-7097
> https://issues.apache.org/jira/browse/MESOS-7097
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added and implemented RegisterAgent ACL.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 46e66e8ed42240e5258890cdaf5aebe8800efcf0 
>   include/mesos/authorizer/authorizer.proto 
> 736f76d552956f2351ffd40fc51d088dff83f8c8 
>   src/authorizer/local/authorizer.cpp 
> be8037299601427e5d5e79f58f77eea3f89579d0 
>   src/tests/authorization_tests.cpp cd15add7d7b01c2b316ac946e017a4d0b502237f 
> 
> 
> Diff: https://reviews.apache.org/r/57534/diff/6/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 57472: Removing deprecated ACLs `SetQuota` and `RemoveQuota`.

[Alexander Rojas]

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

(Updated March 28, 2017, 10:43 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, and Benjamin Bannier.


Changes
---

Rebase, adam's comments.


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


Repository: mesos


Description
---

The ACLs `SetQuota` and `RemoveQuota` where marked for deprecation
more than six months ago, where they were replaced for the more
convenient `UpdateQuota` ACL. The deprecation cycle for these actions
is finally due while at the same time removing will make easier to
implement the hierarchical role feature in a generic way.


Diffs (updated)
-

  CHANGELOG 875ae02008cb22515396cf17c1d2e7da0046d068 
  docs/upgrades.md e7f95aadeee330b3a9dbafc0f716c13df1bc8252 
  include/mesos/authorizer/acls.proto 46e66e8ed42240e5258890cdaf5aebe8800efcf0 
  src/authorizer/local/authorizer.cpp be8037299601427e5d5e79f58f77eea3f89579d0 
  src/master/master.hpp d92c8adef79d997f255cf26ebd10ab0e87da8413 
  src/master/quota_handler.cpp 36ea1acca47014b2fb7a3b597b857c8ec9e2ab67 
  src/tests/authorization_tests.cpp cd15add7d7b01c2b316ac946e017a4d0b502237f 
  src/tests/master_quota_tests.cpp e418f22ea1773f4356ced44b8d57a80e826c8837 


Diff: https://reviews.apache.org/r/57472/diff/6/

Changes: https://reviews.apache.org/r/57472/diff/5-6/


Testing
---

`make check`


Thanks,

Alexander Rojas

Re: Review Request 57535: Applied RegisterAgent ACL to the master.

[Jiang Yan Xu]

> On March 24, 2017, 2:58 p.m., Benjamin Mahler wrote:
> > Nice tests! Overall approach looks good to me. A few comments below.
> > 
> > Unrelated to your changes I noticed a few issues:
> > 
> > There are some inconsistencies between the framework and agent paths. For 
> > example, we don't log when we receive an agent's (re-)registration message 
> > but we log the framework's subscription, not sure why we did that. Also, 
> > since we don't track a framework's pending subscription, if the 
> > authorization futures are re-ordered we could process subscre 2 before 
> > subscre 1, but this is unrelated to your change here.
> > 
> > The "queueing up" logic (example 
> > [here](https://github.com/apache/mesos/blob/1bb7ed28977d9b03c6a9162e8d8d10c7420a47f9/src/master/master.cpp#L5344-L5345))
> >  also allows re-ordering.

Thanks!

- The logging is easy to fix. We can add such logging for the agent 
registration as well.
- Pending framework registration re-ordering: It looks to me we are OK for now 
because we only have one hop in the async flow (subscribe -> authorizer -> 
_subscribe) and do we 
[this](https://github.com/apache/mesos/blob/1bb7ed28977d9b03c6a9162e8d8d10c7420a47f9/src/master/master.cpp#L2902-L2911)
 in the 2nd step. If we later add frameworks to the registrar we would need 
more hops and need to do something similar to what this patch does.
- Can authorization futures be re-ordered? At least with the local authorizer 
they are executed by the same authorizer actor that satifies the futures. I was 
thinking more about other events like ExitedEvent being enqueued between 
duplicate subscribes and registrations.
- The "queueing up" logic: I don't see how `registerSlave` retries can 
themsevles get re-ordered. Perhaps two `Master::registerSlave` dispatches 
enqueued by the authenticator can have an UnregisterSlaveMessage go between 
then due to the async step but `registering` and `reregistering` seem to be 
able to handle it.


> On March 24, 2017, 2:58 p.m., Benjamin Mahler wrote:
> > src/master/master.hpp
> > Line 1640 (original), 1665-1668 (patched)
> > 
> >
> > It looks like the comment about not answering questions about these 
> > transitioning agents was removed, can we restore it?

Given the recent changes in /r/52083/, Perhaps it's best to put the comment 
above `recovered`?

```
// Slaves that have been recovered from the registrar after master
// failover. Slaves are removed from this collection when they
// either re-register with the master or are marked unreachable
// because they do not re-register before `recoveredTimer` fires.
// We must not answer questions related to these slaves (e.g.,
// during task reconciliation) until we determine their fate.
hashmap recovered;
```


- Jiang Yan


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


On March 28, 2017, 1:40 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57535/
> ---
> 
> (Updated March 28, 2017, 1:40 a.m.)
> 
> 
> Review request for mesos, Adam B, Anindya Sinha, Alexander Rojas, Benjamin 
> Mahler, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7097
> https://issues.apache.org/jira/browse/MESOS-7097
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Applied RegisterAgent ACL to the master.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp d92c8adef79d997f255cf26ebd10ab0e87da8413 
>   src/master/master.cpp 43e9d26167c1f405329ea05224c22e7b8c65315f 
>   src/tests/master_authorization_tests.cpp 
> 1a0285a3f345ef21a8256d4123d8bb684f538da4 
> 
> 
> Diff: https://reviews.apache.org/r/57535/diff/6/
> 
> 
> Testing
> ---
> 
> make check.
> 
> The tests added here cover some basic scenarios, I have more tests but will 
> add them when MESOS-7244 is fixed.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 57535: Applied RegisterAgent ACL to the master.

[Jiang Yan Xu]

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

(Updated March 28, 2017, 1:40 a.m.)


Review request for mesos, Adam B, Anindya Sinha, Alexander Rojas, Benjamin 
Mahler, Greg Mann, and Vinod Kone.


Changes
---

Comments.


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


Repository: mesos


Description
---

Applied RegisterAgent ACL to the master.


Diffs (updated)
-

  src/master/master.hpp d92c8adef79d997f255cf26ebd10ab0e87da8413 
  src/master/master.cpp 43e9d26167c1f405329ea05224c22e7b8c65315f 
  src/tests/master_authorization_tests.cpp 
1a0285a3f345ef21a8256d4123d8bb684f538da4 


Diff: https://reviews.apache.org/r/57535/diff/6/

Changes: https://reviews.apache.org/r/57535/diff/5-6/


Testing
---

make check.

The tests added here cover some basic scenarios, I have more tests but will add 
them when MESOS-7244 is fixed.


Thanks,

Jiang Yan Xu

Re: Review Request 57534: Added and implemented RegisterAgent ACL.

[Jiang Yan Xu]

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

(Updated March 28, 2017, 1:24 a.m.)


Review request for mesos, Adam B, Anindya Sinha, Alexander Rojas, and Greg Mann.


Changes
---

Comments.


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


Repository: mesos


Description
---

Added and implemented RegisterAgent ACL.


Diffs (updated)
-

  include/mesos/authorizer/acls.proto 46e66e8ed42240e5258890cdaf5aebe8800efcf0 
  include/mesos/authorizer/authorizer.proto 
736f76d552956f2351ffd40fc51d088dff83f8c8 
  src/authorizer/local/authorizer.cpp be8037299601427e5d5e79f58f77eea3f89579d0 
  src/tests/authorization_tests.cpp cd15add7d7b01c2b316ac946e017a4d0b502237f 


Diff: https://reviews.apache.org/r/57534/diff/6/

Changes: https://reviews.apache.org/r/57534/diff/5-6/


Testing
---

make check.


Thanks,

Jiang Yan Xu

Re: Review Request 56853: Update OCI tests with the latest OCI image spec.

[Qian Zhang]

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

(Updated March 28, 2017, 4:20 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
---

Update OCI tests with the latest OCI image spec.


Diffs (updated)
-

  src/tests/containerizer/oci_spec_tests.cpp 
29a8f23caae3b13c5abfcec484ab8161b334713d 


Diff: https://reviews.apache.org/r/56853/diff/3/

Changes: https://reviews.apache.org/r/56853/diff/2-3/


Testing
---

make -j8 V=0 check GTEST_FILTER="OCISpecTest.*"


Thanks,

Qian Zhang

Re: Review Request 57472: Removing deprecated ACLs `SetQuota` and `RemoveQuota`.

[Adam B]

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


Fix it, then Ship it!




Minor clarification, otherwise great.


CHANGELOG
Lines 7-8 (patched)


This change is only applicable to the local authorizer, right? Other 
authorizers probably don't use `--acls`.


- Adam B


On March 21, 2017, 6:28 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57472/
> ---
> 
> (Updated March 21, 2017, 6:28 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Benjamin Bannier.
> 
> 
> Bugs: MESOS-7259
> https://issues.apache.org/jira/browse/MESOS-7259
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The ACLs `SetQuota` and `RemoveQuota` where marked for deprecation
> more than six months ago, where they were replaced for the more
> convenient `UpdateQuota` ACL. The deprecation cycle for these actions
> is finally due while at the same time removing will make easier to
> implement the hierarchical role feature in a generic way.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 09fbf03b4274098e8051460bf9d7e9dac4acc987 
>   docs/upgrades.md e7f95aadeee330b3a9dbafc0f716c13df1bc8252 
>   include/mesos/authorizer/acls.proto 
> e75e1879435f1c2bce47a86e9feebf9d051e969b 
>   src/authorizer/local/authorizer.cpp 
> be8037299601427e5d5e79f58f77eea3f89579d0 
>   src/master/master.hpp d92c8adef79d997f255cf26ebd10ab0e87da8413 
>   src/master/quota_handler.cpp 36ea1acca47014b2fb7a3b597b857c8ec9e2ab67 
>   src/tests/authorization_tests.cpp cd15add7d7b01c2b316ac946e017a4d0b502237f 
>   src/tests/master_quota_tests.cpp e418f22ea1773f4356ced44b8d57a80e826c8837 
> 
> 
> Diff: https://reviews.apache.org/r/57472/diff/5/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 56852: Updated OCI spec parsing & validation code with latest OCI image spec.

[Qian Zhang]

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

(Updated March 28, 2017, 4:19 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
---

Updated OCI spec parsing & validation code with latest OCI image spec.


Diffs (updated)
-

  include/mesos/oci/spec.hpp ea4f29e3f5a552b671738d664882f08db682ad90 
  src/oci/spec.cpp 0a88edb404836205f11e61ba3edb771df8b618ef 


Diff: https://reviews.apache.org/r/56852/diff/4/

Changes: https://reviews.apache.org/r/56852/diff/3-4/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 57710: Added `register_agents` to authorization.md.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On March 17, 2017, 12:07 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57710/
> ---
> 
> (Updated March 17, 2017, 12:07 a.m.)
> 
> 
> Review request for mesos, Adam B, Anindya Sinha, Alexander Rojas, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-7097
> https://issues.apache.org/jira/browse/MESOS-7097
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `register_agents` to authorization.md.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md 0abb05cce1d1453cdefc42e3510cbe100e64 
> 
> 
> Diff: https://reviews.apache.org/r/57710/diff/1/
> 
> 
> Testing
> ---
> 
> via a markdown viewer.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 57534: Added and implemented RegisterAgent ACL.

[Alexander Rojas]

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




src/authorizer/local/authorizer.cpp
Lines 1057-1067 (patched)


Please move this before the both `LAUNCH_NESTED_CONTAINER_*`, we are 
keeping the last two elements for errors and unknown.


- Alexander Rojas


On March 20, 2017, 5:09 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57534/
> ---
> 
> (Updated March 20, 2017, 5:09 a.m.)
> 
> 
> Review request for mesos, Adam B, Anindya Sinha, Alexander Rojas, and Greg 
> Mann.
> 
> 
> Bugs: MESOS-7097
> https://issues.apache.org/jira/browse/MESOS-7097
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added and implemented RegisterAgent ACL.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 46e66e8ed42240e5258890cdaf5aebe8800efcf0 
>   include/mesos/authorizer/authorizer.proto 
> 736f76d552956f2351ffd40fc51d088dff83f8c8 
>   src/authorizer/local/authorizer.cpp 
> be8037299601427e5d5e79f58f77eea3f89579d0 
>   src/tests/authorization_tests.cpp cd15add7d7b01c2b316ac946e017a4d0b502237f 
> 
> 
> Diff: https://reviews.apache.org/r/57534/diff/5/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 56851: Updated OCI protobuf messages with latest OCI image spec.

[Qian Zhang]

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

(Updated March 28, 2017, 4:10 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
---

Updated OCI protobuf messages with latest OCI image spec.


Diffs (updated)
-

  include/mesos/oci/spec.proto 97f3cf33fa89a0a15a6b7d52c6ca9f82c66bb28f 


Diff: https://reviews.apache.org/r/56851/diff/3/

Changes: https://reviews.apache.org/r/56851/diff/2-3/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 57984: Fixed indents.

[Mesos Reviewbot]

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



Bad review!

Reviews applied: []

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

- Mesos Reviewbot


On March 28, 2017, 7:09 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57984/
> ---
> 
> (Updated March 28, 2017, 7:09 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed indents.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 43e9d26167c1f405329ea05224c22e7b8c65315f 
> 
> 
> Diff: https://reviews.apache.org/r/57984/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Review Request 57984: Fixed indents.

[Klaus Ma]

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

Review request for mesos.


Repository: mesos


Description
---

Fixed indents.


Diffs
-

  src/master/master.cpp 43e9d26167c1f405329ea05224c22e7b8c65315f 


Diff: https://reviews.apache.org/r/57984/diff/1/


Testing
---


Thanks,

Klaus Ma