Fix local/remote assumptions in manila charm

When the remote plugin was developed it made the assumption that the
`enabled_share_backends` included the remote backends. This assumption
is wrong, and it should only contain the locally defined drivers; i.e.
ones that have a [driver] section in the manila.conf file. The cephfsnfs
driver doesn't have a local configuration as the share-server is runs on
the manila-ganesha unit and so shouldn't be local.

This patch splits the config in local/remote and then uses the local and
remote config in the correct places.

Closes-Bug: #2012457
Change-Id: I436452208aaeaf08d1655da4fccbd7a89549b404

src/lib/charm/openstack/manila.py

@@ -44,8 +44,10 @@ 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",
+LOCAL_PLUGIN_RELATION = "manila-plugin.available"
-                    "remote-manila-plugin.available",)
+REMOTE_PLUGIN_RELATION = "remote-manila-plugin.available"
+PLUGIN_RELATIONS = (LOCAL_PLUGIN_RELATION,
+                    REMOTE_PLUGIN_RELATION,)
 
 # select the default release function and ssl feature
 charms_openstack.charm.use_defaults('charm.default-select-release')
@@ -66,17 +68,21 @@ def strip_join(s, divider=" "):
 
 ###
 # Compute some options to help with template rendering
+###
+
 @charms_openstack.adapters.config_property
-def computed_share_backends(config):
+def computed_local_share_backends(config):
     """Determine the backend protocols that are provided as a string.
 
     This asks the charm class what the backend protocols are, and then provides
-    it as a comma separated list of backends.
+    it as a comma separated list of backends for the local share service. This
+    does not include the remote backends (i.e. other non-local units running
+    manila-share.service)
 
     :param config: the config option on which to look up config options
     :returns: string
     """
-    return ','.join(config.charm_instance.configured_backends)
+    return ','.join(config.charm_instance.configured_local_backends)
 
 
 @charms_openstack.adapters.config_property
@@ -227,7 +233,9 @@ class ManilaCharm(charms_openstack.charm.HAOpenStackCharm):
                     'haproxy',
                     'manila-scheduler',
                     'manila-data']
-        if not self.get_adapter('remote-manila-plugin.available'):
+        # BUG: #2012457: only start manila-share if a local share server is
+        # going to be configured.
+        if self.get_adapter(LOCAL_PLUGIN_RELATION):
             services.append('manila-share')
         return services
 
@@ -267,7 +275,7 @@ class ManilaCharm(charms_openstack.charm.HAOpenStackCharm):
             there is a problem. Or (None, None) if there are no issues.
         """
         options = self.options  # tiny optimisation for less typing.
-        backends = options.computed_share_backends
+        backends = self.all_backends
         if not backends:
             return 'blocked', 'No share backends configured'
         default_share_backend = options.default_share_backend
@@ -380,19 +388,40 @@ class ManilaCharm(charms_openstack.charm.HAOpenStackCharm):
         return super().internal_url + "/v2/%(tenant_id)s"
 
     @property
-    def configured_backends(self):
+    def configured_local_backends(self):
         """Return a list of configured backends that come from the associated
         'manila-share.available' state..
 
+        Note, that this mustn't contain the remote backends as they are share
+        server specific (e.g. running manila-share service) and if they are
+        included then the share-service wont' start.
+
         :returns: list of strings: backend sections that are configured.
         """
         # adapter.names is a property that provides a list of backend manila
         # plugin names for the sections
+        try:
+            # if self.local_adapter is None, the raises AttributeError when
+            # trying to access 'relation'
+            return sorted(set(self.local_manila_plugin_adapter.relation.names))
+        except AttributeError:
+            return []
 
+    @property
+    def all_backends(self):
+        """Return a list of all configured backends.
+
+        This is obtained from both of the manila-share adapter and the
+        remote-manila-share adapter.
+
+        :returns: list of strings: backend sections that are configured.
+        """
         names = []
-        for backend_relation in self.adapters:
+        for manila_plugin_adapter in [self.local_manila_plugin_adapter,
-            names.extend(backend_relation.relation.names)
+                                      self.remote_manila_plugin_adapter]:
-        return sorted(list(set(names)))
+            if manila_plugin_adapter:
+                names.extend(manila_plugin_adapter.relation.names)
+        return sorted(set(names))
 
     def config_lines_for(self, config_file):
         """Return the list of configuration lines for `config_file` as returned
@@ -415,7 +444,7 @@ class ManilaCharm(charms_openstack.charm.HAOpenStackCharm):
         """
         config_lines = []
         inverted_config_data = collections.defaultdict(dict)
-        for adapter in self.adapters:
+        for adapter in self.manila_plugin_adapters:
             # get the configuration data for all plugins
             config_data = adapter.relation.get_configuration_data()
 
@@ -439,7 +468,7 @@ class ManilaCharm(charms_openstack.charm.HAOpenStackCharm):
         """
 
         config_files = set()
-        for adapter in self.adapters:
+        for adapter in self.manila_plugin_adapters:
             # get the configuration data for all plugins
             config_data = adapter.relation.get_configuration_data()
 
@@ -449,13 +478,37 @@ class ManilaCharm(charms_openstack.charm.HAOpenStackCharm):
         return list(config_files)
 
     @property
-    def adapters(self):
+    def manila_plugin_adapters(self):
+        """Return a list of manila-plugin adapters (local and remote).
+
+        :returns: the list of adapters, if available.
+        :rtype: List[OpenStackRelationAdapter]
+        """
         return [
             adapter
-            for adapter in map(self.get_adapter, PLUGIN_RELATIONS)
+            for adapter in [self.local_manila_plugin_adapter,
+                            self.remote_manila_plugin_adapter]
             if adapter is not None
         ]
 
+    @property
+    def local_manila_plugin_adapter(self):
+        """Return the local manila-plugin adapter or None.
+
+        :returns: the adapters, if available.
+        :rtype: OpenStackRelationAdapter
+        """
+        return self.get_adapter(LOCAL_PLUGIN_RELATION)
+
+    @property
+    def remote_manila_plugin_adapter(self):
+        """Return the remote manila-plugin adapter or None.
+
+        :returns: the adapters, if available.
+        :rtype: OpenStackRelationAdapter
+        """
+        return self.get_adapter(REMOTE_PLUGIN_RELATION)
+
     def enable_webserver_site(self):
         """Enable Manila API apache2 site if rendered or installed"""
         if os.path.exists(MANILA_WSGI_CONF):

src/templates/mitaka/manila.conf

@@ -7,7 +7,7 @@
 [DEFAULT]
 
 # This all needs to be configurable
-enabled_share_backends = {{ options.computed_share_backends }}
+enabled_share_backends = {{ options.computed_local_share_backends }}
 
 # enabled_share_protocols = NFS,CIFS
 enabled_share_protocols = {{ options.computed_share_protocols }}

src/templates/pike/manila.conf

@@ -7,7 +7,7 @@
 [DEFAULT]
 
 # This all needs to be configurable
-enabled_share_backends = {{ options.computed_share_backends }}
+enabled_share_backends = {{ options.computed_local_share_backends }}
 
 # enabled_share_protocols = NFS,CIFS
 enabled_share_protocols = {{ options.computed_share_protocols }}

src/templates/queens/manila.conf

@@ -7,7 +7,7 @@
 [DEFAULT]
 
 # This all needs to be configurable
-enabled_share_backends = {{ options.computed_share_backends }}
+enabled_share_backends = {{ options.computed_local_share_backends }}
 
 # enabled_share_protocols = NFS,CIFS
 enabled_share_protocols = {{ options.computed_share_protocols }}

src/templates/rocky/manila.conf

@@ -7,7 +7,7 @@
 [DEFAULT]
 
 # This all needs to be configurable
-enabled_share_backends = {{ options.computed_share_backends }}
+enabled_share_backends = {{ options.computed_local_share_backends }}
 
 # enabled_share_protocols = NFS,CIFS
 enabled_share_protocols = {{ options.computed_share_protocols }}

test-requirements.txt

@@ -6,6 +6,7 @@
 pyparsing<3.0.0  # aodhclient is pinned in zaza and needs pyparsing < 3.0.0, but cffi also needs it, so pin here.
 setuptools<50.0.0  # https://github.com/pypa/setuptools/commit/04e3df22df840c6bb244e9b27bc56750c44b7c85
 
+testtools<2.6.0
 stestr>=2.2.0
 
 # Dependency of stestr. Workaround for

unit_tests/test_lib_charm_openstack_manila.py

@@ -47,10 +47,10 @@ class TestManilaCharmUtilities(Helper):
 
 class TestManilaCharmConfigProperties(Helper):
 
-    def test_computed_share_backends(self):
+    def test_computed_local_share_backends(self):
         config = mock.MagicMock()
-        config.charm_instance.configured_backends = ["a", "c", "b"]
+        config.charm_instance.configured_local_backends = ["a", "c", "b"]
-        self.assertEqual(manila.computed_share_backends(config), "a,c,b")
+        self.assertEqual(manila.computed_local_share_backends(config), "a,c,b")
 
     def test_computed_share_protocols(self):
         config = mock.MagicMock()
@@ -107,7 +107,7 @@ class TestManilaCharm(Helper):
     def _patch_get_adapter(self, c, adapters=None):
         self.patch_object(c, 'get_adapter')
         if adapters is None:
-            adapters = ['remote-manila-plugin.available']
+            adapters = ['manila-plugin.available']
 
         def _helper(x):
             if x not in adapters:
@@ -125,7 +125,7 @@ class TestManilaCharm(Helper):
         self._patch_get_adapter(c)
         self.out = None
 
-        self.assertEqual(c.configured_backends, [])
+        self.assertEqual(c.configured_local_backends, [])
         self.assertEqual(c.custom_assess_status_check(),
                          ('blocked', 'No share backends configured'))
         self._patch_get_adapter(c)
@@ -133,7 +133,7 @@ class TestManilaCharm(Helper):
         self.out.relation.names = ['name1']
         self.assertEqual(c.custom_assess_status_check(),
                          ('blocked', "'default-share-backend' is not set"))
-        self.assertEqual(self.var, 'remote-manila-plugin.available')
+        self.assertEqual(self.var, 'manila-plugin.available')
 
     def test_custom_assess_status_check2(self):
         config = {
@@ -237,24 +237,36 @@ class TestManilaCharm(Helper):
         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'])
+        # note that the ONLY local backends show up for the share-server
+        self.assertEqual(c.configured_local_backends, ['local'])
 
-    def test_configured_backends(self):
+    def test_configured_local_backends(self):
         c = self._patch_config_and_charm({})
         self._patch_get_adapter(c)
         self.out = None
-        self.assertEqual(c.configured_backends, [])
+        self.assertEqual(c.configured_local_backends, [])
-        self.assertEqual(self.var, 'remote-manila-plugin.available')
+        self.assertEqual(self.var, 'manila-plugin.available')
         self.out = mock.Mock()
         self.out.relation.names = ['a', 'b']
-        self.assertEqual(c.configured_backends, ['a', 'b'])
+        self.assertEqual(c.configured_local_backends, ['a', 'b'])
+
+    def test_all_backends(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]
+        # note that the ONLY local backends show up for the share-server
+        self.assertEqual(c.all_backends, ['local', 'remote'])
 
     def test_config_lines_for(self):
         c = self._patch_config_and_charm({})
         self._patch_get_adapter(c)
         self.out = None
         self.assertEqual(c.config_lines_for('conf'), [])
-        self.assertEqual(self.var, 'remote-manila-plugin.available')
+        self.assertEqual(self.var, 'manila-plugin.available')
         self.out = mock.Mock()
         self.out.relation.get_configuration_data.return_value = {}
         self.assertEqual(c.config_lines_for('conf'), [])
@@ -288,3 +300,39 @@ class TestManilaCharm(Helper):
         self.NRPE.assert_has_calls([
             mock.call().write(),
         ])
+
+    def test_manila_plugin_adapters__local(self):
+        c = self._patch_config_and_charm({})
+        self.patch_object(c, 'get_adapter')
+        local_adapter = mock.Mock()
+        local_adapter.relation.names = ['local']
+        self.get_adapter.side_effect = [local_adapter, None]
+        self.assertEqual(c.manila_plugin_adapters, [local_adapter])
+        self.get_adapter.assert_has_calls([
+            mock.call(manila.LOCAL_PLUGIN_RELATION),
+            mock.call(manila.REMOTE_PLUGIN_RELATION)])
+
+    def test_manila_plugin_adapters__remote(self):
+        c = self._patch_config_and_charm({})
+        self.patch_object(c, 'get_adapter')
+        remote_adapter = mock.Mock()
+        remote_adapter.relation.names = ['remote']
+        self.get_adapter.side_effect = [None, remote_adapter]
+        self.assertEqual(c.manila_plugin_adapters, [remote_adapter])
+        self.get_adapter.assert_has_calls([
+            mock.call(manila.LOCAL_PLUGIN_RELATION),
+            mock.call(manila.REMOTE_PLUGIN_RELATION)])
+
+    def test_manila_plugin_adapters__both(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.manila_plugin_adapters,
+                         [local_adapter, remote_adapter])
+        self.get_adapter.assert_has_calls([
+            mock.call(manila.LOCAL_PLUGIN_RELATION),
+            mock.call(manila.REMOTE_PLUGIN_RELATION)])