DGNum/netbox-agent
8
0
Fork 0
Code Issues 28 Pull requests 9 Projects Releases 14 Packages Wiki Activity Actions

Rework all network part #21

Merged
Solvik merged 16 commits from feature/rework_network into master 2019-08-09 12:08:12 +02:00
Conversation 12 Commits 16 Files changed 7 +330 -67
Solvik commented 2019-08-07 17:45:29 +02:00 (Migrated from github.com)
Copy link

Fixes #20 and #22

  • Create interface if it doesn't match IGNORE_INTERFACES regex
  • Create IP if it doesn't match IGNORE_IPS regex
  • Create interface if tagged with VLAN
  • Set LAG on bonding interfaces
  • Set Parent LAG Interface on Child interfaces
  • Double check update on every interface's attributes
  • Set or update IPMI interface as mgmt_only interface
  • Get or create VLAN interface and associate it
Fixes #20 and #22 - [x] Create interface if it doesn't match IGNORE_INTERFACES regex - [x] Create IP if it doesn't match IGNORE_IPS regex - [x] Create interface if tagged with VLAN - [x] Set `LAG` on bonding interfaces - [x] Set `Parent LAG Interface` on `Child` interfaces - [x] Double check update on every interface's attributes - [x] Set or update IPMI interface as `mgmt_only` interface - [x] Get or create VLAN interface and associate it
ramnes (Migrated from github.com) reviewed 2019-08-09 12:07:15 +02:00
ramnes (Migrated from github.com) left a comment
Copy link

Comments from this night that I forgot to submit. 😄

Comments from this night that I forgot to submit. :smile:
README.md
@ -16,1 +22,4 @@
- [python3-netaddr](https://github.com/drkjam/netaddr)
- [python3-netifaces](https://github.com/al45tair/netifaces)
# Known limitations
ramnes (Migrated from github.com) commented 2019-08-08 23:25:05 +02:00
Copy link

You should also list the binaries used as subprocesses, and maybe the specific kernel flags they require, if they do.

You should also list the binaries used as subprocesses, and maybe the specific kernel flags they require, if they do.
README.md
@ -19,3 +28,4 @@
Since it uses `ethtool` and parses `/sys/` directory, it's not compatible with *BSD distributions.
* Netbox `>=2.6.0,<=2.6.2` has a caching problem ; if the cache lifetime is too high, the script can get stale data after modification.
We advise to set `CACHE_TIME` to `0`.
ramnes (Migrated from github.com) commented 2019-08-08 23:29:23 +02:00
Copy link

Maybe in the future the cache time could also be handled in the agent by enforcing that amount of time between two requests. Especially if there's a way to know that value through the API, and if we can group multiple calls in one.

Maybe in the future the cache time could also be handled in the agent by enforcing that amount of time between two requests. Especially if there's a way to know that value through the API, and if we can group multiple calls in one.
netbox_agent/config.py
@ -33,0 +35,4 @@
NETWORK_IGNORE_IPS = None
if config.get('network'):
NETWORK_IGNORE_INTERFACES = config['network']['ignore_interfaces']
NETWORK_IGNORE_IPS = config['network']['ignore_ips']
ramnes (Migrated from github.com) commented 2019-08-08 23:19:11 +02:00
Copy link

If one is defined but not the other, you'll get an exception here.

If one is defined but not the other, you'll get an exception here.
netbox_agent/ipmi.py
@ -0,0 +2,4 @@
import subprocess
class Ipmi():
ramnes (Migrated from github.com) commented 2019-08-08 23:44:15 +02:00
Copy link

Abbreviations should be in capital letters

Abbreviations should be in capital letters
netbox_agent/ipmi.py
@ -0,0 +31,4 @@
: o=OPERATOR
: a=ADMIN
: O=OEM
Bad Password Threshold : Not Available
ramnes (Migrated from github.com) commented 2019-08-08 23:22:58 +02:00
Copy link

Can't you use ipmitool -c so that you don't have to manually parse the output?

Can't you use `ipmitool -c` so that you don't have to manually parse the output?
netbox_agent/ipmi.py
@ -0,0 +42,4 @@
ret = {}
if self.ret != 0:
return ret
for line in self.output.split('\n'):
ramnes (Migrated from github.com) commented 2019-08-08 23:43:09 +02:00
Copy link

Good case for str.splitlines()

Good case for `str.splitlines()`
netbox_agent/network.py
ramnes (Migrated from github.com) commented 2019-08-08 23:51:05 +02:00
Copy link

Use a generator comprehension (i.e. use parentheses rather than brackets) or better, use filter

Use a generator comprehension (i.e. use parentheses rather than brackets) or better, use `filter`
netbox_agent/network.py
@ -53,3 +77,3 @@
nic = {
'name': interface,
'mac': open('/sys/class/net/{}/address'.format(interface), 'r').read().strip(),
'mac': mac if mac != '00:00:00:00:00:00' else None,
ramnes (Migrated from github.com) commented 2019-08-08 23:31:51 +02:00
Copy link

Useless else here

Useless `else` here
netbox_agent/network.py
@ -69,0 +123,4 @@
interface = nb.dcim.interfaces.get(
device_id=self.device.id,
mac_address=nic['mac'],
name=nic['name'],
ramnes (Migrated from github.com) commented 2019-08-09 00:01:21 +02:00
Copy link

You could do something like

for slave_int in (self.get_netbox_network_card(slave_nic)
                  for slave_nic in self.nics
                  if slave_nic["name"] in nic["bonding_slaves"]):

and/or split it in several lines for readability

You could do something like ```python for slave_int in (self.get_netbox_network_card(slave_nic) for slave_nic in self.nics if slave_nic["name"] in nic["bonding_slaves"]): ``` and/or split it in several lines for readability
netbox_agent/network.py
@ -148,3 +327,2 @@
mac_address=nic['mac'],
)
interface = self.get_netbox_network_card(nic)
if not interface:
ramnes (Migrated from github.com) commented 2019-08-09 00:05:28 +02:00
Copy link

adress = str(...) would be cleaner, I guess?

`adress = str(...)` would be cleaner, I guess?
Solvik (Migrated from github.com) reviewed 2019-08-09 12:15:34 +02:00
netbox_agent/ipmi.py
@ -0,0 +31,4 @@
: o=OPERATOR
: a=ADMIN
: O=OEM
Bad Password Threshold : Not Available
Solvik (Migrated from github.com) commented 2019-08-09 12:15:34 +02:00
Copy link

doesn't work with lan print

doesn't work with `lan print`
Solvik (Migrated from github.com) reviewed 2019-08-09 12:16:12 +02:00
README.md
@ -19,3 +28,4 @@
Since it uses `ethtool` and parses `/sys/` directory, it's not compatible with *BSD distributions.
* Netbox `>=2.6.0,<=2.6.2` has a caching problem ; if the cache lifetime is too high, the script can get stale data after modification.
We advise to set `CACHE_TIME` to `0`.
Solvik (Migrated from github.com) commented 2019-08-09 12:16:12 +02:00
Copy link

There's a few issue on the netbox project to fix the cache invalidation upon modification
Not our case to fix

There's a few issue on the netbox project to fix the cache invalidation upon modification Not our case to fix
ramnes (Migrated from github.com) reviewed 2019-08-09 12:16:46 +02:00
README.md
@ -19,3 +28,4 @@
Since it uses `ethtool` and parses `/sys/` directory, it's not compatible with *BSD distributions.
* Netbox `>=2.6.0,<=2.6.2` has a caching problem ; if the cache lifetime is too high, the script can get stale data after modification.
We advise to set `CACHE_TIME` to `0`.
ramnes (Migrated from github.com) commented 2019-08-09 12:16:46 +02:00
Copy link

jup right

jup right
Solvik (Migrated from github.com) reviewed 2019-08-09 12:19:18 +02:00
README.md
@ -16,1 +22,4 @@
- [python3-netaddr](https://github.com/drkjam/netaddr)
- [python3-netifaces](https://github.com/al45tair/netifaces)
# Known limitations
Solvik (Migrated from github.com) commented 2019-08-09 12:19:17 +02:00
Copy link

Good idea!

Good idea!
Solvik (Migrated from github.com) reviewed 2019-08-09 12:19:38 +02:00
netbox_agent/ipmi.py
@ -0,0 +42,4 @@
ret = {}
if self.ret != 0:
return ret
for line in self.output.split('\n'):
Solvik (Migrated from github.com) commented 2019-08-09 12:19:38 +02:00
Copy link

Will fix

Will fix
Solvik (Migrated from github.com) reviewed 2019-08-09 12:20:44 +02:00
netbox_agent/network.py
@ -148,3 +327,2 @@
mac_address=nic['mac'],
)
interface = self.get_netbox_network_card(nic)
if not interface:
Solvik (Migrated from github.com) commented 2019-08-09 12:20:43 +02:00
Copy link

Will fix

Will fix
Solvik (Migrated from github.com) reviewed 2019-08-09 12:24:29 +02:00
netbox_agent/network.py
@ -53,3 +77,3 @@
nic = {
'name': interface,
'mac': open('/sys/class/net/{}/address'.format(interface), 'r').read().strip(),
'mac': mac if mac != '00:00:00:00:00:00' else None,
Solvik (Migrated from github.com) commented 2019-08-09 12:24:28 +02:00
Copy link

Good catch, will fix

Good catch, will fix
Solvik (Migrated from github.com) reviewed 2019-08-09 12:24:39 +02:00
netbox_agent/ipmi.py
@ -0,0 +2,4 @@
import subprocess
class Ipmi():
Solvik (Migrated from github.com) commented 2019-08-09 12:24:39 +02:00
Copy link

Will fix

Will fix
Solvik (Migrated from github.com) reviewed 2019-08-09 12:25:36 +02:00
netbox_agent/config.py
@ -33,0 +35,4 @@
NETWORK_IGNORE_IPS = None
if config.get('network'):
NETWORK_IGNORE_INTERFACES = config['network']['ignore_interfaces']
NETWORK_IGNORE_IPS = config['network']['ignore_ips']
Solvik (Migrated from github.com) commented 2019-08-09 12:25:36 +02:00
Copy link

Good catch

Good catch
ramnes (Migrated from github.com) reviewed 2019-08-09 12:30:55 +02:00
netbox_agent/ipmi.py
@ -0,0 +31,4 @@
: o=OPERATOR
: a=ADMIN
: O=OEM
Bad Password Threshold : Not Available
ramnes (Migrated from github.com) commented 2019-08-09 12:30:55 +02:00
Copy link

tristesse

tristesse
Solvik (Migrated from github.com) reviewed 2019-08-09 12:31:54 +02:00
netbox_agent/network.py
Solvik (Migrated from github.com) commented 2019-08-09 12:31:54 +02:00
Copy link

I didn't even know about generator comprehension, thansk a lot !!
Will fixx

I didn't even know about generator comprehension, thansk a lot !! Will fixx
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
help wanted

Extra attention is needed

  
need testing

   
status: accepted

   
status: discussing

   
type: bug

Something isn't working

  
type: documentation

Improvements or additions to documentation

  
type: duplicate

This issue or pull request already exists

  
type: enhancement

New feature or request

  
type:hardware

Mostly new hardware

  
type: invalid

This doesn't seem right

  
type: major feature

   
type: minor feature
  
bug

Something is not working

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
help wanted

Need some help

  
invalid

Something is wrong

  
question

More information is needed

  
wontfix

This won't be fixed

No labels
help wanted
need testing
status: accepted
status: discussing
type: bug
type: documentation
type: duplicate
type: enhancement
type:hardware
type: invalid
type: major feature
type: minor feature
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
DGNum/netbox-agent!21
No description provided.