cloud-init-dev team mailing list archive
-
cloud-init-dev team
-
Mailing list archive
-
Message #01510
[Merge] ~raharper/cloud-init:bug-lp-1645644-ntp into cloud-init:master
Ryan Harper has proposed merging ~raharper/cloud-init:bug-lp-1645644-ntp into cloud-init:master.
Requested reviews:
cloud init development team (cloud-init-dev)
Related bugs:
Bug #1645644 in cloud-init: "ntp not using expected servers"
https://bugs.launchpad.net/cloud-init/+bug/1645644
For more details, see:
https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/314539
cc_ntp: write template before installing and add service restart
On systems which installed ntp and specified servers or pools in the config
ntpd didn't notice the updated configuration file and didn't use the
correct configuration. Resolve this by rendering the template first which
allows the package install to use the existing configuration. Additionally
add a service restart to handle the case where ntp does not need to be installed
but it may not have started.
Add an integration test to confirm that cc_ntp enables ntp to use the specific
servers and pools in the cloud-config.
LP: #1645644
--
Your team cloud init development team is requested to review the proposed merge of ~raharper/cloud-init:bug-lp-1645644-ntp into cloud-init:master.
diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py
index e33032f..15813bf 100644
--- a/cloudinit/config/cc_ntp.py
+++ b/cloudinit/config/cc_ntp.py
@@ -74,10 +74,18 @@ def handle(name, cfg, cloud, log, _args):
"not present or disabled by cfg", name)
return True
- install_ntp(cloud.distro.install_packages, packages=['ntp'],
- check_exe="ntpd")
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())
+ except util.ProcessExecutionError as e:
+ log.warn("Failed to reload/start ntp service", e)
def install_ntp(install_func, packages=None, check_exe="ntpd"):
@@ -90,6 +98,7 @@ def install_ntp(install_func, packages=None, check_exe="ntpd"):
def rename_ntp_conf(config=NTP_CONF):
+ """ Rename any existing ntp.conf file and render from template """
if os.path.exists(config):
util.rename(config, config + ".dist")
@@ -125,4 +134,14 @@ def write_ntp_config_template(cfg, cloud):
templater.render_to_file(template_fn, NTP_CONF, params)
+
+def reload_ntp(systemd=False):
+ service = 'ntp'
+ if systemd:
+ cmd = ['systemctl', 'reload-or-restart', service]
+ else:
+ cmd = ['service', service, 'restart']
+ util.subp(cmd, capture=True)
+
+
# vi: ts=4 expandtab
diff --git a/tests/cloud_tests/configs/modules/ntp_pools.yaml b/tests/cloud_tests/configs/modules/ntp_pools.yaml
index bd0ac29..e040cc3 100644
--- a/tests/cloud_tests/configs/modules/ntp_pools.yaml
+++ b/tests/cloud_tests/configs/modules/ntp_pools.yaml
@@ -5,10 +5,9 @@ cloud_config: |
#cloud-config
ntp:
pools:
- - 0.pool.ntp.org
- - 1.pool.ntp.org
- - 2.pool.ntp.org
- - 3.pool.ntp.org
+ - 0.cloud-init.mypool
+ - 1.cloud-init.mypool
+ - 172.16.15.14
collect_scripts:
ntp_installed_pools: |
#!/bin/bash
@@ -19,5 +18,8 @@ collect_scripts:
ntp_conf_pools: |
#!/bin/bash
grep '^pool' /etc/ntp.conf
+ ntpq_servers: |
+ #!/bin/sh
+ ntpq -p -w
# vi: ts=4 expandtab
diff --git a/tests/cloud_tests/configs/modules/ntp_servers.yaml b/tests/cloud_tests/configs/modules/ntp_servers.yaml
index 934b9c5..e0564a0 100644
--- a/tests/cloud_tests/configs/modules/ntp_servers.yaml
+++ b/tests/cloud_tests/configs/modules/ntp_servers.yaml
@@ -5,16 +5,20 @@ cloud_config: |
#cloud-config
ntp:
servers:
- - pool.ntp.org
+ - 172.16.15.14
+ - 172.16.17.18
collect_scripts:
ntp_installed_servers: |
- #!/bin/bash
- dpkg -l | grep ntp | wc -l
+ #!/bin/sh
+ dpkg -l | grep -c ntp
ntp_conf_dist_servers: |
- #!/bin/bash
- ls /etc/ntp.conf.dist | wc -l
+ #!/bin/sh
+ cat /etc/ntp.conf.dist | wc -l
ntp_conf_servers: |
- #!/bin/bash
+ #!/bin/sh
grep '^server' /etc/ntp.conf
+ ntpq_servers: |
+ #!/bin/sh
+ ntpq -p -w
# vi: ts=4 expandtab
diff --git a/tests/cloud_tests/testcases/modules/ntp.py b/tests/cloud_tests/testcases/modules/ntp.py
index b111925..82d3288 100644
--- a/tests/cloud_tests/testcases/modules/ntp.py
+++ b/tests/cloud_tests/testcases/modules/ntp.py
@@ -13,9 +13,9 @@ class TestNtp(base.CloudTestCase):
self.assertEqual(1, int(out))
def test_ntp_dist_entries(self):
- """Test dist config file has one entry"""
+ """Test dist config file is empty"""
out = self.get_data_file('ntp_conf_dist_empty')
- self.assertEqual(1, int(out))
+ self.assertEqual(0, int(out))
def test_ntp_entires(self):
"""Test config entries"""
diff --git a/tests/cloud_tests/testcases/modules/ntp_pools.py b/tests/cloud_tests/testcases/modules/ntp_pools.py
index d80cb67..0ce4a5e 100644
--- a/tests/cloud_tests/testcases/modules/ntp_pools.py
+++ b/tests/cloud_tests/testcases/modules/ntp_pools.py
@@ -13,16 +13,23 @@ class TestNtpPools(base.CloudTestCase):
self.assertEqual(1, int(out))
def test_ntp_dist_entries(self):
- """Test dist config file has one entry"""
+ """Test dist config file is empty"""
out = self.get_data_file('ntp_conf_dist_pools')
- self.assertEqual(1, int(out))
+ self.assertEqual(0, int(out))
def test_ntp_entires(self):
"""Test config entries"""
out = self.get_data_file('ntp_conf_pools')
- self.assertIn('pool 0.pool.ntp.org iburst', out)
- self.assertIn('pool 1.pool.ntp.org iburst', out)
- self.assertIn('pool 2.pool.ntp.org iburst', out)
- self.assertIn('pool 3.pool.ntp.org iburst', out)
+ pools = self.cloud_config.get('ntp').get('pools')
+ for pool in pools:
+ self.assertIn('pool %s iburst' % pool, out)
+
+ def test_ntpq_servers(self):
+ """Test ntpq output has configured servers"""
+ out = self.get_data_file('ntpq_servers')
+ pools = self.cloud_config.get('ntp').get('pools')
+ for pool in pools:
+ self.assertIn(pool, out)
+
# vi: ts=4 expandtab
diff --git a/tests/cloud_tests/testcases/modules/ntp_servers.py b/tests/cloud_tests/testcases/modules/ntp_servers.py
index 4879bb6..9ef270e 100644
--- a/tests/cloud_tests/testcases/modules/ntp_servers.py
+++ b/tests/cloud_tests/testcases/modules/ntp_servers.py
@@ -13,13 +13,22 @@ class TestNtpServers(base.CloudTestCase):
self.assertEqual(1, int(out))
def test_ntp_dist_entries(self):
- """Test dist config file has one entry"""
+ """Test dist config file is empty"""
out = self.get_data_file('ntp_conf_dist_servers')
- self.assertEqual(1, int(out))
+ self.assertEqual(0, int(out))
def test_ntp_entires(self):
- """Test config entries"""
- out = self.get_data_file('ntp_conf_servers')
- self.assertIn('server pool.ntp.org iburst', out)
+ """Test config pools entries"""
+ out = self.get_data_file('ntp_conf_pools')
+ servers = self.cloud_config.get('ntp').get('servers')
+ for server in servers:
+ self.assertIn('server %s iburst' % server, out)
+
+ def test_ntpq_servers(self):
+ """Test ntpq output has configured servers"""
+ out = self.get_data_file('ntpq_servers')
+ servers = self.cloud_config.get('ntp').get('servers')
+ for server in servers:
+ self.assertIn(server, out)
# vi: ts=4 expandtab
diff --git a/tests/unittests/test_handler/test_handler_ntp.py b/tests/unittests/test_handler/test_handler_ntp.py
index ec60007..5674d7a 100644
--- a/tests/unittests/test_handler/test_handler_ntp.py
+++ b/tests/unittests/test_handler/test_handler_ntp.py
@@ -259,13 +259,46 @@ class TestNtp(FilesystemMockingTestCase):
with mock.patch.object(os.path, 'isfile',
return_value=True):
with mock.patch.object(util, 'rename'):
- cc_ntp.handle("notimportant", cfg,
- mycloud, LOG, None)
+ with mock.patch.object(util, 'subp') as msubp:
+ cc_ntp.handle("notimportant", cfg,
+ mycloud, LOG, None)
mockwrite.assert_called_once_with(
'/etc/ntp.conf',
NTP_EXPECTED_UBUNTU,
mode=420)
+ msubp.assert_any_call(['systemctl', 'reload-or-restart', 'ntp'],
+ capture=True)
+
+ @mock.patch("cloudinit.config.cc_ntp.install_ntp")
+ @mock.patch("cloudinit.config.cc_ntp.write_ntp_config_template")
+ @mock.patch("cloudinit.config.cc_ntp.rename_ntp_conf")
+ def test_write_config_before_install(self, mock_ntp_rename,
+ mock_ntp_write_config,
+ mock_install_ntp):
+ pools = ['0.mycompany.pool.ntp.org']
+ servers = ['192.168.23.3']
+ cfg = {
+ 'ntp': {
+ 'pools': pools,
+ 'servers': servers,
+ }
+ }
+ cc = self._get_cloud('ubuntu')
+ cc.distro = mock.MagicMock()
+ mock_parent = mock.MagicMock()
+ mock_parent.attach_mock(mock_ntp_rename, 'mock_ntp_rename')
+ mock_parent.attach_mock(mock_ntp_write_config, 'mock_ntp_write_config')
+ mock_parent.attach_mock(mock_install_ntp, 'mock_install_ntp')
+
+ cc_ntp.handle('cc_ntp', cfg, cc, LOG, None)
+
+ """Check call order"""
+ mock_parent.assert_has_calls([
+ mock.call.mock_ntp_rename(),
+ mock.call.mock_ntp_write_config(cfg.get('ntp'), cc),
+ mock.call.mock_install_ntp(cc.distro.install_packages,
+ packages=['ntp'], check_exe="ntpd")])
@mock.patch("cloudinit.config.cc_ntp.util")
def test_no_ntpcfg_does_nothing(self, mock_util):
@@ -275,4 +308,19 @@ class TestNtp(FilesystemMockingTestCase):
self.assertFalse(cc.distro.install_packages.called)
self.assertFalse(mock_util.subp.called)
+ @mock.patch("cloudinit.config.cc_ntp.util")
+ def test_reload_ntp_systemd(self, mock_util):
+ cc_ntp.reload_ntp(systemd=True)
+ self.assertTrue(mock_util.subp.called)
+ mock_util.subp.assert_called_with(
+ ['systemctl', 'reload-or-restart', 'ntp'], capture=True)
+
+ @mock.patch("cloudinit.config.cc_ntp.util")
+ def test_reload_ntp_service(self, mock_util):
+ cc_ntp.reload_ntp(systemd=False)
+ self.assertTrue(mock_util.subp.called)
+ mock_util.subp.assert_called_with(
+ ['service', 'ntp', 'restart'], capture=True)
+
+
# vi: ts=4 expandtab
diff --git a/tox.ini b/tox.ini
index e79ea6a..db63275 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,5 +1,5 @@
[tox]
-envlist = py27, py3, flake8, xenial
+envlist = py27, py3, flake8, xenial, citest
recreate = True
[testenv]
@@ -64,6 +64,39 @@ deps =
flake8==2.5.4
hacking==0.10.2
+[testenv:citest]
+basepython = python3
+commands = {envpython} -m tests.cloud_tests {posargs}
+deps =
+ # requirements
+ jinja2==2.8
+ pyyaml==3.11
+ PrettyTable==0.7.2
+ oauthlib==1.0.3
+ pyserial==3.0.1
+ configobj==5.0.6
+ requests==2.9.1
+ # jsonpatch ubuntu is 1.10, not 1.19 (#839779)
+ jsonpatch==1.10
+ six==1.10.0
+ # test-requirements
+ httpretty==0.8.6
+ mock==1.3.0
+ nose==1.3.7
+ unittest2==1.1.0
+ contextlib2==0.5.1
+ pep8==1.7.0
+ pyflakes==1.1.0
+ flake8==2.5.4
+ hacking==0.10.2
+ pylxd==2.1.3
+ babel==1.3
+ python-dateutil==2.4.2
+ pbr==1.8.0
+ pyOpenSSL==0.15.1
+ requests-unixsocket==0.1.5
+ ws4py==0.3.4
+
[testenv:centos6]
basepython = python2.6
commands = nosetests {posargs:tests}
Follow ups
