ROX-22289: drop kube-rbac-proxy sidecar#9808
Conversation
|
Skipping CI for Draft Pull Request. |
|
/test gke-operator-e2e-tests |
|
Images are ready for the commit at a19dc2d. To use with deploy scripts, first |
porridge
force-pushed
the
porridge/ROX-22289-drop
branch
from
deec83c to
2858729
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #9808 +/- ##
==========================================
- Coverage 47.65% 47.64% -0.01%
==========================================
Files 2468 2468
Lines 167575 167575
==========================================
- Hits 79853 79847 -6
- Misses 81135 81139 +4
- Partials 6587 6589 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
porridge
force-pushed
the
porridge/ROX-22289-drop
branch
from
2858729 to
cfdc6e4
Compare
|
/test gke-operator-e2e-tests |
|
/retest |
1 similar comment
|
/retest |
|
/test gke-operator-e2e-tests |
porridge
force-pushed
the
porridge/ROX-22289-drop
branch
from
cfdc6e4 to
97f3c2c
Compare
|
/test gke-operator-e2e-tests |
|
/test gke-operator-e2e-tests |
msugakov
left a comment
msugakov
left a comment
There was a problem hiding this comment.
Thank you, this is great!
Having tests #9773 created before the change and used to test the change is even more awesome.
That said, I wasn't able to fully draw a parallel between tests that you implemented and what I had to do to see metrics flowing in OCP built-in Prometheus. I described that here.
Do you mind trying that as a manual test to confirm the metrics appear in OCP console?
Done, screenshot added to PR description. |
|
@porridge: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
6 participants
Description
Its functionality is now built into the controller-runtime library, so the manager can serve the metrics endpoint directly. This lets us get rid of a dependency that proved to be problematic (see ticket's parent for more background) and make a tiny dent in operator pod's RAM/CPU needs.
AFAICT the
go.modchanges are a result of importing "sigs.k8s.io/controller-runtime/pkg/metrics/filters" which has additional dependencies.I hope making
--replace-rbac-proxyin thepatch-csv.pyscript into a no-op is a reasonable migration strategy, @msugakov ? Additional downstream cleanup will be tracked in ROX-22355.Checklist
Unit test and regression tests added- tested by tests added in ROX-22288: Add test for operator metrics retrieval #9773Evaluated and added CHANGELOG entry if requiredDetermined and documented upgrade stepsDocumented user facing changes (create PR based on openshift/openshift-docs and merge into rhacs-docs)This change should be transparent to users.
Testing Performed
Here I tell how I validated my change
CI should be enough with the recently added metrics test mentioned above.
As requested by @msugakov I also did a manual test following these instructions (
make manifests bundleafter uncommenting the prometheus line, to obtain theServiceMonitordefinition in step one).This works as expected:
Reminder for reviewers
In addition to reviewing code here, reviewers must also review testing and request further testing in case the
performed one does not seem sufficient. As a reviewer, you must not approve the change until you understand the
performed testing and you are satisfied with it.