comments addressed
Diff comments:
> diff --git a/tests/unittests/test_handler/test_handler_landscape.py
> b/tests/unittests/test_handler/test_handler_landscape.py
> new file mode 100644
> index 0000000..d35ba60
> --- /dev/null
> +++ b/tests/unittests/test_handler/test_handler_landscape.py
> @@ -0,0 +1,129 @@
> +# This file is part of cloud-init. See LICENSE file for license information.
> +
> +from cloudinit.config import cc_landscape
> +from cloudinit.sources import DataSourceNone
> +from cloudinit import (distros, helpers, cloud, util)
> +from ..helpers import FilesystemMockingTestCase, mock
> +
> +from configobj import ConfigObj
> +import logging
> +import os
> +
> +
> +LOG = logging.getLogger(__name__)
> +
> +
> +class TestLandscape(FilesystemMockingTestCase):
> +
> + with_logs = True
> +
> + def setUp(self):
> + super(TestLandscape, self).setUp()
> + self.new_root = self.tmp_dir()
> + self.conf = os.path.join(self.new_root, 'client.conf')
> + self.default_file = os.path.join(self.new_root, 'default/landscape')
default_file really can be any filename under new_root because we end up
mock.patching the global before writing it out. I probably shouldn't have
added the default/landscape there as it kindof implies that the test behavior
actually try to write to <tmp>/etc/default/landscape. Really all we care about
is <tmp>/something. I'm converting default/landscape to default_landscape just
as an indicator of the file type in case we need to debug the unittest in the
future.
> +
> + def _get_cloud(self, distro):
> + self.patchUtils(self.new_root)
> + paths = helpers.Paths({'templates_dir': self.new_root})
> + cls = distros.fetch(distro)
> + mydist = cls(distro, {}, paths)
> + myds = DataSourceNone.DataSourceNone({}, mydist, paths)
> + return cloud.Cloud(myds, paths, {}, mydist, None)
> +
> + def test_handler_skips_empty_landscape_cloudconfig(self):
> + """Empty landscape cloud-config section does no work."""
> + mycloud = self._get_cloud('ubuntu')
> + mycloud.distro = mock.MagicMock()
> + cfg = {'landscape': {}}
> + cc_landscape.handle('notimportant', cfg, mycloud, LOG, None)
> + self.assertFalse(mycloud.distro.install_packages.called)
> +
> + def test_handler_error_on_invalid_landscape_type(self):
> + """Raise an error when landscape configuraiton option is invalid."""
> + mycloud = self._get_cloud('ubuntu')
> + cfg = {'landscape': 'wrongtype'}
> + with self.assertRaises(RuntimeError) as context_manager:
> + cc_landscape.handle('notimportant', cfg, mycloud, LOG, None)
> + self.assertIn(
> + "'landscape' key existed in config, but not a dict",
> + str(context_manager.exception))
> +
> + @mock.patch('cloudinit.config.cc_landscape.util')
> + def test_handler_restarts_landscape_client(self, m_util):
> + """handler restarts lansdscape-client after install."""
> + mycloud = self._get_cloud('ubuntu')
> + cfg = {'landscape': {'client': {}}}
> + with mock.patch('cloudinit.config.cc_landscape.LSC_CLIENT_CFG_FILE',
> + self.conf):
> + cc_landscape.handle('notimportant', cfg, mycloud, LOG, None)
> + self.assertEqual(
> + [mock.call(['service', 'landscape-client', 'restart'])],
> + m_util.subp.call_args_list)
> +
> + def test_handler_installs_client_and_creates_config_file(self):
> + """Write landscape client.conf and install landscape-client."""
> + self.assertFalse(os.path.exists(self.conf))
True, I just wanted to set the stage that the file doesn't yet exist. But by
virtue of us using our tmp_dir, the file won't exist yet. I'm dropping it, no
need to retest that
> + mycloud = self._get_cloud('ubuntu')
> + cfg = {'landscape': {'client': {}}}
> + expected = {'client': {
> + 'log_level': 'info',
> + 'url': 'https://landscape.canonical.com/message-system',
> + 'ping_url': 'http://landscape.canonical.com/ping',
> + 'data_path': '/var/lib/landscape/client'}}
> + mycloud.distro = mock.MagicMock()
> + with mock.patch('cloudinit.config.cc_landscape.LSC_CLIENT_CFG_FILE',
> + self.conf):
> + with mock.patch('cloudinit.config.cc_landscape.LS_DEFAULT_FILE',
> + self.default_file):
> + cc_landscape.handle('notimportant', cfg, mycloud, LOG, None)
> + self.assertEqual(
> + [mock.call('landscape-client')],
> + mycloud.distro.install_packages.call_args)
> + self.assertEqual(expected, dict(ConfigObj(self.conf)))
> + self.assertIn(
> + 'Wrote landscape config file to {0}'.format(self.conf),
> + self.logs.getvalue())
> + with open(self.default_file) as stream:
> + default_content = stream.read()
> + self.assertEqual('RUN=1\n', default_content)
> +
> + def test_handler_writes_merged_client_config_file_with_defaults(self):
> + """Merge and write options from LSC_CLIENT_CFG_FILE with defaults."""
> + # Write existing sparse client.conf file
> + util.write_file(self.conf, '[client]\ncomputer_title = My PC\n')
> + mycloud = self._get_cloud('ubuntu')
> + cfg = {'landscape': {'client': {}}}
> + expected = {'client': {
> + 'log_level': 'info',
> + 'url': 'https://landscape.canonical.com/message-system',
> + 'ping_url': 'http://landscape.canonical.com/ping',
> + 'data_path': '/var/lib/landscape/client',
> + 'computer_title': 'My PC'}}
> + with mock.patch("cloudinit.config.cc_landscape.LSC_CLIENT_CFG_FILE",
> + self.conf):
> + cc_landscape.handle('notimportant', cfg, mycloud, LOG, None)
> + self.assertEqual(expected, dict(ConfigObj(self.conf)))
> + self.assertIn(
> + 'Wrote landscape config file to {0}'.format(self.conf),
> + self.logs.getvalue())
> +
> + def test_handler_writes_merged_provided_cloudconfig_with_defaults(self):
> + """Merge and write options from cloud-config options with
> defaults."""
> + # Write empty sparse client.conf file
> + util.write_file(self.conf, '')
> + mycloud = self._get_cloud('ubuntu')
> + cfg = {'landscape': {'client': {'computer_title': 'My PC'}}}
> + expected = {'client': {
> + 'log_level': 'info',
> + 'url': 'https://landscape.canonical.com/message-system',
> + 'ping_url': 'http://landscape.canonical.com/ping',
> + 'data_path': '/var/lib/landscape/client',
> + 'computer_title': 'My PC'}}
> + with mock.patch("cloudinit.config.cc_landscape.LSC_CLIENT_CFG_FILE",
> + self.conf):
> + cc_landscape.handle('notimportant', cfg, mycloud, LOG, None)
> + self.assertEqual(expected, dict(ConfigObj(self.conf)))
> + self.assertIn(
> + 'Wrote landscape config file to {0}'.format(self.conf),
> + self.logs.getvalue())
> diff --git a/tests/unittests/test_handler/test_handler_puppet.py
> b/tests/unittests/test_handler/test_handler_puppet.py
> new file mode 100644
> index 0000000..805c76b
> --- /dev/null
> +++ b/tests/unittests/test_handler/test_handler_puppet.py
> @@ -0,0 +1,142 @@
> +# This file is part of cloud-init. See LICENSE file for license information.
> +
> +from cloudinit.config import cc_puppet
> +from cloudinit.sources import DataSourceNone
> +from cloudinit import (distros, helpers, cloud, util)
> +from ..helpers import CiTestCase, mock
> +
> +import logging
> +
> +
> +LOG = logging.getLogger(__name__)
> +
> +
> +@mock.patch('cloudinit.config.cc_puppet.util')
> +@mock.patch('cloudinit.config.cc_puppet.os')
mock is smart and doesn't seem to add the patches to internal private or even
public non-test methods and since all the unit tests here used both mocks it
let me cut down a bit on duplication.
> +class TestAutostartPuppet(CiTestCase):
> +
> + with_logs = True
> +
> + def test_wb_autostart_puppet_updates_puppet_default(self, m_os, m_util):
> + """Update /etc/default/puppet to autostart if it exists."""
> +
> + def _fake_exists(path):
> + return path == '/etc/default/puppet'
> +
> + m_os.path.exists.side_effect = _fake_exists
> + cc_puppet._autostart_puppet(LOG)
> + self.assertEqual(
> + [mock.call(['sed', '-i', '-e', 's/^START=.*/START=yes/',
> + '/etc/default/puppet'], capture=False)],
> + m_util.subp.call_args_list)
> +
> + def test_wb_autostart_pupppet_enables_puppet_systemctl(self, m_os,
> m_util):
> + """If systemctl is present, enable puppet via systemctl."""
> +
> + def _fake_exists(path):
> + return path == '/bin/systemctl'
> +
> + m_os.path.exists.side_effect = _fake_exists
> + cc_puppet._autostart_puppet(LOG)
> + expected_calls = [mock.call(
> + ['/bin/systemctl', 'enable', 'puppet.service'], capture=False)]
> + self.assertEqual(expected_calls, m_util.subp.call_args_list)
> +
> + def test_wb_autostart_pupppet_enables_puppet_chkconfig(self, m_os,
> m_util):
> + """If chkconfig is present, enable puppet via checkcfg."""
> +
> + def _fake_exists(path):
> + return path == '/sbin/chkconfig'
> +
> + m_os.path.exists.side_effect = _fake_exists
> + cc_puppet._autostart_puppet(LOG)
> + expected_calls = [mock.call(
> + ['/sbin/chkconfig', 'puppet', 'on'], capture=False)]
> + self.assertEqual(expected_calls, m_util.subp.call_args_list)
> +
> +
> +@mock.patch('cloudinit.config.cc_puppet._autostart_puppet')
> +class TestPuppetHandle(CiTestCase):
> +
> + with_logs = True
> +
> + def setUp(self):
> + super(TestPuppetHandle, self).setUp()
> + self.new_root = self.tmp_dir()
> + self.conf = self.tmp_path('puppet.conf')
> +
> + def _get_cloud(self, distro):
> + paths = helpers.Paths({'templates_dir': self.new_root})
> + cls = distros.fetch(distro)
> + mydist = cls(distro, {}, paths)
> + myds = DataSourceNone.DataSourceNone({}, mydist, paths)
> + return cloud.Cloud(myds, paths, {}, mydist, None)
> +
> + def test_handler_skips_missing_puppet_key_in_cloudconfig(self, m_auto):
> + """Cloud-config containing no 'puppet' key is skipped."""
> + mycloud = self._get_cloud('ubuntu')
> + cfg = {}
> + cc_puppet.handle('notimportant', cfg, mycloud, LOG, None)
> + self.assertIn(
> + "no 'puppet' configuration found", self.logs.getvalue())
> + self.assertEqual(0, m_auto.call_count)
> +
> + @mock.patch('cloudinit.config.cc_puppet.util.subp')
> + def test_handler_puppet_config_starts_puppet_service(self, m_subp,
> m_auto):
> + """Cloud-config 'puppet' configuration starts puppet."""
> + mycloud = self._get_cloud('ubuntu')
> + cfg = {'puppet': {'install': False}}
> + cc_puppet.handle('notimportant', cfg, mycloud, LOG, None)
> + self.assertEqual(1, m_auto.call_count)
> + self.assertEqual(
> + [mock.call(['service', 'puppet', 'start'], capture=False)],
> + m_subp.call_args_list)
> +
> + @mock.patch('cloudinit.config.cc_puppet.util.subp')
> + def test_handler_empty_puppet_config_installs_puppet(self, m_subp,
> m_auto):
> + """Cloud-config empty 'puppet' configuration installs latest
> puppet."""
> + mycloud = self._get_cloud('ubuntu')
> + mycloud.distro = mock.MagicMock()
> + cfg = {'puppet': {}}
> + cc_puppet.handle('notimportant', cfg, mycloud, LOG, None)
> + self.assertEqual(
> + [mock.call(('puppet', None))],
> + mycloud.distro.install_packages.call_args_list)
> +
> + @mock.patch('cloudinit.config.cc_puppet.util.subp')
> + def test_handler_puppet_config_installs_puppet_on_true(self, m_subp, _):
> + """Cloud-config with 'puppet' key installs when 'install' is True."""
> + mycloud = self._get_cloud('ubuntu')
> + mycloud.distro = mock.MagicMock()
> + cfg = {'puppet': {'install': True}}
> + cc_puppet.handle('notimportant', cfg, mycloud, LOG, None)
> + self.assertEqual(
> + [mock.call(('puppet', None))],
> + mycloud.distro.install_packages.call_args_list)
> +
> + @mock.patch('cloudinit.config.cc_puppet.util.subp')
> + def test_handler_puppet_config_installs_puppet_version(self, m_subp, _):
> + """Cloud-config 'puppet' configuration can specify a version."""
> + mycloud = self._get_cloud('ubuntu')
> + mycloud.distro = mock.MagicMock()
> + cfg = {'puppet': {'version': '3.8'}}
> + cc_puppet.handle('notimportant', cfg, mycloud, LOG, None)
> + self.assertEqual(
> + [mock.call(('puppet', '3.8'))],
> + mycloud.distro.install_packages.call_args_list)
> +
> + @mock.patch('cloudinit.config.cc_puppet.util.subp')
> + def test_handler_puppet_config_updates_puppet_conf(self, m_subp, m_auto):
> + """When 'conf' is provided update values in PUPPET_CONF_PATH."""
> + mycloud = self._get_cloud('ubuntu')
> + cfg = {
> + 'puppet': {
> + 'conf': {'agent': {'server': 'puppetmaster.example.org'}}}}
> + util.write_file(self.conf, '[agent]\nserver = origpuppet\nother = 3')
> + puppet_conf_path = 'cloudinit.config.cc_puppet.PUPPET_CONF_PATH'
> + mycloud.distro = mock.MagicMock()
> + with mock.patch(puppet_conf_path, self.conf):
> + cc_puppet.handle('notimportant', cfg, mycloud, LOG, None)
> + content = util.load_file(self.conf)
> + expected = '[agent]\nserver = puppetmaster.example.org\nother =
> 3\n\n'
> + self.assertEqual(expected, content)
--
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/329087
Your team cloud-init commiters is requested to review the proposed merge of
~chad.smith/cloud-init:cc-landscape-py3-config-fix into cloud-init:master.
_______________________________________________
Mailing list: https://launchpad.net/~cloud-init-dev
Post to : cloud-init-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~cloud-init-dev
More help : https://help.launchpad.net/ListHelp
