Re: Review Request 46008: Stout: Initialize Windows socket stack in Stout tests.

[Alex Clemmer]

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

(Updated April 14, 2016, 5:21 a.m.)


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


Repository: mesos


Description
---

Stout: Initialize Windows socket stack in Stout tests.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/socket.hpp 
b5aeab75c2365a3431c397743b95ce7fbd6d7a1a 
  3rdparty/libprocess/3rdparty/stout/tests/main.cpp 
c449de87bede4d3bf1df368eaf1d22f857c2298f 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

[Qian Zhang]

> On April 13, 2016, 4:24 a.m., Vinod Kone wrote:
> > src/launcher/http_command_executor.cpp, lines 804-809
> > 
> >
> > Lets fix this hack now that the executor receives acknowledgements for 
> > status updates.
> > 
> > The executor can terminate after it receives an ACK for a terminal 
> > status update. Can you add a TODO here and fix it in a separate review?
> 
> Qian Zhang wrote:
> Agree, will post a separate patch for it soon.
> 
> Qian Zhang wrote:
> But what if framework does not cooperate? E.g., it does not send ACK when 
> it receives a terminal status update from executor. Then the executor will 
> never terminate? I think that may not be what we want ...

Posted the patch: https://reviews.apache.org/r/46187/


- Qian


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


On April 13, 2016, 5:13 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44424/
> ---
> 
> (Updated April 13, 2016, 5:13 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3558
> https://issues.apache.org/jira/browse/MESOS-3558
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated http_command_executor.cpp to use v1 API.
> 
> 
> Diffs
> -
> 
>   src/launcher/http_command_executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44424/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 46187: Terminate when receiving the ACK of terminal status update.

[Qian Zhang]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Terminate when receiving the ACK of terminal status update.


Diffs
-

  src/launcher/http_command_executor.cpp 
ad484e0e6f5067b6c166111c91b1ff1e8c05d9ac 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 46008: Stout: Initialize Windows socket stack in Stout tests.

[Alex Clemmer]

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

(Updated April 14, 2016, 5:05 a.m.)


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


Repository: mesos


Description
---

Stout: Initialize Windows socket stack in Stout tests.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/socket.hpp 
b5aeab75c2365a3431c397743b95ce7fbd6d7a1a 
  3rdparty/libprocess/3rdparty/stout/tests/main.cpp 
c449de87bede4d3bf1df368eaf1d22f857c2298f 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 45806: Replace Master/Slave Terminology Phase I - Duplicate/Rename binaries.

[zhou xing]

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

(Updated 四月 14, 2016, 5:01 a.m.)


Review request for mesos, Kevin Klues and Vinod Kone.


Changes
---

just try to re-kick review bot


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


Repository: mesos


Description
---

[#mesos-3782]
In this patch, we did the following changes:
1. Duplicate executable file 'mesos-slave' with 'mesos-agent'
2. Rename the scripts in folder 'mesos/bin' & 'mesos/src/deploy'
to use term 'agent' instead of term 'slave'
3. Change the content of ths scripts to use term 'agent' instead
of term 'slave'
4. Duplicate the renamed scripts renamed in #2 during the configure
and make process, the duplicated scripts still use term 'slave'
in their names.

Please note that afull bootstrap is required once this patch has
been applied. Otherwise, configure will break because of the
missing mesos-slave-*.sh scripts. Please follow the steps to
re-build the project:

```
cd mesos
./bootstrap
mkdir build
cd build
../configure --prefix=${HOME}/install/mesos
make
```


Diffs (updated)
-

  CHANGELOG 1e07c8c2de8eff87c171378ef207c91a20d435d9 
  bin/gdb-mesos-slave.sh.in dbeec8464b26bd808f7a50b8412a2778f1458f22 
  bin/lldb-mesos-slave.sh.in 896c411b2b05d3c4a14288002520a5391a88d955 
  bin/mesos-local-flags.sh.in ab5b6c8bd8847485c5a47d637c9f4fe88c59ae65 
  bin/mesos-slave-flags.sh.in  
  bin/mesos-slave.sh.in 1e3b748ed4dc32ba6bd8adece20f439bce38abc3 
  bin/valgrind-mesos-slave.sh.in 900c5883d96cf14e121e566bcf1ad4dc9a47abf6 
  configure.ac 4392909873e588b93e435d260276b0f1b0814c08 
  src/Makefile.am 139935fb40f8986427c85537a400ff945e54433f 
  src/deploy/mesos-deploy-env.sh.template 
bea5584fbcc68825b1c35b370aed17b0e432edd5 
  src/deploy/mesos-slave-env.sh.template 
31567d6a47e19385aed56edfc7e67457c8cdde3e 
  src/deploy/mesos-start-cluster.sh.in f7a003d9a8e5bbb4f11353988e55e715da0b2b4f 
  src/deploy/mesos-start-slaves.sh.in 50860f40e33fcbb1787be6c035873de4bcc83de5 
  src/deploy/mesos-stop-cluster.sh.in e5f8c1fb400c56715774889632aa74d9eac33645 
  src/deploy/mesos-stop-slaves.sh.in 3dd9b51edff2beb3ccc8d5dd44f0cdc265f623f9 
  src/slave/containerizer/mesos/launch.cpp 
54079c37143f4b5c22e0c9a8fe4bacb31f60ed1b 
  src/tests/balloon_framework_test.sh ae32753e8942f77f94752543c384d218d6e4d48d 
  support/generate-endpoint-help.py 5d23b10f0bab9c0e5848c0c1c26855522cf44a70 
  support/test-upgrade.py 2c4061d71338f66e432dfa4ac86a9693f3ad38bf 

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


Testing
---

cd mesos
./bootstrap
mkdir build
cd build
../configure --prefix=${HOME}/install/mesos
make
make check


Thanks,

zhou xing

Re: Review Request 46134: Replace Master/Slave Terminology Phase I - Duplicate/Rename binaries(2).

[zhou xing]

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

(Updated 四月 14, 2016, 5 a.m.)


Review request for mesos, Kevin Klues and Vinod Kone.


Changes
---

just try to re-kick review bot


Repository: mesos


Description
---

[#mesos-3782]
In this patch, we did the following change:
1. Change the libprocess cmake target to use 'mesos-agent'
instead of 'mesos-slave'


Diffs (updated)
-

  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
7b53a49baddc8b65f9c5b4b8fe5a53b6883b3652 

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


Testing
---

make
make check


Thanks,

zhou xing

Re: Review Request 45326: Added agent flag dvdcli_dir for specify dvdcli path.

[Guangya Liu]

> On 四月 13, 2016, 9:01 p.m., James DeFelice wrote:
> > src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp, line 64
> > 
> >
> > should we verify that the dvdcli binary actually exists here (and if 
> > not generate an error)?

Yes, will fix this in https://reviews.apache.org/r/46180


- Guangya


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


On 四月 14, 2016, 4:57 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45326/
> ---
> 
> (Updated 四月 14, 2016, 4:57 a.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5084
> https://issues.apache.org/jira/browse/MESOS-5084
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flag dvdcli_dir for specify dvdcli path.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md ce51f26f0584eeecb5c96e39a5bcc21f5906a29c 
>   src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a 
>   src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86 
> 
> Diff: https://reviews.apache.org/r/45326/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46134: Replace Master/Slave Terminology Phase I - Duplicate/Rename binaries(2).

[Kevin Klues]

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


Ship it!




It looks like the ReviewBot ran between the two most recent updates, so it 
failed on the uninstall (which is now fixed).  Could you push this review one 
more time to trigger the ReviewBot again to verify that it actually works? (I 
tested locally and it seems to).

- Kevin Klues


On April 14, 2016, 4:24 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46134/
> ---
> 
> (Updated April 14, 2016, 4:24 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [#mesos-3782]
> In this patch, we did the following change:
> 1. Change the libprocess cmake target to use 'mesos-agent'
> instead of 'mesos-slave'
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> 7b53a49baddc8b65f9c5b4b8fe5a53b6883b3652 
> 
> Diff: https://reviews.apache.org/r/46134/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 45806: Replace Master/Slave Terminology Phase I - Duplicate/Rename binaries.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On April 14, 2016, 4:23 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45806/
> ---
> 
> (Updated April 14, 2016, 4:23 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: MESOS-3782
> https://issues.apache.org/jira/browse/MESOS-3782
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [#mesos-3782]
> In this patch, we did the following changes:
> 1. Duplicate executable file 'mesos-slave' with 'mesos-agent'
> 2. Rename the scripts in folder 'mesos/bin' & 'mesos/src/deploy'
> to use term 'agent' instead of term 'slave'
> 3. Change the content of ths scripts to use term 'agent' instead
> of term 'slave'
> 4. Duplicate the renamed scripts renamed in #2 during the configure
> and make process, the duplicated scripts still use term 'slave'
> in their names.
> 
> Please note that afull bootstrap is required once this patch has
> been applied. Otherwise, configure will break because of the
> missing mesos-slave-*.sh scripts. Please follow the steps to
> re-build the project:
> 
> ```
> cd mesos
> ./bootstrap
> mkdir build
> cd build
> ../configure --prefix=${HOME}/install/mesos
> make
> ```
> 
> 
> Diffs
> -
> 
>   CHANGELOG 1e07c8c2de8eff87c171378ef207c91a20d435d9 
>   bin/gdb-mesos-slave.sh.in dbeec8464b26bd808f7a50b8412a2778f1458f22 
>   bin/lldb-mesos-slave.sh.in 896c411b2b05d3c4a14288002520a5391a88d955 
>   bin/mesos-local-flags.sh.in ab5b6c8bd8847485c5a47d637c9f4fe88c59ae65 
>   bin/mesos-slave-flags.sh.in  
>   bin/mesos-slave.sh.in 1e3b748ed4dc32ba6bd8adece20f439bce38abc3 
>   bin/valgrind-mesos-slave.sh.in 900c5883d96cf14e121e566bcf1ad4dc9a47abf6 
>   configure.ac 4392909873e588b93e435d260276b0f1b0814c08 
>   src/Makefile.am 139935fb40f8986427c85537a400ff945e54433f 
>   src/deploy/mesos-deploy-env.sh.template 
> bea5584fbcc68825b1c35b370aed17b0e432edd5 
>   src/deploy/mesos-slave-env.sh.template 
> 31567d6a47e19385aed56edfc7e67457c8cdde3e 
>   src/deploy/mesos-start-cluster.sh.in 
> f7a003d9a8e5bbb4f11353988e55e715da0b2b4f 
>   src/deploy/mesos-start-slaves.sh.in 
> 50860f40e33fcbb1787be6c035873de4bcc83de5 
>   src/deploy/mesos-stop-cluster.sh.in 
> e5f8c1fb400c56715774889632aa74d9eac33645 
>   src/deploy/mesos-stop-slaves.sh.in 3dd9b51edff2beb3ccc8d5dd44f0cdc265f623f9 
>   src/slave/containerizer/mesos/launch.cpp 
> 54079c37143f4b5c22e0c9a8fe4bacb31f60ed1b 
>   src/tests/balloon_framework_test.sh 
> ae32753e8942f77f94752543c384d218d6e4d48d 
>   support/generate-endpoint-help.py 5d23b10f0bab9c0e5848c0c1c26855522cf44a70 
>   support/test-upgrade.py 2c4061d71338f66e432dfa4ac86a9693f3ad38bf 
> 
> Diff: https://reviews.apache.org/r/45806/diff/
> 
> 
> Testing
> ---
> 
> cd mesos
> ./bootstrap
> mkdir build
> cd build
> ../configure --prefix=${HOME}/install/mesos
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 45806: Replace Master/Slave Terminology Phase I - Duplicate/Rename binaries.

[zhou xing]

> On 四月 14, 2016, 4:04 a.m., Kevin Klues wrote:
> > src/Makefile.am, lines 2137-2141
> > 
> >
> > Assuming this is the right fix, you probably need DESTDIR here too

Fixed in the latest patch, is there any way to re-kick the robot check?


- zhou


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


On 四月 14, 2016, 4:23 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45806/
> ---
> 
> (Updated 四月 14, 2016, 4:23 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: MESOS-3782
> https://issues.apache.org/jira/browse/MESOS-3782
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [#mesos-3782]
> In this patch, we did the following changes:
> 1. Duplicate executable file 'mesos-slave' with 'mesos-agent'
> 2. Rename the scripts in folder 'mesos/bin' & 'mesos/src/deploy'
> to use term 'agent' instead of term 'slave'
> 3. Change the content of ths scripts to use term 'agent' instead
> of term 'slave'
> 4. Duplicate the renamed scripts renamed in #2 during the configure
> and make process, the duplicated scripts still use term 'slave'
> in their names.
> 
> Please note that afull bootstrap is required once this patch has
> been applied. Otherwise, configure will break because of the
> missing mesos-slave-*.sh scripts. Please follow the steps to
> re-build the project:
> 
> ```
> cd mesos
> ./bootstrap
> mkdir build
> cd build
> ../configure --prefix=${HOME}/install/mesos
> make
> ```
> 
> 
> Diffs
> -
> 
>   CHANGELOG 1e07c8c2de8eff87c171378ef207c91a20d435d9 
>   bin/gdb-mesos-slave.sh.in dbeec8464b26bd808f7a50b8412a2778f1458f22 
>   bin/lldb-mesos-slave.sh.in 896c411b2b05d3c4a14288002520a5391a88d955 
>   bin/mesos-local-flags.sh.in ab5b6c8bd8847485c5a47d637c9f4fe88c59ae65 
>   bin/mesos-slave-flags.sh.in  
>   bin/mesos-slave.sh.in 1e3b748ed4dc32ba6bd8adece20f439bce38abc3 
>   bin/valgrind-mesos-slave.sh.in 900c5883d96cf14e121e566bcf1ad4dc9a47abf6 
>   configure.ac 4392909873e588b93e435d260276b0f1b0814c08 
>   src/Makefile.am 139935fb40f8986427c85537a400ff945e54433f 
>   src/deploy/mesos-deploy-env.sh.template 
> bea5584fbcc68825b1c35b370aed17b0e432edd5 
>   src/deploy/mesos-slave-env.sh.template 
> 31567d6a47e19385aed56edfc7e67457c8cdde3e 
>   src/deploy/mesos-start-cluster.sh.in 
> f7a003d9a8e5bbb4f11353988e55e715da0b2b4f 
>   src/deploy/mesos-start-slaves.sh.in 
> 50860f40e33fcbb1787be6c035873de4bcc83de5 
>   src/deploy/mesos-stop-cluster.sh.in 
> e5f8c1fb400c56715774889632aa74d9eac33645 
>   src/deploy/mesos-stop-slaves.sh.in 3dd9b51edff2beb3ccc8d5dd44f0cdc265f623f9 
>   src/slave/containerizer/mesos/launch.cpp 
> 54079c37143f4b5c22e0c9a8fe4bacb31f60ed1b 
>   src/tests/balloon_framework_test.sh 
> ae32753e8942f77f94752543c384d218d6e4d48d 
>   support/generate-endpoint-help.py 5d23b10f0bab9c0e5848c0c1c26855522cf44a70 
>   support/test-upgrade.py 2c4061d71338f66e432dfa4ac86a9693f3ad38bf 
> 
> Diff: https://reviews.apache.org/r/45806/diff/
> 
> 
> Testing
> ---
> 
> cd mesos
> ./bootstrap
> mkdir build
> cd build
> ../configure --prefix=${HOME}/install/mesos
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 45360: Added volume client for mount and unmount.

[Guangya Liu]

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

(Updated 四月 14, 2016, 4:45 a.m.)


Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
Jie Yu.


Repository: mesos


Description
---

Added volume client for mount and unmount.


Diffs (updated)
-

  src/CMakeLists.txt ca59a1824352423f4db6ef8bb41acc6fe602c041 
  src/Makefile.am 139935fb40f8986427c85537a400ff945e54433f 
  src/slave/containerizer/mesos/isolators/docker/volume/volume_client.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/volume/volume_client.cpp 
PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 45500: Keep fs::mount information should be consolidated.

[Deshi Xiao]

> On 四月 6, 2016, 9:04 p.m., Neil Conway wrote:
> > src/tests/containerizer/fs_tests.cpp, line 35
> > 
> >
> > Not yours, but we should sort these alphabetically.

@Neil Conway 

how aobut your feedback.


- Deshi


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


On 四月 12, 2016, 11:49 a.m., Deshi Xiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45500/
> ---
> 
> (Updated 四月 12, 2016, 11:49 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Cong Wang.
> 
> 
> Bugs: MESOS-1104
> https://issues.apache.org/jira/browse/MESOS-1104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> keep fs::mount information should be consolidated and moved to stout,
> along with mount and umount.
> 
> Refers to Benjamin Hindman's TODO:
> https://github.com/apache/mesos/blob/0.22.1/src/linux/cgroups.cpp#L64
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp b57ec05d3e0bf0bc1bf50fca9a9ede767f204253 
>   src/linux/fs.hpp 5951f3f289552d65bc854074135f5177c052caf1 
>   src/linux/fs.cpp 2087b4ac1503e0fd085319b1017389f1f947536f 
>   src/slave/containerizer/mesos/mount.cpp 
> bf17b015e45579882fd31248e8609eec6d58a9da 
>   src/tests/containerizer/fs_tests.cpp 
> 020fd8a4bf5911671e038a96f8b50c0f58e22ed5 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 21ad1e1c53316a3bb6d914aa228ccf3658acdfbf 
> 
> Diff: https://reviews.apache.org/r/45500/diff/
> 
> 
> Testing
> ---
> 
> make test on localhost(ubuntu)
> 
> 
> Thanks,
> 
> Deshi Xiao
> 
>

Re: Review Request 45270: Added spec protobut for external mount.

[Guangya Liu]

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

(Updated 四月 14, 2016, 4:41 a.m.)


Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
Jie Yu.


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


Repository: mesos


Description
---

Added spec protobut for external mount.


Diffs (updated)
-

  src/CMakeLists.txt ca59a1824352423f4db6ef8bb41acc6fe602c041 
  src/Makefile.am 139935fb40f8986427c85537a400ff945e54433f 
  src/slave/containerizer/mesos/isolators/docker/volume/spec.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/volume/spec.proto PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 45214: Updated protobuf to support external storage.

[Guangya Liu]

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

(Updated 四月 14, 2016, 4:37 a.m.)


Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
Jie Yu.


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


Repository: mesos


Description
---

Updated protobuf to support external storage.


Diffs (updated)
-

  include/mesos/mesos.proto 1232bcda053db3451d18413e81e54e51774f7f22 
  include/mesos/v1/mesos.proto 1048fae62733926b5e863a0c1629bba69e35e43b 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 46134: Replace Master/Slave Terminology Phase I - Duplicate/Rename binaries(2).

[zhou xing]

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

(Updated 四月 14, 2016, 4:24 a.m.)


Review request for mesos, Kevin Klues and Vinod Kone.


Repository: mesos


Description
---

[#mesos-3782]
In this patch, we did the following change:
1. Change the libprocess cmake target to use 'mesos-agent'
instead of 'mesos-slave'


Diffs (updated)
-

  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
7b53a49baddc8b65f9c5b4b8fe5a53b6883b3652 

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


Testing
---

make
make check


Thanks,

zhou xing

Re: Review Request 45806: Replace Master/Slave Terminology Phase I - Duplicate/Rename binaries.

[zhou xing]

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

(Updated 四月 14, 2016, 4:23 a.m.)


Review request for mesos, Kevin Klues and Vinod Kone.


Changes
---

add $(DESTDIR) to uninstall hook


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


Repository: mesos


Description
---

[#mesos-3782]
In this patch, we did the following changes:
1. Duplicate executable file 'mesos-slave' with 'mesos-agent'
2. Rename the scripts in folder 'mesos/bin' & 'mesos/src/deploy'
to use term 'agent' instead of term 'slave'
3. Change the content of ths scripts to use term 'agent' instead
of term 'slave'
4. Duplicate the renamed scripts renamed in #2 during the configure
and make process, the duplicated scripts still use term 'slave'
in their names.

Please note that afull bootstrap is required once this patch has
been applied. Otherwise, configure will break because of the
missing mesos-slave-*.sh scripts. Please follow the steps to
re-build the project:

```
cd mesos
./bootstrap
mkdir build
cd build
../configure --prefix=${HOME}/install/mesos
make
```


Diffs (updated)
-

  CHANGELOG 1e07c8c2de8eff87c171378ef207c91a20d435d9 
  bin/gdb-mesos-slave.sh.in dbeec8464b26bd808f7a50b8412a2778f1458f22 
  bin/lldb-mesos-slave.sh.in 896c411b2b05d3c4a14288002520a5391a88d955 
  bin/mesos-local-flags.sh.in ab5b6c8bd8847485c5a47d637c9f4fe88c59ae65 
  bin/mesos-slave-flags.sh.in  
  bin/mesos-slave.sh.in 1e3b748ed4dc32ba6bd8adece20f439bce38abc3 
  bin/valgrind-mesos-slave.sh.in 900c5883d96cf14e121e566bcf1ad4dc9a47abf6 
  configure.ac 4392909873e588b93e435d260276b0f1b0814c08 
  src/Makefile.am 139935fb40f8986427c85537a400ff945e54433f 
  src/deploy/mesos-deploy-env.sh.template 
bea5584fbcc68825b1c35b370aed17b0e432edd5 
  src/deploy/mesos-slave-env.sh.template 
31567d6a47e19385aed56edfc7e67457c8cdde3e 
  src/deploy/mesos-start-cluster.sh.in f7a003d9a8e5bbb4f11353988e55e715da0b2b4f 
  src/deploy/mesos-start-slaves.sh.in 50860f40e33fcbb1787be6c035873de4bcc83de5 
  src/deploy/mesos-stop-cluster.sh.in e5f8c1fb400c56715774889632aa74d9eac33645 
  src/deploy/mesos-stop-slaves.sh.in 3dd9b51edff2beb3ccc8d5dd44f0cdc265f623f9 
  src/slave/containerizer/mesos/launch.cpp 
54079c37143f4b5c22e0c9a8fe4bacb31f60ed1b 
  src/tests/balloon_framework_test.sh ae32753e8942f77f94752543c384d218d6e4d48d 
  support/generate-endpoint-help.py 5d23b10f0bab9c0e5848c0c1c26855522cf44a70 
  support/test-upgrade.py 2c4061d71338f66e432dfa4ac86a9693f3ad38bf 

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


Testing
---

cd mesos
./bootstrap
mkdir build
cd build
../configure --prefix=${HOME}/install/mesos
make
make check


Thanks,

zhou xing

Re: Review Request 45806: Replace Master/Slave Terminology Phase I - Duplicate/Rename binaries.

[Kevin Klues]

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




src/Makefile.am (lines 2135 - 2139)


Assuming this is the right fix, you probably need DESTDIR here too


- Kevin Klues


On April 14, 2016, 3:23 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45806/
> ---
> 
> (Updated April 14, 2016, 3:23 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: MESOS-3782
> https://issues.apache.org/jira/browse/MESOS-3782
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [#mesos-3782]
> In this patch, we did the following changes:
> 1. Duplicate executable file 'mesos-slave' with 'mesos-agent'
> 2. Rename the scripts in folder 'mesos/bin' & 'mesos/src/deploy'
> to use term 'agent' instead of term 'slave'
> 3. Change the content of ths scripts to use term 'agent' instead
> of term 'slave'
> 4. Duplicate the renamed scripts renamed in #2 during the configure
> and make process, the duplicated scripts still use term 'slave'
> in their names.
> 
> Please note that afull bootstrap is required once this patch has
> been applied. Otherwise, configure will break because of the
> missing mesos-slave-*.sh scripts. Please follow the steps to
> re-build the project:
> 
> ```
> cd mesos
> ./bootstrap
> mkdir build
> cd build
> ../configure --prefix=${HOME}/install/mesos
> make
> ```
> 
> 
> Diffs
> -
> 
>   CHANGELOG 1e07c8c2de8eff87c171378ef207c91a20d435d9 
>   bin/gdb-mesos-slave.sh.in dbeec8464b26bd808f7a50b8412a2778f1458f22 
>   bin/lldb-mesos-slave.sh.in 896c411b2b05d3c4a14288002520a5391a88d955 
>   bin/mesos-local-flags.sh.in ab5b6c8bd8847485c5a47d637c9f4fe88c59ae65 
>   bin/mesos-slave-flags.sh.in  
>   bin/mesos-slave.sh.in 1e3b748ed4dc32ba6bd8adece20f439bce38abc3 
>   bin/valgrind-mesos-slave.sh.in 900c5883d96cf14e121e566bcf1ad4dc9a47abf6 
>   configure.ac 4392909873e588b93e435d260276b0f1b0814c08 
>   src/Makefile.am 139935fb40f8986427c85537a400ff945e54433f 
>   src/deploy/mesos-deploy-env.sh.template 
> bea5584fbcc68825b1c35b370aed17b0e432edd5 
>   src/deploy/mesos-slave-env.sh.template 
> 31567d6a47e19385aed56edfc7e67457c8cdde3e 
>   src/deploy/mesos-start-cluster.sh.in 
> f7a003d9a8e5bbb4f11353988e55e715da0b2b4f 
>   src/deploy/mesos-start-slaves.sh.in 
> 50860f40e33fcbb1787be6c035873de4bcc83de5 
>   src/deploy/mesos-stop-cluster.sh.in 
> e5f8c1fb400c56715774889632aa74d9eac33645 
>   src/deploy/mesos-stop-slaves.sh.in 3dd9b51edff2beb3ccc8d5dd44f0cdc265f623f9 
>   src/slave/containerizer/mesos/launch.cpp 
> 54079c37143f4b5c22e0c9a8fe4bacb31f60ed1b 
>   src/tests/balloon_framework_test.sh 
> ae32753e8942f77f94752543c384d218d6e4d48d 
>   support/generate-endpoint-help.py 5d23b10f0bab9c0e5848c0c1c26855522cf44a70 
>   support/test-upgrade.py 2c4061d71338f66e432dfa4ac86a9693f3ad38bf 
> 
> Diff: https://reviews.apache.org/r/45806/diff/
> 
> 
> Testing
> ---
> 
> cd mesos
> ./bootstrap
> mkdir build
> cd build
> ../configure --prefix=${HOME}/install/mesos
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 45214: Updated protobuf to support external storage.

[Guangya Liu]

> On 四月 13, 2016, 9:35 p.m., James DeFelice wrote:
> > include/mesos/mesos.proto, line 1625
> > 
> >
> > I definitely prefer this form, but there's also a Parameters message 
> > type. Which is more idiomatic in this context?
> 
> Guangya Liu wrote:
> I saw that no one in protobuf is using this, and the `Parameters` is just 
> a wrapper of `repeated Parameter parameter`, I think it is OK to use 
> `repeated Parameter parameter` here, comments?

Thanks James, I did some test and found that `Parameters` is more easy to use, 
will update this patch soon.


- Guangya


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


On 四月 13, 2016, 3:09 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45214/
> ---
> 
> (Updated 四月 13, 2016, 3:09 a.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5012
> https://issues.apache.org/jira/browse/MESOS-5012
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated protobuf to support external storage.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 1232bcda053db3451d18413e81e54e51774f7f22 
>   include/mesos/v1/mesos.proto 1048fae62733926b5e863a0c1629bba69e35e43b 
> 
> Diff: https://reviews.apache.org/r/45214/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46134: Replace Master/Slave Terminology Phase I - Duplicate/Rename binaries(2).

[zhou xing]

> On 四月 13, 2016, 11:25 p.m., Vinod Kone wrote:
> > Looks like ReviewBot failed. Can you make sure to fix that issue? Otherwise 
> > LGTM.
> 
> Kevin Klues wrote:
> It looks like it's failing on this step:
> ```
> make[5]: Entering directory `/mesos/mesos-0.29.0/_build/src'
> cp /mesos/mesos-0.29.0/_inst/etc/mesos/mesos-agent-env.sh.template \
> /mesos/mesos-0.29.0/_inst/etc/mesos/mesos-slave-env.sh.template
> cp: cannot stat 
> '/mesos/mesos-0.29.0/_inst/etc/mesos/mesos-agent-env.sh.template': No such 
> file or directory
> make[5]: *** [copy-agent-env-template] Error 1
> ```
> 
> which is something added in https://reviews.apache.org/r/45806/
> 
> The rule is written as:
> ```
> copy-agent-env-template:
>   cp $(pkgsysconfdir)/mesos-agent-env.sh.template \
> $(pkgsysconfdir)/mesos-slave-env.sh.template
> ```
> 
> so somehow during this installation, `mesos-agent-env.sh.template` didn't 
> get installed correctly before this rule ran. Looking a few lines above this 
> error, we see that it actually got installed to 
> `'/tmp/am-dc-13087//mesos/mesos-0.29.0/_inst/etc/mesos`:
> ```
> /usr/bin/install -c -m 644 ../../src/deploy/mesos-agent-env.sh.template 
> ../../src/deploy/mesos-deploy-env.sh.template 
> ../../src/deploy/mesos-master-env.sh.template 
> '/tmp/am-dc-13087//mesos/mesos-0.29.0/_inst/etc/mesos'.
> ```
> 
> Which I'm guessing is `$(DESTDIR)/$(pkgsysconfdir)`.  Looks like we need 
> to update the rule for `copy-agent-env-template` accordingly.

Kevin, you are correct, I added $(DESTDIR) in Makefile.am and tried locally, it 
worked. have submitted the updated patch to see if this can make CI happy, 
thanks


- zhou


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


On 四月 14, 2016, 3:24 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46134/
> ---
> 
> (Updated 四月 14, 2016, 3:24 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [#mesos-3782]
> In this patch, we did the following change:
> 1. Change the libprocess cmake target to use 'mesos-agent'
> instead of 'mesos-slave'
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> 7b53a49baddc8b65f9c5b4b8fe5a53b6883b3652 
> 
> Diff: https://reviews.apache.org/r/46134/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 46134: Replace Master/Slave Terminology Phase I - Duplicate/Rename binaries(2).

[zhou xing]

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

(Updated 四月 14, 2016, 3:24 a.m.)


Review request for mesos, Kevin Klues and Vinod Kone.


Repository: mesos


Description
---

[#mesos-3782]
In this patch, we did the following change:
1. Change the libprocess cmake target to use 'mesos-agent'
instead of 'mesos-slave'


Diffs (updated)
-

  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
7b53a49baddc8b65f9c5b4b8fe5a53b6883b3652 

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


Testing
---

make
make check


Thanks,

zhou xing

Re: Review Request 45806: Replace Master/Slave Terminology Phase I - Duplicate/Rename binaries.

[zhou xing]

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

(Updated 四月 14, 2016, 3:23 a.m.)


Review request for mesos, Kevin Klues and Vinod Kone.


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


Repository: mesos


Description
---

[#mesos-3782]
In this patch, we did the following changes:
1. Duplicate executable file 'mesos-slave' with 'mesos-agent'
2. Rename the scripts in folder 'mesos/bin' & 'mesos/src/deploy'
to use term 'agent' instead of term 'slave'
3. Change the content of ths scripts to use term 'agent' instead
of term 'slave'
4. Duplicate the renamed scripts renamed in #2 during the configure
and make process, the duplicated scripts still use term 'slave'
in their names.

Please note that afull bootstrap is required once this patch has
been applied. Otherwise, configure will break because of the
missing mesos-slave-*.sh scripts. Please follow the steps to
re-build the project:

```
cd mesos
./bootstrap
mkdir build
cd build
../configure --prefix=${HOME}/install/mesos
make
```


Diffs (updated)
-

  CHANGELOG 1e07c8c2de8eff87c171378ef207c91a20d435d9 
  bin/gdb-mesos-slave.sh.in dbeec8464b26bd808f7a50b8412a2778f1458f22 
  bin/lldb-mesos-slave.sh.in 896c411b2b05d3c4a14288002520a5391a88d955 
  bin/mesos-local-flags.sh.in ab5b6c8bd8847485c5a47d637c9f4fe88c59ae65 
  bin/mesos-slave-flags.sh.in  
  bin/mesos-slave.sh.in 1e3b748ed4dc32ba6bd8adece20f439bce38abc3 
  bin/valgrind-mesos-slave.sh.in 900c5883d96cf14e121e566bcf1ad4dc9a47abf6 
  configure.ac 4392909873e588b93e435d260276b0f1b0814c08 
  src/Makefile.am 139935fb40f8986427c85537a400ff945e54433f 
  src/deploy/mesos-deploy-env.sh.template 
bea5584fbcc68825b1c35b370aed17b0e432edd5 
  src/deploy/mesos-slave-env.sh.template 
31567d6a47e19385aed56edfc7e67457c8cdde3e 
  src/deploy/mesos-start-cluster.sh.in f7a003d9a8e5bbb4f11353988e55e715da0b2b4f 
  src/deploy/mesos-start-slaves.sh.in 50860f40e33fcbb1787be6c035873de4bcc83de5 
  src/deploy/mesos-stop-cluster.sh.in e5f8c1fb400c56715774889632aa74d9eac33645 
  src/deploy/mesos-stop-slaves.sh.in 3dd9b51edff2beb3ccc8d5dd44f0cdc265f623f9 
  src/slave/containerizer/mesos/launch.cpp 
54079c37143f4b5c22e0c9a8fe4bacb31f60ed1b 
  src/tests/balloon_framework_test.sh ae32753e8942f77f94752543c384d218d6e4d48d 
  support/generate-endpoint-help.py 5d23b10f0bab9c0e5848c0c1c26855522cf44a70 
  support/test-upgrade.py 2c4061d71338f66e432dfa4ac86a9693f3ad38bf 

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


Testing
---

cd mesos
./bootstrap
mkdir build
cd build
../configure --prefix=${HOME}/install/mesos
make
make check


Thanks,

zhou xing

Re: Review Request 46008: Stout: Initialize Windows socket stack in Stout tests.

[Alex Clemmer]

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

(Updated April 14, 2016, 2:39 a.m.)


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


Repository: mesos


Description
---

Stout: Initialize Windows socket stack in Stout tests.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/socket.hpp 
b5aeab75c2365a3431c397743b95ce7fbd6d7a1a 
  3rdparty/libprocess/3rdparty/stout/tests/main.cpp 
c449de87bede4d3bf1df368eaf1d22f857c2298f 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 46014: Stout:[1/2] Moved process tests to their own file.

[Alex Clemmer]

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

(Updated April 14, 2016, 2:06 a.m.)


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


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


Repository: mesos


Description
---

Stout:[1/2] Moved process tests to their own file.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/Makefile.am 
400c6dc451602926f93b22713af8c66d7ca59ca6 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
c9d331df2f4496183b5734d2434413f68b9c1b4b 
  3rdparty/libprocess/3rdparty/stout/tests/os/process_tests.cpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
9bd34c7508cd813c5de18028956f6a740997c266 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 45987: Fixed `rmdir.hpp` Windows build breaks.

[Alex Clemmer]

> On April 12, 2016, 2:41 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp, line 179
> > 
> >
> > Can you explain why this is not defined on windows?

Well, I don't know how to port it to Windows. It is not clear to me that `rdev` 
or `mknod` make much sense to Windows. You might be able to make a totally 
different test that makes this work, but after spending an hour or two on it I 
decided that it is more important to make progress on other issues instead.

Thoughts? Do you think it is important to port it right now?


- Alex


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


On April 10, 2016, 10:34 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45987/
> ---
> 
> (Updated April 10, 2016, 10:34 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed `rmdir.hpp` Windows build breaks.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> dd5cc6923ecc503d58e56128cf4ae736d8145fd7 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> cc92953ccc53f37c54b87e738b16ea1fb521b987 
> 
> Diff: https://reviews.apache.org/r/45987/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 46180: Implemented create() for docker volume isolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45214, 45217, 45270, 45360, 45373, 46180]

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

- Mesos ReviewBot


On April 13, 2016, 11:41 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46180/
> ---
> 
> (Updated April 13, 2016, 11:41 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented create() for docker volume isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46180/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46160: Exposed zookeeper/authentication.hpp.

[Jie Yu]

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




src/state/zookeeper.hpp (line 23)


Let's put mesos headers after stout/libprocess


- Jie Yu


On April 13, 2016, 11:50 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46160/
> ---
> 
> (Updated April 13, 2016, 11:50 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5171
> https://issues.apache.org/jira/browse/MESOS-5171
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed zookeeper/authentication.hpp.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
>   src/master/contender/contender.cpp 9ad62528ff5cccfa693c2c6289f05060ece5 
>   src/master/detector/detector.cpp 38517324b3b030c5c425a401ee57cfa53c8ce176 
>   src/state/zookeeper.hpp d8f9df8d1f5575e7dcbfaf27e92656baf3b4f0ff 
>   src/state/zookeeper.cpp 5578fa5f0b86f8dfeabf1be2984bfa0443cc049c 
>   src/tests/group_tests.cpp ebf6963cd4d4dcf053a61495a33a74f22d95e7ec 
>   src/tests/zookeeper_tests.cpp 6bde10b9ae9a644d3506abb285786200dc2fbe94 
>   src/zookeeper/authentication.hpp 40a233381e1e0efd2442efc95627d1968a925eae 
>   src/zookeeper/authentication.cpp 62eab1cc9e85ce65314b9a317ab9d62b1d6d7e88 
>   src/zookeeper/group.hpp 2003b60574e471c0dc08b8abf5f170df02f91f5b 
>   src/zookeeper/url.hpp 1de7a082e444c8b8d22b599feaff0beaf5c45106 
> 
> Diff: https://reviews.apache.org/r/46160/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 46161: Exposed log/log.hpp.

[Jie Yu]

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


Fix it, then Ship it!




Please make sure the cmake works. Also, put mesos header after libprocess/stout 
headers. Please all occurances.


src/java/jni/org_apache_mesos_Log.cpp (line 20)


Please put mesos headers after stout/libproces headers.



src/java/jni/org_apache_mesos_state_LogState.cpp (lines 21 - 23)


Ditto. swapping them.


- Jie Yu


On April 13, 2016, 11:50 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46161/
> ---
> 
> (Updated April 13, 2016, 11:50 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5171
> https://issues.apache.org/jira/browse/MESOS-5171
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed log/log.hpp.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
>   src/java/jni/org_apache_mesos_Log.cpp 
> 140b950136417eed7cba363a89537ed2f33832ff 
>   src/java/jni/org_apache_mesos_state_LogState.cpp 
> 528fe5367f55c63916cd4990abcc3c137e77cfec 
>   src/log/log.hpp 0f61777dd04e6d7212ac218c4cde3cd7f95d2896 
>   src/log/log.cpp a37676068dae14b1adc61ef75e2742c16e7a6e42 
>   src/log/tool/benchmark.cpp 770c6d85fec76826ca8369b2afc721c07899e32f 
>   src/log/tool/replica.cpp 49415821a32960c78192b89f9a0f2067b9157a63 
>   src/state/log.hpp 5dd30ca180380121639f23a931486e97b660c0d2 
>   src/state/log.cpp fd9f5ef90cacb56d934dd8603d70c5a14a36a477 
>   src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188 
>   src/tests/cluster.cpp b4d69106388892b88f7de20b248cac8b950b861c 
>   src/tests/log_tests.cpp 85fc9d4dfaee232f5ea7c29b2133310f7c6441a8 
>   src/tests/registrar_tests.cpp f18e8030f69d8ebf8de81ff03632106e08823df1 
>   src/tests/state_tests.cpp 0b5a6abadc24d16aa0e5e5f2f4b0c9f524c399ec 
> 
> Diff: https://reviews.apache.org/r/46161/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 46160: Exposed zookeeper/authentication.hpp.

[Kapil Arya]

> On April 13, 2016, 8:51 p.m., Jie Yu wrote:
> > Do you need to update cmake?

Ohh, yes. Will do that in a separate commit.


- Kapil


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


On April 13, 2016, 7:50 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46160/
> ---
> 
> (Updated April 13, 2016, 7:50 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5171
> https://issues.apache.org/jira/browse/MESOS-5171
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed zookeeper/authentication.hpp.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
>   src/master/contender/contender.cpp 9ad62528ff5cccfa693c2c6289f05060ece5 
>   src/master/detector/detector.cpp 38517324b3b030c5c425a401ee57cfa53c8ce176 
>   src/state/zookeeper.hpp d8f9df8d1f5575e7dcbfaf27e92656baf3b4f0ff 
>   src/state/zookeeper.cpp 5578fa5f0b86f8dfeabf1be2984bfa0443cc049c 
>   src/tests/group_tests.cpp ebf6963cd4d4dcf053a61495a33a74f22d95e7ec 
>   src/tests/zookeeper_tests.cpp 6bde10b9ae9a644d3506abb285786200dc2fbe94 
>   src/zookeeper/authentication.hpp 40a233381e1e0efd2442efc95627d1968a925eae 
>   src/zookeeper/authentication.cpp 62eab1cc9e85ce65314b9a317ab9d62b1d6d7e88 
>   src/zookeeper/group.hpp 2003b60574e471c0dc08b8abf5f170df02f91f5b 
>   src/zookeeper/url.hpp 1de7a082e444c8b8d22b599feaff0beaf5c45106 
> 
> Diff: https://reviews.apache.org/r/46160/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 46160: Exposed zookeeper/authentication.hpp.

[Jie Yu]

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


Ship it!




Do you need to update cmake?

- Jie Yu


On April 13, 2016, 11:50 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46160/
> ---
> 
> (Updated April 13, 2016, 11:50 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5171
> https://issues.apache.org/jira/browse/MESOS-5171
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed zookeeper/authentication.hpp.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
>   src/master/contender/contender.cpp 9ad62528ff5cccfa693c2c6289f05060ece5 
>   src/master/detector/detector.cpp 38517324b3b030c5c425a401ee57cfa53c8ce176 
>   src/state/zookeeper.hpp d8f9df8d1f5575e7dcbfaf27e92656baf3b4f0ff 
>   src/state/zookeeper.cpp 5578fa5f0b86f8dfeabf1be2984bfa0443cc049c 
>   src/tests/group_tests.cpp ebf6963cd4d4dcf053a61495a33a74f22d95e7ec 
>   src/tests/zookeeper_tests.cpp 6bde10b9ae9a644d3506abb285786200dc2fbe94 
>   src/zookeeper/authentication.hpp 40a233381e1e0efd2442efc95627d1968a925eae 
>   src/zookeeper/authentication.cpp 62eab1cc9e85ce65314b9a317ab9d62b1d6d7e88 
>   src/zookeeper/group.hpp 2003b60574e471c0dc08b8abf5f170df02f91f5b 
>   src/zookeeper/url.hpp 1de7a082e444c8b8d22b599feaff0beaf5c45106 
> 
> Diff: https://reviews.apache.org/r/46160/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 45901: Removed unsed headers from master contender/detector files.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 13, 2016, 8:06 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45901/
> ---
> 
> (Updated April 13, 2016, 8:06 p.m.)
> 
> 
> Review request for mesos, Anurag Singh and Jie Yu.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unsed headers from master contender/detector files.
> 
> 
> Diffs
> -
> 
>   src/master/contender/contender.cpp 9ad62528ff5cccfa693c2c6289f05060ece5 
>   src/master/contender/standalone.hpp 
> 511165c8b355ac9732c2e8921e12545c7c6217ed 
>   src/master/contender/standalone.cpp 
> 18959510338c0f2c5b2df254845c119902ff538f 
>   src/master/contender/zookeeper.hpp 0363ecde801e429df162eca634c89cb1dacd82a2 
>   src/master/contender/zookeeper.cpp 847c62eaaa0ba609138f20c3f4946130d87f2bba 
>   src/master/detector/detector.cpp 38517324b3b030c5c425a401ee57cfa53c8ce176 
>   src/master/detector/standalone.hpp a4007a4f342f177afd09939aac81f4caefadbbd5 
>   src/master/detector/standalone.cpp 0b5de9e5bd575255dac6b18ad63a3eccea589fee 
>   src/master/detector/zookeeper.hpp 042505346636de8e1d258ca3f866e7c35acfca01 
>   src/master/detector/zookeeper.cpp 519a1025d804a7191fb89dca03d7acc969c37758 
>   src/tests/master_contender_detector_tests.cpp 
> 1426a173b1f458d67b748016aee311ac008b054a 
> 
> Diff: https://reviews.apache.org/r/45901/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 45900: Removed stale contender/detector files.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 13, 2016, 8:06 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45900/
> ---
> 
> (Updated April 13, 2016, 8:06 p.m.)
> 
> 
> Review request for mesos, Anurag Singh and James Peach.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also updated Makefile.am to move contender/detector module library
> declarations into the existing list.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
>   src/master/contender/contender.hpp ba05551340f9ca1763f2f460121dc008f0262686 
>   src/master/detector/detector.hpp 840026577d81d1b8cd3b079ff1e5b6809d139250 
> 
> Diff: https://reviews.apache.org/r/45900/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 45983: Enabled the `network/cni` isolator in `MesosContainerizer`.

[Jie Yu]

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




src/slave/containerizer/mesos/containerizer.cpp (lines 165 - 168)


Please wrap this with ifdef linux


- Jie Yu


On April 13, 2016, 2:19 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45983/
> ---
> 
> (Updated April 13, 2016, 2:19 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-5130
> https://issues.apache.org/jira/browse/MESOS-5130
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled the `network/cni` isolator in `MesosContainerizer`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> c25fa92d2a5fa9c828e77c3c0f8b1f795d1b8440 
> 
> Diff: https://reviews.apache.org/r/45983/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> * Ran the Agent without the `network/cni` enabled in the `--isolation` flags. 
> Without the `--network_cni_plugins_dir` and the `--network_cni_config_dir`, 
> container using a docker image was launched on the host-network. With the 
> `--network_cni_plugins_dir` and the `--network_cni_config_dir` specified the 
> CNI network isolator correctly invoked the specified plugin to given the 
> container its own network namespace.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 45955: Added CNI helper subcommand to `mesos-containerizer`.

[Jie Yu]

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



This will break mac build. Please make sure to wrap code with ifdef linux

- Jie Yu


On April 12, 2016, 2:10 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45955/
> ---
> 
> (Updated April 12, 2016, 2:10 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-4922
> https://issues.apache.org/jira/browse/MESOS-4922
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will be used by the `network/cni` isolator to setup hostname and
> various network files within the container UTS and mnt namespace.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/main.cpp 
> 63b257e5ea079d516c0cbed9593a116f22677255 
> 
> Diff: https://reviews.apache.org/r/45955/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> * Ran mesos-execute with the a single master/slave setup that the container 
> attaches to the CNI network correctly and that its network files and hostname 
> are setup correctly.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 45956: Added the `_isolate` method.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 14, 2016, midnight, Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45956/
> ---
> 
> (Updated April 14, 2016, midnight)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-4922
> https://issues.apache.org/jira/browse/MESOS-4922
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Once the `isolate` is successful, the `_isolate` method calls out the
> `mesos-cni-helper` to setup the /etc/hosts, /etc/hostname and
> /etc/resolv.conf for the container.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 3f64bc2d5e114da63a76d93b6406e70891dd0242 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> ac01abc29f0e1a145516d8ec7d373c6cedb33b38 
> 
> Diff: https://reviews.apache.org/r/45956/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> *Ran mesos_execute with single master/slave setup to verify that containers 
> get the right hostname and network files when attached to a CNI network.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 45217: Implemented docker volume driver isolator interface.

[Guangya Liu]

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

(Updated 四月 14, 2016, 12:32 a.m.)


Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
Jie Yu.


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


Repository: mesos


Description
---

Implemented docker volume driver isolator interface.


Diffs (updated)
-

  src/CMakeLists.txt ca59a1824352423f4db6ef8bb41acc6fe602c041 
  src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 46175: Added a test for slavePostFetchHook.

[Jie Yu]

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

(Updated April 14, 2016, 12:08 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added a test for slavePostFetchHook.


Diffs (updated)
-

  src/examples/test_hook_module.cpp 3ff9fd71c275fd1a705ab3aca7a8041f29289bb0 
  src/tests/hook_tests.cpp 97ff55ac7ec875b5ba5d4f17d80646c2d1dd4142 

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


Testing
---

`sudo bin/mesos-tests.sh --gtest_filter=*ROOT_DOCKER_VerifySlavePostFetchHook* `


Thanks,

Jie Yu

Re: Review Request 45956: Added the `_isolate` method.

[Avinash sridharan]

> On April 13, 2016, 5:16 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 716-717
> > 
> >
> > What if IP is not found? Should we use 127.0.0.1?
> 
> Avinash sridharan wrote:
> If the container does not have any IP the hostname should resolve to 
> 169.254.x.y (which is a self assigned IP address). In fact I gave this a bit 
> more thought, and realized that we don't need to add and explicit resolution 
> for hostname in /etc/hosts file. If the hostname is not present in /etc/hosts 
> file the kernel will automatically pick the first, non-loopback IP, as the 
> resolution for the hostname, so explicitly setting the hostname to a an IP in 
> /etc/hosts doesn't help. I verified this on my Ubuntu 14.04 VM, hostnames are 
> not stored in /etc/hosts. According have gone ahead and removed the code to 
> pick an IP for the hostname and adding it to /etc/hosts. 
> 
> We still need the /etc/hosts for resolving `localhost`.
> 
> Jie Yu wrote:
> Sorry, I got confused, what happens if I do `ping ` in the 
> container without setting up /etc/hosts?

Looks like you are right, the behavior seems to be dependent on how 
nsswtich.conf is setup. It seems to be safer to leave the hostname in 
/etc/hosts. Bought back the loop that adds the hostname to /etc/hosts.


- Avinash


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


On April 13, 2016, 7:18 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45956/
> ---
> 
> (Updated April 13, 2016, 7:18 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-4922
> https://issues.apache.org/jira/browse/MESOS-4922
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Once the `isolate` is successful, the `_isolate` method calls out the
> `mesos-cni-helper` to setup the /etc/hosts, /etc/hostname and
> /etc/resolv.conf for the container.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 3f64bc2d5e114da63a76d93b6406e70891dd0242 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> ac01abc29f0e1a145516d8ec7d373c6cedb33b38 
> 
> Diff: https://reviews.apache.org/r/45956/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> *Ran mesos_execute with single master/slave setup to verify that containers 
> get the right hostname and network files when attached to a CNI network.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 45956: Added the `_isolate` method.

[Avinash sridharan]

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

(Updated April 14, 2016, midnight)


Review request for mesos, Jie Yu and Qian Zhang.


Changes
---

Adding back the code to set hostname in /etc/hosts.


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


Repository: mesos


Description
---

Once the `isolate` is successful, the `_isolate` method calls out the
`mesos-cni-helper` to setup the /etc/hosts, /etc/hostname and
/etc/resolv.conf for the container.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
3f64bc2d5e114da63a76d93b6406e70891dd0242 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
ac01abc29f0e1a145516d8ec7d373c6cedb33b38 

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


Testing
---

make

*Ran mesos_execute with single master/slave setup to verify that containers get 
the right hostname and network files when attached to a CNI network.


Thanks,

Avinash sridharan

Re: Review Request 46175: Added a test for slavePostFetchHook.

[Jie Yu]

> On April 13, 2016, 11 p.m., Ben Mahler wrote:
> > src/examples/test_hook_module.cpp, line 197
> > 
> >
> > Mind committing this as a separate patch?

Done.


> On April 13, 2016, 11 p.m., Ben Mahler wrote:
> > src/tests/hook_tests.cpp, line 806
> > 
> >
> > How about the following to make the sandbox from 
> > TemporaryDirectoryTest::sandbox usage more explicit?
> > 
> > ```
> > const string file = path::join(sandbox.get(), "post_fetch_hook");
> > ```

Done.


- Jie


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


On April 13, 2016, 10:33 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46175/
> ---
> 
> (Updated April 13, 2016, 10:33 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-5209
> https://issues.apache.org/jira/browse/MESOS-5209
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test for slavePostFetchHook.
> 
> 
> Diffs
> -
> 
>   src/examples/test_hook_module.cpp 3ff9fd71c275fd1a705ab3aca7a8041f29289bb0 
>   src/tests/hook_tests.cpp 97ff55ac7ec875b5ba5d4f17d80646c2d1dd4142 
> 
> Diff: https://reviews.apache.org/r/46175/diff/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh 
> --gtest_filter=*ROOT_DOCKER_VerifySlavePostFetchHook* `
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 46168: Add subdirectory support to URI.filename field.

[Michael Browning]

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

(Updated April 13, 2016, 11:55 p.m.)


Review request for mesos, Bernd Mathiske and Jiang Yan Xu.


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


Repository: mesos


Description (updated)
---

Add subdirectory support to URI.filename field.

URI.filename allows the user to specify the name of the file that will
be saved in the sandbox when the URI is fetched, but previously it would
fail at fetch time if "filename" had a directory component. This change
allows users to specify a relative path for custom filenames within the
sandbox.


Diffs
-

  docs/fetcher.md fd6d8a78bd35c5644dceff7005dd7dfd9f5f2171 
  src/launcher/fetcher.cpp 47583eeaed53b3e7ed4db26fee7cdd2fae5e0c9d 
  src/slave/containerizer/fetcher.cpp d5910ad570371ba54580be5bb94344a1de38d1f9 
  src/tests/fetcher_cache_tests.cpp 9ffcd2375f1203bd3d7c5d0cc898e955d5cb124e 
  src/tests/fetcher_tests.cpp 23a8dc5f4402c5613744753284aabbe3d09bf797 

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


Testing
---

Three tests were added. In fetcher_tests.cpp, CustomFilenameSubdirectory tests 
that the fetcher creates a file in a specified subdirectory in the sandbox, and 
AbsoluteCustomSubdirectoryFails tests that a custom filename with an absolute 
path is rejected. In fetcher_cache_tests.cpp, 
CachedCustomFilenameWithSubdirectory tests that the same behavior holds when 
the URI is fetched from the cache.


Thanks,

Michael Browning

Re: Review Request 45806: Replace Master/Slave Terminology Phase I - Duplicate/Rename binaries.

[Kevin Klues]

> On April 13, 2016, 11:22 p.m., Vinod Kone wrote:
> > Ship It!

Don't commit this yet, pert the comment at the botoom of: 
https://reviews.apache.org/r/46134/


- Kevin


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


On April 13, 2016, 2:57 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45806/
> ---
> 
> (Updated April 13, 2016, 2:57 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: MESOS-3782
> https://issues.apache.org/jira/browse/MESOS-3782
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [#mesos-3782]
> In this patch, we did the following changes:
> 1. Duplicate executable file 'mesos-slave' with 'mesos-agent'
> 2. Rename the scripts in folder 'mesos/bin' & 'mesos/src/deploy'
> to use term 'agent' instead of term 'slave'
> 3. Change the content of ths scripts to use term 'agent' instead
> of term 'slave'
> 4. Duplicate the renamed scripts renamed in #2 during the configure
> and make process, the duplicated scripts still use term 'slave'
> in their names.
> 
> Please note that afull bootstrap is required once this patch has
> been applied. Otherwise, configure will break because of the
> missing mesos-slave-*.sh scripts. Please follow the steps to
> re-build the project:
> 
> ```
> cd mesos
> ./bootstrap
> mkdir build
> cd build
> ../configure --prefix=${HOME}/install/mesos
> make
> ```
> 
> 
> Diffs
> -
> 
>   CHANGELOG 02b6094a6d4c096d7bee3bae294469fd8942998f 
>   bin/gdb-mesos-slave.sh.in dbeec8464b26bd808f7a50b8412a2778f1458f22 
>   bin/lldb-mesos-slave.sh.in 896c411b2b05d3c4a14288002520a5391a88d955 
>   bin/mesos-local-flags.sh.in ab5b6c8bd8847485c5a47d637c9f4fe88c59ae65 
>   bin/mesos-slave-flags.sh.in  
>   bin/mesos-slave.sh.in 1e3b748ed4dc32ba6bd8adece20f439bce38abc3 
>   bin/valgrind-mesos-slave.sh.in 900c5883d96cf14e121e566bcf1ad4dc9a47abf6 
>   configure.ac 4392909873e588b93e435d260276b0f1b0814c08 
>   src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
>   src/deploy/mesos-deploy-env.sh.template 
> bea5584fbcc68825b1c35b370aed17b0e432edd5 
>   src/deploy/mesos-slave-env.sh.template 
> 31567d6a47e19385aed56edfc7e67457c8cdde3e 
>   src/deploy/mesos-start-cluster.sh.in 
> f7a003d9a8e5bbb4f11353988e55e715da0b2b4f 
>   src/deploy/mesos-start-slaves.sh.in 
> 50860f40e33fcbb1787be6c035873de4bcc83de5 
>   src/deploy/mesos-stop-cluster.sh.in 
> e5f8c1fb400c56715774889632aa74d9eac33645 
>   src/deploy/mesos-stop-slaves.sh.in 3dd9b51edff2beb3ccc8d5dd44f0cdc265f623f9 
>   src/slave/containerizer/mesos/launch.cpp 
> 54079c37143f4b5c22e0c9a8fe4bacb31f60ed1b 
>   src/tests/balloon_framework_test.sh 
> ae32753e8942f77f94752543c384d218d6e4d48d 
>   support/generate-endpoint-help.py 5d23b10f0bab9c0e5848c0c1c26855522cf44a70 
>   support/test-upgrade.py 2c4061d71338f66e432dfa4ac86a9693f3ad38bf 
> 
> Diff: https://reviews.apache.org/r/45806/diff/
> 
> 
> Testing
> ---
> 
> cd mesos
> ./bootstrap
> mkdir build
> cd build
> ../configure --prefix=${HOME}/install/mesos
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 46168: Add subdirectory support to URI.filename field.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [46168]

Failed command: ./support/apply-review.sh -n -r 46168

Error:
2016-04-13 23:46:57 URL:https://reviews.apache.org/r/46168/diff/raw/ 
[7349/7349] -> "46168.patch" [1]
Total errors found: 0
Checking 4 files
Error: No line in the commit message summary may exceed 72 characters.

Full log: https://builds.apache.org/job/mesos-reviewbot/12511/console

- Mesos ReviewBot


On April 13, 2016, 9:37 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46168/
> ---
> 
> (Updated April 13, 2016, 9:37 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5119
> https://issues.apache.org/jira/browse/MESOS-5119
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add subdirectory support to URI.filename field.
> 
> URI.filename allows the user to specify the name of the file that will be 
> saved in the sandbox when the URI is fetched, but previously it would fail at 
> fetch time if "filename" had a directory component. This change allows users 
> to specify a relative path for custom filenames within the sandbox.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md fd6d8a78bd35c5644dceff7005dd7dfd9f5f2171 
>   src/launcher/fetcher.cpp 47583eeaed53b3e7ed4db26fee7cdd2fae5e0c9d 
>   src/slave/containerizer/fetcher.cpp 
> d5910ad570371ba54580be5bb94344a1de38d1f9 
>   src/tests/fetcher_cache_tests.cpp 9ffcd2375f1203bd3d7c5d0cc898e955d5cb124e 
>   src/tests/fetcher_tests.cpp 23a8dc5f4402c5613744753284aabbe3d09bf797 
> 
> Diff: https://reviews.apache.org/r/46168/diff/
> 
> 
> Testing
> ---
> 
> Three tests were added. In fetcher_tests.cpp, CustomFilenameSubdirectory 
> tests that the fetcher creates a file in a specified subdirectory in the 
> sandbox, and AbsoluteCustomSubdirectoryFails tests that a custom filename 
> with an absolute path is rejected. In fetcher_cache_tests.cpp, 
> CachedCustomFilenameWithSubdirectory tests that the same behavior holds when 
> the URI is fetched from the cache.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Re: Review Request 46134: Replace Master/Slave Terminology Phase I - Duplicate/Rename binaries(2).

[Kevin Klues]

> On April 13, 2016, 11:25 p.m., Vinod Kone wrote:
> > Looks like ReviewBot failed. Can you make sure to fix that issue? Otherwise 
> > LGTM.

It looks like it's failing on this step:
```
make[5]: Entering directory `/mesos/mesos-0.29.0/_build/src'
cp /mesos/mesos-0.29.0/_inst/etc/mesos/mesos-agent-env.sh.template \
  /mesos/mesos-0.29.0/_inst/etc/mesos/mesos-slave-env.sh.template
cp: cannot stat 
'/mesos/mesos-0.29.0/_inst/etc/mesos/mesos-agent-env.sh.template': No such file 
or directory
make[5]: *** [copy-agent-env-template] Error 1
```

which is something added in https://reviews.apache.org/r/45806/

The rule is written as:
```
copy-agent-env-template:
cp $(pkgsysconfdir)/mesos-agent-env.sh.template \
  $(pkgsysconfdir)/mesos-slave-env.sh.template
```

so somehow during this installation, `mesos-agent-env.sh.template` didn't get 
installed correctly before this rule ran. Looking a few lines above this error, 
we see that it actually got installed to 
`'/tmp/am-dc-13087//mesos/mesos-0.29.0/_inst/etc/mesos`:
```
/usr/bin/install -c -m 644 ../../src/deploy/mesos-agent-env.sh.template 
../../src/deploy/mesos-deploy-env.sh.template 
../../src/deploy/mesos-master-env.sh.template 
'/tmp/am-dc-13087//mesos/mesos-0.29.0/_inst/etc/mesos'.
```

Which I'm guessing is `$(DESTDIR)/$(pkgsysconfdir)`.  Looks like we need to 
update the rule for `copy-agent-env-template` accordingly.


- Kevin


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


On April 13, 2016, 3:57 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46134/
> ---
> 
> (Updated April 13, 2016, 3:57 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [#mesos-3782]
> In this patch, we did the following change:
> 1. Change the libprocess cmake target to use 'mesos-agent'
> instead of 'mesos-slave'
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> 7b53a49baddc8b65f9c5b4b8fe5a53b6883b3652 
> 
> Diff: https://reviews.apache.org/r/46134/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Review Request 46180: Implemented create() for docker volume isolator.

[Guangya Liu]

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

Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
Jie Yu.


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


Repository: mesos


Description
---

Implemented create() for docker volume isolator.


Diffs
-

  src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 46134: Replace Master/Slave Terminology Phase I - Duplicate/Rename binaries(2).

[Vinod Kone]

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



Looks like ReviewBot failed. Can you make sure to fix that issue? Otherwise 
LGTM.

- Vinod Kone


On April 13, 2016, 3:57 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46134/
> ---
> 
> (Updated April 13, 2016, 3:57 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [#mesos-3782]
> In this patch, we did the following change:
> 1. Change the libprocess cmake target to use 'mesos-agent'
> instead of 'mesos-slave'
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> 7b53a49baddc8b65f9c5b4b8fe5a53b6883b3652 
> 
> Diff: https://reviews.apache.org/r/46134/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 46164: Moved mesos::internal::state to mesos::state namespace.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: []

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

- Mesos ReviewBot


On April 13, 2016, 8:06 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46164/
> ---
> 
> (Updated April 13, 2016, 8:06 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5171
> https://issues.apache.org/jira/browse/MESOS-5171
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved mesos::internal::state to mesos::state namespace.
> 
> 
> Diffs
> -
> 
>   include/mesos/state/state.hpp ecae61fa4fdfb30928c1ccb4e85aa6ebe5addbcd 
>   include/mesos/state/storage.hpp 5de3b174cb80924c6b0dc4cab4569674f70c689a 
>   src/java/jni/org_apache_mesos_state_AbstractState.cpp 
> 4fd43ca31c6917e81ea1b331b8507ca42a2249cf 
>   src/java/jni/org_apache_mesos_state_LevelDBState.cpp 
> fb3c5418220f66c6320581033ee7963de47eafd3 
>   src/java/jni/org_apache_mesos_state_LogState.cpp 
> 528fe5367f55c63916cd4990abcc3c137e77cfec 
>   src/java/jni/org_apache_mesos_state_ZooKeeperState.cpp 
> 23a89c3e5c5309a8a9b45c168fb61b12d89db1ec 
>   src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce 
>   src/master/main.cpp ea7f0fc87c8912309a4679105dde5d8d5bb9ead6 
>   src/state/in_memory.hpp 28ad76134f1cfe64d916679b4f6409d6a5fdd532 
>   src/state/leveldb.hpp 626ed82d84e15f65bc6187f9183cc376b705e1a3 
>   src/state/log.hpp 5dd30ca180380121639f23a931486e97b660c0d2 
>   src/state/protobuf.hpp 7ac5bca1c1a0b92549fc6d7e692e8bdb920b8a0e 
>   src/state/zookeeper.hpp d8f9df8d1f5575e7dcbfaf27e92656baf3b4f0ff 
>   src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188 
>   src/tests/registrar_tests.cpp f18e8030f69d8ebf8de81ff03632106e08823df1 
>   src/tests/state_tests.cpp 0b5a6abadc24d16aa0e5e5f2f4b0c9f524c399ec 
> 
> Diff: https://reviews.apache.org/r/46164/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 45806: Replace Master/Slave Terminology Phase I - Duplicate/Rename binaries.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 13, 2016, 2:57 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45806/
> ---
> 
> (Updated April 13, 2016, 2:57 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: MESOS-3782
> https://issues.apache.org/jira/browse/MESOS-3782
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [#mesos-3782]
> In this patch, we did the following changes:
> 1. Duplicate executable file 'mesos-slave' with 'mesos-agent'
> 2. Rename the scripts in folder 'mesos/bin' & 'mesos/src/deploy'
> to use term 'agent' instead of term 'slave'
> 3. Change the content of ths scripts to use term 'agent' instead
> of term 'slave'
> 4. Duplicate the renamed scripts renamed in #2 during the configure
> and make process, the duplicated scripts still use term 'slave'
> in their names.
> 
> Please note that afull bootstrap is required once this patch has
> been applied. Otherwise, configure will break because of the
> missing mesos-slave-*.sh scripts. Please follow the steps to
> re-build the project:
> 
> ```
> cd mesos
> ./bootstrap
> mkdir build
> cd build
> ../configure --prefix=${HOME}/install/mesos
> make
> ```
> 
> 
> Diffs
> -
> 
>   CHANGELOG 02b6094a6d4c096d7bee3bae294469fd8942998f 
>   bin/gdb-mesos-slave.sh.in dbeec8464b26bd808f7a50b8412a2778f1458f22 
>   bin/lldb-mesos-slave.sh.in 896c411b2b05d3c4a14288002520a5391a88d955 
>   bin/mesos-local-flags.sh.in ab5b6c8bd8847485c5a47d637c9f4fe88c59ae65 
>   bin/mesos-slave-flags.sh.in  
>   bin/mesos-slave.sh.in 1e3b748ed4dc32ba6bd8adece20f439bce38abc3 
>   bin/valgrind-mesos-slave.sh.in 900c5883d96cf14e121e566bcf1ad4dc9a47abf6 
>   configure.ac 4392909873e588b93e435d260276b0f1b0814c08 
>   src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
>   src/deploy/mesos-deploy-env.sh.template 
> bea5584fbcc68825b1c35b370aed17b0e432edd5 
>   src/deploy/mesos-slave-env.sh.template 
> 31567d6a47e19385aed56edfc7e67457c8cdde3e 
>   src/deploy/mesos-start-cluster.sh.in 
> f7a003d9a8e5bbb4f11353988e55e715da0b2b4f 
>   src/deploy/mesos-start-slaves.sh.in 
> 50860f40e33fcbb1787be6c035873de4bcc83de5 
>   src/deploy/mesos-stop-cluster.sh.in 
> e5f8c1fb400c56715774889632aa74d9eac33645 
>   src/deploy/mesos-stop-slaves.sh.in 3dd9b51edff2beb3ccc8d5dd44f0cdc265f623f9 
>   src/slave/containerizer/mesos/launch.cpp 
> 54079c37143f4b5c22e0c9a8fe4bacb31f60ed1b 
>   src/tests/balloon_framework_test.sh 
> ae32753e8942f77f94752543c384d218d6e4d48d 
>   support/generate-endpoint-help.py 5d23b10f0bab9c0e5848c0c1c26855522cf44a70 
>   support/test-upgrade.py 2c4061d71338f66e432dfa4ac86a9693f3ad38bf 
> 
> Diff: https://reviews.apache.org/r/45806/diff/
> 
> 
> Testing
> ---
> 
> cd mesos
> ./bootstrap
> mkdir build
> cd build
> ../configure --prefix=${HOME}/install/mesos
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 45360: Added volume client for mount and unmount.

[Guangya Liu]

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

(Updated 四月 13, 2016, 11:12 p.m.)


Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
Jie Yu.


Repository: mesos


Description
---

Added volume client for mount and unmount.


Diffs (updated)
-

  src/CMakeLists.txt ca59a1824352423f4db6ef8bb41acc6fe602c041 
  src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
  src/slave/containerizer/mesos/isolators/docker/volume/volume_client.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/volume/volume_client.cpp 
PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 46173: Added a slave post fetch hook.

[Ben Mahler]

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


Ship it!





src/hook/manager.cpp (lines 243 - 244)


Would you mind getting the quotes on the same line?

```
LOG(WARNING) << "Failed to execute slave post fetch hook for module"
 << " '" << name << "': " << result.error();
```


- Ben Mahler


On April 13, 2016, 10:30 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46173/
> ---
> 
> (Updated April 13, 2016, 10:30 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-5209
> https://issues.apache.org/jira/browse/MESOS-5209
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a slave post fetch hook.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 87e01a9dccb04bfdd5395867a45c46574815731b 
>   src/hook/manager.hpp 3af28a76493a95a3427fecb3a23dd80ecb84dfe9 
>   src/hook/manager.cpp 17a42f8362f58f0857acabeb2c3113354589fa1b 
>   src/slave/containerizer/docker.cpp 9c24227171c88ee89b567ebc5200a9563dfff5be 
>   src/slave/containerizer/mesos/containerizer.cpp 
> c25fa92d2a5fa9c828e77c3c0f8b1f795d1b8440 
> 
> Diff: https://reviews.apache.org/r/46173/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 45956: Added the `_isolate` method.

[Jie Yu]

> On April 13, 2016, 5:16 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 716-717
> > 
> >
> > What if IP is not found? Should we use 127.0.0.1?
> 
> Avinash sridharan wrote:
> If the container does not have any IP the hostname should resolve to 
> 169.254.x.y (which is a self assigned IP address). In fact I gave this a bit 
> more thought, and realized that we don't need to add and explicit resolution 
> for hostname in /etc/hosts file. If the hostname is not present in /etc/hosts 
> file the kernel will automatically pick the first, non-loopback IP, as the 
> resolution for the hostname, so explicitly setting the hostname to a an IP in 
> /etc/hosts doesn't help. I verified this on my Ubuntu 14.04 VM, hostnames are 
> not stored in /etc/hosts. According have gone ahead and removed the code to 
> pick an IP for the hostname and adding it to /etc/hosts. 
> 
> We still need the /etc/hosts for resolving `localhost`.

Sorry, I got confused, what happens if I do `ping ` in the 
container without setting up /etc/hosts?


- Jie


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


On April 13, 2016, 7:18 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45956/
> ---
> 
> (Updated April 13, 2016, 7:18 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-4922
> https://issues.apache.org/jira/browse/MESOS-4922
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Once the `isolate` is successful, the `_isolate` method calls out the
> `mesos-cni-helper` to setup the /etc/hosts, /etc/hostname and
> /etc/resolv.conf for the container.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 3f64bc2d5e114da63a76d93b6406e70891dd0242 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> ac01abc29f0e1a145516d8ec7d373c6cedb33b38 
> 
> Diff: https://reviews.apache.org/r/45956/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> *Ran mesos_execute with single master/slave setup to verify that containers 
> get the right hostname and network files when attached to a CNI network.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 45360: Added volume client for mount and unmount.

[Guangya Liu]

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

(Updated 四月 13, 2016, 11:01 p.m.)


Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
Jie Yu.


Changes
---

Moved to docker/volume folder and renamed to volume_client.hpp|cpp


Summary (updated)
-

Added volume client for mount and unmount.


Repository: mesos


Description (updated)
---

Added volume client for mount and unmount.


Diffs (updated)
-

  src/CMakeLists.txt ca59a1824352423f4db6ef8bb41acc6fe602c041 
  src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
  src/slave/containerizer/mesos/isolators/docker/volume/volume_client.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/volume/volume_client.cpp 
PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 46173: Added a slave post fetch hook.

[Adam B]

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




src/hook/manager.hpp (lines 69 - 70)


Is this all the information the hook will need? Can it easily get from a 
containerId to a task/executorId and Info?



src/slave/containerizer/docker.cpp (lines 1078 - 1082)


Are you confident that we want this hook to run before persistent volumes 
are mounted?
Maybe what we really want is a slavePreLaunchExecutorContainerHook? Or 
maybe we'll have to have both in the future?


- Adam B


On April 13, 2016, 3:30 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46173/
> ---
> 
> (Updated April 13, 2016, 3:30 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-5209
> https://issues.apache.org/jira/browse/MESOS-5209
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a slave post fetch hook.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 87e01a9dccb04bfdd5395867a45c46574815731b 
>   src/hook/manager.hpp 3af28a76493a95a3427fecb3a23dd80ecb84dfe9 
>   src/hook/manager.cpp 17a42f8362f58f0857acabeb2c3113354589fa1b 
>   src/slave/containerizer/docker.cpp 9c24227171c88ee89b567ebc5200a9563dfff5be 
>   src/slave/containerizer/mesos/containerizer.cpp 
> c25fa92d2a5fa9c828e77c3c0f8b1f795d1b8440 
> 
> Diff: https://reviews.apache.org/r/46173/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 46175: Added a test for slavePostFetchHook.

[Ben Mahler]

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




src/examples/test_hook_module.cpp (line 197)


Mind committing this as a separate patch?



src/tests/hook_tests.cpp (lines 751 - 753)


Why did you need the mock here?



src/tests/hook_tests.cpp (line 761)


How about no newline here to make the code block more clear and to be 
consistent with StartMaster above?



src/tests/hook_tests.cpp (lines 764 - 768)


Ditto here, do you need the mock?



src/tests/hook_tests.cpp (line 776)


How about no newline here to make the code block more clear and to be 
consistent with StartMaster above?



src/tests/hook_tests.cpp (line 797)


`offers->size`



src/tests/hook_tests.cpp (line 806)


How about the following to make the sandbox from 
TemporaryDirectoryTest::sandbox usage more explicit?

```
const string file = path::join(sandbox.get(), "post_fetch_hook");
```



src/tests/hook_tests.cpp (line 823)


Seems like you could remove the repeatedly case, we don't expect more 
updates, right?



src/tests/hook_tests.cpp (line 837)


Why `docker.get()->ps` instead of `docker->ps`?



src/tests/hook_tests.cpp (line 843)


Why `docker.get()->rm` instead of `docker->rm`?


- Ben Mahler


On April 13, 2016, 10:33 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46175/
> ---
> 
> (Updated April 13, 2016, 10:33 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-5209
> https://issues.apache.org/jira/browse/MESOS-5209
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test for slavePostFetchHook.
> 
> 
> Diffs
> -
> 
>   src/examples/test_hook_module.cpp 3ff9fd71c275fd1a705ab3aca7a8041f29289bb0 
>   src/tests/hook_tests.cpp 97ff55ac7ec875b5ba5d4f17d80646c2d1dd4142 
> 
> Diff: https://reviews.apache.org/r/46175/diff/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh 
> --gtest_filter=*ROOT_DOCKER_VerifySlavePostFetchHook* `
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 45670: Updated tests for HTTP command executor.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 13, 2016, 9:15 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45670/
> ---
> 
> (Updated April 13, 2016, 9:15 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3558
> https://issues.apache.org/jira/browse/MESOS-3558
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated tests for HTTP command executor.
> 
> 
> Diffs
> -
> 
>   src/tests/command_executor_tests.cpp 
> 843233adaa68ab1f5cedb7b075439bb8b77469a8 
> 
> Diff: https://reviews.apache.org/r/45670/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 45670: Updated tests for HTTP command executor.

[Vinod Kone]

> On April 12, 2016, 9:11 p.m., Vinod Kone wrote:
> > Are you also planning to update slave recovery tests? Those are the most 
> > crucial.
> 
> Qian Zhang wrote:
> Sure, I will update slave recovery tests soon. Just want to double 
> confirm, in `slave_recovery_tests.cpp`, I see there are two TODOs related to 
> HTTP based command executor in two test cases:
> 
> https://github.com/apache/mesos/blob/0.28.0/src/tests/slave_recovery_tests.cpp#L462
> 
> https://github.com/apache/mesos/blob/0.28.0/src/tests/slave_recovery_tests.cpp#L1082
> 
> You mean I need to update these two test cases with the HTTP command 
> executor I implemented in MESOS-3558, right?

Definitely those two and the one we talked about in the earlier review.

It would also be good to add the following tests for HTTP command executor. 
There are already tests for this for driver based command executor.

--> RecoverUnregisteredHTTPExecutor 
--> RecoverTerminatedExecutor
--> KillTask


- Vinod


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


On April 13, 2016, 9:15 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45670/
> ---
> 
> (Updated April 13, 2016, 9:15 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3558
> https://issues.apache.org/jira/browse/MESOS-3558
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated tests for HTTP command executor.
> 
> 
> Diffs
> -
> 
>   src/tests/command_executor_tests.cpp 
> 843233adaa68ab1f5cedb7b075439bb8b77469a8 
> 
> Diff: https://reviews.apache.org/r/45670/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 46013: Stout: Implemented `os::processes` on Windows.

[Alex Naparu]

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




3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 310)


`break` is not needed after `return`


- Alex Naparu


On April 13, 2016, 10:06 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46013/
> ---
> 
> (Updated April 13, 2016, 10:06 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4471
> https://issues.apache.org/jira/browse/MESOS-4471
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Implemented `os::processes` on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 79e30ca04c6d23f92e3a2f80fbe38ae63fde3520 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> edaa76a5322d0bf60b7172405aa754b5aca95458 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> c48106e5905e3be0faeba7177ef534766089faff 
> 
> Diff: https://reviews.apache.org/r/46013/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 45270: Added spec protobut for external mount.

[Guangya Liu]

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

(Updated 四月 13, 2016, 10:40 p.m.)


Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
Jie Yu.


Changes
---

move files to docker/volume


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


Repository: mesos


Description
---

Added spec protobut for external mount.


Diffs (updated)
-

  src/CMakeLists.txt ca59a1824352423f4db6ef8bb41acc6fe602c041 
  src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
  src/slave/containerizer/mesos/isolators/docker/volume/spec.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/volume/spec.proto PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 44427: Added --http_command_executor flag.

[Vinod Kone]

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


Ship it!




Are you going to add the test to this review or the end of this review chain?

- Vinod Kone


On April 13, 2016, 9:14 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44427/
> ---
> 
> (Updated April 13, 2016, 9:14 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3558
> https://issues.apache.org/jira/browse/MESOS-3558
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added --http_command_executor flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md ba00ec563c449345effb3114111812601addcfc2 
>   src/slave/flags.hpp 300db49100d989d6c0409766b1341cb956ea1631 
>   src/slave/flags.cpp dd7bc9a48dfd8481336a2d2ec0beecd19a342644 
>   src/slave/slave.cpp 6ee277d3c69d03a318c88b7dcb1abc14107ed3e2 
> 
> Diff: https://reviews.apache.org/r/44427/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44427: Added --http_command_executor flag.

[Vinod Kone]

> On April 12, 2016, 9:07 p.m., Vinod Kone wrote:
> > src/slave/slave.cpp, lines 5857-5870
> > 
> >
> > I think this will break slave recovery when a slave is restarted with 
> > the flag set to true. This is because it will incorrectly set 
> > `commandExecutor` to be `false` for driver based command line executors.
> > 
> > I think this code should be:
> > 
> > ```
> > {
> >   CHECK_NOTNULL(slave);
> >   
> >   // See if this is driver based command executor.
> >   Result executorPath = 
> > os::realpath(path::join(slave->flags.launcher_dir, 
> > "mesos-executor");
> > 
> >   if (executorPath.isSome()) {
> > commandExecutor = strings::contains(info.command.value(), 
> > executorPath.get());
> >   }
> >   
> >   // See if this is HTTP based commmand executor.
> >   if (!commandExecutor) {
> > executorPath = 
> >   os::realpath(path::join(slave->flags.launcher_dir, 
> > "mesos-http-executor");
> > 
> > commandExecutor = strings::contains(info.command.value(), 
> > executorPath.get());
> >   }
> >   
> >   return commandExecutor;
> > }
> > 
> > ```
> > 
> > Can you please write a test for this i.e., slave restart with flag 
> > change?
> 
> Qian Zhang wrote:
> Thanks for catching this! I will write a test for slave restarting with 
> flag change soon.
> 
> Qian Zhang wrote:
> BTW, I think I need to add a test for slave restarting with flag change 
> in `slave_recovery_tests.cpp`, right?

That's right.


- Vinod


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


On April 13, 2016, 9:14 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44427/
> ---
> 
> (Updated April 13, 2016, 9:14 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3558
> https://issues.apache.org/jira/browse/MESOS-3558
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added --http_command_executor flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md ba00ec563c449345effb3114111812601addcfc2 
>   src/slave/flags.hpp 300db49100d989d6c0409766b1341cb956ea1631 
>   src/slave/flags.cpp dd7bc9a48dfd8481336a2d2ec0beecd19a342644 
>   src/slave/slave.cpp 6ee277d3c69d03a318c88b7dcb1abc14107ed3e2 
> 
> Diff: https://reviews.apache.org/r/44427/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

[Vinod Kone]

> On April 12, 2016, 8:24 p.m., Vinod Kone wrote:
> > src/launcher/http_command_executor.cpp, lines 296-297
> > 
> >
> > No need to capture taskId because you can get it from `task`?
> 
> Qian Zhang wrote:
> I think just capuring `task` is not enough, because `task` will be set to 
> `None()` when we receive `ACKNOWLEDGED` event, but after that we will still 
> need `taskId`, e.g., in `reap()`, or when we receive `KILL` or `SHUTDOWN` 
> event.

Oh I see. You can drop this then.


- Vinod


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


On April 13, 2016, 9:13 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44424/
> ---
> 
> (Updated April 13, 2016, 9:13 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3558
> https://issues.apache.org/jira/browse/MESOS-3558
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated http_command_executor.cpp to use v1 API.
> 
> 
> Diffs
> -
> 
>   src/launcher/http_command_executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44424/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 13, 2016, 9:13 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44424/
> ---
> 
> (Updated April 13, 2016, 9:13 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3558
> https://issues.apache.org/jira/browse/MESOS-3558
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated http_command_executor.cpp to use v1 API.
> 
> 
> Diffs
> -
> 
>   src/launcher/http_command_executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44424/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 46175: Added a test for slavePostFetchHook.

[Jie Yu]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Added a test for slavePostFetchHook.


Diffs
-

  src/examples/test_hook_module.cpp 3ff9fd71c275fd1a705ab3aca7a8041f29289bb0 
  src/tests/hook_tests.cpp 97ff55ac7ec875b5ba5d4f17d80646c2d1dd4142 

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


Testing
---

`sudo bin/mesos-tests.sh --gtest_filter=*ROOT_DOCKER_VerifySlavePostFetchHook* `


Thanks,

Jie Yu

Review Request 46173: Added a slave post fetch hook.

[Jie Yu]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Added a slave post fetch hook.


Diffs
-

  include/mesos/hook.hpp 87e01a9dccb04bfdd5395867a45c46574815731b 
  src/hook/manager.hpp 3af28a76493a95a3427fecb3a23dd80ecb84dfe9 
  src/hook/manager.cpp 17a42f8362f58f0857acabeb2c3113354589fa1b 
  src/slave/containerizer/docker.cpp 9c24227171c88ee89b567ebc5200a9563dfff5be 
  src/slave/containerizer/mesos/containerizer.cpp 
c25fa92d2a5fa9c828e77c3c0f8b1f795d1b8440 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 45217: Implemented docker volume driver isolator interface.

[Guangya Liu]

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

(Updated 四月 13, 2016, 10:29 p.m.)


Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
Jie Yu.


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


Repository: mesos


Description
---

Implemented docker volume driver isolator interface.


Diffs (updated)
-

  src/CMakeLists.txt ca59a1824352423f4db6ef8bb41acc6fe602c041 
  src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 45217: Implemented docker volume driver isolator interface.

[Guangya Liu]

> On 四月 4, 2016, 11:01 p.m., Gilbert Song wrote:
> > src/Makefile.am, line 860
> > 
> >
> > Could you move it below runtime.cpp?
> 
> Guangya Liu wrote:
> There are some files such as spec.proto, dvd_client.cpp, dvd_client.hpp, 
> dvd.cpp, dvd.hpp etc, I want to put them into ta single folder to make it 
> clear, comments?

It was moved to docker/volume


- Guangya


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


On 四月 13, 2016, 3:18 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45217/
> ---
> 
> (Updated 四月 13, 2016, 3:18 a.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented docker volume driver isolator interface.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt ca59a1824352423f4db6ef8bb41acc6fe602c041 
>   src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
>   src/slave/containerizer/mesos/isolators/docker/dvd/dvd.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45217/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46118: Fixed tests impacted by enabling AuthN for HTTP frameworks.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 12, 2016, 10:26 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46118/
> ---
> 
> (Updated April 12, 2016, 10:26 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4459
> https://issues.apache.org/jira/browse/MESOS-4459
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/master_maintenance_tests.cpp 
> 31d6c70e7ecb584c75404556508dd9cdcf63fb78 
>   src/tests/mesos.hpp 20370a277d55efeea8daae7ea5e2f6575b5a2d62 
>   src/tests/scheduler_http_api_tests.cpp 
> d469adf7230ce5bb5e71a241a06e389295905e03 
>   src/tests/scheduler_tests.cpp b630944b1d3163345e912cb2bfc3301daa4c6362 
> 
> Diff: https://reviews.apache.org/r/46118/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 46117: Added `None()` for credentials when creating scheduler lib.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 12, 2016, 10:25 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46117/
> ---
> 
> (Updated April 12, 2016, 10:25 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4459
> https://issues.apache.org/jira/browse/MESOS-4459
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change defaults the credentials argument to `None()` for
> command scheduler/example HTTP framework/long lived framework when
> creating the scheduler library object.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 306779f5227248a9f1d67b6c50854ec47dd59cd5 
>   src/examples/long_lived_framework.cpp 
> 853e676b08897c8ae63861d9d51173caa2c1bb3f 
>   src/examples/test_http_framework.cpp 
> cba520e326ff8b0b4ed36a0f4cea6879b57f400c 
> 
> Diff: https://reviews.apache.org/r/46117/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 46116: Added basic authentication scheme to the scheduler library.

[Vinod Kone]

> On April 13, 2016, 10:05 p.m., Vinod Kone wrote:
> >

Also, I'm wondering if it would be better for the library to take Authorization 
string as a parameter instead of Credential. That way it is more generic 
because the library can just set "Authorization: ". The idea is 
that scheduler will use a library to do any type of auth it needs (e.g. 
kerberos, JWT) and finally gives the scheduler library the AuthZ header. We 
could maybe make it even more generic by taking a list of headers.


- Vinod


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


On April 12, 2016, 10:26 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46116/
> ---
> 
> (Updated April 12, 2016, 10:26 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4459
> https://issues.apache.org/jira/browse/MESOS-4459
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds basic scheme AuthN support to the library.
> It would be good to add support for additional schemes in the
> future.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp 6603075fe187ac7d1b56381ffc6e18f14adbfb5a 
>   src/scheduler/scheduler.cpp f9d54f99f3086a8a4349abd3892413fa6bd44437 
> 
> Diff: https://reviews.apache.org/r/46116/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 46013: Stout: Implemented `os::processes` on Windows.

[Alex Clemmer]

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

(Updated April 13, 2016, 10:06 p.m.)


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


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


Repository: mesos


Description
---

Stout: Implemented `os::processes` on Windows.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
79e30ca04c6d23f92e3a2f80fbe38ae63fde3520 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
edaa76a5322d0bf60b7172405aa754b5aca95458 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
c48106e5905e3be0faeba7177ef534766089faff 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 46116: Added basic authentication scheme to the scheduler library.

[Vinod Kone]

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


Fix it, then Ship it!





include/mesos/v1/scheduler.hpp (line 62)


Can you add a comment here mentioning what happens if Credential is some?


- Vinod Kone


On April 12, 2016, 10:26 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46116/
> ---
> 
> (Updated April 12, 2016, 10:26 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4459
> https://issues.apache.org/jira/browse/MESOS-4459
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds basic scheme AuthN support to the library.
> It would be good to add support for additional schemes in the
> future.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp 6603075fe187ac7d1b56381ffc6e18f14adbfb5a 
>   src/scheduler/scheduler.cpp f9d54f99f3086a8a4349abd3892413fa6bd44437 
> 
> Diff: https://reviews.apache.org/r/46116/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 46009: Stout:[1/2] Added simple tests for `os::` functions.

[Alex Clemmer]

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

(Updated April 13, 2016, 10:05 p.m.)


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


Repository: mesos


Description
---

Stout:[1/2] Added simple tests for `os::` functions.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/Makefile.am 
400c6dc451602926f93b22713af8c66d7ca59ca6 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
c9d331df2f4496183b5734d2434413f68b9c1b4b 
  3rdparty/libprocess/3rdparty/stout/tests/os/env_tests.cpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/os/strerror_tests.cpp 
d5a96ad6b8b1c09b4f016e0c8e3e5c5672b55ef3 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 46115: Added AuthN for HTTP based frameworks.

[Vinod Kone]

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




src/master/master.cpp (line 850)


As discussed lets have a different REALM for http frameworks.



src/master/validation.cpp (lines 80 - 81)


After master sets FrameworkInfo.principal to the authenticating principal, 
please update this comment.


- Vinod Kone


On April 12, 2016, 10:26 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46115/
> ---
> 
> (Updated April 12, 2016, 10:26 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-3923
> https://issues.apache.org/jira/browse/MESOS-3923
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds AuthN support for HTTP based frameworks.
> 
> We allow AuthZ without AuthN for `/scheduler` endpoint. Also,
> we ensure that `FrameworkInfo.principal` (if present) equals
> the authenticated principal. We also allow a framework to
> omit specifying the `FrameworkInfo.principal` in case
> it is not interested in authorization or if it is disabled.
> We do log a warning for this cases similar to what driver
> based frameworks already do.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b8a83b58b60416f61610cad16fc6f70028a5ee10 
>   src/master/master.hpp 1f480f03900a0dc2996c2ed3a9534dfa8036940f 
>   src/master/master.cpp 781402c04fded159183e1ca28894e48355200f0c 
>   src/master/validation.hpp d1f2323172cbf3bb052942a119b8531f9ccad48d 
>   src/master/validation.cpp 13423436a4e6361fde6fa75133eebf5c02c8381f 
> 
> Diff: https://reviews.apache.org/r/46115/diff/
> 
> 
> Testing
> ---
> 
> make check. Enabled AuthN for all the tests later in the chain.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 46013: Stout: Implemented `os::processes` on Windows.

[Alex Naparu]

> On April 12, 2016, 11:10 p.m., Alex Naparu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp, line 383
> > 
> >
> > Can you document this flag a bit please? GetProcessTimes requires 
> > PROCESS_QUERY_LIMITED_INFORMATION, while GetProcessMemoryInfo requires the 
> > additional PROCESS_VM_READ.
> 
> Alex Clemmer wrote:
> Sure, but what part would you like me to document?

I would like to see some explanatory note as to why the THREAD_ALL_ACCESS flag 
is required (and appropriate) in this context. Looking at MSDN, none of the 
functions that are using the process handle (GetProcessTimes and 
GetProcessMemoryInfo) make any note of this flag. It might be that 
THREAD_ALL_ACCESS is overkill in this context.


- Alex


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


On April 13, 2016, 9:49 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46013/
> ---
> 
> (Updated April 13, 2016, 9:49 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4471
> https://issues.apache.org/jira/browse/MESOS-4471
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Implemented `os::processes` on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 79e30ca04c6d23f92e3a2f80fbe38ae63fde3520 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> edaa76a5322d0bf60b7172405aa754b5aca95458 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> c48106e5905e3be0faeba7177ef534766089faff 
> 
> Diff: https://reviews.apache.org/r/46013/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 46013: Stout: Implemented `os::processes` on Windows.

[Alex Clemmer]

> On April 12, 2016, 11:10 p.m., Alex Naparu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp, line 383
> > 
> >
> > Can you document this flag a bit please? GetProcessTimes requires 
> > PROCESS_QUERY_LIMITED_INFORMATION, while GetProcessMemoryInfo requires the 
> > additional PROCESS_VM_READ.

Sure, but what part would you like me to document?


- Alex


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


On April 13, 2016, 9:33 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46013/
> ---
> 
> (Updated April 13, 2016, 9:33 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4471
> https://issues.apache.org/jira/browse/MESOS-4471
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Implemented `os::processes` on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 79e30ca04c6d23f92e3a2f80fbe38ae63fde3520 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> edaa76a5322d0bf60b7172405aa754b5aca95458 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> c48106e5905e3be0faeba7177ef534766089faff 
> 
> Diff: https://reviews.apache.org/r/46013/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 46013: Stout: Implemented `os::processes` on Windows.

[Alex Clemmer]

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

(Updated April 13, 2016, 9:49 p.m.)


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


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


Repository: mesos


Description
---

Stout: Implemented `os::processes` on Windows.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
79e30ca04c6d23f92e3a2f80fbe38ae63fde3520 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
edaa76a5322d0bf60b7172405aa754b5aca95458 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
c48106e5905e3be0faeba7177ef534766089faff 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 45214: Updated protobuf to support external storage.

[Guangya Liu]

> On 四月 13, 2016, 9:35 p.m., James DeFelice wrote:
> > include/mesos/mesos.proto, line 1625
> > 
> >
> > I definitely prefer this form, but there's also a Parameters message 
> > type. Which is more idiomatic in this context?

I saw that no one in protobuf is using this, and the `Parameters` is just a 
wrapper of `repeated Parameter parameter`, I think it is OK to use `repeated 
Parameter parameter` here, comments?


- Guangya


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


On 四月 13, 2016, 3:09 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45214/
> ---
> 
> (Updated 四月 13, 2016, 3:09 a.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5012
> https://issues.apache.org/jira/browse/MESOS-5012
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated protobuf to support external storage.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 1232bcda053db3451d18413e81e54e51774f7f22 
>   include/mesos/v1/mesos.proto 1048fae62733926b5e863a0c1629bba69e35e43b 
> 
> Diff: https://reviews.apache.org/r/45214/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46027: Documented `libprocess` helper function.

[Neil Conway]

> On April 13, 2016, 9:33 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/process.cpp, line 559
> > 
> >
> > s/.  A/. A/
> > 
> > Thanks for clarifying, sorry for the confusion!
> 
> Ben Mahler wrote:
> Also, would you mind wrapping comments at 70 to stay consistent with most 
> of our existing code?
> 
> ```
> // Returns true if `request` contains an inbound libprocess message.
> // A libprocess message can either be sent by another instance of
> // libprocess (i.e. both of the "User-Agent" and "Libprocess-From"
> // headers will be set), or a client that speaks the libprocess
> // protocol (i.e. only the "Libprocess-From" header will be set).
> // This function returns true for either case.
> ```

Per commit f9c2604ea97b91f8a9ec3b2863317761679b1c86, I thought that wasn't a 
requirement anymore?


- Neil


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


On April 12, 2016, 12:23 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46027/
> ---
> 
> (Updated April 12, 2016, 12:23 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a bit confusing, because different libprocess HTTP
> clients set different headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a 
> 
> Diff: https://reviews.apache.org/r/46027/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 46014: Stout:[1/2] Moved process tests to their own file.

[Alex Clemmer]

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

(Updated April 13, 2016, 9:37 p.m.)


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


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


Repository: mesos


Description
---

Stout:[1/2] Moved process tests to their own file.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/Makefile.am 
400c6dc451602926f93b22713af8c66d7ca59ca6 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
c9d331df2f4496183b5734d2434413f68b9c1b4b 
  3rdparty/libprocess/3rdparty/stout/tests/os/process_tests.cpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
9bd34c7508cd813c5de18028956f6a740997c266 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 46027: Documented `libprocess` helper function.

[Ben Mahler]

> On April 13, 2016, 9:33 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/process.cpp, line 559
> > 
> >
> > s/.  A/. A/
> > 
> > Thanks for clarifying, sorry for the confusion!

Also, would you mind wrapping comments at 70 to stay consistent with most of 
our existing code?

```
// Returns true if `request` contains an inbound libprocess message.
// A libprocess message can either be sent by another instance of
// libprocess (i.e. both of the "User-Agent" and "Libprocess-From"
// headers will be set), or a client that speaks the libprocess
// protocol (i.e. only the "Libprocess-From" header will be set).
// This function returns true for either case.
```


- Ben


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


On April 12, 2016, 12:23 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46027/
> ---
> 
> (Updated April 12, 2016, 12:23 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a bit confusing, because different libprocess HTTP
> clients set different headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a 
> 
> Diff: https://reviews.apache.org/r/46027/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 46168: Add subdirectory support to URI.filename field.

[Michael Browning]

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

Review request for mesos, Bernd Mathiske and Jiang Yan Xu.


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


Repository: mesos


Description
---

Add subdirectory support to URI.filename field.

URI.filename allows the user to specify the name of the file that will be saved 
in the sandbox when the URI is fetched, but previously it would fail at fetch 
time if "filename" had a directory component. This change allows users to 
specify a relative path for custom filenames within the sandbox.


Diffs
-

  docs/fetcher.md fd6d8a78bd35c5644dceff7005dd7dfd9f5f2171 
  src/launcher/fetcher.cpp 47583eeaed53b3e7ed4db26fee7cdd2fae5e0c9d 
  src/slave/containerizer/fetcher.cpp d5910ad570371ba54580be5bb94344a1de38d1f9 
  src/tests/fetcher_cache_tests.cpp 9ffcd2375f1203bd3d7c5d0cc898e955d5cb124e 
  src/tests/fetcher_tests.cpp 23a8dc5f4402c5613744753284aabbe3d09bf797 

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


Testing
---

Three tests were added. In fetcher_tests.cpp, CustomFilenameSubdirectory tests 
that the fetcher creates a file in a specified subdirectory in the sandbox, and 
AbsoluteCustomSubdirectoryFails tests that a custom filename with an absolute 
path is rejected. In fetcher_cache_tests.cpp, 
CachedCustomFilenameWithSubdirectory tests that the same behavior holds when 
the URI is fetched from the cache.


Thanks,

Michael Browning

Re: Review Request 46013: Stout: Implemented `os::processes` on Windows.

[Alex Clemmer]

> On April 12, 2016, 11:10 p.m., Alex Naparu wrote:
> >

Hey, sorry, you reviewed an old version of the code. I had a different branch 
checked out when I pushed this. The background is that this is an amalgamation 
of maybe 3 or 4 commits, and the code is not amazing. I went through and fixed 
up a bunch of stuff on behalf of others, since they don't have the cycles to 
deal with it themselves.

Anyway, all this is to say: I've pushed a new revision, so most of these 
comments will be addressed incidentally -- things like `exists` being a 
parameter, stout naming conventions, etc.

Sorry for the confusion.


- Alex


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


On April 13, 2016, 9:33 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46013/
> ---
> 
> (Updated April 13, 2016, 9:33 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4471
> https://issues.apache.org/jira/browse/MESOS-4471
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Implemented `os::processes` on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 79e30ca04c6d23f92e3a2f80fbe38ae63fde3520 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> edaa76a5322d0bf60b7172405aa754b5aca95458 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> c48106e5905e3be0faeba7177ef534766089faff 
> 
> Diff: https://reviews.apache.org/r/46013/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 45214: Updated protobuf to support external storage.

[James DeFelice]

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




include/mesos/mesos.proto (line 1625)


I definitely prefer this form, but there's also a Parameters message type. 
Which is more idiomatic in this context?


- James DeFelice


On April 13, 2016, 3:09 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45214/
> ---
> 
> (Updated April 13, 2016, 3:09 a.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5012
> https://issues.apache.org/jira/browse/MESOS-5012
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated protobuf to support external storage.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 1232bcda053db3451d18413e81e54e51774f7f22 
>   include/mesos/v1/mesos.proto 1048fae62733926b5e863a0c1629bba69e35e43b 
> 
> Diff: https://reviews.apache.org/r/45214/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46027: Documented `libprocess` helper function.

[Ben Mahler]

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


Ship it!





3rdparty/libprocess/src/process.cpp (line 559)


s/.  A/. A/

Thanks for clarifying, sorry for the confusion!


- Ben Mahler


On April 12, 2016, 12:23 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46027/
> ---
> 
> (Updated April 12, 2016, 12:23 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a bit confusing, because different libprocess HTTP
> clients set different headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a 
> 
> Diff: https://reviews.apache.org/r/46027/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 46013: Stout: Implemented `os::processes` on Windows.

[Alex Clemmer]

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

(Updated April 13, 2016, 9:33 p.m.)


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


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


Repository: mesos


Description
---

Stout: Implemented `os::processes` on Windows.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
79e30ca04c6d23f92e3a2f80fbe38ae63fde3520 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
edaa76a5322d0bf60b7172405aa754b5aca95458 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
c48106e5905e3be0faeba7177ef534766089faff 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 46029: Mark a few private functions `static` in libprocess tests.

[Ben Mahler]

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


Ship it!




Ship It!

- Ben Mahler


On April 12, 2016, 12:13 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46029/
> ---
> 
> (Updated April 12, 2016, 12:13 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mark a few private functions `static` in libprocess tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/future_tests.cpp 
> 8a21c6dc9141fe3372f12908a0007dd1c93c5485 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 274a76fa61a5cd4b1324be124f73979d4b980660 
> 
> Diff: https://reviews.apache.org/r/46029/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 45370: Implemented prepare() for dvd isolator.

[James DeFelice]

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




src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp (line 106)


we should probably validate (or else sanitize) the contents of driver, 
name, and options


- James DeFelice


On April 4, 2016, 9:21 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45370/
> ---
> 
> (Updated April 4, 2016, 9:21 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented prepare() for dvd isolator.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp af62f41622e1c43acd8f257c54f8754162c433b8 
>   src/slave/containerizer/mesos/isolators/docker/dvd/dvd.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/dvd/spec.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45370/diff/
> 
> 
> Testing
> ---
> 
> The execute.cpp is mainly for test, will remove this file once test finished.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45326: Added agent flag dvdcli_dir for specify dvdcli path.

[James DeFelice]

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




src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp (line 64)


should we verify that the dvdcli binary actually exists here (and if not 
generate an error)?


- James DeFelice


On April 2, 2016, 3:54 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45326/
> ---
> 
> (Updated April 2, 2016, 3:54 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flag dvdcli_dir for specify dvdcli path.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md da42eaf7069a016fa7eaf929fc285e1fa1f144e9 
>   src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960 
>   src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION 
>   src/slave/flags.hpp d0c606eea74e1a2e69067c43a267047e65a22a04 
>   src/slave/flags.cpp 0551ec334c6747507bf7bb068d27d67f3fdd6c83 
> 
> Diff: https://reviews.apache.org/r/45326/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 46165: Removed dependency on Boost.Foreach.

[Michael Park]

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

Review request for mesos and Benjamin Bannier.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/foreach.hpp 
7fb0044790ee249b69e07b81a26851bd5bfb110f 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 45370: Implemented prepare() for dvd isolator.

[James DeFelice]

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




src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp (line 110)


options in ExternalMount is a repeated Parameter, which has key and value. 
Looks like this needs some refactoring.



src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp (line 113)


other `--flag` params are computed elsewhere (e.g. DvdClient::mount); 
should do the same here. Just construct Parameters instead


- James DeFelice


On April 4, 2016, 9:21 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45370/
> ---
> 
> (Updated April 4, 2016, 9:21 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented prepare() for dvd isolator.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp af62f41622e1c43acd8f257c54f8754162c433b8 
>   src/slave/containerizer/mesos/isolators/docker/dvd/dvd.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/dvd/spec.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45370/diff/
> 
> 
> Testing
> ---
> 
> The execute.cpp is mainly for test, will remove this file once test finished.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45614: Windows: Implemented os::close for Windows.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [45614]

Failed command: ./support/apply-review.sh -n -r 45614

Error:
2016-04-13 20:50:38 URL:https://reviews.apache.org/r/45614/diff/raw/ 
[4160/4160] -> "45614.patch" [1]
error: patch failed: 
3rdparty/libprocess/3rdparty/stout/include/stout/os/close.hpp:14
error: 3rdparty/libprocess/3rdparty/stout/include/stout/os/close.hpp: patch 
does not apply
error: 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/close.hpp: 
already exists in index
error: 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/close.hpp: 
already exists in index
error: patch failed: 
3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp:339
error: 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp: patch does 
not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/12509/console

- Mesos ReviewBot


On April 13, 2016, 6:21 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45614/
> ---
> 
> (Updated April 13, 2016, 6:21 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Alex 
> Clemmer, Joris Van Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented os::close for Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/close.hpp 
> 1c912c3edc815e7e0b1a562286279897d773a516 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/close.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/close.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> ee13d12fcffcd564c7ded2d2f541d7bbdf6633c1 
> 
> Diff: https://reviews.apache.org/r/45614/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 45360: Added dvd client for mount and unmount.

[James DeFelice]

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




src/slave/containerizer/mesos/isolators/docker/dvd/dvd_client.cpp (line 66)


options must be specified as follows:
`--volumeopts={param.key}={param.value}`


- James DeFelice


On April 13, 2016, 9:17 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45360/
> ---
> 
> (Updated April 13, 2016, 9:17 a.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added dvd client for mount and unmount.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt ca59a1824352423f4db6ef8bb41acc6fe602c041 
>   src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
>   src/slave/containerizer/mesos/isolators/docker/dvd/dvd_client.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/dvd/dvd_client.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45360/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 46164: Moved mesos::internal::state to mesos::state namespace.

[Kapil Arya]

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

Review request for mesos.


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


Repository: mesos


Description
---

Moved mesos::internal::state to mesos::state namespace.


Diffs
-

  include/mesos/state/state.hpp ecae61fa4fdfb30928c1ccb4e85aa6ebe5addbcd 
  include/mesos/state/storage.hpp 5de3b174cb80924c6b0dc4cab4569674f70c689a 
  src/java/jni/org_apache_mesos_state_AbstractState.cpp 
4fd43ca31c6917e81ea1b331b8507ca42a2249cf 
  src/java/jni/org_apache_mesos_state_LevelDBState.cpp 
fb3c5418220f66c6320581033ee7963de47eafd3 
  src/java/jni/org_apache_mesos_state_LogState.cpp 
528fe5367f55c63916cd4990abcc3c137e77cfec 
  src/java/jni/org_apache_mesos_state_ZooKeeperState.cpp 
23a89c3e5c5309a8a9b45c168fb61b12d89db1ec 
  src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce 
  src/master/main.cpp ea7f0fc87c8912309a4679105dde5d8d5bb9ead6 
  src/state/in_memory.hpp 28ad76134f1cfe64d916679b4f6409d6a5fdd532 
  src/state/leveldb.hpp 626ed82d84e15f65bc6187f9183cc376b705e1a3 
  src/state/log.hpp 5dd30ca180380121639f23a931486e97b660c0d2 
  src/state/protobuf.hpp 7ac5bca1c1a0b92549fc6d7e692e8bdb920b8a0e 
  src/state/zookeeper.hpp d8f9df8d1f5575e7dcbfaf27e92656baf3b4f0ff 
  src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188 
  src/tests/registrar_tests.cpp f18e8030f69d8ebf8de81ff03632106e08823df1 
  src/tests/state_tests.cpp 0b5a6abadc24d16aa0e5e5f2f4b0c9f524c399ec 

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


Testing
---

make check


Thanks,

Kapil Arya

Review Request 46160: Exposed zookeeper/authentication.hpp.

[Kapil Arya]

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

Review request for mesos.


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


Repository: mesos


Description
---

Exposed zookeeper/authentication.hpp.


Diffs
-

  src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
  src/master/contender/contender.cpp 9ad62528ff5cccfa693c2c6289f05060ece5 
  src/master/detector/detector.cpp 38517324b3b030c5c425a401ee57cfa53c8ce176 
  src/state/zookeeper.hpp d8f9df8d1f5575e7dcbfaf27e92656baf3b4f0ff 
  src/state/zookeeper.cpp 5578fa5f0b86f8dfeabf1be2984bfa0443cc049c 
  src/tests/group_tests.cpp ebf6963cd4d4dcf053a61495a33a74f22d95e7ec 
  src/tests/zookeeper_tests.cpp 6bde10b9ae9a644d3506abb285786200dc2fbe94 
  src/zookeeper/authentication.hpp 40a233381e1e0efd2442efc95627d1968a925eae 
  src/zookeeper/authentication.cpp 62eab1cc9e85ce65314b9a317ab9d62b1d6d7e88 
  src/zookeeper/group.hpp 2003b60574e471c0dc08b8abf5f170df02f91f5b 
  src/zookeeper/url.hpp 1de7a082e444c8b8d22b599feaff0beaf5c45106 

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


Testing
---

make check


Thanks,

Kapil Arya

Re: Review Request 45900: Removed stale contender/detector files.

[Kapil Arya]

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

(Updated April 13, 2016, 4:06 p.m.)


Review request for mesos, Anurag Singh and James Peach.


Changes
---

rebased


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


Repository: mesos


Description
---

Also updated Makefile.am to move contender/detector module library
declarations into the existing list.


Diffs (updated)
-

  src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
  src/master/contender/contender.hpp ba05551340f9ca1763f2f460121dc008f0262686 
  src/master/detector/detector.hpp 840026577d81d1b8cd3b079ff1e5b6809d139250 

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


Testing
---

make check.


Thanks,

Kapil Arya

Review Request 46161: Exposed log/log.hpp.

[Kapil Arya]

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

Review request for mesos.


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


Repository: mesos


Description
---

Exposed log/log.hpp.


Diffs
-

  src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
  src/java/jni/org_apache_mesos_Log.cpp 
140b950136417eed7cba363a89537ed2f33832ff 
  src/java/jni/org_apache_mesos_state_LogState.cpp 
528fe5367f55c63916cd4990abcc3c137e77cfec 
  src/log/log.hpp 0f61777dd04e6d7212ac218c4cde3cd7f95d2896 
  src/log/log.cpp a37676068dae14b1adc61ef75e2742c16e7a6e42 
  src/log/tool/benchmark.cpp 770c6d85fec76826ca8369b2afc721c07899e32f 
  src/log/tool/replica.cpp 49415821a32960c78192b89f9a0f2067b9157a63 
  src/state/log.hpp 5dd30ca180380121639f23a931486e97b660c0d2 
  src/state/log.cpp fd9f5ef90cacb56d934dd8603d70c5a14a36a477 
  src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188 
  src/tests/cluster.cpp b4d69106388892b88f7de20b248cac8b950b861c 
  src/tests/log_tests.cpp 85fc9d4dfaee232f5ea7c29b2133310f7c6441a8 
  src/tests/registrar_tests.cpp f18e8030f69d8ebf8de81ff03632106e08823df1 
  src/tests/state_tests.cpp 0b5a6abadc24d16aa0e5e5f2f4b0c9f524c399ec 

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


Testing
---

make check


Thanks,

Kapil Arya

Review Request 46162: Moved mesos::internal::log::Log to mesos::log namespace.

[Kapil Arya]

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

Review request for mesos.


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


Repository: mesos


Description
---

Moved mesos::internal::log::Log to mesos::log namespace.


Diffs
-

  include/mesos/log/log.hpp 0f61777dd04e6d7212ac218c4cde3cd7f95d2896 
  src/java/jni/org_apache_mesos_Log.cpp 
140b950136417eed7cba363a89537ed2f33832ff 
  src/java/jni/org_apache_mesos_state_LogState.cpp 
528fe5367f55c63916cd4990abcc3c137e77cfec 
  src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce 
  src/log/log.cpp a37676068dae14b1adc61ef75e2742c16e7a6e42 
  src/log/tool/benchmark.cpp 770c6d85fec76826ca8369b2afc721c07899e32f 
  src/log/tool/replica.cpp 49415821a32960c78192b89f9a0f2067b9157a63 
  src/master/main.cpp ea7f0fc87c8912309a4679105dde5d8d5bb9ead6 
  src/state/log.hpp 5dd30ca180380121639f23a931486e97b660c0d2 
  src/state/log.cpp fd9f5ef90cacb56d934dd8603d70c5a14a36a477 
  src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188 
  src/tests/cluster.cpp b4d69106388892b88f7de20b248cac8b950b861c 
  src/tests/log_tests.cpp 85fc9d4dfaee232f5ea7c29b2133310f7c6441a8 
  src/tests/registrar_tests.cpp f18e8030f69d8ebf8de81ff03632106e08823df1 
  src/tests/state_tests.cpp 0b5a6abadc24d16aa0e5e5f2f4b0c9f524c399ec 

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


Testing
---

make check


Thanks,

Kapil Arya

Re: Review Request 45901: Removed unsed headers from master contender/detector files.

[Kapil Arya]

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

(Updated April 13, 2016, 4:06 p.m.)


Review request for mesos, Anurag Singh and Jie Yu.


Changes
---

rebased


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


Repository: mesos


Description
---

Removed unsed headers from master contender/detector files.


Diffs (updated)
-

  src/master/contender/contender.cpp 9ad62528ff5cccfa693c2c6289f05060ece5 
  src/master/contender/standalone.hpp 511165c8b355ac9732c2e8921e12545c7c6217ed 
  src/master/contender/standalone.cpp 18959510338c0f2c5b2df254845c119902ff538f 
  src/master/contender/zookeeper.hpp 0363ecde801e429df162eca634c89cb1dacd82a2 
  src/master/contender/zookeeper.cpp 847c62eaaa0ba609138f20c3f4946130d87f2bba 
  src/master/detector/detector.cpp 38517324b3b030c5c425a401ee57cfa53c8ce176 
  src/master/detector/standalone.hpp a4007a4f342f177afd09939aac81f4caefadbbd5 
  src/master/detector/standalone.cpp 0b5de9e5bd575255dac6b18ad63a3eccea589fee 
  src/master/detector/zookeeper.hpp 042505346636de8e1d258ca3f866e7c35acfca01 
  src/master/detector/zookeeper.cpp 519a1025d804a7191fb89dca03d7acc969c37758 
  src/tests/master_contender_detector_tests.cpp 
1426a173b1f458d67b748016aee311ac008b054a 

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


Testing
---

make check.


Thanks,

Kapil Arya

Review Request 46163: Exposed state/{state,storage}.hpp files.

[Kapil Arya]

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

Review request for mesos.


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


Repository: mesos


Description
---

Exposed state/{state,storage}.hpp files.


Diffs
-

  src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
  src/java/jni/org_apache_mesos_state_AbstractState.cpp 
4fd43ca31c6917e81ea1b331b8507ca42a2249cf 
  src/java/jni/org_apache_mesos_state_LevelDBState.cpp 
fb3c5418220f66c6320581033ee7963de47eafd3 
  src/java/jni/org_apache_mesos_state_LogState.cpp 
528fe5367f55c63916cd4990abcc3c137e77cfec 
  src/java/jni/org_apache_mesos_state_Variable.cpp 
7a69b3d878e6eaa87bfaf56c0500e11efe4ac44d 
  src/java/jni/org_apache_mesos_state_ZooKeeperState.cpp 
23a89c3e5c5309a8a9b45c168fb61b12d89db1ec 
  src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce 
  src/master/main.cpp ea7f0fc87c8912309a4679105dde5d8d5bb9ead6 
  src/messages/state.proto  
  src/state/in_memory.hpp 28ad76134f1cfe64d916679b4f6409d6a5fdd532 
  src/state/in_memory.cpp ac4f34bd2332c8b42a26454809d56c225aa44587 
  src/state/leveldb.hpp 626ed82d84e15f65bc6187f9183cc376b705e1a3 
  src/state/leveldb.cpp f925a73856075f08c4af2af0fcaae8e81358667e 
  src/state/log.hpp 5dd30ca180380121639f23a931486e97b660c0d2 
  src/state/protobuf.hpp 7ac5bca1c1a0b92549fc6d7e692e8bdb920b8a0e 
  src/state/state.hpp ecae61fa4fdfb30928c1ccb4e85aa6ebe5addbcd 
  src/state/storage.hpp 5de3b174cb80924c6b0dc4cab4569674f70c689a 
  src/state/zookeeper.hpp d8f9df8d1f5575e7dcbfaf27e92656baf3b4f0ff 
  src/state/zookeeper.cpp 5578fa5f0b86f8dfeabf1be2984bfa0443cc049c 
  src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188 
  src/tests/cluster.cpp b4d69106388892b88f7de20b248cac8b950b861c 
  src/tests/registrar_tests.cpp f18e8030f69d8ebf8de81ff03632106e08823df1 
  src/tests/state_tests.cpp 0b5a6abadc24d16aa0e5e5f2f4b0c9f524c399ec 

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


Testing
---

make check


Thanks,

Kapil Arya

Re: Review Request 46114: Fixed per framework principal metrics for HTTP frameworks.

[Vinod Kone]

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




src/master/master.cpp (lines 5939 - 5941)


The principal in HTTP based framework will be passed in the HTTP 
Authorization header. The FrameworkInfo.prinicipal is orthongoal to whether the 
framework is using a driver based API or HTTP API; it should be used in cases 
where framework doesn't want to authenticate but still want mesos to authorize 
its actions.

Also, I was expecting the `authenticated` map to contain information about 
if a framework is authenticated or not irrespective of whether it's driver or 
HTTP based. The map will need to be updated to take either a PID or HTTP 
connection.

I haven't seen the other reviews, but this is what I had in mind.



src/master/master.cpp (lines 5944 - 5947)


We were discussing about this in another context this morning. I think we 
should just set FrameworkInfo.principal to Credential.principal (driver based) 
or AuthorizationHeader.principal (HTTP API) when framework has authenticated. 
This should be done by the master at the point of registration/subscription. 
Can you do that in a different review and make this review dependent on that?



src/master/master.cpp (line 5958)


After the above changes, you can kill the above if else loop and change 
this to

if (framework->info.has_principal()) {

}


- Vinod Kone


On April 12, 2016, 10:25 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46114/
> ---
> 
> (Updated April 12, 2016, 10:25 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3923
> https://issues.apache.org/jira/browse/MESOS-3923
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, we never used to initialize the per principal metrics
> that can be accessed by "frameworks/". Currently, it
> only has two metrics that have no relevance for HTTP frameworks
> i.e., `messages_received`, `messages_processed` but that might change
> in the future if we add more such metrics. It also ensures that
> HTTP->PID framework downgrades preserve the metrics and no
> additional logic is needed to deal with this. (Previously
> HTTP frameworks never had AuthN and no principal was passed
> in `FrameworkInfo`, so this was never a concern).
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 781402c04fded159183e1ca28894e48355200f0c 
> 
> Diff: https://reviews.apache.org/r/46114/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45668: Enable CMake build for Linux as a CONFIGURATOR option.

[Juan Larriba]

> On Abr. 9, 2016, 1:34 a.m., Vinod Kone wrote:
> > support/docker_build.sh, line 125
> > 
> >
> > See the ASF CI build error on the attached ticket.
> > 
> > I think specifying configuration options this way doesn't work with 
> > cmake.
> > 
> > Not sure what the right way is. cc @hausdorff
> 
> Juan Larriba wrote:
> export CONFIGURATION='-DVERBOSE="1"'
> 
> Vinod Kone wrote:
> We need to make the CONFIGURATION value work with both autotools and 
> cmake. Can you update the script to append "-D" to CONFIGURATION (you might 
> need to append it once for each configuration value)? Also, test it locally 
> with both autotools and cmake with the following CONFIGURATION value 
> (--verbose --enable_libevent)

The configuration options for autotools and cmake have a completely different 
syntax, so I do not see the point in translating the autotools configuration 
options to cmake options.

I can do the translation, but don't you think it would be more elegant to pass 
the adequate configuration options for the used buildtools?


- Juan


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


On Abr. 8, 2016, 11:26 p.m., Juan Larriba wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45668/
> ---
> 
> (Updated Abr. 8, 2016, 11:26 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer, Joerg Schad, and Vinod Kone.
> 
> 
> Bugs: MESOS-5101
> https://issues.apache.org/jira/browse/MESOS-5101
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable CMake build for Linux as new CONFIGURATOR option.
> 
> 
> Enable CMake build for Linux as a BUILDTOOL option.
> 
> 
> Enable CMake build for Linux as a BUILDTOOL option.
> 
> 
> Enable CMake build for Linux as a BUILDTOOL option.
> 
> 
> Enable CMake build for Linux as a BUILDTOOL option.
> 
> 
> Enable CMake build for Linux as a BUILDTOOL option.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh e9b1d7219b261475fb29118ee27d11743c2c5e0d 
> 
> Diff: https://reviews.apache.org/r/45668/diff/
> 
> 
> Testing
> ---
> 
> Built using docker_build.sh on both centos:7 and ubuntu:14.04 using both 
> cmake and autotools. In ubuntu:14.04 was built using gcc and clang, in 
> centos:7 only gcc.
> 
> 
> Thanks,
> 
> Juan Larriba
> 
>