Re: Review Request 41590: Modulize the containerizer interface.

2016-01-25 Thread Mesos ReviewBot 

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



Bad patch!

Reviews applied: [41590]

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

Error:
 ..
2016-01-25 08:37:13 URL:https://reviews.apache.org/r/41590/diff/raw/ 
[104415/104415] -> "41590.patch" [1]
error: patch failed: src/slave/containerizer/containerizer.cpp:351
error: src/slave/containerizer/containerizer.cpp: patch does not apply
error: patch failed: src/slave/containerizer/mesos/containerizer.hpp:46
error: src/slave/containerizer/mesos/containerizer.hpp: patch does not apply
error: patch failed: src/slave/containerizer/mesos/containerizer.cpp:180
error: src/slave/containerizer/mesos/containerizer.cpp: patch does not apply
error: patch failed: 
src/slave/containerizer/mesos/provisioner/provisioner.hpp:61
error: src/slave/containerizer/mesos/provisioner/provisioner.hpp: patch does 
not apply
error: patch failed: 
src/slave/containerizer/mesos/provisioner/provisioner.cpp:47
error: src/slave/containerizer/mesos/provisioner/provisioner.cpp: patch does 
not apply
error: patch failed: src/tests/container_logger_tests.cpp:46
error: src/tests/container_logger_tests.cpp: patch does not apply
error: patch failed: src/tests/containerizer/filesystem_isolator_tests.cpp:181
error: src/tests/containerizer/filesystem_isolator_tests.cpp: patch does not 
apply
error: patch failed: src/tests/containerizer/mesos_containerizer_tests.cpp:88
error: src/tests/containerizer/mesos_containerizer_tests.cpp: patch does not 
apply
error: patch failed: src/tests/containerizer/provisioner_appc_tests.cpp:214
error: src/tests/containerizer/provisioner_appc_tests.cpp: patch does not apply
error: patch failed: src/tests/mesos.hpp:32
error: src/tests/mesos.hpp: patch does not apply
error: patch failed: src/tests/slave_tests.cpp:2878
error: src/tests/slave_tests.cpp: patch does not apply

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

- Mesos ReviewBot


On Jan. 13, 2016, 6:05 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41590/
> ---
> 
> (Updated Jan. 13, 2016, 6:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Ben Mahler, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-3709
> https://issues.apache.org/jira/browse/MESOS-3709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modulize the containerizer interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/containerizer.hpp PRE-CREATION 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/local/local.cpp 582d4a10444831b0753d20650698e5d3b51cca9f 
>   src/module/manager.cpp a53f71b9965f7ab85aadb6c0c7af18de958faf38 
>   src/slave/containerizer/composing.hpp 
> 6b8757ebc75710b612f6ea5854ee2ff0ca0083ec 
>   src/slave/containerizer/composing.cpp 
> f0a7bba4a56702872c9b73a12128b5292708d0e7 
>   src/slave/containerizer/containerizer.hpp 
> 6964d136818ea9904fa35cd778eb9ef19e2c64fc 
>   src/slave/containerizer/containerizer.cpp 
> 5e7e55e2dd4e451bb754a6a888e9683760d95873 
>   src/slave/containerizer/docker.hpp 8927865a1e7b58bf17de02ead3a175275f1e14d7 
>   src/slave/containerizer/docker.cpp aacf90f2cb6c08f94340936d29b2df513ac9825a 
>   src/slave/containerizer/external_containerizer.hpp 
> feeb0278a3d8fec8c7177a9a3dc443ee0c198c5c 
>   src/slave/containerizer/external_containerizer.cpp 
> fe368dd52644b89e47ffd4d947de290b3998fb1a 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 89d18624b08f2c953b9fa21663aeda694d4aac12 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f3c370aeb331beb6202fd30cd0278877da0b42e0 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> b2e23d82ea255e1c91d7537a58f86632763d9a56 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 89d06e94bd2255bb4f2dad3fae75c1b3d789611d 
>   src/slave/flags.hpp 6857fde027fd57b4934cb43ddf435d12900e0b87 
>   src/slave/flags.cpp 19c2996c4572b992030f8824380f3979ced7e526 
>   src/slave/main.cpp 9d48a0823189ea6505073a2803f02d90dc382ab4 
>   src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
>   src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 
>   src/tests/cluster.hpp 576dcb8b7e27d1905425aa0f7cb319c19c17c15c 
>   src/tests/cluster.cpp 1a3a038e78999bca2fc3f08725f4ffaf4290bcb9 
>   src/tests/container_logger_tests.cpp 
> c6b2e597517c74a55649287dc5ae5a3115f9a640 
>   src/tests/containerizer.hpp 25b64dacca65aa0ac49a05a2fbaf526aa0761471 
>   src/tests/containerizer/composing_containerizer_tests.cpp 
> e7e3b622b6606a812aef046c761bf92368d34af2 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> cb58b7183c36d96b9ac4803c63980c278a50c97b 
>

Re: Review Request 41590: Modulize the containerizer interface.

2016-01-24 Thread Guangya Liu 

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



@haosdent, it is really a big patch, can you please highlight some key points 
for this patch in description part?

- Guangya Liu


On 一月 13, 2016, 6:05 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41590/
> ---
> 
> (Updated 一月 13, 2016, 6:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Ben Mahler, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-3709
> https://issues.apache.org/jira/browse/MESOS-3709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modulize the containerizer interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/containerizer.hpp PRE-CREATION 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/local/local.cpp 582d4a10444831b0753d20650698e5d3b51cca9f 
>   src/module/manager.cpp a53f71b9965f7ab85aadb6c0c7af18de958faf38 
>   src/slave/containerizer/composing.hpp 
> 6b8757ebc75710b612f6ea5854ee2ff0ca0083ec 
>   src/slave/containerizer/composing.cpp 
> f0a7bba4a56702872c9b73a12128b5292708d0e7 
>   src/slave/containerizer/containerizer.hpp 
> 6964d136818ea9904fa35cd778eb9ef19e2c64fc 
>   src/slave/containerizer/containerizer.cpp 
> 5e7e55e2dd4e451bb754a6a888e9683760d95873 
>   src/slave/containerizer/docker.hpp 8927865a1e7b58bf17de02ead3a175275f1e14d7 
>   src/slave/containerizer/docker.cpp aacf90f2cb6c08f94340936d29b2df513ac9825a 
>   src/slave/containerizer/external_containerizer.hpp 
> feeb0278a3d8fec8c7177a9a3dc443ee0c198c5c 
>   src/slave/containerizer/external_containerizer.cpp 
> fe368dd52644b89e47ffd4d947de290b3998fb1a 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 89d18624b08f2c953b9fa21663aeda694d4aac12 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f3c370aeb331beb6202fd30cd0278877da0b42e0 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> b2e23d82ea255e1c91d7537a58f86632763d9a56 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 89d06e94bd2255bb4f2dad3fae75c1b3d789611d 
>   src/slave/flags.hpp 6857fde027fd57b4934cb43ddf435d12900e0b87 
>   src/slave/flags.cpp 19c2996c4572b992030f8824380f3979ced7e526 
>   src/slave/main.cpp 9d48a0823189ea6505073a2803f02d90dc382ab4 
>   src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
>   src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 
>   src/tests/cluster.hpp 576dcb8b7e27d1905425aa0f7cb319c19c17c15c 
>   src/tests/cluster.cpp 1a3a038e78999bca2fc3f08725f4ffaf4290bcb9 
>   src/tests/container_logger_tests.cpp 
> c6b2e597517c74a55649287dc5ae5a3115f9a640 
>   src/tests/containerizer.hpp 25b64dacca65aa0ac49a05a2fbaf526aa0761471 
>   src/tests/containerizer/composing_containerizer_tests.cpp 
> e7e3b622b6606a812aef046c761bf92368d34af2 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> cb58b7183c36d96b9ac4803c63980c278a50c97b 
>   src/tests/containerizer/external_containerizer_test.cpp 
> da52860f4a1d5363d7b61b9a8bb6abad02d89736 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 5bb85034c22caef64054c1629f6fd55d227e48b1 
>   src/tests/containerizer/isolator_tests.cpp 
> 91178b69ccbf5b6cbf64421e5602e6d554fc34ca 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> 4a03af2c9c0643d964b1d76e2096341b59bf5dce 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 95b493c6f479eef52ee0c9a44ac40254ed76ebae 
>   src/tests/containerizer/port_mapping_tests.cpp 
> e3aea53468fa00374320a8b89bdbb64f38e44b01 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 98d5a46149ef63cedcbf4915b5b6e9842ac67dbb 
>   src/tests/disk_quota_tests.cpp 1577cf71c4d1fbad11e2ec939c4ceae2d5b25f97 
>   src/tests/fetcher_cache_tests.cpp 1fb1e213d3c35479789688d1a3a49a3c6058b198 
>   src/tests/health_check_tests.cpp 26d05e7f7db9306c8b9164e2e7f843793c909e67 
>   src/tests/hook_tests.cpp 715b0b07e1befdca5b09172a439edccd7546d77f 
>   src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
>   src/tests/mesos.cpp 365ebe8335c37bfdb983a5424d4c995fa9b76a22 
>   src/tests/slave_recovery_tests.cpp c0e4ff75b35c9e806741aab5696771e66d2c2ea8 
>   src/tests/slave_tests.cpp e4fb490a1d877547fe883c22dbc47bb4969ecef6 
> 
> Diff: https://reviews.apache.org/r/41590/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 41590: Modulize the containerizer interface.

2016-01-11 Thread haosdent huang 

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

(Updated Jan. 11, 2016, 5:52 p.m.)


Review request for mesos, Ben Mahler, Till Toenshoff, and Timothy Chen.


Changes
---

Rebase


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


Repository: mesos


Description
---

Modulize the containerizer interface.


Diffs (updated)
-

  include/mesos/module/containerizer.hpp PRE-CREATION 
  src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
  src/local/local.cpp 582d4a10444831b0753d20650698e5d3b51cca9f 
  src/module/manager.cpp a53f71b9965f7ab85aadb6c0c7af18de958faf38 
  src/slave/containerizer/composing.hpp 
6b8757ebc75710b612f6ea5854ee2ff0ca0083ec 
  src/slave/containerizer/composing.cpp 
f0a7bba4a56702872c9b73a12128b5292708d0e7 
  src/slave/containerizer/containerizer.hpp 
6964d136818ea9904fa35cd778eb9ef19e2c64fc 
  src/slave/containerizer/containerizer.cpp 
5e7e55e2dd4e451bb754a6a888e9683760d95873 
  src/slave/containerizer/docker.hpp 8927865a1e7b58bf17de02ead3a175275f1e14d7 
  src/slave/containerizer/docker.cpp aacf90f2cb6c08f94340936d29b2df513ac9825a 
  src/slave/containerizer/external_containerizer.hpp 
feeb0278a3d8fec8c7177a9a3dc443ee0c198c5c 
  src/slave/containerizer/external_containerizer.cpp 
fe368dd52644b89e47ffd4d947de290b3998fb1a 
  src/slave/containerizer/mesos/containerizer.hpp 
89d18624b08f2c953b9fa21663aeda694d4aac12 
  src/slave/containerizer/mesos/containerizer.cpp 
f3c370aeb331beb6202fd30cd0278877da0b42e0 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
b2e23d82ea255e1c91d7537a58f86632763d9a56 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
89d06e94bd2255bb4f2dad3fae75c1b3d789611d 
  src/slave/flags.hpp 6857fde027fd57b4934cb43ddf435d12900e0b87 
  src/slave/flags.cpp 19c2996c4572b992030f8824380f3979ced7e526 
  src/slave/main.cpp 9d48a0823189ea6505073a2803f02d90dc382ab4 
  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 
  src/tests/cluster.hpp 576dcb8b7e27d1905425aa0f7cb319c19c17c15c 
  src/tests/cluster.cpp 1a3a038e78999bca2fc3f08725f4ffaf4290bcb9 
  src/tests/container_logger_tests.cpp c6b2e597517c74a55649287dc5ae5a3115f9a640 
  src/tests/containerizer.hpp 25b64dacca65aa0ac49a05a2fbaf526aa0761471 
  src/tests/containerizer/composing_containerizer_tests.cpp 
e7e3b622b6606a812aef046c761bf92368d34af2 
  src/tests/containerizer/docker_containerizer_tests.cpp 
cb58b7183c36d96b9ac4803c63980c278a50c97b 
  src/tests/containerizer/external_containerizer_test.cpp 
da52860f4a1d5363d7b61b9a8bb6abad02d89736 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
5bb85034c22caef64054c1629f6fd55d227e48b1 
  src/tests/containerizer/isolator_tests.cpp 
91178b69ccbf5b6cbf64421e5602e6d554fc34ca 
  src/tests/containerizer/memory_pressure_tests.cpp 
4a03af2c9c0643d964b1d76e2096341b59bf5dce 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
95b493c6f479eef52ee0c9a44ac40254ed76ebae 
  src/tests/containerizer/port_mapping_tests.cpp 
e3aea53468fa00374320a8b89bdbb64f38e44b01 
  src/tests/containerizer/provisioner_appc_tests.cpp 
98d5a46149ef63cedcbf4915b5b6e9842ac67dbb 
  src/tests/disk_quota_tests.cpp 1577cf71c4d1fbad11e2ec939c4ceae2d5b25f97 
  src/tests/fetcher_cache_tests.cpp 1fb1e213d3c35479789688d1a3a49a3c6058b198 
  src/tests/health_check_tests.cpp 26d05e7f7db9306c8b9164e2e7f843793c909e67 
  src/tests/hook_tests.cpp 715b0b07e1befdca5b09172a439edccd7546d77f 
  src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
  src/tests/mesos.cpp 365ebe8335c37bfdb983a5424d4c995fa9b76a22 
  src/tests/slave_recovery_tests.cpp c0e4ff75b35c9e806741aab5696771e66d2c2ea8 
  src/tests/slave_tests.cpp e4fb490a1d877547fe883c22dbc47bb4969ecef6 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 41590: Modulize the containerizer interface.

2016-01-11 Thread haosdent huang 


> On Jan. 10, 2016, 7:59 p.m., Benjamin Bannier wrote:
> > src/slave/containerizer/containerizer.hpp, line 49
> > 
> >
> > Just having a brief look, but one of the big issues with modularizing 
> > the containerizer appears to be `SlaveState` which is in internal class; 
> > the task seems to be to isolate, extract, and expose the needed pieces 
> > there, right?

hi, @ben. I also as confuse as you about the `mesos::internal::slave` 
namespace, the public interface in `containerizer.hpp`, the 
`mesos::internal::slaveFlags` parameter. I think maybe we should change them to 
modularize ready better. But because the type of this issue is `bug`, does this 
mean it want a quick way to modular containerizer, and then make the 
containerizer module provide more friendly way to use next step?


- haosdent


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


On Jan. 11, 2016, 5:52 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41590/
> ---
> 
> (Updated Jan. 11, 2016, 5:52 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Till Toenshoff, and Timothy Chen.
> 
> 
> Bugs: MESOS-3709
> https://issues.apache.org/jira/browse/MESOS-3709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modulize the containerizer interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/containerizer.hpp PRE-CREATION 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/local/local.cpp 582d4a10444831b0753d20650698e5d3b51cca9f 
>   src/module/manager.cpp a53f71b9965f7ab85aadb6c0c7af18de958faf38 
>   src/slave/containerizer/composing.hpp 
> 6b8757ebc75710b612f6ea5854ee2ff0ca0083ec 
>   src/slave/containerizer/composing.cpp 
> f0a7bba4a56702872c9b73a12128b5292708d0e7 
>   src/slave/containerizer/containerizer.hpp 
> 6964d136818ea9904fa35cd778eb9ef19e2c64fc 
>   src/slave/containerizer/containerizer.cpp 
> 5e7e55e2dd4e451bb754a6a888e9683760d95873 
>   src/slave/containerizer/docker.hpp 8927865a1e7b58bf17de02ead3a175275f1e14d7 
>   src/slave/containerizer/docker.cpp aacf90f2cb6c08f94340936d29b2df513ac9825a 
>   src/slave/containerizer/external_containerizer.hpp 
> feeb0278a3d8fec8c7177a9a3dc443ee0c198c5c 
>   src/slave/containerizer/external_containerizer.cpp 
> fe368dd52644b89e47ffd4d947de290b3998fb1a 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 89d18624b08f2c953b9fa21663aeda694d4aac12 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f3c370aeb331beb6202fd30cd0278877da0b42e0 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> b2e23d82ea255e1c91d7537a58f86632763d9a56 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 89d06e94bd2255bb4f2dad3fae75c1b3d789611d 
>   src/slave/flags.hpp 6857fde027fd57b4934cb43ddf435d12900e0b87 
>   src/slave/flags.cpp 19c2996c4572b992030f8824380f3979ced7e526 
>   src/slave/main.cpp 9d48a0823189ea6505073a2803f02d90dc382ab4 
>   src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
>   src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 
>   src/tests/cluster.hpp 576dcb8b7e27d1905425aa0f7cb319c19c17c15c 
>   src/tests/cluster.cpp 1a3a038e78999bca2fc3f08725f4ffaf4290bcb9 
>   src/tests/container_logger_tests.cpp 
> c6b2e597517c74a55649287dc5ae5a3115f9a640 
>   src/tests/containerizer.hpp 25b64dacca65aa0ac49a05a2fbaf526aa0761471 
>   src/tests/containerizer/composing_containerizer_tests.cpp 
> e7e3b622b6606a812aef046c761bf92368d34af2 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> cb58b7183c36d96b9ac4803c63980c278a50c97b 
>   src/tests/containerizer/external_containerizer_test.cpp 
> da52860f4a1d5363d7b61b9a8bb6abad02d89736 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 5bb85034c22caef64054c1629f6fd55d227e48b1 
>   src/tests/containerizer/isolator_tests.cpp 
> 91178b69ccbf5b6cbf64421e5602e6d554fc34ca 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> 4a03af2c9c0643d964b1d76e2096341b59bf5dce 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 95b493c6f479eef52ee0c9a44ac40254ed76ebae 
>   src/tests/containerizer/port_mapping_tests.cpp 
> e3aea53468fa00374320a8b89bdbb64f38e44b01 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 98d5a46149ef63cedcbf4915b5b6e9842ac67dbb 
>   src/tests/disk_quota_tests.cpp 1577cf71c4d1fbad11e2ec939c4ceae2d5b25f97 
>   src/tests/fetcher_cache_tests.cpp 1fb1e213d3c35479789688d1a3a49a3c6058b198 
>   src/tests/health_check_tests.cpp 26d05e7f7db9306c8b9164e2e7f843793c909e67 
>   src/tests/hook_tests.cpp 715b0b07e1befdca5b09172a439edccd7546d77f 
>   src/tests/mesos.hpp

Re: Review Request 41590: Modulize the containerizer interface.

2016-01-11 Thread Benjamin Bannier 


> On Jan. 10, 2016, 8:59 p.m., Benjamin Bannier wrote:
> > src/slave/containerizer/containerizer.hpp, line 49
> > 
> >
> > Just having a brief look, but one of the big issues with modularizing 
> > the containerizer appears to be `SlaveState` which is in internal class; 
> > the task seems to be to isolate, extract, and expose the needed pieces 
> > there, right?
> 
> haosdent huang wrote:
> hi, @ben. I also as confuse as you about the `mesos::internal::slave` 
> namespace, the public interface in `containerizer.hpp`, the 
> `mesos::internal::slaveFlags` parameter. I think maybe we should change them 
> to modularize ready better. But because the type of this issue is `bug`, does 
> this mean it want a quick way to modular containerizer, and then make the 
> containerizer module provide more friendly way to use next step?

I have the feeling exposing this internal class as a relatively central piece 
in an API only to remove it later is not something we'd want to do. I outlined 
a handful of approaches in the JIRA issue, but one really needs to decide what 
approach to take (optimally: without breaking stuff all over the place).


- Benjamin


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


On Jan. 11, 2016, 6:52 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41590/
> ---
> 
> (Updated Jan. 11, 2016, 6:52 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Till Toenshoff, and Timothy Chen.
> 
> 
> Bugs: MESOS-3709
> https://issues.apache.org/jira/browse/MESOS-3709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modulize the containerizer interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/containerizer.hpp PRE-CREATION 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/local/local.cpp 582d4a10444831b0753d20650698e5d3b51cca9f 
>   src/module/manager.cpp a53f71b9965f7ab85aadb6c0c7af18de958faf38 
>   src/slave/containerizer/composing.hpp 
> 6b8757ebc75710b612f6ea5854ee2ff0ca0083ec 
>   src/slave/containerizer/composing.cpp 
> f0a7bba4a56702872c9b73a12128b5292708d0e7 
>   src/slave/containerizer/containerizer.hpp 
> 6964d136818ea9904fa35cd778eb9ef19e2c64fc 
>   src/slave/containerizer/containerizer.cpp 
> 5e7e55e2dd4e451bb754a6a888e9683760d95873 
>   src/slave/containerizer/docker.hpp 8927865a1e7b58bf17de02ead3a175275f1e14d7 
>   src/slave/containerizer/docker.cpp aacf90f2cb6c08f94340936d29b2df513ac9825a 
>   src/slave/containerizer/external_containerizer.hpp 
> feeb0278a3d8fec8c7177a9a3dc443ee0c198c5c 
>   src/slave/containerizer/external_containerizer.cpp 
> fe368dd52644b89e47ffd4d947de290b3998fb1a 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 89d18624b08f2c953b9fa21663aeda694d4aac12 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f3c370aeb331beb6202fd30cd0278877da0b42e0 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> b2e23d82ea255e1c91d7537a58f86632763d9a56 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 89d06e94bd2255bb4f2dad3fae75c1b3d789611d 
>   src/slave/flags.hpp 6857fde027fd57b4934cb43ddf435d12900e0b87 
>   src/slave/flags.cpp 19c2996c4572b992030f8824380f3979ced7e526 
>   src/slave/main.cpp 9d48a0823189ea6505073a2803f02d90dc382ab4 
>   src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
>   src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 
>   src/tests/cluster.hpp 576dcb8b7e27d1905425aa0f7cb319c19c17c15c 
>   src/tests/cluster.cpp 1a3a038e78999bca2fc3f08725f4ffaf4290bcb9 
>   src/tests/container_logger_tests.cpp 
> c6b2e597517c74a55649287dc5ae5a3115f9a640 
>   src/tests/containerizer.hpp 25b64dacca65aa0ac49a05a2fbaf526aa0761471 
>   src/tests/containerizer/composing_containerizer_tests.cpp 
> e7e3b622b6606a812aef046c761bf92368d34af2 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> cb58b7183c36d96b9ac4803c63980c278a50c97b 
>   src/tests/containerizer/external_containerizer_test.cpp 
> da52860f4a1d5363d7b61b9a8bb6abad02d89736 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 5bb85034c22caef64054c1629f6fd55d227e48b1 
>   src/tests/containerizer/isolator_tests.cpp 
> 91178b69ccbf5b6cbf64421e5602e6d554fc34ca 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> 4a03af2c9c0643d964b1d76e2096341b59bf5dce 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 95b493c6f479eef52ee0c9a44ac40254ed76ebae 
>   src/tests/containerizer/port_mapping_tests.cpp 
> e3aea53468fa00374320a8b89bdbb64f38e44b01 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 98d5a46149ef63cedcbf4915b5b6e9842ac67dbb 
>

Re: Review Request 41590: Modulize the containerizer interface.

2016-01-10 Thread Benjamin Bannier 

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



src/slave/containerizer/containerizer.hpp (line 46)


Just having a brief look, but one of the big issues with modularizing the 
containerizer appears to be `SlaveState` which is in internal class; the task 
seems to be to isolate, extract, and expose the needed pieces there, right?


- Benjamin Bannier


On Jan. 10, 2016, 7:41 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41590/
> ---
> 
> (Updated Jan. 10, 2016, 7:41 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Till Toenshoff, and Timothy Chen.
> 
> 
> Bugs: MESOS-3709
> https://issues.apache.org/jira/browse/MESOS-3709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modulize the containerizer interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/containerizer.hpp PRE-CREATION 
>   src/Makefile.am bbd0c119321fa9d11fea61b140428dd92d1258c8 
>   src/local/local.cpp 582d4a10444831b0753d20650698e5d3b51cca9f 
>   src/module/manager.cpp a53f71b9965f7ab85aadb6c0c7af18de958faf38 
>   src/slave/containerizer/composing.hpp 
> 6b8757ebc75710b612f6ea5854ee2ff0ca0083ec 
>   src/slave/containerizer/composing.cpp 
> f0a7bba4a56702872c9b73a12128b5292708d0e7 
>   src/slave/containerizer/containerizer.hpp 
> 6964d136818ea9904fa35cd778eb9ef19e2c64fc 
>   src/slave/containerizer/containerizer.cpp 
> 5e7e55e2dd4e451bb754a6a888e9683760d95873 
>   src/slave/containerizer/docker.hpp 8927865a1e7b58bf17de02ead3a175275f1e14d7 
>   src/slave/containerizer/docker.cpp aacf90f2cb6c08f94340936d29b2df513ac9825a 
>   src/slave/containerizer/external_containerizer.hpp 
> feeb0278a3d8fec8c7177a9a3dc443ee0c198c5c 
>   src/slave/containerizer/external_containerizer.cpp 
> fe368dd52644b89e47ffd4d947de290b3998fb1a 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 89d18624b08f2c953b9fa21663aeda694d4aac12 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f3c370aeb331beb6202fd30cd0278877da0b42e0 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> b2e23d82ea255e1c91d7537a58f86632763d9a56 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 89d06e94bd2255bb4f2dad3fae75c1b3d789611d 
>   src/slave/flags.hpp 6857fde027fd57b4934cb43ddf435d12900e0b87 
>   src/slave/flags.cpp 19c2996c4572b992030f8824380f3979ced7e526 
>   src/slave/main.cpp 9d48a0823189ea6505073a2803f02d90dc382ab4 
>   src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
>   src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 
>   src/tests/cluster.hpp 576dcb8b7e27d1905425aa0f7cb319c19c17c15c 
>   src/tests/cluster.cpp 1a3a038e78999bca2fc3f08725f4ffaf4290bcb9 
>   src/tests/container_logger_tests.cpp 
> c6b2e597517c74a55649287dc5ae5a3115f9a640 
>   src/tests/containerizer.hpp 25b64dacca65aa0ac49a05a2fbaf526aa0761471 
>   src/tests/containerizer/composing_containerizer_tests.cpp 
> e7e3b622b6606a812aef046c761bf92368d34af2 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> cb58b7183c36d96b9ac4803c63980c278a50c97b 
>   src/tests/containerizer/external_containerizer_test.cpp 
> da52860f4a1d5363d7b61b9a8bb6abad02d89736 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 5bb85034c22caef64054c1629f6fd55d227e48b1 
>   src/tests/containerizer/isolator_tests.cpp 
> 91178b69ccbf5b6cbf64421e5602e6d554fc34ca 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> 4a03af2c9c0643d964b1d76e2096341b59bf5dce 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 95b493c6f479eef52ee0c9a44ac40254ed76ebae 
>   src/tests/containerizer/port_mapping_tests.cpp 
> e3aea53468fa00374320a8b89bdbb64f38e44b01 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 98d5a46149ef63cedcbf4915b5b6e9842ac67dbb 
>   src/tests/disk_quota_tests.cpp 1577cf71c4d1fbad11e2ec939c4ceae2d5b25f97 
>   src/tests/fetcher_cache_tests.cpp 1fb1e213d3c35479789688d1a3a49a3c6058b198 
>   src/tests/health_check_tests.cpp 26d05e7f7db9306c8b9164e2e7f843793c909e67 
>   src/tests/hook_tests.cpp 715b0b07e1befdca5b09172a439edccd7546d77f 
>   src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
>   src/tests/mesos.cpp 365ebe8335c37bfdb983a5424d4c995fa9b76a22 
>   src/tests/slave_recovery_tests.cpp c0e4ff75b35c9e806741aab5696771e66d2c2ea8 
>   src/tests/slave_tests.cpp e4fb490a1d877547fe883c22dbc47bb4969ecef6 
> 
> Diff: https://reviews.apache.org/r/41590/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>