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

fix: lsp define in nb is not clear when delete vm #3429

Conversation

watermelon-brother
Copy link
Contributor

@watermelon-brother watermelon-brother commented Nov 16, 2023

What type of this PR

fix: lsp define in nb is not clear when delete vm

Which issue(s) this PR fixes

#3424

WHAT

If lsp define in nb is not clear, only gc run can clear it.

HOW

we can think the pod is vm or normal pod when add some logic.

ports, err := c.OVNNbClient.ListNormalLogicalSwitchPorts(true, map[string]string{"pod": key})
isVMPod, vmName := isVMPod(pod)
podkey := key
if isVMPod {
Copy link
Collaborator

Choose a reason for hiding this comment

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

在设置 keep-vm-ip 参数的情况下,可以使用 vm name 作为 key 查找 lsp
如果没有设置该参数,vm pod 的 lsp name 仍然为 pod name 构成。

因此需要添加 c.config.EnableKeepVMIP 参数判断

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

Copy link
Collaborator

Choose a reason for hiding this comment

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

pod ipam release 也需要修改下名字

@watermelon-brother watermelon-brother force-pushed the clear-lsp-when-delete-vm branch 2 times, most recently from b007604 to b94aa09 Compare November 17, 2023 10:03
ports, err := c.OVNNbClient.ListNormalLogicalSwitchPorts(true, map[string]string{"pod": key})
isVMPod, vmName := isVMPod(pod)
isVMPodNeedKeepIP := isVMPod && c.config.EnableKeepVMIP
podkey := key
Copy link
Collaborator

Choose a reason for hiding this comment

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

后面的 c.ipam.ReleaseAddressByPod(key) 应该也有问题

@bobz965
Copy link
Collaborator

bobz965 commented Nov 30, 2023

pr #3476 has fixed this.

@bobz965 bobz965 closed this Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants