From 434bbdf4d047fb206709674a8f0f9141405a84d9 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Sun, 10 May 2015 16:50:36 +0200 Subject: [PATCH] Fix LIO target helper when missing targetcli After patch to fix LIO target persistence was merged, a bug was introduced where target creation/removal would result in an exception if targetcli is not installed on the system. This patch fixes this by checking if path to the default file exists on the system and creating it if doesn't. Change-Id: I5ecc1ae47482ea43a4890ccd81a4e872ab570069 Closes-Bug: #1453518 --- cinder/cmd/rtstool.py | 29 +++++++++++++++++++++----- cinder/tests/unit/test_cmd.py | 38 ++++++++++++++++++++++++++++++++++- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/cinder/cmd/rtstool.py b/cinder/cmd/rtstool.py index d270a2a02..a1ae0b1a9 100644 --- a/cinder/cmd/rtstool.py +++ b/cinder/cmd/rtstool.py @@ -15,6 +15,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os import sys # We always use rtslib-fb, but until version 2.1.52 it didn't have its own @@ -197,12 +198,30 @@ def usage(): def save_to_file(destination_file): rtsroot = rtslib_fb.root.RTSRoot() try: - rtsroot.save_to_file(destination_file) - except OSError: + # If default destination use rtslib default save file if not destination_file: - destination_file = 'default location' - raise RtstoolError(_('Could not save configuration to %(file_path)s'), - {'file_path': destination_file}) + destination_file = rtslib_fb.root.default_save_file + path_to_file = os.path.dirname(destination_file) + + # NOTE(geguileo): With default file we ensure path exists and + # create it if doesn't. + # Cinder's LIO target helper runs this as root, so it will have no + # problem creating directory /etc/target. + # If run manually from the command line without being root you will + # get an error, same as when creating and removing targets. + if not os.path.exists(path_to_file): + os.makedirs(path_to_file, 0o755) + + except OSError as exc: + raise RtstoolError(_('targetcli not installed and could not create ' + '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'), + {'file_path': destination_file, 'exc': exc}) def parse_optional_create(argv): diff --git a/cinder/tests/unit/test_cmd.py b/cinder/tests/unit/test_cmd.py index 7d62805e0..2f25d90c1 100644 --- a/cinder/tests/unit/test_cmd.py +++ b/cinder/tests/unit/test_cmd.py @@ -964,14 +964,50 @@ class TestCinderRtstoolCmd(test.TestCase): target.delete.assert_called_once_with() storage_object.delete.assert_called_once_with() + @mock.patch.object(cinder_rtstool, 'os', autospec=True) @mock.patch.object(cinder_rtstool, 'rtslib_fb', autospec=True) - def test_save(self, mock_rtslib): + def test_save_with_filename(self, mock_rtslib, mock_os): filename = mock.sentinel.filename cinder_rtstool.save_to_file(filename) rtsroot = mock_rtslib.root.RTSRoot rtsroot.assert_called_once_with() + self.assertEqual(0, mock_os.path.dirname.call_count) + self.assertEqual(0, mock_os.path.exists.call_count) + self.assertEqual(0, mock_os.makedirs.call_count) rtsroot.return_value.save_to_file.assert_called_once_with(filename) + @mock.patch.object(cinder_rtstool, 'os', + **{'path.exists.return_value': True, + 'path.dirname.return_value': mock.sentinel.dirname}) + @mock.patch.object(cinder_rtstool, 'rtslib_fb', + **{'root.default_save_file': mock.sentinel.filename}) + def test_save(self, mock_rtslib, mock_os): + """Test that we check path exists with default file.""" + cinder_rtstool.save_to_file(None) + rtsroot = mock_rtslib.root.RTSRoot + rtsroot.assert_called_once_with() + rtsroot.return_value.save_to_file.assert_called_once_with( + mock.sentinel.filename) + mock_os.path.dirname.assert_called_once_with(mock.sentinel.filename) + mock_os.path.exists.assert_called_once_with(mock.sentinel.dirname) + self.assertEqual(0, mock_os.makedirs.call_count) + + @mock.patch.object(cinder_rtstool, 'os', + **{'path.exists.return_value': False, + 'path.dirname.return_value': mock.sentinel.dirname}) + @mock.patch.object(cinder_rtstool, 'rtslib_fb', + **{'root.default_save_file': mock.sentinel.filename}) + def test_save_no_targetcli(self, mock_rtslib, mock_os): + """Test that we create path if it doesn't exist with default file.""" + cinder_rtstool.save_to_file(None) + rtsroot = mock_rtslib.root.RTSRoot + rtsroot.assert_called_once_with() + rtsroot.return_value.save_to_file.assert_called_once_with( + mock.sentinel.filename) + mock_os.path.dirname.assert_called_once_with(mock.sentinel.filename) + mock_os.path.exists.assert_called_once_with(mock.sentinel.dirname) + mock_os.makedirs.assert_called_once_with(mock.sentinel.dirname, 0o755) + def test_usage(self): with mock.patch('sys.stdout', new=six.StringIO()): exit = self.assertRaises(SystemExit, cinder_rtstool.usage) -- 2.45.2