| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-27 | |||
| 15:03:47 | efried | bauzas, dansmith: Should we be cutting an RC2 just for a doc? | |
| 15:04:02 | bauzas | efried: well, maybe not | |
| 15:04:04 | efried | I don't think there's anything else we would be including | |
| 15:04:06 | dansmith | no | |
| 15:04:15 | efried | sean-k-mooney's lower-constraints fix is on the list | |
| 15:04:16 | bauzas | we should be just merging it now for master | |
| 15:04:22 | bauzas | then I'd backport it to stein | |
| 15:04:26 | efried | ++ | |
| 15:04:32 | dansmith | after ga | |
| 15:04:37 | bauzas | but it could be not needing a RC2 | |
| 15:04:40 | efried | ...but the lower-constraints thing has been busted for a while. | |
| 15:04:47 | bauzas | so +W the stein backport after GA | |
| 15:04:57 | bauzas | unless we find other bugs | |
| 15:05:06 | bauzas | and then we could merge it with the other fixes | |
| 15:05:16 | bauzas | but yeah, not needing a RC2 just for it | |
| 15:15:28 | trident | cdent: Are you around to answer a couple of questions about the implementation of forbidden traits? | |
| 15:16:23 | cdent | trident: I am around, either here or in #openstack-placement. Not promising I'll be able to help | |
| 15:17:20 | efried | trident: Is this about https://review.openstack.org/647957 ? | |
| 15:19:01 | trident | cdent: I am working on an issue that prevents forbidden traits in flavors from being acted upon: https://review.openstack.org/#/c/647957/ While working on some tests I found that traits of the trait:CUSTOM_FOO=forbidden in image properties doesn't seem to work either. Is that expected or should they work both for flavors and images? | |
| 15:19:10 | trident | efried: Yup. | |
| 15:19:17 | efried | gibi: if you're around, you may want to join in ^ | |
| 15:20:24 | cdent | trident: is the issue that the wrong request is built and sent to placement, or that placement is behaving incorrectly with a valid request? | |
| 15:20:32 | efried | Mm, I don't remember whether we have implemented support for traits (positive or negative) coming from image props yet. | |
| 15:20:33 | gibi | efried: did we ever said that forbidden trait support is a thing in nova? in placement I know that it is a thing | |
| 15:20:38 | sean-k-mooney | efried: bauzas well my lower constraitn fix is not complete | |
| 15:21:02 | gibi | efried: similarly placement had support for nested a_c for a while but nova need a lot of adaptation to use it | |
| 15:21:05 | trident | efried: The wrong request is sent to placement. | |
| 15:21:24 | efried | sean-k-mooney: I suspect that would be a clusterf*** to backport in any case, so we may want to consider just fixing it in master and moving on. | |
| 15:21:41 | trident | This is the commit implementing it: https://github.com/openstack/nova/commit/2c51688558504f2e8ce80bac06642772be67b2a9 | |
| 15:22:29 | efried | trident: Yup, that looks like it's pulling from flavor extra_specs; do you have any evidence that we did same for image props? | |
| 15:22:40 | trident | Sorry, cdent: The wrong request is sent to placement. | |
| 15:22:51 | sean-k-mooney | efried: ya. the main thing i need to figure out is where the mocking of rabbitmq is borked so that we use the fake message bus when running the unit tests in all cases | |
| 15:23:45 | efried | sean-k-mooney: I'll ask you about it in tomorrow's meeting, but I'm expecting we won't want to try to put it in stein RC2 even if you do get it all fixed up by then. | |
| 15:24:02 | cdent | trident: that patch is only doing it for flavor extra specs | |
| 15:24:17 | gibi | trident: the linked bp https://blueprints.launchpad.net/nova/+spec/forbidden-traits-in-nova explicitly talks about flavor extra_spec so I think there was no intention to support it from image metadata | |
| 15:24:32 | sean-k-mooney | efried: ya that is fair. we can consider backporting it when i have the final fix. | |
| 15:24:50 | efried | trident: It is something we've discussed doing, for sure, but I don't think we've implemented it yet. | |
| 15:24:54 | sean-k-mooney | efried: actully one thing i might try is fixing it on the stabel branches and see if they pass | |
| 15:24:56 | trident | efried, cdent: I am a little bit of a newbie when it comes to the code base, that's why I am asking. But my understanding from following the code path is that the logic for getting forbidden traits from image props is not there. Just wanted to verify that before putting more time into it. | |
| 15:25:08 | cdent | yes, it's not there | |
| 15:25:31 | efried | We do support positive traits from image props, nah? | |
| 15:25:42 | sean-k-mooney | efried: we do | |
| 15:25:46 | trident | efried: Yes. | |
| 15:25:53 | efried | wherezat code? | |
| 15:26:13 | cdent | it's a thing that ought to exist, I think it just didn't happen yet. Perhaps because we couldn't decide precedence? I can't remember | |
| 15:26:22 | sean-k-mooney | in the schduler uitls i think one sec | |
| 15:26:32 | trident | efried: https://github.com/openstack/nova/blob/771e102654b54d257f437f16be2aeeef51fa0783/nova/scheduler/utils.py#L186 | |
| 15:27:25 | sean-k-mooney | yep ^ which is called by resources_from_request_spec | |
| 15:28:36 | sean-k-mooney | and i think but i woudl have to confim when we build the request spec we grab the image properties form a voluem in the bfv case so it should also work in that case too | |
| 15:29:52 | sean-k-mooney | there may however be the same issue. the traits are part of the unnumbered request group which is matched agaist the compute node RP but if we did nto requerst any resouces form it that would not work correctly | |
| 15:30:32 | efried | sean-k-mooney: Yup, same issue we've been thrashing in mriedem's multiattach capability patch. | |
| 15:30:58 | gibi | sean-k-mooney: we don't have to solve the empty unnumbered group issue yet. That will happen when NUMA is modelled in placement. And it requires a lot of other changes | |
| 15:31:04 | efried | I think we decided that *right now* we always request resources in the unnumbered group for the root RP. | |
| 15:31:10 | efried | so <sweep under rug> | |
| 15:31:15 | efried | <fingers in ears> | |
| 15:31:16 | sean-k-mooney | gibi: ya that true | |
| 15:31:19 | efried | <problem for future us> | |
| 15:31:24 | trident | So supporting forbidden traits in image props as well seems like a clear case of something that would make sense to implement. But out of scope of what I am working on with this bug. (had it just been a similar bug as with forbidden traits in flavors I might as well just have fixed it while I was at it). | |
| 15:31:51 | sean-k-mooney | efried: easy fix. all compute node have a inventory of instnace which match the max instance per host value that in the nova.conf | |
| 15:32:12 | sean-k-mooney | and all vms in the futre requerst one allocation of an instance resouce class | |
| 15:32:15 | efried | trident: Right. I agree it's something that makes sense to implement, though we would probably want a clear use case / demand before prioritizing the work. | |
| 15:32:31 | sean-k-mooney | efried: we could allso kill one of the filter if we did that | |
| 15:33:06 | sean-k-mooney | specifically we could kill this filter https://github.com/openstack/nova/blob/master/nova/scheduler/filters/num_instances_filter.py | |
| 15:33:09 | efried | sean-k-mooney: that would require us to tack on that "resource" to every request | |
| 15:33:19 | sean-k-mooney | efried: yes | |
| 15:33:22 | efried | because we can't expect it to be in flavors. | |
| 15:33:32 | sean-k-mooney | but i think that would be fine | |
| 15:33:42 | sean-k-mooney | it requires not change to placement | |
| 15:33:55 | sean-k-mooney | and this is really a nova specific issue with a nova specific fix | |
| 15:33:57 | efried | trident: So given that it's *not* supported currently, do you actually have any other way of introducing a forbidden trait that doesn't get picked up by the scheduler? | |
| 15:34:13 | efried | trident: but still conforms to the rules ("you can't have a request group with no resources")? | |
| 15:35:07 | efried | sean-k-mooney: Isn't there a way to set max instances per host to unlimited? | |
| 15:35:55 | sean-k-mooney | yes but i think max int is close enough | |
| 15:36:13 | gibi | famous last words :) | |
| 15:36:26 | trident | efried: Yes. My initial issue and what the bug report / review is about is when introducing it from flavors. So, that still stands. | |
| 15:36:44 | efried | sean-k-mooney: Could work. Seems like a pretty heavy solution, not sure if it's worth it. But we could talk about it at the PTG if you're into it. | |
| 15:36:46 | sean-k-mooney | efried: if you think there may be some merrit in it i can write up a short spec proposing how it would work | |
| 15:36:46 | trident | efried: And that is supposed to be supported. | |
| 15:37:16 | efried | trident: Okay, so the very best thing you could do is what gibi suggested: come up with a functional test case that demonstrates it being broken. | |
| 15:37:23 | sean-k-mooney | we could proably make it opt in via a config and a prefilter. | |
| 15:37:35 | efried | sean-k-mooney: I have no opinion. It seems like a messy solution to something that's not really a problem. | |
| 15:37:42 | efried | (I guess that's an opinion) | |
| 15:38:05 | efried | sean-k-mooney: I just think we have more important things to focus on for train. | |
| 15:38:20 | efried | it would be nice if we had a future/wishlist bucket we could put such things into... | |
| 15:38:21 | sean-k-mooney | sure it was just a taught | |
| 15:38:38 | trident | efried: That, I have done. Looking at unit tests at the moment as well. | |
| 15:39:05 | efried | trident: Sweet. We'll look for that patch. Thanks for working on this! | |
| 15:39:10 | sean-k-mooney | i might just write it up as a way to migrate eh numInstacnefilter to placemetn and we can decide to do it if/when it makes sense | |
| 15:39:17 | efried | sean-k-mooney: ++ | |
| 15:40:59 | gibi | trident: hit me with the patch adding the functional test and I can review it quickly | |
| 15:42:58 | trident | I have two questions related to the functional tests and the review flow. First, Looking at the test on https://github.com/openstack/nova/blob/771e102654b54d257f437f16be2aeeef51fa0783/nova/tests/functional/test_servers.py#L4524 , wouldn't that one potentially pass around 50% of the times even if the required flavor didn't work? When it ends up on the expected node out of chance? | |
| 15:44:31 | gibi | trident: looking | |
| 15:45:14 | sean-k-mooney | trident yes and no | |
| 15:45:22 | sean-k-mooney | the use of VMX is unfortuete | |
| 15:45:36 | sean-k-mooney | the upsteam ci does not provide nested vert | |
| 15:45:54 | gibi | sean-k-mooney: this is functional test so no virt layer is involved | |
| 15:46:06 | sean-k-mooney | ya just realised that | |
| 15:46:14 | sean-k-mooney | as i was typeing | |
| 15:46:15 | efried | trident: The point of that test is that the flavor (not the image) contains the required trait | |
| 15:46:26 | gibi | trident: I think this test can pass half of the time | |