From ed3c69cb45c234be35fce04413650d9054ac57b4 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Thu, 14 Dec 2017 17:01:11 -0500 Subject: [PATCH] Delete the TypeAffinityFilter Deprecated in Pike: I660e0316b11afcad65c0fe7bd167ddcec9239a8b This filter relies on the flavor.id primary key which will change as (1) flavors were migrated to the API database and (2) when a flavor is changed by deleting and re-creating the flavor. Also, as noted in blueprint put-host-manager-instance-info-on-a-diet, this is one step forward in getting us to a point where the only thing that the in-tree filters care about in the HostState.instances dict is the instance uuid (for the affinity filters). Which means we can eventually stop RPC casting all instance information from all nova-compute services to the scheduler for every instance create, delete, move or periodic sync task - we only would need to send the list of instance UUIDs. That should help with RPC traffic in a large and busy deployment. Change-Id: Icb43fe2ef5252d2838f6f8572c7497840a9797a1 --- doc/source/admin/configuration/schedulers.rst | 11 +----- doc/source/user/filter-scheduler.rst | 12 ------ nova/scheduler/filters/type_filter.py | 38 ------------------- nova/scheduler/filters/utils.py | 13 ------- .../scheduler/filters/test_type_filters.py | 32 ---------------- .../unit/scheduler/filters/test_utils.py | 7 ---- ...e-TypeAffinityFilter-61bb92d1382f4a68.yaml | 11 ++++++ 7 files changed, 12 insertions(+), 112 deletions(-) create mode 100644 releasenotes/notes/delete-TypeAffinityFilter-61bb92d1382f4a68.yaml diff --git a/doc/source/admin/configuration/schedulers.rst b/doc/source/admin/configuration/schedulers.rst index 1d85696b7016..6967b5d8ab32 100644 --- a/doc/source/admin/configuration/schedulers.rst +++ b/doc/source/admin/configuration/schedulers.rst @@ -258,7 +258,7 @@ This filter passes hosts if no ``instance_type`` key is set or the is a string that may contain either a single ``instance_type`` name or a comma-separated list of ``instance_type`` names, such as ``m1.nano`` or ``m1.nano,m1.small``. For information about how to use this filter, see -:ref:`host-aggregates`. See also :ref:`TypeAffinityFilter`. +:ref:`host-aggregates`. AllHostsFilter -------------- @@ -753,15 +753,6 @@ With the API, use the ``os:scheduler_hints`` key: } } -.. _TypeAffinityFilter: - -TypeAffinityFilter ------------------- - -Dynamically limits hosts to one instance type. An instance can only be launched -on a host, if no instance with different instances types are running on it, or -if the host has no running instances at all. - Cell filters ~~~~~~~~~~~~ diff --git a/doc/source/user/filter-scheduler.rst b/doc/source/user/filter-scheduler.rst index 1f2574f2f89f..78f7757c338c 100644 --- a/doc/source/user/filter-scheduler.rst +++ b/doc/source/user/filter-scheduler.rst @@ -154,17 +154,6 @@ There are many standard filter classes which may be used a set of instances. * |RetryFilter| - filters hosts that have been attempted for scheduling. Only passes hosts that have not been previously attempted. -* |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 @@ -485,7 +474,6 @@ in :mod:`nova.tests.scheduler`. .. |DifferentHostFilter| replace:: :class:`DifferentHostFilter ` .. |SameHostFilter| replace:: :class:`SameHostFilter ` .. |RetryFilter| replace:: :class:`RetryFilter ` -.. |TypeAffinityFilter| replace:: :class:`TypeAffinityFilter ` .. |AggregateTypeAffinityFilter| replace:: :class:`AggregateTypeAffinityFilter ` .. |ServerGroupAntiAffinityFilter| replace:: :class:`ServerGroupAntiAffinityFilter ` .. |ServerGroupAffinityFilter| replace:: :class:`ServerGroupAffinityFilter ` diff --git a/nova/scheduler/filters/type_filter.py b/nova/scheduler/filters/type_filter.py index 4248d50b6146..5b386cf83fee 100644 --- a/nova/scheduler/filters/type_filter.py +++ b/nova/scheduler/filters/type_filter.py @@ -14,47 +14,9 @@ # 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): - """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). - """ - - RUN_ON_REBUILD = False - - 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 - - Return False if host has any instance types other than the requested - type. Return True if all instance types match or if host is empty. - """ - instance_type = spec_obj.flavor - instance_type_id = instance_type.id - other_types_on_host = utils.other_types_on_host(host_state, - instance_type_id) - return not other_types_on_host - class AggregateTypeAffinityFilter(filters.BaseHostFilter): """AggregateTypeAffinityFilter limits instance_type by aggregate diff --git a/nova/scheduler/filters/utils.py b/nova/scheduler/filters/utils.py index 922be906c01f..fcc7916ff48d 100644 --- a/nova/scheduler/filters/utils.py +++ b/nova/scheduler/filters/utils.py @@ -83,16 +83,3 @@ def instance_uuids_overlap(host_state, uuids): # host_state.instances is a dict whose keys are the instance uuids host_uuids = set(host_state.instances.keys()) return bool(host_uuids.intersection(set_uuids)) - - -def other_types_on_host(host_state, instance_type_id): - """Tests for overlap between a host_state's instances and an - instance_type_id. - - Returns True if there are any instances in the host_state whose - instance_type_id is different than the supplied instance_type_id value. - """ - host_instances = host_state.instances.values() - host_types = set([inst.instance_type_id for inst in host_instances]) - inst_set = set([instance_type_id]) - return bool(host_types - inst_set) diff --git a/nova/tests/unit/scheduler/filters/test_type_filters.py b/nova/tests/unit/scheduler/filters/test_type_filters.py index 5180dc4c59f6..d3f01a5c0ee8 100644 --- a/nova/tests/unit/scheduler/filters/test_type_filters.py +++ b/nova/tests/unit/scheduler/filters/test_type_filters.py @@ -12,46 +12,14 @@ import mock -import six - from nova import objects from nova.scheduler.filters import type_filter from nova import test from nova.tests.unit.scheduler import fakes -from nova.tests import uuidsentinel as uuids class TestTypeFilter(test.NoDBTestCase): - def test_type_filter(self): - 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 - spec_obj = objects.RequestSpec( - context=mock.MagicMock(), - flavor=objects.Flavor(id=target_id)) - # True since no instances on host - self.assertTrue(self.filt_cls.host_passes(host, spec_obj)) - # Add an instance with the same instance_type_id - inst1 = objects.Instance(uuid=uuids.instance_1, - instance_type_id=target_id) - host.instances = {inst1.uuid: inst1} - # True since only same instance_type_id on host - self.assertTrue(self.filt_cls.host_passes(host, spec_obj)) - # Add an instance with a different instance_type_id - diff_type = target_id + 1 - inst2 = objects.Instance(uuid=uuids.instance_2, - instance_type_id=diff_type) - host.instances.update({inst2.uuid: inst2}) - # False since host now has an instance of a different type - self.assertFalse(self.filt_cls.host_passes(host, spec_obj)) - @mock.patch('nova.scheduler.filters.utils.aggregate_values_from_key') def test_aggregate_type_filter_no_metadata(self, agg_mock): self.filt_cls = type_filter.AggregateTypeAffinityFilter() diff --git a/nova/tests/unit/scheduler/filters/test_utils.py b/nova/tests/unit/scheduler/filters/test_utils.py index 5f2a2d2dfd07..00f75411e70b 100644 --- a/nova/tests/unit/scheduler/filters/test_utils.py +++ b/nova/tests/unit/scheduler/filters/test_utils.py @@ -106,10 +106,3 @@ class TestUtils(test.NoDBTestCase): self.assertTrue(utils.instance_uuids_overlap(host_state, [uuids.instance_1])) self.assertFalse(utils.instance_uuids_overlap(host_state, ['zz'])) - - def test_other_types_on_host(self): - inst1 = objects.Instance(uuid=uuids.instance, instance_type_id=1) - host_state = fakes.FakeHostState('host1', 'node1', {}) - host_state.instances = {inst1.uuid: inst1} - self.assertFalse(utils.other_types_on_host(host_state, 1)) - self.assertTrue(utils.other_types_on_host(host_state, 2)) diff --git a/releasenotes/notes/delete-TypeAffinityFilter-61bb92d1382f4a68.yaml b/releasenotes/notes/delete-TypeAffinityFilter-61bb92d1382f4a68.yaml new file mode 100644 index 000000000000..004741a582cd --- /dev/null +++ b/releasenotes/notes/delete-TypeAffinityFilter-61bb92d1382f4a68.yaml @@ -0,0 +1,11 @@ +--- +upgrade: + - | + The ``TypeAffinityFilter``, which was deprecated in the 16.0.0 Pike + release, has been removed. The filter was flawed in that it relied on the + flavor ``id`` primary key which cannot be relied upon since you cannot + "edit" a flavor to change its disk, vcpu, etc values. Therefore to change + a given flavor, it must be deleted and re-created, which means a new ``id`` + value, thus potentially breaking the usefulness of the filter. Also, the + flavor migration from the ``nova`` database to the ``nova_api`` database + would also have resulted in different ``id`` values.