You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2018/02/22 10:07:58 UTC
Review Request 65751: Added test fixture for a hung Docker daemon.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65751/
-----------------------------------------------------------
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/1/
Testing
-------
Thanks,
Greg Mann
Re: Review Request 65751: Added test fixture for a hung Docker daemon.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
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.
Posted by Greg Mann <gr...@mesosphere.io>.
> On Feb. 23, 2018, 6:06 p.m., Gilbert Song wrote:
> > src/tests/containerizer/docker_containerizer_tests.cpp
> > Lines 5148 (patched)
> > <https://reviews.apache.org/r/65751/diff/3/?file=1963501#file1963501line5148>
> >
> > 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.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/65751/#comment278300>
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.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
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.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
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.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
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.
Posted by Gilbert Song <so...@gmail.com>.
> On Feb. 23, 2018, 8:35 a.m., Andrei Budnik wrote:
> > src/tests/containerizer/docker_containerizer_tests.cpp
> > Lines 5133 (patched)
> > <https://reviews.apache.org/r/65751/diff/3/?file=1963501#file1963501line5133>
> >
> > `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.
Posted by Andrei Budnik <ab...@mesosphere.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/65751/#comment278292>
`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)
<https://reviews.apache.org/r/65751/#comment278291>
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.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
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.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
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.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
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.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
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