You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2019/01/11 06:14:50 UTC
Review Request 69715: Fixed the CNI_NETNS handling in port mapper CNI
plugin.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69715/
-----------------------------------------------------------
Review request for mesos, Deepak Goel, Gilbert Song, and Qian Zhang.
Bugs: MESOS-9518
https://issues.apache.org/jira/browse/MESOS-9518
Repository: mesos
Description
-------
According CNI spec, it is possible that the container runtime does not
set CNI_NETNS environment variable when it is not available. This is
possible in scenarios like a host reboot. In that case, the CNI plugin
should do best effort cleanup, instead of failing.
Diffs
-----
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp 25f49f4b90ec6d0d55fc306b6ab324ba5b4e7403
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp 4e784ffb4ac29861c888fdbed4fcf9902bf4182a
Diff: https://reviews.apache.org/r/69715/diff/1/
Testing
-------
sudo make check
Thanks,
Jie Yu
Re: Review Request 69715: Fixed the CNI_NETNS handling in port mapper
CNI plugin.
Posted by Jie Yu <yu...@gmail.com>.
> On Jan. 11, 2019, 9:53 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
> > Lines 72-77 (original), 72 (patched)
> > <https://reviews.apache.org/r/69715/diff/1/?file=2119178#file2119178line72>
> >
> > I think we still need to make sure `cniNetNs` is not `None()` if `CNI_COMMAND` is `ADD`.
Yeah, good point. Let me add the check
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69715/#review211875
-----------------------------------------------------------
On Jan. 11, 2019, 6:14 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69715/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2019, 6:14 a.m.)
>
>
> Review request for mesos, Deepak Goel, Gilbert Song, and Qian Zhang.
>
>
> Bugs: MESOS-9518
> https://issues.apache.org/jira/browse/MESOS-9518
>
>
> Repository: mesos
>
>
> Description
> -------
>
> According CNI spec, it is possible that the container runtime does not
> set CNI_NETNS environment variable when it is not available. This is
> possible in scenarios like a host reboot. In that case, the CNI plugin
> should do best effort cleanup, instead of failing.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp 25f49f4b90ec6d0d55fc306b6ab324ba5b4e7403
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp 4e784ffb4ac29861c888fdbed4fcf9902bf4182a
>
>
> Diff: https://reviews.apache.org/r/69715/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 69715: Fixed the CNI_NETNS handling in port mapper
CNI plugin.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69715/#review211875
-----------------------------------------------------------
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
Lines 72-77 (original), 72 (patched)
<https://reviews.apache.org/r/69715/#comment297455>
I think we still need to make sure `cniNetNs` is not `None()` if `CNI_COMMAND` is `ADD`.
- Qian Zhang
On Jan. 11, 2019, 2:14 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69715/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2019, 2:14 p.m.)
>
>
> Review request for mesos, Deepak Goel, Gilbert Song, and Qian Zhang.
>
>
> Bugs: MESOS-9518
> https://issues.apache.org/jira/browse/MESOS-9518
>
>
> Repository: mesos
>
>
> Description
> -------
>
> According CNI spec, it is possible that the container runtime does not
> set CNI_NETNS environment variable when it is not available. This is
> possible in scenarios like a host reboot. In that case, the CNI plugin
> should do best effort cleanup, instead of failing.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp 25f49f4b90ec6d0d55fc306b6ab324ba5b4e7403
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp 4e784ffb4ac29861c888fdbed4fcf9902bf4182a
>
>
> Diff: https://reviews.apache.org/r/69715/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 69715: Fixed the CNI_NETNS handling in port mapper
CNI plugin.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69715/#review211864
-----------------------------------------------------------
Ship it!
Ship It!
- Gilbert Song
On Jan. 10, 2019, 10:14 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69715/
> -----------------------------------------------------------
>
> (Updated Jan. 10, 2019, 10:14 p.m.)
>
>
> Review request for mesos, Deepak Goel, Gilbert Song, and Qian Zhang.
>
>
> Bugs: MESOS-9518
> https://issues.apache.org/jira/browse/MESOS-9518
>
>
> Repository: mesos
>
>
> Description
> -------
>
> According CNI spec, it is possible that the container runtime does not
> set CNI_NETNS environment variable when it is not available. This is
> possible in scenarios like a host reboot. In that case, the CNI plugin
> should do best effort cleanup, instead of failing.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp 25f49f4b90ec6d0d55fc306b6ab324ba5b4e7403
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp 4e784ffb4ac29861c888fdbed4fcf9902bf4182a
>
>
> Diff: https://reviews.apache.org/r/69715/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 69715: Fixed the CNI_NETNS handling in port mapper
CNI plugin.
Posted by Deepak Goel <de...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69715/#review211908
-----------------------------------------------------------
Ship it!
Ship It!
- Deepak Goel
On Jan. 11, 2019, 6:10 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69715/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2019, 6:10 p.m.)
>
>
> Review request for mesos, Deepak Goel, Gilbert Song, and Qian Zhang.
>
>
> Bugs: MESOS-9518
> https://issues.apache.org/jira/browse/MESOS-9518
>
>
> Repository: mesos
>
>
> Description
> -------
>
> According CNI spec, it is possible that the container runtime does not
> set CNI_NETNS environment variable when it is not available. This is
> possible in scenarios like a host reboot. In that case, the CNI plugin
> should do best effort cleanup, instead of failing.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp 25f49f4b90ec6d0d55fc306b6ab324ba5b4e7403
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp 4e784ffb4ac29861c888fdbed4fcf9902bf4182a
>
>
> Diff: https://reviews.apache.org/r/69715/diff/2/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 69715: Fixed the CNI_NETNS handling in port mapper
CNI plugin.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69715/#review211899
-----------------------------------------------------------
PASS: Mesos patch 69715 was successfully built and tested.
Reviews applied: `['69706', '69710', '69711', '69712', '69713', '69714', '69715']`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2762/mesos-review-69715
- Mesos Reviewbot Windows
On Jan. 11, 2019, 6:10 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69715/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2019, 6:10 p.m.)
>
>
> Review request for mesos, Deepak Goel, Gilbert Song, and Qian Zhang.
>
>
> Bugs: MESOS-9518
> https://issues.apache.org/jira/browse/MESOS-9518
>
>
> Repository: mesos
>
>
> Description
> -------
>
> According CNI spec, it is possible that the container runtime does not
> set CNI_NETNS environment variable when it is not available. This is
> possible in scenarios like a host reboot. In that case, the CNI plugin
> should do best effort cleanup, instead of failing.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp 25f49f4b90ec6d0d55fc306b6ab324ba5b4e7403
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp 4e784ffb4ac29861c888fdbed4fcf9902bf4182a
>
>
> Diff: https://reviews.apache.org/r/69715/diff/2/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 69715: Fixed the CNI_NETNS handling in port mapper
CNI plugin.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69715/
-----------------------------------------------------------
(Updated Jan. 11, 2019, 6:10 p.m.)
Review request for mesos, Deepak Goel, Gilbert Song, and Qian Zhang.
Bugs: MESOS-9518
https://issues.apache.org/jira/browse/MESOS-9518
Repository: mesos
Description
-------
According CNI spec, it is possible that the container runtime does not
set CNI_NETNS environment variable when it is not available. This is
possible in scenarios like a host reboot. In that case, the CNI plugin
should do best effort cleanup, instead of failing.
Diffs (updated)
-----
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp 25f49f4b90ec6d0d55fc306b6ab324ba5b4e7403
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp 4e784ffb4ac29861c888fdbed4fcf9902bf4182a
Diff: https://reviews.apache.org/r/69715/diff/2/
Changes: https://reviews.apache.org/r/69715/diff/1-2/
Testing
-------
sudo make check
Thanks,
Jie Yu
Re: Review Request 69715: Fixed the CNI_NETNS handling in port mapper
CNI plugin.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69715/#review211867
-----------------------------------------------------------
FAIL: Some of the unit tests failed. Please check the relevant logs.
Reviews applied: `['69706', '69710', '69711', '69712', '69713', '69714', '69715']`
Failed command: `Start-MesosCITesting`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2756/mesos-review-69715
Relevant logs:
- [mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2756/mesos-review-69715/logs/mesos-tests.log):
```
W0111 08:04:32.856523 8832 slave.cpp:3933] Ignoring shutdown framework 2c6bf722-b782-4b8c-b585-748f13f6f069-0000 because it is terminating
I0111 08:04:32.858541 12608 master.cpp:1271] Agent 2c6bf722-b782-4b8c-b585-748f13f6f069-S0 at slave(466)@192.10.1.6:61481 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net) disconnected
I0111 08:04:32.858541 12608 master.cpp:3274] Disconnecting agent 2c6bf722-b782-4b8c-b585-748f13f6f069-S0 at slave(466)@192.10.1.6:61481 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0111 08:04:32.859544 12608 master.cpp:3293] Deactivating agent 2c6bf722-b782-4b8c-b585-748f13f6f069-S0 at slave(466)@192.10.1.6:61481 (windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0111 08:04:32.859544 12352 hierarchical.cpp:358] Removed framework 2c6bf722-b782-4b8c-b585-748f13f6f069-0000
I0111 08:04:32.859544 12352 hierarchical.cpp:802] Agent 2c6bf722-b782-4b8c-b585-748f13f6f069-S0 deactivated
I0111 08:04:32.860528 13372 containerizer.cpp:2469] Destroying container e6cd5ada-7ca8-4586-aa30-9bcace8e6041 in RUNNING state
I0111 08:04:32.860528 13372 containerizer.cpp:3136] Transitioning the state of container e6cd5ada-7ca8-4586-aa30-9bcace8e6041 from RUNNING to DESTROYING
I0111 08:04:32.861521 13372 launcher.cpp:161] Asked to destroy container e6cd5ada-7ca8-4586-aa30-9bcace8e6041
W0111 08:04:32.862534 14056 process.cpp:1423] Failed to recv on socket WindowsFD::Type::SOCKET=8748 to peer '192.10.1.6:63322': IO failed with error code: The specified network name is no longer available.
W0111 08:04:32.862534 14056 process.cpp:838] Failed to re[ OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (689 ms)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest (707 ms total)
[----------] Global test environment tear-down
[==========] 1084 tests from 104 test cases ran. (499115 ms total)
[ PASSED ] 1083 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] ContentType/MasterAPITest.OperationUpdatesUponAgentGone/1, where GetParam() = application/json
1 FAILED TEST
YOU HAVE 231 DISABLED TESTS
cv on socket WindowsFD::Type::SOCKET=9104 to peer '192.10.1.6:63323': IO failed with error code: The specified network name is no longer available.
I0111 08:04:32.925542 12608 containerizer.cpp:2975] Container e6cd5ada-7ca8-4586-aa30-9bcace8e6041 has exited
I0111 08:04:32.956542 8452 master.cpp:1111] Master terminating
I0111 08:04:32.957535 12608 hierarchical.cpp:644] Removed agent 2c6bf722-b782-4b8c-b585-748f13f6f069-S0
I0111 08:04:33.248520 14056 process.cpp:927] Stopped the socket accept loop
```
- Mesos Reviewbot Windows
On Jan. 10, 2019, 10:14 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69715/
> -----------------------------------------------------------
>
> (Updated Jan. 10, 2019, 10:14 p.m.)
>
>
> Review request for mesos, Deepak Goel, Gilbert Song, and Qian Zhang.
>
>
> Bugs: MESOS-9518
> https://issues.apache.org/jira/browse/MESOS-9518
>
>
> Repository: mesos
>
>
> Description
> -------
>
> According CNI spec, it is possible that the container runtime does not
> set CNI_NETNS environment variable when it is not available. This is
> possible in scenarios like a host reboot. In that case, the CNI plugin
> should do best effort cleanup, instead of failing.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp 25f49f4b90ec6d0d55fc306b6ab324ba5b4e7403
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp 4e784ffb4ac29861c888fdbed4fcf9902bf4182a
>
>
> Diff: https://reviews.apache.org/r/69715/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>