diff --git a/bareon_ironic/modules/bareon_base.py b/bareon_ironic/modules/bareon_base.py index 79b5f7c..8914fdb 100644 --- a/bareon_ironic/modules/bareon_base.py +++ b/bareon_ironic/modules/bareon_base.py @@ -23,6 +23,8 @@ import json import os import eventlet +import pkg_resources +import stevedore import six from oslo_concurrency import processutils from oslo_config import cfg @@ -94,6 +96,9 @@ agent_opts = [ cfg.IntOpt('check_terminate_max_retries', help='Max retries to check is node already terminated', default=20), + cfg.StrOpt('agent_data_driver', + default='ironic', + help='Fuel-agent data driver'), ] CONF = cfg.CONF @@ -177,6 +182,10 @@ def _clean_up_images(task): class BareonDeploy(base.DeployInterface): """Interface for deploy-related actions.""" + def __init__(self): + super(BareonDeploy, self).__init__() + self._deployment_config_validators = {} + def get_properties(self): """Return the properties of the interface. @@ -233,6 +242,7 @@ class BareonDeploy(base.DeployInterface): :param task: a TaskManager instance. """ self._fetch_resources(task) + self._validate_deployment_config(task) self._prepare_pxe_boot(task) def clean_up(self, task): @@ -300,6 +310,11 @@ class BareonDeploy(base.DeployInterface): with open(filename, 'w') as f: f.write(json.dumps(config)) + def _validate_deployment_config(self, task): + data_driver_name = bareon_utils.node_data_driver(task.node) + validator = self._get_deployment_config_validator(data_driver_name) + validator(get_provision_json_path(task.node)) + def _get_deploy_config(self, task): node = task.node instance_info = node.instance_info @@ -638,6 +653,14 @@ class BareonDeploy(base.DeployInterface): def can_terminate_deployment(self): return True + def _get_deployment_config_validator(self, driver_name): + try: + validator = self._deployment_config_validators[driver_name] + except KeyError: + validator = DeploymentConfigValidator(driver_name) + self._deployment_config_validators[driver_name] = validator + return validator + class BareonVendor(base.VendorInterface): def get_properties(self): @@ -697,8 +720,9 @@ class BareonVendor(base.VendorInterface): params = BareonDeploy._parse_driver_info(node) params['host'] = kwargs.get('address') - cmd = '%s --data_driver ironic --deploy_driver %s' % ( - params.pop('script'), node.instance_info['deploy_driver']) + cmd = '{} --data_driver "{}" --deploy_driver "{}"'.format( + params.pop('script'), bareon_utils.node_data_driver(node), + node.instance_info['deploy_driver']) if CONF.debug: cmd += ' --debug' instance_info = node.instance_info @@ -1052,6 +1076,56 @@ class BareonVendor(base.VendorInterface): task.node.save() +class DeploymentConfigValidator(object): + _driver = None + _namespace = 'bareon.drivers.data' + _min_version = pkg_resources.parse_version('0.0.2') + + def __init__(self, driver_name): + self.driver_name = driver_name + + LOG.debug('Loading bareon data-driver "%s"', self.driver_name) + try: + manager = stevedore.driver.DriverManager( + self._namespace, self.driver_name, verify_requirements=True) + extension = manager[driver_name] + version = extension.entry_point.dist.version + version = pkg_resources.parse_version(version) + LOG.info('Driver %s-%s loaded', extension.name, version) + + if version < self._min_version: + raise RuntimeError( + 'bareon version less than {} does not support ' + 'deployment config validation'.format(self._min_version)) + except RuntimeError as e: + LOG.warning( + 'Fail to load fuel-agent data-driver "%s": %s', + self.driver_name, e) + return + + self._driver = manager.driver + + def __call__(self, deployment_config): + if self._driver is None: + LOG.info( + 'Skipping deployment config validation due to problem in ' + 'loading bareon data driver') + return + + try: + with open(deployment_config, 'rt') as stream: + payload = json.load(stream) + self._driver.validate_data(payload) + except (IOError, ValueError, TypeError) as e: + raise exception.InvalidParameterValue( + 'Unable to load deployment config "{}": {}'.format( + deployment_config, e)) + except self._driver.exc.WrongInputDataError as e: + raise exception.InvalidParameterValue( + 'Deployment config has failed validation.\n' + '{0.message}'.format(e)) + + def get_provision_json_path(node): return os.path.join(resources.get_node_resources_dir(node), "provision.json") diff --git a/bareon_ironic/modules/bareon_utils.py b/bareon_ironic/modules/bareon_utils.py index 7b614f1..22e452c 100644 --- a/bareon_ironic/modules/bareon_utils.py +++ b/bareon_ironic/modules/bareon_utils.py @@ -49,6 +49,14 @@ def change_node_dict(node, dict_name, new_data): setattr(node, dict_name, dict_data) +def node_data_driver(node): + try: + driver = node.instance_info['data_driver'] + except KeyError: + driver = CONF.fuel.agent_data_driver + return driver + + def str_to_alnum(s): if not s.isalnum(): s = ''.join([c for c in s if c.isalnum()]) diff --git a/bareon_ironic/tests/modules/test_bareon_base.py b/bareon_ironic/tests/modules/test_bareon_base.py index 173dd58..ffd1c25 100644 --- a/bareon_ironic/tests/modules/test_bareon_base.py +++ b/bareon_ironic/tests/modules/test_bareon_base.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +import json + import fixtures import mock from ironic.common import exception @@ -20,6 +22,7 @@ from ironic.conductor import task_manager from ironic.tests.unit.objects import utils as test_utils from bareon_ironic.modules import bareon_base +from bareon_ironic.modules import bareon_utils from bareon_ironic.modules.resources import image_service from bareon_ironic.modules.resources import resources from bareon_ironic.tests import base @@ -109,3 +112,134 @@ class BareonBaseTestCase(base.AbstractDBTestCase): result_image = result_image[0] self.assertEqual(origin_image['name'], result_image['name']) + + @mock.patch.object(bareon_base, 'DeploymentConfigValidator') + def test__get_deployment_config_validator(self, validator_cls): + validator_cls.return_value = mock.Mock() + deploy = bareon_base.BareonDeploy() + + for name in ['driver-AAA'] + ['driver-BBB'] * 3 + ['driver-AAA']: + deploy._get_deployment_config_validator(name) + + # NOTE(aostapenko) check that no more than one instance of the + # DeploymentConfigValidator is created for each driver + self.assertEqual( + [mock.call('driver-AAA'), mock.call('driver-BBB')], + validator_cls.call_args_list) + + @mock.patch.object(bareon_base, 'get_provision_json_path') + @mock.patch.object(bareon_utils, 'node_data_driver') + @mock.patch.object(bareon_base, 'DeploymentConfigValidator') + def test__validate_deployment_config( + self, + get_deploy_config_validator_mock, + node_data_driver_mock, + get_provision_json_path_mock): + deploy = bareon_base.BareonDeploy() + + driver_name = 'fake_driver_name' + provision_json = 'fake_provision_json' + task = mock.Mock() + validator_mock = mock.Mock() + get_provision_json_path_mock.return_value = provision_json + node_data_driver_mock.return_value = driver_name + get_deploy_config_validator_mock.return_value = validator_mock + + deploy._validate_deployment_config(task) + + node_data_driver_mock.assert_called_once_with(task.node) + get_deploy_config_validator_mock.assert_called_once_with(driver_name) + get_provision_json_path_mock.assert_called_once_with(task.node) + validator_mock.assert_called_once_with(provision_json) + + +class TestDeploymentConfigValidator(base.AbstractTestCase): + def setUp(self): + super(TestDeploymentConfigValidator, self).setUp() + + self.tmpdir = fixtures.TempDir() + self.useFixture(self.tmpdir) + + self.payload = { + 'ok.json': {'ok': True}, + 'fail.json': {'ok': False, 'fail': True}} + + for name in self.payload: + with open(self.tmpdir.join(name), 'wt') as stream: + json.dump(self.payload[name], stream) + with open(self.tmpdir.join('corrupted.json'), 'wt') as stream: + stream.write('{"corrupted-json-file') + + self.data_driver = mock.Mock() + self.driver_manager = mock.Mock() + self.driver_manager.return_value = mock.MagicMock() + + self.extension = mock.Mock() + self.extension.name = 'dummy' + self.extension.entry_point.dist.version = str( + bareon_base.DeploymentConfigValidator._min_version) + + driver_manager_instance = self.driver_manager.return_value + driver_manager_instance.driver = self.data_driver + driver_manager_instance.__getitem__.return_value = self.extension + + patch = mock.patch( + 'stevedore.driver.DriverManager', self.driver_manager) + patch.start() + self.addCleanup(patch.stop) + + def test_ok(self): + validator = bareon_base.DeploymentConfigValidator('dummy') + validator(self.tmpdir.join('ok.json')) + + self.data_driver.validate_data.assert_called_once_with( + self.payload['ok.json']) + + def test_fail(self): + self.data_driver.exc.WrongInputDataError = DummyError + self.data_driver.validate_data.side_effect = DummyError + + validator = bareon_base.DeploymentConfigValidator('dummy') + self.assertRaises( + exception.InvalidParameterValue, validator, + self.tmpdir.join('fail.json')) + + self.data_driver.validate_data.assert_called_once_with( + self.payload['fail.json']) + + def test_minimal_version(self): + self.extension.entry_point.dist.version = '0.0.1' + + validator = bareon_base.DeploymentConfigValidator('dummy') + validator(self.tmpdir.join('ok.json')) + + self.assertEqual(0, self.data_driver.validate_data.call_count) + self.assertIsNone(validator._driver) + + def test_load_error(self): + self.driver_manager.side_effect = RuntimeError + + validator = bareon_base.DeploymentConfigValidator('dummy') + validator(self.tmpdir.join('ok.json')) + + self.assertEqual(None, validator._driver) + + def test_ioerror(self): + validator = bareon_base.DeploymentConfigValidator('dummy') + with mock.patch('__builtin__.open') as open_mock: + open_mock.side_effect = IOError + self.assertRaises( + exception.InvalidParameterValue, validator, + self.tmpdir.join('ok.json')) + + def test_malformed_json(self): + validator = bareon_base.DeploymentConfigValidator('dummy') + self.assertRaises( + exception.InvalidParameterValue, validator, + self.tmpdir.join('corrupted.json')) + + +class DummyError(Exception): + @property + def message(self): + return self.args[0] if self.args else None diff --git a/etc/ironic/ironic.conf.bareon_sample b/etc/ironic/ironic.conf.bareon_sample index 308592a..406ac61 100644 --- a/etc/ironic/ironic.conf.bareon_sample +++ b/etc/ironic/ironic.conf.bareon_sample @@ -43,6 +43,9 @@ # value) #check_terminate_max_retries=20 +# Fuel-agent data driver (string value) +#agent_data_driver=ironic + [resources] # A prefix that will be added when resource reference is not