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

inject noop logger by default to ctrl-runtime to avoid warnings #4403

Merged

Conversation

lsviben
Copy link
Contributor

@lsviben lsviben commented Jul 28, 2023

Description of your changes

controller-runtime v0.15.0 introduced a warning if the logger is not set for it through crtl.SetLogger.

Except in debug mode, we dont set a logger for it as its too verbose. But as now we get a stack trace warning, adding a noop logger explicity to avoid it.

Fixes #

I have:

  • Read and followed Crossplane's contribution process.
  • Added or updated unit and E2E tests for my change.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

Signed-off-by: lsviben <sviben.lovro@gmail.com>
@phisco
Copy link
Contributor

phisco commented Jul 28, 2023

Given that we couldn't reproduce it, no need to get it in alongside #4402, still I think it's worth getting it in main and release-1.13 👍

@lsviben lsviben marked this pull request as ready for review July 28, 2023 14:02
@lsviben lsviben requested review from a team as code owners July 28, 2023 14:02
@lsviben lsviben requested review from turkenh and ytsarev July 28, 2023 14:02
@turkenh
Copy link
Member

turkenh commented Jul 28, 2023

I could reproduce this with v1.13.0 by installing provider-nop, and seeing stack trace both in core and rbac manager.

I think it happens after the first log attempt which is triggered by a package installation.

@phisco phisco merged commit fa958bc into crossplane:master Jul 28, 2023
18 of 20 checks passed
@github-actions
Copy link

Successfully created backport PR for release-1.13:

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

Successfully merging this pull request may close these issues.

None yet

3 participants