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

do not allocate MAC address when kube-ovn is called as an IPAM plugin #2816

Merged
merged 1 commit into from
May 17, 2023

Conversation

zhangzujian
Copy link
Member

@zhangzujian zhangzujian commented May 16, 2023

What type of this PR

Examples of user facing changes:

  • Features
  • Bug fixes
  • Docs
  • Tests

Which issue(s) this PR fixes:

Fixes #(issue-number)

WHAT

🤖 Generated by Copilot at 2503313

This pull request changes the type and handling of the mac address parameter in various functions and methods related to IP and MAC allocation, to use pointers to strings instead of strings. This allows for nil values to represent unassigned or unused mac addresses, and simplifies the logic and code readability. The pull request also refactors some code to handle different pod network providers more gracefully, and updates the test and benchmark code to match the new function signatures and arguments. The affected files are pkg/controller/init.go, pkg/controller/node.go, pkg/controller/pod.go, pkg/controller/vip.go, pkg/controller/vpc_nat_gw_eip.go, pkg/daemon/handler.go, pkg/ipam/ipam.go, pkg/ipam/subnet.go, test/unittest/ipam_bench/ipam_test.go, and test/unittest/ipam/ipam.go.

🤖 Generated by Copilot at 2503313

No more empty strings, no more placeholders
We use pointers to strings for our MAC addresses
We refactor the code, we simplify the logic
We handle different providers with grace and magic

HOW

🤖 Generated by Copilot at 2503313

  • Change the mac address parameter of the GetStaticAddress and GetRandomAddress methods in the Subnet struct from a string to a pointer to a string, to indicate whether a mac address was given or not, and handle the case where the pod network provider does not use mac addresses (link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link,link)
  • Remove the mac address parameter from the UpdateIPCr function in the handler and ipam packages, as it was no longer needed to update the IP CRD object (link,link,link,link,link,link)
  • Add a condition to check if the mac address annotation was empty in the pod.go and handler.go files, and set the pointer to nil if so, to handle the case where the pod network provider does not use mac addresses, such as the calico provider (link,link)
  • Conditionally add or delete the mac address, logical switch and pod nic annotations in the pod.go file, based on whether the pod network provider uses mac addresses or is ovn or not, to avoid unnecessary annotations for different providers (link)
  • Change the providerExists function in the handler.go file to return a pointer to a Subnet object along with a boolean value, instead of just a boolean value, to avoid querying the subnet lister again in the handleAdd function, and to pass the subnet object to the UpdateIPCr function (link,link,link)
  • Add a subnet name variable assignment in the handleAdd function in the handler.go file, to check if the subnet name was empty and use the pod subnet name instead, to handle the case where the pod network provider is not ovn and does not have a logical switch annotation (link)
  • Move the mac address variable assignment to the end of the handleAdd function in the handler.go file, as it was only needed to pass to the ovs.Exec function (link)
  • Add a condition to check if the mac address was empty in the GetStaticMac method in the Subnet struct in the ipam package, and return nil if so, to avoid generating a random mac address for providers that do not use mac addresses (link)
  • Change the mac address parameter of the GetStaticMac method in the Subnet struct from a string to a pointer to a string, to match the change in the GetStaticAddress function signature (link,link,link,link)
  • Change the IP address parameter of the GetRandomAddress and GetStaticAddress methods in the Subnet struct from a string to a pointer to an IP struct, to unify the IP allocation logic for both IPv4 and IPv6 addresses, and to avoid parsing strings to IPs repeatedly (link,link,link,link,link,link,link,link)
  • Assign the mac address annotation to a variable instead of directly passing it to the function in the pod.go file, to make the code more consistent and readable (link)

@github-actions
Copy link
Contributor

  • Inconsistent formatting: There are several instances where the indentation and spacing of the code is inconsistent. This can make it difficult to read and understand the code. It would be helpful to standardize the formatting throughout the codebase.
  • Potential performance issue: There is a section of code that appears to be making multiple API calls in a loop. This could potentially cause performance issues, especially if the loop is iterating over a large number of items. Consider refactoring this code to reduce the number of API calls or find a more efficient way to accomplish the same task.
  • Lack of error handling: There are several sections of code where errors are not being handled properly. For example, there are instances where an API call may fail but the code does not check for this and simply continues execution. This can lead to unexpected behavior and potential bugs. It would be beneficial to add proper error handling throughout the codebase.
  • Code duplication: There are several sections of code that appear to be duplicated in multiple places. This can make the code harder to maintain and increase the likelihood of introducing bugs. Consider refactoring these sections of code into reusable functions or modules.
  • Inefficient use of resources: There are several sections of code that appear to be using more resources than necessary. For example, there are instances where the code is making multiple API calls to retrieve the same information when it could be retrieved once and cached. Consider optimizing these sections of code to reduce resource usage and improve performance.

@zhangzujian zhangzujian marked this pull request as ready for review May 17, 2023 05:36
@zhangzujian zhangzujian requested a review from oilbeater May 17, 2023 05:36
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.

2 participants