| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-06-13 | |||
| 17:49:57 | melwitt | jangutter: I think that review is the highest I've seen | |
| 17:50:44 | openstackgerrit | Merged openstack/nova master: api-ref: expand on various bdm parameters https://review.openstack.org/574805 | |
| 17:52:53 | dansmith | mgagne: that's reading deleted during a lazy load | |
| 17:53:16 | dansmith | mgagne: the healer is actually reading deleted initially | |
| 17:54:04 | dansmith | I guess it might need to read deleted instances to remove stale allocations maybe/ | |
| 17:54:08 | dansmith | regardless, that path should be reading deleted during flavor load I would think | |
| 17:54:34 | dansmith | ah, it's not | |
| 17:55:23 | dansmith | and, | |
| 17:55:30 | dansmith | it should be pre-loading flavor it looks like | |
| 18:03:04 | dansmith | mgagne: any chance you have an old nova python module on the system, and are just running nova-manage from newer source? | |
| 18:04:22 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: Allocation.consumer field https://review.openstack.org/565405 | |
| 18:04:23 | openstackgerrit | Jay Pipes proposed openstack/nova master: rework allocation handler _allocations_dict() https://review.openstack.org/565407 | |
| 18:04:24 | openstackgerrit | Jay Pipes proposed openstack/nova master: Add a microversion for consumer generation support https://review.openstack.org/565604 | |
| 18:04:25 | openstackgerrit | Jay Pipes proposed openstack/nova master: Fix nits from change Id609789ef6b4a4c745550cde80dd49cabe03869a https://review.openstack.org/574324 | |
| 18:11:14 | mriedem | jangutter: the multiattach one | |
| 18:12:44 | mriedem | oh maybe not, i guess it was just really old | |
| 18:12:46 | mriedem | https://review.openstack.org/#/c/271047/ | |
| 18:13:44 | mriedem | heh would be a tie with https://review.openstack.org/#/c/408964/114 | |
| 18:14:54 | mriedem | mgagne: hmm, you're running that against queens? | |
| 18:15:39 | melwitt | bpoulos: quick (hopefully) question for you here https://review.openstack.org/#/c/486204/114/nova/compute/api.py@3137 | |
| 18:16:10 | dansmith | mriedem: so on mgagne's issue, I don't know why we're not preloading flavor, but I also think it's wrong to fail lazy-loading it, so: | |
| 18:16:12 | openstackgerrit | Dan Smith proposed openstack/nova master: Always read-deleted=yes on lazy-load https://review.openstack.org/575190 | |
| 18:16:26 | dansmith | biab | |
| 18:16:38 | mriedem | dansmith: we are pre-loading flavor | |
| 18:16:46 | mriedem | _heal_instances_in_cell | |
| 18:16:48 | mriedem | oops | |
| 18:16:52 | mriedem | limit=max_count, expected_attrs=['flavor']) | |
| 18:16:52 | mriedem | ctxt, filters={}, sort_key='created_at', sort_dir='asc', | |
| 18:16:52 | mriedem | instances = objects.InstanceList.get_by_filters( | |
| 18:18:29 | efried | jangutter: https://review.openstack.org/#/c/362766/ is coming up fast | |
| 18:20:38 | jangutter | efried: now THAT is impressive. Dating from 2016 | |
| 18:21:24 | mriedem | psh https://review.openstack.org/#/c/271047/ | |
| 18:21:29 | mriedem | has a good 7 months on that | |
| 18:23:38 | openstackgerrit | Zack Cornelius proposed openstack/nova master: Implement file backed memory for instances in libvirt https://review.openstack.org/567876 | |
| 18:24:52 | bpoulos | melwitt: I'll take a look | |
| 18:25:10 | melwitt | thanks | |
| 18:25:29 | melwitt | bpoulos: I think we figured it out but would be good to get your ack | |
| 18:26:14 | mriedem | melwitt: replied to you inline | |
| 18:28:11 | mgagne | dansmith, mriedem: ok back. there is only one version of Nova installed, Queens. | |
| 18:28:20 | jaypipes | melwitt: more pug love. https://twitter.com/jaypipes/status/1006966218157445120 | |
| 18:29:14 | mgagne | so I'm wondering why preloading isn't happening and fails with lazy-loading later. | |
| 18:29:25 | mriedem | mgagne: me too, looking now | |
| 18:30:27 | mriedem | mgagne: i think i know | |
| 18:30:31 | mgagne | it's failing with an instance from 2014. So I'm wondering is something didn't get migrated (even if it's deleted) or someone messed up the database at that time. | |
| 18:30:38 | mriedem | we don't go through the normal model_query which takes into account the read_deleted value on the context | |
| 18:30:59 | mriedem | mgagne: are you able to tell if the instance it's actually looking at really is deleted? | |
| 18:31:08 | mgagne | deleted=id atm | |
| 18:31:16 | mriedem | mgagne: ok yeah it's a bug in the filter code | |
| 18:31:28 | mgagne | ok, should I had deleted=true somewhere? | |
| 18:31:44 | mriedem | see https://github.com/openstack/nova/blob/5266ac27e78ae2a6fdb4f15e51a222a7c782e59f/nova/db/sqlalchemy/api.py#L2015 | |
| 18:31:49 | mriedem | "Deleted instances will be returned by default, unless there's a filter that says otherwise." | |
| 18:31:59 | mriedem | mgagne: it's just a bug in the heal_allocations stuff - can you report a bug? | |
| 18:32:17 | mgagne | so filters={'deleted': False} ? | |
| 18:32:26 | mriedem | yup | |
| 18:32:36 | mgagne | ok, will test before reporting | |
| 18:33:17 | mriedem | https://github.com/openstack/nova/blob/80dacb2610c16d0f044c30a8ef342971ffa75aab/nova/cmd/manage.py#L1793 and here https://github.com/openstack/nova/blob/80dacb2610c16d0f044c30a8ef342971ffa75aab/nova/cmd/manage.py#L1855 | |
| 18:33:34 | mgagne | yes, testing now | |
| 18:34:24 | mriedem | melwitt: bpoulos: i dropped the +2; i think for the rebuild validation logic wrt volume-backed, we likely want to move the "if 'trusted_certs' in kwargs" check above that so we can just simply determine (1) will this instance have trusted certs, either because they changed them, set them or are resetting to defaults and (2) if it's volume-backed, fail | |
| 18:34:39 | mriedem | bpoulos: i'm happy to make that change quick if you're ok with it | |
| 18:34:47 | mgagne | ok, it fails elsewhere but I think it's related to the fact I only have one compute node in my test setup and it's hitting an instance where host!=node01 | |
| 18:35:35 | mriedem | hmm, https://github.com/openstack/nova/blob/80dacb2610c16d0f044c30a8ef342971ffa75aab/nova/cmd/manage.py#L1755 ? | |
| 18:35:59 | bpoulos | mriedem: that change sounds good to me, thanks! | |
| 18:36:13 | mriedem | melwitt: you agree with that logic? | |
| 18:41:05 | mgagne | mriedem: yes, my setup is just limited in the number of computes I have. (currently only have node01) Therefore I can't populate placement for other nodes because I can't start the nova-compute service once on those, I just don't have the nodes. | |
| 18:41:27 | mgagne | mriedem: I guess I need to find a way to heal the allocations for a specific compute instead. | |
| 18:42:52 | mriedem | mgagne: i'm not quite sure i understand, you've got 1 nova-compute service running to service multiple ironic nodes, right? that is totally fine - and actually, as noted earlier, ironic computes should already be posting allocations for their instances anyway | |
| 18:43:13 | mriedem | ironic instances will be 1:1 with compute_nodes records | |
| 18:43:19 | mgagne | mriedem: we figured out that we don't need to heal Ironic because it auto heals. I'm currently testing libvirt. | |
| 18:43:23 | dansmith | mriedem: I know you're asking for it, but it clearly wasn't preloaded otherwise it'd not be lazy loading | |
| 18:43:51 | mgagne | mriedem: we have 100+ nodes in prod but I can't model as much nodes in my dev setup. I only have node01. | |
| 18:44:16 | mriedem | mgagne: ok so you're saying for your other libvirt nodes, they haven't reported resource providers into placement yet b/c they haven't started | |
| 18:44:22 | mgagne | so I only have to make sure I test the healing with a specific node instead. | |
| 18:44:27 | mgagne | yes | |
| 18:44:33 | mriedem | ok that makes more sense | |
| 18:44:45 | mgagne | just talking out loud atm =) | |
| 18:44:55 | mriedem | PUT allocations fails b/c the specified resource provider wasn't found by id | |
| 18:44:57 | mriedem | np | |
| 18:44:58 | mriedem | i get it | |
| 18:45:16 | mgagne | yes | |
| 18:45:17 | mgagne | exactly | |
| 18:45:18 | mgagne | Failed to create allocations for instance XXX against resource provider YYY. | |
| 18:45:59 | mriedem | ok but you got past that lazy-load issue so i can post a fix for that | |
| 18:46:07 | mgagne | oh yes | |
| 18:46:26 | mgagne | just wanted to make sure I didn't hit a similar bug later with a non-deleted instance for reasons | |
| 18:48:44 | mriedem | joining all my coworker friends in the kitchen | |
| 18:49:00 | dansmith | mriedem: the cats? | |
| 18:49:06 | mgagne | are they pugs like jaypipes ones? | |
| 18:49:41 | mriedem | there is the one cat | |
| 18:50:14 | mgagne | https://bugs.launchpad.net/nova/+bug/1776743 | |
| 18:50:15 | mriedem | left those fish assholes back in the office | |
| 18:50:15 | openstack | Launchpad bug 1776743 in OpenStack Compute (nova) "Allocation healer should ignore deleted instances" [Undecided,New] | |
| 18:50:24 | mriedem | thanks | |
| 18:50:44 | mgagne | should I propose a fix or will you take care of it? | |
| 18:52:53 | jaypipes | mgagne: you mean, basically, "cat-like dogs"? :) | |
| 18:53:15 | mriedem | mgagne: you can if you want, i'm going to be updating this other change first - i think we'd also want a functional test wrinkle for that since unit tests aren't really going to cut it | |
| 18:53:17 | mgagne | jaypipes: I'm not a pug expert unfortunately so I can't tell. ¯\_(ツ)_/¯ | |
| 18:53:31 | mgagne | mriedem: ouch =( | |
| 18:53:33 | mriedem | there are existing functional tests for heal_allocatoins | |
| 18:53:41 | mriedem | so adding one that just creates a server and then deletes it should be easy enough | |
| 18:53:51 | mriedem | create server w/o allocs, delete it, run heal_allocations | |