diff --git a/nodepool/cmd/config_validator.py b/nodepool/cmd/config_validator.py index 8a52a5a82..a2c828f85 100644 --- a/nodepool/cmd/config_validator.py +++ b/nodepool/cmd/config_validator.py @@ -27,6 +27,11 @@ class ConfigValidator: self.config_file = config_file def validate(self): + ''' + Validate a configuration file + + :return: 0 for success, non-zero for failure + ''' provider = ProviderConfig.getCommonSchemaDict() label = { @@ -72,11 +77,42 @@ class ConfigValidator: } log.info("validating %s" % self.config_file) - config = yaml.safe_load(open(self.config_file)) - # validate the overall schema - schema = v.Schema(top_level) - schema(config) - for provider_dict in config.get('providers', []): - provider_schema = get_provider_config(provider_dict).getSchema() - provider_schema.extend(provider)(provider_dict) + try: + config = yaml.safe_load(open(self.config_file)) + except Exception: + log.exception('YAML parsing failed') + return 1 + + try: + # validate the overall schema + schema = v.Schema(top_level) + schema(config) + for provider_dict in config.get('providers', []): + provider_schema = \ + get_provider_config(provider_dict).getSchema() + provider_schema.extend(provider)(provider_dict) + except Exception: + log.exception('Schema validation failed') + return 1 + + errors = False + + # Ensure in openstack provider sections, diskimages have + # top-level labels + labels = [x['name'] for x in config.get('labels', [])] + for provider in config.get('providers', []): + if provider.get('driver', 'openstack') != 'openstack': + continue + for pool in provider.get('pools', []): + for label in pool.get('labels', []): + if label['name'] not in labels: + errors = True + log.error("diskimage %s in provider %s " + "not in top-level labels" % + (label['name'], provider['name'])) + + if errors is True: + return 1 + else: + return 0 diff --git a/nodepool/cmd/nodepoolcmd.py b/nodepool/cmd/nodepoolcmd.py index fab987b53..fdfeab455 100755 --- a/nodepool/cmd/nodepoolcmd.py +++ b/nodepool/cmd/nodepoolcmd.py @@ -340,8 +340,7 @@ class NodePoolCmd(NodepoolApp): def config_validate(self): validator = ConfigValidator(self.args.config) - validator.validate() - log.info("Configuration validation complete") + return validator.validate() # TODO(asselin,yolanda): add validation of secure.conf def request_list(self): diff --git a/nodepool/tests/__init__.py b/nodepool/tests/__init__.py index a452af935..11536f820 100644 --- a/nodepool/tests/__init__.py +++ b/nodepool/tests/__init__.py @@ -366,7 +366,9 @@ class DBTestCase(BaseTestCase): self._config_images_dir = images_dir self._config_build_log_dir = build_log_dir validator = ConfigValidator(path) - validator.validate() + ret = validator.validate() + if ret != 0: + raise ValueError("Config file %s could not be validated" % path) return path def replace_config(self, configfile, filename): diff --git a/nodepool/tests/fixtures/config_validate/missing_top_label.yaml b/nodepool/tests/fixtures/config_validate/missing_top_label.yaml new file mode 100644 index 000000000..ef02f8e62 --- /dev/null +++ b/nodepool/tests/fixtures/config_validate/missing_top_label.yaml @@ -0,0 +1,14 @@ +--- +# NOTE(mnaser): The trusty label missing here will fail during runtime. +labels: [] + +providers: + - name: static + driver: openstack + cloud: potato + pools: + - name: main + labels: + - name: trusty + diskimage: trusty + min-ram: 8192 \ No newline at end of file diff --git a/nodepool/tests/unit/test_config_validator.py b/nodepool/tests/unit/test_config_validator.py index ba1789df5..289369ae4 100644 --- a/nodepool/tests/unit/test_config_validator.py +++ b/nodepool/tests/unit/test_config_validator.py @@ -16,8 +16,6 @@ import os from nodepool.cmd.config_validator import ConfigValidator from nodepool import tests -from yaml.parser import ParserError -from voluptuous import MultipleInvalid class TestConfigValidation(tests.BaseTestCase): @@ -30,14 +28,25 @@ class TestConfigValidation(tests.BaseTestCase): 'fixtures', 'config_validate', 'good.yaml') validator = ConfigValidator(config) - validator.validate() + ret = validator.validate() + self.assertEqual(ret, 0) def test_yaml_error(self): config = os.path.join(os.path.dirname(tests.__file__), 'fixtures', 'config_validate', 'yaml_error.yaml') validator = ConfigValidator(config) - self.assertRaises(ParserError, validator.validate) + ret = validator.validate() + self.assertEqual(ret, 1) + + def test_missing_top_level_label(self): + config = os.path.join(os.path.dirname(tests.__file__), + 'fixtures', 'config_validate', + 'missing_top_label.yaml') + + validator = ConfigValidator(config) + ret = validator.validate() + self.assertEqual(ret, 1) def test_schema(self): config = os.path.join(os.path.dirname(tests.__file__), @@ -45,4 +54,5 @@ class TestConfigValidation(tests.BaseTestCase): 'schema_error.yaml') validator = ConfigValidator(config) - self.assertRaises(MultipleInvalid, validator.validate) + ret = validator.validate() + self.assertEqual(ret, 1) diff --git a/nodepool/tests/unit/test_driver_static.py b/nodepool/tests/unit/test_driver_static.py index fb65e8527..0ab1f92c7 100644 --- a/nodepool/tests/unit/test_driver_static.py +++ b/nodepool/tests/unit/test_driver_static.py @@ -21,7 +21,6 @@ from nodepool import config as nodepool_config from nodepool import tests from nodepool import zk from nodepool.cmd.config_validator import ConfigValidator -from voluptuous import MultipleInvalid class TestDriverStatic(tests.DBTestCase): @@ -32,7 +31,8 @@ class TestDriverStatic(tests.DBTestCase): 'fixtures', 'config_validate', 'static_error.yaml') validator = ConfigValidator(config) - self.assertRaises(MultipleInvalid, validator.validate) + ret = validator.validate() + self.assertEqual(ret, 1) def test_static_config(self): configfile = self.setup_config('static.yaml') diff --git a/nodepool/tests/unit/test_sdk_integration.py b/nodepool/tests/unit/test_sdk_integration.py index 3382f9793..5bb64a869 100644 --- a/nodepool/tests/unit/test_sdk_integration.py +++ b/nodepool/tests/unit/test_sdk_integration.py @@ -15,7 +15,6 @@ import os import fixtures -import voluptuous import yaml from nodepool import config as nodepool_config @@ -45,7 +44,7 @@ class TestShadeIntegration(tests.IntegrationTestCase): # Assert that we get a nodepool error and not an openstacksdk # error. self.assertRaises( - voluptuous.MultipleInvalid, + Exception, self.setup_config, 'integration_noocc.yaml') def test_nodepool_occ_config(self):