OpenStack Compute (nova)

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

  1. lp676266
  2. Merge into 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
=== modified file 'nova/api/ec2/cloud.py'
--- nova/api/ec2/cloud.py 2010-10-31 00:42:40 +0000
+++ nova/api/ec2/cloud.py 2010-11-17 02:43:07 +0000
@@ -679,7 +679,7 @@
679 context.project_id)679 context.project_id)
680 for floating_ip_ref in iterator:680 for floating_ip_ref in iterator:
681 address = floating_ip_ref['address']681 address = floating_ip_ref['address']
682 instance_id = None682 ec2_id = None
683 if (floating_ip_ref['fixed_ip']683 if (floating_ip_ref['fixed_ip']
684 and floating_ip_ref['fixed_ip']['instance']):684 and floating_ip_ref['fixed_ip']['instance']):
685 internal_id = floating_ip_ref['fixed_ip']['instance']['ec2_id']685 internal_id = floating_ip_ref['fixed_ip']['instance']['ec2_id']
@@ -717,8 +717,8 @@
717 "args": {"floating_address": floating_ip_ref['address']}})717 "args": {"floating_address": floating_ip_ref['address']}})
718 return {'releaseResponse': ["Address released."]}718 return {'releaseResponse': ["Address released."]}
719719
720 def associate_address(self, context, ec2_id, public_ip, **kwargs):720 def associate_address(self, context, instance_id, public_ip, **kwargs):
721 internal_id = ec2_id_to_internal_id(ec2_id)721 internal_id = ec2_id_to_internal_id(instance_id)
722 instance_ref = db.instance_get_by_internal_id(context, internal_id)722 instance_ref = db.instance_get_by_internal_id(context, internal_id)
723 fixed_address = db.instance_get_fixed_address(context,723 fixed_address = db.instance_get_fixed_address(context,
724 instance_ref['id'])724 instance_ref['id'])
725725
=== modified file 'nova/tests/cloud_unittest.py'
--- nova/tests/cloud_unittest.py 2010-10-22 07:48:27 +0000
+++ nova/tests/cloud_unittest.py 2010-11-17 02:43:07 +0000
@@ -91,6 +91,37 @@
91 # NOTE(vish): create depends on pool, so just call helper directly91 # NOTE(vish): create depends on pool, so just call helper directly
92 return cloud._gen_key(self.context, self.context.user.id, name)92 return cloud._gen_key(self.context, self.context.user.id, name)
9393
94 def test_describe_addresses(self):
95 """Makes sure describe addresses runs without raising an exception"""
96 address = "10.10.10.10"
97 db.floating_ip_create(self.context,
98 {'address': address,
99 'host': FLAGS.host})
100 self.cloud.allocate_address(self.context)
101 self.cloud.describe_addresses(self.context)
102 self.cloud.release_address(self.context,
103 public_ip=address)
104 db.floating_ip_destroy(self.context, address)
105
106 def test_associate_disassociate_address(self):
107 """Verifies associate runs cleanly without raising an exception"""
108 address = "10.10.10.10"
109 db.floating_ip_create(self.context,
110 {'address': address,
111 'host': FLAGS.host})
112 self.cloud.allocate_address(self.context)
113 inst = db.instance_create(self.context, {})
114 ec2_id = cloud.internal_id_to_ec2_id(inst['internal_id'])
115 self.cloud.associate_address(self.context,
116 instance_id=ec2_id,
117 public_ip=address)
118 self.cloud.disassociate_address(self.context,
119 public_ip=address)
120 self.cloud.release_address(self.context,
121 public_ip=address)
122 db.instance_destroy(self.context, inst['id'])
123 db.floating_ip_destroy(self.context, address)
124
94 def test_console_output(self):125 def test_console_output(self):
95 image_id = FLAGS.default_image126 image_id = FLAGS.default_image
96 instance_type = FLAGS.default_instance_type127 instance_type = FLAGS.default_instance_type
97128
=== modified file 'nova/tests/network_unittest.py'
--- nova/tests/network_unittest.py 2010-10-22 07:48:27 +0000
+++ nova/tests/network_unittest.py 2010-11-17 02:43:07 +0000
@@ -41,7 +41,6 @@
41 # flags in the corresponding section in nova-dhcpbridge41 # flags in the corresponding section in nova-dhcpbridge
42 self.flags(connection_type='fake',42 self.flags(connection_type='fake',
43 fake_network=True,43 fake_network=True,
44 auth_driver='nova.auth.ldapdriver.FakeLdapDriver',
45 network_size=16,44 network_size=16,
46 num_networks=5)45 num_networks=5)
47 logging.getLogger().setLevel(logging.DEBUG)46 logging.getLogger().setLevel(logging.DEBUG)
@@ -127,6 +126,7 @@
127 self.network.deallocate_floating_ip(self.context, float_addr)126 self.network.deallocate_floating_ip(self.context, float_addr)
128 self.network.deallocate_fixed_ip(self.context, fix_addr)127 self.network.deallocate_fixed_ip(self.context, fix_addr)
129 release_ip(fix_addr)128 release_ip(fix_addr)
129 db.floating_ip_destroy(context.get_admin_context(), float_addr)
130130
131 def test_allocate_deallocate_fixed_ip(self):131 def test_allocate_deallocate_fixed_ip(self):
132 """Makes sure that we can allocate and deallocate a fixed ip"""132 """Makes sure that we can allocate and deallocate a fixed ip"""