Include clusterName sha in OBI annotation#2214
Include clusterName sha in OBI annotation#2214MrAlias wants to merge 10 commits intosignalfx:mainfrom
Conversation
This uses the tpl support added in open-telemetry/opentelemetry-helm-charts#2035
MrAlias
force-pushed
the
obi-cluster-name-restart
branch
from
448ed5b to
a95f867
Compare
There was a problem hiding this comment.
Pull request overview
This PR attempts to implement automatic pod restarts for OBI (OpenTelemetry eBPF Instrumentation) when the cluster name ConfigMap changes. It upgrades the OBI subchart from v0.3.0 to v0.4.3 to leverage new template expression support in pod annotations.
Changes:
- Upgraded OBI subchart dependency from version 0.3.0 to 0.4.3
- Added a
checksum/obi-envpod annotation to trigger OBI pod restarts when the ConfigMap changes - Added changelog entry documenting the enhancement
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| helm-charts/splunk-otel-collector/Chart.yaml | Bumps OBI chart version from 0.3.0 to 0.4.3 |
| helm-charts/splunk-otel-collector/values.yaml | Adds pod annotation with checksum calculation to detect ConfigMap changes |
| .chloggen/obi-cluster-name-restart.yaml | Documents the enhancement and new functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 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.
MrAlias
changed the title
|
@MrAlias can you run |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Automatically restart OBI pods when the cluster name ConfigMap changes. | ||
| # This uses the checksum annotation pattern to trigger rollouts when the | ||
| # parent chart's splunk-otel-collector-obi-env ConfigMap is updated. | ||
| checksum/clusterName: '{{ .Values.clusterName | default "" | sha256sum }}' |
There was a problem hiding this comment.
The template expression {{ .Values.clusterName | default "" | sha256sum }} will not correctly resolve the parent chart's clusterName value when evaluated by the OBI subchart. In Helm, when a subchart evaluates a template expression, .Values refers to the subchart's values scope (i.e., values under the obi: key), not the parent chart's root values. The clusterName is defined at the root level of the parent chart's values, so it won't be accessible as .Values.clusterName from the subchart's perspective.
Evidence of this issue can be seen in the rendered manifest at examples/ebpf-instrumentation/rendered_manifests/obi/daemonset.yaml:27, where the checksum shows the SHA256 of an empty string (e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855), despite examples/ebpf-instrumentation/values.yaml:11 setting clusterName to "CHANGEME".
To fix this, the clusterName value needs to be made accessible to the subchart. Consider one of these approaches:
- Pass clusterName as a subchart value: Add
clusterName: {{ .Values.clusterName }}under theobi:section in the parent chart, then reference it as{{ .Values.clusterName }}in the annotation - Use Helm's global values: Move clusterName to be a global value accessible to subcharts as
{{ .Values.global.clusterName }} - Compute the checksum in the parent chart's template and pass the result to the subchart
Support
tplin OBIpodAnnotationsopen-telemetry/opentelemetry-helm-charts#2035