-
Notifications
You must be signed in to change notification settings - Fork 54
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
✨main.go: switch to klog-based logger #1317
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
image: gcr.io/kubebuilder/kube-rbac-proxy:v0.15.0 | ||
args: | ||
- "--secure-listen-address=0.0.0.0:8443" | ||
- "--upstream=http://127.0.0.1:8080/" | ||
- "--logtostderr=true" | ||
- "--v=0" | ||
- --secure-listen-address=0.0.0.0:8443 | ||
- --http2-disable | ||
- --upstream=http://127.0.0.1:8080/ | ||
- --logtostderr=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making arguments for krp look the same as catalogd (which also requires bumping krp version).
I'm doing this to ensure consistency in the contents and ordering of arguments between catalogd and operator controller so that vendors have an easier time manipulating flags for their specific use cases.
l.Info("reconcile starting") | ||
defer l.Info("reconcile ending") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the original zap logger l.V(1).Info
was being output by default.
With klogr, we need to put these log messages at a lower verbosity level to keep them coming by default.
As a test, I manually set |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1317 +/- ##
==========================================
- Coverage 76.09% 76.02% -0.08%
==========================================
Files 40 40
Lines 2380 2377 -3
==========================================
- Hits 1811 1807 -4
- Misses 401 402 +1
Partials 168 168
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
7bb25eb
to
696b799
Compare
Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
696b799
to
f03194f
Compare
Use standard klog now that klog works with controller-runtime's expectation of a logr.Logger interface.
We get a standard --v flag and we are more in line with k8s standards.
We also get client-go and other low-level logging from k8s libraries at the higher verbosity levels which could be immensely helpful for debugging.
See similar PR in catalogd: operator-framework/catalogd#419
Description
Reviewer Checklist