openstack
/
cinder
Code Issues Proposed changes

Make lio iSCSI changes persistent to avoid lost 

To avoid losing iSCSI configuration when target.service is
started/restarted we make persistent any change Cinder makes to the
configuration.

This will allow us to coordinate access from multiple rtslib users
(target daemon, targetcli, cinder...)

This patch changes rtstool and adds an additional command "save" with
optional parameter the filename where we want to save current
configuration. If no parameter is provided it saves to default location
defined by rtslib.

Closes-Bug: #1427301
Change-Id: I74bd09ed16a2e0e584d3e04762aec8cbdf101f6d
This commit is contained in:
Gorka Eguileor 2015-03-02 15:51:07 +01:00
parent 5d3a14da42
commit 706878deaa
4 changed files with 106 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
cinder/cmd/rtstool.py
View File

@ -172,9 +172,21 @@ def usage():
print(sys.argv[0] + " get-targets")
print(sys.argv[0] + " delete [iqn]")
print(sys.argv[0] + " verify")
print(sys.argv[0] + " save [path_to_file]")
sys.exit(1)
def save_to_file(destination_file):
rtsroot = rtslib.root.RTSRoot()
try:
rtsroot.save_to_file(destination_file)
except OSError:
if not destination_file:
destination_file = 'default location'
raise RtstoolError(_('Could not save configuration to %(file_path)s'),
{'file_path': destination_file})
def main(argv=None):
if argv is None:
argv = sys.argv
@ -238,6 +250,14 @@ def main(argv=None):
verify_rtslib()
return 0
elif argv[1] == 'save':
if len(argv) > 3:
usage()
destination_file = argv[2] if len(argv) > 2 else None
save_to_file(destination_file)
return 0
else:
usage()

55
cinder/tests/targets/test_lio_driver.py
View File

@ -102,13 +102,15 @@ class TestLioAdmDriver(test.TestCase):
self.assertEqual(('foo', 'bar'),
self.target._get_target_chap_auth(ctxt, test_vol))
@mock.patch.object(lio.LioAdm, '_persist_configuration')
@mock.patch.object(utils, 'execute')
@mock.patch.object(lio.LioAdm, '_get_target')
def test_create_iscsi_target(self, mget_target, mexecute):
def test_create_iscsi_target(self, mget_target, mexecute, mpersist_cfg):
mget_target.return_value = 1
test_vol = 'iqn.2010-10.org.openstack:'\
'volume-83c2e877-feed-46be-8435-77884fe55b45'
# create_iscsi_target sends volume_name instead of volume_id on error
volume_name = 'volume-83c2e877-feed-46be-8435-77884fe55b45'
test_vol = 'iqn.2010-10.org.openstack:' + volume_name
self.assertEqual(
1,
self.target.create_iscsi_target(
@ -116,10 +118,13 @@ class TestLioAdmDriver(test.TestCase):
1,
0,
self.fake_volumes_dir))
mpersist_cfg.assert_called_once_with(volume_name)
@mock.patch.object(lio.LioAdm, '_persist_configuration')
@mock.patch.object(utils, 'execute')
@mock.patch.object(lio.LioAdm, '_get_target')
def test_create_iscsi_target_already_exists(self, mget_target, mexecute):
def test_create_iscsi_target_already_exists(self, mget_target, mexecute,
mpersist_cfg):
mexecute.side_effect = putils.ProcessExecutionError
test_vol = 'iqn.2010-10.org.openstack:'\
@ -132,12 +137,14 @@ class TestLioAdmDriver(test.TestCase):
0,
self.fake_volumes_dir,
chap_auth)
self.assertEqual(0, mpersist_cfg.call_count)
@mock.patch.object(lio.LioAdm, '_persist_configuration')
@mock.patch.object(utils, 'execute')
def test_remove_iscsi_target(self, mexecute):
def test_remove_iscsi_target(self, mexecute, mpersist_cfg):
test_vol = 'iqn.2010-10.org.openstack:'\
'volume-83c2e877-feed-46be-8435-77884fe55b45'
volume_id = '83c2e877-feed-46be-8435-77884fe55b45'
test_vol = 'iqn.2010-10.org.openstack:volume-' + volume_id
# Test the normal case
self.target.remove_iscsi_target(0,
@ -149,6 +156,8 @@ class TestLioAdmDriver(test.TestCase):
test_vol,
run_as_root=True)
mpersist_cfg.assert_called_once_with(volume_id)
# Test the failure case: putils.ProcessExecutionError
mexecute.side_effect = putils.ProcessExecutionError
self.assertRaises(exception.ISCSITargetRemoveFailed,
@ -158,6 +167,9 @@ class TestLioAdmDriver(test.TestCase):
self.testvol['id'],
self.testvol['name'])
# Ensure there have been no more calls to persist configuration
self.assertEqual(1, mpersist_cfg.call_count)
@mock.patch.object(lio.LioAdm, '_get_target_chap_auth')
@mock.patch.object(lio.LioAdm, 'create_iscsi_target')
def test_ensure_export(self, _mock_create, mock_get_chap):
@ -175,10 +187,13 @@ class TestLioAdmDriver(test.TestCase):
check_exit_code=False,
old_name=None)
@mock.patch.object(lio.LioAdm, '_persist_configuration')
@mock.patch.object(utils, 'execute')
@mock.patch.object(lio.LioAdm, '_get_iscsi_properties')
def test_initialize_connection(self, mock_get_iscsi, mock_execute):
def test_initialize_connection(self, mock_get_iscsi, mock_execute,
mpersist_cfg):
volume_id = '83c2e877-feed-46be-8435-77884fe55b45'
target_id = 'iqn.2010-10.org.openstack:volume-' + volume_id
connector = {'initiator': 'fake_init'}
# Test the normal case
@ -190,13 +205,13 @@ class TestLioAdmDriver(test.TestCase):
connector))
mock_execute.assert_called_once_with(
'cinder-rtstool', 'add-initiator',
'iqn.2010-10.org.openstack:'
'volume-83c2e877-feed-46be-8435-77884fe55b45',
'cinder-rtstool', 'add-initiator', target_id,
'c76370d66b', '2FE0CQ8J196R',
connector['initiator'],
run_as_root=True)
mpersist_cfg.assert_called_once_with(volume_id)
# Test the failure case: putils.ProcessExecutionError
mock_execute.side_effect = putils.ProcessExecutionError
self.assertRaises(exception.ISCSITargetAttachFailed,
@ -204,21 +219,26 @@ class TestLioAdmDriver(test.TestCase):
self.testvol,
connector)
@mock.patch.object(lio.LioAdm, '_persist_configuration')
@mock.patch.object(utils, 'execute')
def test_terminate_connection(self, _mock_execute):
def test_terminate_connection(self, _mock_execute, mpersist_cfg):
volume_id = '83c2e877-feed-46be-8435-77884fe55b45'
target_id = 'iqn.2010-10.org.openstack:volume-' + volume_id
connector = {'initiator': 'fake_init'}
self.target.terminate_connection(self.testvol,
connector)
_mock_execute.assert_called_once_with(
'cinder-rtstool', 'delete-initiator',
'iqn.2010-10.org.openstack:'
'volume-83c2e877-feed-46be-8435-77884fe55b45',
'cinder-rtstool', 'delete-initiator', target_id,
connector['initiator'],
run_as_root=True)
mpersist_cfg.assert_called_once_with(volume_id)
@mock.patch.object(lio.LioAdm, '_persist_configuration')
@mock.patch.object(utils, 'execute')
def test_terminate_connection_fail(self, _mock_execute):
def test_terminate_connection_fail(self, _mock_execute, mpersist_cfg):
_mock_execute.side_effect = putils.ProcessExecutionError
connector = {'initiator': 'fake_init'}
@ -226,6 +246,7 @@ class TestLioAdmDriver(test.TestCase):
self.target.terminate_connection,
self.testvol,
connector)
self.assertEqual(0, mpersist_cfg.call_count)
def test_iscsi_protocol(self):
self.assertEqual(self.target.iscsi_protocol, 'iscsi')

25
cinder/tests/test_cmd.py
View File

@ -908,6 +908,14 @@ class TestCinderRtstoolCmd(test.TestCase):
target.delete.assert_called_once_with()
storage_object.delete.assert_called_once_with()
@mock.patch.object(cinder_rtstool, 'rtslib', autospec=True)
def test_save(self, mock_rtslib):
filename = mock.sentinel.filename
cinder_rtstool.save_to_file(filename)
rtsroot = mock_rtslib.root.RTSRoot
rtsroot.assert_called_once_with()
rtsroot.return_value.save_to_file.assert_called_once_with(filename)
def test_usage(self):
exit = self.assertRaises(SystemExit, cinder_rtstool.usage)
@ -954,6 +962,23 @@ class TestCinderRtstoolCmd(test.TestCase):
self.assertTrue(usage.called)
self.assertEqual(exit.code, 1)
@mock.patch('cinder.cmd.rtstool.save_to_file')
def test_main_save(self, mock_save):
sys.argv = ['cinder-rtstool',
'save']
rc = cinder_rtstool.main()
mock_save.assert_called_once_with(None)
self.assertEqual(0, rc)
@mock.patch('cinder.cmd.rtstool.save_to_file')
def test_main_save_with_file(self, mock_save):
sys.argv = ['cinder-rtstool',
'save',
mock.sentinel.filename]
rc = cinder_rtstool.main()
mock_save.assert_called_once_with(mock.sentinel.filename)
self.assertEqual(0, rc)
def test_main_create(self):
with mock.patch('cinder.cmd.rtstool.create') as create:
sys.argv = ['cinder-rtstool',

23
cinder/volume/targets/lio.py
View File

@ -77,6 +77,17 @@ class LioAdm(iscsi.ISCSITarget):
iscsi_target = 0 # NOTE: Not used by lio.
return iscsi_target, lun
def _persist_configuration(self, vol_id):
try:
utils.execute('cinder-rtstool', 'save', run_as_root=True)
# On persistence failure we don't raise an exception, as target has
# been successfully created.
except putils.ProcessExecutionError:
LOG.warning(_LW("Failed to save iscsi LIO configuration when "
"modifying volume id: %(vol_id)s."),
{'vol_id': vol_id})
def create_iscsi_target(self, name, tid, lun, path,
chap_auth=None, **kwargs):
# tid and lun are not used
@ -113,6 +124,9 @@ class LioAdm(iscsi.ISCSITarget):
"id:%s.") % vol_id)
raise exception.NotFound()
# We make changes persistent
self._persist_configuration(vol_id)
return tid
def remove_iscsi_target(self, tid, lun, vol_id, vol_name, **kwargs):
@ -131,6 +145,9 @@ class LioAdm(iscsi.ISCSITarget):
LOG.error(_LE("%s") % e)
raise exception.ISCSITargetRemoveFailed(volume_id=vol_id)
# We make changes persistent
self._persist_configuration(vol_id)
def initialize_connection(self, volume, connector):
volume_iqn = volume['provider_location'].split(' ')[1]
@ -151,6 +168,9 @@ class LioAdm(iscsi.ISCSITarget):
raise exception.ISCSITargetAttachFailed(
volume_id=volume['id'])
# We make changes persistent
self._persist_configuration(volume['id'])
iscsi_properties = self._get_iscsi_properties(volume,
connector.get(
'multipath'))
@ -173,3 +193,6 @@ class LioAdm(iscsi.ISCSITarget):
LOG.error(_LE("Failed to delete initiator iqn %s to target.") %
connector['initiator'])
raise exception.ISCSITargetDetachFailed(volume_id=volume['id'])
# We make changes persistent
self._persist_configuration(volume['id'])