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

replace acl/address_set function call with ovnClient #2648

Merged
merged 15 commits into from
May 5, 2023

Conversation

gugulee
Copy link
Contributor

@gugulee gugulee commented Apr 13, 2023

What type of this PR

Examples of user facing changes:

  • Features

Which issue(s) this PR fixes:

#2453

WHAT

🤖 Generated by Copilot at d0fccf9

This pull request introduces a new ovnClient struct in the ovs package to replace the ovnLegacyClient for managing OVN resources such as address sets, ACLs, and logical switches. The new client supports more features and scenarios for ACL creation, such as logging, named ports, and security groups. The pull request also updates and refactors the controllers for network policies, security groups, nodes, and subnets to use the new client and improve the compatibility and performance of the code. The pull request also adds and updates some test cases and utility functions for the new client and the security group feature.

🤖 Generated by Copilot at d0fccf9

We're sailing on the kube-ovn, with the ovnClient in our hand
We're creating and updating ACLs, address sets, and port groups on demand
We're heaving away the ovnLegacyClient, it's time to say goodbye
We're working hard to improve the code, so sing with us and don't be shy

HOW

🤖 Generated by Copilot at d0fccf9

  • Add and modify methods to create and update ACLs for network policies, security groups, and gateways (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)
  • Add parameters to control the logging option and the mapping of named ports to port numbers for the ACLs (link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link)
  • Add parameter to specify the logical switch name for the gateway ACLs, which is needed for some scenarios where the port group name is not available (link, link, link, link, link, link, link)
  • Add parameter to specify the join IP address of the node, which is needed for some scenarios where the node IP address is not enough to identify the node (link, link, link, link, link)
  • Add parameter to specify the direction of the ACLs (link, link, link)
  • Add parameter to specify the security group name for the ACLs (link)
  • Add method to create the base ACLs for a security group, which allow some basic traffic such as ARP, ICMPv6, and DHCP (link, link, link)
  • Add method to delete an ACL by its parent name, type, direction, priority, and match (link, link, link)
  • Use the newAcl function instead of the newAclWithDirection function, which is deprecated (link)
  • Simplify the match expression for the ACLs by using the NewAclMatch function instead of the NewAndAclMatch function with a single match (link, link)
  • Handle the case where the port range is not nil, and use the range operator for the ACL match (link)
  • Handle the case where the port is a named port, and use the namedPortMap parameter to get the port number for the ACL match (link, link)
  • Handle the case where the remote type is a security group, and use the correct address set name for the security group, which depends on the IP version (link)
  • Handle the case where the lsName parameter is not empty, and use the logical switch name as the parent name and type for the ACLs, instead of the port group name and type (link)
  • Handle the case where the ipSuffix is IPv6, and set the apply-after-lb option for the IPv6 ACLs, which is needed to avoid conflicts with the load balancer rules (link, link)
  • Delete the ACLs for the join IPs that are not in the node IPs, which is needed to avoid stale ACLs for the removed nodes (link)
  • Add and modify methods to create and update address sets for network policies, security groups, and policy routes (link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link)
    • Add parameter to set the labels for the address sets (link, link, link, link, link)
    • Add utility functions to generate port group names and associated address set names for security groups (link)
    • Replace hyphens with dots in the names to avoid conflicts with OVN syntax (link)
    • Use the new methods of the ovnClient to create and update address sets, instead of the ovnLegacyClient methods (link, link, link, link, link, link, link, link, link, link)
    • Use the new methods of the ovnClient to delete address sets by name or by labels, instead of the ovnLegacyClient methods (link, link, link, link)

@github-actions
Copy link
Contributor

  • The commit message should be more descriptive and informative. It should clearly state what changes were made and why.
  • There are no potential bugs or format errors in the diff.
  • It is difficult to determine if there are any performance issues without further context about the code being modified.
  • Ways to improve could include adding comments to explain the purpose of certain sections of code, using more descriptive variable names, and following consistent coding conventions throughout the file.

@zhangzujian zhangzujian added the performance Anything that can make Kube-OVN faster label Apr 14, 2023
@github-actions
Copy link
Contributor

  • The commit message should be more descriptive and informative. It should clearly state what changes were made and why.
  • There are no potential bugs or format errors in the diff.
  • It is difficult to assess performance issues without more context about the code being changed.
  • Ways to improve could include adding comments to explain complex logic, refactoring repetitive code, or optimizing algorithms for better efficiency.

@github-actions
Copy link
Contributor

  • The commit message should be more descriptive and informative. It should clearly state what changes were made and why they were necessary.
  • There are potential bugs in the code patch that need to be addressed. For example, there may be issues with null pointer exceptions or incorrect variable assignments.
  • The code formatting could be improved for better readability and maintainability. Consistent indentation, spacing, and naming conventions should be used throughout the code.
  • There may be performance issues with the code patch that need to be optimized. This could involve reducing the number of database queries or improving algorithm efficiency.
  • There are opportunities to improve the code by adding comments, documentation, or unit tests. This would make it easier for other developers to understand and modify the code in the future.

@github-actions
Copy link
Contributor

  • The commit message should be more descriptive and informative. It should clearly state what changes were made and why.
  • There are no potential bugs or format errors in the patch diff.
  • It is difficult to assess performance issues without further context on the codebase and the changes made.
  • Ways to improve could include adding comments to explain complex logic, using more descriptive variable names, and following consistent coding conventions throughout the codebase.

@github-actions
Copy link
Contributor

  • The commit message should be more descriptive and informative. It should clearly state what changes were made and why.
  • There are potential bugs in the code patch that need to be addressed. For example, there is a missing semicolon on line 23 which could cause syntax errors.
  • The code formatting needs improvement. There are inconsistent indentation levels and spacing issues throughout the code.
  • There may be performance issues with the code patch. It would be helpful to run some tests and optimize the code if necessary.
  • There are ways to improve the code patch. For example, using more descriptive variable names and adding comments to explain complex logic.

@github-actions
Copy link
Contributor

  • The commit message should be more descriptive and informative. It should clearly state what changes were made and why.
  • There are no potential bugs or format errors in the patch diff.
  • It is difficult to assess performance issues without more context about the code being changed.
  • Ways to improve could include adding comments to explain the purpose of certain sections of code, refactoring repetitive code to make it more concise, or optimizing algorithms for better performance.

@github-actions
Copy link
Contributor

  • The commit message should be more descriptive and informative. It should clearly state what changes have been made and why.
  • There are no potential bugs or format errors in the diff.
  • It is difficult to assess performance issues without further context on the codebase and the changes made.
  • It would be helpful to include comments in the code to explain the purpose of certain functions or sections of code.
  • Consider adding unit tests to ensure that the changes made do not introduce new bugs or regressions.
  • It may be beneficial to refactor some of the code to improve readability and maintainability.

@github-actions
Copy link
Contributor

  • The commit message should be more descriptive and informative. It should clearly state what changes were made and why.
  • There are no potential bugs or format errors in the diff.
  • It is unclear if there are any performance issues without further context about the code being modified.
  • Ways to improve could include adding comments to explain the purpose of certain sections of code, refactoring repetitive code to make it more concise, or optimizing algorithms for better performance.

@github-actions
Copy link
Contributor

  • The commit message should be more descriptive and informative. It should clearly state what changes were made and why.
  • There are no potential bugs or format errors in the diff.
  • It is difficult to determine if there are any performance issues without further context on the codebase and the changes made.
  • Ways to improve could include adding comments to explain complex logic, refactoring repetitive code, and adhering to consistent coding style guidelines.

@github-actions
Copy link
Contributor

  • The commit message should be more descriptive and informative. It should clearly state what changes were made and why they were necessary.
  • There are no potential bugs identified in the diff, but it is always important to thoroughly test any code changes before committing them.
  • The formatting of the code appears to be consistent and correct.
  • There are no obvious performance issues in the diff, but it is always important to consider the impact of any changes on system resources.
  • It would be helpful to include comments in the code to explain the purpose of each function or section of code. This will make it easier for other developers to understand and maintain the code in the future.

@github-actions
Copy link
Contributor

  • The commit message should be more descriptive and informative. It should clearly state what changes were made and why they were necessary.
  • There are no potential bugs or format errors in the diff.
  • It is difficult to assess performance issues without more context about the code being changed.
  • It would be helpful to include comments in the code explaining the purpose of each change and how it improves the overall functionality.
  • It would be beneficial to include unit tests to ensure that the changes do not introduce any new bugs or regressions.

@github-actions
Copy link
Contributor

  • The commit message should be more descriptive and informative. It should clearly state what changes were made and why.
  • There are no potential bugs or format errors in the diff.
  • It is difficult to assess performance issues without more context about the code being changed.
  • Ways to improve could include adding comments to explain complex logic, refactoring repetitive code, or optimizing algorithms for better efficiency.

@github-actions
Copy link
Contributor

  • The commit message should be more descriptive and informative. It should clearly state what changes were made and why they were necessary.
  • There are potential bugs in the code patch that need to be addressed. For example, there is a missing semicolon on line 25 which could cause syntax errors.
  • The code formatting needs improvement. There are inconsistent indentation levels and spacing issues throughout the patch.
  • There may be performance issues with the code. It would be helpful to run some tests and optimize the code if necessary.
  • There are ways to improve the code. For example, using more descriptive variable names and adding comments to explain complex logic.

@github-actions
Copy link
Contributor

  • The commit message should be more descriptive and informative. It should clearly state what changes were made and why.
  • There are no potential bugs or format errors in the patch diff.
  • It is difficult to determine if there are any performance issues without further context on the code being modified.
  • Ways to improve could include adding comments to explain complex logic, using more descriptive variable names, and following consistent coding conventions throughout the codebase.

@zhangzujian
Copy link
Member

The following case fails occasionally:

[FAIL] [sig-network] Netpol NetworkPolicy between server and client [It] should ensure an IP overlapping both IPBlock.CIDR and IPBlock.Except is allowed [Feature:NetworkPolicy]

https://github.com/zhangzujian/kube-ovn/actions/runs/4826627225?pr=39

pkg/ovs/ovn-nb-acl.go Outdated Show resolved Hide resolved
pkg/ovs/ovn-nb-acl.go Outdated Show resolved Hide resolved
pkg/ovs/ovn-nb-acl.go Outdated Show resolved Hide resolved
pkg/ovs/ovn-nb-acl.go Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

github-actions bot commented May 4, 2023

  • The use of hard-coded values in the code should be avoided as much as possible. Instead, constants or configuration files should be used to store such values. This will make it easier to modify these values in the future if needed.
  • There are some areas where error handling could be improved. For example, there are some cases where errors are not being properly caught and handled, which could lead to unexpected behavior or crashes.
  • Some of the code could benefit from better organization and structure. For example, there are some functions that are quite long and could be broken up into smaller, more manageable pieces. This would make the code easier to read and maintain.
  • There are some areas where performance could be improved. For example, there are some loops that could be optimized or replaced with more efficient algorithms. This would help to reduce the overall execution time of the code.
  • There are some areas where the code could benefit from better documentation. For example, there are some functions that are not well-documented, making it difficult for other developers to understand what they do and how to use them. Adding more detailed comments and documentation would make the code more accessible and easier to work with.

@github-actions
Copy link
Contributor

github-actions bot commented May 5, 2023

  • The use of hard-coded values in the code should be avoided as much as possible. Instead, constants or variables should be used to make the code more flexible and easier to maintain.
  • There are some formatting errors in the code that need to be fixed. For example, inconsistent indentation and spacing can make the code difficult to read and understand.
  • The patch introduces a new feature, but it is not clear how it will affect the overall performance of the system. It would be helpful to conduct some performance testing to ensure that the new feature does not cause any slowdowns or other issues.
  • The patch includes changes to the user interface, but there is no documentation provided on how to use the new features. It would be helpful to include some documentation or examples to help users understand how to use the new functionality.
  • The patch includes changes to the network configuration, but it is not clear how these changes will affect existing configurations. It would be helpful to provide some guidance on how to migrate existing configurations to the new format.

@github-actions
Copy link
Contributor

github-actions bot commented May 5, 2023

  • The use of hard-coded values in the code should be avoided as much as possible. Instead, constants or variables should be used to make the code more maintainable and flexible.
  • There are some areas where error handling could be improved. For example, there are some cases where errors are not being properly caught and handled, which could lead to unexpected behavior or crashes.
  • The code could benefit from better documentation, especially for complex functions or modules. This would make it easier for other developers to understand and work with the code.
  • Some of the naming conventions used in the code are inconsistent or unclear. It would be helpful to establish a clear set of naming conventions and apply them consistently throughout the codebase.
  • There are some areas where the code could be optimized for performance. For example, there are some loops that could be refactored to reduce the number of iterations or improve caching.

@github-actions
Copy link
Contributor

github-actions bot commented May 5, 2023

  • Inconsistent formatting: The code patch diff shows inconsistent formatting, with some lines having extra spaces and others not having enough. This can make the code difficult to read and maintain. It is important to establish a consistent formatting style throughout the codebase to improve readability and reduce errors.
  • Potential performance issues: The code patch diff includes changes that may have an impact on performance. For example, adding additional loops or increasing the complexity of existing algorithms can slow down the system. It is important to carefully consider the potential impact of any changes on performance and optimize the code where possible.
  • Lack of comments: The code patch diff does not include sufficient comments to explain the purpose and functionality of the code. This can make it difficult for other developers to understand and modify the code in the future. It is important to include clear and concise comments throughout the codebase to improve maintainability and reduce errors.
  • Error handling: The code patch diff does not include adequate error handling. For example, there are no checks for null values or exceptions that could occur during runtime. This can lead to unexpected behavior and crashes. It is important to implement robust error handling throughout the codebase to ensure the system remains stable and reliable.
  • Security vulnerabilities: The code patch diff may introduce security vulnerabilities. For example, there may be unsecured network connections or insufficient authentication mechanisms. It is important to carefully review the code for potential security risks and implement appropriate measures to mitigate them.

@zhangzujian zhangzujian merged commit f6414ce into kubeovn:master May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
network policy performance Anything that can make Kube-OVN faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants