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

Setup logger in context in the patching reconciler #2830

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

danail-branekov
Copy link
Member

@danail-branekov danail-branekov commented Aug 28, 2023

Is there a related GitHub Issue?

No

What is this change about?

Setup a logger in the patching reconciler and inject it into the context that is passed to concrete object reconcilers. This way object reconcilers can just use that logger and not bother setting it up themselves.

This PR is a response to @davewalter's comment. Dave, could you please review this one?

Does this PR introduce a breaking change?

No

Acceptance Steps

Controllers logs appear

Tag your pair, your PM, and/or team

@davewalter

@danail-branekov danail-branekov requested review from georgethebeatle and davewalter and removed request for georgethebeatle August 28, 2023 09:23
Thus reconcilers that use the patching reconciler (i.e. all Korifi
reconcilers) can take a logger from the context
(`logr.FromContextOrDiscard`) that has been set up with object
namespace, name and an unique logID
@danail-branekov danail-branekov force-pushed the setup-logger-in-patching-reconciler branch from a7068e6 to d73b46a Compare August 28, 2023 10:02
Copy link
Member

@davewalter davewalter left a comment

Choose a reason for hiding this comment

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

Looks good.

@davewalter davewalter merged commit 4346278 into main Aug 29, 2023
7 checks passed
@davewalter davewalter deleted the setup-logger-in-patching-reconciler branch August 29, 2023 00:53
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