Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

network hotplug: add monitoring process for netns #170

Closed
egernst opened this issue Apr 2, 2018 · 20 comments
Closed

network hotplug: add monitoring process for netns #170

egernst opened this issue Apr 2, 2018 · 20 comments
Assignees
Labels
release-gating Release must wait for this to be resolved before release

Comments

@egernst
Copy link
Member

egernst commented Apr 2, 2018

Related to #113

This will track the implementation of the monitoring process. virtcontainer changes to be tracked in #113

@egernst egernst added the release-gating Release must wait for this to be resolved before release label Apr 2, 2018
@sboeuf sboeuf changed the title network hotplug: add monitoring process for nets network hotplug: add monitoring process for netns Apr 2, 2018
@sboeuf
Copy link

sboeuf commented Apr 2, 2018

@WeiZhang555 @egernst @guangxuli @miaoyq @bergwolf @laijs I need your input on what language I should use for this ?
I have been talking with @amshinde about this and we kind of agreed that such a simple program would be worth the shot in C since it would not bring too much overhead. If we choose Go, the footprint is gonna be bigger since we're going to import the netlink package, but it is gonna be very simple to implement too.
Keep in mind that the extra footprint is added per pod, since we need only one watcher per netns(which is tied to the pod).
I am waiting your inputs before I start implementing this.

@egernst
Copy link
Member Author

egernst commented Apr 2, 2018

I don't have a strong opinion on this. Will it be the only C living in the project? IIRC there was similar debate on shim. What was the major takeaway on that decision?

@amshinde
Copy link
Member

amshinde commented Apr 2, 2018

@egernst Yes, this would be the only C component. The main reason for the go implementation of shim was to do with the grpc library in C not being maintained.

@sboeuf
Copy link

sboeuf commented Apr 2, 2018

Yes the gRPC library was the first reason and the ecosystem being all about Golang was the second reason. But that being said, as a future improvement I think the shim might move to a C component too. But the thing is that the shim is already functional and this is not a priority, but I don't want to make the same mistake with this component, I'd rather go with the right language on the first implementation for this one.

@miaoyq
Copy link

miaoyq commented Apr 3, 2018

But the thing is that the shim is already functional and this is not a priority, but I don't want to make the same mistake with this component, I'd rather go with the right language on the first implementation for this one.

@sboeuf Agree with you.

@sboeuf
Copy link

sboeuf commented Apr 3, 2018

@miaoyq so you mean you would prefer C language for this binary, right ?

@miaoyq
Copy link

miaoyq commented Apr 3, 2018

@sboeuf Yeah

@guangxuli
Copy link

@sboeuf Both hands are in favor of using C language.

sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Apr 4, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the proper arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Apr 7, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the proper arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
@miaoyq
Copy link

miaoyq commented Apr 9, 2018

@egernst @sboeuf Could you add network hotplug related code in govmm?
So that we can use it in virtcontainers.

@sboeuf
Copy link

sboeuf commented Apr 9, 2018

@miaoyq Don't hesitate to do it yourself. I bet that you're already looking at the code and you know better what needs to be done here. The idea is to add what you need to govmm and once you get this merged, you will be able to revendor the change into virtcontainers.

sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Apr 9, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the proper arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Apr 9, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the proper arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
@miaoyq
Copy link

miaoyq commented Apr 10, 2018

@sboeuf Glad to do it, but I am not familiar with QMP. I will have a try, and hope to get your guidance in the process. :-)

@sboeuf
Copy link

sboeuf commented Apr 10, 2018

@miaoyq oh no worries, I'll help whenever you need ;)

@miaoyq
Copy link

miaoyq commented Apr 10, 2018

@sboeuf Cool, Thanks!

sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Apr 11, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the proper arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Apr 11, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the proper arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
@sboeuf
Copy link

sboeuf commented Apr 11, 2018

@miaoyq have you started working on this ? Any question so far or help needed so far ?

sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Apr 11, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the proper arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
@miaoyq
Copy link

miaoyq commented Apr 12, 2018

@miaoyq have you started working on this ? Any question so far or help needed so far ?

@sboeuf Yeah.
I read the code about network and found the CNI plugin was called in virtcontainers only for getting DNS, see:

netInfo, err := n.invokePluginsAdd(pod, &networkNS)

this will cause eth0 to be added twice(container runtime call CNI plugin firstly), but the CNI spec includes the following description:

A runtime must not call ADD twice (without a corresponding DEL) for the same (network name, container id). 
In other words, a given container ID must be added to a specific network exactly once.

see https://github.com/containernetworking/cni/blob/master/SPEC.md#general-considerations
Is there any special purpose? I'm not very clear about this.

This also makes kata-runtime dependent on the CNI plugin.

@sboeuf
Copy link

sboeuf commented Apr 12, 2018

@miaoyq Don't worry about this because in case of kata-runtime, the network type used is CNM. Take a look into runtime/virtcontainers/cnm.go instead.
The purpose of this runtime/virtcontainers/cni.go file is out of the OCI spec scope.

@miaoyq
Copy link

miaoyq commented Apr 12, 2018

The purpose of this runtime/virtcontainers/cni.go file is out of the OCI spec scope

@sboeuf Not clear of this :-p
Does k8s + cri-o + kata-runtime also use CNM ?

@sboeuf
Copy link

sboeuf commented Apr 12, 2018

Does k8s + cri-o + kata-runtime also use CNM ?

In short, yes.
But let me try to explain what we mean by CNM. When you use kata-runtime, you're dealing with an OCI compliant runtime, which relies on the CNM hooks for the network. That's why it always uses this implementation cnm.go, but this does not mean that CNI is not used. You have to think about cnm.go from the virtcontainers perspective where the scope was not only to support an OCI runtime on top.
Of course, when you use k8s + cri-o + kata-runtime, k8s will use CNI and the cnm.go implementation is going to be a simple passthrough because nothing will be provided.

@miaoyq
Copy link

miaoyq commented Apr 13, 2018

@sboeuf Got it, Thank you for your detailed explanation. :)

@sboeuf
Copy link

sboeuf commented Apr 13, 2018

No problem! I know this part is really not obvious...

sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Jul 27, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the proper arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Jul 28, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the proper arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Jul 30, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the proper arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Jul 30, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the proper arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Jul 30, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the correct arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Jul 30, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the correct arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Jul 31, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the correct arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Jul 31, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the correct arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Jul 31, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the correct arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Aug 1, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the correct arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Aug 14, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the correct arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Sep 5, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the correct arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Sep 6, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the correct arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Sep 7, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the correct arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Sep 11, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the correct arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Sep 13, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the correct arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Sep 13, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the correct arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/runtime-1 that referenced this issue Sep 14, 2018
This commit introduces a new watcher dedicated to the monitoring
of a specific network namespace in order to detect any change that
could happen to the network.

As a result of such a detection, the watcher should call into the
appropriate runtime path with the correct arguments to modify the
pod network accordingly.

Fixes kata-containers#170

Signed-off-by: Sebastien Boeuf <[email protected]>
zklei pushed a commit to zklei/runtime that referenced this issue Jun 13, 2019
Validate mount parameters where possible and log all parameters to help
debug scenarios where the client gRPC protocol is out-of-sync with the
agents (master) version.

Added a new test for `mount()`.

Fixes kata-containers#170.

Signed-off-by: James O. D. Hunt <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-gating Release must wait for this to be resolved before release
Projects
None yet
Development

No branches or pull requests

5 participants