api: add sandbox hotplug network by caoruidong · Pull Request #287 · kata-containers/runtime

caoruidong · 2018-05-07T14:50:08Z

Fixes #113
Refactor generate interface and route, add network hotplug interface

Signed-off-by: Ruidong Cao caoruidong@huawei.com

codecov · 2018-05-08T05:55:19Z

Codecov Report

Merging #287 into master will decrease coverage by 0.05%.
The diff coverage is 53.26%.

@@            Coverage Diff            @@
##           master    #287      +/-   ##
=========================================
- Coverage   64.76%   64.7%   -0.06%     
=========================================
  Files          83      84       +1     
  Lines        9178    9551     +373     
=========================================
+ Hits         5944    6180     +236     
- Misses       2620    2723     +103     
- Partials      614     648      +34

egernst · 2018-05-15T04:37:22Z

@sboeuf - can you PTAL?

bergwolf · 2018-05-15T13:29:08Z

Generally looks good. Can you please add cli side change so that these APIs are actually used and tested?

devimc · 2018-05-15T13:53:10Z

virtcontainers/agent.go

 	// cpus specifies the number of CPUs that were added and the agent should online
 	onlineCPUMem(cpus uint32) error
+
+	// hotPlugNetwork will tell the agent to add a new nic for an existed Sandbox.


s/hotPlugNetwork/plugRoutes

devimc · 2018-05-15T13:53:42Z

virtcontainers/agent.go

+	// hotPlugNetwork will tell the agent to add a new nic for an existed Sandbox.
+	hotPlugNetwork(sandbox *Sandbox, endpoint Endpoint) error
+
+	// hotPlugNetwork will tell the agent to del a new nic for an existed Sandbox.


s/hotPlugNetwork/hotPullNetwork

sboeuf · 2018-08-15T04:24:52Z

@caoruidong thx!
About the nil being returned, just make sure the result gets tested before being used.

WeiZhang555 · 2018-08-15T10:08:45Z

@bergwolf @amshinde @sboeuf

create an empty sandbox/VM

run prestart hooks

scan and hotplug interfaces

create containers

By Scanning the net namespace from the CLI , I think you mean with NetNs monitor developed by @sboeuf ? If so, I'm totoally fine with it! This is what we agreed on before.

bergwolf · 2018-08-15T13:19:31Z

@WeiZhang555 Yes, I agree that we should rely on the netns monitor to do the (scan/hotplug) work.

sboeuf · 2018-08-15T16:05:44Z

@WeiZhang555 @bergwolf
I'd prefer to keep the network monitor solution a very specific one, only in case we want to detect network changes. But this monitoring process increases the footprint of the whole system, and might not be reliable for now in case of crash/restart. What I would suggest is that we can configure from configuration.toml if we want the network monitoring process or not. Based on this, we would either start the monitoring process before the OCI hooks from the CLI, or we would simply scan the netns from the CLI.

Does that make sense ?

Just to be clear, I'm trying to not push too much complexity with our current architecture, and that's why I'm trying to make sure we don't consider the network monitoring process as mandatory.

sboeuf · 2018-08-15T16:15:10Z

@caoruidong I have rebased and repushed so that we can see the code coverage as it was getting confused.

@WeiZhang555 @devimc you both had requested some changes, could you please take another look and let us know if the PR is okay now?

caoruidong · 2018-08-15T16:39:44Z

@sboeuf Codecov is still not working well. Maybe I need to update it again.

sboeuf · 2018-08-15T16:44:16Z

@caoruidong give it some time, now that the pr has been rebased on master, it should be fine I think ;)

sboeuf · 2018-08-15T18:03:41Z

@amshinde @chavafg do you know why codecov does not work here ? is it down ?

chavafg · 2018-08-15T18:14:54Z

Not sure,
I found this on the codecov.io site report [1].

Missing base report.
Unable to compare commits because the base of the pull request did not upload a coverage report.

Changes found in between 83008d4...cd514b6 (pseudo...base) which prevent comparing this pull request.

Maybe a new rebase is needed?

[1] https://codecov.io/gh/kata-containers/runtime/pull/287

amshinde · 2018-08-15T18:31:15Z

@sboeuf No idea. Maybe as @chavafg suggests, a rebase may help.

sboeuf · 2018-08-15T18:56:44Z

@amshinde @chavafg I did the rebase this morning, but didn't seem to fix the issue...

bergwolf · 2018-08-16T00:49:38Z

@sboeuf I think we should separate the daemon part of netns monitor from the actual implementation that does the scan/hotplug job, -- e.g., by making scan/hotplug a standalone package. Then both the netns monitor process, and the CLI can use the standalone package to kick off network scan/hotplug. The only difference is that netns is a long running daemon and CLI only does it in one shot when/after executing the prestart hooks. When there are no prestart hooks, CLI does not even bother calling scan/hotplug -- which would allow us to create a working sandbox w/o any external NICs.

sboeuf · 2018-08-16T01:02:37Z

@bergwolf yes definitely. I am currently working on reworking the whole OCI hook. This will be the first step. And then, we'll move more code to CLI if needed. But all of this sounds good to me :)

sboeuf · 2018-08-16T05:02:14Z

@caoruidong please try to fix the build for this as I'd like to get it merged by tomorrow (unless @WeiZhang555 @bergwolf or @devimc has any objections).

To support tap network hotplug. Implement netdev_add, netdev_del and corresponding device_add QMP commands. Full list in govmm: 10efa84 qemu/qmp: add function for hotplug network by fds 80ed88e qemu/qmp: implement function to hotplug serial ports ca46f21 qemu/qmp: implement function to hotplug character devices 03f1a1c qemu/qmp: implement getfd 84b212f qemu: add vhostfd and disable-modern to vsock hotplug 12dfa87 qemu/qmp: implement function for hotplug network 4ca232e qmp_test: Fix Warning and Error level logs 430e72c qemu,qmp: Enable gas security checker ffc06e6 qemu,qmp: Add staticcheck to travis and fix errors Add agent ListInterfaces and ListRoutes APIs. Full list in agent: 7c287c6 agent: add ListInterfaces and ListRoutes rpc Signed-off-by: Ruidong Cao <caoruidong@huawei.com>

Add sandbox hotplug network API to meet design Signed-off-by: Ruidong Cao <caoruidong@huawei.com>

Add update and list commands for notwork hotplug Signed-off-by: Ruidong Cao <caoruidong@huawei.com>

opendev-zuul · 2018-08-16T09:11:55Z

Build succeeded (third-party-check pipeline).

kata-runsh : SUCCESS in 54m 35s

WeiZhang555 · 2018-08-16T09:47:24Z

LGTM

I'd prefer to keep the network monitor solution a very specific one, only in case we want to detect network changes. But this monitoring process increases the footprint of the whole system, and might not be reliable for now in case of crash/restart. What I would suggest is that we can configure from configuration.toml if we want the network monitoring process or not. Based on this, we would either start the monitoring process before the OCI hooks from the CLI, or we would simply scan the netns from the CLI.
Does that make sense ?

I think this could make sense :-) @sboeuf

add UTs for network hotplug related fuctions Fixes kata-containers#113 Signed-off-by: Ruidong Cao <caoruidong@huawei.com>

katacontainersbot · 2018-08-16T13:41:28Z

PSS Measurement:
Qemu: 169758 KB
Proxy: 4247 KB
Shim: 8959 KB

Memory inside container:
Total Memory: 2043464 KB
Free Memory: 2003760 KB

caoruidong · 2018-08-16T14:24:05Z

@sboeuf I add some tests. Please check if this is right

sboeuf · 2018-08-16T14:41:05Z

@caoruidong thank you very much!
I'll review and merge if everything's good :)

opendev-zuul · 2018-08-16T15:05:57Z

Build succeeded (third-party-check pipeline).

kata-runsh : SUCCESS in 56m 53s

sboeuf

LGTM

sboeuf · 2018-08-16T15:19:31Z

Merging as -0.06% coverage is clearly negligible here.

CaesarLXL · 2018-08-20T01:04:17Z

This feature is currently not valid because the pull request 534 is not fully implemented.

caoruidong force-pushed the hotplug branch 7 times, most recently from 996402e to 1535dda Compare May 8, 2018 05:55

caoruidong force-pushed the hotplug branch 13 times, most recently from c26214a to 82dd787 Compare May 13, 2018 13:35

caoruidong changed the title ~~[WIP] API: add network hotplug~~ api: add sanbox hotplug network May 14, 2018

egernst requested a review from sboeuf May 15, 2018 04:37

caoruidong force-pushed the hotplug branch 3 times, most recently from 3b02c60 to 685aa44 Compare May 15, 2018 07:52

devimc reviewed May 15, 2018

View reviewed changes

sboeuf approved these changes Aug 15, 2018

View reviewed changes

caoruidong added 3 commits August 16, 2018 16:06

virtcontainers: add sandbox hotplug network API

1a17200

Add sandbox hotplug network API to meet design Signed-off-by: Ruidong Cao <caoruidong@huawei.com>

cli: add network commands

72df219

Add update and list commands for notwork hotplug Signed-off-by: Ruidong Cao <caoruidong@huawei.com>

WeiZhang555 approved these changes Aug 16, 2018

View reviewed changes

devimc approved these changes Aug 16, 2018

View reviewed changes

test: add UTs for network hotplug

7beb309

add UTs for network hotplug related fuctions Fixes kata-containers#113 Signed-off-by: Ruidong Cao <caoruidong@huawei.com>

sboeuf reviewed Aug 16, 2018

View reviewed changes

Conversation

caoruidong commented May 7, 2018

Uh oh!

codecov bot commented May 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

egernst commented May 15, 2018

Uh oh!

bergwolf commented May 15, 2018

Uh oh!

devimc May 15, 2018

Choose a reason for hiding this comment

Uh oh!

devimc May 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caoruidong May 15, 2018

Choose a reason for hiding this comment

Uh oh!

sboeuf commented Aug 15, 2018

Uh oh!

WeiZhang555 commented Aug 15, 2018

Uh oh!

bergwolf commented Aug 15, 2018

Uh oh!

sboeuf commented Aug 15, 2018

Uh oh!

sboeuf commented Aug 15, 2018

Uh oh!

caoruidong commented Aug 15, 2018

Uh oh!

sboeuf commented Aug 15, 2018

Uh oh!

sboeuf commented Aug 15, 2018

Uh oh!

chavafg commented Aug 15, 2018

Uh oh!

amshinde commented Aug 15, 2018

Uh oh!

sboeuf commented Aug 15, 2018

Uh oh!

bergwolf commented Aug 16, 2018

Uh oh!

sboeuf commented Aug 16, 2018

Uh oh!

sboeuf commented Aug 16, 2018

Uh oh!

opendev-zuul bot commented Aug 16, 2018

Uh oh!

WeiZhang555 commented Aug 16, 2018

Uh oh!

katacontainersbot commented Aug 16, 2018

Uh oh!

caoruidong commented Aug 16, 2018

Uh oh!

sboeuf commented Aug 16, 2018

Uh oh!

opendev-zuul bot commented Aug 16, 2018

Uh oh!

sboeuf left a comment

Choose a reason for hiding this comment

Uh oh!

sboeuf commented Aug 16, 2018

Uh oh!

CaesarLXL commented Aug 20, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

codecov bot commented May 8, 2018 •

edited

Loading

devimc May 15, 2018 •

edited

Loading