[Operator] Optional tag for collector

[jdiegosierra]

Resolves #1189

[TylerHelmuth]

Let's move this and the above logic into a helper template so we don't have to do the same condition twice.

[jdiegosierra]

Do you mean, something like this?

{{- define "opentelemetry-operator.collectorImage" -}}
  {{- if and .Values.manager.collectorImage.repository .Values.manager.collectorImage.tag }}
    - --collector-image={{ .Values.manager.collectorImage.repository }}:{{ .Values.manager.collectorImage.tag }}
  {{- else if .Values.manager.collectorImage.repository }}
    - --collector-image={{ .Values.manager.collectorImage.repository }}
  {{- end -}}
{{- end }}

[TylerHelmuth]

I believe we can do something like:

{{ - $tag = "" }}
{{ if .Values.manager.collectorImage.tag }}
{{ - $tag = ":" + .Values.manager.collectorImage.tag }}
{{- end }}
- --collector-image={{ .Values.manager.collectorImage.repository }}{{ $tag }}

[jdiegosierra]

BTW, despite you are forcing the users to add a collector repository. I would add an if condition here so in case you remove the requirement in the future, it can avoid a bug if you forget to add the if condition here later.
Check it out:

{{- if .Values.manager.collectorImage.repository -}}
{{- $tag := "" -}}
{{- if .Values.manager.collectorImage.tag -}}
{{- $tag = print ":" .Values.manager.collectorImage.tag -}}
{{- end }}
- --collector-image={{ .Values.manager.collectorImage.repository }}{{ $tag }}
{{- end }}

[TylerHelmuth]

@jdiegosierra in the situation that we don't pass in a tag, are you sure the tag that ends up being uses is the default collector version from the operator and not the latest version of the set image repository?

[jdiegosierra]

I tried with the latest version, so let me check it with a different one

[jdiegosierra]

@TylerHelmuth It works as expected:
appVersion: 0.100.1

manager:
  image:
    repository: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator
    tag: "0.99.0"
  collectorImage:
    repository: ""
    tag: ""

k describe pod opentelemetry-operator-79cbd68444-wdpl9 | grep Image: 
    Image:         ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.99.0

k describe pod simplest-collector-54bb659675-99d7g | grep Image:
    Image:         ghcr.io/open-telemetry/opentelemetry-collector-releases/opentelemetry-collector:0.99.0

[TylerHelmuth]

What about when you set and image repository and not a tag?

[jdiegosierra]

In that scenario, the operator assumes the collector image is the latest one. If we want to have both components with the same version in that specific scenario, I guess we can pick the tag from the operator.

# Source: opentelemetry-operator/templates/deployment.yaml
  - --collector-image=otel/opentelemetry-collector
image: "ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.99.0"

Image ID: docker.io/otel/opentelemetry-collector@sha256:fe7714849adfd251129be75e39c5c4fa7031d87146645afa5d391ab957845c18

https://hub.docker.com/layers/otel/opentelemetry-collector/0.101.0/images/sha256-9ad0a1312912ac2f52c17f50e2bd3dbfc74d990a60fa6aa02bc62c377a3eac8c?context=explore

[jdiegosierra]

Anyway, that situation can not happen right now because it is mandatory to have both fields set:

opentelemetry-helm-charts/charts/opentelemetry-operator/templates/deployment.yaml

Lines 50 to 51 in ef0e1ac

	{{- if and .Values.manager.collectorImage.repository .Values.manager.collectorImage.tag }}
	- --collector-image={{ .Values.manager.collectorImage.repository }}:{{ .Values.manager.collectorImage.tag }}

[jdiegosierra]

BTW, what do you think about changing this alert with a "warning"?

opentelemetry-helm-charts/charts/opentelemetry-operator/templates/NOTES.txt

Lines 1 to 3 in ef0e1ac

	{{- if not .Values.manager.collectorImage.repository }}
	{{ fail "[ERROR] 'manager.collectorImage.repository' must be set. See https://github.com/open-telemetry/opentelemetry-helm-charts/blob/main/charts/opentelemetry-operator/UPGRADING.md for instructions." }}
	{{ end }}

If the user doesn't set a tag, the image will not take effect, and setting only the repository field as mandatory might create confusion:

opentelemetry-helm-charts/charts/opentelemetry-operator/templates/deployment.yaml

Lines 50 to 53 in ef0e1ac

	{{- if and .Values.manager.collectorImage.repository .Values.manager.collectorImage.tag }}
	- --collector-image={{ .Values.manager.collectorImage.repository }}:{{ .Values.manager.collectorImage.tag }}
	{{- end }}
	{{- if and .Values.manager.opampBridgeImage.repository .Values.manager.opampBridgeImage.tag }}

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[github-actions]

Closed as inactive. Feel free to reopen if this PR is still being worked on.