From eb6437174a63130b21f2404876ac4e840e9e9314 Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Thu, 8 Dec 2022 14:00:05 -0500 Subject: [PATCH] Ensure that local and remote plugins can be used concurrently To ensure that it works, this change also updates the functional test bundles to include manila-netapp and manila-netapp-dhss in the gate. This change also picks up a drive-by change to fix a tox dependency change. Closes-Bug: #1996962 Change-Id: Ic5a2b2fcced129220b70a16539b41813b0feb3be --- src/lib/charm/openstack/manila.py | 61 +++++++++---------- src/reactive/manila_handlers.py | 27 ++++---- src/test-requirements.txt | 1 - src/tests/bundles/jammy-yoga.yaml | 8 +++ src/tests/bundles/jammy-zed.yaml | 8 +++ src/tests/bundles/kinetic-zed.yaml | 8 +++ src/tests/tests.yaml | 1 - unit_tests/test_lib_charm_openstack_manila.py | 19 +++++- unit_tests/test_manila_handlers.py | 8 ++- 9 files changed, 93 insertions(+), 48 deletions(-) diff --git a/src/lib/charm/openstack/manila.py b/src/lib/charm/openstack/manila.py index cdab48a..700bad7 100644 --- a/src/lib/charm/openstack/manila.py +++ b/src/lib/charm/openstack/manila.py @@ -44,6 +44,8 @@ MANILA_LOGGING_CONF = MANILA_DIR + "logging.conf" MANILA_API_PASTE_CONF = MANILA_DIR + "api-paste.ini" MANILA_WEBSERVER_SITE = 'manila-api' MANILA_WSGI_CONF = '/etc/apache2/sites-available/manila-api.conf' +PLUGIN_RELATIONS = ("manila-plugin.available", + "remote-manila-plugin.available",) # select the default release function and ssl feature charms_openstack.charm.use_defaults('charm.default-select-release') @@ -382,17 +384,15 @@ class ManilaCharm(charms_openstack.charm.HAOpenStackCharm): """Return a list of configured backends that come from the associated 'manila-share.available' state.. - TODO: Note that the first backend that becomes 'available' will set - this state. It's not clear how multiple backends will interact yet! - :returns: list of strings: backend sections that are configured. """ - adapter = self.adapter - if adapter is None: - return [] # adapter.names is a property that provides a list of backend manila # plugin names for the sections - return sorted(list(set(adapter.relation.names))) + + names = [] + for backend_relation in self.adapters: + names.extend(backend_relation.relation.names) + return sorted(list(set(names))) def config_lines_for(self, config_file): """Return the list of configuration lines for `config_file` as returned @@ -413,22 +413,19 @@ class ManilaCharm(charms_openstack.charm.HAOpenStackCharm): :param config_file: string, filename for configuration lines :returns: list of strings: config lines for `config_file` """ - adapter = self.adapter - if adapter is None: - return [] - # get the configuration data for all plugins - config_data = adapter.relation.get_configuration_data() - # make the config_data : {: string} format - inverted_config_data = {} - for name, config_files in config_data.items(): - for file, data in config_files.items(): - if file not in inverted_config_data: - inverted_config_data[file] = {} - inverted_config_data[file][name] = data + config_lines = [] + inverted_config_data = collections.defaultdict(dict) + for adapter in self.adapters: + # get the configuration data for all plugins + config_data = adapter.relation.get_configuration_data() + + # make the config_data : {: string} format + for name, config_files in config_data.items(): + for file, data in config_files.items(): + inverted_config_data[file][name] = data # now see if it's the one we want if config_file not in inverted_config_data: return [] - config_lines = [] for name, chunk in inverted_config_data[config_file].items(): config_lines.append(chunk) config_lines.append('') @@ -440,22 +437,24 @@ class ManilaCharm(charms_openstack.charm.HAOpenStackCharm): :returns: [list of config files] """ - adapter = self.adapter - if adapter is None: - return [] - # get the configuration data for all plugins - config_data = adapter.relation.get_configuration_data() config_files = set() - for name, data in config_data.items(): - for config_file, chunks in data.items(): - config_files.add(config_file) + for adapter in self.adapters: + # get the configuration data for all plugins + config_data = adapter.relation.get_configuration_data() + + for name, data in config_data.items(): + for config_file in data.keys(): + config_files.add(config_file) return list(config_files) @property - def adapter(self): - return self.get_adapter('manila-plugin.available') or \ - self.get_adapter('remote-manila-plugin.available') + def adapters(self): + return [ + adapter + for adapter in map(self.get_adapter, PLUGIN_RELATIONS) + if adapter is not None + ] def enable_webserver_site(self): """Enable Manila API apache2 site if rendered or installed""" diff --git a/src/reactive/manila_handlers.py b/src/reactive/manila_handlers.py index 568713e..b0bd0c4 100644 --- a/src/reactive/manila_handlers.py +++ b/src/reactive/manila_handlers.py @@ -65,9 +65,11 @@ def share_to_manila_plugins_auth(): data. """ keystone = charms.reactive.endpoint_from_flag('identity-service.connected') - manila_plugin = \ - charms.reactive.endpoint_from_flag('manila-plugin.connected') or \ - charms.reactive.endpoint_from_flag('remote-manila-plugin.connected') + manila_plugins = [ + relations.endpoint_from_flag('manila-plugin.connected'), + relations.endpoint_from_flag('remote-manila-plugin.connected') + ] + data = { 'username': keystone.service_username(), 'password': keystone.service_password(), @@ -85,7 +87,9 @@ def share_to_manila_plugins_auth(): 'auth_type': 'password', } # Set the auth data to be the same for all plugins - manila_plugin.set_authentication_data(data) + for manila_plugin in manila_plugins: + if manila_plugin is not None: + manila_plugin.set_authentication_data(data) @charms.reactive.when('shared-db.available', @@ -125,11 +129,12 @@ def render_stuff(*args): manila_charm.render_with_interfaces(args) manila_charm.assess_status() charms.reactive.set_state('manila.config.rendered') - manila_plugin = \ - relations.endpoint_from_flag('manila-plugin.changed') or \ + for manila_plugin in [ + relations.endpoint_from_flag('manila-plugin.changed'), relations.endpoint_from_flag('remote-manila-plugin.changed') - if manila_plugin: - manila_plugin.clear_changed() + ]: + if manila_plugin is not None: + manila_plugin.clear_changed() manila_charm.enable_webserver_site() @@ -163,10 +168,10 @@ def update_status(): """ if not os_utils.is_unit_paused_set(): with charms_openstack.charm.provide_charm_instance() as manila_charm: - if manila_charm.get_adapter('remote-manila-plugin.available'): - services = [] - else: + if manila_charm.get_adapter('manila-plugin.connected'): services = ['manila-share'] + else: + services = [] state, message = os_utils._ows_check_services_running( services=services, ports=None) diff --git a/src/test-requirements.txt b/src/test-requirements.txt index da02a4d..e771023 100644 --- a/src/test-requirements.txt +++ b/src/test-requirements.txt @@ -4,7 +4,6 @@ # https://github.com/openstack-charmers/release-tools # - # Functional Test Requirements (let Zaza's dependencies solve all dependencies here!) git+https://github.com/openstack-charmers/zaza.git#egg=zaza git+https://github.com/openstack-charmers/zaza-openstack-tests.git#egg=zaza.openstack diff --git a/src/tests/bundles/jammy-yoga.yaml b/src/tests/bundles/jammy-yoga.yaml index 4b767d0..8ed29bf 100644 --- a/src/tests/bundles/jammy-yoga.yaml +++ b/src/tests/bundles/jammy-yoga.yaml @@ -74,6 +74,7 @@ services: - '2' channel: latest/edge + # manila backends manila-ganesha: num_units: 1 charm: ch:manila-ganesha @@ -82,6 +83,11 @@ services: to: - '3' channel: latest/edge + manila-generic: + charm: ch:manila-generic + channel: latest/edge + options: + driver-handles-share-servers: False ceph-mon: charm: ch:ceph-mon @@ -243,6 +249,8 @@ relations: - - 'manila' - 'manila-ganesha' + - - 'manila:manila-plugin' + - 'manila-generic:manila-plugin' - - 'manila-ganesha:shared-db' - 'manila-ganesha-mysql-router:shared-db' diff --git a/src/tests/bundles/jammy-zed.yaml b/src/tests/bundles/jammy-zed.yaml index 316461e..e989f0a 100644 --- a/src/tests/bundles/jammy-zed.yaml +++ b/src/tests/bundles/jammy-zed.yaml @@ -74,6 +74,7 @@ services: - '2' channel: latest/edge + # manila backends manila-ganesha: num_units: 1 charm: ch:manila-ganesha @@ -82,6 +83,11 @@ services: to: - '3' channel: latest/edge + manila-generic: + charm: ch:manila-generic + channel: latest/edge + options: + driver-handles-share-servers: False ceph-mon: charm: ch:ceph-mon @@ -243,6 +249,8 @@ relations: - - 'manila' - 'manila-ganesha' + - - 'manila:manila-plugin' + - 'manila-generic:manila-plugin' - - 'manila-ganesha:shared-db' - 'manila-ganesha-mysql-router:shared-db' diff --git a/src/tests/bundles/kinetic-zed.yaml b/src/tests/bundles/kinetic-zed.yaml index 39f2029..5d3ad44 100644 --- a/src/tests/bundles/kinetic-zed.yaml +++ b/src/tests/bundles/kinetic-zed.yaml @@ -74,6 +74,7 @@ services: - '2' channel: latest/edge + # manila backends manila-ganesha: num_units: 1 charm: ch:manila-ganesha @@ -82,6 +83,11 @@ services: to: - '3' channel: latest/edge + manila-generic: + charm: ch:manila-generic + channel: latest/edge + options: + driver-handles-share-servers: False ceph-mon: charm: ch:ceph-mon @@ -243,6 +249,8 @@ relations: - - 'manila' - 'manila-ganesha' + - - 'manila:manila-plugin' + - 'manila-generic:manila-plugin' - - 'manila-ganesha:shared-db' - 'manila-ganesha-mysql-router:shared-db' diff --git a/src/tests/tests.yaml b/src/tests/tests.yaml index 06e1ee2..83712ea 100644 --- a/src/tests/tests.yaml +++ b/src/tests/tests.yaml @@ -27,7 +27,6 @@ configure: - zaza.openstack.charm_tests.nova.setup.create_flavors - zaza.openstack.charm_tests.nova.setup.manage_ssh_key - zaza.openstack.charm_tests.manila_ganesha.setup.setup_ganesha_share_type - configure_options: configure_gateway_ext_port_use_juju_wait: False diff --git a/unit_tests/test_lib_charm_openstack_manila.py b/unit_tests/test_lib_charm_openstack_manila.py index a4602c7..201271d 100644 --- a/unit_tests/test_lib_charm_openstack_manila.py +++ b/unit_tests/test_lib_charm_openstack_manila.py @@ -104,10 +104,14 @@ class TestManilaCharm(Helper): self.check_call.assert_called_once_with(["mkdir", "-p", "/etc/nova"]) self.assess_status.assert_called_once_with() - def _patch_get_adapter(self, c): + def _patch_get_adapter(self, c, adapters=None): self.patch_object(c, 'get_adapter') + if adapters is None: + adapters = ['remote-manila-plugin.available'] def _helper(x): + if x not in adapters: + return None self.var = x return self.out @@ -124,11 +128,12 @@ class TestManilaCharm(Helper): self.assertEqual(c.configured_backends, []) self.assertEqual(c.custom_assess_status_check(), ('blocked', 'No share backends configured')) + self._patch_get_adapter(c) self.out = mock.Mock() self.out.relation.names = ['name1'] self.assertEqual(c.custom_assess_status_check(), ('blocked', "'default-share-backend' is not set")) - self.assertEqual(self.var, 'manila-plugin.available') + self.assertEqual(self.var, 'remote-manila-plugin.available') def test_custom_assess_status_check2(self): config = { @@ -224,6 +229,16 @@ class TestManilaCharm(Helper): self.assertEqual(c.internal_url_v2, 'i1/v2/%(tenant_id)s') self.assertEqual(c.admin_url_v2, 'a1/v2/%(tenant_id)s') + def test_configured_backends_local_and_remote(self): + c = self._patch_config_and_charm({}) + self.patch_object(c, 'get_adapter') + local_adapter = mock.Mock() + local_adapter.relation.names = ['local'] + remote_adapter = mock.Mock() + remote_adapter.relation.names = ['remote'] + self.get_adapter.side_effect = [local_adapter, remote_adapter] + self.assertEqual(c.configured_backends, ['local', 'remote']) + def test_configured_backends(self): c = self._patch_config_and_charm({}) self._patch_get_adapter(c) diff --git a/unit_tests/test_manila_handlers.py b/unit_tests/test_manila_handlers.py index 31fbb8e..4d8eeaa 100644 --- a/unit_tests/test_manila_handlers.py +++ b/unit_tests/test_manila_handlers.py @@ -101,12 +101,13 @@ class TestRenderStuff(test_utils.PatchHelper): tls = mock.MagicMock() manila_plugin = mock.MagicMock() + remote_manila_plugin = mock.MagicMock() keystone = mock.MagicMock() flags_to_endpoints = { 'certificates.available': tls, 'identity-service.available': keystone, 'manila-plugin.changed': manila_plugin, - 'remote-manila-plugin.changed': manila_plugin, + 'remote-manila-plugin.changed': remote_manila_plugin, } def fake_endpoint_from_flag(flag): @@ -121,6 +122,7 @@ class TestRenderStuff(test_utils.PatchHelper): manila_charm.assess_status.assert_called_once_with() self.set_state.assert_called_once_with('manila.config.rendered') manila_plugin.clear_changed.assert_called_once_with() + remote_manila_plugin.clear_changed.assert_called_once_with() manila_charm.configure_tls.assert_called_once_with( certificates_interface=tls) manila_charm.register_endpoints.assert_called_once_with(keystone) @@ -132,12 +134,13 @@ class TestRenderStuff(test_utils.PatchHelper): tls = mock.MagicMock() manila_plugin = mock.MagicMock() + remote_manila_plugin = mock.MagicMock() keystone = mock.MagicMock() flags_to_endpoints = { 'certificates.available': tls, 'identity-service.available': keystone, 'manila-plugin.changed': manila_plugin, - 'remote-manila-plugin.changed': manila_plugin, + 'remote-manila-plugin.changed': remote_manila_plugin, } def fake_endpoint_from_flag(flag): @@ -152,6 +155,7 @@ class TestRenderStuff(test_utils.PatchHelper): manila_charm.assess_status.assert_called_once_with() self.set_state.assert_called_once_with('manila.config.rendered') manila_plugin.clear_changed.assert_called_once_with() + remote_manila_plugin.clear_changed.assert_called_once_with() manila_charm.configure_tls.assert_called_once_with( certificates_interface=tls) manila_charm.register_endpoints.assert_not_called()