xiangfu0 opened a new pull request, #17946:
URL: https://github.com/apache/pinot/pull/17946

   ## Summary
   - Replaces the Bitnami ZooKeeper Helm chart dependency with native 
Kubernetes templates using the official Apache ZooKeeper Docker image 
(`zookeeper:3.9.3`)
   - Bitnami has [discontinued their free-tier 
images](https://github.com/bitnami/charts/issues/35164), making the dependency 
unmaintainable. PR #16984 was a short-term fix; this is the long-term solution
   - Closes #16986
   
   ## What changed
   
   ### Removed
   - **Bitnami chart dependency** from `requirements.yaml` (was 
`https://charts.bitnami.com/bitnami` zookeeper `13.x.x`)
   - **`requirements.lock`** (no more external chart dependencies)
   - **`charts/zookeeper-13.8.7.tgz`** (packaged Bitnami chart)
   - **Bitnami-specific config**: `global.security.allowInsecureImages`, 
`image.registry`
   
   ### Added
   - **`templates/zookeeper/statefulset.yaml`** — StatefulSet using the 
[official Apache ZooKeeper Docker image](https://hub.docker.com/_/zookeeper):
     - Extracts `ZOO_MY_ID` from pod hostname ordinal (1-based, as ZooKeeper 
requires)
     - Generates `ZOO_SERVERS` string dynamically for ensemble mode via a Helm 
helper
     - Configures autopurge, JVM heap, jute maxbuffer, 4-letter-word whitelist
     - Liveness/readiness probes using `ruok` 4-letter command
     - Persistent volumes for both `/data` and `/datalog` (separate from 
Bitnami's single-volume approach)
     - Supports multi-replica ZooKeeper ensembles (tested with `replicaCount=3`)
   - **`templates/zookeeper/service-headless.yaml`** — Headless service for 
StatefulSet DNS resolution (client `2181`, follower `2888`, election `3888`)
   - **`templates/zookeeper/service.yaml`** — ClusterIP service for client 
connections
   - **`templates/_helpers.tpl`** — New helpers:
     - `pinot.zookeeperLabels` / `pinot.zookeeperMatchLabels` — standard Helm 
labels
     - `pinot.zookeeper.headless` — headless service name
     - `pinot.zookeeper.servers` — generates `ZOO_SERVERS` env var for N 
replicas
   
   ### Updated
   - **`values.yaml`** — ZooKeeper section now uses:
     - Image: `zookeeper:3.9.3` (official Apache image, was 
`bitnamilegacy/zookeeper:3.9.3-debian-12-r22`)
     - New config knobs: `tickTime`, `initLimit`, `syncLimit`, 
`maxClientCnxns`, `fourLetterWordsWhitelist`, `adminServer`, `probes`, 
`nodeSelector`, `tolerations`, `podAnnotations`, `securityContext`, `extraEnv`
     - Fine-grained persistence: separate `data.size` and `datalog.size`
   
   ## Backward compatibility
   
   - **`zookeeper.enabled`** / **`zookeeper.urlOverride`** — unchanged 
behavior. When disabled, no ZK resources are created and the override URL is 
used
   - **`zookeeper.port`**, **`replicaCount`**, **`resources`**, 
**`persistence`**, **`affinity`**, **`heapSize`**, **`jvmFlags`**, 
**`autopurge`** — all preserved with same defaults
   - The ZooKeeper service name (`{{ .Release.Name }}-zookeeper`) is unchanged, 
so existing Pinot components continue resolving ZK at the same address
   - **Breaking**: Users who were passing Bitnami-specific values (e.g., 
`zookeeper.containerPorts`, `zookeeper.auth`, `zookeeper.tls`) will need to 
adapt to the new native values structure
   
   ## Migration guide
   
   ### From Bitnami chart (current)
   No action needed for default configurations. The image, ports, persistence, 
and service names are compatible.
   
   ### For customized deployments
   If you were using Bitnami-specific values, map them to the new native values:
   
   | Bitnami value | New equivalent |
   |---|---|
   | `zookeeper.image.registry` + `repository` | `zookeeper.image.repository` 
(single field, e.g., `"zookeeper"`) |
   | `zookeeper.image.tag` | `zookeeper.image.tag` (use official tags, e.g., 
`"3.9.3"`) |
   | `zookeeper.containerPorts.client` | `zookeeper.port` |
   | `zookeeper.auth.*` | Configure via `zookeeper.extraEnv` |
   | `zookeeper.tls.*` | Configure via `zookeeper.extraEnv` and volume mounts |
   | `zookeeper.global.security.allowInsecureImages` | Removed (not needed with 
official image) |
   
   ### For production deployments
   As noted in `values.yaml`, production use cases should consider using the 
[ZooKeeper Kubernetes Operator](https://github.com/pravega/zookeeper-operator) 
instead of the bundled StatefulSet. Set `zookeeper.enabled: false` and provide 
`zookeeper.urlOverride`.
   
   ## Test plan
   - [x] `helm template` renders all three ZooKeeper resources (StatefulSet, 
headless Service, client Service)
   - [x] `helm template --set zookeeper.enabled=false` produces no ZooKeeper 
resources and uses URL override
   - [x] `helm template --set zookeeper.replicaCount=3` generates correct 
`ZOO_SERVERS` with 3 ensemble members
   - [x] Controller configmap correctly resolves `controller.zk.str` to the 
ZooKeeper service name
   - [x] Broker statefulset correctly passes `-zkAddress` argument
   - [ ] Deploy to a Kubernetes cluster and verify ZooKeeper starts and Pinot 
components connect successfully
   - [ ] Test with `replicaCount=3` to verify ensemble leader election works
   - [ ] Test persistence: delete a ZK pod and verify data survives restart
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to