Merge "nvmet: Fix setup methods"

cinder/privsep/targets/nvmet.py

@@ -139,7 +139,7 @@ def do_privsep_call(instance, method_name, *args, **kwargs):
 
 
 @privsep.sys_admin_pctxt.entrypoint
-def privsep_setup(cls_name, *args, **kwargs):
+def _privsep_setup(cls_name, *args, **kwargs):
     """Special privsep method for nvmet setup method calls.
 
     The setup method is a special case because it's a class method (which
@@ -152,7 +152,23 @@ def privsep_setup(cls_name, *args, **kwargs):
     cls = getattr(nvmet, cls_name)
     args, kwargs = deserialize_params(args, kwargs)
     kwargs['err_func'] = _nvmet_setup_failure
-    cls.setup(*args, **kwargs)
+    return cls.setup(*args, **kwargs)
+
+
+def privsep_setup(cls_name, *args, **kwargs):
+    """Wrapper for _privsep_setup that accepts err_func argument."""
+    # err_func parameter hardcoded in _privsep_setup as it cannot be serialized
+    if 'err_func' in kwargs:
+        err_func = kwargs.pop('err_func')
+    else:  # positional is always last argument of the args tuple
+        err_func = args[-1]
+        args = args[:-1]
+    try:
+        return _privsep_setup(cls_name, *args, **kwargs)
+    except exception.CinderException as exc:
+        if not err_func:
+            raise
+        err_func(exc.msg)
 
 
 ###################
@@ -177,8 +193,8 @@ class Subsystem(nvmet.Subsystem):
         super().__init__(nqn=nqn, mode=mode)
 
     @classmethod
-    def setup(cls, t):
-        privsep_setup(cls.__name__, t)
+    def setup(cls, t, err_func=None):
+        privsep_setup(cls.__name__, t, err_func)
 
     def delete(self):
         do_privsep_call(serialize(self), 'delete')
@@ -189,8 +205,8 @@ class Port(nvmet.Port):
         super().__init__(portid=portid, mode=mode)
 
     @classmethod
-    def setup(cls, root, n):
-        privsep_setup(cls.__name__, serialize(root), n)
+    def setup(cls, root, n, err_func=None):
+        privsep_setup(cls.__name__, serialize(root), n, err_func)
 
     def add_subsystem(self, nqn):
         do_privsep_call(serialize(self), 'add_subsystem', nqn)

cinder/tests/unit/privsep/targets/test_nvmet.py

@@ -12,6 +12,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import unittest
 from unittest import mock
 
 import ddt
@@ -207,11 +208,50 @@ class TestPrivsep(test.TestCase):
                           nvmet._nvmet_setup_failure, mock.sentinel.message)
         mock_log.assert_called_once_with(mock.sentinel.message)
 
+    @mock.patch.object(nvmet, '_privsep_setup')
+    def test_privsep_setup(self, mock_setup):
+        args = [mock.sentinel.arg1, mock.sentinel.arg2]
+        kwargs = {'kwarg1': mock.sentinel.kwarg1}
+
+        res = nvmet.privsep_setup('MyClass', err_func=None, *args, **kwargs)
+
+        mock_setup.assert_called_once_with('MyClass', *args, **kwargs)
+        self.assertEqual(mock_setup.return_value, res)
+
+    @mock.patch.object(nvmet, '_privsep_setup')
+    def test_privsep_setup_err_func_as_arg_none(self, mock_setup):
+        exc = exception.CinderException('ouch')
+        mock_setup.side_effect = exc
+        args = [mock.sentinel.arg1, mock.sentinel.arg2, None]
+        kwargs = {'kwarg1': mock.sentinel.kwarg1}
+
+        # NOTE: testtools.TestCase were Cinder's tests inherit from masks the
+        # unittest's assertRaises that supports context manager usage, so we
+        # address it directly.
+        with unittest.TestCase.assertRaises(self,
+                                            exception.CinderException) as cm:
+            nvmet.privsep_setup('MyClass', *args, **kwargs)
+
+        self.assertEqual(exc, cm.exception)
+        mock_setup.assert_called_once_with('MyClass', *args[:-1], **kwargs)
+
+    @mock.patch.object(nvmet, '_privsep_setup')
+    def test_privsep_setup_err_func_as_arg(self, mock_setup):
+        def err_func(msg):
+            raise exception.VolumeDriverException()
+
+        mock_setup.side_effect = exception.CinderException('ouch')
+        args = [mock.sentinel.arg1, mock.sentinel.arg2, err_func]
+
+        self.assertRaises(exception.VolumeDriverException,
+                          nvmet.privsep_setup, 'MyClass', *args)
+        mock_setup.assert_called_once_with('MyClass', *args[:-1])
+
     # We mock the privsep context mode to fake that we are not the client
     @mock.patch('cinder.privsep.sys_admin_pctxt.client_mode', False)
     @mock.patch.object(nvmet, 'deserialize_params')
     @mock.patch.object(nvmet.nvmet, 'MyClass')
-    def test_privsep_setup(self, mock_class, mock_deserialize):
+    def test__privsep_setup(self, mock_class, mock_deserialize):
         args = (1, 2, 3)
         kwargs = {'4': 5, '6': 7}
         deserialized_args = (11, 22, 33)
@@ -224,11 +264,12 @@ class TestPrivsep(test.TestCase):
         mock_deserialize.return_value = (deserialized_args,
                                          deserialized_kwargs)
 
-        nvmet.privsep_setup('MyClass', *args, **kwargs)
+        res = nvmet._privsep_setup('MyClass', *args, **kwargs)
 
         mock_deserialize.assert_called_once_with(args, kwargs)
         mock_class.setup.assert_called_once_with(*expected_args,
                                                  **expected_kwargs)
+        self.assertEqual(mock_class.setup.return_value, res)
 
     # We mock the privsep context mode to fake that we are not the client
     @mock.patch('cinder.privsep.sys_admin_pctxt.client_mode', False)
@@ -271,8 +312,14 @@ class TestNvmetClasses(test.TestCase):
 
     @mock.patch.object(nvmet, 'privsep_setup')
     def test_subsystem_setup(self, mock_setup):
+        nvmet.Subsystem.setup(mock.sentinel.t, mock.sentinel.err_func)
+        mock_setup.assert_called_once_with('Subsystem', mock.sentinel.t,
+                                           mock.sentinel.err_func)
+
+    @mock.patch.object(nvmet, 'privsep_setup')
+    def test_subsystem_setup_no_err_func(self, mock_setup):
         nvmet.Subsystem.setup(mock.sentinel.t)
-        mock_setup.assert_called_once_with('Subsystem', mock.sentinel.t)
+        mock_setup.assert_called_once_with('Subsystem', mock.sentinel.t, None)
 
     @mock.patch.object(nvmet, 'serialize')
     @mock.patch.object(nvmet, 'do_privsep_call')
@@ -293,10 +340,20 @@ class TestNvmetClasses(test.TestCase):
     @mock.patch.object(nvmet, 'serialize')
     @mock.patch.object(nvmet, 'privsep_setup')
     def test_port_setup(self, mock_setup, mock_serialize):
+        nvmet.Port.setup(mock.sentinel.root, mock.sentinel.n,
+                         mock.sentinel.err_func)
+        mock_serialize.assert_called_once_with(mock.sentinel.root)
+        mock_setup.assert_called_once_with('Port', mock_serialize.return_value,
+                                           mock.sentinel.n,
+                                           mock.sentinel.err_func)
+
+    @mock.patch.object(nvmet, 'serialize')
+    @mock.patch.object(nvmet, 'privsep_setup')
+    def test_port_setup_no_err_func(self, mock_setup, mock_serialize):
         nvmet.Port.setup(mock.sentinel.root, mock.sentinel.n)
         mock_serialize.assert_called_once_with(mock.sentinel.root)
         mock_setup.assert_called_once_with('Port', mock_serialize.return_value,
-                                           mock.sentinel.n)
+                                           mock.sentinel.n, None)
 
     @mock.patch.object(nvmet, 'serialize')
     @mock.patch.object(nvmet, 'do_privsep_call')