| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-02-05 | |||
| 14:36:49 | efried | which we don't want. | |
| 14:36:54 | bauzas | efried: then, if so, you won't get other hosts | |
| 14:37:19 | efried | If by "other hosts" you mean flat non-NUMA hosts, that's correct, and that's what we want. | |
| 14:37:21 | bauzas | the ones that don't have this trait, ie. the ones that weren't modified | |
| 14:37:28 | bauzas | right | |
| 14:37:30 | bauzas | but, | |
| 14:37:37 | bauzas | see the problem | |
| 14:37:43 | efried | I don't | |
| 14:37:45 | efried | not yet | |
| 14:37:48 | bauzas | example | |
| 14:37:51 | bauzas | : | |
| 14:38:14 | bauzas | I'm an operator and I don't configure any host for asking a NUMA topology | |
| 14:38:26 | bauzas | so, none of my hosts have this trait | |
| 14:38:34 | bauzas | now, I have existing flavors | |
| 14:38:45 | bauzas | but I upgrade to Ussuri | |
| 14:39:01 | bauzas | and then the scheduler now transforms the Placement call to ask for this trait | |
| 14:39:10 | bauzas | then I will get NoValidHosts | |
| 14:39:12 | stephenfin | dansmith: replied (you read it correctly, yeah) | |
| 14:40:01 | bauzas | efried: I don't see a way to say : ask for this trait for all NUMA hosts, but don't ask for it for the others | |
| 14:41:19 | efried | bauzas: We talked about this yesterday a bit. You're correct: if you upgrade but don't switch your hosts to NUMA-aware, you won't be able to land NUMA flavors. That's as designed. | |
| 14:41:24 | dansmith | stephenfin: can you read my reply just now in that case? | |
| 14:42:22 | sean-k-mooney | efried: i need to sumerise the options for the config option in the spec form the irc conversation and other i had. ill try and do that in about an hour | |
| 14:42:37 | sean-k-mooney | i need to run to the bank now | |
| 14:42:57 | stephenfin | dansmith: I think what's there is correct | |
| 14:43:28 | stephenfin | line 2678 is going to retrieve the list of ports, which will look like this https://docs.openstack.org/api-ref/network/v2/?expanded=list-ports-detail#list-ports | |
| 14:44:22 | bauzas | efried: I'm unclear sorry | |
| 14:44:41 | sean-k-mooney | bauzas: efried if we go with the bool config option one of the out standing question is the default for that value. long term it should default to true. not sure if we should default to true in u or v however | |
| 14:44:42 | efried | bauzas: one way or another, you're going to have to decide which hosts are going to NUMA and which are going to be flat. | |
| 14:44:46 | sean-k-mooney | any brb | |
| 14:44:48 | dansmith | stephenfin: I was going on the blob you put in the comment, but maybe that's wrong? I don't see a nested port_details in he api ref like you have in your blob | |
| 14:44:59 | stephenfin | dansmith: Oh, there isn't one | |
| 14:45:08 | stephenfin | This is something I made up | |
| 14:45:20 | stephenfin | wait | |
| 14:45:46 | bauzas | efried: if we say that this trait is needed *anyway* (eg. even with a standard flavor asking for memory and vcpus), then I'm afraid we would get NoValidHosts for "flat" hosts | |
| 14:46:04 | stephenfin | dansmith: Sorry, confusing myself. I made up the 'network_details' field | |
| 14:46:34 | stephenfin | but the port_details field _is_ returned by the '/v2.0/floatingips' API https://docs.openstack.org/api-ref/network/v2/?expanded=list-ports-detail,list-floating-ips-detail#list-floating-ips | |
| 14:46:47 | bauzas | efried: unless "flat" hosts also have this trait | |
| 14:46:51 | efried | bauzas: we were going to get that anyway. One way or another, with the proposed segregation, NUMA flavors will all & only land on hosts with the NUMA flag on; and flat flavors will all & only land on hosts with the NUMA flag off. | |
| 14:46:58 | dansmith | stephenfin: right, isn't that what you're trying to fake/fill-in here? | |
| 14:47:01 | efried | That's what we *want* | |
| 14:47:14 | bauzas | efried: the latter is not what we agreed | |
| 14:47:14 | stephenfin | port_details, yeah | |
| 14:47:17 | stephenfin | but it's not always there | |
| 14:47:25 | efried | um | |
| 14:47:27 | stephenfin | it's dependent on the extension being present/enabled | |
| 14:47:31 | dansmith | stephenfin: right, so I get that part | |
| 14:47:35 | bauzas | efried: flat flavors will get NoValidHosts | |
| 14:47:47 | bauzas | because we're asking for a trait anyway | |
| 14:48:04 | efried | They will only get NVH if you don't have any flat hosts in your cloud | |
| 14:48:06 | dansmith | stephenfin: the thing I'm asking is whether or not ports[port_id] is equivalent to the port_details field on that api, because according to the example blob in your comment from just now, it should have port_details *inside* it | |
| 14:48:19 | bauzas | efried: no, again | |
| 14:48:23 | bauzas | see L62 | |
| 14:48:25 | dansmith | stephenfin: but I don't see that on the list_ports api-ref, so I think that example blob is wrong about that nesting.. right? | |
| 14:48:39 | bauzas | if we ask for MEMORY_PAGE_SIZE_SMALL, then all flat hosts won't get it | |
| 14:48:50 | sean-k-mooney | bauzas: we wont alway add the trait:numa_node=require trait only if you request numa | |
| 14:48:58 | efried | You only add MEMORY_PAGE_SIZE_SMALL if you're translating a *NUMA* flavor | |
| 14:48:58 | efried | bauzas: ohh, I see the confusion. | |
| 14:49:03 | efried | yeah, what sean-k-mooney said. | |
| 14:49:17 | bauzas | again, L62 is very confusing | |
| 14:49:28 | efried | The etherpad is only talking about what we do for NUMA-aware hosts | |
| 14:49:46 | efried | and yeah, the etherpad is confusing, so we should really write it more cleanly, say, in a spec :P | |
| 14:49:47 | bauzas | there are gaps then that I need to provide | |
| 14:50:03 | bauzas | ok, then we need to trigger exactly when asking for this trait | |
| 14:50:33 | stephenfin | dansmith: Not sure I get you, so apologies if I repeat stuff you already know | |
| 14:50:40 | efried | bauzas: All of the stuff in the etherpad is triggered under the same condition: if the flavor asks for a numa topo. | |
| 14:50:45 | stephenfin | if the extension is present, we'd expect a response like this http://paste.openstack.org/show/789164/ | |
| 14:51:06 | stephenfin | If that's not enabled, the port_details field isn't present so we add it manually | |
| 14:51:23 | dansmith | stephenfin: yes, understand, it's the "adding it manually" part I'm talking about | |
| 14:51:27 | dansmith | stephenfin: but let me just stop you for a sec | |
| 14:51:40 | efried | bauzas: there are things other than PAGE_SIZE_* that make sure you always & only land on a NUMA host. For instance, the NUMA_ROOT trait. | |
| 14:51:54 | dansmith | stephenfin: I think the problem is in a comment by you on the gerrit review, not the code as it is now, if i'm reading the api-ref correctly | |
| 14:52:00 | bauzas | efried: correct, but I need to think about this | |
| 14:52:34 | dansmith | stephenfin: are we able to test this with an old or duly configured neutron (i.e. without that extension) to prove it works? other than you manually with a devstack? | |
| 14:52:35 | efried | okay. Happy to help. (I have a call at the top of the hour FYI.) | |
| 14:52:54 | bauzas | cool | |
| 14:53:14 | stephenfin | dansmith: Yeah, let me whip up a functional test real quick | |
| 14:53:37 | stephenfin | this is what I expect our "fake" response to look like with this change, btw http://paste.openstack.org/show/789165/ | |
| 14:53:50 | stephenfin | we only return the list, stripping the outer container, of course | |
| 14:54:18 | dansmith | stephenfin: yeah understand that's the desired output of your function | |
| 14:54:33 | dansmith | stephenfin: a functional test isn't going to tell us anything if you're faking the response from neutron I think | |
| 14:55:34 | stephenfin | It should, assuming our mock of the 'list_ports' API is correct | |
| 14:55:41 | stephenfin | which I really hope it is since we use it everywhere | |
| 14:55:43 | stephenfin | Could we do a depends-on from the affected neutron project(s)? | |
| 14:55:51 | dansmith | oh, maybe I misunderstood you.. stephenfin when you said "Oh, yes, that exactly. This should return something like:" did you mean "this whole function should return..." or did you mean "list_ports should return" | |
| 14:56:59 | stephenfin | Doh /o\ "This" = this function, not "list_ports" | |
| 14:57:19 | dansmith | okay, I had quoted and was asking about list_ports originally, so I thought you were giving me the output of list_ports | |
| 14:57:29 | stephenfin | Yup, my mistake. Sorry | |
| 14:57:38 | dansmith | this is why I was saying I think the comment is the problem, because api-ref makes it look like your code is correct | |
| 14:58:28 | stephenfin | Yeah, per api-ref 'list_ports' will return something like https://docs.openstack.org/api-ref/network/v2/?expanded=list-ports-detail#list-ports of course | |
| 14:58:55 | dansmith | yeah when I first asked, I was expecting a detail=True to get the detailed listing, which I think is how ours works | |
| 15:00:02 | stephenfin | Yup, I don't think we need that since we don't need the detailed view. 'device_id' is the only thing we do care about and that's always there | |
| 15:00:32 | dansmith | you mean for our current usage I guess | |
| 15:00:36 | stephenfin | yeah | |
| 15:00:40 | stephenfin | if the extension is enabled and 'port_details' _is_ present, we'd only be getting a summary view anyway | |
| 15:00:59 | dansmith | if we only put device_id in there, then someday someone could expect port_details to be full and not understand why it's not | |
| 15:01:24 | stephenfin | we could, but given these are only used by the deprecated network API proxies, I figured the risk was low | |
| 15:01:26 | dansmith | I'm assuming this is only a neutron-goes-first upgrade scenario and we'll never hit this in the future because someone turned off this extension | |
| 15:01:37 | dansmith | right | |
| 15:02:20 | stephenfin | I'm under the impression that the networking backends choose their extensions or something | |
| 15:02:30 | stephenfin | because ralonsoh referred to OVN specifically | |