| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-02-12 | |||
| 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 | |
| 15:32:44 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: for doc clarity https://review.openstack.org/543494 | |
| 15:33:44 | openstackgerrit | Chris Dent proposed openstack/nova master: Isolate config parse_args for placement https://review.openstack.org/543495 | |
| 15:35:46 | AJaeger | mriedem, the novaclient reviews I mentioned earlier are: review https://review.openstack.org/543115 , https://review.openstack.org/#/c/543114, https://review.openstack.org/#/c/541370/ - the first two are needed to fix the jobs, the third is needed for Zuul v3. | |
| 15:37:59 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: doc: BFV instances and IsolatedHostsFilter https://review.openstack.org/543494 | |
| 15:38:16 | bauzas | mriedem: just made a first try for doc ^ | |
| 15:40:41 | AJaeger | thanks, mriedem | |
| 15:41:26 | bauzas | stephenfin: sahid: do you have any idea why the NUMATopologyFilter needs to set some limits for CPU and RAM ? https://github.com/openstack/nova/blob/master/nova/scheduler/filters/numa_topology_filter.py#L92-L93 | |
| 15:41:33 | bauzas | context is https://bugs.launchpad.net/nova/+bug/1748858 | |
| 15:41:34 | 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:42:13 | bauzas | stephenfin: sahid: the NUMATopologyFilter should only care in my opinion to enforce the numa limits, not any other resource usage | |
| 15:42:34 | stephenfin | bauzas: Not off the top of my head, no. That whole 'NUMATopologyLimits' object is a weird one I've yet to grasp properly. Hopefully sahid knows more | |
| 15:49:19 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix typo in NUMATopologyFilter docs https://review.openstack.org/543505 | |
| 15:56:17 | mriedem | bauzas: fwiw, the image properties are stored on the volume's volume_image_metadata dict: http://paste.openstack.org/show/670139/ | |
| 15:56:27 | mriedem | i add foo=bar to the image i used to create that bootable volume | |
| 15:56:52 | mriedem | so AggregateImagePropertiesIsolation would only be as good as how accurate that metadata is | |
| 15:56:58 | bauzas | mriedem: are you persuing me to look up the volume if BFV ? | |
| 15:57:05 | mriedem | since nova doesn't re-fetch the image to get the latest meta | |
| 15:57:17 | mriedem | persuing you how? | |
| 15:57:43 | bauzas | the fact is that filters don't lookup the volume image metadata, right? | |
| 15:57:49 | bauzas | they just assume every instance has an image | |
| 15:58:10 | bauzas | well, they rather assume every RequestSpec has an image | |
| 15:58:18 | mriedem | the filter assumes the request spec image has the details from when the api created the request spec | |
| 15:58:22 | mriedem | yes that | |
| 15:58:48 | mriedem | and the request spec for a volume-backed instance will have the image meta stored in the volume from the time the volume was created | |
| 15:59:10 | bauzas | mriedem: if we create a fake image based on that, right? | |
| 15:59:13 | mriedem | but my point is, i could create a volume with some image, then change properties on that image, and then create a server from the volume and the image meta in the volume would be stale | |
| 15:59:21 | bauzas | I agre | |