diff --git a/ironic/api/controllers/v1/driver.py b/ironic/api/controllers/v1/driver.py index dea7abdb65..8bca7b209d 100644 --- a/ironic/api/controllers/v1/driver.py +++ b/ironic/api/controllers/v1/driver.py @@ -81,9 +81,38 @@ class DriverList(base.APIBase): return sample +class DriverPassthruController(rest.RestController): + """REST controller for driver passthru. + + This controller allow vendors to expose cross-node functionality in the + Ironic API. Ironic will merely relay the message from here to the specified + driver, no introspection will be made in the message body. + """ + + @wsme_pecan.wsexpose(wtypes.text, wtypes.text, wtypes.text, + body=wtypes.text, + status_code=200) + def post(self, driver_name, method, data): + """Call a driver API extension. + + :param driver_name: name of the driver to call. + :param method: name of the method, to be passed to the vendor + implementation. + :param data: body of data to supply to the specified method. + """ + if not method: + raise wsme.exc.ClientSideError(_("Method not specified")) + + topic = pecan.request.rpcapi.get_topic_for_driver(driver_name) + return pecan.request.rpcapi.driver_vendor_passthru( + pecan.request.context, driver_name, method, data, topic=topic) + + class DriversController(rest.RestController): """REST controller for Drivers.""" + vendor_passthru = DriverPassthruController() + @wsme_pecan.wsexpose(DriverList) def get_all(self): """Retrieve a list of drivers. diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 9728b9f575..2890dffbcf 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -121,7 +121,7 @@ CONF.register_opts(conductor_opts, 'conductor') class ConductorManager(service.PeriodicService): """Ironic Conductor service main class.""" - RPC_API_VERSION = '1.13' + RPC_API_VERSION = '1.14' def __init__(self, host, topic): serializer = objects_base.IronicObjectSerializer() @@ -134,9 +134,8 @@ class ConductorManager(service.PeriodicService): super(ConductorManager, self).start() self.dbapi = dbapi.get_instance() - df = driver_factory.DriverFactory() - - self.drivers = df.names + self.driver_factory = driver_factory.DriverFactory() + self.drivers = self.driver_factory.names """List of driver names which this conductor supports.""" try: @@ -286,6 +285,45 @@ class ConductorManager(service.PeriodicService): task.driver.vendor.vendor_passthru, task, task.node, method=driver_method, **info) + @messaging.client_exceptions(exception.InvalidParameterValue, + exception.UnsupportedDriverExtension, + exception.DriverNotFound) + def driver_vendor_passthru(self, context, driver_name, driver_method, + info): + """RPC method which synchronously handles driver-level vendor passthru + calls. These calls don't require a node UUID and are executed on a + random conductor with the specified driver. + + :param context: an admin context. + :param driver_name: name of the driver on which to call the method. + :param driver_method: name of the vendor method, for use by the driver. + :param info: user-supplied data to pass through to the driver. + :raises: InvalidParameterValue if supplied info is not valid. + :raises: UnsupportedDriverExtension if current driver does not have + vendor interface, if the vendor interface does not implement + driver-level vendor passthru or if the passthru method is + unsupported. + :raises: DriverNotFound if the supplied driver is not loaded. + + """ + # 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. + LOG.debug(_("RPC driver_vendor_passthru for driver %s.") + % driver_name) + try: + driver = self.driver_factory[driver_name].obj + except KeyError: + raise exception.DriverNotFound(driver_name=driver_name) + + if not getattr(driver, 'vendor', None): + raise exception.UnsupportedDriverExtension( + driver=driver_name, + extension='vendor interface') + + return driver.vendor.driver_vendor_passthru(context, + method=driver_method, + **info) + @messaging.client_exceptions(exception.NoFreeConductorWorker, exception.NodeLocked, exception.NodeInMaintenance, diff --git a/ironic/conductor/rpcapi.py b/ironic/conductor/rpcapi.py index 35b45eac06..a27306098d 100644 --- a/ironic/conductor/rpcapi.py +++ b/ironic/conductor/rpcapi.py @@ -18,6 +18,8 @@ Client side of the conductor RPC API. """ +import random + from oslo.config import cfg from ironic.common import exception @@ -59,10 +61,11 @@ class ConductorAPI(ironic.openstack.common.rpc.proxy.RpcProxy): 1.12 - validate_vendor_action, do_vendor_action replaced by single vendor_passthru method. 1.13 - Added update_port. + 1.14 - Added driver_vendor_passthru. """ - RPC_API_VERSION = '1.13' + RPC_API_VERSION = '1.14' def __init__(self, topic=None): if topic is None: @@ -93,6 +96,20 @@ class ConductorAPI(ironic.openstack.common.rpc.proxy.RpcProxy): 'driver %s.') % node.driver) raise exception.NoValidHost(reason=reason) + def get_topic_for_driver(self, driver_name): + """Get an RPC topic which will route messages to a conductor which + supports the specified driver. A conductor is selected at + random from the set of qualified conductors. + + :param driver_name: the name of the driver to route to. + :returns: an RPC topic string. + :raises: DriverNotFound + + """ + hash_ring = self.ring_manager.get_hash_ring(driver_name) + host = random.choice(hash_ring.hosts) + return self.topic + "." + host + def update_node(self, context, node_obj, topic=None): """Synchronously, have a conductor update the node's information. @@ -159,6 +176,30 @@ class ConductorAPI(ironic.openstack.common.rpc.proxy.RpcProxy): info=info), topic=topic) + def driver_vendor_passthru(self, context, driver_name, driver_method, info, + topic=None): + """Pass vendor-specific calls which don't specify a node to a driver. + + :param context: request context. + :param driver_name: name of the driver on which to call the method. + :param driver_method: name of the vendor method, for use by the driver. + :param info: data to pass through to the driver. + :param topic: RPC topic. Defaults to self.topic. + :raises: InvalidParameterValue for parameter errors. + :raises: UnsupportedDriverExtension if the driver doesn't have a vendor + interface, or if the vendor interface does not support the + specified driver_method. + + """ + topic = topic or self.topic + + return self.call(context, + self.make_msg('driver_vendor_passthru', + driver_name=driver_name, + driver_method=driver_method, + info=info), + topic=topic) + def do_node_deploy(self, context, node_id, topic=None): """Signal to conductor service to perform a deployment. diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index 46ef36491e..01916c8ef8 100644 --- a/ironic/drivers/base.py +++ b/ironic/drivers/base.py @@ -21,6 +21,8 @@ import abc import six +from ironic.common import exception + @six.add_metaclass(abc.ABCMeta) class BaseDriver(object): @@ -298,7 +300,11 @@ class VendorInterface(object): """Interface for all vendor passthru functionality. Additional vendor- or driver-specific capabilities should be implemented as - private methods and invoked from vendor_passthru(). + private methods and invoked from vendor_passthru() or + driver_vendor_passthru(). + + driver_vendor_passthru() is a blocking call - methods implemented here + should be short-lived. """ @abc.abstractmethod @@ -325,3 +331,20 @@ class VendorInterface(object): the supported interfaces. :raises: InvalidParameterValue if **kwargs does not contain 'method'. """ + + def driver_vendor_passthru(self, context, method, **kwargs): + """Handle top-level (ie, no node is specified) vendor actions. These + allow a vendor interface to expose additional cross-node API + functionality. + + VendorInterface subclasses are explicitly not required to implement + this in order to maintain backwards compatibility with existing + drivers. + + :param context: a context for this action. + :param method: an arbitrary string describing the action to be taken. + :param kwargs: arbitrary parameters to the passthru method. + """ + raise exception.UnsupportedDriverExtension( + _('Vendor interface does not support driver vendor_passthru ' + 'method: %s') % method) diff --git a/ironic/drivers/utils.py b/ironic/drivers/utils.py index 254cbf8927..36cce0a79a 100644 --- a/ironic/drivers/utils.py +++ b/ironic/drivers/utils.py @@ -28,14 +28,19 @@ def _raise_unsupported_error(method=None): class MixinVendorInterface(base.VendorInterface): """Wrapper around multiple VendorInterfaces.""" - def __init__(self, mapping): + def __init__(self, mapping, driver_passthru_mapping=None): """Wrapper around multiple VendorInterfaces. :param mapping: dict of {'method': interface} specifying how to combine multiple vendor interfaces into one vendor driver. + :param driver_passthru_mapping: dict of {'method': interface} + specifying how to map + driver_vendor_passthru calls to + interfaces. """ self.mapping = mapping + self.driver_level_mapping = driver_passthru_mapping or {} def _map(self, **kwargs): method = kwargs.get('method') @@ -65,6 +70,21 @@ class MixinVendorInterface(base.VendorInterface): route = self._map(**kwargs) return route.vendor_passthru(task, node, **kwargs) + def driver_vendor_passthru(self, context, method, **kwargs): + """Call driver_vendor_passthru on a mapped interface based on the + specified method. + + Returns or raises according to the requested driver_vendor_passthru + + :raises: UnsupportedDriverExtension if 'method' cannot be mapped to + a supported interface. + """ + iface = self.driver_level_mapping.get(method) + if iface is None: + _raise_unsupported_error(method) + + return iface.driver_vendor_passthru(context, method, **kwargs) + def get_node_mac_addresses(task, node): """Get all MAC addresses for a node. diff --git a/ironic/tests/api/v1/test_drivers.py b/ironic/tests/api/v1/test_drivers.py index 150338ff9e..1d2819c3da 100644 --- a/ironic/tests/api/v1/test_drivers.py +++ b/ironic/tests/api/v1/test_drivers.py @@ -13,8 +13,11 @@ # License for the specific language governing permissions and limitations # under the License. +import json +import mock from testtools.matchers import HasLength +from ironic.conductor import rpcapi from ironic.tests.api import base @@ -64,5 +67,39 @@ class TestListDrivers(base.FunctionalTest): self.validate_link(data['links'][1]['href']) def test_drivers_get_one_not_found(self): - response = self.get_json('/drivers/' + self.d1, expect_errors=True) + response = self.get_json('/drivers/%s' % self.d1, expect_errors=True) self.assertEqual(404, response.status_int) + + @mock.patch.object(rpcapi.ConductorAPI, 'driver_vendor_passthru') + def test_driver_vendor_passthru_ok(self, mocked_driver_vendor_passthru): + self.register_fake_conductors() + mocked_driver_vendor_passthru.return_value = { + 'return_key': 'return_value', + } + 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, + response.json) + + def test_driver_vendor_passthru_driver_not_found(self): + # tests when given driver is not found + # e.g. get_topic_for_driver fails to find the driver + response = self.post_json( + '/drivers/%s/vendor_passthru/do_test' % self.d1, + {'test_key': 'test_value'}, + expect_errors=True) + + self.assertEqual(404, response.status_int) + + def test_driver_vendor_passthru_method_not_found(self): + response = self.post_json( + '/drivers/%s/vendor_passthru' % self.d1, + {'test_key': 'test_value'}, + expect_errors=True) + + self.assertEqual(400, response.status_int) + error = json.loads(response.json['error_message']) + self.assertEqual('Missing argument: "method"', + error['faultstring']) diff --git a/ironic/tests/conductor/test_manager.py b/ironic/tests/conductor/test_manager.py index 95c080b84a..102cb72703 100644 --- a/ironic/tests/conductor/test_manager.py +++ b/ironic/tests/conductor/test_manager.py @@ -477,6 +477,58 @@ class ManagerTestCase(tests_db_base.DbTestCase): # Verify reservation has been cleared. self.assertIsNone(node.reservation) + def test_driver_vendor_passthru_success(self): + expected = {'foo': 'bar'} + self.driver.vendor = vendor = mock.Mock() + vendor.driver_vendor_passthru.return_value = expected + self.service.start() + got = self.service.driver_vendor_passthru(self.context, + 'fake', + 'test_method', + {'test': 'arg'}) + self.assertEqual(expected, got) + vendor.driver_vendor_passthru.assert_called_once_with( + mock.ANY, + method='test_method', + test='arg') + + def test_driver_vendor_passthru_vendor_interface_not_supported(self): + # Test for when no vendor interface is set at all + self.driver.vendor = None + self.service.start() + exc = self.assertRaises(messaging.ClientException, + self.service.driver_vendor_passthru, + self.context, + 'fake', + 'test_method', + {}) + # Compare true exception hidden by @messaging.client_exceptions + self.assertEqual(exception.UnsupportedDriverExtension, + exc._exc_info[0]) + + def test_driver_vendor_passthru_not_supported(self): + # Test for when the vendor interface is set, but hasn't passed a + # driver_passthru_mapping to MixinVendorInterface + self.service.start() + exc = self.assertRaises(messaging.ClientException, + self.service.driver_vendor_passthru, + self.context, + 'fake', + 'test_method', + {}) + # Compare true exception hidden by @messaging.client_exceptions + self.assertEqual(exception.UnsupportedDriverExtension, + exc._exc_info[0]) + + def test_driver_vendor_passthru_driver_not_found(self): + self.service.start() + self.assertRaises(messaging.ClientException, + self.service.driver_vendor_passthru, + self.context, + 'does_not_exist', + 'test_method', + {}) + def test_do_node_deploy_invalid_state(self): # test node['provision_state'] is not NOSTATE ndict = utils.get_test_node(driver='fake', diff --git a/ironic/tests/conductor/test_rpcapi.py b/ironic/tests/conductor/test_rpcapi.py index bbcc4feb29..e58a587730 100644 --- a/ironic/tests/conductor/test_rpcapi.py +++ b/ironic/tests/conductor/test_rpcapi.py @@ -68,6 +68,27 @@ class RPCAPITestCase(base.DbTestCase): rpcapi.get_topic_for, self.fake_node_obj) + def test_get_topic_for_driver_known_driver(self): + CONF.set_override('host', 'fake-host') + self.dbapi.register_conductor({ + 'hostname': 'fake-host', + 'drivers': ['fake-driver'], + }) + rpcapi = conductor_rpcapi.ConductorAPI(topic='fake-topic') + self.assertEqual('fake-topic.fake-host', + rpcapi.get_topic_for_driver('fake-driver')) + + def test_get_topic_for_driver_unknown_driver(self): + CONF.set_override('host', 'fake-host') + self.dbapi.register_conductor({ + 'hostname': 'fake-host', + 'drivers': ['other-driver'], + }) + rpcapi = conductor_rpcapi.ConductorAPI(topic='fake-topic') + self.assertRaises(exception.DriverNotFound, + rpcapi.get_topic_for_driver, + 'fake-driver') + def _test_rpcapi(self, method, rpc_method, **kwargs): ctxt = context.get_admin_context() rpcapi = conductor_rpcapi.ConductorAPI(topic='fake-topic') @@ -121,6 +142,13 @@ class RPCAPITestCase(base.DbTestCase): driver_method='test-driver-method', info={"test_info": "test_value"}) + def test_driver_vendor_passthru(self): + self._test_rpcapi('driver_vendor_passthru', + 'call', + driver_name='test-driver-name', + driver_method='test-driver-method', + info={'test_key': 'test_value'}) + def test_do_node_deploy(self): self._test_rpcapi('do_node_deploy', 'call', diff --git a/ironic/tests/drivers/test_utils.py b/ironic/tests/drivers/test_utils.py index e36b545c1f..abf1dafe94 100644 --- a/ironic/tests/drivers/test_utils.py +++ b/ironic/tests/drivers/test_utils.py @@ -71,6 +71,42 @@ class UtilsTestCase(base.TestCase): method='second_method', param1='fake1', param2='fake2') + def test_driver_passthru_mixin_success(self): + vendor_a = fake.FakeVendorA() + vendor_a.driver_vendor_passthru = mock.Mock() + vendor_b = fake.FakeVendorB() + vendor_b.driver_vendor_passthru = mock.Mock() + driver_vendor_mapping = { + 'method_a': vendor_a, + 'method_b': vendor_b, + } + mixed_vendor = driver_utils.MixinVendorInterface( + {}, + driver_vendor_mapping) + mixed_vendor.driver_vendor_passthru('context', + 'method_a', + param1='p1') + vendor_a.driver_vendor_passthru.assert_called_once_with( + 'context', + 'method_a', + param1='p1') + + def test_driver_passthru_mixin_unsupported(self): + mixed_vendor = driver_utils.MixinVendorInterface({}, {}) + self.assertRaises(exception.UnsupportedDriverExtension, + mixed_vendor.driver_vendor_passthru, + 'context', + 'fake_method', + param='p1') + + def test_driver_passthru_mixin_unspecified(self): + mixed_vendor = driver_utils.MixinVendorInterface({}) + self.assertRaises(exception.UnsupportedDriverExtension, + mixed_vendor.driver_vendor_passthru, + 'context', + 'fake_method', + param='p1') + def test_get_node_mac_addresses(self): ports = [] ports.append(