Skip to content

Commit

Permalink
Merge pull request #10185 from AlonaKaplan/sriov_hotplug
Browse files Browse the repository at this point in the history 
SRIOV migration based hotplug
  • Loading branch information
@kubevirt-bot
kubevirt-bot committed Aug 6, 2023
2 parents 1a24534 + 560e4cb commit ca740d9
Show file tree
Hide file tree
Showing 12 changed files with 704 additions and 460 deletions.
2 changes: 1 addition & 1 deletion pkg/network/sriov/sriov.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func mapNetworkNameToPCIAddress(networks []v1.Network, interfaces []v1.Interface
multusInterfaceName := networkNameScheme[sriovIface.Name]
networkStatusEntry, exist := multusInterfaceNameToNetworkStatusMap[multusInterfaceName]
if !exist {
return nil, fmt.Errorf("failed to find network-status entry with interface %q", multusInterfaceName)
continue // The interface is not plugged yet
}
if networkStatusEntry.DeviceInfo == nil || networkStatusEntry.DeviceInfo.Pci == nil {
return nil, fmt.Errorf("failed to find device-info/pci-address in network-status annotation for SR-IOV interface %q", sriovIface.Name)
Expand Down
34 changes: 18 additions & 16 deletions pkg/network/sriov/sriov_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,22 +200,6 @@ var _ = Describe("SRIOV", func() {
networkPCIAnnotationValue := sriov.CreateNetworkPCIAnnotationValue(networkList, interfaceList, networkStatusAnnotationValue)
Expect(networkPCIAnnotationValue).To(Equal("{}"))
},
Entry("when pod's networkStatus Annotation does not exist",
[]virtv1.Network{newMultusNetwork("foo", "default/nad1")},
[]virtv1.Interface{newSRIOVInterface("foo")},
"",
),
Entry("when networkStatusAnnotation is valid but one SR-IOV entry is missing",
[]virtv1.Network{
newMultusNetwork("foo", "default/nad1"),
newMultusNetwork("boo", "default/nad2"),
},
[]virtv1.Interface{
newSRIOVInterface("foo"),
newSRIOVInterface("boo"),
},
fmt.Sprintf(networkStatusWithOneSRIOVNetworkFmt, fooHashedIfaceName),
),
Entry("when networkStatusAnnotation is valid but with no pci data on one of the SRIOV interfaces",
[]virtv1.Network{
newMultusNetwork("foo", "default/nad1"),
Expand Down Expand Up @@ -308,6 +292,24 @@ var _ = Describe("SRIOV", func() {
fmt.Sprintf(networkStatusWithOneBridgeOneSRIOVNetworksFmt, fooOrdinalIfaceName, booOrdinalIfaceName),
`{"foo":"0000:65:00.2"}`,
),
Entry("when pod's networkStatus Annotation does not exist",
[]virtv1.Network{newMultusNetwork("foo", "default/nad1")},
[]virtv1.Interface{newSRIOVInterface("foo")},
"",
`{}`,
),
Entry("when networkStatusAnnotation is valid but one SR-IOV entry is missing",
[]virtv1.Network{
newMultusNetwork("foo", "default/nad1"),
newMultusNetwork("boo", "default/nad2"),
},
[]virtv1.Interface{
newSRIOVInterface("foo"),
newSRIOVInterface("boo"),
},
fmt.Sprintf(networkStatusWithOneSRIOVNetworkFmt, fooHashedIfaceName),
`{"foo":"0000:04:02.5"}`,
),
)
})

Expand Down
2 changes: 1 addition & 1 deletion pkg/network/vmispec/hotplug.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func NetworksToHotplug(networks []v1.Network, interfaceStatus []v1.VirtualMachin
func IndexInterfacesFromStatus(interfaces []v1.VirtualMachineInstanceNetworkInterface, p func(ifaceStatus v1.VirtualMachineInstanceNetworkInterface) bool) map[string]v1.VirtualMachineInstanceNetworkInterface {
indexedInterfaceStatus := map[string]v1.VirtualMachineInstanceNetworkInterface{}
for _, iface := range interfaces {
if p(iface) {
if p == nil || p(iface) {
indexedInterfaceStatus[iface.Name] = iface
}
}
Expand Down
25 changes: 15 additions & 10 deletions pkg/virt-controller/watch/network.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,28 @@ import (
"kubevirt.io/kubevirt/pkg/network/vmispec"
)

func calculateDynamicInterfaces(vmi *v1.VirtualMachineInstance) ([]v1.Interface, []v1.Network, bool) {
vmiSpecIfaces := vmispec.FilterInterfacesSpec(vmi.Spec.Domain.Devices.Interfaces, func(iface v1.Interface) bool {
func calculateInterfacesAndNetworksForMultusAnnotationUpdate(vmi *v1.VirtualMachineInstance) ([]v1.Interface, []v1.Network, bool) {
vmiNonAbsentSpecIfaces := vmispec.FilterInterfacesSpec(vmi.Spec.Domain.Devices.Interfaces, func(iface v1.Interface) bool {
return iface.State != v1.InterfaceStateAbsent
})
ifacesToHotUnplugExist := len(vmi.Spec.Domain.Devices.Interfaces) > len(vmiSpecIfaces)
ifacesToHotUnplugExist := len(vmi.Spec.Domain.Devices.Interfaces) > len(vmiNonAbsentSpecIfaces)

vmiSpecNets := vmi.Spec.Networks
if ifacesToHotUnplugExist {
vmiSpecNets = vmispec.FilterNetworksByInterfaces(vmi.Spec.Networks, vmiSpecIfaces)
}
ifacesToHotplugExist := len(vmispec.NetworksToHotplug(vmiSpecNets, vmi.Status.Interfaces)) > 0
ifacesStatusByName := vmispec.IndexInterfacesFromStatus(vmi.Status.Interfaces, nil)
ifacesToAnnotate := vmispec.FilterInterfacesSpec(vmiNonAbsentSpecIfaces, func(iface v1.Interface) bool {
_, ifaceInStatus := ifacesStatusByName[iface.Name]
sriovIfaceNotPlugged := iface.SRIOV != nil && !ifaceInStatus
return !sriovIfaceNotPlugged
})

networksToAnnotate := vmispec.FilterNetworksByInterfaces(vmi.Spec.Networks, ifacesToAnnotate)

ifacesToHotplugExist := len(vmispec.NetworksToHotplug(networksToAnnotate, vmi.Status.Interfaces)) > 0

isIfaceChangeRequired := ifacesToHotplugExist || ifacesToHotUnplugExist
if !isIfaceChangeRequired {
return nil, nil, false
}
return vmiSpecIfaces, vmiSpecNets, isIfaceChangeRequired
return ifacesToAnnotate, networksToAnnotate, isIfaceChangeRequired
}

func applyDynamicIfaceRequestOnVMI(vm *v1.VirtualMachine, vmi *v1.VirtualMachineInstance, hasOrdinalIfaces bool) *v1.VirtualMachineInstanceSpec {
Expand All @@ -49,7 +54,7 @@ func applyDynamicIfaceRequestOnVMI(vm *v1.VirtualMachine, vmi *v1.VirtualMachine
vmIndexedNetworks := vmispec.IndexNetworkSpecByName(vm.Spec.Template.Spec.Networks)
for _, vmIface := range vm.Spec.Template.Spec.Domain.Devices.Interfaces {
_, existsInVMISpec := vmiIndexedInterfaces[vmIface.Name]
shouldBeHotPlug := !existsInVMISpec && vmIface.State != v1.InterfaceStateAbsent && vmIface.InterfaceBindingMethod.Bridge != nil
shouldBeHotPlug := !existsInVMISpec && vmIface.State != v1.InterfaceStateAbsent && (vmIface.InterfaceBindingMethod.Bridge != nil || vmIface.InterfaceBindingMethod.SRIOV != nil)
shouldBeHotUnplug := !hasOrdinalIfaces && existsInVMISpec && vmIface.State == v1.InterfaceStateAbsent
if shouldBeHotPlug {
vmiSpecCopy.Networks = append(vmiSpecCopy.Networks, vmIndexedNetworks[vmIface.Name])
Expand Down
60 changes: 53 additions & 7 deletions pkg/virt-controller/watch/network_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,15 @@ var _ = Describe("Network interface hot{un}plug", func() {

testNetworkName1 = "testnet1"
testNetworkName2 = "testnet2"
testNetworkName3 = "testnet3"
testNetworkName4 = "testnet4"

ordinal = true
)
DescribeTable("calculate if changes are required",

func(vmi *v1.VirtualMachineInstance, pod *k8sv1.Pod, expIfaces []v1.Interface, expNets []v1.Network, expToChange bool) {
ifaces, nets, exists := calculateDynamicInterfaces(vmi)
ifaces, nets, exists := calculateInterfacesAndNetworksForMultusAnnotationUpdate(vmi)
Expect(ifaces).To(Equal(expIfaces))
Expect(nets).To(Equal(expNets))
Expect(exists).To(Equal(expToChange))
Expand All @@ -69,23 +71,52 @@ var _ = Describe("Network interface hot{un}plug", func() {
),
Entry("when vmi interfaces have an extra interface which requires hotplug",
libvmi.New(
libvmi.WithInterface(v1.Interface{Name: testNetworkName1}),
libvmi.WithInterface(v1.Interface{Name: testNetworkName1, InterfaceBindingMethod: v1.InterfaceBindingMethod{SRIOV: &v1.InterfaceSRIOV{}}}),
libvmi.WithInterface(v1.Interface{Name: testNetworkName2}),
libvmi.WithInterface(v1.Interface{Name: testNetworkName3}),
libvmi.WithInterface(v1.Interface{Name: testNetworkName4, InterfaceBindingMethod: v1.InterfaceBindingMethod{SRIOV: &v1.InterfaceSRIOV{}}}),
libvmi.WithNetwork(&v1.Network{Name: testNetworkName1}),
libvmi.WithNetwork(&v1.Network{Name: testNetworkName2}),
libvmi.WithNetwork(&v1.Network{Name: testNetworkName3}),
libvmi.WithNetwork(&v1.Network{Name: testNetworkName4}),
withInterfaceStatus(v1.VirtualMachineInstanceNetworkInterface{Name: testNetworkName1}),
withInterfaceStatus(v1.VirtualMachineInstanceNetworkInterface{Name: testNetworkName2}),
),
&k8sv1.Pod{
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{
networkv1.NetworkStatusAnnot: `[
{"interface":"net1", "name":"red-net", "namespace": "default"}
{"interface":"net1", "name":"red-net", "namespace": "default"},
{"interface":"net2", "name":"blue-net", "namespace": "default"}
]`,
}},
},
[]v1.Interface{{Name: testNetworkName1}, {Name: testNetworkName2}},
[]v1.Network{{Name: testNetworkName1}, {Name: testNetworkName2}},
[]v1.Interface{{Name: testNetworkName1, InterfaceBindingMethod: v1.InterfaceBindingMethod{SRIOV: &v1.InterfaceSRIOV{}}}, {Name: testNetworkName2}, {Name: testNetworkName3}},
[]v1.Network{{Name: testNetworkName1}, {Name: testNetworkName2}, {Name: testNetworkName3}},
expectToChange,
),
Entry("when vmi interfaces have an extra SRIOV interface which requires hotplug, change is not required since SRIOV hotplug to a pod is not supported",
libvmi.New(
libvmi.WithInterface(v1.Interface{Name: testNetworkName1}),
libvmi.WithInterface(v1.Interface{Name: testNetworkName2, InterfaceBindingMethod: v1.InterfaceBindingMethod{SRIOV: &v1.InterfaceSRIOV{}}}),
libvmi.WithInterface(v1.Interface{Name: testNetworkName3, InterfaceBindingMethod: v1.InterfaceBindingMethod{SRIOV: &v1.InterfaceSRIOV{}}}),
libvmi.WithNetwork(&v1.Network{Name: testNetworkName1}),
libvmi.WithNetwork(&v1.Network{Name: testNetworkName2}),
libvmi.WithNetwork(&v1.Network{Name: testNetworkName3}),
withInterfaceStatus(v1.VirtualMachineInstanceNetworkInterface{Name: testNetworkName1}),
withInterfaceStatus(v1.VirtualMachineInstanceNetworkInterface{Name: testNetworkName2}),
),
&k8sv1.Pod{
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{
networkv1.NetworkStatusAnnot: `[
{"interface":"net1", "name":"red-net", "namespace": "default"},
{"interface":"net2", "name":"blue-net", "namespace": "default"}
]`,
}},
},
nil,
nil,
expectNoChange,
),
Entry("when a vmi interface has state set to `absent`, requiring hotunplug",
libvmi.New(
libvmi.WithInterface(v1.Interface{Name: testNetworkName1}),
Expand Down Expand Up @@ -148,7 +179,7 @@ var _ = Describe("Network interface hot{un}plug", func() {
libvmi.WithNetwork(&v1.Network{Name: testNetworkName1}),
),
false),
Entry("when an interface has to be hotplugged",
Entry("when a bridge binding interface has to be hotplugged",
libvmi.New(
libvmi.WithInterface(bridgeInterface(testNetworkName1)),
libvmi.WithNetwork(&v1.Network{Name: testNetworkName1}),
Expand All @@ -159,7 +190,18 @@ var _ = Describe("Network interface hot{un}plug", func() {
libvmi.WithNetwork(&v1.Network{Name: testNetworkName1}),
),
!ordinal),
Entry("when an interface has to be hotplugged but it has no bridge binding",
Entry("when an SRIOV binding interface has to be hotplugged",
libvmi.New(
libvmi.WithInterface(sriovInterface(testNetworkName1)),
libvmi.WithNetwork(&v1.Network{Name: testNetworkName1}),
),
libvmi.New(),
libvmi.New(
libvmi.WithInterface(sriovInterface(testNetworkName1)),
libvmi.WithNetwork(&v1.Network{Name: testNetworkName1}),
),
!ordinal),
Entry("when an interface has to be hotplugged but it has no SRIOV or bridge binding",
libvmi.New(
libvmi.WithInterface(v1.Interface{Name: testNetworkName1, InterfaceBindingMethod: v1.InterfaceBindingMethod{Macvtap: &v1.InterfaceMacvtap{}}}),
libvmi.WithNetwork(&v1.Network{Name: testNetworkName1}),
Expand Down Expand Up @@ -275,6 +317,10 @@ func bridgeInterface(name string) v1.Interface {
return v1.Interface{Name: name, InterfaceBindingMethod: v1.InterfaceBindingMethod{Bridge: &v1.InterfaceBridge{}}}
}

func sriovInterface(name string) v1.Interface {
return v1.Interface{Name: name, InterfaceBindingMethod: v1.InterfaceBindingMethod{SRIOV: &v1.InterfaceSRIOV{}}}
}

func bridgeAbsentInterface(name string) v1.Interface {
iface := bridgeInterface(name)
iface.State = v1.InterfaceStateAbsent
Expand Down
6 changes: 3 additions & 3 deletions pkg/virt-controller/watch/vmi.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1267,8 +1267,8 @@ func (c *VMIController) sync(vmi *virtv1.VirtualMachineInstance, pod *k8sv1.Pod,
}
}

if vmiSpecIfaces, vmiSpecNets, dynamicIfacesExist := calculateDynamicInterfaces(vmi); dynamicIfacesExist {
if err := c.handleDynamicInterfaceRequests(vmi.Namespace, vmiSpecIfaces, vmiSpecNets, pod); err != nil {
if vmiSpecIfaces, vmiSpecNets, dynamicIfacesExist := calculateInterfacesAndNetworksForMultusAnnotationUpdate(vmi); dynamicIfacesExist {
if err := c.updateMultusAnnotation(vmi.Namespace, vmiSpecIfaces, vmiSpecNets, pod); err != nil {
return &syncErrorImpl{
err: fmt.Errorf("failed to hot{un}plug network interfaces for vmi [%s/%s]: %w", vmi.GetNamespace(), vmi.GetName(), err),
reason: FailedHotplugSyncReason,
Expand Down Expand Up @@ -2275,7 +2275,7 @@ func (c *VMIController) getVolumePhaseMessageReason(volume *virtv1.Volume, names
return virtv1.VolumePending, PVCNotReadyReason, "PVC is in phase Lost"
}

func (c *VMIController) handleDynamicInterfaceRequests(namespace string, interfaces []virtv1.Interface, networks []virtv1.Network, pod *k8sv1.Pod) error {
func (c *VMIController) updateMultusAnnotation(namespace string, interfaces []virtv1.Interface, networks []virtv1.Network, pod *k8sv1.Pod) error {
podAnnotations := pod.GetAnnotations()

indexedMultusStatusIfaces := services.NonDefaultMultusNetworksIndexedByIfaceName(pod)
Expand Down

0 comments on commit ca740d9

Please sign in to comment.