Skip to content

Conversation

@vishesh92
Copy link
Member

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:

  • Added support for the service.beta.kubernetes.io/cloudstack-load-balancer-source-cidrs annotation 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]
  • Introduced the getCIDRList helper 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:

  • Added detection of the CloudStack management server version using the semver library, storing it in the CSCloud struct for use in feature gating. (cloudstack.go) [1] [2] [3] [4]

Load Balancer Rule Management Improvements:

  • Enhanced checkLoadBalancerRule to 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 helper setsEqual for comparing CIDR lists. (cloudstack_loadbalancer.go)
  • Updated EnsureLoadBalancer and 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:

  • Added github.com/blang/semver/v4 as a dependency for semantic version parsing and comparison. (cloudstack.go, cloudstack_loadbalancer.go) [1] [2]

@vishesh92 vishesh92 force-pushed the update-source-cidrs branch 2 times, most recently from d77cec9 to c073027 Compare November 28, 2025 14:16
Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm

@DaanHoogland DaanHoogland added this to the 1.2.0 milestone Dec 1, 2025
@vishesh92 vishesh92 marked this pull request as ready for review December 3, 2025 06:08
Copilot AI review requested due to automatic review settings December 3, 2025 06:08
Copy link

Copilot AI left a 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-cidrs for 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.

@vishesh92 vishesh92 force-pushed the update-source-cidrs branch from c073027 to d911d1d Compare December 3, 2025 08:30
Copy link

@harikrishna-patnala harikrishna-patnala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code LGTM

@vishesh92 vishesh92 force-pushed the update-source-cidrs branch from d911d1d to f1aaa2a Compare December 9, 2025 07:44
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 70.00000% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 20.21%. Comparing base (282ef97) to head (f1aaa2a).

Files with missing lines Patch % Lines
cloudstack_loadbalancer.go 69.76% 12 Missing and 1 partial ⚠️
cloudstack.go 70.58% 5 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants