Slirp hook sidecar #10272

ormergi · 2023-08-10T18:42:49Z

What this PR does / why we need it:
This PR introduce Slirp hook sidecar, its a new hook sidecar for configuring Slirp network interfaces in Kubevirt.

Currently Slirp interfaces configure by Kubevirt, internally.
With the new sidecar hook, its possible to delegate Slirp interfces configuration outside Kubevirt
and enable reducing code maintenance.

Unlike other hook sidecar examples, Slirp hook-sidcar is set automatically by Kubevirt for VMs who has Slirp interface.
No need to add Kubeviet hook-sidecar annotation or any other configuration.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:
Commits 4-7 are for testing slirp hook sidecar e2e, integrating the new hook-sidecar will be done in a follow up PR.

Release note:

Introduce network binding plugin for Slirp networking, interfacing with Kubevirt new network binding plugin API.

ormergi · 2023-08-10T18:44:25Z

/test pull-kubevirt-unit-test
/test pull-kubevirt-build
/test pull-kubevirt-generate
/test pull-kubevirt-manifests
/test pull-kubevirt-e2e-k8s-1.27-sig-network

kubevirt-bot · 2023-08-10T18:44:34Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

victortoso

Now that we have the cmd/sidecar directory, if you agree in using the sidecar-shim image, I think it would be great to move this from cmd/slirp-hook-sidecar to cmd/sidecar/slirp.

victortoso · 2023-08-10T22:14:53Z

cmd/slirp-hook-sidecar/BUILD.bazel

+        "@io_bazel_rules_go//go/platform:linux_arm64": "arm64",
+        "//conditions:default": "amd64",
+    }),
+    base = ":version-container",


Hey, #9231 just got merged and that introduces a sidecar-shim base image that you could use and reduce some of the grpc boilerplate. This commit is the change for the smbios example

Hi Victor, thanks for the heads up!

This slirp hook sidecar will be used as a reference for other network binding plugins (based on Kubevirt hook points and sidecar containers).
I am not sure we have the new sidecar-shim image available on potential D/S implementations, thus we decided to start with the "classic" hook implantation.

We will revisit the usage of sidecar-shim as a follow-up

victortoso · 2023-08-10T22:15:31Z

cmd/slirp-hook-sidecar/README.md

@@ -0,0 +1,25 @@
+# KubeVirt SMBIOS hook sidecar


Probably best to fill up with slirp related content

victortoso · 2023-08-10T22:17:03Z

BUILD.bazel

@@ -346,6 +347,15 @@ container_push(
    tag = "$(container_tag)",
 )

+container_push(
+    name = "push-slirp-hook-sidecar",


Have you requested to the quay.io container to be created?

Per my understanding having the image built as part of cluster-sync will also include in the release, is that right?

+container_push( + name = "push-network-slirp-binding", + format = "Docker", + image = "//cmd/network-slirp-binding:network-slirp-binding-image", + registry = "$(container_prefix)", + repository = "$(image_prefix)network-slirp-binding", + tag = "$(container_tag)", +)

I meant that there is no network-slirp-binding repository in kubevirt/network-slirp-binding like we have for smbios sidecar example. If you want to create it, you should ping @dhiller and the nightly builds will update the repository.

ormergi · 2023-08-13T15:30:33Z

/test pull-kubevirt-unit-test
/test pull-kubevirt-build
/test pull-kubevirt-generate
/test pull-kubevirt-manifests
/test pull-kubevirt-e2e-k8s-1.27-sig-network

ormergi · 2023-08-13T15:30:42Z

/cc @EdDev @AlonaKaplan

ormergi · 2023-08-14T07:45:32Z

/test pull-kubevirt-unit-test
/test pull-kubevirt-build
/test pull-kubevirt-generate
/test pull-kubevirt-manifests
/test pull-kubevirt-e2e-k8s-1.27-sig-network

EdDev

Thanks!

I think we should attempt and place in one package all the skeleton so we could re-use that package in future bindings.
In terms of dependency, the skeleton should not depend on the specific binding (e.g. slirp).

If this will take too much time, then we should just follow up on this immediately after this PR.

EdDev · 2023-08-14T08:53:02Z

cmd/slirp-hook-sidecar/BUILD.bazel

+)
+
+go_binary(
+    name = "slirp-hook-sidecar",


Could you please use the name network-slirp-binding instead?
Same for the folder name, package etc.

EdDev · 2023-08-14T08:56:49Z

cmd/slirp-hook-sidecar/README.md

@@ -0,0 +1,5 @@
+# KubeVirt Slirp network hook sidecar


I would prefer this is called KubeVirt Network Slirp binding and in the body to describe it is implemented through the sidecar hooks interface.

If there is a ref to the hook interfaces, then we could add it here.
If there is none, we should add it in a follow up.

EdDev · 2023-08-14T08:59:12Z

cmd/slirp-hook-sidecar/README.md

+
+This hook sidecar configures VMs Slirp interface.
+
+It will be used by Kubevirt to offload slip networking configuration and reduce code maintenance.


I think we can drop the reduce code maintenance. and describe what it does and how at high level (e.g. modifies the domain configuration based on the VMI spec).

EdDev · 2023-08-14T09:09:00Z

cmd/slirp-hook-sidecar/callback/callback.go

+	"encoding/xml"
+	"fmt"
+
+	vmSchema "kubevirt.io/api/core/v1"


Strange to see camel-case format on imports.

Fixed all over

EdDev · 2023-08-14T09:10:51Z

cmd/slirp-hook-sidecar/callback/callback.go

+
+	vmSchema "kubevirt.io/api/core/v1"
+
+	domainSchema "kubevirt.io/kubevirt/pkg/virt-launcher/virtwrap/api"


As a follow up: We need to extract the domain API definition to an upper level package in order to declare it as public and make sure no one will break it.

EdDev · 2023-08-14T11:19:29Z

cmd/slirp-hook-sidecar/server/server.go

+}
+
+func (s InfoServer) Info(_ context.Context, _ *hooksInfo.InfoParams) (*hooksInfo.InfoResult, error) {
+	log.Log.Infof("Hook's Info method has been called")


I do not think we need this in production.

EdDev · 2023-08-14T11:19:55Z

cmd/slirp-hook-sidecar/server/server.go

+}
+
+func (s V1alpha2Server) OnDefineDomain(_ context.Context, params *hooksV1alpha2.OnDefineDomainParams) (*hooksV1alpha2.OnDefineDomainResult, error) {
+	log.Log.Info("OnDefineDomain callback method has been called")


I do not think we need this in production.

This is the only indication a callback was invoked from the sidecar perspective.
It helps with debugging so I think its worth keeping it.

EdDev · 2023-08-14T11:20:22Z

cmd/slirp-hook-sidecar/server/server.go

+}
+
+func (s V1alpha2Server) PreCloudInitIso(_ context.Context, params *hooksV1alpha2.PreCloudInitIsoParams) (*hooksV1alpha2.PreCloudInitIsoResult, error) {
+	log.Log.Info("PreCloudInitIso method has been called")


I do not think we need this in production.

This is the only indication a callback was invoked from the sidecar perspective.
It helps with debugging so I think its worth keeping it.

EdDev · 2023-08-14T11:56:37Z

cmd/slirp-hook-sidecar/domain/domain.go

+	return []domainSchema.Arg{
+		{Value: "-netdev"},
+		backendDeviceConf,
+	}, nil


nit: An one-liner should work here.

EdDev · 2023-08-14T11:58:45Z

cmd/slirp-hook-sidecar/domain/domain.go

+		_, _, err := net.ParseCIDR(network.Pod.VMNetworkCIDR)
+		if err != nil {


nit: can be one-liner

ormergi

@EdDev I resolved most of the comments, still new to fix README.
The first two commits were divided to so that the first commit is a bare skeleton one could cherry pick and reuse.
Second commit add dns package (parse resolv.conf)
Third commit add domain package (concrete implementation)
Fourth commit wire things up

Please take another look

ormergi · 2023-08-15T00:42:39Z

cmd/slirp-hook-sidecar/BUILD.bazel

+)
+
+go_binary(
+    name = "slirp-hook-sidecar",


ormergi · 2023-08-15T00:43:09Z

cmd/slirp-hook-sidecar/callback/callback.go

+	"encoding/xml"
+	"fmt"
+
+	vmSchema "kubevirt.io/api/core/v1"


Fixed all over

ormergi · 2023-08-15T00:45:36Z

cmd/slirp-hook-sidecar/callback/callback.go

+	"kubevirt.io/kubevirt/cmd/slirp-hook-sidecar/domain"
+)
+
+type SlirpSpecGenerator interface {


Done

OnDefineDomain changed to not include any logic that related to slirp.
Its now exposes the DomainSpecMutator interface to enable injecting domain spec mutation logic.

ormergi · 2023-08-15T00:45:43Z

cmd/slirp-hook-sidecar/callback/callback.go

+		// Initializing the domain XML namespace with qemu scheme is necessary for
+		// passing commands to underlying QEMU.


ormergi · 2023-08-15T00:46:08Z

cmd/slirp-hook-sidecar/callback/callback.go

+		// Initializing the domain XML namespace with qemu scheme is necessary for
+		// passing commands to underlying QEMU.
+		// https://libvirt.org/drvqemu.html#pass-through-of-arbitrary-qemu-commands
+		XmlNS: "http://libvirt.org/schemas/domain/qemu/1.0",


Done, the string is exported to a const.

ormergi · 2023-08-15T00:47:21Z

cmd/slirp-hook-sidecar/callback/callback.go

+	slirpNetworkQemuCmdArgs, err := slirpSpecGen.Generate(vmi.Spec.Domain.Devices.Interfaces, vmi.Spec.Networks, searchDomains)
+	if err != nil {
+		return nil, err
+	}
+	if len(slirpNetworkQemuCmdArgs) == 0 {
+		log.Log.Warning("no QEMU slirp network cmd args were generated, returning original domain spec")
+		return domainXML, nil
+	}
+	domainSpec.QEMUCmd = domain.InitQemuCmdArgs(domainSpec.QEMUCmd)
+	domainSpec.QEMUCmd.QEMUArg = append(domainSpec.QEMUCmd.QEMUArg, slirpNetworkQemuCmdArgs...)


Done

The above logic is moved to domain package.

ormergi · 2023-08-15T00:47:38Z

cmd/slirp-hook-sidecar/server/server.go

+	log.Log.Infof("Hook's Info method has been called")
+
+	return &hooksInfo.InfoResult{
+		Name: "slirp",


ormergi · 2023-08-15T00:48:01Z

cmd/slirp-hook-sidecar/domain/domain.go

+	return []domainSchema.Arg{
+		{Value: "-netdev"},
+		backendDeviceConf,
+	}, nil


ormergi · 2023-08-15T00:48:05Z

cmd/slirp-hook-sidecar/domain/domain.go

+		_, _, err := net.ParseCIDR(network.Pod.VMNetworkCIDR)
+		if err != nil {


EdDev

I reviewed everything except the DEBUG* commits.

EdDev · 2023-08-16T05:52:03Z

cmd/network-slirp-binding/README.md

+
+This hook sidecar configures VMs Slirp interface.
+
+It will be used by Kubevirt to offload slip networking configuration and reduce code maintenance.


Please drop and reduce code maintenance.

EdDev · 2023-08-16T05:59:02Z

cmd/network-slirp-binding/callback/callback_test.go

+)
+
+var _ = Describe("hook callback handler", func() {
+	Context("on define domain", func() {


What is the purpose of this additional later? If it has no functional usage, consider removing it and embedding the text in the upper or lower layers.

What do you mean by additional later?

I meant "layer". Why do you need the Context?

EdDev · 2023-08-16T06:01:20Z

cmd/network-slirp-binding/callback/callback_test.go

+
+			Expect(callback.OnDefineDomain(domainSpecXML, domSpecMutator)).To(Equal(domainSpecXML))
+		})
+	})


What about:

Showing that a mutation is possible (just replacing the whole thing with something else is enough).

Showing the behavior when there is no input (empty byte stream).

EdDev · 2023-08-16T06:04:41Z

cmd/network-slirp-binding/slirp.go

I think you should call this file main.

EdDev · 2023-08-16T06:13:45Z

cmd/network-slirp-binding/slirp.go

 	srv "kubevirt.io/kubevirt/cmd/network-slirp-binding/server"
 )

 func main() {
+	searchDomains, err := dns.ReadResolvConfSearchDomains()
+	if err != nil {
+		panic(err)


IMO it should exist normally with a message to stderr (using the log) and not panic. Panic fits libraries perhaps, not main().

I see you kept the panic.

Edit: Ignore this, I saw a different one.

It fixed in the second commit daa84f7
I think Github had a hiccup.

EdDev · 2023-08-16T06:22:11Z

cmd/network-slirp-binding/domain/domain_test.go

+			//testMutator := domain.SlirpNetworkConfigurator{
+			//	Interfaces:    []vmschema.Interface{iface},
+			//	Networks:      []vmschema.Network{network},
+			//	SearchDomains: testSearchDomain,
+			//}
+
+			//_, err := testMutator.Mutate(&domainschema.DomainSpec{})
+			//Expect(err).To(HaveOccurred())


EdDev · 2023-08-16T06:22:25Z

cmd/network-slirp-binding/domain/domain_test.go

+				//testMutator := domain.SlirpNetworkConfigurator{
+				//	Interfaces:    []vmschema.Interface{iface},
+				//	Networks:      []vmschema.Network{network},
+				//	SearchDomains: testSearchDomain,
+				//}
+				//
+				//_, err := testMutator.Mutate(&domainschema.DomainSpec{})
+				//Expect(err).To(HaveOccurred())


EdDev · 2023-08-16T06:25:33Z

cmd/network-slirp-binding/server/server.go

+
+	slirpConfigurator, err := domain.NewSlirpNetworkConfigurator(vmi.Spec.Domain.Devices.Interfaces, vmi.Spec.Networks, s.SearchDomains)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create slirp configurator: %v", err)


It seems it will fail if no slirp interface is detected.
Should we really fail or just log an error on stderr?

IMO it should fail, it means that the plugin is called with invalid VM configuration that should have been detected by the webhook.

It also means that the sidecar runs even if slirp is not defined on the VMI spec.
Maybe someone defined in manually to run and not through the new API.

@AlonaKaplan , what do you think?

Seems reasonable to fail. The sidecar should work only with the new API, if somebody configure it manually, it makes sense it will fail...

EdDev · 2023-08-16T07:46:50Z

cmd/network-slirp-binding/domain/domain_test.go

+					{Value: `-device`},
+					{Value: `{"driver":"e1000","netdev":"default","id":"default"}`},
+				},
+			),


There is a need to check that given a domain with an existing slirp configuration defined, it does not create another one but keeps the original (or overrides it).

With the current implementation additional command line args will be added.
I added a test to reflect this behavior.

In practice, when I tested the new sidecar the generated domain XML had the correct command line args although the onDefineDomain hook called twice (no duplicating args).

We can make the slirp plugin smarter up by making it looks into the QMEU command line arg values and act accordingly (not adding args if already exist, add missing args, etc..).but it will require more work and testing.

How about we do that on a follow up PR if we see there is a real issue?

In practice, when I tested the new sidecar the generated domain XML had the correct command line args although the onDefineDomain hook called twice (no duplicating args).

How is this possible? If it is called twice, it is suppose to add twice the configuration.

How about we do that on a follow up PR if we see there is a real issue?

It is a real issue because you cannot assume it is not called 100 times.

And I still expect to see this expressed in a test.

How is this possible? If it is called twice, it is suppose to add twice the configuration.

The added QEMU cmd args get removed along with other attriburte but the network placeholder thing.
This is why it works and wont end up with duplication.

It is a real issue because you cannot assume it is not called 100 times.
And I still expect to see this expressed in a test.

Can we do this on a follow up PR?

OK, we can consider it as a bug. But please follow up on this.

EdDev · 2023-08-16T07:57:02Z

cmd/network-slirp-binding/domain/domain.go

+	domainSpec.QEMUCmd = initQemuCmdArgs(domainSpec.QEMUCmd)
+	domainSpec.QEMUCmd.QEMUArg = append(domainSpec.QEMUCmd.QEMUArg, slirpQemuCmdArgs...)


I am not sure if we can or should override any existing definitions.
Other sidecars hooks may use these commands.

Note that this is no overriding, slirp related command args are appended to the domain spec commandline.

The initQemuCmdArgs is confusing, I though it initializes it (always), but it seems it is conditioned.

I have also added a test to reflect this behavior.

The name init is indeed confusing. Maybe initQemuCmdArgsIfNeeded.

Looking at this again, I dont think we need it at all, I inlined initQemuCmdArgsIfNeeded.

EdDev · 2023-08-16T11:08:04Z

cmd/network-slirp-binding/domain/domain_test.go

+					{Value: `-device`},
+					{Value: `{"driver":"e1000","netdev":"default","id":"default"}`},
+				},
+			),


In practice, when I tested the new sidecar the generated domain XML had the correct command line args although the onDefineDomain hook called twice (no duplicating args).

How is this possible? If it is called twice, it is suppose to add twice the configuration.

How about we do that on a follow up PR if we see there is a real issue?

It is a real issue because you cannot assume it is not called 100 times.

And I still expect to see this expressed in a test.

EdDev · 2023-08-16T11:10:39Z

cmd/network-slirp-binding/domain/domain_test.go

+			testMutator, err := domain.NewSlirpNetworkConfigurator([]vmschema.Interface{iface}, []vmschema.Network{network}, testSearchDomain)
+			Expect(err).ToNot(HaveOccurred())
+
+			//expectedArg := "abc"


Sorry for the mess, fixed

EdDev · 2023-08-16T11:10:45Z

cmd/network-slirp-binding/domain/domain_test.go

+
+			//expectedArg := "abc"
+			domSpec := &domainschema.DomainSpec{
+				// QEMUCmd: &domainschema.Commandline{QEMUArg: []domainschema.Arg{{Value: expectedArg}}},


Sorry for the mess, fixed

EdDev · 2023-08-16T11:15:13Z

cmd/network-slirp-binding/main.go

+	if err != nil {
+		log.Log.Reason(err).Errorf("Failed to initialized socket on path: %s", socket)
+		log.Log.Error("Check whether given directory exists and socket name is not already taken by other file")
+		panic(err)


Can you replace this panic as well?

EdDev

I am not sure the release notes are correct. We have not replaced the backend implementations with sidecars.
I would avoid adding a RN for now, at least until we will really use sidecars instead of the internal implementation.

You could mention this is specific to the network binding plugin API.

EdDev · 2023-08-17T05:00:23Z

cmd/network-slirp-binding/domain/domain_test.go

+					{Value: `-device`},
+					{Value: `{"driver":"e1000","netdev":"default","id":"default"}`},
+				},
+			),


OK, we can consider it as a bug. But please follow up on this.

EdDev · 2023-08-17T05:03:53Z

pkg/virt-launcher/virtwrap/converter/network.go

-		// Slirp configuration works only with e1000 or rtl8139
-		if iface.Model != "e1000" && iface.Model != "rtl8139" {
+		// Slirp configuration works with e1000 only
+		if iface.Model != "e1000" {


I do not understand this change.
Based on what rt18139 is now overridden by e1000?

Sorry, the correct changes were probably overridden by the rebase, the correct fix is in place now.

EdDev · 2023-08-17T05:05:25Z

cmd/network-slirp-binding/domain/domain.go

+		log.Log.Infof("interface (%q) model type (%q) is not supported by QEMU slirp Network, using 'e1000' model type instead",
+			iface.Name, iface.Model)
+	}
+	netDeviceConf := fmt.Sprintf(`"driver":"e1000","netdev":%[1]q,"id":%[1]q`, iface.Name)


What was fixed? I fail to see the change per the comment.

EdDev · 2023-08-17T05:06:16Z

cmd/network-slirp-binding/domain/domain.go

+	var ports []vmschema.Port
+	configuredPorts := make(map[string]struct{}, 0)
+	for _, ifacePort := range iface.Ports {
+		if ifacePort.Port <= 0 || ifacePort.Port >= 65536 {


Pending feedback.

ormergi · 2023-08-17T05:51:55Z

Rebase

cmd/network-slirp-binding: |_ callback |_ server slirp.go server & slirp.go Boilerplate code for registration to Kubevrit's hooks. Can be replaced with sidecar-shim container in the future. callback: Implement Kubevirt OnDefineDomain hook point callback. The DomainSpecMutator interface should be implemented by the concrete slirp binding implementation. Slirp binding implementation will be introduced in follow-up commits. Signed-off-by: Or Mergi <ormergi@redhat.com>

resolv.conf search domains required for slirp networking configuration. Signed-off-by: Or Mergi <ormergi@redhat.com>

Signed-off-by: Or Mergi <ormergi@redhat.com>

AlonaKaplan · 2023-08-17T09:28:22Z

/approve

Thanks!

kubevirt-bot · 2023-08-17T09:28:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AlonaKaplan

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [AlonaKaplan]

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

EdDev

Thank you!!

EdDev · 2023-08-17T12:37:25Z

Unrelated failures:

/test pull-kubevirt-e2e-k8s-1.27-sig-compute
/test pull-kubevirt-e2e-kind-1.27-vgpu

kubevirt-bot requested review from maiqueb, rhrazdil and rthallisey August 10, 2023 18:44

victortoso suggested changes Aug 10, 2023

View reviewed changes

kubevirt-bot assigned victortoso Aug 10, 2023

kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 11, 2023

ormergi force-pushed the slirp-hook-sidecar branch from bfd9ef0 to 99a7701 Compare August 13, 2023 15:17

kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 13, 2023

kubevirt-bot requested review from AlonaKaplan and EdDev August 13, 2023 15:30

ormergi force-pushed the slirp-hook-sidecar branch from 99a7701 to 8f0b198 Compare August 14, 2023 07:45

EdDev reviewed Aug 14, 2023

View reviewed changes

ormergi mentioned this pull request Aug 14, 2023

Integrate net bind api and slirp hook sidecar #10289

Closed

ormergi force-pushed the slirp-hook-sidecar branch from 8f0b198 to 9715b54 Compare August 15, 2023 07:15

ormergi commented Aug 15, 2023

View reviewed changes

EdDev reviewed Aug 16, 2023

View reviewed changes

ormergi force-pushed the slirp-hook-sidecar branch from 9715b54 to d731eab Compare August 16, 2023 10:46

EdDev reviewed Aug 16, 2023

View reviewed changes

ormergi force-pushed the slirp-hook-sidecar branch from d731eab to 95b95bf Compare August 16, 2023 11:18

kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 17, 2023

EdDev reviewed Aug 17, 2023

View reviewed changes

ormergi force-pushed the slirp-hook-sidecar branch from 07333b3 to 0b84604 Compare August 17, 2023 05:51

kubevirt-bot added size/XL and removed size/XXL needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Aug 17, 2023

ormergi added 2 commits August 17, 2023 08:56

slirp binding plugin: Parse resolv.conf search domains

3879018

resolv.conf search domains required for slirp networking configuration. Signed-off-by: Or Mergi <ormergi@redhat.com>

ormergi force-pushed the slirp-hook-sidecar branch 2 times, most recently from 97ebafc to 5d22478 Compare August 17, 2023 09:10

ormergi added 5 commits August 17, 2023 12:19

slirp binding plugin: Implement slirp network configurator

2faae64

Signed-off-by: Or Mergi <ormergi@redhat.com>

slirp binding plugin: Wire up slirp configurator to callback server

8fc6c5a

Signed-off-by: Or Mergi <ormergi@redhat.com>

slirp binding plugin: Add bazel rule to push image to local registry

22cd66a

Signed-off-by: Or Mergi <ormergi@redhat.com>

slirp binding plugin: Build & push image as part of cluster-sync

e27a7ea

Signed-off-by: Or Mergi <ormergi@redhat.com>

slirp configurator: Support ifaces with network binding plugin

487446e

Signed-off-by: Or Mergi <ormergi@redhat.com>

ormergi force-pushed the slirp-hook-sidecar branch from 5d22478 to 487446e Compare August 17, 2023 09:22

ormergi marked this pull request as ready for review August 17, 2023 09:23

kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 17, 2023

kubevirt-bot requested a review from EdDev August 17, 2023 09:26

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 17, 2023

EdDev approved these changes Aug 17, 2023

View reviewed changes

kubevirt-bot assigned EdDev Aug 17, 2023

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 17, 2023

kubevirt-bot merged commit ba271c0 into kubevirt:main Aug 17, 2023
36 checks passed

victortoso mentioned this pull request Aug 22, 2023

Use sidecar-shim base image with other sidecars and move them under cmd/sidecars #10320

Merged

ormergi mentioned this pull request Aug 31, 2023

Delegate Slirp network configuration to hook sidecar using Slirp network binding plugin #10366

Merged

8 tasks


		This hook sidecar configures VMs Slirp interface.

		It will be used by Kubevirt to offload slip networking configuration and reduce code maintenance.


		vmSchema "kubevirt.io/api/core/v1"

		domainSchema "kubevirt.io/kubevirt/pkg/virt-launcher/virtwrap/api"

		_, _, err := net.ParseCIDR(network.Pod.VMNetworkCIDR)
		if err != nil {

		// Initializing the domain XML namespace with qemu scheme is necessary for
		// passing commands to underlying QEMU.

		domainSpec.QEMUCmd = initQemuCmdArgs(domainSpec.QEMUCmd)
		domainSpec.QEMUCmd.QEMUArg = append(domainSpec.QEMUCmd.QEMUArg, slirpQemuCmdArgs...)

Slirp hook sidecar #10272

Slirp hook sidecar #10272

Conversation

ormergi commented Aug 10, 2023 • edited

ormergi commented Aug 10, 2023

kubevirt-bot commented Aug 10, 2023

victortoso 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

ormergi commented Aug 13, 2023

ormergi commented Aug 13, 2023

ormergi commented Aug 14, 2023

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

Choose a reason for hiding this comment

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

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 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

EdDev Aug 16, 2023 • edited

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ormergi commented Aug 10, 2023 •

edited

EdDev Aug 16, 2023 •

edited