Support None value of extra_specs in cinder-scheduler filter

Now cinder backends hava capabilities of None value.
Cinder-api also allow user to set volume type's extra_specs to None
value. However we cann't use these capabilities in cinder scheduler
filter.
This change supports None value of extra_specs in cinder-scheduler
filter.

Further more, Our VolumeType object don't allow none value of
extra_specs and this causs failure when trying to create a
volume from such volume type. This commit also changes that
field type to DictOfNullableStringsField which allows it.

Also object backporting procedure is added to make sure that before
sending data to older services we're switching None to empty string.
(PS: if older service use functions like volume.refersh to read
None value of extra_spec from db itself, it will hit bug #1588798
anyway.)

Co-Authored-By: Michal Dulko <michal.dulko@intel.com>

Change-Id: I301adc5980dbacec5e84de7c66e6d12d132643cd
Closes-bug: #1588798

cinder/objects/base.py

@@ -99,6 +99,7 @@ OBJ_VERSIONS.add('1.1', {'Service': '1.2', 'ServiceList': '1.1'})
 OBJ_VERSIONS.add('1.2', {'Backup': '1.4', 'BackupImport': '1.4'})
 OBJ_VERSIONS.add('1.3', {'Service': '1.3'})
 OBJ_VERSIONS.add('1.4', {'Snapshot': '1.1'})
+OBJ_VERSIONS.add('1.5', {'VolumeType': '1.1'})
 
 
 class CinderObjectRegistry(base.VersionedObjectRegistry):

cinder/objects/volume_type.py

@@ -12,6 +12,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from oslo_utils import versionutils
 from oslo_versionedobjects import fields
 
 from cinder import exception
@@ -28,7 +29,8 @@ OPTIONAL_FIELDS = ['extra_specs', 'projects']
 class VolumeType(base.CinderPersistentObject, base.CinderObject,
                  base.CinderObjectDictCompat, base.CinderComparableObject):
     # Version 1.0: Initial version
-    VERSION = '1.0'
+    # Version 1.1: Changed extra_specs to DictOfNullableStringsField
+    VERSION = '1.1'
 
     fields = {
         'id': fields.UUIDField(),
@@ -36,9 +38,22 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject,
         'description': fields.StringField(nullable=True),
         'is_public': fields.BooleanField(default=True, nullable=True),
         'projects': fields.ListOfStringsField(nullable=True),
-        'extra_specs': fields.DictOfStringsField(nullable=True),
+        'extra_specs': fields.DictOfNullableStringsField(nullable=True),
     }
 
+    def obj_make_compatible(self, primitive, target_version):
+        super(VolumeType, self).obj_make_compatible(primitive, target_version)
+
+        target_version = versionutils.convert_version_to_tuple(target_version)
+        if target_version < (1, 1):
+            if primitive.get('extra_specs'):
+                # Before 1.1 extra_specs field didn't allowed None values. To
+                # make sure we won't explode on receiver side - change Nones to
+                # empty string.
+                for k, v in primitive['extra_specs'].items():
+                    if v is None:
+                        primitive['extra_specs'][k] = ''
+
     @classmethod
     def _get_expected_attrs(cls, context):
         return 'extra_specs', 'projects'

cinder/scheduler/filters/capabilities_filter.py

@@ -46,10 +46,8 @@ class CapabilitiesFilter(filters.BaseHostFilter):
             cap = capabilities
             for index in range(len(scope)):
                 try:
-                    cap = cap.get(scope[index])
-                except AttributeError:
-                    return False
-                if cap is None:
+                    cap = cap[scope[index]]
+                except (TypeError, KeyError):
                     LOG.debug("Host doesn't provide capability '%(cap)s' " %
                               {'cap': scope[index]})
                     return False

cinder/scheduler/filters/extra_specs_ops.py

@@ -39,6 +39,11 @@ _op_methods = {'=': lambda x, y: float(x) >= float(y),
 
 
 def match(value, req):
+    if req is None:
+        if value is None:
+            return True
+        else:
+            return False
     words = req.split()
 
     op = method = None

cinder/tests/unit/objects/test_objects.py

@@ -38,7 +38,7 @@ object_data = {
     'VolumeList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e',
     'VolumeAttachment': '1.0-b30dacf62b2030dd83d8a1603f1064ff',
     'VolumeAttachmentList': '1.0-15ecf022a68ddbb8c2a6739cfc9f8f5e',
-    'VolumeType': '1.0-6673dd9ce7c27e9c85279afb20833877',
+    'VolumeType': '1.1-6673dd9ce7c27e9c85279afb20833877',
     'VolumeTypeList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e',
 }
 

cinder/tests/unit/objects/test_volume_type.py

@@ -31,6 +31,23 @@ class TestVolumeType(test_objects.BaseObjectsTestCase):
                                                    fake.VOLUME_TYPE_ID)
         self._compare(self, db_volume_type, volume_type)
 
+    @mock.patch('cinder.db.sqlalchemy.api._volume_type_get_full')
+    def test_get_by_id_null_spec(self, volume_type_get):
+        db_volume_type = fake_volume.fake_db_volume_type(
+            extra_specs={'foo': None})
+        volume_type_get.return_value = db_volume_type
+        volume_type = objects.VolumeType.get_by_id(self.context,
+                                                   fake.VOLUME_TYPE_ID)
+        self._compare(self, db_volume_type, volume_type)
+
+    def test_obj_make_compatible(self):
+        volume_type = objects.VolumeType(context=self.context)
+        volume_type.extra_specs = {'foo': None, 'bar': 'baz'}
+        primitive = volume_type.obj_to_primitive('1.0')
+        volume_type = objects.VolumeType.obj_from_primitive(primitive)
+        self.assertEqual('', volume_type.extra_specs['foo'])
+        self.assertEqual('baz', volume_type.extra_specs['bar'])
+
     @mock.patch('cinder.volume.volume_types.create')
     def test_create(self, volume_type_create):
         db_volume_type = fake_volume.fake_db_volume_type()

cinder/tests/unit/scheduler/test_host_filters.py

@@ -1236,6 +1236,18 @@ class ExtraSpecsOpsTestCase(test.TestCase):
             req='>= 3',
             matches=False)
 
+    def test_extra_specs_fails_none_req(self):
+        self._do_extra_specs_ops_test(
+            value='foo',
+            req=None,
+            matches=False)
+
+    def test_extra_specs_matches_none_req(self):
+        self._do_extra_specs_ops_test(
+            value=None,
+            req=None,
+            matches=True)
+
 
 class BasicFiltersTestCase(HostFiltersTestCase):
     """Test case for host filters."""
@@ -1316,12 +1328,36 @@ class BasicFiltersTestCase(HostFiltersTestCase):
             especs={'capabilities:scope_lv0:scope_lv1:scope_lv2:opt1': '>= 2'},
             passes=True)
 
+    def test_capability_filter_fails_unenough_level_scope_extra_specs(self):
+        self._do_test_type_filter_extra_specs(
+            ecaps={'scope_lv0': {'scope_lv1': None}},
+            especs={'capabilities:scope_lv0:scope_lv1:scope_lv2:opt1': '>= 2'},
+            passes=False)
+
     def test_capability_filter_fails_wrong_scope_extra_specs(self):
         self._do_test_type_filter_extra_specs(
             ecaps={'scope_lv0': {'opt1': 10}},
             especs={'capabilities:scope_lv1:opt1': '>= 2'},
             passes=False)
 
+    def test_capability_filter_passes_none_extra_specs(self):
+        self._do_test_type_filter_extra_specs(
+            ecaps={'scope_lv0': {'opt1': None}},
+            especs={'capabilities:scope_lv0:opt1': None},
+            passes=True)
+
+    def test_capability_filter_fails_none_extra_specs(self):
+        self._do_test_type_filter_extra_specs(
+            ecaps={'scope_lv0': {'opt1': 10}},
+            especs={'capabilities:scope_lv0:opt1': None},
+            passes=False)
+
+    def test_capability_filter_fails_none_caps(self):
+        self._do_test_type_filter_extra_specs(
+            ecaps={'scope_lv0': {'opt1': None}},
+            especs={'capabilities:scope_lv0:opt1': 'foo'},
+            passes=False)
+
     def test_json_filter_passes(self):
         filt_cls = self.class_map['JsonFilter']()
         filter_properties = {'resource_type': {'memory_mb': 1024,