From 480f5051f0e941b7a7b3a059aa1a5336581d957b Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Fri, 3 Feb 2017 17:50:09 +0000 Subject: [PATCH] Update manila-generic configuration charm for updated plugin i/f The manila-plugin interface has been changed to simplify how backend configuration charms can generate their config for the manila.conf file in the manila principal charm. This change enables the use of a template/Jinja2 templating which simplifies the charm. Change-Id: Id5e909ae352d73239354002abc62e9491c9a42b7 Depends-On: I76866007e3c89bb16bc7985a692fbd8f3e136a71 --- src/lib/charm/openstack/manila_generic.py | 208 +++++------------- src/reactive/manila_generic_handlers.py | 7 +- src/templates/mitaka/manila.conf | 100 +++++++++ src/tests/basic_deployment.py | 3 +- ...test_lib_charm_openstack_manila_generic.py | 133 ++--------- unit_tests/test_manila_generic_handlers.py | 11 +- 6 files changed, 188 insertions(+), 274 deletions(-) create mode 100644 src/templates/mitaka/manila.conf diff --git a/src/lib/charm/openstack/manila_generic.py b/src/lib/charm/openstack/manila_generic.py index 44b0545..8db5a54 100644 --- a/src/lib/charm/openstack/manila_generic.py +++ b/src/lib/charm/openstack/manila_generic.py @@ -20,9 +20,12 @@ from __future__ import absolute_import import os import textwrap +import charmhelpers.contrib.openstack.templating as os_templating import charmhelpers.core.hookenv as hookenv +import charmhelpers.core.templating import charms_openstack.charm import charms_openstack.adapters +import charms.reactive # There are no additional packages to install. PACKAGES = [] @@ -80,6 +83,31 @@ def computed_debug_level(config): return "WARNING" +# Work-around charms.openstack non ability to expose a property on the +# charms.reactive relation to the adapter. it would work if it was a function, +# but sadly not for a property. +@charms_openstack.adapters.adapter_property('manila-plugin') +def authentication_data(manila_plugin): + """Return the authentication dictionary for use in the manila.conf template + + The authentication data format is: + { + 'username': + 'password': + 'project_domain_id': + 'project_name': + 'user_domain_id': + 'auth_uri': + 'auth_url': + 'auth_type': # 'password', typically + } + + :param manila_plugin: the charms.reactive relation instance. + :returns: dict described above + """ + return manila_plugin.relation.authentication_data + + ### # Implementation of the Manila Charm classes @@ -99,13 +127,16 @@ class ManilaGenericCharm(charms_openstack.charm.OpenStackCharm): default_service = None # There is no service for this charm. services = [] - required_relations = [] + required_relations = ['manila-plugin', ] restart_map = {} # This is the command to sync the database sync_cmd = [] + # TODO: remove this when the charms.openstack fix lands + adapters_class = charms_openstack.adapters.OpenStackRelationAdapters + def custom_assess_status_check(self): """Validate that the driver configuration is at least complete, and that it was valid when it used (either at configuration time or config @@ -137,172 +168,51 @@ class ManilaGenericCharm(charms_openstack.charm.OpenStackCharm): """Assuming that the configuration data is valid, return the configuration data for the principal charm. - The format of the returned data is: + The format of the complete returned data is: { - "complete": , - '': { - '
: ( - (key, value), - (key, value), - ) + ": } If the configuration is not complete, or we don't have auth data from - the principal charm, then we return: - { - "complete": false, - "reason": - } + the principal charm, then we return and emtpy dictionary {} :param auth_data: the raw dictionary received from the principal charm :returns: structure described above. """ + # If there is no auth_data yet, then we can't write our config. if not auth_data: - return {"complete": False, "reason": "No authentication data"} + return {} + # If the state from the assess_status is not None then we're blocked, + # so don't send any config to the principal. state, message = self.custom_assess_status_check() if state: - return {"complete": False, "reason": message} + return {} options = self.options # tiny optimisation for less typing. - # We have the auth data & the config is reasonably sensible. + + # If there is no backend name, then we can't send the data yet as the + # manila-charm won't know what to do with it. if not options.share_backend_name: - return {"complete": False, - "reason": "Problem: share-backend-name is not set"} + return {} - # if the driver is not going to handle the share servers then we only - # need a very simple config section - if not options.driver_handles_share_servers: - generic_section = self.process_lines(( - "# Set usage of Generic driver which uses cinder as backend.", - "share_driver = " - "manila.share.drivers.generic.GenericShareDriver", - "", - "# Generic driver supports both driver modes - " - "with and without handling", - "# of share servers. So, we need to define explicitly which " - "one we are", - "# enabling using this driver.", - "driver_handles_share_servers = False", - "# Custom name for share backend.", - ("share_backend_name", options.share_backend_name), - "# Generic driver seems to insist on 'service_instance_user' " - "even if it isn't using it", - ("service_instance_user", - options.driver_service_instance_user))) - return { - "complete": True, - MANILA_CONF: { - "[{}]".format(options.share_backend_name): generic_section, - }, - } - - # we use the same username/password/auth for each section as every - # service user has then same permissions as admin. - auth_section = self.process_lines(( - "# Only needed for the generic drivers as of Mitaka", - ('username', auth_data['username']), - ('password', auth_data['password']), - ('project_domain_id', auth_data['project_domain_id']), - ('project_name', auth_data['project_name']), - ('user_domain_id', auth_data['user_domain_id']), - ('auth_uri', auth_data['auth_uri']), - ('auth_url', auth_data['auth_url']), - ('auth_type', auth_data['auth_type']))) - - # Expression is True if the generic driver should use a password rather - # than an ssh key. - if options.computed_use_password: - service_instance_password = ( - "service_instance_password", - options.driver_service_instance_password) - else: - service_instance_password = "# No generic password section" - - # Expression is True if the generic driver should use a password rather - # than an ssh key. - if options.computed_use_ssh: - ssh_section = tuple(self.process_lines(( - ("path_to_private_key", MANILA_SSH_KEY_PATH), - ("path_to_public_key", MANILA_SSH_KEY_PATH_PUBLIC), - ("manila_service_keypair_name", - options.driver_keypair_name)))) - else: - ssh_section = ("# No ssh section", ) - - # And finally configure the generic section - generic_section = self.process_lines(( - "# Set usage of Generic driver which uses cinder as backend.", - "share_driver = manila.share.drivers.generic.GenericShareDriver", - "", - "# Generic driver supports both driver modes - " - "with and without handling", - "# of share servers. So, we need to define explicitly which one " - "we are", - "# enabling using this driver.", - ("driver_handles_share_servers", - options.driver_handles_share_servers), - "", - "# The flavor that Manila will use to launch the instance.", - ("service_instance_flavor_id", - options.driver_service_instance_flavor_id), - "", - "# Generic driver uses a glance image for building service VMs " - "in nova.", - "# The following options specify the image to use.", - "# We use the latest build of [1].", - "# [1] https://github.com/openstack/manila-image-elements", - ("service_instance_user", - options.driver_service_instance_user), - ("service_image_name", options.driver_service_image_name), - ("connect_share_server_to_tenant_network", - options.driver_connect_share_server_to_tenant_network), - "", - "# These will be used for keypair creation and inserted into", - "# service VMs.", - "# TODO: this presents a problem with HA and failover - as the" - "keys", - "# will no longer be the same -- need to be able to set these via", - "# a config option.", - service_instance_password, ) + - ssh_section + - ("", - "# Custom name for share backend.", - ("share_backend_name", options.share_backend_name))) + # We have the auth data & the config is reasonably sensible. + # We can try and render the config file segment. + # TODO this is horrible, and we should have something in + # charms.openstack to do this, but we need a c.r relation to be able to + # add it to the adapters_instance + manila_plugin = charms.reactive.RelationBase.from_state( + 'manila-plugin.available') + self.adapters_instance.add_relation(manila_plugin) + rendered_configs = charmhelpers.core.templating.render( + source=os.path.basename(MANILA_CONF), + template_loader=os_templating.get_loader( + 'templates/', self.release), + target=None, + context=self.adapters_instance) return { - "complete": True, - MANILA_CONF: { - "[nova]": auth_section, - "[neutron]": auth_section, - "[cinder]": auth_section, - "[{}]".format(options.share_backend_name): generic_section, - }, + MANILA_CONF: rendered_configs } - @staticmethod - def process_lines(lines): - """Process each of the lines. If the line is a string, then just - passes it though; if the line is a tuple (and it must be a 2-tuple) - then the string is interpolated with an equals. - - :param lines: list of strings or 2-tuples of strings - :returns: list of strings - """ - out = [] - for line in lines: - if isinstance(line, str): - out.append(line) - elif isinstance(line, (list, tuple)): - if len(line) != 2: - raise TypeError("Line '{}' must be length 2" - .format(line)) - out.append("{} = {}".format(*line)) - # raise an error on other types - else: - raise TypeError("Line '{}' must be a string, tuple or list." - " Passed a {}" - .format(line, type(line))) - return out - def maybe_write_ssh_keys(self): """Maybe write the ssh keys from the options to the key files where manila will be able to find them. The function only writes them if the diff --git a/src/reactive/manila_generic_handlers.py b/src/reactive/manila_generic_handlers.py index fcdfa50..ebd2d4c 100644 --- a/src/reactive/manila_generic_handlers.py +++ b/src/reactive/manila_generic_handlers.py @@ -29,8 +29,9 @@ charms_openstack.charm.use_defaults( 'update-status') -@charms.reactive.when('manila-plugin.available') -@charms.reactive.when_not('config.changed') +@charms.reactive.when('manila-plugin.changed') +@charms.reactive.when_not('config.changed', + 'update-status') def send_config(manila_plugin): """Send the configuration over to the prinicpal charm""" with charms_openstack.charm.provide_charm_instance() as generic_charm: @@ -42,9 +43,11 @@ def send_config(manila_plugin): manila_plugin.authentication_data)) generic_charm.maybe_write_ssh_keys() generic_charm.assess_status() + manila_plugin.clear_changed() @charms.reactive.when('manila-plugin.available', 'config.changed') +@charms.reactive.when_not('update-status') def update_config(manila_plugin): send_config(manila_plugin) diff --git a/src/templates/mitaka/manila.conf b/src/templates/mitaka/manila.conf new file mode 100644 index 0000000..a2194f9 --- /dev/null +++ b/src/templates/mitaka/manila.conf @@ -0,0 +1,100 @@ +{# if the driver is not going to handle the share servers then we only + need a very simple config section +#} +{% if not options.driver_handles_share_servers -%} +[{{ options.share_backend_name }}] +# Set usage of Generic driver which uses cinder as backend. +share_driver = manila.share.drivers.generic.GenericShareDriver + +# Generic driver supports both driver modes - with and without handling +# of share servers. So, we need to define explicitly which one we are +# enabling using this driver. +driver_handles_share_servers = False +# Custom name for share backend. +share_backend_name = {{ options.share_backend_name }} +# Generic driver seems to insist on 'service_instance_user' even if it isn't using it +service_instance_user = {{ options.driver_service_instance_user }} +{% else %} +{# Otherwise we need to do a full specification for the config. + Mitaka needs nova, neutron and cinder sections to enable management of the + shares +#} +# Only needed for the generic drivers as of Mitaka + +[nova] +username = {{ manila_plugin.authentication_data.username }} +password = {{ manila_plugin.authentication_data.password }} +project_domain_id = {{ manila_plugin.authentication_data.project_domain_id }} +project_name = {{ manila_plugin.authentication_data.project_name }} +user_domain_id = {{ manila_plugin.authentication_data.user_domain_id }} +auth_uri = {{ manila_plugin.authentication_data.auth_uri }} +auth_url = {{ manila_plugin.authentication_data.auth_url }} +auth_type = {{ manila_plugin.authentication_data.auth_type }} + + +[neutron] +username = {{ manila_plugin.authentication_data.username }} +password = {{ manila_plugin.authentication_data.password }} +project_domain_id = {{ manila_plugin.authentication_data.project_domain_id }} +project_name = {{ manila_plugin.authentication_data.project_name }} +user_domain_id = {{ manila_plugin.authentication_data.user_domain_id }} +auth_uri = {{ manila_plugin.authentication_data.auth_uri }} +auth_url = {{ manila_plugin.authentication_data.auth_url }} +auth_type = {{ manila_plugin.authentication_data.auth_type }} + + +[cinder] +username = {{ manila_plugin.authentication_data.username }} +password = {{ manila_plugin.authentication_data.password }} +project_domain_id = {{ manila_plugin.authentication_data.project_domain_id }} +project_name = {{ manila_plugin.authentication_data.project_name }} +user_domain_id = {{ manila_plugin.authentication_data.user_domain_id }} +auth_uri = {{ manila_plugin.authentication_data.auth_uri }} +auth_url = {{ manila_plugin.authentication_data.auth_url }} +auth_type = {{ manila_plugin.authentication_data.auth_type }} + + +[{{ options.share_backend_name}}] +# Set usage of Generic driver which uses cinder as backend. +share_driver = manila.share.drivers.generic.GenericShareDriver + +# Generic driver supports both driver modes - with and without handling +# of share servers. So, we need to define explicitly which one we are +# enabling using this driver. +driver_handles_share_servers = True + +# The flavor that Manila will use to launch the instance. +service_instance_flavor_id = {{ options.driver_service_instance_flavor_id }} + +# Generic driver uses a glance image for building service VMs in nova. +# The following options specify the image to use. +# We use the latest build of [1]. +# [1] https://github.com/openstack/manila-image-elements +service_instance_user = {{ options.driver_service_instance_user }} +service_image_name = {{ options.driver_service_image_name }} +connect_share_server_to_tenant_network = {{ options.driver_connect_share_server_to_tenant_network }} + +# These will be used for keypair creation and inserted into +# service VMs. +# TODO: this presents a problem with HA and failover - as the keys +# will no longer be the same -- need to be able to set these via +# a config option. +{# Expression is True if the generic driver should use a password #} +{% if options.computed_use_password %} +service_instance_password = {{ options.driver_service_instance_password }} +{% else -%} +# No generic password section +{% endif %} + +{# Expression is True if the generic driver should use ssh #} +{% if options.computed_use_ssh %} +path_to_private_key = /etc/manila/ssh_image_key +path_to_public_key = /etc/manila/ssh_image_key.pub +manila_service_keypair_name = {{ options.driver_keypair_name }} +{% else %} +# No ssh section +{% endif %} + +# Custom name for share backend. +share_backend_name = {{ options.share_backend_name }} +{% endif %} diff --git a/src/tests/basic_deployment.py b/src/tests/basic_deployment.py index 4aab63d..685c417 100644 --- a/src/tests/basic_deployment.py +++ b/src/tests/basic_deployment.py @@ -63,8 +63,7 @@ class ManilaGenericBasicDeployment(OpenStackAmuletDeployment): 'constraints': {'mem': '3072M'}}, {'name': 'rabbitmq-server'}, {'name': 'keystone'}, - {'name': 'manila', - 'location': 'cs:~openstack-charmers/xenial/manila'} + {'name': 'manila'} ] super(ManilaGenericBasicDeployment, self)._add_services( this_service, other_services) diff --git a/unit_tests/test_lib_charm_openstack_manila_generic.py b/unit_tests/test_lib_charm_openstack_manila_generic.py index e810ce9..94ed4ed 100644 --- a/unit_tests/test_lib_charm_openstack_manila_generic.py +++ b/unit_tests/test_lib_charm_openstack_manila_generic.py @@ -98,6 +98,15 @@ class TestManilaGenericCharmConfigProperties(Helper): self.assertEqual(manila_generic.computed_debug_level(config), "DEBUG") +class TestManilaGenericCharmManilaPluginProperties(Helper): + + def test_authentication_data(self): + manila_plugin = mock.MagicMock() + manila_plugin.relation.authentication_data = 'test data' + self.assertEqual(manila_generic.authentication_data(manila_plugin), + 'test data') + + class TestManilaGenericCharm(Helper): def _patch_config_and_charm(self, config): @@ -156,9 +165,7 @@ class TestManilaGenericCharm(Helper): def test_get_config_for_principal(self): # note that this indirectly tests 'process_lines' as well. c = manila_generic.ManilaGenericCharm() - self.assertEqual( - c.get_config_for_principal(None), - {'complete': False, 'reason': 'No authentication data'}) + self.assertEqual(c.get_config_for_principal(None), {}) # we want to handle share servers to True to check for misconfig config = { 'driver-handles-share-servers': True, @@ -185,9 +192,7 @@ class TestManilaGenericCharm(Helper): 'auth_type': 'type1', } self.maxDiff = None - self.assertEqual( - c.get_config_for_principal(auth_data), - {'complete': False, 'reason': message}) + self.assertEqual(c.get_config_for_principal(auth_data), {}) # now set up the config to be okay to generate the sections config['driver-handles-share-servers'] = True config['driver-service-image-name'] = 'manila' @@ -197,120 +202,10 @@ class TestManilaGenericCharm(Helper): config['driver-keypair-name'] = 'my-keyname' # test that we've set the backend name c = manila_generic.ManilaGenericCharm() - self.assertEqual( - c.get_config_for_principal(auth_data), - {'complete': False, 'reason': - 'Problem: share-backend-name is not set'}) - # now test that we actually generate some config data - config['share-backend-name'] = 'test-backend' - # simplify the output for the next test - config['driver-handles-share-servers'] = False - c = manila_generic.ManilaGenericCharm() - lines = c.get_config_for_principal(auth_data) - # verify that "# No generic password section" is in the lines - conf = manila_generic.MANILA_CONF - self.assertIn(conf, lines) - self.assertIn('[test-backend]', lines[conf]) - section = lines[conf]['[test-backend]'] - self.assertIn('share_driver = ' - 'manila.share.drivers.generic.GenericShareDriver', - section) - self.assertIn('driver_handles_share_servers = False', section) - self.assertIn('share_backend_name = test-backend', section) + self.assertEqual(c.get_config_for_principal(auth_data), {}) - # Now verify that when we switch the driver handles shares on that the - # sections all appear - config['driver-handles-share-servers'] = True - c = manila_generic.ManilaGenericCharm() - lines = c.get_config_for_principal(auth_data) - self.assertIn(conf, lines) - self.assertIn('[test-backend]', lines[conf]) - self.assertIn('[nova]', lines[conf]) - self.assertIn('[neutron]', lines[conf]) - self.assertIn('[cinder]', lines[conf]) - # check each of the nova, neutron and cinder sections (which are all - # identical) - auth_lines = ['# Only needed for the generic drivers as of Mitaka', - 'username = user', - 'password = pass', - 'project_domain_id = pd1', - 'project_name = p1', - 'user_domain_id = ud1', - 'auth_uri = uri1', - 'auth_url = url1', - 'auth_type = type1'] - - for s in ('[nova]', '[neutron]', '[cinder]'): - section = lines[conf][s] - self._verify_section_contains(section, auth_lines) - - # now check the [test-backend] section - section = lines[conf]['[test-backend]'] - self.assertIn('share_driver = ' - 'manila.share.drivers.generic.GenericShareDriver', - section) - self.assertIn('driver_handles_share_servers = True', section) - self.assertIn('share_backend_name = test-backend', section) - self.assertIn('service_instance_flavor_id = 103', section) - self._verify_section_contains( - section, - ['service_instance_user = manila-user', - 'service_image_name = manila', - 'connect_share_server_to_tenant_network = False']) - self._verify_section_contains( - section, - ['# No generic password section', - '# No ssh section', ]) - - # Now switch on the password section - config['driver-auth-type'] = 'password' - c = manila_generic.ManilaGenericCharm() - lines = c.get_config_for_principal(auth_data) - section = lines[conf]['[test-backend]'] - self.assertNotIn('# No generic password section', section) - self.assertIn('service_instance_password = password', section) - - # Now switch on the SSH section - config['driver_service_ssh_key'] = 'ssh-key' - config['driver-service-ssh-key-public'] = 'ssh-key-public' - config['driver-auth-type'] = 'ssh' - c = manila_generic.ManilaGenericCharm() - lines = c.get_config_for_principal(auth_data) - section = lines[conf]['[test-backend]'] - self.assertNotIn('# No ssh section', section) - self.assertIn('# No generic password section', section) - # test for ssh lines - self._verify_section_contains( - section, - ['path_to_private_key = {}' - .format(manila_generic.MANILA_SSH_KEY_PATH), - 'path_to_public_key = {}' - .format(manila_generic.MANILA_SSH_KEY_PATH_PUBLIC), - 'manila_service_keypair_name = my-keyname', ]) - - # Enable the connect_share_to_tenant_network and both password and ssh - config['driver-auth-type'] = 'both' - config['driver-connect-share-server-to-tenant-network'] = True - c = manila_generic.ManilaGenericCharm() - lines = c.get_config_for_principal(auth_data) - section = lines[conf]['[test-backend]'] - self.assertNotIn('# No ssh section', section) - self.assertNotIn('# No generic password section', section) - self.assertIn('service_instance_password = password', section) - # test for ssh lines - self._verify_section_contains( - section, - ['path_to_private_key = {}' - .format(manila_generic.MANILA_SSH_KEY_PATH), - 'path_to_public_key = {}' - .format(manila_generic.MANILA_SSH_KEY_PATH_PUBLIC), - 'manila_service_keypair_name = my-keyname', ]) - self.assertIn('connect_share_server_to_tenant_network = True', section) - - def _verify_section_contains(self, section, lines): - index = section.index(lines[0]) - for i, line in enumerate(lines): - self.assertEqual(section[index + i], line) + # can't currently test that the outputted template is accurate in tests + # as we mock out the templating logic from charmhelpers. def test_maybe_write_ssh_keys(self): config = { diff --git a/unit_tests/test_manila_generic_handlers.py b/unit_tests/test_manila_generic_handlers.py index fbaab9f..e9a8d9d 100644 --- a/unit_tests/test_manila_generic_handlers.py +++ b/unit_tests/test_manila_generic_handlers.py @@ -30,12 +30,13 @@ class TestRegisteredHooks(test_utils.TestRegisteredHooks): 'update-status'] hook_set = { 'when': { - 'send_config': ('manila-plugin.available', ), + 'send_config': ('manila-plugin.changed', ), 'update_config': ('manila-plugin.available', 'config.changed', ), }, 'when_not': { - 'send_config': ('config.changed', ), + 'send_config': ('config.changed', 'update-status'), + 'update_config': ('update-status', ), }, } # test that the hooks were registered via the @@ -64,6 +65,11 @@ class TestHandlerFunctions(test_utils.PatchHelper): configuration_data = None authentication_data = 'auth data' + _clear_changed = 0 + + def clear_changed(self): + self._clear_changed += 1 + generic.get_config_for_principal.return_value = "some data" manila_plugin = FakeManilaPlugin() handlers.send_config(manila_plugin) @@ -75,3 +81,4 @@ class TestHandlerFunctions(test_utils.PatchHelper): generic.get_config_for_principal.assert_called_once_with('auth data') generic.assess_status.assert_called_once_with() generic.maybe_write_ssh_keys.assert_called_once_with() + self.assertEqual(manila_plugin._clear_changed, 1)