Merge lp:~adam-collard/charm-helpers/fix-amulet-utils-validate-services-by-name into lp:charm-helpers

Proposed by Adam Collard
Status: Merged
Merged at revision: 427
Proposed branch: lp:~adam-collard/charm-helpers/fix-amulet-utils-validate-services-by-name
Merge into: lp:charm-helpers
Diff against target: 190 lines (+125/-10)
3 files modified
charmhelpers/contrib/amulet/utils.py (+18/-10)
test_requirements.txt (+2/-0)
tests/contrib/amulet/test_utils.py (+105/-0)
To merge this branch: bzr merge lp:~adam-collard/charm-helpers/fix-amulet-utils-validate-services-by-name
Reviewer Review Type Date Requested Status
Chris Glass (community) Approve
Review via email: mp+267817@code.launchpad.net

Description of the change

Make the validate_services_by_name check actually verify that Upstart jobs are running and not just "existing".

Drive-by fixes for Python3 compatibility and dependencies for running tests (i.e. amulet, distro-info)

To post a comment you must log in.
428. By Adam Collard

Add test files

Revision history for this message
Chris Glass (tribaal) wrote :

+1, thanks for your fix.

(Unit and amulet tests pass)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'charmhelpers/contrib/amulet/utils.py'
2--- charmhelpers/contrib/amulet/utils.py 2015-08-10 12:58:05 +0000
3+++ charmhelpers/contrib/amulet/utils.py 2015-08-12 14:08:32 +0000
4@@ -14,17 +14,21 @@
5 # You should have received a copy of the GNU Lesser General Public License
6 # along with charm-helpers. If not, see <http://www.gnu.org/licenses/>.
7
8-import amulet
9-import ConfigParser
10-import distro_info
11 import io
12 import logging
13 import os
14 import re
15-import six
16 import sys
17 import time
18-import urlparse
19+
20+import amulet
21+import distro_info
22+import six
23+from six.moves import configparser
24+if six.PY3:
25+ from urllib import parse as urlparse
26+else:
27+ import urlparse
28
29
30 class AmuletUtils(object):
31@@ -143,18 +147,22 @@
32 for service_name in services_list:
33 if (self.ubuntu_releases.index(release) >= systemd_switch or
34 service_name in ['rabbitmq-server', 'apache2']):
35- # init is systemd
36+ # init is systemd (or regular sysv)
37 cmd = 'sudo service {} status'.format(service_name)
38+ output, code = sentry_unit.run(cmd)
39+ service_running = code == 0
40 elif self.ubuntu_releases.index(release) < systemd_switch:
41 # init is upstart
42 cmd = 'sudo status {}'.format(service_name)
43+ output, code = sentry_unit.run(cmd)
44+ service_running = code == 0 and "start/running" in output
45
46- output, code = sentry_unit.run(cmd)
47 self.log.debug('{} `{}` returned '
48 '{}'.format(sentry_unit.info['unit_name'],
49 cmd, code))
50- if code != 0:
51- return "command `{}` returned {}".format(cmd, str(code))
52+ if not service_running:
53+ return u"command `{}` returned {} {}".format(
54+ cmd, output, str(code))
55 return None
56
57 def _get_config(self, unit, filename):
58@@ -164,7 +172,7 @@
59 # NOTE(beisner): by default, ConfigParser does not handle options
60 # with no value, such as the flags used in the mysql my.cnf file.
61 # https://bugs.python.org/issue7005
62- config = ConfigParser.ConfigParser(allow_no_value=True)
63+ config = configparser.ConfigParser(allow_no_value=True)
64 config.readfp(io.StringIO(file_contents))
65 return config
66
67
68=== modified file 'test_requirements.txt'
69--- test_requirements.txt 2015-07-15 17:01:57 +0000
70+++ test_requirements.txt 2015-08-12 14:08:32 +0000
71@@ -7,6 +7,8 @@
72 nose>=1.3.1
73 flake8
74 testtools==0.9.14 # Before dependent on modern 'six'
75+amulet
76+distro-info
77 #
78 # Specify precise versions of runtime dependencies where possible.
79 netaddr==0.7.10 # trusty. precise is 0.7.5, but not in pypi.
80
81=== added directory 'tests/contrib/amulet'
82=== added file 'tests/contrib/amulet/test_utils.py'
83--- tests/contrib/amulet/test_utils.py 1970-01-01 00:00:00 +0000
84+++ tests/contrib/amulet/test_utils.py 2015-08-12 14:08:32 +0000
85@@ -0,0 +1,105 @@
86+# Copyright 2015 Canonical Ltd.
87+#
88+# Authors:
89+# Adam Collard <adam.collard@canonical.com>
90+
91+import unittest
92+
93+from charmhelpers.contrib.amulet.utils import AmuletUtils
94+
95+
96+class FakeSentry(object):
97+
98+ commands = {}
99+
100+ info = {"unit_name": "foo"}
101+
102+ def run(self, command):
103+ return self.commands[command]
104+
105+
106+class ValidateServicesByNameTestCase(unittest.TestCase):
107+
108+ def setUp(self):
109+ self.utils = AmuletUtils()
110+ self.sentry_unit = FakeSentry()
111+
112+ def test_errors_for_unknown_upstart_service(self):
113+ """
114+ Returns a message if the Upstart service is unknown.
115+ """
116+ self.sentry_unit.commands["lsb_release -cs"] = "trusty", 0
117+ self.sentry_unit.commands["sudo status foo"] = (
118+ "status: Unknown job: foo", 1)
119+
120+ result = self.utils.validate_services_by_name(
121+ {self.sentry_unit: ["foo"]})
122+ self.assertIsNotNone(result)
123+
124+ def test_none_for_started_upstart_service(self):
125+ """
126+ Returns None if the Upstart service is running.
127+ """
128+ self.sentry_unit.commands["lsb_release -cs"] = "trusty", 0
129+ self.sentry_unit.commands["sudo status foo"] = (
130+ "foo start/running, process 42", 0)
131+
132+ result = self.utils.validate_services_by_name(
133+ {self.sentry_unit: ["foo"]})
134+ self.assertIsNone(result)
135+
136+ def test_errors_for_stopped_upstart_service(self):
137+ """
138+ Returns a message if the Upstart service is stopped.
139+ """
140+ self.sentry_unit.commands["lsb_release -cs"] = "trusty", 0
141+ self.sentry_unit.commands["sudo status foo"] = "foo stop/waiting", 0
142+
143+ result = self.utils.validate_services_by_name(
144+ {self.sentry_unit: ["foo"]})
145+ self.assertIsNotNone(result)
146+
147+ def test_errors_for_unknown_systemd_service(self):
148+ """
149+ Returns a message if a systemd service is unknown.
150+ """
151+ self.sentry_unit.commands["lsb_release -cs"] = "vivid", 0
152+ self.sentry_unit.commands["sudo service foo status"] = (u"""\
153+\u25cf foo.service
154+ Loaded: not-found (Reason: No such file or directory)
155+ Active: inactive (dead)
156+""", 3)
157+
158+ result = self.utils.validate_services_by_name({
159+ self.sentry_unit: ["foo"]})
160+ self.assertIsNotNone(result)
161+
162+ def test_none_for_started_systemd_service(self):
163+ """
164+ Returns None if a systemd service is running.
165+ """
166+ self.sentry_unit.commands["lsb_release -cs"] = "vivid", 0
167+ self.sentry_unit.commands["sudo service foo status"] = (u"""\
168+\u25cf foo.service - Foo
169+ Loaded: loaded (/lib/systemd/system/foo.service; enabled)
170+ Active: active (exited) since Thu 1970-01-01 00:00:00 UTC; 42h 42min ago
171+ Main PID: 3 (code=exited, status=0/SUCCESS)
172+ CGroup: /system.slice/foo.service
173+""", 0)
174+ result = self.utils.validate_services_by_name(
175+ {self.sentry_unit: ["foo"]})
176+ self.assertIsNone(result)
177+
178+ def test_errors_for_stopped_systemd_service(self):
179+ """
180+ Returns a message if a systemd service is stopped.
181+ """
182+ self.sentry_unit.commands["lsb_release -cs"] = "vivid", 0
183+ self.sentry_unit.commands["sudo service foo status"] = (u"""\
184+\u25cf foo.service - Foo
185+ Loaded: loaded (/lib/systemd/system/foo.service; disabled)
186+ Active: inactive (dead)
187+""", 3)
188+ result = self.utils.validate_services_by_name(
189+ {self.sentry_unit: ["foo"]})
190+ self.assertIsNotNone(result)

Subscribers

People subscribed via source and target branches