]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
DriverFilter: don't check volume_backend_name
authorZhiteng Huang <zhithuang@ebaysf.com>
Thu, 4 Jun 2015 18:23:58 +0000 (11:23 -0700)
committerHuang Zhiteng <winston.d@gmail.com>
Thu, 4 Jun 2015 23:27:46 +0000 (23:27 +0000)
Currently DriverFilter checks if host_state.volume_backend_name
matches type.extra_specs.volume_backend_name.  The same check has
already been done in CapabilitiesFilter and done more thoroughly and
gracefully, for example, volume_backend_name can be a set of names
not limited to one to one match.  DriverFilter should just do one
thing, that is evaluating filter_function supplied by backend.

This change removes the volume_backend_name check from DriverFilter
without touching its core function, thus has no impact on existing
documentation because the checking of volume_backend_name is an
undocumented side-effect of this filter.  This change also reduces
one warning log to debug log when no filter_function found in
host_state, for the reason it would generate a lot of warning log
for those backends don't supply filter_function.

UpgradeImpact:
DriverFilter had an undocumented side-effect before it evaluates
filter_function - it checked if volume_backend_name in host_state
reported by backend matches volume_backend_name in volume type
extra specs, and if they don't, the backend would fail to pass
the filter.  Now that this side-effect has been removed, it
would impact users only under following circumstance: 1) user
enabled DriverFilter but not CapabilitiesFilter; 2) user relied
on DriverFilter to filter backends based on exact match of
'volume_backend_name' in type extra spec.  If unfortunately this
is the case for user, enabling CapabilitiesFilter by adding
it to 'scheduler_default_filters' configure option and restart
cinder scheduler service then everything should work just fine.
CapabilitiesFilter actually does a much better job checking
volume_backend_name.

Change-Id: Ie5c48587368856faa538eebb1b6611d464bd69bd
Closes-bug: #1461770

cinder/scheduler/filters/driver_filter.py
cinder/tests/unit/scheduler/test_host_filters.py

index ac6fe7b0ab7c33f760d10936ea75bc06a4082d16..a406c4ca57d85817432e9e345db44b2d6f957d22 100644 (file)
@@ -48,23 +48,8 @@ class DriverFilter(filters.BaseHostFilter):
            Returns a tuple in the format (filter_passing, filter_invalid).
            Both values are booleans.
         """
-        host_stats = stats['host_stats']
-        extra_specs = stats['extra_specs']
-
-        # Check that the volume types match
-        if (extra_specs is None or 'volume_backend_name' not in extra_specs):
-            LOG.warning(_LW("No 'volume_backend_name' key in extra_specs. "
-                            "Skipping volume backend name check."))
-        elif (extra_specs['volume_backend_name'] !=
-                host_stats['volume_backend_name']):
-            LOG.warning(_LW("Volume backend names do not match: '%(target)s' "
-                            "vs '%(current)s' :: Skipping"),
-                        {'target': extra_specs['volume_backend_name'],
-                         'current': host_stats['volume_backend_name']})
-            return False
-
         if stats['filter_function'] is None:
-            LOG.warning(_LW("Filter function not set :: passing host"))
+            LOG.debug("Filter function not set :: passing host")
             return True
 
         try:
index 1271391bedee6459d0d8bf2cfdc1a449fe55688b..b52100353ee9b6949af67b6e56425af7644cb182 100644 (file)
@@ -627,19 +627,12 @@ class DriverFilterTestCase(HostFiltersTestCase):
         filt_cls = self.class_map['DriverFilter']()
         host1 = fakes.FakeHostState(
             'host1', {
-                'volume_backend_name': 'fake',
                 'capabilities': {
                     'filter_function': '1 == 1',
                 }
             })
 
-        filter_properties = {
-            'volume_type': {
-                'extra_specs': {
-                    'volume_backend_name': 'fake',
-                }
-            }
-        }
+        filter_properties = {'volume_type': {}}
 
         self.assertTrue(filt_cls.host_passes(host1, filter_properties))
 
@@ -647,19 +640,12 @@ class DriverFilterTestCase(HostFiltersTestCase):
         filt_cls = self.class_map['DriverFilter']()
         host1 = fakes.FakeHostState(
             'host1', {
-                'volume_backend_name': 'fake',
                 'capabilities': {
                     'filter_function': '1 == 2',
                 }
             })
 
-        filter_properties = {
-            'volume_type': {
-                'extra_specs': {
-                    'volume_backend_name': 'fake',
-                }
-            }
-        }
+        filter_properties = {'volume_type': {}}
 
         self.assertFalse(filt_cls.host_passes(host1, filter_properties))
 
@@ -667,19 +653,12 @@ class DriverFilterTestCase(HostFiltersTestCase):
         filt_cls = self.class_map['DriverFilter']()
         host1 = fakes.FakeHostState(
             'host1', {
-                'volume_backend_name': 'fake',
                 'capabilities': {
                     'filter_function': None,
                 }
             })
 
-        filter_properties = {
-            'volume_type': {
-                'extra_specs': {
-                    'volume_backend_name': 'fake',
-                }
-            }
-        }
+        filter_properties = {'volume_type': {}}
 
         self.assertTrue(filt_cls.host_passes(host1, filter_properties))
 
@@ -687,17 +666,10 @@ class DriverFilterTestCase(HostFiltersTestCase):
         filt_cls = self.class_map['DriverFilter']()
         host1 = fakes.FakeHostState(
             'host1', {
-                'volume_backend_name': 'fake',
                 'capabilities': {}
             })
 
-        filter_properties = {
-            'volume_type': {
-                'extra_specs': {
-                    'volume_backend_name': 'fake',
-                }
-            }
-        }
+        filter_properties = {'volume_type': {}}
 
         self.assertTrue(filt_cls.host_passes(host1, filter_properties))
 
@@ -705,7 +677,6 @@ class DriverFilterTestCase(HostFiltersTestCase):
         filt_cls = self.class_map['DriverFilter']()
         host1 = fakes.FakeHostState(
             'host1', {
-                'volume_backend_name': 'fake',
                 'capabilities': {
                     'filter_function': '1 == 1',
                 }
@@ -715,31 +686,10 @@ class DriverFilterTestCase(HostFiltersTestCase):
 
         self.assertTrue(filt_cls.host_passes(host1, filter_properties))
 
-    def test_volume_backend_name_different(self):
-        filt_cls = self.class_map['DriverFilter']()
-        host1 = fakes.FakeHostState(
-            'host1', {
-                'volume_backend_name': 'fake',
-                'capabilities': {
-                    'filter_function': '1 == 1',
-                }
-            })
-
-        filter_properties = {
-            'volume_type': {
-                'extra_specs': {
-                    'volume_backend_name': 'fake2',
-                }
-            }
-        }
-
-        self.assertFalse(filt_cls.host_passes(host1, filter_properties))
-
     def test_function_extra_spec_replacement(self):
         filt_cls = self.class_map['DriverFilter']()
         host1 = fakes.FakeHostState(
             'host1', {
-                'volume_backend_name': 'fake',
                 'capabilities': {
                     'filter_function': 'extra.var == 1',
                 }
@@ -748,7 +698,6 @@ class DriverFilterTestCase(HostFiltersTestCase):
         filter_properties = {
             'volume_type': {
                 'extra_specs': {
-                    'volume_backend_name': 'fake',
                     'var': 1,
                 }
             }
@@ -760,20 +709,13 @@ class DriverFilterTestCase(HostFiltersTestCase):
         filt_cls = self.class_map['DriverFilter']()
         host1 = fakes.FakeHostState(
             'host1', {
-                'volume_backend_name': 'fake',
                 'total_capacity_gb': 100,
                 'capabilities': {
                     'filter_function': 'stats.total_capacity_gb < 200',
                 }
             })
 
-        filter_properties = {
-            'volume_type': {
-                'extra_specs': {
-                    'volume_backend_name': 'fake',
-                }
-            }
-        }
+        filter_properties = {'volume_type': {}}
 
         self.assertTrue(filt_cls.host_passes(host1, filter_properties))
 
@@ -781,18 +723,12 @@ class DriverFilterTestCase(HostFiltersTestCase):
         filt_cls = self.class_map['DriverFilter']()
         host1 = fakes.FakeHostState(
             'host1', {
-                'volume_backend_name': 'fake',
                 'capabilities': {
                     'filter_function': 'volume.size < 5',
                 }
             })
 
         filter_properties = {
-            'volume_type': {
-                'extra_specs': {
-                    'volume_backend_name': 'fake',
-                }
-            },
             'request_spec': {
                 'volume_properties': {
                     'size': 1
@@ -806,18 +742,12 @@ class DriverFilterTestCase(HostFiltersTestCase):
         filt_cls = self.class_map['DriverFilter']()
         host1 = fakes.FakeHostState(
             'host1', {
-                'volume_backend_name': 'fake',
                 'capabilities': {
                     'filter_function': 'qos.var == 1',
                 }
             })
 
         filter_properties = {
-            'volume_type': {
-                'extra_specs': {
-                    'volume_backend_name': 'fake',
-                }
-            },
             'qos_specs': {
                 'var': 1
             }
@@ -829,19 +759,12 @@ class DriverFilterTestCase(HostFiltersTestCase):
         filt_cls = self.class_map['DriverFilter']()
         host1 = fakes.FakeHostState(
             'host1', {
-                'volume_backend_name': 'fake',
                 'capabilities': {
                     'filter_function': '1 / 0 == 0',
                 }
             })
 
-        filter_properties = {
-            'volume_type': {
-                'extra_specs': {
-                    'volume_backend_name': 'fake',
-                }
-            }
-        }
+        filter_properties = {}
 
         self.assertFalse(filt_cls.host_passes(host1, filter_properties))
 
@@ -849,18 +772,12 @@ class DriverFilterTestCase(HostFiltersTestCase):
         filt_cls = self.class_map['DriverFilter']()
         host1 = fakes.FakeHostState(
             'host1', {
-                'volume_backend_name': 'fake',
                 'capabilities': {
                     'filter_function': 'qos.maxiops == 1',
                 }
             })
 
         filter_properties = {
-            'volume_type': {
-                'extra_specs': {
-                    'volume_backend_name': 'fake',
-                }
-            },
             'qos_specs': None
         }
 
@@ -870,23 +787,30 @@ class DriverFilterTestCase(HostFiltersTestCase):
         filt_cls = self.class_map['DriverFilter']()
         host1 = fakes.FakeHostState(
             'host1', {
-                'volume_backend_name': 'fake',
                 'capabilities': {
                     'foo': 10,
                     'filter_function': 'capabilities.foo == 10',
                 },
             })
 
-        filter_properties = {
-            'volume_type': {
-                'extra_specs': {
-                    'volume_backend_name': 'fake',
-                }
-            }
-        }
+        filter_properties = {}
 
         self.assertTrue(filt_cls.host_passes(host1, filter_properties))
 
+    def test_wrong_capabilities(self):
+        filt_cls = self.class_map['DriverFilter']()
+        host1 = fakes.FakeHostState(
+            'host1', {
+                'capabilities': {
+                    'bar': 10,
+                    'filter_function': 'capabilities.foo == 10',
+                },
+            })
+
+        filter_properties = {}
+
+        self.assertFalse(filt_cls.host_passes(host1, filter_properties))
+
 
 class InstanceLocalityFilterTestCase(HostFiltersTestCase):
     def setUp(self):