Re: Review Request 49067: Added AFFILIATIONS.yml file.

[Artem Harutyunyan]

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

(Updated June 22, 2016, 12:05 a.m.)


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


Repository: mesos


Description
---

Added AFFILIATIONS.yml file.


Diffs (updated)
-

  AFFILIATIONS.yml PRE-CREATION 

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


Testing (updated)
---

Parsed the file with a test script to validate the syntax and generate the list 
of contributors currently missing from the file.


Thanks,

Artem Harutyunyan

Re: Review Request 49054: Fix ContainerizerTest.ROOT_CGROUPS_BalloonFramework.

[Gilbert Song]

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


Ship it!




LGTM! Thanks for taking this.

BTW, I remember I saw similar issue once, in which case tests expecting a 
failure cannot tell which type of failure it is expecting.

- Gilbert Song


On June 21, 2016, 5 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49054/
> ---
> 
> (Updated June 21, 2016, 5 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-5660
> https://issues.apache.org/jira/browse/MESOS-5660
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> https://reviews.apache.org/r/48954/ did not set the 
> `--executor_environment_variables` agent flag in the correct place.
> This caused the balloon framework to fail with flag validation errors.
> Since this test expects a failure, the test would always succeed
> for the wrong reason.
> 
> 
> Diffs
> -
> 
>   src/tests/balloon_framework_test.sh 
> 54129fb11004c86026086f410198afee002ac088 
> 
> Diff: https://reviews.apache.org/r/49054/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 48056: Windows: Added build and run instructions.

[Daniel Pravat]

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

(Updated June 22, 2016, 7:10 a.m.)


Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joris 
Van Remoortere, and Michael Park.


Repository: mesos


Description
---

Windows: Added build and run instructions.


Diffs (updated)
-

  docs/getting-started.md 04d74c12bd86277ee95f9c6418f55900843746a4 

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


Testing
---


Thanks,

Daniel Pravat

Re: Review Request 49041: Removed `launcher` from the Windows build.

[Daniel Pravat]

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

(Updated June 22, 2016, 7:22 a.m.)


Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

Removed `launcher` from the Windows build.


Diffs (updated)
-

  src/launcher/CMakeLists.txt e6ba509bd03ee9c2a18b216ade948f74a7335df7 

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


Testing
---

Windows: build


Thanks,

Daniel Pravat

Re: Review Request 49063: Updating formatting of some lambda calls for readability.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49059, 49060, 49061, 49062, 49063]

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

- Mesos ReviewBot


On June 22, 2016, 1:17 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49063/
> ---
> 
> (Updated June 22, 2016, 1:17 a.m.)
> 
> 
> Review request for mesos, Joerg Schad and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7daaf12a4086635bbc5aba5e3375c95e8899ac6e 
> 
> Diff: https://reviews.apache.org/r/49063/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 49040: Fixed containerizer.cpp break.

[Daniel Pravat]

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

(Updated June 22, 2016, 7:36 a.m.)


Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

Fixed containerizer.cpp break.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
3d971a5a06993a3f9bf3cf0dee00b60daeff5c26 

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


Testing
---

Windows: build


Thanks,

Daniel Pravat

Re: Review Request 49015: Implemented GET_CONTAINERS Call in v1 Agent API.

[Jay Guo]

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

(Updated June 22, 2016, 8:02 a.m.)


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


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


Repository: mesos


Description
---

Implemented GET_CONTAINERS Call in v1 Agent API.


Diffs (updated)
-

  include/mesos/agent/agent.proto 8f5267367857f45dc5c9ab064b183c9c8f1703e9 
  include/mesos/v1/agent/agent.proto 91fb55db004733c3c5bb42acc32f1afedbc208bb 
  src/slave/http.cpp 22b5930a7af336a749af4b6b50254f60433ecea5 
  src/slave/slave.hpp 58ff2bfac6918d989ab36b67cf6ba2f3657c8356 
  src/tests/api_tests.cpp bf1a294f1fc3c8659c31115beee3876d4d0a45e2 

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


Testing (updated)
---

make check on Ubuntu 14.04 & OSX


Thanks,

Jay Guo

Re: Review Request 49015: Implemented GET_CONTAINERS Call in v1 Agent API.

[Jay Guo]

> On June 21, 2016, 11:10 p.m., Vinod Kone wrote:
> > src/slave/http.cpp, line 660
> > 
> >
> > hmm. looks like this is not reusing any code from `_containers()`. can 
> > you extract the common code from `_containers` (e.g., authz) into 
> > `__containers()` and re-use that here. similar to what we did with 
> > `getTasks()`.

I thought of reusing common logic however returned value (JSON for 
`_containers` and v1Response for `getContainers`) is assembled along with the 
workflow. Unless we define a common data structure and evolve it according to 
request type (v1 call or containers endpoint). Do you think it makes sense to 
jsonify v1response for `_containers`?


- Jay


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


On June 22, 2016, 8:02 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49015/
> ---
> 
> (Updated June 22, 2016, 8:02 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5518
> https://issues.apache.org/jira/browse/MESOS-5518
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented GET_CONTAINERS Call in v1 Agent API.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 8f5267367857f45dc5c9ab064b183c9c8f1703e9 
>   include/mesos/v1/agent/agent.proto 91fb55db004733c3c5bb42acc32f1afedbc208bb 
>   src/slave/http.cpp 22b5930a7af336a749af4b6b50254f60433ecea5 
>   src/slave/slave.hpp 58ff2bfac6918d989ab36b67cf6ba2f3657c8356 
>   src/tests/api_tests.cpp bf1a294f1fc3c8659c31115beee3876d4d0a45e2 
> 
> Diff: https://reviews.apache.org/r/49015/diff/
> 
> 
> Testing
> ---
> 
> make check on Ubuntu 14.04 & OSX
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 48938: Added test case `MasterAPITest.CreateAndDetroyVolumes`.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [48938, 48927, 48926]

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

Error:
2016-06-22 08:08:48 URL:https://reviews.apache.org/r/48938/diff/raw/ 
[6457/6457] -> "48938.patch" [1]
error: patch failed: src/tests/api_tests.cpp:938
error: src/tests/api_tests.cpp: patch does not apply

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

- Mesos ReviewBot


On June 22, 2016, 12:53 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48938/
> ---
> 
> (Updated June 22, 2016, 12:53 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5502
> https://issues.apache.org/jira/browse/MESOS-5502
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test case `MasterAPITest.CreateAndDetroyVolumes`.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.hpp 321e0ccf3fdeb30b3529caaa31cc6b565d467e1c 
>   src/internal/evolve.cpp 6b1cf3c4b69d0c39fca6d3bc8482b599494c4805 
>   src/tests/api_tests.cpp 8bc05432c758f8997bc89db1ff772c6910f0883c 
> 
> Diff: https://reviews.apache.org/r/48938/diff/
> 
> 
> Testing
> ---
> 
> "make check" on ubuntu 14.04 64bit with gcc.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Review Request 49069: Checkpointed CNI network config for container in the container dir.

[Qian Zhang]

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

Review request for mesos, Avinash sridharan and Jie Yu.


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


Repository: mesos


Description
---

Checkpointed CNI network config for container in the container dir.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
8cf5fcb1e63873b2a3eca843130a361f25a6e825 
  src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
8af50c11b00240ac1d24605b119acbd96aaa50be 
  src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
b3fb0a32e3e74ca859ca58085efed5a87e4e626b 

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


Testing
---

make check


Thanks,

Qian Zhang

Review Request 49070: Used checkpointed CNI network config during cleanup.

[Qian Zhang]

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

Review request for mesos, Avinash sridharan and Jie Yu.


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


Repository: mesos


Description
---

Used checkpointed CNI network config during cleanup.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
8cf5fcb1e63873b2a3eca843130a361f25a6e825 

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


Testing
---

make check


Thanks,

Qian Zhang

Review Request 49071: Updated CNI test to verify the checkpointed CNI network config.

[Qian Zhang]

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

Review request for mesos, Avinash sridharan and Jie Yu.


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


Repository: mesos


Description
---

Updated CNI test to verify the checkpointed CNI network config.


Diffs
-

  src/tests/containerizer/cni_isolator_tests.cpp 
a51f24b0c4946ffe464e55ca1f25a1a38090e094 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 48438: Implement GET_AGENTS Call in v1 master API.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48835, 48841, 48438]

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

- Mesos ReviewBot


On June 22, 2016, 1:53 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48438/
> ---
> 
> (Updated June 22, 2016, 1:53 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5491
> https://issues.apache.org/jira/browse/mesos-5491
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement the basic getAgents method for v1 operator master API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7daaf12a4086635bbc5aba5e3375c95e8899ac6e 
>   src/master/master.hpp fe57878dc59637459d5c5cdae0be2aa159133fa4 
>   src/tests/api_tests.cpp bf1a294f1fc3c8659c31115beee3876d4d0a45e2 
> 
> Diff: https://reviews.apache.org/r/48438/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 49015: Implemented GET_CONTAINERS Call in v1 Agent API.

[Jay Guo]

> On June 21, 2016, 11:10 p.m., Vinod Kone wrote:
> > src/slave/http.cpp, line 660
> > 
> >
> > hmm. looks like this is not reusing any code from `_containers()`. can 
> > you extract the common code from `_containers` (e.g., authz) into 
> > `__containers()` and re-use that here. similar to what we did with 
> > `getTasks()`.
> 
> Jay Guo wrote:
> I thought of reusing common logic however returned value (JSON for 
> `_containers` and v1Response for `getContainers`) is assembled along with the 
> workflow. Unless we define a common data structure and evolve it according to 
> request type (v1 call or containers endpoint). Do you think it makes sense to 
> jsonify v1response for `_containers`?

And I think it's better to migrate containers endpoint to leverage jsonify 
first and see how we could reuse the logic. I'll investigate it.


- Jay


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


On June 22, 2016, 8:02 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49015/
> ---
> 
> (Updated June 22, 2016, 8:02 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5518
> https://issues.apache.org/jira/browse/MESOS-5518
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented GET_CONTAINERS Call in v1 Agent API.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 8f5267367857f45dc5c9ab064b183c9c8f1703e9 
>   include/mesos/v1/agent/agent.proto 91fb55db004733c3c5bb42acc32f1afedbc208bb 
>   src/slave/http.cpp 22b5930a7af336a749af4b6b50254f60433ecea5 
>   src/slave/slave.hpp 58ff2bfac6918d989ab36b67cf6ba2f3657c8356 
>   src/tests/api_tests.cpp bf1a294f1fc3c8659c31115beee3876d4d0a45e2 
> 
> Diff: https://reviews.apache.org/r/49015/diff/
> 
> 
> Testing
> ---
> 
> make check on Ubuntu 14.04 & OSX
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 49013: Updated cgroups/devices isolator's prepare() to directly return None().

[Qian Zhang]

> On June 22, 2016, 6:05 a.m., Benjamin Mahler wrote:
> > Can you describe why you made this change?
> > 
> > This follows the same pattern as other isolators (e.g. cpushare.cpp, 
> > mem.cpp), so it's hard to tell what the issue is here.
> 
> Qian Zhang wrote:
> I think for `cgroups/devices` isolator, we need to follow the same 
> pattern as `cgroups/perf_event` isolator rather than `cgroups/cpu` or 
> `cgroups/mem`, the reason is `cgroups/devices` isolator does not implement 
> its own `update()` method, so in its `prepare()`, we do not need to call 
> `update()`, instead we can directly return None() just like what 
> `cgroups/perf_event` isolator does. But for `cgroups/cpu` and `cgroups/mem` 
> isolators, they have their own implementation of `update()` method, so we 
> definitely need to call it in their `prepare()` method.
> 
> Benjamin Mahler wrote:
> Got it, I incorrectly thought this was touching the GPU isolator rather 
> than the devices isolator. Can you please add the reasoning into the 
> description before I commit this? The description will become the commit 
> message and it's a helpful way to provide context for posterity.

Sure, I have updated the description by adding the reasoning.


- Qian


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


On June 21, 2016, 4:43 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49013/
> ---
> 
> (Updated June 21, 2016, 4:43 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kevin Klues.
> 
> 
> Bugs: MESOS-5582
> https://issues.apache.org/jira/browse/MESOS-5582
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated cgroups/devices isolator's prepare() to directly return None().
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/devices.cpp 
> 16f962894e89317a436a0e38465338f42bfb83ef 
> 
> Diff: https://reviews.apache.org/r/49013/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 49013: Updated cgroups/devices isolator's prepare() to directly return None().

[Qian Zhang]

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

(Updated June 22, 2016, 5:51 p.m.)


Review request for mesos, Benjamin Mahler and Kevin Klues.


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


Repository: mesos


Description (updated)
---

Updated cgroups/devices isolator's prepare() to directly return None().

cgroups/devices isolator does not implement its own update() method,
so in its prepare(), we do not need to call update(), instead we should
directly return None() in the end.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/devices.cpp 
16f962894e89317a436a0e38465338f42bfb83ef 

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 49064: Implement v1 operator update weights API.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48924, 48925, 48940, 49064]

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

- Mesos ReviewBot


On June 22, 2016, 5:21 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49064/
> ---
> 
> (Updated June 22, 2016, 5:21 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5496
> https://issues.apache.org/jira/browse/mesos-5496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented updateWeights method in WeightsHandler.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 639fbd110df4aca1cf700cb9e455eecc110a7f66 
>   include/mesos/v1/master/master.proto 
> 11dfab318eb073908a9e302afa33b274fec63a16 
>   src/master/http.cpp 7daaf12a4086635bbc5aba5e3375c95e8899ac6e 
>   src/master/master.hpp fe57878dc59637459d5c5cdae0be2aa159133fa4 
>   src/master/weights_handler.cpp 51c957f9ad278e60541b2bde3de2b6090a541ec4 
>   src/tests/api_tests.cpp bf1a294f1fc3c8659c31115beee3876d4d0a45e2 
> 
> Diff: https://reviews.apache.org/r/49064/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 48926: Implemented CREATE_VOLUMES Call in v1 master API.

[Shuai Lin]

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

(Updated June 22, 2016, 11:28 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebased


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


Repository: mesos


Description
---

Implemented CREATE_VOLUMES Call in v1 master API.


Diffs (updated)
-

  src/master/http.cpp 7daaf12a4086635bbc5aba5e3375c95e8899ac6e 
  src/master/master.hpp fe57878dc59637459d5c5cdae0be2aa159133fa4 

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


Testing
---

Please check https://reviews.apache.org/r/48938/ , which adds the test case for 
both create/destroy volumes api.


Thanks,

Shuai Lin

Re: Review Request 48938: Added test case `MasterAPITest.CreateAndDetroyVolumes`.

[Shuai Lin]

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

(Updated June 22, 2016, 11:28 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebased


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


Repository: mesos


Description
---

Added test case `MasterAPITest.CreateAndDetroyVolumes`.


Diffs (updated)
-

  src/internal/evolve.hpp 321e0ccf3fdeb30b3529caaa31cc6b565d467e1c 
  src/internal/evolve.cpp 6b1cf3c4b69d0c39fca6d3bc8482b599494c4805 
  src/tests/api_tests.cpp bf1a294f1fc3c8659c31115beee3876d4d0a45e2 

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


Testing
---

"make check" on ubuntu 14.04 64bit with gcc.


Thanks,

Shuai Lin

Re: Review Request 48927: Implemented DESTROY_VOLUMES Call in v1 master API.

[Shuai Lin]

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

(Updated June 22, 2016, 11:28 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebased


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


Repository: mesos


Description
---

Implemented DESTROY_VOLUMES Call in v1 master API.


Diffs (updated)
-

  src/master/http.cpp 7daaf12a4086635bbc5aba5e3375c95e8899ac6e 
  src/master/master.hpp fe57878dc59637459d5c5cdae0be2aa159133fa4 

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


Testing
---

Please check https://reviews.apache.org/r/48938/ , which adds the test case for 
both create/destroy volumes api.


Thanks,

Shuai Lin

Review Request 49082: Enabled fine grained authorization in the Agent.

[Alexander Rojas]

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

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


Repository: mesos


Description
---

While information about frameworks, executors and tasks were well
protected in the master, this information was not protected in the
agents, which enabled unauthorized users to verify the data by getting
the agent `/state` endpoint. This was particularly pressing since the
Mesos UI would work as a proxy for agents endpoints so even being
behind a firewall would not had been enough.


Diffs
-

  src/slave/http.cpp 22b5930a7af336a749af4b6b50254f60433ecea5 

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


Testing
---

`make check`, some manual testing in the Mesos UI and the following script:

```sh
#! /usr/bin/env bash

rm -rf /tmp/mesos/*

cat < /tmp/credentials.txt
foo bar
baz bar
EOF

cat < /tmp/acls.json
{
  "permissive": false,
  "access_mesos_logs" : [
{
  "principals" : { "values" : ["foo"] },
  "logs" : { "type" : "ANY" }
}
  ],
  "access_sandboxes" : [
{
  "principals" : { "values" : ["foo"] },
  "users" : { "values" : ["$USER"] }
}
  ],
  "view_tasks" : [
{
  "principals" : { "values" : ["foo"] },
  "users" : { "values" : ["$USER"] }
}
  ],
  "view_executors" : [
{
  "principals" : { "values" : ["foo"] },
  "users" : { "values" : ["$USER"] }
}
  ],
  "view_frameworks" : [
{
  "principals" : { "values" : ["foo"] },
  "users" : { "values" : ["$USER"] }
}
  ],
  "register_frameworks" : [
   {
 "principals" : { "values" : ["foo"] },
 "roles" : { "values" : ["test"] }
   }
  ],
  "run_tasks" : [
   {
 "principals" : { "values" : ["foo"] },
 "users" : { "values" : ["$USER"] }
   }
  ],
  "get_endpoints" : [
   {
 "principals" : { "values" : ["foo"] },
 "paths" : { "type" : "ANY" }
   }
  ]
}
EOF

./bin/mesos-master.sh --work_dir=/tmp/mesos/master \
 --authenticate_http \
 --log_dir=/tmp/mesos/logs/master \
 --http_credentials=file:///tmp/credentials.txt \
 --acls=file:///tmp/acls.json &
./bin/mesos-slave.sh --work_dir=/tmp/mesos/slave \
 --master=127.0.0.1:5050 \
 --authenticate_http \
 --http_credentials=file:///tmp/credentials.txt \
 --acls=file:///tmp/acls.json &
./src/mesos-execute  \
  --command='while true; do echo "Hello world"; sleep 3; done' \
  --role=test \
  --master=127.0.0.1:5050 \
  --name=echoer \
  --principal=foo &

# This should show complete information about frameworks, 
# executors and tasks.
http GET http://127.0.0.1:5051/state -a foo:bar

# This should show partial elements of the state but none
# related to the running framework.
http GET http://127.0.0.1:5051/state -a baz:bar

# This should yield a 401 Unauthorized response.
http GET http://127.0.0.1:5051/state -a bar:bar
```


Thanks,

Alexander Rojas

Review Request 49083: Remove lambda capture inconsistencies.

[Alexander Rojas]

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

Review request for mesos, Joerg Schad and Till Toenshoff.


Repository: mesos


Description
---

Some elements of similar types (Owned pointers to other objects)
where capture as references while others were captured as value
in the same lambda.

Note that capturing by reference could be potentially dangerous
in asynchronous situations (which this was not) by causing dangling
pointer errors.

However, pointer objects are easy and small to copy that capturing
by value in these cases is preferable.


Diffs
-

  src/master/http.cpp 7daaf12a4086635bbc5aba5e3375c95e8899ac6e 

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


Testing
---

`make check`


Thanks,

Alexander Rojas

Re: Review Request 49067: Added AFFILIATIONS.yml file.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49067]

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

- Mesos ReviewBot


On June 22, 2016, 7:05 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49067/
> ---
> 
> (Updated June 22, 2016, 7:05 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added AFFILIATIONS.yml file.
> 
> 
> Diffs
> -
> 
>   AFFILIATIONS.yml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49067/diff/
> 
> 
> Testing
> ---
> 
> Parsed the file with a test script to validate the syntax and generate the 
> list of contributors currently missing from the file.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Review Request 49088: Replaced default capture by value by explicit capture by value.

[Alexander Rukletsov]

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

Review request for mesos, Jay Guo, Joris Van Remoortere, and Michael Park.


Repository: mesos


Description
---

When default capture by value is used in a lambda, it's hard
to reason within what context the lambda has to be executed
without auditing the code.


Diffs
-

  src/master/http.cpp 7daaf12a4086635bbc5aba5e3375c95e8899ac6e 

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


Testing
---

make check


Thanks,

Alexander Rukletsov

Re: Review Request 49063: Updating formatting of some lambda calls for readability.

[Alexander Rukletsov]

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

(Updated June 22, 2016, 2:23 p.m.)


Review request for mesos, Joerg Schad and Michael Park.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/http.cpp 7daaf12a4086635bbc5aba5e3375c95e8899ac6e 

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


Testing
---

make check on Mac OS 10.10.4


Thanks,

Alexander Rukletsov

Re: Review Request 48940: Extract public logic of updating weights into _updateWeights.

[haosdent huang]

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




src/master/master.hpp (line 1131)


No sure whether use
```
process::Future_updateWeights(
const Option& principal,
const google::protobuf::RepeatedPtrField&
weightInfos) const;
```
would be better or not. Need @vinodkone and @anandmazumdar helps to check 
this style. XD



src/master/master.hpp (line 1133)


I think could use `__updateWeights` instead of `_doUpdateWeights`.


- haosdent huang


On June 22, 2016, 5:20 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48940/
> ---
> 
> (Updated June 22, 2016, 5:20 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5496
> https://issues.apache.org/jira/browse/mesos-5496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch refined the logic of updating weights in WeightsHandler.
> Extracted the public code into new _updateWeights method so that
> the new operator API can reuse this method to update weights.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp fe57878dc59637459d5c5cdae0be2aa159133fa4 
>   src/master/weights_handler.cpp 51c957f9ad278e60541b2bde3de2b6090a541ec4 
> 
> Diff: https://reviews.apache.org/r/48940/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 49064: Implement v1 operator update weights API.

[haosdent huang]

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




src/master/master.hpp (lines 1113 - 1115)


Indent incorrect here. Only need 4 spaces.



src/master/weights_handler.cpp (line 192)


Suggest to add a check like
```
CHECK(call.update_weights().has_weight_infos());
```



src/master/weights_handler.cpp (line 193)


We need update the validation.cpp as well.

```
case mesos::master::Call::UPDATE_WEIGHTS:
  return None();

```


- haosdent huang


On June 22, 2016, 5:21 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49064/
> ---
> 
> (Updated June 22, 2016, 5:21 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5496
> https://issues.apache.org/jira/browse/mesos-5496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented updateWeights method in WeightsHandler.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 639fbd110df4aca1cf700cb9e455eecc110a7f66 
>   include/mesos/v1/master/master.proto 
> 11dfab318eb073908a9e302afa33b274fec63a16 
>   src/master/http.cpp 7daaf12a4086635bbc5aba5e3375c95e8899ac6e 
>   src/master/master.hpp fe57878dc59637459d5c5cdae0be2aa159133fa4 
>   src/master/weights_handler.cpp 51c957f9ad278e60541b2bde3de2b6090a541ec4 
>   src/tests/api_tests.cpp bf1a294f1fc3c8659c31115beee3876d4d0a45e2 
> 
> Diff: https://reviews.apache.org/r/49064/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 49082: Enabled fine grained authorization in the Agent.

[Alexander Rojas]

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

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


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


Changes
---

Added unit test.


Repository: mesos


Description (updated)
---

While information about frameworks, executors and tasks were well
protected in the master, this information was not protected in the
agents, which enabled unauthorized users to verify the data by getting
the agent `/state` endpoint. This was particularly pressing since the
Mesos UI would work as a proxy for agents endpoints so even being
behind a firewall would not had been enough.


Diffs (updated)
-

  src/slave/http.cpp 22b5930a7af336a749af4b6b50254f60433ecea5 
  src/tests/mesos.hpp b9de9b6dfe0e62df91d5b82006b799345b49fd3c 
  src/tests/mesos.cpp 157979f227083580afc1349373f3e3364c65 
  src/tests/slave_authorization_tests.cpp 
98fde4914a5fddda4401d366e75322458bc9104c 

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


Testing
---

`make check`, some manual testing in the Mesos UI and the following script:

```sh
#! /usr/bin/env bash

rm -rf /tmp/mesos/*

cat < /tmp/credentials.txt
foo bar
baz bar
EOF

cat < /tmp/acls.json
{
  "permissive": false,
  "access_mesos_logs" : [
{
  "principals" : { "values" : ["foo"] },
  "logs" : { "type" : "ANY" }
}
  ],
  "access_sandboxes" : [
{
  "principals" : { "values" : ["foo"] },
  "users" : { "values" : ["$USER"] }
}
  ],
  "view_tasks" : [
{
  "principals" : { "values" : ["foo"] },
  "users" : { "values" : ["$USER"] }
}
  ],
  "view_executors" : [
{
  "principals" : { "values" : ["foo"] },
  "users" : { "values" : ["$USER"] }
}
  ],
  "view_frameworks" : [
{
  "principals" : { "values" : ["foo"] },
  "users" : { "values" : ["$USER"] }
}
  ],
  "register_frameworks" : [
   {
 "principals" : { "values" : ["foo"] },
 "roles" : { "values" : ["test"] }
   }
  ],
  "run_tasks" : [
   {
 "principals" : { "values" : ["foo"] },
 "users" : { "values" : ["$USER"] }
   }
  ],
  "get_endpoints" : [
   {
 "principals" : { "values" : ["foo"] },
 "paths" : { "type" : "ANY" }
   }
  ]
}
EOF

./bin/mesos-master.sh --work_dir=/tmp/mesos/master \
 --authenticate_http \
 --log_dir=/tmp/mesos/logs/master \
 --http_credentials=file:///tmp/credentials.txt \
 --acls=file:///tmp/acls.json &
./bin/mesos-slave.sh --work_dir=/tmp/mesos/slave \
 --master=127.0.0.1:5050 \
 --authenticate_http \
 --http_credentials=file:///tmp/credentials.txt \
 --acls=file:///tmp/acls.json &
./src/mesos-execute  \
  --command='while true; do echo "Hello world"; sleep 3; done' \
  --role=test \
  --master=127.0.0.1:5050 \
  --name=echoer \
  --principal=foo &

# This should show complete information about frameworks, 
# executors and tasks.
http GET http://127.0.0.1:5051/state -a foo:bar

# This should show partial elements of the state but none
# related to the running framework.
http GET http://127.0.0.1:5051/state -a baz:bar

# This should yield a 401 Unauthorized response.
http GET http://127.0.0.1:5051/state -a bar:bar
```


Thanks,

Alexander Rojas

Re: Review Request 49082: Enabled fine grained authorization in the Agent.

[Alexander Rojas]

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

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


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


Repository: mesos


Description (updated)
---

While information about frameworks, executors and tasks were well
protected in the master, this information was not protected in the
agents, which enabled unauthorized users to verify the data by getting
the agent `/state` endpoint. This was particularly pressing since the
Mesos UI would work as a proxy for agents endpoints so even being
behind a firewall would not had been enough.


Diffs
-

  src/slave/http.cpp 22b5930a7af336a749af4b6b50254f60433ecea5 
  src/tests/mesos.hpp b9de9b6dfe0e62df91d5b82006b799345b49fd3c 
  src/tests/mesos.cpp 157979f227083580afc1349373f3e3364c65 
  src/tests/slave_authorization_tests.cpp 
98fde4914a5fddda4401d366e75322458bc9104c 

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


Testing
---

`make check`, some manual testing in the Mesos UI and the following script:

```sh
#! /usr/bin/env bash

rm -rf /tmp/mesos/*

cat < /tmp/credentials.txt
foo bar
baz bar
EOF

cat < /tmp/acls.json
{
  "permissive": false,
  "access_mesos_logs" : [
{
  "principals" : { "values" : ["foo"] },
  "logs" : { "type" : "ANY" }
}
  ],
  "access_sandboxes" : [
{
  "principals" : { "values" : ["foo"] },
  "users" : { "values" : ["$USER"] }
}
  ],
  "view_tasks" : [
{
  "principals" : { "values" : ["foo"] },
  "users" : { "values" : ["$USER"] }
}
  ],
  "view_executors" : [
{
  "principals" : { "values" : ["foo"] },
  "users" : { "values" : ["$USER"] }
}
  ],
  "view_frameworks" : [
{
  "principals" : { "values" : ["foo"] },
  "users" : { "values" : ["$USER"] }
}
  ],
  "register_frameworks" : [
   {
 "principals" : { "values" : ["foo"] },
 "roles" : { "values" : ["test"] }
   }
  ],
  "run_tasks" : [
   {
 "principals" : { "values" : ["foo"] },
 "users" : { "values" : ["$USER"] }
   }
  ],
  "get_endpoints" : [
   {
 "principals" : { "values" : ["foo"] },
 "paths" : { "type" : "ANY" }
   }
  ]
}
EOF

./bin/mesos-master.sh --work_dir=/tmp/mesos/master \
 --authenticate_http \
 --log_dir=/tmp/mesos/logs/master \
 --http_credentials=file:///tmp/credentials.txt \
 --acls=file:///tmp/acls.json &
./bin/mesos-slave.sh --work_dir=/tmp/mesos/slave \
 --master=127.0.0.1:5050 \
 --authenticate_http \
 --http_credentials=file:///tmp/credentials.txt \
 --acls=file:///tmp/acls.json &
./src/mesos-execute  \
  --command='while true; do echo "Hello world"; sleep 3; done' \
  --role=test \
  --master=127.0.0.1:5050 \
  --name=echoer \
  --principal=foo &

# This should show complete information about frameworks, 
# executors and tasks.
http GET http://127.0.0.1:5051/state -a foo:bar

# This should show partial elements of the state but none
# related to the running framework.
http GET http://127.0.0.1:5051/state -a baz:bar

# This should yield a 401 Unauthorized response.
http GET http://127.0.0.1:5051/state -a bar:bar
```


Thanks,

Alexander Rojas

Review Request 49089: Removed a superfluous future repair invocation.

[Alexander Rukletsov]

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

Review request for mesos, Joerg Schad and Till Toenshoff.


Repository: mesos


Description
---

There is no much value in a repair call here if it neither uses the
original failure, nor tries to actually repair the future. In this
case, repair always overwrites the failure returned from the lambda.


Diffs
-

  src/slave/slave.cpp b41f46b6c7bdbf58549635a6f345b4b5f98d5c52 

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


Testing
---

make check


Thanks,

Alexander Rukletsov

Review Request 49090: Improved formatting of Slave::authorizeSandboxAccess for readability.

[Alexander Rukletsov]

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

Review request for mesos, Joerg Schad and Till Toenshoff.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/slave/slave.cpp b41f46b6c7bdbf58549635a6f345b4b5f98d5c52 

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


Testing
---

None: Not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 49041: Removed `launcher` from the Windows build.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48056, 49037, 49038, 49039, 49040, 49041]

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

- Mesos ReviewBot


On June 22, 2016, 7:22 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49041/
> ---
> 
> (Updated June 22, 2016, 7:22 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed `launcher` from the Windows build.
> 
> 
> Diffs
> -
> 
>   src/launcher/CMakeLists.txt e6ba509bd03ee9c2a18b216ade948f74a7335df7 
> 
> Diff: https://reviews.apache.org/r/49041/diff/
> 
> 
> Testing
> ---
> 
> Windows: build
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Review Request 49095: Enabled fine-grained authorization in the master's frameworks endpoint.

[Alexander Rojas]

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

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


Repository: mesos


Description
---

Even if ACLs were defined for the actions `VIEW_FRAMEWORKS`,
`VIEW_EXECUTORS` and `VIEW_TASKS`, the data these actions were
supposed to protect, could still leaked through the master's
`/frameworks` endpoint, since it didn't enable any authorization
mechanism.


Diffs
-

  src/master/http.cpp 7daaf12a4086635bbc5aba5e3375c95e8899ac6e 
  src/tests/master_authorization_tests.cpp 
9ae6b9dc89538716d397ffd8826187e2bc6d7e8f 

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


Testing
---

`make check`


Thanks,

Alexander Rojas

Re: Review Request 48751: Implement GetState response for master API.

[Zhitao Li]

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




include/mesos/master/master.proto (line 314)


Agreed. I think changing `required` mentioned in this patch to `optional` 
is good since the API here is still progressing.

@vinodkone, what's your opinion?


- Zhitao Li


On June 22, 2016, 6:13 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48751/
> ---
> 
> (Updated June 22, 2016, 6:13 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5489
> https://issues.apache.org/jira/browse/MESOS-5489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement `GetState` response for master API.
> 
> The response protobuf message and new function `_getState`
> will be reused for generating snapshot for Subscribe call in next patch.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 639fbd110df4aca1cf700cb9e455eecc110a7f66 
>   include/mesos/v1/master/master.proto 
> 11dfab318eb073908a9e302afa33b274fec63a16 
>   src/master/http.cpp 7daaf12a4086635bbc5aba5e3375c95e8899ac6e 
>   src/master/master.hpp fe57878dc59637459d5c5cdae0be2aa159133fa4 
>   src/tests/api_tests.cpp bf1a294f1fc3c8659c31115beee3876d4d0a45e2 
> 
> Diff: https://reviews.apache.org/r/48751/diff/
> 
> 
> Testing
> ---
> 
> New test in MasterAPITest.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 48751: Implement GetState response for master API.

[Zhitao Li]

> On June 22, 2016, 6:29 a.m., Jay Guo wrote:
> > src/master/http.cpp, lines 1477-1500
> > 
> >
> > I feel we can extract this logic to a common function since it's being 
> > used by getTasks as well.

Sounds good although I might want to do it in a follow up patch. 

How does a helper funciton `vector>> 
createApprovers(Optional principal, const 
vector& actions)` sounds to you?


- Zhitao


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


On June 22, 2016, 6:13 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48751/
> ---
> 
> (Updated June 22, 2016, 6:13 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5489
> https://issues.apache.org/jira/browse/MESOS-5489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement `GetState` response for master API.
> 
> The response protobuf message and new function `_getState`
> will be reused for generating snapshot for Subscribe call in next patch.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 639fbd110df4aca1cf700cb9e455eecc110a7f66 
>   include/mesos/v1/master/master.proto 
> 11dfab318eb073908a9e302afa33b274fec63a16 
>   src/master/http.cpp 7daaf12a4086635bbc5aba5e3375c95e8899ac6e 
>   src/master/master.hpp fe57878dc59637459d5c5cdae0be2aa159133fa4 
>   src/tests/api_tests.cpp bf1a294f1fc3c8659c31115beee3876d4d0a45e2 
> 
> Diff: https://reviews.apache.org/r/48751/diff/
> 
> 
> Testing
> ---
> 
> New test in MasterAPITest.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49071: Updated CNI test to verify the checkpointed CNI network config.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49069, 49070, 49071]

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

- Mesos ReviewBot


On June 22, 2016, 8:34 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49071/
> ---
> 
> (Updated June 22, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-5310
> https://issues.apache.org/jira/browse/MESOS-5310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated CNI test to verify the checkpointed CNI network config.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> a51f24b0c4946ffe464e55ca1f25a1a38090e094 
> 
> Diff: https://reviews.apache.org/r/49071/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 47999: Added calico information for CNI.

[Avinash sridharan]

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


Ship it!




Ship It!

- Avinash sridharan


On June 22, 2016, 4:40 p.m., Dan Osborne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47999/
> ---
> 
> (Updated June 22, 2016, 4:40 p.m.)
> 
> 
> Review request for mesos and Avinash sridharan.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added calico information for CNI.
> 
> 
> Diffs
> -
> 
>   docs/cni.md cdc50da9db2f19f27be6a39f4d6e0f54834fb964 
> 
> Diff: https://reviews.apache.org/r/47999/diff/
> 
> 
> Testing
> ---
> 
> https://github.com/apache/mesos/compare/master...djosborne:calico-cni-docs?expand=1&name=calico-cni-docs&short_path=088dbb7#diff-088dbb7685be5d67a99111cd00dd3db8
> 
> 
> Thanks,
> 
> Dan Osborne
> 
>

Re: Review Request 49041: Removed `launcher` from the Windows build.

[Daniel Pravat]

> On June 22, 2016, 12:05 a.m., Joseph Wu wrote:
> > src/launcher/CMakeLists.txt, lines 39-41
> > 
> >
> > This results in:
> > ```
> > CMake Error at src/launcher/CMakeLists.txt:57 (add_executable):
> >   Cannot find source file:
> > 
> > if
> > 
> >   Tried extensions .c .C .c++ .cc .cpp .cxx .m .M .mm .h .hh .h++ .hm 
> > .hpp
> >   .hxx .in .txx
> > 
> > 
> > -- Generating done
> > ```

CMake caching.


- Daniel


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


On June 22, 2016, 7:22 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49041/
> ---
> 
> (Updated June 22, 2016, 7:22 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed `launcher` from the Windows build.
> 
> 
> Diffs
> -
> 
>   src/launcher/CMakeLists.txt e6ba509bd03ee9c2a18b216ade948f74a7335df7 
> 
> Diff: https://reviews.apache.org/r/49041/diff/
> 
> 
> Testing
> ---
> 
> Windows: build
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 47999: Added calico information for CNI.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 22, 2016, 4:40 p.m., Dan Osborne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47999/
> ---
> 
> (Updated June 22, 2016, 4:40 p.m.)
> 
> 
> Review request for mesos and Avinash sridharan.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added calico information for CNI.
> 
> 
> Diffs
> -
> 
>   docs/cni.md cdc50da9db2f19f27be6a39f4d6e0f54834fb964 
> 
> Diff: https://reviews.apache.org/r/47999/diff/
> 
> 
> Testing
> ---
> 
> https://github.com/apache/mesos/compare/master...djosborne:calico-cni-docs?expand=1&name=calico-cni-docs&short_path=088dbb7#diff-088dbb7685be5d67a99111cd00dd3db8
> 
> 
> Thanks,
> 
> Dan Osborne
> 
>

Re: Review Request 48751: Implement GetState response for master API.

[Zhitao Li]

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

(Updated June 22, 2016, 5 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Make protobuf fields optional.


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


Repository: mesos


Description
---

Implement `GetState` response for master API.

The response protobuf message and new function `_getState`
will be reused for generating snapshot for Subscribe call in next patch.


Diffs (updated)
-

  include/mesos/master/master.proto 639fbd110df4aca1cf700cb9e455eecc110a7f66 
  include/mesos/v1/master/master.proto 11dfab318eb073908a9e302afa33b274fec63a16 
  src/master/http.cpp 7daaf12a4086635bbc5aba5e3375c95e8899ac6e 
  src/master/master.hpp fe57878dc59637459d5c5cdae0be2aa159133fa4 
  src/tests/api_tests.cpp bf1a294f1fc3c8659c31115beee3876d4d0a45e2 

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


Testing
---

New test in MasterAPITest.


Thanks,

Zhitao Li

Re: Review Request 47999: Added calico information for CNI.

[Jie Yu]

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



Can you rebase. the patch does not apply with the current trunk

- Jie Yu


On June 22, 2016, 4:40 p.m., Dan Osborne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47999/
> ---
> 
> (Updated June 22, 2016, 4:40 p.m.)
> 
> 
> Review request for mesos and Avinash sridharan.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added calico information for CNI.
> 
> 
> Diffs
> -
> 
>   docs/cni.md cdc50da9db2f19f27be6a39f4d6e0f54834fb964 
> 
> Diff: https://reviews.apache.org/r/47999/diff/
> 
> 
> Testing
> ---
> 
> https://github.com/apache/mesos/compare/master...djosborne:calico-cni-docs?expand=1&name=calico-cni-docs&short_path=088dbb7#diff-088dbb7685be5d67a99111cd00dd3db8
> 
> 
> Thanks,
> 
> Dan Osborne
> 
>

Re: Review Request 48988: Reduced test linux rootfs size.

[Jie Yu]

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




src/tests/containerizer/rootfs.hpp (line 104)


This should be called 'files'?



src/tests/containerizer/rootfs.hpp (line 137)


I am not sure if this logic still applies. For instance, if `/lib64 -> 
/xxx/lib64`.

The realpath of `/lib64/librt.so.1` is `/xxx/lib64/librt.so.1`. However, 
`islink('/lib64/librt.so.1') == false`.

I think you should test if `realpath.get() == file`?


- Jie Yu


On June 21, 2016, 2:45 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48988/
> ---
> 
> (Updated June 21, 2016, 2:45 a.m.)
> 
> 
> Review request for mesos, Zhiwei Chen, haosdent huang, Artem Harutyunyan, Jie 
> Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4943
> https://issues.apache.org/jira/browse/MESOS-4943
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reduced test linux rootfs size.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 19ec7bf50e599f5420453e8cb7d12357770d3fd8 
> 
> Diff: https://reviews.apache.org/r/48988/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48988: Reduced test linux rootfs size.

[Jie Yu]

> On June 21, 2016, 3:09 a.m., haosdent huang wrote:
> > src/tests/containerizer/rootfs.hpp, line 109
> > 
> >
> > +1 for this, it would reduce a lot of test cases running time.
> > @zhiwei may worry about running this on PowerPC, maybe we need find the 
> > corresponding path in PowerPC as well.

@zhiwei, we'll ship this for now. Let us konw if it breaks root tests on 
powerpc or not.


- Jie


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


On June 21, 2016, 2:45 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48988/
> ---
> 
> (Updated June 21, 2016, 2:45 a.m.)
> 
> 
> Review request for mesos, Zhiwei Chen, haosdent huang, Artem Harutyunyan, Jie 
> Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4943
> https://issues.apache.org/jira/browse/MESOS-4943
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reduced test linux rootfs size.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 19ec7bf50e599f5420453e8cb7d12357770d3fd8 
> 
> Diff: https://reviews.apache.org/r/48988/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48989: Fixed broken cni isolator test on centos7.

[Jie Yu]

> On June 22, 2016, 2:20 a.m., Qian Zhang wrote:
> > src/tests/containerizer/cni_isolator_tests.cpp, line 185
> > 
> >
> > I just manually ran a Docker container from 'alpine' image in an RHEL 
> > 7.1 machine, and `/bin/sh -c 'ifconfig'` can work successfully. I thought 
> > there should be no much difference between RHEL and CentOS, so not sure why 
> > it does not work in CentOS ...

Qian, i think it depends on your shell.


- Jie


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


On June 21, 2016, 2:46 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48989/
> ---
> 
> (Updated June 21, 2016, 2:46 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Guangya Liu, Artem Harutyunyan, 
> Jie Yu, and Qian Zhang.
> 
> 
> Bugs: MESOS-5667
> https://issues.apache.org/jira/browse/MESOS-5667
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed broken cni isolator test on centos7.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> a51f24b0c4946ffe464e55ca1f25a1a38090e094 
> 
> Diff: https://reviews.apache.org/r/48989/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48989: Fixed broken cni isolator test on centos7.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 21, 2016, 2:46 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48989/
> ---
> 
> (Updated June 21, 2016, 2:46 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Guangya Liu, Artem Harutyunyan, 
> Jie Yu, and Qian Zhang.
> 
> 
> Bugs: MESOS-5667
> https://issues.apache.org/jira/browse/MESOS-5667
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed broken cni isolator test on centos7.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> a51f24b0c4946ffe464e55ca1f25a1a38090e094 
> 
> Diff: https://reviews.apache.org/r/48989/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 49054: Fix ContainerizerTest.ROOT_CGROUPS_BalloonFramework.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 22, 2016, midnight, Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49054/
> ---
> 
> (Updated June 22, 2016, midnight)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-5660
> https://issues.apache.org/jira/browse/MESOS-5660
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> https://reviews.apache.org/r/48954/ did not set the 
> `--executor_environment_variables` agent flag in the correct place.
> This caused the balloon framework to fail with flag validation errors.
> Since this test expects a failure, the test would always succeed
> for the wrong reason.
> 
> 
> Diffs
> -
> 
>   src/tests/balloon_framework_test.sh 
> 54129fb11004c86026086f410198afee002ac088 
> 
> Diff: https://reviews.apache.org/r/49054/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47999: Added calico information for CNI.

[Dan Osborne]

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

(Updated June 22, 2016, 5:33 p.m.)


Review request for mesos and Avinash sridharan.


Repository: mesos


Description
---

Added calico information for CNI.


Diffs (updated)
-

  docs/cni.md e58e2a6359528b2a78e6a08a7ba14199b710e2d4 

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


Testing
---

https://github.com/apache/mesos/compare/master...djosborne:calico-cni-docs?expand=1&name=calico-cni-docs&short_path=088dbb7#diff-088dbb7685be5d67a99111cd00dd3db8


Thanks,

Dan Osborne

Re: Review Request 48990: Added CGROUP namespace to ns helper.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 21, 2016, 2:46 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48990/
> ---
> 
> (Updated June 21, 2016, 2:46 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Artem Harutyunyan, Jie Yu, Qian 
> Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5668
> https://issues.apache.org/jira/browse/MESOS-5668
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added CGROUP namespace to ns helper.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp 244a811b299c29b1dcd6652bd26e861e04df3f54 
> 
> Diff: https://reviews.apache.org/r/48990/diff/
> 
> 
> Testing
> ---
> 
> make check on Ubuntu 16
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48993: Fixed memory pressure test cgroup slave recovery.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 21, 2016, 2:46 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48993/
> ---
> 
> (Updated June 21, 2016, 2:46 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, Artem 
> Harutyunyan, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-5670
> https://issues.apache.org/jira/browse/MESOS-5670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed memory pressure test cgroup slave recovery.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> 4f343af7e44b5ecfee4b1a31cdc46876a15cdbbd 
> 
> Diff: https://reviews.apache.org/r/48993/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./mesos-test.sh --gtest_filter=... with 500+ iterations
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48992: Fixed memory pressure test cgroup statistics.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 21, 2016, 2:46 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48992/
> ---
> 
> (Updated June 21, 2016, 2:46 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, Artem 
> Harutyunyan, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-5671
> https://issues.apache.org/jira/browse/MESOS-5671
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed memory pressure test cgroup statistics.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> 4f343af7e44b5ecfee4b1a31cdc46876a15cdbbd 
> 
> Diff: https://reviews.apache.org/r/48992/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./mesos-test.sh --gtest_filter=... with 500+ iterations
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48994: Fixed portmapping isolator bind mount root non-existed case.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 21, 2016, 2:46 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48994/
> ---
> 
> (Updated June 21, 2016, 2:46 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Artem Harutyunyan, Jie Yu, and Cong 
> Wang.
> 
> 
> Bugs: MESOS-5673
> https://issues.apache.org/jira/browse/MESOS-5673
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed portmapping isolator bind mount root non-existed case.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 5df591893f3015926a7cfe07e9b57749c46f6367 
> 
> Diff: https://reviews.apache.org/r/48994/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48995: Fixed portmapping isolator symlink existed failure.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 2587)


Instead of doing that, can you use a uuid for container id in the 
corresponding test?


- Jie Yu


On June 21, 2016, 2:46 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48995/
> ---
> 
> (Updated June 21, 2016, 2:46 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Artem Harutyunyan, Jie Yu, and Cong 
> Wang.
> 
> 
> Bugs: MESOS-5674
> https://issues.apache.org/jira/browse/MESOS-5674
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed portmapping isolator symlink existed failure.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 5df591893f3015926a7cfe07e9b57749c46f6367 
> 
> Diff: https://reviews.apache.org/r/48995/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48996: Added space for ROOT_NC_HostToContainerUDP shell commands.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 21, 2016, 2:46 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48996/
> ---
> 
> (Updated June 21, 2016, 2:46 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added space for ROOT_NC_HostToContainerUDP shell commands.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 21ad1e1c53316a3bb6d914aa228ccf3658acdfbf 
> 
> Diff: https://reviews.apache.org/r/48996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 49037: Removed mesos-Containerizer-memory_test in Windows.

[Joseph Wu]

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


Ship it!




Built on a couple Linux boxes.


src/CMakeLists.txt (line 494)


This is already defined above in line 453.

I'll remove it before committing.


- Joseph Wu


On June 21, 2016, 1:36 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49037/
> ---
> 
> (Updated June 21, 2016, 1:36 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed mesos-Containerizer-memory_test in Windows.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt d66186217c1319d4497640614ed4beee28602c38 
> 
> Diff: https://reviews.apache.org/r/49037/diff/
> 
> 
> Testing
> ---
> 
> Windows: build
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 48902: Move v1/master/allocator.proto to its own package.

[Zhitao Li]

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

(Updated June 22, 2016, 6:19 p.m.)


Review request for mesos, Anand Mazumdar, Joris Van Remoortere, Joseph Wu, and 
Vinod Kone.


Changes
---

Partial review comments for dealing with v1 part only. Non versioned message 
will be handled separately.


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


Repository: mesos


Description
---

Move v1/master/allocator.proto to its own package.


Diffs (updated)
-

  include/mesos/v1/maintenance/maintenance.proto 
053d72d7d9fa9ec8f572136020546fa4f955ab09 
  include/mesos/v1/master/allocator.proto 
cf416d173bc487aecbbec75c9ee391a54bf5327b 
  src/CMakeLists.txt d66186217c1319d4497640614ed4beee28602c38 
  src/Makefile.am 23cac95a805e46d216e9479fea09f2c1619c45a5 

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


Testing
---

run `make` on Mac.


Thanks,

Zhitao Li

Re: Review Request 48994: Fixed portmapping isolator bind mount root non-existed case.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 1894)


Hum, just realized an issue. If you do that, bindMountRoot is no longer 
guaranteed to be realpath, right? Can you just move mdkir to the top?


- Jie Yu


On June 21, 2016, 2:46 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48994/
> ---
> 
> (Updated June 21, 2016, 2:46 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Artem Harutyunyan, Jie Yu, and Cong 
> Wang.
> 
> 
> Bugs: MESOS-5673
> https://issues.apache.org/jira/browse/MESOS-5673
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed portmapping isolator bind mount root non-existed case.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 5df591893f3015926a7cfe07e9b57749c46f6367 
> 
> Diff: https://reviews.apache.org/r/48994/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 49038: Fixed store.cpp break.

[Joseph Wu]

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


Ship it!





src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 98)


I'll fix this typo before committing.


- Joseph Wu


On June 21, 2016, 3:22 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49038/
> ---
> 
> (Updated June 21, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed store.cpp break.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> a4fccb31cbbd5638eb9341b346dccf191ef4b1b8 
> 
> Diff: https://reviews.apache.org/r/49038/diff/
> 
> 
> Testing
> ---
> 
> Windows: build
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 48902: Move v1/master/allocator.proto to its own package.

[Zhitao Li]

> On June 21, 2016, 12:36 a.m., Vinod Kone wrote:
> > src/CMakeLists.txt, line 24
> > 
> >
> > don't you need to change this?

I'd like to address this in a separate patch, and I have some open question on 
how to handle `mesos::master::allocator::Allocator` class and its namespace. 
See my last question in MESOS-5642.


- Zhitao


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


On June 22, 2016, 6:19 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48902/
> ---
> 
> (Updated June 22, 2016, 6:19 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-5642
> https://issues.apache.org/jira/browse/MESOS-5642
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Move v1/master/allocator.proto to its own package.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/maintenance/maintenance.proto 
> 053d72d7d9fa9ec8f572136020546fa4f955ab09 
>   include/mesos/v1/master/allocator.proto 
> cf416d173bc487aecbbec75c9ee391a54bf5327b 
>   src/CMakeLists.txt d66186217c1319d4497640614ed4beee28602c38 
>   src/Makefile.am 23cac95a805e46d216e9479fea09f2c1619c45a5 
> 
> Diff: https://reviews.apache.org/r/48902/diff/
> 
> 
> Testing
> ---
> 
> run `make` on Mac.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49039: Fixed executor.hpp break.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On June 21, 2016, 12:19 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49039/
> ---
> 
> (Updated June 21, 2016, 12:19 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed executor.hpp break.
> 
> 
> Diffs
> -
> 
>   src/launcher/windows/executor.hpp 0635a2ac6e07b14efbe86a4c3aac214fd615ad19 
> 
> Diff: https://reviews.apache.org/r/49039/diff/
> 
> 
> Testing
> ---
> 
> Windows: build
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 49040: Fixed containerizer.cpp break.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On June 22, 2016, 12:36 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49040/
> ---
> 
> (Updated June 22, 2016, 12:36 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed containerizer.cpp break.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 3d971a5a06993a3f9bf3cf0dee00b60daeff5c26 
> 
> Diff: https://reviews.apache.org/r/49040/diff/
> 
> 
> Testing
> ---
> 
> Windows: build
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 49041: Removed `launcher` from the Windows build.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On June 22, 2016, 12:22 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49041/
> ---
> 
> (Updated June 22, 2016, 12:22 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed `launcher` from the Windows build.
> 
> 
> Diffs
> -
> 
>   src/launcher/CMakeLists.txt e6ba509bd03ee9c2a18b216ade948f74a7335df7 
> 
> Diff: https://reviews.apache.org/r/49041/diff/
> 
> 
> Testing
> ---
> 
> Windows: build
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 48991: Added exception for etc hostname mount in cni isolator.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 1493 - 1495)


Let's not use glog in a subprocess. Please do not log anything.


- Jie Yu


On June 21, 2016, 6:11 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48991/
> ---
> 
> (Updated June 21, 2016, 6:11 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Guangya Liu, Artem Harutyunyan, 
> Jie Yu, and Qian Zhang.
> 
> 
> Bugs: MESOS-5669
> https://issues.apache.org/jira/browse/MESOS-5669
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added exception for etc hostname mount in cni isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 9fc709e95ecff7e97e159ed05a4efcf4e887d5e0 
> 
> Diff: https://reviews.apache.org/r/48991/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 49015: Implemented GET_CONTAINERS Call in v1 Agent API.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49015]

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

- Mesos ReviewBot


On June 22, 2016, 8:02 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49015/
> ---
> 
> (Updated June 22, 2016, 8:02 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5518
> https://issues.apache.org/jira/browse/MESOS-5518
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented GET_CONTAINERS Call in v1 Agent API.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 8f5267367857f45dc5c9ab064b183c9c8f1703e9 
>   include/mesos/v1/agent/agent.proto 91fb55db004733c3c5bb42acc32f1afedbc208bb 
>   src/slave/http.cpp 22b5930a7af336a749af4b6b50254f60433ecea5 
>   src/slave/slave.hpp 58ff2bfac6918d989ab36b67cf6ba2f3657c8356 
>   src/tests/api_tests.cpp bf1a294f1fc3c8659c31115beee3876d4d0a45e2 
> 
> Diff: https://reviews.apache.org/r/49015/diff/
> 
> 
> Testing
> ---
> 
> make check on Ubuntu 14.04 & OSX
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 48988: Reduced test linux rootfs size.

[Gilbert Song]

> On June 22, 2016, 10:23 a.m., Jie Yu wrote:
> > src/tests/containerizer/rootfs.hpp, line 142
> > 
> >
> > I am not sure if this logic still applies. For instance, if `/lib64 -> 
> > /xxx/lib64`.
> > 
> > The realpath of `/lib64/librt.so.1` is `/xxx/lib64/librt.so.1`. 
> > However, `islink('/lib64/librt.so.1') == false`.
> > 
> > I think you should test if `realpath.get() == file`?

The logic should not be changed here. It should be identical to the old logic:

1. get realpath and copy the file.
2. check islink() and copy the link.

For the files listed, some of them are symlink, depending on diff OS. So for 
example, the realpath of `/lib/x86_64-linux-gnu` is 
`/lib/x86_64-linux-gnu/ld-2.19.so`, and in this case 
`islink('/lib/x86_64-linux-gnu')` is not false.

I dont understand why you want to do the `realpath.get() == file` check here. 
If it is a file, it was already copied sbove. We only want to handle all the 
symlinks here altogether. 

PS: In CentOS and Fedora, `/usr/bin/sh` is a symlink as well, pointing to 
`/usr/bin/bash`. They are pretty common cases that some executable/library 
files are symlink.


- Gilbert


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


On June 20, 2016, 7:45 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48988/
> ---
> 
> (Updated June 20, 2016, 7:45 p.m.)
> 
> 
> Review request for mesos, Zhiwei Chen, haosdent huang, Artem Harutyunyan, Jie 
> Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4943
> https://issues.apache.org/jira/browse/MESOS-4943
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reduced test linux rootfs size.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 19ec7bf50e599f5420453e8cb7d12357770d3fd8 
> 
> Diff: https://reviews.apache.org/r/48988/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48988: Reduced test linux rootfs size.

[Gilbert Song]

> On June 20, 2016, 8:09 p.m., haosdent huang wrote:
> > src/tests/containerizer/rootfs.hpp, line 109
> > 
> >
> > +1 for this, it would reduce a lot of test cases running time.
> > @zhiwei may worry about running this on PowerPC, maybe we need find the 
> > corresponding path in PowerPC as well.
> 
> Jie Yu wrote:
> @zhiwei, we'll ship this for now. Let us konw if it breaks root tests on 
> powerpc or not.

Thanks haosdent, I added Zhiwei for this reason:)


- Gilbert


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


On June 20, 2016, 7:45 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48988/
> ---
> 
> (Updated June 20, 2016, 7:45 p.m.)
> 
> 
> Review request for mesos, Zhiwei Chen, haosdent huang, Artem Harutyunyan, Jie 
> Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4943
> https://issues.apache.org/jira/browse/MESOS-4943
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reduced test linux rootfs size.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 19ec7bf50e599f5420453e8cb7d12357770d3fd8 
> 
> Diff: https://reviews.apache.org/r/48988/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48988: Reduced test linux rootfs size.

[Jie Yu]

> On June 22, 2016, 5:23 p.m., Jie Yu wrote:
> > src/tests/containerizer/rootfs.hpp, line 142
> > 
> >
> > I am not sure if this logic still applies. For instance, if `/lib64 -> 
> > /xxx/lib64`.
> > 
> > The realpath of `/lib64/librt.so.1` is `/xxx/lib64/librt.so.1`. 
> > However, `islink('/lib64/librt.so.1') == false`.
> > 
> > I think you should test if `realpath.get() == file`?
> 
> Gilbert Song wrote:
> The logic should not be changed here. It should be identical to the old 
> logic:
> 
> 1. get realpath and copy the file.
> 2. check islink() and copy the link.
> 
> For the files listed, some of them are symlink, depending on diff OS. So 
> for example, the realpath of `/lib/x86_64-linux-gnu` is 
> `/lib/x86_64-linux-gnu/ld-2.19.so`, and in this case 
> `islink('/lib/x86_64-linux-gnu')` is not false.
> 
> I dont understand why you want to do the `realpath.get() == file` check 
> here. If it is a file, it was already copied sbove. We only want to handle 
> all the symlinks here altogether. 
> 
> PS: In CentOS and Fedora, `/usr/bin/sh` is a symlink as well, pointing to 
> `/usr/bin/bash`. They are pretty common cases that some executable/library 
> files are symlink.

what if the file itself is not a link, but the directory that contains it is a 
symlink?


- Jie


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


On June 21, 2016, 2:45 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48988/
> ---
> 
> (Updated June 21, 2016, 2:45 a.m.)
> 
> 
> Review request for mesos, Zhiwei Chen, haosdent huang, Artem Harutyunyan, Jie 
> Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4943
> https://issues.apache.org/jira/browse/MESOS-4943
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reduced test linux rootfs size.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 19ec7bf50e599f5420453e8cb7d12357770d3fd8 
> 
> Diff: https://reviews.apache.org/r/48988/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48902: Move v1/master/allocator.proto to its own package.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 22, 2016, 6:19 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48902/
> ---
> 
> (Updated June 22, 2016, 6:19 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-5642
> https://issues.apache.org/jira/browse/MESOS-5642
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Move v1/master/allocator.proto to its own package.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/maintenance/maintenance.proto 
> 053d72d7d9fa9ec8f572136020546fa4f955ab09 
>   include/mesos/v1/master/allocator.proto 
> cf416d173bc487aecbbec75c9ee391a54bf5327b 
>   src/CMakeLists.txt d66186217c1319d4497640614ed4beee28602c38 
>   src/Makefile.am 23cac95a805e46d216e9479fea09f2c1619c45a5 
> 
> Diff: https://reviews.apache.org/r/48902/diff/
> 
> 
> Testing
> ---
> 
> run `make` on Mac.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 48902: Move v1/master/allocator.proto to its own package.

[Vinod Kone]

> On June 21, 2016, 12:36 a.m., Vinod Kone wrote:
> > src/CMakeLists.txt, line 24
> > 
> >
> > don't you need to change this?
> 
> Zhitao Li wrote:
> I'd like to address this in a separate patch, and I have some open 
> question on how to handle `mesos::master::allocator::Allocator` class and its 
> namespace. See my last question in MESOS-5642.

ok. follow up patch sounds fine. dropping these now.


- Vinod


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


On June 22, 2016, 6:19 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48902/
> ---
> 
> (Updated June 22, 2016, 6:19 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-5642
> https://issues.apache.org/jira/browse/MESOS-5642
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Move v1/master/allocator.proto to its own package.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/maintenance/maintenance.proto 
> 053d72d7d9fa9ec8f572136020546fa4f955ab09 
>   include/mesos/v1/master/allocator.proto 
> cf416d173bc487aecbbec75c9ee391a54bf5327b 
>   src/CMakeLists.txt d66186217c1319d4497640614ed4beee28602c38 
>   src/Makefile.am 23cac95a805e46d216e9479fea09f2c1619c45a5 
> 
> Diff: https://reviews.apache.org/r/48902/diff/
> 
> 
> Testing
> ---
> 
> run `make` on Mac.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 48988: Reduced test linux rootfs size.

[Gilbert Song]

> On June 22, 2016, 10:23 a.m., Jie Yu wrote:
> > src/tests/containerizer/rootfs.hpp, line 142
> > 
> >
> > I am not sure if this logic still applies. For instance, if `/lib64 -> 
> > /xxx/lib64`.
> > 
> > The realpath of `/lib64/librt.so.1` is `/xxx/lib64/librt.so.1`. 
> > However, `islink('/lib64/librt.so.1') == false`.
> > 
> > I think you should test if `realpath.get() == file`?
> 
> Gilbert Song wrote:
> The logic should not be changed here. It should be identical to the old 
> logic:
> 
> 1. get realpath and copy the file.
> 2. check islink() and copy the link.
> 
> For the files listed, some of them are symlink, depending on diff OS. So 
> for example, the realpath of `/lib/x86_64-linux-gnu` is 
> `/lib/x86_64-linux-gnu/ld-2.19.so`, and in this case 
> `islink('/lib/x86_64-linux-gnu')` is not false.
> 
> I dont understand why you want to do the `realpath.get() == file` check 
> here. If it is a file, it was already copied sbove. We only want to handle 
> all the symlinks here altogether. 
> 
> PS: In CentOS and Fedora, `/usr/bin/sh` is a symlink as well, pointing to 
> `/usr/bin/bash`. They are pretty common cases that some executable/library 
> files are symlink.
> 
> Jie Yu wrote:
> what if the file itself is not a link, but the directory that contains it 
> is a symlink?

Gotcha.


- Gilbert


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


On June 20, 2016, 7:45 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48988/
> ---
> 
> (Updated June 20, 2016, 7:45 p.m.)
> 
> 
> Review request for mesos, Zhiwei Chen, haosdent huang, Artem Harutyunyan, Jie 
> Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4943
> https://issues.apache.org/jira/browse/MESOS-4943
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reduced test linux rootfs size.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/rootfs.hpp 19ec7bf50e599f5420453e8cb7d12357770d3fd8 
> 
> Diff: https://reviews.apache.org/r/48988/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48938: Added test case `MasterAPITest.CreateAndDetroyVolumes`.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48926, 48927, 48938]

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

- Mesos ReviewBot


On June 22, 2016, 11:28 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48938/
> ---
> 
> (Updated June 22, 2016, 11:28 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5502
> https://issues.apache.org/jira/browse/MESOS-5502
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test case `MasterAPITest.CreateAndDetroyVolumes`.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.hpp 321e0ccf3fdeb30b3529caaa31cc6b565d467e1c 
>   src/internal/evolve.cpp 6b1cf3c4b69d0c39fca6d3bc8482b599494c4805 
>   src/tests/api_tests.cpp bf1a294f1fc3c8659c31115beee3876d4d0a45e2 
> 
> Diff: https://reviews.apache.org/r/48938/diff/
> 
> 
> Testing
> ---
> 
> "make check" on ubuntu 14.04 64bit with gcc.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 48989: Fixed broken cni isolator test on centos7.

[Gilbert Song]

> On June 21, 2016, 7:20 p.m., Qian Zhang wrote:
> > src/tests/containerizer/cni_isolator_tests.cpp, line 185
> > 
> >
> > I just manually ran a Docker container from 'alpine' image in an RHEL 
> > 7.1 machine, and `/bin/sh -c 'ifconfig'` can work successfully. I thought 
> > there should be no much difference between RHEL and CentOS, so not sure why 
> > it does not work in CentOS ...
> 
> Jie Yu wrote:
> Qian, i think it depends on your shell.

Yeah, it really depends on os default PATH. The test would fail or not on diff 
CentOS 7.x. We encountered similar issues a couple times.


- Gilbert


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


On June 20, 2016, 7:46 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48989/
> ---
> 
> (Updated June 20, 2016, 7:46 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Guangya Liu, Artem Harutyunyan, 
> Jie Yu, and Qian Zhang.
> 
> 
> Bugs: MESOS-5667
> https://issues.apache.org/jira/browse/MESOS-5667
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed broken cni isolator test on centos7.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> a51f24b0c4946ffe464e55ca1f25a1a38090e094 
> 
> Diff: https://reviews.apache.org/r/48989/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48994: Fixed portmapping isolator bind mount root non-existed case.

[Gilbert Song]

> On June 22, 2016, 11:19 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/port_mapping.cpp, line 1894
> > 
> >
> > Hum, just realized an issue. If you do that, bindMountRoot is no longer 
> > guaranteed to be realpath, right? Can you just move mdkir to the top?

I dont see why it is no longer a realpath. Currently 
`PORT_MAPPING_BIND_MOUNT_ROOT()` is hard coded as `/var/run/netns`. If it does 
not exist, we mkdir. the newly created dir should be a realpath as well. 

But yes, moving mkdir above should be a cleaner solution and make more sense.


- Gilbert


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


On June 20, 2016, 7:46 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48994/
> ---
> 
> (Updated June 20, 2016, 7:46 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Artem Harutyunyan, Jie Yu, and Cong 
> Wang.
> 
> 
> Bugs: MESOS-5673
> https://issues.apache.org/jira/browse/MESOS-5673
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed portmapping isolator bind mount root non-existed case.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 5df591893f3015926a7cfe07e9b57749c46f6367 
> 
> Diff: https://reviews.apache.org/r/48994/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48906: Added helper function to get non-scarce resources.

[Benjamin Mahler]

> On June 21, 2016, 10:36 p.m., Benjamin Mahler wrote:
> > include/mesos/resources.hpp, lines 252-256
> > 
> >
> > The Resources abstraction shouldn't need to know anything about scarce 
> > resources, you can just use the 'filter' function above if you want to 
> > perform some custom filtering in the caller.
> 
> Guangya Liu wrote:
> Hi Ben, so you mean that we still need to keep the function of 
> `isNonScarce` here but only remove `nonScarce`? 
> 
> Can you please explain more for why do you think the resourcs abstraction 
> cannot expose anything of `scarce` resources? In my understadnig, I think 
> that we should put `scarce` resources at same level as `revocable`, 
> `reserved` resources etc, all of them can be treated as `different kind of 
> resources types`, so seems putting this function here is reasonable?
> 
> Benjamin Mahler wrote:
> Revocable and reservation information are properties of the resource. 
> Scarcity is depdendent on the environment rather than being a property of the 
> resource itself. If you have a lot of GPUs in a pool, they are not scarce. If 
> you have few GPUs in a pool, then they are scarce.
> 
> Guangya Liu wrote:
> I think that for revocable and reservations, both of them can be 
> configured by cluster admininstrator or framework developer.
> 
> The cluster administrator can define the revocable resources via the 
> resource estimator for now.
> 
> The reservation can be configured by admin or framework developer when 
> starting the agent or using http endpoint or framework. Here we can configure 
> some resources from non reserved to reserved resources. If we did some 
> reserve operations, then the resources will becomre reserved resources, 
> otherwise, it will not be reserved resource.
> 
> Similar with above, the cluster admin can also configure the scarce 
> resources when master start via a flag, it is also configured by cluster 
> admin.
> 
> So I think that revocable, reserved and scarce can all be treated as 
> resource property?

In the discussions I've started around "scarcity", I have used the word 
"scarcity" to describe when a resource is only present on a small number of 
machines. This means that the "scarcity" of the resource is not determined by 
the operator or by the framework, but is rather a property of the pool of 
resources in which the resource is present. If all 4 machines in a cluster 
happen to have 1 GPU, then GPUs are not "scarce". If 1000 machines join the 
cluster without GPUs, now the (4) GPUs fit the definition of a "scarce" 
resource.

So I'm confused as to why you would want to have the operator set the 
"scarcity" property on a resource. What advantage would there be of doing this?

Also note that the Resources abstraction should know nothing about fairness or 
fairness exclusion.


- Benjamin


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


On June 18, 2016, 1:04 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48906/
> ---
> 
> (Updated June 18, 2016, 1:04 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5621
> https://issues.apache.org/jira/browse/MESOS-5621
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper function to get non-scarce resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp a557e97c65194d4aad879fb88d8edefd1c95b8d8 
>   include/mesos/v1/resources.hpp a5ba8fec4c9c3643646308f75a4b28cefe0b3df3 
>   src/common/resources.cpp f6ff92b591c15bc8e93fd85e1896349c3a7bb968 
>   src/tests/resources_tests.cpp dc12bd8f1e2da6972bc8aed598811c55d664036e 
>   src/v1/resources.cpp 8c3f2d1c1529915a59d47fe37bb3fc7a3267079a 
> 
> Diff: https://reviews.apache.org/r/48906/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from ResourcesTest
> [ RUN  ] ResourcesTest.NonScarce
> [   OK ] ResourcesTest.NonScarce (1 ms)
> [--] 1 test from ResourcesTest (1 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (14 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 48995: Fixed portmapping isolator symlink existed failure.

[Gilbert Song]

> On June 22, 2016, 11:03 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/port_mapping.cpp, line 2587
> > 
> >
> > Instead of doing that, can you use a uuid for container id in the 
> > corresponding test?

My bad, should have realize that:(


- Gilbert


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


On June 20, 2016, 7:46 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48995/
> ---
> 
> (Updated June 20, 2016, 7:46 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Artem Harutyunyan, Jie Yu, and Cong 
> Wang.
> 
> 
> Bugs: MESOS-5674
> https://issues.apache.org/jira/browse/MESOS-5674
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed portmapping isolator symlink existed failure.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 5df591893f3015926a7cfe07e9b57749c46f6367 
> 
> Diff: https://reviews.apache.org/r/48995/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48995: Fixed portmapping isolator symlink existed failure.

[Gilbert Song]

> On June 22, 2016, 11:03 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/port_mapping.cpp, line 2587
> > 
> >
> > Instead of doing that, can you use a uuid for container id in the 
> > corresponding test?
> 
> Gilbert Song wrote:
> My bad, should have realize that:(

I fixed them all together in port mapping tests.


- Gilbert


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


On June 20, 2016, 7:46 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48995/
> ---
> 
> (Updated June 20, 2016, 7:46 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Artem Harutyunyan, Jie Yu, and Cong 
> Wang.
> 
> 
> Bugs: MESOS-5674
> https://issues.apache.org/jira/browse/MESOS-5674
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed portmapping isolator symlink existed failure.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 5df591893f3015926a7cfe07e9b57749c46f6367 
> 
> Diff: https://reviews.apache.org/r/48995/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48994: Fixed portmapping isolator bind mount root non-existed case.

[Jie Yu]

> On June 22, 2016, 6:19 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/port_mapping.cpp, line 1894
> > 
> >
> > Hum, just realized an issue. If you do that, bindMountRoot is no longer 
> > guaranteed to be realpath, right? Can you just move mdkir to the top?
> 
> Gilbert Song wrote:
> I dont see why it is no longer a realpath. Currently 
> `PORT_MAPPING_BIND_MOUNT_ROOT()` is hard coded as `/var/run/netns`. If it 
> does not exist, we mkdir. the newly created dir should be a realpath as well. 
> 
> But yes, moving mkdir above should be a cleaner solution and make more 
> sense.

/var/run might be a symlink to /run on some systems. Therefore /var/run/netns 
you just created might not be a realpath.


- Jie


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


On June 21, 2016, 2:46 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48994/
> ---
> 
> (Updated June 21, 2016, 2:46 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Artem Harutyunyan, Jie Yu, and Cong 
> Wang.
> 
> 
> Bugs: MESOS-5673
> https://issues.apache.org/jira/browse/MESOS-5673
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed portmapping isolator bind mount root non-existed case.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 5df591893f3015926a7cfe07e9b57749c46f6367 
> 
> Diff: https://reviews.apache.org/r/48994/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 49107: Cherry-pick of MESOS-5330 to 0.28.x and 0.27.x.

[Benjamin Mahler]

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

Review request for mesos, David Robinson and Jie Yu.


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


Repository: mesos


Description
---

This is a scalability improvement that would be prudent to cherry-pick.

Note that we have not fixed the related 
[MESOS-5200](https://issues.apache.org/jira/browse/MESOS-5200).


Diffs
-

  CHANGELOG 5944f2ae64020a19fcdb312315069a9853f554c0 

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


Testing
---

N/A


Thanks,

Benjamin Mahler

Re: Review Request 47972: Updated rmdir to continue deletion on error.

[Jie Yu]

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



flying by


3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 102 - 103)


indentation here should be 4


- Jie Yu


On June 19, 2016, 3:50 a.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47972/
> ---
> 
> (Updated June 19, 2016, 3:50 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer, Joris Van Remoortere, Joseph Wu, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5196
> https://issues.apache.org/jira/browse/MESOS-5196
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updates made to rmdir to prevent early exit in the event of error.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> 4780a13465ee1323c49db9911bc7083173cfe0df 
>   3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> 772b86dd111d28aefbeeba5f829ffa377fd12efb 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 
> a11bfc9f9e6cbb05f3e9ce0ea48297b8f88fe53f 
> 
> Diff: https://reviews.apache.org/r/47972/diff/
> 
> 
> Testing
> ---
> 
> testing done with 'make check'.
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Review Request 49108: Cherry-pick of MESOS-5330 to 0.27.x.

[Benjamin Mahler]

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

Review request for mesos, David Robinson and Jie Yu.


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


Repository: mesos


Description
---

This was a scalability improvement to make agents backoff before
establishing connections to the master.


Diffs
-

  CHANGELOG 5944f2ae64020a19fcdb312315069a9853f554c0 

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


Testing
---

N/A


Thanks,

Benjamin Mahler

Review Request 49109: Cherry-pick of MESOS-5330 to 0.28.x.

[Benjamin Mahler]

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

Review request for mesos, David Robinson and Jie Yu.


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


Repository: mesos


Description
---

This was a scalability improvement to make agents backoff before
establishing connections to the master.


Diffs
-

  CHANGELOG 5944f2ae64020a19fcdb312315069a9853f554c0 

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


Testing
---

N/A


Thanks,

Benjamin Mahler

Re: Review Request 49108: Cherry-pick of MESOS-5330 to 0.27.x.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 22, 2016, 9:33 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49108/
> ---
> 
> (Updated June 22, 2016, 9:33 p.m.)
> 
> 
> Review request for mesos, David Robinson and Jie Yu.
> 
> 
> Bugs: MESOS-5330
> https://issues.apache.org/jira/browse/MESOS-5330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This was a scalability improvement to make agents backoff before
> establishing connections to the master.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 5944f2ae64020a19fcdb312315069a9853f554c0 
> 
> Diff: https://reviews.apache.org/r/49108/diff/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 49109: Cherry-pick of MESOS-5330 to 0.28.x.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 22, 2016, 9:33 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49109/
> ---
> 
> (Updated June 22, 2016, 9:33 p.m.)
> 
> 
> Review request for mesos, David Robinson and Jie Yu.
> 
> 
> Bugs: MESOS-5330
> https://issues.apache.org/jira/browse/MESOS-5330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This was a scalability improvement to make agents backoff before
> establishing connections to the master.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 5944f2ae64020a19fcdb312315069a9853f554c0 
> 
> Diff: https://reviews.apache.org/r/49109/diff/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 49067: Added AFFILIATIONS.yml file.

[Vinod Kone]

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



I'm wondering if we should just seed this list with Mesosphere employees for 
now, mainly to play safe. Some of the affiliations in this list are out-of-date 
and some might not even be public knowledge. Better to let users send PRs than 
us including them. I was thinking that we could maybeaeadd sending a PR t to 
this file as a s step in our new contributor workflow.


AFFILIATIONS.yml (line 82)


pull this to the top?



AFFILIATIONS.yml (line 120)


s/Mesosphere/Microsoft/ ?



AFFILIATIONS.yml (line 353)


pull name to the top?



AFFILIATIONS.yml (line 366)


s/Mesosphere/Twitter/?


- Vinod Kone


On June 22, 2016, 7:05 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49067/
> ---
> 
> (Updated June 22, 2016, 7:05 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added AFFILIATIONS.yml file.
> 
> 
> Diffs
> -
> 
>   AFFILIATIONS.yml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49067/diff/
> 
> 
> Testing
> ---
> 
> Parsed the file with a test script to validate the syntax and generate the 
> list of contributors currently missing from the file.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 49013: Updated cgroups/devices isolator's prepare() to directly return None().

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On June 22, 2016, 9:51 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49013/
> ---
> 
> (Updated June 22, 2016, 9:51 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kevin Klues.
> 
> 
> Bugs: MESOS-5582
> https://issues.apache.org/jira/browse/MESOS-5582
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated cgroups/devices isolator's prepare() to directly return None().
> 
> cgroups/devices isolator does not implement its own update() method,
> so in its prepare(), we do not need to call update(), instead we should
> directly return None() in the end.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/devices.cpp 
> 16f962894e89317a436a0e38465338f42bfb83ef 
> 
> Diff: https://reviews.apache.org/r/49013/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 49069: Checkpointed CNI network config for container in the container dir.

[Jie Yu]

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


Fix it, then Ship it!





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


s/Conf/Config/



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


space before `'`



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


s/Conf/Config/


- Jie Yu


On June 22, 2016, 8:13 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49069/
> ---
> 
> (Updated June 22, 2016, 8:13 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-5310
> https://issues.apache.org/jira/browse/MESOS-5310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Checkpointed CNI network config for container in the container dir.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 8cf5fcb1e63873b2a3eca843130a361f25a6e825 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
> 8af50c11b00240ac1d24605b119acbd96aaa50be 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
> b3fb0a32e3e74ca859ca58085efed5a87e4e626b 
> 
> Diff: https://reviews.apache.org/r/49069/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 49070: Used checkpointed CNI network config during cleanup.

[Jie Yu]

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


Fix it, then Ship it!





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


Can we add a VLOG here similar to what we did in `attach`?


- Jie Yu


On June 22, 2016, 8:33 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49070/
> ---
> 
> (Updated June 22, 2016, 8:33 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-5310
> https://issues.apache.org/jira/browse/MESOS-5310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used checkpointed CNI network config during cleanup.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 8cf5fcb1e63873b2a3eca843130a361f25a6e825 
> 
> Diff: https://reviews.apache.org/r/49070/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 49071: Updated CNI test to verify the checkpointed CNI network config.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 22, 2016, 8:34 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49071/
> ---
> 
> (Updated June 22, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-5310
> https://issues.apache.org/jira/browse/MESOS-5310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated CNI test to verify the checkpointed CNI network config.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> a51f24b0c4946ffe464e55ca1f25a1a38090e094 
> 
> Diff: https://reviews.apache.org/r/49071/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 49063: Updating formatting of some lambda calls for readability.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49059, 49060, 49061, 49062, 49088, 49063]

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

- Mesos ReviewBot


On June 22, 2016, 2:23 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49063/
> ---
> 
> (Updated June 22, 2016, 2:23 p.m.)
> 
> 
> Review request for mesos, Joerg Schad and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7daaf12a4086635bbc5aba5e3375c95e8899ac6e 
> 
> Diff: https://reviews.apache.org/r/49063/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 49059: Fixed the test suite build for Apple clang-600.0.54.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 22, 2016, 1:09 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49059/
> ---
> 
> (Updated June 22, 2016, 1:09 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Apparently, Apple clang-600.0.54, which is the stock compiler in
> 10.10.4, does not support passing `{}` as an empty set into a
> function.
> 
> 
> Diffs
> -
> 
>   src/tests/log_tests.cpp 362d9dd06b5fb54c3c4ed20426b03676de4ccc69 
> 
> Diff: https://reviews.apache.org/r/49059/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 48988: Reduced test linux rootfs size.

[Gilbert Song]

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

(Updated June 22, 2016, 3:53 p.m.)


Review request for mesos, Zhiwei Chen, haosdent huang, Artem Harutyunyan, Jie 
Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Reduced test linux rootfs size.


Diffs (updated)
-

  src/tests/containerizer/rootfs.hpp 19ec7bf50e599f5420453e8cb7d12357770d3fd8 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 48987: Removed rootfs modules exclusion on appc provisioner test.

[Gilbert Song]

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

(Updated June 22, 2016, 3:52 p.m.)


Review request for mesos, Zhiwei Chen, haosdent huang, Artem Harutyunyan, Jie 
Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Removed rootfs modules exclusion on appc provisioner test.


Diffs (updated)
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
84fe52b6937c3b7d7628b17a2f045eec2f386b4d 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 48990: Added CGROUP namespace to ns helper.

[Gilbert Song]

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

(Updated June 22, 2016, 3:54 p.m.)


Review request for mesos, haosdent huang, Artem Harutyunyan, Jie Yu, Qian 
Zhang, and Zhengju Sha.


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


Repository: mesos


Description
---

Added CGROUP namespace to ns helper.


Diffs (updated)
-

  src/linux/ns.hpp 244a811b299c29b1dcd6652bd26e861e04df3f54 

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


Testing
---

make check on Ubuntu 16


Thanks,

Gilbert Song

Re: Review Request 48989: Fixed broken cni isolator test on centos7.

[Gilbert Song]

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

(Updated June 22, 2016, 3:54 p.m.)


Review request for mesos, Avinash sridharan, Guangya Liu, Artem Harutyunyan, 
Jie Yu, and Qian Zhang.


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


Repository: mesos


Description
---

Fixed broken cni isolator test on centos7.


Diffs (updated)
-

  src/tests/containerizer/cni_isolator_tests.cpp 
a51f24b0c4946ffe464e55ca1f25a1a38090e094 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 48991: Added exception for etc hostname mount in cni isolator.

[Gilbert Song]

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

(Updated June 22, 2016, 3:54 p.m.)


Review request for mesos, Avinash sridharan, Guangya Liu, Artem Harutyunyan, 
Jie Yu, and Qian Zhang.


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


Repository: mesos


Description
---

Added exception for etc hostname mount in cni isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
8cf5fcb1e63873b2a3eca843130a361f25a6e825 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 48993: Fixed memory pressure test cgroup slave recovery.

[Gilbert Song]

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

(Updated June 22, 2016, 3:54 p.m.)


Review request for mesos, Alexander Rukletsov, Bernd Mathiske, Artem 
Harutyunyan, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

Fixed memory pressure test cgroup slave recovery.


Diffs (updated)
-

  src/tests/containerizer/memory_pressure_tests.cpp 
4f343af7e44b5ecfee4b1a31cdc46876a15cdbbd 

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


Testing
---

make check

sudo ./mesos-test.sh --gtest_filter=... with 500+ iterations


Thanks,

Gilbert Song

Re: Review Request 48995: Fixed portmapping isolator tests ContainerId.

[Gilbert Song]

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

(Updated June 22, 2016, 3:55 p.m.)


Review request for mesos, haosdent huang, Artem Harutyunyan, Jie Yu, and Cong 
Wang.


Summary (updated)
-

Fixed portmapping isolator tests ContainerId.


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


Repository: mesos


Description (updated)
---

Fixed portmapping isolator tests ContainerId.


Diffs (updated)
-

  src/tests/containerizer/port_mapping_tests.cpp 
21ad1e1c53316a3bb6d914aa228ccf3658acdfbf 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 48996: Added space for ROOT_NC_HostToContainerUDP shell commands.

[Gilbert Song]

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

(Updated June 22, 2016, 3:55 p.m.)


Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added space for ROOT_NC_HostToContainerUDP shell commands.


Diffs (updated)
-

  src/tests/containerizer/port_mapping_tests.cpp 
21ad1e1c53316a3bb6d914aa228ccf3658acdfbf 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 48994: Fixed portmapping isolator bind mount root non-existed case.

[Gilbert Song]

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

(Updated June 22, 2016, 3:55 p.m.)


Review request for mesos, haosdent huang, Artem Harutyunyan, Jie Yu, and Cong 
Wang.


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


Repository: mesos


Description
---

Fixed portmapping isolator bind mount root non-existed case.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
5df591893f3015926a7cfe07e9b57749c46f6367 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 48992: Fixed memory pressure test cgroup statistics.

[Gilbert Song]

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

(Updated June 22, 2016, 3:54 p.m.)


Review request for mesos, Alexander Rukletsov, Bernd Mathiske, Artem 
Harutyunyan, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

Fixed memory pressure test cgroup statistics.


Diffs (updated)
-

  src/tests/containerizer/memory_pressure_tests.cpp 
4f343af7e44b5ecfee4b1a31cdc46876a15cdbbd 

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


Testing
---

make check

sudo ./mesos-test.sh --gtest_filter=... with 500+ iterations


Thanks,

Gilbert Song

Re: Review Request 48995: Fixed portmapping isolator tests ContainerId.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 22, 2016, 10:55 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48995/
> ---
> 
> (Updated June 22, 2016, 10:55 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Artem Harutyunyan, Jie Yu, and Cong 
> Wang.
> 
> 
> Bugs: MESOS-5674
> https://issues.apache.org/jira/browse/MESOS-5674
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed portmapping isolator tests ContainerId.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 21ad1e1c53316a3bb6d914aa228ccf3658acdfbf 
> 
> Diff: https://reviews.apache.org/r/48995/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>