Deleting compute service only deletes "first" ironic node resource provider from placement

[Eric Fried]

NB: This comes from code inspection, not observed behavior.

When the compute service is deleted, we attempt to delete from placement the resource provider associated with the compute node associated with the service [1].

But ironic deployments can have multiple compute nodes. In this case, the compute node associated with the service is the "first" such node [2].

What happens then is the compute node records are deleted, leaving the remaining N-1 nodes' provider records orphaned. Those get cleaned up (I think?) by update_available_resource when the service is recreated [3].

So we're deleting and recreating the ironic node resource providers, but in a weird order. We should probably either fix the code at [1] to delete all of them, or none of them (and let the orphan handling code do all the work).

[1] https://github.com/openstack/nova/blob/da98f4ba4554139b3901103aa0d26876b11e1d9a/nova/api/openstack/compute/services.py#L244-L247
[2] https://github.com/openstack/nova/blob/da98f4ba4554139b3901103aa0d26876b11e1d9a/nova/objects/service.py#L308-L311
[3] https://github.com/openstack/nova/blob/da98f4ba4554139b3901103aa0d26876b11e1d9a/nova/compute/manager.py#L7757-L7771

[Matt Riedemann (mriedem)]

I don't think [3] is doing what you'd expect for cleaning up orphans since it won't return compute nodes records from the database if the API deleted them:

https://github.com/openstack/nova/blob/da98f4ba4554139b3901103aa0d26876b11e1d9a/nova/compute/manager.py#L7777

So that for loop here:

https://github.com/openstack/nova/blob/da98f4ba4554139b3901103aa0d26876b11e1d9a/nova/compute/manager.py#L7758

Could just be an empty list, right?

[Matt Riedemann (mriedem)]

Re comment 1, in other words, that orphan cleanup code is meant (I think) for cases like the admin deleting ironic nodes via the ironic API so the ironic driver no longer reports them, and nova needs to cleanup the related compute nodes records in the DB (so the admin didn't do anything about deleting the compute service record in the API).

[Matt Riedemann (mriedem)]

FYI, this applies all the way back to Ocata:

https://review.openstack.org/#/q/I7b8622b178d5043ed1556d7bdceaf60f47e5ac80

[Matt Riedemann (mriedem)]

I'd say this is medium/low severity since we do correlate compute_nodes / resource providers / ironic node resources via the same uuid, which comes from the ironic node uuid, so although we orphan any other ironic node resource providers in this case, if a compute service is managing those compute node records (ironic nodes) anywhere, it should re-use the same resource providers based on the same uuid. The possible downside is if you have the scheduler configured with a low value for limiting placement allocation candidates and placement is reporting resource providers that aren't actually being managed by any compute service - in that case you could get NoValidHost errors.

[Matt Riedemann (mriedem)]

Related IRC discussion:

http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-01-14.log.html#t2019-01-14T21:35:45

[Matt Riedemann (mriedem)]

Bug 1817833 could be related and my comment 4 might not be accurate if we can't recreate the providers (or fail if they already exist).

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.opendev.org/657021

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.opendev.org/657070

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/657016
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=650fe118d128f09f78552b82abc114bb4b84930e
Submitter: Zuul
Branch: master

commit 650fe118d128f09f78552b82abc114bb4b84930e
Author: Matt Riedemann <email address hidden>
Date: Fri May 3 15:23:57 2019 -0400

    Delete resource providers for all nodes when deleting compute service

    Change I7b8622b178d5043ed1556d7bdceaf60f47e5ac80 started deleting the
    compute node resource provider associated with a compute node when
    deleting a nova-compute service. However, it would only delete the
    first compute node associated with the service which means for an
    ironic compute service that is managing multiple nodes, the resource
    providers were not cleaned up in placement. This fixes the issue by
    iterating all the compute nodes and cleaning up their providers.
    Note this could be potentially a lot of nodes, but we don't really
    have many good options here but to iterate them and clean them up
    one at a time.

    Note that this is best-effort but because of how the
    SchedulerReportClient.delete_resource_provider method ignores
    ResourceProviderInUse errors, and we could have stale allocations
    on the host for which delete_resource_provider is not accounting,
    namely allocations from evacuated instances (or incomplete migrations
    though you can't migrate baremetal instances today), we could still
    delete the compute service and orphan those in-use providers. That,
    however, is no worse than before this change where we did not try
    to cleanup all providers. The issue described above is being tracked
    with bug 1829479 and will be dealt with separately.

    Change-Id: I9e852e25ea89f32bf19cdaeb1f5dac8f749f5dbc
    Closes-Bug: #1811726

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/657021
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=8921a470ee797cd75def387ce184ee0ffaae6517
Submitter: Zuul
Branch: master

commit 8921a470ee797cd75def387ce184ee0ffaae6517
Author: Matt Riedemann <email address hidden>
Date: Fri May 3 15:40:30 2019 -0400

    Avoid unnecessary joins in delete_resource_provider

    If cascade=True, we're getting all of the instances on the
    compute node just to use the uuid, which will by default
    join on the info_cache and security_groups for the instances.
    This is a simple optimization to avoid those unnecessary joins.

    A TODO is left to further optimize this with a new InstanceList
    query method to just get the instance uuids on a given host/node
    combo, but that requires an RPC change which we can't backport.

    Change-Id: Ie121210456a240c257979d3269db115ddae2d23b
    Related-Bug: #1811726
    Related-Bug: #1756179

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/stein
Review: https://review.opendev.org/666840

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/rocky
Review: https://review.opendev.org/666854

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/queens
Review: https://review.opendev.org/666862

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/pike
Review: https://review.opendev.org/666865

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/ocata
Review: https://review.opendev.org/666874

[OpenStack Infra (hudson-openstack)]

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

commit e9889be94ee58a6da78ef96d09d8f5282b86f648
Author: Matt Riedemann <email address hidden>
Date: Fri May 3 15:23:57 2019 -0400

    Delete resource providers for all nodes when deleting compute service

    Change I7b8622b178d5043ed1556d7bdceaf60f47e5ac80 started deleting the
    compute node resource provider associated with a compute node when
    deleting a nova-compute service. However, it would only delete the
    first compute node associated with the service which means for an
    ironic compute service that is managing multiple nodes, the resource
    providers were not cleaned up in placement. This fixes the issue by
    iterating all the compute nodes and cleaning up their providers.
    Note this could be potentially a lot of nodes, but we don't really
    have many good options here but to iterate them and clean them up
    one at a time.

    Note that this is best-effort but because of how the
    SchedulerReportClient.delete_resource_provider method ignores
    ResourceProviderInUse errors, and we could have stale allocations
    on the host for which delete_resource_provider is not accounting,
    namely allocations from evacuated instances (or incomplete migrations
    though you can't migrate baremetal instances today), we could still
    delete the compute service and orphan those in-use providers. That,
    however, is no worse than before this change where we did not try
    to cleanup all providers. The issue described above is being tracked
    with bug 1829479 and will be dealt with separately.

    Change-Id: I9e852e25ea89f32bf19cdaeb1f5dac8f749f5dbc
    Closes-Bug: #1811726
    (cherry picked from commit 650fe118d128f09f78552b82abc114bb4b84930e)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/666854
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=64d52788831f0e676f0b9bf9780eeadb38232def
Submitter: Zuul
Branch: stable/rocky

commit 64d52788831f0e676f0b9bf9780eeadb38232def
Author: Matt Riedemann <email address hidden>
Date: Fri May 3 15:23:57 2019 -0400

    Delete resource providers for all nodes when deleting compute service

    Change I7b8622b178d5043ed1556d7bdceaf60f47e5ac80 started deleting the
    compute node resource provider associated with a compute node when
    deleting a nova-compute service. However, it would only delete the
    first compute node associated with the service which means for an
    ironic compute service that is managing multiple nodes, the resource
    providers were not cleaned up in placement. This fixes the issue by
    iterating all the compute nodes and cleaning up their providers.
    Note this could be potentially a lot of nodes, but we don't really
    have many good options here but to iterate them and clean them up
    one at a time.

    Note that this is best-effort but because of how the
    SchedulerReportClient.delete_resource_provider method ignores
    ResourceProviderInUse errors, and we could have stale allocations
    on the host for which delete_resource_provider is not accounting,
    namely allocations from evacuated instances (or incomplete migrations
    though you can't migrate baremetal instances today), we could still
    delete the compute service and orphan those in-use providers. That,
    however, is no worse than before this change where we did not try
    to cleanup all providers. The issue described above is being tracked
    with bug 1829479 and will be dealt with separately.

    Change-Id: I9e852e25ea89f32bf19cdaeb1f5dac8f749f5dbc
    Closes-Bug: #1811726
    (cherry picked from commit 650fe118d128f09f78552b82abc114bb4b84930e)
    (cherry picked from commit e9889be94ee58a6da78ef96d09d8f5282b86f648)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/657070
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=4adface8b9c1cdb04f9401d4bbd1c2dc795f94a5
Submitter: Zuul
Branch: master

commit 4adface8b9c1cdb04f9401d4bbd1c2dc795f94a5
Author: Matt Riedemann <email address hidden>
Date: Fri May 3 18:03:39 2019 -0400

    Optimize SchedulerReportClient.delete_resource_provider

    This is used to optimize the SchedulerReportClient
    delete_resource_provider method when deleting a compute
    node and its related resource provider which happens
    in both the API when a compute service is deleted and
    in the compute service in the update_available_resource
    method for orphan nodes (so the new InstanceList query
    method needs to be remotable).

    The actual DB query stuff in here is tested in the
    nova.tests.functional.wsgi.test_services code.

    Change-Id: Id033e8f95f853ccfc34bd350be6df11c1bfb1b7d
    Related-Bug: #1811726

[OpenStack Infra (hudson-openstack)]

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

commit b2f438bab4a0fdaa37b1a57cff0b27fb2a3f3437
Author: Matt Riedemann <email address hidden>
Date: Fri May 3 15:23:57 2019 -0400

    Delete resource providers for all nodes when deleting compute service

    Change I7b8622b178d5043ed1556d7bdceaf60f47e5ac80 started deleting the
    compute node resource provider associated with a compute node when
    deleting a nova-compute service. However, it would only delete the
    first compute node associated with the service which means for an
    ironic compute service that is managing multiple nodes, the resource
    providers were not cleaned up in placement. This fixes the issue by
    iterating all the compute nodes and cleaning up their providers.
    Note this could be potentially a lot of nodes, but we don't really
    have many good options here but to iterate them and clean them up
    one at a time.

    Note that this is best-effort but because of how the
    SchedulerReportClient.delete_resource_provider method ignores
    ResourceProviderInUse errors, and we could have stale allocations
    on the host for which delete_resource_provider is not accounting,
    namely allocations from evacuated instances (or incomplete migrations
    though you can't migrate baremetal instances today), we could still
    delete the compute service and orphan those in-use providers. That,
    however, is no worse than before this change where we did not try
    to cleanup all providers. The issue described above is being tracked
    with bug 1829479 and will be dealt with separately.

    Conflicts:
          nova/tests/unit/api/openstack/compute/test_services.py

    NOTE(mriedem): The conflict is due to not having change
    Ie1aa45a508fbba16e969f216398dee15b37a9569 in Queens.

    Change-Id: I9e852e25ea89f32bf19cdaeb1f5dac8f749f5dbc
    Closes-Bug: #1811726
    (cherry picked from commit 650fe118d128f09f78552b82abc114bb4b84930e)
    (cherry picked from commit e9889be94ee58a6da78ef96d09d8f5282b86f648)
    (cherry picked from commit 64d52788831f0e676f0b9bf9780eeadb38232def)

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/nova 20.0.0.0rc1 release candidate.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/666865
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=bd2e0972993ffd732cde0a3a6154424e74d44f0a
Submitter: Zuul
Branch: stable/pike

commit bd2e0972993ffd732cde0a3a6154424e74d44f0a
Author: Matt Riedemann <email address hidden>
Date: Fri May 3 15:23:57 2019 -0400

    Delete resource providers for all nodes when deleting compute service

    Change I7b8622b178d5043ed1556d7bdceaf60f47e5ac80 started deleting the
    compute node resource provider associated with a compute node when
    deleting a nova-compute service. However, it would only delete the
    first compute node associated with the service which means for an
    ironic compute service that is managing multiple nodes, the resource
    providers were not cleaned up in placement. This fixes the issue by
    iterating all the compute nodes and cleaning up their providers.
    Note this could be potentially a lot of nodes, but we don't really
    have many good options here but to iterate them and clean them up
    one at a time.

    Note that this is best-effort but because of how the
    SchedulerReportClient.delete_resource_provider method ignores
    ResourceProviderInUse errors, and we could have stale allocations
    on the host for which delete_resource_provider is not accounting,
    namely allocations from evacuated instances (or incomplete migrations
    though you can't migrate baremetal instances today), we could still
    delete the compute service and orphan those in-use providers. That,
    however, is no worse than before this change where we did not try
    to cleanup all providers. The issue described above is being tracked
    with bug 1829479 and will be dealt with separately.

    Change-Id: I9e852e25ea89f32bf19cdaeb1f5dac8f749f5dbc
    Closes-Bug: #1811726
    (cherry picked from commit 650fe118d128f09f78552b82abc114bb4b84930e)
    (cherry picked from commit e9889be94ee58a6da78ef96d09d8f5282b86f648)
    (cherry picked from commit 64d52788831f0e676f0b9bf9780eeadb38232def)
    (cherry picked from commit b2f438bab4a0fdaa37b1a57cff0b27fb2a3f3437)

[OpenStack Infra (hudson-openstack)]

Change abandoned by "Lee Yarwood <email address hidden>" on branch: stable/ocata
Review: https://review.opendev.org/c/openstack/nova/+/666874

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/nova pike-eol release.