From 91d007426f109dfef2142e28741edd51dcf1fdbc Mon Sep 17 00:00:00 2001
From: Mark Washenberger <mark.washenberger@rackspace.com>
Date: Fri, 11 May 2012 09:47:10 -0400
Subject: Eliminate a race condition on instance deletes.

- Add constraint and equality conditions to nova.db[.sqlalchemy].api
- Use host constraints to ensure the compute api doesn't simply delete
  an instance from the database that a compute manager has already
  started to run.

This race condition is associated with bug #998117

Change-Id: Id74192d3e66bea073327342f57ce0f26987efd2d
---
 nova/tests/api/openstack/compute/test_servers.py |  4 +--
 nova/tests/compute/test_compute.py               | 23 +++++++++++----
 nova/tests/test_db_api.py                        | 37 ++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 8 deletions(-)

(limited to 'nova/tests')

diff --git a/nova/tests/api/openstack/compute/test_servers.py b/nova/tests/api/openstack/compute/test_servers.py
index 83a8963a5..f9e815c20 100644
--- a/nova/tests/api/openstack/compute/test_servers.py
+++ b/nova/tests/api/openstack/compute/test_servers.py
@@ -1298,7 +1298,7 @@ class ServersControllerTest(test.TestCase):
         self.stubs.Set(nova.db, 'instance_get_by_uuid',
                 fakes.fake_instance_get(vm_state=vm_states.ACTIVE))
 
-        def instance_destroy_mock(context, id):
+        def instance_destroy_mock(*args, **kwargs):
             self.server_delete_called = True
         self.stubs.Set(nova.db, 'instance_destroy', instance_destroy_mock)
 
@@ -1313,7 +1313,7 @@ class ServersControllerTest(test.TestCase):
 
         self.server_delete_called = False
 
-        def instance_destroy_mock(context, id):
+        def instance_destroy_mock(*args, **kwargs):
             self.server_delete_called = True
         self.stubs.Set(nova.db, 'instance_destroy', instance_destroy_mock)
 
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index f1903bd4c..8530eca7b 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -2270,6 +2270,22 @@ class ComputeAPITestCase(BaseTestCase):
 
         db.instance_destroy(self.context, instance['id'])
 
+    def test_delete_fast_if_host_not_set(self):
+        instance = self._create_fake_instance({'host': None})
+        self.compute_api.delete(self.context, instance)
+        self.assertRaises(exception.InstanceNotFound, db.instance_get_by_uuid,
+                          self.context, instance['uuid'])
+
+    def test_delete_handles_host_setting_race_condition(self):
+        instance, instance_uuid = self._run_instance()
+        instance['host'] = None  # make it think host was never set
+        self.compute_api.delete(self.context, instance)
+
+        instance = db.instance_get_by_uuid(self.context, instance_uuid)
+        self.assertEqual(instance['task_state'], task_states.DELETING)
+
+        db.instance_destroy(self.context, instance['id'])
+
     def test_delete_fail(self):
         instance, instance_uuid = self._run_instance()
 
@@ -3950,12 +3966,7 @@ class ComputePolicyTestCase(BaseTestCase):
         nova.compute.api.check_policy(self.context, 'reboot', {})
 
     def test_wrapped_method(self):
-        instance = self._create_fake_instance()
-        # Reset this to None for this policy check. If it's set, it
-        # tries to do a compute_api.update() and we're not testing for
-        # that here.
-        instance['host'] = None
-        self.compute.run_instance(self.context, instance['uuid'])
+        instance = self._create_fake_instance(params={'host': None})
 
         # force delete to fail
         rules = {"compute:delete": [["false:false"]]}
diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py
index 8ce2ab6ee..26feb0fc2 100644
--- a/nova/tests/test_db_api.py
+++ b/nova/tests/test_db_api.py
@@ -840,3 +840,40 @@ class TestIpAllocation(test.TestCase):
         fixed_ip = db.fixed_ip_get_by_address(self.ctxt, address)
         self.assertEqual(fixed_ip.instance_id, self.instance.id)
         self.assertEqual(fixed_ip.network_id, self.network.id)
+
+
+class InstanceDestroyConstraints(test.TestCase):
+
+    def test_destroy_with_equal_any_constraint_met(self):
+        ctx = context.get_admin_context()
+        instance = db.instance_create(ctx, {'task_state': 'deleting'})
+        constraint = db.constraint(task_state=db.equal_any('deleting'))
+        db.instance_destroy(ctx, instance['uuid'], constraint)
+        self.assertRaises(exception.InstanceNotFound, db.instance_get_by_uuid,
+                          ctx, instance['uuid'])
+
+    def test_destroy_with_equal_any_constraint_not_met(self):
+        ctx = context.get_admin_context()
+        instance = db.instance_create(ctx, {'vm_state': 'resize'})
+        constraint = db.constraint(vm_state=db.equal_any('active', 'error'))
+        self.assertRaises(exception.ConstraintNotMet, db.instance_destroy,
+                          ctx, instance['uuid'], constraint)
+        instance = db.instance_get_by_uuid(ctx, instance['uuid'])
+        self.assertFalse(instance['deleted'])
+
+    def test_destroy_with_not_equal_constraint_met(self):
+        ctx = context.get_admin_context()
+        instance = db.instance_create(ctx, {'task_state': 'deleting'})
+        constraint = db.constraint(task_state=db.not_equal('error', 'resize'))
+        db.instance_destroy(ctx, instance['uuid'], constraint)
+        self.assertRaises(exception.InstanceNotFound, db.instance_get_by_uuid,
+                          ctx, instance['uuid'])
+
+    def test_destroy_with_not_equal_constraint_not_met(self):
+        ctx = context.get_admin_context()
+        instance = db.instance_create(ctx, {'vm_state': 'active'})
+        constraint = db.constraint(vm_state=db.not_equal('active', 'error'))
+        self.assertRaises(exception.ConstraintNotMet, db.instance_destroy,
+                          ctx, instance['uuid'], constraint)
+        instance = db.instance_get_by_uuid(ctx, instance['uuid'])
+        self.assertFalse(instance['deleted'])
-- 
cgit