Merge "Add a non-racy check for unique stack names"

3 files changed, 29 insertions, 1 deletions

diff --git a/heat/db/sqlalchemy/api.py b/heat/db/sqlalchemy/api.py
index 3966b0e3a..dd3740a1f 100644
--- a/heat/db/sqlalchemy/api.py
+++ b/heat/db/sqlalchemy/api.py
@@ -571,7 +571,7 @@ def stack_get_by_name(context, stack_name):
              models.Stack.tenant == context.tenant_id,
              models.Stack.stack_user_project_id == context.tenant_id)
              ).filter_by(name=stack_name)
-    return query.first()
+    return query.order_by(models.Stack.created_at).first()
 
 
 def stack_get(context, stack_id, show_deleted=False, eager_load=True):
@@ -757,7 +757,17 @@ def stack_count_all(context, filters=None,
 def stack_create(context, values):
     stack_ref = models.Stack()
     stack_ref.update(values)
+    stack_name = stack_ref.name
     stack_ref.save(context.session)
+
+    # Even though we just created a stack with this name, we may not find
+    # it again because some unit tests create stacks with deleted_at set. Also
+    # some backup stacks may not be found, for reasons that are unclear.
+    earliest = stack_get_by_name(context, stack_name)
+    if earliest is not None and earliest.id != stack_ref.id:
+        context.session.query(models.Stack).filter_by(id=stack_ref.id).delete()
+        raise exception.StackExists(stack_name=stack_name)
+
     return stack_ref
 
 
diff --git a/heat/engine/service.py b/heat/engine/service.py
index d761577b5..62af50a95 100644
--- a/heat/engine/service.py
+++ b/heat/engine/service.py
@@ -674,6 +674,9 @@ class EngineService(service.ServiceBase):
             raise exception.MissingCredentialError(required='X-Auth-Key')
 
     def _validate_new_stack(self, cnxt, stack_name, parsed_template):
+        # We'll check that the stack name is unique in the tenant while
+        # storing it in the database to avoid races, but also check it here
+        # before validating so we can fail early.
         if stack_object.Stack.get_by_name(cnxt, stack_name):
             raise exception.StackExists(stack_name=stack_name)
 
diff --git a/heat/tests/db/test_sqlalchemy_api.py b/heat/tests/db/test_sqlalchemy_api.py
index 856f54c38..a8356178f 100644
--- a/heat/tests/db/test_sqlalchemy_api.py
+++ b/heat/tests/db/test_sqlalchemy_api.py
@@ -357,6 +357,21 @@ class SqlAlchemyTest(common.HeatTestCase):
         st = db_api.stack_get_by_name(self.ctx, name)
         self.assertIsNone(st)
 
+    def test_stack_create_multiple(self):
+        name = 'stack_race'
+        stack = self._setup_test_stack(name, UUID1,
+                                       stack_user_project_id=UUID2)[1]
+        self.assertRaises(exception.StackExists,
+                          self._setup_test_stack,
+                          name, UUID2, stack_user_project_id=UUID2)
+
+        st = db_api.stack_get_by_name(self.ctx, name)
+        self.assertEqual(UUID1, st.id)
+
+        stack.delete()
+
+        self.assertIsNone(db_api.stack_get_by_name(self.ctx, name))
+
     def test_nested_stack_get_by_name(self):
         stack1 = self._setup_test_stack('neststack1', UUID1)[1]
         stack2 = self._setup_test_stack('neststack2', UUID2,