From f356970aebaa150de60a1f2f47f09b2d77478441 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Tue, 21 Oct 2014 13:07:03 +0000 Subject: [PATCH] Remove code to run neutron-db-manage from neutron-api as it's racey with the nova-cc --- hooks/neutron_api_hooks.py | 36 ------------- hooks/neutron_api_utils.py | 34 ------------- unit_tests/test_neutron_api_hooks.py | 76 ++-------------------------- unit_tests/test_neutron_api_utils.py | 26 +--------- 4 files changed, 5 insertions(+), 167 deletions(-) diff --git a/hooks/neutron_api_hooks.py b/hooks/neutron_api_hooks.py index 4a8effd5..8e3cc5ab 100755 --- a/hooks/neutron_api_hooks.py +++ b/hooks/neutron_api_hooks.py @@ -20,7 +20,6 @@ from charmhelpers.core.hookenv import ( from charmhelpers.core.host import ( restart_on_change, - service_restart, ) from charmhelpers.fetch import ( @@ -32,7 +31,6 @@ from charmhelpers.fetch import ( from charmhelpers.contrib.openstack.utils import ( configure_installation_source, openstack_upgrade_available, - os_release, sync_db_with_multi_ipv6_addresses ) from charmhelpers.contrib.openstack.neutron import ( @@ -40,13 +38,11 @@ from charmhelpers.contrib.openstack.neutron import ( ) from neutron_api_utils import ( - CLUSTER_RES, NEUTRON_CONF, api_port, determine_packages, determine_ports, do_openstack_upgrade, - migrate_neutron_database, register_configs, restart_map, setup_ipv6 @@ -58,7 +54,6 @@ from neutron_api_context import ( from charmhelpers.contrib.hahelpers.cluster import ( get_hacluster_config, - is_leader, ) from charmhelpers.payload.execd import execd_preinstall @@ -153,33 +148,6 @@ def amqp_changed(): CONFIGS.write(NEUTRON_CONF) -def conditional_neutron_migration(): - # This is an attempt to stop a race over the db migration between nova-cc - # and neutron-api by having the migration master decided by the presence - # of the neutron-api relation. In the long term this should only be done - # the neutron-api charm and nova-cc should play no hand in it - # * neutron-api refuses to run migrations until neutron-api relation is - # present - # * nova-cc refuses to run migration if neutron-api relations is present - clustered = relation_get('clustered') - if not relation_ids('neutron-api'): - log('Not running neutron database migration, no nova-cloud-controller' - 'is present.') - elif os_release('neutron-server') <= 'icehouse': - log('Not running neutron database migration as migrations are handled' - 'by the neutron-server process.') - else: - if clustered: - if is_leader(CLUSTER_RES): - migrate_neutron_database() - service_restart('neutron-server') - else: - log('Not running neutron database migration, not leader') - else: - migrate_neutron_database() - service_restart('neutron-server') - - @hooks.hook('shared-db-relation-joined') def db_joined(): if is_relation_made('pgsql-db'): @@ -218,7 +186,6 @@ def db_changed(): log('shared-db relation incomplete. Peer not ready?') return CONFIGS.write_all() - conditional_neutron_migration() @hooks.hook('pgsql-db-relation-changed') @@ -227,7 +194,6 @@ def postgresql_neutron_db_changed(): plugin = config('neutron-plugin') # DB config might have been moved to main neutron.conf in H? CONFIGS.write(neutron_plugin_attribute(plugin, 'config')) - conditional_neutron_migration() @hooks.hook('amqp-relation-broken', @@ -294,8 +260,6 @@ def neutron_api_relation_joined(rid=None): @restart_on_change(restart_map()) def neutron_api_relation_changed(): CONFIGS.write(NEUTRON_CONF) - if 'shared-db' in CONFIGS.complete_contexts(): - conditional_neutron_migration() @hooks.hook('neutron-plugin-api-relation-joined') diff --git a/hooks/neutron_api_utils.py b/hooks/neutron_api_utils.py index e7f096e7..4179440a 100644 --- a/hooks/neutron_api_utils.py +++ b/hooks/neutron_api_utils.py @@ -30,7 +30,6 @@ from charmhelpers.core.host import ( ) import neutron_api_context -import subprocess TEMPLATES = 'templates/' @@ -189,7 +188,6 @@ def do_openstack_upgrade(configs): :param configs: The charms main OSConfigRenderer object. """ - cur_os_rel = os_release('neutron-server') new_src = config('openstack-origin') new_os_rel = get_os_codename_install_source(new_src) @@ -212,38 +210,6 @@ def do_openstack_upgrade(configs): # set CONFIGS to load templates from new release configs.set_release(openstack_release=new_os_rel) - if cur_os_rel == 'icehouse': - stamp_neutron_database('icehouse') - migrate_neutron_database() - - -def stamp_neutron_database(release): - '''Stamp the database with the current release before upgrade.''' - log('Stamping the neutron database with release %s.' % release) - plugin = config('neutron-plugin') - cmd = ['neutron-db-manage', - '--config-file', NEUTRON_CONF, - '--config-file', neutron_plugin_attribute(plugin, - 'config', - 'neutron'), - 'stamp', - release] - subprocess.check_output(cmd) - - -def migrate_neutron_database(): - '''Initializes a new database or upgrades an existing database.''' - log('Migrating the neutron database.') - plugin = config('neutron-plugin') - cmd = ['neutron-db-manage', - '--config-file', NEUTRON_CONF, - '--config-file', neutron_plugin_attribute(plugin, - 'config', - 'neutron'), - 'upgrade', - 'head'] - subprocess.check_output(cmd) - def setup_ipv6(): ubuntu_rel = lsb_release()['DISTRIB_CODENAME'].lower() diff --git a/unit_tests/test_neutron_api_hooks.py b/unit_tests/test_neutron_api_hooks.py index fe7258a1..747d66a0 100644 --- a/unit_tests/test_neutron_api_hooks.py +++ b/unit_tests/test_neutron_api_hooks.py @@ -41,7 +41,6 @@ TO_PATCH = [ 'neutron_plugin_attribute', 'open_port', 'openstack_upgrade_available', - 'os_release', 'relation_get', 'relation_ids', 'relation_set', @@ -49,9 +48,6 @@ TO_PATCH = [ 'get_iface_for_address', 'get_netmask_for_address', 'get_address_in_network', - 'migrate_neutron_database', - 'service_restart', - 'is_leader', ] NEUTRON_CONF_DIR = "/etc/neutron" @@ -167,23 +163,19 @@ class NeutronAPIHooksTests(CharmTestCase): 'Attempting to associate a postgresql database when' ' there is already associated a mysql one') - @patch.object(hooks, 'conditional_neutron_migration') - def test_shared_db_changed(self, cond_neutron_mig): + def test_shared_db_changed(self): self.CONFIGS.complete_contexts.return_value = ['shared-db'] self._call_hook('shared-db-relation-changed') self.assertTrue(self.CONFIGS.write_all.called) - cond_neutron_mig.assert_called_with() def test_shared_db_changed_partial_ctxt(self): self.CONFIGS.complete_contexts.return_value = [] self._call_hook('shared-db-relation-changed') self.assertFalse(self.CONFIGS.write_all.called) - @patch.object(hooks, 'conditional_neutron_migration') - def test_pgsql_db_changed(self, cond_neutron_mig): + def test_pgsql_db_changed(self): self._call_hook('pgsql-db-relation-changed') self.assertTrue(self.CONFIGS.write.called) - cond_neutron_mig.assert_called_with() def test_amqp_broken(self): self._call_hook('amqp-relation-broken') @@ -269,20 +261,15 @@ class NeutronAPIHooksTests(CharmTestCase): **_relation_data ) - @patch.object(hooks, 'conditional_neutron_migration') - def test_neutron_api_relation_changed(self, cond_neutron_mig): + def test_neutron_api_relation_changed(self): self.CONFIGS.complete_contexts.return_value = ['shared-db'] self._call_hook('neutron-api-relation-changed') self.assertTrue(self.CONFIGS.write.called_with(NEUTRON_CONF)) - cond_neutron_mig.assert_called_with() - @patch.object(hooks, 'conditional_neutron_migration') - def test_neutron_api_relation_changed_incomplere_ctxt(self, - cond_neutron_mig): + def test_neutron_api_relation_changed_incomplere_ctxt(self): self.CONFIGS.complete_contexts.return_value = [] self._call_hook('neutron-api-relation-changed') self.assertTrue(self.CONFIGS.write.called_with(NEUTRON_CONF)) - self.assertFalse(cond_neutron_mig.called) def test_neutron_plugin_api_relation_joined_nol2(self): _relation_data = { @@ -410,58 +397,3 @@ class NeutronAPIHooksTests(CharmTestCase): self.check_call.assert_called_with(['a2dissite', 'openstack_https_frontend']) self.assertTrue(_id_rel_joined.called) - - def test_conditional_neutron_migration_no_ncc_rel(self): - self.test_relation.set({ - 'clustered': 'false', - }) - self.relation_ids.return_value = [] - hooks.conditional_neutron_migration() - self.log.assert_called_with( - 'Not running neutron database migration, no nova-cloud-controller' - 'is present.' - ) - - def test_conditional_neutron_migration_icehouse(self): - self.test_relation.set({ - 'clustered': 'false', - }) - self.os_release.return_value = 'icehouse' - hooks.conditional_neutron_migration() - self.log.assert_called_with( - 'Not running neutron database migration as migrations are handled' - 'by the neutron-server process.' - ) - - def test_conditional_neutron_migration_ncc_rel_leader(self): - self.test_relation.set({ - 'clustered': 'true', - }) - self.is_leader.return_value = True - self.os_release.return_value = 'juno' - hooks.conditional_neutron_migration() - self.migrate_neutron_database.assert_called_with() - self.service_restart.assert_called_with('neutron-server') - - def test_conditional_neutron_migration_ncc_rel_notleader(self): - self.test_relation.set({ - 'clustered': 'true', - }) - self.is_leader.return_value = False - self.os_release.return_value = 'juno' - hooks.conditional_neutron_migration() - self.assertFalse(self.migrate_neutron_database.called) - self.assertFalse(self.service_restart.called) - self.log.assert_called_with( - 'Not running neutron database migration, not leader' - ) - - def test_conditional_neutron_migration_not_clustered(self): - self.test_relation.set({ - 'clustered': 'false', - }) - self.relation_ids.return_value = ['nova-cc/o'] - self.os_release.return_value = 'juno' - hooks.conditional_neutron_migration() - self.migrate_neutron_database.assert_called_with() - self.service_restart.assert_called_with('neutron-server') diff --git a/unit_tests/test_neutron_api_utils.py b/unit_tests/test_neutron_api_utils.py index 5f68a9b6..b3314862 100644 --- a/unit_tests/test_neutron_api_utils.py +++ b/unit_tests/test_neutron_api_utils.py @@ -28,7 +28,6 @@ TO_PATCH = [ 'log', 'neutron_plugin_attribute', 'os_release', - 'subprocess', ] @@ -146,16 +145,13 @@ class TestNeutronAPIUtils(CharmTestCase): nutils.keystone_ca_cert_b64() self.assertTrue(self.b64encode.called) - @patch.object(nutils, 'migrate_neutron_database') - @patch.object(nutils, 'stamp_neutron_database') - def test_do_openstack_upgrade(self, stamp_neutron_db, migrate_neutron_db): + def test_do_openstack_upgrade(self): self.config.side_effect = self.test_config.get self.test_config.set('openstack-origin', 'cloud:trusty-juno') self.os_release.side_effect = 'icehouse' self.get_os_codename_install_source.return_value = 'juno' configs = MagicMock() nutils.do_openstack_upgrade(configs) - self.os_release.assert_called_with('neutron-server') self.log.assert_called() self.configure_installation_source.assert_called_with( 'cloud:trusty-juno' @@ -174,23 +170,3 @@ class TestNeutronAPIUtils(CharmTestCase): options=dpkg_opts, fatal=True) configs.set_release.assert_called_with(openstack_release='juno') - stamp_neutron_db.assert_called() - migrate_neutron_db.assert_called() - - def test_stamp_neutron_database(self): - nutils.stamp_neutron_database('icehouse') - cmd = ['neutron-db-manage', - '--config-file', '/etc/neutron/neutron.conf', - '--config-file', '/etc/neutron/plugins/ml2/ml2_conf.ini', - 'stamp', - 'icehouse'] - self.subprocess.check_output.assert_called_with(cmd) - - def test_migrate_neutron_database(self): - nutils.migrate_neutron_database() - cmd = ['neutron-db-manage', - '--config-file', '/etc/neutron/neutron.conf', - '--config-file', '/etc/neutron/plugins/ml2/ml2_conf.ini', - 'upgrade', - 'head'] - self.subprocess.check_output.assert_called_with(cmd)