Re: Review Request 58262: Inherited environment from parent when launching a DEBUG container.

2017-04-25 Thread Jie Yu

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




src/slave/containerizer/mesos/containerizer.cpp
Lines 1396 (patched)


I'd suggest we list the priority for environments for debug container here 
in the comments as well.
```
1) user specified env
2) returned by isolators
3) passed from containerizer
4) inheirited from the parent
```



src/slave/containerizer/mesos/containerizer.cpp
Line 1380 (original), 1406 (patched)


Isolator generated environments? I think we should include it for debug 
container as well.


- Jie Yu


On April 25, 2017, 9:06 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58262/
> ---
> 
> (Updated April 25, 2017, 9:06 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-6782
> https://issues.apache.org/jira/browse/MESOS-6782
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 29a99f33e646593127b9dc126f398f7bca88e21d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 56db7eb1193c9812b62b9149c9c2b2dd9b66701c 
>   src/slave/containerizer/mesos/paths.hpp 
> d85fd34660faacd9c73de2ba7b87b3bbd4b6007b 
>   src/slave/containerizer/mesos/paths.cpp 
> ed4bbd2491e71ad1e4a41e0575b514377d02da9b 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> 29007898ec04e922266068a8519731b13d351a82 
> 
> 
> Diff: https://reviews.apache.org/r/58262/diff/4/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/58718/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Re: Review Request 58603: Allowed whitelist additional devices in cgroups devices subsystem.

2017-04-25 Thread haosdent huang

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




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


Describes information abount a device.



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


Describes a device whitelist entry that expose from host to container.



src/common/type_utils.cpp
Lines 471 (patched)


Nit: Remove this blank line.



src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp
Lines 79 (patched)


Nit: Is it possible to avoid use `auto` here?



src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp
Lines 80 (patched)


Nit: `std::string` -> `string`



src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp
Lines 87-88 (patched)


Nit: we prefer to put `+` at the end.


- haosdent huang


On April 25, 2017, 5:40 a.m., Zhongbo Tian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58603/
> ---
> 
> (Updated April 25, 2017, 5:40 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Bugs: MESOS-6791
> https://issues.apache.org/jira/browse/MESOS-6791
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allowed whitelist additional devices in cgroups devices subsystem.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 159f946216299fc52171e0a58c7eb7c888c1eec8 
>   include/mesos/mesos.proto eaa2d2ac697cfc4f5aa56db0fb37363339608f43 
>   include/mesos/type_utils.hpp 5f771aaf2f4e76ac06bfd8f77b0b744ed2854b27 
>   include/mesos/v1/mesos.proto 1a32a7bdc991c77b35a988bf8a34cee936c97608 
>   src/common/parse.hpp e90738a91161e26a48a6e381765e631492294641 
>   src/common/type_utils.cpp dc0dd71f52581e2067fed279677bda8c82aa7298 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp 
> ca2727142a9f257168f3cae0958f7b4665b63cf6 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp 
> 9b5cf83093796b0c0cc5057b612f80bc8b8ba72f 
>   src/slave/flags.hpp c7a4604ed994e15c1db6accfaded2e882f1aec94 
>   src/slave/flags.cpp c50e43c0e0ba633f6b905b0d78668c0a0eebb173 
> 
> 
> Diff: https://reviews.apache.org/r/58603/diff/3/
> 
> 
> Testing
> ---
> 
> For test:
> 
> - Launch without additional devices:
>   1. Start agent with `sudo mesos-agent --master=127.0.0.1:5050 
> --work_dir=/tmp/mesos --isolation=cgroups/devices`
>   2. try open `/dev/rtc0` and failed with permission denied. `sudo 
> mesos-execute --master=127.0.0.1:5050 --name=test --command="head -c 0 
> /dev/rtc0"`
> 
> - Launch with additional devices:
>   1. Start agent with `sudo mesos-agent --master=127.0.0.1:5050 
> --work_dir=/tmp/mesos --isolation=cgroups/devices  
> --allowed_devices='{"allowed_devices":[{"device":{"path":"/dev/rtc0"}, 
> "access":{"mknod":true, "read":true, "write":true}}]}'`
>   2. open `/dev/rtc0` successfully. `sudo mesos-execute 
> --master=127.0.0.1:5050 --name=test --command="head -c 0 /dev/rtc0"`
> 
> 
> Thanks,
> 
> Zhongbo Tian
> 
>



Re: Review Request 58446: Windows: Set CMake generator for Protobuf correctly.

2017-04-25 Thread Joseph Wu

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


Ship it!




LGTM.

- Joseph Wu


On April 13, 2017, 7:03 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58446/
> ---
> 
> (Updated April 13, 2017, 7:03 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reuse the variable rather than hardcode it.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
> 
> 
> Diff: https://reviews.apache.org/r/58446/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 57898: Windows: Add deprecation warning for VS 2015.

2017-04-25 Thread Joseph Wu

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


Ship it!





cmake/CompilationConfigure.cmake
Line 141 (original), 141 (patched)


Might as well be consistent with the full generator name here:
`Visual Studio 15 2017 Win64`


- Joseph Wu


On April 13, 2017, 3:03 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57898/
> ---
> 
> (Updated April 13, 2017, 3:03 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-7391
> https://issues.apache.org/jira/browse/MESOS-7391
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Specifically this checks if the generator used did not match "Visual
> Studio 15 2017", which Mesos prefers. Note that the `CMAKE_GENERATOR`
> variable is expanded fully even when shorthand is used, e.g. `-G"Visual
> Studio 15"` will correctly match.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 5936be07557395ad08b06aa376b5a29f9b11c143 
> 
> 
> Diff: https://reviews.apache.org/r/57898/diff/1/
> 
> 
> Testing
> ---
> 
> Tested using different generators on Windows. Warning is displayed when using 
> `-G"Visual Studio 14"`, `-G"Visual Studio 14 2015"`, and `-G"Visual Studio 14 
> 2015 Win64"`, but not when using `-G"Visual Studio 15"`, `-G"Visual Studio 15 
> 2017"`, or `-G"Visual Studio 15 2017 Win64"`.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 58689: MESOS-7323: Made `addSlave` conditionally activate the framework.

2017-04-25 Thread Benjamin Mahler

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


Fix it, then Ship it!





src/master/allocator/mesos/hierarchical.cpp
Lines 379 (patched)


Hm.. I was looking for a `framework.active = true` in activateFramework but 
seems this was missed?



src/master/allocator/mesos/hierarchical.cpp
Lines 483-485 (original), 487-493 (patched)


Per our conversation, this doesn't seem necessary (intuitively addSlave 
shouldn't induce an activation of the framework within a role).

I guess we can just remove the activation code here and that makes it a 2 
line fix? No need to store the 'active' state anymore either?



src/tests/upgrade_tests.cpp
Lines 537-539 (patched)


This comment seems to be inconsistent with what the test is doing?

Also, this doesn't look like an upgrade test, should we make it a master 
test?



src/tests/upgrade_tests.cpp
Lines 612 (patched)


Hm.. not obvious to me why this was needed, can you add a comment?



src/tests/upgrade_tests.cpp
Lines 657 (patched)


Do you want to just pause the clock for the whole test to avoid relying on 
timers?



src/tests/upgrade_tests.cpp
Lines 682-683 (patched)


... and the master will track this role under the framework, but the 
framework should not receive offers for this role?

Should we test for that?


- Benjamin Mahler


On April 24, 2017, 11:18 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58689/
> ---
> 
> (Updated April 24, 2017, 11:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Neil Conway.
> 
> 
> Bugs: MESOS-7323
> https://issues.apache.org/jira/browse/MESOS-7323
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> f84b0574ce9a392c9528c87b04b01dbb2053cff7 
>   src/master/allocator/mesos/hierarchical.cpp 
> 051f749dd5921a322ca930a042c31814616d38f9 
>   src/tests/upgrade_tests.cpp b5a28b5161d896dff250b8ad012c8aac91c3f861 
> 
> 
> Diff: https://reviews.apache.org/r/58689/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` and added a test.
> 
> 
> Thanks,
> 
> Michael Park
> 
>



Re: Review Request 58673: Fix FlagsFileTest to check for absolute path properly on Windows.

2017-04-25 Thread Jeff Coffler


> On April 25, 2017, 11:16 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/path.hpp
> > Lines 79-86 (patched)
> > 
> >
> > I think this got a bit out of date. We settled on `\...` being an 
> > absolute path, not just `\?...` (per below implementation).

I didn't have a test for this specifically. The above is accurate, though. The 
1-3 cases are for files on disk. Network shares are a special case and, while 
technically an absolute path, isn't related to files on disk.

That said, the function does implement this properly. The comment isn't out of 
date. The note was just to point out that we handled this as well.


> On April 25, 2017, 11:16 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/path.hpp
> > Lines 92-108 (patched)
> > 
> >
> > I'm hoping Joe approves of this style; it's what I came up with while 
> > working with Jeff. It seems to be the most readable.
> > 
> > For what it's worth, we also tried `std::regex`, and profiled it. Even 
> > in release mode with explicit optimization, it was 70x slower.

Yeah. Before this, the code was essentially the same (a bunch of boolean 
conditionals), but rather "ugly" (three or four lines to represent the 
conditions with `||` and `&&`). At first I resisted Andy's suggestion, but 
ultimately decided it was a lot more readable, so I adopted it.

Andy strongly recommended using a RegEx. While use of a RegEx allows for a nice 
one-liner on Windows, it was problematic:

1. Windows and Linux behaved differently. Indeed, Linux would fail given a 
Regex that was perfectly valid and worked fine on Windows. Ultimately, this 
didn't matter, though, since the Windows solution was Windows-specific.

2. I've worked with RegEx engines before, and I knew that it wouldn't perform. 
RegEx engines are great for a lot of cases, but this one (a few hard-coded 
boolean comparisons) isn't one of those cases. And indeed, a RegEx for this was 
> 70x slower, which is pretty dramatic. This code is ultimately a few boolean 
conditions, although it's 15 lines of C++ code. It is nice and readable, 
though, if you understand lambda expressions.

I liked this in the end, so this was what I submitted. But behind it was 
performance analysis and all! :-)


> On April 25, 2017, 11:16 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/tests/CMakeLists.txt
> > Lines 44-59 (original), 44-59 (patched)
> > 
> >
> > Personally, I am very liberal with commits, and would have a separate 
> > patch to touch the build system. But if Joe's happy, I'm happy.

Six of one, a dozen of another. I viewed the unit test changes as "part of" the 
implementation, so I put them in a single commit. If Joe prefers, I can 
separate them out. I suppose I could have had one for the implementation, and 
then one for the unit test changes (and cmake changes, they were related).

In my mind, all code should be tested, so the unit tests were "part of" the 
implementation change. Thus, one commit. But if you take the viewpoint of "each 
commit should stand on it's own", I could have easily had a separate commit for 
the cmake and unit test changes.

Joe's call, if he wants me to make a revision, I'm happy to do it.


> On April 25, 2017, 11:16 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/tests/path_tests.cpp
> > Lines 156-173 (patched)
> > 
> >
> > I am happy to have these, although we're missing cases like a valid 
> > network path `\server\share`, and our explicitly unsupported case of a 
> > relative `\path`.

Trivial to add new test cases now (just one line per test case). I can add 
whatever you'd like if Joe wants me to.


- Jeff


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


On April 25, 2017, 6:24 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58673/
> ---
> 
> (Updated April 25, 2017, 6:24 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-5937
> https://issues.apache.org/jira/browse/MESOS-5937
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 3rdparty/stout/tests/path_tests.cpp tests to Windows platform,
> but disabled tests that did not pass. Enabled absolute path tests,
> adding tests that were appropriate for the Windows platform.
> 
> Note that, for Windows, absolute paths may 

Review Request 58725: Fetching docker image manifests with 'Accept' header.

2017-04-25 Thread Chun-Hung Hsiao

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added 'Accept: application/vnd.docker.distribution.manifest.v1+json'
to the headers of HTTP requests for fetching manifests from any Docker
registry. Some registry services (e.g., Amazon ECR) check the 'Accept'
field strictly and reject the requests if it is not specified.


Diffs
-

  src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 


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


Testing
---

sudo make check
Manually tested on a local docker private registry and an Amazon ECR repository.


Thanks,

Chun-Hung Hsiao



Re: Review Request 58688: Removed an incorrect CHECK in the master.

2017-04-25 Thread Benjamin Mahler

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


Ship it!




Ship It!

- Benjamin Mahler


On April 24, 2017, 11:18 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58688/
> ---
> 
> (Updated April 24, 2017, 11:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Neil Conway.
> 
> 
> Bugs: MESOS-7323
> https://issues.apache.org/jira/browse/MESOS-7323
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This CHECK is incorrect because a task can come back from a partitioned agent.
> Specifically,
> 
> 1. If a framework launches a task in role `R`
> 2. The agent that is running the task gets partitioned
> 3. The framework removes `R` from the list of its roles
> 4. The agent rejoins the cluster, reporting the task
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp d1cdc35a066e190ef8e0bd788e07e63b92f7fce7 
> 
> 
> Diff: https://reviews.apache.org/r/58688/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`, and added a test in https://reviews.apache.org/r/58689
> 
> 
> Thanks,
> 
> Michael Park
> 
>



Re: Review Request 58687: Made the activation of a client in `Sorter::add` explicit.

2017-04-25 Thread Benjamin Mahler

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


Fix it, then Ship it!





src/master/allocator/sorter/drf/sorter.cpp
Lines 151-155 (original)


Do we still want the CHECKs or some variant of the comment?



src/master/allocator/sorter/drf/sorter.cpp
Lines 151-156 (original)


Could we document these new semantics in the sorter interface?


- Benjamin Mahler


On April 24, 2017, 11:18 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58687/
> ---
> 
> (Updated April 24, 2017, 11:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Neil Conway.
> 
> 
> Bugs: MESOS-7323
> https://issues.apache.org/jira/browse/MESOS-7323
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 051f749dd5921a322ca930a042c31814616d38f9 
>   src/master/allocator/sorter/drf/sorter.cpp 
> ed54680cecb637931fc344fbcf8fd3b14cc24295 
>   src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a 
> 
> 
> Diff: https://reviews.apache.org/r/58687/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>



Re: Review Request 58186: Stop logging the HTTP user agent.

2017-04-25 Thread James Peach


> On April 25, 2017, 11:13 p.m., Benjamin Mahler wrote:
> > Hm.. I added this because it was particularly useful for figuring out what 
> > was hitting the master, e.g. someone's browser vs some python program, etc.

No worries. Let's drop this patch then :)


- James


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


On April 18, 2017, 10:23 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58186/
> ---
> 
> (Updated April 18, 2017, 10:23 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7340
> https://issues.apache.org/jira/browse/MESOS-7340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The HTTP user agent can be quite noisy in log files, but is not
> particularly helpful.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 89133e0175ffb792ae95705315844a32550470a1 
> 
> 
> Diff: https://reviews.apache.org/r/58186/diff/3/
> 
> 
> Testing
> ---
> 
> Make check (Fedora 25). Manual inspection of log output.
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 58673: Fix FlagsFileTest to check for absolute path properly on Windows.

2017-04-25 Thread Andrew Schwartzmeyer

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




3rdparty/stout/include/stout/flags/parse.hpp
Line 96 (original), 96 (patched)


There are more instances of erroneous absolute path checking through the 
code base, some of which we should probably fix.

That said, this change should probably be in a separate commit/patch to 
address the particular bug, followed by more patches for the other instances 
(which don't need to come immediately). But I don't think I'd include this 
directly with the porting of `path::absolute`.



3rdparty/stout/include/stout/path.hpp
Lines 79-86 (patched)


I think this got a bit out of date. We settled on `\...` being an absolute 
path, not just `\?...` (per below implementation).



3rdparty/stout/include/stout/path.hpp
Lines 92-108 (patched)


I'm hoping Joe approves of this style; it's what I came up with while 
working with Jeff. It seems to be the most readable.

For what it's worth, we also tried `std::regex`, and profiled it. Even in 
release mode with explicit optimization, it was 70x slower.



3rdparty/stout/tests/CMakeLists.txt
Lines 44-59 (original), 44-59 (patched)


Personally, I am very liberal with commits, and would have a separate patch 
to touch the build system. But if Joe's happy, I'm happy.



3rdparty/stout/tests/path_tests.cpp
Lines 156-173 (patched)


I am happy to have these, although we're missing cases like a valid network 
path `\server\share`, and our explicitly unsupported case of a relative `\path`.


- Andrew Schwartzmeyer


On April 25, 2017, 6:24 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58673/
> ---
> 
> (Updated April 25, 2017, 6:24 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-5937
> https://issues.apache.org/jira/browse/MESOS-5937
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 3rdparty/stout/tests/path_tests.cpp tests to Windows platform,
> but disabled tests that did not pass. Enabled absolute path tests,
> adding tests that were appropriate for the Windows platform.
> 
> Note that, for Windows, absolute paths may not be valid. For example,
> a path like "\\?\abc:file.txt" is not valid. In this case, the
> path::absolute method is undefined; the expectation is that it is
> called with valid paths (absolute or relative, but valid).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/parse.hpp 
> 65edd86372596c2107e9f29cf27301e025e6620e 
>   3rdparty/stout/include/stout/path.hpp 
> 2d2088aadfa1ea82c59424242671c4fb655dede1 
>   3rdparty/stout/tests/CMakeLists.txt 
> 4bbe713f259e7858d423dcb33956d41e62a915eb 
>   3rdparty/stout/tests/flags_tests.cpp 
> e2681f8f68f6478d4c8a20c1e75ddb050d16b56d 
>   3rdparty/stout/tests/path_tests.cpp 
> 0490d93908566c46a10d91b05790e5a7f2f289bc 
> 
> 
> Diff: https://reviews.apache.org/r/58673/diff/2/
> 
> 
> Testing
> ---
> 
> Passes `make check` on the Linux platform.
> 
> On Windows, the FlagsFileTest.JSONFile now passes, along with new 
> PathTest.Absolute tests that I added due to new path::absolute handling on 
> the Windows platform.
> 
> PS C:\mesos> .\build\3rdparty\stout\tests\Debug\stout-tests.exe 
> --gtest_filter="*FlagsFileTest*"
> Note: Google Test filter = *FlagsFileTest*-
> [==] Running 2 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 2 tests from FlagsFileTest
> [ RUN  ] FlagsFileTest.JSONFile
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> W0425 10:05:20.959357 1060440 parse.hpp:97] Specifying an absolute filename 
> to read a command line option out of without using 'file:// is deprecated and 
> will be removed in a future release. Simply adding 'file://' to the beginning 
> of the path should eliminate this warning.
> [   OK ] FlagsFileTest.JSONFile (9 ms)
> [ RUN  ] FlagsFileTest.FilePrefix
> [   OK ] FlagsFileTest.FilePrefix (6 ms)
> [--] 2 tests from FlagsFileTest (18 ms total)
> 
> [--] Global test environment tear-down
> [==] 2 tests from 1 test case ran. (23 ms total)
> [  PASSED  ] 2 tests.
> PS C:\mesos> .\build\3rdparty\stout\tests\Debug\stout-tests.exe 
> --gtest_filter="*PathTest*"
> Note: Google Test filter = *PathTest*-
> [==] Running 2 tests from 1 test case.
> [--] Global test 

Re: Review Request 58186: Stop logging the HTTP user agent.

2017-04-25 Thread Benjamin Mahler

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



Hm.. I added this because it was particularly useful for figuring out what was 
hitting the master, e.g. someone's browser vs some python program, etc.

- Benjamin Mahler


On April 18, 2017, 10:23 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58186/
> ---
> 
> (Updated April 18, 2017, 10:23 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7340
> https://issues.apache.org/jira/browse/MESOS-7340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The HTTP user agent can be quite noisy in log files, but is not
> particularly helpful.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 89133e0175ffb792ae95705315844a32550470a1 
> 
> 
> Diff: https://reviews.apache.org/r/58186/diff/3/
> 
> 
> Testing
> ---
> 
> Make check (Fedora 25). Manual inspection of log output.
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 58358: Update vendored protobuf tar.gz to 3.2.0.

2017-04-25 Thread Zhitao Li

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

(Updated April 25, 2017, 10:51 p.m.)


Review request for mesos, Anand Mazumdar and Joseph Wu.


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


Repository: mesos


Description
---

The content of `3rdparty/protobuf-3.2.0.tar.gz` is generated by:
- On a Mac OS, download and untar protobuf v3.2.0 source at
  https://github.com/google/protobuf/archive/v3.2.0rc2.tar.gz;
- Run `./autogen.sh`;
- Recompress and tar by `tar -czvf`.

Note that reviewboard seems to have problem for uploading/dowloading binary 
patches, so this particular commit is also available at 
https://github.com/apache/mesos/pull/208


Diffs
-

  3rdparty/protobuf-2.6.1.tar.gz e1c2d541c1eb806e837ff5d1e9d3fdb678b40e06 
  3rdparty/protobuf-3.2.0.tar.gz PRE-CREATION 


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


Testing (updated)
---

1. make check;
2. used the modified `test-upgrade.py` in r/58357 to test upgrade/downgrade 
between this chain and Mesos 1.2;
3. build this branch in Apache Aurora's vagrant based box, and verifies that 
Aurora scheduler (using java) and executor (using python) still works with 
`libmesos.so` built from this branch.


Thanks,

Zhitao Li



Re: Review Request 55903: Update XFS disk isolator documentation.

2017-04-25 Thread James Peach

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

(Updated April 25, 2017, 10:38 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Update XFS disk isolator documentation.


Diffs (updated)
-

  docs/mesos-containerizer.md d0b9d762ab83bff1cf2bd98ed566376b7bd566c6 


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

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


Testing
---

None.


Thanks,

James Peach



Re: Review Request 55897: Add support for not enforcing XFS quotas.

2017-04-25 Thread James Peach


> On April 24, 2017, 5:48 a.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/xfs/utils.cpp
> > Lines 196 (patched)
> > 
> >
> > From reading the references it seems that the soft limits and 
> > non-enforcement are two different things? 
> > 
> > Plus, the behavior when the soft limit is hit is subject to [ID zero's 
> > d_btimer 
> > value](http://xfs.org/docs/xfsdocs-xml-dev/XFS_Filesystem_Structure/tmp/en-US/html/Internal_Inodes.html)
> >  which could be set out-of-band so it could totally do things not expected 
> > here?
> > 
> > It looks like we can still just rely on `d_blk_hardlimit`: set it in 
> > the enforcement mode and don't set it at all in the accounting-only mode?
> > 
> > I could be totally mistaken but if I am, we should improve the 
> > documentation to clarify for the readers.

Reimplemented to not use soft limits.


- James


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


On April 25, 2017, 10:37 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55897/
> ---
> 
> (Updated April 25, 2017, 10:37 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5116
> https://issues.apache.org/jira/browse/MESOS-5116
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add XFS disk isolator support for not enforcing disk quotas on
> containers. While there is a global filesystem configuration option
> to turn off quota enforcement, we should not automatically toggle
> that because we don't know why the operator might have changed that
> configuration. Instead, we just apply an unlimited (0) quota, which
> engages XFS space accounting without enforcing any limit.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp 
> 52f0459421a45b01ce38b17c689633301cd97982 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
> 40f1049358ad99d3f213289e36def81c580f07f3 
>   src/slave/containerizer/mesos/isolators/xfs/utils.hpp 
> eddd4c37fb42339ca21ecb392dea47acf6b277bb 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> 7beb60b059910a0d4451b1ace895a35dc974a043 
> 
> 
> Diff: https://reviews.apache.org/r/55897/diff/5/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 55897: Add support for not enforcing XFS quotas.

2017-04-25 Thread James Peach

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

(Updated April 25, 2017, 10:37 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Jiang Yan Xu.


Changes
---

Reimplement to not use soft limits.


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


Repository: mesos


Description (updated)
---

Add XFS disk isolator support for not enforcing disk quotas on
containers. While there is a global filesystem configuration option
to turn off quota enforcement, we should not automatically toggle
that because we don't know why the operator might have changed that
configuration. Instead, we just apply an unlimited (0) quota, which
engages XFS space accounting without enforcing any limit.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/xfs/disk.hpp 
52f0459421a45b01ce38b17c689633301cd97982 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
40f1049358ad99d3f213289e36def81c580f07f3 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp 
eddd4c37fb42339ca21ecb392dea47acf6b277bb 
  src/tests/containerizer/xfs_quota_tests.cpp 
7beb60b059910a0d4451b1ace895a35dc974a043 


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

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


Testing
---

sudo make check (Fedora 25)


Thanks,

James Peach



Re: Review Request 55895: Extract a BasicBlocks class for disk block arithmetic.

2017-04-25 Thread James Peach

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

(Updated April 25, 2017, 10:37 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


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


Repository: mesos


Description
---

Extract a BasicBlocks class to handle disk blocks to clarify block-based
arithmetic in the XFS disk isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/xfs/utils.hpp 
eddd4c37fb42339ca21ecb392dea47acf6b277bb 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
8018ad348d26bd962357543a5fb9f6cb43ff13b1 
  src/tests/containerizer/xfs_quota_tests.cpp 
7beb60b059910a0d4451b1ace895a35dc974a043 


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

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


Testing
---

sudo make check (Fedora 25)


Thanks,

James Peach



Re: Review Request 55896: Stop storing agent flags in the XFS disk isolator.

2017-04-25 Thread James Peach

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

(Updated April 25, 2017, 10:37 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Jiang Yan Xu.


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


Repository: mesos


Description
---

We only need the agent work directory so there's no reason
to store a full copy of the agent flags.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/xfs/disk.hpp 
52f0459421a45b01ce38b17c689633301cd97982 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
40f1049358ad99d3f213289e36def81c580f07f3 


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

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


Testing
---

sudo make check (Fedora 25)


Thanks,

James Peach



Re: Review Request 58706: Improved unit tests for Version in stout.

2017-04-25 Thread Benjamin Mahler

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


Ship it!





3rdparty/stout/tests/version_tests.cpp
Lines 58-63 (original), 70-82 (patched)


We could simplify this a bit:

```
Try actual = Version::parse(input);

ASSERT_SOME_EQ(std::get<0>(expected), actual)
  << "Incorrect parse of input '" << input << "'";

EXPECT_EQ(std::get<1>(expected), stringify(actual.get()))
  << "Incorrect parse of input '" << input << "'";
```



3rdparty/stout/tests/version_tests.cpp
Lines 86 (patched)


This reads to me that malformed input strings can be parsed, maybe 
re-phrase? Also would be nice to use consistent terminology ("malformed" vs 
"invalid") here



3rdparty/stout/tests/version_tests.cpp
Line 64 (original), 86-106 (patched)


Naming wise, how about:

```
const vector inputs = {...};

foreach (const string& input, inputs) {
  Try parse = Version::parse(input);
  // Or:
  Try version = Version::parse(input);
  ...
}
```

Actual seems a bit out of place in this one given there's no "expected" to 
go along with it.


- Benjamin Mahler


On April 25, 2017, 8:03 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58706/
> ---
> 
> (Updated April 25, 2017, 8:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kapil Arya.
> 
> 
> Bugs: MESOS-1987
> https://issues.apache.org/jira/browse/MESOS-1987
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Switch to table-based test cases, validate that version parsing produces
> the expected results more precisely.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/version_tests.cpp 
> 724ed2292fdd3c5f4c98facf82260078b66a0e97 
> 
> 
> Diff: https://reviews.apache.org/r/58706/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Review Request 58719: CLI: Added utility functions related to HTTP.

2017-04-25 Thread Armand Grillet

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

Review request for mesos and Kevin Klues.


Repository: mesos


Description
---

This will be used by future plugins and tests.


Diffs
-

  src/cli_new/lib/cli/http.py PRE-CREATION 


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


Testing
---

Tested manually, PEP8 and Pylint used to make sure that the code style is 
correct.


Thanks,

Armand Grillet



Review Request 58720: CLI: Extended the unit test infrastructure.

2017-04-25 Thread Armand Grillet

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

Review request for mesos and Kevin Klues.


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


Repository: mesos


Description
---

This infrastructure includes the ability to bring up a test cluster to
run the CLI against. Future unit tests will use this infrastructure to
test their commands against a running mesos cluster. The tests require
some binaries created when building Mesos.


Diffs
-

  src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
  src/cli_new/lib/cli/tests/base.py PRE-CREATION 
  src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
  src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
  src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 


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


Testing
---

PEP8 and Pylint used to make sure that the code style is correct. Manuel test:

$ cd src/cli_new
$ ./bootstrap
$ source activate

(mesos-cli) $ mesos-cli-tests


Thanks,

Armand Grillet



Re: Review Request 58645: Updated the tests to verify persistent volume usage.

2017-04-25 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On April 21, 2017, 7:48 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58645/
> ---
> 
> (Updated April 21, 2017, 7:48 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4263
> https://issues.apache.org/jira/browse/MESOS-4263
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the tests to verify persistent volume usage.
> 
> 
> Diffs
> -
> 
>   src/tests/disk_quota_tests.cpp d67867e01fefeb1b94483aea5e4f219c664009e3 
> 
> 
> Diff: https://reviews.apache.org/r/58645/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 58644: Improved POSIX disk isolator to report usage for persistent volumes.

2017-04-25 Thread Gilbert Song

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/posix/disk.cpp
Line 405 (original), 404 (patched)


As you already noticed, this logic is already included in the following 
code.



src/slave/containerizer/mesos/isolators/posix/disk.cpp
Lines 437-444 (patched)


Could we merge these two `if` into one?


- Gilbert Song


On April 21, 2017, 7:48 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58644/
> ---
> 
> (Updated April 21, 2017, 7:48 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4263
> https://issues.apache.org/jira/browse/MESOS-4263
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved POSIX disk isolator to report usage for persistent volumes.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto eaa2d2ac697cfc4f5aa56db0fb37363339608f43 
>   include/mesos/v1/mesos.proto 1a32a7bdc991c77b35a988bf8a34cee936c97608 
>   src/slave/containerizer/mesos/isolators/posix/disk.cpp 
> 127f490cc0622293b4ace34510f8459b340e1742 
> 
> 
> Diff: https://reviews.apache.org/r/58644/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 58263: Inherited working dir from parent when launching a DEBUG container.

2017-04-25 Thread Alexander Rukletsov

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

(Updated April 25, 2017, 9:12 p.m.)


Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/slave/containerizer/mesos/containerizer.hpp 
29a99f33e646593127b9dc126f398f7bca88e21d 
  src/slave/containerizer/mesos/containerizer.cpp 
bc611a5e085de10e9953b5f942d98f2b5747fce6 


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


Testing
---

make check on Mac OS.
Linux pending.


Thanks,

Alexander Rukletsov



Re: Review Request 58262: Inherited environment from parent when launching a DEBUG container.

2017-04-25 Thread Alexander Rukletsov

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

(Updated April 25, 2017, 9:06 p.m.)


Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/slave/containerizer/mesos/containerizer.hpp 
29a99f33e646593127b9dc126f398f7bca88e21d 
  src/slave/containerizer/mesos/containerizer.cpp 
56db7eb1193c9812b62b9149c9c2b2dd9b66701c 
  src/slave/containerizer/mesos/paths.hpp 
d85fd34660faacd9c73de2ba7b87b3bbd4b6007b 
  src/slave/containerizer/mesos/paths.cpp 
ed4bbd2491e71ad1e4a41e0575b514377d02da9b 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
29007898ec04e922266068a8519731b13d351a82 


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


Testing (updated)
---

See https://reviews.apache.org/r/58718/


Thanks,

Alexander Rukletsov



Review Request 58718: Added a test that verifies a task's env var is seen by its check.

2017-04-25 Thread Alexander Rukletsov

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

Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 


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


Testing
---

`make check` on various Linux distributions.


Thanks,

Alexander Rukletsov



Re: Review Request 58262: Inherited environment from parent when launching a DEBUG container.

2017-04-25 Thread Alexander Rukletsov

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

(Updated April 25, 2017, 9:05 p.m.)


Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
29a99f33e646593127b9dc126f398f7bca88e21d 
  src/slave/containerizer/mesos/containerizer.cpp 
56db7eb1193c9812b62b9149c9c2b2dd9b66701c 
  src/slave/containerizer/mesos/paths.hpp 
d85fd34660faacd9c73de2ba7b87b3bbd4b6007b 
  src/slave/containerizer/mesos/paths.cpp 
ed4bbd2491e71ad1e4a41e0575b514377d02da9b 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
29007898ec04e922266068a8519731b13d351a82 


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

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


Testing
---

See https://reviews.apache.org/r/58263/


Thanks,

Alexander Rukletsov



Re: Review Request 57516: Updated CHANGELOG for hierarchical roles.

2017-04-25 Thread Neil Conway

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

(Updated April 25, 2017, 8:07 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Michael Park.


Repository: mesos


Description
---

Updated CHANGELOG for hierarchical roles.


Diffs (updated)
-

  CHANGELOG f98ce8c01041535967b07e8d9360cfbf90ac7814 


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

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


Testing
---

No functional change.


Thanks,

Neil Conway



Re: Review Request 58707: Enhanced Version in Stout to support prerelease and build labels.

2017-04-25 Thread Neil Conway

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

(Updated April 25, 2017, 8:03 p.m.)


Review request for mesos, Benjamin Mahler and Kapil Arya.


Changes
---

Tweak, rebase.


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


Repository: mesos


Description
---

Previously, Stout's `Version` abstraction only supported a subset of
Semver: version numbers with three numeric components (an optional
trailing "label" with a leading hyphen was supported but ignored).

This commit adds support for SemVer 2.0.0, which defines two additional
optional fields: a "prerelease label" and a "build metadata label",
e.g., "1.2.3-alpha.1+foo". Both labels consist of a series of
dot-separated identifiers.


Diffs (updated)
-

  3rdparty/stout/include/stout/version.hpp 
7717c85b95d29cefe8f19f3cada4b7402d4d446f 
  3rdparty/stout/tests/version_tests.cpp 
724ed2292fdd3c5f4c98facf82260078b66a0e97 


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

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


Testing
---

`make check`


Thanks,

Neil Conway



Re: Review Request 58706: Improved unit tests for Version in stout.

2017-04-25 Thread Neil Conway

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

(Updated April 25, 2017, 8:03 p.m.)


Review request for mesos, Benjamin Mahler and Kapil Arya.


Changes
---

Tweak.


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


Repository: mesos


Description
---

Switch to table-based test cases, validate that version parsing produces
the expected results more precisely.


Diffs (updated)
-

  3rdparty/stout/tests/version_tests.cpp 
724ed2292fdd3c5f4c98facf82260078b66a0e97 


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

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


Testing
---

`make check`


Thanks,

Neil Conway



Re: Review Request 58676: Logged when an agent (re-)registration request is received.

2017-04-25 Thread Anindya Sinha

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


Ship it!




Ship It!

- Anindya Sinha


On April 25, 2017, 5:33 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58676/
> ---
> 
> (Updated April 25, 2017, 5:33 p.m.)
> 
> 
> Review request for mesos, Anindya Sinha and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - This log happens after the master has done initial validation but
>before authorization, which is consistent with the (re-)register
>framework code paths.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp d1cdc35a066e190ef8e0bd788e07e63b92f7fce7 
> 
> 
> Diff: https://reviews.apache.org/r/58676/diff/1/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>



Re: Review Request 58673: Fix FlagsFileTest to check for absolute path properly on Windows.

2017-04-25 Thread Jeff Coffler

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

(Updated April 25, 2017, 6:24 p.m.)


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


Changes
---

Fixed problems on Linux platform, added unit tests for path::absolute on 
Windows platform.


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


Repository: mesos


Description (updated)
---

Added 3rdparty/stout/tests/path_tests.cpp tests to Windows platform,
but disabled tests that did not pass. Enabled absolute path tests,
adding tests that were appropriate for the Windows platform.

Note that, for Windows, absolute paths may not be valid. For example,
a path like "\\?\abc:file.txt" is not valid. In this case, the
path::absolute method is undefined; the expectation is that it is
called with valid paths (absolute or relative, but valid).


Diffs (updated)
-

  3rdparty/stout/include/stout/flags/parse.hpp 
65edd86372596c2107e9f29cf27301e025e6620e 
  3rdparty/stout/include/stout/path.hpp 
2d2088aadfa1ea82c59424242671c4fb655dede1 
  3rdparty/stout/tests/CMakeLists.txt 4bbe713f259e7858d423dcb33956d41e62a915eb 
  3rdparty/stout/tests/flags_tests.cpp e2681f8f68f6478d4c8a20c1e75ddb050d16b56d 
  3rdparty/stout/tests/path_tests.cpp 0490d93908566c46a10d91b05790e5a7f2f289bc 


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

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


Testing (updated)
---

Passes `make check` on the Linux platform.

On Windows, the FlagsFileTest.JSONFile now passes, along with new 
PathTest.Absolute tests that I added due to new path::absolute handling on the 
Windows platform.

PS C:\mesos> .\build\3rdparty\stout\tests\Debug\stout-tests.exe 
--gtest_filter="*FlagsFileTest*"
Note: Google Test filter = *FlagsFileTest*-
[==] Running 2 tests from 1 test case.
[--] Global test environment set-up.
[--] 2 tests from FlagsFileTest
[ RUN  ] FlagsFileTest.JSONFile
WARNING: Logging before InitGoogleLogging() is written to STDERR
W0425 10:05:20.959357 1060440 parse.hpp:97] Specifying an absolute filename to 
read a command line option out of without using 'file:// is deprecated and will 
be removed in a future release. Simply adding 'file://' to the beginning of the 
path should eliminate this warning.
[   OK ] FlagsFileTest.JSONFile (9 ms)
[ RUN  ] FlagsFileTest.FilePrefix
[   OK ] FlagsFileTest.FilePrefix (6 ms)
[--] 2 tests from FlagsFileTest (18 ms total)

[--] Global test environment tear-down
[==] 2 tests from 1 test case ran. (23 ms total)
[  PASSED  ] 2 tests.
PS C:\mesos> .\build\3rdparty\stout\tests\Debug\stout-tests.exe 
--gtest_filter="*PathTest*"
Note: Google Test filter = *PathTest*-
[==] Running 2 tests from 1 test case.
[--] Global test environment set-up.
[--] 2 tests from PathTest
[ RUN  ] PathTest.Absolute
[   OK ] PathTest.Absolute (1 ms)
[ RUN  ] PathTest.Comparison
[   OK ] PathTest.Comparison (0 ms)
[--] 2 tests from PathTest (2 ms total)

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

  YOU HAVE 4 DISABLED TESTS

PS C:\mesos>


Thanks,

Jeff Coffler



Review Request 58676: Logged when an agent (re-)registration request is received.

2017-04-25 Thread Jiang Yan Xu

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

Review request for mesos, Anindya Sinha and Benjamin Mahler.


Repository: mesos


Description
---

- This log happens after the master has done initial validation but
   before authorization, which is consistent with the (re-)register
   framework code paths.


Diffs
-

  src/master/master.cpp d1cdc35a066e190ef8e0bd788e07e63b92f7fce7 


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


Testing
---

make check.


Thanks,

Jiang Yan Xu



Re: Review Request 53842: Add role specific metrics for sorting runs.

2017-04-25 Thread James Peach

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


Ship it!




The code looks fine. As per the other revires this needs documentation.

Please update the metric names in the commit. The metrics are 
`allocator/mesos/frameworks/` not 
`allocator/mesos/frameworks/role/`.

- James Peach


On April 19, 2017, 8:52 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53842/
> ---
> 
> (Updated April 19, 2017, 8:52 p.m.)
> 
> 
> Review request for mesos, James Peach and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6579
> https://issues.apache.org/jira/browse/MESOS-6579
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the following 2 metrics which is maintained on a role level
> (as long as there is at least one framework of that role):
> a) allocator/mesos/frameworks/role//sort_runs: Number of framework
>level sorts (based on role) in DRF Sorter.
> b) allocator/mesos/frameworks/role//sort_run: Latency in framework
>level sorts (based on role) in DRF Sorter.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 646f66e67d9c6b8c61fc6e6558a1db976a44c126 
>   src/master/allocator/mesos/hierarchical.cpp 
> 0059ccead90f32491591990c791e7fa905a864b7 
>   src/tests/hierarchical_allocator_tests.cpp 
> cdf1f15b7802439b28405ca8f6634ce83e886630 
> 
> 
> Diff: https://reviews.apache.org/r/53842/diff/3/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>



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

2017-04-25 Thread Jiang Yan Xu

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

(Updated April 25, 2017, 10:30 a.m.)


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


Changes
---

Minor fix.


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


Repository: mesos


Description
---

Applied RegisterAgent ACL to the master.


Diffs (updated)
-

  src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034 
  src/master/master.cpp d1cdc35a066e190ef8e0bd788e07e63b92f7fce7 
  src/tests/master_authorization_tests.cpp 
1a0285a3f345ef21a8256d4123d8bb684f538da4 


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

Changes: https://reviews.apache.org/r/57535/diff/7-8/


Testing
---

make check.

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


Thanks,

Jiang Yan Xu



Re: Review Request 53840: Metric in the allocator to track latency in running allocations.

2017-04-25 Thread James Peach

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



I would also like to see some documentation of this metric in 
`docs/monitoring.md`.


src/master/allocator/mesos/hierarchical.cpp
Line 1427 (original), 1428 (patched)


You also need to stop the latency metric here, otherwise you will capture 
multiple allocator dispatches.



src/master/allocator/mesos/metrics.hpp
Line 69 (original), 69 (patched)


I think we should fix this comment. This measures the time spent in the 
allocation algorithm.



src/master/allocator/mesos/metrics.hpp
Lines 73 (patched)


Since this metric is measuring the latency from when the allocation was 
dispatched to when it executed, how about calling it `allocation_run_latency`? 
I find "interval" misleading since it is not the interval between runs.


- James Peach


On April 21, 2017, 5:53 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53840/
> ---
> 
> (Updated April 21, 2017, 5:53 p.m.)
> 
> 
> Review request for mesos, James Peach and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6579
> https://issues.apache.org/jira/browse/MESOS-6579
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a metric 'allocator/mesos/allocation_run_interval_ms' which
> tracks the latency between scheduling an allocation to an actual
> allocation run.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 051f749dd5921a322ca930a042c31814616d38f9 
>   src/master/allocator/mesos/metrics.hpp 
> ce486eb079dc44bf0bbfad8668426c7ae87c97b3 
>   src/master/allocator/mesos/metrics.cpp 
> ec987fe0ce8605bf9e7c0ccbe3ac384deaf53be1 
>   src/tests/hierarchical_allocator_tests.cpp 
> 33e7b455f8664858eb4f03727b076a10c80cd6e0 
> 
> 
> Diff: https://reviews.apache.org/r/53840/diff/4/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>



Re: Review Request 58622: Document that master message validation is best effort.

2017-04-25 Thread James Peach


> On April 24, 2017, 3:56 p.m., Neil Conway wrote:
> > src/master/validation.hpp
> > Lines 65 (patched)
> > 
> >
> > Can we link to a JIRA here?
> 
> James Peach wrote:
> Did you have a specific JIRA in mind? I think MESOS-6903 is intended for 
> this?
> 
> Neil Conway wrote:
> I didn't have anything specifically in mind, and MESOS-6903 seems too 
> generic, but when we say "the longer term remedy ...", it would be good to 
> have a corresponding JIRA. Maybe create a new JIRA and mark it as "related" 
> to 6903?

I opened [MESOS-7424](https://issues.apache.org/jira/browse/MESOS-7424).


- James


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


On April 25, 2017, 4:03 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58622/
> ---
> 
> (Updated April 25, 2017, 4:03 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-7372
> https://issues.apache.org/jira/browse/MESOS-7372
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Document that master message validation is best effort.
> 
> 
> Diffs
> -
> 
>   src/master/validation.hpp d96287de73ddb30ae2ed841c1b910b0ac6cfa74e 
> 
> 
> Diff: https://reviews.apache.org/r/58622/diff/2/
> 
> 
> Testing
> ---
> 
> None (no code changes).
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 58622: Document that master message validation is best effort.

2017-04-25 Thread James Peach

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

(Updated April 25, 2017, 4:03 p.m.)


Review request for mesos and Neil Conway.


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


Repository: mesos


Description
---

Document that master message validation is best effort.


Diffs (updated)
-

  src/master/validation.hpp d96287de73ddb30ae2ed841c1b910b0ac6cfa74e 


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

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


Testing
---

None (no code changes).


Thanks,

James Peach



Re: Review Request 58621: Add some parameter validation to RegisterSlaveMessage.

2017-04-25 Thread James Peach

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

(Updated April 25, 2017, 4:02 p.m.)


Review request for mesos and Neil Conway.


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


Repository: mesos


Description
---

Add some simple validation checks to the RegisterSlaveMessage message
to ensure that we have a minimally consistent registration request
before proceeding.


Diffs (updated)
-

  src/master/master.cpp d1cdc35a066e190ef8e0bd788e07e63b92f7fce7 
  src/master/validation.hpp d96287de73ddb30ae2ed841c1b910b0ac6cfa74e 
  src/master/validation.cpp 3f70875484bbd856ac79a7d6070ac313d69782fa 
  src/tests/master_validation_tests.cpp 
555380870ae115004312cfbe9f145faa92049030 


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

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


Testing
---

make check (Fedora 25)


Thanks,

James Peach



Re: Review Request 58262: Inherited environment from parent when launching a DEBUG container.

2017-04-25 Thread Alexander Rukletsov


> On April 13, 2017, 12:45 a.m., Vinod Kone wrote:
> > src/slave/containerizer/mesos/containerizer.cpp
> > Lines 1386-1389 (original), 1401-1404 (patched)
> > 
> >
> > Previously debug containers had this set in the env as well? But not 
> > anymore?

Correct. `environment` represents "basic" environment, e.g. agent environment. 
I'd argue that we should not set it for debug containers, but rather inherit 
parent's basic environment as part of inheritance process. Moreover, currently, 
debug container launch path does not set `environment` at all: 
https://github.com/apache/mesos/blob/aca6796b703a8925319087d33d7dc5e5539f50d3/src/slave/containerizer/mesos/containerizer.cpp#L1830


- Alexander


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


On April 7, 2017, 4:21 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58262/
> ---
> 
> (Updated April 7, 2017, 4:21 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-6782
> https://issues.apache.org/jira/browse/MESOS-6782
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 29a99f33e646593127b9dc126f398f7bca88e21d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> bc611a5e085de10e9953b5f942d98f2b5747fce6 
>   src/slave/containerizer/mesos/paths.hpp 
> d85fd34660faacd9c73de2ba7b87b3bbd4b6007b 
>   src/slave/containerizer/mesos/paths.cpp 
> ed4bbd2491e71ad1e4a41e0575b514377d02da9b 
> 
> 
> Diff: https://reviews.apache.org/r/58262/diff/3/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/58263/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Re: Review Request 58706: Improved unit tests for Version in stout.

2017-04-25 Thread Kapil Arya

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


Fix it, then Ship it!





3rdparty/stout/tests/version_tests.cpp
Line 53 (original), 65 (patched)


Please add the tagged version back.


- Kapil Arya


On April 25, 2017, 11:09 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58706/
> ---
> 
> (Updated April 25, 2017, 11:09 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kapil Arya.
> 
> 
> Bugs: MESOS-1987
> https://issues.apache.org/jira/browse/MESOS-1987
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Switch to table-based test cases, validate that version parsing produces
> the expected results more precisely.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/version_tests.cpp 
> 724ed2292fdd3c5f4c98facf82260078b66a0e97 
> 
> 
> Diff: https://reviews.apache.org/r/58706/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Review Request 58709: Prevent old Mesos agents from registering or re-registering.

2017-04-25 Thread Neil Conway

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

Review request for mesos, Benjamin Mahler and Kapil Arya.


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


Repository: mesos


Description
---

Officially, Mesos 1.0.0 (and newer) masters do not support pre-1.0.0
Mesos agents. However, we previously allowed old agents to register,
which resulted in several master crashes. As a short-term solution, we
fixed those crashes by adding backward compatibility mechanisms into the
master, but that backward compatibility code has made the master logic
more complicated and difficult to understand.

This commit changes the master to ignore registration attempts by Mesos
agents that precede Mesos 1.0.0. Now that this safety check is in place,
master logic can safely assume that all agents are running at least
Mesos 1.0.0, which will allow several simplifications to be made.


Diffs
-

  src/master/master.cpp d1cdc35a066e190ef8e0bd788e07e63b92f7fce7 
  src/tests/master_tests.cpp d1828eb42e0aedc9330c3786bbd9bb63aa42a64e 


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


Testing
---

`make check`


Thanks,

Neil Conway



Review Request 58708: Checked validity of master and agent version numbers on startup.

2017-04-25 Thread Neil Conway

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

Review request for mesos, Benjamin Mahler and Kapil Arya.


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


Repository: mesos


Description
---

During startup, we now check that the compiled version number of the
master and agent can be parsed by Stout's `Version::parse` (i.e., that
`MESOS_VERSION` uses the Semver 2.0.0 format).


Diffs
-

  src/master/main.cpp 462ed3229d15b157a92d96860503c06368ab21b7 
  src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 


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


Testing
---

`make check`


Thanks,

Neil Conway



Review Request 58707: Enhanced Version in Stout to support prerelease and build labels.

2017-04-25 Thread Neil Conway

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

Review request for mesos, Benjamin Mahler and Kapil Arya.


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


Repository: mesos


Description
---

Previously, Stout's `Version` abstraction only supported a subset of
Semver: version numbers with three numeric components (an optional
trailing "label" with a leading hyphen was supported but ignored).

This commit adds support for SemVer 2.0.0, which defines two additional
optional fields: a "prerelease label" and a "build metadata label",
e.g., "1.2.3-alpha.1+foo". Both labels consist of a series of
dot-separated identifiers.


Diffs
-

  3rdparty/stout/include/stout/version.hpp 
7717c85b95d29cefe8f19f3cada4b7402d4d446f 
  3rdparty/stout/tests/version_tests.cpp 
724ed2292fdd3c5f4c98facf82260078b66a0e97 


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


Testing
---

`make check`


Thanks,

Neil Conway



Review Request 58706: Improved unit tests for Version in stout.

2017-04-25 Thread Neil Conway

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Switch to table-based test cases, validate that version parsing produces
the expected results more precisely.


Diffs
-

  3rdparty/stout/tests/version_tests.cpp 
724ed2292fdd3c5f4c98facf82260078b66a0e97 


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


Testing
---

`make check`


Thanks,

Neil Conway



Re: Review Request 58621: Add some parameter validation to RegisterSlaveMessage.

2017-04-25 Thread Neil Conway

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




src/tests/master_validation_tests.cpp
Lines 3793 (patched)


Typo.


- Neil Conway


On April 24, 2017, 5 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58621/
> ---
> 
> (Updated April 24, 2017, 5 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-7372
> https://issues.apache.org/jira/browse/MESOS-7372
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add some simple validation checks to the RegisterSlaveMessage message
> to ensure that we have a minimally consistent registration request
> before proceeding.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp d1cdc35a066e190ef8e0bd788e07e63b92f7fce7 
>   src/master/validation.hpp d96287de73ddb30ae2ed841c1b910b0ac6cfa74e 
>   src/master/validation.cpp 3f70875484bbd856ac79a7d6070ac313d69782fa 
>   src/tests/master_validation_tests.cpp 
> 555380870ae115004312cfbe9f145faa92049030 
> 
> 
> Diff: https://reviews.apache.org/r/58621/diff/2/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 58557: Print failure reason when socket accept failed.

2017-04-25 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [58557]

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 April 25, 2017, 1:14 a.m., Andy Pang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58557/
> ---
> 
> (Updated April 25, 2017, 1:14 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Print failure reason when socket accept failed.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 92efa91 
> 
> 
> Diff: https://reviews.apache.org/r/58557/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Andy Pang
> 
>



Re: Review Request 58021: Added storage-related offer operations.

2017-04-25 Thread Jan Schlicht

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

(Updated April 25, 2017, 9:47 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Addressed issues.


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


Repository: mesos


Description
---

Added storage-related offer operations.


Diffs (updated)
-

  include/mesos/mesos.proto 0087343e3e441cfceff154d4a701298ccbe94c85 
  include/mesos/v1/mesos.proto a4426c4053460cf486534804f879feadff1bbadb 
  src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d 
  src/common/resources.cpp 77bac0c4390da905016a942991b14a79877f8455 
  src/master/master.cpp d1cdc35a066e190ef8e0bd788e07e63b92f7fce7 
  src/v1/resources.cpp 2e6b298b1f5b7728b908a4800d6495ad26aedc1d 


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

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


Testing
---

make check


Thanks,

Jan Schlicht