From f532ec4f70c5ac1bf65f2e3545f5b607adb038ba Mon Sep 17 00:00:00 2001 From: Yolanda Robla Date: Mon, 24 Sep 2018 16:11:52 +0200 Subject: [PATCH] Enable configuration of conversion flags for iscsi Add an option conv_flags, that will be passed at ironic.conf into iscsi section. This will pass the conv_flags option to the iscsi disk utils, and this will modify the behaviour of the image copy to disk. It can be used for things as optimization (adding an sparse flag) Change-Id: Ia9e11dda35bb06e5b37b00e1b8fb42f9267a95d6 Story: #2004124 Task: #27573 --- ironic/conf/iscsi.py | 5 ++ ironic/drivers/modules/deploy_utils.py | 7 ++- ironic/drivers/modules/iscsi_deploy.py | 11 +++- .../unit/drivers/modules/test_deploy_utils.py | 53 +++++++++++++++++-- .../unit/drivers/modules/test_iscsi_deploy.py | 4 +- lower-constraints.txt | 2 +- ...nversion_flags_iscsi-d7f846803a647573.yaml | 7 +++ requirements.txt | 2 +- 8 files changed, 78 insertions(+), 13 deletions(-) create mode 100644 releasenotes/notes/add_conversion_flags_iscsi-d7f846803a647573.yaml diff --git a/ironic/conf/iscsi.py b/ironic/conf/iscsi.py index c9b95222e9..c01053f8fd 100644 --- a/ironic/conf/iscsi.py +++ b/ironic/conf/iscsi.py @@ -23,6 +23,11 @@ opts = [ default=3260, help=_('The port number on which the iSCSI portal listens ' 'for incoming connections.')), + cfg.StrOpt('conv_flags', + help=_('Flags that need to be sent to the dd command, ' + 'to control the conversion of the original file ' + 'when copying to the host. It can contain several ' + 'options separated by commas.')) ] diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 3443e9daf8..42811bb1e2 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -410,7 +410,8 @@ def deploy_partition_image( def deploy_disk_image(address, port, iqn, lun, - image_path, node_uuid, configdrive=None): + image_path, node_uuid, configdrive=None, + conv_flags=None): """All-in-one function to deploy a whole disk image to a node. :param address: The iSCSI IP address. @@ -421,12 +422,14 @@ def deploy_disk_image(address, port, iqn, lun, :param node_uuid: node's uuid. :param configdrive: Optional. Base64 encoded Gzipped configdrive content or configdrive HTTP URL. + :param conv_flags: Optional. Add a flag that will modify the behaviour of + the image copy to disk. :returns: a dictionary containing the key 'disk identifier' to identify the disk which was used for deployment. """ with _iscsi_setup_and_handle_errors(address, port, iqn, lun) as dev: - disk_utils.populate_image(image_path, dev) + disk_utils.populate_image(image_path, dev, conv_flags=conv_flags) if configdrive: disk_utils.create_config_drive_partition(node_uuid, dev, diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index ea51b485ab..5bb23eb458 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -85,7 +85,7 @@ def check_image_size(task): @METRICS.timer('get_deploy_info') -def get_deploy_info(node, address, iqn, port=None, lun='1'): +def get_deploy_info(node, address, iqn, port=None, lun='1', conv_flags=None): """Returns the information required for doing iSCSI deploy in a dictionary. :param node: ironic node object @@ -93,6 +93,8 @@ def get_deploy_info(node, address, iqn, port=None, lun='1'): :param iqn: iSCSI iqn for the target disk :param port: iSCSI port, defaults to one specified in the configuration :param lun: iSCSI lun, defaults to '1' + :param conv_flags: flag that will modify the behaviour of the image copy + to disk. :raises: MissingParameterValue, if some required parameters were not passed. :raises: InvalidParameterValue, if any of the parameters have invalid @@ -134,6 +136,9 @@ def get_deploy_info(node, address, iqn, port=None, lun='1'): if is_whole_disk_image: return params + if conv_flags: + params['conv_flags'] = conv_flags + # ephemeral_format is nullable params['ephemeral_format'] = i_info.get('ephemeral_format') @@ -264,6 +269,7 @@ def do_agent_iscsi_deploy(task, agent_client): iqn = 'iqn.2008-10.org.openstack:%s' % node.uuid portal_port = CONF.iscsi.portal_port + conv_flags = CONF.iscsi.conv_flags result = agent_client.start_iscsi_target( node, iqn, portal_port, @@ -278,7 +284,8 @@ def do_agent_iscsi_deploy(task, agent_client): address = parse.urlparse(node.driver_internal_info['agent_url']) address = address.hostname - uuid_dict_returned = continue_deploy(task, iqn=iqn, address=address) + uuid_dict_returned = continue_deploy(task, iqn=iqn, address=address, + conv_flags=conv_flags) root_uuid_or_disk_id = uuid_dict_returned.get( 'root uuid', uuid_dict_returned.get('disk identifier')) diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index fa345cf03b..0225f5a3a7 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -539,11 +539,10 @@ class PhysicalWorkTestCase(tests_base.TestCase): mock.call.logout_iscsi(address, port, iqn), mock.call.delete_iscsi(address, port, iqn)] disk_utils_calls_expected = [mock.call.is_block_device(dev), - mock.call.populate_image(image_path, dev)] - + mock.call.populate_image(image_path, dev, + conv_flags=None)] uuid_dict_returned = utils.deploy_disk_image(address, port, iqn, lun, - image_path, node_uuid, - configdrive=None) + image_path, node_uuid) self.assertEqual(utils_calls_expected, utils_mock.mock_calls) self.assertEqual(disk_utils_calls_expected, disk_utils_mock.mock_calls) @@ -581,7 +580,8 @@ class PhysicalWorkTestCase(tests_base.TestCase): mock.call.delete_iscsi(address, port, iqn)] disk_utils_calls_expected = [mock.call.is_block_device(dev), - mock.call.populate_image(image_path, dev)] + mock.call.populate_image(image_path, dev, + conv_flags=None)] uuid_dict_returned = utils.deploy_disk_image(address, port, iqn, lun, image_path, node_uuid, @@ -593,6 +593,49 @@ class PhysicalWorkTestCase(tests_base.TestCase): config_url) self.assertEqual('0x12345678', uuid_dict_returned['disk identifier']) + @mock.patch.object(disk_utils, 'create_config_drive_partition', + autospec=True) + @mock.patch.object(disk_utils, 'get_disk_identifier', autospec=True) + def test_deploy_whole_disk_image_sparse(self, mock_gdi, + create_config_drive_mock): + """Check loosely all functions are called with right args.""" + address = '127.0.0.1' + port = 3306 + iqn = 'iqn.xyz' + lun = 1 + image_path = '/tmp/xyz/image' + node_uuid = "12345678-1234-1234-1234-1234567890abcxyz" + + dev = '/dev/fake' + utils_name_list = ['get_dev', 'discovery', 'login_iscsi', + 'logout_iscsi', 'delete_iscsi'] + disk_utils_name_list = ['is_block_device', 'populate_image'] + + utils_mock = self._mock_calls(utils_name_list, utils) + utils_mock.get_dev.return_value = dev + + disk_utils_mock = self._mock_calls(disk_utils_name_list, disk_utils) + disk_utils_mock.is_block_device.return_value = True + mock_gdi.return_value = '0x12345678' + utils_calls_expected = [mock.call.get_dev(address, port, iqn, lun), + mock.call.discovery(address, port), + mock.call.login_iscsi(address, port, iqn), + mock.call.logout_iscsi(address, port, iqn), + mock.call.delete_iscsi(address, port, iqn)] + disk_utils_calls_expected = [mock.call.is_block_device(dev), + mock.call.populate_image( + image_path, dev, conv_flags='sparse')] + + uuid_dict_returned = utils.deploy_disk_image(address, port, iqn, lun, + image_path, node_uuid, + configdrive=None, + conv_flags='sparse') + + self.assertEqual(utils_calls_expected, utils_mock.mock_calls) + self.assertEqual(disk_utils_calls_expected, disk_utils_mock.mock_calls) + self.assertFalse(create_config_drive_mock.called) + self.assertEqual('0x12345678', uuid_dict_returned['disk identifier']) + @mock.patch.object(common_utils, 'execute', autospec=True) def test_verify_iscsi_connection_raises(self, mock_exec): iqn = 'iqn.xyz' diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index b6d14f357a..d276a9b644 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py @@ -179,7 +179,7 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): def test_continue_deploy_fail( self, deploy_mock, power_mock, mock_image_cache, mock_disk_layout, mock_collect_logs): - kwargs = {'address': '123456', 'iqn': 'aaa-bbb'} + kwargs = {'address': '123456', 'iqn': 'aaa-bbb', 'conv_flags': None} deploy_mock.side_effect = exception.InstanceDeployFailure( "test deploy error") self.node.provision_state = states.DEPLOYWAIT @@ -462,7 +462,7 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): agent_client_mock.start_iscsi_target.assert_called_once_with( task.node, expected_iqn, 3260, wipe_disk_metadata=True) continue_deploy_mock.assert_called_once_with( - task, iqn=expected_iqn, address='1.2.3.4') + task, iqn=expected_iqn, address='1.2.3.4', conv_flags=None) self.assertEqual( 'some-root-uuid', task.node.driver_internal_info['root_uuid_or_disk_id']) diff --git a/lower-constraints.txt b/lower-constraints.txt index 69c421e849..911ba412eb 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -38,7 +38,7 @@ greenlet==0.4.13 hacking==1.0.0 idna==2.6 imagesize==1.0.0 -ironic-lib==2.14.0 +ironic-lib==2.15.0 iso8601==0.1.11 Jinja2==2.10 jmespath==0.9.3 diff --git a/releasenotes/notes/add_conversion_flags_iscsi-d7f846803a647573.yaml b/releasenotes/notes/add_conversion_flags_iscsi-d7f846803a647573.yaml new file mode 100644 index 0000000000..8624b45cfc --- /dev/null +++ b/releasenotes/notes/add_conversion_flags_iscsi-d7f846803a647573.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Adds a new configuration option ``[iscsi]conv_flags``, that specifies the + conversion options to pass to the ``dd`` utility when copying an image. + For example, passing ``sparse`` may result in less network traffic for + large whole disk images. diff --git a/requirements.txt b/requirements.txt index a566689ff5..507bf933d0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,7 +11,7 @@ python-cinderclient>=3.3.0 # Apache-2.0 python-neutronclient>=6.7.0 # Apache-2.0 python-glanceclient>=2.8.0 # Apache-2.0 keystoneauth1>=3.4.0 # Apache-2.0 -ironic-lib>=2.14.0 # Apache-2.0 +ironic-lib>=2.15.0 # Apache-2.0 python-swiftclient>=3.2.0 # Apache-2.0 pytz>=2013.6 # MIT stevedore>=1.20.0 # Apache-2.0