| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-11-21 | |||
| 17:40:29 | dansmith | artom_: I'll let mriedem make the call on what the name should be | |
| 17:40:44 | artom | Ugh, it'll be some obscure 80s rock band | |
| 17:41:01 | mriedem | i agree it's confusing that _GroupAntiAffinityFilter is a policy filter but _GroupAffinityFilter isn't | |
| 17:41:01 | dansmith | fine by me :P | |
| 17:41:36 | sdague | efried: looking | |
| 17:41:49 | mriedem | i mean, we could just call the damn thing RUN_FOR_REBUILD but that doesn't help a ton with the reasoning behind which filters should be run during rebuild or not | |
| 17:42:14 | dansmith | and I expect we need to re-use much of the logic for some other things where we need to check with the scheduler | |
| 17:42:40 | mriedem | example? | |
| 17:42:41 | dansmith | although I guess resize does need to check the resourcey things | |
| 17:42:44 | dansmith | I dunno | |
| 17:42:51 | dansmith | feels far too targeted to just say it's for resize | |
| 17:42:52 | mriedem | i think rebuild is just the odd duck | |
| 17:43:12 | dansmith | well, if that's really the case, then maybe we should just be specific until we have a counterexample | |
| 17:43:12 | mriedem | resize runs through the scheduler because it's going to do a claim on the chosen host | |
| 17:43:15 | dansmith | yeah | |
| 17:43:25 | mriedem | live migration doesn't do a claim | |
| 17:43:40 | mriedem | but live migration does allocate on the dest host | |
| 17:43:42 | mriedem | using the same flavor | |
| 17:43:42 | sdague | efried: so, on https://review.openstack.org/#/c/490057/27/nova/api/openstack/compute/servers.py, while it's fine to pass context around, did you look into just pulling it from thread local storage? | |
| 17:44:05 | sdague | efried: https://github.com/openstack/oslo.context/blob/18aa6ec496e54b2403c2cc65234ef6447021fdee/oslo_context/context.py#L490-L495 | |
| 17:44:07 | efried | sdague I didn't. | |
| 17:44:38 | efried | sdague tbh, mordred did the code-side work on that patch. I just did the test, and kept it up to date. | |
| 17:44:51 | sdague | ok, the long pass through of some of these variables gets a little spidery. It's probably fine here, but something to think about in the future | |
| 17:45:17 | mriedem | that's why i worried about something doing context.get_admin_context() and that eventually going through here | |
| 17:45:21 | mriedem | but apparently we don't do that with images | |
| 17:45:28 | efried | sdague Yeah, that would have made things a lot easier. Is that the same thing? | |
| 17:49:00 | sdague | mriedem: yeh, I could see that | |
| 17:49:20 | sdague | it's one of those things where context was written originally to not have to pass it around | |
| 17:49:29 | sdague | but that got forgotten at some point | |
| 17:49:38 | sdague | so, meh | |
| 17:50:05 | sdague | efried: +A on that patch, I'll look at the rest of the conversions on top once I get some lunch | |
| 17:50:24 | efried | sdague Thank you sir. | |
| 17:50:44 | efried | sdague (mriedem) Note that the cinder one might still be busted. But now that this one is going through, I'll get back to looking at that. | |
| 17:52:11 | mriedem | efried: it does remind me that someone had a patch to add some code for nova to auth with a service user to do things with cinder like we do for neutron | |
| 17:52:18 | mriedem | i think in one of the periodic tasks | |
| 17:52:34 | efried | mriedem Yeah, I couldn't remember if that was niraj_singh, but I don't think so. | |
| 17:52:59 | mriedem | efried: https://review.openstack.org/#/c/384799/ | |
| 17:53:07 | efried | mriedem Oh, actually I was thinking of someone who was trying to use service auth from *within* cinder. | |
| 17:53:29 | artom | dansmith, mriedem, I think we need to get stephenfin to look at the PCI and NUMA bits when he comes back tomorrow | |
| 17:53:50 | dansmith | okay I'm off tomorrow and mriedem is too I think | |
| 17:53:55 | mriedem | pci is extra specs only | |
| 17:54:01 | mriedem | i asked stephen about that yesterday | |
| 17:54:05 | mriedem | numa is definitely a resource thing | |
| 17:54:11 | mriedem | we do claims on numa limits in the resource tracker | |
| 17:54:13 | artom | mriedem, can an admin change them though? | |
| 17:54:50 | mriedem | an admin could change the pci passthrough settings in flavor extra specs, sure | |
| 17:54:58 | artom | For NUMA the cpu and thread policy can change, I dunno if that can give rise to the situation where the host can no longer accept them | |
| 17:55:22 | mriedem | changing extra specs on an existing flavor would be kind of a bad move i think | |
| 17:55:31 | mriedem | but we allow it | |
| 17:55:33 | dansmith | the flavor embedded in the instance will have the pci stuff then, which means we won't see the changed extra_specs from the top-level flavor in the rebuild | |
| 17:56:12 | dansmith | the cpu and thread policy change in the image meta might be a reason to do the rebuild though, | |
| 17:56:18 | dansmith | so I dunno what to say about that | |
| 17:57:54 | artom | dansmith, oh right, that's true, flavours stay with instances now | |
| 17:58:21 | artom | So anything involving flavor extra specs isn't policy then - ie, no need to check with it on rebuild, it hasn't changed for the actual instance | |
| 17:58:22 | dansmith | well, no | |
| 17:58:31 | dansmith | flavors stay with the instance, flavours are not a thing in nova :P | |
| 17:58:51 | artom | What do you mean not a thing? | |
| 17:59:02 | artom | Oh | |
| 17:59:18 | dansmith | artom: yeah, that's probably a good guideline, although some flavor extra_specs get overridden by image meta in some weird ways I think | |
| 18:00:17 | artom | Ugh. | |
| 18:00:27 | mriedem | NUMATopologyFilter gets values from flavors first, and if not set, checks the image meta | |
| 18:00:34 | artom | Just set everything to policy and wait for the bug reports then >:( | |
| 18:00:35 | mriedem | so flavor extra specs take precedence there | |
| 18:00:52 | dansmith | artom: that's the way it is now, and the bug we're fixing :P | |
| 18:00:54 | mriedem | oh wait nvm that | |
| 18:01:49 | mriedem | re the question about the az filter | |
| 18:01:50 | mriedem | https://review.openstack.org/#/c/446446/ | |
| 18:02:24 | mriedem | "Unfortunately, as an operator | |
| 18:02:24 | mriedem | can modify any metadata, it is possible to change the value of the | |
| 18:02:24 | mriedem | availability_zone key for a specific aggregate and accordingly change what the | |
| 18:02:24 | mriedem | user can see when looking at AZs" | |
| 18:05:35 | openstackgerrit | Merged openstack/nova master: vgpu: add enabled white list https://review.openstack.org/512580 | |
| 18:10:07 | mriedem | we persist the flavor with the request spec yes? | |
| 18:10:41 | dansmith | flavor id you mean? | |
| 18:10:46 | mriedem | the full flavor | |
| 18:11:01 | mriedem | looks like yes | |
| 18:11:03 | dansmith | hrm, I guess we do | |
| 18:11:14 | mriedem | which means, re the TrustedFilter question, | |
| 18:11:46 | mriedem | if the admin changes the flavor extra specs for the trusted filter after you've created an instance, when you rebuild or migrate that instance, the filter is going to work on the original flavor used to create the instance | |
| 18:11:48 | mriedem | not the modified one | |
| 18:12:02 | mriedem | *i think* | |
| 18:12:46 | artom | That's what I understood when dansmith said that flavoUrs stay with the instance, yeah | |
| 18:17:16 | mriedem | my favorite burstlet flavour is tripe | |
| 18:17:55 | mriedem | artom: dansmith: ok, some replies inline https://review.openstack.org/#/c/521186/ | |
| 18:18:48 | mriedem | dansmith: btw, i changed _GroupAntiAffinityFilter to False, ran functional tests and nothing failed | |
| 18:19:12 | dansmith | mriedem: it'll be non-deterministic | |
| 18:19:20 | mriedem | gha | |
| 18:19:22 | mriedem | *gah | |
| 18:19:25 | dansmith | because if it selects the other host by chance it'll be happy | |
| 18:19:49 | dansmith | it was oddly repeatable for me.. two tests validating the same thing (for two microversions) one would always pass the other would always fail | |
| 18:19:53 | dansmith | took me a while to figure out why | |
| 18:20:13 | dansmith | I was able to shake it up with different levels of concurrency and running the tests in isolation | |
| 18:20:30 | mriedem | test_rebuild_with_soft_anti_affinity ? | |
| 18:20:48 | mriedem | or test_rebuild_with_anti_affinity | |
| 18:20:58 | dansmith | soft I think | |
| 18:21:25 | mriedem | that one involves a weigher so yeah might be that one | |
| 18:21:27 | dansmith | I can try to repro if it's important | |
| 18:21:28 | mriedem | will run it a few times | |
| 18:21:58 | openstackgerrit | Eric Fried proposed openstack/nova master: Use ksa adapter for cinder client https://review.openstack.org/508345 | |
| 18:22:13 | mriedem | i am dangerously close to needing some deterministic lunch | |