Re: Review Request 60675: Fixed compilation failure in port-mapping network isolator.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [60674, 60675]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On July 6, 2017, 5:03 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60675/
> ---
> 
> (Updated July 6, 2017, 5:03 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-7762
> https://issues.apache.org/jira/browse/MESOS-7762
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed compilation failure in port-mapping network isolator.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/filter/icmp.hpp bc0aed0f4fc7c48f06cece14aa9ec43d7fa36bc4 
>   src/linux/routing/route.hpp 01d4e7582b19a38bc39f8c394121fe0bed2c0d87 
>   src/linux/routing/route.cpp ceeaf8e557a88569e1f15b3ffc5f0746b66ea638 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> 9d38289c7161d5e931053b587d115684ccc44c94 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> cd008aaebcd42554a9a81d2b059269546f59c966 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 16e015a8ac53a4aa5336b60c40228720b5f6910a 
> 
> 
> Diff: https://reviews.apache.org/r/60675/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 58394: Setup new directory for python http client lib in src/python.

[Armand Grillet]

> On July 5, 2017, 6:02 p.m., Eric Chung wrote:
> > bump

FYI the Mesos Reviewbot Windows reports a bad patch due to an error in 
`apply-reviews.py`, I doubt there is an issue with your review request. A fix 
has been merged a few hours ago: 
https://github.com/apache/mesos/commit/01fd1355cb3cd9b0f01bd042fa3d63d739468be2.


- Armand


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


On June 29, 2017, 6:22 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58394/
> ---
> 
> (Updated June 29, 2017, 6:22 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, Joseph Wu, and Kevin 
> Klues.
> 
> 
> Bugs: MESOS-7310
> https://issues.apache.org/jira/browse/MESOS-7310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Part of MESOS-7310, this patch adds a new directory (src/python/lib/mesos) 
> for placing the http client package, which will be importable via `import 
> mesos` from the cli.
> 
> Review: https://reviews.apache.org/r/58394
> 
> 
> Diffs
> -
> 
>   src/cli_new/bootstrap 6d62e9adf1d543ed00a3a2cf2484edf1c33ee443 
>   src/python/.gitignore PRE-CREATION 
>   src/python/lib/mesos/__init__.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/58394/diff/4/
> 
> 
> Testing
> ---
> 
> Under src/cli_new:
> 1\. ./bootstrap
> 2\. . ./activate
> 3\. python
> 4\. >>> import mesos
> 5\. >>> mesos.\_\_path\_\_
> 6\. verify that the path printed out is indeed at src/python/lib/mesos
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 60675: Fixed compilation failure in port-mapping network isolator.

[Avinash sridharan]

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

(Updated July 6, 2017, 5:03 a.m.)


Review request for mesos and Joseph Wu.


Changes
---

Fixed another missing `IPNetwork` instance.


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


Repository: mesos


Description
---

Fixed compilation failure in port-mapping network isolator.


Diffs (updated)
-

  src/linux/routing/filter/icmp.hpp bc0aed0f4fc7c48f06cece14aa9ec43d7fa36bc4 
  src/linux/routing/route.hpp 01d4e7582b19a38bc39f8c394121fe0bed2c0d87 
  src/linux/routing/route.cpp ceeaf8e557a88569e1f15b3ffc5f0746b66ea638 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
9d38289c7161d5e931053b587d115684ccc44c94 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
cd008aaebcd42554a9a81d2b059269546f59c966 
  src/tests/containerizer/port_mapping_tests.cpp 
16e015a8ac53a4aa5336b60c40228720b5f6910a 


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

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


Testing
---

make check


Thanks,

Avinash sridharan

Re: Review Request 60675: Fixed compilation failure in port-mapping network isolator.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [60674, 60675]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On July 6, 2017, 4:11 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60675/
> ---
> 
> (Updated July 6, 2017, 4:11 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-7762
> https://issues.apache.org/jira/browse/MESOS-7762
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed compilation failure in port-mapping network isolator.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/filter/icmp.hpp bc0aed0f4fc7c48f06cece14aa9ec43d7fa36bc4 
>   src/linux/routing/route.hpp 01d4e7582b19a38bc39f8c394121fe0bed2c0d87 
>   src/linux/routing/route.cpp ceeaf8e557a88569e1f15b3ffc5f0746b66ea638 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> 9d38289c7161d5e931053b587d115684ccc44c94 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> cd008aaebcd42554a9a81d2b059269546f59c966 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 16e015a8ac53a4aa5336b60c40228720b5f6910a 
> 
> 
> Diff: https://reviews.apache.org/r/60675/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 60674: Fixed windows failure due to new `net::IP::Network` class.

[Avinash sridharan]

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

(Updated July 6, 2017, 4:12 a.m.)


Review request for mesos and Joseph Wu.


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


Repository: mesos


Description (updated)
---

Fixed windows build failure due to new `net::IP::Network` class.


Diffs
-

  3rdparty/stout/include/stout/windows/ip.hpp 
e2ed99e74bc4338c3945b89666dbd0d0361e80e1 


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


Testing
---

make check


Thanks,

Avinash sridharan

Review Request 60674: Fixed windows failure due to new `net::IP::Network` class.

[Avinash sridharan]

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

Review request for mesos and Joseph Wu.


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


Repository: mesos


Description
---

Fixed windows failure due to new `net::IP::Network` class.


Diffs
-

  3rdparty/stout/include/stout/windows/ip.hpp 
e2ed99e74bc4338c3945b89666dbd0d0361e80e1 


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


Testing
---

make check


Thanks,

Avinash sridharan

Review Request 60675: Fixed compilation failure in port-mapping network isolator.

[Avinash sridharan]

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

Review request for mesos and Joseph Wu.


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


Repository: mesos


Description
---

Fixed compilation failure in port-mapping network isolator.


Diffs
-

  src/linux/routing/filter/icmp.hpp bc0aed0f4fc7c48f06cece14aa9ec43d7fa36bc4 
  src/linux/routing/route.hpp 01d4e7582b19a38bc39f8c394121fe0bed2c0d87 
  src/linux/routing/route.cpp ceeaf8e557a88569e1f15b3ffc5f0746b66ea638 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
9d38289c7161d5e931053b587d115684ccc44c94 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
cd008aaebcd42554a9a81d2b059269546f59c966 
  src/tests/containerizer/port_mapping_tests.cpp 
16e015a8ac53a4aa5336b60c40228720b5f6910a 


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


Testing
---

make check


Thanks,

Avinash sridharan

Re: Review Request 60585: Add Linux capabilities changes to the CHANGELOG.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On July 3, 2017, 1:21 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60585/
> ---
> 
> (Updated July 3, 2017, 1:21 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add Linux capabilities changes to the CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG f1d350b6f1f19c6a293b9ff29ac901fb6b228265 
> 
> 
> Diff: https://reviews.apache.org/r/60585/diff/1/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60584: Fix bullet-point formatting.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On July 3, 2017, 1:20 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60584/
> ---
> 
> (Updated July 3, 2017, 1:20 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Markdown bullet-points need a blank like after the link target so
> they render as bullets rather than asterisks.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 60d55259d5bed5dbdad5c7f79e7d998af425b0b1 
> 
> 
> Diff: https://reviews.apache.org/r/60584/diff/1/
> 
> 
> Testing
> ---
> 
> Manual inspection of website.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60583: Add upgrade notes for Linux capabilities changes.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On July 3, 2017, 1:20 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60583/
> ---
> 
> (Updated July 3, 2017, 1:20 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add upgrade notes for Linux capabilities changes.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 60d55259d5bed5dbdad5c7f79e7d998af425b0b1 
> 
> 
> Diff: https://reviews.apache.org/r/60583/diff/1/
> 
> 
> Testing
> ---
> 
> Manual inspection of website.
> 
> 
> Thanks,
> 
> James Peach
> 
>

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

[Jie Yu]

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



This will break the windows build. os::which is only in posix header.

I would ask in the #windows channel see if we can port os::which to be on both 
windows and posix (sounds very doable).


src/slave/containerizer/mesos/launch.cpp
Lines 799-814 (patched)


I will do the following:
```
// Search executable in the current working directory as well.
// `execvpe` and `execvp` will only search executable from the
// current working directory if environment variable `PATH` is
// not set.
if (!path::absolute(executable) &&
launchInfo.has_working_directory()) {
  Option which = os::which(
  executable,
  launchInfo.working_directory());

  if (which.isSome()) {
executable = which.get();
  }
}
```



src/slave/containerizer/mesos/launch.cpp
Lines 808 (patched)


This line is longger than 80 chars:

```
Jies-MacBook-Pro:mesos jie$ support/apply-review.sh -r 60280
2017-07-05 19:39:04 URL:https://reviews.apache.org/r/60280/diff/raw/ 
[1397/1397] -> "60280.patch" [1]
Checking 1 C++ file
src/slave/containerizer/mesos/launch.cpp:803:  Lines should be <= 80 
characters long  [whitespace/line_length] [2]
src/slave/containerizer/mesos/launch.cpp:808:  Lines should be <= 80 
characters long  [whitespace/line_length] [2]
Total errors found: 1
No Python files to lint
```

ALso, we need to test if working_directory is set or not.



src/slave/containerizer/mesos/launch.cpp
Lines 810 (patched)


indentation here should be 2


- Jie Yu


On July 5, 2017, 9:03 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60280/
> ---
> 
> (Updated July 5, 2017, 9:03 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/7/
> 
> 
> 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 60551: Added a test for docker container running on a v4/6 host network.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [60551, 59233, 59130, 60149, 59131, 60148, 60136, 59721, 
59739, 59688, 59128, 59127]

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

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 158, in apply_review
fetch_patch(options)
  File "support/apply-reviews.py", line 215, in fetch_patch
shell(cmd, options['dry_run'])
UnboundLocalError: local variable 'cmd' referenced before assignment

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/125/console

- Mesos Reviewbot Windows


On June 29, 2017, 11:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60551/
> ---
> 
> (Updated June 29, 2017, 11:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test for docker container running on a v4/6 host network.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> cf9470245ebc81e799fb2b2a67464298564fc56f 
>   src/tests/mesos.hpp eac2c96985cdbbf1a50cfc054862eae2d44fbfcd 
>   src/tests/mesos.cpp d3e6bc0a92ec9b410712891c6a02d01ed2b587c1 
> 
> 
> Diff: https://reviews.apache.org/r/60551/diff/2/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 60551: Added a test for docker container running on a v4/6 host network.

[Benjamin Hindman]

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


Fix it, then Ship it!





src/tests/containerizer/docker_containerizer_tests.cpp
Lines 4348 (patched)


You can kill the `;` at the end.



src/tests/containerizer/docker_containerizer_tests.cpp
Lines 4350 (patched)


You can kill the `;` at the end.



src/tests/containerizer/docker_containerizer_tests.cpp
Lines 4367-4370 (patched)


What if the host doesn't have both networks? Do we need more checks in 
place to keep this from being run on all machines that just have `ROOT_DOCKER`?



src/tests/mesos.hpp
Lines 248 (patched)


Comment why this class exists.

Also, why doesn't this extend from the test fixture class?

I think we should just factor this into the 
`DockerContainerizerIPv6Test::SetUp` and 
`DockerContainerizerIPv6Test::TearDown` below for now and also add a comment 
that we eventually want to pull this out into it's own class.


- Benjamin Hindman


On June 29, 2017, 11:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60551/
> ---
> 
> (Updated June 29, 2017, 11:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test for docker container running on a v4/6 host network.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> cf9470245ebc81e799fb2b2a67464298564fc56f 
>   src/tests/mesos.hpp eac2c96985cdbbf1a50cfc054862eae2d44fbfcd 
>   src/tests/mesos.cpp d3e6bc0a92ec9b410712891c6a02d01ed2b587c1 
> 
> 
> Diff: https://reviews.apache.org/r/60551/diff/2/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 60546: Harden Mesos when building with cmake.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [60546]

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

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 158, in apply_review
fetch_patch(options)
  File "support/apply-reviews.py", line 215, in fetch_patch
shell(cmd, options['dry_run'])
UnboundLocalError: local variable 'cmd' referenced before assignment

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/125/console

- Mesos Reviewbot Windows


On July 5, 2017, 9:06 a.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60546/
> ---
> 
> (Updated July 5, 2017, 9:06 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-7737
> https://issues.apache.org/jira/browse/MESOS-7737
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Apply stack protectors (stronger stack protectors with newer compilers), 
> position independent code suitable for linking into executables, security 
> warnings, and generate position independent executables.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 3fa2e2f3b 
>   cmake/MesosConfigure.cmake 5ecad2c0f 
> 
> 
> Diff: https://reviews.apache.org/r/60546/diff/2/
> 
> 
> Testing
> ---
> 
> `mkdir build && cd build && cmake .. && make -j$(nproc) && make check 
> -j$(nproc)`
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 59921: Added agent domain to Offer message.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [59921, 59766, 59764, 59763, 59762, 59761, 59760, 59759]

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

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 158, in apply_review
fetch_patch(options)
  File "support/apply-reviews.py", line 215, in fetch_patch
shell(cmd, options['dry_run'])
UnboundLocalError: local variable 'cmd' referenced before assignment

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/125/console

- Mesos Reviewbot Windows


On July 5, 2017, 10:32 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59921/
> ---
> 
> (Updated July 5, 2017, 10:32 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7644
> https://issues.apache.org/jira/browse/MESOS-7644
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a convenience mechanism to allow frameworks to determine the
> domain of an agent when they receive a resource offer.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 4e4b2790615e8bbbf262ba809cbfd67e24368952 
>   include/mesos/v1/mesos.proto b8625d363388bc707bc2458abd47a218cc7b7888 
>   src/master/master.cpp 56b170ed70722eba30d98f6e648b6a31580d6b56 
>   src/tests/master_tests.cpp c778c6c56d47c4033189912cebee6024be79106f 
> 
> 
> Diff: https://reviews.apache.org/r/59921/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

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

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [60280]

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

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 158, in apply_review
fetch_patch(options)
  File "support/apply-reviews.py", line 215, in fetch_patch
shell(cmd, options['dry_run'])
UnboundLocalError: local variable 'cmd' referenced before assignment

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/125/console

- Mesos Reviewbot Windows


On July 5, 2017, 9:03 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60280/
> ---
> 
> (Updated July 5, 2017, 9:03 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/7/
> 
> 
> 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 58394: Setup new directory for python http client lib in src/python.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [58394]

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

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 158, in apply_review
fetch_patch(options)
  File "support/apply-reviews.py", line 215, in fetch_patch
shell(cmd, options['dry_run'])
UnboundLocalError: local variable 'cmd' referenced before assignment

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/125/console

- Mesos Reviewbot Windows


On June 29, 2017, 6:22 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58394/
> ---
> 
> (Updated June 29, 2017, 6:22 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, Joseph Wu, and Kevin 
> Klues.
> 
> 
> Bugs: MESOS-7310
> https://issues.apache.org/jira/browse/MESOS-7310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Part of MESOS-7310, this patch adds a new directory (src/python/lib/mesos) 
> for placing the http client package, which will be importable via `import 
> mesos` from the cli.
> 
> Review: https://reviews.apache.org/r/58394
> 
> 
> Diffs
> -
> 
>   src/cli_new/bootstrap 6d62e9adf1d543ed00a3a2cf2484edf1c33ee443 
>   src/python/.gitignore PRE-CREATION 
>   src/python/lib/mesos/__init__.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/58394/diff/4/
> 
> 
> Testing
> ---
> 
> Under src/cli_new:
> 1\. ./bootstrap
> 2\. . ./activate
> 3\. python
> 4\. >>> import mesos
> 5\. >>> mesos.\_\_path\_\_
> 6\. verify that the path printed out is indeed at src/python/lib/mesos
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 60647: Fixed the mention of default ports in 'configuration.md'.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [60647]

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

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 158, in apply_review
fetch_patch(options)
  File "support/apply-reviews.py", line 215, in fetch_patch
shell(cmd, options['dry_run'])
UnboundLocalError: local variable 'cmd' referenced before assignment

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/125/console

- Mesos Reviewbot Windows


On July 5, 2017, 9:15 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60647/
> ---
> 
> (Updated July 5, 2017, 9:15 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 007d0f556375f331148c9ad09be8b301ab31a01c 
> 
> 
> Diff: https://reviews.apache.org/r/60647/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

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

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [60580]

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

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 158, in apply_review
fetch_patch(options)
  File "support/apply-reviews.py", line 215, in fetch_patch
shell(cmd, options['dry_run'])
UnboundLocalError: local variable 'cmd' referenced before assignment

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/125/console

- Mesos Reviewbot Windows


On July 5, 2017, 4:30 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60580/
> ---
> 
> (Updated July 5, 2017, 4:30 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 '/frameworks' endpoint.
> 
> Add query parameter support for '/frameworks' endpoint.
> To query the information about a specific framework, we
> can use 'framework_id' to specify the framework, thus it
> looks like'/frameworks?framework_id=[framework id]'
> 
> To maintain consistency, the structure for response is
> reserved, thus the response still contains field 'frameworks'
> and 'completed_frameworks' etc..
> 
> Since code for '/state' endpoint and '/frameworks' are coupled
> (they share 'FullFrameworkWriter' class), thus the code for '/state'
> endpoint is also updated.
> 
> 
> Diffs
> -
> 
>   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/
> 
> 
> 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
> 
>

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

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [60598]

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

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 158, in apply_review
fetch_patch(options)
  File "support/apply-reviews.py", line 215, in fetch_patch
shell(cmd, options['dry_run'])
UnboundLocalError: local variable 'cmd' referenced before assignment

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/125/console

- Mesos Reviewbot Windows


On July 3, 2017, 2:58 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60598/
> ---
> 
> (Updated July 3, 2017, 2:58 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, 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/src/subprocess.cpp 
> 0f1532b294d6d6b1e017468cfde47362f3faa84d 
> 
> 
> Diff: https://reviews.apache.org/r/60598/diff/2/
> 
> 
> Testing
> ---
> 
> make check (mac os x, fedora 25)
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 60346: Improved the documentation of 'TASK_LOST'.

[Neil Conway]

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


Ship it!





include/mesos/mesos.proto
Lines 1940 (patched)


We don't support the "strict-registry" flag anymore, so I removed the 
reference to this before committing.


- Neil Conway


On June 21, 2017, 10:17 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60346/
> ---
> 
> (Updated June 21, 2017, 10:17 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-7662
> https://issues.apache.org/jira/browse/MESOS-7662
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the protobuf comments to clarify that 'TASK_LOST' is not
> always a terminal state.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 38449da44a5288afc7e2ef17ef5e1380ecabd5d2 
>   include/mesos/v1/mesos.proto 81eca47ce1074f5e522a0410f01ab2b2cfc76aa2 
> 
> 
> Diff: https://reviews.apache.org/r/60346/diff/1/
> 
> 
> Testing
> ---
> 
> None, this is not functional change.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

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

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [60581, 60107]

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

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 158, in apply_review
fetch_patch(options)
  File "support/apply-reviews.py", line 215, in fetch_patch
shell(cmd, options['dry_run'])
UnboundLocalError: local variable 'cmd' referenced before assignment

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/125/console

- Mesos Reviewbot Windows


On July 5, 2017, 9:28 a.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60581/
> ---
> 
> (Updated July 5, 2017, 9:28 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 '/slaves' endpoint.
> 
> Added query parameter support for 'slaves'
> endpoint. We can use 'slave_id' to specify
> which slave information to query. Thus it
> looks like '/slaves?slave_id=[slave id]'
> 
> If not slave id is specified, it will return
> information of all slaves, just like before
> 
> The structure for response is the same for both
> specifying or not specifying slave id. Thus even
> for request with slave id specified, the response
> will still contain both fields 'slaves', 
> 'recovered_slaves' etc..
> 
> 
> Diffs
> -
> 
>   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/
> 
> 
> 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 59131: Added an IPv6 address storage to UPID.

[Benjamin Hindman]

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




3rdparty/libprocess/include/process/pid.hpp
Lines 97 (patched)


```
struct
{
#ifdef __WINDOWS__
  Option unix;
#endif // __WINDOWS__
  Option v4;
  Option v6;
} address;
```

?


- Benjamin Hindman


On June 29, 2017, 11:23 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59131/
> ---
> 
> (Updated June 29, 2017, 11:23 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an IPv6 address storage to UPID.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/pid.hpp 
> c634916a37f570194945b9c7d7b786ffeae0408a 
>   3rdparty/libprocess/src/pid.cpp 023f881841f466d91243d2773961b67f82da4e91 
> 
> 
> Diff: https://reviews.apache.org/r/59131/diff/5/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

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

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [59989, 59988, 59987, 60109]

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

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 158, in apply_review
fetch_patch(options)
  File "support/apply-reviews.py", line 215, in fetch_patch
shell(cmd, options['dry_run'])
UnboundLocalError: local variable 'cmd' referenced before assignment

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/125/console

- Mesos Reviewbot Windows


On July 3, 2017, 7:44 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59989/
> ---
> 
> (Updated July 3, 2017, 7:44 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Zhitao Li.
> 
> 
> Bugs: MESOS-7656
> https://issues.apache.org/jira/browse/MESOS-7656
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `ProtobufTest.JsonifyMap`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/protobuf_tests.cpp 
> 8877e8934e0f7875bfedcfa88b491ce4b13ca44f 
> 
> 
> Diff: https://reviews.apache.org/r/59989/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60511: Added MockMetadataManager and test concurrent prune and pull.

[Andrew Schwartzmeyer]

> On June 28, 2017, 10:37 p.m., Mesos Reviewbot Windows wrote:
> > Bad patch!
> > 
> > Reviews applied: [60511, 56722, 60471, 56721, 59705, 55727, 59687, 55335, 
> > 55334]
> > 
> > Failed command: python support/apply-reviews.py -n -r 55334
> > 
> > Error:
> > Traceback (most recent call last):
> >   File "support/apply-reviews.py", line 417, in 
> > main()
> >   File "support/apply-reviews.py", line 412, in main
> > reviewboard(options)
> >   File "support/apply-reviews.py", line 402, in reviewboard
> > apply_review(options)
> >   File "support/apply-reviews.py", line 158, in apply_review
> > fetch_patch(options)
> >   File "support/apply-reviews.py", line 194, in fetch_patch
> > patch.write(patch.read())
> > IOError: File not open for reading
> > 
> > Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/124/console

My bad, though I'd set up the bot to pull master each time it built... I had 
the remote named incorrectly. This should be resolved now.


- Andrew


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


On June 28, 2017, 6:48 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60511/
> ---
> 
> (Updated June 28, 2017, 6:48 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jason Lai.
> 
> 
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added MockMetadataManager and test concurrent prune and pull.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 954da1681778878c8aff6150002e52ecb648d1bb 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> d86afd2a6ff0bf87e624db1c99255c85068bf6ab 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 
> 1cf68665d33bd40a7605d26c96fb7b618407fdd0 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 8058dcb31b9de59fa8d3638b553632235542 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 920be77b16178a4458d72145020c015130799ec4 
> 
> 
> Diff: https://reviews.apache.org/r/60511/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 60546: Harden Mesos when building with cmake.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On July 5, 2017, 4:06 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60546/
> ---
> 
> (Updated July 5, 2017, 4:06 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-7737
> https://issues.apache.org/jira/browse/MESOS-7737
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Apply stack protectors (stronger stack protectors with newer compilers), 
> position independent code suitable for linking into executables, security 
> warnings, and generate position independent executables.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 3fa2e2f3b 
>   cmake/MesosConfigure.cmake 5ecad2c0f 
> 
> 
> Diff: https://reviews.apache.org/r/60546/diff/2/
> 
> 
> Testing
> ---
> 
> `mkdir build && cd build && cmake .. && make -j$(nproc) && make check 
> -j$(nproc)`
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 60646: Modifed handling of parallel test configure flag for documentation.

[Mesos Reviewbot Windows]

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



Bad patch!

Reviews applied: [60646, 6]

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

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 158, in apply_review
fetch_patch(options)
  File "support/apply-reviews.py", line 215, in fetch_patch
shell(cmd, options['dry_run'])
UnboundLocalError: local variable 'cmd' referenced before assignment

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/125/console

- Mesos Reviewbot Windows


On July 5, 2017, 7:43 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60646/
> ---
> 
> (Updated July 5, 2017, 7:43 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifed handling of parallel test configure flag for documentation.
> 
> 
> Diffs
> -
> 
>   configure.ac 4d7c4a4679e5c624ee750226d542e0d8c228507a 
> 
> 
> Diff: https://reviews.apache.org/r/60646/diff/1/
> 
> 
> Testing
> ---
> 
> configure and build stout with different settings 
> (`--enable-parallel-test-execution`, `--enable-parallel-test-execution=yes`, 
> `--enable-parallel-test-execution=no`, `--disable-parallel-test-execution`), 
> `make check`.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 59762: Added domain to MasterInfo and SlaveInfo.

[Vinod Kone]

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


Fix it, then Ship it!





include/mesos/mesos.proto
Lines 869 (patched)


Can you also add text here saying all masters should belong to the same 
region?



include/mesos/v1/mesos.proto
Lines 868 (patched)


ditto.



src/tests/slave_tests.cpp
Line 7478 (original), 7478 (patched)


2 new lines.



src/tests/slave_tests.cpp
Lines 7480 (patched)


s/changed/configured/ ?



src/tests/slave_tests.cpp
Lines 7506 (patched)


s/slave/agent/ ?


- Vinod Kone


On July 5, 2017, 10:32 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59762/
> ---
> 
> (Updated July 5, 2017, 10:32 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7610
> https://issues.apache.org/jira/browse/MESOS-7610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This means that each master's domain is stored in ZooKeeper, along with
> the rest of the MasterInfo protobuf message.
> 
> Each agent's domain is stored as part of its checkpointed resources.
> Changing the agent's domain requires a full drain of the agent; this
> behavior might be relaxed in the future.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 4e4b2790615e8bbbf262ba809cbfd67e24368952 
>   include/mesos/v1/mesos.proto b8625d363388bc707bc2458abd47a218cc7b7888 
>   src/common/http.hpp b7e4a8adcbcaa3a962af795c67694a35161b6c1a 
>   src/common/http.cpp fdb591eccf273260902f3f695cf431f72ee3d817 
>   src/common/type_utils.cpp aeb16238e9c3fd71a8e9e57dbb8098ddc004e1f2 
>   src/internal/evolve.hpp 9db5fe6155243576f186a8b974e81068505b9fcb 
>   src/internal/evolve.cpp 93196f301e820b99572ee008b98a124ddafe9697 
>   src/master/http.cpp 175a44ce7fb5be509453c25eaa9ec29f35adba3a 
>   src/master/master.cpp 56b170ed70722eba30d98f6e648b6a31580d6b56 
>   src/slave/http.cpp 700871e1502a65b0bb1fc31219e09219dbdb5340 
>   src/slave/slave.cpp 0e24b8cb8d1020af515e3d1862e121e1daf82ce9 
>   src/tests/api_tests.cpp cdaa72462eac1687185115771265539d8c2b09a9 
>   src/tests/master_tests.cpp c778c6c56d47c4033189912cebee6024be79106f 
>   src/tests/mesos.hpp eac2c96985cdbbf1a50cfc054862eae2d44fbfcd 
>   src/tests/slave_tests.cpp 8a69cc2ede0b2f17a31986e9142aa2081691eb5e 
>   src/v1/mesos.cpp 423510ef14025dba208ef85edf5305c2ce58f01d 
> 
> 
> Diff: https://reviews.apache.org/r/59762/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 59761: Added master and agent flags to specify domain.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On July 5, 2017, 10:31 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59761/
> ---
> 
> (Updated July 5, 2017, 10:31 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7610
> https://issues.apache.org/jira/browse/MESOS-7610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added master and agent flags to specify domain.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 007d0f556375f331148c9ad09be8b301ab31a01c 
>   src/common/parse.hpp 64eabf89f81a6708cfac89081e0d9f9586721693 
>   src/master/flags.hpp 8a9bd2dfa513f1de85dab2cc3cc1d78e80293837 
>   src/master/flags.cpp ca5f02c728a9f39abbbee59fa169f95d20c69d3d 
>   src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 
>   src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 
> 
> 
> Diff: https://reviews.apache.org/r/59761/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 59760: Added REGION_AWARE framework capability.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On July 5, 2017, 10:31 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59760/
> ---
> 
> (Updated July 5, 2017, 10:31 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7609
> https://issues.apache.org/jira/browse/MESOS-7609
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added REGION_AWARE framework capability.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 4e4b2790615e8bbbf262ba809cbfd67e24368952 
>   include/mesos/v1/mesos.proto b8625d363388bc707bc2458abd47a218cc7b7888 
>   src/common/protobuf_utils.hpp 5476d2e429cebc649405504f02169de318f65fd6 
>   src/tests/protobuf_utils_tests.cpp 6ec3a8e606e86406a0f1c9d5de4ed09d0811e49a 
> 
> 
> Diff: https://reviews.apache.org/r/59760/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 59759: Added protobuf definitions for fault domains.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On July 5, 2017, 10:31 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59759/
> ---
> 
> (Updated July 5, 2017, 10:31 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7608
> https://issues.apache.org/jira/browse/MESOS-7608
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This introduces a first-class notion of a "domain", which is a set of
> hosts that have similar characteristics. Mesos will initially only
> support "fault domains", which identify groups of hosts with similar
> failure characteristics.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 4e4b2790615e8bbbf262ba809cbfd67e24368952 
>   include/mesos/type_utils.hpp d2a6591b65b95b4e37e6a87dd3b11377834daf85 
>   include/mesos/v1/mesos.hpp 2479b004f342ffcb05279e3b12b6de6949190a93 
>   include/mesos/v1/mesos.proto b8625d363388bc707bc2458abd47a218cc7b7888 
>   src/common/type_utils.cpp aeb16238e9c3fd71a8e9e57dbb8098ddc004e1f2 
>   src/v1/mesos.cpp 423510ef14025dba208ef85edf5305c2ce58f01d 
> 
> 
> Diff: https://reviews.apache.org/r/59759/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 59762: Added domain to MasterInfo and SlaveInfo.

[Neil Conway]

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

(Updated July 5, 2017, 10:32 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

This means that each master's domain is stored in ZooKeeper, along with
the rest of the MasterInfo protobuf message.

Each agent's domain is stored as part of its checkpointed resources.
Changing the agent's domain requires a full drain of the agent; this
behavior might be relaxed in the future.


Diffs (updated)
-

  include/mesos/mesos.proto 4e4b2790615e8bbbf262ba809cbfd67e24368952 
  include/mesos/v1/mesos.proto b8625d363388bc707bc2458abd47a218cc7b7888 
  src/common/http.hpp b7e4a8adcbcaa3a962af795c67694a35161b6c1a 
  src/common/http.cpp fdb591eccf273260902f3f695cf431f72ee3d817 
  src/common/type_utils.cpp aeb16238e9c3fd71a8e9e57dbb8098ddc004e1f2 
  src/internal/evolve.hpp 9db5fe6155243576f186a8b974e81068505b9fcb 
  src/internal/evolve.cpp 93196f301e820b99572ee008b98a124ddafe9697 
  src/master/http.cpp 175a44ce7fb5be509453c25eaa9ec29f35adba3a 
  src/master/master.cpp 56b170ed70722eba30d98f6e648b6a31580d6b56 
  src/slave/http.cpp 700871e1502a65b0bb1fc31219e09219dbdb5340 
  src/slave/slave.cpp 0e24b8cb8d1020af515e3d1862e121e1daf82ce9 
  src/tests/api_tests.cpp cdaa72462eac1687185115771265539d8c2b09a9 
  src/tests/master_tests.cpp c778c6c56d47c4033189912cebee6024be79106f 
  src/tests/mesos.hpp eac2c96985cdbbf1a50cfc054862eae2d44fbfcd 
  src/tests/slave_tests.cpp 8a69cc2ede0b2f17a31986e9142aa2081691eb5e 
  src/v1/mesos.cpp 423510ef14025dba208ef85edf5305c2ce58f01d 


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

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 59921: Added agent domain to Offer message.

[Neil Conway]

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

(Updated July 5, 2017, 10:32 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

This is a convenience mechanism to allow frameworks to determine the
domain of an agent when they receive a resource offer.


Diffs (updated)
-

  include/mesos/mesos.proto 4e4b2790615e8bbbf262ba809cbfd67e24368952 
  include/mesos/v1/mesos.proto b8625d363388bc707bc2458abd47a218cc7b7888 
  src/master/master.cpp 56b170ed70722eba30d98f6e648b6a31580d6b56 
  src/tests/master_tests.cpp c778c6c56d47c4033189912cebee6024be79106f 


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

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 59764: Ignore registration attempts by agents with misconfigured domain.

[Neil Conway]

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

(Updated July 5, 2017, 10:32 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

We expect the master's domain to be configured first, then the domain of
the agents to be configured. Therefore, if an agent with configured
domain attempts to register or re-register with a master that does not
have a configured domain, the registration attempt should be ignored.
This is important, because the master would have no way of determining
whether the agent is "remote" or not, which might result in placing
inappropriate workloads on it.


Diffs (updated)
-

  src/master/master.cpp 56b170ed70722eba30d98f6e648b6a31580d6b56 
  src/tests/master_tests.cpp c778c6c56d47c4033189912cebee6024be79106f 


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

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


Testing
---

`make check`

Manual testing.


Thanks,

Neil Conway

Re: Review Request 59766: Changed allocator to offer remote resources to region-aware frameworks.

[Neil Conway]

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

(Updated July 5, 2017, 10:32 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Changed allocator to offer remote resources to region-aware frameworks.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
bec9e0b8ef8d51ec22f9e8af69407ed5ecaa3e8f 
  src/master/allocator/mesos/allocator.hpp 
2e780c92d5c5132abff32f1ce051c3bab2947f37 
  src/master/allocator/mesos/hierarchical.hpp 
5c58cf401de9205e54300e0ce8433995cdf5cb7a 
  src/master/allocator/mesos/hierarchical.cpp 
eb01d8e6b1108866ebc049f9f4a46157823a3541 
  src/master/master.cpp 56b170ed70722eba30d98f6e648b6a31580d6b56 
  src/tests/allocator.hpp a990788d5218bbcac499613783750ade022811a7 
  src/tests/api_tests.cpp cdaa72462eac1687185115771265539d8c2b09a9 
  src/tests/master_allocator_tests.cpp f83ca66e64fe6d2d10d104ff24f31afd84c50a3a 
  src/tests/master_quota_tests.cpp bbdbfbe9d9960cdc9fa7b2d56e9da9122d25db6a 
  src/tests/master_tests.cpp c778c6c56d47c4033189912cebee6024be79106f 
  src/tests/reservation_tests.cpp 3b4884be21a0b4f15bcbc1a58fa4a4ad7858a7fb 
  src/tests/resource_offers_tests.cpp 427a6520034a537caf748a126001d131e196fedd 
  src/tests/slave_recovery_tests.cpp f6eafcbe3d89c7a69c03db0fd7bc10ae73d06584 


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

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


Testing
---

`make check`.


Thanks,

Neil Conway

Re: Review Request 59760: Added REGION_AWARE framework capability.

[Neil Conway]

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

(Updated July 5, 2017, 10:31 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added REGION_AWARE framework capability.


Diffs (updated)
-

  include/mesos/mesos.proto 4e4b2790615e8bbbf262ba809cbfd67e24368952 
  include/mesos/v1/mesos.proto b8625d363388bc707bc2458abd47a218cc7b7888 
  src/common/protobuf_utils.hpp 5476d2e429cebc649405504f02169de318f65fd6 
  src/tests/protobuf_utils_tests.cpp 6ec3a8e606e86406a0f1c9d5de4ed09d0811e49a 


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

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 59763: Caused master to abort when joining a mixed-region cluster.

[Neil Conway]

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

(Updated July 5, 2017, 10:32 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

That is, if a standby master is configured to use region X but it learns
that the current master has region Y, the standby master will abort with
an error message. This enforces the requirement that all masters in a
single Mesos cluster are configured to use the same region (they can be
configured to use different zones in that region, however).

To allow graceful upgrades, we only abort the standby master if both the
standby master and the leading master have a configured domain; if
either master has the unset (default) domain, the standby master does
not abort.

NOTE: It would be nice to have unit tests to validate this behavior, but
the current unit test infrastructure does not support starting multiple
masters (MESOS-2976).


Diffs (updated)
-

  include/mesos/type_utils.hpp d2a6591b65b95b4e37e6a87dd3b11377834daf85 
  include/mesos/v1/mesos.hpp 2479b004f342ffcb05279e3b12b6de6949190a93 
  src/master/master.cpp 56b170ed70722eba30d98f6e648b6a31580d6b56 


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

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


Testing
---

`make check`

Manual testing.


Thanks,

Neil Conway

Re: Review Request 59759: Added protobuf definitions for fault domains.

[Neil Conway]

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

(Updated July 5, 2017, 10:31 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

This introduces a first-class notion of a "domain", which is a set of
hosts that have similar characteristics. Mesos will initially only
support "fault domains", which identify groups of hosts with similar
failure characteristics.


Diffs (updated)
-

  include/mesos/mesos.proto 4e4b2790615e8bbbf262ba809cbfd67e24368952 
  include/mesos/type_utils.hpp d2a6591b65b95b4e37e6a87dd3b11377834daf85 
  include/mesos/v1/mesos.hpp 2479b004f342ffcb05279e3b12b6de6949190a93 
  include/mesos/v1/mesos.proto b8625d363388bc707bc2458abd47a218cc7b7888 
  src/common/type_utils.cpp aeb16238e9c3fd71a8e9e57dbb8098ddc004e1f2 
  src/v1/mesos.cpp 423510ef14025dba208ef85edf5305c2ce58f01d 


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

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 59761: Added master and agent flags to specify domain.

[Neil Conway]

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

(Updated July 5, 2017, 10:31 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added master and agent flags to specify domain.


Diffs (updated)
-

  docs/configuration.md 007d0f556375f331148c9ad09be8b301ab31a01c 
  src/common/parse.hpp 64eabf89f81a6708cfac89081e0d9f9586721693 
  src/master/flags.hpp 8a9bd2dfa513f1de85dab2cc3cc1d78e80293837 
  src/master/flags.cpp ca5f02c728a9f39abbbee59fa169f95d20c69d3d 
  src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 
  src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 


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

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 60651: Updated bundled ruby dependencies.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On July 5, 2017, 11:34 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60651/
> ---
> 
> (Updated July 5, 2017, 11:34 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Bugs: MESOS-7761
> https://issues.apache.org/jira/browse/MESOS-7761
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit updates to locked ruby dependencies in the 'Gemfile.lock'
> to the lastest versions.
> 
> This e.g., updates eventmachine from 1.0.3 to 1.2.3. We need to
> require a more recent version of eventmachine to be able to install
> this bundle (which has native extension) on recent macOS.
> 
> 
> Diffs
> -
> 
>   site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 
> 
> 
> Diff: https://reviews.apache.org/r/60651/diff/1/
> 
> 
> Testing
> ---
> 
> `bundle install` now succeeds on macOS-10.12.5, `bundle install` still 
> succeeds e.g., on fedora-25.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 60652: Bundled rake as part of managed ruby dependencies of website.

[Benjamin Bannier]

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

(Updated July 5, 2017, 10:09 p.m.)


Review request for mesos, haosdent huang and Vinod Kone.


Changes
---

Updated review dependency -- @vinodkone


Repository: mesos


Description
---

While rake is available from the package manager on some flavors of
Linux, on e.g., macOS seems to currently only be available as a gem.
In order to provide a working website environment with minimal extra
configuration, bundle rake with bundler as well.

We also update the existing website tooling to use the now bundled
version of rake.


Diffs
-

  site/Dockerfile 8ba0be0c28e924f7a2b89e6e5a3237deb3751a41 
  site/Gemfile 4dcd0a5ad452085e6e3bb152a6547575636cd807 
  site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 
  site/build.sh 11f15e15621c4d3db1472e88911787b9b3100f97 


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


Testing
---

`bundle install && bundle exec rake && bundle exec rake dev` on macOS.


Thanks,

Benjamin Bannier

Re: Review Request 60652: Bundled rake as part of managed ruby dependencies of website.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On July 5, 2017, 11:08 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60652/
> ---
> 
> (Updated July 5, 2017, 11:08 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While rake is available from the package manager on some flavors of
> Linux, on e.g., macOS seems to currently only be available as a gem.
> In order to provide a working website environment with minimal extra
> configuration, bundle rake with bundler as well.
> 
> We also update the existing website tooling to use the now bundled
> version of rake.
> 
> 
> Diffs
> -
> 
>   site/Dockerfile 8ba0be0c28e924f7a2b89e6e5a3237deb3751a41 
>   site/Gemfile 4dcd0a5ad452085e6e3bb152a6547575636cd807 
>   site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 
>   site/build.sh 11f15e15621c4d3db1472e88911787b9b3100f97 
> 
> 
> Diff: https://reviews.apache.org/r/60652/diff/1/
> 
> 
> Testing
> ---
> 
> `bundle install && bundle exec rake && bundle exec rake dev` on macOS.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 60485: Fixed flaky PersistentVolumeEndpointsTest.ReserveAndSlaveRemoval.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 27, 2017, 11:17 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60485/
> ---
> 
> (Updated June 27, 2017, 11:17 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7725
> https://issues.apache.org/jira/browse/MESOS-7725
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed flaky PersistentVolumeEndpointsTest.ReserveAndSlaveRemoval.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 518bdf858096ec9bcfa4f899ead5a6c3d103c521 
> 
> 
> Diff: https://reviews.apache.org/r/60485/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Ran manually -- previously, test failure observed in < 50 iterations. With 
> this change, no failures observed after 2000 iterations.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

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 5, 2017, 9:03 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/7/

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


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 60107: Added filtering to the '/tasks' endpoint.

[Quinn Leng]

> On July 5, 2017, 8:02 p.m., Alexander Rojas wrote:
> > src/common/http.cpp
> > Lines 1199-1200 (patched)
> > 
> >
> > make use of the new constructors.

Yep, the new constructor patch will be merged after these commits submitted


> On July 5, 2017, 8:02 p.m., Alexander Rojas wrote:
> > src/common/http.cpp
> > Lines 1217-1219 (patched)
> > 
> >
> > Make use of the new constructors.

Yep, the new constructor patch will be merged after these commits submitted


- Quinn


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


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 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/#review179693
---




src/common/http.cpp
Lines 1199-1200 (patched)


make use of the new constructors.



src/common/http.cpp
Lines 1217-1219 (patched)


Make use of the new constructors.


- 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 59127: Added IPv6 flags for Mesos agent.

[Benjamin Hindman]

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




src/slave/flags.hpp
Line 173 (original), 173 (patched)


Let's keep this generic to IP not IPv4 and add validation like you did in 
libprocess that we currently only support IPv4.



src/slave/flags.hpp
Lines 181-182 (original), 181-182 (patched)


s/IPv4/IP/g



src/slave/flags.hpp
Lines 187-190 (patched)


How about printing this out as a warning as part of some validation logic 
so that users that set this understand that they're not getting what they might 
think they're getting?



src/slave/flags.cpp
Lines 1101-1104 (patched)


I'd like to make the comments be the same as for the `--ip` set of flags 
with an extra comment that currently the flag is ONLY being used to "advertise 
containers running on the IPv6 network ..." etc. Because the long term 
intention is that this will also be used for specifying a flag that we'll bind 
to?


- Benjamin Hindman


On June 29, 2017, 11:25 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59127/
> ---
> 
> (Updated June 29, 2017, 11:25 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added IPv6 flags for Mesos agent.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.hpp e75c1b4227b443aedf445921b3f2108d930c112c 
>   src/slave/flags.cpp c84aa6724170bba46bbe8410b71d42a1626e 
> 
> 
> Diff: https://reviews.apache.org/r/59127/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 59233: Updated v6 address for containers running on host network.

[Benjamin Hindman]

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


Fix it, then Ship it!





src/slave/slave.cpp
Lines 4543 (patched)


s/Setup/Set up/



src/slave/slave.cpp
Lines 4545 (patched)


... the protocol is set to `IPv4` and therefore we don't explicitly set the 
protocol here.


- Benjamin Hindman


On June 29, 2017, 11:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59233/
> ---
> 
> (Updated June 29, 2017, 11:22 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently the agent is populating only the v4 address for containers
> running on the host network. However, clusters running on a dual stack
> network need to report v4 and v6 address for containers running on the
> host network. This change allows v6 address specified by operators to be
> advertised for containers running on the host network.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3338349c399c9fb5b7fb2fc5886fb492ffb944c4 
>   include/mesos/v1/mesos.proto 6f0ad71260c36a9cb401c716e02c561580fb433b 
>   src/slave/slave.cpp 1f7aed48eb3ab3ee2a797d4061a6e289852b8199 
> 
> 
> Diff: https://reviews.apache.org/r/59233/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 59130: Added storage for IPv6 in a `libprocess` process.

[Benjamin Hindman]

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


Fix it, then Ship it!





3rdparty/libprocess/src/process.cpp
Line 170 (original), 172 (patched)


I like the way you did this where you validated that it's an `IPv4` below 
but still take the type `IP`, so let's not label this description as "IPv4" but 
keep it at "IP".



3rdparty/libprocess/src/process.cpp
Lines 1392-1393 (patched)


Can we move this up to just below where we reset `__address__` please? 
Thanks!


- Benjamin Hindman


On June 29, 2017, 11:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59130/
> ---
> 
> (Updated June 29, 2017, 11:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added storage for IPv6 in a `libprocess` process.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 8ff37d1b5781c42f96be2da391ed000158eea7b8 
> 
> 
> Diff: https://reviews.apache.org/r/59130/diff/6/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 60149: Added support for `net::IPv4` and `net::IPv6` in `FlagsBase`.

[Benjamin Hindman]

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


Ship it!




Ship It!

- Benjamin Hindman


On June 29, 2017, 11:23 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60149/
> ---
> 
> (Updated June 29, 2017, 11:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for `net::IPv4` and `net::IPv6` in `FlagsBase`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/parse.hpp 
> 3448c571ece36b8d45bbe1317efc824468fe3466 
> 
> 
> Diff: https://reviews.apache.org/r/60149/diff/3/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 60148: Mesos updates for new inet4::Address.

[Benjamin Hindman]

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


Ship it!




Ship It!

- Benjamin Hindman


On June 29, 2017, 11:23 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60148/
> ---
> 
> (Updated June 29, 2017, 11:23 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos updates for new inet4::Address.
> 
> 
> Diffs
> -
> 
>   src/tests/utils.cpp 053976d3c4cf120ff5e7dff6e96c6862a5b617b1 
> 
> 
> Diff: https://reviews.apache.org/r/60148/diff/2/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 60136: Refactored inet::Address to base of inet4::Address and inet6::Address.

[Benjamin Hindman]

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


Ship it!




Ship It!

- Benjamin Hindman


On June 29, 2017, 11:23 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60136/
> ---
> 
> (Updated June 29, 2017, 11:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored inet::Address to base of inet4::Address and inet6::Address.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/address.hpp 
> 6b143c3d00c1d7ebd1697c26b6d312a64f30839a 
>   3rdparty/libprocess/include/process/pid.hpp 
> c634916a37f570194945b9c7d7b786ffeae0408a 
>   3rdparty/libprocess/include/process/socket.hpp 
> 42287c128fb352af6a196b7abc77f913a2ddbba0 
>   3rdparty/libprocess/src/http.cpp f317f2f0826e006342cfea20b15b80e7c8fc68a9 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 7d493301bd5c0f24bf89e0b213f07ffe7801508b 
>   3rdparty/libprocess/src/pid.cpp 023f881841f466d91243d2773961b67f82da4e91 
>   3rdparty/libprocess/src/poll_socket.cpp 
> 9d1b2769ac31928d5ce4f64467c12ca0f58c11cf 
>   3rdparty/libprocess/src/process.cpp 
> 8ff37d1b5781c42f96be2da391ed000158eea7b8 
>   3rdparty/libprocess/src/socket.cpp 85920a1abb3a2b2da167647dcf1351b825c72c80 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 09c7297aa0e80accd09ce6d81390a0f662bec6f2 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 38d787a083a5eb31e922d283f4b4bed2bd62eb0a 
>   3rdparty/libprocess/src/tests/socket_tests.cpp 
> fdcd23dcb05d683cfb3ae14956fe3fd8fe166603 
>   3rdparty/libprocess/src/tests/test_linkee.cpp 
> 77df385d3388788658fa40d033816e1fbb8d8f2c 
> 
> 
> Diff: https://reviews.apache.org/r/60136/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 59721: Refactored net::IP and added net::IPv4 and net::IPv6.

[Benjamin Hindman]

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


Ship it!




Ship It!

- Benjamin Hindman


On June 29, 2017, 11:24 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59721/
> ---
> 
> (Updated June 29, 2017, 11:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Classes can now initialize the specialized classes whenever they need to
> create explicit v4 and v6 addresses, rather than relying on the
> `create/Try` semantics of net::IP.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/ip.hpp 
> ee48befac8b9bb4956aec0c5bdebd69347bd5890 
>   3rdparty/stout/tests/ip_tests.cpp 39ee0598e3580510a30894fd3f47d2014f596ef7 
> 
> 
> Diff: https://reviews.apache.org/r/59721/diff/5/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

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

[Greg Mann]

> 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.
> 
> Quinn Leng wrote:
> 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.

@arojas, the current implementation encodes the authorization action in the 
class type, so that the callsite doesn't need to specify it explicitly. Do you 
think this is a reasonable approach? If we go with a single authorization 
acceptor, the action will need to be specified by the callsite.


> 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.
> 
> Quinn Leng wrote:
> Agree, I'll update it in another patch. Thanks for pointing out~

This is the member variable - I'm pretty sure we shouldn't be holding the 
object as a const ref here. If we want to avoid copies when passing the 
approver from `create()` to the `AuthorizationAcceptor` constructor, we could 
pass it as an rvalue reference.


- Greg


---
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 59739: Replaced use of `net::IPNetwork` by `net::IP::Network`.

[Benjamin Hindman]

> On July 5, 2017, 6:35 p.m., Benjamin Hindman wrote:
> > Ship It!

I'll split this into a Stout and Mesos commit for you!


- Benjamin


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


On June 29, 2017, 11:24 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59739/
> ---
> 
> (Updated June 29, 2017, 11:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced use of `net::IPNetwork` by `net::IP::Network`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/posix/ip.hpp 
> 0d7e85a4b4b8263691dfb7654835312bbd117db6 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> b386fbed17698dd6c59bc6c53edfe8a7ece6a3eb 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
>  ce795628f19f0e8138ad9a0b50b7d644b9d734a8 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 17afdf45a839bb80aa19663f97eaebec332ceee8 
>   src/tests/utils.hpp a2ae43c6a20ab3753a3501d8ce23ffb0f6ac0005 
>   src/tests/utils.cpp 053976d3c4cf120ff5e7dff6e96c6862a5b617b1 
> 
> 
> Diff: https://reviews.apache.org/r/59739/diff/5/
> 
> 
> Testing
> ---
> 
> make.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 59739: Replaced use of `net::IPNetwork` by `net::IP::Network`.

[Benjamin Hindman]

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


Ship it!




Ship It!

- Benjamin Hindman


On June 29, 2017, 11:24 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59739/
> ---
> 
> (Updated June 29, 2017, 11:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced use of `net::IPNetwork` by `net::IP::Network`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/posix/ip.hpp 
> 0d7e85a4b4b8263691dfb7654835312bbd117db6 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> b386fbed17698dd6c59bc6c53edfe8a7ece6a3eb 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
>  ce795628f19f0e8138ad9a0b50b7d644b9d734a8 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 17afdf45a839bb80aa19663f97eaebec332ceee8 
>   src/tests/utils.hpp a2ae43c6a20ab3753a3501d8ce23ffb0f6ac0005 
>   src/tests/utils.cpp 053976d3c4cf120ff5e7dff6e96c6862a5b617b1 
> 
> 
> Diff: https://reviews.apache.org/r/59739/diff/5/
> 
> 
> Testing
> ---
> 
> make.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 59688: Moved `net::IPNetwork` to `net::IP:Network`.

[Benjamin Hindman]

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


Ship it!




Ship It!

- Benjamin Hindman


On June 29, 2017, 11:24 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59688/
> ---
> 
> (Updated June 29, 2017, 11:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved `net::IPNetwork` to `net::IP:Network`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/ip.hpp 
> ee48befac8b9bb4956aec0c5bdebd69347bd5890 
>   3rdparty/stout/tests/ip_tests.cpp 39ee0598e3580510a30894fd3f47d2014f596ef7 
> 
> 
> Diff: https://reviews.apache.org/r/59688/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 58394: Setup new directory for python http client lib in src/python.

[Eric Chung]

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



bump

- Eric Chung


On June 29, 2017, 6:22 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58394/
> ---
> 
> (Updated June 29, 2017, 6:22 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, Joseph Wu, and Kevin 
> Klues.
> 
> 
> Bugs: MESOS-7310
> https://issues.apache.org/jira/browse/MESOS-7310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Part of MESOS-7310, this patch adds a new directory (src/python/lib/mesos) 
> for placing the http client package, which will be importable via `import 
> mesos` from the cli.
> 
> Review: https://reviews.apache.org/r/58394
> 
> 
> Diffs
> -
> 
>   src/cli_new/bootstrap 6d62e9adf1d543ed00a3a2cf2484edf1c33ee443 
>   src/python/.gitignore PRE-CREATION 
>   src/python/lib/mesos/__init__.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/58394/diff/4/
> 
> 
> Testing
> ---
> 
> Under src/cli_new:
> 1\. ./bootstrap
> 2\. . ./activate
> 3\. python
> 4\. >>> import mesos
> 5\. >>> mesos.\_\_path\_\_
> 6\. verify that the path printed out is indeed at src/python/lib/mesos
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: 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/
---

(Updated July 5, 2017, 5:43 p.m.)


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

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


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

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


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 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?
> 
> Aaron Wood wrote:
> 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.
> 
> Jie Yu wrote:
> I think there are some cases at the moment where the working_directory is 
> not set (DEBUG containers):
> 
> https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/containerizer.cpp#L1540
> 
> But I think that's a tech debt itself.

Thanks, I didn't realize this existed.


- Aaron


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


On July 3, 2017, 6:35 p.m., Aaron Wood wrote:
> 
> ---
> 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.
> 
> 
> 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/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 60280: Provide full path to the custom executor.

[Aaron Wood via Review Board]

> On July 5, 2017, 4:48 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Lines 799 (patched)
> > 
> >
> > This is still based on the assumption that `launchInfo` has 
> > `working_directory()` set. Can you do the following instead:
> > ```
> > Result realpath = os::realpath(executable);
> > if (!realpath.isSome()) {
> >   ...
> > }
> > ```
> 
> Jie Yu wrote:
> I also realized that using realpath will make the search for PATH not 
> possible. This issue is indeed not trivial. The best way I can think of is to 
> test if executable is relative and see if it exists in the sandbox or not and 
> test if it is executable or not. Alternatively, we can add cwd to PATH, but 
> this will polute PATH i would rather avoid.
> 
> Do you have other thoughts?

Is it possible to assume that any custom executor would always be in the 
sandbox? E.g. assume that a framework will always provide a URI to fetch the 
executor and have Mesos execute it in the usual way. If that's the case then 
would we need to search `PATH` at all?


- Aaron


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


On July 3, 2017, 6:35 p.m., Aaron Wood wrote:
> 
> ---
> 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.
> 
> 
> 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/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 60638: Adjusted allocator interfaces to allow passing new agent total.

[Jie Yu]

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




include/mesos/allocator/allocator.hpp
Line 219 (original), 220-221 (patched)


It's quite confusing to the readers when there are two fields there. Does 
'total' include 'oversubscribed' resources?

I am wondering if we can combine these two fields in the allocator. 
Updating 'oversubscribed' is a special case of updating 'total'?


- Jie Yu


On July 4, 2017, 2:40 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60638/
> ---
> 
> (Updated July 4, 2017, 2:40 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-7755
> https://issues.apache.org/jira/browse/MESOS-7755
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adjust interfaces, implementations and users of the
> 'updateSlave' method. This allows to pass an additional argument
> denoting an updated 'total' amount of resource on the agents.
> 
> In a subsequent commit we will update the 'HierarchicalAllocator' to
> interpret this value.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> bec9e0b8ef8d51ec22f9e8af69407ed5ecaa3e8f 
>   src/master/allocator/mesos/allocator.hpp 
> 2e780c92d5c5132abff32f1ce051c3bab2947f37 
>   src/master/allocator/mesos/hierarchical.hpp 
> 5c58cf401de9205e54300e0ce8433995cdf5cb7a 
>   src/master/allocator/mesos/hierarchical.cpp 
> eb01d8e6b1108866ebc049f9f4a46157823a3541 
>   src/master/master.cpp 95e9691b3e3ef101e3e7ddc7a498d5c5bf2276e3 
>   src/tests/allocator.hpp a990788d5218bbcac499613783750ade022811a7 
>   src/tests/hierarchical_allocator_tests.cpp 
> 2a312a9af4bae679a0a4e7bf45a3c013513c5da2 
> 
> 
> Diff: https://reviews.apache.org/r/60638/diff/1/
> 
> 
> Testing
> ---
> 
> Tested with https://reviews.apache.org/r/60639/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

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

[Jie Yu]

> On July 5, 2017, 4:48 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Lines 799 (patched)
> > 
> >
> > This is still based on the assumption that `launchInfo` has 
> > `working_directory()` set. Can you do the following instead:
> > ```
> > Result realpath = os::realpath(executable);
> > if (!realpath.isSome()) {
> >   ...
> > }
> > ```

I also realized that using realpath will make the search for PATH not possible. 
This issue is indeed not trivial. The best way I can think of is to test if 
executable is relative and see if it exists in the sandbox or not and test if 
it is executable or not. Alternatively, we can add cwd to PATH, but this will 
polute PATH i would rather avoid.

Do you have other thoughts?


- Jie


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


On July 3, 2017, 6:35 p.m., Aaron Wood wrote:
> 
> ---
> 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.
> 
> 
> 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/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 60280: Provide full path to the custom executor.

[Jie Yu]

> 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?
> 
> Aaron Wood wrote:
> 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.

I think there are some cases at the moment where the working_directory is not 
set (DEBUG containers):
https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/containerizer.cpp#L1540

But I think that's a tech debt itself.


- Jie


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


On July 3, 2017, 6:35 p.m., Aaron Wood wrote:
> 
> ---
> 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.
> 
> 
> 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/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 60280: Provide full path to the custom executor.

[Jie Yu]

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




src/slave/containerizer/mesos/launch.cpp
Lines 799 (patched)


This is still based on the assumption that `launchInfo` has 
`working_directory()` set. Can you do the following instead:
```
Result realpath = os::realpath(executable);
if (!realpath.isSome()) {
  ...
}
```


- Jie Yu


On July 3, 2017, 6:35 p.m., Aaron Wood wrote:
> 
> ---
> 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.
> 
> 
> 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/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 60647: Fixed the mention of default ports in 'configuration.md'.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On July 5, 2017, 9:15 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60647/
> ---
> 
> (Updated July 5, 2017, 9:15 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 007d0f556375f331148c9ad09be8b301ab31a01c 
> 
> 
> Diff: https://reviews.apache.org/r/60647/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 60647: Fixed the mention of default ports in 'configuration.md'.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 5, 2017, 2:15 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60647/
> ---
> 
> (Updated July 5, 2017, 2:15 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 007d0f556375f331148c9ad09be8b301ab31a01c 
> 
> 
> Diff: https://reviews.apache.org/r/60647/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

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 5, 2017, 4:30 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.

Add query parameter support for '/frameworks' endpoint.
To query the information about a specific framework, we
can use 'framework_id' to specify the framework, thus it
looks like'/frameworks?framework_id=[framework id]'

To maintain consistency, the structure for response is
reserved, thus the response still contains field 'frameworks'
and 'completed_frameworks' etc..

Since code for '/state' endpoint and '/frameworks' are coupled
(they share 'FullFrameworkWriter' class), thus the code for '/state'
endpoint is also updated.


Diffs
-

  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/


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

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

[Benjamin Bannier]

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




3rdparty/libprocess/src/subprocess.cpp
Lines 172-173 (original), 172-177 (patched)


This does two things, remove `abort()` and fix this code to gracefully 
handle `EINTR`. We usually try to do one thing at a time, so it might make 
sense to pull the `EINTR` fix into a separate RR.



3rdparty/libprocess/src/subprocess.cpp
Lines 176 (patched)


Any reason we cannot do a proper `exit` here?

Here and below.


- Benjamin Bannier


On July 3, 2017, 4:58 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60598/
> ---
> 
> (Updated July 3, 2017, 4:58 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, 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/src/subprocess.cpp 
> 0f1532b294d6d6b1e017468cfde47362f3faa84d 
> 
> 
> Diff: https://reviews.apache.org/r/60598/diff/2/
> 
> 
> Testing
> ---
> 
> make check (mac os x, fedora 25)
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

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 5, 2017, 4:28 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.

Added query parameter support for 'slaves'
endpoint. We can use 'slave_id' to specify
which slave information to query. Thus it
looks like '/slaves?slave_id=[slave id]'

If not slave id is specified, it will return
information of all slaves, just like before

The structure for response is the same for both
specifying or not specifying slave id. Thus even
for request with slave id specified, the response
will still contain both fields 'slaves', 
'recovered_slaves' etc..


Diffs
-

  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/


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 60397: Check perf version compatibility in tests with disabled coredumps.

[Benjamin Bannier]

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


Ship it!




I assume this makes the workaround from 
`fcf8618be59ada6d1607696b13bc362bf7e85b0b` unnecessary? In that case, could you 
submit a RR to revert it?

- Benjamin Bannier


On July 3, 2017, 12:57 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60397/
> ---
> 
> (Updated July 3, 2017, 12:57 p.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 60546: Harden Mesos when building with cmake.

[Aaron Wood via Review Board]

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

(Updated July 5, 2017, 4:06 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, and Joseph Wu.


Changes
---

Use CHECK_CXX_COMPILER_FLAG to detect support for stack protectors and add more 
documentation surrounding why we want position independent code.


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


Repository: mesos


Description
---

Apply stack protectors (stronger stack protectors with newer compilers), 
position independent code suitable for linking into executables, security 
warnings, and generate position independent executables.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake 3fa2e2f3b 
  cmake/MesosConfigure.cmake 5ecad2c0f 


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

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


Testing
---

`mkdir build && cd build && cmake .. && make -j$(nproc) && make check 
-j$(nproc)`


Thanks,

Aaron Wood

Re: Review Request 60546: Harden Mesos when building with cmake.

[Aaron Wood via Review Board]

> On July 4, 2017, 3:06 p.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 213-214 (patched)
> > 
> >
> > Not sure this is needed, see below.

Relating to my comment below, I saw that `-pie` was not used to generate a 
fully position independent executable. Only `-fPIC` was used to generate 
position independent code for dynamic linking.


> On July 4, 2017, 3:06 p.m., Benjamin Bannier wrote:
> > cmake/MesosConfigure.cmake
> > Lines 65 (patched)
> > 
> >
> > It appears as if just setting
> > 
> > set(CMAKE_POSITION_INDEPENDENT_CODE TRUE)
> > 
> > here as well would take care of both the compiler and linker flags; 
> > could you check if it does and potentially streamline this part. It would 
> > also be great if we could have a comment explaining why we enforce position 
> > independent code.

>From my testing I found that `-fPIC` was always set when 
>`set(CMAKE_POSITION_INDEPENDENT_CODE TRUE)` is used but `-fPIE` is never set. 
>I figured I'd add it here since it's very relevant when building static libs. 
>I'll definitely add more comments surrounding this stuff.

Also, I'm assuming that the static libs built here are only for linking into 
executables. Do you think this will hold true going forward?


- Aaron


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


On June 29, 2017, 6:18 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60546/
> ---
> 
> (Updated June 29, 2017, 6:18 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-7737
> https://issues.apache.org/jira/browse/MESOS-7737
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Apply stack protectors (stronger stack protectors with newer compilers), 
> position independent code suitable for linking into executables, security 
> warnings, and generate position independent executables.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 3fa2e2f3b 
>   cmake/MesosConfigure.cmake 5ecad2c0f 
> 
> 
> Diff: https://reviews.apache.org/r/60546/diff/1/
> 
> 
> Testing
> ---
> 
> `mkdir build && cd build && cmake .. && make -j$(nproc) && make check 
> -j$(nproc)`
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 60653: Tightened namespace to fix a lookup error on Windows.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On July 5, 2017, 1:58 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60653/
> ---
> 
> (Updated July 5, 2017, 1:58 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Visual C++ compiler tried to lookup 'protobuf::parse' in the
> 'mesos::internal::protobuf' namespace. This is fixed by explicitly
> setting the namespace.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/daemon.cpp 243396dfe735c0ee421071dfc5df3c9d1c103e45 
> 
> 
> Diff: https://reviews.apache.org/r/60653/diff/3/
> 
> 
> Testing
> ---
> 
> make check and made sure it compiles with Windows.
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 60653: Tightened namespace to fix a lookup error on Windows.

[Jan Schlicht]

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

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


Review request for mesos, Alexander Rukletsov and Benjamin Bannier.


Repository: mesos


Description
---

The Visual C++ compiler tried to lookup 'protobuf::parse' in the
'mesos::internal::protobuf' namespace. This is fixed by explicitly
setting the namespace.


Diffs (updated)
-

  src/resource_provider/daemon.cpp 243396dfe735c0ee421071dfc5df3c9d1c103e45 


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

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


Testing
---

make check and made sure it compiles with Windows.


Thanks,

Jan Schlicht

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

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [60109, 59987, 59988, 59989]

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 July 4, 2017, 3:44 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59989/
> ---
> 
> (Updated July 4, 2017, 3:44 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Zhitao Li.
> 
> 
> Bugs: MESOS-7656
> https://issues.apache.org/jira/browse/MESOS-7656
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `ProtobufTest.JsonifyMap`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/protobuf_tests.cpp 
> 8877e8934e0f7875bfedcfa88b491ce4b13ca44f 
> 
> 
> Diff: https://reviews.apache.org/r/59989/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60653: Tightened namespace to fix a lookup error on Windows.

[Jan Schlicht]

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

(Updated July 5, 2017, 2:28 p.m.)


Review request for mesos, Alexander Rukletsov and Benjamin Bannier.


Repository: mesos


Description (updated)
---

The Visual C++ compiler tried to lookup 'protobuf::parse' in the
'mesos::internal::protobuf' namespace. This is fixed by explicitly
setting the namespace.


Diffs (updated)
-

  src/common/resources_utils.hpp 18e3d9d4baad23669d00542594f5c15a989b7b9e 
  src/common/resources_utils.cpp 821bd0967d55f7fbcb57a4efae5fc390af53ca79 
  src/master/http.cpp 175a44ce7fb5be509453c25eaa9ec29f35adba3a 
  src/master/master.hpp 95c2d0fab32d6b60f29a86037607ff009bd78717 
  src/master/master.cpp 56b170ed70722eba30d98f6e648b6a31580d6b56 
  src/resource_provider/daemon.cpp 243396dfe735c0ee421071dfc5df3c9d1c103e45 
  src/tests/master_tests.cpp c778c6c56d47c4033189912cebee6024be79106f 
  src/tests/persistent_volume_endpoints_tests.cpp 
a66bde2619732c0f9bea6eb5364737077b96e51d 
  src/tests/reservation_endpoints_tests.cpp 
05b505fc4c643f56c7141b6b79f334b000470780 


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

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


Testing
---

make check and made sure it compiles with Windows.


Thanks,

Jan Schlicht

Re: Review Request 60651: Updated bundled ruby dependencies.

[Benjamin Bannier]

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

(Updated July 5, 2017, 1:34 p.m.)


Review request for mesos, haosdent huang and Vinod Kone.


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


Repository: mesos


Description
---

This commit updates to locked ruby dependencies in the 'Gemfile.lock'
to the lastest versions.

This e.g., updates eventmachine from 1.0.3 to 1.2.3. We need to
require a more recent version of eventmachine to be able to install
this bundle (which has native extension) on recent macOS.


Diffs
-

  site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 


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


Testing (updated)
---

`bundle install` now succeeds on macOS-10.12.5, `bundle install` still succeeds 
e.g., on fedora-25.


Thanks,

Benjamin Bannier

Re: Review Request 60653: Tightened namespace to fix a lookup error on Windows.

[Benjamin Bannier]

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


Ship it!




Could you update the commit message to maybe include there error we were 
seeing? Might also be worth calling at that we use this pattern elsewhere 
already when calling `protobuf::parse`.

- Benjamin Bannier


On July 5, 2017, 1:26 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60653/
> ---
> 
> (Updated July 5, 2017, 1:26 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tightened namespace to fix a lookup error on Windows.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/daemon.cpp 243396dfe735c0ee421071dfc5df3c9d1c103e45 
> 
> 
> Diff: https://reviews.apache.org/r/60653/diff/1/
> 
> 
> Testing
> ---
> 
> make check and made sure it compiles with Windows.
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 60653: Tightened namespace to fix a lookup error on Windows.

[Jan Schlicht]

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

Review request for mesos, Alexander Rukletsov and Benjamin Bannier.


Repository: mesos


Description
---

Tightened namespace to fix a lookup error on Windows.


Diffs
-

  src/resource_provider/daemon.cpp 243396dfe735c0ee421071dfc5df3c9d1c103e45 


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


Testing
---

make check and made sure it compiles with Windows.


Thanks,

Jan Schlicht

Re: Review Request 60651: Updated bundled ruby dependencies.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On July 5, 2017, 11:08 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60651/
> ---
> 
> (Updated July 5, 2017, 11:08 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit updates to locked ruby dependencies in the 'Gemfile.lock'
> to the lastest versions.
> 
> This e.g., updates eventmachine from 1.0.3 to 1.2.3. We need to
> require a more recent version of eventmachine to be able to install
> this bundle (which has native extension) on recent macOS.
> 
> 
> Diffs
> -
> 
>   site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 
> 
> 
> Diff: https://reviews.apache.org/r/60651/diff/1/
> 
> 
> Testing
> ---
> 
> `bundle install` now succeeds on macOS.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 60652: Bundled rake as part of managed ruby dependencies of website.

[Benjamin Bannier]

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

Review request for mesos, haosdent huang and Vinod Kone.


Repository: mesos


Description
---

While rake is available from the package manager on some flavors of
Linux, on e.g., macOS seems to currently only be available as a gem.
In order to provide a working website environment with minimal extra
configuration, bundle rake with bundler as well.

We also update the existing website tooling to use the now bundled
version of rake.


Diffs
-

  site/Dockerfile 8ba0be0c28e924f7a2b89e6e5a3237deb3751a41 
  site/Gemfile 4dcd0a5ad452085e6e3bb152a6547575636cd807 
  site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 
  site/build.sh 11f15e15621c4d3db1472e88911787b9b3100f97 


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


Testing
---

`bundle install && bundle exec rake && bundle exec rake dev` on macOS.


Thanks,

Benjamin Bannier

Review Request 60651: Updated bundled ruby dependencies.

[Benjamin Bannier]

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

Review request for mesos, haosdent huang and Vinod Kone.


Repository: mesos


Description
---

This commit updates to locked ruby dependencies in the 'Gemfile.lock'
to the lastest versions.

This e.g., updates eventmachine from 1.0.3 to 1.2.3. We need to
require a more recent version of eventmachine to be able to install
this bundle (which has native extension) on recent macOS.


Diffs
-

  site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 


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


Testing
---

`bundle install` now succeeds on macOS.


Thanks,

Benjamin Bannier

Re: Review Request 60647: Fixed the mention of default ports in 'configuration.md'.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On July 5, 2017, 11:15 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60647/
> ---
> 
> (Updated July 5, 2017, 11:15 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 007d0f556375f331148c9ad09be8b301ab31a01c 
> 
> 
> Diff: https://reviews.apache.org/r/60647/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 60647: Fixed the mention of default ports in 'configuration.md'.

[Jan Schlicht]

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

Review request for mesos, Andrei Budnik and Benjamin Bannier.


Repository: mesos


Description
---

See summary.


Diffs
-

  docs/configuration.md 007d0f556375f331148c9ad09be8b301ab31a01c 


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


Testing
---

make check


Thanks,

Jan Schlicht

Review Request 60646: Modifed handling of parallel test configure flag for documentation.

[Benjamin Bannier]

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

Review request for mesos and Till Toenshoff.


Repository: mesos


Description
---

Modifed handling of parallel test configure flag for documentation.


Diffs
-

  configure.ac 4d7c4a4679e5c624ee750226d542e0d8c228507a 


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


Testing
---

configure and build stout with different settings 
(`--enable-parallel-test-execution`, `--enable-parallel-test-execution=yes`, 
`--enable-parallel-test-execution=no`, `--disable-parallel-test-execution`), 
`make check`.


Thanks,

Benjamin Bannier

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

[Qian Zhang]

> On July 4, 2017, 2:35 a.m., Avinash sridharan wrote:
> > src/docker/docker.hpp
> > Lines 167 (patched)
> > 
> >
> > For consistency can we use `defautlContainerDNS`?
> 
> Qian Zhang wrote:
> Originally I used `defautlContainerDNS`, but that will make this line 
> exceeding 80 chars :-( I think it also makes sense to use `defaultDNS` since 
> here we know we are creating run options for Docker container, so I think we 
> do not have to mention `container` in the parameter name.
> 
> Avinash sridharan wrote:
> You can use this `// NOLINT(whitespace/line_length)` to create 
> exceptions? In `executor.cpp` you use `defaultContainerDNS` so seemed a bit 
> odd.

Agree!


- Qian


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


On July 5, 2017, 3:18 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60558/
> ---
> 
> (Updated July 5, 2017, 3:18 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
> ---
> 
> 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/3/
> 
> 
> 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 60558: Set container DNS with `--default_container_dns` in Docker executor.

[Qian Zhang]

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

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


Review request for mesos, Avinash sridharan and Jie Yu.


Changes
---

Addressed Avinash's comments.


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

  src/docker/docker.hpp 5593cb635e073334c6c2566be3d803cd7febb1c3 
  src/docker/docker.cpp 8ca0c68836ea5d1a1186e79942f6daccf6c6da46 
  src/docker/executor.cpp e03f24461ec7b19cacae79c020406f0a475f2d19 


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

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


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 60558: Set container DNS with `--default_container_dns` in Docker executor.

[Qian Zhang]

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

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


Review request for mesos, Avinash sridharan and Jie Yu.


Changes
---

Addressed Avinash's comments.


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

  src/docker/docker.hpp 5593cb635e073334c6c2566be3d803cd7febb1c3 
  src/docker/docker.cpp 8ca0c68836ea5d1a1186e79942f6daccf6c6da46 
  src/docker/executor.cpp e03f24461ec7b19cacae79c020406f0a475f2d19 


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

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


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 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

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

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


Review request for mesos, Avinash sridharan and Jie Yu.


Changes
---

Addressed Avinash's comments.


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


Repository: mesos


Description
---

Introduced `--default_container_dns` agent flag.


Diffs (updated)
-

  docs/configuration.md 007d0f556375f331148c9ad09be8b301ab31a01c 
  src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
  src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
  src/slave/flags.hpp c6803eb54e09a5497755e1e5fef2872193eacba6 
  src/slave/flags.cpp 398768656b5fa3b7c85474de2b4b008bf7b85cb3 


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

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


Testing
---


Thanks,

Qian Zhang

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

[Avinash sridharan]

> On July 3, 2017, 6:35 p.m., Avinash sridharan wrote:
> > src/docker/docker.hpp
> > Lines 167 (patched)
> > 
> >
> > For consistency can we use `defautlContainerDNS`?
> 
> Qian Zhang wrote:
> Originally I used `defautlContainerDNS`, but that will make this line 
> exceeding 80 chars :-( I think it also makes sense to use `defaultDNS` since 
> here we know we are creating run options for Docker container, so I think we 
> do not have to mention `container` in the parameter name.

You can use this `// NOLINT(whitespace/line_length)` to create exceptions? In 
`executor.cpp` you use `defaultContainerDNS` so seemed a bit odd.


- Avinash


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


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 60558: Set container DNS with `--default_container_dns` in Docker executor.

[Qian Zhang]

> On July 4, 2017, 2:35 a.m., Avinash sridharan wrote:
> > src/docker/docker.hpp
> > Lines 167 (patched)
> > 
> >
> > For consistency can we use `defautlContainerDNS`?

Originally I used `defautlContainerDNS`, but that will make this line exceeding 
80 chars :-( I think it also makes sense to use `defaultDNS` since here we know 
we are creating run options for Docker container, so I think we do not have to 
mention `container` in the parameter name.


- Qian


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


On June 30, 2017, 3:09 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60558/
> ---
> 
> (Updated June 30, 2017, 3:09 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
> ---
> 
> 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 60500: Introduced `--default_container_dns` agent flag.

[Avinash sridharan]

> On July 3, 2017, 5:22 p.m., Avinash sridharan wrote:
> > 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.
> 
> Qian Zhang wrote:
> > 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.
> 
> I have a very old version of Docker (1.5.0) and I have checked its 
> `docker run` command also has the `--dns` option and it can work with 
> `BRIDGE` network but not with `HOST` network. Do you think we need to put 
> such info into this help message (I mean emphasizing the minimal Docker 
> version for `--dns` support with `HOST`, `BRIDGE` and `USER` networks 
> respectively)? That may make this message too big.
> 
> Qian Zhang wrote:
> By checking https://github.com/moby/moby/blob/master/CHANGELOG.md, I got 
> the following info:
> 1. `--dns-opt` was introduced to `docker run` in Docker 1.9.0, see 
> https://github.com/moby/moby/pull/16031.
> 2. `--dns` can be used with `HOST` network in Docker 1.12.0, see 
> https://github.com/moby/moby/pull/22408.
> 3. `--dns-option` was introduced to `docker run` in Docker 1.13.0 and 
> `--dns-opt` was hidden (but it still can be used), see 
> https://github.com/moby/moby/pull/28186.
> 
> I will do Docker version check based on the above info in 
> https://reviews.apache.org/r/60558/.

Sounds good. I think given the variations that docker supports with different 
versions I agree with you that it would be too cryptic to put this message in 
the help. Lets just add the `Docker version` check and be more explicit in our 
error messages when the version check fails.


> On July 3, 2017, 5:22 p.m., Avinash sridharan wrote:
> > 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.
> 
> Qian Zhang wrote:
> I think we do not need the network type since I will always use the 
> network name specified by framework to match `ContainerDNS.docker.network`. 
> And for sure, I will add some comments to explain how the network will be 
> matched for both CNM networks and CNI networks.

Sounds good. Thanks !!


- Avinash


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


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