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:
parent
f70d657f84
commit
2c21ad14ab
|
@ -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')
|
||||
|
|
|
@ -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',
|
||||
|
|
|
@ -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')
|
||||
|
|
|
@ -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',
|
||||
|
|
Loading…
Reference in New Issue