-
Notifications
You must be signed in to change notification settings - Fork 32
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
Various Fixes and Updates for newer NetBox Versions #56
Conversation
I'm not sure if my QSFP changes are correct, but it seems good for Cisco IOS at least. I know the testing is failing for Cisco Nexus though. Probably need to add some testing for QSFP28 100G interfaces. |
Hey, It'll be however complicated for me to test it, but on the other hand, this tool doesn't work with the recent versions of Netbox, and that for a while, so I won't be picky. If anyone at @scaleway wants to test it, it would help, but otherwise I'll merge if things look ok. Anyway, I already had a quick look, I'm going to do a review tomorrow. Overall it looks good! |
Hi, ERROR: netbox_importer: Error when polling device xx-xx--xx: Unexpected format in the OPTIONS response at dcim/ How can we help out? Best, I4Networks |
@i4networks: thanks for the testing. Can you run it again in debug mode so you could paste the OPTIONS request? |
@aruhier what parameter do u use for debug? |
|
@aruhier |
I don't get why it tries to do a query on |
Interesting, I can't find any reason that would be happening. @i4networks, can you post more information like your I thought maybe it was due to not using the latest changes in netboxapi, but it would throw a straight python exception about no The only call to |
@i4networks: Any news about the bug you encountered? |
Hi @aruhier , Sorry for the late response. test.yml config.yml Global options######################## Be more verboseverbose: None Disable ssl warnings in urllib3disable_ssl_warnings: False ################ Netbox################ netbox: Netbox API URLurl: "http://localhost:8001/api" username: "user"password: "password"or to use a token insteadtoken: "letsnotleakthetokenongithub" ########################## Interconnections########################## On some devices, LLDP will expose the host FQDN. If devices are stored onNetbox only by their hostname, the interconnection process will not be ableto find them. Fill this list to strip the domain name from exposed names.remove_domains:
vim: set ts=2 sw=2:netbox version v2.11.4 |
@i4networks, it happens on both Cisco and Juniper devices if you try one at a time? I don't have a Juniper device to test with unfortunately. |
@nahun i tested with only one device at the time in the test.yml |
I can't reproduce that error. You'll need to be very specific on how you're able to reproduce. Versions of everything like Python & Python packages (use Config and device files Model of each device and what version they're running. Provide anything else that might be useful. |
Lets not block this PR for too long, now this project is not working anymore for a lot of people anyway. I'm doing the review right now. |
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.
Globally, in looks good, thanks a lot!
for ip in ip_addresses: | ||
if ip_interface(ip).ip in link_local_v6: |
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 should be in my opinion an option. I remember looking into netbox to know which switch had which LLA.
if line.startswith('System Name'): | ||
hostname = value | ||
elif line.startswith('Local Intf'): | ||
local_port = value | ||
elif line.startswith('Port id'): | ||
port = value | ||
elif line.startswith('Chassis id'): | ||
chassis_id = value | ||
|
||
neighbours.append( | ||
{ | ||
"local_port": local_port, | ||
"hostname": hostname, | ||
"port": port, | ||
"chassis_id": chassis_id | ||
} | ||
) |
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 would prefer having the neighbours attributes (local_port, hostname, port, chassis_id) into a dictionary that you then append to the neighbours list, instead of different variables that are declared in the scope of the for (it's apparently working in python, but it's unintuitive).
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.
Also, instead of appending all the neighbours in a list and yielding at the end, you can yield each neighbour at line 131.
This is due to the fact that the latest changes to the library https://github.com/scaleway/python-netboxapi are not published in the pip repository. Delete the system one and install it from github. |
pip uninstall netboxapi fixed the issue. |
I have the same problem
it didn't work for me, now it tells me: ERROR: netbox_importer: Error when polling device xxx.xxx.xxx: 404 Client Error: Not Found for url: https://test-domain.com/api/dcim/_choices/?limit=50 - any ideas? |
use this branch with commits: |
So, what happened here? Why did these fixes not get merged? This project is broken and dead. Worthless to anyone. @nahun How do I pull this fixed code base, it looks like you deleted it? |
This PR is a little big (sorry about that), but I'll summarize the changes.
fe80::/10
.Other
of an existing interface unlessoverwrite
is specified.assigned_object_id
andassigned_object_type
).Loopback
interface as a Virtual type.I haven't really looked at the testing. Mostly tested on newer Cisco Cat9Ks.
Fixes #39