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

Add Docker support, set hardcoded values to ENV variables #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dky
Copy link

@dky dky commented Sep 29, 2020

Hello, We decided to poke at your script so I made some updates. Specifically:

  • Setup a Docker build with making the least amount of changes to the original script.
  • Made API_URL + API_TOKEN env vars but set them to the defaults if not set.
  • I also added nb.http_session.verify = True which should help those of us who had issues with validating the API_URL certificate.

Let me know your thoughts! I'll have an update to the README on usage soon.

@@ -134,6 +134,9 @@ def write_metrics(self, filename):
#METRICS = "/tmp/netbox.prom"

nb = pynetbox.api(API_URL, token=API_TOKEN)
# Wether or not to validate the TLS certificate of API_URL
nb.http_session.verify = True
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICS, this is true by default (which is what I'd expect):

root@prometheus:~# python3 -i nb
>>> nb.http_session
<requests.sessions.Session object at 0x7f072cbf9160>
>>> nb.http_session.verify
True

However, I guess it's useful to have it as documentation on how to disable HTTPS verification if required.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, we had a bit of an issue when we started playing with this with one of our internally signed certificates. Python request just did not like it so I was digging around to figure out how to disable it. So I found that. The plan was to set another ENV var and set it to true by default for us to disable but I couldn't get it to work. So I just left it True in the commit so I could toggle it off in our internal environment. Getting an ENV variable such as SSL_VALIDATE would be ideal.

@candlerb
Copy link
Owner

candlerb commented Oct 2, 2020

Interesting. How would you expect prometheus to get hold of the targets.d files? Would you run this container in the same host (or pod) as prometheus, and use a volume mount to share it between the containers?

Apparently the moratorium on adding new service discovery mechanisms to prometheus has ended. Therefore, a much cleaner solution to all this would be to add a native Netbox SD (in Go). I just don't have a big driver for doing that right now. The fact that Netbox's REST API has no stability guarantee doesn't help.

@dky
Copy link
Author

dky commented Oct 2, 2020

Hello, we currently run this as a sidecar container. And have a shared volume between prometheus and this container. We use a shared network link to expose the metrics endpoint with nginx. Thanks for the work on this. Our internal ops team was looking for a way to bridge their Netbox data into Prometheus and this seemed to fit the bill. We are still POCing it of course but got it to work without much pain aside from the TLS validation issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants