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.

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

Refined Jaeger instance injection logic #1146

Merged
merged 1 commit into from
Jul 29, 2020

Conversation

rubenvp8510
Copy link
Collaborator

@rubenvp8510 rubenvp8510 commented Jul 29, 2020

Signed-off-by: Ruben Vargas ruben.vp8510@gmail.com

Fixes #1116

…nnotation = true

Signed-off-by: Ruben Vargas <ruben.vp8510@gmail.com>
@codecov
Copy link

codecov bot commented Jul 29, 2020

Codecov Report

Merging #1146 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1146      +/-   ##
==========================================
+ Coverage   88.22%   88.25%   +0.02%     
==========================================
  Files          86       86              
  Lines        5352     5362      +10     
==========================================
+ Hits         4722     4732      +10     
  Misses        466      466              
  Partials      164      164              
Impacted Files Coverage Δ
pkg/inject/sidecar.go 94.73% <100.00%> (+0.26%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 71eacfe...f1f57f6. Read the comment docs.

@rubenvp8510
Copy link
Collaborator Author

@jpkrohling please review.

@jpkrohling jpkrohling changed the title Fix when two jaeger instances in diferent namespaces and deployment a… Refined Jaeger instance injection logic Jul 29, 2020
@jpkrohling
Copy link
Contributor

This PR isn't fixing the problem. You can reproduce it like this:

$ crc start
...
Started the OpenShift cluster

$ make run
INFO[0000] Running the operator locally; watching namespace "" 
INFO[0000] Versions                                      arch=amd64 identity=default.jaeger-operator jaeger=1.18.1 jaeger-operator=v1.18.1-18-gf1f57f66 operator-sdk=v0.18.2 os=linux version=go1.14.6
I0729 10:23:30.164784   26167 request.go:621] Throttling request took 1.04514758s, request: GET:https://api.crc.testing:6443/apis/coordination.k8s.io/v1?timeout=32s
INFO[0002] Auto-detected the platform                    platform=openshift
INFO[0002] Auto-detected ingress api                     ingress-api=networking
INFO[0002] Automatically adjusted the 'es-provision' flag  es-provision=no
INFO[0002] Automatically adjusted the 'kafka-provision' flag  kafka-provision=no
INFO[0002] The service account running this operator has the role 'system:auth-delegator', enabling OAuth Proxy's 'delegate-urls' option 
INFO[0008] OAuthProxy ImageStream namespace and/or name not defined  name= namespace=

Then, in another terminal:

$ kubectl apply -f deploy/examples/business-application-injected-sidecar.yaml 
deployment.apps/myapp created

In the first terminal, you'll see this:

INFO[0010] No suitable Jaeger instances found to inject a sidecar  deployment=myapp
INFO[0010] No suitable Jaeger instances found to inject a sidecar  deployment=myapp
INFO[0013] No suitable Jaeger instances found to inject a sidecar  deployment=myapp

@jpkrohling
Copy link
Contributor

By the way, the steps above can also be reproduced with a regular minikube installation.

@jpkrohling
Copy link
Contributor

My bad: as you pointed out, I forgot one crucial step: the actual Jaeger instance :-) This PR does fix the reported issue.

@jpkrohling jpkrohling merged commit d59b3b8 into jaegertracing:master Jul 29, 2020
@rubenvp8510
Copy link
Collaborator Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sidecar injection cannot find appropriate Jaeger instance
2 participants