OpenStack Compute (nova)

Merge lp:~cbehrens/nova/lp855115 into lp:~hudson-openstack/nova/trunk

  1. lp855115
  2. Merge into trunk
Proposed by Chris Behrens
Status: Merged
Approved by: Vish Ishaya
Approved revision: 1546
Merged at revision: 1612
Proposed branch: lp:~cbehrens/nova/lp855115
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 175 lines (+68/-12)
5 files modified
nova/api/ec2/__init__.py (+4/-0)
nova/api/openstack/contrib/floating_ips.py (+6/-1)
nova/db/sqlalchemy/api.py (+11/-10)
nova/tests/api/openstack/contrib/test_floating_ips.py (+1/-1)
nova/tests/test_network.py (+46/-0)
To merge this branch: bzr merge lp:~cbehrens/nova/lp855115
Reviewer Review Type Date Requested Status
Vish Ishaya (community) Approve
Paul Voccio (community) Approve
Kevin L. Mitchell (community) Approve
Review via email: mp+76475@code.launchpad.net

Description of the change

Fixes lp:855115 -- issue with disassociating floating ips.

To post a comment you must log in.
Revision history for this message
Kevin L. Mitchell (klmitch) wrote :

Looks OK to me...

review: Approve
Revision history for this message
Paul Voccio (pvo) wrote :

lgtm

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

awesome, lgtm

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

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

CloudTestCase
    test_ajax_console OK 1.75
    test_allocate_address OK 0.26
    test_associate_disassociate_address OK 1.60
    test_authorize_revoke_security_group_ingress_by_id OK 0.33
    test_authorize_security_group_fail_missing_source_group OK 0.25
    test_authorize_security_group_ingress OK 0.26
    test_authorize_security_group_ingress_already_exists OK 0.32
    test_authorize_security_group_ingress_ip_permissions_groups OK 0.32
    test_authorize_security_group_ingress_ip_permissions_ip_rangesOK 0.27
    test_authorize_security_group_ingress_missing_group_name_or_idOK 0.18
    test_authorize_security_group_ingress_missing_protocol_paramsOK 0.39
    test_console_output OK 1.23
    test_create_delete_security_group OK 0.26
    test_create_image OK 4.05
    test_create_snapshot OK 0.44
    test_create_volume_from_snapshot OK 0.46
    test_delete_key_pair OK 0.39
    test_delete_security_group_by_id OK 0.23
    test_delete_security_group_no_params OK 0.20
    test_delete_security_group_with_bad_group_id OK 0.22
    test_delete_security_group_with_bad_name OK 0.22
    test_delete_snapshot OK 0.43
    test_deregister_image OK 0.19
    test_deregister_image_wrong_container_type OK 0.21
    test_describe_addresses OK 0.49
    test_describe_availability_zones OK 0.23
    test_describe_image_attribute OK 0.18
    test_describe_image_attribute_block_device_mapping OK 0.19
    test_describe_image_attribute_root_device_name OK 0.18
    test_describe_image_mapping OK 0.19
    test_describe_images OK 0.19
    test_describe_instance_attribute OK 0.18
    test_describe_instances OK 0.35
    test_describe_instances_bdm OK 1.39
    test_describe_instances_deleted OK 0.22
    test_describe_key_pairs OK 0.57
    test_describe_regions OK 0.19
    test_describe_security_group_ingress_groups OK 0.43
    test_describe_security_groups OK 0.27
    test_describe_security_groups_by_id OK 0.32
    test_describe_snapshots OK 0.23
    test_describe_volumes OK 0.25
  ...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'nova/api/ec2/__init__.py'
--- nova/api/ec2/__init__.py 2011-09-20 22:52:03 +0000
+++ nova/api/ec2/__init__.py 2011-09-21 20:25:31 +0000
@@ -386,6 +386,10 @@
386 LOG.debug(_('InvalidPortRange raised: %s'), unicode(ex),386 LOG.debug(_('InvalidPortRange raised: %s'), unicode(ex),
387 context=context)387 context=context)
388 return self._error(req, context, type(ex).__name__, unicode(ex))388 return self._error(req, context, type(ex).__name__, unicode(ex))
389 except exception.NotAuthorized as ex:
390 LOG.info(_('NotAuthorized raised: %s'), unicode(ex),
391 context=context)
392 return self._error(req, context, type(ex).__name__, unicode(ex))
389 except Exception as ex:393 except Exception as ex:
390 extra = {'environment': req.environ}394 extra = {'environment': req.environ}
391 LOG.exception(_('Unexpected error raised: %s'), unicode(ex),395 LOG.exception(_('Unexpected error raised: %s'), unicode(ex),
392396
=== modified file 'nova/api/openstack/contrib/floating_ips.py'
--- nova/api/openstack/contrib/floating_ips.py 2011-09-08 20:18:08 +0000
+++ nova/api/openstack/contrib/floating_ips.py 2011-09-21 20:25:31 +0000
@@ -144,6 +144,8 @@
144 address)144 address)
145 except exception.ApiError, e:145 except exception.ApiError, e:
146 raise webob.exc.HTTPBadRequest(explanation=e.message)146 raise webob.exc.HTTPBadRequest(explanation=e.message)
147 except exception.NotAuthorized, e:
148 raise webob.exc.HTTPUnauthorized()
147149
148 return webob.Response(status_int=202)150 return webob.Response(status_int=202)
149151
@@ -162,7 +164,10 @@
162164
163 floating_ip = self.network_api.get_floating_ip_by_ip(context, address)165 floating_ip = self.network_api.get_floating_ip_by_ip(context, address)
164 if floating_ip.get('fixed_ip'):166 if floating_ip.get('fixed_ip'):
165 self.network_api.disassociate_floating_ip(context, address)167 try:
168 self.network_api.disassociate_floating_ip(context, address)
169 except exception.NotAuthorized, e:
170 raise webob.exc.HTTPUnauthorized()
166171
167 return webob.Response(status_int=202)172 return webob.Response(status_int=202)
168173
169174
=== modified file 'nova/db/sqlalchemy/api.py'
--- nova/db/sqlalchemy/api.py 2011-09-19 22:32:45 +0000
+++ nova/db/sqlalchemy/api.py 2011-09-21 20:25:31 +0000
@@ -534,7 +534,6 @@
534 fixed_address, host):534 fixed_address, host):
535 session = get_session()535 session = get_session()
536 with session.begin():536 with session.begin():
537 # TODO(devcamcar): How to ensure floating_id belongs to user?
538 floating_ip_ref = floating_ip_get_by_address(context,537 floating_ip_ref = floating_ip_get_by_address(context,
539 floating_address,538 floating_address,
540 session=session)539 session=session)
@@ -550,7 +549,6 @@
550def floating_ip_deallocate(context, address):549def floating_ip_deallocate(context, address):
551 session = get_session()550 session = get_session()
552 with session.begin():551 with session.begin():
553 # TODO(devcamcar): How to ensure floating id belongs to user?
554 floating_ip_ref = floating_ip_get_by_address(context,552 floating_ip_ref = floating_ip_get_by_address(context,
555 address,553 address,
556 session=session)554 session=session)
@@ -564,7 +562,6 @@
564def floating_ip_destroy(context, address):562def floating_ip_destroy(context, address):
565 session = get_session()563 session = get_session()
566 with session.begin():564 with session.begin():
567 # TODO(devcamcar): Ensure address belongs to user.
568 floating_ip_ref = floating_ip_get_by_address(context,565 floating_ip_ref = floating_ip_get_by_address(context,
569 address,566 address,
570 session=session)567 session=session)
@@ -575,8 +572,6 @@
575def floating_ip_disassociate(context, address):572def floating_ip_disassociate(context, address):
576 session = get_session()573 session = get_session()
577 with session.begin():574 with session.begin():
578 # TODO(devcamcar): Ensure address belongs to user.
579 # Does get_floating_ip_by_address handle this?
580 floating_ip_ref = floating_ip_get_by_address(context,575 floating_ip_ref = floating_ip_get_by_address(context,
581 address,576 address,
582 session=session)577 session=session)
@@ -645,17 +640,23 @@
645640
646@require_context641@require_context
647def floating_ip_get_by_address(context, address, session=None):642def floating_ip_get_by_address(context, address, session=None):
648 # TODO(devcamcar): Ensure the address belongs to user.
649 if not session:643 if not session:
650 session = get_session()644 session = get_session()
651645
652 result = session.query(models.FloatingIp).\646 result = session.query(models.FloatingIp).\
653 options(joinedload_all('fixed_ip.network')).\647 options(joinedload_all('fixed_ip.network')).\
654 filter_by(address=address).\648 filter_by(address=address).\
655 filter_by(deleted=can_read_deleted(context)).\649 filter_by(deleted=can_read_deleted(context)).\
656 first()650 first()
651
657 if not result:652 if not result:
658 raise exception.FloatingIpNotFoundForAddress(address=address)653 raise exception.FloatingIpNotFoundForAddress(address=address)
654
655 # If the floating IP has a project ID set, check to make sure
656 # the non-admin user has access.
657 if result.project_id and is_user_context(context):
658 authorize_project_context(context, result.project_id)
659
659 return result660 return result
660661
661662
662663
=== modified file 'nova/tests/api/openstack/contrib/test_floating_ips.py'
--- nova/tests/api/openstack/contrib/test_floating_ips.py 2011-09-07 23:38:41 +0000
+++ nova/tests/api/openstack/contrib/test_floating_ips.py 2011-09-21 20:25:31 +0000
@@ -278,7 +278,7 @@
278 req.body = json.dumps(body)278 req.body = json.dumps(body)
279 req.headers["content-type"] = "application/json"279 req.headers["content-type"] = "application/json"
280 resp = req.get_response(fakes.wsgi_app())280 resp = req.get_response(fakes.wsgi_app())
281 self.assertEqual(resp.status_int, 400)281 self.assertEqual(resp.status_int, 401)
282282
283 def test_associate_floating_ip_to_instance_no_project_id(self):283 def test_associate_floating_ip_to_instance_no_project_id(self):
284 def fake_fixed_ip_get_by_address(ctx, address, session=None):284 def fake_fixed_ip_get_by_address(ctx, address, session=None):
285285
=== modified file 'nova/tests/test_network.py'
--- nova/tests/test_network.py 2011-09-15 19:25:52 +0000
+++ nova/tests/test_network.py 2011-09-21 20:25:31 +0000
@@ -436,6 +436,52 @@
436 self.network.add_fixed_ip_to_instance(self.context, 1, HOST,436 self.network.add_fixed_ip_to_instance(self.context, 1, HOST,
437 networks[0]['id'])437 networks[0]['id'])
438438
439 def test_ip_association_and_allocation_of_other_project(self):
440 """Makes sure that we cannot deallocaate or disassociate
441 a public ip of other project"""
442
443 context1 = context.RequestContext('user', 'project1')
444 context2 = context.RequestContext('user', 'project2')
445
446 address = '1.2.3.4'
447 float_addr = db.floating_ip_create(context1.elevated(),
448 {'address': address,
449 'project_id': context1.project_id})
450
451 instance = db.instance_create(context1,
452 {'project_id': 'project1'})
453
454 fix_addr = db.fixed_ip_associate_pool(context1.elevated(),
455 1, instance['id'])
456
457 # Associate the IP with non-admin user context
458 self.assertRaises(exception.NotAuthorized,
459 self.network.associate_floating_ip,
460 context2,
461 float_addr,
462 fix_addr)
463
464 # Deallocate address from other project
465 self.assertRaises(exception.NotAuthorized,
466 self.network.deallocate_floating_ip,
467 context2,
468 float_addr)
469
470 # Now Associates the address to the actual project
471 self.network.associate_floating_ip(context1, float_addr, fix_addr)
472
473 # Now try dis-associating from other project
474 self.assertRaises(exception.NotAuthorized,
475 self.network.disassociate_floating_ip,
476 context2,
477 float_addr)
478
479 # Clean up the ip addresses
480 self.network.deallocate_floating_ip(context1, float_addr)
481 self.network.deallocate_fixed_ip(context1, fix_addr)
482 db.floating_ip_destroy(context1.elevated(), float_addr)
483 db.fixed_ip_disassociate(context1.elevated(), fix_addr)
484
439485
440class CommonNetworkTestCase(test.TestCase):486class CommonNetworkTestCase(test.TestCase):
441 def fake_create_fixed_ips(self, context, network_id):487 def fake_create_fixed_ips(self, context, network_id):