cc_ntp: Restructure cc_ntp unit tests.

Any CiTestCase subclass can now set a class attribute with_logs = True and
tests can now make assertions on self.logs.getvalue(). This branch
restructures a bit of cc_ntp module to get better test coverage of the
module. It also restructures the handler_cc_ntp unit tests to avoid nested
mocks where possible. Deeply nested mocks cause a couple of issues:
  - greater risk: mocks are permanent within the scope, so multiple
    call-sites could be affected by package mocks
  - less legible tests: each mock doesn't advertise the actual call-site
  - tight coupling: the unit test logic to tightly bound to the actual
    implementation in remote (unrelated) modules which makes it more
    costly to maintain code
  - false success: we should be testing the expected behavior not specific
    remote method names as we want to know if that underlying behavior
    changes and breaks us.

LP: #1692794

1 files changed, 10 insertions, 15 deletions

diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py
index 225f898a..5cc54536 100644
--- a/cloudinit/config/cc_ntp.py
+++ b/cloudinit/config/cc_ntp.py
@@ -53,14 +53,12 @@ distros = ['centos', 'debian', 'fedora', 'opensuse', 'ubuntu']
 
 
 def handle(name, cfg, cloud, log, _args):
-    """
-    Enable and configure ntp
+    """Enable and configure ntp."""
 
-    ntp:
-       pools: ['0.{{distro}}.pool.ntp.org', '1.{{distro}}.pool.ntp.org']
-       servers: ['192.168.2.1']
-
-    """
+    if 'ntp' not in cfg:
+        LOG.debug(
+            "Skipping module named %s, not present or disabled by cfg", name)
+        return
 
     ntp_cfg = cfg.get('ntp', {})
 
@@ -69,18 +67,12 @@ def handle(name, cfg, cloud, log, _args):
                             " but not a dictionary type,"
                             " is a %s %instead"), type_utils.obj_name(ntp_cfg))
 
-    if 'ntp' not in cfg:
-        LOG.debug("Skipping module named %s,"
-                  "not present or disabled by cfg", name)
-        return True
-
     rename_ntp_conf()
     # ensure when ntp is installed it has a configuration file
     # to use instead of starting up with packaged defaults
     write_ntp_config_template(ntp_cfg, cloud)
     install_ntp(cloud.distro.install_packages, packages=['ntp'],
                 check_exe="ntpd")
-
     # if ntp was already installed, it may not have started
     try:
         reload_ntp(systemd=cloud.distro.uses_systemd())
@@ -98,8 +90,10 @@ def install_ntp(install_func, packages=None, check_exe="ntpd"):
     install_func(packages)
 
 
-def rename_ntp_conf(config=NTP_CONF):
+def rename_ntp_conf(config=None):
     """Rename any existing ntp.conf file and render from template"""
+    if config is None:  # For testing
+        config = NTP_CONF
     if os.path.exists(config):
         util.rename(config, config + ".dist")
 
@@ -117,8 +111,9 @@ def write_ntp_config_template(cfg, cloud):
     pools = cfg.get('pools', [])
 
     if len(servers) == 0 and len(pools) == 0:
-        LOG.debug('Adding distro default ntp pool servers')
         pools = generate_server_names(cloud.distro.name)
+        LOG.debug(
+            'Adding distro default ntp pool servers: %s', ','.join(pools))
 
     params = {
         'servers': servers,