-
Notifications
You must be signed in to change notification settings - Fork 78
OPRUN-4416: Remove kube-rbac-proxy from PSM #1190
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
base: main
Are you sure you want to change the base?
Conversation
64fe5e3 to
c01c431
Compare
|
@tmshort: This pull request references OPRUN-4416 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this: Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@tmshort: This pull request references OPRUN-4416 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@tmshort: This pull request references OPRUN-4416 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@tmshort: This pull request references OPRUN-4416 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/label qe-approved |
|
@tmshort: This pull request references OPRUN-4416 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@bandrade: This PR has been marked as verified by DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
| defaultMetricsPort = "0" | ||
| defaultHealthCheckPort = ":8080" | ||
| defaultMetricsPort = "0" // Disable controller-runtime metrics (using pkg/lib/server instead) | ||
| defaultHealthCheckPort = "" // Disable controller-runtime health (using pkg/lib/server instead) |
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.
Is the same port? Or did we change 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.
Changed to use a different server at port 8443.
| httpGet: | ||
| path: /healthz | ||
| port: 8080 | ||
| port: 8443 |
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.
I see we changed.
That I do not know if is an issue.
Could someone be using the port? Breaking change?
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.
This ends up being the same external port. Because kube-rbac-proxy was translating 8080 <-> 8443
EDIT: The upstream port was 9090, the original health ones were 8080, I don’t think it will make a difference since these ports were never referenced explicitly, but discovered.
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.
we have the Prometheus metrics configuration
Will not that break it?
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.
The metrics port was previous exposed as 8443, and that remains. It used to go through the kube-rbac-proxy, and now goes directly to PSM. The Health and Liveness ports are auto-discovered.
| tolerationSeconds: 120 | ||
| volumes: | ||
| - name: package-server-manager-serving-cert | ||
| - name: srv-cert |
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.
It seems the same secret, but we mount it twice. Is it as expected?
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.
Fixed, now it's only mounted once (using the original name)
It will need to be re-verified.
|
Test passed. 1. Build an OCP with this unmerged PR via the cluster-bot
launch 4.22,openshift/operator-framework-olm#1190 aws
jiazha-mac:~ jiazha$ oc get clusterversion
NAME VERSION AVAILABLE PROGRESSING SINCE STATUS
version 4.22.0-0-2026-01-21-095004-test-ci-ln-2khk0j2-latest True False 6m53s Cluster version is 4.22.0-0-2026-01-21-095004-test-ci-ln-2khk0j2-latest
2. Check PSM pod, no kube-rbac-proxy container
jiazha-mac:~ jiazha$ oc get pods -n openshift-operator-lifecycle-manager -l app=package-server-manager
NAME READY STATUS RESTARTS AGE
package-server-manager-66cfc57885-cc8rt 1/1 Running 0 37m
jiazha-mac:~ jiazha$ oc get pod package-server-manager-66cfc57885-cc8rt -n openshift-operator-lifecycle-manager -o jsonpath='{.spec.containers[*].name}'
package-server-manager
3. Check PSM metrics
jiazha-mac:~ jiazha$ token=`oc create token prometheus-k8s -n openshift-monitoring`
jiazha-mac:~ jiazha$ oc get service
NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE
catalog-operator-metrics ClusterIP 172.30.154.101 <none> 8443/TCP 47m
olm-operator-metrics ClusterIP 172.30.84.136 <none> 8443/TCP 47m
package-server-manager-metrics ClusterIP 172.30.79.18 <none> 8443/TCP 47m
packageserver-service ClusterIP 172.30.255.148 <none> 5443/TCP 41m
jiazha-mac:~ jiazha$ oc create route reencrypt psm-metrics --service package-server-manager-metrics --port=metrics
route.route.openshift.io/psm-metrics created
jiazha-mac:~ jiazha$
jiazha-mac:~ jiazha$ oc get route
NAME HOST/PORT PATH SERVICES PORT TERMINATION WILDCARD
psm-metrics psm-metrics-openshift-operator-lifecycle-manager.apps.ci-ln-2khk0j2-76ef8.aws-4.ci.openshift.org package-server-manager-metrics metrics reencrypt None
jiazha-mac:~ jiazha$ curl -k -H "Authorization: Bearer $(echo $token)" https://psm-metrics-openshift-operator-lifecycle-manager.apps.ci-ln-2khk0j2-76ef8.aws-4.ci.openshift.org/metrics |grep metric
...
# HELP promhttp_metric_handler_requests_in_flight Current number of scrapes being served.
# TYPE promhttp_metric_handler_requests_in_flight gauge
promhttp_metric_handler_requests_in_flight 1
# HELP promhttp_metric_handler_requests_total Total number of scrapes by HTTP status code.
# TYPE promhttp_metric_handler_requests_total counter
promhttp_metric_handler_requests_total{code="200"} 133
promhttp_metric_handler_requests_total{code="500"} 0
promhttp_metric_handler_requests_total{code="503"} 0
|
|
/verified by @jianzhangbjz |
|
@jianzhangbjz: This PR has been marked as verified by DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
The PSM was using controller-runtime for health/metrics and using kube-rbac-proxy for TLS support. This removes the kube-rbac-proxy and implements the health/metrics servers using the same code that the OLM and Catalog controllers use. This also adds TLS configuration flags identical to those used for OLM and Catalog operators. This will make updating the PSM for OpenShift TLS Profiles significantly easier, as code can be shared between all the operators. Signed-off-by: Todd Short <todd.short@me.com> Assisted-by: Claude code
camilamacedo86
left a comment
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
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, tmshort The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@tmshort: all tests passed! 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-sigs/prow repository. I understand the commands that are listed here. |
The PSM was using controller-runtime for health/metrics and using kube-rbac-proxy for TLS support. This removes the kube-rbac-proxy and implements the health/metrics servers using the same code that the OLM and Catalog controllers use.
This also adds TLS configuration flags identical to those used for OLM and Catalog operators.
This will make updating the PSM for OpenShift TLS Profiles significantly easier, as code can be shared between all the operators.