Merge lp:~soren/nova/metadata-full-instance-objects into lp:~hudson-openstack/nova/trunk

[Soren Hansen]

Replace instance ref from compute.api.get_all with one from instance_get. This should ensure it gets fully populated with all the relevant attributes.

This is a pretty crude, but quite unintrusive patch to fix the linked bug.

We really should make up our minds as to whether we're going to let SQLAlchemy handle lazy attribute lookups or if we should start doing all these attribute lookups explicitly and manually. All these joinedloads that need to be applied in different places is turning in to quite a maintenance nightmare.

[Thierry Carrez (ttx)]

Fixes the issue for me, and looks good and tidy.

review: Approve

[Vish Ishaya (vishvananda)]

Agreed on all points in the description. LGTM

review: Approve

[Dan Prince (dan-prince)]

Hey Soren,

The existing patch will certainly fix this.

The root cause of this regression may be changes I made in revision 955 (bzr diff -c955). Did I miss a model update that is causing the EC2 metadata service not to get a fully populated instance dict?

I'm looking into this a bit more now...

review: Needs Information

[Dan Prince (dan-prince)]

Not trying to hold this up. I do think 955 may have been the breaking change here but this fix works fine and is safe for now.

review: Approve

[Vish Ishaya (vishvananda)]

Yes, I susupect there is a missing instance_types join in one/more of the queries.
On Apr 11, 2011, at 9:18 AM, Dan Prince wrote:

> Review: Approve
> Not trying to hold this up. I do think 955 may have been the breaking change here but this fix works fine and is safe for now.
> --
> https://code.launchpad.net/~soren/nova/metadata-full-instance-objects/+merge/57177
> You are reviewing the proposed merge of lp:~soren/nova/metadata-full-instance-objects into lp:nova.

[Soren Hansen (soren)]

The core issue is that we have waaay too many different ways to get instance objects out of the database, each of them requiring a non-trivial amount of options set on the query to make sure all the related objects are appropriately referenced. At this point in the release cycle, any amount of refactoring should be avoided. This is a simple, safe and minimal patch to address the specific issue at hand.

[Jay Pipes (jaypipes)]

lgtm, and agreed about the general points Soren has raised.

review: Approve

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~soren/nova/metadata-full-instance-objects into lp:nova failed. Below is the output from the failed tests.

AccountsTest
    test_account_create OK
    test_account_delete OK
    test_account_update OK
    test_get_account OK
AdminAPITest
    test_admin_disabled OK
    test_admin_enabled OK
APITest
    test_exceptions_are_converted_to_faults OK
Test
    test_authorize_token OK
    test_authorize_user OK
    test_bad_token OK
    test_bad_user_bad_key OK
    test_bad_user_good_key OK
    test_no_user OK
    test_token_expiry OK
TestFunctional
    test_token_doesnotexist OK
    test_token_expiry OK
TestLimiter
    test_authorize_token OK
LimiterTest
    test_limiter_custom_max_limit OK
    test_limiter_limit_and_offset OK
    test_limiter_limit_medium OK
    test_limiter_limit_over_max OK
    test_limiter_limit_zero OK
    test_limiter_negative_limit OK
    test_limiter_negative_offset OK
    test_limiter_nothing OK
    test_limiter_offset_bad OK
    test_limiter_offset_blank OK
    test_limiter_offset_medium OK
    test_limiter_offset_over_max OK
    test_limiter_offset_zero OK
ActionExtensionTest
    test_extended_action OK
    test_invalid_action OK
    test_invalid_action_body OK
ExtensionControllerTest
    test_get_by_alias OK
    test_index OK
ExtensionManagerTest
    test_get_resources OK
ResourceExtensionTest
    test_get_resources OK
    test_get_resources_with_controller OK
    test_no_extension_present OK
ResponseExtensionTest
    test_get_resources_with_mgr OK
    test_get_resources_with_stub_mgr OK
TestFaults
    test_400_fault_json OK
    test_400_fault_xml ...

[lp:~soren/nova/metadata-full-instance-objects]

975. By Soren Hansen on 2011-04-12

pep8

=== modified file 'nova/api/ec2/cloud.py'
--- nova/api/ec2/cloud.py	2011-04-08 03:39:29 +0000
+++ nova/api/ec2/cloud.py	2011-04-12 07:42:38 +0000
@@ -142,6 +142,11 @@
         instance_ref = self.compute_api.get_all(ctxt, fixed_ip=address)
         if instance_ref is None:
             return None
+
+        # This ensures that all attributes of the instance
+        # are populated.
+        instance_ref = db.instance_get(ctxt, instance_ref['id'])
+
         mpi = self._get_mpi_data(ctxt, instance_ref['project_id'])
         if instance_ref['key_name']:
             keys = {'0': {'_name': instance_ref['key_name'],