Re: Review Request 68230: Added `AgentAPITest.LaunchNestedContainerSessionKillTask` test.

[Andrei Budnik]

> On Sept. 12, 2018, 3:01 p.m., Alexander Rukletsov wrote:
> > src/tests/api_tests.cpp
> > Lines 6766-6784 (patched)
> > 
> >
> > Not yours, but please—for me and my sanity!—let's pull it into a 
> > general helper in `tests/mesos.hpp`. Quick grepping shows there are more 
> > instances where this helper can be applied (can you even maybe fix those in 
> > a separate patch?)
> > 
> > 
> > https://github.com/apache/mesos/blob/af82f887db535c44b559e0d19cbddfb2dbd6134a/src/tests/master_tests.cpp#L9568-L9586
> > 
> > https://github.com/apache/mesos/blob/af82f887db535c44b559e0d19cbddfb2dbd6134a/src/tests/scheduler_tests.cpp#L612-L630

Oh, I found that we can use this code instead (from the default executor test):
```
  mesos.send(
  v1::createCallAccept(
  frameworkId,
  offer,
  {v1::LAUNCH_GROUP(
  executorInfo, v1::createTaskGroupInfo({taskInfo}))}));
```


- Andrei


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


On Aug. 31, 2018, 1:11 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68230/
> ---
> 
> (Updated Aug. 31, 2018, 1:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, and Qian Zhang.
> 
> 
> Bugs: MESOS-9131
> https://issues.apache.org/jira/browse/MESOS-9131
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that IOSwitchboard, which holds an open HTTP input
> connection, terminates once IO redirects finish for the corresponding
> nested container.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 6be0dfa28333a15b1650268798046dd5e76717c5 
> 
> 
> Diff: https://reviews.apache.org/r/68230/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 68230: Added `AgentAPITest.LaunchNestedContainerSessionKillTask` test.

[Alexander Rukletsov]

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


Fix it, then Ship it!





src/tests/api_tests.cpp
Lines 6670 (patched)


Kill this line



src/tests/api_tests.cpp
Lines 6679 (patched)


Kill this line



src/tests/api_tests.cpp
Lines 6714-6721 (patched)


Instead, you can piggy back on connection success, something like this:
```
  EXPECT_CALL(*scheduler, connected(_))
.WillOnce(v1::scheduler::SendSubscribe(v1::DEFAULT_FRAMEWORK_INFO));
```



src/tests/api_tests.cpp
Lines 6752-6753 (patched)


`MesosTest::defaultTaskResourcesString`



src/tests/api_tests.cpp
Lines 6755 (patched)


`SLEEP_COMMAND()`



src/tests/api_tests.cpp
Lines 6766-6784 (patched)


Not yours, but please—for me and my sanity!—let's pull it into a general 
helper in `tests/mesos.hpp`. Quick grepping shows there are more instances 
where this helper can be applied (can you even maybe fix those in a separate 
patch?)


https://github.com/apache/mesos/blob/af82f887db535c44b559e0d19cbddfb2dbd6134a/src/tests/master_tests.cpp#L9568-L9586

https://github.com/apache/mesos/blob/af82f887db535c44b559e0d19cbddfb2dbd6134a/src/tests/scheduler_tests.cpp#L612-L630



src/tests/api_tests.cpp
Lines 6787 (patched)


Kill this line



src/tests/api_tests.cpp
Lines 6833-6835 (patched)


Update the comment please!



src/tests/api_tests.cpp
Lines 6843-6854 (patched)


Does it make sense to make helpers out of these as well?


- Alexander Rukletsov


On Aug. 31, 2018, 1:11 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68230/
> ---
> 
> (Updated Aug. 31, 2018, 1:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, and Qian Zhang.
> 
> 
> Bugs: MESOS-9131
> https://issues.apache.org/jira/browse/MESOS-9131
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that IOSwitchboard, which holds an open HTTP input
> connection, terminates once IO redirects finish for the corresponding
> nested container.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 6be0dfa28333a15b1650268798046dd5e76717c5 
> 
> 
> Diff: https://reviews.apache.org/r/68230/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 68230: Added `AgentAPITest.LaunchNestedContainerSessionKillTask` test.

[Qian Zhang]

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


Ship it!




Ship It!

- Qian Zhang


On Aug. 31, 2018, 9:11 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68230/
> ---
> 
> (Updated Aug. 31, 2018, 9:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, and Qian Zhang.
> 
> 
> Bugs: MESOS-9131
> https://issues.apache.org/jira/browse/MESOS-9131
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that IOSwitchboard, which holds an open HTTP input
> connection, terminates once IO redirects finish for the corresponding
> nested container.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 43541af732df271177f0fd56ddc419adec461110 
> 
> 
> Diff: https://reviews.apache.org/r/68230/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 68230: Added `AgentAPITest.LaunchNestedContainerSessionKillTask` test.

[Andrei Budnik]

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

(Updated Aug. 31, 2018, 1:11 p.m.)


Review request for mesos, Alexander Rukletsov, Gilbert Song, and Qian Zhang.


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


Repository: mesos


Description
---

This test verifies that IOSwitchboard, which holds an open HTTP input
connection, terminates once IO redirects finish for the corresponding
nested container.


Diffs (updated)
-

  src/tests/api_tests.cpp 43541af732df271177f0fd56ddc419adec461110 


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

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


Testing
---


Thanks,

Andrei Budnik

Re: Review Request 68230: Added `AgentAPITest.LaunchNestedContainerSessionKillTask` test.

[Qian Zhang]

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



I think this patch should be put in the end of this patch chain since it can 
only succeed after r/68232 is applied.


src/tests/api_tests.cpp
Lines 6585 (patched)


This is a parameterized test, but I do not see you call `GetParam()` to get 
the parameter.



src/tests/api_tests.cpp
Lines 6740 (patched)


Is `taskContainerId` same with the `status.container_id()` that we got from 
status update? If yes, why not just use that one?



src/tests/api_tests.cpp
Lines 6744-6746 (patched)


Can we just do the following instead?
```
containerId.mutable_parent()->CopyFrom(status.container_id());
```



src/tests/api_tests.cpp
Lines 6777 (patched)


Can we do `pkill sleep` instead?


- Qian Zhang


On Aug. 25, 2018, 6:11 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68230/
> ---
> 
> (Updated Aug. 25, 2018, 6:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, and Qian Zhang.
> 
> 
> Bugs: MESOS-9131
> https://issues.apache.org/jira/browse/MESOS-9131
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that IOSwitchboard, which holds an open HTTP input
> connection, terminates once IO redirects finish for the corresponding
> nested container.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp ee82350f7a6c6d44ba0590608e7d9a223c0be169 
> 
> 
> Diff: https://reviews.apache.org/r/68230/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Review Request 68230: Added `AgentAPITest.LaunchNestedContainerSessionKillTask` test.

[Andrei Budnik]

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

Review request for mesos, Alexander Rukletsov, Gilbert Song, and Qian Zhang.


Summary (updated)
-

Added `AgentAPITest.LaunchNestedContainerSessionKillTask` test.


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


Repository: mesos


Description (updated)
---

This test verifies that IOSwitchboard, which holds an open HTTP input
connection, terminates once IO redirects finish for the corresponding
nested container.


Diffs (updated)
-

  src/tests/api_tests.cpp ee82350f7a6c6d44ba0590608e7d9a223c0be169 


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


Testing
---


Thanks,

Andrei Budnik