You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "Laurianti (via GitHub)" <gi...@apache.org> on 2026/10/02 15:32:01 UTC
[PR] Add job.podAnnotations for the pods created by the chart's Jobs [pulsar-helm-chart]
Laurianti opened a new pull request, #722:
URL: https://github.com/apache/pulsar-helm-chart/pull/722
Fixes #689
### Motivation
`job.annotations` is applied to the Job objects only, not to the pods they create. Annotations that act on pods, such as `sidecar.istio.io/inject: "false"` to keep an Istio sidecar from holding a Job open after its container exits, can't be set for the chart's Jobs.
### Modifications
- Add `job.podAnnotations` (default `{}`) to `values.yaml`.
- Render it in the pod template of the `bookie-init`, `pulsar-init`, `jwt-secret-init` and the broker and zookeeper `sts-cleanup` Jobs. The two `sts-cleanup` Jobs get a pod `metadata` block only when annotations are set.
- The Pulsar Manager init Job is left as is, since #718 removes Pulsar Manager from the chart.
### Verifying this change
- `helm lint` passes.
- With `--set-string 'job.podAnnotations.sidecar\.istio\.io/inject=false'` (and the options that enable all five Jobs), `helm template` puts the annotation on the pod template of each of the five Jobs.
- Without `job.podAnnotations`, the rendered manifests are the same as on `master`, apart from the randomly generated secrets and certificates.
- [ ] Make sure that the change passes the CI checks.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add job.podAnnotations for the pods created by the chart's Jobs [pulsar-helm-chart]
Posted by "Laurianti (via GitHub)" <gi...@apache.org>.
Laurianti commented on code in PR #722:
URL: https://github.com/apache/pulsar-helm-chart/pull/722#discussion_r4167525212
##########
charts/pulsar/templates/pulsar-cluster-initialize.yaml:
##########
@@ -42,6 +42,10 @@ spec:
labels:
{{- include "pulsar.template.labels" . | nindent 8 }}
component: {{ .Values.pulsar_metadata.component }}
+ {{- with .Values.job.podAnnotations }}
+ annotations:
+ {{- toYaml . | nindent 8 }}
Review Comment:
Fixed in 6a908e5: `.ci/templates-all-values.yaml` now sets `job.podAnnotations`, so the all-values render covers the new blocks.
##########
charts/pulsar/values.yaml:
##########
@@ -2407,6 +2407,9 @@ dekaf:
# pulsar-helm-chart/charts/pulsar/templates/pulsar-manager-cluster-initialize.yaml
job:
annotations: {}
+ # Annotations for the pods created by the chart's Jobs, for example
+ # sidecar.istio.io/inject: "false" so that an Istio sidecar doesn't keep a Job from completing
+ podAnnotations: {}
Review Comment:
Fixed in 6a908e5: the Pulsar Manager init Job renders `job.podAnnotations` too, so every Job is covered while it is in the chart.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add job.podAnnotations for the pods created by the chart's Jobs [pulsar-helm-chart]
Posted by "Laurianti (via GitHub)" <gi...@apache.org>.
Laurianti commented on PR #722:
URL: https://github.com/apache/pulsar-helm-chart/pull/722#issuecomment-5966226402
Rebased on master in 427aeda. With Pulsar Manager gone, the Pulsar Manager Job change is dropped, and `job.podAnnotations` covers the five Jobs left.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add job.podAnnotations for the pods created by the chart's Jobs [pulsar-helm-chart]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #722:
URL: https://github.com/apache/pulsar-helm-chart/pull/722#issuecomment-5966197643
Pulsar Manager has now been removed in #718. Please rebase / merge origin/master and resolve conflicts.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add job.podAnnotations for the pods created by the chart's Jobs [pulsar-helm-chart]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #722:
URL: https://github.com/apache/pulsar-helm-chart/pull/722#discussion_r4167379658
##########
charts/pulsar/templates/pulsar-cluster-initialize.yaml:
##########
@@ -42,6 +42,10 @@ spec:
labels:
{{- include "pulsar.template.labels" . | nindent 8 }}
component: {{ .Values.pulsar_metadata.component }}
+ {{- with .Values.job.podAnnotations }}
+ annotations:
+ {{- toYaml . | nindent 8 }}
Review Comment:
The repository's all-values CI render is specifically intended to exercise optional map and `toYaml | nindent` branches, but `job.podAnnotations` is left empty there, so CI only tests the false side of this new `with` block. Add a non-empty annotation map to `.ci/templates-all-values.yaml` so all five rendered pod templates are schema-validated automatically.
##########
charts/pulsar/values.yaml:
##########
@@ -2407,6 +2407,9 @@ dekaf:
# pulsar-helm-chart/charts/pulsar/templates/pulsar-manager-cluster-initialize.yaml
job:
annotations: {}
+ # Annotations for the pods created by the chart's Jobs, for example
+ # sidecar.istio.io/inject: "false" so that an Istio sidecar doesn't keep a Job from completing
+ podAnnotations: {}
Review Comment:
`job.podAnnotations` is documented as applying to the chart's Jobs, but the current chart still renders `pulsar-manager-cluster-initialize.yaml` and that Job's pod template does not consume this value. PR #718 is still open, unmerged, and blocked, so this PR can merge independently while leaving one Job unsupported. Apply the same annotation block to that template, or enforce #718 as a merge prerequisite.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add job.podAnnotations for the pods created by the chart's Jobs [pulsar-helm-chart]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari merged PR #722:
URL: https://github.com/apache/pulsar-helm-chart/pull/722
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org