From 8eb140b15c167aa51c1cbc220f418845b42d82c8 Mon Sep 17 00:00:00 2001 From: James Page Date: Tue, 6 Oct 2015 11:44:28 +0100 Subject: [PATCH 1/4] Add basic status support --- hooks/ceph.py | 14 +++++++++++++- hooks/hooks.py | 29 ++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/hooks/ceph.py b/hooks/ceph.py index ad2a2d51..f9448db2 100644 --- a/hooks/ceph.py +++ b/hooks/ceph.py @@ -18,7 +18,8 @@ from charmhelpers.core.host import ( ) from charmhelpers.core.hookenv import ( log, - ERROR, WARNING + ERROR, WARNING, + status_set, ) from charmhelpers.contrib.storage.linux.utils import ( zap_disk, @@ -333,6 +334,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: @@ -382,3 +384,13 @@ def osdize_dir(path): def filesystem_mounted(fs): return subprocess.call(['grep', '-wqs', fs, '/proc/mounts']) == 0 + + +def get_running_osds(): + '''Returns a list of the pids of the current running OSD daemons''' + cmd = ['pgrep', 'ceph-osd'] + try: + result = subprocess.check_output(cmd) + return result.split() + except subprocess.CalledProcessError: + return [] diff --git a/hooks/hooks.py b/hooks/hooks.py index 9d9a3d4c..7ef5d796 100755 --- a/hooks/hooks.py +++ b/hooks/hooks.py @@ -22,7 +22,8 @@ from charmhelpers.core.hookenv import ( relation_get, Hooks, UnregisteredHookError, - service_name + service_name, + status_set, ) from charmhelpers.core.host import ( umount, @@ -227,8 +228,34 @@ def update_nrpe_config(): nrpe_setup.write() +def assess_status(): + '''Assess status of current unit''' + # Check for mon relation + if len(relation_ids('mon')) < 1: + status_set('blocked', 'Missing relation: monitor') + return + + # Check for monitors with presented addresses + # Check for bootstrap key presentation + monitors = get_mon_hosts() + if len(monitors) < 1 or not get_conf('osd_bootstrap_key'): + status_set('waiting', 'Incomplete relation: monitor') + return + + # Check for OSD device creation parity i.e. at least some devices + # must have been presented and used for this charm to be operational + running_osds = ceph.get_running_osds() + if not running_osds: + status_set('blocked', + 'No block devices detected using current configuration') + else: + status_set('active', + 'Unit is ready ({} OSD)'.format(len(running_osds))) + + if __name__ == '__main__': try: hooks.execute(sys.argv) except UnregisteredHookError as e: log('Unknown hook {} - skipping.'.format(e)) + assess_status() From f66f51c7a3c8a435d7eb67e4a9e11ef636e5cf20 Mon Sep 17 00:00:00 2001 From: James Page Date: Tue, 6 Oct 2015 21:15:38 +0100 Subject: [PATCH 2/4] Add unit tests for service status --- .coveragerc | 7 ++ .pydevproject | 1 + Makefile | 9 +- hooks/{hooks.py => ceph_hooks.py} | 0 hooks/config-changed | 2 +- hooks/install.real | 2 +- hooks/mon-relation-changed | 2 +- hooks/mon-relation-departed | 2 +- hooks/nrpe-external-master-relation-changed | 2 +- hooks/nrpe-external-master-relation-joined | 2 +- hooks/start | 2 +- hooks/stop | 2 +- hooks/update-status | 1 + hooks/upgrade-charm | 2 +- setup.cfg | 5 + unit_tests/__init__.py | 2 + unit_tests/test_status.py | 56 +++++++++ unit_tests/test_utils.py | 121 ++++++++++++++++++++ 18 files changed, 209 insertions(+), 11 deletions(-) create mode 100644 .coveragerc rename hooks/{hooks.py => ceph_hooks.py} (100%) create mode 120000 hooks/update-status create mode 100644 setup.cfg create mode 100644 unit_tests/test_status.py create mode 100644 unit_tests/test_utils.py diff --git a/.coveragerc b/.coveragerc new file mode 100644 index 00000000..7f7b5be3 --- /dev/null +++ b/.coveragerc @@ -0,0 +1,7 @@ +[report] +# Regexes for lines to exclude from consideration +exclude_lines = + if __name__ == .__main__.: +include= + hooks/hooks.py + hooks/ceph*.py diff --git a/.pydevproject b/.pydevproject index bb30cc40..be2105d0 100644 --- a/.pydevproject +++ b/.pydevproject @@ -4,5 +4,6 @@ Default /ceph-osd/hooks +/ceph-osd/unit_tests diff --git a/Makefile b/Makefile index de395a59..306c8444 100644 --- a/Makefile +++ b/Makefile @@ -3,9 +3,14 @@ PYTHON := /usr/bin/env python lint: @flake8 --exclude hooks/charmhelpers,tests/charmhelpers \ - hooks tests unit_tests + hooks tests unit_tests @charm proof +test: + @# Bundletester expects unit tests here. + @echo Starting unit tests... + @$(PYTHON) /usr/bin/nosetests --nologcapture --with-coverage unit_tests + functional_test: @echo Starting Amulet tests... @juju test -v -p AMULET_HTTP_PROXY,AMULET_OS_VIP --timeout 2700 @@ -13,7 +18,7 @@ functional_test: bin/charm_helpers_sync.py: @mkdir -p bin @bzr cat lp:charm-helpers/tools/charm_helpers_sync/charm_helpers_sync.py \ - > bin/charm_helpers_sync.py + > bin/charm_helpers_sync.py sync: bin/charm_helpers_sync.py $(PYTHON) bin/charm_helpers_sync.py -c charm-helpers-hooks.yaml 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/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/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/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/setup.cfg b/setup.cfg new file mode 100644 index 00000000..37083b62 --- /dev/null +++ b/setup.cfg @@ -0,0 +1,5 @@ +[nosetests] +verbosity=2 +with-coverage=1 +cover-erase=1 +cover-package=hooks diff --git a/unit_tests/__init__.py b/unit_tests/__init__.py index e69de29b..f80aab3d 100644 --- a/unit_tests/__init__.py +++ b/unit_tests/__init__.py @@ -0,0 +1,2 @@ +import sys +sys.path.append('hooks') diff --git a/unit_tests/test_status.py b/unit_tests/test_status.py new file mode 100644 index 00000000..0cededfc --- /dev/null +++ b/unit_tests/test_status.py @@ -0,0 +1,56 @@ +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', + 'get_conf', +] + +CEPH_MONS = [ + 'ceph/0', + 'ceph/1', + 'ceph/2', +] + +class ServiceStatusTestCase(test_utils.CharmTestCase): + + def setUp(self): + super(ServiceStatusTestCase, self).setUp(hooks, TO_PATCH) + self.config.side_effect = self.test_config.get + + def test_assess_status_no_monitor_relation(self): + self.relation_ids.return_value = [] + hooks.assess_status() + self.status_set.assert_called_with('blocked', mock.ANY) + + def test_assess_status_monitor_relation_incomplete(self): + self.relation_ids.return_value = ['mon:1'] + self.related_units.return_value = CEPH_MONS + self.get_conf.return_value = None + hooks.assess_status() + self.status_set.assert_called_with('waiting', mock.ANY) + + def test_assess_status_monitor_complete_no_disks(self): + self.relation_ids.return_value = ['mon:1'] + self.related_units.return_value = CEPH_MONS + self.get_conf.return_value = 'monitor-bootstrap-key' + self.ceph.get_running_osds.return_value = [] + hooks.assess_status() + self.status_set.assert_called_with('blocked', mock.ANY) + + def test_assess_status_monitor_complete_disks(self): + self.relation_ids.return_value = ['mon:1'] + self.related_units.return_value = CEPH_MONS + self.get_conf.return_value = 'monitor-bootstrap-key' + self.ceph.get_running_osds.return_value = ['12345', + '67890'] + hooks.assess_status() + self.status_set.assert_called_with('active', mock.ANY) 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 ca5206397e3ec002a7ccf9529032c2cbaee0f2b0 Mon Sep 17 00:00:00 2001 From: James Page Date: Tue, 6 Oct 2015 21:16:21 +0100 Subject: [PATCH 3/4] Tidy import of hooks --- 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 0cededfc..1b5ab315 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', From 36481d284e0938c6a7982414fb1848345231b6ad Mon Sep 17 00:00:00 2001 From: James Page Date: Thu, 8 Oct 2015 04:41:59 -0700 Subject: [PATCH 4/4] Tidy lint --- unit_tests/test_status.py | 1 + 1 file changed, 1 insertion(+) diff --git a/unit_tests/test_status.py b/unit_tests/test_status.py index 1b5ab315..f4342d28 100644 --- a/unit_tests/test_status.py +++ b/unit_tests/test_status.py @@ -19,6 +19,7 @@ CEPH_MONS = [ 'ceph/2', ] + class ServiceStatusTestCase(test_utils.CharmTestCase): def setUp(self):