-
Notifications
You must be signed in to change notification settings - Fork 20
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
[feat] Remove kube-rbac-proxy to expose metrics and replace it with current best practice #556
Conversation
…urrent best practice
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #556 +/- ##
==========================================
- Coverage 64.92% 64.79% -0.13%
==========================================
Files 79 79
Lines 5622 5633 +11
==========================================
Hits 3650 3650
- Misses 1697 1708 +11
Partials 275 275 ☔ View full report in Codecov by Sentry. |
LGTM aside from the above minor comment |
kind: NetworkPolicy | ||
metadata: | ||
labels: | ||
app.kubernetes.io/name: output-dir |
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.
nit pick: I wonder if we want to add a label as app.kubernetes.io/name: output-dir
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.
oh good catch. Let me remove that!
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.
updated!
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.
LGTM
Current way exposing metrics for the controller has changed and we no longer should be using kube-rbac-proxy. For more info: https://book.kubebuilder.io/reference/metrics.html
As part of this PR, I refactored the main() and moved flag parsing, setting up manager, setting up controllers, etc into their own functions for better readability and maintainability.
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #
Special notes for your reviewer:
TODOs: