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

Use non-cached CR on reconciliation #940

Conversation

jpkrohling
Copy link
Contributor

When receiving a reconciliation request, we try to obtain the CR that triggered the event from Kubernetes repository. When using the regular client, we might end up retrieving a cached version that isn't up to date, meaning that we might see errors like the following from time to time:

ERRO[0350] failed to store back the current CustomResource  error="Operation cannot be fulfilled on jaegers.jaegertracing.io \"simple-prod\": the object has been modified; please apply your changes to the latest version and try again" execution="2020-03-02 13:55:31.292270472 +0000 UTC" instance=simple-prod namespace=observability

This error isn't a big problem, as it will trigger another reconciliation loop and things will eventually stabilize, but given that we might have a couple of reconciliation loops for the same object when it is first created, we want to bypass the cache and reduce the risk of facing this error.

Signed-off-by: Juraci Paixão Kröhling [email protected]

@codecov
Copy link

codecov bot commented Mar 10, 2020

Codecov Report

Merging #940 into master will not change coverage by %.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #940   +/-   ##
=======================================
  Coverage   64.45%   64.45%           
=======================================
  Files          82       82           
  Lines        6535     6535           
=======================================
  Hits         4212     4212           
  Misses       2179     2179           
  Partials      144      144           
Impacted Files Coverage Δ
pkg/controller/jaeger/jaeger_controller.go 36.20% <100.00%> (ø)

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 c95d177...d0c870b. Read the comment docs.

Signed-off-by: Juraci Paixão Kröhling <[email protected]>
Signed-off-by: Juraci Paixão Kröhling <[email protected]>
@jpkrohling jpkrohling force-pushed the Use-nocache-client-when-obtaining-CR branch from aac0047 to f534f5a Compare March 10, 2020 11:02
@jpkrohling
Copy link
Contributor Author

jpkrohling commented Mar 10, 2020

@rubenvp8510, @annanay25, would one of you please review this PR?

Copy link
Member

@annanay25 annanay25 left a comment

Choose a reason for hiding this comment

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

Not super familiar with the controller package, but learning as I go. :)

Signed-off-by: Juraci Paixão Kröhling <[email protected]>
Copy link
Member

@annanay25 annanay25 left a comment

Choose a reason for hiding this comment

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

LGTM :)

@jpkrohling jpkrohling merged commit 7d8f068 into jaegertracing:master Mar 10, 2020
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.

2 participants