From 434bbdf4d047fb206709674a8f0f9141405a84d9 Mon Sep 17 00:00:00 2001
From: Gorka Eguileor <geguileo@redhat.com>
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