-
Notifications
You must be signed in to change notification settings - Fork 32
Add support to update the loadbalancer rule when source cidr list is updated #86
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: main
Are you sure you want to change the base?
Conversation
d77cec9 to
c073027
Compare
DaanHoogland
left a comment
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.
clgtm
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.
Pull request overview
This pull request adds support for specifying source CIDR lists for CloudStack load balancers and introduces CloudStack management server version detection to enable version-specific feature gating. The main purpose is to allow updating load balancer rules when the source CIDR list changes, with the update capability depending on the CloudStack version (4.22+ supports updates, earlier versions require rule recreation).
Key changes:
- Added CloudStack management server version detection during initialization
- Introduced new annotation
service.beta.kubernetes.io/cloudstack-load-balancer-source-cidrsfor specifying allowed source CIDRs - Enhanced load balancer rule update logic to handle CIDR list changes based on CloudStack version
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| go.mod | Upgraded cloudstack-go from v2.17.1 to v2.19.0 and moved blang/semver from indirect to direct dependency |
| go.sum | Updated checksums for cloudstack-go v2.19.0 upgrade |
| cloudstack.go | Added version detection logic to query and parse CloudStack management server version on initialization |
| cloudstack_loadbalancer.go | Added CIDR list parsing/validation, updated rule comparison logic to detect CIDR changes, and implemented version-aware update vs recreate decision logic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c073027 to
d911d1d
Compare
harikrishna-patnala
left a comment
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 LGTM
d911d1d to
f1aaa2a
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #86 +/- ##
==========================================
+ Coverage 15.09% 20.21% +5.11%
==========================================
Files 4 4
Lines 808 846 +38
==========================================
+ Hits 122 171 +49
+ Misses 684 671 -13
- Partials 2 4 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This requires a new cloudstack-go SDK release for it to work with ACS 4.22+
Generated summary
This pull request introduces support for specifying source CIDRs for CloudStack load balancers via a new Kubernetes service annotation, and improves load balancer rule management to handle CIDR list changes and CloudStack version compatibility. The changes also add version detection for the CloudStack management server, which is used to determine whether certain features (like updating the CIDR list) are supported.
Load Balancer Source CIDRs and Version Handling:
service.beta.kubernetes.io/cloudstack-load-balancer-source-cidrsannotation to allow specifying a comma-separated list of source CIDRs for load balancers; defaults to allowing all sources if unspecified. (cloudstack_loadbalancer.go,ServiceAnnotationLoadBalancerSourceCidrs) [1] [2]getCIDRListhelper to parse and validate the CIDR list from the service annotation, ensuring only valid CIDRs are accepted. (cloudstack_loadbalancer.go)CloudStack Management Server Version Awareness:
semverlibrary, storing it in theCSCloudstruct for use in feature gating. (cloudstack.go) [1] [2] [3] [4]Load Balancer Rule Management Improvements:
checkLoadBalancerRuleto compare the current and desired CIDR lists, and to decide whether to update or recreate the rule based on changes and the CloudStack version (since updating the CIDR list is only supported in CloudStack 4.22+). Added a helpersetsEqualfor comparing CIDR lists. (cloudstack_loadbalancer.go)EnsureLoadBalancerand related methods to pass the service and version information, enabling proper handling of CIDR list changes and version-specific logic. (cloudstack_loadbalancer.go)Dependency Updates:
github.com/blang/semver/v4as a dependency for semantic version parsing and comparison. (cloudstack.go,cloudstack_loadbalancer.go) [1] [2]