<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jun 25, 2019 at 10:14 PM Matt Riedemann <<a href="mailto:mriedemos@gmail.com">mriedemos@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">There are still quite a few TODOs in the code [1][2][3] from a kilo era <br>
blueprint [4]. At this point I'm pretty sure you can't startup the <br>
nova-compute service without having a ComputeNode record without a host <br>
and hypervisor_hostname field set (we don't set the <br>
ComputeNode.service_id anywhere anymore as far as I can tell, except in <br>
some ComputeNode RPC compat code [5]).<br>
<br>
I've stumbled across all of this code before, but was looking at it <br>
again today because I have a very simple change I need to make which is <br>
going from a ComputeNode object and getting the related nova-compute <br>
Service object for that node.<br>
<br>
Looking at the code one might think this is reasonable:<br>
<br>
service = objects.Service.get_by_id(ctxt, compute_node.service_id)<br>
<br>
But compute_node.service_id is likely None. Or how about:<br>
<br>
service = objects.Service.get_by_compute_host(ctxt, compute_node.host)<br>
<br>
But ComputeNode.host is also nullable (though likely should have a value <br>
as noted above).<br>
<br></blockquote><div><br></div><div>Yeah basically, before this blueprint, a ComputeNode record was only having an hypervisor_hostname value and a service_id which was a FK from the Service record.</div><div>Given we preferred to have a tuple (host, hypervisor_hostname) key for the CN record, we deprecated the service_id and wanted to add a new field named 'host'.</div><div><br></div><div>For that, we were looking at the existing Service record to know the field value. After this, we were directly providing the 'host' field value.<br></div><div><br></div><div>That said, since it was possible to have compute and conductor services having different release versions, that's why we were wanting to still be able to look at the backward compability.</div><div>Now, we're waaaaay after Kilo, so I think we no longer need to support the compatibility ;-)</div><div><br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
This is a long way of me saying this code is all gross and we should <br>
clean it up, which means making sure all of this Kilo era compat code <br>
for old records is no longer necessary, which means all of those records <br>
should be migrated by now but how should we check?<br>
<br>
I *think* this might just be as simple as a "nova-status upgrade check" <br>
check which scans the cells looking for (non-deleted) compute_nodes <br>
records where host is NULL and report an error if any are found. I <br>
believe the recovery action for an operator that hits this is to delete <br>
the busted compute_nodes record and restart the nova-compute service so <br>
a new compute node record is created. I would really think that anything <br>
this scan would find would be orphaned compute_nodes records that could <br>
just be deleted since another compute_nodes record probably already <br>
exists for the same hypervisor_hostname value. IOW, I don't think we <br>
need an online data migration routine for this.<br>
<br></blockquote><div><br></div><div>Yeah, agreed with the above. I don't think we need an online data migration for this and I'm pretty sure an nova-status upgrade check should be enough.</div><div><br></div><div>-Sylvain</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Hopefully at least one person (Sylvain) can agree with me here and the <br>
plan of action I've put forth.<br>
<br>
[1] <br>
<a href="https://github.com/openstack/nova/blob/91647a9b711a8102c79bb17c6b4dff24ad6f8f58/nova/db/sqlalchemy/models.py#L123" rel="noreferrer" target="_blank">https://github.com/openstack/nova/blob/91647a9b711a8102c79bb17c6b4dff24ad6f8f58/nova/db/sqlalchemy/models.py#L123</a><br>
[2] <br>
<a href="https://github.com/openstack/nova/blob/91647a9b711a8102c79bb17c6b4dff24ad6f8f58/nova/objects/compute_node.py#L150" rel="noreferrer" target="_blank">https://github.com/openstack/nova/blob/91647a9b711a8102c79bb17c6b4dff24ad6f8f58/nova/objects/compute_node.py#L150</a><br>
[3] <br>
<a href="https://github.com/openstack/nova/blob/91647a9b711a8102c79bb17c6b4dff24ad6f8f58/nova/objects/compute_node.py#L263" rel="noreferrer" target="_blank">https://github.com/openstack/nova/blob/91647a9b711a8102c79bb17c6b4dff24ad6f8f58/nova/objects/compute_node.py#L263</a><br>
[4] <br>
<a href="https://blueprints.launchpad.net/nova/+spec/detach-service-from-computenode" rel="noreferrer" target="_blank">https://blueprints.launchpad.net/nova/+spec/detach-service-from-computenode</a><br>
[5] <br>
<a href="https://github.com/openstack/nova/blob/91647a9b711a8102c79bb17c6b4dff24ad6f8f58/nova/objects/compute_node.py#L118" rel="noreferrer" target="_blank">https://github.com/openstack/nova/blob/91647a9b711a8102c79bb17c6b4dff24ad6f8f58/nova/objects/compute_node.py#L118</a><br>
<br>
-- <br>
<br>
Thanks,<br>
<br>
Matt<br>
<br>
</blockquote></div></div>