| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-08-09 | |||
| 14:17:09 | mriedem | i guess we don't need to wait for translations https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:zanata/translations | |
| 14:20:20 | melwitt | yeah, I was thinking it seems like openstack doesn't do translations anymore but I didn't know how to check. I'll add that link to the release checklist wiki | |
| 14:20:34 | melwitt | I know we don't translate log messages | |
| 14:20:49 | melwitt | but other user-facing message, still translate? I wasn't sure | |
| 14:21:15 | efried | afaik we're still supposed to _() for exception messages. | |
| 14:21:37 | lyarwood | mdbooth: remind me again where the compute code was that deleted and recreated an attachment? | |
| 14:21:44 | melwitt | aye, I have seen that | |
| 14:22:57 | mdbooth | lyarwood: _terminate_volume_connections | |
| 14:24:17 | lyarwood | mdbooth: urgh was looking at remove_volume_connection | |
| 14:25:01 | mdbooth | lyarwood: IIRC it was triggering a bug in the cinder fixture, which assumed only 1 attachment | |
| 14:25:13 | mdbooth | But with this we've briefly got 2 attachments | |
| 14:26:30 | mdbooth | lyarwood: I don't love the raw-only fix, tbh, because I think it increases the test and maintenance burden. If that code needs to live on I'd prefer to bring it together somehow. | |
| 14:27:39 | mdbooth | I'll abandon the stat thing, though, because as you point out it's not a solution | |
| 14:28:36 | lyarwood | mdbooth: kk, well it improves performance for the raw images user that reported the issue in the short term until we start using disk.info to store the virtual size | |
| 14:29:00 | lyarwood | mdbooth: and given that means we also need to refactor code out of imagebackend I'd rather land something simple first then focus on that | |
| 14:30:28 | mdbooth | I don't think it's a refactor, btw. Just code motion really iirc. | |
| 14:30:50 | mdbooth | Would just be moving it elsewhere to make it easier to call. | |
| 14:31:23 | mriedem | GET /kashyap returns me a 404 | |
| 14:31:29 | mriedem | is his nick not registered? | |
| 14:31:41 | mdbooth | mriedem: Yep. He's out for a few more days yet, I think | |
| 14:31:46 | mriedem | blarg | |
| 14:31:51 | mriedem | wanted him to read the "nova-compute choosing incorrect qemu binary when scheduling 'alternate' (ppc64, armv7l) architectures?" thread in the ML | |
| 14:32:15 | mriedem | wondering if there is a good reason why we don't set guest.arch in the libvirt domain xml based on the hw_architecture image property | |
| 14:32:22 | mriedem | maybe someone could ask danpb? | |
| 14:33:05 | mdbooth | stephenfin_: might have an opinion | |
| 14:33:57 | mdbooth | mriedem: Is it tagged [nova]? | |
| 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 | sean-k-mooney | mriedem: oh ye were talking about setting the arch in libvirtxml based on hw_architecture | |
| 14:44:23 | lyarwood | pretty much | |
| 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 | the API call, i.e "binding:host_id": "mymitkaComputehost", | |
| 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: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 | mriedem | i think that was only ever used by the ironic driver | |
| 15:07:09 | hvvcben | yes, | |
| 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 | |