Merge "[LVM] Restore target config during ensure_export"

cinder/cmd/rtstool.py

@@ -225,6 +225,20 @@ def save_to_file(destination_file):
                            {'file_path': destination_file, 'exc': exc})
 
 
+def restore_from_file(configration_file):
+    rtsroot = rtslib_fb.root.RTSRoot()
+    # If configuration file is None, use rtslib default save file.
+    if not configration_file:
+        configration_file = rtslib_fb.root.default_save_file
+
+    try:
+        rtsroot.restore_from_file(configration_file)
+    except (OSError, IOError) as exc:
+        raise RtstoolError(_('Could not restore configuration file '
+                             '%(file_path)s: %(exc)s'),
+                           {'file_path': configration_file, 'exc': exc})
+
+
 def parse_optional_create(argv):
     optional_args = {}
 
@@ -316,6 +330,14 @@ def main(argv=None):
         save_to_file(destination_file)
         return 0
 
+    elif argv[1] == 'restore':
+        if len(argv) > 3:
+            usage()
+
+        configuration_file = argv[2] if len(argv) > 2 else None
+        restore_from_file(configuration_file)
+        return 0
+
     else:
         usage()
 

cinder/tests/unit/targets/test_lio_driver.py

@@ -201,23 +201,30 @@ class TestLioAdmDriver(tf.TargetDriverFixture):
         # Ensure there have been no calls to persist configuration
         self.assertFalse(mpersist_cfg.called)
 
-    @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):
+    @mock.patch.object(lio.LioAdm, '_get_targets')
+    @mock.patch.object(lio.LioAdm, '_execute', side_effect=lio.LioAdm._execute)
+    @mock.patch('cinder.utils.execute')
+    def test_ensure_export(self, mock_exec, mock_execute, mock_get_targets):
 
         ctxt = context.get_admin_context()
-        mock_get_chap.return_value = ('foo', 'bar')
+        mock_get_targets.return_value = None
         self.target.ensure_export(ctxt,
                                   self.testvol,
                                   self.fake_volumes_dir)
 
-        _mock_create.assert_called_once_with(
-            self.iscsi_target_prefix + 'testvol',
-            0, 0, self.fake_volumes_dir, ('foo', 'bar'),
-            check_exit_code=False,
-            old_name=None,
-            portals_ips=[self.configuration.iscsi_ip_address],
-            portals_port=self.configuration.iscsi_port)
+        expected_args = ('cinder-rtstool', 'restore')
+        mock_exec.assert_called_once_with(*expected_args, run_as_root=True)
+
+    @mock.patch.object(lio.LioAdm, '_get_targets')
+    @mock.patch.object(lio.LioAdm, '_restore_configuration')
+    def test_ensure_export_target_exist(self, mock_restore, mock_get_targets):
+
+        ctxt = context.get_admin_context()
+        mock_get_targets.return_value = 'target'
+        self.target.ensure_export(ctxt,
+                                  self.testvol,
+                                  self.fake_volumes_dir)
+        self.assertFalse(mock_restore.called)
 
     @mock.patch.object(lio.LioAdm, '_execute', side_effect=lio.LioAdm._execute)
     @mock.patch.object(lio.LioAdm, '_persist_configuration')

cinder/tests/unit/test_cmd.py

@@ -1218,6 +1218,32 @@ class TestCinderRtstoolCmd(test.TestCase):
         self.assertRaisesRegexp(cinder_rtstool.RtstoolError, regexp,
                                 cinder_rtstool.save_to_file, 'myfile')
 
+    @mock.patch.object(cinder_rtstool, 'rtslib_fb',
+                       **{'root.default_save_file': mock.sentinel.filename})
+    def test_restore(self, mock_rtslib):
+        """Test that we restore target configuration with default file."""
+        cinder_rtstool.restore_from_file(None)
+        rtsroot = mock_rtslib.root.RTSRoot
+        rtsroot.assert_called_once_with()
+        rtsroot.return_value.restore_from_file.assert_called_once_with(
+            mock.sentinel.filename)
+
+    @mock.patch.object(cinder_rtstool, 'rtslib_fb')
+    def test_restore_with_file(self, mock_rtslib):
+        """Test that we restore target configuration with specified file."""
+        cinder_rtstool.restore_from_file('saved_file')
+        rtsroot = mock_rtslib.root.RTSRoot
+        rtsroot.return_value.restore_from_file.assert_called_once_with(
+            'saved_file')
+
+    @mock.patch('cinder.cmd.rtstool.restore_from_file')
+    def test_restore_error(self, restore_from_file):
+        """Test that we fail to restore target configuration."""
+        restore_from_file.side_effect = OSError
+        self.assertRaises(OSError,
+                          cinder_rtstool.restore_from_file,
+                          mock.sentinel.filename)
+
     def test_usage(self):
         with mock.patch('sys.stdout', new=six.StringIO()):
             exit = self.assertRaises(SystemExit, cinder_rtstool.usage)

cinder/volume/targets/lio.py

@@ -62,6 +62,12 @@ class LioAdm(iscsi.ISCSITarget):
 
         return None
 
+    def _get_targets(self):
+        (out, err) = self._execute('cinder-rtstool',
+                                   'get-targets',
+                                   run_as_root=True)
+        return out
+
     def _get_iscsi_target(self, context, vol_id):
         return 0
 
@@ -81,6 +87,15 @@ class LioAdm(iscsi.ISCSITarget):
                             "modifying volume id: %(vol_id)s."),
                         {'vol_id': vol_id})
 
+    def _restore_configuration(self):
+        try:
+            self._execute('cinder-rtstool', 'restore', 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 restore iscsi LIO configuration."))
+
     def create_iscsi_target(self, name, tid, lun, path,
                             chap_auth=None, **kwargs):
         # tid and lun are not used
@@ -188,3 +203,14 @@ class LioAdm(iscsi.ISCSITarget):
 
         # We make changes persistent
         self._persist_configuration(volume['id'])
+
+    def ensure_export(self, context, volume, volume_path):
+        """Recreate exports for logical volumes."""
+
+        # Restore saved configuration file if no target exists.
+        if not self._get_targets():
+            LOG.info(_LI('Restoring iSCSI target from configuration file'))
+            self._restore_configuration()
+            return
+
+        LOG.info(_LI("Skipping ensure_export. Found existing iSCSI target."))