Skip to content

Commit

Permalink
fix: CRS generated CA Deployment has extra quotes (#867)
Browse files Browse the repository at this point in the history
**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
```
  • Loading branch information
dkoshkin authored Aug 19, 2024
1 parent deb8591 commit b796e7d
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,41 +12,41 @@ data:
kind: PodDisruptionBudget
metadata:
labels:
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
app.kubernetes.io/managed-by: Helm
app.kubernetes.io/name: clusterapi-cluster-autoscaler
helm.sh/chart: cluster-autoscaler-9.37.0
name: 'cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
namespace: '{{ `{{ .Cluster.Namespace }}` }}'
name: cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
namespace: {{ `{{ .Cluster.Namespace }}` }}
spec:
maxUnavailable: 1
selector:
matchLabels:
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
app.kubernetes.io/name: clusterapi-cluster-autoscaler
---
apiVersion: v1
automountServiceAccountToken: true
kind: ServiceAccount
metadata:
labels:
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
app.kubernetes.io/managed-by: Helm
app.kubernetes.io/name: clusterapi-cluster-autoscaler
helm.sh/chart: cluster-autoscaler-9.37.0
name: 'cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
namespace: '{{ `{{ .Cluster.Namespace }}` }}'
name: cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
namespace: {{ `{{ .Cluster.Namespace }}` }}
---
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
labels:
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
app.kubernetes.io/managed-by: Helm
app.kubernetes.io/name: clusterapi-cluster-autoscaler
helm.sh/chart: cluster-autoscaler-9.37.0
name: 'cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
namespace: '{{ `{{ .Cluster.Namespace }}` }}'
name: cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
namespace: {{ `{{ .Cluster.Namespace }}` }}
rules:
- apiGroups:
- ""
Expand Down Expand Up @@ -105,71 +105,71 @@ data:
kind: RoleBinding
metadata:
labels:
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
app.kubernetes.io/managed-by: Helm
app.kubernetes.io/name: clusterapi-cluster-autoscaler
helm.sh/chart: cluster-autoscaler-9.37.0
name: 'cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
namespace: '{{ `{{ .Cluster.Namespace }}` }}'
name: cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
namespace: {{ `{{ .Cluster.Namespace }}` }}
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: 'cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
name: cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
subjects:
- kind: ServiceAccount
name: 'cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
namespace: '{{ `{{ .Cluster.Namespace }}` }}'
name: cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
namespace: {{ `{{ .Cluster.Namespace }}` }}
---
apiVersion: v1
kind: Service
metadata:
labels:
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
app.kubernetes.io/managed-by: Helm
app.kubernetes.io/name: clusterapi-cluster-autoscaler
helm.sh/chart: cluster-autoscaler-9.37.0
name: 'cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
namespace: '{{ `{{ .Cluster.Namespace }}` }}'
name: cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
namespace: {{ `{{ .Cluster.Namespace }}` }}
spec:
ports:
- name: http
port: 8085
protocol: TCP
targetPort: 8085
selector:
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
app.kubernetes.io/name: clusterapi-cluster-autoscaler
type: ClusterIP
---
apiVersion: apps/v1
kind: Deployment
metadata:
labels:
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
app.kubernetes.io/managed-by: Helm
app.kubernetes.io/name: clusterapi-cluster-autoscaler
helm.sh/chart: cluster-autoscaler-9.37.0
name: 'cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
namespace: '{{ `{{ .Cluster.Namespace }}` }}'
name: cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
namespace: {{ `{{ .Cluster.Namespace }}` }}
spec:
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
app.kubernetes.io/name: clusterapi-cluster-autoscaler
template:
metadata:
labels:
app.kubernetes.io/instance: 'ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
app.kubernetes.io/instance: ca-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
app.kubernetes.io/name: clusterapi-cluster-autoscaler
spec:
containers:
- command:
- ./cluster-autoscaler
- --cloud-provider=clusterapi
- --namespace=kube-system
- --node-group-auto-discovery=clusterapi:clusterName='{{ `{{ .Cluster.Name }}` }}',namespace='{{ `{{ .Cluster.Namespace }}` }}'
- --node-group-auto-discovery=clusterapi:clusterName={{ `{{ .Cluster.Name }}` }},namespace={{ `{{ .Cluster.Namespace }}` }}
- --kubeconfig=/cluster/kubeconfig
- --clusterapi-cloud-config-authoritative
- --enforce-node-group-min-size=true
Expand Down Expand Up @@ -201,7 +201,7 @@ data:
readOnly: true
dnsPolicy: ClusterFirst
priorityClassName: system-cluster-critical
serviceAccountName: 'cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}'
serviceAccountName: cluster-autoscaler-{{ `{{ index .Cluster.Annotations "caren.nutanix.com/cluster-uuid" }}` }}
tolerations:
- effect: NoSchedule
key: node-role.kubernetes.io/control-plane
Expand All @@ -211,7 +211,7 @@ data:
items:
- key: value
path: kubeconfig
secretName: '{{ `{{ .Cluster.Name }}` }}-kubeconfig'
secretName: {{ `{{ .Cluster.Name }}` }}-kubeconfig
kind: ConfigMap
metadata:
creationTimestamp: null
Expand Down
4 changes: 2 additions & 2 deletions hack/addons/update-cluster-autoscaler.sh
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ cp "${KUSTOMIZE_BASE_DIR}"/*.yaml "${ASSETS_DIR}"

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

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

kubectl create configmap "{{ .Values.hooks.clusterAutoscaler.crsStrategy.defaultInstallationConfigMap.name }}" --dry-run=client --output yaml \
Expand Down
22 changes: 22 additions & 0 deletions test/e2e/clusterautoscaler_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/ptr"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
addonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1"
"sigs.k8s.io/cluster-api/test/framework"
"sigs.k8s.io/yaml"

"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1"
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/utils"
Expand Down Expand Up @@ -130,4 +132,24 @@ func WaitForClusterAutoscalerToBeReadyForWorkloadCluster(
),
),
)

statusConfigMap := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: "kube-system",
Name: "cluster-autoscaler-status",
},
}

WaitForConfigMapData(ctx, WaitForConfigMapDataInput{
Getter: workloadClusterClient,
ConfigMap: statusConfigMap,
DataValidator: func(data map[string]string) bool {
type clusterAutoscalerStatus struct {
AutoScalerStatus string `json:"autoscalerStatus,omitempty" yaml:"autoscalerStatus,omitempty"`
}
status := &clusterAutoscalerStatus{}
err = yaml.Unmarshal([]byte(data["status"]), status)
return err == nil && status.AutoScalerStatus == "Running"
},
}, input.DeploymentIntervals...)
}
62 changes: 62 additions & 0 deletions test/e2e/configmap_helpers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
//go:build e2e

// Copyright 2024 Nutanix. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

package e2e

import (
"context"
"fmt"
"strings"
"time"

. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
"k8s.io/klog/v2"
capie2e "sigs.k8s.io/cluster-api/test/e2e"
"sigs.k8s.io/cluster-api/test/framework"
"sigs.k8s.io/controller-runtime/pkg/client"
)

type WaitForConfigMapDataInput struct {
Getter framework.Getter
ConfigMap *corev1.ConfigMap
DataValidator func(data map[string]string) bool
}

func WaitForConfigMapData(
ctx context.Context,
input WaitForConfigMapDataInput,
intervals ...interface{},
) {
start := time.Now()
key := client.ObjectKeyFromObject(input.ConfigMap)
capie2e.Byf("waiting for ConfigMap %s to have expected data", key)
Log("starting to wait for ConfigMap to have expected data")
Eventually(func() bool {
if err := input.Getter.Get(ctx, key, input.ConfigMap); err == nil {
return input.DataValidator(input.ConfigMap.Data)
}
return false
}, intervals...).Should(BeTrue(), func() string {
return DescribeIncorrectConfigMapData(input, input.ConfigMap)
})
Logf("ConfigMap %s has expected data, took %v", key, time.Since(start))
}

// DescribeIncorrectConfigMapData returns detailed output to help debug a ConfigMap data validation failure in e2e.
func DescribeIncorrectConfigMapData(
input WaitForConfigMapDataInput,
configMap *corev1.ConfigMap,
) string {
b := strings.Builder{}
b.WriteString(fmt.Sprintf("ConfigMap %s failed to get expected data:\n",
klog.KObj(input.ConfigMap)))
if configMap == nil {
b.WriteString("\nConfigMap: nil\n")
} else {
b.WriteString(fmt.Sprintf("\nConfigMap:\n%s\n", framework.PrettyPrint(configMap)))
}
return b.String()
}

0 comments on commit b796e7d

Please sign in to comment.