ComputeManager.cleanup_host fails when there are waiting events: ValueError: Field value network-vif-plugged-ce531f90-199f-48c0-816c is invalid

Bug #1760303 reported by Matt Riedemann
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
Medium
Dan Smith
Ocata
Triaged
Medium
Unassigned
Pike
Triaged
Medium
Unassigned
Queens
Fix Committed
Medium
Dan Smith

Bug Description

The parsing in cancel_all_events() fails to account for names such as:

network-vif-plugged-ce531f90-199f-48c0-816c

https://github.com/openstack/nova/blob/167023b5074c7cb241b33767d76a116ec95d652c/nova/compute/manager.py#L406

Which can be constructed when registering the event using a tuple of the event name and tag:

https://github.com/openstack/nova/blob/167023b5074c7cb241b33767d76a116ec95d652c/nova/compute/manager.py#L460-L463

https://github.com/openstack/nova/blob/167023b5074c7cb241b33767d76a116ec95d652c/nova/objects/external_event.py#L51

So we almost need something like that make_key function but more like parse_key where using the known EVENT_NAMES we can split the event name to get the name and tag.

Tags: compute
Revision history for this message
Matt Riedemann (mriedem) wrote :

Seeing this in a functional test:

http://logs.openstack.org/06/558006/1/check/nova-tox-functional/b74bcd5/testr_results.html.gz

2018-03-30 23:37:19,432 ERROR [nova.service] Service error occurred during cleanup_host
Traceback (most recent call last):
  File "nova/service.py", line 286, in stop
    self.manager.cleanup_host()
  File "nova/compute/manager.py", line 1166, in cleanup_host
    self.instance_events.cancel_all_events()
  File "nova/compute/manager.py", line 410, in cancel_all_events
    tag=tag, data={})
  File "/home/zuul/src/git.openstack.org/openstack/nova/.tox/functional/local/lib/python2.7/site-packages/oslo_versionedobjects/base.py", line 307, in __init__
    setattr(self, key, kwargs[key])
  File "/home/zuul/src/git.openstack.org/openstack/nova/.tox/functional/local/lib/python2.7/site-packages/oslo_versionedobjects/base.py", line 72, in setter
    field_value = field.coerce(self, name, value)
  File "/home/zuul/src/git.openstack.org/openstack/nova/.tox/functional/local/lib/python2.7/site-packages/oslo_versionedobjects/fields.py", line 195, in coerce
    return self._type.coerce(obj, attr, value)
  File "/home/zuul/src/git.openstack.org/openstack/nova/.tox/functional/local/lib/python2.7/site-packages/oslo_versionedobjects/fields.py", line 317, in coerce
    raise ValueError(msg)
ValueError: Field value network-vif-plugged-ce531f90-199f-48c0-816c is invalid
}}}

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/558059

Changed in nova:
assignee: nobody → Matt Riedemann (mriedem)
status: Triaged → In Progress
Revision history for this message
Matt Riedemann (mriedem) wrote :

The cancel_all_events function has been around since kilo so it's somewhat surprising no one has ever reported this before.

Changed in nova:
assignee: Matt Riedemann (mriedem) → Dan Smith (danms)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (master)

Reviewed: https://review.openstack.org/558059
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=d9369d034cb0694def2510885ddfb9b039cbad49
Submitter: Zuul
Branch: master

commit d9369d034cb0694def2510885ddfb9b039cbad49
Author: Matt Riedemann <email address hidden>
Date: Sat Mar 31 10:28:29 2018 -0400

    Fix cancel_all_events event name parsing

    The cancel_all_events function was incorrectly assuming
    that event tags didn't have dashes in them, but they all
    do since they are either port or volume IDs, which are UUIDs.

    Instead of just storing our event data indexed by the composite
    event key, we should just store the name and tag distinctly so
    that there is no question about how to dissect the two from the
    key later. This is all internal record-keeping inside of the
    compute manager, so we can change it to whatever we want.

    This includes a slight change to the internal virtapi interface
    for wait_for_instance_event(), which removes the ability to
    pass composite event keys as strings instead of (name, tag)
    tuples. In reality, none of the virt code was using the key string
    format, so this just cleans up a few tests that were. It makes
    that interface less ambiguous, which is good anyway.

    Change-Id: I15f041e820c2dabe82e8e25bf77f9cc86330a76a
    Closes-Bug: #1760303

Changed in nova:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/nova 18.0.0.0b1

This issue was fixed in the openstack/nova 18.0.0.0b1 development milestone.

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

Fix proposed to branch: stable/queens
Review: https://review.openstack.org/592086

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

Reviewed: https://review.openstack.org/592086
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=77a831de8b5ef573527b844c852bf22f775d7787
Submitter: Zuul
Branch: stable/queens

commit 77a831de8b5ef573527b844c852bf22f775d7787
Author: Matt Riedemann <email address hidden>
Date: Sat Mar 31 10:28:29 2018 -0400

    Fix cancel_all_events event name parsing

    The cancel_all_events function was incorrectly assuming
    that event tags didn't have dashes in them, but they all
    do since they are either port or volume IDs, which are UUIDs.

    Instead of just storing our event data indexed by the composite
    event key, we should just store the name and tag distinctly so
    that there is no question about how to dissect the two from the
    key later. This is all internal record-keeping inside of the
    compute manager, so we can change it to whatever we want.

    This includes a slight change to the internal virtapi interface
    for wait_for_instance_event(), which removes the ability to
    pass composite event keys as strings instead of (name, tag)
    tuples. In reality, none of the virt code was using the key string
    format, so this just cleans up a few tests that were. It makes
    that interface less ambiguous, which is good anyway.

    Change-Id: I15f041e820c2dabe82e8e25bf77f9cc86330a76a
    Closes-Bug: #1760303
    (cherry picked from commit d9369d034cb0694def2510885ddfb9b039cbad49)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/nova 17.0.6

This issue was fixed in the openstack/nova 17.0.6 release.

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.