Add db initialised detection

Currently whenever the shared-db hook fires we call
migrate_neutron_database() which will always (unless unit
is paused) do a migration and restart the neutron-server
service. This is unnecessary and disruptive so we avoid
doing this by first checking whether we have already
initialised and and skipping migrate and restart if we
have already initialised. We also add support to override
this logic if an upgrade is in progress.

Change-Id: Ia4c104ff21d10a0d24ac3038bb75a5a9dc67ca94
Closes-Bug: 1708459
This commit is contained in:
Edward Hope-Morley 2017-08-15 16:50:09 +01:00
parent f70d657f84
commit 2c21ad14ab
4 changed files with 254 additions and 20 deletions

View File

@ -41,7 +41,6 @@ from charmhelpers.core.hookenv import (
from charmhelpers.core.host import (
mkdir,
service_reload,
service_restart,
)
from charmhelpers.fetch import (
@ -86,6 +85,7 @@ from neutron_api_utils import (
restart_map,
services,
setup_ipv6,
check_local_db_actions_complete,
)
from neutron_api_context import (
get_dns_domain,
@ -134,22 +134,29 @@ CONFIGS = register_configs()
def conditional_neutron_migration():
"""Initialise neutron database if not already done so.
Runs neutron-manage to initialize a new database or migrate existing and
restarts services to ensure that the changes are picked up. The first
(leader) unit to perform this action should have broadcast this information
to its peers so first we check whether this has already occurred.
"""
if CompareOpenStackReleases(os_release('neutron-server')) <= 'icehouse':
log('Not running neutron database migration as migrations are handled '
'by the neutron-server process.')
return
if is_elected_leader(CLUSTER_RES):
allowed_units = relation_get('allowed_units')
if allowed_units and local_unit() in allowed_units.split():
migrate_neutron_database()
if not is_unit_paused_set():
service_restart('neutron-server')
else:
log('Not running neutron database migration, either no'
' allowed_units or this unit is not present')
return
else:
if not is_elected_leader(CLUSTER_RES):
log('Not running neutron database migration, not leader')
return
allowed_units = relation_get('allowed_units')
if not (allowed_units and local_unit() in allowed_units.split()):
log('Not running neutron database migration, either no '
'allowed_units or this unit is not present')
return
migrate_neutron_database()
def configure_https():
@ -534,12 +541,16 @@ def cluster_joined(relation_id=None):
relation_set(relation_id=relation_id, relation_settings=settings)
if not relation_id:
check_local_db_actions_complete()
@hooks.hook('cluster-relation-changed',
'cluster-relation-departed')
@restart_on_change(restart_map(), stopstart=True)
def cluster_changed():
CONFIGS.write_all()
check_local_db_actions_complete()
@hooks.hook('ha-relation-joined')

View File

@ -18,6 +18,7 @@ from functools import partial
import os
import shutil
import subprocess
import uuid
import glob
from base64 import b64encode
from charmhelpers.contrib.openstack import context, templating
@ -55,7 +56,12 @@ from charmhelpers.core.hookenv import (
charm_dir,
config,
log,
DEBUG,
relation_ids,
related_units,
relation_get,
relation_set,
local_unit,
)
from charmhelpers.fetch import (
@ -231,6 +237,90 @@ LIBERTY_RESOURCE_MAP = OrderedDict([
])
NEUTRON_DB_INIT_RKEY = 'neutron-db-initialised'
NEUTRON_DB_INIT_ECHO_RKEY = 'neutron-db-initialised-echo'
def is_db_initialised(cluster_rid=None):
"""
Check whether a db intialisation has been performed by any peer unit.
We base our decision on whether we or any of our peers has previously
sent or echoed an initialisation notification.
@param cluster_rid: current relation id. If none provided, all cluster
relation ids will be checked.
@return: True if there has been a db initialisation otherwise False.
"""
if cluster_rid:
rids = [cluster_rid]
else:
rids = relation_ids('cluster')
shared_db_rel_id = (relation_ids('shared-db') or [None])[0]
if not shared_db_rel_id:
return False
for c_rid in rids:
units = related_units(relid=c_rid) + [local_unit()]
for unit in units:
settings = relation_get(unit=unit, rid=c_rid) or {}
for key in [NEUTRON_DB_INIT_RKEY, NEUTRON_DB_INIT_ECHO_RKEY]:
if shared_db_rel_id in settings.get(key, ''):
return True
return False
def is_new_dbinit_notification(init_id, echoed_init_id):
"""Returns True if we have a received a new db initialisation notification
from a peer unit and we have not previously echoed it to indicate that we
have already performed the necessary actions as result.
Initialisation notification is expected to be of the format:
<unit-id-leader-unit>-<shared-db-rel-id>-<uuid>
@param init_db: received initialisation notification.
@param echoed_init_db: value currently set for the echo key.
@return: True if new notification and False if not.
"""
shared_db_rel_id = (relation_ids('shared-db') or [None])[0]
return (shared_db_rel_id and init_id and
(local_unit() not in init_id) and
(shared_db_rel_id in init_id) and
(echoed_init_id != init_id))
def check_local_db_actions_complete():
"""Check if we have received db init'd notification and restart services
if we have not already.
NOTE: this must only be called from peer relation context.
"""
if not is_db_initialised():
return
settings = relation_get() or {}
if settings:
init_id = settings.get(NEUTRON_DB_INIT_RKEY)
echoed_init_id = relation_get(unit=local_unit(),
attribute=NEUTRON_DB_INIT_ECHO_RKEY)
# If we have received an init notification from a peer unit
# (assumed to be the leader) then restart neutron-api and echo the
# notification and don't restart again unless we receive a new
# (different) notification.
if is_new_dbinit_notification(init_id, echoed_init_id):
if not is_unit_paused_set():
log("Restarting neutron services following db "
"initialisation", level=DEBUG)
service_restart('neutron-server')
# Echo notification
relation_set(**{NEUTRON_DB_INIT_ECHO_RKEY: init_id})
def api_port(service):
return API_PORTS[service]
@ -483,7 +573,7 @@ def do_openstack_upgrade(configs):
# Stamping seems broken and unnecessary in liberty (Bug #1536675)
if CompareOpenStackReleases(os_release('neutron-common')) < 'liberty':
stamp_neutron_database(cur_os_rel)
migrate_neutron_database()
migrate_neutron_database(upgrade=True)
def stamp_neutron_database(release):
@ -531,8 +621,13 @@ def nuage_vsp_juno_neutron_migration():
raise Exception(e)
def migrate_neutron_database():
def migrate_neutron_database(upgrade=False):
'''Initializes a new database or upgrades an existing database.'''
if not upgrade and is_db_initialised():
log("Database is already initialised.", level=DEBUG)
return
log('Migrating the neutron database.')
if(os_release('neutron-server') == 'juno' and
config('neutron-plugin') == 'vsp'):
@ -548,6 +643,21 @@ def migrate_neutron_database():
'head']
subprocess.check_output(cmd)
cluster_rids = relation_ids('cluster')
if cluster_rids:
# Notify peers so that services get restarted
log("Notifying peer(s) that db is initialised and restarting services",
level=DEBUG)
for r_id in cluster_rids:
if not is_unit_paused_set():
service_restart('neutron-server')
# Notify peers that tey should also restart their services
shared_db_rel_id = (relation_ids('shared-db') or [None])[0]
id = "{}-{}-{}".format(local_unit(), shared_db_rel_id,
uuid.uuid4())
relation_set(relation_id=r_id, **{NEUTRON_DB_INIT_RKEY: id})
def get_topics():
return ['q-l3-plugin',

View File

@ -79,7 +79,6 @@ TO_PATCH = [
'relation_ids',
'relation_set',
'related_units',
'service_restart',
'unit_get',
'get_iface_for_address',
'get_netmask_for_address',
@ -783,9 +782,11 @@ class NeutronAPIHooksTests(CharmTestCase):
**_relation_data
)
def test_cluster_changed(self):
@patch.object(hooks, 'check_local_db_actions_complete')
def test_cluster_changed(self, mock_check_local_db_actions_complete):
self._call_hook('cluster-relation-changed')
self.assertTrue(self.CONFIGS.write_all.called)
self.assertTrue(mock_check_local_db_actions_complete.called)
@patch.object(hooks, 'get_hacluster_config')
def test_ha_joined(self, _get_ha_config):
@ -977,7 +978,6 @@ class NeutronAPIHooksTests(CharmTestCase):
self.os_release.return_value = 'kilo'
hooks.conditional_neutron_migration()
self.migrate_neutron_database.assert_called_with()
self.service_restart.assert_called_with('neutron-server')
def test_conditional_neutron_migration_leader_icehouse(self):
self.test_relation.set({
@ -994,7 +994,6 @@ class NeutronAPIHooksTests(CharmTestCase):
self.os_release.return_value = 'icehouse'
hooks.conditional_neutron_migration()
self.assertFalse(self.migrate_neutron_database.called)
self.assertFalse(self.service_restart.called)
def test_etcd_peer_joined(self):
self._call_hook('etcd-proxy-relation-joined')

View File

@ -327,7 +327,8 @@ class TestNeutronAPIUtils(CharmTestCase):
fatal=True)
configs.set_release.assert_called_with(openstack_release='juno')
stamp_neutron_db.assert_called_with('icehouse')
migrate_neutron_db.assert_called_with()
calls = [call(upgrade=True)]
migrate_neutron_db.assert_has_calls(calls)
@patch.object(charmhelpers.contrib.openstack.utils,
'get_os_codename_install_source')
@ -619,7 +620,120 @@ class TestNeutronAPIUtils(CharmTestCase):
'icehouse']
self.subprocess.check_output.assert_called_with(cmd)
def test_migrate_neutron_database(self):
@patch.object(nutils, 'relation_set')
@patch.object(nutils, 'is_db_initialised', lambda: False)
@patch.object(nutils, 'relation_get')
@patch.object(nutils, 'local_unit', lambda *args: 'unit/0')
def test_check_local_db_actions_complete_by_self(self, mock_relation_get,
mock_relation_set):
mock_relation_get.return_value = {}
nutils.check_local_db_actions_complete()
self.assertFalse(mock_relation_set.called)
mock_relation_get.return_value = {nutils.NEUTRON_DB_INIT_RKEY:
'unit/0-1234'}
nutils.check_local_db_actions_complete()
self.assertFalse(mock_relation_set.called)
@patch.object(nutils, 'relation_ids')
@patch.object(nutils, 'relation_set')
@patch.object(nutils, 'relation_get')
@patch.object(nutils, 'is_db_initialised')
@patch.object(nutils, 'local_unit', lambda *args: 'unit/0')
def test_check_local_db_actions_complete(self,
mock_is_db_initialised,
mock_relation_get,
mock_relation_set,
mock_relation_ids):
shared_db_rel_id = 'shared-db:1'
mock_relation_ids.return_value = [shared_db_rel_id]
mock_is_db_initialised.return_value = True
r_settings = {}
def fake_relation_get(unit=None, rid=None, attribute=None):
if attribute:
return r_settings.get(attribute)
else:
return r_settings
mock_relation_get.side_effect = fake_relation_get
nutils.check_local_db_actions_complete()
self.assertFalse(mock_relation_set.called)
init_db_val = 'unit/1-{}-1234'.format(shared_db_rel_id)
r_settings = {nutils.NEUTRON_DB_INIT_RKEY: init_db_val}
nutils.check_local_db_actions_complete()
calls = [call(**{nutils.NEUTRON_DB_INIT_ECHO_RKEY: init_db_val})]
mock_relation_set.assert_has_calls(calls)
self.service_restart.assert_called_with('neutron-server')
@patch.object(nutils, 'local_unit')
@patch.object(nutils, 'relation_get')
@patch.object(nutils, 'relation_ids')
@patch.object(nutils, 'related_units')
def test_is_db_initisalised_false(self, mock_related_units,
mock_relation_ids,
mock_relation_get,
mock_local_unit):
shared_db_rel_id = 'shared-db:1'
mock_relation_ids.return_value = [shared_db_rel_id]
settings = {'0': {}, '1': {}}
def mock_rel_get(unit=None, rid=None, attribute=None):
if not unit:
unit = '0'
if attribute:
return settings[unit].get(attribute)
return settings[unit]
mock_local_unit.return_value = '0'
mock_relation_get.side_effect = mock_rel_get
mock_related_units.return_value = ['1']
mock_relation_ids.return_value = ['cluster:1']
self.assertFalse(nutils.is_db_initialised())
@patch.object(nutils, 'local_unit')
@patch.object(nutils, 'relation_get')
@patch.object(nutils, 'relation_ids')
@patch.object(nutils, 'related_units')
def test_is_db_initisalised_true(self, mock_related_units,
mock_relation_ids,
mock_relation_get,
mock_local_unit):
shared_db_rel_id = 'shared-db:1'
init_db_val = 'unit/1-{}-1234'.format(shared_db_rel_id)
mock_relation_ids.return_value = [shared_db_rel_id]
settings = {'0': {nutils.NEUTRON_DB_INIT_RKEY: init_db_val},
'1': {nutils.NEUTRON_DB_INIT_ECHO_RKEY: init_db_val}}
def mock_rel_ids(name):
if name == 'cluster':
return 'cluster:1'
elif name == 'shared-db':
return 'shared-db:1'
raise Exception("Uknown relation '{}'".format(name))
def mock_rel_get(unit=None, rid=None, attribute=None):
if not unit:
unit = '0'
if attribute:
return settings[unit].get(attribute)
return settings[unit]
mock_relation_ids.side_effect = mock_rel_ids
mock_local_unit.return_value = '0'
mock_relation_get.side_effect = mock_rel_get
mock_related_units.return_value = ['1']
self.assertTrue(nutils.is_db_initialised())
@patch.object(nutils, 'relation_ids')
@patch.object(nutils, 'is_db_initialised')
def test_migrate_neutron_database(self, mock_is_db_initd, mock_rel_ids):
mock_is_db_initd.return_value = False
nutils.migrate_neutron_database()
cmd = ['neutron-db-manage',
'--config-file', '/etc/neutron/neutron.conf',