Rtstool sets wrong exception message on save

Rtstool does not properly set the message on the RtstoolError exception
on the save_to_file method (save operation) because it's passing
formatting arguments as if it were a LOG call instead of formatting
those same arguments with
%.

There are 2 cases where this can happen, when saving to the default
location on a system that has no target.service installed and the
creation of the default directory fails, and when saving the file
actually fails.

This patch properly formats the message before passing it to the
instantiation of RtstoolError exception.

Change-Id: I13492f695dc0a63a616325bcd0893b2f908cf9fe
Closes-Bug: #1547008

diff --git a/cinder/cmd/rtstool.py b/cinder/cmd/rtstool.py
index e095b4d7a1208956a82fb73c827f90c9090ad7cd..a7eeab26edb7a3fef75d6679b3340c5321354df4 100644 (file)
--- a/cinder/cmd/rtstool.py
+++ b/cinder/cmd/rtstool.py
@@ -215,13 +215,13 @@ def save_to_file(destination_file):
 
     except OSError as exc:
         raise RtstoolError(_('targetcli not installed and could not create '
-                             'default directory (%(default_path)s): %(exc)s'),
+                             'default directory (%(default_path)s): %(exc)s') %
                            {'default_path': path_to_file, 'exc': exc})
     try:
         rtsroot.save_to_file(destination_file)
     except (OSError, IOError) as exc:
         raise RtstoolError(_('Could not save configuration to %(file_path)s: '
-                             '%(exc)s'),
+                             '%(exc)s') %
                            {'file_path': destination_file, 'exc': exc})
 
 

diff --git a/cinder/tests/unit/test_cmd.py b/cinder/tests/unit/test_cmd.py
index 4302e5a029cc6fc7dbb097473e6467450359f7ab..a29d9c6ceb280b7714f7f20b4fb47ae3975ad631 100644 (file)
--- a/cinder/tests/unit/test_cmd.py
+++ b/cinder/tests/unit/test_cmd.py
@@ -1197,6 +1197,27 @@ class TestCinderRtstoolCmd(test.TestCase):
         mock_os.path.exists.assert_called_once_with(mock.sentinel.dirname)
         mock_os.makedirs.assert_called_once_with(mock.sentinel.dirname, 0o755)
 
+    @mock.patch.object(cinder_rtstool, 'os', autospec=True)
+    @mock.patch.object(cinder_rtstool, 'rtslib_fb', autospec=True)
+    def test_save_error_creating_dir(self, mock_rtslib, mock_os):
+        mock_os.path.dirname.return_value = 'dirname'
+        mock_os.path.exists.return_value = False
+        mock_os.makedirs.side_effect = OSError('error')
+
+        regexp = (u'targetcli not installed and could not create default '
+                  'directory \(dirname\): error$')
+        self.assertRaisesRegexp(cinder_rtstool.RtstoolError, regexp,
+                                cinder_rtstool.save_to_file, None)
+
+    @mock.patch.object(cinder_rtstool, 'os', autospec=True)
+    @mock.patch.object(cinder_rtstool, 'rtslib_fb', autospec=True)
+    def test_save_error_saving(self, mock_rtslib, mock_os):
+        save = mock_rtslib.root.RTSRoot.return_value.save_to_file
+        save.side_effect = OSError('error')
+        regexp = u'Could not save configuration to myfile: error'
+        self.assertRaisesRegexp(cinder_rtstool.RtstoolError, regexp,
+                                cinder_rtstool.save_to_file, 'myfile')
+
     def test_usage(self):
         with mock.patch('sys.stdout', new=six.StringIO()):
             exit = self.assertRaises(SystemExit, cinder_rtstool.usage)