Race can cause instance to become ACTIVE after build error

Bug #1848666 reported by Matthew Booth
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
In Progress
Undecided
Stephen Finucane

Bug Description

2 functions used in error cleanup in _do_build_and_run_instance: _cleanup_allocated_networks and _set_instance_obj_error_state, call an unguarded instance.save(). The problem with this is that the instance object may have been in an unclean state before the build exception was raised. Calling instance.save() will persist this unclean error state in addition to whatever change was made during cleanup, which is not intended.

Specifically in the case that a build races with a delete, the build can fail when we try to do an atomic save to set the vm_state to active, raising UnexpectedDeletingTaskStateError. However, the instance object still contains the unpersisted vm_state change and task_state reset along with other concomitant changes. These will all be persisted when _cleanup_allocated_networks calls instance.save(). This means that the instance.save(expected_task_state=SPAWNING) which correctly failed due to a race, later succeeds accidentally in cleanup resulting in an inconsistent instance state.

Matthew Booth (mbooth-9)
description: updated
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (master)

Fix proposed to branch: master
Review: https://review.opendev.org/689388

Changed in nova:
assignee: nobody → Matthew Booth (mbooth-9)
status: New → In Progress
Changed in nova:
assignee: Matthew Booth (mbooth-9) → Stephen Finucane (stephenfinucane)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (master)

Reviewed: https://review.opendev.org/689278
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=10434bd229973b37647741e58aff3ac90b3a0a6c
Submitter: Zuul
Branch: master

commit 10434bd229973b37647741e58aff3ac90b3a0a6c
Author: Matthew Booth <email address hidden>
Date: Thu Oct 17 21:29:46 2019 +0100

    Functional test for UnexpectedDeletingTaskStateError

    Adds a regression-style test for two cleanup bugs when
    'UnexpectedDeletingTaskStateError' is raised during build.

    Change-Id: Ief1dfbb6cc9d67b73dfab4c7b63358e76e12866b
    Related-Bug: #1848666
    Related-Bug: #1831771

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/train)

Related fix proposed to branch: stable/train
Review: https://review.opendev.org/711210

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/train)

Reviewed: https://review.opendev.org/711210
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=24600430a2e4c67a4584d1ee466d3376aae96f25
Submitter: Zuul
Branch: stable/train

commit 24600430a2e4c67a4584d1ee466d3376aae96f25
Author: Matthew Booth <email address hidden>
Date: Thu Oct 17 21:29:46 2019 +0100

    Functional test for UnexpectedDeletingTaskStateError

    Adds a regression-style test for two cleanup bugs when
    'UnexpectedDeletingTaskStateError' is raised during build.

    Modified:
     nova/tests/functional/regressions/test_bug_1831771.py
     nova/tests/functional/integrated_helpers.py

    NOTE(stephenfin): The '_build_server' function has to be replaced by
    '_build_minimal_create_server_request' since we don't have change
    I91fa2f73185fef48e9aae9b7f61389c374e06676 here. Similarly, we need to
    make '_wait_until_deleted' handle responses that don't include the
    'status' field, such as the response received when creating a server,
    which was done in change I0c56841d098d3e9d72db65be3143f3c893f0b6ba on
    master. Finally, change I36da36cc5b099174eece0dfba29485fc20b2867b has
    been squashed into this change to avoid the races we saw with this test
    on master.

    Change-Id: Ief1dfbb6cc9d67b73dfab4c7b63358e76e12866b
    Related-Bug: #1848666
    Related-Bug: #1831771
    (cherry picked from commit 10434bd229973b37647741e58aff3ac90b3a0a6c)

tags: added: in-stable-train
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/stein)

Related fix proposed to branch: stable/stein
Review: https://review.opendev.org/715399

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/rocky)

Related fix proposed to branch: stable/rocky
Review: https://review.opendev.org/715403

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

Related fix proposed to branch: stable/queens
Review: https://review.opendev.org/715405

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/stein)

Reviewed: https://review.opendev.org/715399
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=ff2101654376993f248abca03e123d7233af4562
Submitter: Zuul
Branch: stable/stein

commit ff2101654376993f248abca03e123d7233af4562
Author: Matthew Booth <email address hidden>
Date: Thu Oct 17 21:29:46 2019 +0100

    Functional test for UnexpectedDeletingTaskStateError

    Adds a regression-style test for two cleanup bugs when
    'UnexpectedDeletingTaskStateError' is raised during build.

    Change-Id: Ief1dfbb6cc9d67b73dfab4c7b63358e76e12866b
    Related-Bug: #1848666
    Related-Bug: #1831771
    (cherry picked from commit 10434bd229973b37647741e58aff3ac90b3a0a6c)
    (cherry picked from commit 24600430a2e4c67a4584d1ee466d3376aae96f25)

tags: added: in-stable-stein
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/rocky)

Reviewed: https://review.opendev.org/715403
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=5a17dd1e34caa44ce8a1406aea9377d8f04421ed
Submitter: Zuul
Branch: stable/rocky

commit 5a17dd1e34caa44ce8a1406aea9377d8f04421ed
Author: Matthew Booth <email address hidden>
Date: Thu Oct 17 21:29:46 2019 +0100

    Functional test for UnexpectedDeletingTaskStateError

    Adds a regression-style test for two cleanup bugs when
    'UnexpectedDeletingTaskStateError' is raised during build.

    Change-Id: Ief1dfbb6cc9d67b73dfab4c7b63358e76e12866b
    Related-Bug: #1848666
    Related-Bug: #1831771
    (cherry picked from commit 10434bd229973b37647741e58aff3ac90b3a0a6c)
    (cherry picked from commit 24600430a2e4c67a4584d1ee466d3376aae96f25)
    (cherry picked from commit ff2101654376993f248abca03e123d7233af4562)

tags: added: in-stable-rocky
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/queens)

Reviewed: https://review.opendev.org/715405
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=e33f4042c1292fc630fae6740331ccfb8c074717
Submitter: Zuul
Branch: stable/queens

commit e33f4042c1292fc630fae6740331ccfb8c074717
Author: Matthew Booth <email address hidden>
Date: Thu Oct 17 21:29:46 2019 +0100

    Functional test for UnexpectedDeletingTaskStateError

    Adds a regression-style test for two cleanup bugs when
    'UnexpectedDeletingTaskStateError' is raised during build.

    Modified:
     nova/tests/functional/regressions/test_bug_1831771.py

    NOTE(stephenfin): Modifications are necessary since we don't have change
    Iea283322124cb35fc0bc6d25f35548621e8c8c2f, which moved the
    'ProviderUsageBaseTestCase' base test class from 'test_servers.py' to
    'integrated_helpers.py'.

    Change-Id: Ief1dfbb6cc9d67b73dfab4c7b63358e76e12866b
    Related-Bug: #1848666
    Related-Bug: #1831771
    (cherry picked from commit 10434bd229973b37647741e58aff3ac90b3a0a6c)
    (cherry picked from commit 24600430a2e4c67a4584d1ee466d3376aae96f25)
    (cherry picked from commit ff2101654376993f248abca03e123d7233af4562)
    (cherry picked from commit 5a17dd1e34caa44ce8a1406aea9377d8f04421ed)

tags: added: in-stable-queens
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.