You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Timothy Chen <tn...@apache.org> on 2014/08/17 00:23:39 UTC

Review Request 24776: Add docker containerizer destroy tests

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

Review request for mesos, Benjamin Hindman and Jie Yu.


Repository: mesos-git


Description
-------

Add docker containerizer destroy tests


Diffs
-----

  src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
  src/slave/containerizer/docker.cpp fe5b29167811d4ac2fe29070c70a04f84093a6ff 
  src/tests/docker_containerizer_tests.cpp 8654f9c787bd207f6a7b821651e0c083bea9dc8a 

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


Testing
-------

make check


Thanks,

Timothy Chen

Re: Review Request 24776: Add docker containerizer destroy tests

Posted by Timothy Chen <tn...@apache.org>.

> On Sept. 9, 2014, 6:15 p.m., Benjamin Hindman wrote:
> > Why did you need to mock DockerContainerizerProcess in order to write these tests? Couldn't you have just used the existing MockDockerContainerizer?

I wanted to simulate having destroy called in a pull/fetching state, so I thought the only way to do so is to mock the process since the callbacks are on DockerContainerizerProcess and not the Containerizer, so the callbacks for fetch and pull blocks and I can call destroy in that state and verify it was able to destroy.


- Timothy


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


On Aug. 16, 2014, 10:23 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24776/
> -----------------------------------------------------------
> 
> (Updated Aug. 16, 2014, 10:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Add docker containerizer destroy tests
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
>   src/slave/containerizer/docker.cpp fe5b29167811d4ac2fe29070c70a04f84093a6ff 
>   src/tests/docker_containerizer_tests.cpp 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
> 
> Diff: https://reviews.apache.org/r/24776/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 24776: Add docker containerizer destroy tests

Posted by Benjamin Hindman <be...@berkeley.edu>.

> On Sept. 9, 2014, 6:15 p.m., Benjamin Hindman wrote:
> > Why did you need to mock DockerContainerizerProcess in order to write these tests? Couldn't you have just used the existing MockDockerContainerizer?
> 
> Timothy Chen wrote:
>     I wanted to simulate having destroy called in a pull/fetching state, so I thought the only way to do so is to mock the process since the callbacks are on DockerContainerizerProcess and not the Containerizer, so the callbacks for fetch and pull blocks and I can call destroy in that state and verify it was able to destroy.

Yup yup, you're correct, my apologies!


- Benjamin


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


On Sept. 15, 2014, 8:33 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24776/
> -----------------------------------------------------------
> 
> (Updated Sept. 15, 2014, 8:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Review: https://reviews.apache.org/r/24776
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
>   src/slave/containerizer/docker.cpp 0febbac5df4126f6c8d9a06dd0ba1668d041b34a 
>   src/tests/docker_containerizer_tests.cpp 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
> 
> Diff: https://reviews.apache.org/r/24776/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 24776: Add docker containerizer destroy tests

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/#review52758
-----------------------------------------------------------


Why did you need to mock DockerContainerizerProcess in order to write these tests? Couldn't you have just used the existing MockDockerContainerizer?


src/tests/docker_containerizer_tests.cpp
<https://reviews.apache.org/r/24776/#comment91796>

    Please move '=' back on previous line.



src/tests/docker_containerizer_tests.cpp
<https://reviews.apache.org/r/24776/#comment91798>

    Looks like wierd indentation here.


- Benjamin Hindman


On Aug. 16, 2014, 10:23 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24776/
> -----------------------------------------------------------
> 
> (Updated Aug. 16, 2014, 10:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Add docker containerizer destroy tests
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
>   src/slave/containerizer/docker.cpp fe5b29167811d4ac2fe29070c70a04f84093a6ff 
>   src/tests/docker_containerizer_tests.cpp 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
> 
> Diff: https://reviews.apache.org/r/24776/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 24776: Add docker containerizer destroy tests

Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/#review50835
-----------------------------------------------------------


Patch looks great!

Reviews applied: [24776]

All tests passed.

- Mesos ReviewBot


On Aug. 16, 2014, 10:23 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24776/
> -----------------------------------------------------------
> 
> (Updated Aug. 16, 2014, 10:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Add docker containerizer destroy tests
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
>   src/slave/containerizer/docker.cpp fe5b29167811d4ac2fe29070c70a04f84093a6ff 
>   src/tests/docker_containerizer_tests.cpp 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
> 
> Diff: https://reviews.apache.org/r/24776/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 24776: Add docker containerizer destroy tests

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/#review51750
-----------------------------------------------------------


Jie or Ben can you guys commit this?

- Timothy Chen


On Aug. 16, 2014, 10:23 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24776/
> -----------------------------------------------------------
> 
> (Updated Aug. 16, 2014, 10:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Add docker containerizer destroy tests
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
>   src/slave/containerizer/docker.cpp fe5b29167811d4ac2fe29070c70a04f84093a6ff 
>   src/tests/docker_containerizer_tests.cpp 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
> 
> Diff: https://reviews.apache.org/r/24776/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 24776: Add docker containerizer destroy tests

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/#review51751
-----------------------------------------------------------


Jie or Ben can you guys commit this?

- Timothy Chen


On Aug. 16, 2014, 10:23 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24776/
> -----------------------------------------------------------
> 
> (Updated Aug. 16, 2014, 10:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Add docker containerizer destroy tests
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
>   src/slave/containerizer/docker.cpp fe5b29167811d4ac2fe29070c70a04f84093a6ff 
>   src/tests/docker_containerizer_tests.cpp 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
> 
> Diff: https://reviews.apache.org/r/24776/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 24776: Add docker containerizer destroy tests

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/#review53535
-----------------------------------------------------------

Ship it!


LGTM, but as always let's definitely resolve the two issues before committing.


src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/24776/#comment93231>

    Hmm, these are a wierd semantics ... can you please document the construct sufficiently about this (that it also gets terminated in the destructor).



src/tests/docker_containerizer_tests.cpp
<https://reviews.apache.org/r/24776/#comment93235>

    Why is this '* 2'? This raises my eyebrow that you had to do this to get the test to deterministically pass which makes me wonder if we're not waiting for all the right conditions and we could still have a non-deterministic failure if timing doesn't work out. We try and avoid these kind of constants in order to make our tests as deterministic as possible.


- Benjamin Hindman


On Sept. 15, 2014, 8:33 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24776/
> -----------------------------------------------------------
> 
> (Updated Sept. 15, 2014, 8:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Review: https://reviews.apache.org/r/24776
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
>   src/slave/containerizer/docker.cpp 0febbac5df4126f6c8d9a06dd0ba1668d041b34a 
>   src/tests/docker_containerizer_tests.cpp 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
> 
> Diff: https://reviews.apache.org/r/24776/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 24776: Add docker containerizer destroy tests

Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/#review53418
-----------------------------------------------------------


Patch looks great!

Reviews applied: [24776]

All tests passed.

- Mesos ReviewBot


On Sept. 15, 2014, 8:33 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24776/
> -----------------------------------------------------------
> 
> (Updated Sept. 15, 2014, 8:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Review: https://reviews.apache.org/r/24776
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
>   src/slave/containerizer/docker.cpp 0febbac5df4126f6c8d9a06dd0ba1668d041b34a 
>   src/tests/docker_containerizer_tests.cpp 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
> 
> Diff: https://reviews.apache.org/r/24776/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 24776: Add docker containerizer destroy tests

Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/#review53692
-----------------------------------------------------------


Bad patch!

Reviews applied: [24776]

Failed command: git apply --index 24776.patch

Error:
 error: patch failed: src/slave/containerizer/docker.cpp:73
error: src/slave/containerizer/docker.cpp: patch does not apply
error: patch failed: src/tests/docker_containerizer_tests.cpp:1630
error: src/tests/docker_containerizer_tests.cpp: patch does not apply

- Mesos ReviewBot


On Sept. 17, 2014, 12:21 a.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24776/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2014, 12:21 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Review: https://reviews.apache.org/r/24776
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
>   src/slave/containerizer/docker.cpp 0febbac5df4126f6c8d9a06dd0ba1668d041b34a 
>   src/tests/docker_containerizer_tests.cpp 8654f9c787bd207f6a7b821651e0c083bea9dc8a 
> 
> Diff: https://reviews.apache.org/r/24776/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 24776: Add docker containerizer destroy tests and modify docker pull

Posted by Timothy Chen <tn...@apache.org>.

> On Sept. 17, 2014, 11:20 p.m., Adam B wrote:
> > src/slave/containerizer/docker.cpp, lines 353-358
> > <https://reviews.apache.org/r/24776/diff/4/?file=692992#file692992line353>
> >
> >     Why call new DockerContainerizerProcess twice?

Oops, bad merge!


- Timothy


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


On Sept. 17, 2014, 11:11 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24776/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2014, 11:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Review: https://reviews.apache.org/r/24776
> 
> 
> Diffs
> -----
> 
>   src/docker/docker.hpp 443db49318a5923b987c06cda8060ccfb3301a2f 
>   src/docker/docker.cpp 6063114e700d13b1cd5d59cff356518021eb3286 
>   src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
>   src/slave/containerizer/docker.cpp 9a2948951f57f3ab16291df51cd9f33e5e96add4 
>   src/tests/docker_containerizer_tests.cpp 1981f49d228903ccaa094a9747dec49054c1e0f2 
> 
> Diff: https://reviews.apache.org/r/24776/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 24776: Add docker containerizer destroy tests and modify docker pull

Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/#review53767
-----------------------------------------------------------



src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/24776/#comment93577>

    Why call new DockerContainerizerProcess twice?


- Adam B


On Sept. 17, 2014, 4:11 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24776/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2014, 4:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Review: https://reviews.apache.org/r/24776
> 
> 
> Diffs
> -----
> 
>   src/docker/docker.hpp 443db49318a5923b987c06cda8060ccfb3301a2f 
>   src/docker/docker.cpp 6063114e700d13b1cd5d59cff356518021eb3286 
>   src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
>   src/slave/containerizer/docker.cpp 9a2948951f57f3ab16291df51cd9f33e5e96add4 
>   src/tests/docker_containerizer_tests.cpp 1981f49d228903ccaa094a9747dec49054c1e0f2 
> 
> Diff: https://reviews.apache.org/r/24776/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 24776: Add docker containerizer destroy tests and modify docker pull

Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/#review53781
-----------------------------------------------------------


Patch looks great!

Reviews applied: [24776]

All tests passed.

- Mesos ReviewBot


On Sept. 17, 2014, 11:22 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24776/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2014, 11:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Review: https://reviews.apache.org/r/24776
> 
> 
> Diffs
> -----
> 
>   src/docker/docker.hpp 443db49318a5923b987c06cda8060ccfb3301a2f 
>   src/docker/docker.cpp 6063114e700d13b1cd5d59cff356518021eb3286 
>   src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
>   src/slave/containerizer/docker.cpp 9a2948951f57f3ab16291df51cd9f33e5e96add4 
>   src/tests/docker_containerizer_tests.cpp 1981f49d228903ccaa094a9747dec49054c1e0f2 
> 
> Diff: https://reviews.apache.org/r/24776/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 24776: Add destroy tests for docker containerizer.

Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/#review59939
-----------------------------------------------------------


Patch looks great!

Reviews applied: [24776]

All tests passed.

- Mesos ReviewBot


On Nov. 5, 2014, 7:26 a.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24776/
> -----------------------------------------------------------
> 
> (Updated Nov. 5, 2014, 7:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Review: https://reviews.apache.org/r/24776
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/docker.hpp ec6b9cd308e9a16e05f016e8aeadbe77646d1621 
>   src/slave/containerizer/docker.cpp a6689203adbdcb0ad12583389eaeb83329e4ef6b 
>   src/tests/docker_containerizer_tests.cpp 9d4ccc57f58d61c62aab5cdc79a129e987920bf6 
> 
> Diff: https://reviews.apache.org/r/24776/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 24776: Add destroy tests for docker containerizer.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/#review60470
-----------------------------------------------------------

Ship it!



src/slave/containerizer/docker.hpp
<https://reviews.apache.org/r/24776/#comment101871>

    How come you don't need to do 'process::Owned' here? There is probably some bad include file that is including all of the 'process' namespace, but we should really be doing process::Owned in headers (just like you do process::Shared above).



src/slave/containerizer/docker.hpp
<https://reviews.apache.org/r/24776/#comment101872>

    process::Owned



src/slave/containerizer/docker.hpp
<https://reviews.apache.org/r/24776/#comment101873>

    process::PID (here and everywhere else please).



src/slave/containerizer/docker.hpp
<https://reviews.apache.org/r/24776/#comment101874>

    For the code that you have actually changed please go ahead and s/> >/>>/ (but just for the code you've changed, not for all the code in all the files you've changed).



src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/24776/#comment101875>

    Please update all of the 'dispatch' calls that need to be wrapped to have consistent wrapping style in this file. Thanks!



src/tests/docker_containerizer_tests.cpp
<https://reviews.apache.org/r/24776/#comment101876>

    Esoteric style nit: we've only used 1 newline between things declared/defined inside of classes, but two newlines for things declared at the top level. So please kill the double newline here and throughout the review. Thank you Tim!



src/tests/docker_containerizer_tests.cpp
<https://reviews.apache.org/r/24776/#comment101877>

    Did you want a & here? And no need to use the _ prefix here.


- Benjamin Hindman


On Nov. 5, 2014, 7:26 a.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24776/
> -----------------------------------------------------------
> 
> (Updated Nov. 5, 2014, 7:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Review: https://reviews.apache.org/r/24776
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/docker.hpp ec6b9cd308e9a16e05f016e8aeadbe77646d1621 
>   src/slave/containerizer/docker.cpp a6689203adbdcb0ad12583389eaeb83329e4ef6b 
>   src/tests/docker_containerizer_tests.cpp 9d4ccc57f58d61c62aab5cdc79a129e987920bf6 
> 
> Diff: https://reviews.apache.org/r/24776/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 24776: Add destroy tests for docker containerizer.

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/
-----------------------------------------------------------

(Updated Nov. 5, 2014, 7:26 a.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
-------

Rebased and also update the tests to call destroy directly.


Summary (updated)
-----------------

Add destroy tests for docker containerizer.


Repository: mesos-git


Description
-------

Review: https://reviews.apache.org/r/24776


Diffs (updated)
-----

  src/slave/containerizer/docker.hpp ec6b9cd308e9a16e05f016e8aeadbe77646d1621 
  src/slave/containerizer/docker.cpp a6689203adbdcb0ad12583389eaeb83329e4ef6b 
  src/tests/docker_containerizer_tests.cpp 9d4ccc57f58d61c62aab5cdc79a129e987920bf6 

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


Testing
-------

make check


Thanks,

Timothy Chen

Re: Review Request 24776: Add docker containerizer destroy tests

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/
-----------------------------------------------------------

(Updated Nov. 3, 2014, 8:53 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Summary (updated)
-----------------

Add docker containerizer destroy tests


Repository: mesos-git


Description
-------

Review: https://reviews.apache.org/r/24776


Diffs
-----

  src/docker/docker.hpp 443db49318a5923b987c06cda8060ccfb3301a2f 
  src/docker/docker.cpp 6063114e700d13b1cd5d59cff356518021eb3286 
  src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
  src/slave/containerizer/docker.cpp 9a2948951f57f3ab16291df51cd9f33e5e96add4 
  src/tests/docker_containerizer_tests.cpp 1981f49d228903ccaa094a9747dec49054c1e0f2 

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


Testing
-------

make check


Thanks,

Timothy Chen

Re: Review Request 24776: Add docker containerizer destroy tests and modify docker pull

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/
-----------------------------------------------------------

(Updated Sept. 17, 2014, 11:22 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Repository: mesos-git


Description
-------

Review: https://reviews.apache.org/r/24776


Diffs
-----

  src/docker/docker.hpp 443db49318a5923b987c06cda8060ccfb3301a2f 
  src/docker/docker.cpp 6063114e700d13b1cd5d59cff356518021eb3286 
  src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
  src/slave/containerizer/docker.cpp 9a2948951f57f3ab16291df51cd9f33e5e96add4 
  src/tests/docker_containerizer_tests.cpp 1981f49d228903ccaa094a9747dec49054c1e0f2 

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


Testing
-------

make check


Thanks,

Timothy Chen

Re: Review Request 24776: Add docker containerizer destroy tests and modify docker pull

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/
-----------------------------------------------------------

(Updated Sept. 17, 2014, 11:22 p.m.)


Review request for drill, Benjamin Hindman and Jie Yu.


Repository: mesos-git


Description
-------

Review: https://reviews.apache.org/r/24776


Diffs (updated)
-----

  src/docker/docker.hpp 443db49318a5923b987c06cda8060ccfb3301a2f 
  src/docker/docker.cpp 6063114e700d13b1cd5d59cff356518021eb3286 
  src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
  src/slave/containerizer/docker.cpp 9a2948951f57f3ab16291df51cd9f33e5e96add4 
  src/tests/docker_containerizer_tests.cpp 1981f49d228903ccaa094a9747dec49054c1e0f2 

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


Testing
-------

make check


Thanks,

Timothy Chen

Re: Review Request 24776: Add docker containerizer destroy tests and modify docker pull

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/
-----------------------------------------------------------

(Updated Sept. 17, 2014, 11:22 p.m.)


Review request for drill, Benjamin Hindman and Jie Yu.


Repository: mesos-git


Description
-------

Review: https://reviews.apache.org/r/24776


Diffs (updated)
-----

  src/docker/docker.hpp 443db49318a5923b987c06cda8060ccfb3301a2f 
  src/docker/docker.cpp 6063114e700d13b1cd5d59cff356518021eb3286 
  src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
  src/slave/containerizer/docker.cpp 9a2948951f57f3ab16291df51cd9f33e5e96add4 
  src/tests/docker_containerizer_tests.cpp 1981f49d228903ccaa094a9747dec49054c1e0f2 

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


Testing
-------

make check


Thanks,

Timothy Chen

Re: Review Request 24776: Add docker containerizer destroy tests and modify docker pull

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/
-----------------------------------------------------------

(Updated Sept. 17, 2014, 11:11 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Summary (updated)
-----------------

Add docker containerizer destroy tests and modify docker pull


Repository: mesos-git


Description
-------

Review: https://reviews.apache.org/r/24776


Diffs (updated)
-----

  src/docker/docker.hpp 443db49318a5923b987c06cda8060ccfb3301a2f 
  src/docker/docker.cpp 6063114e700d13b1cd5d59cff356518021eb3286 
  src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
  src/slave/containerizer/docker.cpp 9a2948951f57f3ab16291df51cd9f33e5e96add4 
  src/tests/docker_containerizer_tests.cpp 1981f49d228903ccaa094a9747dec49054c1e0f2 

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


Testing
-------

make check


Thanks,

Timothy Chen

Re: Review Request 24776: Add docker containerizer destroy tests

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/
-----------------------------------------------------------

(Updated Sept. 17, 2014, 12:21 a.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Repository: mesos-git


Description
-------

Review: https://reviews.apache.org/r/24776


Diffs (updated)
-----

  src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
  src/slave/containerizer/docker.cpp 0febbac5df4126f6c8d9a06dd0ba1668d041b34a 
  src/tests/docker_containerizer_tests.cpp 8654f9c787bd207f6a7b821651e0c083bea9dc8a 

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


Testing
-------

make check


Thanks,

Timothy Chen

Re: Review Request 24776: Add docker containerizer destroy tests

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24776/
-----------------------------------------------------------

(Updated Sept. 15, 2014, 8:33 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Repository: mesos-git


Description (updated)
-------

Review: https://reviews.apache.org/r/24776


Diffs (updated)
-----

  src/slave/containerizer/docker.hpp fbbd45d77e5f2f74ca893552f85eb893b3dd948f 
  src/slave/containerizer/docker.cpp 0febbac5df4126f6c8d9a06dd0ba1668d041b34a 
  src/tests/docker_containerizer_tests.cpp 8654f9c787bd207f6a7b821651e0c083bea9dc8a 

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


Testing
-------

make check


Thanks,

Timothy Chen