Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-08-09
14:34:23 openstackgerrit Lee Yarwood proposed openstack/nova master: Add regression test for bug#1784353 https://review.openstack.org/587014
14:34:24 openstackgerrit Lee Yarwood proposed openstack/nova master: WIP compute: Terminate volume connections during _shutdown_instance https://review.openstack.org/590348
14:34:37 lyarwood mdbooth: ^ that's the terminate_volume_connections alternative btw, without unit test changes
14:34:53 mdbooth lyarwood: That was fast! Looking.
14:35:49 mdbooth mriedem: Do you have an opinion on ^^^, btw?
14:36:51 mdbooth mriedem: Basically leaves us with a blank attachment when calling _shutdown_instance
14:36:58 lyarwood hmm that removes the call to detach with cinderv2
14:37:05 lyarwood why would we do that during shutdown?
14:38:35 mdbooth lyarwood: remind me what v2 detach() does
14:38:52 mriedem mdbooth: seems fun at first since it's very much the same code,
14:38:55 mriedem but see inline comments
14:39:13 mdbooth terminate_connection() causes the storage backend to kill the connection
14:39:25 mriedem mdbooth: v2 detach is just the os-detach volume action which changes the volume status to 'available'
14:39:31 mriedem does nothing on the volume backend
14:39:33 mdbooth mriedem: Got it.
14:40:32 mriedem so if you did this,
14:41:20 mriedem you'd have to have volume attachment cleanup code in both compute manager if we don't reschedule (max_attempts=1 or force_hosts/nodes is set) or if we do reschedule and conductor build_instances hits MaxRetriesExceeded
14:41:43 mriedem which is essentially what we have for cleaning up ports
14:41:49 sean-k-mooney mdbooth: stephenfin_ is on vaction for the next week and a half just fyi
14:42:04 mriedem i assume that was meant for me
14:42:28 mdbooth mriedem: See, over here in communist Europe everybody takes vacation in August
14:42:40 sean-k-mooney mriedem: well both you and mdbooth since he suggested stephenfin_ would have an oppion on something
14:42:49 sean-k-mooney i was still scrolling back to see what
14:43:39 lyarwood mriedem: kk thanks, so this doesn't really simplify the fix at all
14:43:50 mriedem lyarwood: not really
14:44:01 mriedem reschedules are a minefield
14:44:12 mriedem mdbooth: is that what bauzas is doing as well?
14:44:18 mriedem france gets august off right?
14:44:23 lyarwood pretty much
14:44:23 sean-k-mooney mriedem: oh ye were talking about setting the arch in libvirtxml based on hw_architecture
14:44:24 mriedem for wine and cheese and love making
14:44:25 mdbooth mriedem: Yep
14:44:35 melwitt efried: haha, I am literally looking at that review already right now
14:45:09 efried melwitt: Cool. I wanted to make sure it got attention from someone who knows how to spell "quota" (which ain't me).
14:45:45 dansmith mriedem: question in here: https://review.openstack.org/#/c/590062/1
14:45:54 dansmith I know it's a backport, but want to make sure I understand at least
14:46:09 melwitt efried: yeah, makes sense. I'm conflicted about the sentence they're proposing because it's not true that it's not possible to count keypairs. it's just, for legacy reasons (before pike) it always returned zero so I kept the behavior with the quota work in pike
14:46:14 mdbooth mriedem lyarwood: I think that approach is fundamentally good. We do need to think about the attachment cleanup after the last reschedule failure, but if we're not doing that then we've always been leaking there.
14:46:49 efried melwitt: Ah, okay, then perhaps it should just say, "For legacy reasons, this value is always zero. We'll fix it in a future microversion. Maybe. If you're lucky."
14:47:12 mdbooth That is, we can leak there right now, because it's possible to schedule to a compute and have it fail before touching volumes, so the 'reservation' still exists after failure.
14:47:17 mdbooth ^^^ * 3 == leak
14:47:20 melwitt efried: haha, yeah.
14:47:24 mdbooth Unless we already handle it
14:48:56 mriedem mdbooth: what i'd be most comfortable with is if we suspect we leak today, that we add a functional regression test for that which does reschedules with a volume attached, and asserts at the end of the reschedules when we get novalidhost (for max retries exceeded) that we've cleaned up all attachments to the volume
14:49:08 mriedem mdbooth: because this is too hairy to go on based on review alone
14:49:27 mriedem *functional test (not really a regression if it's always leaked)
14:49:46 mriedem that may have been fixed recently though
14:49:53 mriedem after about 7 years of being broken
14:50:31 mdbooth Got it. I did a bit of an audit here, btw: https://review.openstack.org/#/c/587071/9/nova/tests/unit/conductor/test_conductor.py@1004
14:51:00 mriedem I8b1c05317734e14ea73dc868941351bb31210bf0
14:52:12 mriedem yeah so we'll call _cleanup_volumes which will detach if we abort the build
14:52:14 mriedem but not if we reschedule
14:52:23 mriedem and conductor doesn't do any volume cleanup on MaxRetriesExceeded
14:52:29 mriedem so that's probably a separate bug,
14:52:46 mriedem and would benefit from a functional test since it involves more than a single service
14:52:57 mriedem (really 3 - conductor and 2 computes for the reschedule)
14:53:21 mriedem we might already have a func test that does reschedules with a volume attached
14:54:46 mriedem i don't see one though
14:54:48 mriedem but shouldn't be hard to write
14:58:50 lyarwood mdbooth: https://review.openstack.org/#/c/587014/ does that
14:59:28 mdbooth lyarwood: Thought I'd seen it recently :)
15:00:14 hvvcben Hi - probably a newb question and if this isn't the correct channel please advise. - I am trying to rework an old neutron ML2 driver into Queens and having issues with Nova-compute during port creation. Because of no bind_host_id = $nodeID during instance create. The nova-compute api call to neutron to create port it doesn't have the bind_host_id set in the api call. On older version like Mitaka, that parameter is in
15:00:14 hvvcben the API call, i.e "binding:host_id": "mymitkaComputehost",
15:00:25 hvvcben any help or links to doc pertaining to these changes would be greatly appreciated
15:03:20 mriedem melwitt: is your link to irc in https://review.openstack.org/#/c/589972/ wrong?
15:04:35 melwitt oh, yeah it is now because I used "latest". derp. I think I've done that a few times lately
15:05:59 melwitt added a new comment with the right link
15:06:18 hvvcben mitka api nova-compute api to neutron = "binding:host_id": "mymitkaComputehost", Queens nova-compute api call to neutron more like "binding:host_id": "",
15:06:31 mriedem hvvcben: i see bind_host_id in the neutronv2/api.py code in queens
15:06:43 mriedem are you saying bind_host_id isn't being passed down from the compute manager to allocate_for_instance?
15:07:09 hvvcben yes,
15:07:09 mriedem i think that was only ever used by the ironic driver
15:08:04 mriedem it's still used in queens https://github.com/openstack/nova/blob/stable/queens/nova/compute/manager.py#L1390
15:08:06 hvvcben I am just having trouble figuring out why it does it in mitaka and not in later version, I was thinking the port creation process has been modified and that value would come later in the process
15:08:10 mriedem but as i said, that would only ever have a value for ironic
15:09:44 hvvcben this particular driver does interact with hardware and in its present state fails if no Host_id is passed
15:10:02 hvvcben hardware meaning switch hardware
15:11:17 mriedem the only difference i see when setting binding:host_id between mitaka and queens is that in mitaka we only set that if the neutron port binding extension was available, and we stopped looking for that sometime later and just assumed it would be available
15:11:21 hvvcben i was just curious, since i have default install of mitaka and it passes it(using openvswitch as driver) and the queens version doesn't was there some point where that was changed? Thats what I am having trouble finding. I thought it may relate to livemigration
15:11:43 mriedem you said you're trying to create an instance, not live migrate it, right?
15:12:35 mriedem there is nothing immediately obviously different between mitaka and queens in how binding:host_id is handled,
15:12:35 hvvcben yes yes, but I thought their were some rework done on port creation that affected nova and neutron dealing with port creation as a whole in effort to smooth out live migrations
15:12:38 mriedem so you're going to have to debug
15:12:46 mriedem that's in rocky
15:12:48 hvvcben yes been trying
15:13:00 mriedem maybe you mean the migrating_to stuff?
15:13:02 mriedem for dvr
15:13:14 mriedem if you're not live migrating, you wouldn't hit any of that so shouldn't be a problem
15:14:10 hvvcben gotcha. has the port creation process changed significantly from mitaka to queens?
15:14:45 hvvcben as far as what nova do etc?
15:16:40 mriedem you're talking about like a 2 year window of dev here :)
15:16:52 mriedem i'm not aware of anything significant changing in that flow in that time though, no
15:17:04 hvvcben : ) I know i know
15:17:15 mriedem are you sure you're not using now-invalid config in queens?
15:17:28 mriedem like, we could have deprecated some config options in mitaka/newton and they are gone by the time you get to queens
15:17:52 hvvcben ... appreciate i will dig further.. I just mainly need to find a way to get the nova-compute host_id and pass it to neutron in a way during create_port_precommit
15:18:55 mriedem which virt driver are you using?
15:18:56 mriedem libvirt?
15:18:57 mriedem https://github.com/openstack/nova/blob/mitaka-eol/nova/virt/driver.py#L1587
15:19:01 hvvcben probably all of the above... -- the driver was designed to work with Mitaka and not maintained their have been quite a bit of changes in neutron since then obviously(in a good way)

Earlier   Later