Validate openstack provider pool labels have top-level labels

We broke nodepool configuration with
I3795fee1530045363e3f629f0793cbe6e95c23ca by not having the labels
defined in the OpenStack provider in the top-level label list.

The added check here would have found such a case.

The validate() function is reworked slightly; previously it would
return various exceptions from the tools it was calling (YAML,
voluptuous, etc.).  Now we have more testing (and I'd imagine we could
do even more, similar vaildations too) we'd have to keep adding
exception types.  Just make the function return a value; this also
makes sure the regular exit paths are taken from the caller in
nodepoolcmd.py, rather than dying with an exception at whatever point.

A unit test is added.

Co-Authored-By: Mohammed Naser <mnaser@vexxhost.com>
Change-Id: I5455f5d7eb07abea34c11a3026d630dee62f2185

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