From 0354940e449b6e1787321813431afcc6a552bd5c Mon Sep 17 00:00:00 2001
From: Tzu-Mainn Chen <tzumainn@redhat.com>
Date: Mon, 15 Feb 2021 16:58:23 +0000
Subject: [PATCH] Allow support for multipath volumes

Updates the generated iscsi url if the `target_portals` volume
property is set.

Change-Id: Ie9849d5dec4da50c65e2e864041e07924ae21df7
---
 ironic/common/pxe_utils.py                    | 21 +++++--
 ironic/tests/unit/common/test_pxe_utils.py    | 46 ++++++++++++++-
 ...config_boot_from_volume_multipath.template | 56 +++++++++++++++++++
 .../volume-multipath-63b96f8331e771ae.yaml    |  7 +++
 4 files changed, 121 insertions(+), 9 deletions(-)
 create mode 100644 ironic/tests/unit/drivers/ipxe_config_boot_from_volume_multipath.template
 create mode 100644 releasenotes/notes/volume-multipath-63b96f8331e771ae.yaml

diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py
index 2a78f0c3db..b05306a16d 100644
--- a/ironic/common/pxe_utils.py
+++ b/ironic/common/pxe_utils.py
@@ -861,12 +861,7 @@ def get_volume_pxe_options(task):
             return prop
         return __return_item_or_first_if_list(properties.get(key + 's', ''))
 
-    def __generate_iscsi_url(properties):
-        """Returns iscsi url."""
-        portal = __get_property(properties, 'target_portal')
-        iqn = __get_property(properties, 'target_iqn')
-        lun = __get_property(properties, 'target_lun')
-
+    def __format_portal(portal, iqn, lun):
         if ':' in portal:
             host, port = portal.split(':')
         else:
@@ -875,6 +870,20 @@ def get_volume_pxe_options(task):
         return ("iscsi:%(host)s::%(port)s:%(lun)s:%(iqn)s" %
                 {'host': host, 'port': port, 'lun': lun, 'iqn': iqn})
 
+    def __generate_iscsi_url(properties):
+        """Returns iscsi url."""
+        iqn = __get_property(properties, 'target_iqn')
+        lun = __get_property(properties, 'target_lun')
+        if 'target_portals' in properties:
+            portals = properties.get('target_portals')
+            formatted_portals = []
+            for portal in portals:
+                formatted_portals.append(__format_portal(portal, iqn, lun))
+            return ' '.join(formatted_portals)
+        else:
+            portal = __get_property(properties, 'target_portal')
+            return __format_portal(portal, iqn, lun)
+
     pxe_options = {}
     node = task.node
     boot_volume = node.driver_internal_info.get('boot_from_volume')
diff --git a/ironic/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py
index c5be259b17..ce56eb276c 100644
--- a/ironic/tests/unit/common/test_pxe_utils.py
+++ b/ironic/tests/unit/common/test_pxe_utils.py
@@ -103,6 +103,17 @@ class TestPXEUtils(db_base.DbTestCase):
             'password': 'fake_password',
         })
 
+        self.ipxe_options_boot_from_volume_multipath = \
+            self.ipxe_options.copy()
+        self.ipxe_options_boot_from_volume_multipath.update({
+            'boot_from_volume': True,
+            'iscsi_boot_url': 'iscsi:fake_host::3260:0:fake_iqn '
+                              'iscsi:faker_host::3260:0:fake_iqn',
+            'iscsi_initiator_iqn': 'fake_iqn',
+            'username': 'fake_username',
+            'password': 'fake_password',
+        })
+
         self.ipxe_options_boot_from_volume_no_extra_volume.pop(
             'initrd_filename', None)
         self.ipxe_options_boot_from_volume_extra_volume.pop(
@@ -183,6 +194,25 @@ class TestPXEUtils(db_base.DbTestCase):
 
         self.assertEqual(str(expected_template), rendered_template)
 
+    def test_default_ipxe_boot_from_volume_config_multipath(self):
+        self.config(
+            pxe_config_template='ironic/drivers/modules/ipxe_config.template',
+            group='pxe'
+        )
+        self.config(http_url='http://1.2.3.4:1234', group='deploy')
+        rendered_template = utils.render_template(
+            CONF.pxe.pxe_config_template,
+            {'pxe_options': self.ipxe_options_boot_from_volume_multipath,
+             'ROOT': '{{ ROOT }}',
+             'DISK_IDENTIFIER': '{{ DISK_IDENTIFIER }}'})
+
+        templ_file = 'ironic/tests/unit/drivers/' \
+                     'ipxe_config_boot_from_volume_multipath.template'
+        with open(templ_file) as f:
+            expected_template = f.read().rstrip()
+
+        self.assertEqual(str(expected_template), rendered_template)
+
     def test_default_ipxe_boot_from_volume_config(self):
         self.config(
             pxe_config_template='ironic/drivers/modules/ipxe_config.template',
@@ -1492,7 +1522,8 @@ class iPXEBuildConfigOptionsTestCase(db_base.DbTestCase):
                                             debug=False,
                                             boot_from_volume=False,
                                             mode='deploy',
-                                            iso_boot=False):
+                                            iso_boot=False,
+                                            multipath=False):
         self.config(debug=debug)
         self.config(pxe_append_params='test_param', group='pxe')
         self.config(ipxe_timeout=ipxe_timeout, group='pxe')
@@ -1587,7 +1618,6 @@ class iPXEBuildConfigOptionsTestCase(db_base.DbTestCase):
         if boot_from_volume:
             expected_options.update({
                 'boot_from_volume': True,
-                'iscsi_boot_url': 'iscsi:fake_host::3260:0:fake_iqn',
                 'iscsi_initiator_iqn': 'fake_iqn_initiator',
                 'iscsi_volumes': [{'url': 'iscsi:fake_host::3260:1:fake_iqn',
                                    'username': 'fake_username_1',
@@ -1596,6 +1626,15 @@ class iPXEBuildConfigOptionsTestCase(db_base.DbTestCase):
                 'username': 'fake_username',
                 'password': 'fake_password'
             })
+            if multipath:
+                expected_options.update({
+                    'iscsi_boot_url': 'iscsi:fake_host::3260:0:fake_iqn '
+                                      'iscsi:faker_host::3261:0:fake_iqn',
+                })
+            else:
+                expected_options.update({
+                    'iscsi_boot_url': 'iscsi:fake_host::3260:0:fake_iqn',
+                })
             expected_options.pop('deployment_aki_path')
             expected_options.pop('deployment_ari_path')
             expected_options.pop('initrd_filename')
@@ -1701,7 +1740,8 @@ class iPXEBuildConfigOptionsTestCase(db_base.DbTestCase):
                         'auth_username': 'fake_username_1',
                         'auth_password': 'fake_password_1'})
         self.node.driver_internal_info.update({'boot_from_volume': vol_id})
-        self._test_build_pxe_config_options_ipxe(boot_from_volume=True)
+        self._test_build_pxe_config_options_ipxe(boot_from_volume=True,
+                                                 multipath=True)
 
     def test_get_volume_pxe_options(self):
         vol_id = uuidutils.generate_uuid()
diff --git a/ironic/tests/unit/drivers/ipxe_config_boot_from_volume_multipath.template b/ironic/tests/unit/drivers/ipxe_config_boot_from_volume_multipath.template
new file mode 100644
index 0000000000..f5027b3af2
--- /dev/null
+++ b/ironic/tests/unit/drivers/ipxe_config_boot_from_volume_multipath.template
@@ -0,0 +1,56 @@
+#!ipxe
+
+set attempts:int32 10
+set i:int32 0
+
+goto deploy
+
+:deploy
+imgfree
+kernel http://1.2.3.4:1234/deploy_kernel selinux=0 troubleshoot=0 text test_param BOOTIF=${mac} initrd=deploy_ramdisk || goto retry
+
+initrd http://1.2.3.4:1234/deploy_ramdisk || goto retry
+boot
+
+:retry
+iseq ${i} ${attempts} && goto fail ||
+inc i
+echo No response, retrying in ${i} seconds.
+sleep ${i}
+goto deploy
+
+:fail
+echo Failed to get a response after ${attempts} attempts
+echo Powering off in 30 seconds.
+sleep 30
+poweroff
+
+:boot_partition
+imgfree
+kernel http://1.2.3.4:1234/kernel root={{ ROOT }} ro text test_param initrd=ramdisk || goto boot_partition
+initrd http://1.2.3.4:1234/ramdisk || goto boot_partition
+boot
+
+:boot_ramdisk
+imgfree
+kernel http://1.2.3.4:1234/kernel root=/dev/ram0 text test_param ramdisk_param initrd=ramdisk || goto boot_ramdisk
+initrd http://1.2.3.4:1234/ramdisk || goto boot_ramdisk
+boot
+
+:boot_iscsi
+imgfree
+set username fake_username
+set password fake_password
+set initiator-iqn fake_iqn
+sanhook --drive 0x80 iscsi:fake_host::3260:0:fake_iqn iscsi:faker_host::3260:0:fake_iqn || goto fail_iscsi_retry
+
+
+sanboot --no-describe || goto fail_iscsi_retry
+
+:fail_iscsi_retry
+echo Failed to attach iSCSI volume(s), retrying in 10 seconds.
+sleep 10
+goto boot_iscsi
+
+:boot_whole_disk
+sanboot --no-describe
diff --git a/releasenotes/notes/volume-multipath-63b96f8331e771ae.yaml b/releasenotes/notes/volume-multipath-63b96f8331e771ae.yaml
new file mode 100644
index 0000000000..a12520a722
--- /dev/null
+++ b/releasenotes/notes/volume-multipath-63b96f8331e771ae.yaml
@@ -0,0 +1,7 @@
+---
+features:
+  - |
+    Adds support for multipath volumes. If the volume properties have
+    multiple portals, then it will generate multiple iscsi urls and
+    append them together for use in the generated ipxe file.
+