network-binding-plugin: add plugin for vhostuser interfaces. #294

bgaussen · 2024-05-22T12:26:34Z

What this PR does / why we need it:

This design proposal aims at implementing a new network binding plugin to support vhostuser interfaces. This will allow fast userspace datapath when used with a userspace dataplane like OVS-DPDK or VPP.
This design proposal takes into consideration sockets sharing issues between kubevirt and dataplane pods.

Special notes for your reviewer:

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

Design: A design document was considered and is present.
PR: The PR description is expressive enough and will help future contributors
Community: Announcement to kubevirt-dev was considered

Release note:

NONE

This design proposal implements a new network binding plugin to support vhostuser interfaces. This will allow fast userspace datapath when used with a userspace dataplane like OVS-DPDK or VPP. This design proposal takes into consideration sockets sharing issues between kubevirt and dataplane pods. Signed-off-by: Benoît Gaussen <[email protected]>

kubevirt-bot · 2024-05-22T12:26:42Z

Hi @bgaussen. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

alicefr · 2024-05-22T13:27:31Z

design-proposals/network-vhostuser-binding-plugin/network-vhostuser-binding-plugin.md

+- hides the KubeVirt implementation details. Currently, you need to know where the KubeVirt sockets are located in the virt-launcher filesystem. Potentially, if we change the directory path for the sockets, this would break the CNI plugin
+- can isolate the resources dedicated to that particular plugin
+
+The drawback side is that `virt-launcher` pod would need to be run with `privileged: true` in order to do the bind mount.


I don't think this is true. The directory can be exposed by virt-handler. Hence, no need of privileged for virt-launcher. This would also be unfeasible since virt-launcher is untrusted

I don't see very well the interaction between virt-handler, virt-launcher and the exposed directories. Need to check that.

design-proposals/network-vhostuser-binding-plugin/network-vhostuser-binding-plugin.md

alicefr · 2024-05-22T13:50:33Z

design-proposals/network-vhostuser-binding-plugin/network-vhostuser-binding-plugin.md

+- dataplane: 1
+  This only resource is resquested by the userspace dataplane, and add a `/var/run/vhost_sockets` mount to the dataplane pod.
+- vhostuser sockets: n
+  This as many resources as we want to handle, is requested by the `virt-launcher` pod using vhostuser plugin. This makes the device plugin create a per pod directory like `/var/run/vhost_sockets/<launcher-id>`, and mount it into the `virt-launcher` pod.


Have you tried this? I still worry that the directory created by the dp will still have the original issue with the mount propagation HostToContainer

The only bind mount needed is the one the DP will push through kubelet. As there will be no further bind mount inside the pods, and as it will not be necessary for the CNI to do it neither, there should be no issue with mountPropagation.

Still I'm not sure how kubelet mounts the directory inside the pod. It might be possible that it is with HostToContainer and the socket won't be visible in the host directory. I would really like to check it with real code.

I understand your doubts ;) So I tested the scenario with generic-device-plugin, and the following resources definition:

- --domain - dataplane.io - --device - | name: dataplane groups: - count: 1 paths: - path: /var/lib/sockets mountPath: /var/lib/sockets type: Mount permissions: mrw - --device - | name: sockets groups: - paths: - path: /var/lib/sockets/pod* mountPath: /var/lib/socket type: Mount permissions: mrw

When a pod's container requests a datplane.io/socket, it will get assigned one of the existing /var/lib/sockets/podXXX, that gets bind mounted to /var/lib/socket in the container. Any file created there by the container is available in host /var/lib/sockets/podXXX.

If we have a dataplane pod requesting dataplane.io/dataplane, the whole /var/lib/sockets directory is bind mounted to /var/lib/sockets, and the dataplane container can see the files created in /var/lib/sockets/podXXX.
Reverse is also true if dataplane container creates a file in /var/lib/sockets/podXXX it is available to the earlier pod's container.

I tested with only unprivileged pods.

By the way I checked the propagation option of the target mount, it's the default private.

/ # findmnt -o TARGET,PROPAGATION /var/lib/one-socket TARGET PROPAGATION /var/lib/one-socket private

But as far as we don't create new mount in that target, there is no propagation issue.

alicefr · 2024-05-22T13:52:24Z

design-proposals/network-vhostuser-binding-plugin/network-vhostuser-binding-plugin.md

+- dataplane: 1
+  This only resource is resquested by the userspace dataplane, and add a `/var/run/vhost_sockets` mount to the dataplane pod.
+- vhostuser sockets: n
+  This as many resources as we want to handle, is requested by the `virt-launcher` pod using vhostuser plugin. This makes the device plugin create a per pod directory like `/var/run/vhost_sockets/<launcher-id>`, and mount it into the `virt-launcher` pod.


Is the pod ID know by the CNI plugin or outside KubeVirt?

DP can push annotation to the pod with an id it will generate. So it's not really the virt-launcher pod id, as it does not exist at the time DP is called.
The CNI can read the pod annotation and use it to configure the vhostuser with the right path in the data plane.

alicefr · 2024-05-22T13:54:13Z

design-proposals/network-vhostuser-binding-plugin/network-vhostuser-binding-plugin.md

+We still have to care about directory and sockets permission (and SELinux categories?). 
+
+## API Examples
+(tangible API examples used for discussion)


Please. still add here the examples, it helps with the reviews and when this proposal is merged

EdDev · 2024-05-22T14:53:04Z

/sig-network
/assign

Signed-off-by: Benoît Gaussen <[email protected]>

EdDev

I revisited this a bit late, sorry.

There are several points which I do not fully understand yet.
I would be interested in having a meeting to discuss this in detail, hopefully clarifying a few points and allowing this to be pushed forward.

EdDev · 2024-06-02T06:53:35Z

design-proposals/network-vhostuser-binding-plugin/network-vhostuser-binding-plugin.md

+The `vhostuser` secondary interfaces configuration in the dataplane is under the responsibility of Multus and the CNI such as `userspace CNI`.
+
+## Definition of Users
+Users of the feature are everyone that deploys a VM.


Usually we have these basic users:

Cluster Admin

VM User

Guest User

In this case, I think you also have:

Network Binding Plugin Developer

EdDev · 2024-06-02T06:55:25Z

design-proposals/network-vhostuser-binding-plugin/network-vhostuser-binding-plugin.md

+- As a user, I want the `vhostuser` interface to be configured with a specific MAC address.
+- As a user, I want to enable multi-queue on the `vhostuser` interface
+- As a Network Binding Plugin developper, I want the shared socket path to be accessible to virt-launcher pod 
+- As a CNI developper, I want to access the shared vhostuser sockets from Multus pod


I think this one is also under Network Binding Plugin developer.

EdDev · 2024-06-02T06:56:39Z

design-proposals/network-vhostuser-binding-plugin/network-vhostuser-binding-plugin.md

+- As a CNI developper, I want to access the shared vhostuser sockets from Multus pod
+
+## Repos
+Kubevirt repo, and most specificaly cmd/sidecars.


Can you please provide links for clarity?

EdDev · 2024-06-02T07:02:06Z

design-proposals/network-vhostuser-binding-plugin/network-vhostuser-binding-plugin.md

+1. Creates a new interface with `type='vhostuser'`
+2. Set the MAC address if specified in the VMI spec
+3. If `networkInterfaceMultiqueue` is set to `true`, add the number of queues calculated after the number of cores of the VMI
+4. Add `memAccess='shared'` to all NUMA cells elements


It is worth adding a note that the change needs to be idempotent.

To clarify, the hook is called multiple times and the result needs to be consistent between the first and following changes.

Also, I am unsure what are the side-effects of such a marking. Please share how such a change will influence the VM.

Hi Ed,

You mean the hook is called one time for each device using the same binding plugin? If so yes we need to add some words about that.

The OnDefineDomain hook can be called multiple times by Kubeivrt regardless of devices count.
Thus the sidecar implementation for OnDefineDomain hook should be idempotent.

You mean the hook is called one time for each device using the same binding plugin? If so yes we need to add some words about that.

Just assume that it can be called many times, you should not assume how many times.
Today, it is called twice in a normal flow, for the purpose of calculating some default that libvirt provides.
In that past it was called many more times (but we got it optimized since).

In a flow with failures, reconciliation may cause it to be called several times.

In the future, other hook points in the code flow may trigger it.

EdDev · 2024-06-02T09:03:49Z

design-proposals/network-vhostuser-binding-plugin/network-vhostuser-binding-plugin.md

+</interface>
+```
+
+This design leverages the existing `sockets` emptyDir mounted in `/var/run/kubevirt/sockets`. This allows the CNI to bind mount the socket emptyDir (`/var/lib/kubelet/<pod uid>/volumes/kubernetes.io~empty-dir/sockets`) to a host directory available to the dataplane pod through a hostPath mount.


I'm a bit confused with the changes this proposals has passed through.
Can you please keep the alternatives and the reasons they have been kept out in an appendix?
E.g. #294 (comment) discussed the DP performing the mounting.

Regarding this intro sentence:

How exactly can a CNI perform a mount to the pod/container? At the time the CNI is executed, there is not filesystem namespace defined yet. The CNI is also unlikely to have access to the api-server, it is unsafe and limited to trusted binaries only (e.g. Multus).

The diagram describes in step 4 that qemu creates the socket.
First, it will be nice to see that diagram steps described in details in text as well.
In general, libvirt handles interfaces in an unmanaged manner, i.e. all it needs is provided to it (e.g. tap device). This is done to allow libvirt to operate in an unprivileged/rootless mode.

Is the socket creation possible with the rootless priv?

Can the socket be created by something else and then just consumed by libvirt?

Regarding this intro sentence:

How exactly can a CNI perform a mount to the pod/container? At the time the CNI is executed, there is not filesystem namespace defined yet. The CNI is also unlikely to have access to the api-server, it is unsafe and limited to trusted binaries only (e.g. Multus).

Indeed CNI needs to access api-server. Multus 3 allowed that through its own kubeconfig.multus. With Multus 4 thick mode, it's no longer possible, and CNI requesting api-server access needs to handle their own credentials creation.

The diagram describes in step 4 that qemu creates the socket.
First, it will be nice to see that diagram steps described in details in text as well.
In general, libvirt handles interfaces in an unmanaged manner, i.e. all it needs is provided to it (e.g. tap device). This is done to allow libvirt to operate in an unprivileged/rootless mode.

Is the socket creation possible with the rootless priv?

Yes it is, as far the filesystem permissions AND SElinux allows it.

Can the socket be created by something else and then just consumed by libvirt?

It can. vhostusers sockets are created in server mode, and consumed in client mode. Usually qemu is in server mode, and the data plane the client. By the way the other way is getting deprecated in ovs-dpdk for example.

Thank you for the clarification.
As a generic policy, I would not recommend accessing the api-server from the CNI binary. It is unsafe and can compromise the cluster, especially when a worker node is compromised.
You can add rules to limit the access and mitigate the vulnerability, but it will be much cleaner if it is avoided.

If the mount path is known in advance, I think we can find alternatives.

EdDev · 2024-06-02T09:04:46Z

design-proposals/network-vhostuser-binding-plugin/network-vhostuser-binding-plugin.md

+
+## Alternative designs
+
+Some alternative designs were discussed in ![kubevirt-dev mailing-list](https://groups.google.com/g/kubevirt-dev/c/3w_WStrJfZw/m/yWSBpDAKAQAJ).


Please summarize them in an appendix so we can see the full picture in one place.

EdDev · 2024-06-02T09:16:04Z

design-proposals/network-vhostuser-binding-plugin/network-vhostuser-binding-plugin.md

+This requires to implement a new network binding plugin mechanism we could expose the content of a `virt-launcher` directory to an external plugin.
+The plugin registration in Kubervirt resource would define the target directory on the node where the directory should be exposed.
+
+This diagram explains this mechanism.
+
+![kubevirt-plugin-extension](kubevirt-plugin-extension.png)


I do not understand what is suggested here.
In virt-launcher we have a plugin container (the sidecar) and the compute container in which libvirt runs.
We have virt-handler in which a privileged operations may be conducted on the virt-launcher and on the api-server.

Do you want a new parameter to be added to the network binding plugin spec to share data of the plugin container with the node, the virt-handler or the compute container in the virt-launcher?

I do not understand how this is related to the CNI plugin. I guess this is related to my misunderstanding on how a CNI plugin can mount anything (I asked it in a different thread).

I do not understand what is suggested here. In virt-launcher we have a plugin container (the sidecar) and the compute container in which libvirt runs. We have virt-handler in which a privileged operations may be conducted on the virt-launcher and on the api-server.

I guess both virt-launcher and virt-handler needs to be clarified when it comes to network binding plugins.

Do you want a new parameter to be added to the network binding plugin spec to share data of the plugin container with the node, the virt-handler or the compute container in the virt-launcher?

In that case it'd be a new network binding plugin spec parameter.

I do not understand how this is related to the CNI plugin. I guess this is related to my misunderstanding on how a CNI plugin can mount anything (I asked it in a different thread).

Indeed, the CNI needs to know where the socket is located to configure the data plane with this socket. In current userspace CNI implementation, the CNI does a bind mount of the socket into data plane filesystem namespace.

the CNI does a bind mount of the socket into data plane filesystem namespace.

This part I do not understand.
AFAIU there is no socket yet, as that is created by libvirt when the domain is created.
Perhaps we can clarify this by listing all the steps that are actually done.

Sorry I meant "the CNI does a bind mount of directory where the socket will be created"...
You can refer to the diagram included in the DP.

EdDev · 2024-06-02T09:17:55Z

/sig network

toelke · 2024-06-17T10:40:05Z

I only found this discussion now, so I am very late in offering our experience:

We are productively using a self-build vhost-user management:

An admission webhook patches all pods with label kubevirt.io=virt-launcher to contain a hostPath volume (/run/vpp, in our case) and corresponding volumeMounts
A sidecar that patches the xml in a way very similar to this proposal. We add a new PCI subtree for the virtual devices, but I know that we are breaking kubevirt expectations there. Please note that having <reconnect enabled="yes"/> is useful here.

In our case, the driver creating the socket also creates a config.json in the same folder that is read by the sidecar to communicate information to the sidecar.

I will join the meeting on Wednesday.

ormergi

Thanks for the proposal

Please see my inline comments

ormergi · 2024-06-18T12:26:40Z

design-proposals/network-vhostuser-binding-plugin/network-vhostuser-binding-plugin.md

+        </numa>
+</cpu>
+<interface type='vhostuser'>
+    <source type='unix' path='/var/run/kubevirt/sockets/poda08a0fcbdea' mode='server'/>


I wonder, does the socket is created by the CNI inside virt-launcher pod?

If so, I think the socket path can be reflected on Multus network-status annotation under the subject element device-info, see https://github.com/k8snetworkplumbingwg/device-info-spec/blob/main/SPEC.md#315-vhost-user

The vhost-user device-info can be exposed using the network binding-plugin API's downardAPI.
The sidecar could then read the vhost-user socket from the mounted downwardAPI volume and spesify it in the domain xml.

Please see vDPA example for reference how downardAPI can be used.

Thank you for the hints. Indeed the new network binding plugin downardAPI will be helpful!

ormergi · 2024-06-18T12:29:35Z

design-proposals/network-vhostuser-binding-plugin/network-vhostuser-binding-plugin.md

+1. Creates a new interface with `type='vhostuser'`
+2. Set the MAC address if specified in the VMI spec
+3. If `networkInterfaceMultiqueue` is set to `true`, add the number of queues calculated after the number of cores of the VMI
+4. Add `memAccess='shared'` to all NUMA cells elements


The OnDefineDomain hook can be called multiple times by Kubeivrt regardless of devices count.
Thus the sidecar implementation for OnDefineDomain hook should be idempotent.

ormergi · 2024-06-18T12:30:30Z

design-proposals/network-vhostuser-binding-plugin/network-vhostuser-binding-plugin.md

+- As a user, I want to create a VM with one or serveral `vhostuser` interfaces attached to a userspace dataplane.
+- As a user, I want the `vhostuser` interface to be configured with a specific MAC address.
+- As a user, I want to enable multi-queue on the `vhostuser` interface
+- As a Network Binding Plugin developper, I want the shared socket path to be accessible to virt-launcher pod 


typo

Suggested change

- As a Network Binding Plugin developper, I want the shared socket path to be accessible to virt-launcher pod

- As a Network Binding Plugin developer, I want the shared socket path to be accessible to virt-launcher pod

bgaussen · 2024-06-26T13:02:07Z

Hi all,

I just committed modifications to this design proposal PR to focus on the device plugin based solution to facilitate socket sharing, as discussed during last week meeting.

Regards,

Benoit.

ormergi · 2024-06-27T10:35:02Z

design-proposals/network-vhostuser-binding-plugin/network-vhostuser-binding-plugin.md

+  This as many resources as we want to handle and may represent the number of port of the dataplane vSwitch.  
+  It is requested through VM or VMI definition in resources request spec. In turn the `virt-launcher` pod will request the same resources.  
+  This makes the device plugin create a per pod directory like `/var/run/vhost_sockets/<pod-dp-id>`, and mount it into the `virt-launcher` pod as to well known location `/var/run/vhostuser`.  
+  The device plugin has to generate a `pod-dp-id` and push it as an annotation in `virt-launcher` pod. This will be used later by the CNI or any component that configures the vhostuser socket in the dataplane with right path.


Note that CNI plugin that interacts with the cluster API (e.g.: get pod annotation) is not a good practice and better to avoid (for example if the CNI hangs pod creation hangs).

You're right and we are trying to not rely on API server for the CNI to get annotations.
For example this Mutlus PR could ease the task.

ormergi · 2024-06-27T10:48:28Z

design-proposals/network-vhostuser-binding-plugin/network-vhostuser-binding-plugin.md

+## Repos
+Kubevirt repo, and most specificaly [cmd/sidecars](https://github.com/kubevirt/kubevirt/tree/main/cmd/sidecars).
+
+## Design


Which kind of networks the plugin is going to support? (pod network, secondary networks)

The plugin is designed to support secondary networks.

yuvalk · 2024-07-21T03:41:13Z

design-proposals/network-vhostuser-binding-plugin/network-vhostuser-binding-plugin.md

+- check the VM is running
+
+# Implementation Phases
+1. First implementation of the `network-vhostuser-binding` done


can you link to where that is?

Yuval, sorry it's not yet ready to be shared publicly. We can have a talk if you want to have a look at the current implementation.

bgaussen · 2024-07-31T12:20:51Z

Hi all,

A little update: we're currently implementing the Device Plugin for sockets sharing. We're implementing the device-info spec to share device information between Device Plugin and CNI, and we'll rely on DownwardAPI support in Kubevirt 1.3 to get network-status annotation in the network binding plugin (as suggested by @ormergi, thx 😁).

I'll update the Design Proposal ASAP.

bgaussen · 2024-09-06T15:57:33Z

Hi All,

We made great progress and implemented Device Plugin (DP), CNI and Network Binding Plugin (NBP):

DP handles socketDirs where vhostuser sockets will be created, and injects the mount into the compute container requesting the socket device.
DP is compliant with device-info-spec , as pointed by @ormergi, and creates device plugin devinfo files
CNI implements also device-info-spec. That allows to get allocated device spec, including the socketDir path. It enables the CNI to push the spec, through multus, to virt-launcher pod annotations (that is available at the end in kubevirt.io/network-info annotation)
NBP retrieves the socketDir path from the annotation and DownwardAPI and creates the vhostuser interface in DomainXML with the right path to the socket

Example of this interface:

    <interface type='vhostuser'>
      <mac address='02:15:7e:00:00:05'/>
      <source type='unix' path='/var/lib/sockets/socketDir04/pod6c270ef2f25' mode='server'/>
      <target dev='pod6c270ef2f25'/>
      <model type='virtio-non-transitional'/>
      <driver name='vhost' queues='4' rx_queue_size='1024' tx_queue_size='1024'/>
      <alias name='ua-net1'/>
      <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/>
    </interface>

where /var/lib/sockets/socketDir04 is a random directory managed by the DP.

So far so good... but...

When we live migrate the VM, a new destination pod is created, with a new socket request to DP. The socketDir will obviously not be the same as the one on source pod (ex: /var/lib/sockets/socketDir09)

But from what I understand the source domainXML is sent to the destination pod and qemu tries to start with the source socketDir, and fails:

compute {"component":"virt-launcher","kind":"","level":"error","msg":"Live migration failed.","name":"ubuntu-vm1","namespace":"tests","pos":"live-migration-source.go:845","reason":"error encountered during Migrate
ToURI3 libvirt api call: virError(Code=1, Domain=10, Message='internal error: process exited while connecting to monitor: 2024-09-06T13:12:18.279191Z qemu-kvm: -chardev socket,id=charua-net1,path=/var/lib/sockets/
socketDir04/pod6c270ef2f25,server=on: Failed to bind socket to /var/lib/sockets/socketDir04/pod6c270ef2f25: No such file or directory')","timestamp":"2024-09-06T13:12:18.334616Z","uid":"33c51f90-1843-48ed-851f-4cb
11d75af41"}

Do you have any idea on how to manage this situation? Is there any plan to implements something through NBP livemigrate methods that could alter the destination domainXML?

As a "temporary" workaround, we intend to use a mutating webhook to inject a shared empty-dir volume in virt-launcher pod spec, that will be shared between NBP and compute containers (mounted in /var/lib/vhost-sockets). NBP will use the info from the kubevirt.io/network-info annotation/downwardAPI to create a link in the shared volume (/var/lib/vhost-sockets/net1 -> /var/lib/sockets/socketDir09).
The vhostuser socket path in domainXML will be /var/lib/vhostsockets/net1/pod6c270ef2f25, insuring that it will be accessible from the compute container and be the same on source and destination pods in live migration case.

Do you think the NBP spec could be amended to add a shared volume between compute and NBD sidecars?

Thanks,

Benoit.

ormergi · 2024-09-08T09:15:49Z

Hi @bgaussen , well done!

It looks like Kubevirt calls sidecar hooks also at the migration target here

Could you please ensure the NBP sidecar executes at the target?

bgaussen · 2024-09-08T19:27:26Z

Hi @bgaussen , well done!

It looks like Kubevirt calls sidecar hooks also at the migration target here

Could you please ensure the NBP sidecar executes at the target?

Yes the sidecar hooks get executed, and from its logs it sent back the modified domainXML with the right socketDir on destination pod.

But from what I understand here, the source domain is used for MigrateToURI3 libvirt call, with little modification in PrepareDomainForMigration.

bgaussen · 2024-09-20T07:29:54Z

Hi All,

After some private discussion with @alicefr, it seems that the migration domain takes precedence over the one generated at pod creation. From what I understand, it could be difficult to fix this behavior but I will open an issue with that.

In the meantime, for live migration to work, we need the sockets paths be the same on the source and target pods. Hence the symlinks mechanism seems a good way to achieve that.
But we need the Network Binding Plugin to access the compute container mount namespace, with two options:

A new shared empty-dir volume between compute and NBP sidecar containers.
It requires to be added to NBP spec. Could be interesting if other NBP requires such a feature...
Enable the shareProcessNamespace pod feature on virt-launcher. The sidecar container can target on of the compute container process, and access its mount namespace through /proc/PID/root, where the NBP can create the symlink.
There is already this PR we can benefit of: Refactor containerdisks by accessing the artifacts via proc and pid namespace sharing kubevirt#11845

We already implemented and tested 2., it works as expected. The only drawback is that we need to target virtlogd process. For other ones (virt-launcher-monitor, virt-launcher, qemu*), /proc/PID/root is not accessible.

We can discuss this in a community meeting and/or mailing list.

Benoit.

kubevirt-bot · 2024-10-14T15:13:18Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign jean-edouard for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Benoît Gaussen <[email protected]>

…evice-info-spec, downwardAPI and shareProcessNamespace Signed-off-by: Benoît Gaussen <[email protected]>

aburdenthehand · 2024-10-15T11:09:52Z

@bgaussen Hi Benoit, just to let you know your most recent commit was not signed. I know you're still working on this so whenever you next squash or rebase please ensure that all commits are signed off to avoid any issues further down the line. Thanks!

EdDev · 2024-10-28T05:29:19Z

In the meantime, for live migration to work, we need the sockets paths be the same on the source and target pods. Hence the symlinks mechanism seems a good way to achieve that.

I think that shareProcessNamespace is problematic. It flattens the process NS from the whole pod and in practice drops an expected isolation of containers in the pod.
It feels to me like a hack with major implications which are not easy to asses.

I think there is another option which could be explored: Using libvirt hooks.
Libvirt does have an option to mutate the domain configuration on the target through script files, which could answer the need here. In the past, at least at other projects (e.g. oVirt) this direction encountered opposition but I do not recall the exact reasons.

Libvirt hooks are documented here:
https://wiki.archlinux.org/title/Libvirt#Hooks
https://libvirt.org/hooks.html

bgaussen · 2024-10-28T12:57:25Z

I think that shareProcessNamespace is problematic. It flattens the process NS from the whole pod and in practice drops an expected isolation of containers in the pod. It feels to me like a hack with major implications which are not easy to asses.

Understand this drawback with shareProcessNamespace, but as discussed with @alicefr, if it is to be implemented in the PR #11845, we could be benefit of it.
If not we need another way.

I think there is another option which could be explored: Using libvirt hooks. Libvirt does have an option to mutate the domain configuration on the target through script files, which could answer the need here. In the past, at least at other projects (e.g. oVirt) this direction encountered opposition but I do not recall the exact reasons.

Libvirt hooks are documented here: https://wiki.archlinux.org/title/Libvirt#Hooks https://libvirt.org/hooks.html

I briefly look at libvirt qemu hooks, indeed we could mutate the domain when it's received, before it's started. But we need to figure out how to add the scripts in compute container...

Another way would be to add a dedicated shared dir between compute container and hook sidecar binding plugin where we can create the links...

Benoit.

EdDev · 2024-10-31T12:07:47Z

Another way would be to add a dedicated shared dir between compute container and hook sidecar binding plugin where we can create the links...

That you for helping me understand this well in the unconference meeting.
This one seems to me a good candidate as well, especially when it is so simple.
First, it can be easily implemented with a mutation webhook. I do agree it is better we provide it as a formal integration option between the compute and other sidecars.

Was this option challenged by some risk?
@alicefr , having a well defined are where sidecars and the compute can share data seems a nice additions to the integration "protocol". What is your opinion on this option? (sorry if I missed a prev conversation about this)

alicefr · 2024-11-04T08:56:01Z

Was this option challenged by some risk? @alicefr , having a well defined are where sidecars and the compute can share data seems a nice additions to the integration "protocol". What is your opinion on this option? (sorry if I missed a prev conversation about this)

There is already an option with a PVC and the sidecars , we could modify it to allow an empty dir as well.

bgaussen · 2024-12-05T09:01:09Z

Was this option challenged by some risk? @alicefr , having a well defined are where sidecars and the compute can share data seems a nice additions to the integration "protocol". What is your opinion on this option? (sorry if I missed a prev conversation about this)

There is already an option with a PVC and the sidecars , we could modify it to allow an empty dir as well.

@alicefr, @EdDev,

Following the unconference meeting and the discussion we had, we implemented the shared emtpy-dir logic with a mutating webhook.

The binding plugin creates the link (immutable path and socket name), in this empty-dir, to the real socket, and uses this link in the domainXML vhostuser interface.

This solves the issue observed with Live Migration.

This seems indeed to be the best solution, as there is already the basis to enable a new mount for this empty-dir for sidecar containers.

kubevirt-bot added the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label May 22, 2024

kubevirt-bot requested review from aburdenthehand and jean-edouard May 22, 2024 12:26

kubevirt-bot added the size/L label May 22, 2024

alicefr reviewed May 22, 2024

View reviewed changes

kubevirt-bot assigned EdDev May 22, 2024

bgaussen added 2 commits May 22, 2024 18:13

Fix typo, adding precision for device plugin generating a per pod id.

b3d7cb7

Signed-off-by: Benoît Gaussen <[email protected]>

Fix typos.

3d900ec

Signed-off-by: Benoît Gaussen <[email protected]>

EdDev reviewed Jun 2, 2024

View reviewed changes

kubevirt-bot added the sig/network label Jun 2, 2024

ormergi reviewed Jun 18, 2024

View reviewed changes

kubevirt-bot added dco-signoff: no Indicates the PR's author has not DCO signed all their commits. and removed dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jun 26, 2024

ormergi reviewed Jul 7, 2024

View reviewed changes

yuvalk reviewed Jul 21, 2024

View reviewed changes

bgaussen added 2 commits October 14, 2024 20:15

Re-factor design proposal to focus on device plugin proposal

7cbf677

Signed-off-by: Benoît Gaussen <[email protected]>

Update design with device plugin implementation and requirement for d…

01dfcc6

…evice-info-spec, downwardAPI and shareProcessNamespace Signed-off-by: Benoît Gaussen <[email protected]>

bgaussen force-pushed the vhostuser-network-binding-plugin branch from d74c8f7 to 01dfcc6 Compare October 14, 2024 18:16

kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Oct 14, 2024


		## Alternative designs

		Some alternative designs were discussed in ![kubevirt-dev mailing-list](https://groups.google.com/g/kubevirt-dev/c/3w_WStrJfZw/m/yWSBpDAKAQAJ).

	- As a Network Binding Plugin developper, I want the shared socket path to be accessible to virt-launcher pod
	- As a Network Binding Plugin developer, I want the shared socket path to be accessible to virt-launcher pod

network-binding-plugin: add plugin for vhostuser interfaces. #294

Are you sure you want to change the base?

network-binding-plugin: add plugin for vhostuser interfaces. #294

Conversation

bgaussen commented May 22, 2024

kubevirt-bot commented May 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgaussen May 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EdDev commented May 22, 2024

EdDev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EdDev commented Jun 2, 2024

toelke commented Jun 17, 2024

ormergi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgaussen commented Jun 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgaussen Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

bgaussen commented Jul 31, 2024

bgaussen commented Sep 6, 2024

ormergi commented Sep 8, 2024 • edited Loading

bgaussen commented Sep 8, 2024

bgaussen commented Sep 20, 2024

kubevirt-bot commented Oct 14, 2024

aburdenthehand commented Oct 15, 2024

EdDev commented Oct 28, 2024

bgaussen commented Oct 28, 2024

EdDev commented Oct 31, 2024

alicefr commented Nov 4, 2024

bgaussen commented Dec 5, 2024

bgaussen May 31, 2024 •

edited

Loading

bgaussen Jul 31, 2024 •

edited

Loading

ormergi commented Sep 8, 2024 •

edited

Loading