Merge lp:~james-page/charms/trusty/ceph/status into lp:~openstack-charmers-archive/charms/trusty/ceph/next

[James Page]

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #11479 ceph-next for james-page mp273635
    LINT OK: passed

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

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #10668 ceph-next for james-page mp273635
    UNIT FAIL: unit-test failed

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

Full unit test output: http://paste.ubuntu.com/12702241/
Build: http://10.245.162.77:8080/job/charm_unit_test/10668/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #7176 ceph-next for james-page mp273635
    AMULET OK: passed

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

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #10674 ceph-next for james-page mp273635
    UNIT OK: passed

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

[Chris Holcombe (xfactor973)]

Except for some tiny nits this looks great

review: Needs Fixing

[James Page (james-page)]

Urgh - looks like I managed to mess up something - ceph_broker.py remains unchanged, but has been removed/added to the branch for some reason.

I did have some issues running unit tests - I suspect that's the cause. If that needs some fixes, lets do it under a different MP.

review: Resubmit

[David Ames (thedac)]

Tests pass.
Approved, merging.

review: Approve

=== modified file 'hooks/ceph.py'
--- hooks/ceph.py	2015-03-23 17:59:39 +0000
+++ hooks/ceph.py	2015-10-07 01:57:34 +0000
@@ -21,7 +21,8 @@
     log,
     ERROR,
     WARNING,
-    cached
+    cached,
+    status_set,
 )
 from charmhelpers.contrib.storage.linux.utils import (
     zap_disk,
@@ -365,6 +366,7 @@
         log('Looks like {} is in use, skipping.'.format(dev))
         return
 
+    status_set('maintenance', 'Initializing device {}'.format(dev))
     cmd = ['ceph-disk-prepare']
     # Later versions of ceph support more options
     if cmp_pkgrevno('ceph', '0.48.3') >= 0:

=== added file 'hooks/ceph_broker.py'
--- hooks/ceph_broker.py	1970-01-01 00:00:00 +0000
+++ hooks/ceph_broker.py	2015-10-07 01:57:34 +0000
@@ -0,0 +1,100 @@
+#!/usr/bin/python
+#
+# Copyright 2014 Canonical Ltd.
+#
+import json
+
+from charmhelpers.core.hookenv import (
+    log,
+    DEBUG,
+    INFO,
+    ERROR,
+)
+from charmhelpers.contrib.storage.linux.ceph import (
+    create_pool,
+    pool_exists,
+)
+
+
+def decode_req_encode_rsp(f):
+    """Decorator to decode incoming requests and encode responses."""
+    def decode_inner(req):
+        return json.dumps(f(json.loads(req)))
+
+    return decode_inner
+
+
+@decode_req_encode_rsp
+def process_requests(reqs):
+    """Process Ceph broker request(s).
+
+    This is a versioned api. API version must be supplied by the client making
+    the request.
+    """
+    request_id = reqs.get('request-id')
+    try:
+        version = reqs.get('api-version')
+        if version == 1:
+            log('Processing request {}'.format(request_id), level=DEBUG)
+            resp = process_requests_v1(reqs['ops'])
+            if request_id:
+                resp['request-id'] = request_id
+
+            return resp
+
+    except Exception as exc:
+        log(str(exc), level=ERROR)
+        msg = ("Unexpected error occurred while processing requests: %s" %
+               (reqs))
+        log(msg, level=ERROR)
+        return {'exit-code': 1, 'stderr': msg}
+
+    msg = ("Missing or invalid api version (%s)" % (version))
+    resp = {'exit-code': 1, 'stderr': msg}
+    if request_id:
+        resp['request-id'] = request_id
+
+    return resp
+
+
+def process_requests_v1(reqs):
+    """Process v1 requests.
+
+    Takes a list of requests (dicts) and processes each one. If an error is
+    found, processing stops and the client is notified in the response.
+
+    Returns a response dict containing the exit code (non-zero if any
+    operation failed along with an explanation).
+    """
+    log("Processing %s ceph broker requests" % (len(reqs)), level=INFO)
+    for req in reqs:
+        op = req.get('op')
+        log("Processing op='%s'" % (op), level=DEBUG)
+        # Use admin client since we do not have other client key locations
+        # setup to use them for these operations.
+        svc = 'admin'
+        if op == "create-pool":
+            params = {'pool': req.get('name'),
+                      'replicas': req.get('replicas')}
+            if not all(params.iteritems()):
+                msg = ("Missing parameter(s): %s" %
+                       (' '.join([k for k in params.iterkeys()
+                                  if not params[k]])))
+                log(msg, level=ERROR)
+                return {'exit-code': 1, 'stderr': msg}
+
+            pool = params['pool']
+            replicas = params['replicas']
+            if not pool_exists(service=svc, name=pool):
+                log("Creating pool '%s' (replicas=%s)" % (pool, replicas),
+                    level=INFO)
+                create_pool(service=svc, name=pool, replicas=replicas)
+            else:
+                log("Pool '%s' already exists - skipping create" % (pool),
+                    level=DEBUG)
+        else:
+            msg = "Unknown operation '%s'" % (op)
+            log(msg, level=ERROR)
+            return {'exit-code': 1, 'stderr': msg}
+
+    return {'exit-code': 0}

=== removed file 'hooks/ceph_broker.py'
--- hooks/ceph_broker.py	2015-09-04 10:33:49 +0000
+++ hooks/ceph_broker.py	1970-01-01 00:00:00 +0000
@@ -1,100 +0,0 @@
-#!/usr/bin/python
-#
-# Copyright 2014 Canonical Ltd.
-#
-import json
-
-from charmhelpers.core.hookenv import (
-    log,
-    DEBUG,
-    INFO,
-    ERROR,
-)
-from charmhelpers.contrib.storage.linux.ceph import (
-    create_pool,
-    pool_exists,
-)
-
-
-def decode_req_encode_rsp(f):
-    """Decorator to decode incoming requests and encode responses."""
-    def decode_inner(req):
-        return json.dumps(f(json.loads(req)))
-
-    return decode_inner
-
-
-@decode_req_encode_rsp
-def process_requests(reqs):
-    """Process Ceph broker request(s).
-
-    This is a versioned api. API version must be supplied by the client making
-    the request.
-    """
-    request_id = reqs.get('request-id')
-    try:
-        version = reqs.get('api-version')
-        if version == 1:
-            log('Processing request {}'.format(request_id), level=DEBUG)
-            resp = process_requests_v1(reqs['ops'])
-            if request_id:
-                resp['request-id'] = request_id
-
-            return resp
-
-    except Exception as exc:
-        log(str(exc), level=ERROR)
-        msg = ("Unexpected error occurred while processing requests: %s" %
-               (reqs))
-        log(msg, level=ERROR)
-        return {'exit-code': 1, 'stderr': msg}
-
-    msg = ("Missing or invalid api version (%s)" % (version))
-    resp = {'exit-code': 1, 'stderr': msg}
-    if request_id:
-        resp['request-id'] = request_id
-
-    return resp
-
-
-def process_requests_v1(reqs):
-    """Process v1 requests.
-
-    Takes a list of requests (dicts) and processes each one. If an error is
-    found, processing stops and the client is notified in the response.
-
-    Returns a response dict containing the exit code (non-zero if any
-    operation failed along with an explanation).
-    """
-    log("Processing %s ceph broker requests" % (len(reqs)), level=INFO)
-    for req in reqs:
-        op = req.get('op')
-        log("Processing op='%s'" % (op), level=DEBUG)
-        # Use admin client since we do not have other client key locations
-        # setup to use them for these operations.
-        svc = 'admin'
-        if op == "create-pool":
-            params = {'pool': req.get('name'),
-                      'replicas': req.get('replicas')}
-            if not all(params.iteritems()):
-                msg = ("Missing parameter(s): %s" %
-                       (' '.join([k for k in params.iterkeys()
-                                  if not params[k]])))
-                log(msg, level=ERROR)
-                return {'exit-code': 1, 'stderr': msg}
-
-            pool = params['pool']
-            replicas = params['replicas']
-            if not pool_exists(service=svc, name=pool):
-                log("Creating pool '%s' (replicas=%s)" % (pool, replicas),
-                    level=INFO)
-                create_pool(service=svc, name=pool, replicas=replicas)
-            else:
-                log("Pool '%s' already exists - skipping create" % (pool),
-                    level=DEBUG)
-        else:
-            msg = "Unknown operation '%s'" % (op)
-            log(msg, level=ERROR)
-            return {'exit-code': 1, 'stderr': msg}
-
-    return {'exit-code': 0}

=== renamed file 'hooks/hooks.py' => 'hooks/ceph_hooks.py'
--- hooks/hooks.py	2015-09-22 13:35:02 +0000
+++ hooks/ceph_hooks.py	2015-10-07 01:57:34 +0000
@@ -26,7 +26,9 @@
     remote_unit,
     Hooks, UnregisteredHookError,
     service_name,
-    relations_of_type
+    relations_of_type,
+    status_set,
+    local_unit,
 )
 from charmhelpers.core.host import (
     service_restart,
@@ -152,6 +154,7 @@
 
     # Support use of single node ceph
     if (not ceph.is_bootstrapped() and int(config('monitor-count')) == 1):
+        status_set('maintenance', 'Bootstrapping single Ceph MON')
         ceph.bootstrap_monitor_cluster(config('monitor-secret'))
         ceph.wait_for_bootstrap()
 
@@ -181,6 +184,20 @@
     return hosts
 
 
+def get_peer_units():
+    '''
+    Returns a dictionary of unit names from the mon peer relation with
+    a flag indicating whether the unit has presented its address
+    '''
+    units = {}
+    units[local_unit()] = True
+    for relid in relation_ids('mon'):
+        for unit in related_units(relid):
+            addr = relation_get('ceph-public-address', unit, relid)
+            units[unit] = addr is not None
+    return units
+
+
 def reformat_osd():
     if config('osd-reformat'):
         return True
@@ -210,6 +227,7 @@
 
     moncount = int(config('monitor-count'))
     if len(get_mon_hosts()) >= moncount:
+        status_set('maintenance', 'Bootstrapping MON cluster')
         ceph.bootstrap_monitor_cluster(config('monitor-secret'))
         ceph.wait_for_bootstrap()
         for dev in get_devices():
@@ -384,8 +402,34 @@
     nrpe_setup.write()
 
 
+def assess_status():
+    '''Assess status of current unit'''
+    moncount = int(config('monitor-count'))
+    units = get_peer_units()
+    # not enough peers and mon_count > 1
+    if len(units.keys()) < moncount:
+        status_set('blocked', 'Insufficient peer units to bootstrap'
+                              ' cluster (require {})'.format(moncount))
+        return
+
+    # mon_count > 1, peers, but no ceph-public-address
+    ready = sum(1 for unit_ready in units.itervalues() if unit_ready)
+    if ready < moncount:
+        status_set('waiting', 'Peer units detected, waiting for addresses')
+        return
+
+    # active - bootstrapped + quorum status check
+    if ceph.is_bootstrapped() and ceph.is_quorum():
+        status_set('active', 'Unit is ready and clustered')
+    else:
+        # Unit should be running and clustered, but no quorum
+        # TODO: should this be blocked or waiting?
+        status_set('blocked', 'Unit not clustered (no quorum)')
+
+
 if __name__ == '__main__':
     try:
         hooks.execute(sys.argv)
     except UnregisteredHookError as e:
         log('Unknown hook {} - skipping.'.format(e))
+    assess_status()

=== modified symlink 'hooks/client-relation-changed'
=== target changed u'hooks.py' => u'ceph_hooks.py'
=== modified symlink 'hooks/client-relation-joined'
=== target changed u'hooks.py' => u'ceph_hooks.py'
=== modified symlink 'hooks/config-changed'
=== target changed u'hooks.py' => u'ceph_hooks.py'
=== modified symlink 'hooks/install.real'
=== target changed u'hooks.py' => u'ceph_hooks.py'
=== modified symlink 'hooks/mon-relation-changed'
=== target changed u'hooks.py' => u'ceph_hooks.py'
=== modified symlink 'hooks/mon-relation-departed'
=== target changed u'hooks.py' => u'ceph_hooks.py'
=== modified symlink 'hooks/mon-relation-joined'
=== target changed u'hooks.py' => u'ceph_hooks.py'
=== modified symlink 'hooks/nrpe-external-master-relation-changed'
=== target changed u'hooks.py' => u'ceph_hooks.py'
=== modified symlink 'hooks/nrpe-external-master-relation-joined'
=== target changed u'hooks.py' => u'ceph_hooks.py'
=== modified symlink 'hooks/osd-relation-joined'
=== target changed u'hooks.py' => u'ceph_hooks.py'
=== modified symlink 'hooks/radosgw-relation-joined'
=== target changed u'hooks.py' => u'ceph_hooks.py'
=== modified symlink 'hooks/start'
=== target changed u'hooks.py' => u'ceph_hooks.py'
=== modified symlink 'hooks/stop'
=== target changed u'hooks.py' => u'ceph_hooks.py'
=== added symlink 'hooks/update-status'
=== target is u'ceph_hooks.py'
=== modified symlink 'hooks/upgrade-charm'
=== target changed u'hooks.py' => u'ceph_hooks.py'
=== added file 'unit_tests/test_status.py'
--- unit_tests/test_status.py	1970-01-01 00:00:00 +0000
+++ unit_tests/test_status.py	2015-10-07 01:57:34 +0000
@@ -0,0 +1,94 @@
+import mock
+import test_utils
+
+import ceph_hooks as hooks
+
+TO_PATCH = [
+    'status_set',
+    'config',
+    'ceph',
+    'relation_ids',
+    'relation_get',
+    'related_units',
+    'local_unit',
+]
+
+NO_PEERS = {
+    'ceph-mon1': True
+}
+
+ENOUGH_PEERS_INCOMPLETE = {
+    'ceph-mon1': True,
+    'ceph-mon2': False,
+    'ceph-mon3': False,
+}
+
+ENOUGH_PEERS_COMPLETE = {
+    'ceph-mon1': True,
+    'ceph-mon2': True,
+    'ceph-mon3': True,
+}
+
+
+class ServiceStatusTestCase(test_utils.CharmTestCase):
+
+    def setUp(self):
+        super(ServiceStatusTestCase, self).setUp(hooks, TO_PATCH)
+        self.config.side_effect = self.test_config.get
+        self.test_config.set('monitor-count', 3)
+        self.local_unit.return_value = 'ceph-mon1'
+
+    @mock.patch.object(hooks, 'get_peer_units')
+    def test_assess_status_no_peers(self, _peer_units):
+        _peer_units.return_value = NO_PEERS
+        hooks.assess_status()
+        self.status_set.assert_called_with('blocked', mock.ANY)
+
+    @mock.patch.object(hooks, 'get_peer_units')
+    def test_assess_status_peers_incomplete(self, _peer_units):
+        _peer_units.return_value = ENOUGH_PEERS_INCOMPLETE
+        hooks.assess_status()
+        self.status_set.assert_called_with('waiting', mock.ANY)
+
+    @mock.patch.object(hooks, 'get_peer_units')
+    def test_assess_status_peers_complete_active(self, _peer_units):
+        _peer_units.return_value = ENOUGH_PEERS_COMPLETE
+        self.ceph.is_bootstrapped.return_value = True
+        self.ceph.is_quorum.return_value = True
+        hooks.assess_status()
+        self.status_set.assert_called_with('active', mock.ANY)
+
+    @mock.patch.object(hooks, 'get_peer_units')
+    def test_assess_status_peers_complete_down(self, _peer_units):
+        _peer_units.return_value = ENOUGH_PEERS_COMPLETE
+        self.ceph.is_bootstrapped.return_value = False
+        self.ceph.is_quorum.return_value = False
+        hooks.assess_status()
+        self.status_set.assert_called_with('blocked', mock.ANY)
+
+    def test_get_peer_units_no_peers(self):
+        self.relation_ids.return_value = ['mon:1']
+        self.related_units.return_value = []
+        self.assertEquals({'ceph-mon1': True},
+                          hooks.get_peer_units())
+
+    def test_get_peer_units_peers_incomplete(self):
+        self.relation_ids.return_value = ['mon:1']
+        self.related_units.return_value = ['ceph-mon2',
+                                           'ceph-mon3']
+        self.relation_get.return_value = None
+        self.assertEquals({'ceph-mon1': True,
+                           'ceph-mon2': False,
+                           'ceph-mon3': False},
+                          hooks.get_peer_units())
+
+    def test_get_peer_units_peers_complete(self):
+        self.relation_ids.return_value = ['mon:1']
+        self.related_units.return_value = ['ceph-mon2',
+                                           'ceph-mon3']
+        self.relation_get.side_effect = ['ceph-mon2',
+                                         'ceph-mon3']
+        self.assertEquals({'ceph-mon1': True,
+                           'ceph-mon2': True,
+                           'ceph-mon3': True},
+                          hooks.get_peer_units())

=== added file 'unit_tests/test_utils.py'
--- unit_tests/test_utils.py	1970-01-01 00:00:00 +0000
+++ unit_tests/test_utils.py	2015-10-07 01:57:34 +0000
@@ -0,0 +1,121 @@
+import logging
+import unittest
+import os
+import yaml
+
+from contextlib import contextmanager
+from mock import patch, MagicMock
+
+
+def load_config():
+    '''
+    Walk backwords from __file__ looking for config.yaml, load and return the
+    'options' section'
+    '''
+    config = None
+    f = __file__
+    while config is None:
+        d = os.path.dirname(f)
+        if os.path.isfile(os.path.join(d, 'config.yaml')):
+            config = os.path.join(d, 'config.yaml')
+            break
+        f = d
+
+    if not config:
+        logging.error('Could not find config.yaml in any parent directory '
+                      'of %s. ' % f)
+        raise Exception
+
+    return yaml.safe_load(open(config).read())['options']
+
+
+def get_default_config():
+    '''
+    Load default charm config from config.yaml return as a dict.
+    If no default is set in config.yaml, its value is None.
+    '''
+    default_config = {}
+    config = load_config()
+    for k, v in config.iteritems():
+        if 'default' in v:
+            default_config[k] = v['default']
+        else:
+            default_config[k] = None
+    return default_config
+
+
+class CharmTestCase(unittest.TestCase):
+
+    def setUp(self, obj, patches):
+        super(CharmTestCase, self).setUp()
+        self.patches = patches
+        self.obj = obj
+        self.test_config = TestConfig()
+        self.test_relation = TestRelation()
+        self.patch_all()
+
+    def patch(self, method):
+        _m = patch.object(self.obj, method)
+        mock = _m.start()
+        self.addCleanup(_m.stop)
+        return mock
+
+    def patch_all(self):
+        for method in self.patches:
+            setattr(self, method, self.patch(method))
+
+
+class TestConfig(object):
+
+    def __init__(self):
+        self.config = get_default_config()
+
+    def get(self, attr=None):
+        if not attr:
+            return self.get_all()
+        try:
+            return self.config[attr]
+        except KeyError:
+            return None
+
+    def get_all(self):
+        return self.config
+
+    def set(self, attr, value):
+        if attr not in self.config:
+            raise KeyError
+        self.config[attr] = value
+
+
+class TestRelation(object):
+
+    def __init__(self, relation_data={}):
+        self.relation_data = relation_data
+
+    def set(self, relation_data):
+        self.relation_data = relation_data
+
+    def get(self, attr=None, unit=None, rid=None):
+        if attr is None:
+            return self.relation_data
+        elif attr in self.relation_data:
+            return self.relation_data[attr]
+        return None
+
+
+@contextmanager
+def patch_open():
+    '''Patch open() to allow mocking both open() itself and the file that is
+    yielded.
+
+    Yields the mock for "open" and "file", respectively.'''
+    mock_open = MagicMock(spec=open)
+    mock_file = MagicMock(spec=file)
+
+    @contextmanager
+    def stub_open(*args, **kwargs):
+        mock_open(*args, **kwargs)
+        yield mock_file
+
+    with patch('__builtin__.open', stub_open):
+        yield mock_open, mock_file