This is an automated email from the ASF dual-hosted git repository.
vishesh92 pushed a commit to branch main
in repository
https://gitbox.apache.org/repos/asf/cloudstack-kubernetes-provider.git
The following commit(s) were added to refs/heads/main by this push:
new 5147f764 Add version as a config variable and use listCapabilities to
fetch the version (#101)
5147f764 is described below
commit 5147f76478f66191513d8741ffd8800a35684fc2
Author: Vishesh <[email protected]>
AuthorDate: Mon Sep 21 17:50:30 2026 +0530
Add version as a config variable and use listCapabilities to fetch the
version (#101)
---
README.md | 9 +-
cloudstack.go | 43 +++++--
cloudstack_loadbalancer_test.go | 101 ++++++++++++++++
cloudstack_test.go | 257 ++++++++++++++++++++--------------------
docs/development.md | 6 +-
5 files changed, 272 insertions(+), 144 deletions(-)
diff --git a/README.md b/README.md
index 9d2d93a8..316e8e33 100644
--- a/README.md
+++ b/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
diff --git a/cloudstack.go b/cloudstack.go
index e7224207..c0dfe338 100644
--- a/cloudstack.go
+++ b/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
}
diff --git a/cloudstack_loadbalancer_test.go b/cloudstack_loadbalancer_test.go
index 933f7685..1eba6e34 100644
--- a/cloudstack_loadbalancer_test.go
+++ b/cloudstack_loadbalancer_test.go
@@ -705,6 +705,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)
@@ -2078,6 +2129,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)
diff --git a/cloudstack_test.go b/cloudstack_test.go
index 8817e043..1bc5cc35 100644
--- a/cloudstack_test.go
+++ b/cloudstack_test.go
@@ -66,6 +66,45 @@ func TestReadConfig(t *testing.T) {
if !cfg.Global.SSLNoVerify {
t.Errorf("incorrect ssl-no-verify: %t", cfg.Global.SSLNoVerify)
}
+ if cfg.Global.Version != "" {
+ t.Errorf("version should be empty when not configured: %s",
cfg.Global.Version)
+ }
+
+ cfg, err = readConfig(strings.NewReader(`
+ [Global]
+ api-url = https://cloudstack.url
+ version = 4.21.0.0
+ `))
+ if err != nil {
+ t.Fatalf("Should succeed when a valid config is provided: %v",
err)
+ }
+ if cfg.Global.Version != "4.21.0.0" {
+ t.Errorf("incorrect version: %s", cfg.Global.Version)
+ }
+}
+
+func TestNewCSCloudWithVersionFromConfig(t *testing.T) {
+ cfg := &CSConfig{}
+ cfg.Global.APIURL = "https://cloudstack.url/client/api";
+ cfg.Global.APIKey = "a-valid-api-key"
+ cfg.Global.SecretKey = "a-valid-secret-key"
+ cfg.Global.Version = "4.21.0.0"
+
+ // The version from the config is used as-is, so no API call is made.
+ cs, err := newCSCloud(cfg)
+ if err != nil {
+ t.Fatalf("unexpected error: %v", err)
+ }
+
+ expected := semver.MustParse("4.21.0")
+ if !cs.version.Equals(expected) {
+ t.Fatalf("version = %v, want %v", cs.version, expected)
+ }
+
+ cfg.Global.Version = "not-a-version"
+ if _, err := newCSCloud(cfg); err == nil {
+ t.Fatalf("expected error for an invalid version in the config")
+ }
}
// This allows acceptance testing against an existing CloudStack environment.
@@ -124,32 +163,36 @@ func TestLoadBalancer(t *testing.T) {
}
}
-func TestGetManagementServerVersion(t *testing.T) {
- t.Run("returns parsed version", func(t *testing.T) {
- ctrl := gomock.NewController(t)
- t.Cleanup(ctrl.Finish)
-
- mockMgmt := cloudstack.NewMockManagementServiceIface(ctrl)
- params := &cloudstack.ListManagementServersMetricsParams{}
- resp := &cloudstack.ListManagementServersMetricsResponse{
- Count: 1,
- ManagementServersMetrics:
[]*cloudstack.ManagementServersMetric{
- {Version: "4.17.1.0"},
- },
- }
+// newCSCloudWithCapabilities returns a CSCloud whose Configuration service is
mocked to
+// answer a single listCapabilities call with the given response and error.
+func newCSCloudWithCapabilities(t *testing.T, resp
*cloudstack.ListCapabilitiesResponse, err error) *CSCloud {
+ t.Helper()
- gomock.InOrder(
-
mockMgmt.EXPECT().NewListManagementServersMetricsParams().Return(params),
-
mockMgmt.EXPECT().ListManagementServersMetrics(params).Return(resp, nil),
- )
+ ctrl := gomock.NewController(t)
+ t.Cleanup(ctrl.Finish)
- cs := &CSCloud{
- client: &cloudstack.CloudStackClient{
- Management: mockMgmt,
- },
- }
+ mockConfig := cloudstack.NewMockConfigurationServiceIface(ctrl)
+ params := &cloudstack.ListCapabilitiesParams{}
+
+ gomock.InOrder(
+ mockConfig.EXPECT().NewListCapabilitiesParams().Return(params),
+ mockConfig.EXPECT().ListCapabilities(params).Return(resp, err),
+ )
+
+ return &CSCloud{
+ client: &cloudstack.CloudStackClient{
+ Configuration: mockConfig,
+ },
+ }
+}
+
+func TestGetCloudStackVersion(t *testing.T) {
+ t.Run("returns parsed version", func(t *testing.T) {
+ cs := newCSCloudWithCapabilities(t,
&cloudstack.ListCapabilitiesResponse{
+ Capabilities: &cloudstack.Capability{Cloudstackversion:
"4.17.1.0"},
+ }, nil)
- version, err := cs.getManagementServerVersion()
+ version, err := cs.getCloudStackVersion()
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -161,30 +204,11 @@ func TestGetManagementServerVersion(t *testing.T) {
})
t.Run("returns correct parsed version with development server", func(t
*testing.T) {
- ctrl := gomock.NewController(t)
- t.Cleanup(ctrl.Finish)
-
- mockMgmt := cloudstack.NewMockManagementServiceIface(ctrl)
- params := &cloudstack.ListManagementServersMetricsParams{}
- resp := &cloudstack.ListManagementServersMetricsResponse{
- Count: 1,
- ManagementServersMetrics:
[]*cloudstack.ManagementServersMetric{
- {Version: "4.17.1.0-SNAPSHOT"},
- },
- }
-
- gomock.InOrder(
-
mockMgmt.EXPECT().NewListManagementServersMetricsParams().Return(params),
-
mockMgmt.EXPECT().ListManagementServersMetrics(params).Return(resp, nil),
- )
-
- cs := &CSCloud{
- client: &cloudstack.CloudStackClient{
- Management: mockMgmt,
- },
- }
+ cs := newCSCloudWithCapabilities(t,
&cloudstack.ListCapabilitiesResponse{
+ Capabilities: &cloudstack.Capability{Cloudstackversion:
"4.17.1.0-SNAPSHOT"},
+ }, nil)
- version, err := cs.getManagementServerVersion()
+ version, err := cs.getCloudStackVersion()
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -208,28 +232,11 @@ func TestGetManagementServerVersion(t *testing.T) {
{"24.0.0.0-SNAPSHOT", semver.MustParse("24.0.0")},
} {
t.Run(tc.version, func(t *testing.T) {
- ctrl := gomock.NewController(t)
- t.Cleanup(ctrl.Finish)
-
- mockMgmt :=
cloudstack.NewMockManagementServiceIface(ctrl)
- params :=
&cloudstack.ListManagementServersMetricsParams{}
- resp :=
&cloudstack.ListManagementServersMetricsResponse{
- Count: 1,
- ManagementServersMetrics:
[]*cloudstack.ManagementServersMetric{
- {Version: tc.version},
- },
- }
-
- gomock.InOrder(
-
mockMgmt.EXPECT().NewListManagementServersMetricsParams().Return(params),
-
mockMgmt.EXPECT().ListManagementServersMetrics(params).Return(resp, nil),
- )
-
- cs := &CSCloud{
- client:
&cloudstack.CloudStackClient{Management: mockMgmt},
- }
+ cs := newCSCloudWithCapabilities(t,
&cloudstack.ListCapabilitiesResponse{
+ Capabilities:
&cloudstack.Capability{Cloudstackversion: tc.version},
+ }, nil)
- version, err := cs.getManagementServerVersion()
+ version, err := cs.getCloudStackVersion()
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -241,86 +248,80 @@ func TestGetManagementServerVersion(t *testing.T) {
})
t.Run("returns error when api call fails", func(t *testing.T) {
- ctrl := gomock.NewController(t)
- t.Cleanup(ctrl.Finish)
-
- mockMgmt := cloudstack.NewMockManagementServiceIface(ctrl)
- params := &cloudstack.ListManagementServersMetricsParams{}
- apiErr := errors.New("api failure")
-
- gomock.InOrder(
-
mockMgmt.EXPECT().NewListManagementServersMetricsParams().Return(params),
-
mockMgmt.EXPECT().ListManagementServersMetrics(params).Return(nil, apiErr),
- )
-
- cs := &CSCloud{
- client: &cloudstack.CloudStackClient{
- Management: mockMgmt,
- },
- }
+ cs := newCSCloudWithCapabilities(t, nil, errors.New("api
failure"))
- if _, err := cs.getManagementServerVersion(); err == nil {
+ if _, err := cs.getCloudStackVersion(); err == nil {
t.Fatalf("expected error, got nil")
}
})
- t.Run("returns error when no servers found", func(t *testing.T) {
- ctrl := gomock.NewController(t)
- t.Cleanup(ctrl.Finish)
+ t.Run("returns error when no capabilities returned", func(t *testing.T)
{
+ cs := newCSCloudWithCapabilities(t,
&cloudstack.ListCapabilitiesResponse{}, nil)
- mockMgmt := cloudstack.NewMockManagementServiceIface(ctrl)
- params := &cloudstack.ListManagementServersMetricsParams{}
- resp := &cloudstack.ListManagementServersMetricsResponse{
- Count: 0,
- ManagementServersMetrics:
[]*cloudstack.ManagementServersMetric{},
+ if _, err := cs.getCloudStackVersion(); err == nil {
+ t.Fatalf("expected error for missing capabilities")
}
+ })
- gomock.InOrder(
-
mockMgmt.EXPECT().NewListManagementServersMetricsParams().Return(params),
-
mockMgmt.EXPECT().ListManagementServersMetrics(params).Return(resp, nil),
- )
-
- cs := &CSCloud{
- client: &cloudstack.CloudStackClient{
- Management: mockMgmt,
- },
- }
+ t.Run("returns error when version is empty", func(t *testing.T) {
+ cs := newCSCloudWithCapabilities(t,
&cloudstack.ListCapabilitiesResponse{
+ Capabilities: &cloudstack.Capability{},
+ }, nil)
- if _, err := cs.getManagementServerVersion(); err == nil {
- t.Fatalf("expected error for zero management servers")
+ if _, err := cs.getCloudStackVersion(); err == nil {
+ t.Fatalf("expected error for empty version")
}
})
t.Run("returns error when version cannot be parsed", func(t *testing.T)
{
- ctrl := gomock.NewController(t)
- t.Cleanup(ctrl.Finish)
-
- mockMgmt := cloudstack.NewMockManagementServiceIface(ctrl)
- params := &cloudstack.ListManagementServersMetricsParams{}
- resp := &cloudstack.ListManagementServersMetricsResponse{
- Count: 1,
- ManagementServersMetrics:
[]*cloudstack.ManagementServersMetric{
- {Version: "invalid.version.string"},
- },
- }
-
- gomock.InOrder(
-
mockMgmt.EXPECT().NewListManagementServersMetricsParams().Return(params),
-
mockMgmt.EXPECT().ListManagementServersMetrics(params).Return(resp, nil),
- )
+ cs := newCSCloudWithCapabilities(t,
&cloudstack.ListCapabilitiesResponse{
+ Capabilities: &cloudstack.Capability{Cloudstackversion:
"invalid.version.string"},
+ }, nil)
- cs := &CSCloud{
- client: &cloudstack.CloudStackClient{
- Management: mockMgmt,
- },
- }
-
- if _, err := cs.getManagementServerVersion(); err == nil {
+ if _, err := cs.getCloudStackVersion(); err == nil {
t.Fatalf("expected parse error")
}
})
}
+func TestParseCloudStackVersion(t *testing.T) {
+ tests := []struct {
+ version string
+ want string
+ wantErr bool
+ }{
+ {version: "4.17.1.0", want: "4.17.1"},
+ {version: "4.17.1.0-SNAPSHOT", want: "4.17.1"},
+ {version: "4.21.0", want: "4.21.0"},
+ {version: "4.21", want: "4.21.0"},
+ {version: "4", want: "4.0.0"},
+ {version: "4.23.0.0", want: "4.23.0"},
+ {version: "24.0.0.0", want: "24.0.0"},
+ {version: "24.0.0.0-SNAPSHOT", want: "24.0.0"},
+ {version: "24.0", want: "24.0.0"},
+ {version: "invalid.version.string", wantErr: true},
+ {version: "", wantErr: true},
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.version, func(t *testing.T) {
+ got, err := parseCloudStackVersion(tt.version)
+ if tt.wantErr {
+ if err == nil {
+ t.Fatalf("expected error for %q, got
%v", tt.version, got)
+ }
+ return
+ }
+ if err != nil {
+ t.Fatalf("unexpected error: %v", err)
+ }
+ if !got.Equals(semver.MustParse(tt.want)) {
+ t.Fatalf("version = %v, want %v", got, tt.want)
+ }
+ })
+ }
+}
+
func TestGetRegionFromZone(t *testing.T) {
tests := []struct {
name string
diff --git a/docs/development.md b/docs/development.md
index 73f3d7d5..28757dd1 100644
--- a/docs/development.md
+++ b/docs/development.md
@@ -146,8 +146,8 @@ The upstream simulator README suggests `-p 8080:5050`,
which publishes the
Readiness is checked in three stages rather than with a fixed sleep: jetty
answering at all, then the API accepting admin credentials, then
`listManagementServersMetrics` returning a server. The last one matters
-because the CCM makes exactly that call on startup and refuses to run until it
-succeeds.
+because it proves a management server is registered and running, not just that
+the API answers.
The zone is then deployed with marvin, which is preinstalled in the image:
@@ -414,7 +414,7 @@ so a run is reproducible; avoid floating tags like `latest`.
| Symptom | Cause |
| --- | --- |
| `LB service provider cannot support this rule` on a VPC | The VPC virtual
router accepts only a restricted set of public load balancer ports. 80 and 8080
work; an arbitrary high port such as 8081 is rejected. Pick a port the router
supports when adding a VPC test. |
-| CCM exits with `no management servers found` | The account cannot call
`listManagementServersMetrics`. This is a root-admin API; the default `User`
role does not include it. |
+| CCM exits with `no CloudStack version returned by the management server` |
`listCapabilities` answered without a `cloudstackversion`. Set `version` in the
`cloud-config` to pin it and skip the lookup. |
| Nodes keep the uninitialized taint; CCM logs `provided node ip for node
"..." is not valid` | The CloudStack VM's NIC IP does not match the IP kubelet
registered with. Recreate the VM with `ipaddress=` set to the kind node's
docker IP. |
| Services stay `<pending>`; CCM logs `none of the hosts matched the list of
VMs retrieved from CS API` | No CloudStack VM has a name matching a Kubernetes
node name. |
| CCM logs `found hosts that belong to different networks` | VMs matching the
node names exist on more than one network — typically leftovers from a previous
scenario. |
