From fd0c88cf8e8aa015eadb5ab842e872cb627197ec Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Mon, 22 May 2017 23:11:42 -0600 Subject: 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 --- cloudinit/config/cc_ntp.py | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) (limited to 'cloudinit/config/cc_ntp.py') 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, -- cgit v1.2.3