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

[James Page]

Add support for service-guard configuration to disable services prior to relations being completely formed.

[lp:~james-page/charms/trusty/nova-cloud-controller/service-guard]

95. By James Page on 2014-07-29

Don't add neutron stuff if related to neutron-api charm

96. By James Page on 2014-07-29

Fixup unit tests

97. By James Page on 2014-07-29

Tidy lint

[Liam Young (gnuoy)]

Approved

review: Approve

=== modified file 'config.yaml'
--- config.yaml	2014-07-16 13:42:38 +0000
+++ config.yaml	2014-07-29 13:38:36 +0000
@@ -184,4 +184,22 @@
       192.168.0.0/24)
       .
       This network will be used for public endpoints.
-
+  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-07-25 09:38:56 +0000
+++ hooks/nova_cc_hooks.py	2014-07-29 13:38:36 +0000
@@ -70,7 +70,9 @@
     NOVA_CONF,
     QUANTUM_CONF,
     NEUTRON_CONF,
-    QUANTUM_API_PASTE
+    QUANTUM_API_PASTE,
+    service_guard,
+    guard_map,
 )
 
 from charmhelpers.contrib.hahelpers.cluster import (
@@ -112,6 +114,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
@@ -132,6 +136,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():
@@ -190,6 +196,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():
@@ -205,6 +213,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():
@@ -220,6 +230,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']:
@@ -229,6 +241,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():
@@ -251,6 +265,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():
@@ -274,6 +290,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)
@@ -465,6 +483,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()
@@ -534,6 +554,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()
 
@@ -574,6 +596,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')
@@ -605,6 +629,8 @@
 
 
 @hooks.hook('neutron-api-relation-changed')
+@service_guard(guard_map(), CONFIGS,
+               active=config('service-guard'))
 @restart_on_change(restart_map())
 def neutron_api_relation_changed():
     CONFIGS.write(NOVA_CONF)
@@ -615,6 +641,8 @@
 
 
 @hooks.hook('neutron-api-relation-broken')
+@service_guard(guard_map(), CONFIGS,
+               active=config('service-guard'))
 @restart_on_change(restart_map())
 def neutron_api_relation_broken():
     if os.path.isfile('/etc/init/neutron-server.override'):

=== modified file 'hooks/nova_cc_utils.py'
--- hooks/nova_cc_utils.py	2014-07-25 09:38:56 +0000
+++ hooks/nova_cc_utils.py	2014-07-29 13:38:36 +0000
@@ -40,6 +40,8 @@
 
 from charmhelpers.core.host import (
     service_start,
+    service_stop,
+    service_running
 )
 
 import nova_cc_context
@@ -753,3 +755,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-07-16 09:45:44 +0000
+++ unit_tests/test_nova_cc_hooks.py	2014-07-29 13:38:36 +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-07-25 09:38:56 +0000
+++ unit_tests/test_nova_cc_utils.py	2014-07-29 13:38:36 +0000
@@ -35,7 +35,9 @@
     'remote_unit',
     '_save_script_rc',
     'service_start',
-    'services'
+    'services',
+    'service_running',
+    'service_stop'
 ]
 
 SCRIPTRC_ENV_VARS = {
@@ -596,3 +598,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 13:38:36 +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):