Re: Review Request 36831: Allowed Isolator::prepare to return a list of CommandInfos.

[Kapil Arya]

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

(Updated July 28, 2015, 1:41 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

addressed Jie's comments.


Repository: mesos


Description
---

Previously, the Isolators had to join all the preparation commands into
a single command using `&&`. Now they can pass each command separately.


Diffs (updated)
-

  include/mesos/slave/isolator.proto c04a4e6a5475481c72b22d0d7a0daa9ab78e069a 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
0047945f3dc831720d66c492a1b2c626ceb3bf00 
  src/slave/containerizer/isolators/namespaces/pid.cpp 
14871a9c44103294e33bd9a315cecce078e51afb 
  src/slave/containerizer/isolators/network/port_mapping.cpp 
77e649cc77bf29a84db48d3d914042ab5b2f695f 
  src/slave/containerizer/mesos/containerizer.cpp 
b8045d77526053702db530792060489ee04dd8f2 
  src/tests/containerizer/containerizer_tests.cpp 
18c478aa5e07f839184649815b35b6e5eded92df 
  src/tests/containerizer/isolator_tests.cpp 
51a4db17d13dd8dbff6a5eb2ceabee269275f6f6 
  src/tests/containerizer/port_mapping_tests.cpp 
778dc8cdfee1fd4098a6332b9219764527cd7b59 

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


Testing
---

make check.


Thanks,

Kapil Arya

Re: Review Request 36831: Allowed Isolator::prepare to return a list of CommandInfos.

[Kapil Arya]

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

(Updated July 28, 2015, 1:39 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

rebased


Repository: mesos


Description
---

Previously, the Isolators had to join all the preparation commands into
a single command using `&&`. Now they can pass each command separately.


Diffs (updated)
-

  include/mesos/slave/isolator.proto c04a4e6a5475481c72b22d0d7a0daa9ab78e069a 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
0047945f3dc831720d66c492a1b2c626ceb3bf00 
  src/slave/containerizer/isolators/namespaces/pid.cpp 
14871a9c44103294e33bd9a315cecce078e51afb 
  src/slave/containerizer/isolators/network/port_mapping.cpp 
77e649cc77bf29a84db48d3d914042ab5b2f695f 
  src/slave/containerizer/mesos/containerizer.cpp 
b8045d77526053702db530792060489ee04dd8f2 
  src/tests/containerizer/containerizer_tests.cpp 
18c478aa5e07f839184649815b35b6e5eded92df 
  src/tests/containerizer/isolator_tests.cpp 
51a4db17d13dd8dbff6a5eb2ceabee269275f6f6 
  src/tests/containerizer/port_mapping_tests.cpp 
778dc8cdfee1fd4098a6332b9219764527cd7b59 

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


Testing
---

make check.


Thanks,

Kapil Arya

Re: Review Request 36831: Allowed Isolator::prepare to return a list of CommandInfos.

[Jie Yu]

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

Ship it!


Please do a rebase. The patch does not apply.


src/tests/containerizer/port_mapping_tests.cpp (lines 320 - 322)


Could you reverse this:

```
if (preparation.get().commands().size() != 1) {
  ...
}
```


- Jie Yu


On July 28, 2015, 3:45 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36831/
> ---
> 
> (Updated July 28, 2015, 3:45 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the Isolators had to join all the preparation commands into
> a single command using `&&`. Now they can pass each command separately.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.proto 07c1c1a09d1578dd4c3abd8bb1773782b4aa9549 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 4694025eb9b3adad70cc1ec7541c3d3f95fec2bc 
>   src/slave/containerizer/isolators/namespaces/pid.cpp 
> 29be94b96ed9a3db90c88a4b60517d78e95b7144 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 3f35782c9e0e458169777bab778178c524a58b2b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 11eedf30674b326ea3e1f6f71d27f9eb54126368 
>   src/tests/containerizer/containerizer_tests.cpp 
> a44b6e887c37e55213db068a95ab3e73a033e1ea 
>   src/tests/containerizer/isolator_tests.cpp 
> 59f08c0c175ea800ba23d85666d20b6a7b6d07d6 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 45ef97a4822edacd2c2964e63ec624fe4190b3b9 
> 
> Diff: https://reviews.apache.org/r/36831/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 36831: Allowed Isolator::prepare to return a list of CommandInfos.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [36718, 36754, 36755, 36799]

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

Error:
 2015-07-28 04:19:11 URL:https://reviews.apache.org/r/36799/diff/raw/ 
[44819/44819] -> "36799.patch" [1]
error: patch failed: src/slave/containerizer/mesos/containerizer.hpp:197
error: src/slave/containerizer/mesos/containerizer.hpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On July 28, 2015, 3:45 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36831/
> ---
> 
> (Updated July 28, 2015, 3:45 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the Isolators had to join all the preparation commands into
> a single command using `&&`. Now they can pass each command separately.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.proto 07c1c1a09d1578dd4c3abd8bb1773782b4aa9549 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 4694025eb9b3adad70cc1ec7541c3d3f95fec2bc 
>   src/slave/containerizer/isolators/namespaces/pid.cpp 
> 29be94b96ed9a3db90c88a4b60517d78e95b7144 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 3f35782c9e0e458169777bab778178c524a58b2b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 11eedf30674b326ea3e1f6f71d27f9eb54126368 
>   src/tests/containerizer/containerizer_tests.cpp 
> a44b6e887c37e55213db068a95ab3e73a033e1ea 
>   src/tests/containerizer/isolator_tests.cpp 
> 59f08c0c175ea800ba23d85666d20b6a7b6d07d6 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 45ef97a4822edacd2c2964e63ec624fe4190b3b9 
> 
> Diff: https://reviews.apache.org/r/36831/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 36831: Allowed Isolator::prepare to return a list of CommandInfos.

[Kapil Arya]

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

(Updated July 27, 2015, 11:45 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

removed an ill-placed CHECK.


Repository: mesos


Description
---

Previously, the Isolators had to join all the preparation commands into
a single command using `&&`. Now they can pass each command separately.


Diffs (updated)
-

  include/mesos/slave/isolator.proto 07c1c1a09d1578dd4c3abd8bb1773782b4aa9549 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
4694025eb9b3adad70cc1ec7541c3d3f95fec2bc 
  src/slave/containerizer/isolators/namespaces/pid.cpp 
29be94b96ed9a3db90c88a4b60517d78e95b7144 
  src/slave/containerizer/isolators/network/port_mapping.cpp 
3f35782c9e0e458169777bab778178c524a58b2b 
  src/slave/containerizer/mesos/containerizer.cpp 
11eedf30674b326ea3e1f6f71d27f9eb54126368 
  src/tests/containerizer/containerizer_tests.cpp 
a44b6e887c37e55213db068a95ab3e73a033e1ea 
  src/tests/containerizer/isolator_tests.cpp 
59f08c0c175ea800ba23d85666d20b6a7b6d07d6 
  src/tests/containerizer/port_mapping_tests.cpp 
45ef97a4822edacd2c2964e63ec624fe4190b3b9 

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


Testing
---

make check.


Thanks,

Kapil Arya

Re: Review Request 36831: Allowed Isolator::prepare to return a list of CommandInfos.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36718, 36754, 36755, 36799, 36831]

All tests passed.

- Mesos ReviewBot


On July 28, 2015, 12:02 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36831/
> ---
> 
> (Updated July 28, 2015, 12:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the Isolators had to join all the preparation commands into
> a single command using `&&`. Now they can pass each command separately.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.proto 07c1c1a09d1578dd4c3abd8bb1773782b4aa9549 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 4694025eb9b3adad70cc1ec7541c3d3f95fec2bc 
>   src/slave/containerizer/isolators/namespaces/pid.cpp 
> 29be94b96ed9a3db90c88a4b60517d78e95b7144 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 3f35782c9e0e458169777bab778178c524a58b2b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 11eedf30674b326ea3e1f6f71d27f9eb54126368 
>   src/tests/containerizer/containerizer_tests.cpp 
> a44b6e887c37e55213db068a95ab3e73a033e1ea 
>   src/tests/containerizer/isolator_tests.cpp 
> 59f08c0c175ea800ba23d85666d20b6a7b6d07d6 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 45ef97a4822edacd2c2964e63ec624fe4190b3b9 
> 
> Diff: https://reviews.apache.org/r/36831/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 36831: Allowed Isolator::prepare to return a list of CommandInfos.

[Kapil Arya]

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

(Updated July 27, 2015, 8:02 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

fixed build error


Repository: mesos


Description
---

Previously, the Isolators had to join all the preparation commands into
a single command using `&&`. Now they can pass each command separately.


Diffs (updated)
-

  include/mesos/slave/isolator.proto 07c1c1a09d1578dd4c3abd8bb1773782b4aa9549 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
4694025eb9b3adad70cc1ec7541c3d3f95fec2bc 
  src/slave/containerizer/isolators/namespaces/pid.cpp 
29be94b96ed9a3db90c88a4b60517d78e95b7144 
  src/slave/containerizer/isolators/network/port_mapping.cpp 
3f35782c9e0e458169777bab778178c524a58b2b 
  src/slave/containerizer/mesos/containerizer.cpp 
11eedf30674b326ea3e1f6f71d27f9eb54126368 
  src/tests/containerizer/containerizer_tests.cpp 
a44b6e887c37e55213db068a95ab3e73a033e1ea 
  src/tests/containerizer/isolator_tests.cpp 
59f08c0c175ea800ba23d85666d20b6a7b6d07d6 
  src/tests/containerizer/port_mapping_tests.cpp 
45ef97a4822edacd2c2964e63ec624fe4190b3b9 

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


Testing
---

make check.


Thanks,

Kapil Arya

Re: Review Request 36831: Allowed Isolator::prepare to return a list of CommandInfos.

[Benjamin Hindman]

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

Ship it!



src/tests/containerizer/isolator_tests.cpp (line 964)


Swap expected and actual value please. Here and below!


- Benjamin Hindman


On July 27, 2015, 12:04 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36831/
> ---
> 
> (Updated July 27, 2015, 12:04 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the Isolators had to join all the preparation commands into
> a single command using `&&`. Now they can pass each command separately.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.proto 07c1c1a09d1578dd4c3abd8bb1773782b4aa9549 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 4694025eb9b3adad70cc1ec7541c3d3f95fec2bc 
>   src/slave/containerizer/isolators/namespaces/pid.cpp 
> 29be94b96ed9a3db90c88a4b60517d78e95b7144 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 11eedf30674b326ea3e1f6f71d27f9eb54126368 
>   src/tests/containerizer/containerizer_tests.cpp 
> a44b6e887c37e55213db068a95ab3e73a033e1ea 
>   src/tests/containerizer/isolator_tests.cpp 
> 59f08c0c175ea800ba23d85666d20b6a7b6d07d6 
> 
> Diff: https://reviews.apache.org/r/36831/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 36831: Allowed Isolator::prepare to return a list of CommandInfos.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [36718, 36754, 36755, 36799, 36831]

All tests passed.

- Mesos ReviewBot


On July 27, 2015, 12:04 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36831/
> ---
> 
> (Updated July 27, 2015, 12:04 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the Isolators had to join all the preparation commands into
> a single command using `&&`. Now they can pass each command separately.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.proto 07c1c1a09d1578dd4c3abd8bb1773782b4aa9549 
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 4694025eb9b3adad70cc1ec7541c3d3f95fec2bc 
>   src/slave/containerizer/isolators/namespaces/pid.cpp 
> 29be94b96ed9a3db90c88a4b60517d78e95b7144 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 11eedf30674b326ea3e1f6f71d27f9eb54126368 
>   src/tests/containerizer/containerizer_tests.cpp 
> a44b6e887c37e55213db068a95ab3e73a033e1ea 
>   src/tests/containerizer/isolator_tests.cpp 
> 59f08c0c175ea800ba23d85666d20b6a7b6d07d6 
> 
> Diff: https://reviews.apache.org/r/36831/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>