yahoo-eng-team team mailing list archive

[Matt Riedemann <mriedem.os@xxxxxxxxx>]

Public bug reported:

This is a follow on from bug 1670627. As pointed out in comments 23 and
25, in the local delete case where there is no host and the instance is
in a cell, we're decrementing quota even if the instance.destroy()
operation fails.

We commit the usage decrement here:

https://github.com/openstack/nova/blob/88bc8dc5ce32748452c9d3acda9f35e77fedb6ce/nova/compute/api.py#L1878

Attempt to destroy the instance here:

https://github.com/openstack/nova/blob/88bc8dc5ce32748452c9d3acda9f35e77fedb6ce/nova/compute/api.py#L1887

And if the instance.destroy() fails, we rollback the usage decrement
here:

https://github.com/openstack/nova/blob/88bc8dc5ce32748452c9d3acda9f35e77fedb6ce/nova/compute/api.py#L1891

That rollback has no effect because once we commit a reservation, it's
wiped out in the quotas object:

https://github.com/openstack/nova/blob/88bc8dc5ce32748452c9d3acda9f35e77fedb6ce/nova/objects/quotas.py#L105

Attempting to rollback reservations on a quotas object that has already
committed reservations is a noop:

https://github.com/openstack/nova/blob/88bc8dc5ce32748452c9d3acda9f35e77fedb6ce/nova/objects/quotas.py#L111

--

Unlike the 'normal' (pre-cellsv2) local delete case which does the
commit after the instance is destroyed:

https://github.com/openstack/nova/blob/88bc8dc5ce32748452c9d3acda9f35e77fedb6ce/nova/compute/api.py#L2023

And we rollback (but not commit) if instance.destroy() fails because the
instance is already deleted:

https://github.com/openstack/nova/blob/88bc8dc5ce32748452c9d3acda9f35e77fedb6ce/nova/compute/api.py#L2028

--

The code in _delete() is wrong because it was copied from the code in
_delete_while_booting() which is also wrong:

https://github.com/openstack/nova/blob/88bc8dc5ce32748452c9d3acda9f35e77fedb6ce/nova/compute/api.py#L1784

So we have to fix both places.

I suggest that we also change the noop behavior on the Quotas object
such that if we attempt a reservation commit or rollback operation on a
Quotas object that does not have any reservations, it is considered a
programming error rather than a noop, because that's how this bug was
introduced in the first place.

** Affects: nova
     Importance: High
     Assignee: Matt Riedemann (mriedem)
         Status: Triaged

** Affects: nova/ocata
     Importance: High
         Status: Confirmed


** Tags: api ocata-backport-potential quotas

** Changed in: nova
     Assignee: (unassigned) => Matt Riedemann (mriedem)

** Also affects: nova/ocata
   Importance: Undecided
       Status: New

** Changed in: nova/ocata
   Importance: Undecided => High

** Changed in: nova/ocata
       Status: New => Confirmed

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to OpenStack Compute (nova).
https://bugs.launchpad.net/bugs/1678326

Title:
  Quota is decremented during instance delete in cell0 even if the
  instance destroy fails

Status in OpenStack Compute (nova):
  Triaged
Status in OpenStack Compute (nova) ocata series:
  Confirmed

Bug description:
  This is a follow on from bug 1670627. As pointed out in comments 23
  and 25, in the local delete case where there is no host and the
  instance is in a cell, we're decrementing quota even if the
  instance.destroy() operation fails.

  We commit the usage decrement here:

  https://github.com/openstack/nova/blob/88bc8dc5ce32748452c9d3acda9f35e77fedb6ce/nova/compute/api.py#L1878

  Attempt to destroy the instance here:

  https://github.com/openstack/nova/blob/88bc8dc5ce32748452c9d3acda9f35e77fedb6ce/nova/compute/api.py#L1887

  And if the instance.destroy() fails, we rollback the usage decrement
  here:

  https://github.com/openstack/nova/blob/88bc8dc5ce32748452c9d3acda9f35e77fedb6ce/nova/compute/api.py#L1891

  That rollback has no effect because once we commit a reservation, it's
  wiped out in the quotas object:

  https://github.com/openstack/nova/blob/88bc8dc5ce32748452c9d3acda9f35e77fedb6ce/nova/objects/quotas.py#L105

  Attempting to rollback reservations on a quotas object that has
  already committed reservations is a noop:

  https://github.com/openstack/nova/blob/88bc8dc5ce32748452c9d3acda9f35e77fedb6ce/nova/objects/quotas.py#L111

  --

  Unlike the 'normal' (pre-cellsv2) local delete case which does the
  commit after the instance is destroyed:

  https://github.com/openstack/nova/blob/88bc8dc5ce32748452c9d3acda9f35e77fedb6ce/nova/compute/api.py#L2023

  And we rollback (but not commit) if instance.destroy() fails because
  the instance is already deleted:

  https://github.com/openstack/nova/blob/88bc8dc5ce32748452c9d3acda9f35e77fedb6ce/nova/compute/api.py#L2028

  --

  The code in _delete() is wrong because it was copied from the code in
  _delete_while_booting() which is also wrong:

  https://github.com/openstack/nova/blob/88bc8dc5ce32748452c9d3acda9f35e77fedb6ce/nova/compute/api.py#L1784

  So we have to fix both places.

  I suggest that we also change the noop behavior on the Quotas object
  such that if we attempt a reservation commit or rollback operation on
  a Quotas object that does not have any reservations, it is considered
  a programming error rather than a noop, because that's how this bug
  was introduced in the first place.

To manage notifications about this bug go to:
https://bugs.launchpad.net/nova/+bug/1678326/+subscriptions