Merge lp:~mpontillo/maas/bug-1508695 into lp:~maas-committers/maas/trunk

[Mike Pontillo]

Simplify logic for handling key/value pairs when parsing 'ip addr'.

Remove assert and make the parser more robust, so that it won't crash if it encounters something unexpected.

This is a follow-on to LaMont's branch here:

https://code.launchpad.net/~lamont/maas/bug-1508695

It simplifies the parser due to some follow-on issues that were encountered.

[Andres Rodriguez (andreserl)]

lgtm!

review: Approve

=== modified file 'src/provisioningserver/utils/ipaddr.py'
--- src/provisioningserver/utils/ipaddr.py	2015-10-21 18:38:54 +0000
+++ src/provisioningserver/utils/ipaddr.py	2015-10-22 00:52:03 +0000
@@ -81,11 +81,12 @@
     :return: dict
     """
     settings = settings_line.strip().split()
-    if len(settings) > 0 and settings[0] == "inet":
+    # Some of the tokens on this line aren't key/value pairs, but we don't
+    # care about those, so strip them off if we see an odd number.
+    # This will avoid an index out of bounds error below.
+    num_tokens = len(settings)
+    if num_tokens % 2 != 0:
         settings = settings[:-1]
-    num_tokens = len(settings)
-    assert num_tokens % 2 == 0, \
-        "Unexpected number of params in '%s'" % settings_line
     return {
         settings[2 * i]: settings[2 * i + 1] for i in range(num_tokens / 2)
         }
@@ -137,8 +138,8 @@
     mac = settings.get('link/ether')
     if mac is not None:
         interface['mac'] = mac
-    cumul_settings = ['inet', 'inet6']
-    for name in cumul_settings:
+    address_types = ['inet', 'inet6']
+    for name in address_types:
         value = settings.get(name)
         if value is not None:
             if not IPNetwork(value).is_link_local():

=== modified file 'src/provisioningserver/utils/tests/test_ipaddr.py'
--- src/provisioningserver/utils/tests/test_ipaddr.py	2015-10-21 18:38:54 +0000
+++ src/provisioningserver/utils/tests/test_ipaddr.py	2015-10-22 00:52:03 +0000
@@ -43,11 +43,10 @@
         settings = _get_settings_dict("")
         self.assertEqual({}, settings)
 
-    def test_get_settings_dict_asserts_for_odd_number_of_tokens(self):
-        with ExpectedException(AssertionError):
-            _get_settings_dict("mtu")
-        with ExpectedException(AssertionError):
-            _get_settings_dict("mtu 1500 qdisc")
+    def test_get_settings_dict_handles_odd_number_of_tokens(self):
+        self.assertThat(_get_settings_dict("mtu"), Equals({}))
+        self.assertThat(
+            _get_settings_dict("mtu 1500 qdisc"), Equals({"mtu": "1500"}))
 
     def test_get_settings_dict_creates_correct_dictionary(self):
         settings = _get_settings_dict("mtu 1073741824 state AWESOME")
@@ -260,6 +259,20 @@
         ip_link = parse_ip_addr(testdata)
         self.assertIsNone(ip_link['eth0'].get('inet6'))
 
+    def test_handles_wlan_flags(self):
+        testdata = dedent("""
+        2: wlan0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast \
+state UP mode DEFAULT group default qlen 1000
+            link/ether 80:fa:5c:0d:43:5e brd ff:ff:ff:ff:ff:ff
+            inet6 fe80::3e97:eff:fe0e:56dc/64 scope link
+                valid_lft forever preferred_lft forever
+            inet 192.168.2.112/24 brd 192.168.2.255 scope global dynamic wlan0
+                valid_lft forever preferred_lft forever
+        """)
+        ip_link = parse_ip_addr(testdata)
+        inet = ip_link['wlan0'].get('inet')
+        self.assertEqual(['192.168.2.112/24'], inet)
+
     def test_parses_multiple_interfaces(self):
         testdata = dedent("""
         2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast \
@@ -276,6 +289,10 @@
                 valid_lft forever preferred_lft forever
             inet6 2001:db8:85a3:8d3:1319:8a2e:370:7348/64 scope link
                 valid_lft forever preferred_lft forever
+            inet6 2001:db8:85a3:8d3:1319:8a2e:370:3645/64 scope global dynamic
+                valid_lft forever preferred_lft forever
+            inet6 2001:db8:85a3:8d3::1111/64 scope global tentative dynamic
+                valid_lft forever preferred_lft forever
             inet6 2620:1:260::1/64 scope global
                 valid_lft forever preferred_lft forever
         """)
@@ -290,7 +307,10 @@
         self.assertEquals('48:51:bb:7a:d5:e2', ip_link['eth1']['mac'])
         self.assertEquals(['192.168.0.5/24'], ip_link['eth1']['inet'])
         self.assertEquals(
-            ['2001:db8:85a3:8d3:1319:8a2e:370:7348/64', '2620:1:260::1/64'],
+            ['2001:db8:85a3:8d3:1319:8a2e:370:7348/64',
+             '2001:db8:85a3:8d3:1319:8a2e:370:3645/64',
+             '2001:db8:85a3:8d3::1111/64',
+             '2620:1:260::1/64'],
             ip_link['eth1']['inet6'])