From: Gorka Eguileor <geguileo@redhat.com>
Date: Thu, 18 Feb 2016 12:51:40 +0000 (+0100)
Subject: Rtstool sets wrong exception message on save
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=1e472e4bb3d3c353bbec6e5e0fb2e16e7c48f1fe;p=openstack-build%2Fcinder-build.git

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 e095b4d7a..a7eeab26e 100644
--- 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 4302e5a02..a29d9c6ce 100644
--- 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)