From 88701a54458fec7f69dea325c01ebb274fe8daff Mon Sep 17 00:00:00 2001 From: Liam Young Date: Tue, 21 Apr 2015 08:46:00 +0000 Subject: [PATCH] Test that neutron can be queried before checking dvr and l3ha settings. Add unit tests for this change and a few additional tests to improve l3ha and dvr coverage --- hooks/neutron_api_hooks.py | 24 ++-- hooks/neutron_api_utils.py | 25 +++- .../contrib/openstack/amulet/deployment.py | 2 +- unit_tests/test_neutron_api_hooks.py | 24 ++++ unit_tests/test_neutron_api_utils.py | 126 ++++++++++++++++++ 5 files changed, 188 insertions(+), 13 deletions(-) diff --git a/hooks/neutron_api_hooks.py b/hooks/neutron_api_hooks.py index d1c8cee9..b058ead5 100755 --- a/hooks/neutron_api_hooks.py +++ b/hooks/neutron_api_hooks.py @@ -49,6 +49,7 @@ from neutron_api_utils import ( git_install, dvr_router_present, l3ha_router_present, + neutron_ready, register_configs, restart_map, services, @@ -131,16 +132,19 @@ def install(): @hooks.hook('config-changed') @restart_on_change(restart_map(), stopstart=True) def config_changed(): - if l3ha_router_present() and not get_l3ha(): - e = ('Cannot disable Router HA while ha enabled routers exist. Please' - ' remove any ha routers') - log(e, level=ERROR) - raise Exception(e) - if dvr_router_present() and not get_dvr(): - e = ('Cannot disable dvr while dvr enabled routers exist. Please' - ' remove any distributed routers') - log(e, level=ERROR) - raise Exception(e) + # If neutron is ready to be queried then check for incompatability between + # existing neutron objects and charm settings + if neutron_ready(): + if l3ha_router_present() and not get_l3ha(): + e = ('Cannot disable Router HA while ha enabled routers exist.' + ' Please remove any ha routers') + log(e, level=ERROR) + raise Exception(e) + if dvr_router_present() and not get_dvr(): + e = ('Cannot disable dvr while dvr enabled routers exist. Please' + ' remove any distributed routers') + log(e, level=ERROR) + raise Exception(e) if config('prefer-ipv6'): setup_ipv6() sync_db_with_multi_ipv6_addresses(config('database'), diff --git a/hooks/neutron_api_utils.py b/hooks/neutron_api_utils.py index 0739e33e..73af522a 100644 --- a/hooks/neutron_api_utils.py +++ b/hooks/neutron_api_utils.py @@ -307,8 +307,8 @@ def setup_ipv6(): apt_install('haproxy/trusty-backports', fatal=True) -def router_feature_present(feature): - ''' Check For dvr enabled routers ''' +def get_neutron_client(): + ''' Return a neutron client if possible ''' env = neutron_api_context.IdentityServiceContext()() if not env: log('Unable to check resources at this time') @@ -322,6 +322,12 @@ def router_feature_present(feature): tenant_name=env['admin_tenant_name'], auth_url=auth_url, region_name=env['region']) + return neutron_client + + +def router_feature_present(feature): + ''' Check For dvr enabled routers ''' + neutron_client = get_neutron_client() for router in neutron_client.list_routers()['routers']: if router.get(feature, False): return True @@ -332,6 +338,21 @@ l3ha_router_present = partial(router_feature_present, feature='ha') dvr_router_present = partial(router_feature_present, feature='distributed') +def neutron_ready(): + ''' Check if neutron is ready by running arbitrary query''' + neutron_client = get_neutron_client() + if not neutron_client: + log('No neutron client, neutron not ready') + return False + try: + neutron_client.list_routers() + log('neutron client ready') + return True + except: + log('neutron query failed, neutron not ready ') + return False + + def git_install(projects_yaml): """Perform setup, and install git repos specified in yaml parameter.""" if git_install_requested(): diff --git a/tests/charmhelpers/contrib/openstack/amulet/deployment.py b/tests/charmhelpers/contrib/openstack/amulet/deployment.py index 4538e961..461a702f 100644 --- a/tests/charmhelpers/contrib/openstack/amulet/deployment.py +++ b/tests/charmhelpers/contrib/openstack/amulet/deployment.py @@ -109,7 +109,7 @@ class OpenStackAmuletDeployment(AmuletDeployment): # Must be ordered by OpenStack release (not by Ubuntu release): (self.precise_essex, self.precise_folsom, self.precise_grizzly, self.precise_havana, self.precise_icehouse, - self.trusty_icehouse, self.trusty_juno, self.utopic_juno, + self.trusty_icehouse, self.trusty_juno, self.utopic_juno, self.trusty_kilo, self.vivid_kilo) = range(10) releases = { diff --git a/unit_tests/test_neutron_api_hooks.py b/unit_tests/test_neutron_api_hooks.py index 4a7c045f..9f2e28c9 100644 --- a/unit_tests/test_neutron_api_hooks.py +++ b/unit_tests/test_neutron_api_hooks.py @@ -44,6 +44,7 @@ TO_PATCH = [ 'git_install', 'is_relation_made', 'log', + 'neutron_ready', 'open_port', 'openstack_upgrade_available', 'os_requires_version', @@ -147,6 +148,7 @@ class NeutronAPIHooksTests(CharmTestCase): @patch.object(hooks, 'git_install_requested') def test_config_changed(self, git_requested, conf_https): git_requested.return_value = False + self.neutron_ready.return_value = True self.openstack_upgrade_available.return_value = True self.dvr_router_present.return_value = False self.l3ha_router_present.return_value = False @@ -169,12 +171,34 @@ class NeutronAPIHooksTests(CharmTestCase): self.assertTrue(self.do_openstack_upgrade.called) self.assertTrue(self.apt_install.called) + def test_config_changed_nodvr_disprouters(self): + self.neutron_ready.return_value = True + self.dvr_router_present.return_value = True + self.get_dvr.return_value = False + with self.assertRaises(Exception) as context: + self._call_hook('config-changed') + self.assertEqual(context.exception.message, + 'Cannot disable dvr while dvr enabled routers exist.' + ' Please remove any distributed routers') + + def test_config_changed_nol3ha_harouters(self): + self.neutron_ready.return_value = True + self.dvr_router_present.return_value = False + self.l3ha_router_present.return_value = True + self.get_l3ha.return_value = False + with self.assertRaises(Exception) as context: + self._call_hook('config-changed') + self.assertEqual(context.exception.message, + 'Cannot disable Router HA while ha enabled routers' + ' exist. Please remove any ha routers') + @patch.object(hooks, 'configure_https') @patch.object(hooks, 'git_install_requested') @patch.object(hooks, 'config_value_changed') def test_config_changed_git(self, config_val_changed, git_requested, configure_https): git_requested.return_value = True + self.neutron_ready.return_value = True self.dvr_router_present.return_value = False self.l3ha_router_present.return_value = False self.relation_ids.side_effect = self._fake_relids diff --git a/unit_tests/test_neutron_api_utils.py b/unit_tests/test_neutron_api_utils.py index b5760c0c..10678183 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 neutron_api_context as ncontext templating.OSConfigRenderer = MagicMock() @@ -57,6 +58,15 @@ def _mock_npa(plugin, attr, net_manager=None): return plugins[plugin][attr] +class DummyIdentityServiceContext(): + + def __init__(self, return_value): + self.return_value = return_value + + def __call__(self): + return self.return_value + + class TestNeutronAPIUtils(CharmTestCase): def setUp(self): super(TestNeutronAPIUtils, self).setUp(nutils, TO_PATCH) @@ -200,6 +210,122 @@ class TestNeutronAPIUtils(CharmTestCase): fatal=True) configs.set_release.assert_called_with(openstack_release='juno') + @patch.object(ncontext, 'IdentityServiceContext') + @patch('neutronclient.v2_0.client.Client') + def test_get_neutron_client(self, nclient, IdentityServiceContext): + creds = { + 'auth_protocol': 'http', + 'auth_host': 'myhost', + 'auth_port': '2222', + 'admin_user': 'bob', + 'admin_password': 'pa55w0rd', + 'admin_tenant_name': 'tenant1', + 'region': 'region2', + } + IdentityServiceContext.return_value = \ + DummyIdentityServiceContext(return_value=creds) + nutils.get_neutron_client() + nclient.assert_called_with( + username='bob', + tenant_name='tenant1', + password='pa55w0rd', + auth_url='http://myhost:2222/v2.0', + region_name='region2', + ) + + @patch.object(ncontext, 'IdentityServiceContext') + def test_get_neutron_client_noidservice(self, IdentityServiceContext): + creds = {} + IdentityServiceContext.return_value = \ + DummyIdentityServiceContext(return_value=creds) + self.assertEquals(nutils.get_neutron_client(), None) + + @patch.object(nutils, 'get_neutron_client') + def test_router_feature_present_keymissing(self, get_neutron_client): + routers = { + 'routers': [ + { + u'status': u'ACTIVE', + u'external_gateway_info': { + u'network_id': u'eedffb9b-b93e-49c6-9545-47c656c9678e', + u'enable_snat': True + }, u'name': u'provider-router', + u'admin_state_up': True, + u'tenant_id': u'b240d06e38394780a3ea296138cdd174', + u'routes': [], + u'id': u'84182bc8-eede-4564-9c87-1a56bdb26a90', + } + ] + } + get_neutron_client.list_routers.return_value = routers + self.assertEquals(nutils.router_feature_present('ha'), False) + + @patch.object(nutils, 'get_neutron_client') + def test_router_feature_present_keyfalse(self, get_neutron_client): + routers = { + 'routers': [ + { + u'status': u'ACTIVE', + u'external_gateway_info': { + u'network_id': u'eedffb9b-b93e-49c6-9545-47c656c9678e', + u'enable_snat': True + }, u'name': u'provider-router', + u'admin_state_up': True, + u'tenant_id': u'b240d06e38394780a3ea296138cdd174', + u'routes': [], + u'id': u'84182bc8-eede-4564-9c87-1a56bdb26a90', + u'ha': False, + } + ] + } + dummy_client = MagicMock() + dummy_client.list_routers.return_value = routers + get_neutron_client.return_value = dummy_client + self.assertEquals(nutils.router_feature_present('ha'), False) + + @patch.object(nutils, 'get_neutron_client') + def test_router_feature_present_keytrue(self, get_neutron_client): + routers = { + 'routers': [ + { + u'status': u'ACTIVE', + u'external_gateway_info': { + u'network_id': u'eedffb9b-b93e-49c6-9545-47c656c9678e', + u'enable_snat': True + }, u'name': u'provider-router', + u'admin_state_up': True, + u'tenant_id': u'b240d06e38394780a3ea296138cdd174', + u'routes': [], + u'id': u'84182bc8-eede-4564-9c87-1a56bdb26a90', + u'ha': True, + } + ] + } + + dummy_client = MagicMock() + dummy_client.list_routers.return_value = routers + get_neutron_client.return_value = dummy_client + self.assertEquals(nutils.router_feature_present('ha'), True) + + @patch.object(nutils, 'get_neutron_client') + def test_neutron_ready(self, get_neutron_client): + dummy_client = MagicMock() + dummy_client.list_routers.return_value = [] + get_neutron_client.return_value = dummy_client + self.assertEquals(nutils.neutron_ready(), True) + + @patch.object(nutils, 'get_neutron_client') + def test_neutron_ready_noclient(self, get_neutron_client): + get_neutron_client.return_value = None + self.assertEquals(nutils.neutron_ready(), False) + + @patch.object(nutils, 'get_neutron_client') + def test_neutron_ready_clientexception(self, get_neutron_client): + dummy_client = MagicMock() + dummy_client.list_routers.side_effect = Exception('Boom!') + get_neutron_client.return_value = dummy_client + self.assertEquals(nutils.neutron_ready(), False) + @patch.object(nutils, 'git_install_requested') @patch.object(nutils, 'git_clone_and_install') @patch.object(nutils, 'git_post_install')