Allow irrelevant,self-defined specs in ComputeCapacityFilter

For backward compatibility, ComputeCapacityFilter treats extra spec
keys which contain no colons like 'x' the same as 'capabilities:x',
because hoststate doesn't contain attribute like x, this filter always
return False. So it causes conflict with
AggregateInstanceExtraSpecsFilter, and limits user to define extra spec
keys without colons.

This patch solves the conflict and keep it backward compatible.

This patch also joins two lines into one in method host_passes.

Change-Id: Ia9e7c882bcee131e106e67dc46ed9ce1224e4c67
Closes-Bug: #1582589

doc/source/filter_scheduler.rst

@@ -44,33 +44,47 @@ There are many standard filter classes which may be used
   treated as a namespace and anything after the colon is treated as the key to
   be matched. If a namespace is present and is not ``capabilities``, the filter
   ignores the namespace. For example ``capabilities:cpu_info:features`` is
-  a valid scope format. For backward compatibility, the filter also treats the
-  extra specs key as the key to be matched if no namespace is present; this
-  action is highly discouraged because it conflicts with
-  AggregateInstanceExtraSpecsFilter filter when you enable both filters
+  a valid scope format. For backward compatibility, when a key doesn't contain
+  a colon (:), the key's contents are important. If this key is an attribute of
+  HostState object, like ``free_disk_mb``, the filter also treats the extra
+  specs key as the key to be matched. If not, the filter will ignore the key.
 
   The extra specifications can have an operator at the beginning of the value
   string of a key/value pair. If there is no operator specified, then a
   default operator of ``s==`` is used. Valid operators are:
 
-::
+  ::
 
-  * = (equal to or greater than as a number; same as vcpus case)
-  * == (equal to as a number)
-  * != (not equal to as a number)
-  * >= (greater than or equal to as a number)
-  * <= (less than or equal to as a number)
-  * s== (equal to as a string)
-  * s!= (not equal to as a string)
-  * s>= (greater than or equal to as a string)
-  * s> (greater than as a string)
-  * s<= (less than or equal to as a string)
-  * s< (less than as a string)
-  * <in> (substring)
-  * <all-in> (all elements contained in collection)
-  * <or> (find one of these)
+    * = (equal to or greater than as a number; same as vcpus case)
+    * == (equal to as a number)
+    * != (not equal to as a number)
+    * >= (greater than or equal to as a number)
+    * <= (less than or equal to as a number)
+    * s== (equal to as a string)
+    * s!= (not equal to as a string)
+    * s>= (greater than or equal to as a string)
+    * s> (greater than as a string)
+    * s<= (less than or equal to as a string)
+    * s< (less than as a string)
+    * <in> (substring)
+    * <all-in> (all elements contained in collection)
+    * <or> (find one of these)
 
-  Examples are: ">= 5", "s== 2.1.0", "<in> gcc", "<all-in> aes mmx", and "<or> fpu <or> gpu"
+    Examples are: ">= 5", "s== 2.1.0", "<in> gcc", "<all-in> aes mmx", and "<or> fpu <or> gpu"
+
+  some of attributes that can be used as useful key and their values contains:
+
+  ::
+
+    * free_ram_mb (compared with a number, values like ">= 4096")
+    * free_disk_mb (compared with a number, values like ">= 10240")
+    * host (compared with a string, values like: "<in> compute","s== compute_01")
+    * hypervisor_type (compared with a string, values like: "s== QEMU", "s== powervm")
+    * hypervisor_version (compared with a number, values like : ">= 1005003", "== 2000000")
+    * num_instances (compared with a number, values like: "<= 10")
+    * num_io_ops (compared with a number, values like: "<= 5")
+    * vcpus_total (compared with a number, values like: "= 48", ">=24")
+    * vcpus_used (compared with a number, values like: "= 0", "<= 10")
 
 * |AggregateInstanceExtraSpecsFilter| - checks that the aggregate metadata
   satisfies any extra specifications associated with the instance type (that

nova/scheduler/filters/compute_capabilities_filter.py

@@ -74,7 +74,19 @@ class ComputeCapabilitiesFilter(filters.BaseHostFilter):
         for key, req in six.iteritems(instance_type.extra_specs):
             # Either not scope format, or in capabilities scope
             scope = key.split(':')
-            if len(scope) > 1:
+            # If key does not have a namespace, the scope's size is 1, check
+            # whether host_state contains the key as an attribute. If not,
+            # ignore it. If it contains, deal with it in the same way as
+            # 'capabilities:key'. This is for backward-compatible.
+            # If the key has a namespace, the scope's size will be bigger than
+            # 1, check that whether the namespace is 'capabilities'. If not,
+            # ignore it.
+            if len(scope) == 1:
+                stats = getattr(host_state, 'stats', {})
+                has_attr = hasattr(host_state, key) or key in stats
+                if not has_attr:
+                    continue
+            else:
                 if scope[0] != "capabilities":
                     continue
                 else:
@@ -95,8 +107,7 @@ class ComputeCapabilitiesFilter(filters.BaseHostFilter):
     def host_passes(self, host_state, spec_obj):
         """Return a list of hosts that can create instance_type."""
         instance_type = spec_obj.flavor
-        if not self._satisfies_extra_specs(host_state,
-                instance_type):
+        if not self._satisfies_extra_specs(host_state, instance_type):
             LOG.debug("%(host_state)s fails instance_type extra_specs "
                       "requirements", {'host_state': host_state})
             return False

nova/tests/unit/scheduler/filters/test_compute_capabilities_filters.py

@@ -45,7 +45,7 @@ class TestComputeCapabilitiesFilter(test.NoDBTestCase):
         self.assertTrue(self.filt_cls.host_passes(host, spec_obj))
 
     def test_compute_filter_fails_without_host_state(self):
-        especs = {'capabilities': '1'}
+        especs = {'capabilities:opts': '1'}
         spec_obj = objects.RequestSpec(
             flavor=objects.Flavor(memory_mb=1024, extra_specs=especs))
         self.assertFalse(self.filt_cls.host_passes(None, spec_obj))
@@ -72,11 +72,33 @@ class TestComputeCapabilitiesFilter(test.NoDBTestCase):
             especs={'capabilities:cpu_info:vendor': 'Intel'},
             passes=True)
 
-    def test_compute_filter_fail_cpu_info_as_text_type_not_valid(self):
-        cpu_info = "cpu_info"
+    def test_compute_filter_pass_cpu_info_with_backward_compatibility(self):
+        cpu_info = """ { "vendor": "Intel", "model": "core2duo",
+        "arch": "i686","features": ["lahf_lm", "rdtscp"], "topology":
+        {"cores": 1, "threads":1, "sockets": 1}} """
 
         cpu_info = six.text_type(cpu_info)
 
+        self._do_test_compute_filter_extra_specs(
+            ecaps={'cpu_info': cpu_info},
+            especs={'cpu_info': cpu_info},
+            passes=True)
+
+    def test_compute_filter_fail_cpu_info_with_backward_compatibility(self):
+        cpu_info = """ { "vendor": "Intel", "model": "core2duo",
+        "arch": "i686","features": ["lahf_lm", "rdtscp"], "topology":
+        {"cores": 1, "threads":1, "sockets": 1}} """
+
+        cpu_info = six.text_type(cpu_info)
+
+        self._do_test_compute_filter_extra_specs(
+            ecaps={'cpu_info': cpu_info},
+            especs={'cpu_info': ''},
+            passes=False)
+
+    def test_compute_filter_fail_cpu_info_as_text_type_not_valid(self):
+        cpu_info = "cpu_info"
+        cpu_info = six.text_type(cpu_info)
         self._do_test_compute_filter_extra_specs(
             ecaps={'cpu_info': cpu_info},
             especs={'capabilities:cpu_info:vendor': 'Intel'},
@@ -108,6 +130,13 @@ class TestComputeCapabilitiesFilter(test.NoDBTestCase):
             especs={'capabilities': '1'},
             passes=True)
 
+    def test_compute_filter_pass_self_defined_specs(self):
+        # Make sure this will not reject user's self-defined,irrelevant specs
+        self._do_test_compute_filter_extra_specs(
+            ecaps={'opt1': 1, 'opt2': 2},
+            especs={'XXYY': '1'},
+            passes=True)
+
     def test_compute_filter_extra_specs_simple_with_wrong_scope(self):
         self._do_test_compute_filter_extra_specs(
             ecaps={'opt1': 1, 'opt2': 2},
@@ -121,3 +150,39 @@ class TestComputeCapabilitiesFilter(test.NoDBTestCase):
             especs={'opt1:a': '1', 'capabilities:opt1:b:aa': '2',
                     'trust:trusted_host': 'true'},
             passes=True)
+
+    def test_compute_filter_pass_ram_with_backward_compatibility(self):
+        self._do_test_compute_filter_extra_specs(
+            ecaps={},
+            especs={'free_ram_mb': '>= 300'},
+            passes=True)
+
+    def test_compute_filter_fail_ram_with_backward_compatibility(self):
+        self._do_test_compute_filter_extra_specs(
+            ecaps={},
+            especs={'free_ram_mb': '<= 300'},
+            passes=False)
+
+    def test_compute_filter_pass_cpu_with_backward_compatibility(self):
+        self._do_test_compute_filter_extra_specs(
+            ecaps={},
+            especs={'vcpus_used': '<= 20'},
+            passes=True)
+
+    def test_compute_filter_fail_cpu_with_backward_compatibility(self):
+        self._do_test_compute_filter_extra_specs(
+            ecaps={},
+            especs={'vcpus_used': '>= 20'},
+            passes=False)
+
+    def test_compute_filter_pass_disk_with_backward_compatibility(self):
+        self._do_test_compute_filter_extra_specs(
+            ecaps={},
+            especs={'free_disk_mb': 0},
+            passes=True)
+
+    def test_compute_filter_fail_disk_with_backward_compatibility(self):
+        self._do_test_compute_filter_extra_specs(
+            ecaps={},
+            especs={'free_disk_mb': 1},
+            passes=False)