Merge lp:~adam-collard/charms/trusty/swift-storage/fix-service-status into lp:~openstack-charmers-archive/charms/trusty/swift-storage/next

Proposed by Adam Collard on 2015-08-17
Status: Merged
Merged at revision: 73
Proposed branch: lp:~adam-collard/charms/trusty/swift-storage/fix-service-status
Merge into: lp:~openstack-charmers-archive/charms/trusty/swift-storage/next
Diff against target: 543 lines (+178/-93)
11 files modified
charmhelpers/cli/__init__.py (+1/-5)
charmhelpers/cli/commands.py (+4/-4)
charmhelpers/contrib/openstack/amulet/deployment.py (+2/-2)
charmhelpers/contrib/openstack/utils.py (+51/-14)
charmhelpers/contrib/storage/linux/utils.py (+3/-2)
charmhelpers/core/hookenv.py (+1/-20)
charmhelpers/core/host.py (+2/-2)
charmhelpers/fetch/__init__.py (+8/-0)
tests/basic_deployment.py (+20/-21)
tests/charmhelpers/contrib/amulet/utils.py (+84/-21)
tests/charmhelpers/contrib/openstack/amulet/deployment.py (+2/-2)
To merge this branch: bzr merge lp:~adam-collard/charms/trusty/swift-storage/fix-service-status
Reviewer Review Type Date Requested Status
Liam Young 2015-08-17 Approve on 2015-08-18
Review via email: mp+268228@code.launchpad.net

Description of the change

Sync charm-helpers and use the (new-ish) validate_services_by_name() utility to properly check that all swift services are starting.

To post a comment you must log in.
75. By Adam Collard on 2015-08-17

Fix typo

charm_lint_check #8194 swift-storage-next for adam-collard mp268228
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/8194/

charm_unit_test #7596 swift-storage-next for adam-collard mp268228
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/7596/

charm_lint_check #8195 swift-storage-next for adam-collard mp268228
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/8195/

charm_unit_test #7597 swift-storage-next for adam-collard mp268228
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/7597/

charm_amulet_test #5840 swift-storage-next for adam-collard mp268228
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [test] Error 1
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/12108178/
Build: http://10.245.162.77:8080/job/charm_amulet_test/5840/

charm_amulet_test #5841 swift-storage-next for adam-collard mp268228
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/5841/

Liam Young (gnuoy) wrote :

Approved

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'charmhelpers/cli/__init__.py'
2--- charmhelpers/cli/__init__.py 2015-07-31 13:11:32 +0000
3+++ charmhelpers/cli/__init__.py 2015-08-17 13:36:14 +0000
4@@ -152,15 +152,11 @@
5 arguments = self.argument_parser.parse_args()
6 argspec = inspect.getargspec(arguments.func)
7 vargs = []
8- kwargs = {}
9 for arg in argspec.args:
10 vargs.append(getattr(arguments, arg))
11 if argspec.varargs:
12 vargs.extend(getattr(arguments, argspec.varargs))
13- if argspec.keywords:
14- for kwarg in argspec.keywords.items():
15- kwargs[kwarg] = getattr(arguments, kwarg)
16- output = arguments.func(*vargs, **kwargs)
17+ output = arguments.func(*vargs)
18 if getattr(arguments.func, '_cli_test_command', False):
19 self.exit_code = 0 if output else 1
20 output = ''
21
22=== modified file 'charmhelpers/cli/commands.py'
23--- charmhelpers/cli/commands.py 2015-07-31 13:11:32 +0000
24+++ charmhelpers/cli/commands.py 2015-08-17 13:36:14 +0000
25@@ -26,7 +26,7 @@
26 """
27 Import the sub-modules which have decorated subcommands to register with chlp.
28 """
29-import host # noqa
30-import benchmark # noqa
31-import unitdata # noqa
32-from charmhelpers.core import hookenv # noqa
33+from . import host # noqa
34+from . import benchmark # noqa
35+from . import unitdata # noqa
36+from . import hookenv # noqa
37
38=== modified file 'charmhelpers/contrib/openstack/amulet/deployment.py'
39--- charmhelpers/contrib/openstack/amulet/deployment.py 2015-07-29 10:49:43 +0000
40+++ charmhelpers/contrib/openstack/amulet/deployment.py 2015-08-17 13:36:14 +0000
41@@ -44,7 +44,7 @@
42 Determine if the local branch being tested is derived from its
43 stable or next (dev) branch, and based on this, use the corresonding
44 stable or next branches for the other_services."""
45- base_charms = ['mysql', 'mongodb']
46+ base_charms = ['mysql', 'mongodb', 'nrpe']
47
48 if self.series in ['precise', 'trusty']:
49 base_series = self.series
50@@ -81,7 +81,7 @@
51 'ceph-osd', 'ceph-radosgw']
52 # Most OpenStack subordinate charms do not expose an origin option
53 # as that is controlled by the principle.
54- ignore = ['cinder-ceph', 'hacluster', 'neutron-openvswitch']
55+ ignore = ['cinder-ceph', 'hacluster', 'neutron-openvswitch', 'nrpe']
56
57 if self.openstack:
58 for svc in services:
59
60=== modified file 'charmhelpers/contrib/openstack/utils.py'
61--- charmhelpers/contrib/openstack/utils.py 2015-07-29 10:49:43 +0000
62+++ charmhelpers/contrib/openstack/utils.py 2015-08-17 13:36:14 +0000
63@@ -24,6 +24,7 @@
64 import json
65 import os
66 import sys
67+import re
68
69 import six
70 import yaml
71@@ -69,7 +70,6 @@
72 DISTRO_PROPOSED = ('deb http://archive.ubuntu.com/ubuntu/ %s-proposed '
73 'restricted main multiverse universe')
74
75-
76 UBUNTU_OPENSTACK_RELEASE = OrderedDict([
77 ('oneiric', 'diablo'),
78 ('precise', 'essex'),
79@@ -118,6 +118,34 @@
80 ('2.3.0', 'liberty'),
81 ])
82
83+# >= Liberty version->codename mapping
84+PACKAGE_CODENAMES = {
85+ 'nova-common': OrderedDict([
86+ ('12.0.0', 'liberty'),
87+ ]),
88+ 'neutron-common': OrderedDict([
89+ ('7.0.0', 'liberty'),
90+ ]),
91+ 'cinder-common': OrderedDict([
92+ ('7.0.0', 'liberty'),
93+ ]),
94+ 'keystone': OrderedDict([
95+ ('8.0.0', 'liberty'),
96+ ]),
97+ 'horizon-common': OrderedDict([
98+ ('8.0.0', 'liberty'),
99+ ]),
100+ 'ceilometer-common': OrderedDict([
101+ ('5.0.0', 'liberty'),
102+ ]),
103+ 'heat-common': OrderedDict([
104+ ('5.0.0', 'liberty'),
105+ ]),
106+ 'glance-common': OrderedDict([
107+ ('11.0.0', 'liberty'),
108+ ]),
109+}
110+
111 DEFAULT_LOOPBACK_SIZE = '5G'
112
113
114@@ -201,20 +229,29 @@
115 error_out(e)
116
117 vers = apt.upstream_version(pkg.current_ver.ver_str)
118+ match = re.match('^(\d)\.(\d)\.(\d)', vers)
119+ if match:
120+ vers = match.group(0)
121
122- try:
123- if 'swift' in pkg.name:
124- swift_vers = vers[:5]
125- if swift_vers not in SWIFT_CODENAMES:
126- # Deal with 1.10.0 upward
127- swift_vers = vers[:6]
128- return SWIFT_CODENAMES[swift_vers]
129- else:
130- vers = vers[:6]
131- return OPENSTACK_CODENAMES[vers]
132- except KeyError:
133- e = 'Could not determine OpenStack codename for version %s' % vers
134- error_out(e)
135+ # >= Liberty independent project versions
136+ if (package in PACKAGE_CODENAMES and
137+ vers in PACKAGE_CODENAMES[package]):
138+ return PACKAGE_CODENAMES[package][vers]
139+ else:
140+ # < Liberty co-ordinated project versions
141+ try:
142+ if 'swift' in pkg.name:
143+ swift_vers = vers[:5]
144+ if swift_vers not in SWIFT_CODENAMES:
145+ # Deal with 1.10.0 upward
146+ swift_vers = vers[:6]
147+ return SWIFT_CODENAMES[swift_vers]
148+ else:
149+ vers = vers[:6]
150+ return OPENSTACK_CODENAMES[vers]
151+ except KeyError:
152+ e = 'Could not determine OpenStack codename for version %s' % vers
153+ error_out(e)
154
155
156 def get_os_version_package(pkg, fatal=True):
157
158=== modified file 'charmhelpers/contrib/storage/linux/utils.py'
159--- charmhelpers/contrib/storage/linux/utils.py 2015-07-29 10:49:43 +0000
160+++ charmhelpers/contrib/storage/linux/utils.py 2015-08-17 13:36:14 +0000
161@@ -43,9 +43,10 @@
162
163 :param block_device: str: Full path of block device to clean.
164 '''
165+ # https://github.com/ceph/ceph/commit/fdd7f8d83afa25c4e09aaedd90ab93f3b64a677b
166 # sometimes sgdisk exits non-zero; this is OK, dd will clean up
167- call(['sgdisk', '--zap-all', '--mbrtogpt',
168- '--clear', block_device])
169+ call(['sgdisk', '--zap-all', '--', block_device])
170+ call(['sgdisk', '--clear', '--mbrtogpt', '--', block_device])
171 dev_end = check_output(['blockdev', '--getsz',
172 block_device]).decode('UTF-8')
173 gpt_end = int(dev_end.split()[0]) - 100
174
175=== modified file 'charmhelpers/core/hookenv.py'
176--- charmhelpers/core/hookenv.py 2015-08-03 14:00:44 +0000
177+++ charmhelpers/core/hookenv.py 2015-08-17 13:36:14 +0000
178@@ -34,23 +34,6 @@
179 import tempfile
180 from subprocess import CalledProcessError
181
182-try:
183- from charmhelpers.cli import cmdline
184-except ImportError as e:
185- # due to the anti-pattern of partially synching charmhelpers directly
186- # into charms, it's possible that charmhelpers.cli is not available;
187- # if that's the case, they don't really care about using the cli anyway,
188- # so mock it out
189- if str(e) == 'No module named cli':
190- class cmdline(object):
191- @classmethod
192- def subcommand(cls, *args, **kwargs):
193- def _wrap(func):
194- return func
195- return _wrap
196- else:
197- raise
198-
199 import six
200 if not six.PY3:
201 from UserDict import UserDict
202@@ -91,6 +74,7 @@
203 res = func(*args, **kwargs)
204 cache[key] = res
205 return res
206+ wrapper._wrapped = func
207 return wrapper
208
209
210@@ -190,7 +174,6 @@
211 return os.environ.get('JUJU_RELATION', None)
212
213
214-@cmdline.subcommand()
215 @cached
216 def relation_id(relation_name=None, service_or_unit=None):
217 """The relation ID for the current or a specified relation"""
218@@ -216,13 +199,11 @@
219 return os.environ.get('JUJU_REMOTE_UNIT', None)
220
221
222-@cmdline.subcommand()
223 def service_name():
224 """The name service group this unit belongs to"""
225 return local_unit().split('/')[0]
226
227
228-@cmdline.subcommand()
229 @cached
230 def remote_service_name(relid=None):
231 """The remote service name for a given relation-id (or the current relation)"""
232
233=== modified file 'charmhelpers/core/host.py'
234--- charmhelpers/core/host.py 2015-07-29 10:49:43 +0000
235+++ charmhelpers/core/host.py 2015-08-17 13:36:14 +0000
236@@ -72,7 +72,7 @@
237 stopped = service_stop(service_name)
238 # XXX: Support systemd too
239 override_path = os.path.join(
240- init_dir, '{}.conf.override'.format(service_name))
241+ init_dir, '{}.override'.format(service_name))
242 with open(override_path, 'w') as fh:
243 fh.write("manual\n")
244 return stopped
245@@ -86,7 +86,7 @@
246 if init_dir is None:
247 init_dir = "/etc/init"
248 override_path = os.path.join(
249- init_dir, '{}.conf.override'.format(service_name))
250+ init_dir, '{}.override'.format(service_name))
251 if os.path.exists(override_path):
252 os.unlink(override_path)
253 started = service_start(service_name)
254
255=== modified file 'charmhelpers/fetch/__init__.py'
256--- charmhelpers/fetch/__init__.py 2015-07-29 10:49:43 +0000
257+++ charmhelpers/fetch/__init__.py 2015-08-17 13:36:14 +0000
258@@ -90,6 +90,14 @@
259 'kilo/proposed': 'trusty-proposed/kilo',
260 'trusty-kilo/proposed': 'trusty-proposed/kilo',
261 'trusty-proposed/kilo': 'trusty-proposed/kilo',
262+ # Liberty
263+ 'liberty': 'trusty-updates/liberty',
264+ 'trusty-liberty': 'trusty-updates/liberty',
265+ 'trusty-liberty/updates': 'trusty-updates/liberty',
266+ 'trusty-updates/liberty': 'trusty-updates/liberty',
267+ 'liberty/proposed': 'trusty-proposed/liberty',
268+ 'trusty-liberty/proposed': 'trusty-proposed/liberty',
269+ 'trusty-proposed/liberty': 'trusty-proposed/liberty',
270 }
271
272 # The order of this list is very important. Handlers should be listed in from
273
274=== modified file 'tests/basic_deployment.py'
275--- tests/basic_deployment.py 2015-04-16 21:31:35 +0000
276+++ tests/basic_deployment.py 2015-08-17 13:36:14 +0000
277@@ -1,5 +1,3 @@
278-#!/usr/bin/python
279-
280 import amulet
281 import swiftclient
282
283@@ -122,29 +120,30 @@
284 def test_services(self):
285 """Verify the expected services are running on the corresponding
286 service units."""
287- swift_storage_services = ['status swift-account',
288- 'status swift-account-auditor',
289- 'status swift-account-reaper',
290- 'status swift-account-replicator',
291- 'status swift-container',
292- 'status swift-container-auditor',
293- 'status swift-container-replicator',
294- 'status swift-container-updater',
295- 'status swift-object',
296- 'status swift-object-auditor',
297- 'status swift-object-replicator',
298- 'status swift-object-updater']
299+ swift_storage_services = ['swift-account',
300+ 'swift-account-auditor',
301+ 'swift-account-reaper',
302+ 'swift-account-replicator',
303+ 'swift-container',
304+ 'swift-container-auditor',
305+ 'swift-container-replicator',
306+ 'swift-container-updater',
307+ 'swift-object',
308+ 'swift-object-auditor',
309+ 'swift-object-replicator',
310+ 'swift-object-updater']
311 if self._get_openstack_release() >= self.precise_icehouse:
312- swift_storage_services.append('status swift-container-sync')
313- commands = {
314- self.mysql_sentry: ['status mysql'],
315- self.keystone_sentry: ['status keystone'],
316- self.glance_sentry: ['status glance-registry', 'status glance-api'],
317- self.swift_proxy_sentry: ['status swift-proxy'],
318+ swift_storage_services.append('swift-container-sync')
319+ service_names = {
320+ self.mysql_sentry: ['mysql'],
321+ self.keystone_sentry: ['keystone'],
322+ self.glance_sentry: [
323+ 'glance-registry', 'glance-api'],
324+ self.swift_proxy_sentry: ['swift-proxy'],
325 self.swift_storage_sentry: swift_storage_services
326 }
327
328- ret = u.validate_services(commands)
329+ ret = u.validate_services_by_name(service_names)
330 if ret:
331 amulet.raise_status(amulet.FAIL, msg=ret)
332
333
334=== modified file 'tests/charmhelpers/contrib/amulet/utils.py'
335--- tests/charmhelpers/contrib/amulet/utils.py 2015-07-29 10:49:43 +0000
336+++ tests/charmhelpers/contrib/amulet/utils.py 2015-08-17 13:36:14 +0000
337@@ -14,17 +14,23 @@
338 # You should have received a copy of the GNU Lesser General Public License
339 # along with charm-helpers. If not, see <http://www.gnu.org/licenses/>.
340
341-import amulet
342-import ConfigParser
343-import distro_info
344 import io
345+import json
346 import logging
347 import os
348 import re
349-import six
350+import subprocess
351 import sys
352 import time
353-import urlparse
354+
355+import amulet
356+import distro_info
357+import six
358+from six.moves import configparser
359+if six.PY3:
360+ from urllib import parse as urlparse
361+else:
362+ import urlparse
363
364
365 class AmuletUtils(object):
366@@ -142,19 +148,23 @@
367
368 for service_name in services_list:
369 if (self.ubuntu_releases.index(release) >= systemd_switch or
370- service_name == "rabbitmq-server"):
371- # init is systemd
372+ service_name in ['rabbitmq-server', 'apache2']):
373+ # init is systemd (or regular sysv)
374 cmd = 'sudo service {} status'.format(service_name)
375+ output, code = sentry_unit.run(cmd)
376+ service_running = code == 0
377 elif self.ubuntu_releases.index(release) < systemd_switch:
378 # init is upstart
379 cmd = 'sudo status {}'.format(service_name)
380+ output, code = sentry_unit.run(cmd)
381+ service_running = code == 0 and "start/running" in output
382
383- output, code = sentry_unit.run(cmd)
384 self.log.debug('{} `{}` returned '
385 '{}'.format(sentry_unit.info['unit_name'],
386 cmd, code))
387- if code != 0:
388- return "command `{}` returned {}".format(cmd, str(code))
389+ if not service_running:
390+ return u"command `{}` returned {} {}".format(
391+ cmd, output, str(code))
392 return None
393
394 def _get_config(self, unit, filename):
395@@ -164,7 +174,7 @@
396 # NOTE(beisner): by default, ConfigParser does not handle options
397 # with no value, such as the flags used in the mysql my.cnf file.
398 # https://bugs.python.org/issue7005
399- config = ConfigParser.ConfigParser(allow_no_value=True)
400+ config = configparser.ConfigParser(allow_no_value=True)
401 config.readfp(io.StringIO(file_contents))
402 return config
403
404@@ -450,15 +460,20 @@
405 cmd, code, output))
406 return None
407
408- def get_process_id_list(self, sentry_unit, process_name):
409+ def get_process_id_list(self, sentry_unit, process_name,
410+ expect_success=True):
411 """Get a list of process ID(s) from a single sentry juju unit
412 for a single process name.
413
414- :param sentry_unit: Pointer to amulet sentry instance (juju unit)
415+ :param sentry_unit: Amulet sentry instance (juju unit)
416 :param process_name: Process name
417+ :param expect_success: If False, expect the PID to be missing,
418+ raise if it is present.
419 :returns: List of process IDs
420 """
421- cmd = 'pidof {}'.format(process_name)
422+ cmd = 'pidof -x {}'.format(process_name)
423+ if not expect_success:
424+ cmd += " || exit 0 && exit 1"
425 output, code = sentry_unit.run(cmd)
426 if code != 0:
427 msg = ('{} `{}` returned {} '
428@@ -467,14 +482,23 @@
429 amulet.raise_status(amulet.FAIL, msg=msg)
430 return str(output).split()
431
432- def get_unit_process_ids(self, unit_processes):
433+ def get_unit_process_ids(self, unit_processes, expect_success=True):
434 """Construct a dict containing unit sentries, process names, and
435- process IDs."""
436+ process IDs.
437+
438+ :param unit_processes: A dictionary of Amulet sentry instance
439+ to list of process names.
440+ :param expect_success: if False expect the processes to not be
441+ running, raise if they are.
442+ :returns: Dictionary of Amulet sentry instance to dictionary
443+ of process names to PIDs.
444+ """
445 pid_dict = {}
446- for sentry_unit, process_list in unit_processes.iteritems():
447+ for sentry_unit, process_list in six.iteritems(unit_processes):
448 pid_dict[sentry_unit] = {}
449 for process in process_list:
450- pids = self.get_process_id_list(sentry_unit, process)
451+ pids = self.get_process_id_list(
452+ sentry_unit, process, expect_success=expect_success)
453 pid_dict[sentry_unit].update({process: pids})
454 return pid_dict
455
456@@ -488,7 +512,7 @@
457 return ('Unit count mismatch. expected, actual: {}, '
458 '{} '.format(len(expected), len(actual)))
459
460- for (e_sentry, e_proc_names) in expected.iteritems():
461+ for (e_sentry, e_proc_names) in six.iteritems(expected):
462 e_sentry_name = e_sentry.info['unit_name']
463 if e_sentry in actual.keys():
464 a_proc_names = actual[e_sentry]
465@@ -507,11 +531,23 @@
466 '{}'.format(e_proc_name, a_proc_name))
467
468 a_pids_length = len(a_pids)
469- if e_pids_length != a_pids_length:
470- return ('PID count mismatch. {} ({}) expected, actual: '
471+ fail_msg = ('PID count mismatch. {} ({}) expected, actual: '
472 '{}, {} ({})'.format(e_sentry_name, e_proc_name,
473 e_pids_length, a_pids_length,
474 a_pids))
475+
476+ # If expected is not bool, ensure PID quantities match
477+ if not isinstance(e_pids_length, bool) and \
478+ a_pids_length != e_pids_length:
479+ return fail_msg
480+ # If expected is bool True, ensure 1 or more PIDs exist
481+ elif isinstance(e_pids_length, bool) and \
482+ e_pids_length is True and a_pids_length < 1:
483+ return fail_msg
484+ # If expected is bool False, ensure 0 PIDs exist
485+ elif isinstance(e_pids_length, bool) and \
486+ e_pids_length is False and a_pids_length != 0:
487+ return fail_msg
488 else:
489 self.log.debug('PID check OK: {} {} {}: '
490 '{}'.format(e_sentry_name, e_proc_name,
491@@ -531,3 +567,30 @@
492 return 'Dicts within list are not identical'
493
494 return None
495+
496+ def run_action(self, unit_sentry, action,
497+ _check_output=subprocess.check_output):
498+ """Run the named action on a given unit sentry.
499+
500+ _check_output parameter is used for dependency injection.
501+
502+ @return action_id.
503+ """
504+ unit_id = unit_sentry.info["unit_name"]
505+ command = ["juju", "action", "do", "--format=json", unit_id, action]
506+ self.log.info("Running command: %s\n" % " ".join(command))
507+ output = _check_output(command, universal_newlines=True)
508+ data = json.loads(output)
509+ action_id = data[u'Action queued with id']
510+ return action_id
511+
512+ def wait_on_action(self, action_id, _check_output=subprocess.check_output):
513+ """Wait for a given action, returning if it completed or not.
514+
515+ _check_output parameter is used for dependency injection.
516+ """
517+ command = ["juju", "action", "fetch", "--format=json", "--wait=0",
518+ action_id]
519+ output = _check_output(command, universal_newlines=True)
520+ data = json.loads(output)
521+ return data.get(u"status") == "completed"
522
523=== modified file 'tests/charmhelpers/contrib/openstack/amulet/deployment.py'
524--- tests/charmhelpers/contrib/openstack/amulet/deployment.py 2015-07-29 10:49:43 +0000
525+++ tests/charmhelpers/contrib/openstack/amulet/deployment.py 2015-08-17 13:36:14 +0000
526@@ -44,7 +44,7 @@
527 Determine if the local branch being tested is derived from its
528 stable or next (dev) branch, and based on this, use the corresonding
529 stable or next branches for the other_services."""
530- base_charms = ['mysql', 'mongodb']
531+ base_charms = ['mysql', 'mongodb', 'nrpe']
532
533 if self.series in ['precise', 'trusty']:
534 base_series = self.series
535@@ -81,7 +81,7 @@
536 'ceph-osd', 'ceph-radosgw']
537 # Most OpenStack subordinate charms do not expose an origin option
538 # as that is controlled by the principle.
539- ignore = ['cinder-ceph', 'hacluster', 'neutron-openvswitch']
540+ ignore = ['cinder-ceph', 'hacluster', 'neutron-openvswitch', 'nrpe']
541
542 if self.openstack:
543 for svc in services:

Subscribers

People subscribed via source and target branches