Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-04-24
16:56:26 jroll dansmith: melwitt: oh, right, that docstring tells us exactly that
16:56:54 dansmith melwitt: you can do host binding before spawn, that should be fine, you just can't do the plug before it
16:57:15 melwitt k
16:57:31 jroll melwitt: so, the problem we're seeing is when scheduling races with two instances to a node, they both try to do the plug because we haven't set that reservation. so I'm thinking if we do that reservation first thing, then we never hit this again
16:58:02 melwitt so it sounds like we have two options: decouple the host binding and do that in prepare_networks_before_block_device_mapping, get the IP for the volume attach, then reserve, then plug vifs etc
16:58:17 dansmith jroll: why are two things racing to the same node?
16:58:17 melwitt or, add a way to do the reserve first thing
16:58:24 dansmith jroll: scheduler should have prevented that already
16:58:37 melwitt dansmith: the reserve isn't done until spawn
16:58:44 dansmith melwitt: doesn't matter
16:58:47 jroll dansmith: not a clue
16:58:51 melwitt hm
16:58:55 TheJulia and there can be multiple consumers of ironic's api
16:59:01 dansmith melwitt: we have committed the allocation in scheduler, which means the node isn't a candidate for any other scheduler runs
16:59:13 TheJulia so we can't rely upon nova's scheduler to be the source of truth on locking
16:59:18 dansmith TheJulia: is that it? nova is racing with something else?
16:59:37 melwitt but is that what's happening in the environment you're seeing the problem? multiple consumers of ironic API?
16:59:44 TheJulia dansmith: nova can race with something else, nova can also race if resource classes are not used, which is how we first noticed the issue actually
16:59:47 jroll oh my
17:00:15 dansmith well, "if resource classes are not used" should not be a thing
17:00:24 TheJulia agreed, totally agreed
17:00:27 dansmith but if it's the former, then that's fine
17:00:39 jroll dansmith: I wasn't aware the allocation in scheduler landed, my bad
17:01:10 dansmith jroll: yeah, so nova racing with something else is a thing, I guess, but makes it less concerning to me
17:02:06 melwitt yeah, I mean really I feel like we could just change this to something more generic, "pre-spawn-prep" kind of method and use that to do a node reserve https://review.openstack.org/#/c/468353/19/nova/virt/driver.py
17:02:20 TheJulia jroll: there are some discussions in the hpc/htc world of central ironic, multiple consumers
17:02:27 jroll dansmith: but anyway, I thought there was still an opportunity for a scheduler race, glad to here that's gone
17:02:31 dansmith melwitt: gotta make sure you have a post-spawn-failure method to unreserve too
17:02:38 TheJulia like, ironic ends up being a central broker of baremetal across datacenters *blink* *blink*
17:02:41 dansmith jroll: not if you're doing the resource class thang
17:03:20 melwitt dansmith: ah, yeah
17:03:20 jroll TheJulia: I'm aware people do crazy things with ironic, like using it to manage VMs, I'm not saying it doesn't happen. I'm just saying I'm not sure we should support nova being used with ironic without owning it :)
17:03:42 jroll dansmith: sweet.
17:03:59 TheJulia dansmith: I think we already rip the instance_uuid in the cleanup
17:04:04 TheJulia rip it off the node that is
17:04:18 openstackgerrit Ed Leafe proposed openstack/nova master: Address issues raised in adding member_of to GET /a-c https://review.openstack.org/554357
17:04:18 dansmith TheJulia: not if we fail before we called spawn I imagine
17:04:18 TheJulia so we should be covered if there is a failure
17:04:33 jroll dansmith: ++
17:04:44 dansmith TheJulia: I'm saying if we do a pre-spawn method that could have allocated resources, and then we fail to talk to glance or neutron, we have to make sure we let the virt driver know we were joking
17:04:46 TheJulia dansmith: that is a good point... I thought it still got called but maybe not
17:04:53 TheJulia oh jeeze, that is another way we could be orphaning vifs
17:06:22 jroll yeah, it does not, only if we fail spawn(). whomp whomp
17:06:43 jroll TheJulia: this ensures we don't orphan vifs: https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2198
17:06:43 TheJulia :(
17:07:22 melwitt TheJulia, jroll: so what would y'all rather do? pre-spawn and have to deal with failure cleanup cases or do something to allow the host binding to be done separately from plug_vifs? or another solution I'm not thinking of?
17:07:50 TheJulia jroll: if InstanceDeployFailure is added to https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2192 then that would prevent the case I'm seeing in a couple different cases
17:08:33 jroll melwitt: I don't think we can split the host-binding from the plug_vifs, really. I think either way we have to do a pre-spawn, this started with talking about a new method vs hacking it into plug_vifs :)
17:09:03 melwitt okay
17:09:03 TheJulia what jroll said :)
17:09:16 jroll TheJulia: that only comes from spawn(), right?
17:09:33 jroll maybe not :/
17:09:56 efried mriedem (or anyone): Are you aware of an recent changes in nova or elsewhere that would make instance.uuid stop responding to isinstance(str)?
17:10:17 melwitt yeah, I guess duh, the methods that were added have a cleanup with it already too https://review.openstack.org/#/c/468353/19/nova/virt/driver.py
17:10:20 TheJulia jroll: I just spotted https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2214
17:11:11 mriedem efried: nope
17:11:16 efried k
17:11:17 jroll efried: uuid.uuid4() or whatever will yield some object rather than a string, we tend to use uuidutils.generate<tab>, could be it
17:11:30 jroll TheJulia: ah, yeah
17:11:49 dansmith jroll: instance.uuid will coerce it to a string thouigh
17:11:57 jroll ah ok
17:12:17 efried yeah, see, that's what seems not to be happening anymore in a certain code path for us.
17:15:38 openstackgerrit melanie witt proposed openstack/nova stable/queens: DNM: testing the ceph job https://review.openstack.org/563175
17:16:02 gibi Kevin_Zheng: I have an approved bp that could be interesting for you if you have time: https://blueprints.launchpad.net/nova/+spec/add-full-traceback-to-error-notifications
17:17:11 gibi Kevin_Zheng: it is totally OK if you don't have time either, I just wanted advertise the opportunity
17:20:01 jroll melwitt: dansmith: jaypipes: thanks for the help, btw :)
17:20:21 openstackgerrit Merged openstack/nova master: Add periodic task to clean expired console tokens https://review.openstack.org/325381
17:20:40 jaypipes jroll: I feel like I did nothing but add confusion to the situation, honestly.
17:21:04 jroll jaypipes: thanks for trying to help? :P
17:21:05 dansmith jroll: jaypipes needs a hug
17:21:12 jroll or a pug
17:21:20 melwitt pug hug
17:21:26 dansmith or a pug hug
17:22:23 TheJulia Pug hugs sound far better than cat cuddles... and walking away bleeding
17:24:23 jroll jaypipes: do aussies also cheer you up? https://i.imgur.com/Yswxtpy.jpg
17:24:32 melwitt heh. not our cat. we think he might be part ragdoll
17:24:34 dansmith jroll: I think he only likes ugly dogs
17:24:42 jroll ha, fair
17:25:05 jaypipes jroll: ++ aussie's are a good cheerup.
17:25:11 jroll :)
17:25:18 melwitt jroll: that's adorbz
17:25:26 dansmith jaypipes: but. but.. their eyeballs are fully contained within their heads...
17:25:53 jroll melwitt: thanks, make sure you pass it along to james :)
17:25:59 TheJulia It is side conversations like ^^^ why I love this community
17:26:27 melwitt jroll: forwarding it now
17:27:03 jaypipes dansmith: details, details...
17:27:20 jaypipes dansmith: they are furry, fluffy and perfect.
17:27:31 jaypipes dansmith: even if they are missing googly eyes.
17:27:45 jaypipes heh
17:32:51 TheJulia jroll: Any thoughts on continuing your poc forward or just rewrite mine?
17:33:23 jroll TheJulia: I could do it, or you could take it over, or you could rewrite yours, whatever you'd like to do :)
17:34:56 TheJulia I've got it on the brain so I'll pickup yours and continue it along
17:36:23 jroll awesome, thanks! happy to review if needed
17:36:43 jroll TheJulia: fyi I probably picked up your bug from patchset 1 where you needed the patch wrapped in []
17:37:27 TheJulia my brain is currently trying to think of any other ways we could be leaking vifs and thus blocking future deploys outside of tearing down an instance and I'm not thinking of any
17:37:39 openstackgerrit Jay Pipes proposed openstack/os-traits master: Add compute capabilities traits https://review.openstack.org/546713
17:37:58 TheJulia jroll: easy fix :)
17:38:11 jaypipes artom_, gibi, efried: addressed the rewording in https://review.openstack.org/#/c/546713/.
17:38:14 melwitt mriedem: why do you think we can't drop the API code until T? I thought once all computes are upgraded to rocky, then things are guaranteed to all be using the database backend, so removal would be in Stein? https://review.openstack.org/#/c/325414/31/nova/compute/api.py@3613
17:38:25 jroll TheJulia: yep, just a heads up so you don't wait an hour for CI to find it :)

Earlier   Later