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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix IPAM allocation caused by incorrect pod annotations patch #2624

Merged
merged 1 commit into from Apr 10, 2023

Conversation

zhangzujian
Copy link
Member

@zhangzujian zhangzujian commented Apr 8, 2023

What type of this PR

  • Bug fixes

Which issue(s) this PR fixes:

Fixes #2611 introduced in #2563

WHAT

馃 Generated by Copilot at 34d6019

Improve error handling and pod caching in pod.go. Return pod object from reconcileAllocateSubnets and use it to update or recycle pod resources. Handle pod deletion cases more robustly.

馃 Generated by Copilot at 34d6019

Oh we're the brave coders of the Kubernetes sea
And we handle errors and pods with skill and glee
We reconcileAllocateSubnets and handleAddOrUpdatePod
And we heave ho, heave ho, on the count of three

HOW

馃 Generated by Copilot at 34d6019

  • Modify handleAddOrUpdatePod to handle pod object and error returned by reconcileAllocateSubnets (link)
  • Change reconcileAllocateSubnets signature to return pod object and error (link)
  • Return pod object and error in reconcileAllocateSubnets when various operations fail (link, link, link, link, link, link, link)
  • Return pod object and error in reconcileAllocateSubnets when no error occurs (link)

@zhangzujian zhangzujian added the bug Something isn't working label Apr 8, 2023
@zhangzujian zhangzujian added this to In progress in 2023-4 via automation Apr 8, 2023
@zhangzujian zhangzujian marked this pull request as ready for review April 8, 2023 07:57
@github-actions
Copy link
Contributor

github-actions bot commented Apr 8, 2023

  • The commit message should be more descriptive and informative. It should explain what changes are being made and why they are necessary.
  • In the handleAddOrUpdatePod function, the comment "check if route subnet is need" should be corrected to "check if route subnet is needed".
  • In the reconcileAllocateSubnets function, the parameters cachedPod and pod should have their types specified.
  • In the reconcileAllocateSubnets function, the return type should be specified in the function signature instead of being returned as a tuple.
  • In the reconcileAllocateSubnets function, the error messages should be more specific and informative. They should indicate which operation failed and why.
  • In the reconcileAllocateSubnets function, the patchedPod variable should be checked for nil before calling the DeepCopy() method on it to avoid a panic.

@oilbeater
Copy link
Collaborator

LGTM, but I was wondering if we should replace patch with update to avoid such conflict. Maybe there are still issues when the cache is diverge from the apiserver but the patch will still update the pod annotations.

@zhangzujian zhangzujian merged commit cfff2db into kubeovn:master Apr 10, 2023
52 of 53 checks passed
2023-4 automation moved this from In progress to Done Apr 10, 2023
@zhangzujian zhangzujian deleted the fix-pod-annotation branch April 10, 2023 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Deployment IPPool E2E failed
2 participants