Re: Review Request 59989: Added a test `ProtobufTest.JsonifyMap`.

[Qian Zhang]

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

(Updated July 4, 2017, 10:44 a.m.)


Review request for mesos, Anand Mazumdar and Zhitao Li.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added a test `ProtobufTest.JsonifyMap`.


Diffs (updated)
-

  3rdparty/stout/tests/protobuf_tests.cpp 
8877e8934e0f7875bfedcfa88b491ce4b13ca44f 


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

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


Testing
---

sudo make check


Thanks,

Qian Zhang

Re: Review Request 59987: Added protobuf map support.

[Qian Zhang]

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

(Updated July 4, 2017, 10:43 a.m.)


Review request for mesos, Anand Mazumdar and Zhitao Li.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Map is a feature of proto2 syntax, but it can only be compiled
with proto3 compiler, see the following discussion for details:
https://groups.google.com/forum/#!topic/protobuf/p4WxcplrlA4
We have already upgraded the compiler from 2.6.1 to 3.3.0 in
MESOS-7228, however, to use protobuf map in Mesos code, we also
need to add the protobuf map support to the code in Mesos for
converting protobuf message to JSON object and parsing JSON
object as protobuf message, that is what I have done in this patch.


Diffs (updated)
-

  3rdparty/stout/include/stout/protobuf.hpp 
15690b66cc4ae0c1bf2c2176d73c385ca75d3c20 


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

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 59988: Added a new protobuf message `MapMessage` for protobuf tests.

[Qian Zhang]

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

(Updated July 4, 2017, 10:44 a.m.)


Review request for mesos, Anand Mazumdar and Zhitao Li.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added a new protobuf message `MapMessage` for protobuf tests.


Diffs (updated)
-

  3rdparty/stout/tests/protobuf_tests.proto 
d16726aa8060aea2b830040b20dbdd467c801483 


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

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 60107: Added filtering to the '/tasks' endpoint.

[Quinn Leng]

> On July 3, 2017, 9:51 a.m., Alexander Rojas wrote:
> > I think the acceptor files do not belong in the `common/http.?pp` files. 
> > Perhaps create a new header called acceptors?

Agree, since these Acceptors contain logic about not only HTTP request but also 
Authorization. It's better to move these logic into one specific file. Do you 
think it's good to call it common/acceptor.?pp


> On July 3, 2017, 9:51 a.m., Alexander Rojas wrote:
> > src/common/http.hpp
> > Lines 163-171 (patched)
> > 
> >
> > This class is not necessary. The acceptor as a concept exist, but we 
> > don't use any of the advantages of inheritance, I think this would be 
> > served better in a namespace.

It's kind of inbetween logically consistent and programacally consistent. We 
haven't used a lot of inheritance feature (overriding) of the concept, but 
there do exist some logical inheritance connection between them. This 
ObjectAcceptor connects these AuthorizeAcceptor and IDAcceptor.


> On July 3, 2017, 9:51 a.m., Alexander Rojas wrote:
> > src/common/http.hpp
> > Lines 175-229 (patched)
> > 
> >
> > What is the point of having multiple different 
> > `AuthorizationAcceptors`? Each have a method with a different signature, so 
> > I can see them merge into one common acceptor.

One possible issue with single AuthorizationAcceptor is that we initialize an 
approver for each AuthorizationAcceptor, and these approvers are initialized 
with different parameters (specifically, the authorization::VIEW_TASK, 
authorization::VIEW_FRAMEWORK .etc..), thus it's not good idea to have one 
AuthorizationAcceptor providing authorization for different kind of objects. 
Alternatively, we can use multiple AuthorizationAcceptor objects, each is an 
instance of AuthorizationAcceptor initialized with different constructor, but 
then it will be necessary that we call the correct kind of 'accept' for that 
AuthorizationAcceptor. That might be confusing.


> On July 3, 2017, 9:51 a.m., Alexander Rojas wrote:
> > src/common/http.hpp
> > Lines 181 (patched)
> > 
> >
> > This should be a const reference, we don't want to copy `Owned` 
> > pointers.

Agree, I'll update it in another patch. Thanks for pointing out~


- Quinn


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


On June 30, 2017, 11:57 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60107/
> ---
> 
> (Updated June 30, 2017, 11:57 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added filtering to the '/tasks' endpoint.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
>   src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 
>   src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
>   src/tests/master_tests.cpp 6cd4be7e5ba48c6562ce91b28e76b065522cfbea 
> 
> 
> Diff: https://reviews.apache.org/r/60107/diff/15/
> 
> 
> Testing
> ---
> 
> Passed "make check"
> Passed "GTEST_FILTER="MasterTest.TasksEndpoint" make check -j48"
> Passed "GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="MasterTest.TasksEndpoint" --gtest_repeat=1000 
> --gtest_break_on_failure"
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60425: Added MESOS-7581 to the 1.3.1 CHANGELOG.

[Till Toenshoff]

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


Fix it, then Ship it!




Thanks!


CHANGELOG
Lines 14 (patched)


Will fix the ordering while committing.


- Till Toenshoff


On June 26, 2017, 10:42 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60425/
> ---
> 
> (Updated June 26, 2017, 10:42 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added MESOS-7581 to the 1.3.1 CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 7536717b85bb188d8a463b36ce75e4e1cecc445d 
> 
> 
> Diff: https://reviews.apache.org/r/60425/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 60427: Added MESOS-7581 to the 1.2.2 CHANGELOG.

[Till Toenshoff]

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


Fix it, then Ship it!





CHANGELOG
Lines 306 (patched)


Ordering gets fixed while committing.


- Till Toenshoff


On June 26, 2017, 10:42 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60427/
> ---
> 
> (Updated June 26, 2017, 10:42 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added MESOS-7581 to the 1.2.2 CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 7536717b85bb188d8a463b36ce75e4e1cecc445d 
> 
> 
> Diff: https://reviews.apache.org/r/60427/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 60428: Added MESOS-7581 to the 1.1.3 CHANGELOG.

[Till Toenshoff]

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


Fix it, then Ship it!





CHANGELOG
Lines 801 (patched)


Ordering gets fixed while committing.


- Till Toenshoff


On June 26, 2017, 10:43 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60428/
> ---
> 
> (Updated June 26, 2017, 10:43 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added MESOS-7581 to the 1.1.3 CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 7536717b85bb188d8a463b36ce75e4e1cecc445d 
> 
> 
> Diff: https://reviews.apache.org/r/60428/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 60580: Added filtering to the '/frameworks' endpoint.

[Quinn Leng]

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

(Updated July 3, 2017, 8:10 p.m.)


Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod 
Kone.


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


Repository: mesos


Description (updated)
---

Added filtering to the '/frameworks' endpoint.


Diffs (updated)
-

  src/common/http.hpp b7e4a8adcbcaa3a962af795c67694a35161b6c1a 
  src/common/http.cpp fdb591eccf273260902f3f695cf431f72ee3d817 
  src/master/http.cpp 64b7cdd1c42a067bf24e302b9496bb75cb916590 
  src/tests/master_tests.cpp d21194fb19781a24223d6b0d8f99987f4edb5a9a 


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

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


Testing
---

Passed 'make check -j48'
Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check 
-j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh 
--gtest_filter="MasterTest.FrameworksEndpointQueryFramework" 
--gtest_repeat=1000 --gtest_break_on_failure'


Thanks,

Quinn Leng

Review Request 60628: Enable fetcher_tests.cpp unit test module on Windows platform.

[Jeff Coffler]

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

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


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


Repository: mesos


Description
---

Document that some FetcherTest tests won't work on Windows platform.
  Tests for tar, gzip, and such won't be working on Windows for
  the time being. Thoughts are to provide this capability to the
  Fetcher in a cross-platform manner via a programmatic code library
  rather than Linux-specific command line tools (tar, gzip, etc).

  In the short term, zip and unzip will work since PowerShell can
  support that natively.

Fix FetcherTest.AbsoluteCustomSubdirectoryFails on Windows platform.
Test FetcherTest.InvalidUser can't work on Windows for now.
Fix test FetcherTest.NonExistingFile on Windows platform.
Fix test FetcherTest.AbsolutePath on Windows platform.
Fix test FetcherTest.RelativeFilePath on Windows platform.
Fix test FetcherTest.OSNetUriTest on Windows platform.
Fix test FetcherTest.OSNetUriSpaceTest on Windows platform.
Fix test FetcherTest.FileLocalhostURI on Windows platform.
Fix test FetcherTest.Unzip_ExtractFile on Windows platform.
Fix test FetcherTest.Unzip_ExtractInvalidFile on Windows platform.
Fix test Unzip_ExtractFileWithDuplicatedEntries on Windows platform.
Fix test FetcherTest.UseCustomOutputFile on Windows platform.


Diffs
-

  src/launcher/CMakeLists.txt 97edc4a28a995da8a5e963aaa62c4c955cc59719 
  src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87 
  src/slave/containerizer/fetcher.cpp e3c786b36ad16b33ef9d3eed15f722890e80f0bb 
  src/tests/fetcher_tests.cpp 99149baa1c7abfabf572a0d0f4512a8e84d1e5be 


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


Testing
---

See upstream.


Thanks,

Jeff Coffler

Review Request 60626: Eliminate os::shell calls from HDFS for Windows compatibility.

[Jeff Coffler]

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

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


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


Repository: mesos


Description
---

Eliminate os::shell calls from HDFS for Windows compatibility.


Diffs
-

  src/hdfs/hdfs.cpp 2c95a5ea43a4289e1168c527b9ccc35690a751a4 


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


Testing
---

See upstream


Thanks,

Jeff Coffler

Review Request 60625: Normalize file separation characters on Windows when building path.

[Jeff Coffler]

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

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


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


Repository: mesos


Description
---

Normalize file separation characters on Windows when building path.


Diffs
-

  3rdparty/stout/include/stout/path.hpp 
6ee3a44cd6a878fe383aa68df40b82857b93d0b4 


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


Testing
---

See upstream


Thanks,

Jeff Coffler

Review Request 60624: Enable HDFS compilation and associated tests.

[Jeff Coffler]

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

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


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


Repository: mesos


Description
---

Note that tests are disabled for Windows due to dependence on 'sh'
shell.


Diffs
-

  src/CMakeLists.txt 51b67428f823454665db695ba70a0dc896a7595c 
  src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87 
  src/tests/CMakeLists.txt 9c0acaf43f451dbc9ba5077529a36aa4cef40c34 
  src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515 


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


Testing
---

See upstream


Thanks,

Jeff Coffler

Review Request 60623: Convert "file://" URI handling to use new path::uri() function.

[Jeff Coffler]

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

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


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


Repository: mesos


Description
---

Convert "file://" URI handling to use new path::uri() function.


Diffs
-

  src/tests/credentials_tests.cpp 7eb5abd21a1be35d4739c4928cb922f028cfc9a7 
  src/tests/fetcher_tests.cpp 99149baa1c7abfabf572a0d0f4512a8e84d1e5be 
  src/tests/master_contender_detector_tests.cpp 
f499136c0b981072af5bc8accf2238b7ba4bf430 
  src/tests/script.cpp 8d40e01da005cb05e7804f0b3975e3e0edb8f3bd 


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


Testing
---

See upstream


Thanks,

Jeff Coffler

Review Request 60622: Add new stout function: path::uri (convert filename to valid URI).

[Jeff Coffler]

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

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


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


Repository: mesos


Description
---

Add new stout function: path::uri (convert filename to valid URI).


Diffs
-

  3rdparty/stout/include/stout/path.hpp 
6ee3a44cd6a878fe383aa68df40b82857b93d0b4 
  3rdparty/stout/tests/path_tests.cpp f8c14d5aefe0b49adb778da784143a328c96183d 


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


Testing
---

See upstream


Thanks,

Jeff Coffler

Review Request 60620: Modify os::write to write binary files on Windows.

[Jeff Coffler]

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

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


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


Repository: mesos


Description
---

Modify os::write to write binary files on Windows.


Diffs
-

  3rdparty/stout/include/stout/os/write.hpp 
beb5bd83b52565a75e34d32b5bb17951bc799578 


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


Testing
---

Built successfully on both Linux (with autotools and cmake) and Windows (with 
cmake).

Ran stout-tests and mesos-tests successfully on both Windows and Linux.


Thanks,

Jeff Coffler

Re: Review Request 60581: Added filtering to the '/slaves' endpoint.

[Quinn Leng]

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

(Updated July 3, 2017, 7:03 p.m.)


Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod 
Kone.


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


Repository: mesos


Description (updated)
---

Added filtering to the '/slaves' endpoint.


Diffs (updated)
-

  src/common/http.hpp b7e4a8adcbcaa3a962af795c67694a35161b6c1a 
  src/common/http.cpp fdb591eccf273260902f3f695cf431f72ee3d817 
  src/master/http.cpp 64b7cdd1c42a067bf24e302b9496bb75cb916590 
  src/tests/master_tests.cpp d21194fb19781a24223d6b0d8f99987f4edb5a9a 


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

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


Testing
---

Passed 'make check -j48'
Passed 'GTEST_FILTER="MasterTest.SlavesEndpointQuerySlave" make check -j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh 
--gtest_filter="MasterTest.SlavesEndpointQuerySlave" --gtest_repeat=1000 
--gtest_break_on_failure'


Thanks,

Quinn Leng

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Avinash sridharan]

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




src/messages/flags.hpp
Lines 58 (patched)


For CNI, since we are adding these entries to the `resolv.conf` we can't 
support more than three name servers for a given network.

Similarly for CNM we probably need to check if we can support more than 3 
nameservers for HOST and BRIDGE networks?


- Avinash sridharan


On June 28, 2017, 2:57 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated June 28, 2017, 2:57 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-7709
> https://issues.apache.org/jira/browse/MESOS-7709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced `--default_container_dns` agent flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 0eb696a949003ff11831aed5e4f4ab384cf9992e 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp e75c1b4227b443aedf445921b3f2108d930c112c 
>   src/slave/flags.cpp c84aa6724170bba46bbe8410b71d42a1626e 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60557: Passed default container DNS info to Docker executor.

[Avinash sridharan]

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


Ship it!




Ship It!

- Avinash sridharan


On June 30, 2017, 6:56 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60557/
> ---
> 
> (Updated June 30, 2017, 6:56 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-7709
> https://issues.apache.org/jira/browse/MESOS-7709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Passed default container DNS info to Docker executor.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.hpp a4a8ec9905d3c6b3afcf0e2ba4e174c41fbcd75a 
>   src/docker/executor.cpp e03f24461ec7b19cacae79c020406f0a475f2d19 
>   src/slave/containerizer/docker.cpp 5cd3b6d95ff78fb114a06d49122b7161a6688646 
> 
> 
> Diff: https://reviews.apache.org/r/60557/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60280: Provide full path to the custom executor.

[Aaron Wood via Review Board]

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

(Updated July 3, 2017, 6:35 p.m.)


Review request for mesos, Jie Yu, James Peach, and Zhitao Li.


Changes
---

Exit with failure status if we can't get the absolute path.


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


Repository: mesos


Description
---

If a framework specifies use of its own executor and sets shell to false the 
executor is never found. This provides the full path to the executor so that 
the `execvp` or `execvpe` is successful.


Diffs (updated)
-

  src/slave/containerizer/mesos/launch.cpp 162ca1c2e 


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

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


Testing
---

`cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4`
Also spun up a master and agent, connected and sent a task using the UCR (both 
with and without the use of an OCI image) via our own framework, and checked 
the sandbox to verify that things went accordingly.


Thanks,

Aaron Wood

Re: Review Request 60558: Set container DNS with `--default_container_dns` in Docker executor.

[Avinash sridharan]

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




src/docker/docker.hpp
Lines 167 (patched)


For consistency can we use `defautlContainerDNS`?



src/docker/docker.cpp
Line 839 (original), 867 (patched)


We would need to perform a version check for supported DNS options here as 
well.


- Avinash sridharan


On June 30, 2017, 7:09 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60558/
> ---
> 
> (Updated June 30, 2017, 7:09 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-7709
> https://issues.apache.org/jira/browse/MESOS-7709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set container DNS with `--default_container_dns` in Docker executor.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 5593cb635e073334c6c2566be3d803cd7febb1c3 
>   src/docker/docker.cpp 8ca0c68836ea5d1a1186e79942f6daccf6c6da46 
>   src/docker/executor.cpp e03f24461ec7b19cacae79c020406f0a475f2d19 
> 
> 
> Diff: https://reviews.apache.org/r/60558/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 1. Start Mesos master.
> ```
> $ sudo ./bin/mesos-master.sh --work_dir=/opt/mesos
> ```
> 
> 2. Start Mesos agent.
> ```
> $ sudo ./bin/mesos-slave.sh --master=192.168.122.216:5050 
> --containerizers=mesos,docker --image_providers=docker 
> --image_provisioner_backend=aufs 
> --isolation=filesystem/linux,docker/runtime,network/cni,cgroups/cpu,cgroups/mem
>  --network_cni_config_dir=/opt/cni/net_configs 
> --network_cni_plugins_dir=/opt/cni/plugins --work_dir=/opt/mesos 
> --docker_store_dir=/opt/mesos/store/docker 
> --executor_registration_timeout=60mins 
> --default_container_dns=file:///home/stack/dns.json
> 
> $ cat /home/stack/dns.json
> {
>   "mesos": [
> {
>   "network": "net1",
>   "dns": {
> "nameservers": [ "8.8.8.8", "8.8.4.4" ]
>   }
> }
>   ],
>   "docker": [
> {
>   "network": "bridge",
>   "dns": {
> "nameservers": [ "8.8.8.8", "8.8.4.4" ],
> "search": [ "xxx.com", "yyy.com" ],
> "options": [ "timeout:3", "attempts:2" ]
>   }
> }
>   ]
> }
> ```
> 
> 3. Launch a Docker container with `mesos-execute`.
> ```
> $ sudo src/mesos-execute --master=192.168.122.216:5050 
> --task=file:///home/stack/task-docker.json
> 
> $cat /home/stack/task-docker.json 
> {
>   "name": "test",
>   "task_id": {"value" : "test"},
>   "agent_id": {"value" : ""},
>   "resources": [
> {
>   "name": "cpus",
>   "type": "SCALAR",
>   "scalar": {
> "value": 0.1
>   }
> },
> {
>   "name": "mem",
>   "type": "SCALAR",
>   "scalar": {
> "value": 32
>   }
> }
>   ],
>   "command": {
> "shell": false
>   },
>   "container": {
> "type": "DOCKER",
> "docker": {
>   "image": "nginx",
>   "network": "BRIDGE"
> }
>   }
> }
> ```
> 
> 4. Check the DNS configuration of the Docker container.
> ```
> $ docker ps 
> CONTAINER IDIMAGE   COMMAND  CREATED  
> STATUS  PORTS   NAMES
> ca642bf31a9fnginx   "nginx -g 'daemon off"   About a 
> minute ago   Up About a minute   80/tcp, 443/tcp 
> mesos-1d48fc32-a138-4c31-a5a9-fd7d279231da
> 
> $ docker exec ca642bf31a9f cat /etc/resolv.conf 
> search xxx.com yyy.com
> nameserver 8.8.8.8
> nameserver 8.8.4.4
> options timeout:3 attempts:2
> ```
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60370: Updated agent webui page to display allocated resources per each role.

[haosdent huang]

> On June 27, 2017, 5:33 p.m., haosdent huang wrote:
> > src/webui/master/static/js/controllers.js
> > Lines 604 (patched)
> > 
> >
> > `ng-repeat` support map, have you try
> > 
> > ```
> > ng-repeat="(role, reservation) in $data"
> > ```
> 
> Andrei Budnik wrote:
> If I change this:
> ```html
> 
>   {{reservation.role}}
>   
> {{(state.reserved_resources_allocated[reservation.role].cpus | number) || 
> 0}} / {{reservation.cpus | number}}
>   
> {{(state.reserved_resources_allocated[reservation.role].gpus | number) || 
> 0}} / {{reservation.gpus | number}}
>   {{(state.reserved_resources_allocated[reservation.role].mem 
> * (1024 * 1024) | dataSize) || 0}} / {{reservation.mem * (1024 * 1024) | 
> dataSize}}
>   
> {{(state.reserved_resources_allocated[reservation.role].disk * (1024 * 
> 1024) | dataSize) || 0}} / {{reservation.disk * (1024 * 1024) | 
> dataSize}}
> 
> ```
> to that:
> ```html
> 
>   {{role}}
>   
> {{(state.reserved_resources_allocated[reservation.role].cpus | number) || 
> 0}} / {{reservation.cpus | number}}
>   
> {{(state.reserved_resources_allocated[reservation.role].gpus | number) || 
> 0}} / {{reservation.gpus | number}}
>   {{(state.reserved_resources_allocated[reservation.role].mem 
> * (1024 * 1024) | dataSize) || 0}} / {{reservation.mem * (1024 * 1024) | 
> dataSize}}
>   
> {{(state.reserved_resources_allocated[reservation.role].disk * (1024 * 
> 1024) | dataSize) || 0}} / {{reservation.disk * (1024 * 1024) | 
> dataSize}}
> 
> ```
> Then it will look like in the 
> [picture](https://reviews.apache.org/r/60370/file/1508/).
> 
> haosdent huang wrote:
> It is because you convert it from map to arrary at
> 
> ```
> $scope.agent.resource_reservations = roles.map
> ```
> 
> Have you remove it?
> 
> I test with 
> 
> ```
>   
> {{role}}
>   
> ```
> 
> and work for me.
> 
> Andrei Budnik wrote:
> Yeah, I can use this code:
> ```html
> 
>   {{role}}
>   {{(state.reserved_resources_allocated[role].cpus || 0) | 
> number}} / {{reservation.cpus | number}}
>   {{(state.reserved_resources_allocated[role].gpus || 0) | 
> number}} / {{reservation.gpus | number}}
>   {{(state.reserved_resources_allocated[role].mem * (1024 * 
> 1024) || 0) | dataSize}} / {{reservation.mem * (1024 * 1024) | dataSize}}
>   {{(state.reserved_resources_allocated[role].disk * (1024 * 
> 1024) || 0) | dataSize}} / {{reservation.disk * (1024 * 1024) | 
> dataSize}}
> 
> ```
> but filter becomes missing, see 
> [picture](https://reviews.apache.org/r/60370/file/1509/).

I see, it is because datatable don't support map. so it would hide filter.


- haosdent


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


On June 28, 2017, 8:52 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60370/
> ---
> 
> (Updated June 28, 2017, 8:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and haosdent huang.
> 
> 
> Bugs: MESOS-6441
> https://issues.apache.org/jira/browse/MESOS-6441
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated agent webui page to display allocated resources per each role.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/agent.html 71e5e702e5e64e6f46c84791247aa5156c046ed9 
>   src/webui/master/static/js/controllers.js 
> 67bfd030649dd21840c16188a4964f814aa232d7 
> 
> 
> Diff: https://reviews.apache.org/r/60370/diff/3/
> 
> 
> Testing
> ---
> 
> See screenshot.
> 
> 
> File Attachments
> 
> 
> agent.html
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/22/66cc61b4-04c8-451e-a434-58f556397724__Resource_Reservations.png
> ui_bug
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/28/1b7755e3-0b11-46ab-9ae4-71f19bce0cd6__ui_bug.png
> ui_bug2
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/28/b89990c9-9aa5-432a-a0da-3f90c9428127__ui_bug2.png
> ui_bug3
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/28/51553e9d-7fb9-40c0-b47c-b62fc6c37393__ui_bug3.png
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60370: Updated agent webui page to display allocated resources per each role.

[haosdent huang]

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




src/webui/master/static/js/controllers.js
Lines 610 (patched)


How about

```
$scope.agent.resource_reservations = 
_($scope.state.reserved_resources).map(function(role, reservation)) {
  reservation.role = role
  return reservation;
}

```


- haosdent huang


On June 28, 2017, 8:52 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60370/
> ---
> 
> (Updated June 28, 2017, 8:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and haosdent huang.
> 
> 
> Bugs: MESOS-6441
> https://issues.apache.org/jira/browse/MESOS-6441
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated agent webui page to display allocated resources per each role.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/agent.html 71e5e702e5e64e6f46c84791247aa5156c046ed9 
>   src/webui/master/static/js/controllers.js 
> 67bfd030649dd21840c16188a4964f814aa232d7 
> 
> 
> Diff: https://reviews.apache.org/r/60370/diff/3/
> 
> 
> Testing
> ---
> 
> See screenshot.
> 
> 
> File Attachments
> 
> 
> agent.html
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/22/66cc61b4-04c8-451e-a434-58f556397724__Resource_Reservations.png
> ui_bug
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/28/1b7755e3-0b11-46ab-9ae4-71f19bce0cd6__ui_bug.png
> ui_bug2
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/28/b89990c9-9aa5-432a-a0da-3f90c9428127__ui_bug2.png
> ui_bug3
>   
> https://reviews.apache.org/media/uploaded/files/2017/06/28/51553e9d-7fb9-40c0-b47c-b62fc6c37393__ui_bug3.png
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60369: Exposed allocated resources per each role in /state endpoint on agent.

[Andrei Budnik]

> On July 3, 2017, 1:22 p.m., Benjamin Mahler wrote:
> > src/slave/http.cpp
> > Lines 1330-1334 (patched)
> > 
> >
> > Whoops, I should have noticed this earlier, but when going to commit 
> > this I was taking a careful look at the code. This isn't accounting for the 
> > pending tasks here:
> > 
> > 
> > https://github.com/apache/mesos/blob/0d277bb64fa5a4d0b4f741daedf64095beab4773/src/slave/slave.hpp#L904-L905
> > 
> > Unfortunately the logic is a little tricky. We have to include all of 
> > the pending task resources. Also, for each executor in the pending map, we 
> > have to add the ExecutorInfo resources only if it's absent from the 
> > 'executors' map. You'll want to do a read through of how the data 
> > structures are used to confirm whether I'm right. :)
> > 
> > My suggestion here would be to have an `allocatedResources()` function 
> > within the `Framework` struct that contains this logic, and here in the 
> > http handler we just sum `allocatedResources()` across the frameworks.

Fixed.


- Andrei


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


On July 3, 2017, 5:52 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60369/
> ---
> 
> (Updated July 3, 2017, 5:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and haosdent huang.
> 
> 
> Bugs: MESOS-6441
> https://issues.apache.org/jira/browse/MESOS-6441
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The JSON key for this information is "reserved_resources_allocated"
> and "unreserved_resources_allocated".
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 700871e1502a65b0bb1fc31219e09219dbdb5340 
>   src/slave/slave.hpp 5ede32d00c5dbf18dc0639ec7af5d2a86f86f619 
>   src/slave/slave.cpp 0e24b8cb8d1020af515e3d1862e121e1daf82ce9 
> 
> 
> Diff: https://reviews.apache.org/r/60369/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60369: Exposed allocated resources per each role in /state endpoint on agent.

[Andrei Budnik]

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

(Updated July 3, 2017, 5:52 p.m.)


Review request for mesos, Benjamin Mahler and haosdent huang.


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


Repository: mesos


Description
---

The JSON key for this information is "reserved_resources_allocated"
and "unreserved_resources_allocated".


Diffs (updated)
-

  src/slave/http.cpp 700871e1502a65b0bb1fc31219e09219dbdb5340 
  src/slave/slave.hpp 5ede32d00c5dbf18dc0639ec7af5d2a86f86f619 
  src/slave/slave.cpp 0e24b8cb8d1020af515e3d1862e121e1daf82ce9 


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

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


Testing
---

make check


Thanks,

Andrei Budnik

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Avinash sridharan]

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




docs/configuration.md
Lines 1309 (patched)


s/CNI plugin/ a CNI plugin,



docs/configuration.md
Lines 1312 (patched)


`ContainerInfo.docker.parameter`



docs/configuration.md
Lines 1329 (patched)


This is interesting. I think this example highlights a subtle difference 
between the behavior of the DNS option for CNI and CNM. For CNI networks we 
don't really have a concept of HOST or BRIDGE networks. However, for CNM 
networks since we support this option for HOST, BRIDGE and USER networks, we 
should be explicit about this information. Also, we need to emphasize that the 
`--dns` options for `HOST` and `BRIDGE` networks are supported only for docker 
1.13 and above (for `USER` it has been available since 1.10). We probably want 
to enforce these version checks for docker.



src/messages/flags.proto
Line 21 (original), 23 (patched)


Not yours but might as well fix the comment formatting to conform with the 
rest of the `.proto` files?



src/messages/flags.proto
Lines 32 (patched)


2 lines apart? Also, I think for protobuf definitions we are using the 
following format for comments:
/**
 * 
 */



src/messages/flags.proto
Lines 36 (patched)


Just a thought, should we be more explicit and keep two different DNS 
structures for Mesos and Docker? For example, Docker has the concept of network 
modes where as Mesos doesn't. With this structure we would have to use some 
kind of pattern matching to identify the network type in case of docker, e.g., 
`network="host"` for host-mode networking, `network="bridge"` for bridge-mode 
networking. This is just a thought, I am fine with these validation checks 
being introduced if this seems cleaner.

At the very least we should definitely have a comment here explaining how 
the network-modes are selected for `docker` based on the network names.



src/slave/flags.cpp
Lines 766 (patched)


Ditto for comments on `ocnfigure.md`.


- Avinash sridharan


On June 28, 2017, 2:57 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60500/
> ---
> 
> (Updated June 28, 2017, 2:57 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-7709
> https://issues.apache.org/jira/browse/MESOS-7709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced `--default_container_dns` agent flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 0eb696a949003ff11831aed5e4f4ab384cf9992e 
>   src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
>   src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
>   src/slave/flags.hpp e75c1b4227b443aedf445921b3f2108d930c112c 
>   src/slave/flags.cpp c84aa6724170bba46bbe8410b71d42a1626e 
> 
> 
> Diff: https://reviews.apache.org/r/60500/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60440: Updated local development workflow of mesos website.

[haosdent huang]

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



Should we make `support/mesos-website/entrypoint.sh` support a environment 
variable `MODE=dev` and then we `export MODE=dev` in 
`site/mesos-website-dev.sh` and just call `support/mesos-website.sh`? It would 
help to avoid write duplicate code in different places.


site/entrypoint.sh
Lines 26 (patched)


s/MacOSX/MacOS


- haosdent huang


On June 29, 2017, 9:59 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60440/
> ---
> 
> (Updated June 29, 2017, 9:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-7625
> https://issues.apache.org/jira/browse/MESOS-7625
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the layout and scripts consistent with CI based automatic
> publishing of the website.
> 
> 
> Diffs
> -
> 
>   site/Dockerfile 8ba0be0c28e924f7a2b89e6e5a3237deb3751a41 
>   site/README.md ebd3e6a0fea7ae0fe3b28719bcab28ee8f7c356c 
>   site/build.sh 11f15e15621c4d3db1472e88911787b9b3100f97 
>   site/entrypoint.sh PRE-CREATION 
>   site/mesos-website-dev.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60440/diff/1/
> 
> 
> Testing
> ---
> 
> Tested by running the script locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 60439: Added scripts to automate website publishing.

[haosdent huang]

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



Really sorry for the delay. LGTM, just want to sure is it possible to merge 
build.sh into entrypoint.sh

- haosdent huang


On June 29, 2017, 9:59 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60439/
> ---
> 
> (Updated June 29, 2017, 9:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-7625
> https://issues.apache.org/jira/browse/MESOS-7625
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These scripts are expected to be run by ASF CI on any commit to
> the mesos repo. The directory layout is similar to tidybot CI job.
> 
> 
> Diffs
> -
> 
>   support/jenkins/websitebot.sh PRE-CREATION 
>   support/mesos-website.sh PRE-CREATION 
>   support/mesos-website/Dockerfile PRE-CREATION 
>   support/mesos-website/build.sh PRE-CREATION 
>   support/mesos-website/entrypoint.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60439/diff/1/
> 
> 
> Testing
> ---
> 
> Tested by running a CI job pointing to a branch containing this patch.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 60439: Added scripts to automate website publishing.

[haosdent huang]

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




support/mesos-website/entrypoint.sh
Lines 32 (patched)


Sorry, just saw `LOCAL_USER_ID`.


- haosdent huang


On June 29, 2017, 9:59 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60439/
> ---
> 
> (Updated June 29, 2017, 9:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-7625
> https://issues.apache.org/jira/browse/MESOS-7625
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These scripts are expected to be run by ASF CI on any commit to
> the mesos repo. The directory layout is similar to tidybot CI job.
> 
> 
> Diffs
> -
> 
>   support/jenkins/websitebot.sh PRE-CREATION 
>   support/mesos-website.sh PRE-CREATION 
>   support/mesos-website/Dockerfile PRE-CREATION 
>   support/mesos-website/build.sh PRE-CREATION 
>   support/mesos-website/entrypoint.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60439/diff/1/
> 
> 
> Testing
> ---
> 
> Tested by running a CI job pointing to a branch containing this patch.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 60439: Added scripts to automate website publishing.

[haosdent huang]

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




support/mesos-website/build.sh
Lines 42-43 (patched)


Should we move `bundle install` to here?


- haosdent huang


On June 29, 2017, 9:59 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60439/
> ---
> 
> (Updated June 29, 2017, 9:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-7625
> https://issues.apache.org/jira/browse/MESOS-7625
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These scripts are expected to be run by ASF CI on any commit to
> the mesos repo. The directory layout is similar to tidybot CI job.
> 
> 
> Diffs
> -
> 
>   support/jenkins/websitebot.sh PRE-CREATION 
>   support/mesos-website.sh PRE-CREATION 
>   support/mesos-website/Dockerfile PRE-CREATION 
>   support/mesos-website/build.sh PRE-CREATION 
>   support/mesos-website/entrypoint.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60439/diff/1/
> 
> 
> Testing
> ---
> 
> Tested by running a CI job pointing to a branch containing this patch.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 60439: Added scripts to automate website publishing.

[haosdent huang]

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




support/mesos-website/entrypoint.sh
Lines 32 (patched)


Is it OK to run support/mesos-website/build.sh as root?


- haosdent huang


On June 29, 2017, 9:59 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60439/
> ---
> 
> (Updated June 29, 2017, 9:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-7625
> https://issues.apache.org/jira/browse/MESOS-7625
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These scripts are expected to be run by ASF CI on any commit to
> the mesos repo. The directory layout is similar to tidybot CI job.
> 
> 
> Diffs
> -
> 
>   support/jenkins/websitebot.sh PRE-CREATION 
>   support/mesos-website.sh PRE-CREATION 
>   support/mesos-website/Dockerfile PRE-CREATION 
>   support/mesos-website/build.sh PRE-CREATION 
>   support/mesos-website/entrypoint.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60439/diff/1/
> 
> 
> Testing
> ---
> 
> Tested by running a CI job pointing to a branch containing this patch.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 60280: Provide full path to the custom executor.

[Aaron Wood via Review Board]

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

(Updated July 3, 2017, 4:18 p.m.)


Review request for mesos, Jie Yu, James Peach, and Zhitao Li.


Changes
---

Fix comment.


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


Repository: mesos


Description
---

If a framework specifies use of its own executor and sets shell to false the 
executor is never found. This provides the full path to the executor so that 
the `execvp` or `execvpe` is successful.


Diffs (updated)
-

  src/slave/containerizer/mesos/launch.cpp 162ca1c2e 


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

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


Testing
---

`cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4`
Also spun up a master and agent, connected and sent a task using the UCR (both 
with and without the use of an OCI image) via our own framework, and checked 
the sandbox to verify that things went accordingly.


Thanks,

Aaron Wood

Re: Review Request 60280: Provide full path to the custom executor.

[Aaron Wood via Review Board]

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

(Updated July 3, 2017, 4 p.m.)


Review request for mesos, Jie Yu, James Peach, and Zhitao Li.


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


Repository: mesos


Description
---

If a framework specifies use of its own executor and sets shell to false the 
executor is never found. This provides the full path to the executor so that 
the `execvp` or `execvpe` is successful.


Diffs (updated)
-

  src/slave/containerizer/mesos/launch.cpp 162ca1c2e 


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

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


Testing
---

`cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4`
Also spun up a master and agent, connected and sent a task using the UCR (both 
with and without the use of an OCI image) via our own framework, and checked 
the sandbox to verify that things went accordingly.


Thanks,

Aaron Wood

Review Request 60598: Replaced abort() with _exit() in ChildHook::SUPERVISOR from subprocess.

[Andrei Budnik]

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

Review request for mesos, Benjamin Bannier, Benjamin Mahler, and James Peach.


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


Repository: mesos


Description
---

Previously, abort() was called in SUPERVISOR hook when child process
exited with an error code, or waitpid() failed, or parent process
exited. All these cases shouldn't lead to abnormal program termination
with coredumps.


Diffs
-

  3rdparty/libprocess/include/process/posix/subprocess.hpp 
a5d10ce49d348b4fe7b2c27c48e624b0a1c31e77 
  3rdparty/libprocess/src/subprocess.cpp 
0f1532b294d6d6b1e017468cfde47362f3faa84d 


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


Testing
---

make check (mac os x, fedora 25)


Thanks,

Andrei Budnik

Re: Review Request 60280: Provide full path to the custom executor.

[Aaron Wood via Review Board]

> On June 30, 2017, 11:48 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Lines 799 (patched)
> > 
> >
> > What if `launchInfo.working_directory()` is not set? Maybe use 
> > os::realpath here to get the absolute path?

Sure, I'll alter that. Is it wrong to assume that the working directory will 
always be set at this point? I thought that without the sandbox work dir no 
container would be able to move forward.


- Aaron


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


On June 22, 2017, 3:20 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60280/
> ---
> 
> (Updated June 22, 2017, 3:20 p.m.)
> 
> 
> Review request for mesos, Jie Yu, James Peach, and Zhitao Li.
> 
> 
> Bugs: MESOS-7703
> https://issues.apache.org/jira/browse/MESOS-7703
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a framework specifies use of its own executor and sets shell to false the 
> executor is never found. This provides the full path to the executor so that 
> the `execvp` or `execvpe` is successful.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 162ca1c2e 
> 
> 
> Diff: https://reviews.apache.org/r/60280/diff/3/
> 
> 
> Testing
> ---
> 
> `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4`
> Also spun up a master and agent, connected and sent a task using the UCR 
> (both with and without the use of an OCI image) via our own framework, and 
> checked the sandbox to verify that things went accordingly.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Review Request 60600: Set container DNS with `--default_container_dns` in CNI isolator.

[Qian Zhang]

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

Review request for mesos, Avinash sridharan and Jie Yu.


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


Repository: mesos


Description
---

Set container DNS with `--default_container_dns` in CNI isolator.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
b386fbed17698dd6c59bc6c53edfe8a7ece6a3eb 


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


Testing
---

sudo make check

1. Start Mesos master.
```
$ sudo ./bin/mesos-master.sh --work_dir=/opt/mesos
```

2. Start Mesos agent.
```
$ sudo ./bin/mesos-slave.sh --master=192.168.122.216:5050 
--containerizers=mesos,docker --image_providers=docker 
--image_provisioner_backend=aufs 
--isolation=filesystem/linux,docker/runtime,network/cni,cgroups/cpu,cgroups/mem 
--network_cni_config_dir=/opt/cni/net_configs 
--network_cni_plugins_dir=/opt/cni/plugins --work_dir=/opt/mesos 
--docker_store_dir=/opt/mesos/store/docker 
--executor_registration_timeout=60mins 
--default_container_dns=file:///home/stack/dns.json

$ cat /opt/cni/net_configs/net1 
{
"name": "net1",
"type": "bridge",
"bridge": "br1",
"isGateway": true,
"ipMasq": true,
"ipam": {
"type": "host-local",
"subnet": "192.168.1.0/24",
"routes": [
{ "dst": "0.0.0.0/0" }
]
}
}

$ cat /opt/cni/net_configs/net2 
{
"name": "net2",
"type": "bridge",
"bridge": "br2",
"isGateway": true,
"ipMasq": true,
"ipam": {
"type": "host-local",
"subnet": "192.168.2.0/24",
"routes": [
{ "dst": "0.0.0.0/0" }
]
},
"dns": {
"nameservers": [ "8.8.4.4" ],
"domain": "net2.com",
"search": [ "yyy.com" ],
"options": [ "attempts:3" ]
}
}

$ cat /home/stack/dns.json
{
  "mesos": [
{
  "network": "net1",
  "dns": {
"nameservers": [ "8.8.8.8" ],
"search": [ "xxx.com" ],
"options": [ "timeout:4" ]
  }
}
  ]
}
```

3. Launch a unified container with `mesos-execute`.
```
$ sudo src/mesos-execute --master=192.168.122.216:5050 
--task=file:///home/stack/task.json

$ cat /home/stack/task.json 
{
  "name": "test",
  "task_id": {"value" : "test"},
  "agent_id": {"value" : ""},
  "resources": [
{
  "name": "cpus",
  "type": "SCALAR",
  "scalar": {
"value": 0.1
  }
},
{
  "name": "mem",
  "type": "SCALAR",
  "scalar": {
"value": 32
  }
}
  ],
  "command": {
"value": "sleep 300"
  },
  "container": {
"type": "MESOS",
"mesos": {
  "image": {
"type": "DOCKER",
"docker": {
  "name": "busybox"
}
  }
},
"network_infos": [
  {
"name": "net1"
  },
  {
"name": "net2"
  }
]
  }
}
```

4. Check the DNS configuration of the unified container.
```
$ ps -ef | grep sleep 
root 20060 20037  2 21:45 ?00:00:00 sh -c sleep 300
root 20074 20060  0 21:45 ?00:00:00 sleep 300

$ sudo nsenter -t 20060 -m -u -n cat /etc/resolv.conf   
domain net2.com
search yyy.com xxx.com
options attempts:3 timeout:4
nameserver 8.8.4.4
nameserver 8.8.8.8
```


Thanks,

Qian Zhang

Re: Review Request 60369: Exposed allocated resources per each role in /state endpoint on agent.

[Benjamin Mahler]

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




src/slave/http.cpp
Lines 1330-1334 (patched)


Whoops, I should have noticed this earlier, but when going to commit this I 
was taking a careful look at the code. This isn't accounting for the pending 
tasks here:


https://github.com/apache/mesos/blob/0d277bb64fa5a4d0b4f741daedf64095beab4773/src/slave/slave.hpp#L904-L905

Unfortunately the logic is a little tricky. We have to include all of the 
pending task resources. Also, for each executor in the pending map, we have to 
add the ExecutorInfo resources only if it's absent from the 'executors' map. 
You'll want to do a read through of how the data structures are used to confirm 
whether I'm right. :)

My suggestion here would be to have an `allocatedResources()` function 
within the `Framework` struct that contains this logic, and here in the http 
handler we just sum `allocatedResources()` across the frameworks.


- Benjamin Mahler


On June 29, 2017, 3:23 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60369/
> ---
> 
> (Updated June 29, 2017, 3:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and haosdent huang.
> 
> 
> Bugs: MESOS-6441
> https://issues.apache.org/jira/browse/MESOS-6441
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The JSON key for this information is "reserved_resources_allocated"
> and "unreserved_resources_allocated".
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp cbbc1dc27cc90bac8d48cbbc84266c3d87490a3c 
> 
> 
> Diff: https://reviews.apache.org/r/60369/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60397: Check perf version compatibility in tests with disabled coredumps.

[Andrei Budnik]

> On June 27, 2017, 5:44 a.m., Benjamin Mahler wrote:
> > src/tests/environment.cpp
> > Line 542 (original), 542-548 (patched)
> > 
> >
> > Shouldn't this just be covered by `perf::suppported`?
> > 
> > Would also be great to include some additional context here, like a 
> > pointer to the ticket or mentioning that this is more likely if running 
> > within a docker image?
> 
> Andrei Budnik wrote:
> Ticket is [MESOS-7160](https://issues.apache.org/jira/browse/MESOS-7160).
> 
> What I've got from the description of the problem is that `perf 
> --version` segfaults on an incompatible kernel version.
> The solution for this problem is provided in this patch. Then, we have 
> found that it is more likely caused by incorrect `os::subprocess()` behavior. 
> So, I have suspicion that `perf --version` might never really cause 
> segfaults. I'm going to fix `os::subprocess()` and send a new patch. 
> Afterwards, if the problem reproduces after the fix, then I'll continue to 
> work on this patch.
> 
> `perf::suppported()` invokes `os::subprocess()` which calls `abort()` 
> when path to a binary is invalid. SIGABRT causes coredumps.
> 
> James Peach wrote:
> Both of these are true :) `perf` will crash with kernel version 
> mismatches, and I strongly suspect there is a race in the process supervisor 
> that can cause a similar symptom when `perf` is not installed.
> 
> Benjamin Mahler wrote:
> I think my comment was misunderstood, I'm asking why we wouldn't have 
> perf::supported handle this correctly, rather than every caller having to 
> write logic to defend against an implementation issue in perf::supported.

I agree with you. Fixed.


- Andrei


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


On July 3, 2017, 10:57 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60397/
> ---
> 
> (Updated July 3, 2017, 10:57 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James Peach, and Neil Conway.
> 
> 
> Bugs: MESOS-7160
> https://issues.apache.org/jira/browse/MESOS-7160
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools build, the docker-build script performs a 'distcheck'
> build. This type of build warns if any unexpected files are left in
> the build directory after an uninstall, mainly to detect broken
> uninstall Makefile rules. The return status of the build container is
> the result of the distcheck.
> This fixes an issue where in some dockerized configurations
> invocations of 'perf' segfaulted (producing a core file as a
> side-effect), where the failure case was already anticipated and
> handled by the caller.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp b301e25d5cac6488c57e4f983e4867c72368a040 
>   src/tests/containerizer/perf_tests.cpp 
> d8aab08eb131f974821fb85662cbc6cc685d2f3e 
> 
> 
> Diff: https://reviews.apache.org/r/60397/diff/3/
> 
> 
> Testing
> ---
> 
> 1. make check (mac os x 10.12, fedora 25)
> 2. internal CI
> 
> Needs to be confirmed by Apache CI, e.g., reviewbot.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60397: Check perf version compatibility in tests with disabled coredumps.

[Andrei Budnik]

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

(Updated July 3, 2017, 10:57 a.m.)


Review request for mesos, Benjamin Bannier, James Peach, and Neil Conway.


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


Repository: mesos


Description
---

For autotools build, the docker-build script performs a 'distcheck'
build. This type of build warns if any unexpected files are left in
the build directory after an uninstall, mainly to detect broken
uninstall Makefile rules. The return status of the build container is
the result of the distcheck.
This fixes an issue where in some dockerized configurations
invocations of 'perf' segfaulted (producing a core file as a
side-effect), where the failure case was already anticipated and
handled by the caller.


Diffs (updated)
-

  src/linux/perf.cpp b301e25d5cac6488c57e4f983e4867c72368a040 
  src/tests/containerizer/perf_tests.cpp 
d8aab08eb131f974821fb85662cbc6cc685d2f3e 


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

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


Testing
---

1. make check (mac os x 10.12, fedora 25)
2. internal CI

Needs to be confirmed by Apache CI, e.g., reviewbot.


Thanks,

Andrei Budnik

Review Request 60594: Add a`network/ports` isolator nested container test.

[James Peach]

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

Review request for mesos.


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


Repository: mesos


Description
---

Add a test to verify that the `network/ports` isolator works
correctly with nested containers.


Diffs
-

  src/tests/containerizer/network_ports_isolator_tests.cpp PRE-CREATION 


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


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Review Request 60593: Test the `network/ports` isolator recovery.

[James Peach]

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

Review request for mesos.


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


Repository: mesos


Description
---

Add a test for the `network/ports` isolator recovery by starting a
task that is listening on a rogue port. We only configure the
isolator when we restart the agent to simulate the case where a
task only starts to misbehave after an agent recovery.


Diffs
-

  src/tests/containerizer/network_ports_isolator_tests.cpp PRE-CREATION 


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


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Review Request 60592: Configure the `network/ports` isolator watch interval.

[James Peach]

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

Review request for mesos.


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


Repository: mesos


Description
---

Add the `--container_ports_watch_interval` option to tune the
interval at which the `network/ports` isolator scans for rogue
listening ports.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 
  src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 
  src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 
  src/tests/containerizer/network_ports_isolator_tests.cpp PRE-CREATION 


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


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Review Request 60591: Optionally isolate only the agent network ports.

[James Peach]

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

Review request for mesos.


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


Repository: mesos


Description
---

Normally, the `network/ports` isolator will kill any task that
listens on a port that it does not have resources for. However,
executors that are based on the libprocess API will always listen
on a port in the ephemeral range, and we want to make it possible
to use libprocess-based executors.

Add the `--container_ports_watch_resources_only` option to only
kill tasks when they listen on un-allocated ports within the port
range published by the agent resources. This still prevents port
collisions between tasks, but doesn't kill them just because the
executor is listening on a port.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 
  src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 
  src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 
  src/tests/containerizer/network_ports_isolator_tests.cpp PRE-CREATION 


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


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 60107: Added filtering to the '/tasks' endpoint.

[Alexander Rojas]

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




src/common/http.hpp
Lines 181 (patched)


Better even, keep a raw pointer the same way `Master::Http` keeps a 
reference to `Master`.


- Alexander Rojas


On July 1, 2017, 1:57 a.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60107/
> ---
> 
> (Updated July 1, 2017, 1:57 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added filtering to the '/tasks' endpoint.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
>   src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 
>   src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
>   src/tests/master_tests.cpp 6cd4be7e5ba48c6562ce91b28e76b065522cfbea 
> 
> 
> Diff: https://reviews.apache.org/r/60107/diff/15/
> 
> 
> Testing
> ---
> 
> Passed "make check"
> Passed "GTEST_FILTER="MasterTest.TasksEndpoint" make check -j48"
> Passed "GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="MasterTest.TasksEndpoint" --gtest_repeat=1000 
> --gtest_break_on_failure"
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60279: Add constructor for ObjectApprover::Object.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On June 22, 2017, 10:22 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60279/
> ---
> 
> (Updated June 22, 2017, 10:22 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add constructor for ObjectApprover::Object.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> 95cbcf3f9a962a896c9c23e961f02eb16dff36f8 
>   src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 
>   src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
>   src/slave/http.cpp cbbc1dc27cc90bac8d48cbbc84266c3d87490a3c 
> 
> 
> Diff: https://reviews.apache.org/r/60279/diff/2/
> 
> 
> Testing
> ---
> 
> Passed "make check -j48"
> No specific test case related.
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60107: Added filtering to the '/tasks' endpoint.

[Alexander Rojas]

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



I think the acceptor files do not belong in the `common/http.?pp` files. 
Perhaps create a new header called acceptors?


src/common/http.hpp
Lines 163-171 (patched)


This class is not necessary. The acceptor as a concept exist, but we don't 
use any of the advantages of inheritance, I think this would be served better 
in a namespace.



src/common/http.hpp
Lines 175-229 (patched)


What is the point of having multiple different `AuthorizationAcceptors`? 
Each have a method with a different signature, so I can see them merge into one 
common acceptor.



src/common/http.hpp
Lines 181 (patched)


This should be a const reference, we don't want to copy `Owned` pointers.


- Alexander Rojas


On July 1, 2017, 1:57 a.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60107/
> ---
> 
> (Updated July 1, 2017, 1:57 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added filtering to the '/tasks' endpoint.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
>   src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 
>   src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
>   src/tests/master_tests.cpp 6cd4be7e5ba48c6562ce91b28e76b065522cfbea 
> 
> 
> Diff: https://reviews.apache.org/r/60107/diff/15/
> 
> 
> Testing
> ---
> 
> Passed "make check"
> Passed "GTEST_FILTER="MasterTest.TasksEndpoint" make check -j48"
> Passed "GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="MasterTest.TasksEndpoint" --gtest_repeat=1000 
> --gtest_break_on_failure"
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>