Fix late comments (#29)

* more fixes
* add external binary requirements
* better error handling for bonding

README.md

@@ -22,6 +22,10 @@ The goal is to generate an existing infrastructure on Netbox and have the abilit
 - [python3-netaddr](https://github.com/drkjam/netaddr)
 - [python3-netifaces](https://github.com/al45tair/netifaces)
 
+- ethtool
+- dmidecode
+- ipmitool
+
 # Known limitations
 
 * The project is only compatible with Linux.

netbox_agent/config.py

@@ -34,5 +34,5 @@ if config.get('rack_location'):
 NETWORK_IGNORE_INTERFACES = None
 NETWORK_IGNORE_IPS = None
 if config.get('network'):
-    NETWORK_IGNORE_INTERFACES = config['network']['ignore_interfaces']
+    NETWORK_IGNORE_INTERFACES = config['network'].get('ignore_interfaces')
-    NETWORK_IGNORE_IPS = config['network']['ignore_ips']
+    NETWORK_IGNORE_IPS = config['network'].get('ignore_ips')

netbox_agent/ipmi.py

@@ -2,7 +2,7 @@ import logging
 import subprocess
 
 
-class Ipmi():
+class IPMI():
     """
     Parse IPMI output
     ie:
@@ -42,7 +42,7 @@ class Ipmi():
         ret = {}
         if self.ret != 0:
             return ret
-        for line in self.output.split('\n'):
+        for line in self.output.splitlines():
             key = line.split(':')[0].strip()
             value = ':'.join(line.split(':')[1:]).strip()
             ret[key] = value

netbox_agent/network.py

@@ -9,7 +9,7 @@ import netifaces
 from netbox_agent.config import netbox_instance as nb
 from netbox_agent.config import NETWORK_IGNORE_INTERFACES, NETWORK_IGNORE_IPS
 from netbox_agent.ethtool import Ethtool
-from netbox_agent.ipmi import Ipmi
+from netbox_agent.ipmi import IPMI
 
 IFACE_TYPE_100ME_FIXED = 800
 IFACE_TYPE_1GE_FIXED = 1000
@@ -56,58 +56,59 @@ class Network():
                re.match(NETWORK_IGNORE_INTERFACES, interface):
                 logging.debug('Ignore interface {interface}'.format(interface=interface))
                 continue
-            else:
-                ip_addr = netifaces.ifaddresses(interface).get(netifaces.AF_INET)
-                if NETWORK_IGNORE_IPS and ip_addr:
-                    for i, ip in enumerate(ip_addr):
-                        if re.match(NETWORK_IGNORE_IPS, ip['addr']):
-                            ip_addr.pop(i)
 
-                mac = open('/sys/class/net/{}/address'.format(interface), 'r').read().strip()
+            ip_addr = netifaces.ifaddresses(interface).get(netifaces.AF_INET)
-                vlan = None
+            if NETWORK_IGNORE_IPS and ip_addr:
-                if len(interface.split('.')) > 1:
+                for i, ip in enumerate(ip_addr):
-                    vlan = int(interface.split('.')[1])
+                    if re.match(NETWORK_IGNORE_IPS, ip['addr']):
-                bonding = False
+                        ip_addr.pop(i)
-                bonding_slaves = []
+
-                if os.path.isdir('/sys/class/net/{}/bonding'.format(interface)):
+            mac = open('/sys/class/net/{}/address'.format(interface), 'r').read().strip()
-                    bonding = True
+            vlan = None
-                    bonding_slaves = open(
+            if len(interface.split('.')) > 1:
-                        '/sys/class/net/{}/bonding/slaves'.format(interface)
+                vlan = int(interface.split('.')[1])
-                    ).read().split()
+            bonding = False
-                nic = {
+            bonding_slaves = []
-                    'name': interface,
+            if os.path.isdir('/sys/class/net/{}/bonding'.format(interface)):
-                    'mac': mac if mac != '00:00:00:00:00:00' else None,
+                bonding = True
-                    'ip': [
+                bonding_slaves = open(
-                        '{}/{}'.format(
+                    '/sys/class/net/{}/bonding/slaves'.format(interface)
-                            x['addr'],
+                ).read().split()
-                            IPAddress(x['netmask']).netmask_bits()
+            nic = {
-                        ) for x in ip_addr
+                'name': interface,
-                        ] if ip_addr else None,  # FIXME: handle IPv6 addresses
+                'mac': mac if mac != '00:00:00:00:00:00' else None,
-                    'ethtool': Ethtool(interface).parse(),
+                'ip': [
-                    'vlan': vlan,
+                    '{}/{}'.format(
-                    'bonding': bonding,
+                        x['addr'],
-                    'bonding_slaves': bonding_slaves,
+                        IPAddress(x['netmask']).netmask_bits()
-                }
+                    ) for x in ip_addr
-                self.nics.append(nic)
+                    ] if ip_addr else None,  # FIXME: handle IPv6 addresses
+                'ethtool': Ethtool(interface).parse(),
+                'vlan': vlan,
+                'bonding': bonding,
+                'bonding_slaves': bonding_slaves,
+            }
+            self.nics.append(nic)
 
     def _set_bonding_interfaces(self):
-        bonding_nics = [x for x in self.nics if x['bonding']]
+        bonding_nics = (x for x in self.nics if x['bonding'])
-        if not len(bonding_nics):
-            return False
-        logging.debug('Setting bonding interfaces..')
         for nic in bonding_nics:
             bond_int = self.get_netbox_network_card(nic)
             logging.debug('Setting slave interface for {name}'.format(
                 name=bond_int.name
             ))
-            for slave in nic['bonding_slaves']:
+            for slave_int in (
-                slave_nic = next(item for item in self.nics if item['name'] == slave)
+                    self.get_netbox_network_card(slave_nic)
-                slave_int = self.get_netbox_network_card(slave_nic)
+                    for slave_nic in self.nics
-                logging.debug('Settting interface {name} as slave of {master}'.format(
+                    if slave_nic['name'] in nic['bonding_slaves']):
-                    name=slave_int.name, master=bond_int.name
+                if slave_int.lag is None or slave_int.lag.id != bond_int.id:
-                ))
+                    logging.debug('Settting interface {name} as slave of {master}'.format(
-                slave_int.lag = bond_int
+                        name=slave_int.name, master=bond_int.name
-                slave_int.save()
+                    ))
+                    slave_int.lag = bond_int
+                    slave_int.save()
+        else:
+            return False
         return True
 
     def get_network_cards(self):
@@ -149,7 +150,7 @@ class Network():
         return IFACE_TYPE_OTHER
 
     def get_ipmi(self):
-        ipmi = Ipmi().parse()
+        ipmi = IPMI().parse()
         return ipmi
 
     def get_netbox_ipmi(self):
@@ -199,7 +200,7 @@ class Network():
         ip = ipmi['IP Address']
         netmask = ipmi['Subnet Mask']
         vlan = int(ipmi['802.1q VLAN ID']) if ipmi['802.1q VLAN ID'] != 'Disabled' else None
-        address = IPNetwork('{}/{}'.format(ip, netmask)).__str__()
+        address = str(IPNetwork('{}/{}'.format(ip, netmask)))
 
         interface = nb.dcim.interfaces.get(
             device_id=self.device.id,