Re: Review Request 45057: Made `unzip` overwrite existing files without prompting.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45057]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 20, 2016, 12:05 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45057/
> ---
> 
> (Updated March 20, 2016, 12:05 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4885
> https://issues.apache.org/jira/browse/MESOS-4885
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made `unzip` overwrite existing files without prompting.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45057/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 45010: Fixed newlines for include directives.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 18, 2016, 12:44 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45010/
> ---
> 
> (Updated March 18, 2016, 12:44 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed newlines for include directives.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> daa46e05d113fd62ea9dad042ec41aaab28ad003 
> 
> Diff: https://reviews.apache.org/r/45010/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 45057: Made `unzip` overwrite existing files without prompting.

[haosdent huang]

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


Fix it, then Ship it!




Ship It!


src/tests/fetcher_tests.cpp (line 703)


should we replace `af083b2d` to `440a6aa5` here?


- haosdent huang


On March 20, 2016, 12:05 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45057/
> ---
> 
> (Updated March 20, 2016, 12:05 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4885
> https://issues.apache.org/jira/browse/MESOS-4885
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made `unzip` overwrite existing files without prompting.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45057/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 44669: Added createFromModule methods to MasterContender and MasterDetector.

[Anurag Singh]

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

(Updated March 18, 2016, 12:29 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

The createFromModule will be used to create a MasterContender/Detector
from a module (specified using the --modules flag on the command
line).


Diffs (updated)
-

  src/master/contenders/contender.cpp PRE-CREATION 
  src/master/detectors/detector.cpp PRE-CREATION 

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


Testing
---

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


Thanks,

Anurag Singh

Re: Review Request 44758: Upgrade to clang-format-3.8 (MESOS-4906).

[Yong Tang]

> On March 17, 2016, 4:09 p.m., Michael Park wrote:
> > The patch looks good to me. Would you like to send an announcement email to 
> > the dev list about this upgrade? I can do it if you'd rather not.
> 
> Yong Tang wrote:
> Hi Michael, Thanks a lot for the help! I will send out the email to the 
> dev list shortly.

Hi Michael, just sent out the email. Let me know if there is any additional 
issues.


- Yong


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


On March 16, 2016, 1:37 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44758/
> ---
> 
> (Updated March 16, 2016, 1:37 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4906
> https://issues.apache.org/jira/browse/MESOS-4906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade to clang-format-3.8 (MESOS-4906).
> 
> 
> Diffs
> -
> 
>   docs/clang-format.md 7f1c1dfd70e1fe9bfa186df1bdda7bdcf867db04 
>   support/clang-format 499d0e749e14e50256ae649afa0ced2b04589a0e 
> 
> Diff: https://reviews.apache.org/r/44758/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 44082: Stout: Un-commented out functions and marked them as deleted instead.

[Michael Park]

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



Is this the right approach for all of them?

https://reviews.apache.org/r/44081 introduces the `setPaths` function in 
`stout/os.hpp` (not `stout/posix/os.hpp`),
which uses `setenv`. If `setenv` is marked `= delete`, then this will be a 
compiler error.

It seems that the intent may have been to say, "if `setPaths` is __used__, then 
it should be a compiler error since
`setPaths` uses `setenv` which is marked `= delete`. This is not true.

I think perhaps what we want is to mark `setPaths` also `= delete` for Windows?

Please help me understand! :)

- Michael Park


On March 14, 2016, 9:12 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44082/
> ---
> 
> (Updated March 14, 2016, 9:12 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Un-commented out functions and marked them as deleted instead.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> 6a391ff198ab724f689bcef79d4e2e05a786cbc2 
> 
> Diff: https://reviews.apache.org/r/44082/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 44944: Handled chunked responses in docker URI fetcher.

[James Peach]

> On March 17, 2016, 3:44 p.m., James Peach wrote:
> > Ship It!

Verified that this patch fixes the chunked response parsing.


- James


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


On March 17, 2016, 1:50 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44944/
> ---
> 
> (Updated March 17, 2016, 1:50 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, James Peach, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-4964
> https://issues.apache.org/jira/browse/MESOS-4964
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Handled chunked responses in docker URI fetcher.
> 
> This is because if the response is chunked, curl will output the dechunked 
> version. That will confuse the http response decoder because the header shows 
> that it is chunked.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 2ceee199c6b1bf92c5eca80c8d3344e97e394a09 
> 
> Diff: https://reviews.apache.org/r/44944/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 44851: Renamed an allocator metric.

[Benjamin Bannier]

> On March 18, 2016, 3:59 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/metrics.hpp, lines 40-42
> > 
> >
> > Can you please file a JIRA to desribe the problem? Not quite catch why 
> > do we need to add a metrics in 0.29 and then remove it in 0.30?
> > 
> > Or else can you please update the comments to be more clear?

Follow-up commits will add more allocator-specific metrics. Since they are all 
specific to the mesos allocator we should properly communicate that in their 
names.


- Benjamin


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


On March 17, 2016, 6:41 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44851/
> ---
> 
> (Updated March 17, 2016, 6:41 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since allocators can be replaced with a custom module instead use a
> name clearly communicating that the metrics reported here are related
> to the default (hierarchical) allocator.
> 
> For backwards compatibility we continue to support the old metrics
> name.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/metrics.hpp 
> d04e9a11c77b6fb2d522608e3085f81f8a6657ca 
>   src/master/allocator/mesos/metrics.cpp 
> 46dd7f7b3d8d49c59b42e789fe5dcd49f1693ace 
> 
> Diff: https://reviews.apache.org/r/44851/diff/
> 
> 
> Testing
> ---
> 
> check make check (OS X, clang-trunk) here and with later patches using other 
> allocator metrics.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 44703: Added Doxygen docs for basic HTTP authenticator.

[Adam B]

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



Please be careful how you use "should" vs. "must" vs. "may". See 
http://tools.ietf.org/html/rfc2119


include/mesos/authentication/http/basic_authenticator_factory.hpp (line 40)


s/should/must/? Which ones are required vs. optional?



include/mesos/authentication/http/basic_authenticator_factory.hpp (line 42)


s/should/must/?



include/mesos/authentication/http/basic_authenticator_factory.hpp (lines 56 - 
57)


Why are these quotes escaped, but the others aren't? Because they're inside 
a string "value"? Seems odd.



include/mesos/authentication/http/basic_authenticator_factory.hpp (line 64)


s/should/must/ or s/should/may/?



include/mesos/authentication/http/basic_authenticator_factory.hpp (line 66)


s/which contains/containing/
s/the authenticator/a new authenticator/


- Adam B


On March 17, 2016, 12:40 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44703/
> ---
> 
> (Updated March 17, 2016, 12:40 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Doxygen docs for basic HTTP authenticator.
> 
> The structure of the module parameters for the basic HTTP authenticator has 
> changed, so Doxygen comments were added to the module's header file to 
> provide an example.
> 
> 
> Diffs
> -
> 
>   include/mesos/authentication/http/basic_authenticator_factory.hpp 
> c11bb47c8e02f2e8645cf387d18eb64d1c8cb604 
> 
> Diff: https://reviews.apache.org/r/44703/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44949: Add XFS disk isolator tests.

[James Peach]

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

(Updated March 18, 2016, 3:46 a.m.)


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


Changes
---

Rebased.


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


Repository: mesos


Description
---

Add basic XFS disk isolator tests by cloning the POSIX disk isolator
tests and making minor changes for the differences in semantics.


Diffs (updated)
-

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

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


Testing
---

Make check. Manual testing.


Thanks,

James Peach

Re: Review Request 44948: Add XFS disk resource isolator.

[James Peach]

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

(Updated March 17, 2016, 10:42 p.m.)


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


Changes
---

Rebased and fixed review comments.


Bugs: MESOs-4828
https://issues.apache.org/jira/browse/MESOs-4828


Repository: mesos


Description
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
  src/slave/containerizer/mesos/containerizer.cpp 
4638d08328127a8c4ae37555f2be74fe9695da31 
  src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 

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


Testing
---

Make check. Manual testing. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44874: Added support for FTS_SLNONE in rmdir.

[Neil Conway]

> On March 16, 2016, 12:29 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp, line 222
> > 
> >
> > Why is this a `string&` and `newDirectory` is a `string`? I'd think 
> > both should probably be `string`.
> 
> Jie Yu wrote:
> Good catch, we should use 'string'. We avoid using const ref to capture 
> temp variable.
> 
> Jojy Varghese wrote:
> A pattern I copied from `os_tests.cpp`. Copy paste horror. I think we 
> need to fix it there as well as we fix it here.

Using a const ref should be safe (it _must_ be a const ref, in which case C++ 
will extend the lifetime of the temporary to that of the const reference -- see 
https://stackoverflow.com/questions/2822243/store-return-value-of-function-in-reference-c).
 However, it would be good to be consistent. Given RVO, I'm not sure there's 
anything to be gained by using the reference form.


- Neil


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


On March 15, 2016, 11:47 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44874/
> ---
> 
> (Updated March 15, 2016, 11:47 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for FTS_SLNONE in rmdir.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cf21e7fe458626c7533e596997cab3afdabb55f4 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> 291a22b5aae53b0bc32ae18b9343ceb5a638b37b 
> 
> Diff: https://reviews.apache.org/r/44874/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44934: Updated mesos-execute to add support for Appc.

[Jojy Varghese]

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

(Updated March 20, 2016, 4:19 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Updated mesos-execute to add support for Appc.


Diffs (updated)
-

  src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 

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


Testing
---

Tested with various Appc images.


Thanks,

Jojy Varghese

Re: Review Request 45002: Added FTS_PHYSICAL option to fts_open for rmdir.

[Jojy Varghese]

> On March 20, 2016, 2:36 a.m., Neil Conway wrote:
> > Per discussion in https://reviews.apache.org/r/45003/, if we can actually 
> > run into `FTS_SLNONE`, we should have a test-case for it.

I dont think with FTS_PHYSICAL (and without FTW_COMFOLLOW) we will get 
FTS_SLNONE. I could be wrong but having a peek inside the glibc code for 
fts_open, I believe thats the case.


- Jojy


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


On March 18, 2016, 12:16 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45002/
> ---
> 
> (Updated March 18, 2016, 12:16 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> According to the documentation for fts_open, either FTS_PHYSICAL or 
> FTS_LOGICAL
> SHOULD be provided. We need FTS_PHYSICAL for the case of rmdir as we dont want
> to resolve the symlink targets while deleting them.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> daa46e05d113fd62ea9dad042ec41aaab28ad003 
> 
> Diff: https://reviews.apache.org/r/45002/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 45003: Fixed rmdir comment for FTS_SLNONE as per coding guidelines.

[Jojy Varghese]

> On March 20, 2016, 2:33 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp, line 71
> > 
> >
> > This comment seems incorrect:
> > 
> > (1) The relevant parameters are `FTS_PHYSICAL` vs. `FTS_LOGICAL`, as 
> > well as `FTS_COMFOLLOW`.
> > 
> > (2) Since we _do_ set `FTS_PHYSICAL`, isn't it possible for 
> > `FTS_SLNONE` to be returned?

I believe FTS_LOGICAL or FTS_COMFOLLOW needs to be set in order for FTS_SLNONE 
to be returned.


- Jojy


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


On March 18, 2016, 12:17 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45003/
> ---
> 
> (Updated March 18, 2016, 12:17 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed rmdir comment for FTS_SLNONE as per coding guidelines.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
> 
> Diff: https://reviews.apache.org/r/45003/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45046]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 19, 2016, 10:03 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45046/
> ---
> 
> (Updated March 19, 2016, 10:03 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Zhitao Li.
> 
> 
> Bugs: MESOS-4735
> https://issues.apache.org/jira/browse/MESOS-4735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created URI.filename to name fetched files in sandbox where appropriate.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/slave/containerizer/fetcher.cpp 
> 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
>   src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45046/diff/
> 
> 
> Testing
> ---
> 
> There are two paths by which a file gets fetched to the executor sandbox: the 
> without-cache path, where the fetcher downloads the file directly from the 
> specified URI, and the with-cache path, where it copies it from the cache. In 
> both cases, we verify that the file is saved to the sandbox directory with 
> the name specified by the "filename" field in the CommandInfo.URI proto.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Re: Review Request 44661: Deprecated the `docker_stop_timeout` flag.

[Alexander Rukletsov]

> On March 15, 2016, 11:34 p.m., Ben Mahler wrote:
> > Looking pretty good. Would be great to have a CHANGELOG update here that 
> > outlines the deprecation and what we're advising users to do in each 
> > version.

I'll update the changelog in the next patch, hope this is fine.


> On March 15, 2016, 11:34 p.m., Ben Mahler wrote:
> > src/docker/executor.hpp, line 74
> > 
> >
> > Just to be clear, is the intention to remove this in 0.30? If so, can 
> > you say that explicitly? Or are we giving time for users to set kill 
> > policies and planning to remove it 6 months from 0.29.0?

I think our *current* strategy is to remove in 6 months. That's why I put the 
version when we started the deprecation cycle.


> On March 15, 2016, 11:34 p.m., Ben Mahler wrote:
> > src/slave/flags.cpp, line 532
> > 
> >
> > poly?

Yeah, a small pony sneaked in : ).


> On March 15, 2016, 11:34 p.m., Ben Mahler wrote:
> > src/slave/containerizer/docker.cpp, line 967
> > 
> >
> > Hm.. I'm a bit confused about the deprecation taking place here. Could 
> > you outline the steps in this description? Specifically, what does the 
> > 0.28, 0.29, and final versions look like and what are we advising users to 
> > do in each version?
> > 
> > This information would be great to have in the CHANGELOG deprecation 
> > message I suggested above.

I think we can remove this timeout now, but it's hard to say whether there are 
any users relying on the unfortunate fact that the containerizer does not kill 
immediately. Hence I decided to remove it via a deprecation cycle. On the other 
side, deprecating a bug is a bit strange. What do you think?


- Alexander


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


On March 15, 2016, 2:28 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44661/
> ---
> 
> (Updated March 15, 2016, 2:28 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Bugs: MESOS-4910
> https://issues.apache.org/jira/browse/MESOS-4910
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead, a combination of `executor_shutdown_grace_period`
> agent flag and task kill policies should be used.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 739d4ff9aeeb1ba70ce11033168d63d37b6ef56b 
>   src/docker/executor.hpp abbc419533ab40312e917931a2fc2ce78b38da41 
>   src/slave/containerizer/docker.cpp fb9188a19a5cd8211d4f36f9647ebb70de560109 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 4d10818105627738e258116647ccada374e3d7b9 
> 
> Diff: https://reviews.apache.org/r/44661/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44421: Added support for "overlay" keyword.

[Jie Yu]

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




src/linux/fs.hpp (lines 160 - 161)


Please use namespace instead.

```
namespace overlay {

Try supported();

} // namespace overlay {
```



src/linux/fs.cpp (line 71)


Please put '{' to a new line.



src/linux/fs.cpp (line 74)


Remove this line? I would do:
```
Try overlay = fs::supported("overlay");
if (overlay.isError()) {
  return Error("...");
} else if (overlay.get() == true) {
  return true;
}

Try overlayfs = fs::supported("overlayfs");
if (overlayfs.isError()) {
  return Error("...");
} else if (overlayfs.get() == true) {
  return true;
}

return false;
```


- Jie Yu


On March 11, 2016, 11:58 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44421/
> ---
> 
> (Updated March 11, 2016, 11:58 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Shuai Lin.
> 
> 
> Bugs: MESOS-4874
> https://issues.apache.org/jira/browse/MESOS-4874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The "overlayfs" was renamed to "overlay" in kernel 4.2, for overlay
> support check function, it should check both "overlay" and "overlayfs"
> in "/proc/filesystems".
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp 4525a5d5566e2bc913894e993ac7350f1bbd9cc0 
>   src/linux/fs.cpp dbf94759b6382bfafd3f3b8b4c2047af36a53ad5 
>   src/slave/containerizer/mesos/provisioner/backend.cpp 
> 55652540e35f9c451ad85cfead575a788aa3eba1 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
> 5cc0f8b5a8cd4c945023f874056a8184113186c5 
>   src/tests/environment.cpp ee1bbe6b4e3dda1e27b63d71a08ef0d1d254741a 
> 
> Diff: https://reviews.apache.org/r/44421/diff/
> 
> 
> Testing
> ---
> 
> root@mesos002:~/src/mesos/m1/mesos/build# uname -r
> 4.2.3-040203-generic
> root@mesos002:~/src/mesos/m1/mesos/build# lsmod  | grep over
> overlay45056  1 
> root@mesos002:~/src/mesos/m1/mesos/build# cat /proc/filesystems | grep over
> nodev overlay
> root@mesos002:~/src/
> 
> make
> make check
> ./bin/mesos-tests.sh 
> --gtest_filter="OverlayBackendTest.ROOT_OVERLAYFS_OverlayFSBackend" --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44766: Enabled Authentication information in endpoint HELP.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On March 17, 2016, 10:32 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44766/
> ---
> 
> (Updated March 17, 2016, 10:32 a.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Bugs: MESOS-4934
> https://issues.apache.org/jira/browse/MESOS-4934
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled endpoint HELP string to display information about authentication 
> requirements.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> 783304e2fc78db70f1a5fccbf5e96fcc76a88fd8 
>   3rdparty/libprocess/src/help.cpp 5f368801affecacb0d1daaeb6ccf5895ccb231d2 
>   3rdparty/libprocess/src/logging.cpp 
> 015a43db77fd7015aeee0c45fa10c292f3e9cf58 
> 
> Diff: https://reviews.apache.org/r/44766/diff/
> 
> 
> Testing
> ---
> 
> Viewed master endpoint help in browser.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 44996: Fixed how we detect C++11 compiler support in libprocess.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On March 17, 2016, 11:52 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44996/
> ---
> 
> (Updated March 17, 2016, 11:52 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4963
> https://issues.apache.org/jira/browse/MESOS-4963
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adopt a new version of the M4 macro from the upstream macro archive.
> We no longer care about whether the compiler supports particular
> C++11 features, so we can discard some previous local modifications
> we had to this macro. This should also make it easier to mandate
> C++14 support in the future.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/configure.ac 134f8ef4395d059e2901bbb48826fbfc4e6e3124 
>   3rdparty/libprocess/m4/ax_cxx_compile_stdcxx.m4 PRE-CREATION 
>   3rdparty/libprocess/m4/ax_cxx_compile_stdcxx_11.m4 
> 251213a7e5e57da4b367f033865594b3678d53aa 
> 
> Diff: https://reviews.apache.org/r/44996/diff/
> 
> 
> Testing
> ---
> 
> make check with GCC6
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 44946: Add utility functions to manipulate XFS project quotas.

[James Peach]

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

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


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


Repository: mesos


Description
---

Add utility functions to manipulate XFS project quotas.


Diffs
-

  src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
  src/linux/xfs.hpp PRE-CREATION 
  src/linux/xfs.cpp PRE-CREATION 

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


Testing
---

Make check. Manual verification. Tests in subsequent patches.


Thanks,

James Peach

Review Request 45013: Removed un-needed methods from command scheduler.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This change cleans up the unused scheduler driver methods
that would not be needed after moving to the scheduler library.

This review mainly eases reviewing later in the chain and can't
be committed separately.


Diffs
-

  src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 

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


Testing
---

This change should not be committed with the review after it.


Thanks,

Anand Mazumdar

Re: Review Request 43935: Allow setting role in mesos-execute.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43935]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 17, 2016, 1:39 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated March 17, 2016, 1:39 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 45002: Added FTS_PHYSICAL option to fts_open for rmdir.

[Neil Conway]

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



Per discussion in https://reviews.apache.org/r/45003/, if we can actually run 
into `FTS_SLNONE`, we should have a test-case for it.


3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp (line 50)


This merits a comment explaining why we're setting `FTS_PHYSICAL`, I think.


- Neil Conway


On March 18, 2016, 12:16 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45002/
> ---
> 
> (Updated March 18, 2016, 12:16 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> According to the documentation for fts_open, either FTS_PHYSICAL or 
> FTS_LOGICAL
> SHOULD be provided. We need FTS_PHYSICAL for the case of rmdir as we dont want
> to resolve the symlink targets while deleting them.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> daa46e05d113fd62ea9dad042ec41aaab28ad003 
> 
> Diff: https://reviews.apache.org/r/45002/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 45078: MESOS-3902: Fix location header in redirect from non-leader.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45078]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 19, 2016, 9:57 p.m., Ashwin Murthy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45078/
> ---
> 
> (Updated March 19, 2016, 9:57 p.m.)
> 
> 
> Review request for mesos, Ben Whitehead, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3902: Fix location header in redirect from non-leader. Adding the path 
> of the original request into the redirect URL.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp bfea8fa811c24d37b2d7a8109728e1f40217e02f 
> 
> Diff: https://reviews.apache.org/r/45078/diff/
> 
> 
> Testing
> ---
> 
> Manual testing to be done. Testing framework does not support multi-master 
> tests as yet.
> 
> 
> Thanks,
> 
> Ashwin Murthy
> 
>

Re: Review Request 45003: Fixed rmdir comment for FTS_SLNONE as per coding guidelines.

[Neil Conway]

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




3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp (line 71)


This comment seems incorrect:

(1) The relevant parameters are `FTS_PHYSICAL` vs. `FTS_LOGICAL`, as well 
as `FTS_COMFOLLOW`.

(2) Since we _do_ set `FTS_PHYSICAL`, isn't it possible for `FTS_SLNONE` to 
be returned?


- Neil Conway


On March 18, 2016, 12:17 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45003/
> ---
> 
> (Updated March 18, 2016, 12:17 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed rmdir comment for FTS_SLNONE as per coding guidelines.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
> 
> Diff: https://reviews.apache.org/r/45003/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44954: Regenerated master endpoint documentation.

[Joerg Schad]

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

(Updated March 17, 2016, 5:33 p.m.)


Review request for mesos, Adam B and Greg Mann.


Changes
---

Retrigger build after removing old dependency.


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


Repository: mesos


Description
---

Reran the generate-endpoint-help.py scipt after adding the AUTHENTICATION 
information to HELP.


Diffs (updated)
-

  docs/endpoints/master/api/v1/scheduler.md 
7a75fd75eef458f26f9e5067b792329f310d5d24 
  docs/endpoints/master/create-volumes.md 
2ab116a16581fc5cfc0fc5cf70c6e001a28d8493 
  docs/endpoints/master/destroy-volumes.md 
88679fe12baf7642b63e71688b90031aaedab1ef 
  docs/endpoints/master/flags.md 739d6bfae5ae35223d6bc7c4782ff0f16b868018 
  docs/endpoints/master/frameworks.md 95c1f0e63f6a61d0e6622a8ec9d821a646656cfc 
  docs/endpoints/master/health.md 7f9352025a6364b812b91e9ccbe380a3a69ba4e1 
  docs/endpoints/master/machine/down.md 
6eec9ecc6e92fd1299b27499f78055c6f4f68e81 
  docs/endpoints/master/machine/up.md 7cbbf04859a218f90672453e5037480676f79fe5 
  docs/endpoints/master/maintenance/schedule.md 
5f5267d24e392d6302d126e2367be08e6cc5e174 
  docs/endpoints/master/maintenance/status.md 
4d0c7551acb89fb375834fd703c406d68f8bdcfc 
  docs/endpoints/master/observe.md fae1ee062350d7b25775bef98a0bebda1892ab62 
  docs/endpoints/master/quota.md 812874d2dd6c3548887e3044ba1f3c3c8c9d1dd6 
  docs/endpoints/master/redirect.md ac9d0fa3eae485726b10a3ac756228d0cb5aeb27 
  docs/endpoints/master/reserve.md 1a4f67961baf761f79a693780f42a1a8ce2244fc 
  docs/endpoints/master/roles.json.md 863715386791ef9192f38bf390fcd2b31d988547 
  docs/endpoints/master/roles.md 171e0163c4c2db34bf34c8303846793f3c29bdf5 
  docs/endpoints/master/slaves.md ec169c15507d3c731b8833f2656949caf0efcc33 
  docs/endpoints/master/state-summary.md 
fb10ac7db5b94ea7982c245f1d884312d818c6b5 
  docs/endpoints/master/state.json.md 0415cfdd63cee0b350b3e33c496bacfc1ba53f8a 
  docs/endpoints/master/state.md ae1ec718b8d718718727dbd2e7ce4739cca41680 
  docs/endpoints/master/tasks.json.md 46a1253d33cdfdca0a7158808d1e29da1a634933 
  docs/endpoints/master/tasks.md 2ada97b5dfdaf5f1f7578fa42eb4e61233e6a753 
  docs/endpoints/master/teardown.md f68d083a4271ed7dd0ddcc87da104a43eb40c7ec 
  docs/endpoints/master/unreserve.md e059d8d888343c5036346f7a615f04375f44f517 
  docs/endpoints/master/weights.md 632be242d85fbd61ded62d846e385009a321533c 

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


Testing
---

viewed docs with docker-website generator.


Thanks,

Joerg Schad

Re: Review Request 44672: Added normalize method to registry puller.

[Guangya Liu]

> On 三月 19, 2016, 6:54 a.m., Shuai Lin wrote:
> > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp, line 
> > 182
> > 
> >
> > I would suggest to add a new test for the `normalize` function.

I think that the test was already covered in the followint patch here 
https://reviews.apache.org/r/44673/


- Guangya


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


On 三月 14, 2016, 5:32 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44672/
> ---
> 
> (Updated 三月 14, 2016, 5:32 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4877
> https://issues.apache.org/jira/browse/MESOS-4877
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added normalize method to registry puller.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6d637ed14f35feb554c8fcc63a7a7e046aaca574 
> 
> Diff: https://reviews.apache.org/r/44672/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh 
> --gtest_filter="*ProvisionerDockerRegistryPullerTest*"
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44892: Remove `Fetcher` in `Containerizer::create`.

[haosdent huang]

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

(Updated March 16, 2016, 5:56 p.m.)


Review request for mesos and Till Toenshoff.


Changes
---

Rebase.


Repository: mesos


Description
---

Remove `Fetcher` in `Containerizer::create`.


Diffs (updated)
-

  src/local/local.cpp f8599e7378e9a0065bbd01ad8f23f11debb30c91 
  src/slave/containerizer/containerizer.hpp 
ff78b4d0fd4a3b862f6019fc757c16b7367cd3cf 
  src/slave/containerizer/containerizer.cpp 
f6fc7863d0c215611f170dc0c89aa229407b5137 
  src/slave/main.cpp 33a1af84aeb079224b15e92caf97bcf081ea4646 
  src/tests/cluster.hpp 06424dd741aed2261a926429bb0fc7dea141c11b 
  src/tests/cluster.cpp 22167da70a855a39fd9c3ca980304372c70bd8d3 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 44678: Modified basic HTTP authenticator creator to accept realm.

[Adam B]

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



Needs more const references, but otherwise in nearly-shippable shape.


include/mesos/authentication/http/basic_authenticator_factory.hpp (line 41)


std::string&



include/mesos/authentication/http/basic_authenticator_factory.hpp (line 45)


std::string&



src/authentication/http/basic_authenticator_factory.cpp (line 44)


string&



src/authentication/http/basic_authenticator_factory.cpp (line 68)


Looks like you're parsing it as a JSON::Value, not a JSON::Array, right?



src/authentication/http/basic_authenticator_factory.cpp (line 75)


Could you elaborate on the error returned from protobuf::parse, to put it 
into scope?
`Error("Unable to parse credentials for basic authenticator: " + 
credentials_.error());`



src/authentication/http/basic_authenticator_factory.cpp (line 83)


s/module //, since this is not necessarily for a module.



src/authentication/http/basic_authenticator_factory.cpp (lines 87 - 91)


Is it ok to specify a realm but no credentials? Does that just mean that 
nobody can authenticate? Is that still a valid authenticator?



src/authentication/http/basic_authenticator_factory.cpp (line 96)


string&



src/examples/test_http_authenticator_module.cpp (line 43)


"Failed to create basic HTTP authenticator: "?



src/tests/http_authentication_tests.cpp (line 88)


s/Auth/Authenticator/



src/tests/http_authentication_tests.cpp (lines 89 - 90)


const Option& (const refs!)



src/tests/http_authentication_tests.cpp (line 95)


Any reason you called it `entry` instead of `parameter`?



src/tests/http_authentication_tests.cpp (line 100)


Why the second check? This is a test helper, and a test may want to test 
with an empty credential set. Why the unnecessary restriction?


- Adam B


On March 17, 2016, 5:20 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44678/
> ---
> 
> (Updated March 17, 2016, 5:20 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4850
> https://issues.apache.org/jira/browse/MESOS-4850
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modified basic HTTP authenticator creator to accept realm.
> 
> To accommodate different authentication realms for the master and agent, the 
> default basic HTTP authenticator needs to accept its authentication realm as 
> a parameter. This patch adds this parameter and modifies the HTTP 
> authentication tests to validate it appropriately. A new test was also added: 
> `HttpAuthenticationTest.BasicWithoutRealm`.
> 
> 
> Diffs
> -
> 
>   include/mesos/authentication/http/basic_authenticator_factory.hpp 
> c11bb47c8e02f2e8645cf387d18eb64d1c8cb604 
>   src/authentication/http/basic_authenticator_factory.cpp 
> 62f851685db3b42c52bbcb7cff3e4f4703004ed7 
>   src/examples/test_http_authenticator_module.cpp 
> 459b7046bd76d3043d2484a2dd30c10d7deaedd4 
>   src/master/master.cpp e6290ea686ccf17813d6faeaf2f2012f79cf3b7f 
>   src/tests/http_authentication_tests.cpp 
> cf2bb762272fa38e04e5c26aef2858300bbd0459 
> 
> Diff: https://reviews.apache.org/r/44678/diff/
> 
> 
> Testing
> ---
> 
> HTTP authentication tests were updated to pass the authentication realm to 
> the basic HTTP authenticator, and to adhere to the new credentials format in 
> the module parameters. A new test was also added: 
> `HttpAuthenticationTest.BasicWithoutRealm`
> 
> `make check` was used to test on both OSX and CentOS 7.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44514: Implemented prepare() method of "network/cni" isolator.

[Avinash sridharan]

> On March 17, 2016, 5:35 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp, line 88
> > 
> >
> > Should this be a `Result` instead of an `Option` ? Since either the 
> > container was able to join the network or wasn't due to an error. 
> > 
> > I am thinking about a case where a container wants to join multiple 
> > networks, it is successful in joining a few but fails due to misconfig or 
> > unavailable IP addresses. In this case should we store an Error?
> 
> Qian Zhang wrote:
> I think `Result` is usually used to define the return value of a method 
> but not a field/member in a struct/class. And I am not sure why we want to 
> store an Error in the isolator. I think if a container wants to join multiple 
> networks and fails to join any one of them, we should just return an Error.

Valid argument. All or none makes sense. We should just log an ERROR during the 
isolate phase if this event happens and return an ERROR.


> On March 17, 2016, 5:35 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 240-244
> > 
> >
> > I think this comment is for static IP addresses. You should be 
> > prepoulating the NetworkResultInfo with the IP Address and use it when 
> > calling the plugin. 
> > 
> > `host-local` is something different right? It just uses a local IPAM to 
> > allocated addresses.
> 
> Qian Zhang wrote:
> In future, when we support framework to request static IP for its 
> container, we will need to introduce a new field in `NetworkResultInfo` and 
> fill it here with the value of `containerInfo.networkInfo.ip_addresses`, and 
> then in `isolate()`, use it as `CNI_ARGS` to call plugin. And we should only 
> do these if the IPAM plugin is `host-local`, since currently `host-local` is 
> the only IPAM plugin which can support to request a static IP.

Firstly, the CNI isolator should plugin agnostic. It shouldn'be aware of which 
plugin to invoke. Also host-local is just a poor man's DHCP server, there is 
nothing special that host-local provides in terms of static address allocation. 

I do agree that the CNI spec currently doesn't allow for the passing of an IP 
address as a command line argument. However, if we do want to support static IP 
addressing we could do the following:

a) If IP address has been set on the NetworkInfo, store it in 
`NetworkResultInfo`
b) During the isolate phase when we get the JSON from the config, modify the 
JSON for IPAM, by setting a /32 subnet. This is effectively requesting a 
specific IP address from the IPAM. Pass the mutated JSON to the plugin. 

Ofcourse we need to try this out. 

My suggestion is to remove this comment (since its not accurate), and then 
revisit static IP addressing once we have tried out the feasibility of the 
above algorithm.


- Avinash


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


On March 17, 2016, 1:17 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44514/
> ---
> 
> (Updated March 17, 2016, 1:17 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented prepare() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44514/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44549: Introduced a protobuf message "NetworkConfig".

[Qian Zhang]

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

(Updated March 17, 2016, 4:56 p.m.)


Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


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


Repository: mesos


Description
---

Introduced a protobuf message "NetworkConfig".


Diffs (updated)
-

  src/CMakeLists.txt 0517dd1ae7125ac1ae85cc83b6daaca66a10b8b4 
  src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
  src/slave/containerizer/mesos/isolators/network/cni/spec.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni/spec.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni/spec.proto PRE-CREATION 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45033]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 19, 2016, 6:26 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated March 19, 2016, 6:26 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h, and .md files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/post-rewrite e3747a320fef0b71c06bcf0f2c5532958c416646 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45057: Made `unzip` overwrite existing files without prompting.

[Tomasz Janiszewski]

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

(Updated March 20, 2016, 12:05 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Made `unzip` overwrite existing files without prompting.


Diffs (updated)
-

  src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
  src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 

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


Testing
---


Thanks,

Tomasz Janiszewski

Re: Review Request 44975: Updated cgroups test cases for cgroups device support.

[Kevin Klues]

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




src/tests/containerizer/cgroups_tests.cpp (lines 1347 - 1349)


This comment doesn't make clear the real reason this is necessary.

The reason is that without first clearing out the `"a *:* rwm"`, no matter 
how many devices you write to `devices.deny`, they will never show up in 
`devices.list` (because it's a whitelist).  The cgroups driver for this is very 
simple and will only add/remove the **exact** entries you write to the 
respective `devices.allow` and `devices.deny` file.  So tpo get **any** devices 
to ever show up in `devices.list`, you first have to clear out the `"a *:* 
rwm"` entry and then start allowing/denying devices explicitly.



src/tests/containerizer/cgroups_tests.cpp (lines 1350 - 1353)


I probably wouldn't embed the `deny()` call inside the `CHECK_SOME()` call. 
 I'd break them out into two separate calls as the rest of the cod ein this 
file does.



src/tests/containerizer/cgroups_tests.cpp (line 1356)


You shouldn't need the `std::` here.



src/tests/containerizer/cgroups_tests.cpp (lines 1357 - 1358)


Why the newline here?



src/tests/containerizer/cgroups_tests.cpp (lines 1360 - 1368)


This pattern doesn't look very mesos-like to me. I'll defer to @bmahler or 
@nnielson for suggestions here.


- Kevin Klues


On March 17, 2016, 7:35 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44975/
> ---
> 
> (Updated March 17, 2016, 7:35 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Felix Abecassis, Kevin Klues, and 
> Niklas Nielsen.
> 
> 
> Bugs: MESOS-3368
> https://issues.apache.org/jira/browse/MESOS-3368
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated cgroups test cases for cgroups device support.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_tests.cpp 
> acaed9b3f8a04964092cef413133834d0cf5a145 
> 
> Diff: https://reviews.apache.org/r/44975/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 44909: Update containerizer construction in docker_containerizer_tests.cpp.

[haosdent huang]

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

(Updated March 16, 2016, 5:57 p.m.)


Review request for mesos and Till Toenshoff.


Changes
---

Rebase.


Repository: mesos


Description
---

Update containerizer construction in docker_containerizer_tests.cpp.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
f6fce7df82417e029fadf805d6e0b793f396aa69 

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


Testing
---


Thanks,

haosdent huang

Review Request 45009: Transition `filesystem_tests.cpp` to use `path::join`.

[Alex Clemmer]

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

Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

Transition `filesystem_tests.cpp` to use `path::join`.

This commit will fix a test that fails due to inconsistent usage of path
delimiting characters.

Historically in Mesos, it has been common to join paths by adding
strings. For example: `somePath + "/yourFile"`. The '/' character in
particular is contentious, and can cause problems if you're not
consistent in your usage. For example, if `somePath` already uses '\' as
a path separator, then joining the string `"/yourFile"` introduces
inconsistency in the path delimiter usage, resulting in undesirable
behavior.

Review: https://reviews.apache.org/r/45009


Diffs
-

  3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
4c30189bb8261ccfc699da0f31b8b1fd3e9b3c83 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 44421: Added support for "overlay" keyword.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On March 18, 2016, 9:22 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44421/
> ---
> 
> (Updated March 18, 2016, 9:22 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Shuai Lin.
> 
> 
> Bugs: MESOS-4874
> https://issues.apache.org/jira/browse/MESOS-4874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The "overlayfs" was renamed to "overlay" in kernel 4.2, for overlay
> support check function, it should check both "overlay" and "overlayfs"
> in "/proc/filesystems".
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp 4525a5d5566e2bc913894e993ac7350f1bbd9cc0 
>   src/linux/fs.cpp dbf94759b6382bfafd3f3b8b4c2047af36a53ad5 
>   src/slave/containerizer/mesos/provisioner/backend.cpp 
> 55652540e35f9c451ad85cfead575a788aa3eba1 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
> 5cc0f8b5a8cd4c945023f874056a8184113186c5 
>   src/tests/environment.cpp ee1bbe6b4e3dda1e27b63d71a08ef0d1d254741a 
> 
> Diff: https://reviews.apache.org/r/44421/diff/
> 
> 
> Testing
> ---
> 
> root@mesos002:~/src/mesos/m1/mesos/build# uname -r
> 4.2.3-040203-generic
> root@mesos002:~/src/mesos/m1/mesos/build# lsmod  | grep over
> overlay45056  1 
> root@mesos002:~/src/mesos/m1/mesos/build# cat /proc/filesystems | grep over
> nodev overlay
> root@mesos002:~/src/
> 
> make
> make check
> ./bin/mesos-tests.sh 
> --gtest_filter="OverlayBackendTest.ROOT_OVERLAYFS_OverlayFSBackend" --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44974: Added device support in cgroups abstraction.

[Kevin Klues]

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




src/linux/cgroups.hpp (line 629)


I talked with Ben (who actually has commit rights), and he'd prefer if this 
was just called `ListEntry`. So it becomes `cgroups::devices::ListEntry` when 
it actually gets used.



src/linux/cgroups.hpp (lines 633 - 655)


I talked to Ben (who actually has commit rights), and he wants us to just 
smash all of these structs down into a collection of fields with no 
constructors.  He also suggested using an Option for the major/minor 
numbers i.e.:

```
  struct Selector
  {
enum Type
{
  ALL,
  BLOCK,
  CHARACTER
};

Type type;
Option major;
Option minor;
  };
```

This way we can attempt use the `Option<>` type to determine if the value 
is an integer or not. If `major/minor.isNone()`, we assume `major/minor == 
"*"`. If `major/minor.isSome()`, we use its `dev_t` value via 
`major/minor.get()`.

The `dev_t` type is defined in `linux/types.h`.



src/linux/cgroups.hpp (lines 659 - 668)


Likewise, Ben would like to remove all constructors here.



src/linux/cgroups.hpp (lines 673 - 681)


And no constructors here either.



src/linux/cgroups.cpp (lines 2427 - 2438)


According to the style, `case` entries should be indented by 2 spaces.



src/linux/cgroups.cpp (lines 2499 - 2562)


These will go away.



src/linux/cgroups.cpp (lines 2663 - 2697)


These will go away.


- Kevin Klues


On March 18, 2016, 9:41 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44974/
> ---
> 
> (Updated March 18, 2016, 9:41 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Felix Abecassis, Kevin Klues, and 
> Niklas Nielsen.
> 
> 
> Bugs: MESOS-3368
> https://issues.apache.org/jira/browse/MESOS-3368
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There are some helper methods added to support device cgroups in cgroups 
> abstraction to aid isolators controlling access to devices.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 51ccefd1fd2d0989b9bd31342d3d1e3701f88ed2 
>   src/linux/cgroups.cpp df18ed46a2a96871f67c7eb4233c3b4c27b7aa1c 
> 
> Diff: https://reviews.apache.org/r/44974/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 44653: Cleaned up formatting in command executor.

[Alexander Rukletsov]

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

(Updated March 17, 2016, 11:28 p.m.)


Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

The general pattern for formatting the code is to
reduce jaggedness and increase readability.


Diffs (updated)
-

  src/launcher/executor.cpp 4149f084b0b234fb995df04e9d1ca704feec082e 

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


Testing
---

The complete chain was tested. See https://reviews.apache.org/r/44662/.


Thanks,

Alexander Rukletsov

Re: Review Request 44853: Added benchmark test for the allocator metrics endpoint.

[Benjamin Bannier]

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

(Updated March 17, 2016, 3:49 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
---

Used stringify instead of string::to_string.


Repository: mesos


Description
---

Added benchmark test for the allocator metrics endpoint.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
459e02576f6d05abbbcc83ae5cabac5c66e93f05 

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


Testing
---

The benchmark uses the same parametrized setup as other 
`HierarchicalAllocator_BENCHMARK_Tests` which already elsewhere take 
considerable time. The reason for covering the same parameter space here was 
the assumption that that parameter space does capture the relevant scenarios.

The benchmark shows that the time needed to obtain the metrics has a linear 
relationship with the number of registered frameworks, roughly independent of 
the number of slaves. With my setup, the time per framework was well below 1 ms 
after already a few frameworks.


Thanks,

Benjamin Bannier

Re: Review Request 44989: Fixed a race in the resource offers tests.

[Adam B]

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



"The test is slowed considerably" - Can you provide some stats on how fast it 
is before/after the patch?


src/tests/resource_offers_tests.cpp (line 63)


Why pause so soon? You can wait until after the master is started, but just 
before you start calling StartSlave() in the loop



src/tests/resource_offers_tests.cpp (line 94)


Not yours, but ".Times(1)" is redundant as it is the default behavior. 
Please remove.


- Adam B


On March 17, 2016, 6 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44989/
> ---
> 
> (Updated March 17, 2016, 6 p.m.)
> 
> 
> Review request for mesos, Adam B and Joerg Schad.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a race in the resource offers tests.
> 
> Adding HTTP credentials to `StartSlave` in 'src/tests/mesos.cpp' has exposed 
> a race condition in ResourceOffersTest.ResourceOfferWithMultipleSlaves. The 
> test quickly runs `StartSlave` 10 times to create 10 agents. Under the 
> covers, `StartSlave` writes data to disk, and it seems that with the 
> additional data being written to disk for HTTP credentials, the filesystem 
> operations for one `StartSlave` call were not completing before the next call.
> 
> By settling the clock in between each invocation of `StartSlave`, this patch 
> fixes the race. The test is slowed considerably, but it is now reliable.
> 
> 
> Diffs
> -
> 
>   src/tests/resource_offers_tests.cpp 
> 1cf292ee7931207596f8f06677386bef5965ef15 
> 
> Diff: https://reviews.apache.org/r/44989/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="ResourceOffersTest.ResourceOfferWithMultipleSlaves" 
> bin/mesos-tests.sh --gtest_repeat=1000 --gtest_break_on_failure=1` was used 
> to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44883: Fix the mis-leading URI fetcher error message (MESOS-4954).

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 17, 2016, 1:52 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44883/
> ---
> 
> (Updated March 17, 2016, 1:52 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Jie Yu.
> 
> 
> Bugs: MESOS-4954
> https://issues.apache.org/jira/browse/MESOS-4954
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix changes the URL fetcher error message from LOG(ERROR)
> to LOG(INFO) as the plugin is actually skipped if it is not
> created.
> 
> 
> Diffs
> -
> 
>   src/uri/fetcher.cpp 8645b66f6c64c76b6c02ef0b9827a7d694d5ba97 
> 
> Diff: https://reviews.apache.org/r/44883/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 43884: Added allocator metrics for used quotas.

[Ben Mahler]

> On March 17, 2016, 9:07 p.m., Ben Mahler wrote:
> > docs/monitoring.md, lines 876-883
> > 
> >
> > Thanks! How about:
> > 
> > ```
> > 
> >   
> >   allocator/mesos/quota//_allocated
> >   
> >   Amount of resources considered allocated towards a role's quota 
> > guarantee
> >   Gauge
> > 
> > ```
> > 
> > Also, can you add the guarantee to this patch?
> > 
> > ```
> > 
> >   
> >   allocator/mesos/quota//_guarantee
> >   
> >   Amount of resources guaranteed for a role via quota
> >   Gauge
> > 
> > ```
> > 
> > Then we can easily monitor and alert on the distance between the two, 
> > and you can imagine future additions to enable better visibility into the 
> > system, for example:
> > 
> > ```
> > allocator/mesos/quota/role/cpus_guarantee
> > allocator/mesos/quota/role/cpus_allocated
> > allocator/mesos/quota/role/cpus_filtered
> > allocator/mesos/quota/role/cpus_unsatisfiable
> > ```
> 
> Ben Mahler wrote:
> Sorry one more thought here. After chatting with vinod about our metric 
> naming mistakes we made in the past (e.g. master metrics), we're thinking the 
> following would be better here:
> 
> Per-role metrics:
> `"allocator/mesos/quota/roles//resources//guarantee"`
> 
> Aggregated across-role metrics:
> `"allocator/mesos/quota/resources//guarantee"`
> 
> For dynamically generated metrics, we now prefer using an explicit 
> delimiter (like `"roles/"`) to delimit metric names when there is a potential 
> for additional metrics to be added or aggregated versions are needed. Let's 
> say we want to add a quota metric that isn't tied to resources, we can easily 
> do so. For example, if we added priorities we could easily add 
> `"allocator/mesos/quota/priority/1"` without the potential of `"priority"` 
> colliding with a role (thanks to the `"roles/"` and `"resources"` delimiter. 
> For the aggregated case, clearly we'll have a conflict with my original 
> suggestion above if one were to set quota for a role called `"cpus"`.
> 
> Thoughts?

And another thought :)

I'd like to avoid using the word "allocated" here to indicate both offered and 
allocated, because in the future (when the allocator understands offers), we 
will want to distinguish the two. After chatting with Vinod about this one, 
we're thinking the following is the best way to deal with this for now:

`"allocator/mesos/quota/roles//resources//offered_or_allocated"`

Later, we'll remove this in favor of having two seperate metrics:

`"allocator/mesos/quota/roles//resources//allocated"`
`"allocator/mesos/quota/roles//resources//offered"`

If we added `"allocated"` now, splitting the metrics would be a backwards 
incompatible change because we would be changing the meaning of `"allocated"`.


- Ben


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


On March 15, 2016, 2:51 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43884/
> ---
> 
> (Updated March 15, 2016, 2:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4723
> https://issues.apache.org/jira/browse/MESOS-4723
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metrics for used quotas.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
>   src/master/allocator/mesos/hierarchical.hpp 
> 52b3a9bfbe73d24968f7ddb0672aee1e05636ab0 
>   src/master/allocator/mesos/hierarchical.cpp 
> 70291075c00a9a557529c2562dedcfc6c6c3ec32 
>   src/master/allocator/mesos/metrics.hpp 
> d04e9a11c77b6fb2d522608e3085f81f8a6657ca 
>   src/master/allocator/mesos/metrics.cpp 
> 46dd7f7b3d8d49c59b42e789fe5dcd49f1693ace 
>   src/tests/hierarchical_allocator_tests.cpp 
> 459e02576f6d05abbbcc83ae5cabac5c66e93f05 
> 
> Diff: https://reviews.apache.org/r/43884/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 44883: Fix the mis-leading URI fetcher error message (MESOS-4954).

[Yong Tang]

> On March 16, 2016, 7:57 p.m., Anand Mazumdar wrote:
> > LGTM. Thanks for putting out a patch for this.
> > 
> > Can you also add Jie as a reviewer and as a shepherd to the JIRA?

Thanks a lot Anand! Just updated the review request and also added Jie as the 
reviewer. Let me know if there is anything else I need to do.


- Yong


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


On March 17, 2016, 1:52 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44883/
> ---
> 
> (Updated March 17, 2016, 1:52 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Jie Yu.
> 
> 
> Bugs: MESOS-4954
> https://issues.apache.org/jira/browse/MESOS-4954
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix changes the URL fetcher error message from LOG(ERROR)
> to LOG(INFO) as the plugin is actually skipped if it is not
> created.
> 
> 
> Diffs
> -
> 
>   src/uri/fetcher.cpp 8645b66f6c64c76b6c02ef0b9827a7d694d5ba97 
> 
> Diff: https://reviews.apache.org/r/44883/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Review Request 45039: Updated the scheduler `launchTasks()` comment.

[Alexander Rukletsov]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

See summary,


Diffs
-

  include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 37168: Add an example framework using dynamic reservation.

[Klaus Ma]

> On March 17, 2016, 2:27 a.m., Greg Mann wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 119
> > 
> >
> > Could you clarify for me how the slave gets into the RESERVED state the 
> > first time? It seems to me we should have something here similar to what's 
> > in the switch for `State::UNRESERVING`, i.e., check the offer and see if 
> > the reserved resources are contained in it? The framework test passes for 
> > me though, so it seems to be working nonetheless?
> 
> Klaus Ma wrote:
> Good catch! I think I missed `updateState(dispatched, offer.slave_id(), 
> State::RESERVED);` just before launching task; it works because both 
> `State::RESERVED` and `State::RESERVING` are using the same logic.
> 
> When slave transfer from `State::RESERVING` to `State::RESERVED`, it's 
> better to launch tasks directly; otherwise, we have to wait for another 
> allocation interval to launch task. I used to try following code to seperate 
> action in those two state, but `case State::RESERVING:` without `break;` is 
> not a good style :).
> 
> ```
> case State::RESERVING:
>   {
> Resources resources = offer.resources();
> Resources reserved = resources.reserved(role);
> 
> if (reserved.contains(taskResources)) {
>   updateState(dispatched, offer.slave_id(), State::RESERVED);
> }
>   }
>   // NOTE: No `break;`. Launch task after transfering to
>   // `State::RESERVED`.
> case State::RESERVED:
>   {
> if (tasksLaunched == totalTasks) {
>   // If all tasks were launched, unreserve those resources.
>   Try reserved = unreserveResources(driver, offer);
>   updateState(reserved, offer.slave_id(), State::UNRESERVING);
> } else if (tasksLaunched < totalTasks) {
>   // Framework dispatches task on the reserved resources.
>   Try dispatched = dispatchTasks(driver, offer);
>   updateState(dispatched, offer.slave_id(), 
> State::TASK_RUNNING);
> }
>   }
>   break;
> ```
> 
> Greg Mann wrote:
> Ah yea, this makes sense :-) Thanks for the explanation! Do you think you 
> could add a comment to the code here to explain this reasoning?

Agree; updated the patches for that :).


- Klaus


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


On March 18, 2016, 2:47 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 18, 2016, 2:47 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 44945: Add autoconf tests for XFS project quotas.

[Jiang Yan Xu]

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




configure.ac (line 923)


Sorry this should have been a continued discussion on 
https://reviews.apache.org/r/44342/#comment184051 but anyways:

Your response to my initial comments:
> There's no AC_ARG_WITH. If the dependencies area available we will build 
the isolator and the operator can configure it at runtime. I don't think 
there's any need to disable this at build time since it is not enabled by 
default anyway.

I am still favoring building XFS isolator based on `--with-xfs-isolator` 
(AC_ARG_ENABLE is probably better than AC_ARG_WITH) for the following reasons:

1. Explicitness: In general, if we build things solely based on the 
availability of headers, users can inadvertently build more things into the 
deployed binary which could result in larger binaries and perhaps other side 
effects (through bugs, etc.).
2. Preventing user mistake: I am thinking about how we would advise users 
on enabling this feature:
  1. "The XFS isolator feature is disabled by default, to enable it in 
build, install these packages whose name could be different for different 
distros and if you haven't made mistakes in finding them, the feature will be 
built." vs. 
  2. "The XFS isolator feature is disabled by default, to enable it in 
build, install these packages (which could be named differently depending on 
your distro), if the dependencies are not satisfied, the build aborts with an 
message stating such error." The latter feels better to me.
3. Consistency: Other optional features of Mesos follow this pattern.


- Jiang Yan Xu


On March 17, 2016, 8:45 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44945/
> ---
> 
> (Updated March 17, 2016, 8:45 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
> https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add autoconf tests for XFS project quotas.
> 
> 
> Diffs
> -
> 
>   configure.ac 8e4f03593df4a8ba13f00292963e351acc3f71c1 
> 
> Diff: https://reviews.apache.org/r/44945/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual verification.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44543: Removed unnecessary MasterContender and MasterDetector definitions.

[Anurag Singh]

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

(Updated March 18, 2016, 12:29 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

MasterContender and MasterDetector are now defined in
include/mesos/master/contender.hpp and detector.hpp.


Diffs (updated)
-

  src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
  src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 

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


Testing
---

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


Thanks,

Anurag Singh

Re: Review Request 44748: Stout: Added implementation of `read` that works on Windows.

[Alex Clemmer]

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

(Updated March 17, 2016, 7:34 p.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


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


Repository: mesos


Description
---

Stout: Added implementation of `read` that works on Windows.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
7bd4bfbc2ec5922879dcefddc12137336b11be52 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/read.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp 
d494cbf8a2a24c88b0569634cfcbf29de0784797 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/read.hpp 
PRE-CREATION 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 45000: MESOS-3902: Fix in location header during redirect from non-leader.

[Ashwin Murthy]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

MESOS-3902: Fix in location header during redirect from non-leader. The fix is 
to add the scheme and path to the URL in the location header. Previously, we 
did not have the scheme and path. We need to update the mesos http API 
documentation section on master redirect to reflect this change.


Diffs
-

  src/master/http.cpp b47ab7cc86c0a56a81815a98bd63f37a1175ba7f 

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


Testing
---

Was trying to write unit test in scheduler http api test but it turns out multi 
master tests cannot be written at this point. Need to manually verify this by 
setting up multiple masters with ZK.


Thanks,

Ashwin Murthy

Re: Review Request 44767: Added authentication information to master endpoints.

[Adam B]

> On March 14, 2016, 3:51 p.m., Greg Mann wrote:
> > docs/endpoints/master/create-volumes.md, line 24
> > 
> >
> > I wonder if the shorthand "iff" might be a bit too esoteric for these 
> > help strings?
> 
> Joerg Schad wrote:
> We actually use it at several places in HELP master endpoint strings.
> E.g., '''Returns 200 OK iff the Master is healthy.'''.
> No strong opinion happy to change (not now this has moved to the earlier 
> review).

`iff` is good enough for me.


- Adam


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


On March 17, 2016, 4:57 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44767/
> ---
> 
> (Updated March 17, 2016, 4:57 a.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Bugs: MESOS-4934
> https://issues.apache.org/jira/browse/MESOS-4934
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master endpoints now display information whether authentication (if enabled) 
> is required in HELP.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b47ab7cc86c0a56a81815a98bd63f37a1175ba7f 
> 
> Diff: https://reviews.apache.org/r/44767/diff/
> 
> 
> Testing
> ---
> 
> Viewed master endpoint help in browser.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 44992: Reordered function declarations in `TestContainerizer`.

[Ben Mahler]

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


Fix it, then Ship it!





src/tests/containerizer.hpp (line 74)


If you're moving this line, can you remove the >> space here to avoid more 
churn?



src/tests/containerizer.hpp (line 91)


Seems this shouldn't be documented here, unless this isn't overriding the 
interface? Looks like this is just an implementation of the interface function, 
where it is already documented.


- Ben Mahler


On March 17, 2016, 11:47 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44992/
> ---
> 
> (Updated March 17, 2016, 11:47 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The common pattern is to follow the order in the parent class.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer.hpp 67fbe7fedbe170c3f22a2dcbb5aebf4195a5aabc 
> 
> Diff: https://reviews.apache.org/r/44992/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 44911: Fix the issue related to --disable-optimize (MESOS-4621).

[Yong Tang]

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

Review request for mesos and Till Toenshoff.


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


Repository: mesos


Description
---

This fix tries to address MESOS-4621 where --disable-optimize is
not properly handled by configure.ac.


Diffs
-

  configure.ac 8e4f03593df4a8ba13f00292963e351acc3f71c1 

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


Testing
---

make check

Also tested with configure --disable-optimize manually.


Thanks,

Yong Tang

Re: Review Request 44922: Fixed the port mapping tests compile errors.

[Joseph Wu]

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


Ship it!




Thanks!  Didn't realize this file was not being built without 
`--with-network-isolator`.

- Joseph Wu


On March 16, 2016, 11:05 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44922/
> ---
> 
> (Updated March 16, 2016, 11:05 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the port mapping tests compile errors.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/port_mapping_tests.cpp 
> d89780f0c62e27f18b08fbd99bc25ac62d0f5f3c 
> 
> Diff: https://reviews.apache.org/r/44922/diff/
> 
> 
> Testing
> ---
> 
> make check on fedora 23
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 44946: Add utility functions to manipulate XFS project quotas.

[James Peach]

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

(Updated March 17, 2016, 10:43 p.m.)


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


Changes
---

Rebased.


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


Repository: mesos


Description
---

Add utility functions to manipulate XFS project quotas.


Diffs (updated)
-

  src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
  src/linux/xfs.hpp PRE-CREATION 
  src/linux/xfs.cpp PRE-CREATION 

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


Testing
---

Make check. Manual verification. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44766: Enabled Authentication information in endpoint HELP.

[Joerg Schad]

> On March 14, 2016, 8:28 a.m., Adam B wrote:
> > 3rdparty/libprocess/src/help.cpp, line 43
> > 
> >
> > What if this was a bool instead of a string? How many values do you 
> > expect?
> 
> Greg Mann wrote:
> I agree that we should be able to auto-generate the text associated with 
> authentication being enabled or disabled. The one thing we *might* want to 
> pass through is the authentication realm that the endpoint is associated 
> with? This could become relevant in the future if we allow enabling/disabling 
> authentication on a per-endpoint basis. But since we don't do that currently, 
> perhaps a boolean would be sufficient?

We could later add more information, for now I agree a bool is sufficient.


- Joerg


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


On March 17, 2016, 11:55 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44766/
> ---
> 
> (Updated March 17, 2016, 11:55 a.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Bugs: MESOS-4934
> https://issues.apache.org/jira/browse/MESOS-4934
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled endpoint HELP string to display information about authentication 
> requirements.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> 783304e2fc78db70f1a5fccbf5e96fcc76a88fd8 
>   3rdparty/libprocess/src/help.cpp 5f368801affecacb0d1daaeb6ccf5895ccb231d2 
>   3rdparty/libprocess/src/logging.cpp 
> 015a43db77fd7015aeee0c45fa10c292f3e9cf58 
> 
> Diff: https://reviews.apache.org/r/44766/diff/
> 
> 
> Testing
> ---
> 
> Viewed master endpoint help in browser.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 44992: Reordered function declarations in `TestContainerizer`.

[Alexander Rukletsov]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

The common pattern is to follow the order in the parent class.


Diffs
-

  src/tests/containerizer.hpp 67fbe7fedbe170c3f22a2dcbb5aebf4195a5aabc 

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


Testing
---

The complete chain was tested. See https://reviews.apache.org/r/44662/.


Thanks,

Alexander Rukletsov

Re: Review Request 45073: Restructured authentication.md to group common flags.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44678, 44703, 44515, 44523, 44989, 44553, 44554, 45073]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 19, 2016, 11:51 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45073/
> ---
> 
> (Updated March 19, 2016, 11:51 a.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Restructured authentication.md to group common flags.
> 
> 
> Diffs
> -
> 
>   docs/authentication.md e7c0bf3ed331411f607e7622419f14853006a480 
> 
> Diff: https://reviews.apache.org/r/45073/diff/
> 
> 
> Testing
> ---
> 
> viewed via gist: https://gist.github.com/joerg84/63e5919fef724b5c3508 and 
> docker website container
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 44545: Separated standalone and zookeeper classes.

[Anurag Singh]

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

(Updated March 18, 2016, 12:29 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Instead of keeping standalone and zookeper contender/detector class
definitions and implementations in the same file, separated them. Also
made the necessary changes in users of class headers to point to the
new locations.


Diffs (updated)
-

  src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
  src/local/local.cpp f8599e7378e9a0065bbd01ad8f23f11debb30c91 
  src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
  src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
  src/master/contenders/contender.hpp PRE-CREATION 
  src/master/contenders/contender.cpp PRE-CREATION 
  src/master/contenders/standalone.cpp PRE-CREATION 
  src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
  src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
  src/master/detectors/detector.hpp PRE-CREATION 
  src/master/detectors/detector.cpp PRE-CREATION 
  src/master/detectors/standalone.cpp PRE-CREATION 
  src/master/main.cpp 61210d9f275d4073967c3468179307cf09e88551 
  src/master/master.hpp 124d43931a5c8a00ee0aaa604feb1761795209f2 
  src/slave/main.cpp 33a1af84aeb079224b15e92caf97bcf081ea4646 
  src/tests/authentication_tests.cpp 8143cd7a22bbdbcd0fc613cb44eae8b55fd458e7 
  src/tests/cluster.cpp 22167da70a855a39fd9c3ca980304372c70bd8d3 
  src/tests/containerizer/external_containerizer_test.cpp 
5e2116355418f5a0716cfd1573bab48ba75df596 
  src/tests/containerizer/isolator_tests.cpp 
6a2e25b967742c034364d19372f06aa9f9cdf828 
  src/tests/fault_tolerance_tests.cpp f99413f56e96a796d3d45decad1f049e6a238789 
  src/tests/gc_tests.cpp 42059b2d6544f360cdc9230fe6ed33a11a15bc50 
  src/tests/master_allocator_tests.cpp b41ba2bda4d680f6fc42f525719973d56c11fe31 
  src/tests/master_authorization_tests.cpp 
8b9b8991fbb8c5a5beb69416a9c4a4ef3525942d 
  src/tests/master_contender_detector_tests.cpp 
bbce379e5a0a0ca608579d0ab2b10970e9cd5ef1 
  src/tests/master_slave_reconciliation_tests.cpp 
988f1d46580ab5a707fe801824e24f94d4f50da7 
  src/tests/master_tests.cpp d34ba0bdd71efd261850d8c205c16cecb701ac7c 
  src/tests/mesos.hpp 93b9340d94d91663283fe5df5ad9febe69ffd2a3 
  src/tests/oversubscription_tests.cpp ba036810758d99a6fb0034c5e2bc7829e2343a44 
  src/tests/partition_tests.cpp 349adbf67686e6044a2e6a4b673043ad74fce44e 
  src/tests/persistent_volume_tests.cpp 
26fff19daa8b175fdcc06fd9467224d5920a1967 
  src/tests/reconciliation_tests.cpp 5f541f5fe004ede943a1b022daab92f01d1f4853 
  src/tests/reservation_tests.cpp a9261bdf48c0af933e7fc303b7af356a60b49506 
  src/tests/scheduler_event_call_tests.cpp 
00e99777ba0294c9c12ac86594987afbd9388b51 
  src/tests/scheduler_http_api_tests.cpp 
b65790a9aad0ca68c6a93dd1d872442b906598fd 
  src/tests/scheduler_tests.cpp 917058f4dcf32ddaaeda8a3ff21898571f4829dd 
  src/tests/slave_recovery_tests.cpp 0d59a06f8e32f3d88f6c3a222bc6756a889a142e 
  src/tests/slave_tests.cpp ea1d776077bf638885db8421194aa4427c772169 

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


Testing
---

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


Thanks,

Anurag Singh

Re: Review Request 44435: Fixed a bug that causes the task stuck in staging state.

[haosdent huang]

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


Fix it, then Ship it!




Ship It!


src/tests/containerizer/mesos_containerizer_tests.cpp (line 798)


Should we `AWAIT_FAILED(launch)` return by `.launch` here? Because 
`provisioner->provision` return `Failure`.


- haosdent huang


On March 6, 2016, 9:23 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44435/
> ---
> 
> (Updated March 6, 2016, 9:23 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4878
> https://issues.apache.org/jira/browse/MESOS-4878
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a bug that causes the task stuck in staging state.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> af3ff5750649497d8852b4761c78d4cae5455a02 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 15f0f93d2e5c19a22f6cc4a71a7d94be4aaec2c1 
> 
> Diff: https://reviews.apache.org/r/44435/diff/
> 
> 
> Testing
> ---
> 
> - Added a new test `"MesosContainerizerProvisionerTest.ProvisionFailed"`
> - make check
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 44946: Add utility functions to manipulate XFS project quotas.

[Jie Yu]

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




src/linux/xfs.cpp (line 17)


Move this after below stout headers.



src/linux/xfs.cpp (line 49)


Kill one line here.



src/linux/xfs.cpp (line 53)


Instead of relying on parameter, can we use os::stat::isdir here?



src/linux/xfs.cpp (line 79)


Let's try to be consistent here. Either use "xfsctl" or "::xfsctl", but 
consistently.



src/linux/xfs.cpp (line 118)


isError



src/linux/xfs.cpp (line 149)


A nits: can you move all helpers above public functions.



src/linux/xfs.cpp (line 185)


2 lines apart.



src/linux/xfs.cpp (line 187)


This interface looks weird. I understand that project quota is set on the 
device level. Can we make it explicit? and expose another public method to get 
device by path?



src/linux/xfs.cpp (line 191)


Can you return Error instead of CHECK here?



src/linux/xfs.cpp (line 195)


is block size always 512 bytes in xfs? If not, worth adding a TODO.



src/linux/xfs.cpp (line 196)


indentation.


- Jie Yu


On March 18, 2016, 3:46 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44946/
> ---
> 
> (Updated March 18, 2016, 3:46 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
> https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add utility functions to manipulate XFS project quotas.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/linux/xfs.hpp PRE-CREATION 
>   src/linux/xfs.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44946/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual verification. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

[Michael Browning]

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

(Updated March 19, 2016, 10:03 p.m.)


Review request for mesos, Vinod Kone and Zhitao Li.


Changes
---

Addressed style comments.


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


Repository: mesos


Description
---

Created URI.filename to name fetched files in sandbox where appropriate.


Diffs (updated)
-

  docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
  include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
  include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
  src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
  src/slave/containerizer/fetcher.cpp 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
  src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
  src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 

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


Testing
---

There are two paths by which a file gets fetched to the executor sandbox: the 
without-cache path, where the fetcher downloads the file directly from the 
specified URI, and the with-cache path, where it copies it from the cache. In 
both cases, we verify that the file is saved to the sandbox directory with the 
name specified by the "filename" field in the CommandInfo.URI proto.


Thanks,

Michael Browning

Re: Review Request 45046: Created URI.filename to name fetched files in sandbox where appropriate.

[Michael Browning]

> On March 19, 2016, 10:07 a.m., Guangya Liu wrote:
> > src/tests/fetcher_cache_tests.cpp, lines 690-691
> > 
> >
> > blank line here

I'd like to push back against this one -- I think adding a line break here (I 
assume because of the comment block?) would suggest that the previous line is 
distinct in some way from the rest of the paragraph, and that's not really the 
case. If anything, I would expect it between the assignments and the following 
assertions, but I don't have a strong preference between break/no break in that 
case.


- Michael


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


On March 18, 2016, 11:30 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45046/
> ---
> 
> (Updated March 18, 2016, 11:30 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Zhitao Li.
> 
> 
> Bugs: MESOS-4735
> https://issues.apache.org/jira/browse/MESOS-4735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created URI.filename to name fetched files in sandbox where appropriate.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md f70939d8410516c9387a8cba86b5b75539a5fe9a 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/slave/containerizer/fetcher.cpp 
> 33dfcade6beb53a5a6dbc41a8f3380f5cb30a161 
>   src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45046/diff/
> 
> 
> Testing
> ---
> 
> There are two paths by which a file gets fetched to the executor sandbox: the 
> without-cache path, where the fetcher downloads the file directly from the 
> specified URI, and the with-cache path, where it copies it from the cache. In 
> both cases, we verify that the file is saved to the sandbox directory with 
> the name specified by the "filename" field in the CommandInfo.URI proto.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Review Request 45078: MESOS-3902: Fix location header in redirect from non-leader.

[Ashwin Murthy]

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

Review request for mesos, Ben Whitehead, Joris Van Remoortere, and Vinod Kone.


Repository: mesos


Description
---

MESOS-3902: Fix location header in redirect from non-leader. Adding the path of 
the original request into the redirect URL.


Diffs
-

  src/master/http.cpp bfea8fa811c24d37b2d7a8109728e1f40217e02f 

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


Testing
---

Manual testing to be done. Testing framework does not support multi-master 
tests as yet.


Thanks,

Ashwin Murthy

Review Request 44974: Added device support in cgroups abstraction.

[Abhishek Dasgupta]

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

Review request for mesos, Ben Mahler, Felix Abecassis, Kevin Klues, and Niklas 
Nielsen.


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


Repository: mesos


Description
---

There are some helper methods added to support device cgroups in cgroups 
abstraction to aid isolators controlling access to devices.


Diffs
-

  src/linux/cgroups.hpp 51ccefd1fd2d0989b9bd31342d3d1e3701f88ed2 
  src/linux/cgroups.cpp df18ed46a2a96871f67c7eb4233c3b4c27b7aa1c 

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


Testing
---

make check


Thanks,

Abhishek Dasgupta

Review Request 44909: Update containerizer construction in docker_containerizer_tests.cpp.

[haosdent huang]

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

Review request for mesos and Till Toenshoff.


Repository: mesos


Description
---

Update containerizer construction in docker_containerizer_tests.cpp.


Diffs
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
8afaa4dab3984e9866b7b223e8e2e70ef83a39dc 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 44554: Added agent HTTP authentication to the docs.

[Greg Mann]

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

(Updated March 17, 2016, 7:43 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
Toenshoff.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added agent HTTP authentication to the docs.


Diffs (updated)
-

  docs/authentication.md e7c0bf3ed331411f607e7622419f14853006a480 
  docs/configuration.md d10fa2e7fc7c477de2f0e30e10da6d817ecbf404 
  docs/home.md fd7794f56b4a95268dbb82288d99cab19a89f5cd 

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


Testing
---

Viewed with the Mesos website container: 
https://github.com/mesosphere/mesos-website-container


Thanks,

Greg Mann

Re: Review Request 33174: Fix for docker not configuring CFS quotas correctly

[Jie Yu]

> On March 2, 2016, 2:55 p.m., Steve Niemitz wrote:
> > @jieyu ping?

Sorry about the delay. Just saw this. Will get to it today. Can you ping me 
using email directly next time. The volume of emails from rb is too large so I 
tend to ignore them.


- Jie


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


On Feb. 20, 2016, 6:47 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33174/
> ---
> 
> (Updated Feb. 20, 2016, 6:47 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2617
> https://issues.apache.org/jira/browse/MESOS-2617
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix for docker containerizer not configuring CFS quotas correctly.
> 
> It would be nice to refactor all this isolation code in a way that can be 
> shared between all containerizers, as this is basically just copied from the 
> CgroupsCpushareIsolator, but that's a much bigger undertaking.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 2d2dd4e0df36207c5f3cbb4fe2c50df51c0f3e9e 
> 
> Diff: https://reviews.apache.org/r/33174/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 44934: Updated mesos-execute to add support for Appc.

[Guangya Liu]

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




src/cli/execute.cpp (line 241)


The default value of `containerizer` is `mesos`, do we need to check this?

What about make the logic as this? This will involve less code change and 
the logic may be more clear?

// Do not touch old dockerImage logic.
if (dockerImage.isSome()) {
  old logic here;
}

if (appcImage.isSome() && containerizer == "mesos") {
  ContainerInfo containerInfo;
  containerInfo.set_type(ContainerInfo::MESOS);
  ContainerInfo::MesosInfo mesosInfo;

  Image::Appc appc;

  appc.set_name(appcImage.get());

  // TODO(jojy): Labels are hard coded right now.
  // Consider adding label flags for customization.
  Label arch;
  arch.set_key("arch");
  arch.set_value("amd64");

  Label os;
  os.set_key("os");
  os.set_value("linux");

  Labels labels;
  labels.add_labels()->CopyFrom(os);
  labels.add_labels()->CopyFrom(arch);

  appc.mutable_labels()->CopyFrom(labels);

  Image mesosImage;
  
  mesosImage.set_type(Image::APPC);
  mesosImage.mutable_appc()->CopyFrom(appc);
  
  mesosInfo.mutable_image()->CopyFrom(mesosImage);
  
  containerInfo.mutable_mesos()->CopyFrom(mesosInfo);

  task.mutable_container()->CopyFrom(containerInfo);
}


- Guangya Liu


On 三月 17, 2016, 6:54 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44934/
> ---
> 
> (Updated 三月 17, 2016, 6:54 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated mesos-execute to add support for Appc.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/44934/diff/
> 
> 
> Testing
> ---
> 
> Tested with various Appc images.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44767: Added authentication information to master endpoints.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On March 17, 2016, 4:57 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44767/
> ---
> 
> (Updated March 17, 2016, 4:57 a.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Bugs: MESOS-4934
> https://issues.apache.org/jira/browse/MESOS-4934
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master endpoints now display information whether authentication (if enabled) 
> is required in HELP.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b47ab7cc86c0a56a81815a98bd63f37a1175ba7f 
> 
> Diff: https://reviews.apache.org/r/44767/diff/
> 
> 
> Testing
> ---
> 
> Viewed master endpoint help in browser.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 44622: Introduced a protobuf message "NetworkResult".

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/network/cni/spec.proto (lines 55 - 59)


Let's move this to the top level:
https://github.com/appc/cni/blob/master/pkg/types/types.go#L94

Also, rename it to IPConfig like above.


- Jie Yu


On March 16, 2016, 7:58 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44622/
> ---
> 
> (Updated March 16, 2016, 7:58 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced a protobuf message "NetworkResult".
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.proto PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44622/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44678: Modified basic HTTP authenticator creator to accept realm.

[Greg Mann]

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

(Updated March 17, 2016, 7:39 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
Toenshoff.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Modified basic HTTP authenticator creator to accept realm.

To accommodate different authentication realms for the master and agent, the 
default basic HTTP authenticator needs to accept its authentication realm as a 
parameter. This patch adds this parameter and modifies the HTTP authentication 
tests to validate it appropriately. A new test was also added: 
`HttpAuthenticationTest.BasicWithoutRealm`.


Diffs (updated)
-

  include/mesos/authentication/http/basic_authenticator_factory.hpp 
c11bb47c8e02f2e8645cf387d18eb64d1c8cb604 
  src/authentication/http/basic_authenticator_factory.cpp 
62f851685db3b42c52bbcb7cff3e4f4703004ed7 
  src/examples/test_http_authenticator_module.cpp 
459b7046bd76d3043d2484a2dd30c10d7deaedd4 
  src/master/master.cpp e6290ea686ccf17813d6faeaf2f2012f79cf3b7f 
  src/tests/http_authentication_tests.cpp 
cf2bb762272fa38e04e5c26aef2858300bbd0459 

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


Testing
---

HTTP authentication tests were updated to pass the authentication realm to the 
basic HTTP authenticator, and to adhere to the new credentials format in the 
module parameters. A new test was also added: 
`HttpAuthenticationTest.BasicWithoutRealm`

`make check` was used to test on both OSX and CentOS 7.


Thanks,

Greg Mann

Re: Review Request 44906: Update containerizer construction in fetcher_cache_tests.cpp.

[haosdent huang]

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

(Updated March 16, 2016, 5:57 p.m.)


Review request for mesos and Till Toenshoff.


Changes
---

Rebase.


Repository: mesos


Description
---

Update containerizer construction in fetcher_cache_tests.cpp.


Diffs (updated)
-

  src/tests/fetcher_cache_tests.cpp 645dae208cb2b0aa2d2181d96eb1fd8893975430 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 44945: Add autoconf tests for XFS project quotas.

[Jie Yu]

> On March 18, 2016, 6:36 p.m., Jiang Yan Xu wrote:
> > configure.ac, line 923
> > 
> >
> > Sorry this should have been a continued discussion on 
> > https://reviews.apache.org/r/44342/#comment184051 but anyways:
> > 
> > Your response to my initial comments:
> > > There's no AC_ARG_WITH. If the dependencies area available we will 
> > build the isolator and the operator can configure it at runtime. I don't 
> > think there's any need to disable this at build time since it is not 
> > enabled by default anyway.
> > 
> > I am still favoring building XFS isolator based on 
> > `--with-xfs-isolator` (AC_ARG_ENABLE is probably better than AC_ARG_WITH) 
> > for the following reasons:
> > 
> > 
> > 1. Explicitness: In general, if we build things solely based on the 
> > availability of headers, users can inadvertently build more things into the 
> > deployed binary which could result in larger binaries and perhaps other 
> > side effects (through bugs, etc.).
> > 2. Preventing user mistake: I am thinking about how we would advise 
> > users on enabling this feature:
> >   1. "The XFS isolator feature is disabled by default, to enable it in 
> > build, install these packages whose name could be different for different 
> > distros and if you haven't made mistakes in finding them, the feature will 
> > be built." vs. 
> >   2. "The XFS isolator feature is disabled by default, to enable it in 
> > build, install these packages (which could be named differently depending 
> > on your distro), if the dependencies are not satisfied, the build aborts 
> > with an message stating such error." The latter feels better to me.
> > 3. Consistency: Other optional features of Mesos follow this pattern.
> 
> Jiang Yan Xu wrote:
> Sorry I meant `--enable-xfs-isolator`.

For consistency: I think the network isolator is using 
`--with-network-isolator` (`AC_ARG_WITH([network-isolator],`), so it'e better 
to be consistent and use 'AC_ARG_WITH' here.

I agree with Yan that we should be more explicit, instead of relying on headers.


- Jie


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


On March 18, 2016, 3:45 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44945/
> ---
> 
> (Updated March 18, 2016, 3:45 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
> https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add autoconf tests for XFS project quotas.
> 
> 
> Diffs
> -
> 
>   configure.ac 8e4f03593df4a8ba13f00292963e351acc3f71c1 
> 
> Diff: https://reviews.apache.org/r/44945/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual verification.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 45067: Updated the long-lived-framework example.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45067]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 19, 2016, 1:06 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45067/
> ---
> 
> (Updated March 19, 2016, 1:06 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Artem Harutyunyan, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives the example `long-lived-framework` enough options to run outside 
> of the build environment.
> 
> This also updates the style of the framework code and allows the 
> `ExecutorInfo` to be run with some cgroups isolators.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> 289a0b9dd3d1ce30f20dd9bb381126bff30c 
> 
> Diff: https://reviews.apache.org/r/45067/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Ran this on the master node on a Mesos cluster:
> ```
> ./long-lived-framework --master=zk://localhost:2181/mesos 
> --executor_uri="https://s3.amazonaws.com/url/to/long-lived-executor; 
> --executor_command="LD_LIBRARY_PATH=/path/to/libmesos && 
> ./long-lived-executor"
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 44989: Fixed a race in the resource offers tests.

[Greg Mann]

> On March 17, 2016, 11:16 p.m., Neil Conway wrote:
> > src/tests/resource_offers_tests.cpp, line 63
> > 
> >
> > Style-wise, do we want all tests to resume if they initially pause it? 
> > I think we do a mix of both.

Hmmm good question. I know that I'm responsible for some instances where 
`resume` is *not* called at the end of the test, but now that you mention it, 
it does seem like a good idea. I've added it here. We should do a sweep and 
make this consistent across the tests; I could have a look next week.


- Greg


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


On March 18, 2016, 1 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44989/
> ---
> 
> (Updated March 18, 2016, 1 a.m.)
> 
> 
> Review request for mesos, Adam B and Joerg Schad.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a race in the resource offers tests.
> 
> Adding HTTP credentials to `StartSlave` in 'src/tests/mesos.cpp' has exposed 
> a race condition in ResourceOffersTest.ResourceOfferWithMultipleSlaves. The 
> test quickly runs `StartSlave` 10 times to create 10 agents. Under the 
> covers, `StartSlave` writes data to disk, and it seems that with the 
> additional data being written to disk for HTTP credentials, the filesystem 
> operations for one `StartSlave` call were not completing before the next call.
> 
> By settling the clock in between each invocation of `StartSlave`, this patch 
> fixes the race. The test is slowed considerably, but it is now reliable.
> 
> 
> Diffs
> -
> 
>   src/tests/resource_offers_tests.cpp 
> 1cf292ee7931207596f8f06677386bef5965ef15 
> 
> Diff: https://reviews.apache.org/r/44989/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="ResourceOffersTest.ResourceOfferWithMultipleSlaves" 
> bin/mesos-tests.sh --gtest_repeat=1000 --gtest_break_on_failure=1` was used 
> to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44606: Returning `202` (Accepted) for /reserve and related endpoints.

[Vinod Kone]

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



Looks good to me. Couple of things before this can get committed.

--> Have you sent an email to dev/user list about this backwards incompatible 
change? If not, you should.
 
--> If users are depending on the return code (need to ask on the above email) 
we need to do a proper deprecation warning in 0.29.0 CHANGELOG (Deprecations 
section) and change the behavior in 0.30.0.

--> If no one is depending on the return code, we might just do the change in 
0.29.0. This should go into the 0.29.0 CHANGELOG though (API Changes section).

- Vinod Kone


On March 10, 2016, 7:11 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44606/
> ---
> 
> (Updated March 10, 2016, 7:11 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Neil Conway, Qian Zhang, and Vinod 
> Kone.
> 
> 
> Bugs: mesos-4580
> https://issues.apache.org/jira/browse/mesos-4580
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modify the return code of the following endpoints to 202:
> 1. /reserve
> 2. /unreserve
> 3. /create-volumes
> 4. /destroy-volumes
> 
> [#MESOS-4580]
> 
> Signed-off-by: Guo Jiannan 
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/create-volumes.md 
> 542f555d9b6f07bde58d49ab1bc408b4d0aa7b9f 
>   docs/endpoints/master/destroy-volumes.md 
> d5d98198e70fae4f6ea6791511f3b26e792f66d1 
>   docs/endpoints/master/reserve.md 3e2a857ce784496d4de872bd00b1560c058b667d 
>   docs/endpoints/master/unreserve.md d26ae7cb7ec7a2c75ac45b792213fe8d82e8929d 
>   docs/persistent-volume.md 4b9c59daf6fdcee4a102e19d6eb4df9b5eddfa54 
>   docs/reservation.md 55924adb94028702e15db7e191915157552981d0 
>   docs/upgrades.md e888b233351b2da05a5e5c63138de5f60708afea 
>   src/master/http.cpp a3ad57a1c3f8a01aa609b28c12825670bb243387 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 81185a161498394020a27f1f5bf747bac5425f43 
>   src/tests/reservation_endpoints_tests.cpp 
> f95ae7a32c3809d150adf1e9e515a3b527e61699 
> 
> Diff: https://reviews.apache.org/r/44606/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 44546: Moved functions in promises to a common header file.

[Anurag Singh]

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

(Updated March 18, 2016, 12:29 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Moved functions in promises to a common header file.


Diffs (updated)
-

  src/master/detectors/standalone.cpp PRE-CREATION 
  src/master/detectors/zookeeper.cpp 9274435802d6292b183be48f42b43999476e016e 

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


Testing
---

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


Thanks,

Anurag Singh

Re: Review Request 45057: Made `unzip` overwrite existing files without prompting.

[Jie Yu]

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


Fix it, then Ship it!




Thanks for the patch! This looks great! I made some comments on the first test, 
but the same comments should apply to other tests as well. Once the path is 
updated, I'll commit it. Thanks!


src/tests/fetcher_tests.cpp (line 642)


2 lines apart



src/tests/fetcher_tests.cpp (lines 647 - 649)


Can you create a temp file under os::getcwd()? The TemporaryDirectoryTest 
fixture will cleanup the current working directory after the test 
finishes/failed.

If the test fails, the existing code will leak the tmp zip file.



src/tests/fetcher_tests.cpp (line 648)


Kill this line.



src/tests/fetcher_tests.cpp (lines 657 - 660)


Indentation here should be 4:

```
ASSERT_SOME(...
"UES."
""
"...").get()));
```



src/tests/fetcher_tests.cpp (line 661)


Insert a new line above. We typically insert a new line after a multi-line 
statement.



src/tests/fetcher_tests.cpp (line 682)


s/extract/extractedFile/

Please use path::join(os::getcwd(), "hello");



src/tests/fetcher_tests.cpp (line 687)


This is not necessary if you put the zip file under os::getcwd() as I 
mentioned above.



src/tests/fetcher_tests.cpp (line 689)


2 lines apart please.



src/tests/fetcher_tests.cpp (line 694)


Ditto. Kill this line.


- Jie Yu


On March 19, 2016, 10:26 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45057/
> ---
> 
> (Updated March 19, 2016, 10:26 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4885
> https://issues.apache.org/jira/browse/MESOS-4885
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made `unzip` overwrite existing files without prompting.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e 
> 
> Diff: https://reviews.apache.org/r/45057/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 44823: Remove `SlaveState` in `DockerContainerizer` during recover.

[haosdent huang]

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

(Updated March 16, 2016, 11:26 a.m.)


Review request for mesos and Till Toenshoff.


Changes
---

Rebase.


Repository: mesos


Description
---

Remove `SlaveState` in `DockerContainerizer` during recover.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 79cd955e9c241becff52cc4bbef81dcc16802ee7 
  src/slave/containerizer/docker.cpp fb9188a19a5cd8211d4f36f9647ebb70de560109 
  src/tests/containerizer/docker_containerizer_tests.cpp 
8afaa4dab3984e9866b7b223e8e2e70ef83a39dc 

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


Testing
---


Thanks,

haosdent huang

Review Request 44991: Enabled mocking on `TestContainerizer::destroy`.

[Alexander Rukletsov]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/containerizer.hpp 67fbe7fedbe170c3f22a2dcbb5aebf4195a5aabc 
  src/tests/containerizer.cpp c6772ce5908edaab6c3189a65e8446217d1c7c27 

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


Testing
---

The complete chain was tested. See https://reviews.apache.org/r/44662/.


Thanks,

Alexander Rukletsov

Re: Review Request 44946: Add utility functions to manipulate XFS project quotas.

[James Peach]

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

(Updated March 18, 2016, 3:46 a.m.)


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


Changes
---

Rebased.


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


Repository: mesos


Description
---

Add utility functions to manipulate XFS project quotas.


Diffs (updated)
-

  src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
  src/linux/xfs.hpp PRE-CREATION 
  src/linux/xfs.cpp PRE-CREATION 

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


Testing
---

Make check. Manual verification. Tests in subsequent patches.


Thanks,

James Peach

Re: Review Request 44975: Updated cgroups test cases for cgroups device support.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44974, 44975]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 17, 2016, 7:35 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44975/
> ---
> 
> (Updated March 17, 2016, 7:35 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Felix Abecassis, Kevin Klues, and 
> Niklas Nielsen.
> 
> 
> Bugs: MESOS-3368
> https://issues.apache.org/jira/browse/MESOS-3368
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated cgroups test cases for cgroups device support.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_tests.cpp 
> acaed9b3f8a04964092cef413133834d0cf5a145 
> 
> Diff: https://reviews.apache.org/r/44975/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 44944: Handled chunked responses in docker URI fetcher.

[Jie Yu]

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

(Updated March 17, 2016, 1:50 a.m.)


Review request for mesos, Gilbert Song, Jojy Varghese, James Peach, and Timothy 
Chen.


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


Repository: mesos


Description (updated)
---

Handled chunked responses in docker URI fetcher.

This is because if the response is chunked, curl will output the dechunked 
version. That will confuse the http response decoder because the header shows 
that it is chunked.


Diffs
-

  src/uri/fetchers/docker.cpp 2ceee199c6b1bf92c5eca80c8d3344e97e394a09 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Review Request 44998: Fixed m4 macro to enable strict C++11 conformance.

[Neil Conway]

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

Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description
---

If the compiler supports C++11 or greater by default, we might
still need to specify "-std=c++11" to disable compiler support for
GNU-specific extensions or C++14. This is the case for GCC 6, which
defaults to "-std=g++14".


Diffs
-

  m4/ax_cxx_compile_stdcxx.m4 PRE-CREATION 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 43613: Refactor cluster test helpers into self-contained objects.

[Michael Park]

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



Committed with the following formatting fixes.


src/tests/cluster.cpp (lines 157 - 158)


Fits on one line.



src/tests/cluster.cpp (lines 220 - 221)


Fits on one line.



src/tests/cluster.cpp (lines 240 - 241)


Fits on one line.



src/tests/cluster.cpp (lines 249 - 250)


Fits on one line.



src/tests/cluster.cpp (lines 266 - 268)


```
  slaveRemovalLimiter.isSome() ? slaveRemovalLimiter
   : master->slaveRemovalLimiter,
```



src/tests/cluster.cpp (lines 274 - 276)


```
StandaloneMasterDetector* _detector = CHECK_NOTNULL(
dynamic_cast(master->detector.get()));
```


- Michael Park


On March 14, 2016, 9:31 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43613/
> ---
> 
> (Updated March 14, 2016, 9:31 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Artem 
> Harutyunyan, and Michael Park.
> 
> 
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Major rewrite of the `tests/cluster` helpers.  This strongly ties the scope 
> of test objects to the test body.
> 
> Changes the `Cluster` class into two RAII objects (`Master` and `Slave`).  
> The `Slave` object performs cleanup originally found in 
> `cluster::Slave::stop`.  `cluster::Master::start` and `cluster::Slave::start` 
> were changed to factory methods.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
>   src/tests/cluster.cpp e5796d3cc17f814bec8f02dccf40515b65cfea91 
> 
> Diff: https://reviews.apache.org/r/43613/diff/
> 
> 
> Testing
> ---
> 
> Tests are run at the end of this review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 44893: Remove fetcher header file in containerizer.hpp.

[haosdent huang]

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

Review request for mesos and Till Toenshoff.


Repository: mesos


Description
---

Remove fetcher header file in containerizer.hpp.


Diffs
-

  src/local/local.cpp f8599e7378e9a0065bbd01ad8f23f11debb30c91 
  src/slave/containerizer/containerizer.hpp 
ff78b4d0fd4a3b862f6019fc757c16b7367cd3cf 
  src/slave/containerizer/docker.hpp 79cd955e9c241becff52cc4bbef81dcc16802ee7 
  src/slave/containerizer/mesos/containerizer.hpp 
3ef6a6752a6656e97be9f48bd4d2d060d1f9cb46 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 44767: Added authentication information to master endpoints.

[Joerg Schad]

> On March 14, 2016, 10:51 p.m., Greg Mann wrote:
> > docs/endpoints/master/create-volumes.md, line 24
> > 
> >
> > I wonder if the shorthand "iff" might be a bit too esoteric for these 
> > help strings?

We actually use it at several places in HELP master endpoint strings.
E.g., '''Returns 200 OK iff the Master is healthy.'''.
No strong opinion happy to change (not now this has moved to the earlier 
review).


- Joerg


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


On March 17, 2016, 11:57 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44767/
> ---
> 
> (Updated March 17, 2016, 11:57 a.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Bugs: MESOS-4934
> https://issues.apache.org/jira/browse/MESOS-4934
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master endpoints now display information whether authentication (if enabled) 
> is required in HELP.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b47ab7cc86c0a56a81815a98bd63f37a1175ba7f 
> 
> Diff: https://reviews.apache.org/r/44767/diff/
> 
> 
> Testing
> ---
> 
> Viewed master endpoint help in browser.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 44707: Added validation for task's kill policy.

[Alexander Rukletsov]

> On March 15, 2016, 8:35 p.m., Ben Mahler wrote:
> > Could you add a corresponding test? Having it in the header should make 
> > this really easy :)

Sure, but I'll write a more complex test than just unit testing the validation 
function.


- Alexander


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


On March 15, 2016, 3:47 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44707/
> ---
> 
> (Updated March 15, 2016, 3:47 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4909
> https://issues.apache.org/jira/browse/MESOS-4909
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b 
>   src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca 
> 
> Diff: https://reviews.apache.org/r/44707/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44379: Correctly parse perf stat format for non-vanilla 3.10 kernel.

[Benjamin Mahler]

Hey Fan,

Left some comments, mostly I'm a bit confused by your comment that OS
vendors can tweak the format. How did you know they only tweak it that
particular way?

Ben

On Wed, Mar 9, 2016 at 11:53 PM, Mesos ReviewBot 
wrote:

> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44379/
>
> Patch looks great!
>
> Reviews applied: [44379]
>
> Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
> COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
> ./support/docker_build.sh
>
>
> - Mesos ReviewBot
>
> On March 10th, 2016, 3:26 a.m. UTC, fan du wrote:
> Review request for mesos and Ben Mahler.
> By fan du.
>
> *Updated March 10, 2016, 3:26 a.m.*
> *Bugs: * MESOS-4705 
> *Repository: * mesos
> Description
>
> For vanilla kernel <= 3.12, 'perf stat' format is:
>   value,event,cgroup
> Tested with 3.12, 3.11, 3.10 vanilla kernel.
>
> OS vendors may enhance perf tools with additional format:
>   value,unit,event,cgroup,running,ratio
> Tested on Centos 7 with kernel 3.10.0-{123 OR 229}.el7.x86_64
>
> Testing
>
>
>1. {Found and Test} with Serenity, ema filter could get perf event 
> statistics correctly as expected.
>2. ./bin/mesos-tests.sh --gtest_filter=PerfEventIsolatorTest* 
> --log_dir=/tmp/mesos/
>
> Diffs
>
>- src/linux/perf.cpp (1c113a2b3f57877e132bbd65e01fb2f045132128)
>
> View Diff 
>

Re: Review Request 44547: Added functions in promises to the collect header.

[Anurag Singh]

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

(Updated March 18, 2016, 12:29 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Added functions in promises to the collect header.


Diffs (updated)
-

  3rdparty/libprocess/include/process/collect.hpp 
5a92b72eb7668494dc832ec446a41b3d673a20cc 

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


Testing
---

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


Thanks,

Anurag Singh

Re: Review Request 44543: Removed unnecessary MasterContender and MasterDetector definitions.

[Joseph Wu]

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




src/master/contender.hpp (lines 16 - 17)


You might have missed my earlier comment (because I didn't make it a RB 
"issue").  Are these two files deleted somewhere else?

I can't seem to find where you deleted them.


- Joseph Wu


On March 17, 2016, 5:29 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44543/
> ---
> 
> (Updated March 17, 2016, 5:29 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector are now defined in
> include/mesos/master/contender.hpp and detector.hpp.
> 
> 
> Diffs
> -
> 
>   src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
>   src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
> 
> Diff: https://reviews.apache.org/r/44543/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44670/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44555: Implemented the framework and create() method of "network/cni" isolator.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/network/cni/cni.hpp (line 91)


let's use networkConfig here to be consistent.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 65)


you can use ->empty() without 'get()'



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 70)


Ditto.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 104)


s/netConfig/networkConfig/



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 127)


s/netConfig/networkConfig/


- Jie Yu


On March 16, 2016, 7:25 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44555/
> ---
> 
> (Updated March 16, 2016, 7:25 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the framework and create() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0517dd1ae7125ac1ae85cc83b6daaca66a10b8b4 
>   src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44555/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44975: Updated cgroups test cases for cgroups device support.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44974, 44975]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 18, 2016, 9:35 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44975/
> ---
> 
> (Updated March 18, 2016, 9:35 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Felix Abecassis, Kevin Klues, and 
> Niklas Nielsen.
> 
> 
> Bugs: MESOS-3368
> https://issues.apache.org/jira/browse/MESOS-3368
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated cgroups test cases to include two test cases 
> CgroupsAnyHierarchyDevicesTest.ParseTest and 
> CgroupsAnyHierarchyDevicesTest.ROOT_CGROUPS_Devices in order to test device 
> support in cgroup.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_tests.cpp 
> acaed9b3f8a04964092cef413133834d0cf5a145 
> 
> Diff: https://reviews.apache.org/r/44975/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 44553: Added authentication to agent HTTP endpoints.

[Joerg Schad]

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




src/slave/slave.cpp (line 677)


Where is this coming from? I would expect this in slave/constants.hpp 
similar as for the master

I am probably missing something here... :-)


- Joerg Schad


On March 17, 2016, 10:56 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44553/
> ---
> 
> (Updated March 17, 2016, 10:56 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4850
> https://issues.apache.org/jira/browse/MESOS-4850
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authentication to agent HTTP endpoints.
> 
> This patch adds HTTP authentication to the `/state`, `/state.json`, and 
> `/flags` endpoints. Tests are also updated to use authentication when hitting 
> these endpoints, and a new test was added to probe these endpoints' behavior 
> when bad credentials are supplied: `SlaveTest.HTTPEndpointsBadAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 
>   src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> f6fce7df82417e029fadf805d6e0b793f396aa69 
>   src/tests/fault_tolerance_tests.cpp 
> f99413f56e96a796d3d45decad1f049e6a238789 
>   src/tests/health_check_tests.cpp d32164aeb1eb439bd062afa28614dd919e24f06b 
>   src/tests/slave_tests.cpp ea1d776077bf638885db8421194aa4427c772169 
> 
> Diff: https://reviews.apache.org/r/44553/diff/
> 
> 
> Testing
> ---
> 
> Tests were updated to use authentication when hitting the affected agent 
> endpoints, and new tests were added to probe the endpoint behavior when 
> invalid credentials are supplied.
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04. The new test 
> was run 1000 times to look for flakiness.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 44981: Marked some private `json` functions as `static`.

[Neil Conway]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Marked some private `json` functions as `static`.


Diffs
-

  src/master/http.cpp b47ab7cc86c0a56a81815a98bd63f37a1175ba7f 
  src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 

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


Testing
---

make check


Thanks,

Neil Conway

Review Request 45006: Added `devolve` overloads for `v1::TaskStatus`.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/internal/devolve.hpp cf5aad45fe3a1ca17161ec27e68884a93934cc42 
  src/internal/devolve.cpp 27918f1fc385b1770843697c16a29fd0d376f39d 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 44897: Update containerizer construction in hook_tests.cpp.

[haosdent huang]

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

Review request for mesos and Till Toenshoff.


Repository: mesos


Description
---

Update containerizer construction in hook_tests.cpp.


Diffs
-

  src/tests/hook_tests.cpp bb287c77493209e663a37e7f88d09a0459855f7d 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 44854: Added validation for executor's shutdown grace period.

[Alexander Rukletsov]

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

(Updated March 17, 2016, 11:34 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca 

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


Testing
---

The complete chain was tested. See https://reviews.apache.org/r/44662/.


Thanks,

Alexander Rukletsov