Merge ~powersj/cloud-init:fix_ntp_tests into cloud-init:master

Proposed by Joshua Powers
Status: Merged
Approved by: Scott Moser
Approved revision: 21872a1f007934130b3025f544796d810acfa45d
Merged at revision: f0529559f9688a397f59f041dc6362e91faf96b6
Proposed branch: ~powersj/cloud-init:fix_ntp_tests
Merge into: cloud-init:master
Diff against target: 152 lines (+15/-36)
6 files modified
tests/cloud_tests/configs/modules/ntp.yaml (+5/-15)
tests/cloud_tests/configs/modules/ntp_pools.yaml (+2/-6)
tests/cloud_tests/configs/modules/ntp_servers.yaml (+2/-6)
tests/cloud_tests/testcases/modules/ntp.py (+4/-7)
tests/cloud_tests/testcases/modules/ntp_pools.py (+1/-1)
tests/cloud_tests/testcases/modules/ntp_servers.py (+1/-1)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve
Scott Moser Needs Fixing
Review via email: mp+326312@code.launchpad.net

Commit message

tests: update ntp tests after sntp added

A recent change to ntp in artful has added the sntp package
whenever ntp is installed. The tests, rather poorly, did a
`dpkg -l` instead of checking with `which`. This fixes the
ntp tests to all use `which` over expecting a certain number
of lines using dpkg and as a result make the tests OS
independent.

Description of the change

Tested with:
* python3 -m tests.cloud_tests run --os-name artful --preserve-data --data-dir results --verbose -t modules/ntp -t modules/ntp_servers -t modules/ntp_pools --deb cloud-init_0.7.9-194-gecb408af-1~bddeb_all.deb

* python3 -m tests.cloud_tests run --os-name xenial --preserve-data --data-dir results --verbose -t modules/ntp -t modules/ntp_servers -t modules/ntp_pools --deb cloud-init_0.7.9-194-gecb408af-1~bddeb_all.deb

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:bff48b6d1ef2a443dbd1d47f1fd8ab89d64bc1d6
https://jenkins.ubuntu.com/server/job/cloud-init-ci/11/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/11/rebuild

review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

I think that 'command -v' will be exactly equivalent to 'which'.
That is preferable in that in that it is a shell builtin rather than a external program.

Specifically, we've seen that bite us with the centos lxd images which do not have 'which' installed.

Unless you object, lets use 'command -v' instead of 'which'.

review: Needs Fixing
Revision history for this message
Joshua Powers (powersj) wrote :

I think I was using a centos img that I had added which then. Booting a fresh one shows it is in fact missing. I'll update to use -v, thanks!

Revision history for this message
Joshua Powers (powersj) wrote :

Updated check to verify rc of 0, otherwise, fail. Tested on centos 6, xenial, and artful. The output of --version is different and in order to stop relying on output, focus on return code instead.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:21872a1f007934130b3025f544796d810acfa45d
https://jenkins.ubuntu.com/server/job/cloud-init-ci/14/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/14/rebuild

review: Approve (continuous-integration)

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/tests/cloud_tests/configs/modules/ntp.yaml b/tests/cloud_tests/configs/modules/ntp.yaml
2index 0d07ef5..fbef431 100644
3--- a/tests/cloud_tests/configs/modules/ntp.yaml
4+++ b/tests/cloud_tests/configs/modules/ntp.yaml
5@@ -1,31 +1,21 @@
6 #
7 # Emtpy NTP config to setup using defaults
8 #
9-# NOTE: this should not require apt feature, use 'which' rather than 'dpkg -l'
10-# NOTE: this should not require no_ntpdate feature, use 'which' to check for
11-# installation rather than 'dpkg -l', as 'grep ntp' matches 'ntpdate'
12-# NOTE: the verifier should check for any ntp server not 'ubuntu.pool.ntp.org'
13-required_features:
14- - apt
15- - no_ntpdate
16- - ubuntu_ntp
17 cloud_config: |
18 #cloud-config
19 ntp:
20 pools: {}
21 servers: {}
22 collect_scripts:
23- ntp_installed_empty: |
24+ ntp_installed: |
25 #!/bin/bash
26- dpkg -l | grep ntp | wc -l
27+ ntpd --version > /dev/null 2>&1
28+ echo $?
29 ntp_conf_dist_empty: |
30 #!/bin/bash
31 ls /etc/ntp.conf.dist | wc -l
32- ntp_conf_empty: |
33+ ntp_conf_pool_list: |
34 #!/bin/bash
35- grep '^pool' /etc/ntp.conf
36- ntp_installed_list: |
37- #!/bin/bash
38- dpkg -l | grep ntp
39+ grep 'pool.ntp.org' /etc/ntp.conf | grep -v ^#
40
41 # vi: ts=4 expandtab
42diff --git a/tests/cloud_tests/configs/modules/ntp_pools.yaml b/tests/cloud_tests/configs/modules/ntp_pools.yaml
43index 7561c7f..3a93faa 100644
44--- a/tests/cloud_tests/configs/modules/ntp_pools.yaml
45+++ b/tests/cloud_tests/configs/modules/ntp_pools.yaml
46@@ -1,15 +1,10 @@
47 #
48 # NTP config using specific pools
49 #
50-# NOTE: this should not require apt feature, use 'which' rather than 'dpkg -l'
51-# NOTE: this should not require no_ntpdate feature, use 'which' to check for
52-# installation rather than 'dpkg -l', as 'grep ntp' matches 'ntpdate'
53 # NOTE: lsb_release listed here because with recent cloud-init deb with
54 # (LP: 1628337) resolved, cloud-init will attempt to configure archives.
55 # this fails without lsb_release as UNAVAILABLE is used for $RELEASE
56 required_features:
57- - apt
58- - no_ntpdate
59 - lsb_release
60 cloud_config: |
61 #cloud-config
62@@ -21,7 +16,8 @@ cloud_config: |
63 collect_scripts:
64 ntp_installed_pools: |
65 #!/bin/bash
66- dpkg -l | grep ntp | wc -l
67+ ntpd --version > /dev/null 2>&1
68+ echo $?
69 ntp_conf_dist_pools: |
70 #!/bin/bash
71 ls /etc/ntp.conf.dist | wc -l
72diff --git a/tests/cloud_tests/configs/modules/ntp_servers.yaml b/tests/cloud_tests/configs/modules/ntp_servers.yaml
73index 9d1d65e..d59d45a 100644
74--- a/tests/cloud_tests/configs/modules/ntp_servers.yaml
75+++ b/tests/cloud_tests/configs/modules/ntp_servers.yaml
76@@ -1,12 +1,7 @@
77 #
78 # NTP config using specific servers
79 #
80-# NOTE: this should not require apt feature, use 'which' rather than 'dpkg -l'
81-# NOTE: this should not require no_ntpdate feature, use 'which' to check for
82-# installation rather than 'dpkg -l', as 'grep ntp' matches 'ntpdate'
83 required_features:
84- - apt
85- - no_ntpdate
86 - lsb_release
87 cloud_config: |
88 #cloud-config
89@@ -17,7 +12,8 @@ cloud_config: |
90 collect_scripts:
91 ntp_installed_servers: |
92 #!/bin/sh
93- dpkg -l | grep -c ntp
94+ ntpd --version > /dev/null 2>&1
95+ echo $?
96 ntp_conf_dist_servers: |
97 #!/bin/sh
98 cat /etc/ntp.conf.dist | wc -l
99diff --git a/tests/cloud_tests/testcases/modules/ntp.py b/tests/cloud_tests/testcases/modules/ntp.py
100index a4b8c3d..b50e52f 100644
101--- a/tests/cloud_tests/testcases/modules/ntp.py
102+++ b/tests/cloud_tests/testcases/modules/ntp.py
103@@ -9,8 +9,8 @@ class TestNtp(base.CloudTestCase):
104
105 def test_ntp_installed(self):
106 """Test ntp installed"""
107- out = self.get_data_file('ntp_installed_empty')
108- self.assertEqual(1, int(out))
109+ out = self.get_data_file('ntp_installed')
110+ self.assertEqual(0, int(out))
111
112 def test_ntp_dist_entries(self):
113 """Test dist config file is empty"""
114@@ -19,10 +19,7 @@ class TestNtp(base.CloudTestCase):
115
116 def test_ntp_entires(self):
117 """Test config entries"""
118- out = self.get_data_file('ntp_conf_empty')
119- self.assertIn('pool 0.ubuntu.pool.ntp.org iburst', out)
120- self.assertIn('pool 1.ubuntu.pool.ntp.org iburst', out)
121- self.assertIn('pool 2.ubuntu.pool.ntp.org iburst', out)
122- self.assertIn('pool 3.ubuntu.pool.ntp.org iburst', out)
123+ out = self.get_data_file('ntp_conf_pool_list')
124+ self.assertIn('pool.ntp.org iburst', out)
125
126 # vi: ts=4 expandtab
127diff --git a/tests/cloud_tests/testcases/modules/ntp_pools.py b/tests/cloud_tests/testcases/modules/ntp_pools.py
128index 336076d..152fd3f 100644
129--- a/tests/cloud_tests/testcases/modules/ntp_pools.py
130+++ b/tests/cloud_tests/testcases/modules/ntp_pools.py
131@@ -10,7 +10,7 @@ class TestNtpPools(base.CloudTestCase):
132 def test_ntp_installed(self):
133 """Test ntp installed"""
134 out = self.get_data_file('ntp_installed_pools')
135- self.assertEqual(1, int(out))
136+ self.assertEqual(0, int(out))
137
138 def test_ntp_dist_entries(self):
139 """Test dist config file is empty"""
140diff --git a/tests/cloud_tests/testcases/modules/ntp_servers.py b/tests/cloud_tests/testcases/modules/ntp_servers.py
141index 4010cf8..8d2a68b 100644
142--- a/tests/cloud_tests/testcases/modules/ntp_servers.py
143+++ b/tests/cloud_tests/testcases/modules/ntp_servers.py
144@@ -10,7 +10,7 @@ class TestNtpServers(base.CloudTestCase):
145 def test_ntp_installed(self):
146 """Test ntp installed"""
147 out = self.get_data_file('ntp_installed_servers')
148- self.assertEqual(1, int(out))
149+ self.assertEqual(0, int(out))
150
151 def test_ntp_dist_entries(self):
152 """Test dist config file is empty"""

Subscribers

People subscribed via source and target branches