Merge lp:~jtv/maas/netifaces into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

Replace ifconfig-parser with netifaces. We need this to avoid fork()ing off ifconfig, which confuses Upstart's fork tracking.

Briefly discussed with Julian. This one's needed to get tests passing on the branch that will make celery run as the maas user: we were forking once to get our networks information *before* the crucial fork of the celery "daemon" which we wanted upstart to track.

Jeroen

[Julian Edwards (julian-edwards)]

Nice.

review: Approve

[Gavin Panella (allenap)]

I like this too. An alternative approach, should we have problems with other forks, would be to fork, then exec celery in the *parent* and continue doing whatever it is we need to do in the child process. Then upstart wouldn't need to follow forks, and there would be no chance of it getting confused.

=== modified file 'required-packages/base'
--- required-packages/base	2012-09-22 15:38:25 +0000
+++ required-packages/base	2012-10-02 06:34:21 +0000
@@ -23,6 +23,7 @@
 python-httplib2
 python-lockfile
 python-netaddr
+python-netifaces
 python-oauth
 python-oops
 python-oops-amqp

=== modified file 'src/provisioningserver/network.py'
--- src/provisioningserver/network.py	2012-09-26 17:21:53 +0000
+++ src/provisioningserver/network.py	2012-10-02 06:34:21 +0000
@@ -18,17 +18,21 @@
     'discover_networks',
     ]
 
-import os
-from subprocess import check_output
+
+from netifaces import (
+    AF_INET,
+    ifaddresses,
+    interfaces,
+    )
 
 
 class InterfaceInfo:
     """The details of a network interface we are interested in."""
 
-    def __init__(self, interface):
+    def __init__(self, interface, ip=None, subnet_mask=None):
         self.interface = interface
-        self.ip = None
-        self.subnet_mask = None
+        self.ip = ip
+        self.subnet_mask = subnet_mask
 
     def may_be_subnet(self):
         """Could this be a subnet that MAAS is interested in?"""
@@ -51,57 +55,24 @@
         }
 
 
-def run_ifconfig():
-    """Run `ifconfig` to list active interfaces.  Return output."""
-    env = dict(os.environ, LC_ALL='C')
-    output = check_output(['/sbin/ifconfig'], env=env)
-    return output.decode('ascii')
-
-
-def extract_ip_and_subnet_mask(line):
-    """Get IP address and subnet mask from an inet address line."""
-    # This line consists of key:value pairs separated by double spaces.
-    # The "inet addr" key contains a space.  There is typically a
-    # trailing separator.
-    settings = dict(
-        tuple(pair.split(':', 1))
-        for pair in line.split('  '))
-    return settings.get('inet addr'), settings.get('Mask')
-
-
-def parse_stanza(stanza):
-    """Return a :class:`InterfaceInfo` representing this ifconfig stanza.
-
-    May return `None` if it's immediately clear that the interface is not
-    relevant for MAAS.
-    """
-    lines = [line.strip() for line in stanza.splitlines()]
-    header = lines[0]
-    if 'Link encap:Ethernet' not in header:
-        return None
-    info = InterfaceInfo(header.split()[0])
-    for line in lines[1:]:
-        if line.split()[0] == 'inet':
-            info.ip, info.subnet_mask = extract_ip_and_subnet_mask(line)
-    return info
-
-
-def split_stanzas(output):
-    """Split `ifconfig` output into stanzas, one per interface."""
-    stanzas = [stanza.strip() for stanza in output.strip().split('\n\n')]
-    return [stanza for stanza in stanzas if len(stanza) > 0]
-
-
-def parse_ifconfig(output):
-    """List `InterfaceInfo` for each interface found in `ifconfig` output."""
-    infos = [parse_stanza(stanza) for stanza in split_stanzas(output)]
-    return [info for info in infos if info is not None]
+def get_interface_info(interface):
+    """Return a list of `InterfaceInfo` for the named `interface`."""
+    addresses = ifaddresses(interface).get(AF_INET)
+    if addresses is None:
+        return []
+    else:
+        return [
+            InterfaceInfo(
+                interface, ip=address.get('addr'),
+                subnet_mask=address.get('netmask'))
+            for address in addresses]
 
 
 def discover_networks():
     """Find the networks attached to this system."""
-    output = run_ifconfig()
+    infos = sum(
+        [get_interface_info(interface) for interface in interfaces()], [])
     return [
-        interface.as_dict()
-        for interface in parse_ifconfig(output)
-            if interface.may_be_subnet()]
+        info.as_dict()
+        for info in infos
+            if info.may_be_subnet()]

=== modified file 'src/provisioningserver/tests/test_network.py'
--- src/provisioningserver/tests/test_network.py	2012-09-26 17:21:53 +0000
+++ src/provisioningserver/tests/test_network.py	2012-10-02 06:34:21 +0000
@@ -12,267 +12,79 @@
 __metaclass__ = type
 __all__ = []
 
-from random import (
-    choice,
-    randint,
-    )
-
 from maastesting.factory import factory
 from maastesting.testcase import TestCase
+from netaddr import IPNetwork
+from netifaces import AF_INET
 from provisioningserver import network
 
 
-def make_address_line(**kwargs):
-    """Create an inet address line."""
-    # First word on this line is inet or inet6.
-    kwargs.setdefault('inet', 'inet')
-    kwargs.setdefault('broadcast', '10.255.255.255')
-    kwargs.setdefault('subnet_mask', '255.0.0.0')
-    items = [
-        "%(inet)s addr:%(ip)s"
-        ]
-    if len(kwargs['broadcast']) > 0:
-        items.append("Bcast:%(broadcast)s")
-    items.append("Mask:%(subnet_mask)s")
-    return '  '.join(items) % kwargs
-
-
-def make_stats_line(direction, **kwargs):
-    """Create one of the incoming/outcoming packet-count lines."""
-    assert direction in {'RX', 'TX'}
-    if direction == 'RX':
-        variable_field = 'frame'
-    else:
-        variable_field = 'carrier'
-    kwargs.setdefault('variable_field', variable_field)
-    kwargs.setdefault('packets', randint(0, 100000))
-    kwargs.setdefault('errors', randint(0, 100))
-    kwargs.setdefault('dropped', randint(0, 100))
-    kwargs.setdefault('overruns', randint(0, 100))
-    kwargs.setdefault('variable', randint(0, 100))
-
-    return " ".join([
-        direction,
-        "packets:%(packets)d",
-        "errors:%(errors)d",
-        "dropped:%(dropped)d",
-        "overruns:%(overruns)d",
-        "%(variable_field)s:%(variable)d"
-        ]) % kwargs
-
-
-def make_payload_stats(direction, **kwargs):
-    assert direction in {'RX', 'TX'}
-    kwargs.setdefault('bytes', randint(0, 1000000))
-    kwargs.setdefault('bigger_unit', randint(10, 10240) / 10.0)
-    kwargs.setdefault('unit', choice(['B', 'KB', 'GB']))
-    return " ".join([
-        direction,
-        "bytes:%(bytes)s",
-        "(%(bigger_unit)d %(unit)s)",
-        ]) % kwargs
-
-
-def make_stanza(**kwargs):
-    """Create an ifconfig output stanza.
-
-    Variable values can be specified, but will be given random values by
-    default.  Values that interfaces may not have, such as broadcast
-    address or allocated interrupt, may be set to the empty string to
-    indicate that they should be left out of the output.
-    """
-    kwargs.setdefault('interface', factory.make_name('eth'))
-    kwargs.setdefault('encapsulation', 'Ethernet')
-    kwargs.setdefault('mac', factory.getRandomMACAddress())
-    kwargs.setdefault('ip', factory.getRandomIPAddress())
-    kwargs.setdefault('broadcast', factory.getRandomIPAddress())
-    kwargs.setdefault('mtu', randint(100, 10000))
-    kwargs.setdefault('rxline', make_stats_line('RX', **kwargs))
-    kwargs.setdefault('txline', make_stats_line('TX', **kwargs))
-    kwargs.setdefault('collisions', randint(0, 100))
-    kwargs.setdefault('txqueuelen', randint(0, 100))
-    kwargs.setdefault('rxbytes', make_payload_stats('RX', **kwargs))
-    kwargs.setdefault('txbytes', make_payload_stats('TX', **kwargs))
-    kwargs.setdefault('interrupt', randint(1, 30))
-
-    # The real-life output seems to have two trailing spaces here.
-    header = "%(interface)s Link encap:%(encapsulation)s  HWaddr %(mac)s  "
-    body_lines = [
-        "UP BROADCAST MULTICAST  MTU:%(mtu)d  Metric:1",
-        ]
-    if len(kwargs['ip']) > 0:
-        body_lines.append(make_address_line(inet='inet', **kwargs))
-    body_lines += [
-        "%(rxline)s",
-        "%(txline)s",
-        # This line has a trailing space in the real-life output.
-        "collisions:%(collisions)d txqueuelen:%(txqueuelen)d ",
-        "%(rxbytes)s  %(txbytes)s",
-        ]
-    if kwargs['interrupt'] != '':
-        body_lines.append("Interrupt:%(interrupt)d")
-
-    text = '\n'.join(
-        [header] +
-        [(10 * " ") + line for line in body_lines])
-    return (text + "\n") % kwargs
-
-
-def join_stanzas(stanzas):
-    """Format a sequence of interface stanzas like ifconfig does."""
-    return '\n'.join(stanzas) + '\n'
-
-
-# Tragically can't afford to indent and then dedent() this.  This output
-# isn't entirely realistic: the real output has trailing spaces here and
-# there, which we don't tolerate in our source code.
-sample_output = """\
-eth0      Link encap:Ethernet  HWaddr 00:25:bc:e6:0b:c2
-          UP BROADCAST MULTICAST  MTU:1500  Metric:1
-          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
-          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
-          collisions:0 txqueuelen:1000
-          RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)
-
-eth1      Link encap:Ethernet  HWaddr 00:14:73:ad:29:62
-          inet addr:192.168.12.103  Bcast:192.168.12.255  Mask:255.255.255.0
-          inet6 addr: fe81::210:9ff:fcd3:6120/64 Scope:Link
-          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
-          RX packets:5272 errors:1 dropped:0 overruns:0 frame:3274
-          TX packets:5940 errors:2 dropped:0 overruns:0 carrier:0
-          collisions:0 txqueuelen:1000
-          RX bytes:2254714 (2.2 MB)  TX bytes:4045385 (4.0 MB)
-          Interrupt:22
-
-lo        Link encap:Local Loopback
-          inet addr:127.0.0.1  Mask:255.0.0.0
-          inet6 addr: ::1/128 Scope:Host
-          UP LOOPBACK RUNNING  MTU:16436  Metric:1
-          RX packets:297493 errors:0 dropped:0 overruns:0 frame:0
-          TX packets:297493 errors:0 dropped:0 overruns:0 carrier:0
-          collisions:0 txqueuelen:0
-          RX bytes:43708 (43.7 KB)  TX bytes:43708 (43.7 KB)
-
-maasbr0   Link encap:Ethernet  HWaddr 46:a1:20:8b:77:14
-          inet addr:192.168.64.1  Bcast:192.168.64.255  Mask:255.255.255.0
-          UP BROADCAST MULTICAST  MTU:1500  Metric:1
-          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
-          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
-          collisions:0 txqueuelen:0
-          RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)
-
-virbr0    Link encap:Ethernet  HWaddr 68:14:23:c0:6d:bf
-          inet addr:192.168.80.1  Bcast:192.168.80.255  Mask:255.255.255.0
-          UP BROADCAST MULTICAST  MTU:1500  Metric:1
-          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
-          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
-          collisions:0 txqueuelen:0
-          RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)
-
-    """
+def make_inet_address(subnet=None):
+    """Fake an AF_INET address."""
+    if subnet is None:
+        subnet = factory.getRandomNetwork()
+    return {
+        'broadcast': subnet.broadcast,
+        'netmask': subnet.netmask,
+        'addr': factory.getRandomIPInNetwork(subnet),
+    }
+
+
+def make_loopback():
+    """Fake a loopback AF_INET address."""
+    return make_inet_address(IPNetwork('127.0.0.0/8'))
+
+
+def make_interface(inet_address=None):
+    """Minimally fake up an interface definition as returned by netifaces."""
+    if inet_address is None:
+        inet_address = make_inet_address()
+    return {AF_INET: [inet_address]}
 
 
 class TestNetworks(TestCase):
 
-    def test_run_ifconfig_returns_ifconfig_output(self):
-        text = join_stanzas([make_stanza()])
-        self.patch(network, 'check_output').return_value = text.encode('ascii')
-        self.assertEqual(text, network.run_ifconfig())
-
-    def test_parse_ifconfig_produces_interface_info(self):
-        num_interfaces = randint(1, 3)
-        text = join_stanzas([
-            make_stanza()
-            for counter in range(num_interfaces)])
-        info = network.parse_ifconfig(text)
-        self.assertEqual(num_interfaces, len(info))
-        self.assertIsInstance(info[0], network.InterfaceInfo)
-
-    def test_parse_stanza_reads_interface_with_ip_and_interrupt(self):
-        parms = {
-            'interface': factory.make_name('eth'),
-            'ip': factory.getRandomIPAddress(),
-            'subnet_mask': '255.255.255.128',
-        }
-        info = network.parse_stanza(make_stanza(**parms))
-        self.assertEqual(parms, info.as_dict())
-
-    def test_parse_stanza_reads_interface_without_interrupt(self):
-        parms = {
-            'interface': factory.make_name('eth'),
-            'ip': factory.getRandomIPAddress(),
-            'subnet_mask': '255.255.255.128',
-            'interrupt': '',
-        }
-        info = network.parse_stanza(make_stanza(**parms))
-        expected = parms.copy()
-        del expected['interrupt']
-        self.assertEqual(expected, info.as_dict())
-
-    def test_parse_stanza_reads_interface_without_ip(self):
-        parms = {
-            'interface': factory.make_name('eth'),
-            'ip': '',
-        }
-        info = network.parse_stanza(make_stanza(**parms))
-        expected = parms.copy()
-        expected['ip'] = None
-        expected['subnet_mask'] = None
-        self.assertEqual(expected, info.as_dict())
-
-    def test_parse_stanza_returns_nothing_for_loopback(self):
-        parms = {
-            'interface': 'lo',
-            'ip': '127.1.2.3',
-            'subnet_mask': '255.0.0.0',
-            'encapsulation': 'Local Loopback',
-            'broadcast': '',
-            'interrupt': '',
-        }
-        self.assertIsNone(network.parse_stanza(make_stanza(**parms)))
-
-    def test_split_stanzas_returns_empty_for_empty_input(self):
-        self.assertEqual([], network.split_stanzas(''))
-
-    def test_split_stanzas_returns_single_stanza(self):
-        stanza = make_stanza()
-        self.assertEqual([stanza.strip()], network.split_stanzas(stanza))
-
-    def test_split_stanzas_splits_multiple_stanzas(self):
-        stanzas = [make_stanza() for counter in range(3)]
-        full_output = join_stanzas(stanzas)
+    def patch_netifaces(self, interfaces):
+        """Patch up netifaces to pretend we have given `interfaces`.
+
+        :param interfaces: A dict mapping each interface's name to its
+            definition as `netifaces` would return it.
+        """
+        self.patch(network, 'interfaces').return_value = interfaces.keys()
+        self.patch(
+            network, 'ifaddresses', lambda interface: interfaces[interface])
+
+    def test_discover_networks_ignores_interface_without_IP_address(self):
+        self.patch_netifaces({factory.make_name('eth'): {}})
+        self.assertEqual([], network.discover_networks())
+
+    def test_discover_networks_ignores_loopback(self):
+        self.patch_netifaces({'lo': make_interface(make_loopback())})
+        self.assertEqual([], network.discover_networks())
+
+    def test_discover_networks_represents_interface(self):
+        eth = factory.make_name('eth')
+        interface = make_interface()
+        self.patch_netifaces({eth: interface})
+        self.assertEqual([{
+            'interface': eth,
+            'ip': interface[AF_INET][0]['addr'],
+            'subnet_mask': interface[AF_INET][0]['netmask'],
+            }],
+            network.discover_networks())
+
+    def test_discover_networks_returns_suitable_interfaces(self):
+        eth = factory.make_name('eth')
+        self.patch_netifaces({
+            eth: make_interface(),
+            'lo': make_interface(make_loopback()),
+            factory.make_name('dummy'): make_interface({}),
+            })
         self.assertEqual(
-            [stanza.strip() for stanza in stanzas],
-            network.split_stanzas(full_output))
+            [eth], [
+                interface['interface']
+                for interface in network.discover_networks()])
 
     def test_discover_networks_runs_in_real_life(self):
         interfaces = network.discover_networks()
         self.assertIsInstance(interfaces, list)
-
-    def test_discover_networks_returns_suitable_interfaces(self):
-        params = {
-            'interface': factory.make_name('eth'),
-            'ip': factory.getRandomIPAddress(),
-            'subnet_mask': '255.255.255.0',
-        }
-        regular_interface = make_stanza(**params)
-        loopback = make_stanza(
-            interface='lo', encapsulation='Local loopback', broadcast='',
-            interrupt='')
-        disabled_interface = make_stanza(ip='', broadcast='', subnet_mask='')
-
-        text = join_stanzas([regular_interface, loopback, disabled_interface])
-        self.patch(network, 'run_ifconfig').return_value = text
-
-        interfaces = network.discover_networks()
-
-        self.assertEqual(
-            [params],
-            [interface for interface in interfaces])
-
-    def test_discover_networks_processes_real_ifconfig_output(self):
-        self.patch(network, 'run_ifconfig').return_value = sample_output
-        info = network.discover_networks()
-        self.assertEqual(
-            ['eth1', 'maasbr0', 'virbr0'],
-            [interface['interface'] for interface in info])