Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-23 Thread Greg Mann 

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

(Updated Feb. 23, 2018, 11:24 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.


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


Repository: mesos


Description (updated)
---

The new 'HungDockerTest' class allows test authors to force
certain Docker daemon calls to be delayed for a specified
duration.


Diffs
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
d1e657050d623ad0412208b3aa3e3101e3654e99 


Diff: https://reviews.apache.org/r/65751/diff/4/


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-23 Thread Gilbert Song 

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


Ship it!




Ship It!

- Gilbert Song


On Feb. 23, 2018, 2:39 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65751/
> ---
> 
> (Updated Feb. 23, 2018, 2:39 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Bugs: MESOS-8591
> https://issues.apache.org/jira/browse/MESOS-8591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test fixture for a hung Docker daemon.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d1e657050d623ad0412208b3aa3e3101e3654e99 
> 
> 
> Diff: https://reviews.apache.org/r/65751/diff/4/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-23 Thread Greg Mann 

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

(Updated Feb. 23, 2018, 10:39 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.


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


Repository: mesos


Description (updated)
---

Added test fixture for a hung Docker daemon.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
d1e657050d623ad0412208b3aa3e3101e3654e99 


Diff: https://reviews.apache.org/r/65751/diff/4/

Changes: https://reviews.apache.org/r/65751/diff/3-4/


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-23 Thread Greg Mann 


> On Feb. 23, 2018, 6:06 p.m., Gilbert Song wrote:
> > src/tests/containerizer/docker_containerizer_tests.cpp
> > Lines 5148 (patched)
> > 
> >
> > Should we use `double`?

The `sleep` command only accepts integer arguments. I opted for `sleep` due to 
portability, but if there is a highly-portable option which allows sleeping for 
fractions of a second, that would be great.


- Greg


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


On Feb. 22, 2018, 11:28 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65751/
> ---
> 
> (Updated Feb. 22, 2018, 11:28 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Bugs: MESOS-8591
> https://issues.apache.org/jira/browse/MESOS-8591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new `HungDockerTest` class allows test authors to force
> certain Docker daemon calls to be delayed for a specified
> duration.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d1e657050d623ad0412208b3aa3e3101e3654e99 
> 
> 
> Diff: https://reviews.apache.org/r/65751/diff/3/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-23 Thread Gilbert Song 


> On Feb. 23, 2018, 8:35 a.m., Andrei Budnik wrote:
> > src/tests/containerizer/docker_containerizer_tests.cpp
> > Lines 5133 (patched)
> > 
> >
> > `os::write()` is not atomical, so we should write to a temporary file, 
> > then rename it: `mv new_env.tmp test-docker.env`. Otherwise, if someone 
> > launches docker cli, while the script is being updated - `test-docker.sh` 
> > might read/import half-written env file.
> > 
> > Probably, we don't need to fix it now.

A TODO sounds ok to me for now.


- Gilbert


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


On Feb. 22, 2018, 3:28 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65751/
> ---
> 
> (Updated Feb. 22, 2018, 3:28 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Bugs: MESOS-8591
> https://issues.apache.org/jira/browse/MESOS-8591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new `HungDockerTest` class allows test authors to force
> certain Docker daemon calls to be delayed for a specified
> duration.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d1e657050d623ad0412208b3aa3e3101e3654e99 
> 
> 
> Diff: https://reviews.apache.org/r/65751/diff/3/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-23 Thread Gilbert Song 

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


Ship it!




Ship It!

- Gilbert Song


On Feb. 22, 2018, 3:28 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65751/
> ---
> 
> (Updated Feb. 22, 2018, 3:28 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Bugs: MESOS-8591
> https://issues.apache.org/jira/browse/MESOS-8591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new `HungDockerTest` class allows test authors to force
> certain Docker daemon calls to be delayed for a specified
> duration.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d1e657050d623ad0412208b3aa3e3101e3654e99 
> 
> 
> Diff: https://reviews.apache.org/r/65751/diff/3/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-23 Thread Gilbert Song 

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




src/tests/containerizer/docker_containerizer_tests.cpp
Lines 5148 (patched)


Should we use `double`?


- Gilbert Song


On Feb. 22, 2018, 3:28 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65751/
> ---
> 
> (Updated Feb. 22, 2018, 3:28 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Bugs: MESOS-8591
> https://issues.apache.org/jira/browse/MESOS-8591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new `HungDockerTest` class allows test authors to force
> certain Docker daemon calls to be delayed for a specified
> duration.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d1e657050d623ad0412208b3aa3e3101e3654e99 
> 
> 
> Diff: https://reviews.apache.org/r/65751/diff/3/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-23 Thread Andrei Budnik 

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




src/tests/containerizer/docker_containerizer_tests.cpp
Lines 5133 (patched)


`os::write()` is not atomical, so we should write to a temporary file, then 
rename it: `mv new_env.tmp test-docker.env`. Otherwise, if someone launches 
docker cli, while the script is being updated - `test-docker.sh` might 
read/import half-written env file.

Probably, we don't need to fix it now.



src/tests/containerizer/docker_containerizer_tests.cpp
Lines 5164-5165 (patched)


Why do we need `export TEST_DOCKER_ARGUMENTS=$@`?

Let's get rid of `shift 2` command and `ARGS` variable.


- Andrei Budnik


On Feb. 22, 2018, 11:28 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65751/
> ---
> 
> (Updated Feb. 22, 2018, 11:28 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Bugs: MESOS-8591
> https://issues.apache.org/jira/browse/MESOS-8591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new `HungDockerTest` class allows test authors to force
> certain Docker daemon calls to be delayed for a specified
> duration.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d1e657050d623ad0412208b3aa3e3101e3654e99 
> 
> 
> Diff: https://reviews.apache.org/r/65751/diff/3/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-22 Thread Greg Mann 

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

(Updated Feb. 22, 2018, 11:28 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.


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


Repository: mesos


Description
---

The new `HungDockerTest` class allows test authors to force
certain Docker daemon calls to be delayed for a specified
duration.


Diffs
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
d1e657050d623ad0412208b3aa3e3101e3654e99 


Diff: https://reviews.apache.org/r/65751/diff/3/


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-22 Thread Greg Mann 

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

(Updated Feb. 22, 2018, 10:56 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.


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


Repository: mesos


Description (updated)
---

The new `HungDockerTest` class allows test authors to force
certain Docker daemon calls to be delayed for a specified
duration.


Diffs
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
d1e657050d623ad0412208b3aa3e3101e3654e99 


Diff: https://reviews.apache.org/r/65751/diff/3/


Testing (updated)
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-22 Thread Greg Mann 

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

(Updated Feb. 22, 2018, 10:55 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.


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


Repository: mesos


Description
---

Added test fixture for a hung Docker daemon.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
d1e657050d623ad0412208b3aa3e3101e3654e99 


Diff: https://reviews.apache.org/r/65751/diff/3/

Changes: https://reviews.apache.org/r/65751/diff/2-3/


Testing
---


Thanks,

Greg Mann

Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-22 Thread Greg Mann 

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

(Updated Feb. 22, 2018, 10:46 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.


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


Repository: mesos


Description (updated)
---

Added test fixture for a hung Docker daemon.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
d1e657050d623ad0412208b3aa3e3101e3654e99 


Diff: https://reviews.apache.org/r/65751/diff/2/

Changes: https://reviews.apache.org/r/65751/diff/1-2/


Testing
---


Thanks,

Greg Mann