Skip to content
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

Prometheus: add firewall rules for node exporter #7860

Closed
wants to merge 1 commit into from

Conversation

codificat
Copy link
Member

Adds the framework for firewall rules to the prometheus role and the settings for the node exporter port to use it.

The firewall setup is called from the playbook as it affects a different set of hosts than the rest of the role (first master vs nodes).

@codificat codificat requested a review from aweiteka April 9, 2018 15:18
@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 9, 2018
Adds the framework for firewall rules to the prometheus role and the settings
for the node exporter port to use it. Invoked from the playbook as it affects a
different set of hosts than the rest of the role (first master vs nodes).
@davidaah
Copy link
Contributor

This is nice! It should solve the issue i ran in to when trying to fix this bug #6636

@kisamoto
Copy link

Also seeing this for issue #7999 👍

@codificat
Copy link
Member Author

/retest

@simonpasquier
Copy link
Contributor

@zgalor @pgier @sdodson would you mind having a look at it?

@sdodson
Copy link
Member

sdodson commented Jun 8, 2018

/assign @danmace @pgier

@openshift-ci-robot
Copy link

@sdodson: GitHub didn't allow me to assign the following users: danmace.

Note that only openshift members and repo collaborators can be assigned.

In response to this:

/assign @danmace @pgier

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.

@sdodson
Copy link
Member

sdodson commented Jun 8, 2018

/assign @ironcladlou

@ironcladlou
Copy link
Contributor

@sdodson I don't think I'm the right person to review this one- @jwforres can you recommend a person to assume ownership?

@sdodson sdodson requested a review from brancz June 13, 2018 15:14
@sdodson sdodson assigned brancz and unassigned ironcladlou Jun 13, 2018
@brancz
Copy link
Contributor

brancz commented Jun 13, 2018

IIRC there was the same problem on GCE and I remember that we require an entire range of ports to be open. 9000-10000 if I recall correctly. I’m not the right person to review this beyond that either though, I have little knowledge of ansible itself and the modules involved.

@sdodson
Copy link
Member

sdodson commented Jun 13, 2018

@brancz if you can define the firewall requirements for the node exporter to work properly then I can make sure the ansible code applies those requirements.

@brancz
Copy link
Contributor

brancz commented Jun 13, 2018

@smarterclayton mentioned here, that the whole 9k-10k range is to be open for use of OpenShift cluster/node services. Sounds to me like we should be doing the same on all platforms.

@smarterclayton
Copy link
Contributor

smarterclayton commented Jun 13, 2018 via email

@simonpasquier
Copy link
Contributor

#9072 has been merged so this change becomes obsolete.

@codificat
Copy link
Member Author

Ok, closing it then.

@codificat codificat closed this Jul 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet