Re: Review Request 43960: Added an end-to-end test for docker registry puller.

[Gilbert Song]

---
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.

[Mesos ReviewBot]

---
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.

[Jie Yu]

---
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

Re: Review Request 43960: Added an end-to-end test for docker registry puller.

[Jie Yu]

> On Feb. 24, 2016, 8:03 p.m., Jojy Varghese wrote:
> > src/tests/containerizer/provisioner_docker_tests.cpp, line 361
> > 
> >
> > 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.

[Guangya Liu]

---
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)


.WillRepeatedly(Return()); // Ignore subsequent offers.



src/tests/containerizer/provisioner_docker_tests.cpp (line 348)


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)


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.

[Jojy Varghese]

---
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)


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)


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
> 
>