-
Notifications
You must be signed in to change notification settings - Fork 494
IPPool sync to address set #5920
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
base: master
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @zbb88888, 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 IPPool functionality by enabling direct synchronization with OVN address sets. This integration allows IPPools to be seamlessly utilized for advanced networking features such as VPC policy-based routing and Access Control Lists (ACLs), providing a more dynamic and automated way to manage network policies based on defined IP ranges. The changes involve extending the IPPool API, implementing the necessary controller logic for OVN interaction, and ensuring proper resource lifecycle management through finalizers and comprehensive testing. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new feature to synchronize IPPools with OVN address sets, which is crucial for enabling policy-based routing and ACLs. The changes include adding an EnableAddressSet field to the IPPool CRD, implementing controller logic to manage OVN address sets, and providing comprehensive unit and E2E tests. The IP parsing and range expansion logic appears robust and well-tested. However, there are a couple of areas that could be improved for maintainability and robustness.
Pull Request Test Coverage Report for Build 19498592422Details
💛 - Coveralls |
Signed-off-by: zbb88888 <[email protected]>
Signed-off-by: zbb88888 <[email protected]>
Signed-off-by: zbb88888 <[email protected]>
Signed-off-by: zbb88888 <[email protected]>
Signed-off-by: zbb88888 <[email protected]>
Signed-off-by: zbb88888 <[email protected]>
Signed-off-by: zbb88888 <[email protected]>
…agement Signed-off-by: zbb88888 <[email protected]>
Signed-off-by: zbb88888 <[email protected]>
…dleAddOrUpdateIPPool Signed-off-by: zbb88888 <[email protected]>
…methods Signed-off-by: zbb88888 <[email protected]>
…tility functions for CIDR normalization Signed-off-by: zbb88888 <[email protected]>
…dling - Introduced edge case tests for ExpandIPPoolAddresses, covering scenarios like empty inputs, CIDR normalization, and complex mixed inputs. - Added error condition tests for ExpandIPPoolAddresses to handle invalid inputs and ranges. - Implemented ToCIDRs method in IPRangeList to convert IP ranges to CIDR notation, ensuring proper handling of single IPs and ranges. - Created unit tests for ToCIDRs to validate various cases including empty lists, single IPs, CIDRs, and overlapping ranges. - Enhanced the ExpandIPPoolAddresses function documentation to clarify its behavior regarding overlapping ranges. - Added tests for canonicalization and normalization functions to ensure correctness in handling IP addresses and CIDR formats. Signed-off-by: zbb88888 <[email protected]>
…ed IP family handling Signed-off-by: zbb88888 <[email protected]>
…dress set limitations and update related tests Signed-off-by: zbb88888 <[email protected]>
… add related tests Signed-off-by: zbb88888 <[email protected]>
…dge cases Signed-off-by: zbb88888 <[email protected]>
…dling in expandIPPoolAddressesInternal Signed-off-by: zbb88888 <[email protected]>
3c5f094 to
3cdd2a2
Compare
Signed-off-by: zbb88888 <[email protected]>
Signed-off-by: zbb88888 <[email protected]>
|
@oilbeater 大佬,帮忙 review 下吧,主要是想扩展 ippool 映射到 address set,这样可以和 vpc 的策略路由一起用 |
Signed-off-by: zbb88888 <[email protected]>
Signed-off-by: zbb88888 <[email protected]>
Signed-off-by: zbb88888 <[email protected]>
Signed-off-by: zbb88888 <[email protected]>
Signed-off-by: zbb88888 <[email protected]>
Signed-off-by: zbb88888 <[email protected]>
Pull Request
What type of this PR
Examples of user facing changes:
Features
IPPool support to sync its ips into address set.
Enable IPPool to work with vpc policy route or ACL
Which issue(s) this PR fixes
Fixes #(issue-number)