Converted the instance table to use a uuid instead of a auto_increment ID and a random internal_id. I had to use a String(32) column with hex and not a String(16) with bytes because SQLAlchemy doesn't like non-unicode strings going in for String types. We could try another type, but I didn't want a primary_key on blob types.

8 files changed, 78 insertions, 98 deletions

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index 05f8c3d0b..a831b2a62 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -72,18 +72,14 @@ def _gen_key(context, user_id, key_name):
     return {'private_key': private_key, 'fingerprint': fingerprint}
 
 
-def ec2_id_to_internal_id(ec2_id):
-    """Convert an ec2 ID (i-[base 36 number]) to an internal id (int)"""
-    return int(ec2_id[2:], 36)
+def ec2_id_to_id(ec2_id):
+    """Convert an ec2 ID and an instance ID."""
+    return ec2_id[2:]
 
 
-def internal_id_to_ec2_id(internal_id):
-    """Convert an internal ID (int) to an ec2 ID (i-[base 36 number])"""
-    digits = []
-    while internal_id != 0:
-        internal_id, remainder = divmod(internal_id, 36)
-        digits.append('0123456789abcdefghijklmnopqrstuvwxyz'[remainder])
-    return "i-%s" % ''.join(reversed(digits))
+def id_to_ec2_id(instance_id):
+    """Convert an instance ID to an ec2 ID."""
+    return "i-%s" % instance_id
 
 
 class CloudController(object):
@@ -153,7 +149,7 @@ class CloudController(object):
         hostname = instance_ref['hostname']
         floating_ip = db.instance_get_floating_address(ctxt,
                                                        instance_ref['id'])
-        ec2_id = internal_id_to_ec2_id(instance_ref['internal_id'])
+        ec2_id = id_to_ec2_id(instance_ref['id'])
         data = {
             'user-data': base64.b64decode(instance_ref['user_data']),
             'meta-data': {
@@ -437,8 +433,8 @@ class CloudController(object):
     def get_console_output(self, context, instance_id, **kwargs):
         # instance_id is passed in as a list of instances
         ec2_id = instance_id[0]
-        internal_id = ec2_id_to_internal_id(ec2_id)
-        instance_ref = self.compute_api.get_instance(context, internal_id)
+        instance_id = ec2_id_to_id(ec2_id)
+        instance_ref = self.compute_api.get_instance(context, instance_id)
         output = rpc.call(context,
                           '%s.%s' % (FLAGS.compute_topic,
                                      instance_ref['host']),
@@ -464,8 +460,8 @@ class CloudController(object):
         instance_ec2_id = None
         instance_data = None
         if volume.get('instance', None):
-            internal_id = volume['instance']['internal_id']
-            instance_ec2_id = internal_id_to_ec2_id(internal_id)
+            instance_id = volume['instance']['id']
+            instance_ec2_id = id_to_ec2_id(instance_id)
             instance_data = '%s[%s]' % (instance_ec2_id,
                                         volume['instance']['host'])
         v = {}
@@ -534,8 +530,8 @@ class CloudController(object):
             raise exception.ApiError("Volume status must be available")
         if volume_ref['attach_status'] == "attached":
             raise exception.ApiError("Volume is already attached")
-        internal_id = ec2_id_to_internal_id(instance_id)
-        instance_ref = self.compute_api.get_instance(context, internal_id)
+        instance_id = ec2_id_to_id(instance_id)
+        instance_ref = self.compute_api.get_instance(context, instance_id)
         host = instance_ref['host']
         rpc.cast(context,
                  db.queue_get_for(context, FLAGS.compute_topic, host),
@@ -570,11 +566,11 @@ class CloudController(object):
             # If the instance doesn't exist anymore,
             # then we need to call detach blind
             db.volume_detached(context)
-        internal_id = instance_ref['internal_id']
-        ec2_id = internal_id_to_ec2_id(internal_id)
+        instance_id = instance_ref['id']
+        ec2_id = id_to_ec2_id(instance_id)
         return {'attachTime': volume_ref['attach_time'],
                 'device': volume_ref['mountpoint'],
-                'instanceId': internal_id,
+                'instanceId': instance_id,
                 'requestId': context.request_id,
                 'status': volume_ref['attach_status'],
                 'volumeId': volume_ref['id']}
@@ -619,8 +615,8 @@ class CloudController(object):
                 if instance['image_id'] == FLAGS.vpn_image_id:
                     continue
             i = {}
-            internal_id = instance['internal_id']
-            ec2_id = internal_id_to_ec2_id(internal_id)
+            instance_id = instance['id']
+            ec2_id = id_to_ec2_id(instance_id)
             i['instanceId'] = ec2_id
             i['imageId'] = instance['image_id']
             i['instanceState'] = {
@@ -673,8 +669,8 @@ class CloudController(object):
             ec2_id = None
             if (floating_ip_ref['fixed_ip']
                 and floating_ip_ref['fixed_ip']['instance']):
-                internal_id = floating_ip_ref['fixed_ip']['instance']['ec2_id']
-                ec2_id = internal_id_to_ec2_id(internal_id)
+                instance_id = floating_ip_ref['fixed_ip']['instance']['ec2_id']
+                ec2_id = id_to_ec2_id(instance_id)
             address_rv = {'public_ip': address,
                           'instance_id': ec2_id}
             if context.user.is_admin():
@@ -709,8 +705,8 @@ class CloudController(object):
         return {'releaseResponse': ["Address released."]}
 
     def associate_address(self, context, instance_id, public_ip, **kwargs):
-        internal_id = ec2_id_to_internal_id(instance_id)
-        instance_ref = self.compute_api.get_instance(context, internal_id)
+        instance_id = ec2_id_to_id(instance_id)
+        instance_ref = self.compute_api.get_instance(context, instance_id)
         fixed_address = db.instance_get_fixed_address(context,
                                                       instance_ref['id'])
         floating_ip_ref = db.floating_ip_get_by_address(context, public_ip)
@@ -755,7 +751,7 @@ class CloudController(object):
             description=kwargs.get('display_description'),
             key_name=kwargs.get('key_name'),
             security_group=kwargs.get('security_group'),
-            generate_hostname=internal_id_to_ec2_id)
+            generate_hostname=id_to_ec2_id)
         return self._format_run_instances(context,
                                           instances[0]['reservation_id'])
 
@@ -764,27 +760,27 @@ class CloudController(object):
         instance_id is a kwarg so its name cannot be modified."""
         logging.debug("Going to start terminating instances")
         for ec2_id in instance_id:
-            internal_id = ec2_id_to_internal_id(ec2_id)
-            self.compute_api.delete_instance(context, internal_id)
+            instance_id = ec2_id_to_id(ec2_id)
+            self.compute_api.delete_instance(context, instance_id)
         return True
 
     def reboot_instances(self, context, instance_id, **kwargs):
         """instance_id is a list of instance ids"""
         for ec2_id in instance_id:
-            internal_id = ec2_id_to_internal_id(ec2_id)
-            self.compute_api.reboot(context, internal_id)
+            instance_id = ec2_id_to_id(ec2_id)
+            self.compute_api.reboot(context, instance_id)
         return True
 
     def rescue_instance(self, context, instance_id, **kwargs):
         """This is an extension to the normal ec2_api"""
-        internal_id = ec2_id_to_internal_id(instance_id)
-        self.compute_api.rescue(context, internal_id)
+        instance_id = ec2_id_to_id(instance_id)
+        self.compute_api.rescue(context, instance_id)
         return True
 
     def unrescue_instance(self, context, instance_id, **kwargs):
         """This is an extension to the normal ec2_api"""
-        internal_id = ec2_id_to_internal_id(instance_id)
-        self.compute_api.unrescue(context, internal_id)
+        instance_id = ec2_id_to_id(instance_id)
+        self.compute_api.unrescue(context, instance_id)
         return True
 
     def update_instance(self, context, ec2_id, **kwargs):
@@ -794,8 +790,8 @@ class CloudController(object):
             if field in kwargs:
                 changes[field] = kwargs[field]
         if changes:
-            internal_id = ec2_id_to_internal_id(ec2_id)
-            inst = self.compute_api.get_instance(context, internal_id)
+            instance_id = ec2_id_to_id(ec2_id)
+            inst = self.compute_api.get_instance(context, instance_id)
             db.instance_update(context, inst['id'], kwargs)
         return True
 
diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py
index 6f2f6fed9..f8f40b764 100644
--- a/nova/api/openstack/servers.py
+++ b/nova/api/openstack/servers.py
@@ -46,7 +46,7 @@ def _entity_detail(inst):
     inst_dict = {}
 
     mapped_keys = dict(status='state', imageId='image_id',
-        flavorId='instance_type', name='display_name', id='internal_id')
+        flavorId='instance_type', name='display_name', id='id')
 
     for k, v in mapped_keys.iteritems():
         inst_dict[k] = inst[v]
@@ -61,7 +61,7 @@ def _entity_detail(inst):
 
 def _entity_inst(inst):
     """ Filters all model attributes save for id and name """
-    return dict(server=dict(id=inst['internal_id'], name=inst['display_name']))
+    return dict(server=dict(id=inst['id'], name=inst['display_name']))
 
 
 class Controller(wsgi.Controller):
diff --git a/nova/compute/api.py b/nova/compute/api.py
index 8e0efa4cc..f310c575f 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -36,9 +36,9 @@ from nova.db import base
 FLAGS = flags.FLAGS
 
 
-def generate_default_hostname(internal_id):
+def generate_default_hostname(instance_id):
     """Default function to generate a hostname given an instance reference."""
-    return str(internal_id)
+    return str(instance_id)
 
 
 class ComputeAPI(base.Base):
@@ -127,7 +127,6 @@ class ComputeAPI(base.Base):
                             **base_options)
             instance = self.db.instance_create(context, instance)
             instance_id = instance['id']
-            internal_id = instance['internal_id']
 
             elevated = context.elevated()
             if not security_groups:
@@ -138,9 +137,9 @@ class ComputeAPI(base.Base):
                                                     security_group_id)
 
             # Set sane defaults if not specified
-            updates = dict(hostname=generate_hostname(internal_id))
+            updates = dict(hostname=generate_hostname(instance_id))
             if 'display_name' not in instance:
-                updates['display_name'] = "Server %s" % internal_id
+                updates['display_name'] = "Server %s" % instance_id
 
             instance = self.update_instance(context, instance_id, **updates)
             instances.append(instance)
@@ -199,17 +198,16 @@ class ComputeAPI(base.Base):
         return self.db.instance_update(context, instance_id, kwargs)
 
     def delete_instance(self, context, instance_id):
-        logging.debug("Going to try and terminate %d" % instance_id)
+        logging.debug("Going to try and terminate %s" % instance_id)
         try:
-            instance = self.db.instance_get_by_internal_id(context,
-                                                           instance_id)
+            instance = self.db.instance_get_by_id(context, instance_id)
         except exception.NotFound as e:
-            logging.warning("Instance %d was not found during terminate",
+            logging.warning("Instance %s was not found during terminate",
                             instance_id)
             raise e
 
         if (instance['state_description'] == 'terminating'):
-            logging.warning("Instance %d is already being terminated",
+            logging.warning("Instance %s is already being terminated",
                             instance_id)
             return
 
@@ -264,11 +262,11 @@ class ComputeAPI(base.Base):
         return self.db.instance_get_all(context)
 
     def get_instance(self, context, instance_id):
-        return self.db.instance_get_by_internal_id(context, instance_id)
+        return self.db.instance_get_by_id(context, instance_id)
 
     def reboot(self, context, instance_id):
         """Reboot the given instance."""
-        instance = self.db.instance_get_by_internal_id(context, instance_id)
+        instance = self.db.instance_get_by_id(context, instance_id)
         host = instance['host']
         rpc.cast(context,
                  self.db.queue_get_for(context, FLAGS.compute_topic, host),
@@ -277,7 +275,7 @@ class ComputeAPI(base.Base):
 
     def rescue(self, context, instance_id):
         """Rescue the given instance."""
-        instance = self.db.instance_get_by_internal_id(context, instance_id)
+        instance = self.db.instance_get_by_id(context, instance_id)
         host = instance['host']
         rpc.cast(context,
                  self.db.queue_get_for(context, FLAGS.compute_topic, host),
@@ -286,7 +284,7 @@ class ComputeAPI(base.Base):
 
     def unrescue(self, context, instance_id):
         """Unrescue the given instance."""
-        instance = self.db.instance_get_by_internal_id(context, instance_id)
+        instance = self.db.instance_get_by_id(context, instance_id)
         host = instance['host']
         rpc.cast(context,
                  self.db.queue_get_for(context, FLAGS.compute_topic, host),
diff --git a/nova/db/api.py b/nova/db/api.py
index 8f9dc2443..e607ac938 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -304,9 +304,9 @@ def instance_get_floating_address(context, instance_id):
     return IMPL.instance_get_floating_address(context, instance_id)
 
 
-def instance_get_by_internal_id(context, internal_id):
-    """Get an instance by internal id."""
-    return IMPL.instance_get_by_internal_id(context, internal_id)
+def instance_get_by_id(context, instance_id):
+    """Get an instance by id."""
+    return IMPL.instance_get_by_id(context, instance_id)
 
 
 def instance_is_vpn(context, instance_id):
@@ -658,7 +658,7 @@ def security_group_get_all(context):
 
 
 def security_group_get(context, security_group_id):
-    """Get security group by its internal id."""
+    """Get security group by its id."""
     return IMPL.security_group_get(context, security_group_id)
 
 
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index 55036d1d1..0505b77a6 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -19,7 +19,7 @@
 Implementation of SQLAlchemy backend.
 """
 
-import random
+import uuid
 import warnings
 
 from nova import db
@@ -525,28 +525,18 @@ def fixed_ip_update(context, address, values):
 ###################
 
 
-#TODO(gundlach): instance_create and volume_create are nearly identical
-#and should be refactored.  I expect there are other copy-and-paste
-#functions between the two of them as well.
 @require_context
 def instance_create(context, values):
     """Create a new Instance record in the database.
 
     context - request context object
     values - dict containing column values.
-             'internal_id' is auto-generated and should not be specified.
     """
     instance_ref = models.Instance()
     instance_ref.update(values)
 
     session = get_session()
     with session.begin():
-        while instance_ref.internal_id == None:
-            # Instances have integer internal ids.
-            internal_id = random.randint(0, 2 ** 31 - 1)
-            if not instance_internal_id_exists(context, internal_id,
-                                               session=session):
-                instance_ref.internal_id = internal_id
         instance_ref.save(session=session)
     return instance_ref
 
@@ -652,38 +642,32 @@ def instance_get_all_by_reservation(context, reservation_id):
 
 
 @require_context
-def instance_get_by_internal_id(context, internal_id):
+def instance_get_by_id(context, instance_id):
     session = get_session()
 
+    if type(instance_id) is int:
+        instance_id = uuid.UUID(int=instance_id).hex
+
     if is_admin_context(context):
         result = session.query(models.Instance).\
                          options(joinedload('security_groups')).\
-                         filter_by(internal_id=internal_id).\
+                         filter_by(id=instance_id).\
                          filter_by(deleted=can_read_deleted(context)).\
                          first()
     elif is_user_context(context):
         result = session.query(models.Instance).\
                          options(joinedload('security_groups')).\
                          filter_by(project_id=context.project_id).\
-                         filter_by(internal_id=internal_id).\
+                         filter_by(id=instance_id).\
                          filter_by(deleted=False).\
                          first()
     if not result:
-        raise exception.NotFound('Instance %s not found' % (internal_id))
+        raise exception.NotFound('Instance %s not found' % (instance_id))
 
     return result
 
 
 @require_context
-def instance_internal_id_exists(context, internal_id, session=None):
-    if not session:
-        session = get_session()
-    return session.query(exists().\
-                         where(models.Instance.internal_id == internal_id)).\
-                   one()[0]
-
-
-@require_context
 def instance_get_fixed_address(context, instance_id):
     session = get_session()
     with session.begin():
diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index fe0a9a921..a0b6fff89 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -20,6 +20,7 @@ SQLAlchemy models for nova data.
 """
 
 import datetime
+import uuid
 
 from sqlalchemy.orm import relationship, backref, object_mapper
 from sqlalchemy import Column, Integer, String, schema
@@ -39,6 +40,10 @@ FLAGS = flags.FLAGS
 BASE = declarative_base()
 
 
+def make_uuid():
+    return uuid.uuid1().hex
+
+
 class NovaBase(object):
     """Base class for Nova Models."""
     __table_args__ = {'mysql_engine': 'InnoDB'}
@@ -154,11 +159,13 @@ class Service(BASE, NovaBase):
 class Instance(BASE, NovaBase):
     """Represents a guest vm."""
     __tablename__ = 'instances'
-    id = Column(Integer, primary_key=True)
-    internal_id = Column(Integer, unique=True)
+    id = Column(String(32), primary_key=True, default=make_uuid)
 
-    admin_pass = Column(String(255))
+    @property
+    def name(self):
+        return "instance-%s" % self.id
 
+    admin_pass = Column(String(255))
     user_id = Column(String(255))
     project_id = Column(String(255))
 
@@ -170,10 +177,6 @@ class Instance(BASE, NovaBase):
     def project(self):
         return auth.manager.AuthManager().get_project(self.project_id)
 
-    @property
-    def name(self):
-        return "instance-%d" % self.internal_id
-
     image_id = Column(String(255))
     kernel_id = Column(String(255))
     ramdisk_id = Column(String(255))
@@ -238,7 +241,7 @@ class Volume(BASE, NovaBase):
     host = Column(String(255))  # , ForeignKey('hosts.id'))
     size = Column(Integer)
     availability_zone = Column(String(255))  # TODO(vish): foreign key?
-    instance_id = Column(Integer, ForeignKey('instances.id'), nullable=True)
+    instance_id = Column(String(32), ForeignKey('instances.id'), nullable=True)
     instance = relationship(Instance,
                             backref=backref('volumes'),
                             foreign_keys=instance_id,
@@ -311,7 +314,7 @@ class SecurityGroupInstanceAssociation(BASE, NovaBase):
     __tablename__ = 'security_group_instance_association'
     id = Column(Integer, primary_key=True)
     security_group_id = Column(Integer, ForeignKey('security_groups.id'))
-    instance_id = Column(Integer, ForeignKey('instances.id'))
+    instance_id = Column(String(32), ForeignKey('instances.id'))
 
 
 class SecurityGroup(BASE, NovaBase):
@@ -431,7 +434,7 @@ class FixedIp(BASE, NovaBase):
     address = Column(String(255))
     network_id = Column(Integer, ForeignKey('networks.id'), nullable=True)
     network = relationship(Network, backref=backref('fixed_ips'))
-    instance_id = Column(Integer, ForeignKey('instances.id'), nullable=True)
+    instance_id = Column(String(32), ForeignKey('instances.id'), nullable=True)
     instance = relationship(Instance,
                             backref=backref('fixed_ip', uselist=False),
                             foreign_keys=instance_id,
diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py
index 8444b6fce..d2b0e0fc8 100644
--- a/nova/tests/api/openstack/test_servers.py
+++ b/nova/tests/api/openstack/test_servers.py
@@ -56,8 +56,8 @@ def instance_address(context, instance_id):
 
 
 def stub_instance(id, user_id=1):
-    return Instance(id=id + 123456, state=0, image_id=10, user_id=user_id,
-                    display_name='server%s' % id, internal_id=id)
+    return Instance(id=id, state=0, image_id=10, user_id=user_id,
+                    display_name='server%s' % id)
 
 
 class ServersTest(unittest.TestCase):
@@ -71,8 +71,7 @@ class ServersTest(unittest.TestCase):
         fakes.stub_out_key_pair_funcs(self.stubs)
         fakes.stub_out_image_service(self.stubs)
         self.stubs.Set(nova.db.api, 'instance_get_all', return_servers)
-        self.stubs.Set(nova.db.api, 'instance_get_by_internal_id',
-                       return_server)
+        self.stubs.Set(nova.db.api, 'instance_get_by_id', return_server)
         self.stubs.Set(nova.db.api, 'instance_get_all_by_user',
                        return_servers)
         self.stubs.Set(nova.db.api, 'instance_add_security_group',
@@ -107,7 +106,7 @@ class ServersTest(unittest.TestCase):
 
     def test_create_instance(self):
         def instance_create(context, inst):
-            return {'id': 1, 'internal_id': 1, 'display_name': ''}
+            return {'id': '1', 'display_name': ''}
 
         def server_update(context, id, params):
             return instance_create(context, id)
diff --git a/nova/tests/cloud_unittest.py b/nova/tests/cloud_unittest.py
index 9886a2449..f63eed65a 100644
--- a/nova/tests/cloud_unittest.py
+++ b/nova/tests/cloud_unittest.py
@@ -113,7 +113,7 @@ class CloudTestCase(test.TrialTestCase):
         self.cloud.allocate_address(self.context)
         inst = db.instance_create(self.context, {})
         fixed = self.network.allocate_fixed_ip(self.context, inst['id'])
-        ec2_id = cloud.internal_id_to_ec2_id(inst['internal_id'])
+        ec2_id = cloud.id_to_ec2_id(inst['id'])
         self.cloud.associate_address(self.context,
                                      instance_id=ec2_id,
                                      public_ip=address)
@@ -289,7 +289,7 @@ class CloudTestCase(test.TrialTestCase):
 
     def test_update_of_instance_display_fields(self):
         inst = db.instance_create(self.context, {})
-        ec2_id = cloud.internal_id_to_ec2_id(inst['internal_id'])
+        ec2_id = cloud.id_to_ec2_id(inst['id'])
         self.cloud.update_instance(self.context, ec2_id,
                                    display_name='c00l 1m4g3')
         inst = db.instance_get(self.context, inst['id'])