Decouple EC2 API from using instance id

Continuation of 0dc3269

Some code in ec2 api use Instance.id rather than InstanceIdMapping.id
directly while converting ec2 id to an instance object. This is very
dangerous if Instance.id is not equal to InstanceIdMapping.id for the
same instance uuid.

To avoid the id mapping confusion, this patch:

 * uses instance_id_mappings.id instead of instances.id
 * uses instance_uuid in ec2utils.id_to_ec2_inst_id
 * removes ec2utils.ec2_instance_id_to_uuid

Fixes bug 1145490

Change-Id: I22638f667c18eefe542b03e31f1a3aa2ce782db7
(cherry picked from commit 79cc2a2)

nova/api/ec2/cloud.py

@@ -694,8 +694,8 @@ def get_console_output(self, context, instance_id, **kwargs):
         else:
             ec2_id = instance_id
         validate_ec2_id(ec2_id)
-        instance_id = ec2utils.ec2_id_to_id(ec2_id)
-        instance = self.compute_api.get(context, instance_id)
+        instance_uuid = ec2utils.ec2_inst_id_to_uuid(context, ec2_id)
+        instance = self.compute_api.get(context, instance_uuid)
         output = self.compute_api.get_console_output(context, instance)
         now = timeutils.utcnow()
         return {"InstanceId": ec2_id,
@@ -805,8 +805,8 @@ def attach_volume(self, context,
         validate_ec2_id(instance_id)
         validate_ec2_id(volume_id)
         volume_id = ec2utils.ec2_vol_id_to_uuid(volume_id)
-        instance_id = ec2utils.ec2_id_to_id(instance_id)
-        instance = self.compute_api.get(context, instance_id)
+        instance_uuid = ec2utils.ec2_inst_id_to_uuid(context, instance_id)
+        instance = self.compute_api.get(context, instance_uuid)
         msg = _("Attach volume %(volume_id)s to instance %(instance_id)s"
                 " at %(device)s") % locals()
         LOG.audit(msg, context=context)
@@ -820,7 +820,7 @@ def attach_volume(self, context,
         volume = self.volume_api.get(context, volume_id)
         return {'attachTime': volume['attach_time'],
                 'device': volume['mountpoint'],
-                'instanceId': ec2utils.id_to_ec2_inst_id(instance_id),
+                'instanceId': ec2utils.id_to_ec2_inst_id(instance_uuid),
                 'requestId': context.request_id,
                 'status': volume['attach_status'],
                 'volumeId': ec2utils.id_to_ec2_vol_id(volume_id)}
@@ -919,11 +919,10 @@ def _format_attr_user_data(instance, result):
             raise exception.EC2APIError(
                 _('attribute not supported: %s') % attribute)
 
-        ec2_instance_id = instance_id
         validate_ec2_id(instance_id)
-        instance_id = ec2utils.ec2_id_to_id(ec2_instance_id)
-        instance = self.compute_api.get(context, instance_id)
-        result = {'instance_id': ec2_instance_id}
+        instance_uuid = ec2utils.ec2_inst_id_to_uuid(context, instance_id)
+        instance = self.compute_api.get(context, instance_uuid)
+        result = {'instance_id': instance_id}
         fn(instance, result)
         return result
 
@@ -956,8 +955,8 @@ def _format_terminate_instances(self, context, instance_id,
             i['previousState'] = _state_description(previous_state['vm_state'],
                                         previous_state['shutdown_terminate'])
             try:
-                internal_id = ec2utils.ec2_id_to_id(ec2_id)
-                instance = self.compute_api.get(context, internal_id)
+                instance_uuid = ec2utils.ec2_inst_id_to_uuid(context, ec2_id)
+                instance = self.compute_api.get(context, instance_uuid)
                 i['currentState'] = _state_description(instance['vm_state'],
                                             instance['shutdown_terminate'])
             except exception.NotFound:
@@ -1152,8 +1151,8 @@ def release_address(self, context, public_ip, **kwargs):
     def associate_address(self, context, instance_id, public_ip, **kwargs):
         LOG.audit(_("Associate address %(public_ip)s to"
                 " instance %(instance_id)s") % locals(), context=context)
-        instance_id = ec2utils.ec2_id_to_id(instance_id)
-        instance = self.compute_api.get(context, instance_id)
+        instance_uuid = ec2utils.ec2_inst_id_to_uuid(context, instance_id)
+        instance = self.compute_api.get(context, instance_uuid)
 
         cached_ipinfo = ec2utils.get_ip_info_for_instance(context, instance)
         fixed_ips = cached_ipinfo['fixed_ips'] + cached_ipinfo['fixed_ip6s']
@@ -1247,8 +1246,8 @@ def _ec2_ids_to_instances(self, context, instance_id):
         instances = []
         for ec2_id in instance_id:
             validate_ec2_id(ec2_id)
-            _instance_id = ec2utils.ec2_id_to_id(ec2_id)
-            instance = self.compute_api.get(context, _instance_id)
+            instance_uuid = ec2utils.ec2_inst_id_to_uuid(context, ec2_id)
+            instance = self.compute_api.get(context, instance_uuid)
             instances.append(instance)
         return instances
 
@@ -1511,8 +1510,8 @@ def create_image(self, context, instance_id, **kwargs):
         name = kwargs.get('name')
         validate_ec2_id(instance_id)
         ec2_instance_id = instance_id
-        instance_id = ec2utils.ec2_id_to_id(ec2_instance_id)
-        instance = self.compute_api.get(context, instance_id)
+        instance_uuid = ec2utils.ec2_inst_id_to_uuid(context, ec2_instance_id)
+        instance = self.compute_api.get(context, instance_uuid)
 
         bdms = self.compute_api.get_instance_bdms(context, instance)
 
@@ -1542,7 +1541,7 @@ def create_image(self, context, instance_id, **kwargs):
             start_time = time.time()
             while vm_state != vm_states.STOPPED:
                 time.sleep(1)
-                instance = self.compute_api.get(context, instance_id)
+                instance = self.compute_api.get(context, instance_uuid)
                 vm_state = instance['vm_state']
                 # NOTE(yamahata): timeout and error. 1 hour for now for safety.
                 #                 Is it too short/long?

nova/api/ec2/ec2utils.py

@@ -222,10 +222,6 @@ def get_snapshot_uuid_from_int_id(context, int_id):
     return db.get_snapshot_uuid_by_ec2_id(context, int_id)
 
 
-def ec2_instance_id_to_uuid(context, ec2_id):
-    int_id = ec2_id_to_id(ec2_id)
-    return db.instance_get(context, int_id)['uuid']
-
 _c2u = re.compile('(((?<=[a-z])[A-Z])|([A-Z](?![A-Z]|$)))')
 
 

nova/api/metadata/base.py

@@ -109,7 +109,7 @@ def __init__(self, instance, address=None, content=[], extra_md=None):
         self.ec2_ids = {}
 
         self.ec2_ids['instance-id'] = ec2utils.id_to_ec2_inst_id(
-                instance['id'])
+            instance['uuid'])
         self.ec2_ids['ami-id'] = ec2utils.glance_id_to_ec2_id(ctxt,
             instance['image_ref'])
 

nova/tests/api/ec2/test_cinder_cloud.py

@@ -684,8 +684,8 @@ def test_stop_start_with_volume(self):
                                             'delete_on_termination': True},
                                            ]}
         ec2_instance_id = self._run_instance(**kwargs)
-        instance_uuid = ec2utils.ec2_instance_id_to_uuid(self.context,
-                                                         ec2_instance_id)
+        instance_uuid = ec2utils.ec2_inst_id_to_uuid(self.context,
+                                                     ec2_instance_id)
         vols = self.volume_api.get_all(self.context)
         vols = [v for v in vols if v['instance_uuid'] == instance_uuid]
 
@@ -762,9 +762,8 @@ def test_stop_with_attached_volume(self):
                                             'volume_id': vol1_uuid,
                                             'delete_on_termination': True}]}
         ec2_instance_id = self._run_instance(**kwargs)
-        instance_id = ec2utils.ec2_id_to_id(ec2_instance_id)
-        instance_uuid = ec2utils.ec2_instance_id_to_uuid(self.context,
-                                                         ec2_instance_id)
+        instance_uuid = ec2utils.ec2_inst_id_to_uuid(self.context,
+                                                     ec2_instance_id)
 
         vols = self.volume_api.get_all(self.context)
         vols = [v for v in vols if v['instance_uuid'] == instance_uuid]
@@ -775,7 +774,7 @@ def test_stop_with_attached_volume(self):
         vol = self.volume_api.get(self.context, vol2_uuid)
         self._assert_volume_detached(vol)
 
-        instance = db.instance_get(self.context, instance_id)
+        instance = db.instance_get_by_uuid(self.context, instance_uuid)
         self.cloud.compute_api.attach_volume(self.context,
                                              instance,
                                              volume_id=vol2_uuid,
@@ -847,8 +846,8 @@ def test_run_with_snapshot(self):
                                             'snapshot_id': snap2_uuid,
                                             'delete_on_termination': True}]}
         ec2_instance_id = self._run_instance(**kwargs)
-        instance_uuid = ec2utils.ec2_instance_id_to_uuid(self.context,
-                                                         ec2_instance_id)
+        instance_uuid = ec2utils.ec2_inst_id_to_uuid(self.context,
+                                                     ec2_instance_id)
 
         vols = self.volume_api.get_all(self.context)
         vols = [v for v in vols if v['instance_uuid'] == instance_uuid]

nova/tests/api/ec2/test_cloud.py

@@ -270,7 +270,7 @@ def test_associate_disassociate_address(self):
                                                  project_id=project_id)
 
         fixed_ips = nw_info.fixed_ips()
-        ec2_id = ec2utils.id_to_ec2_inst_id(inst['id'])
+        ec2_id = ec2utils.id_to_ec2_inst_id(inst['uuid'])
 
         self.stubs.Set(ec2utils, 'get_ip_info_for_instance',
                        lambda *args: {'fixed_ips': ['10.0.0.1'],
@@ -2128,8 +2128,8 @@ def test_stop_start_with_volume(self):
                                             'delete_on_termination': True},
                                            ]}
         ec2_instance_id = self._run_instance(**kwargs)
-        instance_uuid = ec2utils.ec2_instance_id_to_uuid(self.context,
-                                                         ec2_instance_id)
+        instance_uuid = ec2utils.ec2_inst_id_to_uuid(self.context,
+                                                     ec2_instance_id)
         instance_id = ec2utils.ec2_id_to_id(ec2_instance_id)
 
         vols = db.volume_get_all_by_instance_uuid(self.context, instance_uuid)
@@ -2192,8 +2192,8 @@ def test_stop_with_attached_volume(self):
                                             'delete_on_termination': True}]}
         ec2_instance_id = self._run_instance(**kwargs)
         instance_id = ec2utils.ec2_id_to_id(ec2_instance_id)
-        instance_uuid = ec2utils.ec2_instance_id_to_uuid(self.context,
-                                                         ec2_instance_id)
+        instance_uuid = ec2utils.ec2_inst_id_to_uuid(self.context,
+                                                     ec2_instance_id)
 
         vols = db.volume_get_all_by_instance_uuid(self.context, instance_uuid)
         self.assertEqual(len(vols), 1)
@@ -2269,8 +2269,8 @@ def test_run_with_snapshot(self):
                                             'delete_on_termination': True}]}
         ec2_instance_id = self._run_instance(**kwargs)
         instance_id = ec2utils.ec2_vol_id_to_uuid(ec2_instance_id)
-        instance_uuid = ec2utils.ec2_instance_id_to_uuid(self.context,
-                                                         ec2_instance_id)
+        instance_uuid = ec2utils.ec2_inst_id_to_uuid(self.context,
+                                                     ec2_instance_id)
 
         vols = db.volume_get_all_by_instance_uuid(self.context, instance_uuid)
         self.assertEqual(len(vols), 2)
@@ -2525,6 +2525,13 @@ def fake_volume_get(ctxt, volume_id, session=None):
             raise exception.VolumeNotFound(volume_id=volume_id)
         self.stubs.Set(db, 'volume_get', fake_volume_get)
 
+        def fake_get_instance_uuid_by_ec2_id(ctxt, int_id):
+            if int_id == 305419896:
+                return 'e5fe5518-0288-4fa3-b0c4-c79764101b85'
+            raise exception.InstanceNotFound(instance_id=int_id)
+        self.stubs.Set(db, 'get_instance_uuid_by_ec2_id',
+                       fake_get_instance_uuid_by_ec2_id)
+
         get_attribute = functools.partial(
             self.cloud.describe_instance_attribute,
             self.context, 'i-12345678')