NH-70086 add init container for OTEL connection to node and event collectors #807
Conversation
Signed-off-by: jakub-racek-swi <jakub.racek@solarwinds.com>
…ps://github.com/solarwinds/swi-k8s-opentelemetry-collector into NH-70086-add-init-container-to-nodes-and-events
|
Is there a reason for not adding this also for the node collector for Windows? |
|
Please make sure there are unit tests for the |
There was a problem hiding this comment.
If the old value is deprecated we need to communicate it and we should remove it from values.yaml (but continue support it in templates).
I recommend to use following condition (haven't tested):
{{- if .Values.otel.swi_endpoint_check.enabled | default .Values.otel.metrics.swi_endpoint_check | default true }}
and we should also put to NOTES.txt deprecation warning, something like this:
{{- if and (not (hasKey .Values.otel "swi_endpoint_check")) (hasKey .Values.otel.metrics "swi_endpoint_check") }}
{{- $warning := printf "Value 'otel.metrics.swi_endpoint_check' is deprecated and will be removed in a future release. Please use 'otel.swi_endpoint_check.enabled' instead." }}
{{- warn $warning }}
{{- end }}
… logic and adjust configuration logic
Good point. The condition check was adjusted, though using a different logic. |
| @@ -7,7 +7,7 @@ WARNING: Prometheus is no longer included in this chart. To scrape custom metric | |||
|
|
|||
| {{- if and .Values.otel.metrics.extra_scrape_metrics (and .Values.otel.metrics.autodiscovery.prometheusEndpoints.enabled (not .Values.otel.metrics.force_extra_scrape_metrics)) -}} | |||
| WARNING: You have enabled autodiscovery of prometheus endpoints, so `extra_scrape_metrics` is ignored. If you are sure that those metrics won't be covered by autodiscovery set `otel.metrics.force_extra_scrape_metrics` to `true`. | |||
|
|
|||
| {{- println -}} | |||
There was a problem hiding this comment.
Added to fix NOTES.txt unit tests.
| Usage: | ||
| {{ isSwiEndpointCheckEnabled . }} | ||
| */}} | ||
| {{- define "isSwiEndpointCheckEnabled" -}} |
There was a problem hiding this comment.
I don't really like templates, but the condition was too complex and in too many places not to create a template for it.
| @@ -60,11 +60,11 @@ spec: | |||
| values: | |||
| - linux | |||
| {{- end }} | |||
| {{- if or (and .Values.otel.metrics.prometheus_check .Values.otel.metrics.prometheus.url) .Values.otel.metrics.swi_endpoint_check }} | |||
| {{- if .Values.imagePullSecrets }} | |||
| imagePullSecrets: | |||
There was a problem hiding this comment.
I'm not sure why imagePullSecrets was under the init containers condition and not separate, as it is in other deployments.
| @@ -51,6 +51,7 @@ func sendTestMessage(endpoint, apiToken, clusterUid string, insecure bool) { | |||
| record.SetTimestamp(time.Now()) | |||
|
|
|||
| logger.Emit(ctx, record) | |||
| log.Print("Connection check was successful") | |||
There was a problem hiding this comment.
Added so that the init container shows some success message instead of just ending. However, this change will actually be applied only after we release a new image.
pstranak-sw
deleted the
NH-70086-add-init-container-to-nodes-and-events
branch
link: https://swicloud.atlassian.net/browse/NH-70086
modified values.yaml and moved 'otel.metrics.swi_endpoint_check' to 'otel.swi_endpoint_check' to be used for all the init containers