Expose router stats port when prometheus is installed #6636
Conversation
|
Hi @davidaah. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with I understand the commands that are listed here. 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 kubernetes/test-infra repository. |
openshift-ci-robot
added
needs-ok-to-test
|
Can one of the admins verify this patch?
|
| @@ -26,6 +26,7 @@ openshift_cluster_domain: 'cluster.local' | |||
| ########## | |||
| r_openshift_hosted_router_firewall_enabled: "{{ os_firewall_enabled | default(True) }}" | |||
| r_openshift_hosted_router_use_firewalld: "{{ os_firewall_use_firewalld | default(False) }}" | |||
| r_openshift_hosted_router_prometheus_enabled: "{{ openshift_prometheus_state == 'present' | default(True) }}" | |||
There was a problem hiding this comment.
note to reviewer, this may need openshift_prometheus_state and openshift_prometheus_state == 'present' but wasn't sure
There was a problem hiding this comment.
you probably need to verify openshift_prometheus_state is defined. openshift-hosted playbook could run independently of prometheus
There was a problem hiding this comment.
should be fixed now
|
@zgalor @michaelgugino would you be able to review this? |
|
I'm not sure that this is the correct way to expose the stats port. This would implement a firewall rule on the first_master; this workload might be scheduled any place. @sdodson thoughts? |
|
yea, ultimately it needs to hit every server that the router will be deployed on. i believe this should be a well known entity by leveraging the host labels indicating router deployment and it should be static unless a user adds another host with the router specific label update: @michaelgugino after additional review, I agree it does seem the way firewall.yml is invoked from router.yml makes the firewall tasks not execute against the hosts where the router or registry is actually living. unfortunately, i am not sure what exactly the original intent of the firewall.yml but to use it in this way, I would need to re-factor it out as a separate task in the top level hosted router playbook which only runs on nodes which have the host var corresponding to the router node selector used by the daemonset |
|
one thing that may be of potential concern is that this type of change would expose the stats port outside of the cluster (this may be somewhat unavoidable given that prometheus targets are set to the public IPs of the routers/router nodes) |
|
Is there any progress on this? Prometheus just got to GA and this seems like it should be fixed for GA :) |
|
This is no longer needed - in 3.11 routers is configured to expose metrics and prometheus operator automatically configures it as a target Please reopen this PR for release-3.10 is needed |
|
Hi, I made a clean install and it still fails to access all routers in a multi-router setup. It can only access one router -- my guess: the one thats running on the same node as prometheus. Greetings |
The PR 6636 has been closed on the officially repository, but the port issue is not resolved.
Today prometheus is configured out of the box to try and scrape the openshift hosted router stats/metrics endpoint however it is unable to do so because the firewall on each physical node is not permitting this port to exposed.
This change exposes the firewall port when prometheus is installed (default here is True corresponding to
openshift_prometheus_statedefault state ofpresent)Currently getting:
Get http://<ip of node hosting router>:1936/metrics: dial tcp <ip of node hosting router>:1936: getsockopt: no route to hostBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1552235