From 2858f1b02d6e5216234ff16a3feba34dae337538 Mon Sep 17 00:00:00 2001 From: James Page Date: Mon, 7 Jan 2019 17:18:07 +0000 Subject: [PATCH] Switch to using systemd units for radosgw Switch to using systemd configurations to manage radosgw instances; the radosgw init script is obsolete and will be removed at some point in time, and the newer style of managing radosgw daemons is inline with current best-practice. This changeset also changes the way cephx keys are issues; before all rgw instances shared a key, now a key is issued per host. The key is named 'rgw.`hostname`' to identify the application and host using the key. Existing deployments using the radosgw init script will be switched to use the new systemd named units; this occurs once the new key for the unit has been presented by the ceph-mon cluster over the mon relation. A small period of outage will occur as the radosgw init based daemon is stopped and disabled prior to the start of the new systemd based radosgw unit. This commit also includes a resync for charmhelpers to pickup support for '@' in NRPE service check names. Change-Id: Ic0d634e619185931633712cb3e3685051a28749d Depends-On: I289b75a2935184817b424c5eceead16235c3f53b Closes-Bug: 1808140 --- hooks/ceph.py | 38 +++- hooks/ceph_radosgw_context.py | 8 +- .../contrib/storage/linux/ceph.py | 27 ++- hooks/hooks.py | 168 ++++++++++-------- hooks/utils.py | 49 ++++- templates/ceph.conf | 10 +- tests/basic_deployment.py | 24 --- unit_tests/test_ceph.py | 8 +- unit_tests/test_ceph_radosgw_context.py | 18 +- unit_tests/test_ceph_radosgw_utils.py | 72 ++++++++ unit_tests/test_hooks.py | 67 ++++--- 11 files changed, 344 insertions(+), 145 deletions(-) diff --git a/hooks/ceph.py b/hooks/ceph.py index 41a2a531..f5ac5963 100644 --- a/hooks/ceph.py +++ b/hooks/ceph.py @@ -21,23 +21,51 @@ from utils import get_pkg_version from charmhelpers.core.hookenv import ( config, ) + +from charmhelpers.core.host import ( + mkdir +) from charmhelpers.contrib.storage.linux.ceph import ( CephBrokerRq, ) -_radosgw_keyring = "/etc/ceph/keyring.rados.gateway" +CEPH_DIR = '/etc/ceph' +CEPH_RADOSGW_DIR = '/var/lib/ceph/radosgw' +_radosgw_keyring = "keyring.rados.gateway" -def import_radosgw_key(key): - if not os.path.exists(_radosgw_keyring): +def import_radosgw_key(key, name=None): + if name: + keyring_path = os.path.join(CEPH_RADOSGW_DIR, + 'ceph-{}'.format(name), + 'keyring') + owner = group = 'ceph' + else: + keyring_path = os.path.join(CEPH_DIR, _radosgw_keyring) + owner = group = 'root' + + if not os.path.exists(keyring_path): + mkdir(path=os.path.dirname(keyring_path), + owner=owner, group=group, perms=0o750) cmd = [ 'ceph-authtool', - _radosgw_keyring, + keyring_path, '--create-keyring', - '--name=client.radosgw.gateway', + '--name=client.{}'.format( + name or 'radosgw.gateway' + ), '--add-key={}'.format(key) ] subprocess.check_call(cmd) + cmd = [ + 'chown', + '{}:{}'.format(owner, group), + keyring_path + ] + subprocess.check_call(cmd) + return True + + return False def get_create_rgw_pools_rq(prefix=None): diff --git a/hooks/ceph_radosgw_context.py b/hooks/ceph_radosgw_context.py index 5c0654a1..aec22968 100644 --- a/hooks/ceph_radosgw_context.py +++ b/hooks/ceph_radosgw_context.py @@ -144,6 +144,10 @@ class MonContext(context.CephContext): def __call__(self): if not relation_ids('mon'): return {} + + host = socket.gethostname() + systemd_rgw = False + mon_hosts = [] auths = [] @@ -161,6 +165,8 @@ class MonContext(context.CephContext): ceph_addr = format_ipv6_addr(ceph_addr) or ceph_addr if ceph_addr: mon_hosts.append(ceph_addr) + if relation_get('rgw.{}_key'.format(host), rid=rid, unit=unit): + systemd_rgw = True if len(set(auths)) != 1: e = ("Inconsistent or absent auth returned by mon units. Setting " @@ -172,7 +178,6 @@ class MonContext(context.CephContext): # /etc/init.d/radosgw mandates that a dns name is used for this # parameter so ensure that address is resolvable - host = socket.gethostname() if config('prefer-ipv6'): ensure_host_resolvable_v6(host) @@ -186,6 +191,7 @@ class MonContext(context.CephContext): 'mon_hosts': ' '.join(mon_hosts), 'hostname': host, 'old_auth': cmp_pkgrevno('radosgw', "0.51") < 0, + 'systemd_rgw': systemd_rgw, 'use_syslog': str(config('use-syslog')).lower(), 'loglevel': config('loglevel'), 'port': port, diff --git a/hooks/charmhelpers/contrib/storage/linux/ceph.py b/hooks/charmhelpers/contrib/storage/linux/ceph.py index 76828201..63c93044 100644 --- a/hooks/charmhelpers/contrib/storage/linux/ceph.py +++ b/hooks/charmhelpers/contrib/storage/linux/ceph.py @@ -856,12 +856,22 @@ def _keyring_path(service): return KEYRING.format(service) -def create_keyring(service, key): - """Create a new Ceph keyring containing key.""" +def add_key(service, key): + """ + Add a key to a keyring. + + Creates the keyring if it doesn't already exist. + + Logs and returns if the key is already in the keyring. + """ keyring = _keyring_path(service) if os.path.exists(keyring): - log('Ceph keyring exists at %s.' % keyring, level=WARNING) - return + with open(keyring, 'r') as ring: + if key in ring.read(): + log('Ceph keyring exists at %s and has not changed.' % keyring, + level=DEBUG) + return + log('Updating existing keyring %s.' % keyring, level=DEBUG) cmd = ['ceph-authtool', keyring, '--create-keyring', '--name=client.{}'.format(service), '--add-key={}'.format(key)] @@ -869,6 +879,11 @@ def create_keyring(service, key): log('Created new ceph keyring at %s.' % keyring, level=DEBUG) +def create_keyring(service, key): + """Deprecated. Please use the more accurately named 'add_key'""" + return add_key(service, key) + + def delete_keyring(service): """Delete an existing Ceph keyring.""" keyring = _keyring_path(service) @@ -905,7 +920,7 @@ def get_ceph_nodes(relation='ceph'): def configure(service, key, auth, use_syslog): """Perform basic configuration of Ceph.""" - create_keyring(service, key) + add_key(service, key) create_key_file(service, key) hosts = get_ceph_nodes() with open('/etc/ceph/ceph.conf', 'w') as ceph_conf: @@ -1068,7 +1083,7 @@ def ensure_ceph_keyring(service, user=None, group=None, if not key: return False - create_keyring(service=service, key=key) + add_key(service=service, key=key) keyring = _keyring_path(service) if user and group: check_call(['chown', '%s.%s' % (user, group), keyring]) diff --git a/hooks/hooks.py b/hooks/hooks.py index 8481032d..8c6fef88 100755 --- a/hooks/hooks.py +++ b/hooks/hooks.py @@ -17,6 +17,7 @@ import os import subprocess import sys +import socket import ceph @@ -45,6 +46,9 @@ from charmhelpers.core.host import ( cmp_pkgrevno, is_container, service_reload, + service_restart, + service_stop, + service, ) from charmhelpers.contrib.network.ip import ( get_relation_ip, @@ -84,6 +88,10 @@ from utils import ( disable_unused_apache_sites, pause_unit_helper, resume_unit_helper, + restart_map, + service_name, + systemd_based_radosgw, + request_per_unit_key, ) from charmhelpers.contrib.charmsupport import nrpe from charmhelpers.contrib.hardening.harden import harden @@ -137,58 +145,83 @@ def install(): @hooks.hook('config-changed') -@restart_on_change({'/etc/ceph/ceph.conf': ['radosgw'], - '/etc/haproxy/haproxy.cfg': ['haproxy']}) @harden() def config_changed(): - # if we are paused, delay doing any config changed hooks. - # It is forced on the resume. - if is_unit_paused_set(): - log("Unit is pause or upgrading. Skipping config_changed", "WARN") - return + @restart_on_change(restart_map()) + def _config_changed(): + # if we are paused, delay doing any config changed hooks. + # It is forced on the resume. + if is_unit_paused_set(): + log("Unit is pause or upgrading. Skipping config_changed", "WARN") + return - install_packages() - disable_unused_apache_sites() + install_packages() + disable_unused_apache_sites() - if config('prefer-ipv6'): - status_set('maintenance', 'configuring ipv6') - setup_ipv6() + if config('prefer-ipv6'): + status_set('maintenance', 'configuring ipv6') + setup_ipv6() - for r_id in relation_ids('identity-service'): - identity_changed(relid=r_id) + for r_id in relation_ids('identity-service'): + identity_changed(relid=r_id) - for r_id in relation_ids('cluster'): - cluster_joined(rid=r_id) + for r_id in relation_ids('cluster'): + cluster_joined(rid=r_id) - # NOTE(jamespage): Re-exec mon relation for any changes to - # enable ceph pool permissions restrictions - for r_id in relation_ids('mon'): - for unit in related_units(r_id): - mon_relation(r_id, unit) + # NOTE(jamespage): Re-exec mon relation for any changes to + # enable ceph pool permissions restrictions + for r_id in relation_ids('mon'): + for unit in related_units(r_id): + mon_relation(r_id, unit) - CONFIGS.write_all() - configure_https() + CONFIGS.write_all() + configure_https() - update_nrpe_config() + update_nrpe_config() + + open_port(port=config('port')) + _config_changed() @hooks.hook('mon-relation-departed', 'mon-relation-changed') -@restart_on_change({'/etc/ceph/ceph.conf': ['radosgw']}) def mon_relation(rid=None, unit=None): - rq = ceph.get_create_rgw_pools_rq( - prefix=config('pool-prefix')) - if is_request_complete(rq, relation='mon'): - log('Broker request complete', level=DEBUG) - CONFIGS.write_all() - key = relation_get(attribute='radosgw_key', - rid=rid, unit=unit) - if key: - ceph.import_radosgw_key(key) - if not is_unit_paused_set(): - restart() # TODO figure out a better way todo this - else: - send_request_if_needed(rq, relation='mon') + @restart_on_change(restart_map()) + def _mon_relation(): + key_name = 'rgw.{}'.format(socket.gethostname()) + if request_per_unit_key(): + relation_set(relation_id=rid, + key_name=key_name) + rq = ceph.get_create_rgw_pools_rq( + prefix=config('pool-prefix')) + if is_request_complete(rq, relation='mon'): + log('Broker request complete', level=DEBUG) + CONFIGS.write_all() + # New style per unit keys + key = relation_get(attribute='{}_key'.format(key_name), + rid=rid, unit=unit) + if not key: + # Fallback to old style global key + key = relation_get(attribute='radosgw_key', + rid=rid, unit=unit) + key_name = None + + if key: + new_keyring = ceph.import_radosgw_key(key, + name=key_name) + # NOTE(jamespage): + # Deal with switch from radosgw init script to + # systemd named units for radosgw instances by + # stopping and disabling the radosgw unit + if systemd_based_radosgw(): + service_stop('radosgw') + service('disable', 'radosgw') + if not is_unit_paused_set() and new_keyring: + service('enable', service_name()) + service_restart(service_name()) + else: + send_request_if_needed(rq, relation='mon') + _mon_relation() @hooks.hook('gateway-relation-joined') @@ -197,21 +230,6 @@ def gateway_relation(): port=config('port')) -def start(): - subprocess.call(['service', 'radosgw', 'start']) - open_port(port=config('port')) - - -def stop(): - subprocess.call(['service', 'radosgw', 'stop']) - open_port(port=config('port')) - - -def restart(): - subprocess.call(['service', 'radosgw', 'restart']) - open_port(port=config('port')) - - @hooks.hook('identity-service-relation-joined') def identity_joined(relid=None): if cmp_pkgrevno('radosgw', '0.55') < 0: @@ -233,38 +251,42 @@ def identity_joined(relid=None): @hooks.hook('identity-service-relation-changed') -@restart_on_change({'/etc/ceph/ceph.conf': ['radosgw']}) def identity_changed(relid=None): - identity_joined(relid) - CONFIGS.write_all() - if not is_unit_paused_set(): - restart() - configure_https() + @restart_on_change(restart_map()) + def _identity_changed(): + identity_joined(relid) + CONFIGS.write_all() + configure_https() + _identity_changed() @hooks.hook('cluster-relation-joined') -@restart_on_change({'/etc/haproxy/haproxy.cfg': ['haproxy']}) def cluster_joined(rid=None): - settings = {} + @restart_on_change(restart_map()) + def _cluster_joined(): + settings = {} - for addr_type in ADDRESS_TYPES: - address = get_relation_ip( - addr_type, - cidr_network=config('os-{}-network'.format(addr_type))) - if address: - settings['{}-address'.format(addr_type)] = address + for addr_type in ADDRESS_TYPES: + address = get_relation_ip( + addr_type, + cidr_network=config('os-{}-network'.format(addr_type))) + if address: + settings['{}-address'.format(addr_type)] = address - settings['private-address'] = get_relation_ip('cluster') + settings['private-address'] = get_relation_ip('cluster') - relation_set(relation_id=rid, relation_settings=settings) + relation_set(relation_id=rid, relation_settings=settings) + _cluster_joined() @hooks.hook('cluster-relation-changed') -@restart_on_change({'/etc/haproxy/haproxy.cfg': ['haproxy']}) def cluster_changed(): - CONFIGS.write_all() - for r_id in relation_ids('identity-service'): - identity_joined(relid=r_id) + @restart_on_change(restart_map()) + def _cluster_changed(): + CONFIGS.write_all() + for r_id in relation_ids('identity-service'): + identity_joined(relid=r_id) + _cluster_changed() @hooks.hook('ha-relation-joined') diff --git a/hooks/utils.py b/hooks/utils.py index c79e0022..b6622adc 100644 --- a/hooks/utils.py +++ b/hooks/utils.py @@ -14,6 +14,7 @@ import os import re +import socket import subprocess import sys @@ -57,6 +58,7 @@ from charmhelpers.core.host import ( lsb_release, mkdir, CompareHostReleases, + init_is_systemd, ) from charmhelpers.fetch import ( apt_cache, @@ -124,7 +126,7 @@ BASE_RESOURCE_MAP = OrderedDict([ }), (CEPH_CONF, { 'contexts': [ceph_radosgw_context.MonContext()], - 'services': ['radosgw'], + 'services': [], }), (APACHE_SITE_CONF, { 'contexts': [ceph_radosgw_context.ApacheSSLContext()], @@ -152,14 +154,25 @@ def resource_map(): """ resource_map = deepcopy(BASE_RESOURCE_MAP) - if os.path.exists('/etc/apache2/conf-available'): + if not https(): resource_map.pop(APACHE_SITE_CONF) - else: resource_map.pop(APACHE_SITE_24_CONF) + else: + if os.path.exists('/etc/apache2/conf-available'): + resource_map.pop(APACHE_SITE_CONF) + else: + resource_map.pop(APACHE_SITE_24_CONF) + resource_map[CEPH_CONF]['services'] = [service_name()] return resource_map +def restart_map(): + return OrderedDict([(cfg, v['services']) + for cfg, v in resource_map().items() + if v['services']]) + + # Hardcoded to icehouse to enable use of charmhelper templating/context tools # Ideally these function would support non-OpenStack services def register_configs(release='icehouse'): @@ -180,12 +193,9 @@ def register_configs(release='icehouse'): def services(): """Returns a list of services associate with this charm.""" _services = [] - for v in BASE_RESOURCE_MAP.values(): + for v in resource_map().values(): _services.extend(v.get('services', [])) - _set_services = set(_services) - if not https(): - _set_services.remove('apache2') - return list(_set_services) + return list(set(_services)) def enable_pocket(pocket): @@ -560,3 +570,26 @@ def disable_unused_apache_sites(): with open(APACHE_PORTS_FILE, 'w') as ports: ports.write("") + + +def systemd_based_radosgw(): + """Determine if install should use systemd based radosgw instances""" + host = socket.gethostname() + for rid in relation_ids('mon'): + for unit in related_units(rid): + if relation_get('rgw.{}_key'.format(host), rid=rid, unit=unit): + return True + return False + + +def request_per_unit_key(): + """Determine if a per-unit cephx key should be requested""" + return (cmp_pkgrevno('radosgw', '12.2.0') >= 0 and init_is_systemd()) + + +def service_name(): + """Determine the name of the RADOS Gateway service""" + if systemd_based_radosgw(): + return 'ceph-radosgw@rgw.{}'.format(socket.gethostname()) + else: + return 'radosgw' diff --git a/templates/ceph.conf b/templates/ceph.conf index d89902e4..7b403a82 100644 --- a/templates/ceph.conf +++ b/templates/ceph.conf @@ -22,12 +22,18 @@ ms bind ipv6 = true {% endfor %} {% endif %} -[client.radosgw.gateway] +{% if systemd_rgw -%} +[client.rgw.{{ hostname }}] host = {{ hostname }} -rgw init timeout = 1200 +{% else -%} +[client.radosgw.gateway] keyring = /etc/ceph/keyring.rados.gateway +host = {{ hostname }} rgw socket path = /tmp/radosgw.sock log file = /var/log/ceph/radosgw.log +{% endif %} + +rgw init timeout = 1200 rgw frontends = civetweb port={{ port }} {% if auth_type == 'keystone' %} rgw keystone url = {{ auth_protocol }}://{{ auth_host }}:{{ auth_port }}/ diff --git a/tests/basic_deployment.py b/tests/basic_deployment.py index afd88839..b8de999b 100644 --- a/tests/basic_deployment.py +++ b/tests/basic_deployment.py @@ -329,7 +329,6 @@ class CephRadosGwBasicDeployment(OpenStackAmuletDeployment): relation = ['radosgw', 'ceph-radosgw:mon'] expected = { 'private-address': u.valid_ip, - 'radosgw_key': u.not_null, 'auth': 'none', 'ceph-public-address': u.valid_ip, } @@ -394,10 +393,6 @@ class CephRadosGwBasicDeployment(OpenStackAmuletDeployment): u.log.debug('Checking ceph config file data...') unit = self.ceph_radosgw_sentry conf = '/etc/ceph/ceph.conf' - keystone_sentry = self.keystone_sentry - relation = keystone_sentry.relation('identity-service', - 'ceph-radosgw:identity-service') - keystone_ip = relation['auth_host'] expected = { 'global': { 'auth cluster required': 'none', @@ -407,26 +402,7 @@ class CephRadosGwBasicDeployment(OpenStackAmuletDeployment): 'err to syslog': 'false', 'clog to syslog': 'false' }, - 'client.radosgw.gateway': { - 'keyring': '/etc/ceph/keyring.rados.gateway', - 'rgw socket path': '/tmp/radosgw.sock', - 'log file': '/var/log/ceph/radosgw.log', - 'rgw keystone url': 'http://{}:35357/'.format(keystone_ip), - 'rgw keystone accepted roles': 'Member,Admin', - 'rgw keystone token cache size': '500', - 'rgw keystone revocation interval': '600', - 'rgw frontends': 'civetweb port=70', - }, } - if self._get_openstack_release() >= self.xenial_queens: - expected['client.radosgw.gateway']['rgw keystone admin domain'] = ( - 'service_domain') - (expected['client.radosgw.gateway'] - ['rgw keystone admin project']) = 'services' - else: - expected['client.radosgw.gateway']['rgw keystone admin token'] = ( - 'ubuntutesting') - for section, pairs in expected.items(): ret = u.validate_config_data(unit, conf, section, pairs) if ret: diff --git a/unit_tests/test_ceph.py b/unit_tests/test_ceph.py index 673dd72f..044d24e2 100644 --- a/unit_tests/test_ceph.py +++ b/unit_tests/test_ceph.py @@ -32,6 +32,7 @@ TO_PATCH = [ 'config', 'os', 'subprocess', + 'mkdir', ] @@ -42,6 +43,7 @@ class CephRadosGWCephTests(CharmTestCase): def test_import_radosgw_key(self): self.os.path.exists.return_value = False + self.os.path.join.return_value = '/etc/ceph/keyring.rados.gateway' ceph.import_radosgw_key('mykey') cmd = [ 'ceph-authtool', @@ -50,7 +52,11 @@ class CephRadosGWCephTests(CharmTestCase): '--name=client.radosgw.gateway', '--add-key=mykey' ] - self.subprocess.check_call.assert_called_with(cmd) + self.subprocess.check_call.assert_has_calls([ + call(cmd), + call(['chown', 'root:root', + '/etc/ceph/keyring.rados.gateway']) + ]) @patch('charmhelpers.contrib.storage.linux.ceph.CephBrokerRq' '.add_op_create_pool') diff --git a/unit_tests/test_ceph_radosgw_context.py b/unit_tests/test_ceph_radosgw_context.py index 848e3c27..7bf051b2 100644 --- a/unit_tests/test_ceph_radosgw_context.py +++ b/unit_tests/test_ceph_radosgw_context.py @@ -29,7 +29,8 @@ TO_PATCH = [ 'cmp_pkgrevno', 'socket', 'unit_public_ip', - 'determine_api_port' + 'determine_api_port', + 'cmp_pkgrevno', ] @@ -312,6 +313,8 @@ class MonContextTest(CharmTestCase): return addresses.pop() elif attr == 'auth': return 'cephx' + elif attr == 'rgw.testhost_key': + return 'testkey' self.relation_get.side_effect = _relation_get self.relation_ids.return_value = ['mon:6'] @@ -322,6 +325,7 @@ class MonContextTest(CharmTestCase): 'hostname': 'testhost', 'mon_hosts': '10.5.4.1 10.5.4.2 10.5.4.3', 'old_auth': False, + 'systemd_rgw': True, 'unit_public_ip': '10.255.255.255', 'use_syslog': 'false', 'loglevel': 1, @@ -346,12 +350,15 @@ class MonContextTest(CharmTestCase): self.socket.gethostname.return_value = 'testhost' mon_ctxt = context.MonContext() addresses = ['10.5.4.1 10.5.4.2 10.5.4.3'] + self.cmp_pkgrevno.return_value = 1 def _relation_get(attr, unit, rid): if attr == 'ceph-public-address': return addresses.pop() elif attr == 'auth': return 'cephx' + elif attr == 'rgw.testhost_key': + return 'testkey' self.relation_get.side_effect = _relation_get self.relation_ids.return_value = ['mon:6'] @@ -362,6 +369,7 @@ class MonContextTest(CharmTestCase): 'hostname': 'testhost', 'mon_hosts': '10.5.4.1 10.5.4.2 10.5.4.3', 'old_auth': False, + 'systemd_rgw': True, 'unit_public_ip': '10.255.255.255', 'use_syslog': 'false', 'loglevel': 1, @@ -402,6 +410,9 @@ class MonContextTest(CharmTestCase): return addresses.pop() elif attr == 'auth': return auths.pop() + elif attr == 'rgw.testhost_key': + return 'testkey' + self.relation_get.side_effect = _relation_get self.relation_ids.return_value = ['mon:6'] self.related_units.return_value = ['ceph/0', 'ceph/1', 'ceph/2'] @@ -411,6 +422,7 @@ class MonContextTest(CharmTestCase): 'hostname': 'testhost', 'mon_hosts': '10.5.4.1 10.5.4.2 10.5.4.3', 'old_auth': False, + 'systemd_rgw': True, 'unit_public_ip': '10.255.255.255', 'use_syslog': 'false', 'loglevel': 1, @@ -433,6 +445,9 @@ class MonContextTest(CharmTestCase): return addresses.pop() elif attr == 'auth': return auths.pop() + elif attr == 'rgw.testhost_key': + return 'testkey' + self.relation_get.side_effect = _relation_get self.relation_ids.return_value = ['mon:6'] self.related_units.return_value = ['ceph/0', 'ceph/1', 'ceph/2'] @@ -442,6 +457,7 @@ class MonContextTest(CharmTestCase): 'hostname': 'testhost', 'mon_hosts': '10.5.4.1 10.5.4.2 10.5.4.3', 'old_auth': False, + 'systemd_rgw': True, 'unit_public_ip': '10.255.255.255', 'use_syslog': 'false', 'loglevel': 1, diff --git a/unit_tests/test_ceph_radosgw_utils.py b/unit_tests/test_ceph_radosgw_utils.py index 8df4214d..fcb1dd98 100644 --- a/unit_tests/test_ceph_radosgw_utils.py +++ b/unit_tests/test_ceph_radosgw_utils.py @@ -28,6 +28,12 @@ TO_PATCH = [ 'get_upstream_version', 'format_endpoint', 'https', + 'relation_ids', + 'relation_get', + 'related_units', + 'socket', + 'cmp_pkgrevno', + 'init_is_systemd', ] @@ -35,6 +41,7 @@ class CephRadosGWUtilTests(CharmTestCase): def setUp(self): super(CephRadosGWUtilTests, self).setUp(utils, TO_PATCH) self.get_upstream_version.return_value = '10.2.2' + self.socket.gethostname.return_value = 'testhost' def test_assess_status(self): with patch.object(utils, 'assess_status_func') as asf: @@ -219,3 +226,68 @@ class CephRadosGWUtilTests(CharmTestCase): c = ['openssl', 'x509', '-in', '/foo/bar/ca.pem', '-pubkey'] mock_check_output.assert_called_with(c) + + def _setup_relation_data(self, data): + self.relation_ids.return_value = data.keys() + self.related_units.side_effect = ( + lambda rid: data[rid].keys() + ) + self.relation_get.side_effect = ( + lambda attr, rid, unit: data[rid][unit].get(attr) + ) + + def test_systemd_based_radosgw_old_style(self): + _relation_data = { + 'mon:1': { + 'ceph-mon/0': { + 'radosgw_key': 'testkey', + }, + 'ceph-mon/1': { + 'radosgw_key': 'testkey', + }, + 'ceph-mon/2': { + 'radosgw_key': 'testkey', + }, + } + } + self._setup_relation_data(_relation_data) + self.assertFalse(utils.systemd_based_radosgw()) + + def test_systemd_based_radosgw_new_style(self): + _relation_data = { + 'mon:1': { + 'ceph-mon/0': { + 'rgw.testhost_key': 'testkey', + }, + 'ceph-mon/1': { + 'rgw.testhost_key': 'testkey', + }, + 'ceph-mon/2': { + 'rgw.testhost_key': 'testkey', + }, + } + } + self._setup_relation_data(_relation_data) + self.assertTrue(utils.systemd_based_radosgw()) + + def test_request_per_unit_key(self): + self.init_is_systemd.return_value = False + self.cmp_pkgrevno.return_value = -1 + self.assertFalse(utils.request_per_unit_key()) + self.init_is_systemd.return_value = True + self.cmp_pkgrevno.return_value = 1 + self.assertTrue(utils.request_per_unit_key()) + self.init_is_systemd.return_value = False + self.cmp_pkgrevno.return_value = 1 + self.assertFalse(utils.request_per_unit_key()) + + self.cmp_pkgrevno.assert_called_with('radosgw', '12.2.0') + + @patch.object(utils, 'systemd_based_radosgw') + def test_service_name(self, mock_systemd_based_radosgw): + mock_systemd_based_radosgw.return_value = True + self.assertEqual(utils.service_name(), + 'ceph-radosgw@rgw.testhost') + mock_systemd_based_radosgw.return_value = False + self.assertEqual(utils.service_name(), + 'radosgw') diff --git a/unit_tests/test_hooks.py b/unit_tests/test_hooks.py index 121f06cc..5c65ab30 100644 --- a/unit_tests/test_hooks.py +++ b/unit_tests/test_hooks.py @@ -55,7 +55,15 @@ TO_PATCH = [ 'get_relation_ip', 'disable_unused_apache_sites', 'service_reload', + 'service_stop', + 'service_restart', + 'service', 'setup_keystone_certs', + 'service_name', + 'socket', + 'restart_map', + 'systemd_based_radosgw', + 'request_per_unit_key', ] @@ -68,6 +76,9 @@ class CephRadosGWTests(CharmTestCase): self.test_config.set('key', 'secretkey') self.test_config.set('use-syslog', False) self.cmp_pkgrevno.return_value = 0 + self.service_name.return_value = 'radosgw' + self.request_per_unit_key.return_value = False + self.systemd_based_radosgw.return_value = False def test_install_packages(self): ceph_hooks.install_packages() @@ -95,22 +106,46 @@ class CephRadosGWTests(CharmTestCase): lambda *args, **kwargs: True) def test_mon_relation(self): _ceph = self.patch('ceph') - _restart = self.patch('restart') + _ceph.import_radosgw_key.return_value = True self.relation_get.return_value = 'seckey' + self.socket.gethostname.return_value = 'testinghostname' ceph_hooks.mon_relation() - self.assertTrue(_restart.called) - _ceph.import_radosgw_key.assert_called_with('seckey') + self.relation_set.assert_not_called() + self.service_restart.assert_called_once_with('radosgw') + self.service.assert_called_once_with('enable', 'radosgw') + _ceph.import_radosgw_key.assert_called_with('seckey', + name='rgw.testinghostname') + self.CONFIGS.write_all.assert_called_with() + + @patch.object(ceph_hooks, 'is_request_complete', + lambda *args, **kwargs: True) + def test_mon_relation_request_key(self): + _ceph = self.patch('ceph') + _ceph.import_radosgw_key.return_value = True + self.relation_get.return_value = 'seckey' + self.socket.gethostname.return_value = 'testinghostname' + self.request_per_unit_key.return_value = True + ceph_hooks.mon_relation() + self.relation_set.assert_called_with( + relation_id=None, + key_name='rgw.testinghostname' + ) + self.service_restart.assert_called_once_with('radosgw') + self.service.assert_called_once_with('enable', 'radosgw') + _ceph.import_radosgw_key.assert_called_with('seckey', + name='rgw.testinghostname') self.CONFIGS.write_all.assert_called_with() @patch.object(ceph_hooks, 'is_request_complete', lambda *args, **kwargs: True) def test_mon_relation_nokey(self): _ceph = self.patch('ceph') - _restart = self.patch('restart') + _ceph.import_radosgw_key.return_value = False self.relation_get.return_value = None ceph_hooks.mon_relation() self.assertFalse(_ceph.import_radosgw_key.called) - self.assertFalse(_restart.called) + self.service_restart.assert_not_called() + self.service.assert_not_called() self.CONFIGS.write_all.assert_called_with() @patch.object(ceph_hooks, 'send_request_if_needed') @@ -119,10 +154,11 @@ class CephRadosGWTests(CharmTestCase): def test_mon_relation_send_broker_request(self, mock_send_request_if_needed): _ceph = self.patch('ceph') - _restart = self.patch('restart') + _ceph.import_radosgw_key.return_value = False self.relation_get.return_value = 'seckey' ceph_hooks.mon_relation() - self.assertFalse(_restart.called) + self.service_restart.assert_not_called() + self.service.assert_not_called() self.assertFalse(_ceph.import_radosgw_key.called) self.assertFalse(self.CONFIGS.called) self.assertTrue(mock_send_request_if_needed.called) @@ -132,21 +168,6 @@ class CephRadosGWTests(CharmTestCase): ceph_hooks.gateway_relation() self.relation_set.assert_called_with(hostname='10.0.0.1', port=80) - def test_start(self): - ceph_hooks.start() - cmd = ['service', 'radosgw', 'start'] - self.subprocess.call.assert_called_with(cmd) - - def test_stop(self): - ceph_hooks.stop() - cmd = ['service', 'radosgw', 'stop'] - self.subprocess.call.assert_called_with(cmd) - - def test_restart(self): - ceph_hooks.restart() - cmd = ['service', 'radosgw', 'restart'] - self.subprocess.call.assert_called_with(cmd) - @patch('charmhelpers.contrib.openstack.ip.service_name', lambda *args: 'ceph-radosgw') @patch('charmhelpers.contrib.openstack.ip.config') @@ -200,10 +221,8 @@ class CephRadosGWTests(CharmTestCase): @patch.object(ceph_hooks, 'identity_joined') def test_identity_changed(self, mock_identity_joined): - _restart = self.patch('restart') ceph_hooks.identity_changed() self.CONFIGS.write_all.assert_called_with() - self.assertTrue(_restart.called) self.assertTrue(mock_identity_joined.called) @patch('charmhelpers.contrib.openstack.ip.is_clustered')