From: John Mulligan <jmulligan@redhat.com>
Date: Sat, 20 Jan 2024 16:22:51 +0000 (-0500)
Subject: pybind/mgr/rook: use AllMatcher when size is not provided
X-Git-Tag: v19.3.0~130^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=90f9a98c8cdbff957414ee7952b4632a2d387bb8;p=ceph.git

pybind/mgr/rook: use AllMatcher when size is not provided

Issue found by mypy 1.6.1. The previous code was:
`and ((sizematcher != None) or sizematcher.compare(device)`
meaning that if sizematcher is not none "return" true, but if
sizematcher is not none execute the compare method. This is of course
impossible as None will never have a compare method. I assume that the

At Kefu Chai's suggestion we can replace the None condition with an
AllMatcher object when no size is provided to create a size matcher.
This both corrects the error and makes the code much simpler and more
readable.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
---

diff --git a/src/pybind/mgr/rook/rook_cluster.py b/src/pybind/mgr/rook/rook_cluster.py
index 21a8e77ba1f..c7d2a34a7b7 100644
--- a/src/pybind/mgr/rook/rook_cluster.py
+++ b/src/pybind/mgr/rook/rook_cluster.py
@@ -33,7 +33,11 @@ from ceph.deployment.service_spec import (
     HostPattern,
 )
 from ceph.utils import datetime_now
-from ceph.deployment.drive_selection.matchers import SizeMatcher
+from ceph.deployment.drive_selection.matchers import (
+    AllMatcher,
+    Matcher,
+    SizeMatcher,
+)
 from nfs.cluster import create_ganesha_pool
 from nfs.module import Module
 from nfs.export import NFSRados
@@ -416,7 +420,7 @@ class DefaultCreator():
     def filter_devices(self, rook_pods: KubernetesResource, drive_group: DriveGroupSpec, matching_hosts: List[str]) -> List[Device]:
         device_list = []
         assert drive_group.data_devices is not None
-        sizematcher: Optional[SizeMatcher] = None
+        sizematcher: Matcher = AllMatcher('', None)
         if drive_group.data_devices.size:
             sizematcher = SizeMatcher('size', drive_group.data_devices.size)
         limit = getattr(drive_group.data_devices, 'limit', None)
@@ -444,7 +448,7 @@ class DefaultCreator():
                             all 
                             or (
                                 device.sys_api['node'] in matching_hosts
-                                and ((sizematcher != None) or sizematcher.compare(device))
+                                and sizematcher.compare(device)
                                 and (
                                     not drive_group.data_devices.paths
                                     or (device.path in paths)
@@ -481,7 +485,7 @@ class LSOCreator(DefaultCreator):
     def filter_devices(self, rook_pods: KubernetesResource, drive_group: DriveGroupSpec, matching_hosts: List[str]) -> List[Device]:
         device_list = []
         assert drive_group.data_devices is not None
-        sizematcher = None
+        sizematcher: Matcher = AllMatcher('', None)
         if drive_group.data_devices.size:
             sizematcher = SizeMatcher('size', drive_group.data_devices.size)
         limit = getattr(drive_group.data_devices, 'limit', None)
@@ -511,7 +515,7 @@ class LSOCreator(DefaultCreator):
                             all 
                             or (
                                 device.sys_api['node'] in matching_hosts
-                                and ((sizematcher != None) or sizematcher.compare(device))
+                                and sizematcher.compare(device)
                                 and (
                                     not drive_group.data_devices.paths
                                     or device.path in paths