Properly create and delete Aggregates

* If deleted aggregate with same name exists, ignore it
* When delete aggregate, delete metadata as well
* Removes aggregates.name unique constraint

Fix bug 1052479

Change-Id: I430e69367bdedbf65049a5142d137ab788763ae3
(cherry picked from commit 852dd4d5cf)

nova/db/sqlalchemy/api.py

@@ -4856,16 +4856,11 @@ def aggregate_create(context, values, metadata=None):
                                      models.Aggregate.name,
                                      values['name'],
                                      session=session,
-                                     read_deleted='yes').first()
+                                     read_deleted='no').first()
     if not aggregate:
         aggregate = models.Aggregate()
         aggregate.update(values)
         aggregate.save(session=session)
-    elif aggregate.deleted:
-        values['deleted'] = False
-        values['deleted_at'] = None
-        aggregate.update(values)
-        aggregate.save(session=session)
     else:
         raise exception.AggregateNameExists(aggregate_name=values['name'])
     if metadata:
@@ -4950,6 +4945,14 @@ def aggregate_delete(context, aggregate_id):
     else:
         raise exception.AggregateNotFound(aggregate_id=aggregate_id)
 
+    #Delete Metadata
+    rows = model_query(context,
+                       models.AggregateMetadata).\
+                       filter_by(aggregate_id=aggregate_id).\
+                       update({'deleted': True,
+                      'deleted_at': timeutils.utcnow(),
+                      'updated_at': literal_column('updated_at')})
+
 
 @require_admin_context
 def aggregate_get_all(context):

nova/db/sqlalchemy/migrate_repo/versions/133_aggregate_delete_fix.py

@@ -0,0 +1,48 @@
+# Copyright 2012 OpenStack LLC.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from sqlalchemy import String, Column, MetaData, Table
+from migrate.changeset import UniqueConstraint
+
+from nova.openstack.common import log as logging
+
+LOG = logging.getLogger(__name__)
+
+
+def upgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+    dialect = migrate_engine.url.get_dialect().name
+
+    aggregates = Table('aggregates', meta, autoload=True)
+    if dialect.startswith('sqlite'):
+        aggregates.c.name.alter(unique=False)
+    elif dialect.startswith('postgres'):
+        ucon = UniqueConstraint('name',
+                                name='aggregates_name_key',
+                                table=aggregates)
+        ucon.drop()
+
+    else:
+        col2 = aggregates.c.name
+        UniqueConstraint(col2, name='name').drop()
+
+
+def downgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+    aggregates = Table('aggregates', meta, autoload=True)
+    aggregates.c.name.alter(unique=True)

nova/db/sqlalchemy/models.py

@@ -879,7 +879,7 @@ class Aggregate(BASE, NovaBase):
     """Represents a cluster of hosts that exists in this zone."""
     __tablename__ = 'aggregates'
     id = Column(Integer, primary_key=True, autoincrement=True)
-    name = Column(String(255), unique=True)
+    name = Column(String(255))
     availability_zone = Column(String(255), nullable=False)
     _hosts = relationship(AggregateHost,
                           lazy="joined",

nova/tests/test_db_api.py

@@ -605,6 +605,17 @@ class AggregateDBApiTestCase(test.TestCase):
         expected_metadata = db.aggregate_metadata_get(ctxt, result['id'])
         self.assertDictMatch(expected_metadata, _get_fake_aggr_metadata())
 
+    def test_aggregate_create_delete_create_with_metadata(self):
+        """Ensure aggregate metadata is deleted bug 1052479."""
+        ctxt = context.get_admin_context()
+        result = _create_aggregate(context=ctxt)
+        expected_metadata = db.aggregate_metadata_get(ctxt, result['id'])
+        self.assertDictMatch(expected_metadata, _get_fake_aggr_metadata())
+        db.aggregate_delete(ctxt, result['id'])
+        result = _create_aggregate(metadata=None)
+        expected_metadata = db.aggregate_metadata_get(ctxt, result['id'])
+        self.assertEqual(expected_metadata, {})
+
     def test_aggregate_create_low_privi_context(self):
         """Ensure right context is applied when creating aggregate."""
         self.assertRaises(exception.AdminRequired,

nova/tests/test_migrations.py

@@ -480,3 +480,26 @@ class TestMigrations(test.TestCase):
             migration_api.downgrade(engine, TestMigrations.REPOSITORY, 111)
             agg = sqlalchemy.select([aggregate_hosts.c.host]).execute().first()
             self.assertEqual(host, agg.host)
+
+    def test_migration_133(self):
+        for key, engine in self.engines.items():
+            migration_api.version_control(engine, TestMigrations.REPOSITORY,
+                                          migration.INIT_VERSION)
+            migration_api.upgrade(engine, TestMigrations.REPOSITORY, 132)
+
+            # Set up a single volume, values don't matter
+            metadata = sqlalchemy.schema.MetaData()
+            metadata.bind = engine
+            aggregates = sqlalchemy.Table('aggregates', metadata,
+                    autoload=True)
+            name = 'name'
+            aggregates.insert().values(id=1, availability_zone='nova',
+                    aggregate_name=1, name=name).execute()
+
+            migration_api.upgrade(engine, TestMigrations.REPOSITORY, 133)
+            aggregates.insert().values(id=2, availability_zone='nova',
+                    aggregate_name=2, name=name).execute()
+
+            migration_api.downgrade(engine, TestMigrations.REPOSITORY, 132)
+            agg = sqlalchemy.select([aggregates.c.name]).execute().first()
+            self.assertEqual(name, agg.name)