Merge lp:~anso/nova/lp676266 into lp:~hudson-openstack/nova/trunk

Proposed by Vish Ishaya
Status: Merged
Approved by: Vish Ishaya
Approved revision: 402
Merged at revision: 401
Proposed branch: lp:~anso/nova/lp676266
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 85 lines (+35/-4)
3 files modified
nova/api/ec2/cloud.py (+3/-3)
nova/tests/cloud_unittest.py (+31/-0)
nova/tests/network_unittest.py (+1/-1)
To merge this branch: bzr merge lp:~anso/nova/lp676266
Reviewer Review Type Date Requested Status
Todd Willey (community) Approve
Jay Pipes (community) Approve
Devin Carlen (community) Approve
Review via email: mp+41021@code.launchpad.net

Description of the change

ec2_api commands for describe_addresses and associate_address are broken in trunk. This happened during the switch to ec2_id and internal_id. We clearly didn't have any unit tests for this, so I've added a couple in addition to the three line change to actually fix the bugs.

To post a comment you must log in.
Revision history for this message
Devin Carlen (devcamcar) wrote :

lgtm

review: Approve
Revision history for this message
Jay Pipes (jaypipes) wrote :

yup, looks like a reasonable fix.

review: Approve
Revision history for this message
Todd Willey (xtoddx) wrote :

lgtm

review: Approve
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :
Download full text (14.5 KiB)

The attempt to merge lp:~anso/nova/lp676266 into lp:nova failed. Below is the output from the failed tests.

nova.tests.access_unittest
  AccessTestCase
    test_001_allow_all ... [OK]
    test_002_allow_none ... [OK]
    test_003_allow_project_manager ... [OK]
    test_004_allow_sys_and_net ... [OK]
nova.tests.api_unittest
  ApiEc2TestCase
    test_authorize_revoke_security_group_cidr ... [OK]
    test_authorize_revoke_security_group_foreign_group ... [OK]
    test_create_delete_security_group ... [OK]
    test_describe_instances ... [OK]
    test_get_all_key_pairs ... [OK]
    test_get_all_security_groups ... [OK]
  XmlConversionTestCase
    test_number_conversion ... [OK]
nova.tests.auth_unittest
  AuthManagerDbTestCase
    test_004_signature_is_valid ... [OK]
    test_005_can_get_credentials ... [OK]
    test_add_user_role_doesnt_infect_project_roles ... [OK]
    test_adding_role_to_project_is_ignored_unless_added_to_user ... [OK]
    test_can_add_and_remove_user_role ... [OK]
    test_can_add_remove_user_with_role ... [OK]
    test_can_add_user_to_project ... [OK]
    test_can_create_and_get_project ... [OK]
    test_can_create_and_get_project_with_attributes ... [OK]
    test_can_create_project_with_manager ... [OK]
    test_can_delete_project ... [OK]
    test_can_delete_user ... [OK]
    test_can_generate_x509 ... [OK]
    test_can_list_project_roles ... [OK]
    test_can_list_projects ... [OK]
    test_can_list_user_roles ... [OK]
    test_can_list_users ... [OK]
    test_can_modify_project ... [OK]
    test_can_modify_users ... [OK]
    test_can_remove_project_role_but_keep_user_role ... [OK]
    test_can_remove_user_from_project ... [OK]
    test_can_remove_user_roles ... [OK]
    test_can_retrieve_project_by_user ... [OK]
    test_create_and_find_user ... [OK]
    test_create_and_find_with_properties ... [OK]
    test_create_project_assigns_manager_to_members .....

Revision history for this message
Vish Ishaya (vishvananda) wrote :

Hmm, I didn't get this error locally. I suspect that it is an intermittent failure based on row ordering in the db. I've cleaned up the leaking floating_ip in network tests. I also noticed that the network tests were trying to use redis driver when run by themselves so that was removed as well.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :
Download full text (13.5 KiB)

The attempt to merge lp:~anso/nova/lp676266 into lp:nova failed. Below is the output from the failed tests.

nova.tests.access_unittest
  AccessTestCase
    test_001_allow_all ... [OK]
    test_002_allow_none ... [OK]
    test_003_allow_project_manager ... [OK]
    test_004_allow_sys_and_net ... [OK]
nova.tests.api_unittest
  ApiEc2TestCase
    test_authorize_revoke_security_group_cidr ... [OK]
    test_authorize_revoke_security_group_foreign_group ... [OK]
    test_create_delete_security_group ... [OK]
    test_describe_instances ... [OK]
    test_get_all_key_pairs ... [OK]
    test_get_all_security_groups ... [OK]
  XmlConversionTestCase
    test_number_conversion ... [OK]
nova.tests.auth_unittest
  AuthManagerDbTestCase
    test_004_signature_is_valid ... [OK]
    test_005_can_get_credentials ... [OK]
    test_add_user_role_doesnt_infect_project_roles ... [OK]
    test_adding_role_to_project_is_ignored_unless_added_to_user ... [OK]
    test_can_add_and_remove_user_role ... [OK]
    test_can_add_remove_user_with_role ... [OK]
    test_can_add_user_to_project ... [OK]
    test_can_create_and_get_project ... [OK]
    test_can_create_and_get_project_with_attributes ... [OK]
    test_can_create_project_with_manager ... [OK]
    test_can_delete_project ... [OK]
    test_can_delete_user ... [OK]
    test_can_generate_x509 ... [OK]
    test_can_list_project_roles ... [OK]
    test_can_list_projects ... [OK]
    test_can_list_user_roles ... [OK]
    test_can_list_users ... [OK]
    test_can_modify_project ... [OK]
    test_can_modify_users ... [OK]
    test_can_remove_project_role_but_keep_user_role ... [OK]
    test_can_remove_user_from_project ... [OK]
    test_can_remove_user_roles ... [OK]
    test_can_retrieve_project_by_user ... [OK]
    test_create_and_find_user ... [OK]
    test_create_and_find_with_properties ... [OK]
    test_create_project_assigns_manager_to_members .....

lp:~anso/nova/lp676266 updated
402. By Vish Ishaya

delete floating ips after tests

Revision history for this message
Vish Ishaya (vishvananda) wrote :

++ for jays patch on cleaning db between tests. Dirty DB causing intermittent failures.
It works locally, so it is a bit hard to debug. I think this one is it though.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'nova/api/ec2/cloud.py'
2--- nova/api/ec2/cloud.py 2010-10-31 00:42:40 +0000
3+++ nova/api/ec2/cloud.py 2010-11-17 02:43:07 +0000
4@@ -679,7 +679,7 @@
5 context.project_id)
6 for floating_ip_ref in iterator:
7 address = floating_ip_ref['address']
8- instance_id = None
9+ ec2_id = None
10 if (floating_ip_ref['fixed_ip']
11 and floating_ip_ref['fixed_ip']['instance']):
12 internal_id = floating_ip_ref['fixed_ip']['instance']['ec2_id']
13@@ -717,8 +717,8 @@
14 "args": {"floating_address": floating_ip_ref['address']}})
15 return {'releaseResponse': ["Address released."]}
16
17- def associate_address(self, context, ec2_id, public_ip, **kwargs):
18- internal_id = ec2_id_to_internal_id(ec2_id)
19+ def associate_address(self, context, instance_id, public_ip, **kwargs):
20+ internal_id = ec2_id_to_internal_id(instance_id)
21 instance_ref = db.instance_get_by_internal_id(context, internal_id)
22 fixed_address = db.instance_get_fixed_address(context,
23 instance_ref['id'])
24
25=== modified file 'nova/tests/cloud_unittest.py'
26--- nova/tests/cloud_unittest.py 2010-10-22 07:48:27 +0000
27+++ nova/tests/cloud_unittest.py 2010-11-17 02:43:07 +0000
28@@ -91,6 +91,37 @@
29 # NOTE(vish): create depends on pool, so just call helper directly
30 return cloud._gen_key(self.context, self.context.user.id, name)
31
32+ def test_describe_addresses(self):
33+ """Makes sure describe addresses runs without raising an exception"""
34+ address = "10.10.10.10"
35+ db.floating_ip_create(self.context,
36+ {'address': address,
37+ 'host': FLAGS.host})
38+ self.cloud.allocate_address(self.context)
39+ self.cloud.describe_addresses(self.context)
40+ self.cloud.release_address(self.context,
41+ public_ip=address)
42+ db.floating_ip_destroy(self.context, address)
43+
44+ def test_associate_disassociate_address(self):
45+ """Verifies associate runs cleanly without raising an exception"""
46+ address = "10.10.10.10"
47+ db.floating_ip_create(self.context,
48+ {'address': address,
49+ 'host': FLAGS.host})
50+ self.cloud.allocate_address(self.context)
51+ inst = db.instance_create(self.context, {})
52+ ec2_id = cloud.internal_id_to_ec2_id(inst['internal_id'])
53+ self.cloud.associate_address(self.context,
54+ instance_id=ec2_id,
55+ public_ip=address)
56+ self.cloud.disassociate_address(self.context,
57+ public_ip=address)
58+ self.cloud.release_address(self.context,
59+ public_ip=address)
60+ db.instance_destroy(self.context, inst['id'])
61+ db.floating_ip_destroy(self.context, address)
62+
63 def test_console_output(self):
64 image_id = FLAGS.default_image
65 instance_type = FLAGS.default_instance_type
66
67=== modified file 'nova/tests/network_unittest.py'
68--- nova/tests/network_unittest.py 2010-10-22 07:48:27 +0000
69+++ nova/tests/network_unittest.py 2010-11-17 02:43:07 +0000
70@@ -41,7 +41,6 @@
71 # flags in the corresponding section in nova-dhcpbridge
72 self.flags(connection_type='fake',
73 fake_network=True,
74- auth_driver='nova.auth.ldapdriver.FakeLdapDriver',
75 network_size=16,
76 num_networks=5)
77 logging.getLogger().setLevel(logging.DEBUG)
78@@ -127,6 +126,7 @@
79 self.network.deallocate_floating_ip(self.context, float_addr)
80 self.network.deallocate_fixed_ip(self.context, fix_addr)
81 release_ip(fix_addr)
82+ db.floating_ip_destroy(context.get_admin_context(), float_addr)
83
84 def test_allocate_deallocate_fixed_ip(self):
85 """Makes sure that we can allocate and deallocate a fixed ip"""