Merge lp:~james-page/charms/trusty/nova-cloud-controller/service-guard-stable into lp:~openstack-charmers-archive/charms/trusty/nova-cloud-controller/trunk

[James Page]

Backport support for service-guard option to ensure that services are not
started until all required interfaces have been related and fully configured.

[Adam Collard (adam-collard)]

A few style comments inline below, nothing blocking :)

[Adam Collard (adam-collard)]

Tested successfully fixes the bugs in Landscape that were caused by having services run prematurely ( #1344413 and #1347986).

review: Approve

=== modified file 'config.yaml'
--- config.yaml	2014-06-17 10:01:21 +0000
+++ config.yaml	2014-07-29 15:06:19 +0000
@@ -165,3 +165,22 @@
     description: |
       This is uuid of the default NVP/NSX L3 Gateway Service.
   # end of NVP/NSX configuration
+  service-guard:
+    type: boolean
+    default: false
+    description: |
+      Ensure required relations are made and complete before allowing services
+      to be started
+      .
+      By default, services may be up and accepting API request from install
+      onwards.
+      .
+      Enabling this flag ensures that services will not be started until the
+      minimum 'core relations' have been made between this charm and other
+      charms.
+      .
+      For this charm the following relations must be made:
+      .
+        * shared-db or (pgsql-nova-db, pgsql-neutron-db)
+        * amqp
+        * identity-service

=== modified file 'hooks/nova_cc_hooks.py'
--- hooks/nova_cc_hooks.py	2014-04-11 16:41:42 +0000
+++ hooks/nova_cc_hooks.py	2014-07-29 15:06:19 +0000
@@ -63,7 +63,9 @@
     NOVA_CONF,
     QUANTUM_CONF,
     NEUTRON_CONF,
-    QUANTUM_API_PASTE
+    QUANTUM_API_PASTE,
+    service_guard,
+    guard_map,
 )
 
 from charmhelpers.contrib.hahelpers.cluster import (
@@ -96,6 +98,8 @@
 
 
 @hooks.hook('config-changed')
+@service_guard(guard_map(), CONFIGS,
+               active=config('service-guard'))
 @restart_on_change(restart_map(), stopstart=True)
 def config_changed():
     global CONFIGS
@@ -114,6 +118,8 @@
 
 @hooks.hook('amqp-relation-changed')
 @hooks.hook('amqp-relation-departed')
+@service_guard(guard_map(), CONFIGS,
+               active=config('service-guard'))
 @restart_on_change(restart_map())
 def amqp_changed():
     if 'amqp' not in CONFIGS.complete_contexts():
@@ -171,6 +177,8 @@
 
 
 @hooks.hook('shared-db-relation-changed')
+@service_guard(guard_map(), CONFIGS,
+               active=config('service-guard'))
 @restart_on_change(restart_map())
 def db_changed():
     if 'shared-db' not in CONFIGS.complete_contexts():
@@ -186,6 +194,8 @@
 
 
 @hooks.hook('pgsql-nova-db-relation-changed')
+@service_guard(guard_map(), CONFIGS,
+               active=config('service-guard'))
 @restart_on_change(restart_map())
 def postgresql_nova_db_changed():
     if 'pgsql-nova-db' not in CONFIGS.complete_contexts():
@@ -201,6 +211,8 @@
 
 
 @hooks.hook('pgsql-neutron-db-relation-changed')
+@service_guard(guard_map(), CONFIGS,
+               active=config('service-guard'))
 @restart_on_change(restart_map())
 def postgresql_neutron_db_changed():
     if network_manager() in ['neutron', 'quantum']:
@@ -210,6 +222,8 @@
 
 
 @hooks.hook('image-service-relation-changed')
+@service_guard(guard_map(), CONFIGS,
+               active=config('service-guard'))
 @restart_on_change(restart_map())
 def image_service_changed():
     if 'image-service' not in CONFIGS.complete_contexts():
@@ -228,6 +242,8 @@
 
 
 @hooks.hook('identity-service-relation-changed')
+@service_guard(guard_map(), CONFIGS,
+               active=config('service-guard'))
 @restart_on_change(restart_map())
 def identity_changed():
     if 'identity-service' not in CONFIGS.complete_contexts():
@@ -249,6 +265,8 @@
 
 @hooks.hook('nova-volume-service-relation-joined',
             'cinder-volume-service-relation-joined')
+@service_guard(guard_map(), CONFIGS,
+               active=config('service-guard'))
 @restart_on_change(restart_map())
 def volume_joined():
     CONFIGS.write(NOVA_CONF)
@@ -391,6 +409,8 @@
 
 @hooks.hook('cluster-relation-changed',
             'cluster-relation-departed')
+@service_guard(guard_map(), CONFIGS,
+               active=config('service-guard'))
 @restart_on_change(restart_map(), stopstart=True)
 def cluster_changed():
     CONFIGS.write_all()
@@ -447,6 +467,8 @@
             'pgsql-nova-db-relation-broken',
             'pgsql-neutron-db-relation-broken',
             'quantum-network-service-relation-broken')
+@service_guard(guard_map(), CONFIGS,
+               active=config('service-guard'))
 def relation_broken():
     CONFIGS.write_all()
 
@@ -487,6 +509,8 @@
 
 
 @hooks.hook('nova-vmware-relation-changed')
+@service_guard(guard_map(), CONFIGS,
+               active=config('service-guard'))
 @restart_on_change(restart_map())
 def nova_vmware_relation_changed():
     CONFIGS.write('/etc/nova/nova.conf')

=== modified file 'hooks/nova_cc_utils.py'
--- hooks/nova_cc_utils.py	2014-05-21 10:03:01 +0000
+++ hooks/nova_cc_utils.py	2014-07-29 15:06:19 +0000
@@ -33,12 +33,15 @@
     relation_get,
     relation_ids,
     remote_unit,
+    is_relation_made,
     INFO,
     ERROR,
 )
 
 from charmhelpers.core.host import (
-    service_start
+    service_start,
+    service_stop,
+    service_running
 )
 
 
@@ -695,3 +698,59 @@
     # quantum-plugin config setting can be safely overriden
     # as we only supported OVS in G/neutron
     return config('neutron-plugin') or config('quantum-plugin')
+
+
+def guard_map():
+    '''Map of services and required interfaces that must be present before
+    the service should be allowed to start'''
+    gmap = {}
+    nova_services = deepcopy(BASE_SERVICES)
+    if os_release('nova-common') not in ['essex', 'folsom']:
+        nova_services.append('nova-conductor')
+
+    nova_interfaces = ['identity-service', 'amqp']
+    if relation_ids('pgsql-nova-db'):
+        nova_interfaces.append('pgsql-nova-db')
+    else:
+        nova_interfaces.append('shared-db')
+
+    for svc in nova_services:
+        gmap[svc] = nova_interfaces
+
+    net_manager = network_manager()
+    if net_manager in ['neutron', 'quantum'] and \
+            not is_relation_made('neutron-api'):
+        neutron_interfaces = ['identity-service', 'amqp']
+        if relation_ids('pgsql-neutron-db'):
+            neutron_interfaces.append('pgsql-neutron-db')
+        else:
+            neutron_interfaces.append('shared-db')
+        if network_manager() == 'quantum':
+            gmap['quantum-server'] = neutron_interfaces
+        else:
+            gmap['neutron-server'] = neutron_interfaces
+
+    return gmap
+
+
+def service_guard(guard_map, contexts, active=False):
+    '''Inhibit services in guard_map from running unless
+    required interfaces are found complete in contexts.'''
+    def wrap(f):
+        def wrapped_f(*args):
+            if active is True:
+                incomplete_services = []
+                for svc in guard_map:
+                    for interface in guard_map[svc]:
+                        if interface not in contexts.complete_contexts():
+                            incomplete_services.append(svc)
+                f(*args)
+                for svc in incomplete_services:
+                    if service_running(svc):
+                        log('Service {} has unfulfilled '
+                            'interface requirements, stopping.'.format(svc))
+                        service_stop(svc)
+            else:
+                f(*args)
+        return wrapped_f
+    return wrap

=== modified file 'unit_tests/test_nova_cc_hooks.py'
--- unit_tests/test_nova_cc_hooks.py	2014-05-21 10:03:01 +0000
+++ unit_tests/test_nova_cc_hooks.py	2014-07-29 15:06:19 +0000
@@ -11,7 +11,11 @@
 utils.register_configs = MagicMock()
 utils.restart_map = MagicMock()
 
-import nova_cc_hooks as hooks
+with patch('nova_cc_utils.guard_map') as gmap:
+    with patch('charmhelpers.core.hookenv.config') as config:
+        config.return_value = False
+        gmap.return_value = {}
+        import nova_cc_hooks as hooks
 
 utils.register_configs = _reg
 utils.restart_map = _map

=== modified file 'unit_tests/test_nova_cc_utils.py'
--- unit_tests/test_nova_cc_utils.py	2014-05-02 10:06:23 +0000
+++ unit_tests/test_nova_cc_utils.py	2014-07-29 15:06:19 +0000
@@ -31,10 +31,13 @@
     'os_release',
     'register_configs',
     'relation_ids',
+    'is_relation_made',
     'remote_unit',
     '_save_script_rc',
     'service_start',
-    'services'
+    'services',
+    'service_running',
+    'service_stop'
 ]
 
 SCRIPTRC_ENV_VARS = {
@@ -555,3 +558,115 @@
             utils.do_openstack_upgrade()
             expected = [call('cloud:precise-icehouse')]
             self.assertEquals(_do_openstack_upgrade.call_args_list, expected)
+
+    def test_guard_map_nova(self):
+        self.relation_ids.return_value = []
+        self.os_release.return_value = 'havana'
+        self.assertEqual(
+            {'nova-api-ec2': ['identity-service', 'amqp', 'shared-db'],
+             'nova-api-os-compute': ['identity-service', 'amqp', 'shared-db'],
+             'nova-cert': ['identity-service', 'amqp', 'shared-db'],
+             'nova-conductor': ['identity-service', 'amqp', 'shared-db'],
+             'nova-objectstore': ['identity-service', 'amqp', 'shared-db'],
+             'nova-scheduler': ['identity-service', 'amqp', 'shared-db']},
+            utils.guard_map()
+        )
+        self.os_release.return_value = 'essex'
+        self.assertEqual(
+            {'nova-api-ec2': ['identity-service', 'amqp', 'shared-db'],
+             'nova-api-os-compute': ['identity-service', 'amqp', 'shared-db'],
+             'nova-cert': ['identity-service', 'amqp', 'shared-db'],
+             'nova-objectstore': ['identity-service', 'amqp', 'shared-db'],
+             'nova-scheduler': ['identity-service', 'amqp', 'shared-db']},
+            utils.guard_map()
+        )
+
+    def test_guard_map_neutron(self):
+        self.relation_ids.return_value = []
+        self.network_manager.return_value = 'neutron'
+        self.os_release.return_value = 'icehouse'
+        self.is_relation_made.return_value = False
+        self.assertEqual(
+            {'neutron-server': ['identity-service', 'amqp', 'shared-db'],
+             'nova-api-ec2': ['identity-service', 'amqp', 'shared-db'],
+             'nova-api-os-compute': ['identity-service', 'amqp', 'shared-db'],
+             'nova-cert': ['identity-service', 'amqp', 'shared-db'],
+             'nova-conductor': ['identity-service', 'amqp', 'shared-db'],
+             'nova-objectstore': ['identity-service', 'amqp', 'shared-db'],
+             'nova-scheduler': ['identity-service', 'amqp', 'shared-db'], },
+            utils.guard_map()
+        )
+        self.network_manager.return_value = 'quantum'
+        self.os_release.return_value = 'grizzly'
+        self.assertEqual(
+            {'quantum-server': ['identity-service', 'amqp', 'shared-db'],
+             'nova-api-ec2': ['identity-service', 'amqp', 'shared-db'],
+             'nova-api-os-compute': ['identity-service', 'amqp', 'shared-db'],
+             'nova-cert': ['identity-service', 'amqp', 'shared-db'],
+             'nova-conductor': ['identity-service', 'amqp', 'shared-db'],
+             'nova-objectstore': ['identity-service', 'amqp', 'shared-db'],
+             'nova-scheduler': ['identity-service', 'amqp', 'shared-db'], },
+            utils.guard_map()
+        )
+
+    def test_guard_map_pgsql(self):
+        self.relation_ids.return_value = ['pgsql:1']
+        self.network_manager.return_value = 'neutron'
+        self.is_relation_made.return_value = False
+        self.os_release.return_value = 'icehouse'
+        self.assertEqual(
+            {'neutron-server': ['identity-service', 'amqp',
+                                'pgsql-neutron-db'],
+             'nova-api-ec2': ['identity-service', 'amqp', 'pgsql-nova-db'],
+             'nova-api-os-compute': ['identity-service', 'amqp',
+                                     'pgsql-nova-db'],
+             'nova-cert': ['identity-service', 'amqp', 'pgsql-nova-db'],
+             'nova-conductor': ['identity-service', 'amqp', 'pgsql-nova-db'],
+             'nova-objectstore': ['identity-service', 'amqp',
+                                  'pgsql-nova-db'],
+             'nova-scheduler': ['identity-service', 'amqp',
+                                'pgsql-nova-db'], },
+            utils.guard_map()
+        )
+
+    def test_service_guard_inactive(self):
+        '''Ensure that if disabled, service guards nothing'''
+        contexts = MagicMock()
+
+        @utils.service_guard({'test': ['interfacea', 'interfaceb']},
+                             contexts, False)
+        def dummy_func():
+            pass
+        dummy_func()
+        self.assertFalse(self.service_running.called)
+        self.assertFalse(contexts.complete_contexts.called)
+
+    def test_service_guard_active_guard(self):
+        '''Ensure services with incomplete interfaces are stopped'''
+        contexts = MagicMock()
+        contexts.complete_contexts.return_value = ['interfacea']
+        self.service_running.return_value = True
+
+        @utils.service_guard({'test': ['interfacea', 'interfaceb']},
+                             contexts, True)
+        def dummy_func():
+            pass
+        dummy_func()
+        self.service_running.assert_called_with('test')
+        self.service_stop.assert_called_with('test')
+        self.assertTrue(contexts.complete_contexts.called)
+
+    def test_service_guard_active_release(self):
+        '''Ensure services with complete interfaces are not stopped'''
+        contexts = MagicMock()
+        contexts.complete_contexts.return_value = ['interfacea',
+                                                   'interfaceb']
+
+        @utils.service_guard({'test': ['interfacea', 'interfaceb']},
+                             contexts, True)
+        def dummy_func():
+            pass
+        dummy_func()
+        self.assertFalse(self.service_running.called)
+        self.assertFalse(self.service_stop.called)
+        self.assertTrue(contexts.complete_contexts.called)

=== modified file 'unit_tests/test_utils.py'
--- unit_tests/test_utils.py	2013-11-08 05:41:39 +0000
+++ unit_tests/test_utils.py	2014-07-29 15:06:19 +0000
@@ -82,9 +82,9 @@
         return self.config
 
     def set(self, attr, value):
-            if attr not in self.config:
-                raise KeyError
-            self.config[attr] = value
+        if attr not in self.config:
+            raise KeyError
+        self.config[attr] = value
 
 
 class TestRelation(object):