fix: emit correct Istio pod labels for ambient mode#1266
Merged
fantapsody merged 2 commits intomasterfrom Mar 11, 2026
Merged
Conversation
When istio.dataplaneMode is set to "ambient", the pulsar.template.labels helper now emits istio.io/dataplane-mode: "ambient" instead of the default sidecar.istio.io/inject: "true". This prevents sidecar injection for pods that should be enrolled in Istio ambient mesh via ztunnel. Without this fix, pods in ambient mode get both labels and sidecar injection takes precedence, resulting in 2/2 containers instead of 1/1. This affects both operator-managed components (broker, bookie, zk, proxy) where the Helm label conflicts with the operator's CRD-driven label, and Helm-only components (toolset, function-worker, grafana, console, etc.) that rely entirely on the Helm template for mesh enrollment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@fantapsody:Thanks for your contribution. For this PR, do we need to update docs? |
Split the broker's spec.istio conditional so that base Istio settings (enabled, dataplaneMode, mtls) only require istio.enabled, while the gateway block additionally requires ingress.broker.enabled. Previously the entire spec.istio block was gated behind both conditions, meaning users couldn't enable Istio mesh enrollment (e.g. ambient mode) without also enabling broker ingress. This was inconsistent with ZK, BK, and Proxy templates which only check istio.enabled. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
maxsxu
approved these changes
Mar 11, 2026
Member
maxsxu
left a comment
There was a problem hiding this comment.
PR Review
Summary of Changes
This PR addresses an issue where Istio configurations for the broker were previously coupled with the ingress.broker.enabled flag.
- It correctly decouples the
istioconfiguration block from the ingress requirement, ensuring that Istio settings (such asdataplaneModeandmtls.mode) can be applied regardless of whether the broker ingress gateway is enabled. - The PR also adds the
istio.io/dataplane-mode: "ambient"label to pods when the dataplane mode is set to "ambient".
Review
The changes look good and correctly resolve the configuration dependency issue. The logic for applying the ambient label and conditionally rendering the Istio gateway block is sound.
Approving the PR.
tuteng
approved these changes
Mar 11, 2026
labuladong
approved these changes
Mar 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two fixes for Istio ambient mode support in sn-platform-slim:
1. Emit correct pod labels for ambient mode (
_helpers.tpl)istio.dataplaneModeis"ambient", thepulsar.template.labelshelper now emitsistio.io/dataplane-mode: "ambient"instead of the defaultsidecar.istio.io/inject: "true"2. Decouple broker Istio mesh enrollment from ingress config (
broker-cluster.yaml)spec.istioconditional so base settings (enabled,dataplaneMode,mtls) only requireistio.enabled, while thegatewayblock additionally requiresingress.broker.enabledspec.istiowas gated behind both conditions, so users couldn't enable mesh enrollment without also enabling broker ingressistio.enabledThe existing
istio.labelscustom override still takes highest precedence, preserving backward compatibility.Test plan
helm templateverified correct label output for all three modes: ambient, sidecar (default), sidecar (explicit)helm templateverified broker CRD outputs correctspec.istiowith and withoutingress.broker.enabledistio.io/dataplane-mode: ambientlabels, produce/consume verified🤖 Generated with Claude Code