cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Diff comments:

> diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py
> index 225f898..257bb40 100644
> --- a/cloudinit/config/cc_ntp.py
> +++ b/cloudinit/config/cc_ntp.py
> @@ -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)
> +    write_ntp_config_template(cfg['ntp'], cloud)

Sorry this leaked in from the follow up branch in which I flipped it back. https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/324499 I'll pull this commit back in from the subsequent branch.

>      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())
> diff --git a/tests/unittests/helpers.py b/tests/unittests/helpers.py
> index d24f817..9ff1599 100644
> --- a/tests/unittests/helpers.py
> +++ b/tests/unittests/helpers.py
> @@ -87,6 +92,27 @@ class TestCase(unittest2.TestCase):
>  class CiTestCase(TestCase):
>      """This is the preferred test case base class unless user
>         needs other test case classes below."""
> +
> +    # Subclass overrides for specific test behavior
> +    # Whether or not a unit test needs logfile setup
> +    with_logs = False
> +
> +    def setUp(self):
> +        super(CiTestCase, self).setUp()
> +        if self.with_logs:
> +            # Create a log handler so unit tests can search expected logs.
> +            logger = logging.getLogger()
> +            self.logs = StringIO()
> +            handler = logging.StreamHandler(self.logs)
> +            self.old_handlers = logger.handlers
> +            logger.handlers = [handler]

I couldn't think of any reason not to have logs by default other than impact to unit test runtime. I'll check how long unit tests take to run with and without global log setup.

> +
> +    def tearDown(self):
> +        if self.with_logs:
> +            # Remove the handler we setup
> +            logging.getLogger().handlers = self.old_handlers
> +        super(CiTestCase, self).tearDown()
> +
>      def tmp_dir(self, dir=None, cleanup=True):
>          # return a full path to a temporary directory that will be cleaned up.
>          if dir is None:
> diff --git a/tests/unittests/test_handler/test_handler_ntp.py b/tests/unittests/test_handler/test_handler_ntp.py
> index 02bd304..aa6c8ae 100644
> --- a/tests/unittests/test_handler/test_handler_ntp.py
> +++ b/tests/unittests/test_handler/test_handler_ntp.py
> @@ -2,54 +2,31 @@
>  
>  from cloudinit.config import cc_ntp
>  from cloudinit.sources import DataSourceNone
> -from cloudinit import templater
>  from cloudinit import (distros, helpers, cloud, util)
>  from ..helpers import FilesystemMockingTestCase, mock
>  
> -import logging
> -import os
> -import shutil
> -import tempfile
>  
> -LOG = logging.getLogger(__name__)
> +import os
>  
> -NTP_TEMPLATE = """
> +NTP_TEMPLATE = b"""\
>  ## template: jinja
> -
> -{% if pools %}# pools

Adding a test for the actual template now.

> -{% endif %}
> -{% for pool in pools -%}
> -pool {{pool}} iburst
> -{% endfor %}
> -{%- if servers %}# servers
> -{% endif %}
> -{% for server in servers -%}
> -server {{server}} iburst
> -{% endfor %}
> -
> -"""
> -
> -
> -NTP_EXPECTED_UBUNTU = """
> -# pools
> -pool 0.mycompany.pool.ntp.org iburst
> -# servers
> -server 192.168.23.3 iburst
> -
> +servers {{servers}}
> +pools {{pools}}
>  """
>  
>  
>  class TestNtp(FilesystemMockingTestCase):
>  
> +    with_logs = True
> +
>      def setUp(self):
>          super(TestNtp, self).setUp()
>          self.subp = util.subp
> -        self.new_root = tempfile.mkdtemp()
> -        self.addCleanup(shutil.rmtree, self.new_root)
> +        self.new_root = self.tmp_dir()
>  
>      def _get_cloud(self, distro, metadata=None):
>          self.patchUtils(self.new_root)
> -        paths = helpers.Paths({})
> +        paths = helpers.Paths({'templates_dir': self.new_root})
>          cls = distros.fetch(distro)
>          mydist = cls(distro, {}, paths)
>          myds = DataSourceNone.DataSourceNone({}, mydist, paths)


-- 
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/324450
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:cc-ntp-testing into cloud-init:master.