-
Notifications
You must be signed in to change notification settings - Fork 9
fix: add hash calculation #389
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Orca Security Scan Summary
Status | Check | Issues by priority | |
---|---|---|---|
![]() |
Infrastructure as Code | ![]() ![]() ![]() ![]() |
View in Orca |
![]() |
SAST | ![]() ![]() ![]() ![]() |
View in Orca |
![]() |
Secrets | ![]() ![]() ![]() ![]() |
View in Orca |
![]() |
Vulnerabilities | ![]() ![]() ![]() ![]() |
View in Orca |
This sets the existingPath to go with existingName so that hash calculation happens. See the following code for reference: - https://github.com/open-telemetry/opentelemetry-helm-charts/blob/a3074b4a0c9839054488ff08d5daabeb5d89d522/charts/opentelemetry-collector/values.yaml#L87-L89 - https://github.com/open-telemetry/opentelemetry-helm-charts/blob/a3074b4a0c9839054488ff08d5daabeb5d89d522/charts/opentelemetry-collector/templates/_helpers.tpl#L234-L249
@icco thanks for your pull request! Were you able to get this to work locally? When I try it, the subchart is unable to find the template since it looks for a relative path. Here's the error I see:
Seems like someone else has had this same issue when using the collector chart as a subchart: |
I didn't have a local test environment for this so I didn't test it, but you're right, running helm template, I also see this error. open-telemetry/opentelemetry-helm-charts#1737 also highlights this issue. I tried reading through https://helm.sh/docs/chart_template_guide/subcharts_and_globals/#sharing-templates-with-subcharts and I'm not enough of a helm expert to understand how to take advantage of this. I wonder if we could make a fix to the otel helm chart to make this work. Do you have a test env setup to test changes to the otel chart with your observe chart? Or should we try the hash option that the issue I linked suggests as a work around? |
@icco I looked into the possibility of sharing templates, and my most promising lead was that we could re-define {{ $forwarderConfig := include "observe.daemonset.applyForwarderConfig" . }}
{{- define "opentelemetry-collector.configName" -}}
{{- if eq .Chart.Name "forwarder" -}}
forwarder-{{ print $forwarderConfig | sha256sum }}
{{- else -}}
{{ .Chart.Name }}
{{- end -}}
{{- end }} but the result is The workaround of always restarting by adding a random annotation seems solid to me if that's the behavior that you want. You can do that with our current helm chart by adding this to your cluster-events:
podAnnotations:
randAlphaNum/autoRestartEverytime: "{{ randAlphaNum 5 }}"
prometheus-scraper:
podAnnotations:
randAlphaNum/autoRestartEverytime: "{{ randAlphaNum 5 }}"
cluster-metrics:
podAnnotations:
randAlphaNum/autoRestartEverytime: "{{ randAlphaNum 5 }}"
node-logs-metrics:
podAnnotations:
randAlphaNum/autoRestartEverytime: "{{ randAlphaNum 5 }}"
monitor:
podAnnotations:
randAlphaNum/autoRestartEverytime: "{{ randAlphaNum 5 }}"
forwarder:
podAnnotations:
randAlphaNum/autoRestartEverytime: "{{ randAlphaNum 5 }}" I also was able to find a workaround using config map hashing when deploying with Kustomize, but that's a completely separate tool and wouldn't help unless you happen to be using it. |
This sets the existingPath to go with existingName so that hash calculation happens.
See the following code for reference: