From b1829e983ae3b56e016cf9de437fde35b1f6a83f Mon Sep 17 00:00:00 2001 From: James Page Date: Thu, 10 May 2018 11:48:38 +0100 Subject: [PATCH] Tidy ceph backend configuration Drop generation of upstart override file and /etc/environment and scrub any existing charm configuration in these locations from an existing install. These where required way back in the dawn of time when ceph support was alpha/beta in cinder. Provide backend specific configuration file path, allowing multiple ceph clusters to be used with a single cinder application. Change-Id: I8a097e4de1c5c980f118a587a1a64792fad2fa05 Closes-Bug: 1769196 --- hooks/cinder_contexts.py | 7 +++++++ hooks/cinder_hooks.py | 5 ++--- hooks/cinder_utils.py | 31 +++++++++++++++++++++--------- tests/basic_deployment.py | 5 ++++- unit_tests/test_cinder_contexts.py | 5 +++++ unit_tests/test_cinder_hooks.py | 5 ++--- 6 files changed, 42 insertions(+), 16 deletions(-) diff --git a/hooks/cinder_contexts.py b/hooks/cinder_contexts.py index d2bbdb0..cd8c999 100644 --- a/hooks/cinder_contexts.py +++ b/hooks/cinder_contexts.py @@ -27,6 +27,12 @@ from charmhelpers.contrib.openstack.utils import ( CompareOpenStackReleases, ) +CHARM_CEPH_CONF = '/var/lib/charm/{}/ceph.conf' + + +def ceph_config_file(): + return CHARM_CEPH_CONF.format(service_name()) + class CephSubordinateContext(OSContextGenerator): interfaces = ['ceph-cinder'] @@ -54,6 +60,7 @@ class CephSubordinateContext(OSContextGenerator): ('rbd_pool', service), ('rbd_user', service), ('rbd_secret_uuid', leader_get('secret-uuid')), + ('rbd_ceph_conf', ceph_config_file()), ] } } diff --git a/hooks/cinder_hooks.py b/hooks/cinder_hooks.py index b6a08fe..74c223e 100755 --- a/hooks/cinder_hooks.py +++ b/hooks/cinder_hooks.py @@ -22,7 +22,7 @@ import uuid from cinder_utils import ( register_configs, restart_map, - set_ceph_env_variables, + scrub_old_style_ceph, PACKAGES, REQUIRED_INTERFACES, VERSION_PACKAGE, @@ -122,7 +122,6 @@ def ceph_changed(): if is_request_complete(get_ceph_request()): log('Request complete') CONFIGS.write_all() - set_ceph_env_variables(service=service) for rid in relation_ids('storage-backend'): storage_backend(rid) for r_id in relation_ids('ceph-access'): @@ -180,9 +179,9 @@ def storage_backend_changed(): def upgrade_charm(): if 'ceph' in CONFIGS.complete_contexts(): CONFIGS.write_all() - set_ceph_env_variables(service=service_name()) for rid in relation_ids('storage-backend'): storage_backend(rid) + scrub_old_style_ceph() @hooks.hook('leader-settings-changed') diff --git a/hooks/cinder_utils.py b/hooks/cinder_utils.py index e5330de..e8c4701 100644 --- a/hooks/cinder_utils.py +++ b/hooks/cinder_utils.py @@ -12,8 +12,14 @@ # See the License for the specific language governing permissions and # limitations under the License. +from __future__ import print_function + import os +import re + from collections import OrderedDict +from tempfile import NamedTemporaryFile + from charmhelpers.core.hookenv import ( relation_ids, @@ -115,12 +121,19 @@ def restart_map(): return OrderedDict(_map) -def set_ceph_env_variables(service): - # XXX: Horrid kludge to make cinder-volume use - # a different ceph username than admin - env = open('/etc/environment', 'r').read() - if 'CEPH_ARGS' not in env: - with open('/etc/environment', 'a') as out: - out.write('CEPH_ARGS="--id %s"\n' % service) - with open('/etc/init/cinder-volume.override', 'w') as out: - out.write('env CEPH_ARGS="--id %s"\n' % service) +def scrub_old_style_ceph(): + """Purge any legacy ceph configuration from install""" + # NOTE: purge old override file - no longer needed + if os.path.exists('/etc/init/cinder-volume.override'): + os.remove('/etc/init/cinder-volume.override') + # NOTE: purge any CEPH_ARGS data from /etc/environment + env_file = '/etc/environment' + ceph_match = re.compile("^CEPH_ARGS.*").search + with open(env_file, 'r') as input_file: + with NamedTemporaryFile(mode='w', + delete=False, + dir=os.path.dirname(env_file)) as outfile: + for line in input_file: + if not ceph_match(line): + print(line, end='', file=outfile) + os.rename(outfile.name, input_file.name) diff --git a/tests/basic_deployment.py b/tests/basic_deployment.py index e849e0a..85c70dc 100644 --- a/tests/basic_deployment.py +++ b/tests/basic_deployment.py @@ -427,6 +427,8 @@ class CinderCephBasicDeployment(OpenStackAmuletDeployment): ["rbd_pool", "cinder-ceph"], ["rbd_user", "cinder-ceph"], ["rbd_secret_uuid", backend_uuid], + ['rbd_ceph_conf', + '/var/lib/charm/cinder-ceph/ceph.conf'], ] } } @@ -584,7 +586,8 @@ class CinderCephBasicDeployment(OpenStackAmuletDeployment): 'volume_backend_name': 'cinder-ceph', 'volume_driver': 'cinder.volume.drivers.rbd.RBDDriver', 'rbd_pool': 'cinder-ceph', - 'rbd_user': 'cinder-ceph' + 'rbd_user': 'cinder-ceph', + 'rbd_ceph_conf': '/var/lib/charm/cinder-ceph/ceph.conf', }, } if self._get_openstack_release() < self.xenial_ocata: diff --git a/unit_tests/test_cinder_contexts.py b/unit_tests/test_cinder_contexts.py index 1c87a3c..b6bae55 100644 --- a/unit_tests/test_cinder_contexts.py +++ b/unit_tests/test_cinder_contexts.py @@ -31,6 +31,7 @@ class TestCinderContext(CharmTestCase): def setUp(self): super(TestCinderContext, self).setUp(contexts, TO_PATCH) self.leader_get.return_value = 'libvirt-uuid' + self.maxDiff = None def test_ceph_not_related(self): self.is_relation_made.return_value = False @@ -53,6 +54,8 @@ class TestCinderContext(CharmTestCase): ('rbd_pool', service), ('rbd_user', service), ('rbd_secret_uuid', 'libvirt-uuid'), + ('rbd_ceph_conf', + '/var/lib/charm/mycinder/ceph.conf') ] } } @@ -75,6 +78,8 @@ class TestCinderContext(CharmTestCase): ('rbd_pool', service), ('rbd_user', service), ('rbd_secret_uuid', 'libvirt-uuid'), + ('rbd_ceph_conf', + '/var/lib/charm/mycinder/ceph.conf') ] } } diff --git a/unit_tests/test_cinder_hooks.py b/unit_tests/test_cinder_hooks.py index 8d95aa8..b4edf35 100644 --- a/unit_tests/test_cinder_hooks.py +++ b/unit_tests/test_cinder_hooks.py @@ -31,7 +31,7 @@ TO_PATCH = [ 'ensure_ceph_keyring', 'register_configs', 'restart_map', - 'set_ceph_env_variables', + 'scrub_old_style_ceph', 'is_request_complete', 'send_request_if_needed', 'CONFIGS', @@ -98,7 +98,6 @@ class TestCinderHooks(CharmTestCase): user='cinder', group='cinder') self.assertTrue(self.CONFIGS.write_all.called) - self.set_ceph_env_variables.assert_called_with(service='cinder') @patch.object(hooks, 'get_ceph_request') @patch('charmhelpers.core.hookenv.config') @@ -178,7 +177,7 @@ class TestCinderHooks(CharmTestCase): hooks.hooks.execute(['hooks/upgrade-charm']) _storage_backend.assert_called_with('ceph:1') assert self.CONFIGS.write_all.called - assert self.set_ceph_env_variables.called + self.scrub_old_style_ceph.assert_called_once_with() @patch('charmhelpers.core.hookenv.config') @patch.object(hooks, 'storage_backend')