Merge ~chad.smith/cloud-init:ntp-schema into cloud-init:master

[Chad Smith]

ntp: fix config module schema to allow empty ntp config

Fix three things related to the ntp module:
  1. Fix invalid cloud-config schema in the integration test which
     provided empty dicts instead of emptylists for pools and servers
  2. Correct logic in the ntp module to allow support for the minimal
     cloud-config 'ntp:' without raising a RuntimeError. Docs and schema
     definitions already describe that cloud-config's ntp can be empty.
     An ntp configuration with neither pools nor servers will be
     configured with a default set of ntp pools. As such, the ntp module
     now officially allows the following ntp cloud-configs:
     - ntp:
     - ntp: {}
     - ntp:
         servers: []
         pools: []
  3. Add a simple unit test which validates all cloud-config provided to
      our integration tests to ensure it adheres to any defined module
      schema so as more jsonschema definitions are added, we validate our
      integration test configs.

LP: #1724951

ntp: fix config module schema to allow empty ntp config

Fix three things related to the ntp module:
  1. Fix invalid cloud-config schema in the integration test which
     provided empty dicts instead of emptylists for pools and servers
  2. Correct logic in the ntp module to allow support for the minimal
     cloud-config 'ntp:' without raising a RuntimeError. Docs and schema
     definitions already describe that cloud-config's ntp can be empty.
     An ntp configuration with neither pools nor servers will be
     configured with a default set of ntp pools. As such, the ntp module
     now officially allows the following ntp cloud-configs:
     - ntp:
     - ntp: {}
     - ntp:
         servers: []
         pools: []
  3. Add a simple unit test which validates all cloud-config provided to
      our integration tests to ensure it adheres to any defined module
      schema so as more jsonschema definitions are added, we validate our
      integration test configs.

LP: #1724951

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:545b0dd39f6e3741d6e9b698bdfd36d73e82abf7
https://jenkins.ubuntu.com/server/job/cloud-init-ci/415/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    FAILED: MAAS Compatability Testing

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/415/rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:545b0dd39f6e3741d6e9b698bdfd36d73e82abf7
https://jenkins.ubuntu.com/server/job/cloud-init-ci/416/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/416/rebuild

review: Approve (continuous-integration)

[Joshua Powers (powersj)]

LGTM - Thanks for fixing the integration test!

review: Approve

[Scott Moser (smoser)]

review: Approve

diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py
index 15ae1ec..d43d060 100644
--- a/cloudinit/config/cc_ntp.py
+++ b/cloudinit/config/cc_ntp.py
@@ -100,7 +100,9 @@ def handle(name, cfg, cloud, log, _args):
         LOG.debug(
             "Skipping module named %s, not present or disabled by cfg", name)
         return
-    ntp_cfg = cfg.get('ntp', {})
+    ntp_cfg = cfg['ntp']
+    if ntp_cfg is None:
+        ntp_cfg = {}  # Allow empty config which will install the package
 
     # TODO drop this when validate_cloudconfig_schema is strict=True
     if not isinstance(ntp_cfg, (dict)):
diff --git a/tests/cloud_tests/testcases/modules/ntp.yaml b/tests/cloud_tests/testcases/modules/ntp.yaml
index fbef431..2530d72 100644
--- a/tests/cloud_tests/testcases/modules/ntp.yaml
+++ b/tests/cloud_tests/testcases/modules/ntp.yaml
@@ -4,8 +4,8 @@
 cloud_config: |
   #cloud-config
   ntp:
-    pools: {}
-    servers: {}
+    pools: []
+    servers: []
 collect_scripts:
   ntp_installed: |
     #!/bin/bash
diff --git a/tests/unittests/test_handler/test_handler_ntp.py b/tests/unittests/test_handler/test_handler_ntp.py
index 4f29124..3abe578 100644
--- a/tests/unittests/test_handler/test_handler_ntp.py
+++ b/tests/unittests/test_handler/test_handler_ntp.py
@@ -293,23 +293,24 @@ class TestNtp(FilesystemMockingTestCase):
 
     def test_ntp_handler_schema_validation_allows_empty_ntp_config(self):
         """Ntp schema validation allows for an empty ntp: configuration."""
-        invalid_config = {'ntp': {}}
+        valid_empty_configs = [{'ntp': {}}, {'ntp': None}]
         distro = 'ubuntu'
         cc = self._get_cloud(distro)
         ntp_conf = os.path.join(self.new_root, 'ntp.conf')
         with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
             stream.write(NTP_TEMPLATE)
-        with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
-            cc_ntp.handle('cc_ntp', invalid_config, cc, None, [])
+        for valid_empty_config in valid_empty_configs:
+            with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
+                cc_ntp.handle('cc_ntp', valid_empty_config, cc, None, [])
+            with open(ntp_conf) as stream:
+                content = stream.read()
+            default_pools = [
+                "{0}.{1}.pool.ntp.org".format(x, distro)
+                for x in range(0, cc_ntp.NR_POOL_SERVERS)]
+            self.assertEqual(
+                "servers []\npools {0}\n".format(default_pools),
+                content)
         self.assertNotIn('Invalid config:', self.logs.getvalue())
-        with open(ntp_conf) as stream:
-            content = stream.read()
-        default_pools = [
-            "{0}.{1}.pool.ntp.org".format(x, distro)
-            for x in range(0, cc_ntp.NR_POOL_SERVERS)]
-        self.assertEqual(
-            "servers []\npools {0}\n".format(default_pools),
-            content)
 
     @skipIf(_missing_jsonschema_dep, "No python-jsonschema dependency")
     def test_ntp_handler_schema_validation_warns_non_string_item_type(self):
diff --git a/tests/unittests/test_handler/test_schema.py b/tests/unittests/test_handler/test_schema.py
index b8fc893..648573f 100644
--- a/tests/unittests/test_handler/test_schema.py
+++ b/tests/unittests/test_handler/test_schema.py
@@ -4,11 +4,12 @@ from cloudinit.config.schema import (
     CLOUD_CONFIG_HEADER, SchemaValidationError, annotated_cloudconfig_file,
     get_schema_doc, get_schema, validate_cloudconfig_file,
     validate_cloudconfig_schema, main)
-from cloudinit.util import write_file
+from cloudinit.util import subp, write_file
 
 from cloudinit.tests.helpers import CiTestCase, mock, skipIf
 
 from copy import copy
+import os
 from six import StringIO
 from textwrap import dedent
 from yaml import safe_load
@@ -364,4 +365,38 @@ class MainTest(CiTestCase):
         self.assertIn(
             'Valid cloud-config file {0}'.format(myyaml), m_stdout.getvalue())
 
+
+class CloudTestsIntegrationTest(CiTestCase):
+    """Validate all cloud-config yaml schema provided in integration tests.
+
+    It is less expensive to have unittests validate schema of all cloud-config
+    yaml provided to integration tests, than to run an integration test which
+    raises Warnings or errors on invalid cloud-config schema.
+    """
+
+    @skipIf(_missing_jsonschema_dep, "No python-jsonschema dependency")
+    def test_all_integration_test_cloud_config_schema(self):
+        """Validate schema of cloud_tests yaml files looking for warnings."""
+        schema = get_schema()
+        testsdir = os.path.dirname(os.path.dirname(os.path.dirname(__file__)))
+        integration_testdir = os.path.sep.join(
+            [testsdir, 'cloud_tests', 'testcases'])
+        errors = []
+        out, _ = subp(['find', integration_testdir, '-name', '*yaml'])
+        for filename in out.splitlines():
+            test_cfg = safe_load(open(filename))
+            cloud_config = test_cfg.get('cloud_config')
+            if cloud_config:
+                cloud_config = safe_load(
+                    cloud_config.replace("#cloud-config\n", ""))
+                try:
+                    validate_cloudconfig_schema(
+                        cloud_config, schema, strict=True)
+                except SchemaValidationError as e:
+                    errors.append(
+                        '{0}: {1}'.format(
+                            filename, e))
+        if errors:
+            raise AssertionError(', '.join(errors))
+
 # vi: ts=4 expandtab syntax=python