Do not garbage collect IPs of stopped VMs using non-default multus networks

[andrewlee1089]

Do not garbage collect IPs of stopped VMs using non-default multus networks

If a kubevirt VM:

• is not using the default multus network
• is stopped
• does not have any annotation under vm.Spec.Template.ObjectMeta.Annotations[nadv1.NetworkAttachmentAnnot]

Then GC would delete the IP of this VM.

To fix, we ensure that we find the LSPs of VMs on the spec before it tries (and maybe fails) to get the LSPs defined as annotations

Pull Request

• Make sure you have followed Kube-OVN Code Style.

What type of this PR

Examples of user facing changes:

• Features
• Bug fixes
• Docs
• Tests

Which issue(s) this PR fixes

Fixes #5556

[gemini-code-assist]

Warning

Gemini encountered an error creating the summary. You can try again by commenting /gemini summary.

[coveralls]

Pull Request Test Coverage Report for Build 16744851721

Details

• 0 of 11 (0.0%) changed or added relevant lines in 1 file are covered.
• 2 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-0.005%) to 21.461%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/controller/gc.go	0	11	0.0%

Files with Coverage Reduction	New Missed Lines	%
pkg/ovs/ovn-nb-bfd.go	2	61.61%

Totals
Change from base Build 16741249923:	-0.005%
Covered Lines:	10569
Relevant Lines:	49247

---

💛 - Coveralls

[SkalaNetworks]

What is the purpose of vm.Spec.Template.ObjectMeta.Annotations[nadv1.NetworkAttachmentAnnot] in normal conditions?

Could it be used to mount networks on the VM without using the native integration of KubeVirt?

I struggle to see why someone would append Multus networks in this annotation when they could simply use the networks of the VM. Kubevirt handles creating the annotation for us.

[oilbeater]

/gemini summary

[gemini-code-assist]

Summary of Changes

I've addressed a bug where the IP addresses of stopped Kubevirt VMs were being incorrectly garbage collected. This happened when VMs were configured with non-default Multus networks but did not have a specific network attachment annotation. My change ensures that the system correctly identifies and retains the IPs for these VMs by only attempting to process network attachment annotations when they are explicitly defined.

Highlights

• Bug Fix: I've implemented a fix to prevent the premature garbage collection of IPs for stopped Kubevirt VMs. This issue occurred specifically when these VMs were using non-default Multus networks but lacked a specific network attachment annotation.
• Improved Network Attachment Annotation Handling: The logic for finding Logical Switch Ports (LSPs) from Multus network annotations now includes a check to ensure the nadv1.NetworkAttachmentAnnot annotation is present and non-empty before attempting to parse it. This prevents errors and ensures that LSPs are only collected when the relevant annotation exists.

Changelog

• pkg/controller/gc.go
  • Modified the getVMLsps function to add a conditional check for the nadv1.NetworkAttachmentAnnot annotation. Previously, the code would attempt to parse this annotation unconditionally. Now, it first verifies that the annotation string is not empty before proceeding to parse it and extract Multus network Logical Switch Ports (LSPs). This prevents errors and ensures correct LSP collection when the annotation is absent.

Activity

• A bot reported an error when attempting to create a summary.
• Coveralls reported that test coverage decreased by 0.005% to 21.461%, with 0% of the 11 changed or added relevant lines in pkg/controller/gc.go being covered.
• SkalaNetworks asked for clarification on the purpose of vm.Spec.Template.ObjectMeta.Annotations[nadv1.NetworkAttachmentAnnot] in normal conditions and questioned why Multus networks would be appended to this annotation instead of using the VM's native networks.
• oilbeater requested a new Gemini summary.

[oilbeater]

I struggle to see why someone would append Multus networks in this annotation when they could simply use the networks of the VM. Kubevirt handles creating the annotation for us.

I guess someone would like to inject other multus config like default-route through this way.