[jaeger] Add support for podAnnotations to all-in-one deployment#739
[jaeger] Add support for podAnnotations to all-in-one deployment#739m-dot-jung wants to merge 1 commit intojaegertracing:mainfrom
Conversation
baa033c to
49975a6
Compare
There was a problem hiding this comment.
Pull request overview
Adds configurable pod-level annotations for the Jaeger all-in-one Deployment so users can inject metadata required by service meshes and external scrapers.
Changes:
- Add
jaeger.podAnnotationsto chart values. - Render
jaeger.podAnnotationsinto the all-in-one Deployment pod template annotations. - Bump chart version and update
Chart.lock(Bitnamicommondependency lock).
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| charts/jaeger/values.yaml | Introduces jaeger.podAnnotations value (and touches adjacent scheduling-related values). |
| charts/jaeger/templates/jaeger/jaeger-deploy.yaml | Injects jaeger.podAnnotations into pod template annotations for all-in-one. |
| charts/jaeger/Chart.yaml | Bumps chart version to 4.4.8. |
| charts/jaeger/Chart.lock | Updates locked common dependency version/digest/timestamp. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -212,6 +212,7 @@ jaeger: | |||
| tolerations: [] | |||
| affinity: {} | |||
| topologySpreadContraints: [] | |||
There was a problem hiding this comment.
topologySpreadContraints is misspelled in values.yaml, but the templates reference .Values.jaeger.topologySpreadConstraints (with an extra “s”). As-is, user-provided values under this key won’t be picked up by the deployment. Rename the values key to topologySpreadConstraints (keeping a backwards-compat alias if you need to avoid a breaking change).
| topologySpreadContraints: [] | |
| topologySpreadConstraints: [] |
There was a problem hiding this comment.
I didn't touch any other settings
| prometheus.io/port: "8888" | ||
| prometheus.io/scrape: "true" | ||
| {{- if .Values.jaeger.podAnnotations }} | ||
| {{- toYaml .Values.jaeger.podAnnotations | nindent 8 }} | ||
| {{- end }} |
There was a problem hiding this comment.
Appending podAnnotations directly under annotations: can lead to duplicate YAML keys if a user sets something like prometheus.io/scrape in jaeger.podAnnotations, which can be rejected by some YAML parsers or behave inconsistently. Consider building a single annotations map (e.g., start with the hardcoded Prometheus annotations, merge in jaeger.podAnnotations with a defined precedence), then render it once to avoid duplicates.
| prometheus.io/port: "8888" | |
| prometheus.io/scrape: "true" | |
| {{- if .Values.jaeger.podAnnotations }} | |
| {{- toYaml .Values.jaeger.podAnnotations | nindent 8 }} | |
| {{- end }} | |
| {{- $podAnnotations := dict "prometheus.io/port" "8888" "prometheus.io/scrape" "true" -}} | |
| {{- if .Values.jaeger.podAnnotations }} | |
| {{- $podAnnotations = merge $podAnnotations .Values.jaeger.podAnnotations -}} | |
| {{- end }} | |
| {{- toYaml $podAnnotations | nindent 8 }} |
There was a problem hiding this comment.
Instead, I moved the default values from the template to the values.
859e5bc to
d052648
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The legacy components such as query supported podAnnotations the same way the other components still do. This adds the podAnnotations field to the jaeger values section, following the same pattern already used by the spark, esIndexCleaner, esRollover, and esLookback job templates. The previously hard-coded prometheus podAnnotations are moved to the new values as a default setup. Signed-off-by: Michael Jung <mike_jung@gmx.net>
d052648 to
9a4e636
Compare
|
I'd need this to specify a log parser for fluent-bit |
Summary
Currently, the
all-in-onedeployment template has a hardcodedannotationsblock. This prevents users from injecting critical metadata required by various ecosystem tools that rely on Pod-level annotations.Example Use Cases
consul.hashicorp.com/connect-injectto inject sidecar proxies for secure mTLS communication (e.g., securing OTLP traffic on TCP port 4317).Changes
podAnnotationsfield to the jaeger values section and its usage to theall-in-onedeployment template.querysupportedpodAnnotationsthe same way the other components still do.Checklist