Repository navigation
Add job.podAnnotations for the pods created by the chart's Jobs - #722
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Pulsar Manager remains unsupported while its removal is unmerged, and the new rendering branch lacks automated coverage.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Adds configurable annotations to pods created by chart Jobs.
Changes:
- Adds
job.podAnnotations. - Applies annotations to five Job pod templates.
| File | Description |
|---|---|
charts/pulsar/values.yaml |
Defines the new setting. |
charts/pulsar/templates/zookeeper-statefulset-upgrade.yaml |
Annotates cleanup pods. |
charts/pulsar/templates/pulsar-cluster-initialize.yaml |
Annotates initialization pods. |
charts/pulsar/templates/jwt-secret-init.yaml |
Annotates JWT initialization pods. |
charts/pulsar/templates/broker-statefulset-upgrade.yaml |
Annotates cleanup pods. |
charts/pulsar/templates/bookkeeper-cluster-initialize.yaml |
Annotates initialization pods. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Pulsar Manager has now been removed in #718. Please rebase / merge origin/master and resolve conflicts. |
6a908e5 to
427aeda
Compare
|
Rebased on master in 427aeda. With Pulsar Manager gone, the Pulsar Manager Job change is dropped, and |
lhotari
left a comment
lhotari
left a comment
There was a problem hiding this comment.
LGTM. Thanks for adding this and for the quick rebase. The Pulsar Manager Job is gone and job.podAnnotations now covers the five remaining Jobs. Rendering with the all-values CI file produces the annotation on all five Job pod templates, and nothing is added with the default empty value.
Resolve conflicts with the nodeAffinity support (apache#711) and job.podAnnotations (apache#722) changes by keeping both: the sts-cleanup hook pod templates get the component labels and the optional job pod annotations, and the pod security context is combined with the imagePullSecrets, scheduling and nodeAffinity settings.
Fixes #689
Motivation
job.annotationsis applied to the Job objects only, not to the pods they create. Annotations that act on pods, such assidecar.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
job.podAnnotations(default{}) tovalues.yaml.bookie-init,pulsar-init,jwt-secret-initand the broker and zookeepersts-cleanupJobs. The twosts-cleanupJobs get a podmetadatablock only when annotations are set.job.podAnnotationsin.ci/templates-all-values.yaml, so the all-values CI render covers the new blocks.Verifying this change
helm lintpasses.--set-string 'job.podAnnotations.sidecar\.istio\.io/inject=false'(and the options that enable all five Jobs),helm templateputs the annotation on the pod template of each Job. With.ci/templates-all-values.yaml, all five rendered Jobs carry it.job.podAnnotations, the rendered manifests are the same as onmaster, apart from the randomly generated secrets and certificates.