From d27b27679f71a41e114dea42ecf92a17287686ad Mon Sep 17 00:00:00 2001
From: Nguyen Van Trung <trungnv@vn.fujitsu.com>
Date: Wed, 23 May 2018 15:26:24 +0700
Subject: [PATCH] Fix authentication issues along with add multi extra volumes

This commit will fix authentication in boot from volume (BFV) feature,
which use the volumes from Cinder for Baremetal via Ironic.

Each volume will need pair of account for authentication when perform
sanhook into SAN device via iPXE. And sandboot from drive 0x80 (default)
also need pair accounts same with the iscsi sanhook on drive 0x80 with
multi volumes has supported.

NOTE:
- We could add more than two volumes into iSCSI Boot Firmware Table(iBFT)
- Due to Linux does not support an iBFT that has more than two volumes,
  thus BFV only support for add one etra volume. If over two volume in iBFT
  then machine will raise "iBFT error: Control header is invalid!".
- Our code-base already for more than two volumes in iBFT, If Linux kernel
  bugs[1] is fixing this issue then we can use BFV with more than two volumes.

Tested successfully on Fujitsu Baremetal Server TX2540 M1.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/acpi/actbl1.h#n1567

Co-authored-By: Hoang Trung Hieu <hieuht@vn.fujitsu.com>
Co-authored-By: Dao Cong Tien <tiendc@vn.fujitsu.com>

Change-Id: I98f658cced8491872d39adbd8e0a1a643dd24868
Story: #2001824
Task: #12573
---
 ironic/common/utils.py                        |  2 +-
 ironic/drivers/modules/ipxe_config.template   | 13 ++++---
 ironic/drivers/modules/pxe.py                 | 13 +++++--
 ironic/tests/unit/common/test_pxe_utils.py    | 39 ++++++++++++++-----
 ...ig_boot_from_volume_extra_volume.template} |  4 ++
 ...oot_from_volume_no_extra_volumes.template} |  2 +
 ironic/tests/unit/drivers/modules/test_pxe.py | 27 ++++++++++---
 ...lti-attached-volumes-092ffedbdcf0feac.yaml |  5 +++
 8 files changed, 79 insertions(+), 26 deletions(-)
 rename ironic/tests/unit/drivers/{ipxe_config_boot_from_volume.template => ipxe_config_boot_from_volume_extra_volume.template} (89%)
 rename ironic/tests/unit/drivers/{ipxe_config_boot_from_volume_no_volumes.template => ipxe_config_boot_from_volume_no_extra_volumes.template} (99%)
 create mode 100644 releasenotes/notes/fix-multi-attached-volumes-092ffedbdcf0feac.yaml

diff --git a/ironic/common/utils.py b/ironic/common/utils.py
index acde43b6e4..a75eb88153 100644
--- a/ironic/common/utils.py
+++ b/ironic/common/utils.py
@@ -497,7 +497,7 @@ def render_template(template, params, is_file=True):
         loader = jinja2.DictLoader({tmpl_name: template})
     env = jinja2.Environment(loader=loader)
     tmpl = env.get_template(tmpl_name)
-    return tmpl.render(params)
+    return tmpl.render(params, enumerate=enumerate)
 
 
 def warn_about_deprecated_extra_vif_port_id():
diff --git a/ironic/drivers/modules/ipxe_config.template b/ironic/drivers/modules/ipxe_config.template
index ff6f8494fe..375febdf4f 100644
--- a/ironic/drivers/modules/ipxe_config.template
+++ b/ironic/drivers/modules/ipxe_config.template
@@ -22,11 +22,14 @@ imgfree
 {% if pxe_options.password %}set password {{ pxe_options.password }}{% endif %}
 {% if pxe_options.iscsi_initiator_iqn %}set initiator-iqn {{ pxe_options.iscsi_initiator_iqn }}{% endif %}
 sanhook --drive 0x80 {{ pxe_options.iscsi_boot_url }} || goto fail_iscsi_retry
-{%- if pxe_options.iscsi_volumes %}{% for volume in pxe_options
-.iscsi_volumes %}
-{%- set drive_id = 80 + loop.index %}
-sanhook --drive 0x{{ drive_id }} {{ volume }} || goto fail_iscsi_retry
+{%- if pxe_options.iscsi_volumes %}{% for i, volume in enumerate(pxe_options.iscsi_volumes) %}
+set username {{ volume.username }}
+set password {{ volume.password }}
+{%- set drive_id = 129 + i %}
+sanhook --drive {{ '0x%x' % drive_id }} {{ volume.url }} || goto fail_iscsi_retry
 {%- endfor %}{% endif %}
+{% if pxe_options.iscsi_volumes %}set username {{ pxe_options.username }}{% endif %}
+{% if pxe_options.iscsi_volumes %}set password {{ pxe_options.password }}{% endif %}
 sanboot --no-describe || goto fail_iscsi_retry
 
 :fail_iscsi_retry
@@ -36,4 +39,4 @@ goto boot_iscsi
 {%- endif %}
 
 :boot_whole_disk
-sanboot --no-describe
+sanboot --no-describe
\ No newline at end of file
diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py
index 897a0bfb34..c2c2ee3da9 100644
--- a/ironic/drivers/modules/pxe.py
+++ b/ironic/drivers/modules/pxe.py
@@ -334,12 +334,17 @@ def _get_volume_pxe_options(task):
              'iscsi_initiator_iqn': iscsi_initiator_iqn})
         # NOTE(TheJulia): This may be the route to multi-path, define
         # volumes via sanhook in the ipxe template and let the OS sort it out.
-        additional_targets = []
+        extra_targets = []
+
         for target in task.volume_targets:
             if target.boot_index != 0 and 'iscsi' in target.volume_type:
-                additional_targets.append(
-                    __generate_iscsi_url(target.properties))
-        pxe_options.update({'iscsi_volumes': additional_targets,
+                iscsi_url = __generate_iscsi_url(target.properties)
+                username = target.properties['auth_username']
+                password = target.properties['auth_password']
+                extra_targets.append({'url': iscsi_url,
+                                      'username': username,
+                                      'password': password})
+        pxe_options.update({'iscsi_volumes': extra_targets,
                             'boot_from_volume': True})
     # TODO(TheJulia): FibreChannel boot, i.e. wwpn in volume_type
     # for FCoE, should go here.
diff --git a/ironic/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py
index b3f7d94736..fce88e37fc 100644
--- a/ironic/tests/unit/common/test_pxe_utils.py
+++ b/ironic/tests/unit/common/test_pxe_utils.py
@@ -63,16 +63,35 @@ class TestPXEUtils(db_base.DbTestCase):
             'ipxe_timeout': 120
         })
 
-        self.ipxe_options_boot_from_volume = self.ipxe_options.copy()
-        self.ipxe_options_boot_from_volume.update({
+        self.ipxe_options_boot_from_volume_no_extra_volume = \
+            self.ipxe_options.copy()
+        self.ipxe_options_boot_from_volume_no_extra_volume.update({
             'boot_from_volume': True,
             'iscsi_boot_url': 'iscsi:fake_host::3260:0:fake_iqn',
             'iscsi_initiator_iqn': 'fake_iqn',
-            'iscsi_volumes': ['iscsi:fake_host::3260:1:fake_iqn'],
+            'iscsi_volumes': [],
             'username': 'fake_username',
-            'password': 'fake_password'
+            'password': 'fake_password',
         })
-        self.ipxe_options_boot_from_volume.pop('initrd_filename', None)
+
+        self.ipxe_options_boot_from_volume_extra_volume = \
+            self.ipxe_options.copy()
+        self.ipxe_options_boot_from_volume_extra_volume.update({
+            'boot_from_volume': True,
+            'iscsi_boot_url': 'iscsi:fake_host::3260:0:fake_iqn',
+            'iscsi_initiator_iqn': 'fake_iqn',
+            'iscsi_volumes': [{'url': 'iscsi:fake_host::3260:1:fake_iqn',
+                               'username': 'fake_username_1',
+                               'password': 'fake_password_1',
+                               }],
+            '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(
+            'initrd_filename', None)
 
         self.node = object_utils.create_test_node(self.context)
 
@@ -151,25 +170,25 @@ class TestPXEUtils(db_base.DbTestCase):
         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,
+            {'pxe_options': self.ipxe_options_boot_from_volume_extra_volume,
              'ROOT': '{{ ROOT }}',
              'DISK_IDENTIFIER': '{{ DISK_IDENTIFIER }}'})
 
         templ_file = 'ironic/tests/unit/drivers/' \
-                     'ipxe_config_boot_from_volume.template'
+                     'ipxe_config_boot_from_volume_extra_volume.template'
         with open(templ_file) as f:
             expected_template = f.read().rstrip()
 
         self.assertEqual(six.text_type(expected_template), rendered_template)
 
-    def test_default_ipxe_boot_from_volume_config_no_volumes(self):
+    def test_default_ipxe_boot_from_volume_config_no_extra_volumes(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')
 
-        pxe_options = self.ipxe_options_boot_from_volume
+        pxe_options = self.ipxe_options_boot_from_volume_no_extra_volume
         pxe_options['iscsi_volumes'] = []
 
         rendered_template = utils.render_template(
@@ -179,7 +198,7 @@ class TestPXEUtils(db_base.DbTestCase):
              'DISK_IDENTIFIER': '{{ DISK_IDENTIFIER }}'})
 
         templ_file = 'ironic/tests/unit/drivers/' \
-                     'ipxe_config_boot_from_volume_no_volumes.template'
+                     'ipxe_config_boot_from_volume_no_extra_volumes.template'
         with open(templ_file) as f:
             expected_template = f.read().rstrip()
         self.assertEqual(six.text_type(expected_template), rendered_template)
diff --git a/ironic/tests/unit/drivers/ipxe_config_boot_from_volume.template b/ironic/tests/unit/drivers/ipxe_config_boot_from_volume_extra_volume.template
similarity index 89%
rename from ironic/tests/unit/drivers/ipxe_config_boot_from_volume.template
rename to ironic/tests/unit/drivers/ipxe_config_boot_from_volume_extra_volume.template
index 32bda03073..03e6276505 100644
--- a/ironic/tests/unit/drivers/ipxe_config_boot_from_volume.template
+++ b/ironic/tests/unit/drivers/ipxe_config_boot_from_volume_extra_volume.template
@@ -21,7 +21,11 @@ set username fake_username
 set password fake_password
 set initiator-iqn fake_iqn
 sanhook --drive 0x80 iscsi:fake_host::3260:0:fake_iqn || goto fail_iscsi_retry
+set username fake_username_1
+set password fake_password_1
 sanhook --drive 0x81 iscsi:fake_host::3260:1:fake_iqn || goto fail_iscsi_retry
+set username fake_username
+set password fake_password
 sanboot --no-describe || goto fail_iscsi_retry
 
 :fail_iscsi_retry
diff --git a/ironic/tests/unit/drivers/ipxe_config_boot_from_volume_no_volumes.template b/ironic/tests/unit/drivers/ipxe_config_boot_from_volume_no_extra_volumes.template
similarity index 99%
rename from ironic/tests/unit/drivers/ipxe_config_boot_from_volume_no_volumes.template
rename to ironic/tests/unit/drivers/ipxe_config_boot_from_volume_no_extra_volumes.template
index 77c8f418b1..c9dbab2c98 100644
--- a/ironic/tests/unit/drivers/ipxe_config_boot_from_volume_no_volumes.template
+++ b/ironic/tests/unit/drivers/ipxe_config_boot_from_volume_no_extra_volumes.template
@@ -21,6 +21,8 @@ set username fake_username
 set password fake_password
 set initiator-iqn fake_iqn
 sanhook --drive 0x80 iscsi:fake_host::3260:0:fake_iqn || goto fail_iscsi_retry
+
+
 sanboot --no-describe || goto fail_iscsi_retry
 
 :fail_iscsi_retry
diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py
index 9f4f3f91d8..39bf7e1217 100644
--- a/ironic/tests/unit/drivers/modules/test_pxe.py
+++ b/ironic/tests/unit/drivers/modules/test_pxe.py
@@ -428,9 +428,13 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase):
                 'boot_from_volume': True,
                 'iscsi_boot_url': 'iscsi:fake_host::3260:0:fake_iqn',
                 'iscsi_initiator_iqn': 'fake_iqn_initiator',
-                'iscsi_volumes': ['iscsi:fake_host::3260:1:fake_iqn'],
+                'iscsi_volumes': [{'url': 'iscsi:fake_host::3260:1:fake_iqn',
+                                   'username': 'fake_username_1',
+                                   'password': 'fake_password_1'
+                                   }],
                 'username': 'fake_username',
-                'password': 'fake_password'})
+                'password': 'fake_password'
+            })
             expected_options.pop('deployment_aki_path')
             expected_options.pop('deployment_ari_path')
             expected_options.pop('initrd_filename')
@@ -488,7 +492,9 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase):
             boot_index=1, volume_id='1235', uuid=vol_id2,
             properties={'target_lun': 1,
                         'target_portal': 'fake_host:3260',
-                        'target_iqn': 'fake_iqn'})
+                        'target_iqn': 'fake_iqn',
+                        '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)
 
@@ -515,7 +521,9 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase):
             boot_index=1, volume_id='1235', uuid=vol_id2,
             properties={'target_lun': [1, 3],
                         'target_portal': ['fake_host:3260', 'faker_host:3261'],
-                        'target_iqn': ['fake_iqn', 'faker_iqn']})
+                        'target_iqn': ['fake_iqn', 'faker_iqn'],
+                        '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)
 
@@ -541,7 +549,9 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase):
             boot_index=1, volume_id='1235', uuid=vol_id2,
             properties={'target_lun': 1,
                         'target_portal': 'fake_host:3260',
-                        'target_iqn': 'fake_iqn'})
+                        'target_iqn': 'fake_iqn',
+                        'auth_username': 'fake_username_1',
+                        'auth_password': 'fake_password_1'})
         self.node.driver_internal_info.update({'boot_from_volume': vol_id})
         driver_internal_info = self.node.driver_internal_info
         driver_internal_info['boot_from_volume'] = vol_id
@@ -552,7 +562,12 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase):
                     'username': 'fake_username', 'password': 'fake_password',
                     'iscsi_boot_url': 'iscsi:fake_host::3260:0:fake_iqn',
                     'iscsi_initiator_iqn': 'fake_iqn_initiator',
-                    'iscsi_volumes': ['iscsi:fake_host::3260:1:fake_iqn']}
+                    'iscsi_volumes': [{
+                        'url': 'iscsi:fake_host::3260:1:fake_iqn',
+                        'username': 'fake_username_1',
+                        'password': 'fake_password_1'
+                    }]
+                    }
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=True) as task:
             options = pxe._get_volume_pxe_options(task)
diff --git a/releasenotes/notes/fix-multi-attached-volumes-092ffedbdcf0feac.yaml b/releasenotes/notes/fix-multi-attached-volumes-092ffedbdcf0feac.yaml
new file mode 100644
index 0000000000..ba4aacd5cd
--- /dev/null
+++ b/releasenotes/notes/fix-multi-attached-volumes-092ffedbdcf0feac.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fix an issue with iPXE is used for authentication during boot from volume
+    with multi attached volumes.