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

fix: take namespace modifications into account #1839

Merged
merged 5 commits into from
Apr 5, 2022

Conversation

frzifus
Copy link
Member

@frzifus frzifus commented Mar 31, 2022

Which problem is this PR solving?

Short description of the changes

  • namespace reconciler is no longer adding or removing sidecars to deployments.
  • namespace reconciler increases revision annotations of affected deployment by namespace change.

@frzifus frzifus force-pushed the feat/simplify_ns_reconcile branch from 2d0a24c to 09cd76a Compare March 31, 2022 20:39
@frzifus frzifus marked this pull request as ready for review March 31, 2022 20:40
@codecov
Copy link

codecov bot commented Mar 31, 2022

Codecov Report

Merging #1839 (689a6ee) into main (49ebb6a) will increase coverage by 0.38%.
The diff coverage is 92.30%.

@@            Coverage Diff             @@
##             main    #1839      +/-   ##
==========================================
+ Coverage   89.06%   89.45%   +0.38%     
==========================================
  Files         100      100              
  Lines        6147     6116      -31     
==========================================
- Hits         5475     5471       -4     
+ Misses        495      470      -25     
+ Partials      177      175       -2     
Impacted Files Coverage Δ
controllers/appsv1/namespace_controller.go 0.00% <0.00%> (ø)
pkg/controller/jaeger/jaeger_controller.go 56.81% <85.71%> (-1.78%) ⬇️
pkg/controller/namespace/namespace_controller.go 68.18% <100.00%> (+23.44%) ⬆️
pkg/inject/sidecar.go 97.56% <100.00%> (+0.08%) ⬆️

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 49ebb6a...689a6ee. Read the comment docs.

frzifus added 3 commits April 1, 2022 14:32
Previously the namespace reconciler was called by reconcile
requests generated by the JaegerOnSync method. In jaegertracing#1838 the
JaegerOnSync method changes an annotation of the namespace
to call the reconcile loop. Since the reconciler was not
registered for namespace changes, those were not immediately
taken into account.

Signed-off-by: Benedikt Bongartz <[email protected]>
@frzifus frzifus force-pushed the feat/simplify_ns_reconcile branch from 29a7400 to e93c092 Compare April 1, 2022 12:32
Copy link
Collaborator

@rubenvp8510 rubenvp8510 left a comment

Choose a reason for hiding this comment

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

LGTM Good Job!

@rubenvp8510 rubenvp8510 enabled auto-merge (squash) April 5, 2022 04:46
@rubenvp8510 rubenvp8510 merged commit 399b087 into jaegertracing:main Apr 5, 2022
@frzifus frzifus deleted the feat/simplify_ns_reconcile branch April 5, 2022 05:21
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.

operator doesnt observe ns changes
2 participants