Skip to content

Manage instance like zookeeper and operator #618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nlamirault wants to merge 8 commits into from
Closed

Manage instance like zookeeper and operator #618

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
79342ca
feat(instance): Manage instance like zookeeper and operator
nlamirault Feb 11, 2025
3e52197
feat(operator): choice to install the Operator
nlamirault Feb 11, 2025
078b12c
Merge branch 'main' into feat/instance
nlamirault Feb 20, 2025
f08f072
Merge branch 'main' into feat/instance
nlamirault Feb 27, 2025
aac5fbb
Merge branch 'main' into feat/instance
nlamirault May 31, 2025
1f0e5b9
Merge branch 'main' into feat/instance
nlamirault Sep 1, 2025
e25403f
fix(helm): typo
nlamirault Sep 1, 2025
73dbaac
fix(helm): typo
nlamirault Sep 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 20 additions & 20 deletions charts/clickhouse/templates/_helper.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ app.kubernetes.io/component: {{ default "clickhouse" .Values.name }}
Create the name of the service account to use
*/}}
{{- define "clickhouse.serviceAccountName" -}}
{{- if .Values.serviceAccount.create -}}
{{ default (include "clickhouse.fullname" .) .Values.serviceAccount.name }}
{{- if .Values.clickhouseInstance.serviceAccount.create -}}
{{ default (include "clickhouse.fullname" .) .Values.clickhouseInstance.serviceAccount.name }}
{{- else -}}
{{ default "default" .Values.serviceAccount.name }}
{{ default "default" .Values.clickhouseInstance.serviceAccount.name }}
{{- end -}}
{{- end -}}

Expand Down Expand Up @@ -102,9 +102,9 @@ Return suffix part of the headless service
Return the initContainers image name
*/}}
{{- define "clickhouse.initContainers.init.image" -}}
{{- $registryName := default .Values.initContainers.init.image.registry .Values.global.imageRegistry -}}
{{- $repositoryName := .Values.initContainers.init.image.repository -}}
{{- $tag := .Values.initContainers.init.image.tag | toString -}}
{{- $registryName := default .Values.clickhouseInstance.initContainers.init.image.registry .Values.global.imageRegistry -}}
{{- $repositoryName := .Values.clickhouseInstance.initContainers.init.image.repository -}}
{{- $tag := .Values.clickhouseInstance.initContainers.init.image.tag | toString -}}
{{- if $registryName -}}
{{- printf "%s/%s:%s" $registryName $repositoryName $tag -}}
{{- else -}}
Expand All @@ -116,9 +116,9 @@ Return the initContainers image name
Return the initContainers image name for UDF
*/}}
{{- define "clickhouse.initContainers.udf.image" -}}
{{- $registryName := default .Values.initContainers.udf.image.registry .Values.global.imageRegistry -}}
{{- $repositoryName := .Values.initContainers.udf.image.repository -}}
{{- $tag := .Values.initContainers.udf.image.tag | toString -}}
{{- $registryName := default .Values.clickhouseInstance.initContainers.udf.image.registry .Values.global.imageRegistry -}}
{{- $repositoryName := .Values.clickhouseInstance.initContainers.udf.image.repository -}}
{{- $tag := .Values.clickhouseInstance.initContainers.udf.image.tag | toString -}}
{{- if $registryName -}}
{{- printf "%s/%s:%s" $registryName $repositoryName $tag -}}
{{- else -}}
Expand All @@ -130,9 +130,9 @@ Return the initContainers image name for UDF
Return the proper clickhouse image name
*/}}
{{- define "clickhouse.image" -}}
{{- $registryName := default .Values.image.registry .Values.global.imageRegistry -}}
{{- $repositoryName := .Values.image.repository -}}
{{- $tag := .Values.image.tag | toString -}}
{{- $registryName := default .Values.clickhouseInstance.image.registry .Values.global.imageRegistry -}}
{{- $repositoryName := .Values.clickhouseInstance.image.repository -}}
{{- $tag := .Values.clickhouseInstance.image.tag | toString -}}
{{- if $registryName -}}
{{- printf "%s/%s:%s" $registryName $repositoryName $tag -}}
{{- else -}}
Expand All @@ -144,9 +144,9 @@ Return the proper clickhouse image name
Return the proper exporter image name
*/}}
{{- define "logs.system.image" -}}
{{- $registryName := default .Values.logs.system.image.registry .Values.global.imageRegistry -}}
{{- $repositoryName := .Values.logs.system.image.repository -}}
{{- $tag := .Values.logs.system.image.tag | toString -}}
{{- $registryName := default .Values.clickhouseInstance.logs.system.image.registry .Values.global.imageRegistry -}}
{{- $repositoryName := .Values.clickhouseInstance.logs.system.image.repository -}}
{{- $tag := .Values.clickhouseInstance.logs.system.image.tag | toString -}}
{{- if $registryName -}}
{{- printf "%s/%s:%s" $registryName $repositoryName $tag -}}
{{- else -}}
Expand All @@ -172,7 +172,7 @@ imagePullSecrets:
{{- range .Values.global.imagePullSecrets }}
- name: {{ . }}
{{- end }}
{{- range .Values.imagePullSecrets }}
{{- range .Values.clickhouseInstance.imagePullSecrets }}
Copy link

@coderabbitai coderabbitai bot Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify image pull secrets configuration.

Ensure that existing deployments using global image pull secrets continue to work after moving instance-specific secrets under .Values.clickhouseInstance.

🏁 Script executed:

#!/bin/bash
# Search for any references to the old imagePullSecrets configuration
rg -l "imagePullSecrets" | grep -v "_helper.tpl"

Length of output: 1460

Global imagePullSecrets fallback missing might break legacy deployments

The current change in the _helper.tpl file directly loops over .Values.clickhouseInstance.imagePullSecrets without checking or falling back to any global configuration that might be defined (for example, in .Values.imagePullSecrets). This could cause deployments that have not migrated to the instance‐specific configuration to lose their image pull secrets settings. Please check whether you want to either move all global settings to the instance-specific field or add fallback logic in the template so that deployments using global settings continue working.

  • File to inspect: charts/clickhouse/templates/_helper.tpl (line 175)
  • Action needed: Either migrate global image pull secrets to .Values.clickhouseInstance.imagePullSecrets or modify the template (e.g., using a default function) to fall back to the global configuration if instance-specific values are not provided.

- name: {{ . }}
{{- end }}
{{- end }}
Expand All @@ -183,11 +183,11 @@ Return service account annotations of ClickHouse instance.
*/}}
{{- define "clickhouse.serviceAccountAnnotations" -}}
{{- $annotations := dict }}
{{- if .Values.serviceAccount.create }}
{{- $annotations = merge $annotations .Values.serviceAccount.annotations }}
{{- if .Values.clickhouseInstance.serviceAccount.create }}
{{- $annotations = merge $annotations .Values.clickhouseInstance.serviceAccount.annotations }}
{{- end }}
{{- if and .Values.coldStorage.enabled .Values.coldStorage.role.enabled }}
{{- $annotations = merge $annotations .Values.coldStorage.role.annotations }}
{{- if and .Values.clickhouseInstance.coldStorage.enabled .Values.clickhouseInstance.coldStorage.role.enabled }}
{{- $annotations = merge $annotations .Values.clickhouseInstance.coldStorage.role.annotations }}
{{- end -}}
annotations:
{{- toYaml $annotations | nindent 2 }}
Expand Down
Loading