Per namespace agent injection

[pavolloffay]

Resolves #373

This adds a capability to auto inject Jaeger agent to namespaces annotated with sidecar.jaegertracing.io/inject - the same annotation which is used for deployments.

It brings a complication when the annotation is present on both namespace and deployment. Then the deployment has higher precedence, however if the specified instance on deployment is not found it does not fallback to the instance specified on the namespace.

Other notable changes

• added namespaces to cluster role binding
• tests now use cluster role binding from deploy/ directory

There was a discussion that Jaeger should not require access to the cluster resources and this PR adds the requirement on accessing Namespaces. At the moment the operator requires access to ClusterRoleBinding. To make the operator work without cluster-wide RBAC it will need more refactoring in a separate PR - related to #791.

Once merged

• Documentation

My test notes:

oc new-project project33
oc create deployment vertx --image=jaegertracing/vertx-create-span:operator-e2e-tests

oc annotate namespace project33 sidecar.jaegertracing.io/inject=true
oc annotate namespace default sidecar.jaegertracing.io/inject-
oc annotate deployment vertx sidecar.jaegertracing.io/inject=true 

kubectl create namespace project33
kubectl config set-context minikube --namespace=project33  

Signed-off-by: Pavol Loffay [email protected]

[jpkrohling]

Only a couple of things that can be improved, but looks very good overall!

[jpkrohling]

Not sure I get it. Are you saying that if a Jaeger CR for all-in-one is created, it will get a sidecar injected if the namespace is annotated, unless the deployment is annotated with a non-existing Jaeger instance?

I think the operator should handle this special case: we can detect that the deployment is a Jaeger deployment, skip the deployment.

[pavolloffay]

Not sure I get it. Are you saying that if a Jaeger CR for all-in-one is created, it will get a sidecar injected if the namespace is annotated, unless the deployment is annotated with a non-existing Jaeger instance?

Yes, it will get sidecar injected.

[pavolloffay]

I will create an issue for it

[pavolloffay]

#900

[jpkrohling]

Would it be possible to fix it in this PR? This currently isn't a problem, as the Jaeger deployment has to be explicitly annotated to reach this situation, whereas current deployment will fail once this PR is merged and the namespace gets annotated. It should be relatively simple, as the Needed could just check for label app: jaeger, or app.kubernetes.io/part-of: jaeger.

[pavolloffay]

I would prefer a separate PR this is already a medium size.

[jpkrohling]

Alright, let's just keep in mind that we probably don't want to release 1.17 with this bug.

[pavolloffay]

@jpkrohling I have applied requested changes. It should be ready from my side.

[jpkrohling]

Just need to clarify #900.