Re: Review Request 67398: Changed default executor tests to not use pipes for synchronization.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 15, 2018, 1 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67398/
> ---
> 
> (Updated June 15, 2018, 1 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-8917
> https://issues.apache.org/jira/browse/MESOS-8917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some tests of nested container functionality used pipes passed to
> launched tasks to detect whether a task has actually started executing
> the workload (`TASK_RUNNING` updates might be sent before the task
> workload is actually started).
> 
> Once we avoid leaking unspecified file descriptors into forked
> processes, this test setup will be broken. In this patch we replace
> the use of pipes for synchronization with HTTP requests to an actor
> running in the tests, or wait on other observable side effects.
> 
> 
> Diffs
> -
> 
>   src/tests/check_tests.cpp d48febfca220a9633b9884963bcf5a205db7f5e5 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> 6050e6ebed87249382d56aedb6d98d3cf9812bb9 
> 
> 
> Diff: https://reviews.apache.org/r/67398/diff/4/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> Tested in internal CI on a number of platforms.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67398: Changed default executor tests to not use pipes for synchronization.

[Benjamin Bannier]

> On June 12, 2018, 9:47 p.m., Jie Yu wrote:
> > src/tests/containerizer/nested_mesos_containerizer_tests.cpp
> > Lines 1017-1028 (original), 1012-1028 (patched)
> > 
> >
> > Any reason we cannot use a named pipe as well here?
> 
> Benjamin Bannier wrote:
> The container mounts its own filesystem. I am not sure how we'd make a 
> named pipe in the host filesystem visible (we need to be able to access the 
> pipe from thr outside to make sure we only inspect the filesystem in the 
> nested container once the parent filesystem is ready).

Changed test to use a named pipe in a volume now.


- Benjamin


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


On June 15, 2018, 3 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67398/
> ---
> 
> (Updated June 15, 2018, 3 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-8917
> https://issues.apache.org/jira/browse/MESOS-8917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some tests of nested container functionality used pipes passed to
> launched tasks to detect whether a task has actually started executing
> the workload (`TASK_RUNNING` updates might be sent before the task
> workload is actually started).
> 
> Once we avoid leaking unspecified file descriptors into forked
> processes, this test setup will be broken. In this patch we replace
> the use of pipes for synchronization with HTTP requests to an actor
> running in the tests, or wait on other observable side effects.
> 
> 
> Diffs
> -
> 
>   src/tests/check_tests.cpp d48febfca220a9633b9884963bcf5a205db7f5e5 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> 6050e6ebed87249382d56aedb6d98d3cf9812bb9 
> 
> 
> Diff: https://reviews.apache.org/r/67398/diff/4/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> Tested in internal CI on a number of platforms.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67398: Changed default executor tests to not use pipes for synchronization.

[Benjamin Bannier]

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

(Updated June 15, 2018, 3 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Removed `Waiter`.


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


Repository: mesos


Description
---

Some tests of nested container functionality used pipes passed to
launched tasks to detect whether a task has actually started executing
the workload (`TASK_RUNNING` updates might be sent before the task
workload is actually started).

Once we avoid leaking unspecified file descriptors into forked
processes, this test setup will be broken. In this patch we replace
the use of pipes for synchronization with HTTP requests to an actor
running in the tests, or wait on other observable side effects.


Diffs (updated)
-

  src/tests/check_tests.cpp d48febfca220a9633b9884963bcf5a205db7f5e5 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
6050e6ebed87249382d56aedb6d98d3cf9812bb9 


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

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


Testing
---

`sudo make check`

Tested in internal CI on a number of platforms.


Thanks,

Benjamin Bannier

Re: Review Request 67398: Changed default executor tests to not use pipes for synchronization.

[Benjamin Bannier]

> On Juni 12, 2018, 9:47 nachm., Jie Yu wrote:
> > src/tests/containerizer/nested_mesos_containerizer_tests.cpp
> > Lines 1017-1028 (original), 1012-1028 (patched)
> > 
> >
> > Any reason we cannot use a named pipe as well here?

The container mounts its own filesystem. I am not sure how we'd make a named 
pipe in the host filesystem visible (we need to be able to access the pipe from 
thr outside to make sure we only inspect the filesystem in the nested container 
once the parent filesystem is ready).


- Benjamin


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


On Juni 7, 2018, 2:40 nachm., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67398/
> ---
> 
> (Updated Juni 7, 2018, 2:40 nachm.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-8917
> https://issues.apache.org/jira/browse/MESOS-8917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some tests of nested container functionality used pipes passed to
> launched tasks to detect whether a task has actually started executing
> the workload (`TASK_RUNNING` updates might be sent before the task
> workload is actually started).
> 
> Once we avoid leaking unspecified file descriptors into forked
> processes, this test setup will be broken. In this patch we replace
> the use of pipes for synchronization with HTTP requests to an actor
> running in the tests, or wait on other observable side effects.
> 
> 
> Diffs
> -
> 
>   src/tests/check_tests.cpp d48febfca220a9633b9884963bcf5a205db7f5e5 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> 6050e6ebed87249382d56aedb6d98d3cf9812bb9 
> 
> 
> Diff: https://reviews.apache.org/r/67398/diff/3/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> Tested in internal CI on a number of platforms.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67398: Changed default executor tests to not use pipes for synchronization.

[Jie Yu]

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




src/tests/containerizer/nested_mesos_containerizer_tests.cpp
Lines 1017-1028 (original), 1012-1028 (patched)


Any reason we cannot use a named pipe as well here?


- Jie Yu


On June 7, 2018, 12:40 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67398/
> ---
> 
> (Updated June 7, 2018, 12:40 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-8917
> https://issues.apache.org/jira/browse/MESOS-8917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some tests of nested container functionality used pipes passed to
> launched tasks to detect whether a task has actually started executing
> the workload (`TASK_RUNNING` updates might be sent before the task
> workload is actually started).
> 
> Once we avoid leaking unspecified file descriptors into forked
> processes, this test setup will be broken. In this patch we replace
> the use of pipes for synchronization with HTTP requests to an actor
> running in the tests, or wait on other observable side effects.
> 
> 
> Diffs
> -
> 
>   src/tests/check_tests.cpp d48febfca220a9633b9884963bcf5a205db7f5e5 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> 6050e6ebed87249382d56aedb6d98d3cf9812bb9 
> 
> 
> Diff: https://reviews.apache.org/r/67398/diff/3/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> Tested in internal CI on a number of platforms.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67398: Changed default executor tests to not use pipes for synchronization.

[Benjamin Bannier]

> On June 5, 2018, 1:30 a.m., Jie Yu wrote:
> > src/tests/containerizer/nested_mesos_containerizer_tests.cpp
> > Lines 583-585 (original), 584-586 (patched)
> > 
> >
> > I think there's a race here. If the process is spawned, but the 
> > initailize is not called yet, the command might return a failure?
> > 
> > I think a better way is to have a helper in the Waiter so that we can 
> > wait until the route has been setup.

I added an explicitly promise for users to detect whether the process was 
initialized.


> On June 5, 2018, 1:30 a.m., Jie Yu wrote:
> > src/tests/containerizer/nested_mesos_containerizer_tests.cpp
> > Line 1435 (original), 1426-1427 (patched)
> > 
> >
> > Hum, I think if the route is not setup, the wget will get a 404?
> > 
> > My suggestion is to make this more explicit. For instance, have a 
> > rourte called `/trigger` and a Promise called `triggered` (for client to 
> > trigger an event that test is waiting), and have another route called 
> > `/wait` and have a Promise called `notify` (for test to notify the client).

This is not needed anymore since we use named pipes in this test now.


- Benjamin


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


On June 7, 2018, 2:40 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67398/
> ---
> 
> (Updated June 7, 2018, 2:40 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-8917
> https://issues.apache.org/jira/browse/MESOS-8917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some tests of nested container functionality used pipes passed to
> launched tasks to detect whether a task has actually started executing
> the workload (`TASK_RUNNING` updates might be sent before the task
> workload is actually started).
> 
> Once we avoid leaking unspecified file descriptors into forked
> processes, this test setup will be broken. In this patch we replace
> the use of pipes for synchronization with HTTP requests to an actor
> running in the tests, or wait on other observable side effects.
> 
> 
> Diffs
> -
> 
>   src/tests/check_tests.cpp d48febfca220a9633b9884963bcf5a205db7f5e5 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> 6050e6ebed87249382d56aedb6d98d3cf9812bb9 
> 
> 
> Diff: https://reviews.apache.org/r/67398/diff/3/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> Tested in internal CI on a number of platforms.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67398: Changed default executor tests to not use pipes for synchronization.

[Benjamin Bannier]

> On June 5, 2018, 1:13 a.m., Andrew Schwartzmeyer wrote:
> > src/tests/containerizer/nested_mesos_containerizer_tests.cpp
> > Line 461 (original), 486 (patched)
> > 
> >
> > wget is not usually installed by default...
> 
> Jie Yu wrote:
> That's true. One possibility is to use named pipe
> https://linux.die.net/man/3/mkfifo

I changed almost all tests to use named pipes now. The exception is 
`ROOT_CGROUPS_INTERNET_CURL_LaunchNestedDebugCheckMntNamespace` where we do not 
have access to the host filesystem and still use HTTP access to a process 
running in the test.


- Benjamin


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


On June 7, 2018, 2:40 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67398/
> ---
> 
> (Updated June 7, 2018, 2:40 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-8917
> https://issues.apache.org/jira/browse/MESOS-8917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some tests of nested container functionality used pipes passed to
> launched tasks to detect whether a task has actually started executing
> the workload (`TASK_RUNNING` updates might be sent before the task
> workload is actually started).
> 
> Once we avoid leaking unspecified file descriptors into forked
> processes, this test setup will be broken. In this patch we replace
> the use of pipes for synchronization with HTTP requests to an actor
> running in the tests, or wait on other observable side effects.
> 
> 
> Diffs
> -
> 
>   src/tests/check_tests.cpp d48febfca220a9633b9884963bcf5a205db7f5e5 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> 6050e6ebed87249382d56aedb6d98d3cf9812bb9 
> 
> 
> Diff: https://reviews.apache.org/r/67398/diff/3/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> Tested in internal CI on a number of platforms.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67398: Changed default executor tests to not use pipes for synchronization.

[Benjamin Bannier]

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

(Updated June 7, 2018, 2:40 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Addressed issues raised by Andrew and Jie.


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


Repository: mesos


Description
---

Some tests of nested container functionality used pipes passed to
launched tasks to detect whether a task has actually started executing
the workload (`TASK_RUNNING` updates might be sent before the task
workload is actually started).

Once we avoid leaking unspecified file descriptors into forked
processes, this test setup will be broken. In this patch we replace
the use of pipes for synchronization with HTTP requests to an actor
running in the tests, or wait on other observable side effects.


Diffs (updated)
-

  src/tests/check_tests.cpp d48febfca220a9633b9884963bcf5a205db7f5e5 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
6050e6ebed87249382d56aedb6d98d3cf9812bb9 


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

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


Testing
---

`sudo make check`

Tested in internal CI on a number of platforms.


Thanks,

Benjamin Bannier

Re: Review Request 67398: Changed default executor tests to not use pipes for synchronization.

[Jie Yu]

> On June 4, 2018, 11:13 p.m., Andrew Schwartzmeyer wrote:
> > src/tests/containerizer/nested_mesos_containerizer_tests.cpp
> > Line 461 (original), 486 (patched)
> > 
> >
> > wget is not usually installed by default...

That's true. One possibility is to use named pipe
https://linux.die.net/man/3/mkfifo


- Jie


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


On June 1, 2018, 3:55 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67398/
> ---
> 
> (Updated June 1, 2018, 3:55 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-8917
> https://issues.apache.org/jira/browse/MESOS-8917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some tests of nested container functionality used pipes passed to
> launched tasks to detect whether a task has actually started executing
> the workload (`TASK_RUNNING` updates might be sent before the task
> workload is actually started).
> 
> Once we avoid leaking unspecified file descriptors into forked
> processes, this test setup will be broken. In this patch we replace
> the use of pipes for synchronization with HTTP requests to an actor
> running in the tests, or wait on other observable side effects.
> 
> 
> Diffs
> -
> 
>   src/tests/check_tests.cpp d48febfca220a9633b9884963bcf5a205db7f5e5 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> 6050e6ebed87249382d56aedb6d98d3cf9812bb9 
> 
> 
> Diff: https://reviews.apache.org/r/67398/diff/2/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> Tested in internal CI on a number of platforms.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67398: Changed default executor tests to not use pipes for synchronization.

[Jie Yu]

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




src/tests/containerizer/nested_mesos_containerizer_tests.cpp
Lines 583-585 (original), 584-586 (patched)


I think there's a race here. If the process is spawned, but the initailize 
is not called yet, the command might return a failure?

I think a better way is to have a helper in the Waiter so that we can wait 
until the route has been setup.



src/tests/containerizer/nested_mesos_containerizer_tests.cpp
Line 1435 (original), 1426-1427 (patched)


Hum, I think if the route is not setup, the wget will get a 404?

My suggestion is to make this more explicit. For instance, have a rourte 
called `/trigger` and a Promise called `triggered` (for client to trigger an 
event that test is waiting), and have another route called `/wait` and have a 
Promise called `notify` (for test to notify the client).


- Jie Yu


On June 1, 2018, 3:55 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67398/
> ---
> 
> (Updated June 1, 2018, 3:55 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-8917
> https://issues.apache.org/jira/browse/MESOS-8917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some tests of nested container functionality used pipes passed to
> launched tasks to detect whether a task has actually started executing
> the workload (`TASK_RUNNING` updates might be sent before the task
> workload is actually started).
> 
> Once we avoid leaking unspecified file descriptors into forked
> processes, this test setup will be broken. In this patch we replace
> the use of pipes for synchronization with HTTP requests to an actor
> running in the tests, or wait on other observable side effects.
> 
> 
> Diffs
> -
> 
>   src/tests/check_tests.cpp d48febfca220a9633b9884963bcf5a205db7f5e5 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> 6050e6ebed87249382d56aedb6d98d3cf9812bb9 
> 
> 
> Diff: https://reviews.apache.org/r/67398/diff/2/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> Tested in internal CI on a number of platforms.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67398: Changed default executor tests to not use pipes for synchronization.

[Andrew Schwartzmeyer]

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




src/tests/containerizer/nested_mesos_containerizer_tests.cpp
Line 461 (original), 486 (patched)


wget is not usually installed by default...


- Andrew Schwartzmeyer


On June 1, 2018, 8:55 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67398/
> ---
> 
> (Updated June 1, 2018, 8:55 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-8917
> https://issues.apache.org/jira/browse/MESOS-8917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some tests of nested container functionality used pipes passed to
> launched tasks to detect whether a task has actually started executing
> the workload (`TASK_RUNNING` updates might be sent before the task
> workload is actually started).
> 
> Once we avoid leaking unspecified file descriptors into forked
> processes, this test setup will be broken. In this patch we replace
> the use of pipes for synchronization with HTTP requests to an actor
> running in the tests, or wait on other observable side effects.
> 
> 
> Diffs
> -
> 
>   src/tests/check_tests.cpp d48febfca220a9633b9884963bcf5a205db7f5e5 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> 6050e6ebed87249382d56aedb6d98d3cf9812bb9 
> 
> 
> Diff: https://reviews.apache.org/r/67398/diff/2/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> Tested in internal CI on a number of platforms.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67398: Changed default executor tests to not use pipes for synchronization.

[Benjamin Bannier]

> On May 31, 2018, 5:48 p.m., Benjamin Bannier wrote:
> > src/tests/containerizer/nested_mesos_containerizer_tests.cpp
> > Line 429 (original), 454 (patched)
> > 
> >
> > I still see this test failing in our internal CI, need to investigate 
> > what is amiss here.

The string comparison here was not portable, fixed now.


- Benjamin


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


On June 1, 2018, 5:55 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67398/
> ---
> 
> (Updated June 1, 2018, 5:55 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-8917
> https://issues.apache.org/jira/browse/MESOS-8917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some tests of nested container functionality used pipes passed to
> launched tasks to detect whether a task has actually started executing
> the workload (`TASK_RUNNING` updates might be sent before the task
> workload is actually started).
> 
> Once we avoid leaking unspecified file descriptors into forked
> processes, this test setup will be broken. In this patch we replace
> the use of pipes for synchronization with HTTP requests to an actor
> running in the tests, or wait on other observable side effects.
> 
> 
> Diffs
> -
> 
>   src/tests/check_tests.cpp d48febfca220a9633b9884963bcf5a205db7f5e5 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> 6050e6ebed87249382d56aedb6d98d3cf9812bb9 
> 
> 
> Diff: https://reviews.apache.org/r/67398/diff/2/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> Tested in internal CI on a number of platforms.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67398: Changed default executor tests to not use pipes for synchronization.

[Benjamin Bannier]

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

(Updated June 1, 2018, 5:55 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Fixed also `DefaultExecutorCheckTest.CommandCheckSeesParentsEnv`; used portable 
string comparison in 
`NestedMesosContainerizerTest.ROOT_CGROUPS_DebugNestedContainerInheritsMesosSandbox`.


Summary (updated)
-

Changed default executor tests to not use pipes for synchronization.


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


Repository: mesos


Description (updated)
---

Some tests of nested container functionality used pipes passed to
launched tasks to detect whether a task has actually started executing
the workload (`TASK_RUNNING` updates might be sent before the task
workload is actually started).

Once we avoid leaking unspecified file descriptors into forked
processes, this test setup will be broken. In this patch we replace
the use of pipes for synchronization with HTTP requests to an actor
running in the tests, or wait on other observable side effects.


Diffs (updated)
-

  src/tests/check_tests.cpp d48febfca220a9633b9884963bcf5a205db7f5e5 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
6050e6ebed87249382d56aedb6d98d3cf9812bb9 


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

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


Testing (updated)
---

`sudo make check`

Tested in internal CI on a number of platforms.


Thanks,

Benjamin Bannier