Merge lp:~james-page/charm-helpers/systemd-support into lp:charm-helpers
- systemd-support
- Merge into devel
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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Liam Young (community) | Approve | ||
Adam Collard (community) | Needs Fixing | ||
Review via email: mp+281742@code.launchpad.net |
Commit message
Description of the change
To post a comment you must log in.
- 516. By James Page
-
Drop else for System ValueError raise.
- 517. By James Page
-
Tweak
- 518. By James Page
-
Feedback from review
- 519. By James Page
-
optimize is systemd
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')) |
First pass, several cleanups needed