| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-05-31 | |||
| 15:07:08 | mriedem | belmoreira: what are you trying to solve with it? | |
| 15:07:14 | mriedem | missing allocations? | |
| 15:07:15 | sahid | jaypipes: yes yes :) | |
| 15:07:23 | jaypipes | sahid: ok, just making sure :) | |
| 15:07:36 | belmoreira | mriedem we had an issue with ironic that triggered the recreation of all compute_nodes; now the new compute_nodes can't be registered in placement because the previous resource_provider/allocations | |
| 15:08:12 | mriedem | so can't create the new resource provider because another with the same name/uuid already exists right? | |
| 15:08:15 | jaypipes | sahid, stephenfin: you'll note I tried as much as possible to remove any and all mention of NUMA topology (host or guest/virtual) in the CPU resources spec. | |
| 15:08:23 | mriedem | and can't delete the existing one because the existing one has allocations already? | |
| 15:08:48 | belmoreira | mriedem: yes | |
| 15:08:54 | belmoreira | the cleanest way to recover would be to delete everything related to ironic resources providers and recreate again | |
| 15:09:06 | jaypipes | sahid, stephenfin: at least in so much as the NUMA topology stuff wasn't really germane to the "request different providers supply some X amount of dedicated or shared CPU resources for the guest" | |
| 15:09:25 | belmoreira | unfortunately I have ~2000 nodes in this state | |
| 15:09:28 | mriedem | belmoreira: yeah i think what i'd probably do is pick one resource provider / allocation combo since they should be 1:1 with ironic, | |
| 15:09:40 | mriedem | clean it up using the osc-placement CLI | |
| 15:09:57 | mriedem | and then run heal_allocations but probably want to put a cap on the --max-count | |
| 15:09:59 | stephenfin | jaypipes: ack. In a meeting/working on NUMA-aware vswitches, but I'll take a look before EOD | |
| 15:10:12 | mriedem | belmoreira: this is why i had a note in the code about allowing passing in a single instance id to heal | |
| 15:10:17 | mriedem | that would be ideal in this case as a test | |
| 15:10:38 | jaypipes | stephenfin: no worries, you already reviewed again (and I will update to your comments) | |
| 15:10:46 | belmoreira | mriedem I will first test all on copies of the DBs | |
| 15:11:04 | mriedem | belmoreira: ok yeah definitely doing it in a test env would be good for both of us :) | |
| 15:12:26 | belmoreira | other use-case for this code is updating ironic allocations that were created before a resource_class was defined | |
| 15:13:38 | mriedem | so the existing allocations for the ironic instance are against vcpu/memory_mb/disk_gb and you want to change then to be against custom_baremetal_belmiro, yes? | |
| 15:13:46 | mriedem | *them | |
| 15:14:31 | belmoreira | mriedem: but the recreation of the compute_node in nova is something that needs to be reviewed; has been creating few issues. Don't know yet why this happened... | |
| 15:14:35 | mriedem | that's probably where the --force option mentioned in the comments would come in, since currently the command will skip any instances that already have allocations, not attempt to overwrite them | |
| 15:15:27 | belmoreira | mriedem:yes, that's way I was pointing this as another use-case | |
| 15:15:27 | mriedem | belmoreira: so i thought that sounded very much like https://review.openstack.org/#/c/508555/ | |
| 15:16:53 | belmoreira | mriedem: this was not rebalance... we had an operator working in ironic/nova-compute and know is very hard to track why this happened | |
| 15:18:07 | belmoreira | mriedem: but anyway it looks a very dangerous operation to do when having placement. Before placement this was OK (recreating a new compute_node) | |
| 15:18:13 | mriedem | was the related nova-compute service deleted by any chance? | |
| 15:18:37 | belmoreira | mriedem: no, I don't yet | |
| 15:18:40 | mriedem | ok | |
| 15:18:53 | mriedem | for ironic the compute node record should have a predictable uuid | |
| 15:19:21 | mriedem | unless... | |
| 15:19:33 | mriedem | we don't use the ironic node uuid when creating the compute node | |
| 15:20:37 | stephenfin | mriedem, dansmith: We've been talking about extending the limits dict but I just noticed this https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1968-L1991 | |
| 15:20:39 | mriedem | looks like we don't https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L580 | |
| 15:21:10 | stephenfin | mriedem, dansmith: Given that we've essentially deprecated 'limits' and have 'requested_networks' available to us, couldn't I just use that as is? | |
| 15:21:14 | jaypipes | stephenfin: who's been talking about extending the limits dict? | |
| 15:21:16 | belmoreira | mriedem: that's the problem. When the compute_node is recreated we get a new uuid | |
| 15:21:18 | mriedem | belmoreira: so i think in that code, resources['hypervisor_hostname'] for ironic is always the node uuid, | |
| 15:21:26 | stephenfin | jaypipes: not me | |
| 15:21:27 | mriedem | we should use that when creating the compute node record if we think it's new | |
| 15:21:35 | mriedem | belmoreira: yup, seems like a trivial fix | |
| 15:21:49 | mriedem | belmoreira: not sure if you have enough words to open a bug? | |
| 15:22:39 | mriedem | stephenfin: my guess is requested_networks aren't passed back to the cell conductor during a reschedule | |
| 15:22:55 | mriedem | oh it looks like they are https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1890 | |
| 15:22:56 | belmoreira | This is related with https://review.openstack.org/#/c/508555/ | |
| 15:23:16 | belmoreira | sorry not this one... | |
| 15:23:34 | stephenfin | mriedem: Yeah, I checked - the info is there | |
| 15:24:16 | stephenfin | For migrate and unshelve, I would need to check 'instance.info_cache.network_info' but that's a-ok | |
| 15:25:03 | kashyap | stephenfin: Thanks for the review here, https://review.openstack.org/#/c/567258/. Yep, that could go... | |
| 15:25:32 | kashyap | I'll respin. | |
| 15:26:37 | mriedem | belmoreira: hmm, this should discover if there is already a compute node in the db with the same hypervisor_hostname, which is the ironic node uuid https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L518 | |
| 15:26:44 | belmoreira | mriedem: https://bugs.launchpad.net/nova/+bug/1771806 (the main issue here is the new UUID that's generated. Don't think is required to have a different bug) | |
| 15:26:45 | openstack | Launchpad bug 1771806 in OpenStack Compute (nova) "Ironic nova-compute failover creates new resource provider removing the resource_provider_aggregates link " [Medium,Confirmed] - Assigned to Surya Seetharaman (tssurya) | |
| 15:26:50 | openstackgerrit | Merged openstack/nova stable/ocata: Fix shelving a paused instance https://review.openstack.org/569655 | |
| 15:26:50 | mriedem | so it might not be a rebalance, but that code should have found it | |
| 15:27:29 | mriedem | unless you're hitting https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L531 | |
| 15:27:50 | mriedem | belmoreira: yeah, and in https://bugs.launchpad.net/nova/+bug/1771806 i asked why https://review.openstack.org/#/c/508555/ doesn't fix the issue, and i'm still kind of wondering that | |
| 15:27:51 | openstack | Launchpad bug 1771806 in OpenStack Compute (nova) "Ironic nova-compute failover creates new resource provider removing the resource_provider_aggregates link " [Medium,Confirmed] - Assigned to Surya Seetharaman (tssurya) | |
| 15:28:02 | mriedem | are you seeing the error from this condition? https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L531 | |
| 15:28:44 | mriedem | jroll: oath is cells v2 now right? | |
| 15:28:47 | mriedem | + placement | |
| 15:28:56 | mriedem | has to be if ocata | |
| 15:29:13 | jroll | mriedem: yeah | |
| 15:29:15 | jroll | single cell | |
| 15:29:25 | mriedem | is that cell VMs or BMs? | |
| 15:29:38 | jroll | VM | |
| 15:29:38 | mriedem | just wondering if you've seen similar issues to what belmiro is describing above | |
| 15:29:41 | mriedem | oh | |
| 15:29:44 | jroll | still working on the baremetal upgrade | |
| 15:29:47 | mriedem | ok | |
| 15:30:00 | jroll | about to jump in a meeting but will read back shortly | |
| 15:31:01 | belmoreira | mriedem: I didn't hit that because in that case doesn't rebalance | |
| 15:31:40 | mriedem | belmoreira: sure but you don't need to rebalance to hit this check | |
| 15:31:52 | mriedem | the code that creates the new compute node record with the new uuid is here https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L580 | |
| 15:32:02 | mriedem | and right before that, we check the rebalance thing https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L574 | |
| 15:32:25 | mriedem | which will query the db for existing compute node recoreds by nodename (ironic node uuid) https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L518 | |
| 15:32:34 | jroll | mriedem: belmoreira: at a skim, this looks like https://bugs.launchpad.net/nova/+bug/1750450 | |
| 15:32:35 | openstack | Launchpad bug 1750450 in OpenStack Compute (nova) "ironic: n-cpu fails to recover after losing connection to ironic-api and placement-api" [Low,Fix released] - Assigned to Jim Rollenhagen (jim-rollenhagen) | |
| 15:32:35 | mriedem | so unless the node uuid changed, we should find the existing compute node reord | |
| 15:33:13 | jroll | or similar | |
| 15:33:30 | mriedem | similar, but that says, | |
| 15:33:31 | mriedem | "When ironic-api and placement-api return, nova will see nodes, create compute_node records for them, and try to create new resource providers (as they are new compute_node records). This will fail with a name conflict, and the nodes will be unusable." | |
| 15:33:42 | mriedem | which, unless the node uuids change, should still find the existing compute node records in the db | |
| 15:33:49 | mriedem | here https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L518 | |
| 15:34:49 | tssurya | mriedem: could be we return false here : https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L511 ? | |
| 15:35:00 | mriedem | tssurya: not if it's the ironic driver | |
| 15:35:12 | mriedem | that's the only one that has rebalances_nodes = True | |
| 15:35:22 | mriedem | for the libvirt driver, yes | |
| 15:35:26 | tssurya | oh okay | |
| 15:35:47 | mriedem | but for the libvirt driver, you should only be able to get into this weird scenario if the physical hostname changes | |
| 15:35:51 | mriedem | for the compute host | |
| 15:36:12 | tssurya | yea understood | |
| 15:37:29 | mriedem | we probably want/need https://review.openstack.org/#/c/545479/ in stable branches | |
| 15:37:35 | belmoreira | wouldn't the logic be simpler if compute_nodes aren't recreated and only have the "host" updated | |
| 15:38:04 | mriedem | belmoreira: that's what that _check_for_nodes_rebalance method tries to do | |
| 15:38:27 | mriedem | if it finds a compute node with the same nodename (ironic node uuid), update the host on the existing compute node record | |