mgr/rook: apply_drivegroups fix and coding style fixes

This commit fixes the apply_drivegroups method in RookOrchestrator
to process the entire list of drive groups passed.

This commit also fixes some coding style errors in RookCluster.

Signed-off-by: Joseph Sawaya <jsawaya@redhat.com>

diff --git a/src/pybind/mgr/rook/module.py b/src/pybind/mgr/rook/module.py
index be7e4d14affa985301691ee799792994e8bd78ce..e1066340c63b8b1dd431481d112dcfff2a017ef0 100644 (file)
--- a/src/pybind/mgr/rook/module.py
+++ b/src/pybind/mgr/rook/module.py
@@ -459,6 +459,7 @@ class RookOrchestrator(MgrModule, orchestrator.Orchestrator):
         return self.rook_cluster.remove_pods(names)
 
     def apply_drivegroups(self, specs: List[DriveGroupSpec]) -> OrchResult[List[str]]:
+        result_list = []
         all_hosts = raise_if_exception(self.get_hosts())
         for drive_group in specs:
             matching_hosts = drive_group.placement.filter_matching_hosts(lambda label=None, as_hostspec=None: all_hosts)
@@ -472,7 +473,8 @@ class RookOrchestrator(MgrModule, orchestrator.Orchestrator):
             if not self.rook_cluster.can_create_osd():
                 raise RuntimeError("Rook cluster configuration does not "
                                 "support OSD creation.")
-        return OrchResult([self.rook_cluster.add_osds(drive_group, matching_hosts)])
+            result_list.append(self.rook_cluster.add_osds(drive_group, matching_hosts))
+        return OrchResult(result_list)
     """
     @handle_orch_error
     def create_osds(self, drive_group):

diff --git a/src/pybind/mgr/rook/rook_cluster.py b/src/pybind/mgr/rook/rook_cluster.py
index 9f69447bca267b8fbddfd71b3322ed4e5a9e6646..aadb3ae96e63fac96b85d0b528abe9e621f1ac4a 100644 (file)
--- a/src/pybind/mgr/rook/rook_cluster.py
+++ b/src/pybind/mgr/rook/rook_cluster.py
@@ -404,13 +404,18 @@ class DefaultCreator():
         device_list = []
         assert drive_group.data_devices is not None
         low, high = self.parse_drive_group_size(drive_group.data_devices.size)
-        limit = drive_group.data_devices.limit if hasattr(drive_group.data_devices, 'limit') else None
+        limit = getattr(drive_group.data_devices, 'limit', None)
         count = 0
-        all = drive_group.data_devices.all if hasattr(drive_group.data_devices, 'all') else None
+        all = getattr(drive_group.data_devices, 'all', None)
         paths = [device.path for device in drive_group.data_devices.paths]
         osd_list = []
         for pod in rook_pods.items:
-            if hasattr(pod, 'metadata') and hasattr(pod.metadata, 'labels') and 'osd' in pod.metadata.labels and 'ceph.rook.io/DeviceSet' in pod.metadata.labels:
+            if (
+                hasattr(pod, 'metadata') 
+                and hasattr(pod.metadata, 'labels') 
+                and 'osd' in pod.metadata.labels 
+                and 'ceph.rook.io/DeviceSet' in pod.metadata.labels
+            ):
                 osd_list.append(pod.metadata.labels['ceph.rook.io/DeviceSet'])
         for _, node in self.inventory.items():
             for device in node:
@@ -420,7 +425,17 @@ class DefaultCreator():
             for device in node:
                 if not limit or (count < limit):
                     if device.available:
-                        if all or ((device.sys_api['node'] in matching_hosts) and (self.check_bounds(low, high, int(device.sys_api['size']))) and ((not drive_group.data_devices.paths) or (device.path in paths))):
+                        if (
+                            all 
+                            or (
+                                device.sys_api['node'] in matching_hosts 
+                                and self.check_bounds(low, high, int(device.sys_api['size']))
+                                and (
+                                    not drive_group.data_devices.paths
+                                    or (device.path in paths)
+                                )
+                            )
+                        ):
                             device_list.append(device)
                             count += 1
         return device_list
@@ -447,15 +462,20 @@ class LSOCreator(DefaultCreator):
         device_list = []
         assert drive_group.data_devices is not None
         low, high = self.parse_drive_group_size(drive_group.data_devices.size)
-        limit = drive_group.data_devices.limit if hasattr(drive_group.data_devices, 'limit') else None
+        limit = getattr(drive_group.data_devices, 'limit', None)
         count = 0
-        all = drive_group.data_devices.all if hasattr(drive_group.data_devices, 'all') else None
+        all = getattr(drive_group.data_devices, 'all', None)
         paths = [device.path for device in drive_group.data_devices.paths]
-        vendor = drive_group.data_devices.model if hasattr(drive_group.data_devices, 'vendor') else None
-        model = drive_group.data_devices.model if hasattr(drive_group.data_devices, 'model') else None
+        vendor = getattr(drive_group.data_devices, 'vendor', None)
+        model = getattr(drive_group.data_devices, 'model', None)
         osd_list = []
         for pod in rook_pods.items:
-            if hasattr(pod, 'metadata') and hasattr(pod.metadata, 'labels') and 'osd' in pod.metadata.labels and 'ceph.rook.io/DeviceSet' in pod.metadata.labels:
+            if (
+                hasattr(pod, 'metadata') 
+                and hasattr(pod.metadata, 'labels') 
+                and 'osd' in pod.metadata.labels 
+                and 'ceph.rook.io/DeviceSet' in pod.metadata.labels
+            ):
                 osd_list.append(pod.metadata.labels['ceph.rook.io/DeviceSet'])
         for _, node in self.inventory.items():
             for device in node:
@@ -465,7 +485,25 @@ class LSOCreator(DefaultCreator):
             for device in node:
                 if not limit or (count < limit):
                     if device.available:
-                        if all or ((device.sys_api['node'] in matching_hosts) and (self.check_bounds(low, high, int(device.sys_api['size']))) and ((not drive_group.data_devices.paths) or (device.path in paths)) and (not vendor or (device.sys_api['vendor'] == vendor)) and (not model or (device.sys_api['model'].startsWith(model)))):
+                        if (
+                            all 
+                            or (
+                                device.sys_api['node'] in matching_hosts
+                                and self.check_bounds(low, high, int(device.sys_api['size']))
+                                and (
+                                    not drive_group.data_devices.paths
+                                    or device.path in paths
+                                ) 
+                                and (
+                                    not vendor 
+                                    or device.sys_api['vendor'] == vendor
+                                )
+                                and (
+                                    not model 
+                                    or device.sys_api['model'].startsWith(model)
+                                )
+                            )
+                        ):
                             device_list.append(device)
                             count += 1
         return device_list