| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-06-13 | |||
| 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 | instances = objects.InstanceList.get_by_filters( | |
| 18:16:52 | mriedem | ctxt, filters={}, sort_key='created_at', sort_dir='asc', | |
| 18:16:52 | mriedem | limit=max_count, expected_attrs=['flavor']) | |
| 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 | openstack | Launchpad bug 1776743 in OpenStack Compute (nova) "Allocation healer should ignore deleted instances" [Undecided,New] | |
| 18:50:15 | mriedem | left those fish assholes back in the office | |
| 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 | |
| 18:54:28 | mgagne | ok, I might not work on it right away, still trying to test the actual healer first | |
| 18:56:42 | zcorneli | dansmith: Updated service_version here: https://review.openstack.org/#/c/567876/14/nova/objects/service.py , Want to see that pass the tests before I post the doc changes (just to make sure the rebase is fully passing) | |
| 18:56:53 | dansmith | zcorneli: I had already done that for you this morning | |
| 18:57:12 | dansmith | zcorneli: because I was trying to get a good run on the devstack change, which I guess we need to recheck now | |
| 18:57:49 | zcorneli | Your patch had failed in Zuul, added the service version to the log, but didn't increment the constant | |
| 18:58:07 | dansmith | zcorneli: https://review.openstack.org/#/c/574792 | |
| 18:58:08 | dansmith | zcorneli: ah, okay sorry, hadn't seen, thought maybe you didn't notice | |
| 18:58:16 | dansmith | zcorneli: I think we probably also need to set allocation_ratio=1.0 in that devstack patch now that I think of it | |
| 18:58:43 | zcorneli | dansmith: Yea, without setting ram_allocation_ratio=1.0, it should fail to start once file_backed_memory is enabled. | |
| 18:58:51 | dansmith | yeah | |
| 19:00:40 | dansmith | tssurya: mriedem melwitt: cells meeting today? I don't think I have anything to discuss | |
| 19:01:51 | tssurya | dansmith: me neither, just working on mriedem's improvisations regarding host states in our env, will provide input on it when we have something solid | |
| 19:02:26 | dansmith | cool | |
| 19:03:44 | mriedem | dansmith: get_minimum_version_all_cells doesn't cache does it | |
| 19:04:03 | mriedem | so on multi-create, we query that for each instance in some cases | |
| 19:04:17 | dansmith | mriedem: doesn't cache what? | |
| 19:04:38 | melwitt | dansmith: +1 on a skip | |