Skip to content

Commit 8e5cc5b

Browse files
dkoshkindkoshkin
authored
fix: cluster-autoscaler Helm values for workload clusters (#658)
**What problem does this PR solve?**: Fixes Cluster Autoscaler templating for workload clusters. `.Cluster.metadata.name` will always be the management Cluster, so deploying a workload cluster resulted in wrong helm values. See example error. ``` - lastTransitionTime: "2024-05-14T01:09:26Z" message: 'Unable to continue with install: PodDisruptionBudget "cluster-autoscaler-dkoshkin-ca-test-1" in namespace "default" exists and cannot be imported into the current release: invalid ownership metadata; annotation validation error: key "meta.helm.sh/release-name" must equal "cluster-autoscaler-dkoshkin-ca-test-wrkld-1": current value is "cluster-autoscaler-dkoshkin-ca-test-1"' ``` **Which issue(s) this PR fixes**: Fixes # **How Has This Been Tested?**: <!-- Please describe the tests that you ran to verify your changes. Provide output from the tests and any manual steps needed to replicate the tests. --> See the `values` getting templated: ``` $ kubectl get helmreleaseproxies.addons.cluster.x-k8s.io cluster-autoscaler-dkoshkin-ca-fix-1-mtl2b -o yaml apiVersion: addons.cluster.x-k8s.io/v1alpha1 kind: HelmReleaseProxy metadata: creationTimestamp: "2024-05-14T04:13:59Z" finalizers: - helmreleaseproxy.addons.cluster.x-k8s.io generateName: cluster-autoscaler-dkoshkin-ca-fix-1- generation: 1 labels: cluster.x-k8s.io/cluster-name: dkoshkin-ca-fix-1 helmreleaseproxy.addons.cluster.x-k8s.io/helmchartproxy-name: cluster-autoscaler-dkoshkin-ca-fix-1 name: cluster-autoscaler-dkoshkin-ca-fix-1-mtl2b namespace: default ownerReferences: - apiVersion: addons.cluster.x-k8s.io/v1alpha1 blockOwnerDeletion: true controller: true kind: HelmChartProxy name: cluster-autoscaler-dkoshkin-ca-fix-1 uid: 064278fb-2ce5-4fd5-897c-31c65e8eb010 resourceVersion: "2348" uid: 0f64c718-bd06-44cb-b704-935a56b29e83 spec: chartName: cluster-autoscaler clusterRef: apiVersion: cluster.x-k8s.io/v1beta1 kind: Cluster name: dkoshkin-ca-fix-1 namespace: default namespace: default options: enableClientCache: false install: createNamespace: true timeout: 10m0s upgrade: maxHistory: 10 releaseName: cluster-autoscaler-dkoshkin-ca-fix-1 repoURL: https://kubernetes.github.io/autoscaler values: |- --- fullnameOverride: "cluster-autoscaler-dkoshkin-ca-fix-1" ... ``` **Special notes for your reviewer**: <!-- Use this to provide any additional information to the reviewers. This may include: - Best way to review the PR. - Where the author wants the most review attention on. - etc. -->
1 parent d205864 commit 8e5cc5b

File tree

5 files changed

+316
-11
lines changed

5 files changed

+316
-11
lines changed

‎charts/cluster-api-runtime-extensions-nutanix/templates/cluster-autoscaler/manifests/helm-addon-installation.yaml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ metadata:
99
data:
1010
values.yaml: |-
1111
---
12-
fullnameOverride: "cluster-autoscaler-{{ `{{ .Cluster.metadata.name }}` }}"
12+
fullnameOverride: "cluster-autoscaler-{{ `{{ .Cluster.Name }}` }}"
1313
1414
cloudProvider: clusterapi
1515
@@ -24,19 +24,19 @@ data:
2424
2525
# Limit a single cluster-autoscaler Deployment to a single Cluster.
2626
autoDiscovery:
27-
clusterName: "{{ `{{ .Cluster.metadata.name }}` }}"
27+
clusterName: "{{ `{{ .Cluster.Name }}` }}"
2828
# The controller failed with an RBAC error trying to watch CAPI objects at the cluster scope without this.
2929
labels:
30-
- namespace: "{{ `{{ .Cluster.metadata.namespace }}` }}"
30+
- namespace: "{{ `{{ .Cluster.Namespace }}` }}"
3131
32-
clusterAPIConfigMapsNamespace: "{{ `{{ .Cluster.metadata.namespace }}` }}"
32+
clusterAPIConfigMapsNamespace: "{{ `{{ .Cluster.Namespace }}` }}"
3333
# For workload clusters it is not possible to use the in-cluster client.
3434
# To simplify the configuration, use the admin kubeconfig generated by CAPI for all clusters.
3535
clusterAPIMode: kubeconfig-incluster
3636
clusterAPIWorkloadKubeconfigPath: /cluster/kubeconfig
3737
extraVolumeSecrets:
3838
kubeconfig:
39-
name: "{{ `{{ .Cluster.metadata.name }}` }}-kubeconfig"
39+
name: "{{ `{{ .Cluster.Name }}` }}-kubeconfig"
4040
mountPath: /cluster
4141
readOnly: true
4242
items:

‎pkg/handlers/generic/lifecycle/clusterautoscaler/strategy_crs.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func (s crsStrategy) apply(
6868

6969
cluster := &req.Cluster
7070

71-
data := templateData(defaultCM.Data, cluster.Name, cluster.Namespace)
71+
data := templateData(cluster, defaultCM.Data)
7272
cm := &corev1.ConfigMap{
7373
TypeMeta: metav1.TypeMeta{
7474
APIVersion: corev1.SchemeGroupVersion.String(),

‎pkg/handlers/generic/lifecycle/clusterautoscaler/strategy_helmaddon.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,14 @@ func (s helmAddonStrategy) apply(
7373
return err
7474
}
7575

76+
// Cannot rely directly on Cluster.metadata.Name and Cluster.metadata.Namespace values
77+
// because the selected Cluster will always be the management cluster.
78+
// By templating the values, we will have unique Deployment name for each cluster.
79+
values, err = templateValues(&req.Cluster, values)
80+
if err != nil {
81+
return fmt.Errorf("failed to template Helm values read from ConfigMap: %w", err)
82+
}
83+
7684
hcp := &caaphv1.HelmChartProxy{
7785
TypeMeta: metav1.TypeMeta{
7886
APIVersion: caaphv1.GroupVersion.String(),

‎pkg/handlers/generic/lifecycle/clusterautoscaler/template.go

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,50 @@
33

44
package clusterautoscaler
55

6-
import "strings"
6+
import (
7+
"bytes"
8+
"fmt"
9+
"strings"
10+
"text/template"
11+
12+
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
13+
)
714

815
const (
916
nameTemplate = "tmpl-clustername-tmpl"
1017
namespaceTemplate = "tmpl-clusternamespace-tmpl"
1118
)
1219

13-
// templateData replaces templates 'tmpl-clustername-tmpl' and 'tmpl-clusternamespace-tmpl' in data
14-
// with clusterName and clusterNamespace.
15-
func templateData(data map[string]string, clusterName, clusterNamespace string) map[string]string {
20+
// templateData replaces 'tmpl-clustername-tmpl' and 'tmpl-clusternamespace-tmpl' in data.
21+
func templateData(cluster *clusterv1.Cluster, data map[string]string) map[string]string {
1622
templated := make(map[string]string, len(data))
1723
for k, v := range data {
18-
r := strings.NewReplacer(nameTemplate, clusterName, namespaceTemplate, clusterNamespace)
24+
r := strings.NewReplacer(nameTemplate, cluster.Name, namespaceTemplate, cluster.Namespace)
1925
templated[k] = r.Replace(v)
2026
}
2127
return templated
2228
}
29+
30+
// templateValues replaces Cluster.Name and Cluster.Namespace in Helm values text.
31+
func templateValues(cluster *clusterv1.Cluster, text string) (string, error) {
32+
clusterAutoscalerTemplate, err := template.New("").Parse(text)
33+
if err != nil {
34+
return "", fmt.Errorf("failed to parse template: %w", err)
35+
}
36+
37+
type input struct {
38+
Cluster *clusterv1.Cluster
39+
}
40+
41+
templateInput := input{
42+
Cluster: cluster,
43+
}
44+
45+
var b bytes.Buffer
46+
err = clusterAutoscalerTemplate.Execute(&b, templateInput)
47+
if err != nil {
48+
return "", fmt.Errorf("failed setting target Cluster name and namespace in template: %w", err)
49+
}
50+
51+
return b.String(), nil
52+
}

‎pkg/handlers/generic/lifecycle/clusterautoscaler/template_test.go

Lines changed: 267 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,267 @@
1+
// Copyright 2024 Nutanix. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package clusterautoscaler
5+
6+
import (
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
11+
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
12+
)
13+
14+
func Test_templateData(t *testing.T) {
15+
tests := []struct {
16+
name string
17+
cluster *clusterv1.Cluster
18+
data map[string]string
19+
want map[string]string
20+
}{
21+
{
22+
name: "template data",
23+
cluster: &clusterv1.Cluster{
24+
ObjectMeta: metav1.ObjectMeta{
25+
Name: "test-cluster",
26+
Namespace: "test-namespace",
27+
},
28+
},
29+
data: map[string]string{
30+
mapKey: testDeployment,
31+
},
32+
want: map[string]string{
33+
mapKey: templatedDeployment,
34+
},
35+
},
36+
{
37+
name: "no data to template",
38+
cluster: &clusterv1.Cluster{
39+
ObjectMeta: metav1.ObjectMeta{
40+
Name: "test-cluster",
41+
Namespace: "test-namespace",
42+
},
43+
},
44+
data: map[string]string{
45+
mapKey: templatedDeployment,
46+
},
47+
want: map[string]string{
48+
mapKey: templatedDeployment,
49+
},
50+
},
51+
}
52+
for _, tt := range tests {
53+
t.Run(tt.name, func(t *testing.T) {
54+
got := templateData(tt.cluster, tt.data)
55+
assert.Equal(t, tt.want, got)
56+
})
57+
}
58+
}
59+
60+
func Test_templateValues(t *testing.T) {
61+
tests := []struct {
62+
name string
63+
cluster *clusterv1.Cluster
64+
text string
65+
want string
66+
}{
67+
{
68+
name: "template values",
69+
cluster: &clusterv1.Cluster{
70+
ObjectMeta: metav1.ObjectMeta{
71+
Name: "test-cluster",
72+
Namespace: "test-namespace",
73+
},
74+
},
75+
text: testValues,
76+
want: templatedValues,
77+
},
78+
{
79+
name: "no values to template",
80+
cluster: &clusterv1.Cluster{
81+
ObjectMeta: metav1.ObjectMeta{
82+
Name: "test-cluster",
83+
Namespace: "test-namespace",
84+
},
85+
},
86+
text: templatedValues,
87+
want: templatedValues,
88+
},
89+
}
90+
for _, tt := range tests {
91+
t.Run(tt.name, func(t *testing.T) {
92+
got, err := templateValues(tt.cluster, tt.text)
93+
assert.NoError(t, err)
94+
assert.Equal(t, tt.want, got)
95+
})
96+
}
97+
}
98+
99+
const (
100+
mapKey = "deployment.yaml"
101+
102+
testDeployment = `---
103+
apiVersion: apps/v1
104+
kind: Deployment
105+
metadata:
106+
name: cluster-autoscaler-tmpl-clustername-tmpl
107+
namespace: tmpl-clusternamespace-tmpl
108+
spec:
109+
replicas: 1
110+
revisionHistoryLimit: 10
111+
selector:
112+
matchLabels:
113+
app.kubernetes.io/instance: cluster-autoscaler-tmpl-clustername-tmpl
114+
app.kubernetes.io/name: clusterapi-cluster-autoscaler
115+
template:
116+
metadata:
117+
labels:
118+
app.kubernetes.io/instance: cluster-autoscaler-tmpl-clustername-tmpl
119+
app.kubernetes.io/name: clusterapi-cluster-autoscaler
120+
spec:
121+
containers:
122+
- command:
123+
- ./cluster-autoscaler
124+
- --cloud-provider=clusterapi
125+
- --namespace=tmpl-clusternamespace-tmpl
126+
- --node-group-auto-discovery=clusterapi:clusterName=tmpl-clustername-tmpl
127+
- --kubeconfig=/cluster/kubeconfig
128+
- --clusterapi-cloud-config-authoritative
129+
- --enforce-node-group-min-size=true
130+
- --logtostderr=true
131+
- --stderrthreshold=info
132+
- --v=4
133+
name: clusterapi-cluster-autoscaler
134+
volumeMounts:
135+
- mountPath: /cluster
136+
name: kubeconfig
137+
readOnly: true
138+
serviceAccountName: cluster-autoscaler-tmpl-clustername-tmpl
139+
volumes:
140+
- name: kubeconfig
141+
secret:
142+
items:
143+
- key: value
144+
path: kubeconfig
145+
secretName: tmpl-clustername-tmpl-kubeconfig`
146+
147+
templatedDeployment = `---
148+
apiVersion: apps/v1
149+
kind: Deployment
150+
metadata:
151+
name: cluster-autoscaler-test-cluster
152+
namespace: test-namespace
153+
spec:
154+
replicas: 1
155+
revisionHistoryLimit: 10
156+
selector:
157+
matchLabels:
158+
app.kubernetes.io/instance: cluster-autoscaler-test-cluster
159+
app.kubernetes.io/name: clusterapi-cluster-autoscaler
160+
template:
161+
metadata:
162+
labels:
163+
app.kubernetes.io/instance: cluster-autoscaler-test-cluster
164+
app.kubernetes.io/name: clusterapi-cluster-autoscaler
165+
spec:
166+
containers:
167+
- command:
168+
- ./cluster-autoscaler
169+
- --cloud-provider=clusterapi
170+
- --namespace=test-namespace
171+
- --node-group-auto-discovery=clusterapi:clusterName=test-cluster
172+
- --kubeconfig=/cluster/kubeconfig
173+
- --clusterapi-cloud-config-authoritative
174+
- --enforce-node-group-min-size=true
175+
- --logtostderr=true
176+
- --stderrthreshold=info
177+
- --v=4
178+
name: clusterapi-cluster-autoscaler
179+
volumeMounts:
180+
- mountPath: /cluster
181+
name: kubeconfig
182+
readOnly: true
183+
serviceAccountName: cluster-autoscaler-test-cluster
184+
volumes:
185+
- name: kubeconfig
186+
secret:
187+
items:
188+
- key: value
189+
path: kubeconfig
190+
secretName: test-cluster-kubeconfig`
191+
192+
testValues = ` ---
193+
fullnameOverride: "cluster-autoscaler-{{ .Cluster.Name }}"
194+
195+
cloudProvider: clusterapi
196+
197+
# Always trigger a scale-out if replicas are less than the min.
198+
extraArgs:
199+
enforce-node-group-min-size: true
200+
201+
# Enable it to run in a 1 Node cluster.
202+
tolerations:
203+
- effect: NoSchedule
204+
key: node-role.kubernetes.io/control-plane
205+
206+
# Limit a single cluster-autoscaler Deployment to a single Cluster.
207+
autoDiscovery:
208+
clusterName: "{{ .Cluster.Name }}"
209+
# The controller failed with an RBAC error trying to watch CAPI objects at the cluster scope without this.
210+
labels:
211+
- namespace: "{{ .Cluster.Namespace }}"
212+
213+
clusterAPIConfigMapsNamespace: "{{ .Cluster.Namespace }}"
214+
# For workload clusters it is not possible to use the in-cluster client.
215+
# To simplify the configuration, use the admin kubeconfig generated by CAPI for all clusters.
216+
clusterAPIMode: kubeconfig-incluster
217+
clusterAPIWorkloadKubeconfigPath: /cluster/kubeconfig
218+
extraVolumeSecrets:
219+
kubeconfig:
220+
name: "{{ .Cluster.Name }}-kubeconfig"
221+
mountPath: /cluster
222+
readOnly: true
223+
items:
224+
- key: value
225+
path: kubeconfig
226+
rbac:
227+
# Create a Role instead of a ClusterRoles to update cluster-api objects
228+
clusterScoped: false`
229+
230+
templatedValues = ` ---
231+
fullnameOverride: "cluster-autoscaler-test-cluster"
232+
233+
cloudProvider: clusterapi
234+
235+
# Always trigger a scale-out if replicas are less than the min.
236+
extraArgs:
237+
enforce-node-group-min-size: true
238+
239+
# Enable it to run in a 1 Node cluster.
240+
tolerations:
241+
- effect: NoSchedule
242+
key: node-role.kubernetes.io/control-plane
243+
244+
# Limit a single cluster-autoscaler Deployment to a single Cluster.
245+
autoDiscovery:
246+
clusterName: "test-cluster"
247+
# The controller failed with an RBAC error trying to watch CAPI objects at the cluster scope without this.
248+
labels:
249+
- namespace: "test-namespace"
250+
251+
clusterAPIConfigMapsNamespace: "test-namespace"
252+
# For workload clusters it is not possible to use the in-cluster client.
253+
# To simplify the configuration, use the admin kubeconfig generated by CAPI for all clusters.
254+
clusterAPIMode: kubeconfig-incluster
255+
clusterAPIWorkloadKubeconfigPath: /cluster/kubeconfig
256+
extraVolumeSecrets:
257+
kubeconfig:
258+
name: "test-cluster-kubeconfig"
259+
mountPath: /cluster
260+
readOnly: true
261+
items:
262+
- key: value
263+
path: kubeconfig
264+
rbac:
265+
# Create a Role instead of a ClusterRoles to update cluster-api objects
266+
clusterScoped: false`
267+
)

0 commit comments

Comments
 (0)