Merge lp:~lamont/maas/bug-1645912-2.1b into lp:maas/2.1

[LaMont Jones]

Backport r5710 from trunk: Have ipmi_autodetect wrap IPv6 literals in brackets. Parse those correctly.

Backport r5710 from trunk: Have ipmi_autodetect wrap IPv6 literals in brackets. Parse those correctly.

[Mike Pontillo (mpontillo)]

LGTM.

review: Approve

=== modified file 'src/maasserver/models/bmc.py'
--- src/maasserver/models/bmc.py	2016-06-04 00:21:58 +0000
+++ src/maasserver/models/bmc.py	2017-02-10 15:17:08 +0000
@@ -93,7 +93,10 @@
         parameters has changed. """
         new_ip = BMC.extract_ip_address(self.power_type, self.power_parameters)
         current_ip = None if self.ip_address is None else self.ip_address.ip
-        # Set the ip_address field.
+        # Set the ip_address field.  If we have a bracketed address, assume
+        # it's IPv6, and strip the brackets.
+        if new_ip and new_ip.startswith('[') and new_ip.endswith(']'):
+            new_ip = new_ip[1:-1]
         if new_ip != current_ip:
             if new_ip is None:
                 self.ip_address = None

=== modified file 'src/maasserver/models/tests/test_bmc.py'
--- src/maasserver/models/tests/test_bmc.py	2016-09-23 01:54:50 +0000
+++ src/maasserver/models/tests/test_bmc.py	2017-02-10 15:17:08 +0000
@@ -97,6 +97,30 @@
         self.assertEqual(sticky_ip.ip, bmc.ip_address.ip)
         self.assertEqual(subnet, bmc.ip_address.subnet)
 
+    def test_bmc_save_accepts_naked_ipv6_address(self):
+        subnet = factory.make_Subnet(cidr=factory.make_ipv6_network())
+        sticky_ip = factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.STICKY, subnet=subnet)
+        power_parameters = {
+            'power_address': '%s' % sticky_ip.ip,
+        }
+        bmc = factory.make_BMC(
+            power_type="ipmi", power_parameters=power_parameters)
+        self.assertEqual(sticky_ip.ip, bmc.ip_address.ip)
+        self.assertEqual(subnet, bmc.ip_address.subnet)
+
+    def test_bmc_save_accepts_bracketed_ipv6_address(self):
+        subnet = factory.make_Subnet(cidr=factory.make_ipv6_network())
+        sticky_ip = factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.STICKY, subnet=subnet)
+        power_parameters = {
+            'power_address': '[%s]' % sticky_ip.ip,
+        }
+        bmc = factory.make_BMC(
+            power_type="ipmi", power_parameters=power_parameters)
+        self.assertEqual(sticky_ip.ip, bmc.ip_address.ip)
+        self.assertEqual(subnet, bmc.ip_address.subnet)
+
     def test_bmc_changing_power_parameters_changes_ip(self):
         ip = factory.make_ipv4_address()
         power_parameters = {

=== modified file 'src/provisioningserver/templates/commissioning-user-data/snippets/maas_ipmi_autodetect.py'
--- src/provisioningserver/templates/commissioning-user-data/snippets/maas_ipmi_autodetect.py	2016-09-23 16:21:57 +0000
+++ src/provisioningserver/templates/commissioning-user-data/snippets/maas_ipmi_autodetect.py	2017-02-10 15:17:08 +0000
@@ -142,14 +142,36 @@
     bmc_set('Lan_Conf:IP_Address_Source', source)
 
 
+def _bmc_get_ipmi_addresses(address_type):
+    try:
+        return bmc_get(address_type)
+    except subprocess.CalledProcessError:
+        return ""
+
+
 def get_ipmi_ip_address():
-    output = bmc_get('Lan_Conf:IP_Address')
     show_re = re.compile(
         '((?:[0-9]{1,3}\.){3}[0-9]{1,3}|[0-9a-fA-F]*:[0-9a-fA-F:.]+)')
-    res = show_re.search(output)
-    if res is None:
-        return None
-    return res.group()
+    for address_type in [
+            'Lan_Conf:IP_Address',
+            'Lan6_Conf:IPv6_Static_Addresses',
+            'Lan6_Conf:IPv6_Dynamic_Addresses']:
+        output = _bmc_get_ipmi_addresses(address_type)
+        # Loop through the addreses by preference: IPv4, static IPv6, dynamic
+        # IPv6.  Return the first valid, non-link-local address we find.
+        # While we could conceivably allow link-local addresses, we would need
+        # to devine which of our interfaces is the correct link, and then we
+        # would need support for link-local addresses in freeipmi-tools.
+        res = show_re.findall(output)
+        for ip in res:
+            if ip.lower().startswith('fe80::') or ip == '0.0.0.0':
+                time.sleep(2)
+                continue
+            if address_type.startswith('Lan6_'):
+                return '[%s]' % ip
+            return ip
+    # No valid IP address was found.
+    return None
 
 
 def verify_ipmi_user_settings(user_number, user_settings):
@@ -281,15 +303,15 @@
 
     # get the IP address
     IPMI_IP_ADDRESS = get_ipmi_ip_address()
-    if IPMI_IP_ADDRESS == "0.0.0.0":
-        # if IPMI_IP_ADDRESS is 0.0.0.0, wait 60 seconds and retry.
+    if IPMI_IP_ADDRESS is None:
+        # if IPMI_IP_ADDRESS not set (or reserved), wait 60 seconds and retry.
         set_ipmi_network_source("Static")
         time.sleep(2)
         set_ipmi_network_source("Use_DHCP")
         time.sleep(60)
         IPMI_IP_ADDRESS = get_ipmi_ip_address()
 
-    if IPMI_IP_ADDRESS is None or IPMI_IP_ADDRESS == "0.0.0.0":
+    if IPMI_IP_ADDRESS is None:
         # Exit (to not set power params in MAAS) if no IPMI_IP_ADDRESS
         # has been detected
         exit(1)

=== modified file 'src/provisioningserver/templates/commissioning-user-data/snippets/tests/test_maas_ipmi_autodetect.py'
--- src/provisioningserver/templates/commissioning-user-data/snippets/tests/test_maas_ipmi_autodetect.py	2016-09-23 19:44:23 +0000
+++ src/provisioningserver/templates/commissioning-user-data/snippets/tests/test_maas_ipmi_autodetect.py	2017-02-10 15:17:08 +0000
@@ -497,19 +497,74 @@
 
     scenarios = [
         ('none', dict(
-            output='  IP_Address    \n\n', expected=None)),
+            output4='  IP_Address    \n\n',
+            output_st='', output_dy='', expected=None)),
         ('bogus', dict(
-            output='  IP_Address    bogus\n\n', expected=None)),
+            output4='  IP_Address    bogus\n\n',
+            output_st='', output_dy='', expected=None)),
         ('ipv4', dict(
-            output='  IP_Address    192.168.1.1\n\n', expected='192.168.1.1')),
-        ('ipv6', dict(
-            output='  IP_Address    2001:db8::3\n\n', expected='2001:db8::3')),
+            output4='  IP_Address    192.168.1.1\n\n',
+            output_st='', output_dy='', expected='192.168.1.1')),
+        ('ipv4-ipv6', dict(
+            output4='  IP_Address    192.168.1.1\n\n',
+            output_st='  IPv6_Static_Addresses 2001:db8::3\n\n',
+            output_dy='', expected='192.168.1.1')),
+        ('static6', dict(
+            output4='  IP_Address    0.0.0.0\n\n',
+            output_st='  IPv6_Static_Addresses 2001:db8::3\n\n',
+            output_dy='',
+            expected='[2001:db8::3]')),
+        ('static6 multiple', dict(
+            output4='  IP_Address    0.0.0.0\n\n',
+            output_st='  IPv6_Static_Addresses fe80::3:7 2001:db8::3\n\n',
+            output_dy='',
+            expected='[2001:db8::3]')),
+        ('mixed6', dict(
+            output4='  IP_Address    0.0.0.0\n\n',
+            output_st='  IPv6_Static_Addresses 2001:db8::9:5\n\n',
+            output_dy='  ## IPv6_Dynamic_Addresses 2001:db8::3\n\n',
+            expected='[2001:db8::9:5]')),
+        ('dynamic6', dict(
+            output4='  IP_Address    0.0.0.0\n\n',
+            output_st='',
+            output_dy='  ## IPv6_Dynamic_Addresses 2001:db8::3\n\n',
+            expected='[2001:db8::3]')),
+        ('dynamic6 with link-local', dict(
+            output4='  IP_Address    0.0.0.0\n\n',
+            output_st='',
+            output_dy='  ## IPv6_Dynamic_Addresses fe80::3:7 2001:db8::3\n\n',
+            expected='[2001:db8::3]')),
+        ('dynamic6 multiple', dict(
+            output4='  IP_Address    0.0.0.0\n\n',
+            output_st='',
+            output_dy=(
+                '  ## IPv6_Dynamic_Addresses fe80::7 2001:db8::3 2001::5\n\n'
+            ),
+            expected='[2001:db8::3]')),
         ('link-local', dict(
-            output='  IP_Address    fe80::3:7\n\n', expected='fe80::3:7')),
+            output4='',
+            output_st='',
+            output_dy='  ## IPv6_Dynamic_IP_Addresses  fe80::7 fe80::3:9\n\n',
+            expected=None)),
+        ('0.0.0.0', dict(
+            output4='  IP_Address    0.0.0.0\n\n',
+            output_st='',
+            output_dy='',
+            expected=None)),
     ]
 
     def test_get_ipmi_ip_address(self):
-        run_command = self.patch(maas_ipmi_autodetect, 'run_command')
-        run_command.return_value = self.output
+        ret_values = {
+            'Lan_Conf:IP_Address': self.output4,
+            'Lan6_Conf:IPv6_Static_Addresses': self.output_st,
+            'Lan6_Conf:IPv6_Dynamic_Addresses': self.output_dy
+        }
+
+        def ret_val(arg):
+            return ret_values[arg]
+
+        self.patch(
+            maas_ipmi_autodetect,
+            '_bmc_get_ipmi_addresses').side_effect = ret_val
         actual = get_ipmi_ip_address()
         self.assertEqual(self.expected, actual)