| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-02-12 | |||
| 14:57:09 | bauzas | also | |
| 14:57:09 | gibi | copying image_id over from the volume meta to the request_spec is fine by me | |
| 14:57:11 | mriedem | but requestspec.image.id == None....? | |
| 14:57:55 | mriedem | bauzas: if you rebuild a volume-backed instance with a new image, we fail today | |
| 14:58:09 | jroll | cdent: thanks | |
| 14:58:23 | mriedem | bauzas: https://github.com/openstack/nova/blob/master/nova/compute/api.py#L2979 | |
| 14:59:24 | bauzas | mriedem: okay, gtk | |
| 15:01:18 | takashin | mriedem: Would you check my reply to you comment in https://review.openstack.org/#/c/513721/ ? | |
| 15:04:11 | mriedem | takashin: done | |
| 15:05:18 | takashin | mriedem: Thanks. | |
| 15:08:59 | mriedem | bauzas: https://bugs.launchpad.net/nova/+bug/1748858 | |
| 15:09:00 | openstack | Launchpad bug 1748858 in OpenStack Compute (nova) "allocation_ratio set by aggregate metadata don't work in NUMATopologyFilter" [Undecided,New] - Assigned to yangjie (yang.jie) | |
| 15:09:07 | mriedem | sounds familiar | |
| 15:09:30 | bauzas | mriedem: sec, thinking hard of https://review.openstack.org/#/c/543263/2/nova/scheduler/filters/isolated_hosts_filter.py | |
| 15:09:44 | bauzas | the change isn't trivial | |
| 15:10:04 | mriedem | bauzas: really? | |
| 15:10:40 | mriedem | bauzas: that's basically what it did before it was regressed here https://review.openstack.org/#/c/202678/35/nova/scheduler/filters/isolated_hosts_filter.py | |
| 15:10:41 | bauzas | mriedem: yeah, my guts feel some problem that I still don't see | |
| 15:10:52 | mriedem | maybe you need to go to the bathroom | |
| 15:11:07 | bauzas | probably | |
| 15:11:13 | bauzas | but I'm looking at https://review.openstack.org/#/c/543263/2/nova/scheduler/filters/isolated_hosts_filter.py@43 | |
| 15:11:16 | gibi | bauzas: shall we pull the patch from the gate as I +Wd it recently | |
| 15:11:42 | mriedem | if you look at how it worked before https://review.openstack.org/#/c/202678/35/nova/scheduler/filters/isolated_hosts_filter.py | |
| 15:11:43 | bauzas | gibi: I'm not saying we *have* a problem | |
| 15:11:50 | mriedem | the image id was always something it handled as optional | |
| 15:11:53 | mriedem | using dict.get() | |
| 15:11:59 | bauzas | the thing is | |
| 15:12:01 | mriedem | changing to request spec in that patch made it required | |
| 15:12:06 | bauzas | if we say we don't have an image | |
| 15:12:17 | bauzas | then the image isn't isolated, right | |
| 15:12:28 | mriedem | bauzas: that's the point of my email, | |
| 15:12:39 | bauzas | in that case, we should accept host1 | |
| 15:12:40 | mriedem | which is, this never did anything for volume-backed instances, | |
| 15:12:44 | mriedem | but that's not the point of my fix right now | |
| 15:12:56 | mriedem | i'm restoring this filter to how it previously worked, before mitaka | |
| 15:13:01 | bauzas | then the doc is invalid | |
| 15:13:09 | bauzas | at least the docstring I mean | |
| 15:13:16 | bauzas | and I'd like to fix that | |
| 15:13:19 | mriedem | so clean that up in a follow up? | |
| 15:13:45 | bauzas | well now that gibi pulled the trigger, /me shrufs | |
| 15:13:46 | cdent | Is there a general opinion about the merit of conditional imports in nova code? I'd like to only import rpc and sqlalchemy_api here https://github.com/openstack/nova/blob/master/nova/config.py#L54-L58 if those conditionals are true. The other option (which might make better long term sense) is to make a different parse_args | |
| 15:13:54 | bauzas | shrugs* | |
| 15:14:13 | bauzas | mriedem: the thing is, I thought you wanna backport ? | |
| 15:14:17 | bauzas | moar fun then | |
| 15:14:20 | mriedem | bauzas: i do | |
| 15:14:35 | bauzas | so, we should backport both your change *and* the doc change | |
| 15:14:37 | mriedem | i don't think the docstring is something that bars us from backporting this fix | |
| 15:14:38 | bauzas | looks sad | |
| 15:14:43 | bauzas | I know | |
| 15:15:04 | mriedem | bauzas: if you want to call out the volume-backed scenario, then push a patch on top of this https://review.openstack.org/#/c/543264/ | |
| 15:15:06 | bauzas | I just would have appreciated if we could have amended the upstream filters documentation *and* the docstring in the sme change | |
| 15:15:35 | mriedem | i didn't say anything about volume-backed behavior in https://review.openstack.org/#/c/543264/ because, as i said in the ML, i'm not sure if that behavior is intentional or not | |
| 15:15:44 | mriedem | since the filter predates my involvement in nova | |
| 15:15:55 | bauzas | mriedem: I don't really care of the BFV case | |
| 15:16:34 | bauzas | the thing is, should we assume that an instance having no image is behaving identical as if we were having an instance with a non-isolated image ? | |
| 15:16:38 | bauzas | my guts feel yues | |
| 15:16:46 | bauzas | hence my trouble | |
| 15:16:47 | mriedem | fwiw essex https://github.com/openstack/nova/commit/972fc7d80fae386cf4c72b7891f6601d6f7fb00b#diff-b4fa3e44aeb7757f04d296529b8492e5 | |
| 15:16:58 | mriedem | bauzas: well, that's the point of my ML thread | |
| 15:17:03 | bauzas | again, I know | |
| 15:17:07 | mriedem | and why i also sent it to the ops ML | |
| 15:17:17 | bauzas | but people pulled the trigger before I had time to both review and reply | |
| 15:17:26 | bauzas | that's fine, it's my bad | |
| 15:17:29 | mriedem | i wanted to find out (1) does anyone use this filter (2) if so, do they know about this bfv thing, and (3) if so, do they care? | |
| 15:17:32 | bauzas | I need to be quicker | |
| 15:19:15 | gibi | I feel there is two different change on that is now on the gate to restore the regressed behavior and one that is on the ML to find out the what is the intended behavior for BFV | |
| 15:19:16 | bauzas | mriedem: like I said in the meeting, if people use that filter, I'd be surprised | |
| 15:19:36 | bauzas | gibi: after thinking hard, I think we can leave the patch merge | |
| 15:19:49 | bauzas | mriedem: I'd be surprised because it's conf-driven | |
| 15:19:54 | bauzas | which operators hate | |
| 15:20:06 | edleafe | cdent: you'd need to import rpc for line 44 :) | |
| 15:20:19 | bauzas | while ImageAggragateExtraSpecsMyStuffFilter does that programatically with aggregates :) | |
| 15:20:22 | cdent | edleafe: would isolate that too | |
| 15:20:42 | cdent | edleafe: but just going to write another one, as there's redundancy happening in that stuff that placement doesn't care about | |
| 15:21:06 | cdent | edleafe: still curious about the general question (even though I'm not going to do it) | |
| 15:21:16 | cdent | some people hate them | |
| 15:21:21 | edleafe | cdent: yeah, just funnin' ya | |
| 15:22:00 | edleafe | FWIW, I tend to not use them, except when performance is affected. | |
| 15:23:12 | mriedem | melwitt: looks like functional test failures in https://review.openstack.org/#/c/340614/ are real | |
| 15:26:06 | cdent | stephenfin: wait, DPDK isn't solving _all_ your problems. unpossible. | |
| 15:26:23 | mriedem | alex_xu: have fun, eat lots of dumplings | |
| 15:26:28 | bauzas | stephenfin: DPDK like Don't Play Donkey Kong ? | |
| 15:27:12 | alex_xu | mriedem: thanks :) | |
| 15:27:29 | bauzas | mriedem: what does the follow-up button on the Gerrit UI if I'm clicking it on your top change ? | |
| 15:27:42 | mriedem | bauzas: no idea | |
| 15:27:43 | bauzas | mriedem: I'd like to just amend the doc to explain about BFV instances | |
| 15:28:06 | mriedem | https://stackoverflow.com/questions/45621912/what-is-the-gerrit-follow-up-button-doing | |
| 15:28:15 | stephenfin | cdent: It's not even solving one of them yet. I probably have it configured incorrectly...again | |
| 15:28:33 | bauzas | mriedem: and the fact that if you have an image that is isolated, but then you create a volume from that image and then boot from that volume, Nova won't guarantee your host will be excluded | |
| 15:28:50 | stephenfin | bauzas: Don't be dissing the Kong now, ya hear ;) | |
| 15:29:15 | mriedem | bauzas: well, it will depending on the other config options | |
| 15:29:21 | bauzas | mriedem: exactly | |
| 15:29:47 | bauzas | mriedem: but consider the case where I have an image that is restricted to host1 | |
| 15:29:50 | mriedem | i'd say, if you care, use AggregateImagePropertiesIsolation | |
| 15:30:13 | mriedem | actually, | |
| 15:30:20 | mriedem | AggregateImagePropertiesIsolation likely doesn't work for volume-backed instances properly either | |
| 15:30:30 | bauzas | mriedem: if I'm making a volume from that image and then booting from it, then the instance will be exclused from that host if strict_host_blah=True | |
| 15:31:09 | mriedem | for a volume-backed instance, the only image properties we store in the request spec are the ones from the volume_image_metadata https://github.com/openstack/nova/blob/master/nova/utils.py#L1033 | |
| 15:31:35 | mriedem | and i don't know if that actually stores the properties from the backing image, and even if it does, they could be stale | |
| 15:32:16 | mriedem | so yeah, any filter that relies on image properties is likely semi-broken with boot from volume | |