| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-18 | |||
| 15:06:34 | mriedem | i thought it was compute and triggered a retry | |
| 15:06:50 | johnthetubaguy | yeah, its on the compute, ironic find the node is already in use by another instance | |
| 15:06:57 | mriedem | so spawn fails? | |
| 15:07:01 | johnthetubaguy | yeah | |
| 15:07:16 | johnthetubaguy | I should double check the logs on that | |
| 15:07:21 | stephenfin | jaypipes: Thank you sir. Much appreciated, as always | |
| 15:07:51 | jaypipes | stephenfin: no problem. or however one says no problem in Gaelic. | |
| 15:09:05 | mriedem | ? | |
| 15:09:05 | mriedem | bauzas: you were +2 on https://review.openstack.org/#/c/361140/ before so are you still good | |
| 15:09:23 | efried | An dtugann stephenfin Gaeilge? I was kind of guessing not... | |
| 15:10:15 | mriedem | johnthetubaguy: not sure i like the spawn fail / refresh thing, | |
| 15:10:28 | mriedem | trying to think of how we can set a flag for instances that have migrated but need their allocations updated by the RT | |
| 15:10:55 | mriedem | including instances that have had their flavors migrated already | |
| 15:11:06 | dansmith | the problem being we do the migration async right? | |
| 15:11:24 | dansmith | we come up and before we can do all of them we might get a boot request? | |
| 15:11:25 | mriedem | ah crap didn't think about that either | |
| 15:11:46 | dansmith | we could disable ourselves until the migration is complete | |
| 15:12:22 | mriedem | the async migration also means that if the RT asked the driver for any instances to forcefully update allocations, they might not be set yet, but the next pass or the periodic would hit them | |
| 15:12:32 | mriedem | i was thinking the driver is shoving stuff onto a work queue, | |
| 15:12:36 | mriedem | the RT pulls off that queue | |
| 15:12:50 | mriedem | once all instances are processed the queue remains empty | |
| 15:13:09 | johnthetubaguy | get_inventory, could we do the migration on that call, and return something if the allocation needs refreshing? | |
| 15:13:16 | dansmith | if we disable ourselves we can still do management operations of existing instances, and the scheduler won't send us new stuff until we're ready | |
| 15:13:32 | dansmith | johnthetubaguy: that's kinda icky | |
| 15:13:39 | dansmith | johnthetubaguy: I mean, really icky | |
| 15:13:50 | dansmith | johnthetubaguy: gross even | |
| 15:14:04 | johnthetubaguy | worse than virt driver calling to placement? | |
| 15:14:15 | dansmith | also recall that people can do this before anything starts back up during an upgrade | |
| 15:14:52 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Transform instance.exists notification https://review.openstack.org/403660 | |
| 15:14:53 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add sample test for instance audit https://review.openstack.org/480955 | |
| 15:15:04 | dansmith | johnthetubaguy: it's just overloading get_inventory() as a general hook to do potentially a ton of db maintenance | |
| 15:15:11 | johnthetubaguy | so the alternative is dropping the resources:VCPU=0 in the new flavor | |
| 15:15:13 | dansmith | johnthetubaguy: it'd be better to just provide a proper hook I think | |
| 15:15:58 | johnthetubaguy | dansmith: I am agreeing its horrid, just thinking relative horrid here really | |
| 15:16:10 | dansmith | johnthetubaguy: it's relatively horrid yes :) | |
| 15:16:21 | dansmith | johnthetubaguy: you don't like the self disable? | |
| 15:16:37 | dansmith | if we do it in get_inventory we still have a race right? | |
| 15:16:41 | dansmith | with the scheduler | |
| 15:16:52 | johnthetubaguy | so its a race with when the flavor gets updated I guess | |
| 15:17:45 | mriedem | i'm not sure at which point you auto-disable | |
| 15:17:57 | dansmith | mriedem: on init_host(), if there are unmigrated instances | |
| 15:18:15 | dansmith | and then re-enable when you're done with the migration | |
| 15:18:44 | mriedem | so originally when we did the flavor migration it was on init_host and then that moved because of the hash ring stuff | |
| 15:18:44 | johnthetubaguy | but if the resource class isn't set on the node, we are never done | |
| 15:19:05 | dansmith | johnthetubaguy: that had to be set before queens, right? | |
| 15:19:09 | mriedem | yeah i'm trying to think if we re-enable too soon, or disable too long | |
| 15:19:29 | dansmith | so queens starting up can assume it's set no? | |
| 15:19:35 | johnthetubaguy | dansmith: yes, but this is really about making pike work | |
| 15:20:05 | johnthetubaguy | so right know I can't update the flavors in pike, for them to be set in time for queens, because of this bug | |
| 15:20:09 | dansmith | oh, was this migration in pike? I'm misremembering the timing | |
| 15:20:23 | johnthetubaguy | yeah, sadly | |
| 15:20:39 | mriedem | yeah we have to backport the fix | |
| 15:21:05 | dansmith | johnthetubaguy: but you can set the node class before pike, and you can run the migration before you start anything else up right/ | |
| 15:21:07 | mriedem | so i'm still thinking queue and hook callback from RT to driver | |
| 15:21:51 | dansmith | mriedem: explain your queue thing again? | |
| 15:22:18 | mriedem | so before https://github.com/openstack/nova/blob/e11a8687aef527eee9f7c733db6cccd5b902afbb/nova/virt/ironic/driver.py#L523 we're going to have to see if the instance already has an allocation for the normalized_rc, | |
| 15:22:22 | cdent | does anyone have a reference to previous discussion/decisions on why we don’t do regular allocation updates? (something I can read, rather than taking us off track here) | |
| 15:22:28 | mriedem | if it doesn't, we have to put the instance/rc into a queue, | |
| 15:22:52 | mriedem | when the RT periodic runs, it calls into the driver to say, give me any instances that need their allocations updated and we'd dequeue at that point | |
| 15:23:38 | mriedem | then ^ should auto-heal during the periodic as the admin is setting node rcs in ironic | |
| 15:23:40 | mriedem | while things are running | |
| 15:24:27 | mriedem | cdent: https://review.openstack.org/#/c/491012/ | |
| 15:24:43 | cdent | thanks | |
| 15:24:50 | dansmith | mriedem: you mean the first time we run the RT periodic we end up migrating all the instances the driver needs before we update inventory? | |
| 15:25:18 | mriedem | cdent: in a nutshell, in pike we want the scheduler to create allocations, including doubling them up for moves and shared providers (before we stopped trying to make shared providers work in pike) - the problem is the periodic task in ocata computes will overwrite the allocations created by the scheduler | |
| 15:25:39 | mriedem | dansmith: need to see if the RT updates inventory before allocations | |
| 15:26:14 | mriedem | it doesn't | |
| 15:26:15 | mriedem | shit | |
| 15:26:24 | mriedem | the RT would update allocations before inventory | |
| 15:26:35 | dansmith | even still, we run most of that code at other times than just the perioidic | |
| 15:27:47 | mriedem | the allocatoin update does happen during instance_claim | |
| 15:27:55 | dansmith | mriedem: so we still do allocation updates if we have ocata computes.. what if we keep doing them if we have ocata nodes and the driver says we need to? | |
| 15:28:06 | dansmith | s/and/or | |
| 15:28:37 | mriedem | and the driver says we need to only for ironic and only until we remove the flavor migration code | |
| 15:28:39 | sean-k-mooney | stephenfin: gladly. ill review it in the next 20 mins or so. i was pretty happy with the previous version so i dont expect that ill see anything wrong with it | |
| 15:28:59 | dansmith | mriedem: yeah | |
| 15:29:02 | mriedem | dansmith: that would be simpler | |
| 15:29:02 | johnthetubaguy | ... now that I like | |
| 15:29:21 | johnthetubaguy | will code that up now | |
| 15:29:33 | dansmith | the commit message would need to be "Extend existing shitpile with more shit because.. why not" | |
| 15:29:53 | mriedem | you can't migrate ironic instances anyway right | |
| 15:29:54 | mriedem | ? | |
| 15:30:05 | mriedem | so the ocata compute allocation overwrite thing is less of a concern there | |
| 15:30:07 | johnthetubaguy | mriedem: there is a spec on that ;) | |
| 15:30:13 | dansmith | no migrations at all? | |
| 15:30:16 | dansmith | I thought evac worked at least | |
| 15:30:16 | mriedem | johnthetubaguy: sure, but not in pike | |
| 15:30:24 | mriedem | evac is the only one i can think of | |
| 15:30:27 | mriedem | but, | |
| 15:30:28 | johnthetubaguy | oh, rebuild does I guess | |
| 15:30:35 | mriedem | you don't care about the allocations on the dead source host | |
| 15:30:45 | mriedem | although these are nodes, not hosts | |
| 15:31:16 | mriedem | hmm, so how does evac work for ironic - the nodes might not be down, but the nova-compute service is, | |
| 15:31:29 | mriedem | so you evacuate and move all of those instances to other ironic nodes managed by another compute host? | |
| 15:32:07 | mriedem | well regardless, | |
| 15:32:12 | mriedem | because of gibi's fixes, | |
| 15:32:26 | mriedem | when/if the source compute comes back up, we remove the allocations from the old instances that were on it | |
| 15:33:01 | mriedem | so yeah the callback to the driver to ask if it should update allocatoins is probably ok for ironic, | |
| 15:33:03 | mriedem | simpler, | |
| 15:33:06 | mriedem | lesser of all evils, | |