[Fix] Stop Kafka CR delete+recreate on helm upgrade

[tal-haim]

📝 Description

Fixes: Kafka broker crash-loops with Invalid cluster.id after helm upgrade --install on a cluster with retained PVCs.

Root cause: Kafka and KafkaNodePool were Helm hooks (post-install,post-upgrade with the default before-hook-creation delete policy). Every upgrade deleted and recreated the CRs, minting a new KRaft cluster id while the PVC still held the old one.

Strimzi ships its CRDs via the sub-chart crds/ directory, so Helm installs CRDs before templates — the Kafka CRs do not need hooks. This PR makes them ordinary release-managed manifests that helm upgrade patches in place, preserving cluster identity on retained PVCs.

Also exposes kafka.storage.deleteClaim (default false) on the KafkaNodePool storage spec.

---

🛠️ Changes Made

• charts/mlrun-ce/templates/kafka/kafka-cluster.yaml — removed helm.sh/hook / hook-weight annotations from the Kafka CR
• charts/mlrun-ce/templates/kafka/kafka-nodepool.yaml — removed hook annotations; render storage.deleteClaim from values when storage type is persistent-claim
• charts/mlrun-ce/values.yaml — added kafka.storage.deleteClaim: false with documentation
• charts/mlrun-ce/README.md — added “Kafka data retention” section
• AGENTS.md — updated Kafka dependency notes and CRD/hook guidance
• charts/mlrun-ce/Chart.yaml — bumped 0.12.0-rc.6 → 0.12.0-rc.7

---

✅ Checklist

• I have tested the changes in this PR
• I confirmed whether my changes require a change in documentation and if so, I created another PR in MLRun for the relevant documentation.
• I confirmed whether my changes require a changes in QA tests, for example: credentials changes, resources naming change and if so, I updated the relevant Jira ticket for QA.
• I increased the Chart version in charts/mlrun-ce/Chart.yaml.
• I confirmed that the installation works both on a local Docker Desktop environment and on a real cluster when using the required prerequisites.
  • If installation issues were found, I updated the relevant Jira ticket with the issue and steps to reproduce, or updated the prerequisites documentation if the issue is related to missing or outdated prerequisites.
• If needed, update https://github.com/mlrun/ce/blob/development/charts/mlrun-ce/README.md with the relevant installation instructions and version Matrix.
• If needed, update the following values files for multi namespace support:
  • Admin values — no change needed (deleteClaim inherits default from values.yaml)
  • User values Node Port — no change needed
  • User values ClusterIP — no change needed

---

🧪 Testing

Tested on a real cluster via the on-prem MLRun CE deploy pipeline (fresh install from this branch, then a second helm upgrade).

Fresh install + second upgrade (CEML-724 core scenario)

Check	Result
Helm rev 1 → rev 2 upgrade	Upgrade complete
Kafka CR UIDs	Unchanged across upgrade (CRs patched in place)
helm.sh/hook on Kafka CRs	Absent
Broker pod	Running 1/1, Kafka Server started
Logs	No Invalid cluster.id
PVC	Same bound volume retained

Hook → no-hook migration (upgrade from pre-fix chart)

Simulated the failure seen when upgrading a cluster that was first installed with the hook-based chart (rc.6) to this fix (rc.7). Helm initially failed with:

UPGRADE FAILED: Kafka "kafka-stream" exists and cannot be imported into the current release:
missing key "meta.helm.sh/release-name": must be set to "mlrun-ce"

After applying the one-time migration below, upgrade succeeded (rc.6 → rc.7, rev 2), CR UIDs unchanged, broker healthy.

---

🔗 References

• Ticket link: CEML-724

---

🚨 Breaking Changes?

• Yes (explain below)
• No

Existing deployments that installed Kafka with the hook-based chart must run a one-time migration before upgrading to this version.

Hook-created CRs lack Helm ownership metadata (meta.helm.sh/release-name / meta.helm.sh/release-namespace). When hooks are removed, Helm refuses to adopt the existing CRs and the upgrade fails with invalid ownership metadata.

One-time migration (run once per affected cluster/namespace before helm upgrade):

NS=mlrun  # adjust if needed

# 1. Adopt existing CRs into the mlrun-ce release
kubectl annotate kafka kafka-stream -n "$NS" \
  meta.helm.sh/release-name=mlrun-ce \
  meta.helm.sh/release-namespace="$NS" --overwrite
kubectl annotate kafkanodepool kafka-stream-pool -n "$NS" \
  meta.helm.sh/release-name=mlrun-ce \
  meta.helm.sh/release-namespace="$NS" --overwrite

# 2. Remove stale hook annotations (run separately per resource type)
kubectl annotate kafka kafka-stream -n "$NS" helm.sh/hook- helm.sh/hook-weight-
kubectl annotate kafkanodepool kafka-stream-pool -n "$NS" helm.sh/hook- helm.sh/hook-weight-

# 3. Proceed with helm upgrade as usual

Fresh installs are unaffected — CRs are created as release-managed resources from the start.

Note: This differs from #307 (cluster-id pinning via init Job + hooks). This PR removes the root cause (hook delete+recreate) rather than working around it.

---

🔍️ Additional Notes

• kafka.storage.deleteClaim defaults to false, retaining Kafka PVCs (and KRaft cluster id) on uninstall. Set true only for disposable environments.
• After migration, subsequent helm upgrade runs preserve CR UIDs and PVC data — no repeat migration needed.

Made with Cursor