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

diff --git a/cinder/cmd/rtstool.py b/cinder/cmd/rtstool.py
index d270a2a02d66d158da8fe151fc62c1e3a7edb5ba..a1ae0b1a9a0df6d5a34fe66144df4f1a82af5fe3 100644 (file)
--- 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 7d62805e0ecc9d9a958a164e76d74e45ab417e6d..2f25d90c10684fb0525817f8da0ff4ee739af14b 100644 (file)
--- 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)