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 67d2852a38..ba7990117c 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -414,7 +414,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. @@ -425,12 +426,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 4e09ee81fd..3cecc5ec1f 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 02bfd0e4a1..d0cdc66056 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