Address review comments

config.yaml

@@ -49,14 +49,18 @@ options:
     type: string
 
   cephfs-tolerations:
-    default: "=,Exists"
+    default: "$csi-cephfsplugin-legacy$"
     description: |
       Tolerations to be used when creating the cephfs pods for Daemonsets and Deployments.
 
       Declare tolerations in key=value,operator,effect format, separating each by spaces.
 
       Optional tolerations can be found in the kubernetes documentation:
       https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/
+
+      Note:  The default value "$csi-cephfsplugin-legacy$" is a special token
+      which applies a "=,Exists" toleration only to pods associated with
+      csi-cephfsplugin.
     type: string
 
   ceph-rbd-tolerations:

src/manifests_base.py

@@ -2,7 +2,7 @@
 import pickle
 from abc import ABCMeta, abstractmethod
 from hashlib import md5
-from typing import Any, Dict, Generator, List, Optional, Tuple
+from typing import Any, Dict, Generator, List, Optional
 
 from lightkube.codecs import AnyResource
 from lightkube.core.resource import NamespacedResource
@@ -123,7 +123,7 @@ def update_parameters(self, parameters: Dict[str, str]) -> None:
 
 class CephToleration(Toleration):
     @classmethod
-    def from_string(cls, toleration_str: str) -> "CephToleration":
+    def _from_string(cls, toleration_str: str) -> "CephToleration":
         """Parses a toleration string into a Toleration object.
 
         Raises:
@@ -150,11 +150,13 @@ def from_string(cls, toleration_str: str) -> "CephToleration":
         )
 
     @classmethod
-    def from_comma_separated(
-        cls, tolerations: str
-    ) -> Tuple[List["CephToleration"], Optional[str]]:
-        """Parses a comma separated string of tolerations into a list of Toleration objects."""
+    def from_space_separated(cls, tolerations: str) -> List["CephToleration"]:
+        """Parses a space separated string of tolerations into a list of Toleration objects.
+
+        Raises:
+            ValueError: If any of the tolerations are invalid
+        """
         try:
-            return [cls.from_string(toleration) for toleration in tolerations.split()], None
+            return [cls._from_string(toleration) for toleration in tolerations.split()]
         except ValueError as e:
-            return [], f"Invalid tolerations: {e}"
+            raise ValueError(f"Invalid tolerations: {e}") from e

src/manifests_cephfs.py

@@ -121,13 +121,15 @@ def __call__(self) -> Optional[AnyResource]:
 class ProvisionerAdjustments(Patch):
     """Update Cephfs provisioner."""
 
-    def tolerations(self) -> Tuple[List[CephToleration], Optional[str]]:
+    def tolerations(self) -> Tuple[List[CephToleration], bool]:
         cfg = self.manifests.config.get("cephfs-tolerations") or ""
-        return CephToleration.from_comma_separated(cfg)
+        if cfg == "$csi-cephfsplugin-legacy$":
+            return [], True
+        return CephToleration.from_space_separated(cfg), False
 
     def __call__(self, obj: AnyResource) -> None:
         """Use the provisioner-replicas and enable-host-networking to update obj."""
-        tolerations, _ = self.tolerations()
+        tolerations, legacy = self.tolerations()
         if (
             obj.kind == "Deployment"
             and obj.metadata
@@ -144,8 +146,10 @@ def __call__(self, obj: AnyResource) -> None:
             )
             log.info(f"Updating deployment hostNetwork to {host_network}")
         if obj.kind == "DaemonSet" and obj.metadata and obj.metadata.name == "csi-cephfsplugin":
-            obj.spec.template.spec.tolerations = tolerations
-            log.info("Updating daemonset tolerations to operator=Exists")
+            obj.spec.template.spec.tolerations = (
+                tolerations if not legacy else [CephToleration(operator="Exists")]
+            )
+            log.info("Updating daemonset tolerations")
 
             kubelet_dir = self.manifests.config.get("kubelet_dir", "/var/lib/kubelet")
 
@@ -243,8 +247,9 @@ def evaluate(self) -> Optional[str]:
         pa_manipulator = next(
             m for m in self.manipulations if isinstance(m, ProvisionerAdjustments)
         )
-        _, err = pa_manipulator.tolerations()
-        if err:
+        try:
+            pa_manipulator.tolerations()
+        except ValueError as err:
             return f"Cannot adjust CephFS Pods: {err}"
 
         return None

src/manifests_rbd.py

@@ -3,7 +3,7 @@
 """Implementation of rbd specific details of the kubernetes manifests."""
 
 import logging
-from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple
+from typing import TYPE_CHECKING, Any, Dict, List, Optional
 
 from lightkube.codecs import AnyResource
 from lightkube.resources.core_v1 import Secret
@@ -106,13 +106,13 @@ def __call__(self) -> Optional[AnyResource]:
 class ProvisionerAdjustments(Patch):
     """Update RBD provisioner."""
 
-    def tolerations(self) -> Tuple[List[CephToleration], Optional[str]]:
+    def tolerations(self) -> List[CephToleration]:
         cfg = self.manifests.config.get("ceph-rbd-tolerations") or ""
-        return CephToleration.from_comma_separated(cfg)
+        return CephToleration.from_space_separated(cfg)
 
     def __call__(self, obj: AnyResource) -> None:
         """Mutates CSI RBD Provisioner Deployment replicas/hostNetwork and DaemonSet kubelet_dir paths."""
-        tolerations, _ = self.tolerations()
+        tolerations = self.tolerations()
         if (
             obj.kind == "Deployment"
             and obj.metadata
@@ -213,8 +213,9 @@ def evaluate(self) -> Optional[str]:
         pa_manipulator = next(
             m for m in self.manipulations if isinstance(m, ProvisionerAdjustments)
         )
-        _, err = pa_manipulator.tolerations()
-        if err:
+        try:
+            pa_manipulator.tolerations()
+        except ValueError as err:
             return f"Cannot adjust CephRBD Pods: {err}"
 
         return None