Support alternate names for ceph-fs charm and associated storage class (

#33)

* Support alternate names for ceph-fs charm and associated storage class

* Pythonic error handling

config.yaml

@@ -19,6 +19,24 @@ options:
     description: "Whether or not csi-*plugin-provisioner deployments use host-networking"
     type: boolean
 
+  cephfs-storage-class-name-formatter:
+    default: "cephfs"
+    description: |
+      Formatter for the cephfs storage class name
+
+      The formatter is a string that can contain the following placeholders:
+        - {app}              - the name of the juju application
+        - {namespace}        - the charm configured namespace
+        - {name}             - the name of the filesystem
+        - {pool}             - the name of the data-pool
+        - {pool-id}          - the id   of the data-pool
+
+      Example:
+        juju config ceph-csi cephfs-storage-class-name-formatter="{cluster}-{namespace}-{storageclass}"
+
+      The default is to use the storage class name
+    type: string
+
   cephfs-enable:
     default: false
     description: |
@@ -38,8 +56,8 @@ options:
   cephfs-storage-class-parameters:
     default: ""
     description: |
-      Parameters to be used when creating the cephfs storage class.
-      Changes are only applied to the storage class if it does not exist.
+      Parameters to be used when creating the cephfs storage classes.
+      Changes are only applied to the storage classes if it does not exist.
 
       Declare additional/replacement parameters in key=value format, separated by spaces.
       Declare removed parameters in the key- format, separated by spaces.

src/charm.py

@@ -23,7 +23,7 @@
 from ops.manifests import Collector, ManifestClientError
 
 from manifests_base import Manifests, SafeManifest
-from manifests_cephfs import CephFSManifests, CephStorageClass
+from manifests_cephfs import CephFilesystem, CephFSManifests, CephStorageClass
 from manifests_config import ConfigManifests
 from manifests_rbd import RBDManifests
 
@@ -228,20 +228,17 @@ def get_ceph_fsid(self) -> str:
             return ""
 
     @lru_cache(maxsize=None)
-    def get_ceph_fsname(self) -> Optional[str]:
-        """Get the Ceph FS Name."""
+    def get_ceph_fs_list(self) -> List[CephFilesystem]:
+        """Get a list of CephFS names and list of associated pools."""
         try:
             data = json.loads(self.ceph_cli("fs", "ls", "-f", "json"))
         except (subprocess.SubprocessError, ValueError) as e:
             logger.error(
-                "get_ceph_fsname: Failed to get CephFS name, reporting as None, error: %s", e
+                "get_ceph_fs_list: Failed to find CephFS name, reporting as None, error: %s", e
             )
-            return None
-        for fs in data:
-            if CephStorageClass.POOL in fs["data_pools"]:
-                logger.error("get_ceph_fsname: got cephfs name: %s", fs["name"])
-                return fs["name"]
-        return None
+            return []
+
+        return [CephFilesystem(**fs) for fs in data]
 
     @property
     def provisioner_replicas(self) -> int:
@@ -285,7 +282,7 @@ def ceph_context(self) -> Dict[str, Any]:
             "user": self.app.name,
             "provisioner_replicas": self.provisioner_replicas,
             "enable_host_network": json.dumps(self.enable_host_network),
-            "fsname": self.get_ceph_fsname(),
+            CephStorageClass.FILESYSTEM_LISTING: self.get_ceph_fs_list(),
         }
 
     @status.on_error(ops.WaitingStatus("Waiting for kubeconfig"))
@@ -306,20 +303,11 @@ def check_namespace(self) -> None:
                 status.add(ops.WaitingStatus("Waiting for Kubernetes API"))
                 raise status.ReconcilerError("Waiting for Kubernetes API")
 
-    @status.on_error(ops.BlockedStatus("CephFS is not usable; set 'cephfs-enable=False'"))
-    def check_cephfs(self) -> None:
-        self.unit.status = ops.MaintenanceStatus("Evaluating CephFS capability")
+    def enforce_cephfs_enabled(self) -> None:
+        """Determine if CephFS should be enabled or disabled."""
         disabled = self.config.get("cephfs-enable") is False
-        if disabled:
-            # not enabled, not a problem
-            logger.info("CephFS is disabled")
-        elif not self.ceph_context.get("fsname", None):
-            logger.error(
-                "Ceph CLI failed to find a CephFS fsname. Run 'juju config cephfs-enable=False' until ceph-fs is usable."
-            )
-            raise status.ReconcilerError("CephFS is not usable; set 'cephfs-enable=False'")
-
         if disabled and self.unit.is_leader():
+            self.unit.status = ops.MaintenanceStatus("Disabling CephFS")
             self._purge_manifest_by_name("cephfs")
 
     def evaluate_manifests(self) -> int:
@@ -390,7 +378,7 @@ def reconcile(self, event: ops.EventBase) -> None:
         self.check_namespace()
         self.check_ceph_client()
         self.configure_ceph_cli()
-        self.check_cephfs()
+        self.enforce_cephfs_enabled()
         hash = self.evaluate_manifests()
         self.install_manifests(config_hash=hash)
         self._update_status()

src/manifests_base.py

@@ -1,13 +1,12 @@
 import logging
 import pickle
-from abc import ABCMeta, abstractmethod
 from hashlib import md5
 from typing import Any, Dict, Generator, List, Optional
 
 from lightkube.codecs import AnyResource
 from lightkube.core.resource import NamespacedResource
 from lightkube.models.core_v1 import Toleration
-from ops.manifests import Addition, Manifests, Patch
+from ops.manifests import Manifests, Patch
 
 log = logging.getLogger(__name__)
 
@@ -92,35 +91,6 @@ def filter_containers(self, containers: list) -> Generator:
             yield container
 
 
-class StorageClassAddition(Addition):
-    """Base class for storage class additions."""
-
-    __metaclass__ = ABCMeta
-
-    @property
-    @abstractmethod
-    def name(self) -> str:
-        """Name of the storage class."""
-        raise NotImplementedError
-
-    def update_parameters(self, parameters: Dict[str, str]) -> None:
-        """Adjust parameters for storage class."""
-        config = f"{self.name}-storage-class-parameters"
-        adjustments = self.manifests.config.get(config)
-        if not adjustments:
-            log.info(f"No adjustments for {self.name} storage-class parameters")
-            return
-
-        for adjustment in adjustments.split(" "):
-            key_value = adjustment.split("=", 1)
-            if len(key_value) == 2:
-                parameters[key_value[0]] = key_value[1]
-            elif adjustment.endswith("-"):
-                parameters.pop(adjustment[:-1], None)
-            else:
-                log.warning("Invalid parameter: %s in %s", adjustment, config)
-
-
 class CephToleration(Toleration):
     @classmethod
     def _from_string(cls, toleration_str: str) -> "CephToleration":
@@ -160,3 +130,20 @@ def from_space_separated(cls, tolerations: str) -> List["CephToleration"]:
             return [cls._from_string(toleration) for toleration in tolerations.split()]
         except ValueError as e:
             raise ValueError(f"Invalid tolerations: {e}") from e
+
+
+def update_storage_params(ceph_type: str, config: Dict[str, Any], params: Dict[str, str]) -> None:
+    """Adjust parameters for storage class."""
+    cfg_name = f"{ceph_type}-storage-class-parameters"
+    if not (adjustments := config.get(cfg_name)):
+        log.info(f"No adjustments for {ceph_type} storage-class parameters")
+        return
+
+    for adjustment in adjustments.split(" "):
+        key_value = adjustment.split("=", 1)
+        if len(key_value) == 2:
+            params[key_value[0]] = key_value[1]
+        elif adjustment.endswith("-"):
+            params.pop(adjustment[:-1], None)
+        else:
+            log.warning("Invalid parameter: %s in %s", adjustment, cfg_name)