Re: Review Request 45951: Implemented http basic auth to get docker auth token.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 11, 2016, 12:42 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45951/
> ---
> 
> (Updated June 11, 2016, 12:42 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4938
> https://issues.apache.org/jira/browse/MESOS-4938
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented http basic auth to get docker auth token.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 
> 
> Diff: https://reviews.apache.org/r/45951/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Tested with mesos-execute, using a private repo from docker hub.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48570: Fixed docker fetcher plugin process indentation.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 11, 2016, 12:42 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48570/
> ---
> 
> (Updated June 11, 2016, 12:42 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed docker fetcher plugin process indentation.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 
> 
> Diff: https://reviews.apache.org/r/48570/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47808: Added test for parsing docker config.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 11, 2016, 12:42 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47808/
> ---
> 
> (Updated June 11, 2016, 12:42 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4938
> https://issues.apache.org/jira/browse/MESOS-4938
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for parsing docker config.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_spec_tests.cpp 
> 944a559bb3a8ef602b4d8d52773afdb8bf8e5bf6 
> 
> Diff: https://reviews.apache.org/r/47808/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47806: Add docker config auth protobuf to docker spec.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 11, 2016, 12:42 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47806/
> ---
> 
> (Updated June 11, 2016, 12:42 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4938
> https://issues.apache.org/jira/browse/MESOS-4938
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add docker config auth protobuf to docker spec.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.proto 4f2ff2d7d88ceca2f25864b60a04ff5ec05317ff 
> 
> Diff: https://reviews.apache.org/r/47806/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47807: Implemented parsing a docker config to a hashmap.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 11, 2016, 12:42 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47807/
> ---
> 
> (Updated June 11, 2016, 12:42 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4938
> https://issues.apache.org/jira/browse/MESOS-4938
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented parsing a docker config to a hashmap.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp b37c7731590ec144df720d7b28eeed5db76f750e 
>   src/docker/spec.cpp 7270d182b8c3a5d5c34cbda2bd736ba489089720 
> 
> Diff: https://reviews.apache.org/r/47807/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48062: Recalculated share when weight was updated.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48579, 48062]

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 June 11, 2016, 4:02 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48062/
> ---
> 
> (Updated June 11, 2016, 4:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Klaus Ma.
> 
> 
> Bugs: MESOS-5601
> https://issues.apache.org/jira/browse/MESOS-5601
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Recalculated share when weight was updated.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 65d473a5da0d846214c930c14d333040b2085b13 
>   src/tests/sorter_tests.cpp eb207a36c18198588cd8a98b3860a66c2ff7a641 
> 
> Diff: https://reviews.apache.org/r/48062/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> [ RUN  ] SorterTest.WDRFSorterUpdateWeight
> [   OK ] SorterTest.WDRFSorterUpdateWeight (1 ms)
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 48563: Fixed continuation logic in endpoint authorization.

[Joerg Schad]

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

(Updated June 11, 2016, 4:28 a.m.)


Review request for mesos and Adam B.


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


Repository: mesos


Description (updated)
---

Previously the continuation followed via `.then([=]`,
which potentially executes the continuation on a different
process. This patch fixes this behavior and avoids race 
conditions between the master process changing the 
related data-structures and another process reading them.


Diffs (updated)
-

  src/master/http.cpp 4b2d1386e1ecb447b597a432f9df9adaa5c3aa37 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 48566: Fixed continuation logic for sandbox authorization.

[Joerg Schad]

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

(Updated June 11, 2016, 4:19 a.m.)


Review request for mesos, Adam B and Alexander Rojas.


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


Repository: mesos


Description (updated)
---

Previously the continuation followed via `.then([=]`,
which potentially executes the continuation on a different
process. This patch fixes this behavior (it should
run on the same process) and avoids potential race 
conditions if the file logic is handled by different 
processes.


Diffs
-

  src/files/files.cpp 55c310af149cb03abfa1ffbb6477dc1ac5dc14be 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 48579: Reset "dirty" to false in sort().

[Klaus Ma]

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




src/master/allocator/sorter/drf/sorter.cpp (lines 137 - 142)


Dup the comments from JIRA:

In {{DRFSorter::allocated()}}, the {{allocations}} of client was updated so 
{{dirty}} should be true to trigger {{sort()}} and {{update(name)}} seems not 
necessary as the {{share}} will be re-calculated in {{sort()}}.
 
Further more, I'm thinking how many performance contribution will {{dirty}} 
help? In each allocator loop, the allocation maybe changed so the sorter should 
re-calculate the order.

To improve the performance, I think we can only update the single client in 
{{allocated}} & {{unallocated}}; and only return clients list in {{sort()}} 
(the client was sorted when inserted in {{allocated}} & {{unallocated}}).


- Klaus Ma


On June 11, 2016, 11:42 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48579/
> ---
> 
> (Updated June 11, 2016, 11:42 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Klaus Ma.
> 
> 
> Bugs: MESOS-5600
> https://issues.apache.org/jira/browse/MESOS-5600
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The "dirty" was set to "true" when the total resources in cluster was
> updated. But in sorter, the "dirty" was never set back as "false" after
> re-calculate share for each clients.
> 
> This patch reset the "dirty" to "false" in sort(), this can make sure
> only one client share was updated when there are allocation changes but
> not all clients in the cluster.
> 
> This can improve the performance of sorter.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 65d473a5da0d846214c930c14d333040b2085b13 
>   src/tests/sorter_tests.cpp eb207a36c18198588cd8a98b3860a66c2ff7a641 
> 
> Diff: https://reviews.apache.org/r/48579/diff/
> 
> 
> Testing
> ---
> 
> [==] Running 9 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 9 tests from SorterTest
> [ RUN  ] SorterTest.DRFSorter
> [   OK ] SorterTest.DRFSorter (1 ms)
> [ RUN  ] SorterTest.WDRFSorter
> [   OK ] SorterTest.WDRFSorter (1 ms)
> [ RUN  ] SorterTest.SplitResourceShares
> [   OK ] SorterTest.SplitResourceShares (0 ms)
> [ RUN  ] SorterTest.UpdateAllocation
> [   OK ] SorterTest.UpdateAllocation (0 ms)
> [ RUN  ] SorterTest.MultipleSlaves
> [   OK ] SorterTest.MultipleSlaves (0 ms)
> [ RUN  ] SorterTest.MultipleSlavesUpdateAllocation
> [   OK ] SorterTest.MultipleSlavesUpdateAllocation (1 ms)
> [ RUN  ] SorterTest.UpdateTotal
> [   OK ] SorterTest.UpdateTotal (0 ms)
> [ RUN  ] SorterTest.MultipleSlavesUpdateTotal
> [   OK ] SorterTest.MultipleSlavesUpdateTotal (0 ms)
> [ RUN  ] SorterTest.RevocableResources
> [   OK ] SorterTest.RevocableResources (0 ms)
> [--] 9 tests from SorterTest (37 ms total)
> 
> [--] Global test environment tear-down
> [==] 9 tests from 1 test case ran. (48 ms total)
> [  PASSED  ] 9 tests.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 48062: Recalculated share when weight was updated.

[Guangya Liu]

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

Review request for mesos, Benjamin Mahler and Klaus Ma.


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


Repository: mesos


Description
---

Recalculated share when weight was updated.


Diffs
-

  src/master/allocator/sorter/drf/sorter.cpp 
65d473a5da0d846214c930c14d333040b2085b13 
  src/tests/sorter_tests.cpp eb207a36c18198588cd8a98b3860a66c2ff7a641 

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


Testing
---

make
make check

[ RUN  ] SorterTest.WDRFSorterUpdateWeight
[   OK ] SorterTest.WDRFSorterUpdateWeight (1 ms)


Thanks,

Guangya Liu

Review Request 48579: Reset "dirty" to false in sort().

[Guangya Liu]

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

Review request for mesos, Benjamin Mahler and Klaus Ma.


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


Repository: mesos


Description
---

The "dirty" was set to "true" when the total resources in cluster was
updated. But in sorter, the "dirty" was never set back as "false" after
re-calculate share for each clients.

This patch reset the "dirty" to "false" in sort(), this can make sure
only one client share was updated when there are allocation changes but
not all clients in the cluster.

This can improve the performance of sorter.


Diffs
-

  src/master/allocator/sorter/drf/sorter.cpp 
65d473a5da0d846214c930c14d333040b2085b13 
  src/tests/sorter_tests.cpp eb207a36c18198588cd8a98b3860a66c2ff7a641 

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


Testing
---

[==] Running 9 tests from 1 test case.
[--] Global test environment set-up.
[--] 9 tests from SorterTest
[ RUN  ] SorterTest.DRFSorter
[   OK ] SorterTest.DRFSorter (1 ms)
[ RUN  ] SorterTest.WDRFSorter
[   OK ] SorterTest.WDRFSorter (1 ms)
[ RUN  ] SorterTest.SplitResourceShares
[   OK ] SorterTest.SplitResourceShares (0 ms)
[ RUN  ] SorterTest.UpdateAllocation
[   OK ] SorterTest.UpdateAllocation (0 ms)
[ RUN  ] SorterTest.MultipleSlaves
[   OK ] SorterTest.MultipleSlaves (0 ms)
[ RUN  ] SorterTest.MultipleSlavesUpdateAllocation
[   OK ] SorterTest.MultipleSlavesUpdateAllocation (1 ms)
[ RUN  ] SorterTest.UpdateTotal
[   OK ] SorterTest.UpdateTotal (0 ms)
[ RUN  ] SorterTest.MultipleSlavesUpdateTotal
[   OK ] SorterTest.MultipleSlavesUpdateTotal (0 ms)
[ RUN  ] SorterTest.RevocableResources
[   OK ] SorterTest.RevocableResources (0 ms)
[--] 9 tests from SorterTest (37 ms total)

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


Thanks,

Guangya Liu

Re: Review Request 48365: Bundled NVML headers for Nvidia GPU support.

[Kevin Klues]

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

(Updated June 11, 2016, 3:37 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Reupload binary data that was missing in the previous revision.


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


Repository: mesos


Description
---

Bundled NVML headers for Nvidia GPU support.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 3622602 
  3rdparty/Makefile.am fb4a37d 
  3rdparty/cmake/Versions.cmake 86c51ed 
  3rdparty/nvml-352.79.tar.gz PRE-CREATION 
  3rdparty/versions.am 7dcd6bf 
  configure.ac e344c56 
  src/Makefile.am b656702 
  support/install-nvidia-gdk.sh fcb075f 

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


Testing
---

GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 48376: Changed semantics for granting access to /dev/nvidiactl, etc.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [48376, 48375, 48374, 48373, 48372, 48371, 48370, 48369, 
48368, 48367, 48366, 48365, 48364, 48363, 48578]

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

Error:
2016-06-11 03:31:29 URL:https://reviews.apache.org/r/48365/diff/raw/ 
[16129/16129] -> "48365.patch" [1]
error: missing binary patch data for '3rdparty/nvml-352.79.tar.gz'
error: binary patch does not apply to '3rdparty/nvml-352.79.tar.gz'
error: 3rdparty/nvml-352.79.tar.gz: patch does not apply

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

- Mesos ReviewBot


On June 11, 2016, 3:05 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48376/
> ---
> 
> (Updated June 11, 2016, 3:05 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-
> https://issues.apache.org/jira/browse/MESOS-
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, access to `/dev/nvidiactl` and `/dev/nvidia-uvm` was
> only granted to / revoked from a container as GPUs were added and
> removed from them. On some level, this makes sense because most jobs
> don't need access to these devices unless they are also using a GPU.
> However, there are cases when access to these files is appropriate,
> even when not making use of a GPU. Running `nvidia-smi` to control the
> global state of the underlying nvidia driver, for example.
> 
> This commit adds `/dev/nvidiactl` and `/dev/nvidia-uvm` to the default
> whitelist of devices to include in every container when the
> `gpu/nvidia` isolator is enabled. This allows a container to run
> standard nvidia driver tools (such as `nvidia-smi`) without failing
> with abnormal errors when no GPUs have been granted to it. As such,
> these tools will now report that no GPUs are installed instead of
> failing abnormally.
> 
> NOTE: Once we allow GPUs to be granted to containers with filesystem
> isolation turned on, other criteria will be used to determine when /
> if to grant access to these control devices.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
> d7557a0c338e8c0e51461b2326600c03f89c2e8b 
> 
> Diff: https://reviews.apache.org/r/48376/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 48364: Removed hard dependence on `libnvidia-ml.so` for Nvidia GPU support.

[Kevin Klues]

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

(Updated June 11, 2016, 3:16 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Updated name of library to `dlopen()` from libnvidia-ml.so --> libnvidia-ml.so.1
Updated name of symbol to load using `dlsym()` from `"nvmlInit"` to 
`"nvmlInit_v2"`.

These changes were made after an external review of the code made by Nividia.


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


Repository: mesos


Description
---

We now use a singleton class called `NvidiaManagementLibrary` that
loads `libnvidia-ml` at runtime once it is initialized. By loading
this library dynamically, `libmesos` no longer has a hard dependence
on it, so it doesn't have to be installed on every machine where mesos
is deployed.

This was a problem previously, whereby the master and agents that
didn't even have GPUs would unnecessarily need to have `libnvidia-ml`
installed on their systems. This library is not easily installable
(it's not bundled in standard apt-get or yum repositories), so this
was a major inconvenience.


Diffs (updated)
-

  configure.ac e344c56e1be5e232ee331c933b8c04c4c2e55d1e 
  src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 
  src/slave/containerizer/mesos/isolators/gpu/nvidia.hpp 
181a2aad97da9ee0f6ffa42cdba9c93dc0077ff7 
  src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp 
d7557a0c338e8c0e51461b2326600c03f89c2e8b 
  src/slave/containerizer/mesos/isolators/gpu/nvml.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/gpu/nvml.cpp PRE-CREATION 

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


Testing
---

GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 48368: Changed major/minor device types for Nvidia GPUs to `unsigned int`.

[Kevin Klues]

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

(Updated June 11, 2016, 3:06 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased for https://reviews.apache.org/r/48578


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


Repository: mesos


Description
---

Previously, the GPU struct specified the type of its `major` and
`minor` fields as `dev_t`, which is actually a concatenation of both
the major and minor device numbers accessible through the `major()`
and `minor()` macros. These macros return an `unsigned int` when
handed a `dev_t`, so it makes sense for these fields to be of that
type instead.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/nvidia.hpp 
181a2aad97da9ee0f6ffa42cdba9c93dc0077ff7 
  src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp 
d7557a0c338e8c0e51461b2326600c03f89c2e8b 

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


Testing
---

GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 48366: Added auto-discovery of GPUs for Nvidia GPU support.

[Kevin Klues]

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

(Updated June 11, 2016, 3:07 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased for https://reviews.apache.org/r/48578


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


Repository: mesos


Description
---

This commit also includes updates to the tests that would start
failing once GPU autodiscovery was turned on. Since GPUs are now a
first class resource, we have to explicilty list them in resource
vectors that attempt to set resources for the whole machine and then
verify that they get the same vector back in an offer.


Diffs (updated)
-

  src/slave/containerizer/containerizer.cpp 
faa0c789dda8a6f36fdb6217b0bae270b6b8f2e2 
  src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp 
d7557a0c338e8c0e51461b2326600c03f89c2e8b 
  src/tests/master_allocator_tests.cpp 34e3add78d0dec8fb55d7b35710694bea3c58d6f 
  src/tests/master_quota_tests.cpp 62aaafe7b11a7df1ca7bc1f2f29af4cb626fc35d 
  src/tests/master_tests.cpp 0d64e353f0682017953b524a5f5246d76f52fa2e 
  src/tests/mesos.hpp e9361a65eb31cced99e9ed32fd18a65d1bda26e3 
  src/tests/persistent_volume_endpoints_tests.cpp 
70b7626c8e87d1de59b13f9fd88fe9abb5f6b6a5 
  src/tests/slave_tests.cpp 7ba03e8c06b8aaac1e57124c257660acd8c1d78a 

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


Testing
---

GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 48367: Added test to verify that GPU auto-discovery works.

[Kevin Klues]

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

(Updated June 11, 2016, 3:06 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased for https://reviews.apache.org/r/48578


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


Repository: mesos


Description
---

Added test to verify that GPU auto-discovery works.


Diffs (updated)
-

  src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
65ffbafffeb5dac372b5770d2a1560a942a822e2 

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


Testing
---

GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 48364: Removed hard dependence on `libnvidia-ml.so` for Nvidia GPU support.

[Kevin Klues]

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

(Updated June 11, 2016, 3:07 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased for https://reviews.apache.org/r/48578


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


Repository: mesos


Description
---

We now use a singleton class called `NvidiaManagementLibrary` that
loads `libnvidia-ml` at runtime once it is initialized. By loading
this library dynamically, `libmesos` no longer has a hard dependence
on it, so it doesn't have to be installed on every machine where mesos
is deployed.

This was a problem previously, whereby the master and agents that
didn't even have GPUs would unnecessarily need to have `libnvidia-ml`
installed on their systems. This library is not easily installable
(it's not bundled in standard apt-get or yum repositories), so this
was a major inconvenience.


Diffs (updated)
-

  configure.ac e344c56e1be5e232ee331c933b8c04c4c2e55d1e 
  src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 
  src/slave/containerizer/mesos/isolators/gpu/nvidia.hpp 
181a2aad97da9ee0f6ffa42cdba9c93dc0077ff7 
  src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp 
d7557a0c338e8c0e51461b2326600c03f89c2e8b 
  src/slave/containerizer/mesos/isolators/gpu/nvml.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/gpu/nvml.cpp PRE-CREATION 

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


Testing
---

GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 48369: Fixed comment in Nvidia GPU device isolator.

[Kevin Klues]

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

(Updated June 11, 2016, 3:06 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased for https://reviews.apache.org/r/48578


Repository: mesos


Description
---

Fixed comment in Nvidia GPU device isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp 
d7557a0c338e8c0e51461b2326600c03f89c2e8b 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 48372: Updated `Containerizer::resources()` to use the `NvidiaGpuAllocator`.

[Kevin Klues]

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

(Updated June 11, 2016, 3:05 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased for https://reviews.apache.org/r/48578


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


Repository: mesos


Description
---

Updated `Containerizer::resources()` to use the `NvidiaGpuAllocator`.


Diffs (updated)
-

  src/slave/containerizer/containerizer.cpp 
faa0c789dda8a6f36fdb6217b0bae270b6b8f2e2 

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


Testing
---

GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 48365: Bundled NVML headers for Nvidia GPU support.

[Kevin Klues]

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

(Updated June 11, 2016, 3:04 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased for https://reviews.apache.org/r/48578


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


Repository: mesos


Description
---

Bundled NVML headers for Nvidia GPU support.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 3622602953b31f2f6d0dc5f5cb32fb611e8c3a61 
  3rdparty/Makefile.am fb4a37d50e751703b4ccddb0e004b58560707067 
  3rdparty/cmake/Versions.cmake 86c51edb3aa2daf6451459aaf18278f09b91b000 
  3rdparty/nvml-352.79.tar.gz PRE-CREATION 
  3rdparty/versions.am 7dcd6bf914de3213755ec9d4e701a190750424e9 
  configure.ac e344c56e1be5e232ee331c933b8c04c4c2e55d1e 
  src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 
  support/install-nvidia-gdk.sh fcb075f75ef5dade6a05cfafcfb6b3d516764ae6 

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


Testing
---

GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 48374: Added class to share Nvidia-specific components between containerizers.

[Kevin Klues]

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

(Updated June 11, 2016, 3:05 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased for https://reviews.apache.org/r/48578


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


Repository: mesos


Description
---

Previously, the `NvidiaGpuAllocator` component was created directly
inside the `NvidiaGpuIsolatorProcess` even though it was designed to
be used by multiple containerizers at the same time. To allow for
simultaneous access, we created a new `NvidiaComponents` class to hold
a reference to a shared `NvidiaGpuAllocator` instance and pass it to
each containerizer as it is created. This component can easily be
extended to include more cross-containerizer components later on.

We create the `NvidiaComponents` instance inside
`Containerizer::create()` and pass it to both the docker containerizer
and the mesos containerizer when they are created.  The docker
containerizer currently doesn't do anything with this information, but
its interface has been updated to accomodate it. The mesos
containerizer has been updated to pass this all the way down to the
`NvidiaGpuIsolatorProcess` and exploit it.


Diffs (updated)
-

  src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 
  src/slave/containerizer/containerizer.cpp 
faa0c789dda8a6f36fdb6217b0bae270b6b8f2e2 
  src/slave/containerizer/docker.hpp 311dca23ac17fb533866aba1de2b81d750bbe6df 
  src/slave/containerizer/docker.cpp 1af5b451922e1eeb0af025f29c53a85ab9deec3b 
  src/slave/containerizer/mesos/containerizer.hpp 
a1a00020668f6da8d611f26e5637afffc87d09ba 
  src/slave/containerizer/mesos/containerizer.cpp 
92c9898fb41ca0ffbda05e53b595b05c96f4f596 
  src/slave/containerizer/mesos/isolators/gpu/components.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/gpu/nvidia.hpp 
181a2aad97da9ee0f6ffa42cdba9c93dc0077ff7 
  src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp 
d7557a0c338e8c0e51461b2326600c03f89c2e8b 

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


Testing
---

GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 48373: Integrated the `NvidiaGpuAllocator` into the `NvidiaGpuIsolator`.

[Kevin Klues]

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

(Updated June 11, 2016, 3:05 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased for https://reviews.apache.org/r/48578


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


Repository: mesos


Description
---

All logic to do GPU allocation is now conducted asynchronously through
the `NvidiaGpuAllocator` component.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/nvidia.hpp 
181a2aad97da9ee0f6ffa42cdba9c93dc0077ff7 
  src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp 
d7557a0c338e8c0e51461b2326600c03f89c2e8b 

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


Testing
---

GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 48376: Changed semantics for granting access to /dev/nvidiactl, etc.

[Kevin Klues]

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

(Updated June 11, 2016, 3:05 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased for https://reviews.apache.org/r/48578


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


Repository: mesos


Description
---

Previously, access to `/dev/nvidiactl` and `/dev/nvidia-uvm` was
only granted to / revoked from a container as GPUs were added and
removed from them. On some level, this makes sense because most jobs
don't need access to these devices unless they are also using a GPU.
However, there are cases when access to these files is appropriate,
even when not making use of a GPU. Running `nvidia-smi` to control the
global state of the underlying nvidia driver, for example.

This commit adds `/dev/nvidiactl` and `/dev/nvidia-uvm` to the default
whitelist of devices to include in every container when the
`gpu/nvidia` isolator is enabled. This allows a container to run
standard nvidia driver tools (such as `nvidia-smi`) without failing
with abnormal errors when no GPUs have been granted to it. As such,
these tools will now report that no GPUs are installed instead of
failing abnormally.

NOTE: Once we allow GPUs to be granted to containers with filesystem
isolation turned on, other criteria will be used to determine when /
if to grant access to these control devices.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
d7557a0c338e8c0e51461b2326600c03f89c2e8b 

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


Testing
---

GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 48371: Added `NvidiaGpuAllocator` component.

[Kevin Klues]

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

(Updated June 11, 2016, 3:04 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased for https://reviews.apache.org/r/48578


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


Repository: mesos


Description
---

This component is designed to serve as a centralized component for
allocating / deallocating Nvidia GPUs. The goal is to share an
instance of this across both the mesos containerizer and the docker
containerizer so they can allocate GPUs from the same shared pool.

It is overloaded to also do resource enumeration of GPUs based on
the agent flags. This keeps all code for enumerating GPUs and the
resources they represent in a single centralized location.


Diffs (updated)
-

  src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 
  src/slave/containerizer/mesos/isolators/gpu/allocator.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp PRE-CREATION 
  src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
65ffbafffeb5dac372b5770d2a1560a942a822e2 

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


Testing
---

GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 48363: Moved 'isolators/cgroups/devices/gpus' to 'isolators/gpu'.

[Kevin Klues]

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

(Updated June 11, 2016, 3:04 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased for https://reviews.apache.org/r/48578


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


Repository: mesos


Description
---

Moved 'isolators/cgroups/devices/gpus' to 'isolators/gpu'.


Diffs (updated)
-

  src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 
  src/slave/containerizer/mesos/containerizer.cpp 
92c9898fb41ca0ffbda05e53b595b05c96f4f596 
  src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp 
181a2aad97da9ee0f6ffa42cdba9c93dc0077ff7 
  src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp 
d7557a0c338e8c0e51461b2326600c03f89c2e8b 

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


Testing
---

GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests


Thanks,

Kevin Klues

Review Request 48578: Fixed bug with double destruction of cgroups devices subsystem.

[Kevin Klues]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

In a previous commit, the `cgroups/devices` isolator was abstracted
out of the Nvidia GPU isolator. However, the code for destroying the
devices susbsystem wasn't removed from the GPU isolator, causing it to
"double destroy" the subsystem upon cleanup. This commit removed this
redundant code.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp 
181a2aad97da9ee0f6ffa42cdba9c93dc0077ff7 
  src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp 
d7557a0c338e8c0e51461b2326600c03f89c2e8b 

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


Testing
---

make -j
GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 48574: Added assertion for a write in CoordinatorTest.Elect.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48571, 48572, 48573, 48574]

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 June 11, 2016, 12:46 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48574/
> ---
> 
> (Updated June 11, 2016, 12:46 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Artem Harutyunyan, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The extra assertions check that a quorum of replicas have written the
> action after leader election.
> 
> 
> Diffs
> -
> 
>   src/tests/log_tests.cpp 06bf5b8eb0336e015285ef749f50b8ad5be9aa62 
> 
> Diff: https://reviews.apache.org/r/48574/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 48572: Updated whitespace style in log replica tests.

[Joseph Wu]

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

Review request for mesos, Benjamin Mahler, Artem Harutyunyan, and Jie Yu.


Repository: mesos


Description
---

Updates the old template braces `> >` to `>>`.
And adds a missing space on line 458.


Diffs
-

  src/tests/log_tests.cpp 06bf5b8eb0336e015285ef749f50b8ad5be9aa62 

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


Testing
---

make check


Thanks,

Joseph Wu

Review Request 48574: Added assertion for a write in CoordinatorTest.Elect.

[Joseph Wu]

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

Review request for mesos, Benjamin Mahler, Artem Harutyunyan, and Jie Yu.


Repository: mesos


Description
---

The extra assertions check that a quorum of replicas have written the
action after leader election.


Diffs
-

  src/tests/log_tests.cpp 06bf5b8eb0336e015285ef749f50b8ad5be9aa62 

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


Testing
---

make check


Thanks,

Joseph Wu

Review Request 48573: Added expectations for response types in log replica tests.

[Joseph Wu]

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

Review request for mesos, Benjamin Mahler, Artem Harutyunyan, and Jie Yu.


Repository: mesos


Description
---

The `okay` field in `PromiseResponse` and `WriteResponse` are 
deprecated. This adds a the analogous check for the `type` field that 
replaces the `okay` field.


Diffs
-

  src/tests/log_tests.cpp 06bf5b8eb0336e015285ef749f50b8ad5be9aa62 

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


Testing
---

make check


Thanks,

Joseph Wu

Review Request 48571: Added assertion for initialization success to the log replica tests.

[Joseph Wu]

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

Review request for mesos, Benjamin Mahler, Artem Harutyunyan, and Jie Yu.


Repository: mesos


Description
---

The log replica tests use a tool to set an `EMPTY` replica to `VOTING`.
This tool has a few potential failure cases, and the tool's result
should be checked in tests that expect `VOTING` replicas.


Diffs
-

  src/tests/log_tests.cpp 06bf5b8eb0336e015285ef749f50b8ad5be9aa62 

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 47743: Supported private registry per-container in unified containerizer.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47743, 47742, 45952, 48570, 45951, 47808, 47807, 47806, 
45950, 45949]

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

Error:
2016-06-11 00:45:02 URL:https://reviews.apache.org/r/47807/diff/raw/ 
[2067/2067] -> "47807.patch" [1]
error: patch failed: include/mesos/docker/spec.hpp:76
error: include/mesos/docker/spec.hpp: patch does not apply

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

- Mesos ReviewBot


On June 11, 2016, 12:42 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47743/
> ---
> 
> (Updated June 11, 2016, 12:42 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported private registry per-container in unified containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6545a6d29cb91d6cf8919c7796c283084178e499 
>   src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 
>   src/uri/schemes/docker.hpp 68dbf3ffb20c59c7af9558f5ed0bb5706d8aa057 
> 
> Diff: https://reviews.apache.org/r/47743/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Tested by mesos-execute.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47743: Supported private registry per-container in unified containerizer.

[Gilbert Song]

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

(Updated June 10, 2016, 5:42 p.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Repository: mesos


Description
---

Supported private registry per-container in unified containerizer.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
6545a6d29cb91d6cf8919c7796c283084178e499 
  src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 
  src/uri/schemes/docker.hpp 68dbf3ffb20c59c7af9558f5ed0bb5706d8aa057 

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


Testing
---

make check

Tested by mesos-execute.


Thanks,

Gilbert Song

Review Request 48570: Fixed docker fetcher plugin process indentation.

[Gilbert Song]

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

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


Repository: mesos


Description
---

Fixed docker fetcher plugin process indentation.


Diffs
-

  src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 47742: Modified docker puller interface to pass credential.

[Gilbert Song]

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

(Updated June 10, 2016, 5:42 p.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Repository: mesos


Description
---

Modified docker puller interface to pass credential.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
5f5aaa3688ba11463942f0e08716d6650d7762c3 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
4be26faf0d46ee29fb4169bfe69264c57a6b9fce 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
bbd6005317bed3fff3d86e2527ca3ead839d49e3 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
6545a6d29cb91d6cf8919c7796c283084178e499 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
cd5849bb9cdd12f2240885a0eae90569d2a9502e 
  src/tests/containerizer/provisioner_docker_tests.cpp 
ffe3382da8b1199e257a72ca9034fbccec9494b1 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 47808: Added test for parsing docker config.

[Gilbert Song]

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

(Updated June 10, 2016, 5:42 p.m.)


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


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


Repository: mesos


Description
---

Added test for parsing docker config.


Diffs (updated)
-

  src/tests/containerizer/docker_spec_tests.cpp 
944a559bb3a8ef602b4d8d52773afdb8bf8e5bf6 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 45952: Implemented support for passing agent default docker config.

[Gilbert Song]

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

(Updated June 10, 2016, 5:42 p.m.)


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


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


Repository: mesos


Description
---

Implemented support for passing agent default docker config.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
cd5849bb9cdd12f2240885a0eae90569d2a9502e 
  src/uri/fetchers/docker.hpp c855a2b55a07bb398f7547b44a85b8ba2d2b2ec3 
  src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 

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


Testing
---

make check

Tested with mesos-execute, using a private repo from docker hub. Both cases are 
tested:
1. --docker_registry=private_registry
2. private_registry/repo


Thanks,

Gilbert Song

Re: Review Request 47806: Add docker config auth protobuf to docker spec.

[Gilbert Song]

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

(Updated June 10, 2016, 5:42 p.m.)


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


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


Repository: mesos


Description
---

Add docker config auth protobuf to docker spec.


Diffs (updated)
-

  include/mesos/docker/spec.proto 4f2ff2d7d88ceca2f25864b60a04ff5ec05317ff 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 45951: Implemented http basic auth to get docker auth token.

[Gilbert Song]

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

(Updated June 10, 2016, 5:42 p.m.)


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


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


Repository: mesos


Description
---

Implemented http basic auth to get docker auth token.


Diffs (updated)
-

  src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 

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


Testing
---

make check

Tested with mesos-execute, using a private repo from docker hub.


Thanks,

Gilbert Song

Re: Review Request 47807: Implemented parsing a docker config to a hashmap.

[Gilbert Song]

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

(Updated June 10, 2016, 5:42 p.m.)


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


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


Repository: mesos


Description
---

Implemented parsing a docker config to a hashmap.


Diffs (updated)
-

  include/mesos/docker/spec.hpp b37c7731590ec144df720d7b28eeed5db76f750e 
  src/docker/spec.cpp 7270d182b8c3a5d5c34cbda2bd736ba489089720 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 47668: Obtained uid/gids before changing filesystem root.

[Jie Yu]

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




src/launcher/executor.cpp (line 474)


Can you add a NOTE here saying that we need to put change user, chdir 
logics in command executor because these depend on the new root filesystem. If  
the command task does not change root fiesystem, these will be handled in the 
containerizer.



src/launcher/executor.cpp (lines 485 - 492)


Let's simply assume that container user does not exist. Please add a TODO 
here to handle the case where contianer user exists.



src/launcher/executor.cpp (lines 549 - 570)


I would move that up right below fs::enter



src/slave/containerizer/mesos/launch.cpp (lines 229 - 236)


Let's remove the logic here simply assume that container user does not 
exist. Add a TODO here as I suggested above.


- Jie Yu


On May 26, 2016, 8:55 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47668/
> ---
> 
> (Updated May 26, 2016, 8:55 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Obtained uid/gids before changing filesystem root.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp e4c3b75b647b54ffecedfdfa34fb3925f686c0c7 
>   src/slave/containerizer/mesos/launch.cpp 
> e22106b014c871e2184a15c2ab154a0674874e47 
> 
> Diff: https://reviews.apache.org/r/47668/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47668: Obtained uid/gids before changing filesystem root.

[Jie Yu]

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



Have you tested switch user on mac?

- Jie Yu


On May 26, 2016, 8:55 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47668/
> ---
> 
> (Updated May 26, 2016, 8:55 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Obtained uid/gids before changing filesystem root.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp e4c3b75b647b54ffecedfdfa34fb3925f686c0c7 
>   src/slave/containerizer/mesos/launch.cpp 
> e22106b014c871e2184a15c2ab154a0674874e47 
> 
> Diff: https://reviews.apache.org/r/47668/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47374: Separated mesos test helpers into a separate library.

[Joseph Wu]

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

(Updated June 10, 2016, 5:03 p.m.)


Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, Kapil Arya, Jan 
Schlicht, and Till Toenshoff.


Changes
---

Removed "experiment" label after some builds + writing module tests in a 
separate repository.
Updated description with some insights gained from the above.


Summary (updated)
-

Separated mesos test helpers into a separate library.


Repository: mesos


Description (updated)
---

This gives external projects easier access to the test helpers used in
mesos tests.  

For example, a module writer may want to write a test like 
`src/tests/oversubscription_tests.cpp`.  To build and link against 
this library, the module writer would mimic the build flags for tests:
```
# Main test file is taken directly from Mesos.
my_module_tests_SOURCES = \
  $(MESOS)/src/tests/main.cpp

my_module_tests_CPPFLAGS = \
  -I$(GMOCK)/include   \
  -I$(GTEST)/include   \
  -I$(MESOS)/include/mesos \
  -I$(ZOOKEEPER)/include   \
  -I$(ZOOKEEPER)/generated \
  $(AM_CPPFLAGS)
  
my_module_tests_LDADD = \
  $(MESOS)/3rdparty/.libs/libgmock.la \
  $(MESOS)/src/.libs/libmesos.la  \
  $(MESOS)/src/.libs/libmesos_tests.la
```


Diffs
-

  src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 

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


Testing (updated)
---

make check on OSX, CentOS 7, Ubuntu 14


Thanks,

Joseph Wu

Re: Review Request 47667: Added stout functions to get and set supplementary groud ids.

[Jie Yu]

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




3rdparty/stout/include/stout/os/posix/su.hpp (line 177)


Do you need to follow the same to get maxgroups here?



3rdparty/stout/include/stout/os/posix/su.hpp (lines 187 - 189)


You can do `return std::vector(gids, gids + ngroups);



3rdparty/stout/include/stout/os/posix/su.hpp (lines 205 - 209)


I would say that we cannot simply call 'setgroups' here because it only 
updates the in kernel cache, but not the ones in opendirectoryd. Darwin kernel 
caches part of the groups in kernel, and the rest in opendirectoryd.



3rdparty/stout/include/stout/os/posix/su.hpp (line 217)


indentation


- Jie Yu


On May 26, 2016, 8:55 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47667/
> ---
> 
> (Updated May 26, 2016, 8:55 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added stout functions to get and set supplementary groud ids.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/su.hpp 
> 43e91d6baca9b5e0bb7d6661c4d8002b07d22452 
> 
> Diff: https://reviews.apache.org/r/47667/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47666: Added stout functions to set uid and gid.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 26, 2016, 8:55 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47666/
> ---
> 
> (Updated May 26, 2016, 8:55 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added stout functions to set uid and gid.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/su.hpp 
> 43e91d6baca9b5e0bb7d6661c4d8002b07d22452 
> 
> Diff: https://reviews.apache.org/r/47666/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47665: Passed image user to mesos containerizer launch.

[Jie Yu]

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



Container user is not supported for now. No need to do that change for now. 
Simply return Failure in docker runtime isolaotr.

- Jie Yu


On May 26, 2016, 8:55 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47665/
> ---
> 
> (Updated May 26, 2016, 8:55 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Passed image user to mesos containerizer launch.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 75e5a32a3e70ec60a6800e21a621673184ea0956 
>   src/slave/containerizer/mesos/launch.hpp 
> c716e0396736d1f2f60ec31540f12f4f7597d081 
>   src/slave/containerizer/mesos/launch.cpp 
> e22106b014c871e2184a15c2ab154a0674874e47 
> 
> Diff: https://reviews.apache.org/r/47665/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48566: Fixed continuation logic for sandbox authorization.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48563, 48566]

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 June 10, 2016, 9:25 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48566/
> ---
> 
> (Updated June 10, 2016, 9:25 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-5587
> https://issues.apache.org/jira/browse/MESOS-5587
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the continuation followed via `.then([=]`,
> which potentially executes the continuation on a different
> process. This patch fixes this behavior (it should
> run on the same process).
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp 55c310af149cb03abfa1ffbb6477dc1ac5dc14be 
> 
> Diff: https://reviews.apache.org/r/48566/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 48566: Fixed continuation logic for sandbox authorization.

[Joseph Wu]

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


Fix it, then Ship it!




LGTM!


src/files/files.cpp (lines 723 - 737)


It would be nice to add, in the review description, that you are fixing the 
potential race between this section of `::resolve` and  `::detach`


- Joseph Wu


On June 10, 2016, 2:25 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48566/
> ---
> 
> (Updated June 10, 2016, 2:25 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-5587
> https://issues.apache.org/jira/browse/MESOS-5587
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the continuation followed via `.then([=]`,
> which potentially executes the continuation on a different
> process. This patch fixes this behavior (it should
> run on the same process).
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp 55c310af149cb03abfa1ffbb6477dc1ac5dc14be 
> 
> Diff: https://reviews.apache.org/r/48566/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 48563: Fixed continuation logic in endpoint authorization.

[Joseph Wu]

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


Fix it, then Ship it!




Can you include the reason this segfaults in the description?

i.e. because `jsonify` is not running in the master actor, it may try to read a 
field as the field is removed.


src/master/http.cpp (lines 1869 - 1871)


You may want to update the spacing here.  Spacing elsewhere looks fine.


- Joseph Wu


On June 10, 2016, 1:41 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48563/
> ---
> 
> (Updated June 10, 2016, 1:41 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-5587
> https://issues.apache.org/jira/browse/MESOS-5587
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the continuation followed via `.then([=]`,
> which potentially executes the continuation on a differen
> process. This patch fixes this behavior (it should
> run on the master process).
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 4b2d1386e1ecb447b597a432f9df9adaa5c3aa37 
> 
> Diff: https://reviews.apache.org/r/48563/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 48566: Fixed continuation logic for sandbox authorization.

[Joerg Schad]

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

(Updated June 10, 2016, 9:25 p.m.)


Review request for mesos, Adam B and Alexander Rojas.


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


Repository: mesos


Description (updated)
---

Previously the continuation followed via `.then([=]`,
which potentially executes the continuation on a different
process. This patch fixes this behavior (it should
run on the same process).


Diffs
-

  src/files/files.cpp 55c310af149cb03abfa1ffbb6477dc1ac5dc14be 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 48566: Fixed continuation logic for sandbox authorization.

[Joerg Schad]

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

(Updated June 10, 2016, 9:22 p.m.)


Review request for mesos, Adam B and Alexander Rojas.


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


Repository: mesos


Description
---

Previously the continuation followed via `.then([=]`,
which potentially executes the continuation on a differen
process. This patch fixes this behavior (it should
run on the same process).


Diffs
-

  src/files/files.cpp 55c310af149cb03abfa1ffbb6477dc1ac5dc14be 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 48563: Fixed continuation logic in endpoint authorization.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48563]

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 June 10, 2016, 8:41 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48563/
> ---
> 
> (Updated June 10, 2016, 8:41 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-5587
> https://issues.apache.org/jira/browse/MESOS-5587
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the continuation followed via `.then([=]`,
> which potentially executes the continuation on a differen
> process. This patch fixes this behavior (it should
> run on the master process).
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 4b2d1386e1ecb447b597a432f9df9adaa5c3aa37 
> 
> Diff: https://reviews.apache.org/r/48563/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 48563: Fixed continuation logic in endpoint authorization.

[Joerg Schad]

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

Review request for mesos and Adam B.


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


Repository: mesos


Description
---

Previously the continuation followed via `.then([=]`,
which potentially executes the continuation on a differen
process. This patch fixes this behavior (it should
run on the master process).


Diffs
-

  src/master/http.cpp 4b2d1386e1ecb447b597a432f9df9adaa5c3aa37 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 47661: Improved the mesos containerizer windows related logic.

[Timothy Chen]

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


Ship it!




Ship It!

- Timothy Chen


On May 26, 2016, 8:55 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47661/
> ---
> 
> (Updated May 26, 2016, 8:55 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved the mesos containerizer windows related logic.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 75e5a32a3e70ec60a6800e21a621673184ea0956 
> 
> Diff: https://reviews.apache.org/r/47661/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48094: Implemented GET_ROLES Call in v1 master API.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48094]

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 June 10, 2016, 7:13 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48094/
> ---
> 
> (Updated June 10, 2016, 7:13 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5494
> https://issues.apache.org/jira/browse/MESOS-5494
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented GET_ROLES Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 
>   include/mesos/v1/mesos.proto 39967fa09d2774d564f3df28277edea8ebcfb50d 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48094/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> 
> sudo GTEST_FILTER="*MasterAPITest.*" make -j2 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 48094: Implemented GET_ROLES Call in v1 master API.

[Abhishek Dasgupta]

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

(Updated June 10, 2016, 7:13 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Implemented GET_ROLES Call in v1 master API.


Diffs
-

  include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 
  include/mesos/v1/mesos.proto 39967fa09d2774d564f3df28277edea8ebcfb50d 
  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing (updated)
---

On Ubuntu 16.04:

sudo GTEST_FILTER="*MasterAPITest.*" make -j2 check


Thanks,

Abhishek Dasgupta

Re: Review Request 48094: Implemented GET_ROLES Call in v1 master API.

[Abhishek Dasgupta]

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

(Updated June 10, 2016, 6:45 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Implemented GET_ROLES Call in v1 master API.


Diffs (updated)
-

  include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 
  include/mesos/v1/mesos.proto 39967fa09d2774d564f3df28277edea8ebcfb50d 
  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---

On Ubuntu 16.04:

sudo GTEST_FILTER="MasterAPITest.*" make -j2 check


Thanks,

Abhishek Dasgupta

Re: Review Request 48286: Implemented STOP_MAINTENANCE Call in v1 master API.

[haosdent huang]

> On June 9, 2016, 10:56 p.m., Vinod Kone wrote:
> > No tests for this and the previous review?

Hi, @vinodkone. `START_MAINTENANCE` and `STOP_MAINTENANCE` are still in 
progress because I have some remain problems about api_tests.cpp as discussions 
in [48259](https://reviews.apache.org/r/48259/) and 
[48260](https://reviews.apache.org/r/48260/).
I would post the test cases with below patches fixes as well. :)


- haosdent


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


On June 9, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48286/
> ---
> 
> (Updated June 9, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5507
> https://issues.apache.org/jira/browse/MESOS-5507
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented STOP_MAINTENANCE Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48286/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 45605: Introduced HTB class.

[Jie Yu]

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



Sorry about the long wait. I am now volunteer for shepherding this work. Would 
still love to get this committed. So I re-opened the review.


src/linux/routing/queueing/discipline.hpp (line 52)


This class is already under `namespace queueing {`. So adding another 
Queueing to class name does not make sense.

I would create another file  `src/linux/routing/queueing/class.hpp` and 
move this definition there:
```
namespace queueing {
template 
struct Class {...};
}
```



src/linux/routing/queueing/htb.hpp (line 38)


Since we not have both `class` and `qdisc`, `exists` become ambiguous. I 
would rename the exiting funcitons to:
```
existsDiscipline
createDiscipline
removeDiscipline
statisticsDiscipline

createClass
removeClass
...
```



src/linux/routing/queueing/htb.cpp (lines 41 - 50)


I think we need two Config now: one for Discipline and one for Class.

Looking at 
https://www.infradead.org/~tgr/libnl/doc/api/group__qdisc__htb.html, the 
configuration for htb qdisc is different from configuration for htb class.



src/linux/routing/queueing/htb.cpp (line 64)


See my comments below:

```
template <>
Try encode(...);

template <>
Result decode(..);
```



src/linux/routing/queueing/htb.cpp (line 92)


Given that you have two Config: `DisciplineConfig` and `ClassConfig`, you 
can still use `encode` here:
```
template <>
Try encode(...);

template <>
Result decode(...);
```



src/linux/routing/queueing/internal.hpp (lines 335 - 350)


Can you move these two forward declarations up right below `Result 
decode(const Netlink& qdisc);`. Per my comments above, you 
can rename it to be encode and decode.



src/linux/routing/queueing/internal.hpp (lines 356 - 359)


Can you move this up right below encodeDiscipline. Those are helpers for 
{en}decoding.


- Jie Yu


On April 1, 2016, 9:50 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45605/
> ---
> 
> (Updated April 1, 2016, 9:50 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Bugs: mesos-4749
> https://issues.apache.org/jira/browse/mesos-4749
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced HTB class API, prepare for per-container bandwidth limit.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/internal.hpp 8f68119819f7c79ece1a13ac1894b1802ddc8e19 
>   src/linux/routing/queueing/discipline.hpp 
> 54d6b214ef6a38fd8279f6d01e6f4e3ccfddf634 
>   src/linux/routing/queueing/htb.hpp 857646190d21387f98832f5094128505a52a0776 
>   src/linux/routing/queueing/htb.cpp faadf32bd48cc6bf968b1229789903c0d01fd75c 
>   src/linux/routing/queueing/internal.hpp 
> 768ed325f9b259e150779eb3ad74f4e5d4bcc7a2 
> 
> Diff: https://reviews.apache.org/r/45605/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 45605: Introduced HTB class.

[Cong Wang]

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

(Updated June 10, 2016, 5:57 p.m.)


Review request for mesos, Ian Downes and Jie Yu.


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


Repository: mesos


Description
---

Introduced HTB class API, prepare for per-container bandwidth limit.


Diffs
-

  src/linux/routing/internal.hpp 8f68119819f7c79ece1a13ac1894b1802ddc8e19 
  src/linux/routing/queueing/discipline.hpp 
54d6b214ef6a38fd8279f6d01e6f4e3ccfddf634 
  src/linux/routing/queueing/htb.hpp 857646190d21387f98832f5094128505a52a0776 
  src/linux/routing/queueing/htb.cpp faadf32bd48cc6bf968b1229789903c0d01fd75c 
  src/linux/routing/queueing/internal.hpp 
768ed325f9b259e150779eb3ad74f4e5d4bcc7a2 

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


Testing
---

make && make check


Thanks,

Cong Wang

Re: Review Request 48257: Implemented GET_MAINTENANCE_SCHEDULE Call in v1 master API.

[haosdent huang]

> On June 9, 2016, 9:09 p.m., Joseph Wu wrote:
> > src/master/http.cpp, line 662
> > 
> >
> > Here it would make sense to use the `.then(serializer)`.

As we discuss in http://search-hadoop.com/m/0Vlr6Uz9otVwkdv we prefer to use 
`Future`, so let me drop this.


- haosdent


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


On June 9, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48257/
> ---
> 
> (Updated June 9, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5504
> https://issues.apache.org/jira/browse/MESOS-5504
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented GET_MAINTENANCE_SCHEDULE Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48257/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48552: Added the missed bracket.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 10, 2016, 1:24 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48552/
> ---
> 
> (Updated June 10, 2016, 1:24 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the missed bracket.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 106ff35320f37b2e75ed60381de1406459e6d515 
> 
> Diff: https://reviews.apache.org/r/48552/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 48530: Moved working groups from wiki to page.

[Jie Yu]

> On June 10, 2016, 4:25 p.m., Jie Yu wrote:
> > In fact, for working groups, I would prefer adding that to the docs so that 
> > people can find/read it on github. It is also constantly changing, updating 
> > it in docs sounds easier than updating the website. What do you think?
> 
> Tomasz Janiszewski wrote:
> So this will end the same as [powered by 
> Mesos](http://mesos.apache.org/documentation/latest/powered-by-mesos/) which 
> has noting to do with latest documentation but only with community. I think 
> real problem is site updating. Using docs for anything that change often is 
> not a solution.
> 
> haosdent huang wrote:
> In my opinion, if the target of document are users, I prefer to host it 
> in project website. If the target of document are devlopers(framework 
> developers, contributors, committer and etc), I perfer to host it in docs.

+1 on what haosdent said.

I also looked at k8s
https://github.com/kubernetes/kubernetes/tree/master/docs

They also put design docs, design proposals, roadmap (community related) stuff 
in docs.


- Jie


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


On June 10, 2016, 1:53 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48530/
> ---
> 
> (Updated June 10, 2016, 1:53 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5591
> https://issues.apache.org/jira/browse/MESOS-5591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change was discussed at
> http://www.mail-archive.com/dev@mesos.apache.org/msg35506.html
> 
> 
> Diffs
> -
> 
>   site/data/working_groups.yaml PRE-CREATION 
>   site/source/working-groups.html.erb PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48530/diff/
> 
> 
> Testing
> ---
> 
> Page could be viewed at 
> http://janisz.github.io/mesos/site/publish/working-groups/
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 48438: Implement GET_AGENTS Call in v1 master API.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On June 10, 2016, 1:16 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48438/
> ---
> 
> (Updated June 10, 2016, 1:16 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: mesos-5491
> https://issues.apache.org/jira/browse/mesos-5491
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement the basic getAgents method for v1 operator master API.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 
>   include/mesos/v1/mesos.proto 39967fa09d2774d564f3df28277edea8ebcfb50d 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48438/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 48530: Moved working groups from wiki to page.

[haosdent huang]

> On June 10, 2016, 4:25 p.m., Jie Yu wrote:
> > In fact, for working groups, I would prefer adding that to the docs so that 
> > people can find/read it on github. It is also constantly changing, updating 
> > it in docs sounds easier than updating the website. What do you think?
> 
> Tomasz Janiszewski wrote:
> So this will end the same as [powered by 
> Mesos](http://mesos.apache.org/documentation/latest/powered-by-mesos/) which 
> has noting to do with latest documentation but only with community. I think 
> real problem is site updating. Using docs for anything that change often is 
> not a solution.

In my opinion, if the target of document are users, I prefer to host it in 
project website. If the target of document are devlopers(framework developers, 
contributors, committer and etc), I perfer to host it in docs.


- haosdent


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


On June 10, 2016, 1:53 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48530/
> ---
> 
> (Updated June 10, 2016, 1:53 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5591
> https://issues.apache.org/jira/browse/MESOS-5591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change was discussed at
> http://www.mail-archive.com/dev@mesos.apache.org/msg35506.html
> 
> 
> Diffs
> -
> 
>   site/data/working_groups.yaml PRE-CREATION 
>   site/source/working-groups.html.erb PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48530/diff/
> 
> 
> Testing
> ---
> 
> Page could be viewed at 
> http://janisz.github.io/mesos/site/publish/working-groups/
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 48530: Moved working groups from wiki to page.

[Tomasz Janiszewski]

> On Cze 10, 2016, 4:25 po południu, Jie Yu wrote:
> > In fact, for working groups, I would prefer adding that to the docs so that 
> > people can find/read it on github. It is also constantly changing, updating 
> > it in docs sounds easier than updating the website. What do you think?

So this will end the same as [powered by 
Mesos](http://mesos.apache.org/documentation/latest/powered-by-mesos/) which 
has noting to do with latest documentation but only with community. I think 
real problem is site updating. Using docs for anything that change often is not 
a solution.


- Tomasz


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


On Cze 10, 2016, 1:53 rano, Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48530/
> ---
> 
> (Updated Cze 10, 2016, 1:53 rano)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5591
> https://issues.apache.org/jira/browse/MESOS-5591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change was discussed at
> http://www.mail-archive.com/dev@mesos.apache.org/msg35506.html
> 
> 
> Diffs
> -
> 
>   site/data/working_groups.yaml PRE-CREATION 
>   site/source/working-groups.html.erb PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48530/diff/
> 
> 
> Testing
> ---
> 
> Page could be viewed at 
> http://janisz.github.io/mesos/site/publish/working-groups/
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 48530: Moved working groups from wiki to page.

[Tomasz Janiszewski]

> On Cze 10, 2016, 4:15 po południu, haosdent huang wrote:
> > Verify this local and looks great for me. Another question, should we have 
> > a link to working-groups in community? Otherwise user could not find it 
> > except it remember the link 'http://mesos.apache.org/working-groups/'.

I agree. This should be moved under community. I think this happened too fast. 
Moving all data from wiki is one thing but deciding how and where preset them 
is another. IMO we should have roadmap for website or at least design doc. 
Patching website will make it less consistent.


- Tomasz


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


On Cze 10, 2016, 1:53 rano, Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48530/
> ---
> 
> (Updated Cze 10, 2016, 1:53 rano)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5591
> https://issues.apache.org/jira/browse/MESOS-5591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change was discussed at
> http://www.mail-archive.com/dev@mesos.apache.org/msg35506.html
> 
> 
> Diffs
> -
> 
>   site/data/working_groups.yaml PRE-CREATION 
>   site/source/working-groups.html.erb PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48530/diff/
> 
> 
> Testing
> ---
> 
> Page could be viewed at 
> http://janisz.github.io/mesos/site/publish/working-groups/
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 48530: Moved working groups from wiki to page.

[Jie Yu]

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



In fact, for working groups, I would prefer adding that to the docs so that 
people can find/read it on github. It is also constantly changing, updating it 
in docs sounds easier than updating the website. What do you think?

- Jie Yu


On June 10, 2016, 1:53 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48530/
> ---
> 
> (Updated June 10, 2016, 1:53 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5591
> https://issues.apache.org/jira/browse/MESOS-5591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change was discussed at
> http://www.mail-archive.com/dev@mesos.apache.org/msg35506.html
> 
> 
> Diffs
> -
> 
>   site/data/working_groups.yaml PRE-CREATION 
>   site/source/working-groups.html.erb PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48530/diff/
> 
> 
> Testing
> ---
> 
> Page could be viewed at 
> http://janisz.github.io/mesos/site/publish/working-groups/
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 48530: Moved working groups from wiki to page.

[haosdent huang]

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



Verify this local and looks great for me. Another question, should we have a 
link to working-groups in community? Otherwise user could not find it except it 
remember the link 'http://mesos.apache.org/working-groups/'.

- haosdent huang


On June 10, 2016, 1:53 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48530/
> ---
> 
> (Updated June 10, 2016, 1:53 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5591
> https://issues.apache.org/jira/browse/MESOS-5591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change was discussed at
> http://www.mail-archive.com/dev@mesos.apache.org/msg35506.html
> 
> 
> Diffs
> -
> 
>   site/data/working_groups.yaml PRE-CREATION 
>   site/source/working-groups.html.erb PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48530/diff/
> 
> 
> Testing
> ---
> 
> Page could be viewed at 
> http://janisz.github.io/mesos/site/publish/working-groups/
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 48380: Updated CHANGELOG for libprocess HTTP authorization.

[Greg Mann]

> On June 8, 2016, 8:34 p.m., Vinod Kone wrote:
> > CHANGELOG, line 154
> > 
> >
> > Is there an example in the user doc on what changes are specifically 
> > needed that can be linked here?

There isn't such an example yet, but I'll try to test and verify the relevant 
ACLs and post another patch for those user docs, so I can link them here.


- Greg


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


On June 10, 2016, 4:04 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48380/
> ---
> 
> (Updated June 10, 2016, 4:04 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated CHANGELOG for libprocess HTTP authorization.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 0513c1594098c8afdb7085ecab5edf07f93f7d0e 
> 
> Diff: https://reviews.apache.org/r/48380/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 48380: Updated CHANGELOG for libprocess HTTP authorization.

[Greg Mann]

> On June 8, 2016, 3:01 p.m., Joerg Schad wrote:
> > CHANGELOG, line 149
> > 
> >
> > Could you doublecheck that we added all newly authorized endpoints?

I updated this with references to the other endpoints we authorized that don't 
use authorization-based filtering, since MESOS-4931 appears in this list as 
well, which covers the filtered endpoints. What do you think?


- Greg


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


On June 10, 2016, 4:04 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48380/
> ---
> 
> (Updated June 10, 2016, 4:04 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated CHANGELOG for libprocess HTTP authorization.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 0513c1594098c8afdb7085ecab5edf07f93f7d0e 
> 
> Diff: https://reviews.apache.org/r/48380/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47082: LostSlaveMessage should be sent to affected frameworks only.

[Jiang Yan Xu]

> On June 10, 2016, 5:47 a.m., Neil Conway wrote:
> > Something else to consider here: this change means that frameworks will 
> > miss a `slaveLost()` signal that they care about in some circumstances. For 
> > example, suppose an agent has a persistent volume on an agent; the master 
> > fails over, and the agent fails to reregister with the master. We'll remove 
> > the agent, but we _won't_ send `SlaveLostMessage` to the framework in this 
> > case, because the master doesn't know the framework has a persistent volume 
> > on the agent.
> > 
> > Since `slaveLost()` is unreliable to begin with, I don't think this is a 
> > show-stopper, but it's a bit unfortunate...

Yeah in this case we'll have to fall back to broadcasting, also a bit 
unfortunate but there's not a better way IMO.


- Jiang Yan


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


On June 8, 2016, 6:08 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47082/
> ---
> 
> (Updated June 8, 2016, 6:08 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5143
> https://issues.apache.org/jira/browse/MESOS-5143
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a slave is removed, master sends a LostSlaveMessage to affected
> frameworks only (instead of all registered frameworks). An affected
> framework is a framework which satisfied one or more conditions of
> the following:
> 
> 1. There are tasks on this slave belonging to the framework.
> 2. There are pending tasks on this slave belonging to the framework.
> 3. Reserved resources on the slave have a matching role with the
>role of the framework.
> 4. There are pending offers or pending inverse offers from this slave
>for the framework.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 790da3ce686401c378ad9c62d497d60893c4ce41 
>   src/master/master.cpp 21ec70491ce6b79be57ff8db51d4b2fa682b32ce 
>   src/tests/master_tests.cpp 34be015aa314a7574e9065efb7b1bb8e1570c5b7 
> 
> Diff: https://reviews.apache.org/r/47082/diff/
> 
> 
> Testing
> ---
> 
> All existing and modified tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 48380: Updated CHANGELOG for libprocess HTTP authorization.

[Greg Mann]

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

(Updated June 10, 2016, 4:04 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Included more authorized endpoints in the changelog.


Repository: mesos


Description
---

Updated CHANGELOG for libprocess HTTP authorization.


Diffs (updated)
-

  CHANGELOG 0513c1594098c8afdb7085ecab5edf07f93f7d0e 

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 48554: Fixed a GMock warning in scheduler failover test.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [48554]

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

Error:
2016-06-10 15:03:50 URL:https://reviews.apache.org/r/48554/diff/raw/ 
[1105/1105] -> "48554.patch" [1]
error: patch failed: src/tests/fault_tolerance_tests.cpp:529
error: src/tests/fault_tolerance_tests.cpp: patch does not apply

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

- Mesos ReviewBot


On June 10, 2016, 1:37 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48554/
> ---
> 
> (Updated June 10, 2016, 1:37 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Bugs: MESOS-5595
> https://issues.apache.org/jira/browse/MESOS-5595
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a GMock warning in scheduler failover test.
> 
> 
> Diffs
> -
> 
>   src/tests/fault_tolerance_tests.cpp 
> c39a0ccf8fa124b2c230864f3ba1f7e8e05b36dc 
> 
> Diff: https://reviews.apache.org/r/48554/diff/
> 
> 
> Testing
> ---
> 
> `./src/mesos-tests 
> --gtest_filter="FaultToleranceTest.SchedulerReregisterAfterFailoverTimeout" 
> --gtest_repeat=100` => no warnings observed.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 48552: Added the missed bracket.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48552]

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 June 10, 2016, 1:24 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48552/
> ---
> 
> (Updated June 10, 2016, 1:24 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the missed bracket.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 106ff35320f37b2e75ed60381de1406459e6d515 
> 
> Diff: https://reviews.apache.org/r/48552/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 45574: Add `PerfEventSubsystem` for cgroups unified isolator.

[Qian Zhang]

> On June 6, 2016, 9:38 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, lines 150-156
> > 
> >
> > I see in this hashmap, the keys and values are exactly same, e.g., 
> > CGROUP_SUBSYSTEM_CPU_NAME is "cpu", CGROUP_SUBSYSTEM_CPUACCT_NAME is 
> > "cpuacct", etc. It is a bit strange to have same keys and values in a 
> > hashmap, maybe we should use hashset?
> 
> haosdent huang wrote:
> Because we want to map 'mem' -> 'memory', we could not use hashet here. 
> Currently `CgroupsMemIsolatorProcess` use `cgroups/mem` as isolation flag. 
> However, the memory subsystem name in Linux us `memory`. So we need to map 
> `mem` -> `memory`.
> 
> And I think we may have other inconsistent between our internal name and 
> Linux cgroup subsystem name, e.g. `devices/gpu` -> `devices`. So I think it 
> is necessary.

Agree, thanks haosdent!


- Qian


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


On April 16, 2016, 6:28 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45574/
> ---
> 
> (Updated April 16, 2016, 6:28 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5047
> https://issues.apache.org/jira/browse/MESOS-5047
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add `PerfEventSubsystem` for cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp 674d5f886ea41b939a8e48832ee6595a78b2f6ce 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45574/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46158: Completed implementation of the cgroups unified isolator.

[Qian Zhang]

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




src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 76 - 78)


I think these 3 lines can be merged into 1 line which will not exceed 80 
chars.



src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 368 - 370)


If we return from here, then I think we will not have chance to destroy the 
cgroups that we created for this container in `prepareHierarchy()`. The reason 
is, here we have not put `info` into `infos`, so in 
`CgroupsIsolatorProcess::cleanup()`, we will not do anything for this container 
since `infos` does not contains its `info`.
So I would suggest to do `infos[containerId] = info;` right after we get a 
newly created `info` from `info::create()`


- Qian Zhang


On April 16, 2016, 6:14 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46158/
> ---
> 
> (Updated April 16, 2016, 6:14 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Completed implementation of the cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46158/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 48554: Fixed a GMock warning in scheduler failover test.

[Neil Conway]

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

Review request for mesos and Anand Mazumdar.


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


Repository: mesos


Description
---

Fixed a GMock warning in scheduler failover test.


Diffs
-

  src/tests/fault_tolerance_tests.cpp c39a0ccf8fa124b2c230864f3ba1f7e8e05b36dc 

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


Testing
---

`./src/mesos-tests 
--gtest_filter="FaultToleranceTest.SchedulerReregisterAfterFailoverTimeout" 
--gtest_repeat=100` => no warnings observed.


Thanks,

Neil Conway

Review Request 48552: Added the missed bracket.

[Qian Zhang]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added the missed bracket.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
106ff35320f37b2e75ed60381de1406459e6d515 

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 47082: LostSlaveMessage should be sent to affected frameworks only.

[Neil Conway]

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



Something else to consider here: this change means that frameworks will miss a 
`slaveLost()` signal that they care about in some circumstances. For example, 
suppose an agent has a persistent volume on an agent; the master fails over, 
and the agent fails to reregister with the master. We'll remove the agent, but 
we _won't_ send `SlaveLostMessage` to the framework in this case, because the 
master doesn't know the framework has a persistent volume on the agent.

Since `slaveLost()` is unreliable to begin with, I don't think this is a 
show-stopper, but it's a bit unfortunate...

- Neil Conway


On June 9, 2016, 1:08 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47082/
> ---
> 
> (Updated June 9, 2016, 1:08 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5143
> https://issues.apache.org/jira/browse/MESOS-5143
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a slave is removed, master sends a LostSlaveMessage to affected
> frameworks only (instead of all registered frameworks). An affected
> framework is a framework which satisfied one or more conditions of
> the following:
> 
> 1. There are tasks on this slave belonging to the framework.
> 2. There are pending tasks on this slave belonging to the framework.
> 3. Reserved resources on the slave have a matching role with the
>role of the framework.
> 4. There are pending offers or pending inverse offers from this slave
>for the framework.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 790da3ce686401c378ad9c62d497d60893c4ce41 
>   src/master/master.cpp 21ec70491ce6b79be57ff8db51d4b2fa682b32ce 
>   src/tests/master_tests.cpp 34be015aa314a7574e9065efb7b1bb8e1570c5b7 
> 
> Diff: https://reviews.apache.org/r/47082/diff/
> 
> 
> Testing
> ---
> 
> All existing and modified tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>