Use hexhyp instead of hexraw iPXE type

This patch is using hexhyp instead of hexraw for the type in the iPXE
script (boot.ipxe) since hexraw is not supported on older versions
of iPXE.

When creating a link between the configuration file and the node's mac
addresses, we make sure that we generate the 2 versions of it one using
the dash as a delimiter (hexhyp) and one without delimiter (hexraw)
for backwards compatibility.

Closes-Bug: #1441710
Change-Id: I19be3a04e7d53eeb45a67908e34884935dc84a91

3 files changed, 61 insertions, 11 deletions

diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py
index a125da034..09528a218 100644
--- a/ironic/common/pxe_utils.py
+++ b/ironic/common/pxe_utils.py
@@ -80,12 +80,20 @@ def _link_mac_pxe_configs(task):
     :param task: A TaskManager instance.
 
     """
-    pxe_config_file_path = get_pxe_config_file_path(task.node.uuid)
-    for mac in driver_utils.get_node_mac_addresses(task):
-        mac_path = _get_pxe_mac_path(mac)
+
+    def create_link(mac_path):
         utils.unlink_without_raise(mac_path)
         utils.create_link_without_raise(pxe_config_file_path, mac_path)
 
+    pxe_config_file_path = get_pxe_config_file_path(task.node.uuid)
+    for mac in driver_utils.get_node_mac_addresses(task):
+        create_link(_get_pxe_mac_path(mac))
+        # TODO(lucasagomes): Backward compatibility with :hexraw,
+        # to be removed in M.
+        # see: https://bugs.launchpad.net/ironic/+bug/1441710
+        if CONF.pxe.ipxe_enabled:
+            create_link(_get_pxe_mac_path(mac, delimiter=''))
+
 
 def _link_ip_address_pxe_configs(task):
     """Link each IP address with the PXE configuration file.
@@ -110,17 +118,20 @@ def _link_ip_address_pxe_configs(task):
                                          ip_address_path)
 
 
-def _get_pxe_mac_path(mac):
+def _get_pxe_mac_path(mac, delimiter=None):
     """Convert a MAC address into a PXE config file name.
 
     :param mac: A MAC address string in the format xx:xx:xx:xx:xx:xx.
+    :param delimiter: The MAC address delimiter. Defaults to dash ('-').
     :returns: the path to the config file.
 
     """
-    if CONF.pxe.ipxe_enabled:
-        mac_file_name = mac.replace(':', '').lower()
-    else:
-        mac_file_name = "01-" + mac.replace(":", "-").lower()
+    if delimiter is None:
+        delimiter = '-'
+
+    mac_file_name = mac.replace(':', delimiter).lower()
+    if not CONF.pxe.ipxe_enabled:
+        mac_file_name = '01-' + mac_file_name
 
     return os.path.join(get_root_dir(), PXE_CFG_DIR_NAME, mac_file_name)
 
@@ -221,6 +232,12 @@ def clean_up_pxe_config(task):
     else:
         for mac in driver_utils.get_node_mac_addresses(task):
             utils.unlink_without_raise(_get_pxe_mac_path(mac))
+            # TODO(lucasagomes): Backward compatibility with :hexraw,
+            # to be removed in M.
+            # see: https://bugs.launchpad.net/ironic/+bug/1441710
+            if CONF.pxe.ipxe_enabled:
+                utils.unlink_without_raise(_get_pxe_mac_path(mac,
+                                           delimiter=''))
 
     utils.rmtree_without_raise(os.path.join(get_root_dir(),
                                             task.node.uuid))
diff --git a/ironic/drivers/modules/boot.ipxe b/ironic/drivers/modules/boot.ipxe
index 25a0ea8dc..3567dc029 100644
--- a/ironic/drivers/modules/boot.ipxe
+++ b/ironic/drivers/modules/boot.ipxe
@@ -1,7 +1,7 @@
 #!ipxe
 
 # load the MAC-specific file or fail if it's not found
-chain --autofree pxelinux.cfg/${mac:hexraw} || goto error_no_config
+chain --autofree pxelinux.cfg/${mac:hexhyp} || goto error_no_config
 
 :error_no_config
 echo PXE boot failed. No configuration found for MAC ${mac}
diff --git a/ironic/tests/test_pxe_utils.py b/ironic/tests/test_pxe_utils.py
index f16cd3e1b..1792d840a 100644
--- a/ironic/tests/test_pxe_utils.py
+++ b/ironic/tests/test_pxe_utils.py
@@ -144,7 +144,40 @@ class TestPXEUtils(db_base.DbTestCase):
         ]
         unlink_calls = [
             mock.call('/tftpboot/pxelinux.cfg/01-00-11-22-33-44-55-66'),
-            mock.call('/tftpboot/pxelinux.cfg/01-00-11-22-33-44-55-67')
+            mock.call('/tftpboot/pxelinux.cfg/01-00-11-22-33-44-55-67'),
+        ]
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            pxe_utils._link_mac_pxe_configs(task)
+
+        unlink_mock.assert_has_calls(unlink_calls)
+        create_link_mock.assert_has_calls(create_link_calls)
+
+    @mock.patch('ironic.common.utils.create_link_without_raise', autospec=True)
+    @mock.patch('ironic.common.utils.unlink_without_raise', autospec=True)
+    @mock.patch('ironic.drivers.utils.get_node_mac_addresses', autospec=True)
+    def test__write_mac_ipxe_configs(self, get_macs_mock, unlink_mock,
+                                     create_link_mock):
+        self.config(ipxe_enabled=True, group='pxe')
+        macs = [
+            '00:11:22:33:44:55:66',
+            '00:11:22:33:44:55:67'
+        ]
+        get_macs_mock.return_value = macs
+        create_link_calls = [
+            mock.call(u'/httpboot/1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config',
+                      '/httpboot/pxelinux.cfg/00-11-22-33-44-55-66'),
+            mock.call(u'/httpboot/1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config',
+                      '/httpboot/pxelinux.cfg/00112233445566'),
+            mock.call(u'/httpboot/1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config',
+                      '/httpboot/pxelinux.cfg/00-11-22-33-44-55-67'),
+            mock.call(u'/httpboot/1be26c0b-03f2-4d2e-ae87-c02d7f33c123/config',
+                      '/httpboot/pxelinux.cfg/00112233445567'),
+        ]
+        unlink_calls = [
+            mock.call('/httpboot/pxelinux.cfg/00-11-22-33-44-55-66'),
+            mock.call('/httpboot/pxelinux.cfg/00112233445566'),
+            mock.call('/httpboot/pxelinux.cfg/00-11-22-33-44-55-67'),
+            mock.call('/httpboot/pxelinux.cfg/00112233445567'),
         ]
         with task_manager.acquire(self.context, self.node.uuid) as task:
             pxe_utils._link_mac_pxe_configs(task)
@@ -218,7 +251,7 @@ class TestPXEUtils(db_base.DbTestCase):
         self.config(ipxe_enabled=True, group='pxe')
         self.config(http_root='/httpboot', group='pxe')
         mac = '00:11:22:33:AA:BB:CC'
-        self.assertEqual('/httpboot/pxelinux.cfg/00112233aabbcc',
+        self.assertEqual('/httpboot/pxelinux.cfg/00-11-22-33-aa-bb-cc',
                          pxe_utils._get_pxe_mac_path(mac))
 
     def test__get_pxe_ip_address_path(self):