Re: Review Request 61849: Improved consistency of cout/cerr and glog usage in main functions.

2017-08-25 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [61849]

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 Aug. 25, 2017, 5:34 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61849/
> ---
> 
> (Updated Aug. 25, 2017, 5:34 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The main functions now have a common initialization pattern:
>   1. Load flags.
>   2. Check if the --help flag has been passed.
>   3. Check if the flags are correct.
>   4. Initialize logging.
>   5. Parse and setup the environment variables.
>   6. Initialize libprocess.
> 
> This change reduces the number of messages "WARNING: Logging
> before InitGoogleLogging() is written to STDERR" as we use
> glog only after the fourth step in the modified files.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp aff97939b088e3747111ca6ec00fe76c59e9b396 
>   src/launcher/default_executor.cpp 30bae5c931c81b81ff9590eea8051b2232388813 
>   src/launcher/executor.cpp 65b9a2c52ba5a5a514fff992018e356f388468fc 
>   src/local/main.cpp f0d7e8ca1a64b5409632ea0c0894a1dc84f9796e 
>   src/master/main.cpp 1a78a55fd52c5eebad689c2a7f16c95979260d79 
>   src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 
> 
> 
> Diff: https://reviews.apache.org/r/61849/diff/3/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 61690: Added `--zk_session_timeout` flag for agent.

2017-08-25 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [61689, 61690]

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 Aug. 25, 2017, 5:05 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61690/
> ---
> 
> (Updated Aug. 25, 2017, 5:05 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7895
> https://issues.apache.org/jira/browse/MESOS-7895
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently `ZooKeeperMasterDetector` in agent uses default session
> timeout and there's no way to configure it. This patch introduces
> `--zk_session_timeout` flag for agent similar to the one in master.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.hpp 2970fea0cfac6af275a758d4bfedfe9a943c2b60 
>   src/slave/flags.cpp 3b02f3e909a554f15104739832ae3f252926b45f 
>   src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 
> 
> 
> Diff: https://reviews.apache.org/r/61690/diff/1/
> 
> 
> Testing
> ---
> 
> Ran `make check`.
> 
> Manually verified that ZK session negotiates 20 secs timeout instead of 10 
> secs when the agent is started with `--zk_session_timeout=20secs`.
> ```
> 2017-08-15 17:50:32,065:7763(0x7051c000):ZOO_INFO@check_events@1728: 
> initiated connection to server [192.168.99.100:2181]
> 2017-08-15 17:50:32,069:7763(0x7051c000):ZOO_INFO@check_events@1775: 
> session establishment complete on server [192.168.99.100:2181], 
> sessionId=0x15de6c7f8920001, negotiated timeout=2
> ```
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>



Re: Review Request 61801: Used _EXIT macro in `CgroupsAnyHierarchyTest.ROOT_CGROUPS_Write` test.

2017-08-25 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [61797, 61798, 61799, 61800, 61801]

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 Aug. 21, 2017, 1:54 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61801/
> ---
> 
> (Updated Aug. 21, 2017, 1:54 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Benjamin 
> Bannier, Benjamin Mahler, and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used _EXIT macro in `CgroupsAnyHierarchyTest.ROOT_CGROUPS_Write` test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_tests.cpp 
> c5d83e667da0b2c8c03937c66e2a973d547cd3d2 
> 
> 
> Diff: https://reviews.apache.org/r/61801/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check (mac os x, fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Review Request 61921: Added tests to ensure that tasks can access their parent's volumes.

2017-08-25 Thread Gastón Kleiman

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

Review request for mesos and Greg Mann.


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


Repository: mesos


Description
---

These tests verifies that sibling tasks can share a Volume owned by
their parent executor using 'sandbox_path' volumes.


Diffs
-

  src/tests/default_executor_tests.cpp afe0afabf784fb65eb833beadd3c584722c321e1 


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


Testing
---

`sudo bin/mesos-tests.sh --gtest_filter="*TasksSharingViaSandboxVolumes*" 
--gtest_repeat=500 --gtest_break_on_failure` on GNU/Linux.


Thanks,

Gastón Kleiman



Review Request 61920: Added a test that uses environment secrets and the DefaultExecutor.

2017-08-25 Thread Gastón Kleiman

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

Review request for mesos and Greg Mann.


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


Repository: mesos


Description
---

This test checks that environment secrets are properly resolved and
exposed to tasks started by the DefaultExecutor.


Diffs
-

  src/tests/containerizer/environment_secret_isolator_tests.cpp 
2ba156f081c3a7c47d746f4750dbd74771b341c1 


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


Testing
---

The test succeeds on GNU/Linux.


Thanks,

Gastón Kleiman



Review Request 61919: Adjusted the test helpers for creating host and sandbox path volumes.

2017-08-25 Thread Jie Yu

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

Review request for mesos, Gilbert Song and Joseph Wu.


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


Repository: mesos


Description
---

Previously, we used the same helper for creating both host and sandbox
path volumes. This patch split that into two separate helpers.


Diffs
-

  src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
77131870ccfec0ec426962e56dc4f2cd35433946 
  src/tests/containerizer/volume_host_path_isolator_tests.cpp PRE-CREATION 
  src/tests/mesos.hpp f80e5fb0b76146e4748192361e8d1feff1ed687e 


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


Testing
---

sudo make check


Thanks,

Jie Yu



Re: Review Request 61915: Enabled `DockerContainerizerProcess::usage` for all platforms.

2017-08-25 Thread Andrew Schwartzmeyer

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

(Updated Aug. 25, 2017, 1:32 p.m.)


Review request for mesos, Jie Yu, Joseph Wu, Kevin Klues, and Li Li.


Changes
---

Add Linux test notes.


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


Repository: mesos


Description
---

When originally implemented this code was enabled only for Linux.
However, this code should work anywhere Docker is being used, as it
obtains the information by shelling out to the `docker` executable.

This patch fixes the resources information provided by the `mesos-agent`
on Windows platforms for Docker containers by making the `statistics`
JSON object available.

Note that the `cgroupsStatistics` is Linux specific, and so the
pre-processor guard was not removed but reduced in scope for just it.


Diffs
-

  src/slave/containerizer/docker.cpp cbcff39a5d8398f71570849615aeb4368130b433 


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


Testing (updated)
---

Built on Windows, started a Windows Docker container, observed the `statistics` 
field from `/container` being populated where it was not previously 
included.

`make check` passed on CentOS 7.

Diagnosing the Windows tests, have a machine configuration with Docker 
containers right now.


Thanks,

Andrew Schwartzmeyer



Review Request 61915: Enabled `DockerContainerizerProcess::usage` for all platforms.

2017-08-25 Thread Andrew Schwartzmeyer

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

Review request for mesos, Jie Yu, Joseph Wu, and Li Li.


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


Repository: mesos


Description
---

When originally implemented this code was enabled only for Linux.
However, this code should work anywhere Docker is being used, as it
obtains the information by shelling out to the `docker` executable.

This patch fixes the resources information provided by the `mesos-agent`
on Windows platforms for Docker containers by making the `statistics`
JSON object available.

Note that the `cgroupsStatistics` is Linux specific, and so the
pre-processor guard was not removed but reduced in scope for just it.


Diffs
-

  src/slave/containerizer/docker.cpp cbcff39a5d8398f71570849615aeb4368130b433 


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


Testing
---

Built on Windows, started a Windows Docker container, observed the `statistics` 
field from `/container` being populated where it was not previously 
included.

Building and running `mesos-tests`, update pending.


Thanks,

Andrew Schwartzmeyer



Re: Review Request 61473: Do not kill non partition aware tasks.

2017-08-25 Thread Megha Sharma


> On Aug. 10, 2017, 9 p.m., Jiang Yan Xu wrote:
> > Some of the comments below were made before I started to feel that we are 
> > probably doing too many conversions to justify storing these tasks in 
> > TASK_UNREACHABLE. Perhaps we can just store them in 
> > `Framework.unreachableTasks` but in TASK_LOST state?
> > 
> > It's possible that we can add another map `BoundedHashMap > process::Owned> unreachableNonPartitionAwareTasks;` for these tasks 
> > but it's clunky in the sense that you have to clarify that 
> > `unreachableTasks` is only for partition aware tasks but in fact all of 
> > these tasks belong to the same framework which is either parition aware or 
> > not, however with the possibility of changing capability... so it's 
> > probably easier to describe things if we just put all of them in 
> > `unreachableTasks` and simply say that "if the framework is not 
> > partition-aware, the tasks stored in `unreachableTasks` may be in 
> > `TASK_LOST`".
> > 
> > If we do that, then some of the comments below don't apply any more but I 
> > am keep them just for posterity (some styling issues etc).

Dicussed in person, +1 for keeping the non partition aware but unreachable 
tasks in Framework.unreachableTasks in state TASK_LOST.


- Megha


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


On Aug. 10, 2017, 4:07 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61473/
> ---
> 
> (Updated Aug. 10, 2017, 4:07 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7215
> https://issues.apache.org/jira/browse/MESOS-7215
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master will not kill the tasks for non-Partition aware frameworks
> when an unreachable agent re-registers with the master.
> Master used to send a ShutdownFrameworkMessages to the agent
> to kill the tasks from non partition aware frameworks including the
> ones that are still registered which was problematic because the offer
> from this agent could still go to the same framework which could then
> launch new tasks. The agent would then receive tasks of the same
> framework and ignore them because it thinks the framework is shutting
> down. The framework is not shutting down of course, so from the master
> and the scheduler’s perspective the task is pending in STAGING forever
> until the next agent reregistration, which could happen much later.
> This commit fixes the problem by not shutting down the non-partition
> aware frameworks on such an agent.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
>   src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
>   src/tests/partition_tests.cpp 62a84f797201ccd18b71490949e3130d2b9c3668 
> 
> 
> Diff: https://reviews.apache.org/r/61473/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>



Re: Review Request 61849: Improved consistency of cout/cerr and glog usage in main functions.

2017-08-25 Thread Armand Grillet

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

(Updated Aug. 25, 2017, 5:34 p.m.)


Review request for mesos, Andrei Budnik and Alexander Rukletsov.


Changes
---

Moved comments to resolve issues.


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


Repository: mesos


Description (updated)
---

The main functions now have a common initialization pattern:
  1. Load flags.
  2. Check if the --help flag has been passed.
  3. Check if the flags are correct.
  4. Initialize logging.
  5. Parse and setup the environment variables.
  6. Initialize libprocess.

This change reduces the number of messages "WARNING: Logging
before InitGoogleLogging() is written to STDERR" as we use
glog only after the fourth step in the modified files.


Diffs (updated)
-

  src/docker/executor.cpp aff97939b088e3747111ca6ec00fe76c59e9b396 
  src/launcher/default_executor.cpp 30bae5c931c81b81ff9590eea8051b2232388813 
  src/launcher/executor.cpp 65b9a2c52ba5a5a514fff992018e356f388468fc 
  src/local/main.cpp f0d7e8ca1a64b5409632ea0c0894a1dc84f9796e 
  src/master/main.cpp 1a78a55fd52c5eebad689c2a7f16c95979260d79 
  src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 


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

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


Testing
---

```
$ make check
```


Thanks,

Armand Grillet



Re: Review Request 61849: Improved consistency of cout/cerr and glog usage in main functions.

2017-08-25 Thread Andrei Budnik

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




src/master/main.cpp
Line 138 (original), 138 (patched)


Should we update this comment?



src/slave/main.cpp
Line 231 (original), 231 (patched)


Should we update this comment as well?


- Andrei Budnik


On Aug. 23, 2017, 2:21 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61849/
> ---
> 
> (Updated Aug. 23, 2017, 2:21 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The main functions now have a common pattern, in 6 steps:
>   1. Load flags.
>   2. Check if the --help flag has been passed.
>   3. Check if the flags are correct.
>   4. Initialize logging.
>   5. Parse and setup the environment variables.
>   6. Initialize libprocess.
> 
> This change reduces the number of messages "WARNING: Logging
> before InitGoogleLogging() is written to STDERR" as we use
> glog only after the fourth step in the modified files.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 99a62244b7805d25b926d0293cb62574981d2878 
>   src/launcher/default_executor.cpp 30bae5c931c81b81ff9590eea8051b2232388813 
>   src/launcher/executor.cpp 65b9a2c52ba5a5a514fff992018e356f388468fc 
>   src/local/main.cpp f0d7e8ca1a64b5409632ea0c0894a1dc84f9796e 
>   src/master/main.cpp 1a78a55fd52c5eebad689c2a7f16c95979260d79 
>   src/slave/main.cpp a4a8ced6825ece1b25003824d987ff83fb799ad7 
> 
> 
> Diff: https://reviews.apache.org/r/61849/diff/2/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 61799: Replaced ABORT with _EXIT in `childMain` to handle `os::execvpe` error.

2017-08-25 Thread Andrei Budnik


> On Aug. 24, 2017, 10:59 p.m., James Peach wrote:
> > 3rdparty/libprocess/src/subprocess_posix.hpp
> > Lines 200 (patched)
> > 
> >
> > Is this required to be async-signal-safe?

After latest changes `_EXIT` supports formatted output.
I've eliminated signal unsafe dependency to `os::strerror`.


- Andrei


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


On Aug. 21, 2017, 8:54 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61799/
> ---
> 
> (Updated Aug. 21, 2017, 8:54 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Benjamin 
> Bannier, Benjamin Mahler, and James Peach.
> 
> 
> Bugs: MESOS-7791
> https://issues.apache.org/jira/browse/MESOS-7791
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Using ABORT causes coredump when path to an executable is invalid.
> In addition, using of _EXIT allows us to get rid of string
> concatenation, thereby making `childMain` more async-signal safe.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess_posix.hpp 
> b478beccff22e2ffa08b9b91cfd5b9c6ada9b697 
> 
> 
> Diff: https://reviews.apache.org/r/61799/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check (mac os x, fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 61798: Added _EXIT as alternative to ABORT.

2017-08-25 Thread Andrei Budnik


> On Aug. 24, 2017, 10:59 p.m., James Peach wrote:
> > 3rdparty/stout/include/stout/exit.hpp
> > Lines 39 (patched)
> > 
> >
> > Move this comment to the `_EXIT()` macro.

Done.


> On Aug. 24, 2017, 10:59 p.m., James Peach wrote:
> > 3rdparty/stout/include/stout/exit.hpp
> > Lines 42 (patched)
> > 
> >
> > The `EXIT` macro produces something like this:
> > ```
> > E0824 15:33:17.790952 32725 main.cpp:497] EXIT with status 1: Failed to 
> > create a master detector: Failed to parse '///'
> > ```
> > 
> > It would be good to make `_EXIT` consistent, and I think you can use 
> > the `RAW_LOG_XXX` interfaces in `glog/raw_logging.h` for this.

Fixed: `_EXIT` uses `RAW_LOG_XXX`.


> On Aug. 24, 2017, 10:59 p.m., James Peach wrote:
> > 3rdparty/stout/include/stout/exit.hpp
> > Lines 77 (patched)
> > 
> >
> > Lets be consistent with other code by using `::_exit(status)` here.

Fixed.
BTW, `std::_Exit` was introduced in c++11.


- Andrei


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


On Aug. 21, 2017, 8:54 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61798/
> ---
> 
> (Updated Aug. 21, 2017, 8:54 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Benjamin 
> Bannier, Benjamin Mahler, and James Peach.
> 
> 
> Bugs: MESOS-7791
> https://issues.apache.org/jira/browse/MESOS-7791
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> EXIT macro can't be used when async-signal safety is required.
> 
> Currently, ABORT is used for terminating a program after both
> unexpected and expected errors, when async-signal safety is required.
> In case of expected errors, `abort` causes coredumps, which might be
> undesirable.
> 
> In addition, _EXIT supports formatted output conversion,
> thereby _EXIT interface doesn't force users to concatenate strings,
> thus making its usage more async-signal safe.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/exit.hpp 
> e5c2d3440a85ab2d2cae551ee4094cd965e38dfc 
> 
> 
> Diff: https://reviews.apache.org/r/61798/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check (mac os x, fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 61797: Added async-signal safe implementation of `write` function in stout.

2017-08-25 Thread Andrei Budnik


> On Aug. 24, 2017, 6:28 p.m., James Peach wrote:
> > 3rdparty/stout/include/stout/os/write.hpp
> > Lines 66 (patched)
> > 
> >
> > Write returns `ssize_t` here and in all the other variants in this file.
> > 
> > OK I ended up later in the patch series and realized that this is 
> > returning `errno`. If you really want to do that, it needs to be much more 
> > explicit, since everyone reading this is going to assume that `write` 
> > returns a count.
> > 
> > Other than introducing a new type, one option might be to return `bool` 
> > from these functions and let the caller examine `errno`.
> 
> Andrei Budnik wrote:
> I agree, returning `ssize_t` seems more reasonable to me.

Fixed: `write` returns `ssize_t`.


- Andrei


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


On Aug. 21, 2017, 8:54 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61797/
> ---
> 
> (Updated Aug. 21, 2017, 8:54 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Benjamin 
> Bannier, Benjamin Mahler, and James Peach.
> 
> 
> Bugs: MESOS-7791
> https://issues.apache.org/jira/browse/MESOS-7791
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Async-signal safe `write` function, which can take variable number of
> arguments, should be used to improve signal safety.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/abort.hpp 
> d3291dce8ec3db6141124e9cd231d34409998a18 
>   3rdparty/stout/include/stout/os/write.hpp 
> beb5bd83b52565a75e34d32b5bb17951bc799578 
> 
> 
> Diff: https://reviews.apache.org/r/61797/diff/5/
> 
> 
> Testing
> ---
> 
> sudo make check (mac os x, fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



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

2017-08-25 Thread Alexander Rukletsov

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


Ship it!




Ship It!

- Alexander Rukletsov


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 61597: Fixed linking to `IPHlpAPI` library.

2017-08-25 Thread Alexander Rukletsov


> On Aug. 18, 2017, 9:29 a.m., Alexander Rukletsov wrote:
> > Ship It!

I've tested the chain up to this commit in my CMake setup (integration with Qt 
Creator + ccache) and confirm it works as expected (targets are set up 
properly, building and running tests works).


- Alexander


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


On Aug. 11, 2017, 11:27 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61597/
> ---
> 
> (Updated Aug. 11, 2017, 11:27 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler and Joseph Wu.
> 
> 
> Bugs: MESOS-7704
> https://issues.apache.org/jira/browse/MESOS-7704
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the `#pragma comment(lib...)` method works with MSVC, it is
> inconsistent with how we link to the rest of the Windows libraries. So
> it was deleted and `IPHlpAPI` was added to stout's Windows dependencies.
> 
> Furthermore, the `` header was removed from the individual
> files and placed in `windows.hpp` to ensure it is included in the
> correct order with respect to the other Windows headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 76b2a67419cb6836b598a0070f26632d4ca048ff 
>   3rdparty/stout/include/stout/windows.hpp 
> 92db7278aea5cc9b94bec77071b9803f58042624 
>   3rdparty/stout/include/stout/windows/ip.hpp 
> 76f23c823662a54162e64160980512b191bb88e8 
>   3rdparty/stout/include/stout/windows/mac.hpp 
> 09c4c9626d135705a502b6d148f5b6ba64b688cd 
>   3rdparty/stout/include/stout/windows/net.hpp 
> 1418b5c981a2286c9ae390d801a8021e3a442f5b 
> 
> 
> Diff: https://reviews.apache.org/r/61597/diff/1/
> 
> 
> Testing
> ---
> 
> stout-tests on Windows
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 61874: Added a test for IPv6 containers on docker user networks.

2017-08-25 Thread Avinash sridharan

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

(Updated Aug. 25, 2017, 6:59 a.m.)


Review request for mesos, Jie Yu and Qian Zhang.


Changes
---

Addressed Qian's comments.


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


Repository: mesos


Description
---

The test creates a IPv6 docker user network. It than launches an alpine
image on the docker user network. Finally it checks if the IP address
allocated to the container are reflected correctly in state.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
6856ca22570183b022d401d1fa8f59d819783eed 


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

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


Testing
---

sudo bin/mesos-tests.sh 
--gtest_filter="*.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container"
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from DockerContainerizerIPv6UserNetworkTest
[ RUN  ] 
DockerContainerizerIPv6UserNetworkTest.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container
[   OK ] 
DockerContainerizerIPv6UserNetworkTest.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container
 (15660 ms)
[--] 1 test from DockerContainerizerIPv6UserNetworkTest (15664 ms total)

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


Thanks,

Avinash sridharan



Re: Review Request 61874: Added a test for IPv6 containers on docker user networks.

2017-08-25 Thread Avinash sridharan

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

(Updated Aug. 25, 2017, 6:56 a.m.)


Review request for mesos, Jie Yu and Qian Zhang.


Changes
---

Addressed Qian's comments.


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


Repository: mesos


Description
---

The test creates a IPv6 docker user network. It than launches an alpine
image on the docker user network. Finally it checks if the IP address
allocated to the container are reflected correctly in state.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
6856ca22570183b022d401d1fa8f59d819783eed 


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

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


Testing
---

sudo bin/mesos-tests.sh 
--gtest_filter="*.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container"
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from DockerContainerizerIPv6UserNetworkTest
[ RUN  ] 
DockerContainerizerIPv6UserNetworkTest.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container
[   OK ] 
DockerContainerizerIPv6UserNetworkTest.ROOT_DOCKER_USERNETWORK_LaunchIPv6Container
 (15660 ms)
[--] 1 test from DockerContainerizerIPv6UserNetworkTest (15664 ms total)

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


Thanks,

Avinash sridharan



Re: Review Request 61873: Added test filter for docker user network tests.

2017-08-25 Thread Avinash sridharan

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

(Updated Aug. 25, 2017, 6:54 a.m.)


Review request for mesos, Jie Yu and Qian Zhang.


Changes
---

Fixed Qian's comments.


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


Repository: mesos


Description
---

Added test filter for docker user network tests.


Diffs (updated)
-

  src/tests/environment.cpp a7262cd97361b55ff31238341657e764df6c9ea5 


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

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


Testing
---

make, make check


Thanks,

Avinash sridharan



Re: Review Request 61237: Updated docker executor to return IPv6 address of a container.

2017-08-25 Thread Avinash sridharan

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

(Updated Aug. 25, 2017, 6:54 a.m.)


Review request for mesos, Jie Yu and Qian Zhang.


Changes
---

Fixed Qian's comments.


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


Repository: mesos


Description
---

A docker container can have a v4 and a v6 address. Currently the docker
executor was returning only the IPv4 address. This changes allows the
executor to return the IPv4 and IPv6 address of the container.


Diffs (updated)
-

  src/docker/docker.hpp 95e60a7dbbd6ccc659f70ca3dca8d13433f3ea07 
  src/docker/docker.cpp 192e170acce895f80df6c83e437da020489de468 
  src/docker/executor.cpp 99a62244b7805d25b926d0293cb62574981d2878 


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

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


Testing
---

Start agent:
sudo ./bin/mesos-agent.sh --ip=10.0.2.15 --master=10.0.2.15:5050 
--work_dir=/tmp/mesos --containerizers=mesos,docker

Start master:
sudo ./bin/mesos-master.sh --ip=10.0.2.15 --work_dir=/tmp/mesos-master

Use mesos-execute to launch a docker container on an IPv6 network:
sudo src/mesos-execute --master=10.0.2.15:5050 
--task=file:///home/vagrant/dev/mesos_tasks/docker.json 2> log.txt

cat /home/vagrant/dev/mesos_tasks/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": "USER"
},
"network_infos": [
{
  "name": "ipv6"
}
]
  }
}


Curl the Mesos state to make sure that the right IPv6 addresses are reflected 
for the container:
 curl http://10.0.2.15:5051/state | jq

"role": "*",
  "statuses": [
{
  "state": "TASK_RUNNING",
  "timestamp": 1501288232.79384,
  "container_status": {
"container_id": {
  "value": "59319a5b-a28b-46bf-a20f-8472e6598abd"
},
"network_infos": [
  {
"ip_addresses": [
  {
"protocol": "IPv4",
"ip_address": "172.19.0.2"
  },
  {
"protocol": "IPv6",
"ip_address": "fd00:0:0:1::2"
  }
],
"name": "ipv6"
  }
]
  }
}
  ],
  "container": {
"type": "DOCKER",
"docker": {
  "image": "nginx",
  "network": "USER",
  "privileged": false
},
"network_infos": [
  {
"name": "ipv6"
  }
]
  }
}
  ],


Thanks,

Avinash sridharan