Merge lp:~gnuoy/charms/trusty/neutron-api/fix-neutron-query into lp:~openstack-charmers-archive/charms/trusty/neutron-api/next

[Liam Young]

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #3682 neutron-api-next for gnuoy mp256889
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/3682/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #3470 neutron-api-next for gnuoy mp256889
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/3470/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #3476 neutron-api-next for gnuoy mp256889
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [test] Error 1
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/10860721/
Build: http://10.245.162.77:8080/job/charm_amulet_test/3476/

[James Page (james-page)]

review: Approve

=== modified file 'hooks/neutron_api_hooks.py'
--- hooks/neutron_api_hooks.py	2015-04-20 14:11:30 +0000
+++ hooks/neutron_api_hooks.py	2015-04-21 08:47:17 +0000
@@ -49,6 +49,7 @@
     git_install,
     dvr_router_present,
     l3ha_router_present,
+    neutron_ready,
     register_configs,
     restart_map,
     services,
@@ -131,16 +132,19 @@
 @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'),

=== modified file 'hooks/neutron_api_utils.py'
--- hooks/neutron_api_utils.py	2015-04-17 12:12:02 +0000
+++ hooks/neutron_api_utils.py	2015-04-21 08:47:17 +0000
@@ -307,8 +307,8 @@
         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 @@
                                    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 @@
 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():

=== modified file 'tests/charmhelpers/contrib/openstack/amulet/deployment.py'
--- tests/charmhelpers/contrib/openstack/amulet/deployment.py	2015-04-20 19:13:34 +0000
+++ tests/charmhelpers/contrib/openstack/amulet/deployment.py	2015-04-21 08:47:17 +0000
@@ -109,7 +109,7 @@
         # 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 = {

=== modified file 'unit_tests/test_neutron_api_hooks.py'
--- unit_tests/test_neutron_api_hooks.py	2015-04-02 20:15:28 +0000
+++ unit_tests/test_neutron_api_hooks.py	2015-04-21 08:47:17 +0000
@@ -44,6 +44,7 @@
     'git_install',
     'is_relation_made',
     'log',
+    'neutron_ready',
     'open_port',
     'openstack_upgrade_available',
     'os_requires_version',
@@ -147,6 +148,7 @@
     @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 @@
         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

=== modified file 'unit_tests/test_neutron_api_utils.py'
--- unit_tests/test_neutron_api_utils.py	2015-04-17 12:12:02 +0000
+++ unit_tests/test_neutron_api_utils.py	2015-04-21 08:47:17 +0000
@@ -3,6 +3,7 @@
 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 @@
     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 @@
                                             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')