Merge "Validate openstack provider pool labels have top-level labels"

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

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):

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):

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

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)

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')

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):