From c827b6d5415668e1af2098835442fe37a2bf660a Mon Sep 17 00:00:00 2001 From: Liam Young Date: Wed, 22 Apr 2015 13:38:00 +0000 Subject: [PATCH 1/7] Run db migrations when deployment is kilo or migrating to kilo --- hooks/neutron_api_hooks.py | 25 +++++++++++++++++++++++++ hooks/neutron_api_utils.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/hooks/neutron_api_hooks.py b/hooks/neutron_api_hooks.py index d1c8cee9..f0ccaad7 100755 --- a/hooks/neutron_api_hooks.py +++ b/hooks/neutron_api_hooks.py @@ -23,6 +23,7 @@ from charmhelpers.core.hookenv import ( from charmhelpers.core.host import ( restart_on_change, service_reload, + service_restart, ) from charmhelpers.fetch import ( @@ -37,10 +38,12 @@ from charmhelpers.contrib.openstack.utils import ( git_install_requested, openstack_upgrade_available, os_requires_version, + os_release, sync_db_with_multi_ipv6_addresses ) from neutron_api_utils import ( + CLUSTER_RES, NEUTRON_CONF, api_port, determine_packages, @@ -49,6 +52,7 @@ from neutron_api_utils import ( git_install, dvr_router_present, l3ha_router_present, + migrate_neutron_database, register_configs, restart_map, services, @@ -65,6 +69,7 @@ from neutron_api_context import ( from charmhelpers.contrib.hahelpers.cluster import ( get_hacluster_config, + is_leader, ) from charmhelpers.payload.execd import execd_preinstall @@ -90,6 +95,24 @@ hooks = Hooks() CONFIGS = register_configs() +def conditional_neutron_migration(): + clustered = relation_get('clustered') + if os_release('neutron-server') < 'kilo': + log('Not running neutron database migration as migrations are handled' + 'by the neutron-server process or nova-cloud-controller charm.') + return + + 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') + + def configure_https(): ''' Enables SSL API Apache config if appropriate and kicks identity-service @@ -227,12 +250,14 @@ def db_changed(): log('shared-db relation incomplete. Peer not ready?') return CONFIGS.write_all() + conditional_neutron_migration() @hooks.hook('pgsql-db-relation-changed') @restart_on_change(restart_map()) def postgresql_neutron_db_changed(): CONFIGS.write(NEUTRON_CONF) + conditional_neutron_migration() @hooks.hook('amqp-relation-broken', diff --git a/hooks/neutron_api_utils.py b/hooks/neutron_api_utils.py index 0739e33e..f872fcff 100644 --- a/hooks/neutron_api_utils.py +++ b/hooks/neutron_api_utils.py @@ -3,6 +3,7 @@ from copy import deepcopy from functools import partial import os import shutil +import subprocess from base64 import b64encode from charmhelpers.contrib.openstack import context, templating from charmhelpers.contrib.openstack.neutron import ( @@ -258,6 +259,7 @@ 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) @@ -279,6 +281,38 @@ def do_openstack_upgrade(configs): # set CONFIGS to load templates from new release configs.set_release(openstack_release=new_os_rel) + # Before kilo it's nova-cloud-controllers job + if new_os_rel >= 'kilo': + stamp_neutron_database(cur_os_rel) + 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 get_topics(): From f4a50eac652df40c0b62858654b5a17ad3a7dcdb Mon Sep 17 00:00:00 2001 From: Liam Young Date: Wed, 22 Apr 2015 14:15:37 +0000 Subject: [PATCH 2/7] Check allowed units before running neutron migration --- hooks/neutron_api_hooks.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/hooks/neutron_api_hooks.py b/hooks/neutron_api_hooks.py index f0ccaad7..8009b888 100755 --- a/hooks/neutron_api_hooks.py +++ b/hooks/neutron_api_hooks.py @@ -11,6 +11,7 @@ from charmhelpers.core.hookenv import ( UnregisteredHookError, config, is_relation_made, + local_unit, log, ERROR, relation_get, @@ -104,8 +105,13 @@ def conditional_neutron_migration(): if clustered: if is_leader(CLUSTER_RES): - migrate_neutron_database() - service_restart('neutron-server') + allowed_units = relation_get('neutron_allowed_units') + if allowed_units and local_unit() in allowed_units.split(): + migrate_neutron_database() + service_restart('neutron-server') + else: + log('allowed_units either not presented, or local unit ' + 'not in acl list: %s' % repr(allowed_units)) else: log('Not running neutron database migration, not leader') else: From 494e007e689adc8017987a2d00c447e996560442 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Wed, 22 Apr 2015 14:39:42 +0000 Subject: [PATCH 3/7] Fix spaces in log message --- hooks/neutron_api_hooks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hooks/neutron_api_hooks.py b/hooks/neutron_api_hooks.py index 8009b888..1b96751b 100755 --- a/hooks/neutron_api_hooks.py +++ b/hooks/neutron_api_hooks.py @@ -99,7 +99,7 @@ CONFIGS = register_configs() def conditional_neutron_migration(): clustered = relation_get('clustered') if os_release('neutron-server') < 'kilo': - log('Not running neutron database migration as migrations are handled' + log('Not running neutron database migration as migrations are handled ' 'by the neutron-server process or nova-cloud-controller charm.') return From 8cd29a3d9f35fee79d148d9852daf3fa2acc0dfc Mon Sep 17 00:00:00 2001 From: Liam Young Date: Wed, 22 Apr 2015 14:43:05 +0000 Subject: [PATCH 4/7] Fix allowed_units param name --- hooks/neutron_api_hooks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hooks/neutron_api_hooks.py b/hooks/neutron_api_hooks.py index 1b96751b..c632251a 100755 --- a/hooks/neutron_api_hooks.py +++ b/hooks/neutron_api_hooks.py @@ -105,7 +105,7 @@ def conditional_neutron_migration(): if clustered: if is_leader(CLUSTER_RES): - allowed_units = relation_get('neutron_allowed_units') + allowed_units = relation_get('allowed_units') if allowed_units and local_unit() in allowed_units.split(): migrate_neutron_database() service_restart('neutron-server') From 25a505a9869421600033d16c9879579a70e4e287 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Wed, 22 Apr 2015 18:50:48 +0000 Subject: [PATCH 5/7] Add unit tests and fix up conditional_neutron_migration --- hooks/neutron_api_hooks.py | 22 ++++----- hooks/neutron_api_utils.py | 1 + unit_tests/test_neutron_api_hooks.py | 63 ++++++++++++++++++++++++- unit_tests/test_neutron_api_utils.py | 69 ++++++++++++++++++++++++++-- 4 files changed, 136 insertions(+), 19 deletions(-) diff --git a/hooks/neutron_api_hooks.py b/hooks/neutron_api_hooks.py index c632251a..aef2192a 100755 --- a/hooks/neutron_api_hooks.py +++ b/hooks/neutron_api_hooks.py @@ -70,7 +70,7 @@ from neutron_api_context import ( from charmhelpers.contrib.hahelpers.cluster import ( get_hacluster_config, - is_leader, + is_elected_leader, ) from charmhelpers.payload.execd import execd_preinstall @@ -97,26 +97,20 @@ CONFIGS = register_configs() def conditional_neutron_migration(): - clustered = relation_get('clustered') if os_release('neutron-server') < 'kilo': log('Not running neutron database migration as migrations are handled ' 'by the neutron-server process or nova-cloud-controller charm.') return - if clustered: - if is_leader(CLUSTER_RES): - allowed_units = relation_get('allowed_units') - if allowed_units and local_unit() in allowed_units.split(): - migrate_neutron_database() - service_restart('neutron-server') - else: - log('allowed_units either not presented, or local unit ' - 'not in acl list: %s' % repr(allowed_units)) - else: - log('Not running neutron database migration, not leader') - else: + if is_elected_leader(CLUSTER_RES): + allowed_units = relation_get('allowed_units') + if allowed_units and local_unit() not in allowed_units.split(): + log('Allowed_units list provided and this unit not present') + return migrate_neutron_database() service_restart('neutron-server') + else: + log('Not running neutron database migration, not leader') def configure_https(): diff --git a/hooks/neutron_api_utils.py b/hooks/neutron_api_utils.py index f872fcff..e7ed1812 100644 --- a/hooks/neutron_api_utils.py +++ b/hooks/neutron_api_utils.py @@ -260,6 +260,7 @@ def do_openstack_upgrade(configs): :param configs: The charms main OSConfigRenderer object. """ cur_os_rel = os_release('neutron-server') + print cur_os_rel new_src = config('openstack-origin') new_os_rel = get_os_codename_install_source(new_src) diff --git a/unit_tests/test_neutron_api_hooks.py b/unit_tests/test_neutron_api_hooks.py index 4a7c045f..745fb42e 100644 --- a/unit_tests/test_neutron_api_hooks.py +++ b/unit_tests/test_neutron_api_hooks.py @@ -34,6 +34,7 @@ TO_PATCH = [ 'determine_ports', 'do_openstack_upgrade', 'dvr_router_present', + 'local_unit', 'l3ha_router_present', 'execd_preinstall', 'filter_installed_packages', @@ -42,14 +43,18 @@ TO_PATCH = [ 'get_l2population', 'get_overlay_network_type', 'git_install', + 'is_elected_leader', 'is_relation_made', 'log', + 'migrate_neutron_database', 'open_port', 'openstack_upgrade_available', + 'os_release', 'os_requires_version', 'relation_get', 'relation_ids', 'relation_set', + 'service_restart', 'unit_get', 'get_iface_for_address', 'get_netmask_for_address', @@ -268,19 +273,23 @@ class NeutronAPIHooksTests(CharmTestCase): 'Attempting to associate a postgresql database when' ' there is already associated a mysql one') - def test_shared_db_changed(self): + @patch.object(hooks, 'conditional_neutron_migration') + 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) - def test_pgsql_db_changed(self): + @patch.object(hooks, 'conditional_neutron_migration') + 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') @@ -644,3 +653,53 @@ class NeutronAPIHooksTests(CharmTestCase): call('service', 'apache2', 'reload')] self.check_call.assert_called_has_calls(calls) self.assertTrue(_id_rel_joined.called) + + def test_conditional_neutron_migration_icehouse(self): + 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 or nova-cloud-controller charm.' + ) + + def test_conditional_neutron_migration_ncc_rel_leader_juno(self): + self.test_relation.set({ + 'allowed_units': 'neutron-api/0 neutron-api/1 neutron-api/4', + }) + self.local_unit.return_value = 'neutron-api/1' + self.is_elected_leader.return_value = True + self.os_release.return_value = 'juno' + hooks.conditional_neutron_migration() + self.log.assert_called_with( + 'Not running neutron database migration as migrations are handled' + ' by the neutron-server process or nova-cloud-controller charm.' + ) + + def test_conditional_neutron_migration_ncc_rel_leader_kilo(self): + self.test_relation.set({ + 'allowed_units': 'neutron-api/0 neutron-api/1 neutron-api/4', + }) + self.local_unit.return_value = 'neutron-api/1' + self.is_elected_leader.return_value = True + 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_ncc_rel_notleader(self): + self.is_elected_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 as migrations are handled ' + 'by the neutron-server process or nova-cloud-controller charm.' + ) + + def test_conditional_neutron_migration_not_clustered(self): + self.relation_ids.return_value = ['nova-cc/o'] + 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') diff --git a/unit_tests/test_neutron_api_utils.py b/unit_tests/test_neutron_api_utils.py index b5760c0c..9e2f214d 100644 --- a/unit_tests/test_neutron_api_utils.py +++ b/unit_tests/test_neutron_api_utils.py @@ -3,6 +3,7 @@ from mock import MagicMock, patch, call from collections import OrderedDict from copy import deepcopy import charmhelpers.contrib.openstack.templating as templating +import charmhelpers.contrib.openstack.utils templating.OSConfigRenderer = MagicMock() @@ -29,6 +30,7 @@ TO_PATCH = [ 'log', 'neutron_plugin_attribute', 'os_release', + 'subprocess', ] openstack_origin_git = \ @@ -63,7 +65,6 @@ class TestNeutronAPIUtils(CharmTestCase): self.config.side_effect = self.test_config.get self.test_config.set('region', 'region101') self.neutron_plugin_attribute.side_effect = _mock_npa - self.os_release.side_effect = 'trusty' def tearDown(self): # Reset cached cache @@ -172,15 +173,19 @@ 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') @patch.object(nutils, 'git_install_requested') - def test_do_openstack_upgrade(self, git_requested): + def test_do_openstack_upgrade_juno(self, git_requested, + stamp_neutron_db, migrate_neutron_db): git_requested.return_value = False self.config.side_effect = self.test_config.get self.test_config.set('openstack-origin', 'cloud:trusty-juno') - self.os_release.side_effect = 'icehouse' + self.os_release.return_value = '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' @@ -199,6 +204,46 @@ class TestNeutronAPIUtils(CharmTestCase): options=dpkg_opts, fatal=True) configs.set_release.assert_called_with(openstack_release='juno') + self.assertItemsEqual(stamp_neutron_db.call_args_list, []) + self.assertItemsEqual(migrate_neutron_db.call_args_list, []) + + @patch.object(charmhelpers.contrib.openstack.utils, + 'get_os_codename_install_source') + @patch.object(nutils, 'migrate_neutron_database') + @patch.object(nutils, 'stamp_neutron_database') + @patch.object(nutils, 'git_install_requested') + def test_do_openstack_upgrade_kilo(self, git_requested, + stamp_neutron_db, migrate_neutron_db, + gsrc): + git_requested.return_value = False + self.os_release.return_value = 'juno' + self.config.side_effect = self.test_config.get + self.test_config.set('openstack-origin', 'cloud:trusty-kilo') + gsrc.return_value = 'kilo' + self.get_os_codename_install_source.return_value = 'kilo' + 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-kilo' + ) + self.apt_update.assert_called_with(fatal=True) + dpkg_opts = [ + '--option', 'Dpkg::Options::=--force-confnew', + '--option', 'Dpkg::Options::=--force-confdef', + ] + self.apt_upgrade.assert_called_with(options=dpkg_opts, + fatal=True, + dist=True) + pkgs = nutils.determine_packages() + pkgs.sort() + self.apt_install.assert_called_with(packages=pkgs, + options=dpkg_opts, + fatal=True) + configs.set_release.assert_called_with(openstack_release='kilo') + stamp_neutron_db.assert_called_with('juno') + migrate_neutron_db.assert_called_with() @patch.object(nutils, 'git_install_requested') @patch.object(nutils, 'git_clone_and_install') @@ -276,3 +321,21 @@ class TestNeutronAPIUtils(CharmTestCase): call('neutron-server'), ] self.assertEquals(service_restart.call_args_list, expected) + + 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) From 52a55ec0e89e9a11de2830ea71239f6946f03617 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Thu, 23 Apr 2015 09:49:03 +0100 Subject: [PATCH 6/7] Only run migration if allowed units is present --- hooks/neutron_api_hooks.py | 10 ++++++---- unit_tests/test_neutron_api_hooks.py | 7 ------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/hooks/neutron_api_hooks.py b/hooks/neutron_api_hooks.py index e65104d3..fde9e188 100755 --- a/hooks/neutron_api_hooks.py +++ b/hooks/neutron_api_hooks.py @@ -105,11 +105,13 @@ def conditional_neutron_migration(): if is_elected_leader(CLUSTER_RES): allowed_units = relation_get('allowed_units') - if allowed_units and local_unit() not in allowed_units.split(): - log('Allowed_units list provided and this unit not present') + if allowed_units and local_unit() in allowed_units.split(): + migrate_neutron_database() + service_restart('neutron-server') + else: + log('Not running neutron database migration, either no' + ' allowed_units or this unit is not present') return - migrate_neutron_database() - service_restart('neutron-server') else: log('Not running neutron database migration, not leader') diff --git a/unit_tests/test_neutron_api_hooks.py b/unit_tests/test_neutron_api_hooks.py index 85cde0be..70ee610c 100644 --- a/unit_tests/test_neutron_api_hooks.py +++ b/unit_tests/test_neutron_api_hooks.py @@ -720,10 +720,3 @@ class NeutronAPIHooksTests(CharmTestCase): 'Not running neutron database migration as migrations are handled ' 'by the neutron-server process or nova-cloud-controller charm.' ) - - def test_conditional_neutron_migration_not_clustered(self): - self.relation_ids.return_value = ['nova-cc/o'] - 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') From 14cf554341fcecacb6c2823de4b3cb28fe78cf6c Mon Sep 17 00:00:00 2001 From: Liam Young Date: Thu, 23 Apr 2015 09:50:42 +0100 Subject: [PATCH 7/7] Remove erronious print --- hooks/neutron_api_utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/hooks/neutron_api_utils.py b/hooks/neutron_api_utils.py index bbde73fb..433a1d72 100644 --- a/hooks/neutron_api_utils.py +++ b/hooks/neutron_api_utils.py @@ -260,7 +260,6 @@ def do_openstack_upgrade(configs): :param configs: The charms main OSConfigRenderer object. """ cur_os_rel = os_release('neutron-server') - print cur_os_rel new_src = config('openstack-origin') new_os_rel = get_os_codename_install_source(new_src)