Skip to content
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.

Sign up for GitHub

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

[Operator] Optional tag for collector #1196

Closed
wants to merge 5 commits into from
Closed

[Operator] Optional tag for collector #1196

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1d48481
Optional tag for collector
jdiegosierra May 21, 2024
558ae1a
Merge main
jdiegosierra May 23, 2024
694d91a
revert check collector repository
jdiegosierra May 23, 2024
89c6a5d
Bump version
jdiegosierra May 23, 2024
a9c6f32
change variable name
jdiegosierra May 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion charts/opentelemetry-operator/CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

1. Increase the minor version of the chart by one and set the patch version to zero.
1. Update the chart's `appVersion` to match the new operator version.
1. In the values.yaml, update `manager.collectorImage.tag` to match the version of the collector managed by default by the operator.
1. Run `make generate-examples CHARTS=opentelemetry-operator`.
1. Run `make update-operator-crds` to update the CRDs in this chart to match the operator's.
1. Review the [Operator release notes](https://github.com/open-telemetry/opentelemetry-operator/releases). If any changes affect the helm chart, adjust the helm chart accordingly.
2 changes: 1 addition & 1 deletion charts/opentelemetry-operator/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
apiVersion: v2
name: opentelemetry-operator
version: 0.58.2
version: 0.58.3
description: OpenTelemetry Operator Helm chart for Kubernetes
type: application
home: https://opentelemetry.io/
Expand Down
4 changes: 2 additions & 2 deletions ...ator/examples/default/rendered/admission-webhooks/operator-webhook-with-cert-manager.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ metadata:
annotations:
cert-manager.io/inject-ca-from: default/example-opentelemetry-operator-serving-cert
labels:
helm.sh/chart: opentelemetry-operator-0.58.2
helm.sh/chart: opentelemetry-operator-0.58.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.99.0"
app.kubernetes.io/managed-by: Helm
Expand Down Expand Up @@ -91,7 +91,7 @@ metadata:
annotations:
cert-manager.io/inject-ca-from: default/example-opentelemetry-operator-serving-cert
labels:
helm.sh/chart: opentelemetry-operator-0.58.2
helm.sh/chart: opentelemetry-operator-0.58.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.99.0"
app.kubernetes.io/managed-by: Helm
Expand Down
4 changes: 2 additions & 2 deletions charts/opentelemetry-operator/examples/default/rendered/certmanager.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.58.2
helm.sh/chart: opentelemetry-operator-0.58.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.99.0"
app.kubernetes.io/managed-by: Helm
Expand All @@ -30,7 +30,7 @@ apiVersion: cert-manager.io/v1
kind: Issuer
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.58.2
helm.sh/chart: opentelemetry-operator-0.58.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.99.0"
app.kubernetes.io/managed-by: Helm
Expand Down
6 changes: 3 additions & 3 deletions charts/opentelemetry-operator/examples/default/rendered/clusterrole.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.58.2
helm.sh/chart: opentelemetry-operator-0.58.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.99.0"
app.kubernetes.io/managed-by: Helm
Expand Down Expand Up @@ -223,7 +223,7 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.58.2
helm.sh/chart: opentelemetry-operator-0.58.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.99.0"
app.kubernetes.io/managed-by: Helm
Expand All @@ -242,7 +242,7 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.58.2
helm.sh/chart: opentelemetry-operator-0.58.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.99.0"
app.kubernetes.io/managed-by: Helm
Expand Down
4 changes: 2 additions & 2 deletions charts/opentelemetry-operator/examples/default/rendered/clusterrolebinding.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.58.2
helm.sh/chart: opentelemetry-operator-0.58.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.99.0"
app.kubernetes.io/managed-by: Helm
Expand All @@ -26,7 +26,7 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.58.2
helm.sh/chart: opentelemetry-operator-0.58.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.99.0"
app.kubernetes.io/managed-by: Helm
Expand Down
4 changes: 2 additions & 2 deletions charts/opentelemetry-operator/examples/default/rendered/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ apiVersion: apps/v1
kind: Deployment
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.58.2
helm.sh/chart: opentelemetry-operator-0.58.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.99.0"
app.kubernetes.io/managed-by: Helm
Expand Down Expand Up @@ -34,7 +34,7 @@ spec:
- --enable-leader-election
- --health-probe-addr=:8081
- --webhook-port=9443
- --collector-image=otel/opentelemetry-collector-k8s:0.99.0
- --collector-image=otel/opentelemetry-collector-k8s
command:
- /manager
env:
Expand Down
2 changes: 1 addition & 1 deletion charts/opentelemetry-operator/examples/default/rendered/role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.58.2
helm.sh/chart: opentelemetry-operator-0.58.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.99.0"
app.kubernetes.io/managed-by: Helm
Expand Down
2 changes: 1 addition & 1 deletion charts/opentelemetry-operator/examples/default/rendered/rolebinding.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.58.2
helm.sh/chart: opentelemetry-operator-0.58.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.99.0"
app.kubernetes.io/managed-by: Helm
Expand Down
4 changes: 2 additions & 2 deletions charts/opentelemetry-operator/examples/default/rendered/service.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ apiVersion: v1
kind: Service
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.58.2
helm.sh/chart: opentelemetry-operator-0.58.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.99.0"
app.kubernetes.io/managed-by: Helm
Expand Down Expand Up @@ -32,7 +32,7 @@ apiVersion: v1
kind: Service
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.58.2
helm.sh/chart: opentelemetry-operator-0.58.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.99.0"
app.kubernetes.io/managed-by: Helm
Expand Down
2 changes: 1 addition & 1 deletion charts/opentelemetry-operator/examples/default/rendered/serviceaccount.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ metadata:
name: opentelemetry-operator
namespace: default
labels:
helm.sh/chart: opentelemetry-operator-0.58.2
helm.sh/chart: opentelemetry-operator-0.58.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.99.0"
app.kubernetes.io/managed-by: Helm
Expand Down
2 changes: 1 addition & 1 deletion ...s/opentelemetry-operator/examples/default/rendered/tests/test-certmanager-connection.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ metadata:
name: "example-opentelemetry-operator-cert-manager"
namespace: default
labels:
helm.sh/chart: opentelemetry-operator-0.58.2
helm.sh/chart: opentelemetry-operator-0.58.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.99.0"
app.kubernetes.io/managed-by: Helm
Expand Down
4 changes: 2 additions & 2 deletions charts/opentelemetry-operator/examples/default/rendered/tests/test-service-connection.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ metadata:
name: "example-opentelemetry-operator-metrics"
namespace: default
labels:
helm.sh/chart: opentelemetry-operator-0.58.2
helm.sh/chart: opentelemetry-operator-0.58.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.99.0"
app.kubernetes.io/managed-by: Helm
Expand Down Expand Up @@ -44,7 +44,7 @@ metadata:
name: "example-opentelemetry-operator-webhook"
namespace: default
labels:
helm.sh/chart: opentelemetry-operator-0.58.2
helm.sh/chart: opentelemetry-operator-0.58.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.99.0"
app.kubernetes.io/managed-by: Helm
Expand Down
2 changes: 2 additions & 0 deletions charts/opentelemetry-operator/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ spec:
{{- end }}
{{- 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 }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 }}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 }}

Copy link
Contributor Author

@jdiegosierra jdiegosierra May 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 }}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@jdiegosierra jdiegosierra May 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@jdiegosierra jdiegosierra May 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 }}

{{- end }}
{{- if and .Values.manager.opampBridgeImage.repository .Values.manager.opampBridgeImage.tag }}
- --operator-opamp-bridge-image={{ .Values.manager.opampBridgeImage.repository }}:{{ .Values.manager.opampBridgeImage.tag }}
Expand Down
2 changes: 1 addition & 1 deletion charts/opentelemetry-operator/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ manager:
tag: ""
collectorImage:
repository: ""
tag: 0.99.0
tag: ""
opampBridgeImage:
repository: ""
tag: ""
Expand Down
Loading