openstack
/
ironic
Code Issues Proposed changes

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
This commit is contained in:
Yolanda Robla 2018-09-24 16:11:52 +02:00
parent 2fbafab8a8
commit f532ec4f70
8 changed files with 78 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
ironic/conf/iscsi.py
View File

@ -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.'))
]

7
ironic/drivers/modules/deploy_utils.py
View File

@ -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,

11
ironic/drivers/modules/iscsi_deploy.py
View File

@ -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'))

53
ironic/tests/unit/drivers/modules/test_deploy_utils.py
View File

@ -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'

4
ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
View File

@ -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'])

2
lower-constraints.txt
View File

@ -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

7
releasenotes/notes/add_conversion_flags_iscsi-d7f846803a647573.yaml Normal file
View File

@ -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.

2
requirements.txt
View File

@ -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