Add version as a config variable and use listCapabilities to fetch the version

README.md

@@ -42,6 +42,7 @@ project-id = <CloudStack Project UUID (optional)>
 zone = <CloudStack Zone Name (optional)>
 region = <Region Name (optional)>
 ssl-no-verify = <Disable SSL certificate validation: true or false (optional)>
+version = <CloudStack version, e.g. 4.21.0.0 (optional)>
 ```
 
 If `zone` is not set, it is auto-detected from the node the controller runs on.
@@ -50,12 +51,10 @@ If `zone` is not set, it is auto-detected from the node the controller runs on.
 name. Some workloads (such as Rook/Ceph) require the zone and region labels to differ. You need to
 explicitly set `region` in that case.
 
-The access token needs to be able to fetch VM information and deploy load balancers in the project or domain where the nodes reside.
+`version` is normally detected automatically using the `listCapabilities` API. Set it to pin the version manually,
+for example when the API user is not allowed to call `listCapabilities`.
 
-The account must also be allowed to call `listManagementServersMetrics`, which the controller uses
-on startup to determine the management server version. This is a root admin API and is **not**
-included in the default `User` role; without it the controller exits immediately with
-`no management servers found`.
+The access token needs to be able to fetch VM information and deploy load balancers in the project or domain where the nodes reside.
 
 To create the secret, use the following command:
 ```bash

cloudstack.go

@@ -55,6 +55,9 @@ type CSConfig struct {
 		ProjectID   string `gcfg:"project-id"`
 		Zone        string `gcfg:"zone"`
 		Region      string `gcfg:"region"`
+		// Version overrides the CloudStack version that is otherwise
+		// detected via the listCapabilities API.
+		Version string `gcfg:"version"`
 	}
 }
 
@@ -110,7 +113,18 @@ func newCSCloud(cfg *CSConfig) (*CSCloud, error) {
 		return nil, errors.New("no cloud provider config given")
 	}
 
-	version, err := cs.getManagementServerVersion()
+	if cfg.Global.Version != "" {
+		version, err := parseCloudStackVersion(cfg.Global.Version)
+		if err != nil {
+			return nil, fmt.Errorf("could not parse the version given in the cloud provider config: %v", err)
+		}
+		klog.V(2).Infof("Using CloudStack version %v from the cloud provider config", version)
+		cs.version = version
+
+		return cs, nil
+	}
+
+	version, err := cs.getCloudStackVersion()
 	if err != nil {
 		return nil, err
 	}
@@ -119,20 +133,33 @@ func newCSCloud(cfg *CSConfig) (*CSCloud, error) {
 	return cs, nil
 }
 
-func (cs *CSCloud) getManagementServerVersion() (semver.Version, error) {
-	msServersResp, err := cs.client.Management.ListManagementServersMetrics(cs.client.Management.NewListManagementServersMetricsParams())
+// getCloudStackVersion returns the version of the CloudStack management server,
+// as reported by the listCapabilities API.
+func (cs *CSCloud) getCloudStackVersion() (semver.Version, error) {
+	capabilitiesResp, err := cs.client.Configuration.ListCapabilities(cs.client.Configuration.NewListCapabilitiesParams())
 	if err != nil {
 		return semver.Version{}, err
 	}
-	if msServersResp.Count == 0 {
-		return semver.Version{}, errors.New("no management servers found")
+	if capabilitiesResp.Capabilities == nil || capabilitiesResp.Capabilities.Cloudstackversion == "" {
+		return semver.Version{}, errors.New("no CloudStack version returned by the management server")
 	}
-	version := msServersResp.ManagementServersMetrics[0].Version
+
+	v, err := parseCloudStackVersion(capabilitiesResp.Capabilities.Cloudstackversion)
+	if err != nil {
+		klog.Error(err)
+		return semver.Version{}, err
+	}
+	return v, nil
+}
+
+// parseCloudStackVersion parses a CloudStack version such as "4.17.1.0" or
+// "4.17.1.0-SNAPSHOT" into a semver version, discarding everything after the
+// patch level.
+func parseCloudStackVersion(version string) (semver.Version, error) {
 	parts := strings.Split(version, ".")
 	v, err := semver.ParseTolerant(strings.Join(parts[:min(len(parts), 3)], "."))
 	if err != nil {
-		klog.Errorf("failed to parse management server version: %v", err)
-		return semver.Version{}, err
+		return semver.Version{}, fmt.Errorf("failed to parse CloudStack version %q: %v", version, err)
 	}
 	return v, nil
 }

cloudstack_loadbalancer_test.go

@@ -680,6 +680,57 @@ func TestCheckLoadBalancerRule(t *testing.T) {
 		}
 	})
 
+	// CloudStack moves from 4.x to 24.0 after 4.23, so the 4.22 feature gate has
+	// to keep treating the new numbering as newer rather than older.
+	t.Run("cidr change triggers update on the 24.0 series", func(t *testing.T) {
+		ctrl := gomock.NewController(t)
+		t.Cleanup(ctrl.Finish)
+
+		// No expectations on the mock; any delete call would fail the test.
+		mockLB := cloudstack.NewMockLoadBalancerServiceIface(ctrl)
+
+		lbRule := &cloudstack.LoadBalancerRule{
+			Id:          "rule-id",
+			Name:        "rule",
+			Publicip:    "1.1.1.1",
+			Privateport: "30000",
+			Publicport:  "80",
+			Cidrlist:    "10.0.0.0/8",
+			Algorithm:   "roundrobin",
+			Protocol:    LoadBalancerProtocolTCP.CSProtocol(),
+		}
+
+		lb := &loadBalancer{
+			CloudStackClient: &cloudstack.CloudStackClient{
+				LoadBalancer: mockLB,
+			},
+			ipAddr:    "1.1.1.1",
+			algorithm: "roundrobin",
+			rules: map[string]*cloudstack.LoadBalancerRule{
+				"rule": lbRule,
+			},
+		}
+		port := corev1.ServicePort{Port: 80, NodePort: 30000, Protocol: corev1.ProtocolTCP}
+		service := &corev1.Service{
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations: map[string]string{
+					ServiceAnnotationLoadBalancerSourceCidrs: "10.0.0.0/8,192.168.0.0/16",
+				},
+			},
+		}
+
+		rule, needsUpdate, err := lb.checkLoadBalancerRule("rule", port, LoadBalancerProtocolTCP, service, semver.MustParse("24.0.0"))
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if rule != lbRule {
+			t.Fatalf("expected existing rule to be returned")
+		}
+		if !needsUpdate {
+			t.Fatalf("expected needsUpdate to be true due to CIDR change")
+		}
+	})
+
 	t.Run("cidr change triggers delete with older version", func(t *testing.T) {
 		ctrl := gomock.NewController(t)
 		t.Cleanup(ctrl.Finish)
@@ -2053,6 +2104,56 @@ func TestUpdateLoadBalancerRule(t *testing.T) {
 		}
 	})
 
+	// The release after 4.23 is numbered 24.0, which must still reach the
+	// in-place CIDR update rather than falling back to delete-and-recreate.
+	t.Run("update CIDR list on the 24.0 series", func(t *testing.T) {
+		ctrl := gomock.NewController(t)
+		t.Cleanup(ctrl.Finish)
+
+		mockLB := cloudstack.NewMockLoadBalancerServiceIface(ctrl)
+		updateParams := &cloudstack.UpdateLoadBalancerRuleParams{}
+
+		gomock.InOrder(
+			mockLB.EXPECT().NewUpdateLoadBalancerRuleParams("rule-123").Return(updateParams),
+			mockLB.EXPECT().UpdateLoadBalancerRule(gomock.Any()).Return(&cloudstack.UpdateLoadBalancerRuleResponse{}, nil),
+		)
+
+		lb := &loadBalancer{
+			CloudStackClient: &cloudstack.CloudStackClient{
+				LoadBalancer: mockLB,
+			},
+			algorithm: "roundrobin",
+			rules: map[string]*cloudstack.LoadBalancerRule{
+				"test-rule-tcp-80": {
+					Id:        "rule-123",
+					Algorithm: "roundrobin",
+					Protocol:  "tcp",
+					Cidrlist:  defaultAllowedCIDR,
+				},
+			},
+		}
+
+		service := &corev1.Service{
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations: map[string]string{
+					ServiceAnnotationLoadBalancerSourceCidrs: "10.0.0.0/8",
+				},
+			},
+		}
+
+		if err := lb.updateLoadBalancerRule("test-rule-tcp-80", LoadBalancerProtocolTCP, service, semver.MustParse("24.0.0")); err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+
+		cidrList, ok := updateParams.GetCidrlist()
+		if !ok {
+			t.Fatalf("expected the CIDR list to be set on the update params")
+		}
+		if len(cidrList) != 1 || cidrList[0] != "10.0.0.0/8" {
+			t.Fatalf("cidrlist = %v, want [10.0.0.0/8]", cidrList)
+		}
+	})
+
 	t.Run("error updating rule", func(t *testing.T) {
 		ctrl := gomock.NewController(t)
 		t.Cleanup(ctrl.Finish)