Merge lp:~termie/nova/ec2_ids into lp:~hudson-openstack/nova/trunk

[termie]

Fixes related to how EC2 ids are displayed and dealt with.

Additionally adds two flags that define a template string that is used for the internal naming of things (like the volume name of a logical volume on disk), default being similar to the EC2 format, so that the ids are easy to match while testing when you may need to manually delete or check something.

[Todd Willey (xtoddx)]

lgtm

review: Approve

[Jay Pipes (jaypipes)]

I likee.

review: Approve

[Eric Day (eday)]

lgtm

review: Approve

=== modified file 'nova/api/ec2/cloud.py'
--- nova/api/ec2/cloud.py	2011-01-12 11:34:16 +0000
+++ nova/api/ec2/cloud.py	2011-01-13 00:59:40 +0000
@@ -73,17 +73,13 @@
 
 
 def ec2_id_to_id(ec2_id):
-    """Convert an ec2 ID (i-[base 36 number]) to an instance id (int)"""
-    return int(ec2_id[2:], 36)
-
-
-def id_to_ec2_id(instance_id):
-    """Convert an instance ID (int) to an ec2 ID (i-[base 36 number])"""
-    digits = []
-    while instance_id != 0:
-        instance_id, remainder = divmod(instance_id, 36)
-        digits.append('0123456789abcdefghijklmnopqrstuvwxyz'[remainder])
-    return "i-%s" % ''.join(reversed(digits))
+    """Convert an ec2 ID (i-[base 16 number]) to an instance id (int)"""
+    return int(ec2_id.split('-')[-1], 16)
+
+
+def id_to_ec2_id(instance_id, template='i-%08x'):
+    """Convert an instance ID (int) to an ec2 ID (i-[base 16 number])"""
+    return template % instance_id
 
 
 class CloudController(object):
@@ -541,6 +537,8 @@
         return self.compute_api.get_ajax_console(context, internal_id)
 
     def describe_volumes(self, context, volume_id=None, **kwargs):
+        if volume_id:
+            volume_id = [ec2_id_to_id(x) for x in volume_id]
         volumes = self.volume_api.get_all(context)
         # NOTE(vish): volume_id is an optional list of volume ids to filter by.
         volumes = [self._format_volume(context, v) for v in volumes
@@ -556,7 +554,7 @@
             instance_data = '%s[%s]' % (instance_ec2_id,
                                         volume['instance']['host'])
         v = {}
-        v['volumeId'] = volume['id']
+        v['volumeId'] = id_to_ec2_id(volume['id'], 'vol-%08x')
         v['status'] = volume['status']
         v['size'] = volume['size']
         v['availabilityZone'] = volume['availability_zone']
@@ -574,7 +572,8 @@
                                    'device': volume['mountpoint'],
                                    'instanceId': instance_ec2_id,
                                    'status': 'attached',
-                                   'volume_id': volume['ec2_id']}]
+                                   'volumeId': id_to_ec2_id(volume['id'],
+                                                            'vol-%08x')}]
         else:
             v['attachmentSet'] = [{}]
 
@@ -593,10 +592,12 @@
         return {'volumeSet': [self._format_volume(context, dict(volume_ref))]}
 
     def delete_volume(self, context, volume_id, **kwargs):
+        volume_id = ec2_id_to_id(volume_id)
         self.volume_api.delete(context, volume_id)
         return True
 
     def update_volume(self, context, volume_id, **kwargs):
+        volume_id = ec2_id_to_id(volume_id)
         updatable_fields = ['display_name', 'display_description']
         changes = {}
         for field in updatable_fields:
@@ -607,18 +608,21 @@
         return True
 
     def attach_volume(self, context, volume_id, instance_id, device, **kwargs):
+        volume_id = ec2_id_to_id(volume_id)
+        instance_id = ec2_id_to_id(instance_id)
         LOG.audit(_("Attach volume %s to instacne %s at %s"), volume_id,
                   instance_id, device, context=context)
         self.compute_api.attach_volume(context, instance_id, volume_id, device)
         volume = self.volume_api.get(context, volume_id)
         return {'attachTime': volume['attach_time'],
                 'device': volume['mountpoint'],
-                'instanceId': instance_id,
+                'instanceId': id_to_ec2_id(instance_id),
                 'requestId': context.request_id,
                 'status': volume['attach_status'],
-                'volumeId': volume_id}
+                'volumeId': id_to_ec2_id(volume_id, 'vol-%08x')}
 
     def detach_volume(self, context, volume_id, **kwargs):
+        volume_id = ec2_id_to_id(volume_id)
         LOG.audit(_("Detach volume %s"), volume_id, context=context)
         volume = self.volume_api.get(context, volume_id)
         instance = self.compute_api.detach_volume(context, volume_id)
@@ -627,7 +631,7 @@
                 'instanceId': id_to_ec2_id(instance['id']),
                 'requestId': context.request_id,
                 'status': volume['attach_status'],
-                'volumeId': volume_id}
+                'volumeId': id_to_ec2_id(volume_id, 'vol-%08x')}
 
     def _convert_to_set(self, lst, label):
         if lst == None or lst == []:

=== modified file 'nova/db/api.py'
--- nova/db/api.py	2011-01-12 11:34:16 +0000
+++ nova/db/api.py	2011-01-13 00:59:40 +0000
@@ -42,6 +42,10 @@
                     'The backend to use for db')
 flags.DEFINE_boolean('enable_new_services', True,
                      'Services to be added to the available pool on create')
+flags.DEFINE_string('instance_name_template', 'instance-%08x',
+                    'Template string to be used to generate instance names')
+flags.DEFINE_string('volume_name_template', 'volume-%08x',
+                    'Template string to be used to generate instance names')
 
 
 IMPL = utils.LazyPluggable(FLAGS['db_backend'],

=== modified file 'nova/db/sqlalchemy/models.py'
--- nova/db/sqlalchemy/models.py	2011-01-12 11:34:16 +0000
+++ nova/db/sqlalchemy/models.py	2011-01-13 00:59:40 +0000
@@ -169,7 +169,7 @@
 
     @property
     def name(self):
-        return "instance-%08x" % self.id
+        return FLAGS.instance_name_template % self.id
 
     admin_pass = Column(String(255))
     user_id = Column(String(255))
@@ -256,7 +256,7 @@
 
     @property
     def name(self):
-        return "volume-%08x" % self.id
+        return FLAGS.volume_name_template % self.id
 
     user_id = Column(String(255))
     project_id = Column(String(255))

=== modified file 'nova/tests/test_cloud.py'
--- nova/tests/test_cloud.py	2011-01-12 11:34:16 +0000
+++ nova/tests/test_cloud.py	2011-01-13 00:59:40 +0000
@@ -126,10 +126,13 @@
         vol2 = db.volume_create(self.context, {})
         result = self.cloud.describe_volumes(self.context)
         self.assertEqual(len(result['volumeSet']), 2)
+        volume_id = cloud.id_to_ec2_id(vol2['id'], 'vol-%08x')
         result = self.cloud.describe_volumes(self.context,
-                                             volume_id=[vol2['id']])
+                                             volume_id=[volume_id])
         self.assertEqual(len(result['volumeSet']), 1)
-        self.assertEqual(result['volumeSet'][0]['volumeId'], vol2['id'])
+        self.assertEqual(
+                cloud.ec2_id_to_id(result['volumeSet'][0]['volumeId']),
+                vol2['id'])
         db.volume_destroy(self.context, vol1['id'])
         db.volume_destroy(self.context, vol2['id'])
 
@@ -385,7 +388,8 @@
 
     def test_update_of_volume_display_fields(self):
         vol = db.volume_create(self.context, {})
-        self.cloud.update_volume(self.context, vol['id'],
+        self.cloud.update_volume(self.context,
+                                 cloud.id_to_ec2_id(vol['id'], 'vol-%08x'),
                                  display_name='c00l v0lum3')
         vol = db.volume_get(self.context, vol['id'])
         self.assertEqual('c00l v0lum3', vol['display_name'])
@@ -393,8 +397,9 @@
 
     def test_update_of_volume_wont_update_private_fields(self):
         vol = db.volume_create(self.context, {})
-        self.cloud.update_volume(self.context, vol['id'],
-                                   mountpoint='/not/here')
+        self.cloud.update_volume(self.context,
+                                 cloud.id_to_ec2_id(vol['id'], 'vol-%08x'),
+                                 mountpoint='/not/here')
         vol = db.volume_get(self.context, vol['id'])
         self.assertEqual(None, vol['mountpoint'])
         db.volume_destroy(self.context, vol['id'])