From 3eecde8d0349c02ea2d830f113eadd73a6126efe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Wa=C5=9B?= Date: Thu, 30 May 2024 13:20:31 +0200 Subject: [PATCH 1/2] Use standard labels on all resources Use standard Helm labels, following https://helm.sh/docs/chart_best_practices/labels/#standard-labels. Use labels on all resources, so the chart can be uninstall correctly. --- charts/trino/templates/NOTES.txt | 2 +- charts/trino/templates/_helpers.tpl | 3 +++ charts/trino/templates/autoscaler.yaml | 4 +--- charts/trino/templates/configmap-catalog.yaml | 10 ++------ .../templates/configmap-coordinator.yaml | 10 ++------ charts/trino/templates/configmap-worker.yaml | 12 ++-------- .../templates/deployment-coordinator.yaml | 24 +++++++------------ charts/trino/templates/deployment-worker.yaml | 24 +++++++------------ charts/trino/templates/ingress.yaml | 13 +++------- charts/trino/templates/secret.yaml | 3 --- charts/trino/templates/service.yaml | 13 +++------- .../templates/tests/test-connection.yaml | 11 +++------ 12 files changed, 36 insertions(+), 93 deletions(-) diff --git a/charts/trino/templates/NOTES.txt b/charts/trino/templates/NOTES.txt index 251ff875..90930a1d 100644 --- a/charts/trino/templates/NOTES.txt +++ b/charts/trino/templates/NOTES.txt @@ -4,7 +4,7 @@ Get the application URL by running these commands: export NODE_IP=$(kubectl get nodes --namespace {{ .Release.Namespace }} -o jsonpath="{.items[0].status.addresses[0].address}") echo http://$NODE_IP:$NODE_PORT {{- else if contains "ClusterIP" .Values.service.type }} - export POD_NAME=$(kubectl get pods --namespace {{ .Release.Namespace }} -l "app={{ template "trino.name" . }},release={{ .Release.Name }},component=coordinator" -o jsonpath="{.items[0].metadata.name}") + export POD_NAME=$(kubectl get pods --namespace {{ .Release.Namespace }} --selector "app.kubernetes.io/name={{ template "trino.name" . }},app.kubernetes.io/instance={{ .Release.Name }},app.kubernetes.io/component=coordinator" --output name) echo "Visit http://127.0.0.1:8080 to use your application" kubectl port-forward $POD_NAME 8080:8080 {{- end }} diff --git a/charts/trino/templates/_helpers.tpl b/charts/trino/templates/_helpers.tpl index 9765d0a8..26aa8a6b 100644 --- a/charts/trino/templates/_helpers.tpl +++ b/charts/trino/templates/_helpers.tpl @@ -72,6 +72,9 @@ helm.sh/chart: {{ include "trino.chart" . }} app.kubernetes.io/version: {{ .Chart.AppVersion | quote }} {{- end }} app.kubernetes.io/managed-by: {{ .Release.Service }} +{{- if .Values.commonLabels }} +{{ tpl (toYaml .Values.commonLabels) . }} +{{- end }} {{- end }} {{/* diff --git a/charts/trino/templates/autoscaler.yaml b/charts/trino/templates/autoscaler.yaml index f5b6d622..8028b03c 100644 --- a/charts/trino/templates/autoscaler.yaml +++ b/charts/trino/templates/autoscaler.yaml @@ -4,10 +4,8 @@ kind: HorizontalPodAutoscaler metadata: name: {{ template "trino.worker" . }} namespace: {{ .Release.Namespace }} - {{- if .Values.commonLabels }} labels: - {{- tpl (toYaml .Values.commonLabels) . | nindent 4 }} - {{- end }} + {{- include "trino.labels" . | nindent 4 }} spec: maxReplicas: {{ .Values.server.autoscaling.maxReplicas }} minReplicas: {{ .Values.server.workers }} diff --git a/charts/trino/templates/configmap-catalog.yaml b/charts/trino/templates/configmap-catalog.yaml index 4a726b18..52de3403 100644 --- a/charts/trino/templates/configmap-catalog.yaml +++ b/charts/trino/templates/configmap-catalog.yaml @@ -4,14 +4,8 @@ metadata: name: {{ template "trino.catalog" . }} namespace: {{ .Release.Namespace }} labels: - app: {{ template "trino.name" . }} - chart: {{ template "trino.chart" . }} - release: {{ .Release.Name }} - heritage: {{ .Release.Service }} - role: catalogs - {{- if .Values.commonLabels }} - {{- tpl (toYaml .Values.commonLabels) . | nindent 4 }} - {{- end }} + {{- include "trino.labels" . | nindent 4 }} + app.kubernetes.io/component: catalogs data: tpch.properties: | connector.name=tpch diff --git a/charts/trino/templates/configmap-coordinator.yaml b/charts/trino/templates/configmap-coordinator.yaml index dd2cf1e2..5e3ac68e 100644 --- a/charts/trino/templates/configmap-coordinator.yaml +++ b/charts/trino/templates/configmap-coordinator.yaml @@ -4,14 +4,8 @@ metadata: name: {{ template "trino.coordinator" . }} namespace: {{ .Release.Namespace }} labels: - app: {{ template "trino.name" . }} - chart: {{ template "trino.chart" . }} - release: {{ .Release.Name }} - heritage: {{ .Release.Service }} - component: coordinator - {{- if .Values.commonLabels }} - {{- tpl (toYaml .Values.commonLabels) . | nindent 4 }} - {{- end }} + {{- include "trino.labels" . | nindent 4 }} + app.kubernetes.io/component: coordinator data: node.properties: | node.environment={{ .Values.server.node.environment }} diff --git a/charts/trino/templates/configmap-worker.yaml b/charts/trino/templates/configmap-worker.yaml index d93035e8..5e70c90a 100644 --- a/charts/trino/templates/configmap-worker.yaml +++ b/charts/trino/templates/configmap-worker.yaml @@ -5,14 +5,8 @@ metadata: name: {{ template "trino.worker" . }} namespace: {{ .Release.Namespace }} labels: - app: {{ template "trino.name" . }} - chart: {{ template "trino.chart" . }} - release: {{ .Release.Name }} - heritage: {{ .Release.Service }} - component: worker - {{- if .Values.commonLabels }} - {{- tpl (toYaml .Values.commonLabels) . | nindent 4 }} - {{- end }} + {{- include "trino.labels" . | nindent 4 }} + app.kubernetes.io/component: worker data: node.properties: | node.environment={{ .Values.server.node.environment }} @@ -85,9 +79,7 @@ data: {{ $fileName }}: | {{- $fileContent | nindent 4 }} {{- end }} - --- - apiVersion: v1 kind: ConfigMap metadata: diff --git a/charts/trino/templates/deployment-coordinator.yaml b/charts/trino/templates/deployment-coordinator.yaml index 2207b201..ba630a19 100644 --- a/charts/trino/templates/deployment-coordinator.yaml +++ b/charts/trino/templates/deployment-coordinator.yaml @@ -4,20 +4,16 @@ metadata: name: {{ template "trino.coordinator" . }} namespace: {{ .Release.Namespace }} labels: - app: {{ template "trino.name" . }} - chart: {{ template "trino.chart" . }} - release: {{ .Release.Name }} - heritage: {{ .Release.Service }} - component: coordinator - {{- if .Values.commonLabels }} - {{- tpl (toYaml .Values.commonLabels) . | nindent 4 }} + {{- include "trino.labels" . | nindent 4 }} + app.kubernetes.io/component: coordinator + {{- if .Values.coordinator.labels }} + {{- tpl (toYaml .Values.coordinator.labels) . | nindent 4 }} {{- end }} spec: selector: matchLabels: - app: {{ template "trino.name" . }} - release: {{ .Release.Name }} - component: coordinator + {{- include "trino.selectorLabels" . | nindent 6 }} + app.kubernetes.io/component: coordinator template: metadata: annotations: @@ -28,15 +24,11 @@ spec: {{- end }} labels: - app: {{ template "trino.name" . }} - release: {{ .Release.Name }} - component: coordinator + {{- include "trino.labels" . | nindent 8 }} + app.kubernetes.io/component: coordinator {{- if .Values.coordinator.labels }} {{- tpl (toYaml .Values.coordinator.labels) . | nindent 8 }} {{- end }} - {{- if .Values.commonLabels }} - {{- tpl (toYaml .Values.commonLabels) . | nindent 8 }} - {{- end }} spec: serviceAccountName: {{ include "trino.serviceAccountName" . }} {{- with .Values.securityContext }} diff --git a/charts/trino/templates/deployment-worker.yaml b/charts/trino/templates/deployment-worker.yaml index 60fadae5..ad94b221 100644 --- a/charts/trino/templates/deployment-worker.yaml +++ b/charts/trino/templates/deployment-worker.yaml @@ -5,13 +5,10 @@ metadata: name: {{ template "trino.worker" . }} namespace: {{ .Release.Namespace }} labels: - app: {{ template "trino.name" . }} - chart: {{ template "trino.chart" . }} - release: {{ .Release.Name }} - heritage: {{ .Release.Service }} - component: worker - {{- if .Values.commonLabels }} - {{- tpl (toYaml .Values.commonLabels) . | nindent 4 }} + {{- include "trino.labels" . | nindent 4 }} + app.kubernetes.io/component: worker + {{- if .Values.worker.labels }} + {{- tpl (toYaml .Values.worker.labels) . | nindent 4 }} {{- end }} spec: {{- if not .Values.server.autoscaling.enabled }} @@ -19,9 +16,8 @@ spec: {{- end }} selector: matchLabels: - app: {{ template "trino.name" . }} - release: {{ .Release.Name }} - component: worker + {{- include "trino.selectorLabels" . | nindent 6 }} + app.kubernetes.io/component: worker template: metadata: annotations: @@ -30,15 +26,11 @@ spec: {{- tpl (toYaml .Values.worker.annotations) . | nindent 8 }} {{- end }} labels: - app: {{ template "trino.name" . }} - release: {{ .Release.Name }} - component: worker + {{- include "trino.labels" . | nindent 8 }} + app.kubernetes.io/component: worker {{- if .Values.worker.labels }} {{- tpl (toYaml .Values.worker.labels) . | nindent 8 }} {{- end }} - {{- if .Values.commonLabels }} - {{- tpl (toYaml .Values.commonLabels) . | nindent 8 }} - {{- end }} spec: serviceAccountName: {{ include "trino.serviceAccountName" . }} {{- with .Values.securityContext }} diff --git a/charts/trino/templates/ingress.yaml b/charts/trino/templates/ingress.yaml index 136259f9..518c6bae 100644 --- a/charts/trino/templates/ingress.yaml +++ b/charts/trino/templates/ingress.yaml @@ -1,6 +1,4 @@ {{- if .Values.ingress.enabled -}} -{{- $fullName := include "trino.fullname" . -}} -{{- $svcPort := .Values.service.port -}} apiVersion: networking.k8s.io/v1 kind: Ingress metadata: @@ -8,13 +6,8 @@ metadata: namespace: {{ .Release.Namespace }} labels: {{- include "trino.labels" . | nindent 4 }} - {{- if .Values.commonLabels }} - {{- tpl (toYaml .Values.commonLabels) . | nindent 4 }} - {{- end }} - {{- with .Values.ingress.annotations }} annotations: - {{- toYaml . | nindent 4 }} - {{- end }} + {{- toYaml .Values.ingress.annotations | nindent 4 }} spec: ingressClassName: {{ .Values.ingress.className }} {{- if .Values.ingress.tls }} @@ -37,9 +30,9 @@ spec: pathType: {{ .pathType }} backend: service: - name: {{ $fullName }} + name: {{ include "trino.fullname" . }} port: - number: {{ $svcPort }} + number: {{ .Values.service.port }} {{- end }} {{- end }} {{- end }} diff --git a/charts/trino/templates/secret.yaml b/charts/trino/templates/secret.yaml index 5f1a0b7d..dd48f90a 100644 --- a/charts/trino/templates/secret.yaml +++ b/charts/trino/templates/secret.yaml @@ -6,9 +6,6 @@ metadata: namespace: {{ .Release.Namespace }} labels: {{- include "trino.labels" . | nindent 4 }} - {{- if .Values.commonLabels }} - {{- tpl (toYaml .Values.commonLabels) . | nindent 4 }} - {{- end }} data: {{- if .Values.auth.passwordAuth }} password.db: {{ .Values.auth.passwordAuth | b64enc }} diff --git a/charts/trino/templates/service.yaml b/charts/trino/templates/service.yaml index e0592d9d..687d6959 100644 --- a/charts/trino/templates/service.yaml +++ b/charts/trino/templates/service.yaml @@ -4,13 +4,7 @@ metadata: name: {{ template "trino.fullname" . }} namespace: {{ .Release.Namespace }} labels: - app: {{ template "trino.name" . }} - chart: {{ template "trino.chart" . }} - release: {{ .Release.Name }} - heritage: {{ .Release.Service }} - {{- if .Values.commonLabels }} - {{- tpl (toYaml .Values.commonLabels) . | nindent 4 }} - {{- end }} + {{- include "trino.labels" . | nindent 4 }} annotations: {{- toYaml .Values.service.annotations | nindent 4 }} spec: @@ -27,6 +21,5 @@ spec: protocol: {{ $value.protocol }} {{- end }} selector: - app: {{ template "trino.name" . }} - release: {{ .Release.Name }} - component: coordinator + {{- include "trino.selectorLabels" . | nindent 4 }} + app.kubernetes.io/component: coordinator diff --git a/charts/trino/templates/tests/test-connection.yaml b/charts/trino/templates/tests/test-connection.yaml index 53e40d3a..c7f2b7f0 100644 --- a/charts/trino/templates/tests/test-connection.yaml +++ b/charts/trino/templates/tests/test-connection.yaml @@ -1,15 +1,10 @@ apiVersion: v1 kind: Pod metadata: - name: "{{ include "trino.fullname" . }}-test-connection" + name: {{ include "trino.fullname" . }}-test-connection labels: - app: {{ template "trino.name" . }} - chart: {{ template "trino.chart" . }} - release: {{ .Release.Name }} - heritage: {{ .Release.Service }} - {{- if .Values.commonLabels }} - {{- tpl (toYaml .Values.commonLabels) . | nindent 4 }} - {{- end }} + {{- include "trino.labels" . | nindent 4 }} + app.kubernetes.io/component: test annotations: "helm.sh/hook": test-success spec: From 15719149ebfdbdcecfdc4d97b278e4e418c0418e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Wa=C5=9B?= Date: Thu, 30 May 2024 12:11:17 +0200 Subject: [PATCH 2/2] Print logs after failed installs Because `ct install` calls `helm test` without `--logs`, always pass `--skip-clean-up`, and get the logs and cleanup manually after a failure. --- test.sh | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/test.sh b/test.sh index 0e4a86f2..5983aff3 100755 --- a/test.sh +++ b/test.sh @@ -18,7 +18,7 @@ function join_by { # default to randomly generated namespace, same as chart-testing would do, but we need to load secrets into the same namespace NAMESPACE=trino-$(LC_ALL=C tr -dc 'a-z0-9' /dev/null 2>&1 && pwd)" cd "${SCRIPT_DIR}" || exit 2 -echo "Generating a self-signed TLS certificate" +echo 1>&2 "Generating a self-signed TLS certificate" openssl req -new -newkey rsa:4096 -days 365 -nodes -x509 \ -subj "/O=Trino Software Foundation" \ -addext "subjectAltName=DNS:localhost,DNS:*.$NAMESPACE,DNS:*.$NAMESPACE.svc,DNS:*.$NAMESPACE.svc.cluster.local,IP:127.0.0.1" \ @@ -76,14 +75,29 @@ kubectl -n "$NAMESPACE" create secret tls certificates --cert=cert.crt --key=cer CT_ARGS+=(--namespace "$NAMESPACE") +result=0 for test_name in "${TEST_NAMES[@]}"; do - echo "" - echo "🧪 Running test $test_name" - echo "" - time ct install "${CT_ARGS[@]}" --helm-extra-set-args "$HELM_EXTRA_SET_ARGS ${testCases[$test_name]}" - echo "Test $test_name completed" + echo 1>&2 "" + echo 1>&2 "🧪 Running test $test_name" + echo 1>&2 "" + if ! time ct install "${CT_ARGS[@]}" --helm-extra-set-args "$HELM_EXTRA_SET_ARGS ${testCases[$test_name]}"; then + echo 1>&2 "❌ Test $test_name failed" + echo 1>&2 "Test logs:" + kubectl --namespace "$NAMESPACE" logs --tail=-1 --selector app.kubernetes.io/component=test + result=1 + else + echo 1>&2 "✅ Test $test_name completed" + fi + if [ "$CLEANUP_NAMESPACE" == "true" ]; then + for release in $(helm --namespace "$NAMESPACE" ls --all --short); do + echo 1>&2 "Cleaning up Helm release $release" + helm --namespace "$NAMESPACE" delete "$release" + done + fi done if [ "$CLEANUP_NAMESPACE" == "true" ]; then kubectl delete namespace "$NAMESPACE" fi + +exit $result