search

open-telemetry / opentelemetry-operator

Kubernetes Operator for OpenTelemetry Collector
Apache License 2.0
1.16k stars 412 forks source link

consider using labels instead of annotations #821

Open frzifus opened 2 years ago

frzifus commented 2 years ago

Hi, I would like to pick up on the labeling/annotation topic raised by @pavolloffay here again. I recommend to read his comment first.

Recently we had a problem in the Jaeger-operator that the operator itself (rolled out via deployment) had to go through the deployment webhook provided by itself. The question came up, if it makes sense to let every deployment pass this stage. Using labels we could pre-filter objects by using e.g. an objectSelector. Unfortunately this only works with labels and not with annotations.

In v1.15+, webhooks may optionally limit which requests are intercepted based on the labels of the objects they would be sent, by specifying an objectSelector.

As a solution we exclude deployments with the label app.kubernetes.io/name: jaeger-operator from the webhook. However, it is solved more efficient in e.g. Istio and Kuma. There the webhook is based on labels like in this project. By using a namespace and object selector, only pods with an injection label, in a labeled namespace and without an injected label will be considered.

Why is this here a thing?

As far as I understand, each pod passes through the webhook handler. This could prevent the creation and update of any pods in case of an error. A nice side effect would be that objects can be filtered immediately for api queries.

Looking forward to your thoughts.

jaronoff97 commented 9 months ago

@frzifus any updates for this one?

fxshlein commented 8 months ago

It seems that this could also be useful with the admissionWebhooks.pods.failurePolicy you can configure here: https://github.com/open-telemetry/opentelemetry-helm-charts/blob/main/charts/opentelemetry-operator/values.yaml#L191-L195

When we set the failurePolicy to Fail, we had an issue where the operator was not able to start because it tries to go through it's own webhook, so no other pod could be started too. Having an easy way to just filter "every pod with sidecar.opentelemetry.io/inject" would probably solve this, but since sidecar.opentelemetry.io/inject is an annotation this doesn't work.