Remove code to run neutron-db-manage from neutron-api as it's racey with the nova-cc

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')

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()

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):
-    def test_shared_db_changed(self, cond_neutron_mig):
         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):
-    def test_pgsql_db_changed(self, cond_neutron_mig):
         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):
-    def test_neutron_api_relation_changed(self, cond_neutron_mig):
         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):
-    def test_neutron_api_relation_changed_incomplere_ctxt(self,
-                                                          cond_neutron_mig):
         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')

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')
+    def test_do_openstack_upgrade(self):
-    @patch.object(nutils, 'stamp_neutron_database')
-    def test_do_openstack_upgrade(self, stamp_neutron_db, migrate_neutron_db):
         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)