You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rukletsov <ru...@gmail.com> on 2017/04/27 22:18:03 UTC

Review Request 58821: Added a test that verifies a task and its check share the work dir.

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

Review request for mesos, Gast�n Kleiman, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
-------

See summary.


Diffs
-----

  src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 


Diff: https://reviews.apache.org/r/58821/diff/1/


Testing
-------

`make check` on Mac OS and Fedora 24


Thanks,

Alexander Rukletsov


Re: Review Request 58821: Added a test that verifies a task and its check share the work dir.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58821/#review173288
-----------------------------------------------------------



Patch looks great!

Reviews applied: [58262, 58718, 58817, 58818, 58819, 58820, 58263, 58821]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On April 27, 2017, 10:18 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58821/
> -----------------------------------------------------------
> 
> (Updated April 27, 2017, 10:18 p.m.)
> 
> 
> Review request for mesos, Gast�n Kleiman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7433
>     https://issues.apache.org/jira/browse/MESOS-7433
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 
> 
> 
> Diff: https://reviews.apache.org/r/58821/diff/1/
> 
> 
> Testing
> -------
> 
> `make check` on Mac OS and Fedora 24
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 58821: Added a test that verifies a task and its check share the work dir.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58821/#review174572
-----------------------------------------------------------


Ship it!




Ship It!

- Vinod Kone


On May 10, 2017, 1:55 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58821/
> -----------------------------------------------------------
> 
> (Updated May 10, 2017, 1:55 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7433
>     https://issues.apache.org/jira/browse/MESOS-7433
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 
> 
> 
> Diff: https://reviews.apache.org/r/58821/diff/5/
> 
> 
> Testing
> -------
> 
> `make check` on Mac OS and Fedora 24
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 58821: Added a test that verifies a task and its check share the work dir.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58821/
-----------------------------------------------------------

(Updated May 10, 2017, 1:55 p.m.)


Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.


Changes
-------

Rebased. NNTR.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 


Diff: https://reviews.apache.org/r/58821/diff/5/

Changes: https://reviews.apache.org/r/58821/diff/4-5/


Testing
-------

`make check` on Mac OS and Fedora 24


Thanks,

Alexander Rukletsov


Re: Review Request 58821: Added a test that verifies a task and its check share the work dir.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On May 9, 2017, 9:17 p.m., Vinod Kone wrote:
> > src/tests/check_tests.cpp
> > Lines 706-730 (patched)
> > <https://reviews.apache.org/r/58821/diff/4/?file=1711319#file1711319line706>
> >
> >     I wonder if it might be better to improve the check command instead of doing this (is there a guarantee that the file exists by the second check update?) For example the check command could wait until the file exists in a while loop and we can rely on the check timeout to break the loop incase of a failure.

Yes, there is a guarantee, that the file exists when we get the second update. Indeed, the second check might not see the file, but in this case no check status update is sent.

We can change the test so that the check command only returns when the file exists. In this case both approaches achieve the same thing. I'm slightly inclined to keep the current one because it gives a reader a better understanding about the check mechanics.


- Alexander


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


On May 9, 2017, 2:32 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58821/
> -----------------------------------------------------------
> 
> (Updated May 9, 2017, 2:32 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7433
>     https://issues.apache.org/jira/browse/MESOS-7433
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 
> 
> 
> Diff: https://reviews.apache.org/r/58821/diff/4/
> 
> 
> Testing
> -------
> 
> `make check` on Mac OS and Fedora 24
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 58821: Added a test that verifies a task and its check share the work dir.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58821/#review174378
-----------------------------------------------------------




src/tests/check_tests.cpp
Lines 706-730 (patched)
<https://reviews.apache.org/r/58821/#comment247506>

    I wonder if it might be better to improve the check command instead of doing this (is there a guarantee that the file exists by the second check update?) For example the check command could wait until the file exists in a while loop and we can rely on the check timeout to break the loop incase of a failure.


- Vinod Kone


On May 9, 2017, 2:32 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58821/
> -----------------------------------------------------------
> 
> (Updated May 9, 2017, 2:32 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7433
>     https://issues.apache.org/jira/browse/MESOS-7433
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 
> 
> 
> Diff: https://reviews.apache.org/r/58821/diff/4/
> 
> 
> Testing
> -------
> 
> `make check` on Mac OS and Fedora 24
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 58821: Added a test that verifies a task and its check share the work dir.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58821/
-----------------------------------------------------------

(Updated May 9, 2017, 2:32 p.m.)


Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.


Changes
-------

Avoided a potential race in the test.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 


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

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


Testing
-------

`make check` on Mac OS and Fedora 24


Thanks,

Alexander Rukletsov


Re: Review Request 58821: Added a test that verifies a task and its check share the work dir.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58821/
-----------------------------------------------------------

(Updated May 3, 2017, 4:46 p.m.)


Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 29007898ec04e922266068a8519731b13d351a82 


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

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


Testing
-------

`make check` on Mac OS and Fedora 24


Thanks,

Alexander Rukletsov


Re: Review Request 58821: Added a test that verifies a task and its check share the work dir.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58821/#review173339
-----------------------------------------------------------




src/tests/check_tests.cpp
Lines 612-616 (patched)
<https://reviews.apache.org/r/58821/#comment246358>

    This test appears to be flaky on some Ubuntu and CentOS machines (but nor Fedora 24). I will investigate and update accordingly.


- Alexander Rukletsov


On April 28, 2017, 4:14 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58821/
> -----------------------------------------------------------
> 
> (Updated April 28, 2017, 4:14 p.m.)
> 
> 
> Review request for mesos, Gast�n Kleiman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7433
>     https://issues.apache.org/jira/browse/MESOS-7433
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 
> 
> 
> Diff: https://reviews.apache.org/r/58821/diff/2/
> 
> 
> Testing
> -------
> 
> `make check` on Mac OS and Fedora 24
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 58821: Added a test that verifies a task and its check share the work dir.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58821/#review173389
-----------------------------------------------------------



Patch looks great!

Reviews applied: [58847, 58262, 58718, 58817, 58818, 58819, 58820, 58263, 58821]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On April 28, 2017, 9:14 a.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58821/
> -----------------------------------------------------------
> 
> (Updated April 28, 2017, 9:14 a.m.)
> 
> 
> Review request for mesos, Gast�n Kleiman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7433
>     https://issues.apache.org/jira/browse/MESOS-7433
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 
> 
> 
> Diff: https://reviews.apache.org/r/58821/diff/2/
> 
> 
> Testing
> -------
> 
> `make check` on Mac OS and Fedora 24
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 58821: Added a test that verifies a task and its check share the work dir.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58821/
-----------------------------------------------------------

(Updated April 28, 2017, 4:14 p.m.)


Review request for mesos, Gast�n Kleiman, Jie Yu, and Vinod Kone.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 


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

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


Testing
-------

`make check` on Mac OS and Fedora 24


Thanks,

Alexander Rukletsov