Skip to content

Commit

Permalink
Merge pull request kubevirt#13277 from orelmisan/mv-hotplug-logic
Browse files Browse the repository at this point in the history 
net annotations-gen: Cleanup NIC hotplug / unplug logic
  • Loading branch information
@kubevirt-bot
kubevirt-bot authored Dec 29, 2024
2 parents 0e46f7e + 32ed1ce commit fa41b79
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 169 deletions.
2 changes: 2 additions & 0 deletions pkg/network/pod/annotations/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@ go_test(
deps = [
":go_default_library",
"//pkg/libvmi:go_default_library",
"//pkg/libvmi/status:go_default_library",
"//pkg/network/downwardapi:go_default_library",
"//pkg/network/istio:go_default_library",
"//pkg/network/multus:go_default_library",
"//pkg/network/vmispec:go_default_library",
"//pkg/testutils:go_default_library",
"//pkg/virt-config:go_default_library",
"//staging/src/kubevirt.io/api/core/v1:go_default_library",
Expand Down
32 changes: 30 additions & 2 deletions pkg/network/pod/annotations/generator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ func (g Generator) GenerateFromActivePod(vmi *v1.VirtualMachineInstance, pod *k8
}

func (g Generator) generateMultusAnnotation(vmi *v1.VirtualMachineInstance, pod *k8scorev1.Pod) (string, bool) {
vmiSpecIfaces, vmiSpecNets, dynamicIfacesExist := vmispec.CalculateInterfacesAndNetworksForMultusAnnotationUpdate(vmi)
if !dynamicIfacesExist {
vmiSpecIfaces, vmiSpecNets, ifaceChangeRequired := ifacesAndNetsForMultusAnnotationUpdate(vmi)
if !ifaceChangeRequired {
return "", false
}

Expand Down Expand Up @@ -170,3 +170,31 @@ func shouldAddIstioKubeVirtAnnotation(vmi *v1.VirtualMachineInstance) bool {

return len(interfacesWithMasqueradeBinding) > 0
}

func ifacesAndNetsForMultusAnnotationUpdate(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(vmiNonAbsentSpecIfaces)

ifacesStatusByName := vmispec.IndexInterfaceStatusByName(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)

ifacesToHotplug := vmispec.FilterInterfacesSpec(ifacesToAnnotate, func(iface v1.Interface) bool {
_, inStatus := ifacesStatusByName[iface.Name]
return !inStatus
})
ifacesToHotplugExist := len(ifacesToHotplug) > 0

ifaceChangeRequired := ifacesToHotplugExist || ifacesToHotUnplugExist
if !ifaceChangeRequired {
return nil, nil, false
}
return ifacesToAnnotate, networksToAnnotate, ifaceChangeRequired
}
68 changes: 68 additions & 0 deletions pkg/network/pod/annotations/generator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,12 @@ import (
"kubevirt.io/client-go/kubecli"

"kubevirt.io/kubevirt/pkg/libvmi"
libvmistatus "kubevirt.io/kubevirt/pkg/libvmi/status"
"kubevirt.io/kubevirt/pkg/network/downwardapi"
"kubevirt.io/kubevirt/pkg/network/istio"
"kubevirt.io/kubevirt/pkg/network/multus"
"kubevirt.io/kubevirt/pkg/network/pod/annotations"
"kubevirt.io/kubevirt/pkg/network/vmispec"
"kubevirt.io/kubevirt/pkg/testutils"
virtconfig "kubevirt.io/kubevirt/pkg/virt-config"
)
Expand Down Expand Up @@ -520,6 +522,22 @@ var _ = Describe("Annotations Generator", func() {
clusterConfig = newClusterConfig(kv)
})

It("Should not generate network attachment annotation when there are no networks", func() {
vmi := libvmi.New(
libvmi.WithNamespace(testNamespace),
libvmi.WithAutoAttachPodInterface(false),
)

pod := newStubVirtLauncherPod(vmi, map[string]string{
networkv1.NetworkStatusAnnot: multusNetworkStatusWithPrimaryNet,
})

generator := annotations.NewGenerator(clusterConfig)

annotations := generator.GenerateFromActivePod(vmi, pod)
Expect(annotations).ToNot(HaveKey(networkv1.NetworkAttachmentAnnot))
})

DescribeTable("Should not generate network attachment annotation", func(podAnnotations map[string]string) {
vmi := libvmi.New(
libvmi.WithNamespace(testNamespace),
Expand Down Expand Up @@ -569,6 +587,34 @@ var _ = Describe("Annotations Generator", func() {
),
)

It("Should not generate network attachment annotation when all spec interfaces are present in status", func() {
vmi := libvmi.New(
libvmi.WithNamespace(testNamespace),
libvmi.WithInterface(*v1.DefaultBridgeNetworkInterface()),
libvmi.WithInterface(libvmi.InterfaceDeviceWithBridgeBinding(network1Name)),
libvmi.WithNetwork(v1.DefaultPodNetwork()),
libvmi.WithNetwork(libvmi.MultusNetwork(network1Name, networkAttachmentDefinitionName1)),
libvmistatus.WithStatus(
libvmistatus.New(
libvmistatus.WithInterfaceStatus(v1.VirtualMachineInstanceNetworkInterface{Name: "default", PodInterfaceName: "eth0"}),
libvmistatus.WithInterfaceStatus(v1.VirtualMachineInstanceNetworkInterface{
Name: network1Name,
InfoSource: vmispec.InfoSourceMultusStatus,
}),
),
),
)

pod := newStubVirtLauncherPod(vmi, map[string]string{
networkv1.NetworkAttachmentAnnot: multusNetworksAnnotation,
networkv1.NetworkStatusAnnot: multusNetworkStatusWithPrimaryAndSecondaryNets,
})
generator := annotations.NewGenerator(clusterConfig)

annotations := generator.GenerateFromActivePod(vmi, pod)
Expect(annotations).ToNot(HaveKey(networkv1.NetworkAttachmentAnnot))
})

It("Should generate network attachment annotation when VMI is not connected to secondary networks and an interface is hot plugged",
func() {
vmi := libvmi.New(
Expand Down Expand Up @@ -637,6 +683,28 @@ var _ = Describe("Annotations Generator", func() {
Expect(annotations[networkv1.NetworkAttachmentAnnot]).To(MatchJSON(multusNetworksAnnotationWithTwoNets))
})

It("Should not generate network attachment annotation when an SR-IOV iface is hot plugged", func() {
vmi := libvmi.New(
libvmi.WithNamespace(testNamespace),
libvmi.WithInterface(*v1.DefaultBridgeNetworkInterface()),
libvmi.WithInterface(libvmi.InterfaceDeviceWithSRIOVBinding(network1Name)),
libvmi.WithNetwork(v1.DefaultPodNetwork()),
libvmi.WithNetwork(libvmi.MultusNetwork(network1Name, networkAttachmentDefinitionName1)),
libvmistatus.WithStatus(libvmistatus.New(
libvmistatus.WithInterfaceStatus(v1.VirtualMachineInstanceNetworkInterface{Name: "default"}),
)),
)

podAnnotations := map[string]string{
networkv1.NetworkStatusAnnot: multusNetworkStatusWithPrimaryNet,
}

generator := annotations.NewGenerator(clusterConfig)
annotations := generator.GenerateFromActivePod(vmi, newStubVirtLauncherPod(vmi, podAnnotations))

Expect(annotations).ToNot(HaveKey(networkv1.NetworkAttachmentAnnot))
})

It("Should generate network attachment annotation when a secondary interface is hot unplugged", func() {
vmi := libvmi.New(
libvmi.WithNamespace(testNamespace),
Expand Down
3 changes: 0 additions & 3 deletions pkg/network/vmispec/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,7 @@ go_test(
"//pkg/libvmi/status:go_default_library",
"//staging/src/kubevirt.io/api/core/v1:go_default_library",
"//staging/src/kubevirt.io/client-go/testutils:go_default_library",
"//vendor/github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1:go_default_library",
"//vendor/github.com/onsi/ginkgo/v2:go_default_library",
"//vendor/github.com/onsi/gomega:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
],
)
28 changes: 0 additions & 28 deletions pkg/network/vmispec/hotplug.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,34 +25,6 @@ import (
v1 "kubevirt.io/api/core/v1"
)

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

ifacesStatusByName := IndexInterfaceStatusByName(vmi.Status.Interfaces, nil)
ifacesToAnnotate := FilterInterfacesSpec(vmiNonAbsentSpecIfaces, func(iface v1.Interface) bool {
_, ifaceInStatus := ifacesStatusByName[iface.Name]
sriovIfaceNotPlugged := iface.SRIOV != nil && !ifaceInStatus
return !sriovIfaceNotPlugged
})

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

ifacesToHotplug := FilterInterfacesSpec(ifacesToAnnotate, func(iface v1.Interface) bool {
_, inStatus := ifacesStatusByName[iface.Name]
return !inStatus
})
ifacesToHotplugExist := len(ifacesToHotplug) > 0

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

func NetworksToHotplugWhosePodIfacesAreReady(vmi *v1.VirtualMachineInstance) []v1.Network {
var networksToHotplug []v1.Network
interfacesToHoplug := IndexInterfaceStatusByName(
Expand Down
136 changes: 0 additions & 136 deletions pkg/network/vmispec/hotplug_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,6 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

k8sv1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"

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

"kubevirt.io/kubevirt/pkg/libvmi"
Expand Down Expand Up @@ -80,135 +75,4 @@ var _ = Describe("utilitary funcs to identify attachments to hotplug", func() {
),
)
})

Context("CalculateInterfacesAndNetworksForMultusAnnotationUpdate", func() {
const (
expectNoChange = false
expectToChange = !expectNoChange

testNetworkName1 = "testnet1"
testNetworkName2 = "testnet2"
testNetworkName3 = "testnet3"
testNetworkName4 = "testnet4"
)
DescribeTable("calculate if changes are required",
func(vmi *v1.VirtualMachineInstance, pod *k8sv1.Pod, expIfaces []v1.Interface, expNets []v1.Network, expToChange bool) {
ifaces, nets, exists := vmispec.CalculateInterfacesAndNetworksForMultusAnnotationUpdate(vmi)
Expect(ifaces).To(Equal(expIfaces))
Expect(nets).To(Equal(expNets))
Expect(exists).To(Equal(expToChange))
},
Entry("when no interfaces exist, change is not required", libvmi.New(), nil, nil, nil, expectNoChange),
Entry("when vmi interfaces match pod multus annotation and status, change is not required",
libvmi.New(
libvmi.WithInterface(v1.Interface{Name: testNetworkName1}),
libvmi.WithNetwork(&v1.Network{Name: testNetworkName1}),
withInterfaceStatus(v1.VirtualMachineInstanceNetworkInterface{Name: testNetworkName1}),
),
&k8sv1.Pod{
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{
networkv1.NetworkStatusAnnot: `[
{"interface":"net1", "name":"red-net", "namespace": "default"}
]`,
}},
}, nil, nil, expectNoChange,
),
Entry("when vmi interfaces have an extra interface which requires hotplug",
libvmi.New(
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":"net2", "name":"blue-net", "namespace": "default"}
]`,
}},
},
[]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}),
libvmi.WithInterface(v1.Interface{Name: testNetworkName2, State: v1.InterfaceStateAbsent}),
libvmi.WithNetwork(&v1.Network{Name: testNetworkName1}),
libvmi.WithNetwork(&v1.Network{Name: testNetworkName2}),
withInterfaceStatus(v1.VirtualMachineInstanceNetworkInterface{Name: testNetworkName1}),
withInterfaceStatus(v1.VirtualMachineInstanceNetworkInterface{Name: testNetworkName2}),
),
&k8sv1.Pod{
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{
networkv1.NetworkStatusAnnot: `[
{"interface":"pod1", "name":"red-net", "namespace": "default"},
{"interface":"pod2", "name":"blue-net", "namespace": "default"}
]`,
}},
},
[]v1.Interface{{Name: testNetworkName1}},
[]v1.Network{{Name: testNetworkName1}},
expectToChange,
),
Entry("when vmi interfaces have an interface to hotplug and one to hot-unplug, given hashed names",
libvmi.New(
libvmi.WithInterface(v1.Interface{Name: testNetworkName1, State: v1.InterfaceStateAbsent}),
libvmi.WithInterface(v1.Interface{Name: testNetworkName2}),
libvmi.WithNetwork(&v1.Network{Name: testNetworkName1}),
libvmi.WithNetwork(&v1.Network{Name: testNetworkName2}),
withInterfaceStatus(v1.VirtualMachineInstanceNetworkInterface{Name: testNetworkName1}),
),
&k8sv1.Pod{
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{
networkv1.NetworkStatusAnnot: `[
{"interface":"pod1a2b3c", "name":"red-net", "namespace": "default"}
]`,
}},
},
[]v1.Interface{{Name: testNetworkName2}},
[]v1.Network{{Name: testNetworkName2}},
expectToChange,
),
)
})
})

func withInterfaceStatus(ifaceStatus v1.VirtualMachineInstanceNetworkInterface) libvmi.Option {
return func(vmi *v1.VirtualMachineInstance) {
vmi.Status.Interfaces = append(
vmi.Status.Interfaces, ifaceStatus,
)
}
}

0 comments on commit fa41b79

Please sign in to comment.