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

kube-ovn-controller: fix subnet update #2882

Merged
merged 1 commit into from Jun 5, 2023

Conversation

zhangzujian
Copy link
Member

@zhangzujian zhangzujian commented May 30, 2023

What type of this PR

  • Bug fixes

Which issue(s) this PR fixes:

Fixes #(issue-number)

WHAT

馃 Generated by Copilot at 4b68f69

This pull request improves the formatSubnet function and its usage in the subnet controller. It makes the function return the formatted subnet object and the error, and simplifies the code logic in pkg/controller/subnet.go.

馃 Generated by Copilot at 4b68f69

Sing, O Muse, of the skillful coder who reforged
The formatSubnet function, a mighty tool of cloud
That shapes the subnet objects, like Hephaestus molds the ore
And gives them back to callers, with error or success.

HOW

馃 Generated by Copilot at 4b68f69

  • Change the formatSubnet function to return the updated subnet object as well as the error, and update the callers to handle the new return type (link, link, link, link, link, link)
  • Return the deep copy of the updated subnet object from formatSubnet to avoid mutating the cache (link)
  • Remove the redundant code to get the subnet object from the lister in syncSubnet, and move the handleSubnetFinalizer function up to use the updated subnet object as the input (link)

@github-actions
Copy link
Contributor

  • The formatSubnet function should return a copy of the updated subnet instead of returning an error. This will make it easier to use the updated subnet in other parts of the code.
  • In the initDefaultLogicalSwitch and initNodeSwitch functions, the err variable is being shadowed by the err returned from formatSubnet. It would be clearer to use a different variable name for the return value of formatSubnet.
  • The checkSubnetChanged function could be simplified by using the reflect.DeepEqual function to compare the old and new subnets.
  • The handleAddOrUpdateSubnet function should not call c.subnetsLister.Get(key) after calling formatSubnet, as this may result in getting stale data. Instead, it should use the updated subnet returned by formatSubnet.
  • The formatSubnet function could benefit from better error handling, such as wrapping errors with more context or returning custom error types.

@hongzhen-ma hongzhen-ma mentioned this pull request May 31, 2023
1 task
@zhangzujian zhangzujian marked this pull request as ready for review June 5, 2023 02:37
@zhangzujian zhangzujian added the bug Something isn't working label Jun 5, 2023
@zhangzujian zhangzujian merged commit 6acecb6 into kubeovn:master Jun 5, 2023
51 of 57 checks passed
@zhangzujian zhangzujian deleted the fix-subnet branch June 5, 2023 05:16
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
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants