check ovn0 mac should be set expected#5748
Conversation
Signed-off-by: clyi <clyi@alauda.io>
Summary of ChangesHello @changluyi, I'm Gemini Code Assist1! 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! This pull request enhances the robustness of the OVN daemon's network configuration by adding a crucial validation step. It ensures that the Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request adds a check to verify that the MAC address of the ovn0 interface is set as expected. However, there's a logical issue in the error handling for a MAC address mismatch that results in a misleading error message. My review includes a suggestion to correct this.
| if actualMac.String() != macAddr.String() { | ||
| macAddr = actualMac | ||
| err := fmt.Errorf("MAC address mismatch on %s: expected %s, actual %s", util.NodeNic, macAddr.String(), actualMac.String()) | ||
| klog.Error(err) | ||
| return err | ||
| } |
There was a problem hiding this comment.
The error message for the MAC address mismatch is incorrect because the macAddr variable is reassigned before the error string is formatted. This results in the 'expected' and 'actual' MAC addresses in the log message being identical, which is misleading and hinders debugging. The reassignment of macAddr is also unnecessary as the function returns immediately after.
if actualMac.String() != macAddr.String() {
err := fmt.Errorf("MAC address mismatch on %s: expected %s, actual %s", util.NodeNic, macAddr.String(), actualMac.String())
klog.Error(err)
return err
}
Pull Request
What type of this PR
Examples of user facing changes:
There's a chance that the MAC address of the ovn0 network interface card (NIC) may differ from the MAC address assigned to it by kube-ovn-controller. To check if the MAC address is incorrect, restart kube-ovn-cni.
Which issue(s) this PR fixes
Fixes #(issue-number)