Re: Review Request 68762: Tested container cleanup in `AgentResourceProviderConfigApiTest.Remove`.

[Mesos Reviewbot Windows]

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['68755', '68756', '68757', '68777', '68758', '68790', 
'68763', '68762']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2372/mesos-review-68762

Relevant logs:

- 
[mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2372/mesos-review-68762/logs/mesos-tests.log):

```
I0921 21:40:15.800307  3028 exec.cpp:445] Executor asked to shutdown
I0921 21:40:15.800307  6180 executor.cpp:182] Received SHUTDOWN event
I0921 21:40:15.801311  6180 executor.cpp:796] Shutting down
I0921 21:40:15.801311  6180 executor.cpp:909] Sending SIGTERM to process tree 
at pid 442pp:3917] Ignoring shutdown framework 
2fe21b88-cf60-42b9-a4d5-cb1625c6c66b- because it is terminating
I0921 21:40:15.803310  3532 master.cpp:1251] Agent 
2fe21b88-cf60-42b9-a4d5-cb1625c6c66b-S0 at slave(463)@192.10.1.5:54060 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net) disconnected
I0921 21:40:15.803310  3532 master.cpp:3267] Disconnecting agent 
2fe21b88-cf60-42b9-a4d5-cb1625c6c66b-S0 at slave(463)@192.10.1.5:54060 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I0921 21:40:15.803310  3532 master.cpp:3286] Deactivating agent 
2fe21b88-cf60-42b9-a4d5-cb1625c6c66b-S0 at slave(463)@192.10.1.5:54060 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I0921 21:40:15.803310  1412 hierarchical.cpp:359] Removed framework 
2fe21b88-cf60-42b9-a4d5-cb1625c6c66b-
I0921 21:40:15.804322  1412 hierarchical.cpp:795] Agent 
2fe21b88-cf60-42b9-a4d5-cb1625c6c66b-S0 deactivated
I0921 21:40:15.805306  2064 containerizer.cpp:2455] Destroying container 
409e5c50-2258-4d2f-b90e-a10[   OK ] 
IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (582 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (602 ms total)

[--] Global test environment tear-down
[==] 1052 tests from 103 test cases ran. (597768 ms total)
[  PASSED  ] 1050 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName

 2 FAILED TESTS
  YOU HAVE 231 DISABLED TESTS

10fa964de in RUNNING state
I0921 21:40:15.805306  2064 containerizer.cpp:3118] Transitioning the state of 
container 409e5c50-2258-4d2f-b90e-a1010fa964de from RUNNING to DESTROYING
I0921 21:40:15.805306  2064 launcher.cpp:166] Asked to destroy container 
409e5c50-2258-4d2f-b90e-a1010fa964de
I0921 21:40:15.837321  3532 containerizer.cpp:2957] Container 
409e5c50-2258-4d2f-b90e-a1010fa964de has exited
I0921 21:40:15.864353  2660 master.cpp:1093] Master terminating
I0921 21:40:15.866312  3528 hierarchical.cpp:637] Removed agent 
2fe21b88-cf60-42b9-a4d5-cb1625c6c66b-S0
I0921 21:40:16.102308  6560 process.cpp:926] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Sept. 20, 2018, 4:15 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68762/
> ---
> 
> (Updated Sept. 20, 2018, 4:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-9228
> https://issues.apache.org/jira/browse/MESOS-9228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tested container cleanup in `AgentResourceProviderConfigApiTest.Remove`.
> 
> 
> Diffs
> -
> 
>   src/tests/agent_resource_provider_config_api_tests.cpp 
> e6a68bae1a9e3e773ea45deae4951664ab81a857 
> 
> 
> Diff: https://reviews.apache.org/r/68762/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> The test will fail without r/68763.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 68762: Tested container cleanup in `AgentResourceProviderConfigApiTest.Remove`.

[Chun-Hung Hsiao]

> On Sept. 21, 2018, 10:05 a.m., Benjamin Bannier wrote:
> > src/tests/agent_resource_provider_config_api_tests.cpp
> > Line 35 (original), 35 (patched)
> > 
> >
> > Like you wrote in your _Testing Done_ section, this test is broken 
> > without the next patch https://reviews.apache.org/r/68763/. Let's only 
> > commit it in the same patch as the feature it is testing, or after it; 
> > otherwise we might break e.g., `git bisect` workflows without benefit.

Hmm I thought it would be fine as long as we commit them together. I'll adjust 
the commit order.


> On Sept. 21, 2018, 10:05 a.m., Benjamin Bannier wrote:
> > src/tests/agent_resource_provider_config_api_tests.cpp
> > Lines 932 (patched)
> > 
> >
> > Nit: We could also write this as `pluginContainer[0]`.

I don't think so because `pluginContainers` is a `Future>` ;)


- Chun-Hung


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


On Sept. 20, 2018, 11:15 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68762/
> ---
> 
> (Updated Sept. 20, 2018, 11:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-9228
> https://issues.apache.org/jira/browse/MESOS-9228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tested container cleanup in `AgentResourceProviderConfigApiTest.Remove`.
> 
> 
> Diffs
> -
> 
>   src/tests/agent_resource_provider_config_api_tests.cpp 
> e6a68bae1a9e3e773ea45deae4951664ab81a857 
> 
> 
> Diff: https://reviews.apache.org/r/68762/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> The test will fail without the next patch.
> 
> NOTE: It might be more appropriate to not reuse this test for container 
> cleanup. Instead, I'm considering making a similar test in 
> `StorageLocalResourceProviderTest`.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 68762: Tested container cleanup in `AgentResourceProviderConfigApiTest.Remove`.

[Benjamin Bannier]

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


Fix it, then Ship it!





src/tests/agent_resource_provider_config_api_tests.cpp
Line 35 (original), 35 (patched)


Like you wrote in your _Testing Done_ section, this test is broken without 
the next patch https://reviews.apache.org/r/68763/. Let's only commit it in the 
same patch as the feature it is testing, or after it; otherwise we might break 
e.g., `git bisect` workflows without benefit.



src/tests/agent_resource_provider_config_api_tests.cpp
Lines 932 (patched)


Nit: We could also write this as `pluginContainer[0]`.


- Benjamin Bannier


On Sept. 21, 2018, 1:15 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68762/
> ---
> 
> (Updated Sept. 21, 2018, 1:15 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-9228
> https://issues.apache.org/jira/browse/MESOS-9228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tested container cleanup in `AgentResourceProviderConfigApiTest.Remove`.
> 
> 
> Diffs
> -
> 
>   src/tests/agent_resource_provider_config_api_tests.cpp 
> e6a68bae1a9e3e773ea45deae4951664ab81a857 
> 
> 
> Diff: https://reviews.apache.org/r/68762/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> The test will fail without the next patch.
> 
> NOTE: It might be more appropriate to not reuse this test for container 
> cleanup. Instead, I'm considering making a similar test in 
> `StorageLocalResourceProviderTest`.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 68762: Tested container cleanup in `AgentResourceProviderConfigApiTest.Remove`.

[Chun-Hung Hsiao]

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

(Updated Sept. 20, 2018, 11:15 p.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.


Changes
---

Fixed the test flakiness.


Summary (updated)
-

Tested container cleanup in `AgentResourceProviderConfigApiTest.Remove`.


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


Repository: mesos


Description
---

Tested container cleanup in `AgentResourceProviderConfigApiTest.Remove`.


Diffs (updated)
-

  src/tests/agent_resource_provider_config_api_tests.cpp 
e6a68bae1a9e3e773ea45deae4951664ab81a857 


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

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


Testing
---

make check

The test will fail without the next patch.

NOTE: It might be more appropriate to not reuse this test for container 
cleanup. Instead, I'm considering making a similar test in 
`StorageLocalResourceProviderTest`.


Thanks,

Chun-Hung Hsiao

Review Request 68762: Tested container cleanup in `AgentResourceProviderConfigApiTest.Remove`.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.


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


Repository: mesos


Description
---

Tested container cleanup in `AgentResourceProviderConfigApiTest.Remove`.


Diffs
-

  src/tests/agent_resource_provider_config_api_tests.cpp 
e6a68bae1a9e3e773ea45deae4951664ab81a857 


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


Testing
---

make check

The test will fail without the next patch.


Thanks,

Chun-Hung Hsiao