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

diff --git a/cinder/cmd/rtstool.py b/cinder/cmd/rtstool.py
index 4ec404a9ec6a3bf7c988cbbd14d970c20338b02c..58794ebf2430005a33ed0e4535c2a3af21a0bfab 100644 (file)
--- a/cinder/cmd/rtstool.py
+++ b/cinder/cmd/rtstool.py
@@ -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()
 

diff --git a/cinder/tests/targets/test_lio_driver.py b/cinder/tests/targets/test_lio_driver.py
index ee9a992b58b9ae775ad4a6538cb53240cd96a1b1..478d276f70b95c0860f0ec186a713d5ccc4951e3 100644 (file)
--- a/cinder/tests/targets/test_lio_driver.py
+++ b/cinder/tests/targets/test_lio_driver.py
@@ -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')

diff --git a/cinder/tests/test_cmd.py b/cinder/tests/test_cmd.py
index 5c18be9732865e554937f101f748cc2d724779e1..c3306ba8d381ffb347c9c432392aeb4c328665ad 100644 (file)
--- a/cinder/tests/test_cmd.py
+++ b/cinder/tests/test_cmd.py
@@ -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',

diff --git a/cinder/volume/targets/lio.py b/cinder/volume/targets/lio.py
index a8f7276a2ad03623a8913f7e45a7d84111f203d9..21f9b635e98b5df82078ace5e52796cbf0b8b126 100644 (file)
--- a/cinder/volume/targets/lio.py
+++ b/cinder/volume/targets/lio.py
@@ -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'])