deletes fail when instance in RESIZED

Bug #1056601 reported by Chris Behrens
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
High
Chris Behrens
Folsom
Fix Released
High
Chuck Short
nova (Ubuntu)
Fix Released
Undecided
Unassigned
Quantal
Fix Released
Undecided
Unassigned

Bug Description

db.migration_get_by_instance_and_status requires admin status

compute/api.py _delete() will call this method if instance is in RESIZED state with a non-elevated context.

I don't think we should be calling a DB API call like this that requires elevated context, however.

revert_resize and confirm_resize elevate the context when calling this method.

I think we should make this method not require admin context, verify matching project_id if non-admin... but the other calls of this need to be audited.

Chris Behrens (cbehrens)
Changed in nova:
assignee: nobody → Chris Behrens (cbehrens)
status: New → In Progress
Revision history for this message
Johannes Erdfelt (johannes.erdfelt) wrote :

The use of elevated context in nova/compute/manager.py confuses me a bit. I think every method in there elevates the context before continuing. I guess I don't fully understand the use of it if every method requires an elevated context.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (master)

Fix proposed to branch: master
Review: https://review.openstack.org/13702

Changed in nova:
importance: Undecided → High
tags: added: folsom-backport-potential
Revision history for this message
Vish Ishaya (vishvananda) wrote :

Johannes: elevating every context was a temporary workaround so we didn't have to audit every call and figure out which db calls needed to be admin. I think the admin checking in the db layer has likely outlived its usefulness. It probably should be converted to policy checking or removed completely.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (master)

Reviewed: https://review.openstack.org/13702
Committed: http://github.com/openstack/nova/commit/8c9c380935b4aac40269754a291134c130bccec9
Submitter: Jenkins
Branch: master

commit 8c9c380935b4aac40269754a291134c130bccec9
Author: Chris Behrens <email address hidden>
Date: Wed Sep 26 10:46:49 2012 +0000

    Fix issues deleting instances in RESIZED state

    DB API call migration_get_by_instance_and_status requires admin
    context...and it'll also raise an exception if no migration matches.

    _delete() in compute/api was not using admin context, and was checking
    for a false return from the DB call vs checking for an exception.

    Added a test that covers both problems.

    Fixes bug 1056601

    Change-Id: Ib3c631dccbb2776761cf5acb8bab1d814bf3e282

Changed in nova:
status: In Progress → Fix Committed
Revision history for this message
Chris Behrens (cbehrens) wrote :

I went to try to change the DB call to not require admin context... such that if it's a user context, it'll verify project id matches. However, I found that it doesn't join the instance... and the Migration model doesn't have project_id. So, I guess I'm just going to ignore that for now and live with the elevated context being passed in..

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (stable/folsom)

Fix proposed to branch: stable/folsom
Review: https://review.openstack.org/14751

Chuck Short (zulcss)
tags: removed: folsom-backport-potential
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (stable/folsom)

Reviewed: https://review.openstack.org/14751
Committed: http://github.com/openstack/nova/commit/ded04737acfa55a0f84ee270702b2f011970b1f3
Submitter: Jenkins
Branch: stable/folsom

commit ded04737acfa55a0f84ee270702b2f011970b1f3
Author: Chris Behrens <email address hidden>
Date: Wed Sep 26 10:46:49 2012 +0000

    Fix issues deleting instances in RESIZED state

    DB API call migration_get_by_instance_and_status requires admin
    context...and it'll also raise an exception if no migration matches.

    _delete() in compute/api was not using admin context, and was checking
    for a false return from the DB call vs checking for an exception.

    Added a test that covers both problems.

    Fixes bug 1056601

    Change-Id: Ib3c631dccbb2776761cf5acb8bab1d814bf3e282
    (cherry picked from commit 8c9c380935b4aac40269754a291134c130bccec9)

Revision history for this message
Jian Wen (wenjianhn) wrote :

+1 for converting admin checking in the db layer converted to policy checking.

Revision history for this message
Jian Wen (wenjianhn) wrote :

Or use temporary_mutation instead?
 with utils.temporary_mutation(context, is_admin=True):
           db_call_require_admin_context(context, ...)

Thierry Carrez (ttx)
Changed in nova:
milestone: none → grizzly-1
status: Fix Committed → Fix Released
Changed in nova (Ubuntu):
status: New → Fix Released
Changed in nova (Ubuntu Quantal):
status: New → Confirmed
Revision history for this message
Clint Byrum (clint-fewbar) wrote : Please test proposed package

Hello Chris, or anyone else affected,

Accepted nova into quantal-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/nova/2012.2.1+stable-20121212-a99a802e-0ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in nova (Ubuntu Quantal):
status: Confirmed → Fix Committed
tags: added: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (8.3 KiB)

This bug was fixed in the package nova - 2012.2.1+stable-20121212-a99a802e-0ubuntu1

---------------
nova (2012.2.1+stable-20121212-a99a802e-0ubuntu1) quantal-proposed; urgency=low

  * Ubuntu updates:
    - debian/control: Ensure novaclient is upgraded with nova,
      require python-keystoneclient >= 1:2.9.0. (LP: #1073289)
    - d/p/avoid_setuptools_git_dependency.patch: Refresh.
  * Dropped patches, applied upstream:
    - debian/patches/CVE-2012-5625.patch: [a99a802]
  * Resynchronize with stable/folsom (b55014ca) (LP: #1085255):
    - [a99a802] create_lvm_image allocates dirty blocks (LP: #1070539)
    - [670b388] RPC exchange name defaults to 'openstack' (LP: #1083944)
    - [3ede373] disassociate_floating_ip with multi_host=True fails
      (LP: #1074437)
    - [22d7c3b] libvirt imagecache should handle shared image storage
      (LP: #1075018)
    - [e787786] Detached and deleted RBD volumes remain associated with insance
      (LP: #1083818)
    - [9265eb0] live_migration missing migrate_data parameter in Hyper-V driver
      (LP: #1066513)
    - [3d99848] use_single_default_gateway does not function correctly
      (LP: #1075859)
    - [65a2d0a] resize does not migrate DHCP host information (LP: #1065440)
    - [102c76b] Nova backup image fails (LP: #1065053)
    - [48a3521] Fix config-file overrides for nova-dhcpbridge
    - [69663ee] Cloudpipe in Folsom: no such option: cnt_vpn_clients
      (LP: #1069573)
    - [6e47cc8] DisassociateAddress can cause Internal Server Error
      (LP: #1080406)
    - [22c3d7b] API calls to dis-associate an auto-assigned floating IP should
      return proper warning (LP: #1061499)
    - [bd11d15] libvirt: if exception raised during volume_detach, volume state
      is inconsistent (LP: #1057756)
    - [dcb59c3] admin can't describe all images in ec2 api (LP: #1070138)
    - [78de622] Incorrect Exception raised during Create server when metadata
      over 255 characters (LP: #1004007)
    - [c313de4] Fixed IP isn't released before updating DHCP host file
      (LP: #1078718)
    - [f4ab42d] Enabling Return Reservation ID with XML create server request
      returns no body (LP: #1061124)
    - [3db2a38] 'BackupCreate' should accept rotation parameter greater than or
      equal to zero (LP: #1071168)
    - [f7e5dde] libvirt reboot sometimes fails to reattach volumes
      (LP: #1073720)
    - [ff776d4] libvirt: detaching volume may fail while terminating other
      instances on the same host concurrently (LP: #1060836)
    - [85a8bc2] Used instance uuid rather than id in remove-fixed-ip
    - [42a85c0] Fix error on invalid delete_on_termination value
    - [6a17579] xenapi migrations fail w/ swap (LP: #1064083)
    - [97649b8] attach-time field for volumes is not updated for detach volume
      (LP: #1056122)
    - [8f6a718] libvirt: rebuild is not using kernel and ramdisk associated with
      the new image (LP: #1060925)
    - [fbe835f] live-migration and volume host assignement (LP: #1066887)
    - [c2a9150] typo prevents volume_tmp_dir flag from working (LP: #1071536)
    - [93efa21] Instances deleted during spawn leak network allocations
      (LP: #1068716)
    - [ebabd02] After restartin...

Read more...

Changed in nova (Ubuntu Quantal):
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in nova:
milestone: grizzly-1 → 2013.1
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.