Skip to content

Commit b796e7d

Browse files
dkoshkindkoshkin
authored
fix: CRS generated CA Deployment has extra quotes (#867)
**What problem does this PR solve?**: Fixes an issue with CRS genertated CA Deployment not working because of extra quotes. ``` I0815 20:12:58.376871 1 reflector.go:332] Listing and watching cluster.x-k8s.io/v1beta1, Resource=machines from k8s.io/client-go/dynamic/dynamicinformer/informer.go:108 W0815 20:12:58.379140 1 reflector.go:547] k8s.io/client-go/dynamic/dynamicinformer/informer.go:108: failed to list cluster.x-k8s.io/v1beta1, Resource=machines: machines.cluster.x-k8s.io is forbidden: User "system:serviceaccount:default:cluster-autoscaler-0191579a-2104-7ace-a5a2-ceae4590d7fe" cannot list resource "machines" in API group "cluster.x-k8s.io" in the namespace "'default'" E0815 20:12:58.379170 1 reflector.go:150] k8s.io/client-go/dynamic/dynamicinformer/informer.go:108: Failed to watch cluster.x-k8s.io/v1beta1, Resource=machines: failed to list cluster.x-k8s.io/v1beta1, Resource=machines: machines.cluster.x-k8s.io is forbidden: User "system:serviceaccount:default:cluster-autoscaler-0191579a-2104-7ace-a5a2-ceae4590d7fe" cannot list resource "machines" in API group "cluster.x-k8s.io" in the namespace "'default'" ``` The extra quotes are only an issue in `--node-group-auto-discovery=`, but the [same script](https://github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/blob/f632224257cb159b04abc2c6c6eb6874c503bb1c/hack/addons/update-cluster-autoscaler.sh#L28) replaces all occurrences. It should be safe to drop the single quotes everywhere though because the name and namespace will be strings and won't be interpreted as numbers by yaml. **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. --> **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. --> ~I think we can improve the e2e tests by checking that all Deployments are Ready on the self-managed clusters, but I did not do that as part of this PR.~ The tests already do that https://github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/blob/f632224257cb159b04abc2c6c6eb6874c503bb1c/test/e2e/clusterautoscaler_helpers.go#L115 Instead we can also wait for the status ConfigMap to be `Running` This is what the data will contain for a working Deployment: ``` data: status: |+ time: 2024-05-22 19:33:34.074058252 +0000 UTC autoscalerStatus: Running ``` And for non working: ``` data: status: |+ time: 2024-08-15 20:07:37.204175116 +0000 UTC autoscalerStatus: Initializing ```
1 parent deb8591 commit b796e7d

File tree

4 files changed

+114
-30
lines changed

4 files changed

+114
-30
lines changed

‎charts/cluster-api-runtime-extensions-nutanix/templates/cluster-autoscaler/manifests/cluster-autoscaler-configmap.yaml

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,41 +12,41 @@ data:
1212
kind: PodDisruptionBudget
1313
metadata:
1414
labels:
15-
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
15+
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
1616
app.kubernetes.io/managed-by: Helm
1717
app.kubernetes.io/name: clusterapi-cluster-autoscaler
1818
helm.sh/chart: cluster-autoscaler-9.37.0
19-
name: 'cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
20-
namespace: '{{ `{{ .Cluster.Namespace }}` }}'
19+
name: cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
20+
namespace: {{ `{{ .Cluster.Namespace }}` }}
2121
spec:
2222
maxUnavailable: 1
2323
selector:
2424
matchLabels:
25-
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
25+
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
2626
app.kubernetes.io/name: clusterapi-cluster-autoscaler
2727
---
2828
apiVersion: v1
2929
automountServiceAccountToken: true
3030
kind: ServiceAccount
3131
metadata:
3232
labels:
33-
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
33+
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
3434
app.kubernetes.io/managed-by: Helm
3535
app.kubernetes.io/name: clusterapi-cluster-autoscaler
3636
helm.sh/chart: cluster-autoscaler-9.37.0
37-
name: 'cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
38-
namespace: '{{ `{{ .Cluster.Namespace }}` }}'
37+
name: cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
38+
namespace: {{ `{{ .Cluster.Namespace }}` }}
3939
---
4040
apiVersion: rbac.authorization.k8s.io/v1
4141
kind: Role
4242
metadata:
4343
labels:
44-
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
44+
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
4545
app.kubernetes.io/managed-by: Helm
4646
app.kubernetes.io/name: clusterapi-cluster-autoscaler
4747
helm.sh/chart: cluster-autoscaler-9.37.0
48-
name: 'cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
49-
namespace: '{{ `{{ .Cluster.Namespace }}` }}'
48+
name: cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
49+
namespace: {{ `{{ .Cluster.Namespace }}` }}
5050
rules:
5151
- apiGroups:
5252
- ""
@@ -105,71 +105,71 @@ data:
105105
kind: RoleBinding
106106
metadata:
107107
labels:
108-
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
108+
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
109109
app.kubernetes.io/managed-by: Helm
110110
app.kubernetes.io/name: clusterapi-cluster-autoscaler
111111
helm.sh/chart: cluster-autoscaler-9.37.0
112-
name: 'cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
113-
namespace: '{{ `{{ .Cluster.Namespace }}` }}'
112+
name: cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
113+
namespace: {{ `{{ .Cluster.Namespace }}` }}
114114
roleRef:
115115
apiGroup: rbac.authorization.k8s.io
116116
kind: Role
117-
name: 'cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
117+
name: cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
118118
subjects:
119119
- kind: ServiceAccount
120-
name: 'cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
121-
namespace: '{{ `{{ .Cluster.Namespace }}` }}'
120+
name: cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
121+
namespace: {{ `{{ .Cluster.Namespace }}` }}
122122
---
123123
apiVersion: v1
124124
kind: Service
125125
metadata:
126126
labels:
127-
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
127+
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
128128
app.kubernetes.io/managed-by: Helm
129129
app.kubernetes.io/name: clusterapi-cluster-autoscaler
130130
helm.sh/chart: cluster-autoscaler-9.37.0
131-
name: 'cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
132-
namespace: '{{ `{{ .Cluster.Namespace }}` }}'
131+
name: cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
132+
namespace: {{ `{{ .Cluster.Namespace }}` }}
133133
spec:
134134
ports:
135135
- name: http
136136
port: 8085
137137
protocol: TCP
138138
targetPort: 8085
139139
selector:
140-
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
140+
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
141141
app.kubernetes.io/name: clusterapi-cluster-autoscaler
142142
type: ClusterIP
143143
---
144144
apiVersion: apps/v1
145145
kind: Deployment
146146
metadata:
147147
labels:
148-
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
148+
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
149149
app.kubernetes.io/managed-by: Helm
150150
app.kubernetes.io/name: clusterapi-cluster-autoscaler
151151
helm.sh/chart: cluster-autoscaler-9.37.0
152-
name: 'cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
153-
namespace: '{{ `{{ .Cluster.Namespace }}` }}'
152+
name: cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
153+
namespace: {{ `{{ .Cluster.Namespace }}` }}
154154
spec:
155155
replicas: 1
156156
revisionHistoryLimit: 10
157157
selector:
158158
matchLabels:
159-
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
159+
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
160160
app.kubernetes.io/name: clusterapi-cluster-autoscaler
161161
template:
162162
metadata:
163163
labels:
164-
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
164+
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
165165
app.kubernetes.io/name: clusterapi-cluster-autoscaler
166166
spec:
167167
containers:
168168
- command:
169169
- ./cluster-autoscaler
170170
- --cloud-provider=clusterapi
171171
- --namespace=kube-system
172-
- --node-group-auto-discovery=clusterapi:clusterName='{{ `{{ .Cluster.Name }}` }}',namespace='{{ `{{ .Cluster.Namespace }}` }}'
172+
- --node-group-auto-discovery=clusterapi:clusterName={{ `{{ .Cluster.Name }}` }},namespace={{ `{{ .Cluster.Namespace }}` }}
173173
- --kubeconfig=/cluster/kubeconfig
174174
- --clusterapi-cloud-config-authoritative
175175
- --enforce-node-group-min-size=true
@@ -201,7 +201,7 @@ data:
201201
readOnly: true
202202
dnsPolicy: ClusterFirst
203203
priorityClassName: system-cluster-critical
204-
serviceAccountName: 'cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
204+
serviceAccountName: cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
205205
tolerations:
206206
- effect: NoSchedule
207207
key: node-role.kubernetes.io/control-plane
@@ -211,7 +211,7 @@ data:
211211
items:
212212
- key: value
213213
path: kubeconfig
214-
secretName: '{{ `{{ .Cluster.Name }}` }}-kubeconfig'
214+
secretName: {{ `{{ .Cluster.Name }}` }}-kubeconfig
215215
kind: ConfigMap
216216
metadata:
217217
creationTimestamp: null

‎hack/addons/update-cluster-autoscaler.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ cp "${KUSTOMIZE_BASE_DIR}"/*.yaml "${ASSETS_DIR}"
2525

2626
kustomize build --enable-helm "${ASSETS_DIR}" >"${ASSETS_DIR}/${FILE_NAME}"
2727

28-
sed -i -e "s/\([a-z-]*\)tmpl-clustername\([^-]*\)-tmpl\([a-z-]*\)/'\1{{ \`{{ .Cluster.Name\2 }}\` }}\3'/g" \
29-
-e "s/\([a-z-]*\)tmpl-clusteruuid-tmpl\([a-z-]*\)/'\1{{ \`{{ index .Cluster.Annotations \"caren.nutanix.com\/cluster-uuid\" }}\` }}\2'/g" \
28+
sed -i -e "s/\([a-z-]*\)tmpl-clustername\([^-]*\)-tmpl\([a-z-]*\)/\1{{ \`{{ .Cluster.Name\2 }}\` }}\3/g" \
29+
-e "s/\([a-z-]*\)tmpl-clusteruuid-tmpl\([a-z-]*\)/\1{{ \`{{ index .Cluster.Annotations \"caren.nutanix.com\/cluster-uuid\" }}\` }}\2/g" \
3030
"${ASSETS_DIR}/${FILE_NAME}"
3131

3232
kubectl create configmap "{{ .Values.hooks.clusterAutoscaler.crsStrategy.defaultInstallationConfigMap.name }}" --dry-run=client --output yaml \

‎test/e2e/clusterautoscaler_helpers.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,14 @@ import (
1414
. "github.com/onsi/ginkgo/v2"
1515
. "github.com/onsi/gomega"
1616
appsv1 "k8s.io/api/apps/v1"
17+
corev1 "k8s.io/api/core/v1"
1718
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1819
"k8s.io/apimachinery/pkg/types"
1920
"k8s.io/utils/ptr"
2021
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
2122
addonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1"
2223
"sigs.k8s.io/cluster-api/test/framework"
24+
"sigs.k8s.io/yaml"
2325

2426
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1"
2527
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/utils"
@@ -130,4 +132,24 @@ func WaitForClusterAutoscalerToBeReadyForWorkloadCluster(
130132
),
131133
),
132134
)
135+
136+
statusConfigMap := &corev1.ConfigMap{
137+
ObjectMeta: metav1.ObjectMeta{
138+
Namespace: "kube-system",
139+
Name: "cluster-autoscaler-status",
140+
},
141+
}
142+
143+
WaitForConfigMapData(ctx, WaitForConfigMapDataInput{
144+
Getter: workloadClusterClient,
145+
ConfigMap: statusConfigMap,
146+
DataValidator: func(data map[string]string) bool {
147+
type clusterAutoscalerStatus struct {
148+
AutoScalerStatus string `json:"autoscalerStatus,omitempty" yaml:"autoscalerStatus,omitempty"`
149+
}
150+
status := &clusterAutoscalerStatus{}
151+
err = yaml.Unmarshal([]byte(data["status"]), status)
152+
return err == nil && status.AutoScalerStatus == "Running"
153+
},
154+
}, input.DeploymentIntervals...)
133155
}

‎test/e2e/configmap_helpers.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
//go:build e2e
2+
3+
// Copyright 2024 Nutanix. All rights reserved.
4+
// SPDX-License-Identifier: Apache-2.0
5+
6+
package e2e
7+
8+
import (
9+
"context"
10+
"fmt"
11+
"strings"
12+
"time"
13+
14+
. "github.com/onsi/gomega"
15+
corev1 "k8s.io/api/core/v1"
16+
"k8s.io/klog/v2"
17+
capie2e "sigs.k8s.io/cluster-api/test/e2e"
18+
"sigs.k8s.io/cluster-api/test/framework"
19+
"sigs.k8s.io/controller-runtime/pkg/client"
20+
)
21+
22+
type WaitForConfigMapDataInput struct {
23+
Getter framework.Getter
24+
ConfigMap *corev1.ConfigMap
25+
DataValidator func(data map[string]string) bool
26+
}
27+
28+
func WaitForConfigMapData(
29+
ctx context.Context,
30+
input WaitForConfigMapDataInput,
31+
intervals ...interface{},
32+
) {
33+
start := time.Now()
34+
key := client.ObjectKeyFromObject(input.ConfigMap)
35+
capie2e.Byf("waiting for ConfigMap %s to have expected data", key)
36+
Log("starting to wait for ConfigMap to have expected data")
37+
Eventually(func() bool {
38+
if err := input.Getter.Get(ctx, key, input.ConfigMap); err == nil {
39+
return input.DataValidator(input.ConfigMap.Data)
40+
}
41+
return false
42+
}, intervals...).Should(BeTrue(), func() string {
43+
return DescribeIncorrectConfigMapData(input, input.ConfigMap)
44+
})
45+
Logf("ConfigMap %s has expected data, took %v", key, time.Since(start))
46+
}
47+
48+
// DescribeIncorrectConfigMapData returns detailed output to help debug a ConfigMap data validation failure in e2e.
49+
func DescribeIncorrectConfigMapData(
50+
input WaitForConfigMapDataInput,
51+
configMap *corev1.ConfigMap,
52+
) string {
53+
b := strings.Builder{}
54+
b.WriteString(fmt.Sprintf("ConfigMap %s failed to get expected data:\n",
55+
klog.KObj(input.ConfigMap)))
56+
if configMap == nil {
57+
b.WriteString("\nConfigMap: nil\n")
58+
} else {
59+
b.WriteString(fmt.Sprintf("\nConfigMap:\n%s\n", framework.PrettyPrint(configMap)))
60+
}
61+
return b.String()
62+
}

0 commit comments

Comments
 (0)