feat: implement safe clickhouse upgrade process + resource limits support (#129)

- Recreate deployment strategy: Ensures old pod terminates completely before new one starts, preventing multiple pods from accessing the same persistent volume
- imagePullPolicy: IfNotPresent: Consistent image pull behavior
- Resource limits support: Allows users to configure CPU/memory to prevent OOM kills
- Enhanced preStop hook:
  includes:
    - SYSTEM STOP MOVES - Stops data movement between disks
    - SYSTEM STOP MERGES - Stops background merge operations
    - SYSTEM FLUSH DISTRIBUTED - Flushes pending distributed table data
    - SYSTEM FLUSH LOGS - Flushes buffered logs to system tables
    - 5-second grace period before SIGTERM
- startupProbe: Gives up to 5 minutes (30 failures × 10s) for ClickHouse to start, crucial for upgrades with schema migrations
- shutdown_wait_unfinished:  60: This gives ClickHouse 60 seconds to wait for active connections to close gracefully before forcing shutdown
- Bumping default clickhouse version to `v25.7`

Ref: HDX-2569

Author: wrn14897

.changeset/puny-hats-turn.md

@@ -0,0 +1,5 @@
+---
+"helm-charts": minor
+---
+
+feat: implement safe clickhouse upgrade process + resource limits support

.changeset/shaggy-squids-wonder.md

@@ -0,0 +1,5 @@
+---
+"helm-charts": minor
+---
+
+chore: bump clickhouse to v25.7

charts/hdx-oss-v2/data/config.xml

@@ -1,5 +1,11 @@
 <?xml version="1.0"?>
 <clickhouse>
+   <user_directories>
+        <users_xml>
+            <path>/etc/clickhouse-server/users.xml</path>
+        </users_xml>
+    </user_directories>
+
     <logger>
         <level>information</level>
         <console>true</console>
@@ -27,6 +33,9 @@
     <timezone>UTC</timezone>
     <mlock_executable>false</mlock_executable>
 
+    <!-- Graceful shutdown settings -->
+    <shutdown_wait_unfinished>60</shutdown_wait_unfinished>
+
     {{- if .Values.clickhouse.prometheus.enabled }}
     <!-- Prometheus exporter -->
     <prometheus>
@@ -154,4 +163,4 @@
     </distributed_ddl>
 
     <format_schema_path>/var/lib/clickhouse/format_schemas/</format_schema_path>
-</clickhouse>
+</clickhouse>

charts/hdx-oss-v2/templates/clickhouse-deployment.yaml

@@ -8,6 +8,8 @@ metadata:
     app: clickhouse
 spec:
   replicas: 1
+  strategy:
+    type: Recreate
   selector:
     matchLabels:
       {{- include "hdx-oss.selectorLabels" . | nindent 6 }}
@@ -18,6 +20,7 @@ spec:
         {{- include "hdx-oss.selectorLabels" . | nindent 8 }}
         app: clickhouse
     spec:
+      terminationGracePeriodSeconds: {{ .Values.clickhouse.terminationGracePeriodSeconds | default 90 }}
       {{- if .Values.clickhouse.nodeSelector }}
       nodeSelector:
         {{- toYaml .Values.clickhouse.nodeSelector | nindent 8 }}
@@ -33,12 +36,28 @@ spec:
       containers:
         - name: clickhouse
           image: "{{ .Values.clickhouse.image }}"
+          imagePullPolicy: IfNotPresent
           ports:
             - containerPort: {{ .Values.clickhouse.port }}
             - containerPort: {{ .Values.clickhouse.nativePort }}
           env:
             - name: CLICKHOUSE_DEFAULT_ACCESS_MANAGEMENT
               value: "1"
+          {{- if .Values.clickhouse.resources }}
+          resources:
+            {{- toYaml .Values.clickhouse.resources | nindent 12 }}
+          {{- end }}
+          lifecycle:
+            preStop:
+              exec:
+                command:
+                  - /bin/sh
+                  - -c
+                  - |
+                    clickhouse-client --query "SYSTEM STOP MERGES" || true
+                    clickhouse-client --query "SYSTEM STOP MOVES" || true
+                    clickhouse-client --query "SYSTEM FLUSH LOGS" || true
+                    sleep 5
           {{- if .Values.clickhouse.livenessProbe.enabled }}
           livenessProbe:
             httpGet:
@@ -59,6 +78,16 @@ spec:
             timeoutSeconds: {{ .Values.clickhouse.readinessProbe.timeoutSeconds }}
             failureThreshold: {{ .Values.clickhouse.readinessProbe.failureThreshold }}
           {{- end }}
+          {{- if .Values.clickhouse.startupProbe.enabled }}
+          startupProbe:
+            httpGet:
+              path: /ping
+              port: {{ .Values.clickhouse.port }}
+            initialDelaySeconds: {{ .Values.clickhouse.startupProbe.initialDelaySeconds }}
+            periodSeconds: {{ .Values.clickhouse.startupProbe.periodSeconds }}
+            timeoutSeconds: {{ .Values.clickhouse.startupProbe.timeoutSeconds }}
+            failureThreshold: {{ .Values.clickhouse.startupProbe.failureThreshold }}
+          {{- end }}
           volumeMounts:
             - name: config
               mountPath: /etc/clickhouse-server/config.xml

charts/hdx-oss-v2/tests/clickhouse-deployment_test.yaml

@@ -384,4 +384,172 @@ tests:
       - documentIndex: 0
         equal:
           path: spec.template.spec.imagePullSecrets[0].name
-          value: regcred
+          value: regcred
+
+  - it: should have Recreate deployment strategy
+    set:
+      clickhouse:
+        enabled: true
+    asserts:
+      - documentSelector: *deployment-selector
+        equal:
+          path: spec.strategy.type
+          value: Recreate
+
+  - it: should have imagePullPolicy set to IfNotPresent
+    set:
+      clickhouse:
+        enabled: true
+    asserts:
+      - documentSelector: *deployment-selector
+        equal:
+          path: spec.template.spec.containers[0].imagePullPolicy
+          value: IfNotPresent
+
+  - it: should include resources when configured
+    set:
+      clickhouse:
+        enabled: true
+        resources:
+          requests:
+            memory: "512Mi"
+            cpu: "500m"
+          limits:
+            memory: "2Gi"
+            cpu: "2000m"
+    asserts:
+      - documentSelector: *deployment-selector
+        equal:
+          path: spec.template.spec.containers[0].resources.requests.memory
+          value: "512Mi"
+      - documentSelector: *deployment-selector
+        equal:
+          path: spec.template.spec.containers[0].resources.requests.cpu
+          value: "500m"
+      - documentSelector: *deployment-selector
+        equal:
+          path: spec.template.spec.containers[0].resources.limits.memory
+          value: "2Gi"
+      - documentSelector: *deployment-selector
+        equal:
+          path: spec.template.spec.containers[0].resources.limits.cpu
+          value: "2000m"
+
+  - it: should not include resources when not configured
+    set:
+      clickhouse:
+        enabled: true
+        resources: {}
+    asserts:
+      - documentSelector: *deployment-selector
+        isNull:
+          path: spec.template.spec.containers[0].resources
+
+  - it: should include preStop lifecycle hook with correct commands
+    set:
+      clickhouse:
+        enabled: true
+    asserts:
+      - documentSelector: *deployment-selector
+        isNotNull:
+          path: spec.template.spec.containers[0].lifecycle.preStop
+      - documentSelector: *deployment-selector
+        equal:
+          path: spec.template.spec.containers[0].lifecycle.preStop.exec.command[0]
+          value: /bin/sh
+      - documentSelector: *deployment-selector
+        equal:
+          path: spec.template.spec.containers[0].lifecycle.preStop.exec.command[1]
+          value: -c
+      - documentSelector: *deployment-selector
+        matchRegex:
+          path: spec.template.spec.containers[0].lifecycle.preStop.exec.command[2]
+          pattern: ".*SYSTEM STOP MERGES.*"
+      - documentSelector: *deployment-selector
+        matchRegex:
+          path: spec.template.spec.containers[0].lifecycle.preStop.exec.command[2]
+          pattern: ".*SYSTEM STOP MOVES.*"
+      - documentSelector: *deployment-selector
+        matchRegex:
+          path: spec.template.spec.containers[0].lifecycle.preStop.exec.command[2]
+          pattern: ".*SYSTEM FLUSH LOGS.*"
+
+  - it: should include startupProbe with default values when enabled
+    set:
+      clickhouse:
+        enabled: true
+        port: 8123
+        startupProbe:
+          enabled: true
+          initialDelaySeconds: 5
+          periodSeconds: 10
+          timeoutSeconds: 5
+          failureThreshold: 30
+    asserts:
+      - documentSelector: *deployment-selector
+        isSubset:
+          path: spec.template.spec.containers[0].startupProbe
+          content:
+            httpGet:
+              path: /ping
+              port: 8123
+            initialDelaySeconds: 5
+            periodSeconds: 10
+            timeoutSeconds: 5
+            failureThreshold: 30
+
+  - it: should not include startupProbe when disabled
+    set:
+      clickhouse:
+        enabled: true
+        startupProbe:
+          enabled: false
+    asserts:
+      - documentSelector: *deployment-selector
+        isNull:
+          path: spec.template.spec.containers[0].startupProbe
+
+  - it: should use custom startupProbe values when provided
+    set:
+      clickhouse:
+        enabled: true
+        port: 8123
+        startupProbe:
+          enabled: true
+          initialDelaySeconds: 10
+          periodSeconds: 15
+          timeoutSeconds: 10
+          failureThreshold: 60
+    asserts:
+      - documentSelector: *deployment-selector
+        isSubset:
+          path: spec.template.spec.containers[0].startupProbe
+          content:
+            httpGet:
+              path: /ping
+              port: 8123
+            initialDelaySeconds: 10
+            periodSeconds: 15
+            timeoutSeconds: 10
+            failureThreshold: 60
+
+  - it: should have default terminationGracePeriodSeconds of 90
+    set:
+      clickhouse:
+        enabled: true
+    asserts:
+      - documentSelector: *deployment-selector
+        equal:
+          path: spec.template.spec.terminationGracePeriodSeconds
+          value: 90
+
+  - it: should use custom terminationGracePeriodSeconds when provided
+    set:
+      clickhouse:
+        enabled: true
+        terminationGracePeriodSeconds: 120
+    asserts:
+      - documentSelector: *deployment-selector
+        equal:
+          path: spec.template.spec.terminationGracePeriodSeconds
+          value: 120