You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2016/02/24 19:51:10 UTC
Review Request 43960: Added an end-to-end test for docker registry
puller.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43960/
-----------------------------------------------------------
Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
Repository: mesos
Description
-------
Added an end-to-end test for docker registry puller.
Diffs
-----
src/tests/containerizer/provisioner_docker_tests.cpp b3c6f8848f2ceb2bd4dce35d6e7f813c2d4d2bd9
Diff: https://reviews.apache.org/r/43960/diff/
Testing
-------
sudo make check
Thanks,
Jie Yu
Re: Review Request 43960: Added an end-to-end test for docker registry
puller.
Posted by Jie Yu <yu...@gmail.com>.
> On Feb. 24, 2016, 8:03 p.m., Jojy Varghese wrote:
> > src/tests/containerizer/provisioner_docker_tests.cpp, line 361
> > <https://reviews.apache.org/r/43960/diff/1/?file=1268379#file1268379line361>
> >
> > Would it add value if we assert that the store does not currently have the image?
> >
> > Also, have assertions around the image being placed in the right dircetory?
The MesosTest fixture will create a new slave work directory for each slave started. So this is not needed.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43960/#review120565
-----------------------------------------------------------
On Feb. 24, 2016, 6:51 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43960/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2016, 6:51 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added an end-to-end test for docker registry puller.
>
>
> Diffs
> -----
>
> src/tests/containerizer/provisioner_docker_tests.cpp b3c6f8848f2ceb2bd4dce35d6e7f813c2d4d2bd9
>
> Diff: https://reviews.apache.org/r/43960/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 43960: Added an end-to-end test for docker registry
puller.
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43960/#review120565
-----------------------------------------------------------
src/tests/containerizer/provisioner_docker_tests.cpp (line 360)
<https://reviews.apache.org/r/43960/#comment182033>
Would it add value if we assert that the store does not currently have the image?
Also, have assertions around the image being placed in the right dircetory?
src/tests/containerizer/provisioner_docker_tests.cpp (line 365)
<https://reviews.apache.org/r/43960/#comment182032>
Would it make sense to name these as per their semantics? `status1` does not say a lot about them.
- Jojy Varghese
On Feb. 24, 2016, 6:51 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43960/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2016, 6:51 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added an end-to-end test for docker registry puller.
>
>
> Diffs
> -----
>
> src/tests/containerizer/provisioner_docker_tests.cpp b3c6f8848f2ceb2bd4dce35d6e7f813c2d4d2bd9
>
> Diff: https://reviews.apache.org/r/43960/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 43960: Added an end-to-end test for docker registry
puller.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43960/#review120638
-----------------------------------------------------------
src/tests/containerizer/provisioner_docker_tests.cpp (line 343)
<https://reviews.apache.org/r/43960/#comment182099>
.WillRepeatedly(Return()); // Ignore subsequent offers.
src/tests/containerizer/provisioner_docker_tests.cpp (line 348)
<https://reviews.apache.org/r/43960/#comment182100>
I think here `ASSERT_NE(0u, offers.get().size());` or `ASSERT_EQ(1u, offers.get().size());` may be more accurate
src/tests/containerizer/provisioner_docker_tests.cpp (line 359)
<https://reviews.apache.org/r/43960/#comment182101>
Why you are changing back to `busybox` but not `alpine`?
- Guangya Liu
On 二月 24, 2016, 6:51 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43960/
> -----------------------------------------------------------
>
> (Updated 二月 24, 2016, 6:51 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added an end-to-end test for docker registry puller.
>
>
> Diffs
> -----
>
> src/tests/containerizer/provisioner_docker_tests.cpp b3c6f8848f2ceb2bd4dce35d6e7f813c2d4d2bd9
>
> Diff: https://reviews.apache.org/r/43960/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 43960: Added an end-to-end test for docker registry
puller.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43960/#review120940
-----------------------------------------------------------
Patch looks great!
Reviews applied: [43958, 43959, 43960]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Feb. 26, 2016, 5:29 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43960/
> -----------------------------------------------------------
>
> (Updated Feb. 26, 2016, 5:29 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added an end-to-end test for docker registry puller.
>
>
> Diffs
> -----
>
> src/tests/containerizer/provisioner_docker_tests.cpp b3c6f8848f2ceb2bd4dce35d6e7f813c2d4d2bd9
>
> Diff: https://reviews.apache.org/r/43960/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 43960: Added an end-to-end test for docker registry
puller.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43960/#review120948
-----------------------------------------------------------
Ship it!
Ship It!
- Gilbert Song
On Feb. 26, 2016, 9:29 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43960/
> -----------------------------------------------------------
>
> (Updated Feb. 26, 2016, 9:29 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added an end-to-end test for docker registry puller.
>
>
> Diffs
> -----
>
> src/tests/containerizer/provisioner_docker_tests.cpp b3c6f8848f2ceb2bd4dce35d6e7f813c2d4d2bd9
>
> Diff: https://reviews.apache.org/r/43960/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 43960: Added an end-to-end test for docker registry
puller.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43960/
-----------------------------------------------------------
(Updated Feb. 26, 2016, 5:29 p.m.)
Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
Changes
-------
Addressed review comments.
Repository: mesos
Description
-------
Added an end-to-end test for docker registry puller.
Diffs (updated)
-----
src/tests/containerizer/provisioner_docker_tests.cpp b3c6f8848f2ceb2bd4dce35d6e7f813c2d4d2bd9
Diff: https://reviews.apache.org/r/43960/diff/
Testing
-------
sudo make check
Thanks,
Jie Yu