Re: Review Request 48612: Removed conditional for of setting `--prefix=/`.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On June 13, 2016, 1:15 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48612/
> ---
> 
> (Updated June 13, 2016, 1:15 a.m.)
> 
> 
> Review request for mesos and Kapil Arya.
> 
> 
> Bugs: MESOS-5577
> https://issues.apache.org/jira/browse/MESOS-5577
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For bundled 3rdparty packages the `--prefix` flag should always be set
> to `/`, else the 3rdparty installation path might append the
> $MESOS_INSTALL to the installation path.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am fb4a37d50e751703b4ccddb0e004b58560707067 
> 
> Diff: https://reviews.apache.org/r/48612/diff/
> 
> 
> Testing
> ---
> 
> make and make install
> 
> Bundled zookeeper installation takes place on the following path:
> vagrant@mesos-dev:~/mesosphere/mesos/build/3rdparty$ find ~/mesos_install/ 
> -name "zookeeper*"
> /home/vagrant/mesos_install/lib/mesos/3rdparty/include/zookeeper
> /home/vagrant/mesos_install/lib/mesos/3rdparty/include/zookeeper/zookeeper.h
> /home/vagrant/mesos_install/lib/mesos/3rdparty/include/zookeeper/zookeeper_version.h
> /home/vagrant/mesos_install/lib/mesos/3rdparty/include/zookeeper/zookeeper_log.h
> /home/vagrant/mesos_install/lib/mesos/3rdparty/include/zookeeper/zookeeper.jute.h
> /home/vagrant/mesos_install/include/mesos/zookeeper
> /home/vagrant/mesos_install/include/mesos/zookeeper/zookeeper.hpp
> /home/vagrant/mesos_install/include/mesos/state/zookeeper.hpp
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 48527: Add NetworkInfo.labels to CNI Network before passing to CNI plugin.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On June 14, 2016, 1:05 a.m., Dan Osborne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48527/
> ---
> 
> (Updated June 14, 2016, 1:05 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add NetworkInfo.labels to CNI Network before passing to CNI plugin.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 09890cedf2e7a1846bd1cb250e117be1680a1b80 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 106ff35320f37b2e75ed60381de1406459e6d515 
> 
> Diff: https://reviews.apache.org/r/48527/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Osborne
> 
>

Re: Review Request 47352: Fixed the configuration.md for docker volume checkpoint flag.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47352]

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

- Mesos ReviewBot


On June 14, 2016, 12:06 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47352/
> ---
> 
> (Updated June 14, 2016, 12:06 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the configuration.md for docker volume checkpoint flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 7612d39d88dc6c0229b5def9697a97ab387f6ef1 
> 
> Diff: https://reviews.apache.org/r/47352/diff/
> 
> 
> Testing
> ---
> 
> gist view.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48671: Restored ordering of --isolation entry processsing.

[Kevin Klues]

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


Fix it, then Ship it!




Looks good overall, just some small nits!


src/slave/containerizer/mesos/containerizer.cpp (line 32)


You can probably safely remove the `#include` for `` and the 
`using std::pair` as they were only introduced for the recent ordering changes 
that are now being reverted.



src/slave/containerizer/mesos/containerizer.cpp (line 347)


You probably don't want to print `+ isolation +` here as it will double 
print below.


- Kevin Klues


On June 14, 2016, 1:31 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48671/
> ---
> 
> (Updated June 14, 2016, 1:31 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Kevin Klues.
> 
> 
> Bugs: MESOS-5581
> https://issues.apache.org/jira/browse/MESOS-5581
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In 6fb9c024, the ordering of isolators was changed from being
> specified by the operator to being specified by the code. It
> turns out that the operator ordering was intended to ensure
> that isolators can check their ordering requirements. If an
> isolator detects that its ordering requirements are not
> met, it can inform the operator to adjust the --isolation flag
> via a failure message.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e5a339d965aeef2e3289c0ec3d9fb6eb281a567c 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp 
> d7557a0c338e8c0e51461b2326600c03f89c2e8b 
> 
> Diff: https://reviews.apache.org/r/48671/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Review Request 48671: Restored ordering of --isolation entry processsing.

[Benjamin Mahler]

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

Review request for mesos, Jie Yu and Kevin Klues.


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


Repository: mesos


Description
---

In 6fb9c024, the ordering of isolators was changed from being
specified by the operator to being specified by the code. It
turns out that the operator ordering was intended to ensure
that isolators can check their ordering requirements. If an
isolator detects that its ordering requirements are not
met, it can inform the operator to adjust the --isolation flag
via a failure message.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
e5a339d965aeef2e3289c0ec3d9fb6eb281a567c 
  src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp 
d7557a0c338e8c0e51461b2326600c03f89c2e8b 

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


Testing
---

make check


Thanks,

Benjamin Mahler

Re: Review Request 47667: Added stout functions to get and set supplementary groud ids.

[James Peach]

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




3rdparty/stout/include/stout/os/posix/su.hpp (line 177)


Bear in mind that ``SC_NGROUPS_MAX`` is not meaningful on Darwin other than 
it is the number of groups returned by ``getgroups(2)``. I don't remembee 
whether ``getgrouplist(3)`` gives you the fill list, but the manpage sounds 
promising.

AFAICT this code will always truncate the list at 16.


- James Peach


On June 13, 2016, 11:50 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47667/
> ---
> 
> (Updated June 13, 2016, 11:50 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added stout functions to get and set supplementary groud ids.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/su.hpp 
> 0a3f23918be9f4d0b044cd9c45001114bc36fce5 
> 
> Diff: https://reviews.apache.org/r/47667/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47668: Obtained uid/gids before changing filesystem root.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47660, 47661, 47663, 47664, 47666, 47667, 48669, 47668]

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

- Mesos ReviewBot


On June 13, 2016, 11:53 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47668/
> ---
> 
> (Updated June 13, 2016, 11:53 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Obtained uid/gids before changing filesystem root.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp ffebb3a496be6a20396f8f539372a1e7527c9b6d 
>   src/slave/containerizer/mesos/launch.cpp 
> e6c83a358148d281dc5f9c170eda6e092bd40a32 
> 
> Diff: https://reviews.apache.org/r/47668/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48669: Modified os user test for user switching.

[James Peach]

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




3rdparty/stout/tests/os_tests.cpp (line 685)


Since you need root, consider making this a separate test using the "ROOT_" 
filter.



3rdparty/stout/tests/os_tests.cpp (line 689)


In general, this kind of stuff is hard to validate in unit tests. Ideally 
you would find a user account with > 16 groups, then create a resource that 
only allows access to the high groups and verify that you have access when you 
impersonate that use. That's pretty involved, so just mentioning it for the 
future :)


- James Peach


On June 13, 2016, 11:51 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48669/
> ---
> 
> (Updated June 13, 2016, 11:51 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies 'setuid', 'setgid', 'getgrouplist' and
> 'setgroups' work properly on both linux and os x.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os_tests.cpp af1a76c3d950cf9062a985c6040d92bd8be14cb8 
> 
> Diff: https://reviews.apache.org/r/48669/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48527: Add NetworkInfo.labels to CNI Network before passing to CNI plugin.

[Dan Osborne]

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

(Updated June 14, 2016, 1:05 a.m.)


Review request for mesos.


Repository: mesos


Description
---

Add NetworkInfo.labels to CNI Network before passing to CNI plugin.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
09890cedf2e7a1846bd1cb250e117be1680a1b80 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
106ff35320f37b2e75ed60381de1406459e6d515 

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


Testing
---


Thanks,

Dan Osborne

Re: Review Request 47352: Fixed the configuration.md for docker volume checkpoint flag.

[Guangya Liu]

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


Ship it!




Ship It!

- Guangya Liu


On 六月 14, 2016, 12:06 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47352/
> ---
> 
> (Updated 六月 14, 2016, 12:06 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the configuration.md for docker volume checkpoint flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 7612d39d88dc6c0229b5def9697a97ab387f6ef1 
> 
> Diff: https://reviews.apache.org/r/47352/diff/
> 
> 
> Testing
> ---
> 
> gist view.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47374: Separated mesos test helpers into a separate library.

[Till Toenshoff]

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



Overall, I am not convinced this is reaching the goals we have just yet...

Let me try to specify our goals;
(a) we want to enable mesos-modules (tests) to reuse our test helpers/utils - 
things like `cluster.cpp` etc. 
(b) we want to make sure that everything used within the modules is actually 
publicly exposed to prevent creating dependencies on internal stuff that is not 
going through deprecations
(c) we try to be least disruptive on mesos

a. reached
b. not reached as we do not expose the headers - a module test using that 
library will still have to tap into the non public mesos folders to get to the 
needed headers
c. reached - this patch only changes a makefile

So (b) is the culprit and solving it properly will likely require us to do some 
serious refactoring of those headers (e.g. `src/tests/mesos.hpp`) to have a 
clean cut between stuff we want to expose and things we can keep internal to 
mesos.

Having public test helpers available for module developers is great and very 
much needed - but I also believe that it might need much more work.


src/Makefile.am (line 1930)


Why would we want to install this static library together with our tests?

The installation of tests was meant as provisioning tests; to make sure the 
host system can successfully run mesos which is validated by running 
mesos-tests without having to build them on that host.



src/Makefile.am (line 1933)


I would propose to move this out of the if-clause and make it a general 
`noinst`.



src/Makefile.am (line 1963)


Not yours but our indentation seems to be inconsistent here. We might want 
to clean that up in another patch.



src/Makefile.am (lines 1971 - 1980)


Good point, I very much agree.


- Till Toenshoff


On June 11, 2016, 12:03 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47374/
> ---
> 
> (Updated June 11, 2016, 12:03 a.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, Kapil Arya, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives external projects easier access to the test helpers used in
> mesos tests.  
> 
> For example, a module writer may want to write a test like 
> `src/tests/oversubscription_tests.cpp`.  To build and link against 
> this library, the module writer would mimic the build flags for tests:
> ```
> # Main test file is taken directly from Mesos.
> my_module_tests_SOURCES = \
>   $(MESOS)/src/tests/main.cpp
> 
> my_module_tests_CPPFLAGS = \
>   -I$(GMOCK)/include   \
>   -I$(GTEST)/include   \
>   -I$(MESOS)/include/mesos \
>   -I$(ZOOKEEPER)/include   \
>   -I$(ZOOKEEPER)/generated \
>   $(AM_CPPFLAGS)
>   
> my_module_tests_LDADD = \
>   $(MESOS)/3rdparty/.libs/libgmock.la \
>   $(MESOS)/src/.libs/libmesos.la  \
>   $(MESOS)/src/.libs/libmesos_tests.la
> ```
> 
> 
> Diffs
> -
> 
>   src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
> 
> Diff: https://reviews.apache.org/r/47374/diff/
> 
> 
> Testing
> ---
> 
> make check on OSX, CentOS 7, Ubuntu 14
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47352: Fixed the configuration.md for docker volume checkpoint flag.

[Gilbert Song]

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

(Updated June 13, 2016, 5:06 p.m.)


Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Fixed the configuration.md for docker volume checkpoint flag.


Diffs (updated)
-

  docs/configuration.md 7612d39d88dc6c0229b5def9697a97ab387f6ef1 

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


Testing
---

gist view.


Thanks,

Gilbert Song

Re: Review Request 48511: Renamed `responseContentType` to `contentType`/`acceptType`.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 13, 2016, 11:58 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48511/
> ---
> 
> (Updated June 13, 2016, 11:58 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change renames existing occurences of `responseContentType` to
> `contentType` if its a handler argument or `acceptType` inside
> `api()`, `scheduler()` and `executor()` functions in Master/Agent.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
>   src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
> 
> Diff: https://reviews.apache.org/r/48511/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 47893: Changed initialization order of `Anonymous` modules in Master.

[Avinash sridharan]

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

(Updated June 14, 2016, 12:01 a.m.)


Review request for mesos, Jie Yu and Kapil Arya.


Changes
---

Added comment to describe the intitalization sequence.


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


Repository: mesos


Description
---

Changed initialization order of `Anonymous` modules in Master.


Diffs (updated)
-

  src/master/main.cpp ce6291f2595163a578abac515cb8250b066cbc52 

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


Testing
---

make check 
and sudo build/bin/mesos-tests.sh


Thanks,

Avinash sridharan

Re: Review Request 47892: Re-ordered initialization for Agent `Anonymous` modules.

[Avinash sridharan]

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

(Updated June 14, 2016, midnight)


Review request for mesos, Jie Yu and Kapil Arya.


Changes
---

Added comment to describe sequence of initialization. Also, initialized the 
`VersionProcess` before firewall.


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


Repository: mesos


Description
---

Re-ordered initialization for Agent `Anonymous` modules.


Diffs (updated)
-

  src/slave/main.cpp fddb05c2d2756f823a4fa393373a9ebc0f14895c 

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


Testing
---

make check
sudo ./build/bin/mesos-tests.sh


Thanks,

Avinash sridharan

Re: Review Request 48511: Renamed `responseContentType` to `contentType`/`acceptType`.

[Anand Mazumdar]

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

(Updated June 13, 2016, 11:58 p.m.)


Review request for mesos, haosdent huang and Vinod Kone.


Changes
---

Review comments


Summary (updated)
-

Renamed `responseContentType` to `contentType`/`acceptType`.


Repository: mesos


Description (updated)
---

This change renames existing occurences of `responseContentType` to
`contentType` if its a handler argument or `acceptType` inside
`api()`, `scheduler()` and `executor()` functions in Master/Agent.


Diffs (updated)
-

  src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
  src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
  src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 

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


Testing
---


Thanks,

Anand Mazumdar

Re: Review Request 48511: Renamed `responseContentType` to `contentType`/`acceptType`.

[Anand Mazumdar]

> On June 13, 2016, 9:29 p.m., Vinod Kone wrote:
> > hmm. i dont think `responseType` is that clear. what is a http response 
> > type? type could as well refer to status code. or maybe accept type. 
> > `responseContentType` is explicit.
> 
> Anand Mazumdar wrote:
> hmm .. The _type_ of the variable is `ContentType`. Hence, just having 
> the variable name as `responseType` is _intuitive_ to the reader that this 
> refers to the content type of the response. That being said, looks like this 
> reasoning might not be that obvious.
> 
> How about just renaming it as `acceptType` to be more intuitive?
> 
> Vinod Kone wrote:
> `acceptType` sounds good to me inside `master::Http::api()`. The handlers 
> themselves should call the parameter `contentType` instead of `acceptType`?

+1. Updated the diff.


- Anand


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


On June 13, 2016, 11:58 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48511/
> ---
> 
> (Updated June 13, 2016, 11:58 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change renames existing occurences of `responseContentType` to
> `contentType` if its a handler argument or `acceptType` inside
> `api()`, `scheduler()` and `executor()` functions in Master/Agent.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
>   src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
> 
> Diff: https://reviews.apache.org/r/48511/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 47668: Obtained uid/gids before changing filesystem root.

[Gilbert Song]

> On June 10, 2016, 5:14 p.m., Jie Yu wrote:
> > Have you tested switch user on mac?

Tested by both methods on mac and linux: 1. manually with command task. 2. 
unite test case.


- Gilbert


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


On May 26, 2016, 1:55 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47668/
> ---
> 
> (Updated May 26, 2016, 1:55 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Obtained uid/gids before changing filesystem root.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp e4c3b75b647b54ffecedfdfa34fb3925f686c0c7 
>   src/slave/containerizer/mesos/launch.cpp 
> e22106b014c871e2184a15c2ab154a0674874e47 
> 
> Diff: https://reviews.apache.org/r/47668/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47668: Obtained uid/gids before changing filesystem root.

[Gilbert Song]

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

(Updated June 13, 2016, 4:53 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin Klues, 
and Timothy Chen.


Repository: mesos


Description
---

Obtained uid/gids before changing filesystem root.


Diffs (updated)
-

  src/launcher/executor.cpp ffebb3a496be6a20396f8f539372a1e7527c9b6d 
  src/slave/containerizer/mesos/launch.cpp 
e6c83a358148d281dc5f9c170eda6e092bd40a32 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 48669: Modified os user test for user switching.

[Gilbert Song]

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

Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin Klues, 
and Timothy Chen.


Repository: mesos


Description
---

This test verifies 'setuid', 'setgid', 'getgrouplist' and
'setgroups' work properly on both linux and os x.


Diffs
-

  3rdparty/stout/tests/os_tests.cpp af1a76c3d950cf9062a985c6040d92bd8be14cb8 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 47667: Added stout functions to get and set supplementary groud ids.

[Gilbert Song]

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

(Updated June 13, 2016, 4:50 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin Klues, 
and Timothy Chen.


Repository: mesos


Description
---

Added stout functions to get and set supplementary groud ids.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/posix/su.hpp 
0a3f23918be9f4d0b044cd9c45001114bc36fce5 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 47664: Implemented image user support in docker runtime isolator.

[Gilbert Song]

> On June 9, 2016, 1:51 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/runtime.cpp, line 101
> > 
> >
> > I suggest we return a Failure if container user is specified since we 
> > don't support it right now.

I would consider a `LOG(WARNING)` more appropriate, since images with user 
defined can still run as ROOT.


- Gilbert


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


On June 13, 2016, 4:48 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47664/
> ---
> 
> (Updated June 13, 2016, 4:48 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented image user support in docker runtime isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/runtime.hpp 
> 272636fcf12c865b31a11364f23d7aee9c6225db 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> eb2790d44ee08c81d31fbe28e7a8ffe88edba870 
> 
> Diff: https://reviews.apache.org/r/47664/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47666: Added stout functions to set uid and gid.

[Gilbert Song]

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

(Updated June 13, 2016, 4:49 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin Klues, 
and Timothy Chen.


Repository: mesos


Description
---

Added stout functions to set uid and gid.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/posix/su.hpp 
0a3f23918be9f4d0b044cd9c45001114bc36fce5 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 47664: Implemented image user support in docker runtime isolator.

[Gilbert Song]

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

(Updated June 13, 2016, 4:48 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin Klues, 
and Timothy Chen.


Repository: mesos


Description
---

Implemented image user support in docker runtime isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/docker/runtime.hpp 
272636fcf12c865b31a11364f23d7aee9c6225db 
  src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
eb2790d44ee08c81d31fbe28e7a8ffe88edba870 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 47663: Added image user to 'ContainerLaunchInfo'.

[Gilbert Song]

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

(Updated June 13, 2016, 4:47 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin Klues, 
and Timothy Chen.


Repository: mesos


Description
---

Added image user to 'ContainerLaunchInfo'.


Diffs (updated)
-

  include/mesos/slave/isolator.proto 60a9bb637e12593a97ed1a7c510ebccd4e5a9615 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 47661: Improved the mesos containerizer windows related logic.

[Gilbert Song]

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

(Updated June 13, 2016, 4:46 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin Klues, 
and Timothy Chen.


Repository: mesos


Description
---

Improved the mesos containerizer windows related logic.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
c7b9744463cf8e1921dcb5e2b7dec7d4e2c0e45f 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 47660: Fixed agent switch user redundant logic in 'getExecutorInfo'.

[Gilbert Song]

> On June 7, 2016, 10:48 a.m., Jie Yu wrote:
> > src/slave/slave.cpp, lines 3943-3950
> > 
> >
> > I don't understand this change. Can you describe the motivation in the 
> > description of this rb? For instance, what if both 
> > task.command().has_user() and frameworkInfo.has_user() are false?

Just fix the redundant check because has_user() always true. Please see commit 
description.


- Gilbert


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


On June 13, 2016, 4:45 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47660/
> ---
> 
> (Updated June 13, 2016, 4:45 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the 'user' in 'FrameworkInfo' is a required field,
> so 'fromeworkInfo.has_user()' is always true. Currently
> in our semantics, if the operator specifies the framework
> user as an empty string, mesos will automagically set it
> as the current host user. So the slave will always obtain
> the user from 'FrameworkInfo', and 'switch_user' defaults
> to be true from agent flag. We would prefer not to change
> this semantics because a long deprecation cycle is needed.
> So in this patch, we just fix the redundant check in slave
> 'getExecutorInfo' logic.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp d635dd2c6f6fce5a9eeefc5dcdf84e00cdc833b6 
> 
> Diff: https://reviews.apache.org/r/47660/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47660: Fixed agent switch user redundant logic in 'getExecutorInfo'.

[Gilbert Song]

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

(Updated June 13, 2016, 4:45 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin Klues, 
and Timothy Chen.


Summary (updated)
-

Fixed agent switch user redundant logic in 'getExecutorInfo'.


Repository: mesos


Description (updated)
---

Since the 'user' in 'FrameworkInfo' is a required field,
so 'fromeworkInfo.has_user()' is always true. Currently
in our semantics, if the operator specifies the framework
user as an empty string, mesos will automagically set it
as the current host user. So the slave will always obtain
the user from 'FrameworkInfo', and 'switch_user' defaults
to be true from agent flag. We would prefer not to change
this semantics because a long deprecation cycle is needed.
So in this patch, we just fix the redundant check in slave
'getExecutorInfo' logic.


Diffs (updated)
-

  src/slave/slave.cpp d635dd2c6f6fce5a9eeefc5dcdf84e00cdc833b6 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 48666: Captured alias variables by const ref in the Master.

[Anand Mazumdar]

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

(Updated June 13, 2016, 11:29 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Updated desc.


Repository: mesos


Description (updated)
---

Mainly done for consitency rather than for performance benefits.
These are the only inconsistent aliasing by value in this file.


Diffs
-

  src/master/master.cpp dd80367768a1edbb6ff94d40819b9755a8b8135f 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 48666: Captured alias variables by const ref in the Master.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 13, 2016, 11:19 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48666/
> ---
> 
> (Updated June 13, 2016, 11:19 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp dd80367768a1edbb6ff94d40819b9755a8b8135f 
> 
> Diff: https://reviews.apache.org/r/48666/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

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

(Updated June 13, 2016, 8:20 p.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


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


Repository: mesos


Description
---

The scheduler library has been updated to wait a little deley before
initiate a connection with the master. The maximum amount of time waited
by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
the master has been detected the scheduler picks a random delay that
can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359


Diffs
-

  src/scheduler/constants.hpp PRE-CREATION 
  src/scheduler/flags.hpp PRE-CREATION 
  src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 

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


Testing
---


Thanks,

Jose Guilherme Vanz

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

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

(Updated June 13, 2016, 8:20 p.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Repository: mesos


Description (updated)
---

The scheduler library has been updated to wait a little deley before
initiate a connection with the master. The maximum amount of time waited
by the scheduler is defined by a flag: CONNECTION_DELAY_MAX. After
the master has been detected the scheduler picks a random delay that
can be between 0 and the CONNECTION_DELAY_MAX value. MESOS-5359


Diffs (updated)
-

  src/scheduler/constants.hpp PRE-CREATION 
  src/scheduler/flags.hpp PRE-CREATION 
  src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 

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


Testing
---


Thanks,

Jose Guilherme Vanz

Review Request 48666: Captured alias variables by const ref in the Master.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/master.cpp dd80367768a1edbb6ff94d40819b9755a8b8135f 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

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

(Updated June 13, 2016, 8:16 p.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Repository: mesos


Description
---

The scheduler library has been updated to wait a little deley before
initiate a connection with the master. The maximum amount of time waited
by the scheduler is defined by a flag: CONNECTION_BACKOFF_MAX. After
the master has been detected the scheduler picks a random delay that
can be between 0 and the CONNECTION_BACKOFF_MAX value. MESOS-5359


Diffs (updated)
-

  src/scheduler/constants.hpp PRE-CREATION 
  src/scheduler/flags.hpp PRE-CREATION 
  src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 

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


Testing
---


Thanks,

Jose Guilherme Vanz

Re: Review Request 48116: Implemented UPDATE_MAINTENANCE_SCHEDULE Call in v1 master API.

[Joseph Wu]

> On June 13, 2016, 2:31 p.m., Joseph Wu wrote:
> > src/master/http.cpp, line 2901
> > 
> >
> > Do we want to standardize this return code (200 -> 202)?  From the 
> > design doc, it seemed like empty responses should be handled with a 202.
> > 
> > Feel free to drop this is not an issue.
> 
> Vinod Kone wrote:
> No response could result in 200 or 202 depending on the semantics of the 
> request.
> 
> Vinod Kone wrote:
> But looks like `_updateMaintenanceSchedule()` should actually return 202 
> because it calls `allocator->updateUnavailability()` which is asynchronous.

Ah.  The updating/persisting of the maintenance schedule is synchronous (does 
not return until the schedule is written to the registry).  

So 200 OK should be correct.  Dropping...


- Joseph


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


On June 12, 2016, 10:46 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48116/
> ---
> 
> (Updated June 12, 2016, 10:46 a.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5505
> https://issues.apache.org/jira/browse/MESOS-5505
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented UPDATE_MAINTENANCE_SCHEDULE Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48116/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48511: Renamed `responseContentType` to `responseType`.

[Vinod Kone]

> On June 13, 2016, 9:29 p.m., Vinod Kone wrote:
> > hmm. i dont think `responseType` is that clear. what is a http response 
> > type? type could as well refer to status code. or maybe accept type. 
> > `responseContentType` is explicit.
> 
> Anand Mazumdar wrote:
> hmm .. The _type_ of the variable is `ContentType`. Hence, just having 
> the variable name as `responseType` is _intuitive_ to the reader that this 
> refers to the content type of the response. That being said, looks like this 
> reasoning might not be that obvious.
> 
> How about just renaming it as `acceptType` to be more intuitive?

`acceptType` sounds good to me inside `master::Http::api()`. The handlers 
themselves should call the parameter `contentType` instead of `acceptType`?


- Vinod


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


On June 9, 2016, 9:30 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48511/
> ---
> 
> (Updated June 9, 2016, 9:30 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Minor: The type of the variable is already `ContentType`. The
> name `responseType` has sufficient information that this
> refers to the content type of the response.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
>   src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
> 
> Diff: https://reviews.apache.org/r/48511/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 48662: Fixed a regression in the creation of the isolator modules.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48662]

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

- Mesos ReviewBot


On June 13, 2016, 9:12 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48662/
> ---
> 
> (Updated June 13, 2016, 9:12 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Kevin Klues.
> 
> 
> Bugs: MESOS-5581
> https://issues.apache.org/jira/browse/MESOS-5581
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While refactoring the isolator creation in 6fb9c024, we introduced a
> regression in that isolator modules were no longer being created via
> the --isolation flag values.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 92c9898fb41ca0ffbda05e53b595b05c96f4f596 
> 
> Diff: https://reviews.apache.org/r/48662/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 48116: Implemented UPDATE_MAINTENANCE_SCHEDULE Call in v1 master API.

[Vinod Kone]

> On June 13, 2016, 9:31 p.m., Joseph Wu wrote:
> > src/master/http.cpp, line 2901
> > 
> >
> > Do we want to standardize this return code (200 -> 202)?  From the 
> > design doc, it seemed like empty responses should be handled with a 202.
> > 
> > Feel free to drop this is not an issue.
> 
> Vinod Kone wrote:
> No response could result in 200 or 202 depending on the semantics of the 
> request.

But looks like `_updateMaintenanceSchedule()` should actually return 202 
because it calls `allocator->updateUnavailability()` which is asynchronous.


- Vinod


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


On June 12, 2016, 5:46 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48116/
> ---
> 
> (Updated June 12, 2016, 5:46 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5505
> https://issues.apache.org/jira/browse/MESOS-5505
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented UPDATE_MAINTENANCE_SCHEDULE Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48116/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48116: Implemented UPDATE_MAINTENANCE_SCHEDULE Call in v1 master API.

[Vinod Kone]

> On June 13, 2016, 9:31 p.m., Joseph Wu wrote:
> > src/master/http.cpp, line 2901
> > 
> >
> > Do we want to standardize this return code (200 -> 202)?  From the 
> > design doc, it seemed like empty responses should be handled with a 202.
> > 
> > Feel free to drop this is not an issue.

No response could result in 200 or 202 depending on the semantics of the 
request.


- Vinod


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


On June 12, 2016, 5:46 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48116/
> ---
> 
> (Updated June 12, 2016, 5:46 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5505
> https://issues.apache.org/jira/browse/MESOS-5505
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented UPDATE_MAINTENANCE_SCHEDULE Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48116/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48585: Devolved v1 operator protos to unversioned operator protos in Master.

[Vinod Kone]

> On June 13, 2016, 10:07 p.m., Vinod Kone wrote:
> > src/master/http.cpp, lines 596-632
> > 
> >
> > s/mesos:://
> 
> Anand Mazumdar wrote:
> I guess @haosdent had to add them due to there being an already existing 
> `master` namespace in our code. The compiler would have barfed otherwise due 
> to it not being able to find a `Call` class in the existing `master` 
> namespace. Hence, it _had_ to be fully qualified.
> 
> Since, we _only_ use the unversioned protobuf internally in our code, we 
> should just do `using mesos::master::Call` and then just use `Call::TYPE` for 
> referring to it. Sound good?

I see. Qualified mesos::master::Call seems fine then. `using 
mesos::master::Call`  might be weird because we also have scheduler::Call in 
this file.


- Vinod


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


On June 12, 2016, 6:01 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48585/
> ---
> 
> (Updated June 12, 2016, 6:01 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Devolved v1 operator protos to unversioned operator protos in Master.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/master/validation.hpp e1271bbaebaac52a078eedfdc2299c7c6f4474f8 
>   src/master/validation.cpp 7b9c2281b2ab1295211f0dd385cd77947fbd63be 
> 
> Diff: https://reviews.apache.org/r/48585/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47973: Updated gc to prevent early exit in case of error.

[Jiang Yan Xu]

> On June 8, 2016, 9:54 a.m., Jiang Yan Xu wrote:
> > src/tests/gc_tests.cpp, lines 948-956
> > 
> >
> > Why do we still need `containerPaths`? We can get the sandbox path 
> > directly:
> > 
> > ```
> > const string sandbox = slave::paths::getExecutorLatestRunPath(
> > flags.work_dir,
> > slaveId,
> > frameworkId.get(),
> > executor_id);
> > 
> > ASSERT_TRUE(os::exists(sandbox));
> > ```
> > 
> > right?
> 
> Megha Sharma wrote:
> getExecutorLatestPath(...) gives us the executor run path for latest 
> symlink which gets collected by GC. So, this path can't be used for post GC 
> assertions. Since we didn't create a container ourselves in the test so we 
> didn't have a container id to call the method getExecutorRunPath(..., 
> containerId). Here, I am getting all the run paths with 
> getExecutorRunPaths(..) and then elimating the one with latest symlink to get 
> the actual path.

Chatted offline about the use of `os::realpath()` to solve this.


- Jiang Yan


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


On June 8, 2016, 9:57 a.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47973/
> ---
> 
> (Updated June 8, 2016, 9:57 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5196
> https://issues.apache.org/jira/browse/MESOS-5196
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It's possible for tasks and isolators to lay down files that are not 
> deletable by GC. In the face of such errors GC needs to free up disk 
> space wherever it can because it's already re-offered to frameworks.
> 
> 
> Diffs
> -
> 
>   src/slave/gc.cpp eedb8ca713d7f5195055bb6417f03ab70731af97 
>   src/tests/gc_tests.cpp 4cb7c2f612984f7f5a9378a7f972f2438bbf28c5 
> 
> Diff: https://reviews.apache.org/r/47973/diff/
> 
> 
> Testing
> ---
> 
> make check.
> Tested with option --gtest_also_run_disabled_tests.
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 47973: Updated gc to prevent early exit in case of error.

[Megha Sharma]

> On June 8, 2016, 4:54 p.m., Jiang Yan Xu wrote:
> > src/tests/gc_tests.cpp, lines 948-956
> > 
> >
> > Why do we still need `containerPaths`? We can get the sandbox path 
> > directly:
> > 
> > ```
> > const string sandbox = slave::paths::getExecutorLatestRunPath(
> > flags.work_dir,
> > slaveId,
> > frameworkId.get(),
> > executor_id);
> > 
> > ASSERT_TRUE(os::exists(sandbox));
> > ```
> > 
> > right?

getExecutorLatestPath(...) gives us the executor run path for latest symlink 
which gets collected by GC. So, this path can't be used for post GC assertions. 
Since we didn't create a container ourselves in the test so we didn't have a 
container id to call the method getExecutorRunPath(..., containerId). Here, I 
am getting all the run paths with getExecutorRunPaths(..) and then elimating 
the one with latest symlink to get the actual path.


- Megha


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


On June 8, 2016, 4:57 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47973/
> ---
> 
> (Updated June 8, 2016, 4:57 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5196
> https://issues.apache.org/jira/browse/MESOS-5196
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It's possible for tasks and isolators to lay down files that are not 
> deletable by GC. In the face of such errors GC needs to free up disk 
> space wherever it can because it's already re-offered to frameworks.
> 
> 
> Diffs
> -
> 
>   src/slave/gc.cpp eedb8ca713d7f5195055bb6417f03ab70731af97 
>   src/tests/gc_tests.cpp 4cb7c2f612984f7f5a9378a7f972f2438bbf28c5 
> 
> Diff: https://reviews.apache.org/r/47973/diff/
> 
> 
> Testing
> ---
> 
> make check.
> Tested with option --gtest_also_run_disabled_tests.
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 48614: Added validation for UUID's to master/agent validation helpers.

[Vinod Kone]

> On June 13, 2016, 7:41 a.m., Neil Conway wrote:
> > src/tests/scheduler_http_api_tests.cpp, line 974
> > 
> >
> > Why are we fetching the stream ID?

looks like you forgot to set the stream ID header when sending the acknowledge 
call. if you don't set it the BadRequest response might be due to the missing 
stream id and not malformed uuid. after you fix this, make sure you set an 
expectation on the response body and not just response status code.


- Vinod


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


On June 13, 2016, 6:51 a.m., Deshna Jain wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48614/
> ---
> 
> (Updated June 13, 2016, 6:51 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5224
> https://issues.apache.org/jira/browse/MESOS-5224
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds logic to the master/agent validation helpers
> to see if the status update acknowledgments have a valid UUID
> set conforming to one of the supported versions.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 309fbed81c9ff0ccc4ff4ee3ee70cf8f1fb2ac55 
>   src/slave/validation.cpp 189d10caf4517a1aff62b523d8eb99d9d1ca4b6a 
>   src/tests/scheduler_http_api_tests.cpp 
> c12205f90e4f2da2c6ad7a0ea75777601eb0ef13 
> 
> Diff: https://reviews.apache.org/r/48614/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Deshna Jain
> 
>

Re: Review Request 48447: Changed the severity level of a log line in Provisioner::destroy().

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 8, 2016, 11:51 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48447/
> ---
> 
> (Updated June 8, 2016, 11:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4952
> https://issues.apache.org/jira/browse/MESOS-4952
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See MESOS-4952.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 4e3d3fa7cf8a33eef0ebc7a4919936987a5af9bd 
> 
> Diff: https://reviews.apache.org/r/48447/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Jose Guilherme Vanz]

> On June 13, 2016, 6:40 p.m., Anand Mazumdar wrote:
> > Jose, any updates on this? I can see that you marked all the outstanding 
> > issues as fixed. Did you forgot publishing the updated diff?

Hi!

Actually, I had to solve some personal issues and forgot to upload the diff. My 
bad
I'm buildind it again with the lastest commits again and I'll upload it right 
after that.


- Jose Guilherme


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


On June 8, 2016, 5:53 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 8, 2016, 5:53 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_BACKOFF_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_BACKOFF_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 48585: Devolved v1 operator protos to unversioned operator protos in Master.

[Anand Mazumdar]

> On June 13, 2016, 10:07 p.m., Vinod Kone wrote:
> > src/master/http.cpp, lines 596-632
> > 
> >
> > s/mesos:://

I guess @haosdent had to add them due to there being an already existing 
`master` namespace in our code. The compiler would have barfed otherwise due to 
it not being able to find a `Call` class in the existing `master` namespace. 
Hence, it _had_ to be fully qualified.

Since, we _only_ use the unversioned protobuf internally in our code, we should 
just do `using mesos::master::Call` and then just use `Call::TYPE` for 
referring to it. Sound good?


- Anand


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


On June 12, 2016, 6:01 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48585/
> ---
> 
> (Updated June 12, 2016, 6:01 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Devolved v1 operator protos to unversioned operator protos in Master.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/master/validation.hpp e1271bbaebaac52a078eedfdc2299c7c6f4474f8 
>   src/master/validation.cpp 7b9c2281b2ab1295211f0dd385cd77947fbd63be 
> 
> Diff: https://reviews.apache.org/r/48585/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48614: Added validation for UUID's to master/agent validation helpers.

[Vinod Kone]

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




src/master/validation.cpp (line 308)


`uuid` is a required field so don't need to do the if check here.

after addressing the previous comments this should be,

```
Try uuid = UUID::fromBytes(call.acknowledge.uuid());
if (uuid.isError()) {
  return Error("Invalid UUID: " + uuid.error());
}

```



src/slave/validation.cpp (line 148)


if blocks have "{" on the same line as the if statement. see above.

```
Try uuid = UUID::fromBytes(status.uuid());
if (uuid.isError()) {
  return Error("Invalid UUID: " + uuid.error());
}

```


- Vinod Kone


On June 13, 2016, 6:51 a.m., Deshna Jain wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48614/
> ---
> 
> (Updated June 13, 2016, 6:51 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5224
> https://issues.apache.org/jira/browse/MESOS-5224
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds logic to the master/agent validation helpers
> to see if the status update acknowledgments have a valid UUID
> set conforming to one of the supported versions.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 309fbed81c9ff0ccc4ff4ee3ee70cf8f1fb2ac55 
>   src/slave/validation.cpp 189d10caf4517a1aff62b523d8eb99d9d1ca4b6a 
>   src/tests/scheduler_http_api_tests.cpp 
> c12205f90e4f2da2c6ad7a0ea75777601eb0ef13 
> 
> Diff: https://reviews.apache.org/r/48614/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Deshna Jain
> 
>

Re: Review Request 48613: Added validation logic for UUID's.

[Vinod Kone]

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




3rdparty/stout/include/stout/uuid.hpp (lines 54 - 78)


As suggested by Neil in the subsequent review, lets have `fromBytes()` 
return a Try instead and do the validation there.

Should we also protect `fromString()` from invalid UUID strings? If yes, 
lets change that to return Try as well.


- Vinod Kone


On June 13, 2016, 6:51 a.m., Deshna Jain wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48613/
> ---
> 
> (Updated June 13, 2016, 6:51 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5224
> https://issues.apache.org/jira/browse/MESOS-5224
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a `validateFromBytes` function for validating
> UUID's by checking if they are one of the supported UUID
> versions.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/uuid.hpp 
> e3cf21ff986112694a417981c22009323cb23987 
>   3rdparty/stout/tests/uuid_tests.cpp 
> 50295ad25843a3938a5a58f5ba7f082f8ffeba45 
> 
> Diff: https://reviews.apache.org/r/48613/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Deshna Jain
> 
>

Re: Review Request 48588: Devolved v1 operator protos to unversioned operator protos in Agent.

[Vinod Kone]

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



please remove mesos:: prefix for calls and responses. also shouldn't use 
`slave` keyword per previous review comments.

- Vinod Kone


On June 12, 2016, 6:01 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48588/
> ---
> 
> (Updated June 12, 2016, 6:01 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Devolved v1 operator protos to unversioned operator protos in Agent.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
>   src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
>   src/slave/validation.hpp 59b12f0109f6fa4c570f02d90834502a8edebe45 
>   src/slave/validation.cpp 189d10caf4517a1aff62b523d8eb99d9d1ca4b6a 
> 
> Diff: https://reviews.apache.org/r/48588/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48587: Added unversioned protos for agent API.

[Vinod Kone]

> On June 13, 2016, 8:18 p.m., Anand Mazumdar wrote:
> > Can we name the unversioned proto file as just `agent.proto`? It would be 
> > good to not introduce the `Slave` terminology going forward :-)

also the package name, class name, namespace etc.


- Vinod


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


On June 12, 2016, 6:01 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48587/
> ---
> 
> (Updated June 12, 2016, 6:01 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unversioned protos for agent API.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/slave.hpp PRE-CREATION 
>   include/mesos/slave/slave.proto PRE-CREATION 
>   src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 
> 
> Diff: https://reviews.apache.org/r/48587/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48585: Devolved v1 operator protos to unversioned operator protos in Master.

[Vinod Kone]

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




src/master/http.cpp (line 573)


s/mesos:://



src/master/http.cpp (lines 596 - 632)


s/mesos:://



src/master/http.cpp (line 1289)


s/mesos:://



src/master/http.cpp (line 1293)


s/mesos:://



src/master/http.cpp (lines 1324 - 1381)


s/mesos:://



src/master/master.hpp (lines 1286 - 1306)


s/mesos:://



src/master/validation.hpp (line 49)


s/mesos:://



src/master/validation.cpp (lines 57 - 129)


s/mesos:://


- Vinod Kone


On June 12, 2016, 6:01 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48585/
> ---
> 
> (Updated June 12, 2016, 6:01 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Devolved v1 operator protos to unversioned operator protos in Master.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/master/validation.hpp e1271bbaebaac52a078eedfdc2299c7c6f4474f8 
>   src/master/validation.cpp 7b9c2281b2ab1295211f0dd385cd77947fbd63be 
> 
> Diff: https://reviews.apache.org/r/48585/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48584: Added unversioned protos for master API.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 12, 2016, 6:01 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48584/
> ---
> 
> (Updated June 12, 2016, 6:01 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unversioned protos for master API.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.hpp PRE-CREATION 
>   include/mesos/master/master.proto PRE-CREATION 
>   src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 
> 
> Diff: https://reviews.apache.org/r/48584/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48583: Added `VersionInfo`, `Flag`, `Metrics` to unversioned mesos protos.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 12, 2016, 6 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48583/
> ---
> 
> (Updated June 12, 2016, 6 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We add `VersionInfo`, `Flag`, `Metrics` to v1 mesos protos in 549a0340,
> but didn't add them to unversioned mesos protos as well. This change
> attempt to fix this.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dbffb16f4317df786ee38999a49e4e89733ff9ff 
> 
> Diff: https://reviews.apache.org/r/48583/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48315: Restart slave if a volume is CREATED on a non-empty path.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48313, 48314, 48315]

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

- Mesos ReviewBot


On June 13, 2016, 9:03 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48315/
> ---
> 
> (Updated June 13, 2016, 9:03 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5448
> https://issues.apache.org/jira/browse/MESOS-5448
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Root of a MOUNT disk is not deleted on volume DELETE. When we do a
> CREATE on a persistent volume and the root directory exists (which
> can happen for MOUNT disks), we allow the operation only if the
> contents within the root directory is empty. If not, we do not update
> the checkpoint with this volume and exit, so as to cleanup when the
> slave restarts and handles the CheckpointResourcesMessage.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 0ccc56f153024ff45d2fd1f25c79c7bb6ed7120e 
> 
> Diff: https://reviews.apache.org/r/48315/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 48662: Fixed a regression in the creation of the isolator modules.

[Jie Yu]

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


Ship it!




Let's commit this patch now as the current master branch is broken.

However, the existing code does not allow operators to specify the ordering 
between built-in isolators and module isolators because built-in isolators 
always take precedence. Can you leave a TODO here to address it later?

- Jie Yu


On June 13, 2016, 9:12 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48662/
> ---
> 
> (Updated June 13, 2016, 9:12 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Kevin Klues.
> 
> 
> Bugs: MESOS-5581
> https://issues.apache.org/jira/browse/MESOS-5581
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While refactoring the isolator creation in 6fb9c024, we introduced a
> regression in that isolator modules were no longer being created via
> the --isolation flag values.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 92c9898fb41ca0ffbda05e53b595b05c96f4f596 
> 
> Diff: https://reviews.apache.org/r/48662/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 48380: Updated CHANGELOG for libprocess HTTP authorization.

[Greg Mann]

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

(Updated June 13, 2016, 9:45 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


Repository: mesos


Description
---

Updated CHANGELOG for libprocess HTTP authorization.


Diffs (updated)
-

  CHANGELOG 0513c1594098c8afdb7085ecab5edf07f93f7d0e 

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 48511: Renamed `responseContentType` to `responseType`.

[Anand Mazumdar]

> On June 13, 2016, 9:29 p.m., Vinod Kone wrote:
> > hmm. i dont think `responseType` is that clear. what is a http response 
> > type? type could as well refer to status code. or maybe accept type. 
> > `responseContentType` is explicit.

hmm .. The _type_ of the variable is `ContentType`. Hence, just having the 
variable name as `responseType` is _intuitive_ to the reader that this refers 
to the content type of the response. That being said, looks like this reasoning 
might not be that obvious.

How about just renaming it as `acceptType` to be more intuitive?


- Anand


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


On June 9, 2016, 9:30 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48511/
> ---
> 
> (Updated June 9, 2016, 9:30 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Minor: The type of the variable is already `ContentType`. The
> name `responseType` has sufficient information that this
> refers to the content type of the response.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
>   src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
> 
> Diff: https://reviews.apache.org/r/48511/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 48381: Updated CHANGELOG for 'work_dir' flag changes.

[Greg Mann]

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

(Updated June 13, 2016, 9:45 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


Repository: mesos


Description
---

Updated CHANGELOG for 'work_dir' flag changes.


Diffs (updated)
-

  CHANGELOG 0513c1594098c8afdb7085ecab5edf07f93f7d0e 

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 48299: Updated balloon framework spacing and logging.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 9, 2016, 11:09 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48299/
> ---
> 
> (Updated June 9, 2016, 11:09 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5174
> https://issues.apache.org/jira/browse/MESOS-5174
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes some whitespace for the balloon framework and
> changes most logging messages to use glog.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp 739fb504e93154bf032b4c621151fa3c99b60037 
> 
> Diff: https://reviews.apache.org/r/48299/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 48387: Delay before initiating a connection with master.

[Anand Mazumdar]

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



Jose, any updates on this? I can see that you marked all the outstanding issues 
as fixed. Did you forgot publishing the updated diff?

- Anand Mazumdar


On June 8, 2016, 8:53 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48387/
> ---
> 
> (Updated June 8, 2016, 8:53 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scheduler library has been updated to wait a little deley before
> initiate a connection with the master. The maximum amount of time waited
> by the scheduler is defined by a flag: CONNECTION_BACKOFF_MAX. After
> the master has been detected the scheduler picks a random delay that
> can be between 0 and the CONNECTION_BACKOFF_MAX value. MESOS-5359
> 
> 
> Diffs
> -
> 
>   src/scheduler/constants.hpp PRE-CREATION 
>   src/scheduler/flags.hpp PRE-CREATION 
>   src/scheduler/scheduler.cpp c79837c93e7329dbfa22e4288b44237f33408ba9 
> 
> Diff: https://reviews.apache.org/r/48387/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 46407: Updated balloon executor.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 9, 2016, 11:09 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46407/
> ---
> 
> (Updated June 9, 2016, 11:09 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5174
> https://issues.apache.org/jira/browse/MESOS-5174
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes the logic in the example `balloon-executor` to work in cases not
> exercised by the `ROOT_CGROUPS_BalloonFramework` test.
> 
> * The "task" logic was moved into a separate thread.  This fixes the 
>   case where the balloon executor does not exceed the memory limit.
>   (i.e. by Unblocking the driver's thread while sending status updates.)
> * Changed log messages to use glog.
> * Added logic to prevent multiple task launches with the same executor.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_executor.cpp 2a79c353e96930d25495732d15b83c82247974bb 
> 
> Diff: https://reviews.apache.org/r/46407/diff/
> 
> 
> Testing
> ---
> 
> See next review in this chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 45604: Updated balloon framework code with flags and better resource math.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 9, 2016, 11:48 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45604/
> ---
> 
> (Updated June 9, 2016, 11:48 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5174
> https://issues.apache.org/jira/browse/MESOS-5174
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives the example `balloon-framework` enough options to run
> outside of the build environment:
> 
> * Adds an option for restricting the number of resources per task
>   (otherwise, it will eat up an entire node).
> * Adds an option for persisting the framework and launching one task
>   after another.
> * Adds filters for declined offers.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp 739fb504e93154bf032b4c621151fa3c99b60037 
>   src/tests/balloon_framework_test.sh 
> a242f6cb9ca1850e5fef90e0938f41044bdaddbf 
> 
> Diff: https://reviews.apache.org/r/45604/diff/
> 
> 
> Testing
> ---
> 
> ```
> make check 
> 
> sudo bin/mesos-tests.sh --gtest_filter="*ROOT_CGROUPS_BalloonFramework"
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 48116: Implemented UPDATE_MAINTENANCE_SCHEDULE Call in v1 master API.

[Joseph Wu]

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


Fix it, then Ship it!




LGTM!


src/master/http.cpp (line 2864)


Do we want to standardize this return code (200 -> 202)?  From the design 
doc, it seemed like empty responses should be handled with a 202.

Feel free to drop this is not an issue.


- Joseph Wu


On June 12, 2016, 10:46 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48116/
> ---
> 
> (Updated June 12, 2016, 10:46 a.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5505
> https://issues.apache.org/jira/browse/MESOS-5505
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented UPDATE_MAINTENANCE_SCHEDULE Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
>   src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
>   src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
> 
> Diff: https://reviews.apache.org/r/48116/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48303: Split the balloon scheduler into a Process to support metrics.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 9, 2016, 11:53 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48303/
> ---
> 
> (Updated June 9, 2016, 11:53 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5174
> https://issues.apache.org/jira/browse/MESOS-5174
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds a `BalloonSchedulerProcess` which holds a subset of the `Scheduler`
> class's methods.  The `BalloonScheduler` dispatches to the process
> when for non-logging business logic.  This also opens up the balloon 
> framework for libprocess metrics.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp 739fb504e93154bf032b4c621151fa3c99b60037 
> 
> Diff: https://reviews.apache.org/r/48303/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 48511: Renamed `responseContentType` to `responseType`.

[Vinod Kone]

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



hmm. i dont think `responseType` is that clear. what is a http response type? 
type could as well refer to status code. or maybe accept type. 
`responseContentType` is explicit.

- Vinod Kone


On June 9, 2016, 9:30 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48511/
> ---
> 
> (Updated June 9, 2016, 9:30 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Minor: The type of the variable is already `ContentType`. The
> name `responseType` has sufficient information that this
> refers to the content type of the response.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
>   src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
> 
> Diff: https://reviews.apache.org/r/48511/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 48510: Passed `ContentType` enum by value instead of const ref.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 10, 2016, midnight, Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48510/
> ---
> 
> (Updated June 10, 2016, midnight)
> 
> 
> Review request for mesos, Benjamin Mahler, haosdent huang, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
>   src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
>   src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
>   src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
> 
> Diff: https://reviews.apache.org/r/48510/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 48662: Fixed a regression in the creation of the isolator modules.

[Benjamin Mahler]

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

Review request for mesos, Jie Yu and Kevin Klues.


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


Repository: mesos


Description
---

While refactoring the isolator creation in 6fb9c024, we introduced a
regression in that isolator modules were no longer being created via
the --isolation flag values.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
92c9898fb41ca0ffbda05e53b595b05c96f4f596 

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


Testing
---

sudo make check


Thanks,

Benjamin Mahler

Re: Review Request 48524: Moved flag validation after check for required flags.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 9, 2016, 11:54 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48524/
> ---
> 
> (Updated June 9, 2016, 11:54 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved flag validation after check for required flags.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/flags.hpp 
> 9271f722e5281d7be5e165021c2c48d3b3453a8e 
> 
> Diff: https://reviews.apache.org/r/48524/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 48314: Added os::empty(path) to check if contents in a directory is empty.

[Anindya Sinha]

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

(Updated June 13, 2016, 9:03 p.m.)


Review request for mesos, Neil Conway and Jiang Yan Xu.


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


Repository: mesos


Description
---

Added os::empty(path) to check if contents in a directory is empty.


Diffs (updated)
-

  3rdparty/stout/include/Makefile.am 7f31582c176e653873402bd3f19b0c0195503d45 
  3rdparty/stout/include/stout/os.hpp 53b00932693fba7cf6514da6a519269a904de345 
  3rdparty/stout/include/stout/os/empty.hpp PRE-CREATION 
  3rdparty/stout/tests/os/rmdir_tests.cpp 
a11bfc9f9e6cbb05f3e9ce0ea48297b8f88fe53f 

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 48315: Restart slave if a volume is CREATED on a non-empty path.

[Anindya Sinha]

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

(Updated June 13, 2016, 9:03 p.m.)


Review request for mesos, Neil Conway and Jiang Yan Xu.


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


Repository: mesos


Description
---

Root of a MOUNT disk is not deleted on volume DELETE. When we do a
CREATE on a persistent volume and the root directory exists (which
can happen for MOUNT disks), we allow the operation only if the
contents within the root directory is empty. If not, we do not update
the checkpoint with this volume and exit, so as to cleanup when the
slave restarts and handles the CheckpointResourcesMessage.


Diffs (updated)
-

  src/slave/slave.cpp 0ccc56f153024ff45d2fd1f25c79c7bb6ed7120e 

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

[Anindya Sinha]

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

(Updated June 13, 2016, 9:03 p.m.)


Review request for mesos, Neil Conway and Jiang Yan Xu.


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


Repository: mesos


Description
---

o Checkpoints on the agent are updated only after successful handling
  of persistent volume creation and deletion to maintain consistency.
o If volume creation or deletion fails, checkpoint is updated up until
  that point, and the agent exits.
o This ensures that after a agent restart, checkpoints are in sync
  between the master and the agent after the reregistration workflow.


Diffs (updated)
-

  include/mesos/resources.hpp a557e97c65194d4aad879fb88d8edefd1c95b8d8 
  include/mesos/v1/resources.hpp a5ba8fec4c9c3643646308f75a4b28cefe0b3df3 
  src/common/resources.cpp f6ff92b591c15bc8e93fd85e1896349c3a7bb968 
  src/slave/slave.cpp 0ccc56f153024ff45d2fd1f25c79c7bb6ed7120e 
  src/v1/resources.cpp 8c3f2d1c1529915a59d47fe37bb3fc7a3267079a 

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 48587: Added unversioned protos for agent API.

[Anand Mazumdar]

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



Can we name the unversioned proto file as just `agent.proto`? It would be good 
to not introduce the `Slave` terminology going forward :-)


src/Makefile.am (line 1229)


Nit: Alignment of ``.


- Anand Mazumdar


On June 12, 2016, 6:01 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48587/
> ---
> 
> (Updated June 12, 2016, 6:01 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5593
> https://issues.apache.org/jira/browse/MESOS-5593
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unversioned protos for agent API.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/slave.hpp PRE-CREATION 
>   include/mesos/slave/slave.proto PRE-CREATION 
>   src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 
> 
> Diff: https://reviews.apache.org/r/48587/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

[Anindya Sinha]

> On June 8, 2016, 1:28 p.m., Neil Conway wrote:
> > Overall seems like a reasonable approach.
> > 
> > One thing that isn't clear to me: what is the advantage of updating the 
> > checkpoint to reflect any partial work that was done before exiting? It 
> > seems that adds a bunch of complexity and room for error. Why not only 
> > update the checkpoint if all changes were made successfully?
> 
> Anindya Sinha wrote:
> We would need to maintain what was actually successful in any case since 
> in a DESTROY, a failed rmdir does not lead to the agent exiting. So, if we 
> were to do it at one place, we would still need to keep account of the 
> successful operations so as to not update the checkpoint based on a failed 
> rmdir as an example (and hence can be a partial update).
> 
> Since we are keeping track of result of the operations anyway, I think it 
> is a good idea to update before exiting (only place we do that when CREATE 
> fails and the agent exits) so that the subsequent handling of 
> CheckpointResources does not need to redo such operations when the agent 
> reregisters.
> 
> Neil Conway wrote:
> On reflection, I wonder whether we should be handling `CREATE` errors 
> differently from `DESTROY` errors. In both cases, the user has asked the 
> agent to do something it wasn't able to do. A failed `DESTROY` has the 
> addditional concern that we might have destroyed some but not all of the data 
> on the volume.
> 
> Do you think handling `CREATE` vs. `DESTROY` errors differently is a good 
> idea?
> 
> Anindya Sinha wrote:
> Good point. Here is what I think are the use cases:
> Say we have checkpointed resources (CR) as {V1,V2} where V1, V2 are 2 
> persistent volumes. So, CR(master) = {V1,V2}, and CR(agent) = {V1,V2}.
> If we now receive a DESTROY(V2): CR(master) = {V1} [since master's view 
> is not dependent on what happened on the agent]. Suppose that fails on the 
> agent, so CR(agent) = {V1,V2} [since we do not update checkpoint resources on 
> agent on failure in DESTROY, which results in inconsistency between master 
> and agent at this point of time].
> 
> Case 1 (current implementation): Agent does not restart on failure in 
> DESTROY. Hence, CR(agent) = {V1,V2}. When the next CheckpointResources is 
> received, ie. on a subsequent CREATE/DESTROY/RESERVE/UNRESERVE on a different 
> resource, DESTROY(V2) will be reattempted and if that is successful, we will 
> in sync between agent and master. However if the next CheckpointResources is 
> due to a CREATE(V2) [that can happen since V2 is available as a resource 
> based on offer from master], that would be a no-op on agent since agent does 
> not treat it as a new resource based on the checkpoint since at this point 
> CR(master) = {V1,V2}, and CR(agent) = {V1,V2}, which would be a problem.
> 
> Case 2 (if we exit agent on failure): The agent restarts which triggers a 
> CheckpointResources from master->agent on ReregisterSlave. That would force a 
> reattempt of DESTROY(V2) since current view is CR(master) = {V1} and 
> CR(agent) = {V1,V2} which will reattempt to bring the checkpointed resources 
> back in sync between master and agent.
> 
> So, I think it might be a better option to exit the agent on failure in 
> DESTROY as well. However, I think we should still update the checkpoint based 
> on the status of successful operations (other CREATE/DESTROY) on failure 
> (when agent exits) so as to avoid these operations to be repeated in a 
> subsequent CheckpointResources message. Does that sound reasonable to you?
> 
> Note: I think this use case probably is a good example to consider 
> StatusUpdates (or something similar) for operations on reserved resources, 
> viz. CREATE/DESTROY/RESERVE/UNRESERVE to ensure master and agent are in sync 
> to ensure guaranteed view of offers (to frameworks) for reserved resources.
> 
> Neil Conway wrote:
> Thanks for the writeup! Makes sense.
> 
> If we cause the agent to exit on DESTROY failure, if we only re-apply 
> DESTROYs at `ReregisterSlave`, it seems to me there is still a window in 
> which another `CREATE` can be applied at the master. That would mean we 
> wouldn't reapply the `DESTROY`, which would be bad.
> 
> My concern about updating the checkpointed resources to reflect *partial* 
> results is that (a) it seems unnecessary, (b) it means the checkpointed 
> resources don't reflect either the *original* or the *desired* state of the 
> agent, which seems problematic.
> 
> What about the following: when an agent gets a 
> `CheckpointedResourcesMessage` that differs from its current state, it first 
> writes the desired resource state to a new checkpoint file, 
> `checkpoint.target`. Then it tries to apply the delta between 
> `checkpoint.target` and the current checkpoint. If the on-disk state at the 
> agent is updated successfully to match `checkpoint.target`, we rename 
> 

Re: Review Request 46375: Updated MACHINE_UP_HELP's comments.

[Joseph Wu]

> On April 19, 2016, 1:25 p.m., Joseph Wu wrote:
> > Ship It!
> 
> Klaus Ma wrote:
> @Joseph, would you help to commit it?

I haven't finished setting up my committer's credentials yet, but once I do, 
I'll commit this :)


- Joseph


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


On April 19, 2016, 12:51 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46375/
> ---
> 
> (Updated April 19, 2016, 12:51 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated MACHINE_UP_HELP's comments.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp a9cb99a92ff5a783e719df5e5cfb6e8301241df9 
> 
> Diff: https://reviews.apache.org/r/46375/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 47660: Fixed slave switch user logic in 'getExecutorInfo'.

[Gilbert Song]

> On May 28, 2016, 7:28 p.m., Guangya Liu wrote:
> > src/slave/slave.cpp, line 3942
> > 
> >
> > Just FYI, I filed a JIRA 
> > https://issues.apache.org/jira/browse/MESOS-5476 to trace the `switch_user` 
> > documentation issue.
> > 
> > If `switch_user` is set as true, it will first try to see if the `task 
> > command` also has a user, this user value will takes precedence than user 
> > defined in `FrameworkInfo`.

Cool! thansk!


- Gilbert


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


On May 26, 2016, 1:55 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47660/
> ---
> 
> (Updated May 26, 2016, 1:55 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed slave switch user logic in 'getExecutorInfo'.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 470b5c82ea6ff01d799b06245609725853300ef1 
> 
> Diff: https://reviews.apache.org/r/47660/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48616: Add v1 changes for shared resources.

[Anindya Sinha]

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

(Updated June 13, 2016, 5:47 p.m.)


Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

Add v1 changes for shared resources.


Diffs
-

  include/mesos/v1/resources.hpp a5ba8fec4c9c3643646308f75a4b28cefe0b3df3 
  src/v1/resources.cpp 8c3f2d1c1529915a59d47fe37bb3fc7a3267079a 

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


Testing
---


Thanks,

Anindya Sinha

Re: Review Request 48639: Adjusted the namespace order for docker volume isolator test.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48639]

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

- Mesos ReviewBot


On June 13, 2016, 2:16 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48639/
> ---
> 
> (Updated June 13, 2016, 2:16 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adjusted the namespace order for docker volume isolator test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> 8d24583fef5a1f99763ece26ba55d3b389011a48 
> 
> Diff: https://reviews.apache.org/r/48639/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 48635: Fixed various typos and grammar nits in upgrade notes.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48635]

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

- Mesos ReviewBot


On June 13, 2016, 1:16 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48635/
> ---
> 
> (Updated June 13, 2016, 1:16 p.m.)
> 
> 
> Review request for mesos, Joerg Schad and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed various typos and grammar nits in upgrade notes.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md afc76f7a0e74042bd095b925f8119bfa5daa489c 
> 
> Diff: https://reviews.apache.org/r/48635/diff/
> 
> 
> Testing
> ---
> 
> Previewed in site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 48610: Renamed docker spec helper functions.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 13, 2016, 4:20 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48610/
> ---
> 
> (Updated June 13, 2016, 4:20 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed docker spec helper functions.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp b37c7731590ec144df720d7b28eeed5db76f750e 
>   src/docker/spec.cpp 7270d182b8c3a5d5c34cbda2bd736ba489089720 
>   src/tests/containerizer/docker_spec_tests.cpp 
> 944a559bb3a8ef602b4d8d52773afdb8bf8e5bf6 
>   src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 
> 
> Diff: https://reviews.apache.org/r/48610/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 46762: Enabled volume support for mesos-execute.

[Guangya Liu]

> On 六月 12, 2016, 7:21 p.m., Jie Yu wrote:
> > src/cli/execute.cpp, lines 612-613
> > 
> >
> > Hum, any reason this is MesosContainerizer only?

This patch was created before https://reviews.apache.org/r/36440/ (Enabled 
docker volume support for DockerContainerizer.), will update this part in next 
patch.


> On 六月 12, 2016, 7:21 p.m., Jie Yu wrote:
> > src/cli/execute.cpp, line 239
> > 
> >
> > Would love to support JSON::Array parsing. Right now, i think in only 
> > support JSON::Object parsing. See here:
> > 
> > https://github.com/apache/mesos/blob/master/3rdparty/stout/include/stout/flags/parse.hpp#L80

I saw that the executor command is using 
https://github.com/apache/mesos/blob/master/src/common/parse.hpp#L93 to handle 
`environment`, perhaps we can put the logic of `JSON::Array parsing` to  
https://github.com/apache/mesos/blob/master/src/common/parse.hpp ?


- Guangya


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


On 四月 28, 2016, 1:56 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46762/
> ---
> 
> (Updated 四月 28, 2016, 1:56 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5265
> https://issues.apache.org/jira/browse/MESOS-5265
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled volume support for mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 4711e80314e2fc4cde077bebd9a8167324c3254e 
> 
> Diff: https://reviews.apache.org/r/46762/diff/
> 
> 
> Testing
> ---
> 
> root@mesos002:~/src/mesos/m3/mesos/build# cat /root/test/volume4.json
>   [{
>   "container_path":"\/tmp\/abc1",
>   "mode":"RW",
>   "source":
> {
>   "docker_volume":
> {
>   "driver":"convoy",
>   "name":"dvd1"
> },
> "type":"DOCKER_VOLUME"
> }
> },
> {
>   "container_path":"\/tmp\/abc2",
>   "mode":"RW",
>   "source":
> {
>   "docker_volume":
> {
>   "driver":"convoy",
>   "driver_options":
> {"parameter":[
>   {
> "key":"iops",
> "value":"150"
>   }
> ]},
> "name":"dvd2"
>  },
>  "type":"DOCKER_VOLUME"
> }
> }]
> 
> root@mesos002:~/src/mesos/m3/mesos/build# src/mesos-execute 
> --master=192.168.56.12:5050 --name=test  --command="sleep 100" 
> --volumes=/root/test/volume4.json
> I0428 21:27:52.406168  3775 scheduler.cpp:177] Version: 0.29.0
> Subscribed with ID '2a69e042-4e51-4cd9-bcab-e303d6b4b1dd-0003'
> Submitted task 'test' to agent '2a69e042-4e51-4cd9-bcab-e303d6b4b1dd-S0'
> Received status update TASK_RUNNING for task 'test'
>   source: SOURCE_EXECUTOR
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 48617: Docs: Updated systemd documentation for agent recovery.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48617]

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

- Mesos ReviewBot


On June 13, 2016, 11:48 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48617/
> ---
> 
> (Updated June 13, 2016, 11:48 a.m.)
> 
> 
> Review request for mesos, Cody Maloney and Neil Conway.
> 
> 
> Bugs: mesos-3007
> https://issues.apache.org/jira/browse/mesos-3007
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Docs: Updated systemd documentation for agent recovery.
> 
> 
> Diffs
> -
> 
>   docs/agent-recovery.md d4f0cc0ac98e5f802bdfca0313a719fd254cea87 
> 
> Diff: https://reviews.apache.org/r/48617/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Review Request 48639: Adjusted the namespace order for docker volume isolator test.

[Guangya Liu]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Adjusted the namespace order for docker volume isolator test.


Diffs
-

  src/tests/containerizer/docker_volume_isolator_tests.cpp 
8d24583fef5a1f99763ece26ba55d3b389011a48 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Review Request 48635: Fixed various typos and grammar nits in upgrade notes.

[Neil Conway]

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

Review request for mesos, Joerg Schad and Till Toenshoff.


Repository: mesos


Description
---

Fixed various typos and grammar nits in upgrade notes.


Diffs
-

  docs/upgrades.md afc76f7a0e74042bd095b925f8119bfa5daa489c 

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


Testing
---

Previewed in site-docker.


Thanks,

Neil Conway

Re: Review Request 48619: Removed fs::aufs::supported and fs::overlay::supported.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48618, 48619]

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

- Mesos ReviewBot


On June 13, 2016, 9:33 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48619/
> ---
> 
> (Updated June 13, 2016, 9:33 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Shuai Lin.
> 
> 
> Bugs: MESOS-5607
> https://issues.apache.org/jira/browse/MESOS-5607
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed fs::aufs::supported and fs::overlay::supported.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp 55d4c8f381c97469112feae8d7dd524224aa5b89 
>   src/linux/fs.cpp 906f787e1d31173b1ab39e81f29c6fe11b64b869 
> 
> Diff: https://reviews.apache.org/r/48619/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

[Joris Van Remoortere]

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




src/common/values.cpp (lines 399 - 410)


Thanks for digging into this issue further.
Please do read my response on your e-mail thread for further context.

This patch looks like it fixes a particular bottleneck by converting to 
intervalset inline.
I understand why you did this, as my e-mail outlines that changing the 
stored data-type is more complicated.

I can see how this improves performance; however, is the resource 
validation of ranges also not a problem?

Your benchmarks are specific to this operation. Have you tried this patch 
your example 10K task / 20 node cluster to identify if the validation is still 
a bottleneck?

If we run all the benchmarks in the test suite, are all of them positively 
impacted? Or are some negatively?

If this is purely a win, then we can consider this approach as a temporary 
fix assuming the following:
- Simple patch (yes! :-D )
- A JIRA to follow up with changing the stored data-type so we don't treat 
this as a permanent solution


- Joris Van Remoortere


On June 12, 2016, 8:03 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 8:03 a.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 47374: Separated mesos test helpers into a separate library.

[Till Toenshoff]

> On June 12, 2016, 9:52 a.m., Kapil Arya wrote:
> > src/Makefile.am, lines 1950-1951
> > 
> >
> > I don't think we should be embedding absolute paths to source/build 
> > directories in a library that we are planning to install on the system. 
> > What is the rationale behind it?

My guess, getting `flags.cpp` as added to this library to compile. However, I 
believe installing this library is not something we need or want to do. And the 
point you are raising Kapil adds to that believe :).


- Till


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


On June 11, 2016, 12:03 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47374/
> ---
> 
> (Updated June 11, 2016, 12:03 a.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, Kapil Arya, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives external projects easier access to the test helpers used in
> mesos tests.  
> 
> For example, a module writer may want to write a test like 
> `src/tests/oversubscription_tests.cpp`.  To build and link against 
> this library, the module writer would mimic the build flags for tests:
> ```
> # Main test file is taken directly from Mesos.
> my_module_tests_SOURCES = \
>   $(MESOS)/src/tests/main.cpp
> 
> my_module_tests_CPPFLAGS = \
>   -I$(GMOCK)/include   \
>   -I$(GTEST)/include   \
>   -I$(MESOS)/include/mesos \
>   -I$(ZOOKEEPER)/include   \
>   -I$(ZOOKEEPER)/generated \
>   $(AM_CPPFLAGS)
>   
> my_module_tests_LDADD = \
>   $(MESOS)/3rdparty/.libs/libgmock.la \
>   $(MESOS)/src/.libs/libmesos.la  \
>   $(MESOS)/src/.libs/libmesos_tests.la
> ```
> 
> 
> Diffs
> -
> 
>   src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
> 
> Diff: https://reviews.apache.org/r/47374/diff/
> 
> 
> Testing
> ---
> 
> make check on OSX, CentOS 7, Ubuntu 14
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 48617: Docs: Updated systemd documentation for agent recovery.

[Joris Van Remoortere]

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

(Updated June 13, 2016, 11:48 a.m.)


Review request for mesos, Cody Maloney and Neil Conway.


Changes
---

Clarified some wording.


Bugs: mesos-3007
https://issues.apache.org/jira/browse/mesos-3007


Repository: mesos


Description (updated)
---

Docs: Updated systemd documentation for agent recovery.


Diffs (updated)
-

  docs/agent-recovery.md d4f0cc0ac98e5f802bdfca0313a719fd254cea87 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 48598: Added changes to upgraded.md.

[Joerg Schad]

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

(Updated June 13, 2016, 11:04 a.m.)


Review request for mesos, Greg Mann, Till Toenshoff, and Vinod Kone.


Repository: mesos


Description
---

Mesos 1.0 adds a number of changes which
will impact operators. This patch adds two
changes (see reviews 48381 and 48380) to
the upgrades.md.


Diffs (updated)
-

  docs/upgrades.md 3d31d15ea3327d1d0734b0d79be3cc86a66ab1f4 

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


Testing
---


Thanks,

Joerg Schad

Re: Review Request 48598: Added changes to upgraded.md.

[Till Toenshoff]

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


Fix it, then Ship it!





docs/upgrades.md (line 251)


Would it make sense to link the authorization documentation here?


- Till Toenshoff


On June 13, 2016, 9:50 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48598/
> ---
> 
> (Updated June 13, 2016, 9:50 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Till Toenshoff, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos 1.0 adds a number of changes which
> will impact operators. This patch adds two
> changes (see reviews 48381 and 48380) to
> the upgrades.md.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 3d31d15ea3327d1d0734b0d79be3cc86a66ab1f4 
> 
> Diff: https://reviews.apache.org/r/48598/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 48626: Fixed typo in markdown styleguide.

[Till Toenshoff]

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


Ship it!




Ship It!

- Till Toenshoff


On June 13, 2016, 9:54 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48626/
> ---
> 
> (Updated June 13, 2016, 9:54 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typo in markdown styleguide.
> 
> 
> Diffs
> -
> 
>   docs/markdown-style-guide.md 46024206876354cfb1ed2d053695ffc07996d78f 
> 
> Diff: https://reviews.apache.org/r/48626/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 48420: Added some links to the docs.

[Till Toenshoff]

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


Ship it!




Ship It!

- Till Toenshoff


On June 8, 2016, 4:58 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48420/
> ---
> 
> (Updated June 8, 2016, 4:58 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also improved consistency slightly in HTTP API doc pages.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md 7af7cd2f14dc6b324c9681bad70228972d5299f0 
>   docs/executor-http-api.md f0108d50e785ea7e075d9ef7069421bfba0420ce 
>   docs/mesos-containerizer.md 1bd278392badcf34a305900491c1fd9b30c4420f 
>   docs/monitoring.md 1464cc8dbff79ff0733ceaeb98e6fdfe0f7a6ad1 
>   docs/scheduler-http-api.md 6e62de71ff3276565b8f90512d434ee284b58685 
> 
> Diff: https://reviews.apache.org/r/48420/diff/
> 
> 
> Testing
> ---
> 
> Previewed in site-docker, checked links.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 48409: Fixed broken links in the docs.

[Till Toenshoff]

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


Ship it!




Ship It!

- Till Toenshoff


On June 8, 2016, 1:59 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48409/
> ---
> 
> (Updated June 8, 2016, 1:59 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed broken links in the docs.
> 
> 
> Diffs
> -
> 
>   docs/allocation-module.md d8164b388391d8c2d57c3b5da42348482edcff31 
>   docs/authorization.md 7af7cd2f14dc6b324c9681bad70228972d5299f0 
>   docs/documentation-guide.md ecb209cd2f5d7154a16e9b0835e3728a1146d20e 
>   docs/executor-http-api.md f0108d50e785ea7e075d9ef7069421bfba0420ce 
>   docs/external-containerizer.md a97d7933f11003708bbacb16563ff3c17ba8eec9 
>   docs/high-availability.md 2c049aba67cf3b5e62065fd707ae68588db5aaa2 
>   docs/mesos-containerizer.md 1bd278392badcf34a305900491c1fd9b30c4420f 
>   docs/network-monitoring.md dc7580d838a7ff057974285b552eb7c30f27696f 
>   docs/operational-guide.md 429c2d59821b062e73c93dd62aa2f3a09f1b1a16 
>   docs/persistent-volume.md 3faaff04e893bccc197b7cb64b6a463c7932f407 
>   docs/reservation.md bbdaf8e3dad605e1fff8a7090865d6a772eb6c92 
>   docs/scheduler-http-api.md 6e62de71ff3276565b8f90512d434ee284b58685 
>   docs/versioning.md 2211071153c5ebf3e190b05c4bc0384446e83fa8 
> 
> Diff: https://reviews.apache.org/r/48409/diff/
> 
> 
> Testing
> ---
> 
> Previewed with site-docker, checked that links work.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 48268: Implemented SET_QUOTA Call in v1 master API.

[Abhishek Dasgupta]

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

(Updated June 13, 2016, 10:47 a.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description (updated)
---

Implemented SET_QUOTA Call in v1 master API.


Diffs (updated)
-

  include/mesos/v1/quota/quota.hpp PRE-CREATION 
  src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 
  src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
  src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
  src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
  src/master/quota_handler.cpp 06cf0d3f24844ec9e884237db3f33145ff406e80 
  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---

On Ubuntu 16.04:
sudo GTEST_FILTER="*MasterAPITest.SetQuota*" make -j4 check

[==] Running 2 tests from 1 test case.
[--] Global test environment set-up.
[--] 2 tests from ContentType/MasterAPITest
[ RUN  ] ContentType/MasterAPITest.SetQuota/0
[   OK ] ContentType/MasterAPITest.SetQuota/0 (129 ms)
[ RUN  ] ContentType/MasterAPITest.SetQuota/1
[   OK ] ContentType/MasterAPITest.SetQuota/1 (98 ms)
[--] 2 tests from ContentType/MasterAPITest (227 ms total)

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


Thanks,

Abhishek Dasgupta

Re: Review Request 48617: Docs: Updated systemd documentation for agent recovery.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48617]

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

- Mesos ReviewBot


On June 13, 2016, 7:19 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48617/
> ---
> 
> (Updated June 13, 2016, 7:19 a.m.)
> 
> 
> Review request for mesos, Cody Maloney and Neil Conway.
> 
> 
> Bugs: mesos-3007
> https://issues.apache.org/jira/browse/mesos-3007
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the documentation to reflect the systemd integration work.
> 
> 
> Diffs
> -
> 
>   docs/agent-recovery.md d4f0cc0ac98e5f802bdfca0313a719fd254cea87 
> 
> Diff: https://reviews.apache.org/r/48617/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 47095: Added tests for MESOS_SANDBOX env for unified containerizer.

[Guangya Liu]

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



Yu Jie, can you please help check if we can merge this? Thanks.

- Guangya Liu


On 五月 14, 2016, 4:30 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47095/
> ---
> 
> (Updated 五月 14, 2016, 4:30 p.m.)
> 
> 
> Review request for mesos, Guangya Liu and Jie Yu.
> 
> 
> Bugs: MESOS-5312
> https://issues.apache.org/jira/browse/MESOS-5312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests for MESOS_SANDBOX env for unified containerizer.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 09742ff21513dc2570684d384b257868dd57a9ce 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 69505c922617273effc2eb52b0c567a3c01bf640 
> 
> Diff: https://reviews.apache.org/r/47095/diff/
> 
> 
> Testing
> ---
> 
> "make check" on ubuntu 14.04 64bit with gcc 4.8.4
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 48618: Refactored overlay, overlayfs and aufs check to fs::supported.

[Guangya Liu]

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

(Updated 六月 13, 2016, 10:18 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Shuai Lin.


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


Repository: mesos


Description
---

Refactored overlay, overlayfs and aufs check to fs::supported.


Diffs (updated)
-

  src/linux/fs.cpp 906f787e1d31173b1ab39e81f29c6fe11b64b869 
  src/slave/containerizer/mesos/provisioner/backend.cpp 
93a2c3acc0c783ad4977240674bf8a8fce6de7e8 
  src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
838c728b7da375ce5baa9ffc50d4996a465b0b57 
  src/tests/environment.cpp ebfacc3b98c71b375e74c4f04ce724759f0fc4f9 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Review Request 48626: Fixed typo in markdown styleguide.

[Joerg Schad]

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

Review request for mesos and Till Toenshoff.


Repository: mesos


Description
---

Fixed typo in markdown styleguide.


Diffs
-

  docs/markdown-style-guide.md 46024206876354cfb1ed2d053695ffc07996d78f 

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


Testing
---


Thanks,

Joerg Schad

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

[Neil Conway]

> On June 8, 2016, 1:28 p.m., Neil Conway wrote:
> > Overall seems like a reasonable approach.
> > 
> > One thing that isn't clear to me: what is the advantage of updating the 
> > checkpoint to reflect any partial work that was done before exiting? It 
> > seems that adds a bunch of complexity and room for error. Why not only 
> > update the checkpoint if all changes were made successfully?
> 
> Anindya Sinha wrote:
> We would need to maintain what was actually successful in any case since 
> in a DESTROY, a failed rmdir does not lead to the agent exiting. So, if we 
> were to do it at one place, we would still need to keep account of the 
> successful operations so as to not update the checkpoint based on a failed 
> rmdir as an example (and hence can be a partial update).
> 
> Since we are keeping track of result of the operations anyway, I think it 
> is a good idea to update before exiting (only place we do that when CREATE 
> fails and the agent exits) so that the subsequent handling of 
> CheckpointResources does not need to redo such operations when the agent 
> reregisters.
> 
> Neil Conway wrote:
> On reflection, I wonder whether we should be handling `CREATE` errors 
> differently from `DESTROY` errors. In both cases, the user has asked the 
> agent to do something it wasn't able to do. A failed `DESTROY` has the 
> addditional concern that we might have destroyed some but not all of the data 
> on the volume.
> 
> Do you think handling `CREATE` vs. `DESTROY` errors differently is a good 
> idea?
> 
> Anindya Sinha wrote:
> Good point. Here is what I think are the use cases:
> Say we have checkpointed resources (CR) as {V1,V2} where V1, V2 are 2 
> persistent volumes. So, CR(master) = {V1,V2}, and CR(agent) = {V1,V2}.
> If we now receive a DESTROY(V2): CR(master) = {V1} [since master's view 
> is not dependent on what happened on the agent]. Suppose that fails on the 
> agent, so CR(agent) = {V1,V2} [since we do not update checkpoint resources on 
> agent on failure in DESTROY, which results in inconsistency between master 
> and agent at this point of time].
> 
> Case 1 (current implementation): Agent does not restart on failure in 
> DESTROY. Hence, CR(agent) = {V1,V2}. When the next CheckpointResources is 
> received, ie. on a subsequent CREATE/DESTROY/RESERVE/UNRESERVE on a different 
> resource, DESTROY(V2) will be reattempted and if that is successful, we will 
> in sync between agent and master. However if the next CheckpointResources is 
> due to a CREATE(V2) [that can happen since V2 is available as a resource 
> based on offer from master], that would be a no-op on agent since agent does 
> not treat it as a new resource based on the checkpoint since at this point 
> CR(master) = {V1,V2}, and CR(agent) = {V1,V2}, which would be a problem.
> 
> Case 2 (if we exit agent on failure): The agent restarts which triggers a 
> CheckpointResources from master->agent on ReregisterSlave. That would force a 
> reattempt of DESTROY(V2) since current view is CR(master) = {V1} and 
> CR(agent) = {V1,V2} which will reattempt to bring the checkpointed resources 
> back in sync between master and agent.
> 
> So, I think it might be a better option to exit the agent on failure in 
> DESTROY as well. However, I think we should still update the checkpoint based 
> on the status of successful operations (other CREATE/DESTROY) on failure 
> (when agent exits) so as to avoid these operations to be repeated in a 
> subsequent CheckpointResources message. Does that sound reasonable to you?
> 
> Note: I think this use case probably is a good example to consider 
> StatusUpdates (or something similar) for operations on reserved resources, 
> viz. CREATE/DESTROY/RESERVE/UNRESERVE to ensure master and agent are in sync 
> to ensure guaranteed view of offers (to frameworks) for reserved resources.

Thanks for the writeup! Makes sense.

If we cause the agent to exit on DESTROY failure, if we only re-apply DESTROYs 
at `ReregisterSlave`, it seems to me there is still a window in which another 
`CREATE` can be applied at the master. That would mean we wouldn't reapply the 
`DESTROY`, which would be bad.

My concern about updating the checkpointed resources to reflect *partial* 
results is that (a) it seems unnecessary, (b) it means the checkpointed 
resources don't reflect either the *original* or the *desired* state of the 
agent, which seems problematic.

What about the following: when an agent gets a `CheckpointedResourcesMessage` 
that differs from its current state, it first writes the desired resource state 
to a new checkpoint file, `checkpoint.target`. Then it tries to apply the delta 
between `checkpoint.target` and the current checkpoint. If the on-disk state at 
the agent is updated successfully to match `checkpoint.target`, we rename 
`checkpoint.target` -> `checkpoint` and we're done. Otherwise, if any I/O 
operation fails, we exit the 

Re: Review Request 48598: Added changes to upgraded.md.

[Joerg Schad]

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

(Updated June 13, 2016, 9:50 a.m.)


Review request for mesos, Greg Mann, Till Toenshoff, and Vinod Kone.


Summary (updated)
-

Added changes to upgraded.md.


Repository: mesos


Description
---

Mesos 1.0 adds a number of changes which
will impact operators. This patch adds two
changes (see reviews 48381 and 48380) to
the upgrades.md.


Diffs (updated)
-

  docs/upgrades.md 3d31d15ea3327d1d0734b0d79be3cc86a66ab1f4 

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


Testing
---


Thanks,

Joerg Schad

Re: Review Request 47396: Added aufs provisioning backend.

[Guangya Liu]

> On 六月 7, 2016, 2:09 a.m., Guangya Liu wrote:
> > src/tests/environment.cpp, lines 492-495
> > 
> >
> > I think that here also needs to be updated to use your new function 
> > `fs::aufs::supported()` to check `aufs`?
> 
> Jie Yu wrote:
> Hum... this looks weird to me. I am wondering if we can make 
> `fs::supported` understand the sutble details related to overlayfs? Can you 
> guys follow up with a patch. Basically, here, it should be `Try check = 
> fs::supported(fsname);` and inside `fs::supported`, we check special case for 
> overlayfs.
> 
> Guangya Liu wrote:
> I'm now thinking that we may not need the API of `fs::aufs::supported`, 
> the reason that we introduced `fs::overlay::supported` is becuase of keyword 
> issue, the overlay fs keyword in `/proc/filesystems` can be `overlay` or 
> `overlayfs`.
> 
> But for `aufs`, I think we do not need to introduce the new API of 
> `fs::aufs::supported` but call `fs::supported` directly, as the API of 
> `fs::aufs::supported` is just a wrapper of `fs::supported`. Comments?
> 
> Jie Yu wrote:
> Yeah, I agree with you Guangya. I think we should make 
> `fs::supported(fsname)` understand both overlayfs and overlay and get rid of 
> all fs::xxx::supported logics. Do you have time to follow up with a patch?
> 
> Guangya Liu wrote:
> Hi Yu Jie, it may be difficult to make `fs::supported(fsname)` to 
> understand both overlayfs and overlay, so my thinking is that we only keep 
> `fs::overlay::supported` but remove the `fs::aufs::supported`, comments?
> 
> Jie Yu wrote:
> Why it's difficult to make fs::supported(fsname) to understand both 
> overlayfs and overlay?
> 
> Guangya Liu wrote:
> My first thinking is as https://reviews.apache.org/r/44421/diff/1/
> 
> But considering this will change the parameter of `fs::supported`, so we 
> decide to add `fs::overlay::supported`.  Comments?
> 
> Jie Yu wrote:
> Let's not change the parameter of fs::supported, instead, change the 
> logic inside fs::supported so that it handles overlay/overlayfs well.
> 
> Guangya Liu wrote:
> Got it, then the `fs::supported` may have some special logic for 
> `overlay` and `overlayfs`, I will post a patch later for this.

I posted two patches here

https://reviews.apache.org/r/48618/ Refactored overlay, overlayfs and aufs 
check to fs::supported.
https://reviews.apache.org/r/48619/ Removed fs::aufs::supported and 
fs::overlay::supported.


- Guangya


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


On 六月 6, 2016, 11:57 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47396/
> ---
> 
> (Updated 六月 6, 2016, 11:57 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, and Jie Yu.
> 
> 
> Bugs: MESOS-4672
> https://issues.apache.org/jira/browse/MESOS-4672
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added aufs provisioning backend.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a08ea407d631f6ae56ac36b122bfdf0e849e8b56 
>   src/slave/containerizer/mesos/provisioner/backend.cpp 
> b2a20b7c08fa790da09ba05b725248e42b8d3bc4 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> bc04760c9504ea55cd4bb72c7e9012e43a5911aa 
>   src/tests/environment.cpp 849e9ce05864f6ec1a736dfc1a7a31d2364c84bf 
> 
> Diff: https://reviews.apache.org/r/47396/diff/
> 
> 
> Testing
> ---
> 
> - Make check on ubuntu 14.04 64bit
> - Test manually: start slave with aufs backend, and write a simple script 
> that launches tasks with alpine/ubuntu images
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>