Nova informs Placement too early upon Ironic instance deletion

In theory, we first call _shutdown_instance() [1] before destroying the instance which disallocate the resources [2].

When we call driver.destroy() in shutdown_instance(), we asynchronously call the Ironic API to unprovision the node and we hold until we are sure that the node is unprovisioned [3]

[1] https://github.com/openstack/nova/blob/90777d7/nova/compute/manager.py#L2980
[2] https://github.com/openstack/nova/blob/90777d7/nova/compute/manager.py#L3013
[3] https://github.com/openstack/nova/blob/90777d7/nova/virt/ironic/driver.py#L1317

Could you please check the compute logs and tell us whether the timings tell us that the instance was destroyed *before* the node was unprovisioned ?

Thanks for having a look, Sylvain!

I have a question for the hold loop you mention in [3]: it looks to me like the driver is only waiting for the node to be in one of

 ironic_states.NOSTATE
 ironic_states.CLEANING
 ironic_states.CLEANWAIT
 ironic_states.CLEANFAIL
 ironic_states.AVAILABLE

to consider the node unprovisioned. Is my understanding correct?

If so, a node in CLEANWAIT, for instance, is not ready to be re-instantiated:
- cleaning could still take hours or days to clean
- it could fail cleaning
So, telling placement there is an allocation candidate for a node in this state is too early I would think.

The same is true for CLEANING, of course, as this is just the state before CLEANWAIT.
Even worse for CLEANFAIL: the node will not become AVAILABLE any time soon.

But maybe I am just missing sth :)

[Expired for OpenStack Compute (nova) because there has been no activity for 60 days.]

Hi, please re-open as this is relevant still in train.

I re-opened it by setting the status to new.

Sylvain: the bug was marked Incomplete, but I had added a question/comment related to your request for more information. Would you mind having another look and let me know if you (still) need sth from our side?

I might not have the full context but the code Sylvain is linked to are waiting for the instance being destroyed[1]. If we change the states we are waiting in _unprovision() then we hold up the destroying of the instance object in the upper layers. Therefor holding up volume / network resources, instance quotas. Also if the ironic node ends up in ironic_states.CLEANFAIL then that would mean that the end user visible instance will be in DELETING state for a very long time.

I think that in ironic we need to decouple the instance destroyed state from the resource is ready to be used again state. E.g.: _unprovision could put a trait (e.g. CUSTOM_IRONIC_NEEDS_CLEANING) on the ironic node RP and we can have a placement pre-filter that filters out nodes with that trait during scheduling.

I don't know how this is happen:
"This is fixed once the resource tracker comes along and corrects the information in placement,"
but that logic might be extended to the _unprovision case to solve the issue.

bottom line: I agree that this is possible a bug (I haven't reproduced myself). But the suggested solution needs further discussion.

I let others with more ironic knowledge to confirm / triage this.

[1] https://github.com/openstack/nova/blob/90777d790d7c268f50851ac3e5b4e02617f5ae1c/nova/virt/ironic/driver.py#L1295-L1297

We have encountered this in our production environment. It causes havoc with out event driven scheduling system and leads to failed deployments daily.

I'm going to propose that the destroy method doesn't return until the node state is AVAILABLE (also state CLEANFAIL will raise an exception). This means ironic CLEANING and CLEANWAIT will map to nova DELETING, which may last hours depending on the cleaning steps.

As far as I can tell from ironic/driver.py and nova/compute/manager.py, volume[1][2] and vif[3] cleanup happens before the state polling I intend to add, so we might be fine here unless Balazs is referring to other volume/network cleanup.

[1] https://opendev.org/openstack/nova/src/branch/master/nova/compute/manager.py#L3035
[2] https://opendev.org/openstack/nova/src/branch/master/nova/virt/ironic/driver.py#L507
[3] https://opendev.org/openstack/nova/src/branch/master/nova/virt/ironic/driver.py#L508

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/nova/+/813729

> I don't know how this is happen:
"This is fixed once the resource tracker comes along and corrects the information in placement,"
but that logic might be extended to the _unprovision case to solve the issue.

I think this is referring to how the resource tracker sets a node to reserved=True if the node info from ironic indicates the node is not AVAILABLE for provisioning in update_provider_tree during the update_available_resource periodic task in nova-compute.

[1] https://github.com/openstack/nova/blob/d5b6412ef52b1e5ad797a49850c9c6701b0405db/nova/virt/ironic/driver.py#L876-L885

Steve Baker, still on this?