From 46ba7e8df5a2d11fbca72f9b8a0b7d8cc1bbe06f Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Tue, 18 Apr 2017 18:10:06 -0400 Subject: [PATCH] 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 --- doc/source/filter_scheduler.rst | 9 +++++++++ nova/scheduler/filters/type_filter.py | 18 +++++++++++++++++- .../scheduler/filters/test_type_filters.py | 9 ++++++++- ...te-TypeAffinityFilter-465c47a6b2a7bd77.yaml | 10 ++++++++++ 4 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/deprecate-TypeAffinityFilter-465c47a6b2a7bd77.yaml diff --git a/doc/source/filter_scheduler.rst b/doc/source/filter_scheduler.rst index 8e41e8703983..407f23e7df96 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 b10060db0eb9..7b9a0ad54fba 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 da7efee539b5..5180dc4c59f6 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 000000000000..c21fff8882de --- /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.