Merge "Change return value of [driver_]vendor_passthru to dict"

ironic/api/controllers/v1/driver.py

@@ -150,11 +150,11 @@ class DriverPassthruController(rest.RestController):
 
         http_method = pecan.request.method.upper()
         topic = pecan.request.rpcapi.get_topic_for_driver(driver_name)
-        ret, is_async = pecan.request.rpcapi.driver_vendor_passthru(
+        response = pecan.request.rpcapi.driver_vendor_passthru(
             pecan.request.context, driver_name, method,
             http_method, data, topic=topic)
-        status_code = 202 if is_async else 200
+        status_code = 202 if response['async'] else 200
-        return wsme.api.Response(ret, status_code=status_code)
+        return wsme.api.Response(response['return'], status_code=status_code)
 
 
 class DriversController(rest.RestController):

ironic/api/controllers/v1/node.py

@@ -718,11 +718,11 @@ class NodeVendorPassthruController(rest.RestController):
             data = {}
 
         http_method = pecan.request.method.upper()
-        ret, is_async = pecan.request.rpcapi.vendor_passthru(
+        response = pecan.request.rpcapi.vendor_passthru(
             pecan.request.context, rpc_node.uuid, method,
             http_method, data, topic)
-        status_code = 202 if is_async else 200
+        status_code = 202 if response['async'] else 200
-        return wsme.api.Response(ret, status_code=status_code)
+        return wsme.api.Response(response['return'], status_code=status_code)
 
 
 class NodeMaintenanceController(rest.RestController):

ironic/conductor/manager.py

@@ -202,7 +202,7 @@ class ConductorManager(periodic_task.PeriodicTasks):
     """Ironic Conductor manager main class."""
 
     # NOTE(rloo): This must be in sync with rpcapi.ConductorAPI's.
-    RPC_API_VERSION = '1.28'
+    RPC_API_VERSION = '1.29'
 
     target = messaging.Target(version=RPC_API_VERSION)
 
@@ -449,11 +449,13 @@ class ConductorManager(periodic_task.PeriodicTasks):
         :raises: NoFreeConductorWorker when there is no free worker to start
                  async task.
         :raises: NodeLocked if node is locked by another conductor.
-        :returns: A tuple containing the response of the invoked method
+        :returns: A dictionary containing:
-                  and a boolean value indicating whether the method was
+
-                  invoked asynchronously (True) or synchronously (False).
+            :return: The response of the invoked vendor method
-                  If invoked asynchronously the response field will be
+            :async: Boolean value. Whether the method was invoked
-                  always None.
+                asynchronously (True) or synchronously (False). When invoked
+                asynchronously the response will be always None.
+
         """
         LOG.debug("RPC vendor_passthru called for node %s." % node_id)
         # NOTE(max_lobur): Even though not all vendor_passthru calls may
@@ -495,7 +497,8 @@ class ConductorManager(periodic_task.PeriodicTasks):
             else:
                 ret = vendor_func(task, **info)
 
-            return (ret, is_async)
+            return {'return': ret,
+                    'async': is_async}
 
     @messaging.expected_exceptions(exception.NoFreeConductorWorker,
                                    exception.InvalidParameterValue,
@@ -526,11 +529,13 @@ class ConductorManager(periodic_task.PeriodicTasks):
         :raises: DriverNotFound if the supplied driver is not loaded.
         :raises: NoFreeConductorWorker when there is no free worker to start
                  async task.
-        :returns: A tuple containing the response of the invoked method
+        :returns: A dictionary containing:
-                  and a boolean value indicating whether the method was
+
-                  invoked asynchronously (True) or synchronously (False).
+            :return: The response of the invoked vendor method
-                  If invoked asynchronously the response field will be
+            :async: Boolean value. Whether the method was invoked
-                  always None.
+                asynchronously (True) or synchronously (False). When invoked
+                asynchronously the response will be always None.
+
         """
         # Any locking in a top-level vendor action will need to be done by the
         # implementation, as there is little we could reasonably lock on here.
@@ -586,7 +591,8 @@ class ConductorManager(periodic_task.PeriodicTasks):
         else:
             ret = vendor_func(context, **info)
 
-        return (ret, is_async)
+        return {'return': ret,
+                'async': is_async}
 
     @messaging.expected_exceptions(exception.UnsupportedDriverExtension)
     def get_node_vendor_passthru_methods(self, context, node_id):

ironic/conductor/rpcapi.py

@@ -71,11 +71,13 @@ class ConductorAPI(object):
     |    1.26 - Added continue_node_clean
     |    1.27 - Convert continue_node_clean to cast
     |    1.28 - Change exceptions raised by destroy_node
+    |    1.29 - Change return value of vendor_passthru and
+    |           driver_vendor_passthru to a dictionary
 
     """
 
     # NOTE(rloo): This must be in sync with manager.ConductorManager's.
-    RPC_API_VERSION = '1.28'
+    RPC_API_VERSION = '1.29'
 
     def __init__(self, topic=None):
         super(ConductorAPI, self).__init__()
@@ -190,11 +192,12 @@ class ConductorAPI(object):
         :raises: NoFreeConductorWorker when there is no free worker to start
                  async task.
         :raises: NodeLocked if node is locked by another conductor.
-        :returns: A tuple containing the response of the invoked method
+        :returns: A dictionary containing:
-                  and a boolean value indicating whether the method was
+
-                  invoked asynchronously (True) or synchronously (False).
+            :return: The response of the invoked vendor method
-                  If invoked asynchronously the response field will be
+            :async: Boolean value. Whether the method was invoked
-                  always None.
+                asynchronously (True) or synchronously (False). When invoked
+                asynchronously the response will be always None.
 
         """
         cctxt = self.client.prepare(topic=topic or self.topic, version='1.20')
@@ -226,11 +229,12 @@ class ConductorAPI(object):
         :raises: DriverNotFound if the supplied driver is not loaded.
         :raises: NoFreeConductorWorker when there is no free worker to start
                  async task.
-        :returns: A tuple containing the response of the invoked method
+        :returns: A dictionary containing:
-                  and a boolean value indicating whether the method was
+
-                  invoked asynchronously (True) or synchronously (False).
+            :return: The response of the invoked vendor method
-                  If invoked asynchronously the response field will be
+            :async: Boolean value. Whether the method was invoked
-                  always None.
+                asynchronously (True) or synchronously (False). When invoked
+                asynchronously the response will be always None.
 
         """
         cctxt = self.client.prepare(topic=topic or self.topic, version='1.20')

ironic/tests/api/v1/test_drivers.py

@@ -76,55 +76,56 @@ class TestListDrivers(base.FunctionalTest):
     @mock.patch.object(rpcapi.ConductorAPI, 'driver_vendor_passthru')
     def test_driver_vendor_passthru_sync(self, mocked_driver_vendor_passthru):
         self.register_fake_conductors()
-        mocked_driver_vendor_passthru.return_value = ({
+        mocked_driver_vendor_passthru.return_value = {
-            'return_key': 'return_value',
+            'return': {'return_key': 'return_value'},
-        }, False)
+            'async': False}
         response = self.post_json(
             '/drivers/%s/vendor_passthru/do_test' % self.d1,
             {'test_key': 'test_value'})
         self.assertEqual(200, response.status_int)
-        self.assertEqual(mocked_driver_vendor_passthru.return_value[0],
+        self.assertEqual(mocked_driver_vendor_passthru.return_value['return'],
                          response.json)
 
     @mock.patch.object(rpcapi.ConductorAPI, 'driver_vendor_passthru')
     def test_driver_vendor_passthru_async(self, mocked_driver_vendor_passthru):
         self.register_fake_conductors()
-        mocked_driver_vendor_passthru.return_value = (None, True)
+        mocked_driver_vendor_passthru.return_value = {'return': None,
+                                                      'async': True}
         response = self.post_json(
             '/drivers/%s/vendor_passthru/do_test' % self.d1,
             {'test_key': 'test_value'})
         self.assertEqual(202, response.status_int)
-        self.assertIsNone(mocked_driver_vendor_passthru.return_value[0])
+        self.assertIsNone(mocked_driver_vendor_passthru.return_value['return'])
 
     @mock.patch.object(rpcapi.ConductorAPI, 'driver_vendor_passthru')
     def test_driver_vendor_passthru_put(self, mocked_driver_vendor_passthru):
         self.register_fake_conductors()
-        return_value = (None, 'async')
+        return_value = {'return': None, 'async': True}
         mocked_driver_vendor_passthru.return_value = return_value
         response = self.put_json(
             '/drivers/%s/vendor_passthru/do_test' % self.d1,
             {'test_key': 'test_value'})
         self.assertEqual(202, response.status_int)
-        self.assertEqual(return_value[0], response.json)
+        self.assertEqual(return_value['return'], response.json)
 
     @mock.patch.object(rpcapi.ConductorAPI, 'driver_vendor_passthru')
     def test_driver_vendor_passthru_get(self, mocked_driver_vendor_passthru):
         self.register_fake_conductors()
-        return_value = ('foo', 'sync')
+        return_value = {'return': 'foo', 'async': False}
         mocked_driver_vendor_passthru.return_value = return_value
         response = self.get_json(
             '/drivers/%s/vendor_passthru/do_test' % self.d1)
-        self.assertEqual(return_value[0], response)
+        self.assertEqual(return_value['return'], response)
 
     @mock.patch.object(rpcapi.ConductorAPI, 'driver_vendor_passthru')
     def test_driver_vendor_passthru_delete(self, mock_driver_vendor_passthru):
         self.register_fake_conductors()
-        return_value = (None, 'async')
+        return_value = {'return': None, 'async': True}
         mock_driver_vendor_passthru.return_value = return_value
         response = self.delete(
             '/drivers/%s/vendor_passthru/do_test' % self.d1)
         self.assertEqual(202, response.status_int)
-        self.assertEqual(return_value[0], response.json)
+        self.assertEqual(return_value['return'], response.json)
 
     def test_driver_vendor_passthru_driver_not_found(self):
         # tests when given driver is not found

ironic/tests/api/v1/test_nodes.py

@@ -1214,7 +1214,7 @@ class TestPost(test_api_base.FunctionalTest):
 
         node = obj_utils.create_test_node(self.context)
         info = {'foo': 'bar'}
-        mock_vendor.return_value = (return_value, is_async)
+        mock_vendor.return_value = {'return': return_value, 'async': is_async}
         response = self.post_json('/nodes/%s/vendor_passthru/test' % node.uuid,
                                   info)
         mock_vendor.assert_called_once_with(
@@ -1231,7 +1231,7 @@ class TestPost(test_api_base.FunctionalTest):
 
         node = obj_utils.create_test_node(self.context, name='node-109')
         info = {'foo': 'bar'}
-        mock_vendor.return_value = (return_value, is_async)
+        mock_vendor.return_value = {'return': return_value, 'async': is_async}
         response = self.post_json('/nodes/%s/vendor_passthru/test' % node.name,
                                   info,
                                   headers={api_base.Version.string: "1.5"})
@@ -1253,13 +1253,13 @@ class TestPost(test_api_base.FunctionalTest):
     @mock.patch.object(rpcapi.ConductorAPI, 'vendor_passthru')
     def test_vendor_passthru_put(self, mocked_vendor_passthru):
         node = obj_utils.create_test_node(self.context)
-        return_value = (None, 'async')
+        return_value = {'return': None, 'async': True}
         mocked_vendor_passthru.return_value = return_value
         response = self.put_json(
             '/nodes/%s/vendor_passthru/do_test' % node.uuid,
             {'test_key': 'test_value'})
         self.assertEqual(202, response.status_int)
-        self.assertEqual(return_value[0], response.json)
+        self.assertEqual(return_value['return'], response.json)
 
     @mock.patch.object(rpcapi.ConductorAPI, 'vendor_passthru')
     def test_vendor_passthru_by_name(self, mock_vendor):
@@ -1268,21 +1268,21 @@ class TestPost(test_api_base.FunctionalTest):
     @mock.patch.object(rpcapi.ConductorAPI, 'vendor_passthru')
     def test_vendor_passthru_get(self, mocked_vendor_passthru):
         node = obj_utils.create_test_node(self.context)
-        return_value = ('foo', 'sync')
+        return_value = {'return': 'foo', 'async': False}
         mocked_vendor_passthru.return_value = return_value
         response = self.get_json(
             '/nodes/%s/vendor_passthru/do_test' % node.uuid)
-        self.assertEqual(return_value[0], response)
+        self.assertEqual(return_value['return'], response)
 
     @mock.patch.object(rpcapi.ConductorAPI, 'vendor_passthru')
     def test_vendor_passthru_delete(self, mock_vendor_passthru):
         node = obj_utils.create_test_node(self.context)
-        return_value = (None, 'async')
+        return_value = {'return': None, 'async': True}
         mock_vendor_passthru.return_value = return_value
         response = self.delete(
             '/nodes/%s/vendor_passthru/do_test' % node.uuid)
         self.assertEqual(202, response.status_int)
-        self.assertEqual(return_value[0], response.json)
+        self.assertEqual(return_value['return'], response.json)
 
     def test_vendor_passthru_no_such_method(self):
         node = obj_utils.create_test_node(self.context)

ironic/tests/conductor/test_manager.py

@@ -564,16 +564,16 @@ class VendorPassthruTestCase(_ServiceSetUpMixin, tests_db_base.DbTestCase):
         info = {'bar': 'baz'}
         self._start_service()
 
-        ret, is_async = self.service.vendor_passthru(self.context, node.uuid,
+        response = self.service.vendor_passthru(self.context, node.uuid,
-                                                     'first_method', 'POST',
+                                                'first_method', 'POST',
-                                                     info)
+                                                info)
         # Waiting to make sure the below assertions are valid.
         self.service._worker_pool.waitall()
 
         # Assert spawn_after was called
         self.assertTrue(mock_spawn.called)
-        self.assertIsNone(ret)
+        self.assertIsNone(response['return'])
-        self.assertTrue(is_async)
+        self.assertTrue(response['async'])
 
         node.refresh()
         self.assertIsNone(node.last_error)
@@ -586,16 +586,16 @@ class VendorPassthruTestCase(_ServiceSetUpMixin, tests_db_base.DbTestCase):
         info = {'bar': 'meow'}
         self._start_service()
 
-        ret, is_async = self.service.vendor_passthru(self.context, node.uuid,
+        response = self.service.vendor_passthru(self.context, node.uuid,
-                                                     'third_method_sync',
+                                                'third_method_sync',
-                                                     'POST', info)
+                                                'POST', info)
         # Waiting to make sure the below assertions are valid.
         self.service._worker_pool.waitall()
 
         # Assert no workers were used
         self.assertFalse(mock_spawn.called)
-        self.assertTrue(ret)
+        self.assertTrue(response['return'])
-        self.assertFalse(is_async)
+        self.assertFalse(response['async'])
 
         node.refresh()
         self.assertIsNone(node.last_error)
@@ -754,14 +754,14 @@ class VendorPassthruTestCase(_ServiceSetUpMixin, tests_db_base.DbTestCase):
         mock_spawn.reset_mock()
 
         vendor_args = {'test': 'arg'}
-        got, is_async = self.service.driver_vendor_passthru(
+        response = self.service.driver_vendor_passthru(
             self.context, 'fake', 'test_method', 'POST', vendor_args)
 
         # Assert that the vendor interface has no custom
         # driver_vendor_passthru()
         self.assertFalse(hasattr(self.driver.vendor, 'driver_vendor_passthru'))
-        self.assertEqual(expected, got)
+        self.assertEqual(expected, response['return'])
-        self.assertFalse(is_async)
+        self.assertFalse(response['async'])
         test_method.assert_called_once_with(self.context, **vendor_args)
         # No worker was spawned
         self.assertFalse(mock_spawn.called)
@@ -779,14 +779,14 @@ class VendorPassthruTestCase(_ServiceSetUpMixin, tests_db_base.DbTestCase):
         mock_spawn.reset_mock()
 
         vendor_args = {'test': 'arg'}
-        got, is_async = self.service.driver_vendor_passthru(
+        response = self.service.driver_vendor_passthru(
             self.context, 'fake', 'test_sync_method', 'POST', vendor_args)
 
         # Assert that the vendor interface has no custom
         # driver_vendor_passthru()
         self.assertFalse(hasattr(self.driver.vendor, 'driver_vendor_passthru'))
-        self.assertIsNone(got)
+        self.assertIsNone(response['return'])
-        self.assertTrue(is_async)
+        self.assertTrue(response['async'])
         mock_spawn.assert_called_once_with(test_method, self.context,
                                            **vendor_args)