Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UDN: Patch Kubevirt CR to support managedTap binding #4773

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions contrib/kind-common
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ install_kubevirt() {
# vX.Y.Z - install specific stable (i.e v1.3.1)
# nightly - install newest nightly
# nightly tag - install specific nightly (i.e 20240910)
KUBEVIRT_VERSION=${KUBEVIRT_VERSION:-"stable"}
KUBEVIRT_VERSION=${KUBEVIRT_VERSION:-"nightly"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we wait for a kubevirt release ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will ask if a special one can be created but i doubt as the minor should change maybe,
will also ask when the stable is out, it might take time

imho we can use 20241029, it is safer, see please
#4773 (comment)
we will just need to update once there is a stable and wont be blocked
but your call

Copy link
Contributor Author

@oshoval oshoval Oct 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disregard, stable.txt wont have it, checking about alternatives (i.e new file on that path etc)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make this clear: this is the only thing holding this PR. I.e. we want to pin a 1.4 RC version.

Right ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep


for node in $(kubectl get node --no-headers -o custom-columns=":metadata.name"); do
$OCI_BIN exec -t $node bash -c "echo 'fs.inotify.max_user_watches=1048576' >> /etc/sysctl.conf"
Expand Down Expand Up @@ -356,11 +356,11 @@ install_kubevirt() {
done
fi

kubectl -n kubevirt patch kubevirt kubevirt --type=json --patch '[{"op":"add","path":"/spec/configuration/developerConfiguration","value":{"featureGates":[]}},{"op":"add","path":"/spec/configuration/developerConfiguration/featureGates/-","value":"NetworkBindingPlugins"}]'
kubectl -n kubevirt patch kubevirt kubevirt --type=json --patch '[{"op":"add","path":"/spec/configuration/developerConfiguration","value":{"featureGates":[]}},{"op":"add","path":"/spec/configuration/developerConfiguration/featureGates/-","value":"NetworkBindingPlugins"},{"op":"add","path":"/spec/configuration/developerConfiguration/featureGates/-","value":"DynamicPodInterfaceNaming"}]'

local kubevirt_stable_release_url=$(get_kubevirt_release_url "stable")
local passt_binding_image="quay.io/kubevirt/network-passt-binding:${kubevirt_stable_release_url##*/}"
kubectl -n kubevirt patch kubevirt kubevirt --type=json --patch '[{"op":"add","path":"/spec/configuration/network","value":{}},{"op":"add","path":"/spec/configuration/network/binding","value":{"passt":{"computeResourceOverhead":{"requests":{"memory":"500Mi"}},"migration":{"method":"link-refresh"},"networkAttachmentDefinition":"default/primary-udn-kubevirt-binding","sidecarImage":"'"${passt_binding_image}"'"}}}]'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't we want both of them ? and let the user pick which binding to use ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just because you asked so i dropped
i can return it if desired as we spoke

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's have both.

kubectl -n kubevirt patch kubevirt kubevirt --type=json --patch '[{"op":"add","path":"/spec/configuration/network","value":{}},{"op":"add","path":"/spec/configuration/network/binding","value":{"passt":{"computeResourceOverhead":{"requests":{"memory":"500Mi"}},"migration":{"method":"link-refresh"},"networkAttachmentDefinition":"default/primary-udn-kubevirt-binding","sidecarImage":"'"${passt_binding_image}"'"},"managedTap":{"domainAttachmentType":"managedTap","migration":{}}}}]'

if [ ! -d "./bin" ]
then
Expand Down
16 changes: 4 additions & 12 deletions test/e2e/kubevirt.go
Original file line number Diff line number Diff line change
Expand Up @@ -1201,7 +1201,7 @@ passwd:
}, butane)
Expect(err).ToNot(HaveOccurred())
vm.Spec.Template.Spec.Domain.Devices.Interfaces[0].Bridge = nil
vm.Spec.Template.Spec.Domain.Devices.Interfaces[0].Binding = &kubevirtv1.PluginBinding{Name: "passt"}
vm.Spec.Template.Spec.Domain.Devices.Interfaces[0].Binding = &kubevirtv1.PluginBinding{Name: "managedTap"}
createVirtualMachine(vm)
return vm.Name
},
Expand Down Expand Up @@ -1232,7 +1232,7 @@ passwd:
}, butane)
Expect(err).ToNot(HaveOccurred())
vmi.Spec.Domain.Devices.Interfaces[0].Bridge = nil
vmi.Spec.Domain.Devices.Interfaces[0].Binding = &kubevirtv1.PluginBinding{Name: "passt"}
vmi.Spec.Domain.Devices.Interfaces[0].Binding = &kubevirtv1.PluginBinding{Name: "managedTap"}
createVirtualMachineInstance(vmi)
return vmi.Name
},
Expand Down Expand Up @@ -1317,11 +1317,7 @@ passwd:

step := by(vmi.Name, "Login to virtual machine for the first time")
Eventually(func() error {
if td.role != "primary" {
return kubevirt.LoginToFedora(vmi, "core", "fedora")
} else {
return kubevirt.LoginToFedoraWithHostname(vmi, "core", "fedora", "localhost")
}
maiqueb marked this conversation as resolved.
Show resolved Hide resolved
return kubevirt.LoginToFedora(vmi, "core", "fedora")
}).
WithTimeout(5*time.Second).
WithPolling(time.Second).
Expand Down Expand Up @@ -1350,11 +1346,7 @@ passwd:
td.test.cmd()

step = by(vm.Name, fmt.Sprintf("Login to virtual machine after %s %s", td.resource.description, td.test.description))
if td.role != "primary" {
Expect(kubevirt.LoginToFedora(vmi, "core", "fedora")).To(Succeed(), step)
} else {
Expect(kubevirt.LoginToFedoraWithHostname(vmi, "core", "fedora", "localhost")).To(Succeed(), step)
}
Expect(kubevirt.LoginToFedora(vmi, "core", "fedora")).To(Succeed(), step)
var obtainedAddresses []string

if td.role != "primary" { // expect 2 addresses on dual-stack deployments; 1 on single-stack
Expand Down
Loading