Merge lp:~gnuoy/charms/trusty/ceph/1453940-stable into lp:~openstack-charmers-archive/charms/trusty/ceph/trunk

Proposed by Liam Young on 2015-09-16
Status: Merged
Merged at revision: 110
Proposed branch: lp:~gnuoy/charms/trusty/ceph/1453940-stable
Merge into: lp:~openstack-charmers-archive/charms/trusty/ceph/trunk
Diff against target: 487 lines (+278/-38)
8 files modified
hooks/ceph_broker.py (+12/-2)
hooks/charmhelpers/cli/__init__.py (+1/-5)
hooks/charmhelpers/cli/commands.py (+4/-4)
hooks/charmhelpers/cli/hookenv.py (+23/-0)
hooks/charmhelpers/contrib/storage/linux/ceph.py (+224/-2)
hooks/charmhelpers/core/hookenv.py (+1/-20)
hooks/charmhelpers/core/host.py (+2/-2)
hooks/hooks.py (+11/-3)
To merge this branch: bzr merge lp:~gnuoy/charms/trusty/ceph/1453940-stable
Reviewer Review Type Date Requested Status
Chris Glass (community) 2015-09-16 Approve on 2015-09-22
Review via email: mp+271260@code.launchpad.net
To post a comment you must log in.

charm_lint_check #10408 ceph for gnuoy mp271260
    LINT OK: passed

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

charm_unit_test #9558 ceph for gnuoy mp271260
    UNIT OK: passed

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

charm_amulet_test #6552 ceph for gnuoy mp271260
    AMULET OK: passed

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

charm_lint_check #10438 ceph for gnuoy mp271260
    LINT OK: passed

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

charm_unit_test #9630 ceph for gnuoy mp271260
    UNIT OK: passed

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

charm_amulet_test #6579 ceph for gnuoy mp271260
    AMULET OK: passed

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

Chris Glass (tribaal) wrote :

Looks good! +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'hooks/ceph_broker.py'
2--- hooks/ceph_broker.py 2014-11-19 21:33:12 +0000
3+++ hooks/ceph_broker.py 2015-09-16 09:02:26 +0000
4@@ -31,10 +31,16 @@
5 This is a versioned api. API version must be supplied by the client making
6 the request.
7 """
8+ request_id = reqs.get('request-id')
9 try:
10 version = reqs.get('api-version')
11 if version == 1:
12- return process_requests_v1(reqs['ops'])
13+ log('Processing request {}'.format(request_id), level=DEBUG)
14+ resp = process_requests_v1(reqs['ops'])
15+ if request_id:
16+ resp['request-id'] = request_id
17+
18+ return resp
19
20 except Exception as exc:
21 log(str(exc), level=ERROR)
22@@ -44,7 +50,11 @@
23 return {'exit-code': 1, 'stderr': msg}
24
25 msg = ("Missing or invalid api version (%s)" % (version))
26- return {'exit-code': 1, 'stderr': msg}
27+ resp = {'exit-code': 1, 'stderr': msg}
28+ if request_id:
29+ resp['request-id'] = request_id
30+
31+ return resp
32
33
34 def process_requests_v1(reqs):
35
36=== modified file 'hooks/charmhelpers/cli/__init__.py'
37--- hooks/charmhelpers/cli/__init__.py 2015-08-10 16:32:52 +0000
38+++ hooks/charmhelpers/cli/__init__.py 2015-09-16 09:02:26 +0000
39@@ -152,15 +152,11 @@
40 arguments = self.argument_parser.parse_args()
41 argspec = inspect.getargspec(arguments.func)
42 vargs = []
43- kwargs = {}
44 for arg in argspec.args:
45 vargs.append(getattr(arguments, arg))
46 if argspec.varargs:
47 vargs.extend(getattr(arguments, argspec.varargs))
48- if argspec.keywords:
49- for kwarg in argspec.keywords.items():
50- kwargs[kwarg] = getattr(arguments, kwarg)
51- output = arguments.func(*vargs, **kwargs)
52+ output = arguments.func(*vargs)
53 if getattr(arguments.func, '_cli_test_command', False):
54 self.exit_code = 0 if output else 1
55 output = ''
56
57=== modified file 'hooks/charmhelpers/cli/commands.py'
58--- hooks/charmhelpers/cli/commands.py 2015-08-10 16:32:52 +0000
59+++ hooks/charmhelpers/cli/commands.py 2015-09-16 09:02:26 +0000
60@@ -26,7 +26,7 @@
61 """
62 Import the sub-modules which have decorated subcommands to register with chlp.
63 """
64-import host # noqa
65-import benchmark # noqa
66-import unitdata # noqa
67-from charmhelpers.core import hookenv # noqa
68+from . import host # noqa
69+from . import benchmark # noqa
70+from . import unitdata # noqa
71+from . import hookenv # noqa
72
73=== added file 'hooks/charmhelpers/cli/hookenv.py'
74--- hooks/charmhelpers/cli/hookenv.py 1970-01-01 00:00:00 +0000
75+++ hooks/charmhelpers/cli/hookenv.py 2015-09-16 09:02:26 +0000
76@@ -0,0 +1,23 @@
77+# Copyright 2014-2015 Canonical Limited.
78+#
79+# This file is part of charm-helpers.
80+#
81+# charm-helpers is free software: you can redistribute it and/or modify
82+# it under the terms of the GNU Lesser General Public License version 3 as
83+# published by the Free Software Foundation.
84+#
85+# charm-helpers is distributed in the hope that it will be useful,
86+# but WITHOUT ANY WARRANTY; without even the implied warranty of
87+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
88+# GNU Lesser General Public License for more details.
89+#
90+# You should have received a copy of the GNU Lesser General Public License
91+# along with charm-helpers. If not, see <http://www.gnu.org/licenses/>.
92+
93+from . import cmdline
94+from charmhelpers.core import hookenv
95+
96+
97+cmdline.subcommand('relation-id')(hookenv.relation_id._wrapped)
98+cmdline.subcommand('service-name')(hookenv.service_name)
99+cmdline.subcommand('remote-service-name')(hookenv.remote_service_name._wrapped)
100
101=== modified file 'hooks/charmhelpers/contrib/storage/linux/ceph.py'
102--- hooks/charmhelpers/contrib/storage/linux/ceph.py 2015-08-10 16:32:52 +0000
103+++ hooks/charmhelpers/contrib/storage/linux/ceph.py 2015-09-16 09:02:26 +0000
104@@ -28,6 +28,7 @@
105 import shutil
106 import json
107 import time
108+import uuid
109
110 from subprocess import (
111 check_call,
112@@ -35,8 +36,10 @@
113 CalledProcessError,
114 )
115 from charmhelpers.core.hookenv import (
116+ local_unit,
117 relation_get,
118 relation_ids,
119+ relation_set,
120 related_units,
121 log,
122 DEBUG,
123@@ -411,17 +414,52 @@
124
125 The API is versioned and defaults to version 1.
126 """
127- def __init__(self, api_version=1):
128+ def __init__(self, api_version=1, request_id=None):
129 self.api_version = api_version
130+ if request_id:
131+ self.request_id = request_id
132+ else:
133+ self.request_id = str(uuid.uuid1())
134 self.ops = []
135
136 def add_op_create_pool(self, name, replica_count=3):
137 self.ops.append({'op': 'create-pool', 'name': name,
138 'replicas': replica_count})
139
140+ def set_ops(self, ops):
141+ """Set request ops to provided value.
142+
143+ Useful for injecting ops that come from a previous request
144+ to allow comparisons to ensure validity.
145+ """
146+ self.ops = ops
147+
148 @property
149 def request(self):
150- return json.dumps({'api-version': self.api_version, 'ops': self.ops})
151+ return json.dumps({'api-version': self.api_version, 'ops': self.ops,
152+ 'request-id': self.request_id})
153+
154+ def _ops_equal(self, other):
155+ if len(self.ops) == len(other.ops):
156+ for req_no in range(0, len(self.ops)):
157+ for key in ['replicas', 'name', 'op']:
158+ if self.ops[req_no][key] != other.ops[req_no][key]:
159+ return False
160+ else:
161+ return False
162+ return True
163+
164+ def __eq__(self, other):
165+ if not isinstance(other, self.__class__):
166+ return False
167+ if self.api_version == other.api_version and \
168+ self._ops_equal(other):
169+ return True
170+ else:
171+ return False
172+
173+ def __ne__(self, other):
174+ return not self.__eq__(other)
175
176
177 class CephBrokerRsp(object):
178@@ -431,14 +469,198 @@
179
180 The API is versioned and defaults to version 1.
181 """
182+
183 def __init__(self, encoded_rsp):
184 self.api_version = None
185 self.rsp = json.loads(encoded_rsp)
186
187 @property
188+ def request_id(self):
189+ return self.rsp.get('request-id')
190+
191+ @property
192 def exit_code(self):
193 return self.rsp.get('exit-code')
194
195 @property
196 def exit_msg(self):
197 return self.rsp.get('stderr')
198+
199+
200+# Ceph Broker Conversation:
201+# If a charm needs an action to be taken by ceph it can create a CephBrokerRq
202+# and send that request to ceph via the ceph relation. The CephBrokerRq has a
203+# unique id so that the client can identity which CephBrokerRsp is associated
204+# with the request. Ceph will also respond to each client unit individually
205+# creating a response key per client unit eg glance/0 will get a CephBrokerRsp
206+# via key broker-rsp-glance-0
207+#
208+# To use this the charm can just do something like:
209+#
210+# from charmhelpers.contrib.storage.linux.ceph import (
211+# send_request_if_needed,
212+# is_request_complete,
213+# CephBrokerRq,
214+# )
215+#
216+# @hooks.hook('ceph-relation-changed')
217+# def ceph_changed():
218+# rq = CephBrokerRq()
219+# rq.add_op_create_pool(name='poolname', replica_count=3)
220+#
221+# if is_request_complete(rq):
222+# <Request complete actions>
223+# else:
224+# send_request_if_needed(get_ceph_request())
225+#
226+# CephBrokerRq and CephBrokerRsp are serialized into JSON. Below is an example
227+# of glance having sent a request to ceph which ceph has successfully processed
228+# 'ceph:8': {
229+# 'ceph/0': {
230+# 'auth': 'cephx',
231+# 'broker-rsp-glance-0': '{"request-id": "0bc7dc54", "exit-code": 0}',
232+# 'broker_rsp': '{"request-id": "0da543b8", "exit-code": 0}',
233+# 'ceph-public-address': '10.5.44.103',
234+# 'key': 'AQCLDttVuHXINhAAvI144CB09dYchhHyTUY9BQ==',
235+# 'private-address': '10.5.44.103',
236+# },
237+# 'glance/0': {
238+# 'broker_req': ('{"api-version": 1, "request-id": "0bc7dc54", '
239+# '"ops": [{"replicas": 3, "name": "glance", '
240+# '"op": "create-pool"}]}'),
241+# 'private-address': '10.5.44.109',
242+# },
243+# }
244+
245+def get_previous_request(rid):
246+ """Return the last ceph broker request sent on a given relation
247+
248+ @param rid: Relation id to query for request
249+ """
250+ request = None
251+ broker_req = relation_get(attribute='broker_req', rid=rid,
252+ unit=local_unit())
253+ if broker_req:
254+ request_data = json.loads(broker_req)
255+ request = CephBrokerRq(api_version=request_data['api-version'],
256+ request_id=request_data['request-id'])
257+ request.set_ops(request_data['ops'])
258+
259+ return request
260+
261+
262+def get_request_states(request):
263+ """Return a dict of requests per relation id with their corresponding
264+ completion state.
265+
266+ This allows a charm, which has a request for ceph, to see whether there is
267+ an equivalent request already being processed and if so what state that
268+ request is in.
269+
270+ @param request: A CephBrokerRq object
271+ """
272+ complete = []
273+ requests = {}
274+ for rid in relation_ids('ceph'):
275+ complete = False
276+ previous_request = get_previous_request(rid)
277+ if request == previous_request:
278+ sent = True
279+ complete = is_request_complete_for_rid(previous_request, rid)
280+ else:
281+ sent = False
282+ complete = False
283+
284+ requests[rid] = {
285+ 'sent': sent,
286+ 'complete': complete,
287+ }
288+
289+ return requests
290+
291+
292+def is_request_sent(request):
293+ """Check to see if a functionally equivalent request has already been sent
294+
295+ Returns True if a similair request has been sent
296+
297+ @param request: A CephBrokerRq object
298+ """
299+ states = get_request_states(request)
300+ for rid in states.keys():
301+ if not states[rid]['sent']:
302+ return False
303+
304+ return True
305+
306+
307+def is_request_complete(request):
308+ """Check to see if a functionally equivalent request has already been
309+ completed
310+
311+ Returns True if a similair request has been completed
312+
313+ @param request: A CephBrokerRq object
314+ """
315+ states = get_request_states(request)
316+ for rid in states.keys():
317+ if not states[rid]['complete']:
318+ return False
319+
320+ return True
321+
322+
323+def is_request_complete_for_rid(request, rid):
324+ """Check if a given request has been completed on the given relation
325+
326+ @param request: A CephBrokerRq object
327+ @param rid: Relation ID
328+ """
329+ broker_key = get_broker_rsp_key()
330+ for unit in related_units(rid):
331+ rdata = relation_get(rid=rid, unit=unit)
332+ if rdata.get(broker_key):
333+ rsp = CephBrokerRsp(rdata.get(broker_key))
334+ if rsp.request_id == request.request_id:
335+ if not rsp.exit_code:
336+ return True
337+ else:
338+ # The remote unit sent no reply targeted at this unit so either the
339+ # remote ceph cluster does not support unit targeted replies or it
340+ # has not processed our request yet.
341+ if rdata.get('broker_rsp'):
342+ request_data = json.loads(rdata['broker_rsp'])
343+ if request_data.get('request-id'):
344+ log('Ignoring legacy broker_rsp without unit key as remote '
345+ 'service supports unit specific replies', level=DEBUG)
346+ else:
347+ log('Using legacy broker_rsp as remote service does not '
348+ 'supports unit specific replies', level=DEBUG)
349+ rsp = CephBrokerRsp(rdata['broker_rsp'])
350+ if not rsp.exit_code:
351+ return True
352+
353+ return False
354+
355+
356+def get_broker_rsp_key():
357+ """Return broker response key for this unit
358+
359+ This is the key that ceph is going to use to pass request status
360+ information back to this unit
361+ """
362+ return 'broker-rsp-' + local_unit().replace('/', '-')
363+
364+
365+def send_request_if_needed(request):
366+ """Send broker request if an equivalent request has not already been sent
367+
368+ @param request: A CephBrokerRq object
369+ """
370+ if is_request_sent(request):
371+ log('Request already sent but not complete, not sending new request',
372+ level=DEBUG)
373+ else:
374+ for rid in relation_ids('ceph'):
375+ log('Sending request {}'.format(request.request_id), level=DEBUG)
376+ relation_set(relation_id=rid, broker_req=request.request)
377
378=== modified file 'hooks/charmhelpers/core/hookenv.py'
379--- hooks/charmhelpers/core/hookenv.py 2015-08-10 16:32:52 +0000
380+++ hooks/charmhelpers/core/hookenv.py 2015-09-16 09:02:26 +0000
381@@ -34,23 +34,6 @@
382 import tempfile
383 from subprocess import CalledProcessError
384
385-try:
386- from charmhelpers.cli import cmdline
387-except ImportError as e:
388- # due to the anti-pattern of partially synching charmhelpers directly
389- # into charms, it's possible that charmhelpers.cli is not available;
390- # if that's the case, they don't really care about using the cli anyway,
391- # so mock it out
392- if str(e) == 'No module named cli':
393- class cmdline(object):
394- @classmethod
395- def subcommand(cls, *args, **kwargs):
396- def _wrap(func):
397- return func
398- return _wrap
399- else:
400- raise
401-
402 import six
403 if not six.PY3:
404 from UserDict import UserDict
405@@ -91,6 +74,7 @@
406 res = func(*args, **kwargs)
407 cache[key] = res
408 return res
409+ wrapper._wrapped = func
410 return wrapper
411
412
413@@ -190,7 +174,6 @@
414 return os.environ.get('JUJU_RELATION', None)
415
416
417-@cmdline.subcommand()
418 @cached
419 def relation_id(relation_name=None, service_or_unit=None):
420 """The relation ID for the current or a specified relation"""
421@@ -216,13 +199,11 @@
422 return os.environ.get('JUJU_REMOTE_UNIT', None)
423
424
425-@cmdline.subcommand()
426 def service_name():
427 """The name service group this unit belongs to"""
428 return local_unit().split('/')[0]
429
430
431-@cmdline.subcommand()
432 @cached
433 def remote_service_name(relid=None):
434 """The remote service name for a given relation-id (or the current relation)"""
435
436=== modified file 'hooks/charmhelpers/core/host.py'
437--- hooks/charmhelpers/core/host.py 2015-08-10 16:32:52 +0000
438+++ hooks/charmhelpers/core/host.py 2015-09-16 09:02:26 +0000
439@@ -72,7 +72,7 @@
440 stopped = service_stop(service_name)
441 # XXX: Support systemd too
442 override_path = os.path.join(
443- init_dir, '{}.conf.override'.format(service_name))
444+ init_dir, '{}.override'.format(service_name))
445 with open(override_path, 'w') as fh:
446 fh.write("manual\n")
447 return stopped
448@@ -86,7 +86,7 @@
449 if init_dir is None:
450 init_dir = "/etc/init"
451 override_path = os.path.join(
452- init_dir, '{}.conf.override'.format(service_name))
453+ init_dir, '{}.override'.format(service_name))
454 if os.path.exists(override_path):
455 os.unlink(override_path)
456 started = service_start(service_name)
457
458=== modified file 'hooks/hooks.py'
459--- hooks/hooks.py 2015-03-23 17:40:42 +0000
460+++ hooks/hooks.py 2015-09-16 09:02:26 +0000
461@@ -144,8 +144,8 @@
462 umount(e_mountpoint)
463
464 osd_journal = config('osd-journal')
465- if (osd_journal and not os.path.exists(JOURNAL_ZAPPED)
466- and os.path.exists(osd_journal)):
467+ if (osd_journal and not os.path.exists(JOURNAL_ZAPPED) and
468+ os.path.exists(osd_journal)):
469 ceph.zap_disk(osd_journal)
470 with open(JOURNAL_ZAPPED, 'w') as zapped:
471 zapped.write('DONE')
472@@ -319,7 +319,15 @@
473 log("Not leader - ignoring broker request", level=DEBUG)
474 else:
475 rsp = process_requests(settings['broker_req'])
476- relation_set(relation_settings={'broker_rsp': rsp})
477+ unit_id = remote_unit().replace('/', '-')
478+ unit_response_key = 'broker-rsp-' + unit_id
479+ # broker_rsp is being left for backward compatibility,
480+ # unit_response_key superscedes it
481+ data = {
482+ 'broker_rsp': rsp,
483+ unit_response_key: rsp,
484+ }
485+ relation_set(relation_settings=data)
486 else:
487 log('mon cluster not in quorum', level=DEBUG)
488

Subscribers

People subscribed via source and target branches