Prevent unnecessary nova-compute restarts on ceph_changed
Nova-compute service restarts make sense only on relation config updates received from the ceph-mon leader. Also, synced charm helpers as PR #347 is used as part of the fix. Change-Id: I406f369b1e376db82b8683fd48fbe4de106da16f Closes-bug: #1835045
This commit is contained in:
parent
a190b7f09a
commit
69a6fdeaf4
@ -33,6 +33,7 @@ from charmhelpers.core.hookenv import (
|
||||
hook_name,
|
||||
local_unit,
|
||||
log,
|
||||
relation_get,
|
||||
relation_ids,
|
||||
relation_set,
|
||||
relations_of_type,
|
||||
@ -260,11 +261,23 @@ class NRPE(object):
|
||||
relation = relation_ids('nrpe-external-master')
|
||||
if relation:
|
||||
log("Setting charm primary status {}".format(primary))
|
||||
for rid in relation_ids('nrpe-external-master'):
|
||||
for rid in relation:
|
||||
relation_set(relation_id=rid, relation_settings={'primary': self.primary})
|
||||
self.remove_check_queue = set()
|
||||
|
||||
def add_check(self, *args, **kwargs):
|
||||
shortname = None
|
||||
if kwargs.get('shortname') is None:
|
||||
if len(args) > 0:
|
||||
shortname = args[0]
|
||||
else:
|
||||
shortname = kwargs['shortname']
|
||||
|
||||
self.checks.append(Check(*args, **kwargs))
|
||||
try:
|
||||
self.remove_check_queue.remove(shortname)
|
||||
except KeyError:
|
||||
pass
|
||||
|
||||
def remove_check(self, *args, **kwargs):
|
||||
if kwargs.get('shortname') is None:
|
||||
@ -281,6 +294,7 @@ class NRPE(object):
|
||||
|
||||
check = Check(*args, **kwargs)
|
||||
check.remove(self.hostname)
|
||||
self.remove_check_queue.add(kwargs['shortname'])
|
||||
|
||||
def write(self):
|
||||
try:
|
||||
@ -313,7 +327,24 @@ class NRPE(object):
|
||||
monitor_ids = relation_ids("local-monitors") + \
|
||||
relation_ids("nrpe-external-master")
|
||||
for rid in monitor_ids:
|
||||
relation_set(relation_id=rid, monitors=yaml.dump(monitors))
|
||||
reldata = relation_get(unit=local_unit(), rid=rid)
|
||||
if 'monitors' in reldata:
|
||||
# update the existing set of monitors with the new data
|
||||
old_monitors = yaml.safe_load(reldata['monitors'])
|
||||
old_nrpe_monitors = old_monitors['monitors']['remote']['nrpe']
|
||||
# remove keys that are in the remove_check_queue
|
||||
old_nrpe_monitors = {k: v for k, v in old_nrpe_monitors.items()
|
||||
if k not in self.remove_check_queue}
|
||||
# update/add nrpe_monitors
|
||||
old_nrpe_monitors.update(nrpe_monitors)
|
||||
old_monitors['monitors']['remote']['nrpe'] = old_nrpe_monitors
|
||||
# write back to the relation
|
||||
relation_set(relation_id=rid, monitors=yaml.dump(old_monitors))
|
||||
else:
|
||||
# write a brand new set of monitors, as no existing ones.
|
||||
relation_set(relation_id=rid, monitors=yaml.dump(monitors))
|
||||
|
||||
self.remove_check_queue.clear()
|
||||
|
||||
|
||||
def get_nagios_hostcontext(relation_name='nrpe-external-master'):
|
||||
|
@ -217,19 +217,35 @@ def full_restart():
|
||||
service('force-reload-kmod', 'openvswitch-switch')
|
||||
|
||||
|
||||
def enable_ipfix(bridge, target):
|
||||
'''Enable IPfix on bridge to target.
|
||||
def enable_ipfix(bridge, target,
|
||||
cache_active_timeout=60,
|
||||
cache_max_flows=128,
|
||||
sampling=64):
|
||||
'''Enable IPFIX on bridge to target.
|
||||
:param bridge: Bridge to monitor
|
||||
:param target: IPfix remote endpoint
|
||||
:param target: IPFIX remote endpoint
|
||||
:param cache_active_timeout: The maximum period in seconds for
|
||||
which an IPFIX flow record is cached
|
||||
and aggregated before being sent
|
||||
:param cache_max_flows: The maximum number of IPFIX flow records
|
||||
that can be cached at a time
|
||||
:param sampling: The rate at which packets should be sampled and
|
||||
sent to each target collector
|
||||
'''
|
||||
cmd = ['ovs-vsctl', 'set', 'Bridge', bridge, 'ipfix=@i', '--',
|
||||
'--id=@i', 'create', 'IPFIX', 'targets="{}"'.format(target)]
|
||||
cmd = [
|
||||
'ovs-vsctl', 'set', 'Bridge', bridge, 'ipfix=@i', '--',
|
||||
'--id=@i', 'create', 'IPFIX',
|
||||
'targets="{}"'.format(target),
|
||||
'sampling={}'.format(sampling),
|
||||
'cache_active_timeout={}'.format(cache_active_timeout),
|
||||
'cache_max_flows={}'.format(cache_max_flows),
|
||||
]
|
||||
log('Enabling IPfix on {}.'.format(bridge))
|
||||
subprocess.check_call(cmd)
|
||||
|
||||
|
||||
def disable_ipfix(bridge):
|
||||
'''Diable IPfix on target bridge.
|
||||
'''Diable IPFIX on target bridge.
|
||||
:param bridge: Bridge to modify
|
||||
'''
|
||||
cmd = ['ovs-vsctl', 'clear', 'Bridge', bridge, 'ipfix']
|
||||
|
@ -126,7 +126,11 @@ def _config_ini(path):
|
||||
:returns: Configuration contained in path
|
||||
:rtype: Dict
|
||||
"""
|
||||
conf = configparser.ConfigParser()
|
||||
# When strict is enabled, duplicate options are not allowed in the
|
||||
# parsed INI; however, Oslo allows duplicate values. This change
|
||||
# causes us to ignore the duplicate values which is acceptable as
|
||||
# long as we don't validate any multi-value options
|
||||
conf = configparser.ConfigParser(strict=False)
|
||||
conf.read(path)
|
||||
return dict(conf)
|
||||
|
||||
@ -204,7 +208,7 @@ def validate_file_ownership(config):
|
||||
"Invalid ownership configuration: {}".format(key))
|
||||
owner = options.get('owner', config.get('owner', 'root'))
|
||||
group = options.get('group', config.get('group', 'root'))
|
||||
optional = options.get('optional', config.get('optional', 'False'))
|
||||
optional = options.get('optional', config.get('optional', False))
|
||||
if '*' in file_name:
|
||||
for file in glob.glob(file_name):
|
||||
if file not in files.keys():
|
||||
@ -226,7 +230,7 @@ def validate_file_permissions(config):
|
||||
raise RuntimeError(
|
||||
"Invalid ownership configuration: {}".format(key))
|
||||
mode = options.get('mode', config.get('permissions', '600'))
|
||||
optional = options.get('optional', config.get('optional', 'False'))
|
||||
optional = options.get('optional', config.get('optional', False))
|
||||
if '*' in file_name:
|
||||
for file in glob.glob(file_name):
|
||||
if file not in files.keys():
|
||||
|
@ -106,9 +106,11 @@ class CertRequest(object):
|
||||
sans = sorted(list(set(entry['addresses'])))
|
||||
request[entry['cn']] = {'sans': sans}
|
||||
if self.json_encode:
|
||||
return {'cert_requests': json.dumps(request, sort_keys=True)}
|
||||
req = {'cert_requests': json.dumps(request, sort_keys=True)}
|
||||
else:
|
||||
return {'cert_requests': request}
|
||||
req = {'cert_requests': request}
|
||||
req['unit_name'] = local_unit().replace('/', '_')
|
||||
return req
|
||||
|
||||
|
||||
def get_certificate_request(json_encode=True):
|
||||
|
@ -443,8 +443,10 @@ class IdentityServiceContext(OSContextGenerator):
|
||||
'api_version': api_version})
|
||||
|
||||
if float(api_version) > 2:
|
||||
ctxt.update({'admin_domain_name':
|
||||
rdata.get('service_domain')})
|
||||
ctxt.update({
|
||||
'admin_domain_name': rdata.get('service_domain'),
|
||||
'service_project_id': rdata.get('service_tenant_id'),
|
||||
'service_domain_id': rdata.get('service_domain_id')})
|
||||
|
||||
# we keep all veriables in ctxt for compatibility and
|
||||
# add nested dictionary for keystone_authtoken generic
|
||||
@ -1710,6 +1712,10 @@ class NeutronAPIContext(OSContextGenerator):
|
||||
'rel_key': 'enable-nsg-logging',
|
||||
'default': False,
|
||||
},
|
||||
'enable_nfg_logging': {
|
||||
'rel_key': 'enable-nfg-logging',
|
||||
'default': False,
|
||||
},
|
||||
'global_physnet_mtu': {
|
||||
'rel_key': 'global-physnet-mtu',
|
||||
'default': 1500,
|
||||
|
@ -1482,6 +1482,21 @@ def send_request_if_needed(request, relation='ceph'):
|
||||
relation_set(relation_id=rid, broker_req=request.request)
|
||||
|
||||
|
||||
def has_broker_rsp(rid=None, unit=None):
|
||||
"""Return True if the broker_rsp key is 'truthy' (i.e. set to something) in the relation data.
|
||||
|
||||
:param rid: The relation to check (default of None means current relation)
|
||||
:type rid: Union[str, None]
|
||||
:param unit: The remote unit to check (default of None means current unit)
|
||||
:type unit: Union[str, None]
|
||||
:returns: True if broker key exists and is set to something 'truthy'
|
||||
:rtype: bool
|
||||
"""
|
||||
rdata = relation_get(rid=rid, unit=unit) or {}
|
||||
broker_rsp = rdata.get(get_broker_rsp_key())
|
||||
return True if broker_rsp else False
|
||||
|
||||
|
||||
def is_broker_action_done(action, rid=None, unit=None):
|
||||
"""Check whether broker action has completed yet.
|
||||
|
||||
|
@ -110,17 +110,19 @@ def is_device_mounted(device):
|
||||
return bool(re.search(r'MOUNTPOINT=".+"', out))
|
||||
|
||||
|
||||
def mkfs_xfs(device, force=False):
|
||||
def mkfs_xfs(device, force=False, inode_size=1024):
|
||||
"""Format device with XFS filesystem.
|
||||
|
||||
By default this should fail if the device already has a filesystem on it.
|
||||
:param device: Full path to device to format
|
||||
:ptype device: tr
|
||||
:param force: Force operation
|
||||
:ptype: force: boolean"""
|
||||
:ptype: force: boolean
|
||||
:param inode_size: XFS inode size in bytes
|
||||
:ptype inode_size: int"""
|
||||
cmd = ['mkfs.xfs']
|
||||
if force:
|
||||
cmd.append("-f")
|
||||
|
||||
cmd += ['-i', 'size=1024', device]
|
||||
cmd += ['-i', "size={}".format(inode_size), device]
|
||||
check_call(cmd)
|
||||
|
@ -77,6 +77,7 @@ from charmhelpers.contrib.storage.linux.ceph import (
|
||||
is_request_complete,
|
||||
is_broker_action_done,
|
||||
mark_broker_action_done,
|
||||
has_broker_rsp,
|
||||
)
|
||||
from charmhelpers.payload.execd import execd_preinstall
|
||||
from nova_compute_utils import (
|
||||
@ -408,6 +409,11 @@ def ceph_changed(rid=None, unit=None):
|
||||
create_libvirt_secret(secret_file=CEPH_SECRET,
|
||||
secret_uuid=CEPH_SECRET_UUID, key=key)
|
||||
|
||||
if not has_broker_rsp(rid, unit):
|
||||
log("Unit {} does not have a broker_rsp. "
|
||||
"Aborting ceph_changed".format(unit))
|
||||
return
|
||||
|
||||
if is_request_complete(get_ceph_request()):
|
||||
log('Request complete')
|
||||
# Ensure that nova-compute is restarted since only now can we
|
||||
|
@ -459,14 +459,17 @@ class NovaComputeRelationsTests(CharmTestCase):
|
||||
|
||||
@patch.object(hooks, 'mark_broker_action_done')
|
||||
@patch.object(hooks, 'is_broker_action_done')
|
||||
@patch.object(hooks, 'has_broker_rsp')
|
||||
@patch('nova_compute_context.service_name')
|
||||
@patch.object(hooks, 'CONFIGS')
|
||||
def test_ceph_changed_with_key_and_relation_data(self, configs,
|
||||
service_name,
|
||||
has_broker_rsp,
|
||||
is_broker_action_done,
|
||||
mark_broker_action_done):
|
||||
self.test_config.set('libvirt-image-backend', 'rbd')
|
||||
self.is_request_complete.return_value = True
|
||||
has_broker_rsp.return_value = True
|
||||
self.assert_libvirt_rbd_imagebackend_allowed.return_value = True
|
||||
configs.complete_contexts = MagicMock()
|
||||
configs.complete_contexts.return_value = ['ceph']
|
||||
@ -489,6 +492,12 @@ class NovaComputeRelationsTests(CharmTestCase):
|
||||
hooks.ceph_changed()
|
||||
self.assertFalse(mark_broker_action_done.called)
|
||||
|
||||
is_broker_action_done.return_value = False
|
||||
has_broker_rsp.return_value = False
|
||||
mark_broker_action_done.reset_mock()
|
||||
hooks.ceph_changed()
|
||||
self.assertFalse(mark_broker_action_done.called)
|
||||
|
||||
@patch('charmhelpers.contrib.storage.linux.ceph.CephBrokerRq'
|
||||
'.add_op_request_access_to_group')
|
||||
@patch('charmhelpers.contrib.storage.linux.ceph.CephBrokerRq'
|
||||
|
Loading…
Reference in New Issue
Block a user