Deprecate TypeAffinityFilter

This deprecates the TypeAffinityFilter. This filter, which
is really an anti-affinity filter for flavors, attempts to
ensure that no two flavors show up on the same host. However,
to do this it relies on the flavors.id primary key, which is
subject to "change" if/when the admin deletes and recreates a
flavor (this is how Horizon allows you to 'edit' a flavor).
When you do that, you have a new flavor with a new id primary
key and the filter will not know the difference. So you could
really end up with more than one m1.large instance on the
same host, which defeats the purpose of the filter.

This filter is also problematic for
blueprint put-host-manager-instance-info-on-a-diet because in
that blueprint we want to stop sending full instance objects
over RPC from all compute services to the scheduler just to
track affinity and anti-affinity. All we really need for the
ServerGroupAffinityFilter and ServerGroupAntiAffinityFilter
is the list of instance uuids on a host, not the full object.
If we can get rid of TypeAffinityFilter, we can change the
compute<>scheduler RPC calls to just pass the list of instance
uuids for update_instance_info rather than sending an InstanceList.

Change-Id: I660e0316b11afcad65c0fe7bd167ddcec9239a8b

4 files changed, 44 insertions, 2 deletions

diff --git a/doc/source/filter_scheduler.rst b/doc/source/filter_scheduler.rst
index 8e41e87039..407f23e7df 100644
--- a/doc/source/filter_scheduler.rst
+++ b/doc/source/filter_scheduler.rst
@@ -164,6 +164,15 @@ There are many standard filter classes which may be used
 
 * |TypeAffinityFilter| - Only passes hosts that are not already running an
   instance of the requested type.
+
+  .. warning:: TypeAffinityFilter is deprecated for removal in the
+    17.0.0 Queens release. There is no replacement planned for this
+    filter. It is fundamentally flawed in that it relies on the
+    ``flavors.id`` primary key and if a flavor "changed", i.e. deleted
+    and re-created with new values, it will result in this filter
+    thinking it is a different flavor, thus breaking the usefulness of
+    this filter.
+
 * |AggregateTypeAffinityFilter| - limits instance_type by aggregate.
    This filter passes hosts if no instance_type key is set or
    the instance_type aggregate metadata value contains the name of the
diff --git a/nova/scheduler/filters/type_filter.py b/nova/scheduler/filters/type_filter.py
index b10060db0e..7b9a0ad54f 100644
--- a/nova/scheduler/filters/type_filter.py
+++ b/nova/scheduler/filters/type_filter.py
@@ -14,17 +14,33 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from oslo_log import log as logging
+
 from nova.scheduler import filters
 from nova.scheduler.filters import utils
 
+LOG = logging.getLogger(__name__)
+
 
 class TypeAffinityFilter(filters.BaseHostFilter):
-    """TypeAffinityFilter doesn't allow more than one VM type per host.
+    """DEPRECATED: TypeAffinityFilter doesn't allow more than one VM type
+    per host.
 
     Note: this works best with ram_weight_multiplier
     (spread) set to 1 (default).
     """
 
+    def __init__(self):
+        super(TypeAffinityFilter, self).__init__()
+        LOG.warning('TypeAffinityFilter is deprecated for removal in the '
+                    '17.0.0 Queens release. There is no replacement planned '
+                    'for this filter. It is fundamentally flawed in that it '
+                    'relies on the flavors.id primary key and if a flavor '
+                    '\"changed\" (deleted and re-created with new values) '
+                    'it will result in this filter thinking it is a '
+                    'different flavor, thus breaking the usefulness of this '
+                    'filter.')
+
     def host_passes(self, host_state, spec_obj):
         """Dynamically limits hosts to one instance type
 
diff --git a/nova/tests/unit/scheduler/filters/test_type_filters.py b/nova/tests/unit/scheduler/filters/test_type_filters.py
index da7efee539..5180dc4c59 100644
--- a/nova/tests/unit/scheduler/filters/test_type_filters.py
+++ b/nova/tests/unit/scheduler/filters/test_type_filters.py
@@ -12,6 +12,8 @@
 
 import mock
 
+import six
+
 from nova import objects
 from nova.scheduler.filters import type_filter
 from nova import test
@@ -22,7 +24,12 @@ from nova.tests import uuidsentinel as uuids
 class TestTypeFilter(test.NoDBTestCase):
 
     def test_type_filter(self):
-        self.filt_cls = type_filter.TypeAffinityFilter()
+        with mock.patch.object(type_filter.LOG, 'warning') as mock_warning:
+            self.filt_cls = type_filter.TypeAffinityFilter()
+        # make sure we logged a deprecation warning
+        self.assertEqual(1, mock_warning.call_count)
+        self.assertIn('TypeAffinityFilter is deprecated for removal',
+                      six.text_type(mock_warning.call_args_list[0][0]))
         host = fakes.FakeHostState('fake_host', 'fake_node', {})
         host.instances = {}
         target_id = 1
diff --git a/releasenotes/notes/deprecate-TypeAffinityFilter-465c47a6b2a7bd77.yaml b/releasenotes/notes/deprecate-TypeAffinityFilter-465c47a6b2a7bd77.yaml
new file mode 100644
index 0000000000..c21fff8882
--- /dev/null
+++ b/releasenotes/notes/deprecate-TypeAffinityFilter-465c47a6b2a7bd77.yaml
@@ -0,0 +1,10 @@
+---
+deprecations:
+  - |
+    TypeAffinityFilter is deprecated for removal in the
+    17.0.0 Queens release. There is no replacement planned for this
+    filter. It is fundamentally flawed in that it relies on the
+    ``flavors.id`` primary key and if a flavor "changed", i.e. deleted
+    and re-created with new values, it will result in this filter
+    thinking it is a different flavor, thus breaking the usefulness of
+    this filter.