From d0e7bd2973dd5ec03544b55965c805926d210eb7 Mon Sep 17 00:00:00 2001 From: Tan Lin Date: Thu, 13 Nov 2014 22:25:08 +0800 Subject: [PATCH] Add driver_validate() vendor.driver_vendor_passthru() should be validated before called on Ironic conductor, like vendor_passthru(). Add a new method driver_validate() to support this. Change-Id: Idff802cece01940b8ade34fa343f1bc8e76f1630 Closes-Bug: #1391580 --- ironic/conductor/manager.py | 11 ++++------- ironic/drivers/base.py | 13 +++++++++++++ ironic/drivers/modules/agent.py | 20 +++++++++++++++----- ironic/tests/conductor/test_manager.py | 17 +++++++++++++++++ ironic/tests/drivers/test_agent.py | 19 +++++++++++++++++++ ironic/tests/drivers/test_base.py | 3 +++ 6 files changed, 71 insertions(+), 12 deletions(-) diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 93edc53b5c..27c5347266 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -506,6 +506,8 @@ class ConductorManager(periodic_task.PeriodicTasks): "of driver_vendor_passthru() has been " "deprecated. Please update the code to use " "the @driver_passthru decorator.")) + + driver.vendor.driver_validate(method=driver_method, **info) ret = driver.vendor.driver_vendor_passthru( context, method=driver_method, **info) # DriverVendorPassthru was always sync @@ -527,16 +529,11 @@ class ConductorManager(periodic_task.PeriodicTasks): # Inform the vendor method which HTTP method it was invoked with info['http_method'] = http_method - # FIXME(lucasagomes): This code should be able to call - # validate(). The problem is that at the moment the validate() - # expects a task object as the first parameter and in this - # method we don't have it because we don't have a Node to - # acquire. - # See: https://bugs.launchpad.net/ironic/+bug/1391580 - # Invoke the vendor method accordingly with the mode is_async = vendor_opts['async'] ret = None + driver.vendor.driver_validate(method=driver_method, **info) + if is_async: self._spawn_worker(vendor_func, context, **info) else: diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index 572ff90903..23253e86b5 100644 --- a/ironic/drivers/base.py +++ b/ironic/drivers/base.py @@ -481,6 +481,19 @@ class VendorInterface(object): :raises: MissingParameterValue """ + def driver_validate(self, method, **kwargs): + """Validate driver-vendor-passthru actions. + + If invalid, raises an exception; otherwise returns None. + + :param method: method to be validated + :param kwargs: info for action. + :raises: MissingParameterValue if kwargs does not contain + certain parameter. + :raises: InvalidParameterValue if parameter does not match. + """ + pass + @six.add_metaclass(abc.ABCMeta) class ManagementInterface(object): diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index e716a09c89..4528002ca7 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -328,6 +328,21 @@ class AgentVendorInterface(base.VendorInterface): """ pass + def driver_validate(self, method, **kwargs): + """Validate the driver deployment info. + + :param method: method to be validated. + """ + version = kwargs.get('version') + + if not version: + raise exception.MissingParameterValue(_('Missing parameter ' + 'version')) + if version not in self.supported_payload_versions: + raise exception.InvalidParameterValue(_('Unknown lookup ' + 'payload version: %s') + % version) + @base.passthru(['POST']) def heartbeat(self, task, **kwargs): """Method for agent to periodically check in. @@ -476,11 +491,6 @@ class AgentVendorInterface(base.VendorInterface): :raises: NotFound if no matching node is found. :raises: InvalidParameterValue with unknown payload version """ - version = kwargs.get('version') - - if version not in self.supported_payload_versions: - raise exception.InvalidParameterValue(_('Unknown lookup payload ' - 'version: %s') % version) inventory = kwargs.get('inventory') interfaces = self._get_interfaces(inventory) mac_addresses = self._get_mac_addresses(interfaces) diff --git a/ironic/tests/conductor/test_manager.py b/ironic/tests/conductor/test_manager.py index e7685ba183..ecf0548303 100644 --- a/ironic/tests/conductor/test_manager.py +++ b/ironic/tests/conductor/test_manager.py @@ -841,6 +841,23 @@ class VendorPassthruTestCase(_ServiceSetUpMixin, tests_db_base.DbTestCase): self.assertEqual(exception.UnsupportedDriverExtension, exc.exc_info[0]) + @mock.patch.object(drivers_base.VendorInterface, 'driver_validate') + def test_driver_vendor_passthru_validation_failed(self, validate_mock): + validate_mock.side_effect = exception.MissingParameterValue('error') + test_method = mock.Mock() + self.driver.vendor.driver_routes = {'test_method': + {'func': test_method, + 'async': False, + 'http_methods': ['POST']}} + self.service.init_host() + exc = self.assertRaises(messaging.ExpectedException, + self.service.driver_vendor_passthru, + self.context, 'fake', 'test_method', + 'POST', {}) + self.assertEqual(exception.MissingParameterValue, + exc.exc_info[0]) + self.assertFalse(test_method.called) + @_mock_record_keepalive class DoNodeDeployTearDownTestCase(_ServiceSetUpMixin, diff --git a/ironic/tests/drivers/test_agent.py b/ironic/tests/drivers/test_agent.py index d9c3191125..3eafb39d19 100644 --- a/ironic/tests/drivers/test_agent.py +++ b/ironic/tests/drivers/test_agent.py @@ -135,6 +135,25 @@ class TestAgentVendor(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid) as task: self.passthru.validate(task) + def test_driver_validate(self): + kwargs = {'version': '2'} + method = 'lookup' + self.passthru.driver_validate(method, **kwargs) + + def test_driver_validate_invalid_paremeter(self): + method = 'lookup' + kwargs = {'version': '1'} + self.assertRaises(exception.InvalidParameterValue, + self.passthru.driver_validate, + method, **kwargs) + + def test_driver_validate_missing_parameter(self): + method = 'lookup' + kwargs = {} + self.assertRaises(exception.MissingParameterValue, + self.passthru.driver_validate, + method, **kwargs) + @mock.patch('ironic.common.image_service.Service') def test_continue_deploy(self, image_service_mock): test_temp_url = 'http://image' diff --git a/ironic/tests/drivers/test_base.py b/ironic/tests/drivers/test_base.py index bb1a7ad09e..1f4f35f139 100644 --- a/ironic/tests/drivers/test_base.py +++ b/ironic/tests/drivers/test_base.py @@ -39,6 +39,9 @@ class FakeVendorInterface(driver_base.VendorInterface): def validate(self, task, **kwargs): pass + def driver_validate(self, **kwargs): + pass + class PassthruDecoratorTestCase(base.TestCase):