Merge lp:~james-page/charm-helpers/systemd-support into lp:charm-helpers

Proposed by James Page
Status: Merged
Merged at revision: 513
Proposed branch: lp:~james-page/charm-helpers/systemd-support
Merge into: lp:charm-helpers
Diff against target: 414 lines (+140/-36)
3 files modified
charmhelpers/core/host.py (+34/-17)
tests/contrib/charmsupport/test_nrpe.py (+3/-3)
tests/core/test_host.py (+103/-16)
To merge this branch: bzr merge lp:~james-page/charm-helpers/systemd-support
Reviewer Review Type Date Requested Status
Liam Young Approve
Adam Collard (community) Needs Fixing
Review via email: mp+281742@code.launchpad.net
To post a comment you must log in.
516. By James Page

Drop else for System ValueError raise.

517. By James Page

Tweak

Revision history for this message
Adam Collard (adam-collard) wrote :

First pass, several cleanups needed

review: Needs Fixing
518. By James Page

Feedback from review

519. By James Page

optimize is systemd

Revision history for this message
Liam Young (gnuoy) wrote :

approve

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'charmhelpers/core/host.py'
2--- charmhelpers/core/host.py 2015-11-27 20:57:05 +0000
3+++ charmhelpers/core/host.py 2016-01-06 14:04:36 +0000
4@@ -72,7 +72,9 @@
5 stopped = service_stop(service_name)
6 upstart_file = os.path.join(init_dir, "{}.conf".format(service_name))
7 sysv_file = os.path.join(initd_dir, service_name)
8- if os.path.exists(upstart_file):
9+ if init_is_systemd():
10+ service('disable', service_name)
11+ elif os.path.exists(upstart_file):
12 override_path = os.path.join(
13 init_dir, '{}.override'.format(service_name))
14 with open(override_path, 'w') as fh:
15@@ -80,9 +82,9 @@
16 elif os.path.exists(sysv_file):
17 subprocess.check_call(["update-rc.d", service_name, "disable"])
18 else:
19- # XXX: Support SystemD too
20 raise ValueError(
21- "Unable to detect {0} as either Upstart {1} or SysV {2}".format(
22+ "Unable to detect {0} as SystemD, Upstart {1} or"
23+ " SysV {2}".format(
24 service_name, upstart_file, sysv_file))
25 return stopped
26
27@@ -94,7 +96,9 @@
28 Reenable starting again at boot. Start the service"""
29 upstart_file = os.path.join(init_dir, "{}.conf".format(service_name))
30 sysv_file = os.path.join(initd_dir, service_name)
31- if os.path.exists(upstart_file):
32+ if init_is_systemd():
33+ service('enable', service_name)
34+ elif os.path.exists(upstart_file):
35 override_path = os.path.join(
36 init_dir, '{}.override'.format(service_name))
37 if os.path.exists(override_path):
38@@ -102,9 +106,9 @@
39 elif os.path.exists(sysv_file):
40 subprocess.check_call(["update-rc.d", service_name, "enable"])
41 else:
42- # XXX: Support SystemD too
43 raise ValueError(
44- "Unable to detect {0} as either Upstart {1} or SysV {2}".format(
45+ "Unable to detect {0} as SystemD, Upstart {1} or"
46+ " SysV {2}".format(
47 service_name, upstart_file, sysv_file))
48
49 started = service_running(service_name)
50@@ -115,23 +119,29 @@
51
52 def service(action, service_name):
53 """Control a system service"""
54- cmd = ['service', service_name, action]
55+ if init_is_systemd():
56+ cmd = ['systemctl', action, service_name]
57+ else:
58+ cmd = ['service', service_name, action]
59 return subprocess.call(cmd) == 0
60
61
62-def service_running(service):
63+def service_running(service_name):
64 """Determine whether a system service is running"""
65- try:
66- output = subprocess.check_output(
67- ['service', service, 'status'],
68- stderr=subprocess.STDOUT).decode('UTF-8')
69- except subprocess.CalledProcessError:
70- return False
71+ if init_is_systemd():
72+ return service('is-active', service_name)
73 else:
74- if ("start/running" in output or "is running" in output):
75- return True
76- else:
77+ try:
78+ output = subprocess.check_output(
79+ ['service', service_name, 'status'],
80+ stderr=subprocess.STDOUT).decode('UTF-8')
81+ except subprocess.CalledProcessError:
82 return False
83+ else:
84+ if ("start/running" in output or "is running" in output):
85+ return True
86+ else:
87+ return False
88
89
90 def service_available(service_name):
91@@ -146,6 +156,13 @@
92 return True
93
94
95+SYSTEMD_SYSTEM = '/run/systemd/system'
96+
97+
98+def init_is_systemd():
99+ return os.path.isdir(SYSTEMD_SYSTEM)
100+
101+
102 def adduser(username, password=None, shell='/bin/bash', system_user=False,
103 primary_group=None, secondary_groups=None):
104 """
105
106=== modified file 'tests/contrib/charmsupport/test_nrpe.py'
107--- tests/contrib/charmsupport/test_nrpe.py 2015-10-29 23:52:26 +0000
108+++ tests/contrib/charmsupport/test_nrpe.py 2016-01-06 14:04:36 +0000
109@@ -25,6 +25,7 @@
110 'relation_ids': {'object': nrpe},
111 'relation_set': {'object': nrpe},
112 'relations_of_type': {'object': nrpe},
113+ 'service': {'object': nrpe},
114 }
115
116 def setUp(self):
117@@ -108,10 +109,9 @@
118
119 self.assertEqual(None, checker.write())
120
121- expected = ['service', 'nagios-nrpe-server', 'restart']
122- self.assertEqual(expected, self.patched['call'].call_args[0][0])
123+ self.patched['service'].assert_called_with('restart', 'nagios-nrpe-server')
124 self.check_call_counts(config=1, getpwnam=1, getgrnam=1,
125- exists=1, call=1)
126+ exists=1, service=1)
127
128 def test_update_nrpe(self):
129 self.patched['config'].return_value = {'nagios_context': 'a',
130
131=== modified file 'tests/core/test_host.py'
132--- tests/core/test_host.py 2015-11-27 20:57:05 +0000
133+++ tests/core/test_host.py 2016-01-06 14:04:36 +0000
134@@ -62,8 +62,25 @@
135
136
137 class HelpersTest(TestCase):
138+
139+ @patch('os.path')
140+ def test_init_is_systemd_upstart(self, path):
141+ """Upstart based init is correctly detected"""
142+ path.isdir.return_value = False
143+ self.assertFalse(host.init_is_systemd())
144+ path.isdir.assert_called_with('/run/systemd/system')
145+
146+ @patch('os.path')
147+ def test_init_is_systemd_system(self, path):
148+ """Systemd based init is correctly detected"""
149+ path.isdir.return_value = True
150+ self.assertTrue(host.init_is_systemd())
151+ path.isdir.assert_called_with('/run/systemd/system')
152+
153+ @patch.object(host, 'init_is_systemd')
154 @patch('subprocess.call')
155- def test_runs_service_action(self, mock_call):
156+ def test_runs_service_action(self, mock_call, systemd):
157+ systemd.return_value = False
158 mock_call.return_value = 0
159 action = 'some-action'
160 service_name = 'foo-service'
161@@ -73,8 +90,24 @@
162 self.assertTrue(result)
163 mock_call.assert_called_with(['service', service_name, action])
164
165- @patch('subprocess.call')
166- def test_returns_false_when_service_fails(self, mock_call):
167+ @patch.object(host, 'init_is_systemd')
168+ @patch('subprocess.call')
169+ def test_runs_systemctl_action(self, mock_call, systemd):
170+ """Ensure that service calls under systemd call 'systemctl'."""
171+ systemd.return_value = True
172+ mock_call.return_value = 0
173+ action = 'some-action'
174+ service_name = 'foo-service'
175+
176+ result = host.service(action, service_name)
177+
178+ self.assertTrue(result)
179+ mock_call.assert_called_with(['systemctl', action, service_name])
180+
181+ @patch.object(host, 'init_is_systemd')
182+ @patch('subprocess.call')
183+ def test_returns_false_when_service_fails(self, mock_call, systemd):
184+ systemd.return_value = False
185 mock_call.return_value = 1
186 action = 'some-action'
187 service_name = 'foo-service'
188@@ -108,13 +141,43 @@
189
190 service.assert_called_with('restart', service_name)
191
192+ @patch.object(host, 'service_running')
193+ @patch.object(host, 'init_is_systemd')
194+ @patch.object(host, 'service')
195+ def test_pauses_a_running_systemd_unit(self, service, systemd,
196+ service_running):
197+ """Pause on a running systemd unit will be stopped and disabled."""
198+ service_name = 'foo-service'
199+ service_running.return_value = True
200+ systemd.return_value = True
201+ self.assertTrue(host.service_pause(service_name))
202+ service.assert_has_calls([
203+ call('stop', service_name),
204+ call('disable', service_name)])
205+
206+ @patch.object(host, 'service_running')
207+ @patch.object(host, 'init_is_systemd')
208+ @patch.object(host, 'service')
209+ def test_resumes_a_stopped_systemd_unit(self, service, systemd,
210+ service_running):
211+ """Resume on a stopped systemd unit will be started and enabled."""
212+ service_name = 'foo-service'
213+ service_running.return_value = False
214+ systemd.return_value = True
215+ self.assertTrue(host.service_resume(service_name))
216+ service.assert_has_calls([
217+ call('enable', service_name),
218+ call('start', service_name)])
219+
220+ @patch.object(host, 'init_is_systemd')
221 @patch('subprocess.check_output')
222 @patch.object(host, 'service')
223- def test_pauses_a_running_upstart_service(self, service, check_output):
224+ def test_pauses_a_running_upstart_service(self, service, check_output, systemd):
225 """Pause on a running service will call service stop."""
226 check_output.return_value = b'foo-service start/running, process 123'
227 service_name = 'foo-service'
228 service.side_effect = [True]
229+ systemd.return_value = False
230 tempdir = mkdtemp(prefix="test_pauses_an_upstart_service")
231 conf_path = os.path.join(tempdir, "{}.conf".format(service_name))
232 # Just needs to exist
233@@ -130,13 +193,15 @@
234 override_contents = fh.read()
235 self.assertEqual("manual\n", override_contents)
236
237+ @patch.object(host, 'init_is_systemd')
238 @patch('subprocess.check_output')
239 @patch.object(host, 'service')
240- def test_pauses_a_stopped_upstart_service(self, service, check_output):
241+ def test_pauses_a_stopped_upstart_service(self, service, check_output, systemd):
242 """Pause on a stopped service will not call service stop."""
243 check_output.return_value = b'foo-service stop/waiting'
244 service_name = 'foo-service'
245 service.side_effect = [True]
246+ systemd.return_value = False
247 tempdir = mkdtemp(prefix="test_pauses_an_upstart_service")
248 conf_path = os.path.join(tempdir, "{}.conf".format(service_name))
249 # Just needs to exist
250@@ -154,15 +219,17 @@
251 override_contents = fh.read()
252 self.assertEqual("manual\n", override_contents)
253
254+ @patch.object(host, 'init_is_systemd')
255 @patch('subprocess.check_output')
256 @patch('subprocess.check_call')
257 @patch.object(host, 'service')
258 def test_pauses_a_running_sysv_service(self, service, check_call,
259- check_output):
260+ check_output, systemd):
261 """Pause calls service stop on a running sysv service."""
262 check_output.return_value = b'foo-service start/running, process 123'
263 service_name = 'foo-service'
264 service.side_effect = [True]
265+ systemd.return_value = False
266 tempdir = mkdtemp(prefix="test_pauses_a_sysv_service")
267 sysv_path = os.path.join(tempdir, service_name)
268 # Just needs to exist
269@@ -175,15 +242,17 @@
270 service.assert_called_with('stop', service_name)
271 check_call.assert_called_with(["update-rc.d", service_name, "disable"])
272
273+ @patch.object(host, 'init_is_systemd')
274 @patch('subprocess.check_output')
275 @patch('subprocess.check_call')
276 @patch.object(host, 'service')
277 def test_pauses_a_stopped_sysv_service(self, service, check_call,
278- check_output):
279+ check_output, systemd):
280 """Pause does not call service stop on a stopped sysv service."""
281 check_output.return_value = b'foo-service stop/waiting'
282 service_name = 'foo-service'
283 service.side_effect = [True]
284+ systemd.return_value = False
285 tempdir = mkdtemp(prefix="test_pauses_a_sysv_service")
286 sysv_path = os.path.join(tempdir, service_name)
287 # Just needs to exist
288@@ -198,10 +267,12 @@
289 AssertionError, service.assert_called_with, 'stop', service_name)
290 check_call.assert_called_with(["update-rc.d", service_name, "disable"])
291
292+ @patch.object(host, 'init_is_systemd')
293 @patch.object(host, 'service')
294- def test_pause_with_unknown_service(self, service):
295+ def test_pause_with_unknown_service(self, service, systemd):
296 service_name = 'foo-service'
297 service.side_effect = [True]
298+ systemd.return_value = False
299 tempdir = mkdtemp(prefix="test_pauses_with_unknown_service")
300 self.addCleanup(rmtree, tempdir)
301 exception = self.assertRaises(
302@@ -211,13 +282,15 @@
303 "Unable to detect {0}".format(service_name), str(exception))
304 self.assertIn(tempdir, str(exception))
305
306+ @patch.object(host, 'init_is_systemd')
307 @patch('subprocess.check_output')
308 @patch.object(host, 'service')
309- def test_resumes_a_running_upstart_service(self, service, check_output):
310+ def test_resumes_a_running_upstart_service(self, service, check_output, systemd):
311 """When the service is already running, service start isn't called."""
312 check_output.return_value = b'foo-service start/running, process 111'
313 service_name = 'foo-service'
314 service.side_effect = [True]
315+ systemd.return_value = False
316 tempdir = mkdtemp(prefix="test_resumes_an_upstart_service")
317 conf_path = os.path.join(tempdir, "{}.conf".format(service_name))
318 with open(conf_path, "w") as fh:
319@@ -232,13 +305,15 @@
320 tempdir, "{}.override".format(service_name))
321 self.assertFalse(os.path.exists(override_path))
322
323+ @patch.object(host, 'init_is_systemd')
324 @patch('subprocess.check_output')
325 @patch.object(host, 'service')
326- def test_resumes_a_stopped_upstart_service(self, service, check_output):
327+ def test_resumes_a_stopped_upstart_service(self, service, check_output, systemd):
328 """When the service is stopped, service start is called."""
329 check_output.return_value = b'foo-service stop/waiting'
330 service_name = 'foo-service'
331 service.side_effect = [True]
332+ systemd.return_value = False
333 tempdir = mkdtemp(prefix="test_resumes_an_upstart_service")
334 conf_path = os.path.join(tempdir, "{}.conf".format(service_name))
335 with open(conf_path, "w") as fh:
336@@ -251,14 +326,16 @@
337 tempdir, "{}.override".format(service_name))
338 self.assertFalse(os.path.exists(override_path))
339
340+ @patch.object(host, 'init_is_systemd')
341 @patch('subprocess.check_output')
342 @patch('subprocess.check_call')
343 @patch.object(host, 'service')
344- def test_resumes_a_sysv_service(self, service, check_call, check_output):
345+ def test_resumes_a_sysv_service(self, service, check_call, check_output, systemd):
346 """When process is in a stop/waiting state, service start is called."""
347 check_output.return_value = b'foo-service stop/waiting'
348 service_name = 'foo-service'
349 service.side_effect = [True]
350+ systemd.return_value = False
351 tempdir = mkdtemp(prefix="test_resumes_a_sysv_service")
352 sysv_path = os.path.join(tempdir, service_name)
353 # Just needs to exist
354@@ -271,15 +348,17 @@
355 service.assert_called_with('start', service_name)
356 check_call.assert_called_with(["update-rc.d", service_name, "enable"])
357
358+ @patch.object(host, 'init_is_systemd')
359 @patch('subprocess.check_output')
360 @patch('subprocess.check_call')
361 @patch.object(host, 'service')
362 def test_resume_a_running_sysv_service(self, service, check_call,
363- check_output):
364+ check_output, systemd):
365 """When process is already running, service start isn't called."""
366 check_output.return_value = b'foo-service start/running, process 123'
367 service_name = 'foo-service'
368 service.side_effect = [True]
369+ systemd.return_value = False
370 tempdir = mkdtemp(prefix="test_resumes_a_sysv_service")
371 sysv_path = os.path.join(tempdir, service_name)
372 # Just needs to exist
373@@ -294,10 +373,12 @@
374 AssertionError, service.assert_called_with, 'start', service_name)
375 check_call.assert_called_with(["update-rc.d", service_name, "enable"])
376
377+ @patch.object(host, 'init_is_systemd')
378 @patch.object(host, 'service')
379- def test_resume_with_unknown_service(self, service):
380+ def test_resume_with_unknown_service(self, service, systemd):
381 service_name = 'foo-service'
382 service.side_effect = [True]
383+ systemd.return_value = False
384 tempdir = mkdtemp(prefix="test_resumes_with_unknown_service")
385 self.addCleanup(rmtree, tempdir)
386 exception = self.assertRaises(
387@@ -379,18 +460,24 @@
388 call('restart', service_name)
389 ])
390
391+ @patch.object(host, 'init_is_systemd')
392 @patch('subprocess.check_output')
393- def test_service_running_on_stopped_service(self, check_output):
394+ def test_service_running_on_stopped_service(self, check_output, systemd):
395+ systemd.return_value = False
396 check_output.return_value = b'foo stop/waiting'
397 self.assertFalse(host.service_running('foo'))
398
399+ @patch.object(host, 'init_is_systemd')
400 @patch('subprocess.check_output')
401- def test_service_running_on_running_service(self, check_output):
402+ def test_service_running_on_running_service(self, check_output, systemd):
403+ systemd.return_value = False
404 check_output.return_value = b'foo start/running, process 23871'
405 self.assertTrue(host.service_running('foo'))
406
407+ @patch.object(host, 'init_is_systemd')
408 @patch('subprocess.check_output')
409- def test_service_running_on_unknown_service(self, check_output):
410+ def test_service_running_on_unknown_service(self, check_output, systemd):
411+ systemd.return_value = False
412 exc = subprocess.CalledProcessError(1, ['status'])
413 check_output.side_effect = exc
414 self.assertFalse(host.service_running('foo'))

Subscribers

People subscribed via source and target branches