Skip to content

Commit

Permalink
Fix an error in affinity filters
Browse files Browse the repository at this point in the history
Fix a call to compute_api.get_all to properly exclude deleted instances.
Add tests to prohibit this error in the future.

After some thought, this can be further optimized to let the instance
query do all the filtering/matching.

Resolves bug 1107156.

Change-Id: I6d6a6ba44d38d363489853d0407ad4cc94046656
(cherry picked from commit 8bc80ca)
  • Loading branch information
hanlind committed Jan 28, 2013
1 parent f6081d0 commit d6b9d33
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 15 deletions.
22 changes: 7 additions & 15 deletions nova/scheduler/filters/affinity_filter.py
Expand Up @@ -25,28 +25,22 @@ class AffinityFilter(filters.BaseHostFilter):
def __init__(self):
self.compute_api = compute.API()

def _all_hosts(self, context):
all_hosts = {}
for instance in self.compute_api.get_all(context):
all_hosts[instance['uuid']] = instance['host']
return all_hosts


class DifferentHostFilter(AffinityFilter):
'''Schedule the instance on a different host from a set of instances.'''

def host_passes(self, host_state, filter_properties):
context = filter_properties['context']
scheduler_hints = filter_properties.get('scheduler_hints') or {}
me = host_state.host

affinity_uuids = scheduler_hints.get('different_host', [])
if isinstance(affinity_uuids, basestring):
affinity_uuids = [affinity_uuids]
if affinity_uuids:
all_hosts = self._all_hosts(context)
return not any([i for i in affinity_uuids
if all_hosts.get(i) == me])
return not self.compute_api.get_all(context,
{'host': host_state.host,
'uuid': affinity_uuids,
'deleted': False})
# With no different_host key
return True

Expand All @@ -59,16 +53,14 @@ class SameHostFilter(AffinityFilter):
def host_passes(self, host_state, filter_properties):
context = filter_properties['context']
scheduler_hints = filter_properties.get('scheduler_hints') or {}
me = host_state.host

affinity_uuids = scheduler_hints.get('same_host', [])
if isinstance(affinity_uuids, basestring):
affinity_uuids = [affinity_uuids]
if affinity_uuids:
all_hosts = self._all_hosts(context)
return any([i for i
in affinity_uuids
if all_hosts.get(i) == me])
return self.compute_api.get_all(context, {'host': host_state.host,
'uuid': affinity_uuids,
'deleted': False})
# With no same_host key
return True

Expand Down
28 changes: 28 additions & 0 deletions nova/tests/scheduler/test_host_filters.py
Expand Up @@ -350,6 +350,20 @@ def test_affinity_different_filter_handles_none(self):

self.assertTrue(filt_cls.host_passes(host, filter_properties))

def test_affinity_different_filter_handles_deleted_instance(self):
filt_cls = self.class_map['DifferentHostFilter']()
host = fakes.FakeHostState('host1', 'node1', {})
instance = fakes.FakeInstance(context=self.context,
params={'host': 'host1'})
instance_uuid = instance.uuid
db.instance_destroy(self.context, instance_uuid)

filter_properties = {'context': self.context.elevated(),
'scheduler_hints': {
'different_host': [instance_uuid], }}

self.assertTrue(filt_cls.host_passes(host, filter_properties))

def test_affinity_same_filter_no_list_passes(self):
filt_cls = self.class_map['SameHostFilter']()
host = fakes.FakeHostState('host1', 'compute', {})
Expand Down Expand Up @@ -401,6 +415,20 @@ def test_affinity_same_filter_handles_none(self):

self.assertTrue(filt_cls.host_passes(host, filter_properties))

def test_affinity_same_filter_handles_deleted_instance(self):
filt_cls = self.class_map['SameHostFilter']()
host = fakes.FakeHostState('host1', 'node1', {})
instance = fakes.FakeInstance(context=self.context,
params={'host': 'host1'})
instance_uuid = instance.uuid
db.instance_destroy(self.context, instance_uuid)

filter_properties = {'context': self.context.elevated(),
'scheduler_hints': {
'same_host': [instance_uuid], }}

self.assertFalse(filt_cls.host_passes(host, filter_properties))

def test_affinity_simple_cidr_filter_passes(self):
filt_cls = self.class_map['SimpleCIDRAffinityFilter']()
host = fakes.FakeHostState('host1', 'compute', {})
Expand Down

0 comments on commit d6b9d33

Please sign in to comment.