fix: clean up garbage lsp

[changluyi]

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 #(issue-number)

复现步骤：

17:09:52 第一个kube-ovn-controller 分配node-x.x.x.x 的ip 和 mac

由于环境上和 api-server 的连接出现问题，导致patch失败，所以logical_switch_port 创建完成， 但是annotation并没有配置好

17:10:14

kube-ovn-controller 发生了选举另外一个kube-ovn-controller上了。

17:10:18 重新分配join 的ip 和 mac

但是由于之前已经创建了lsp ,导致不会再创建，所以lsp仍然是之前的

annoation 反而更新成新分配的mac

[gemini-code-assist]

Hello @changluyi, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request aims to clean up potentially leftover logical switch ports (LSPs) in OVN. Specifically, it adds a step to delete any existing LSP associated with a node when the node is being added, but only if the node already has the AllocatedAnnotation, IPAddressAnnotation, and MacAddressAnnotation annotations. This prevents issues arising from previous failed configurations where LSPs might not have been properly removed.

Highlights

• LSP Cleanup: The primary change is the addition of a DeleteLogicalSwitchPort call within the handleAddNode function in pkg/controller/node.go. This deletion occurs before potentially allocating new IP addresses for the node.
• Conditional Deletion: The LSP deletion is conditional, only executing if the node already has the AllocatedAnnotation, IPAddressAnnotation, and MacAddressAnnotation annotations set to avoid deleting valid ports.

Changelog

• pkg/controller/node.go
  • Added a call to OVNNbClient.DeleteLogicalSwitchPort(portName) to delete any existing logical switch port associated with the node.
  • The deletion is performed only when the node has already been allocated an IP address and MAC address, indicated by the presence of specific annotations.
  • The deletion happens before IP address allocation to avoid conflicts with potentially existing configurations.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

A switch port's life,
Fleeting, until cleanup's strife,
Then it's gone away.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

The pull request aims to clean up garbage LSP (Logical Switch Port) in the kube-ovn controller. The changes involve adding a cleanup step to delete potentially existing logical switch ports when allocating new IP addresses to a node. This helps to avoid leftover issues from previous failed configurations. The changes look reasonable, but there are a few points to consider.

Summary of Findings

• Potential race condition: There is a potential race condition where the LSP might be deleted while another process is still using it. Consider adding a check to ensure that the LSP is not in use before deleting it.
• Error handling: The error message for deleting the logical switch port is logged as a warning. Consider logging it as an error and retrying the deletion if it fails.

Merge Readiness

The pull request introduces a cleanup mechanism for logical switch ports, which is a valuable addition. However, before merging, it's crucial to address the potential race condition and enhance the error handling for LSP deletion. I am unable to approve this pull request, and recommend that it not be merged until the aforementioned issues are addressed. Please have others review and approve this code before merging.

[patriziobassi]

@changluyi can you please consider backporting to 1.12 branch too?

we have a issue where statefulset sometimes cannot be re-deployed because the old lsp was not removed and i need to do it with del-lsp command manually.