From 8bc80cab3c134d64953b4f6493cab80b5bdc1b69 Mon Sep 17 00:00:00 2001
From: Hans Lindgren <hanlind@kth.se>
Date: Sun, 27 Jan 2013 21:19:16 +0100
Subject: Fix an error in affinity filters

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
---
 nova/scheduler/filters/affinity_filter.py | 22 +++++++---------------
 nova/tests/scheduler/test_host_filters.py | 28 ++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/nova/scheduler/filters/affinity_filter.py b/nova/scheduler/filters/affinity_filter.py
index 03bf0dd6e..7e51a15f2 100644
--- a/nova/scheduler/filters/affinity_filter.py
+++ b/nova/scheduler/filters/affinity_filter.py
@@ -25,12 +25,6 @@ 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.'''
@@ -38,15 +32,15 @@ class DifferentHostFilter(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('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
 
@@ -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
 
diff --git a/nova/tests/scheduler/test_host_filters.py b/nova/tests/scheduler/test_host_filters.py
index 6fcd19d92..230e2ea03 100644
--- a/nova/tests/scheduler/test_host_filters.py
+++ b/nova/tests/scheduler/test_host_filters.py
@@ -337,6 +337,20 @@ class HostFiltersTestCase(test.TestCase):
 
         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', 'node1', {})
@@ -388,6 +402,20 @@ class HostFiltersTestCase(test.TestCase):
 
         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', 'node1', {})
-- 
cgit