From d27b27679f71a41e114dea42ecf92a17287686ad Mon Sep 17 00:00:00 2001 From: Nguyen Van Trung 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 Co-authored-By: Dao Cong Tien 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.