From 3649d58e1ec597b1ad520631a3a824aeece771ef Mon Sep 17 00:00:00 2001 From: James Page Date: Tue, 6 Oct 2015 11:06:36 +0100 Subject: [PATCH 1/6] Add basic status assessment for monitor role --- hooks/ceph.py | 4 +++- hooks/hooks.py | 46 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/hooks/ceph.py b/hooks/ceph.py index d4b98b58..1d77030a 100644 --- a/hooks/ceph.py +++ b/hooks/ceph.py @@ -21,7 +21,8 @@ from charmhelpers.core.hookenv import ( log, ERROR, WARNING, - cached + cached, + status_set, ) from charmhelpers.contrib.storage.linux.utils import ( zap_disk, @@ -365,6 +366,7 @@ def osdize_dev(dev, osd_format, osd_journal, reformat_osd=False, 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: diff --git a/hooks/hooks.py b/hooks/hooks.py index 2306fcff..c680a4aa 100755 --- a/hooks/hooks.py +++ b/hooks/hooks.py @@ -26,7 +26,9 @@ from charmhelpers.core.hookenv import ( 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 @@ def config_changed(): # 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 @@ def get_mon_hosts(): 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 @@ def mon_relation(): 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 @@ def update_nrpe_config(): 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 active 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() From 05fb30809060deabd3fe88b87be1b33ccaecd257 Mon Sep 17 00:00:00 2001 From: James Page Date: Tue, 6 Oct 2015 11:21:45 +0100 Subject: [PATCH 2/6] Make unit messaging consistent --- hooks/hooks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hooks/hooks.py b/hooks/hooks.py index c680a4aa..ccd575ab 100755 --- a/hooks/hooks.py +++ b/hooks/hooks.py @@ -420,7 +420,7 @@ def assess_status(): # active - bootstrapped + quorum status check if ceph.is_bootstrapped() and ceph.is_quorum(): - status_set('active', 'Unit active and clustered') + 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? From d489ba4484093d71c03d74c4e60b2306c4803126 Mon Sep 17 00:00:00 2001 From: James Page Date: Tue, 6 Oct 2015 21:02:06 +0100 Subject: [PATCH 3/6] Add some unit tests to cover service status --- hooks/{hooks.py => ceph_hooks.py} | 0 hooks/client-relation-changed | 2 +- hooks/client-relation-joined | 2 +- hooks/config-changed | 2 +- hooks/install.real | 2 +- hooks/mon-relation-changed | 2 +- hooks/mon-relation-departed | 2 +- hooks/mon-relation-joined | 2 +- hooks/nrpe-external-master-relation-changed | 2 +- hooks/nrpe-external-master-relation-joined | 2 +- hooks/osd-relation-joined | 2 +- hooks/radosgw-relation-joined | 2 +- hooks/start | 2 +- hooks/stop | 2 +- hooks/update-status | 1 + hooks/upgrade-charm | 2 +- unit_tests/test_status.py | 95 +++++++++++++++ unit_tests/test_utils.py | 121 ++++++++++++++++++++ 18 files changed, 231 insertions(+), 14 deletions(-) rename hooks/{hooks.py => ceph_hooks.py} (100%) create mode 120000 hooks/update-status create mode 100644 unit_tests/test_status.py create mode 100644 unit_tests/test_utils.py diff --git a/hooks/hooks.py b/hooks/ceph_hooks.py similarity index 100% rename from hooks/hooks.py rename to hooks/ceph_hooks.py diff --git a/hooks/client-relation-changed b/hooks/client-relation-changed index 9416ca6a..52d96630 120000 --- a/hooks/client-relation-changed +++ b/hooks/client-relation-changed @@ -1 +1 @@ -hooks.py \ No newline at end of file +ceph_hooks.py \ No newline at end of file diff --git a/hooks/client-relation-joined b/hooks/client-relation-joined index 9416ca6a..52d96630 120000 --- a/hooks/client-relation-joined +++ b/hooks/client-relation-joined @@ -1 +1 @@ -hooks.py \ No newline at end of file +ceph_hooks.py \ No newline at end of file diff --git a/hooks/config-changed b/hooks/config-changed index 9416ca6a..52d96630 120000 --- a/hooks/config-changed +++ b/hooks/config-changed @@ -1 +1 @@ -hooks.py \ No newline at end of file +ceph_hooks.py \ No newline at end of file diff --git a/hooks/install.real b/hooks/install.real index 9416ca6a..52d96630 120000 --- a/hooks/install.real +++ b/hooks/install.real @@ -1 +1 @@ -hooks.py \ No newline at end of file +ceph_hooks.py \ No newline at end of file diff --git a/hooks/mon-relation-changed b/hooks/mon-relation-changed index 9416ca6a..52d96630 120000 --- a/hooks/mon-relation-changed +++ b/hooks/mon-relation-changed @@ -1 +1 @@ -hooks.py \ No newline at end of file +ceph_hooks.py \ No newline at end of file diff --git a/hooks/mon-relation-departed b/hooks/mon-relation-departed index 9416ca6a..52d96630 120000 --- a/hooks/mon-relation-departed +++ b/hooks/mon-relation-departed @@ -1 +1 @@ -hooks.py \ No newline at end of file +ceph_hooks.py \ No newline at end of file diff --git a/hooks/mon-relation-joined b/hooks/mon-relation-joined index 9416ca6a..52d96630 120000 --- a/hooks/mon-relation-joined +++ b/hooks/mon-relation-joined @@ -1 +1 @@ -hooks.py \ No newline at end of file +ceph_hooks.py \ No newline at end of file diff --git a/hooks/nrpe-external-master-relation-changed b/hooks/nrpe-external-master-relation-changed index 9416ca6a..52d96630 120000 --- a/hooks/nrpe-external-master-relation-changed +++ b/hooks/nrpe-external-master-relation-changed @@ -1 +1 @@ -hooks.py \ No newline at end of file +ceph_hooks.py \ No newline at end of file diff --git a/hooks/nrpe-external-master-relation-joined b/hooks/nrpe-external-master-relation-joined index 9416ca6a..52d96630 120000 --- a/hooks/nrpe-external-master-relation-joined +++ b/hooks/nrpe-external-master-relation-joined @@ -1 +1 @@ -hooks.py \ No newline at end of file +ceph_hooks.py \ No newline at end of file diff --git a/hooks/osd-relation-joined b/hooks/osd-relation-joined index 9416ca6a..52d96630 120000 --- a/hooks/osd-relation-joined +++ b/hooks/osd-relation-joined @@ -1 +1 @@ -hooks.py \ No newline at end of file +ceph_hooks.py \ No newline at end of file diff --git a/hooks/radosgw-relation-joined b/hooks/radosgw-relation-joined index 9416ca6a..52d96630 120000 --- a/hooks/radosgw-relation-joined +++ b/hooks/radosgw-relation-joined @@ -1 +1 @@ -hooks.py \ No newline at end of file +ceph_hooks.py \ No newline at end of file diff --git a/hooks/start b/hooks/start index 9416ca6a..52d96630 120000 --- a/hooks/start +++ b/hooks/start @@ -1 +1 @@ -hooks.py \ No newline at end of file +ceph_hooks.py \ No newline at end of file diff --git a/hooks/stop b/hooks/stop index 9416ca6a..52d96630 120000 --- a/hooks/stop +++ b/hooks/stop @@ -1 +1 @@ -hooks.py \ No newline at end of file +ceph_hooks.py \ No newline at end of file diff --git a/hooks/update-status b/hooks/update-status new file mode 120000 index 00000000..52d96630 --- /dev/null +++ b/hooks/update-status @@ -0,0 +1 @@ +ceph_hooks.py \ No newline at end of file diff --git a/hooks/upgrade-charm b/hooks/upgrade-charm index 9416ca6a..52d96630 120000 --- a/hooks/upgrade-charm +++ b/hooks/upgrade-charm @@ -1 +1 @@ -hooks.py \ No newline at end of file +ceph_hooks.py \ No newline at end of file diff --git a/unit_tests/test_status.py b/unit_tests/test_status.py new file mode 100644 index 00000000..bd8b0241 --- /dev/null +++ b/unit_tests/test_status.py @@ -0,0 +1,95 @@ +import mock +import test_utils + +with mock.patch('utils.get_unit_hostname'): + 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()) \ No newline at end of file diff --git a/unit_tests/test_utils.py b/unit_tests/test_utils.py new file mode 100644 index 00000000..663a0488 --- /dev/null +++ b/unit_tests/test_utils.py @@ -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 From 4e804c420cf4c4a657ec05157050226b287d0e17 Mon Sep 17 00:00:00 2001 From: James Page Date: Tue, 6 Oct 2015 21:02:31 +0100 Subject: [PATCH 4/6] Tidy lint --- unit_tests/test_status.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/unit_tests/test_status.py b/unit_tests/test_status.py index bd8b0241..6fc136a5 100644 --- a/unit_tests/test_status.py +++ b/unit_tests/test_status.py @@ -92,4 +92,5 @@ class ServiceStatusTestCase(test_utils.CharmTestCase): self.assertEquals({'ceph-mon1': True, 'ceph-mon2': True, 'ceph-mon3': True}, - hooks.get_peer_units()) \ No newline at end of file + hooks.get_peer_units()) + From ba049165c93b4c5bcf58c78a2fb996c183b5aac2 Mon Sep 17 00:00:00 2001 From: James Page Date: Tue, 6 Oct 2015 21:02:54 +0100 Subject: [PATCH 5/6] Tidy harder --- unit_tests/test_status.py | 1 - 1 file changed, 1 deletion(-) diff --git a/unit_tests/test_status.py b/unit_tests/test_status.py index 6fc136a5..973f2b6e 100644 --- a/unit_tests/test_status.py +++ b/unit_tests/test_status.py @@ -93,4 +93,3 @@ class ServiceStatusTestCase(test_utils.CharmTestCase): 'ceph-mon2': True, 'ceph-mon3': True}, hooks.get_peer_units()) - From a1fac3ad6bead004de187e74077b4c137a35b89e Mon Sep 17 00:00:00 2001 From: James Page Date: Tue, 6 Oct 2015 21:16:42 +0100 Subject: [PATCH 6/6] Tidy imports --- unit_tests/test_status.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/unit_tests/test_status.py b/unit_tests/test_status.py index 973f2b6e..c4330185 100644 --- a/unit_tests/test_status.py +++ b/unit_tests/test_status.py @@ -1,8 +1,7 @@ import mock import test_utils -with mock.patch('utils.get_unit_hostname'): - import ceph_hooks as hooks +import ceph_hooks as hooks TO_PATCH = [ 'status_set',